From e0801f5a5da589118d1e331db3cbd68edeb80486 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 25 Jun 2025 11:28:17 +0200 Subject: [PATCH 1/6] discovery: use errors.Is for error check Ensure that the check works for wrapped errors. Otherwise, this breaks for a SQLStore graph backend which wraps the error it returns. --- discovery/bootstrapper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/discovery/bootstrapper.go b/discovery/bootstrapper.go index 1a0f99735..6263db3f7 100644 --- a/discovery/bootstrapper.go +++ b/discovery/bootstrapper.go @@ -236,7 +236,7 @@ func (c *ChannelGraphBootstrapper) SampleNodeAddrs(_ context.Context, return errFound }) - if err != nil && err != errFound { + if err != nil && !errors.Is(err, errFound) { return nil, err } From c06036d2b43ab09186f1730ad181ba3984fd5ba3 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Mon, 26 May 2025 13:44:22 +0200 Subject: [PATCH 2/6] multi: use MakeTestGraphNew for all remaining unit tests And delete the old MakeTestGraph function. Here we need to update some of our unit tests to ensure that any updates to existing nodes or channel policies have newer timestamps. This is so that we don't violate our SQL DB constraints that prevent updates to a node or channel policy record if the new update is not newer (has a newer LastUpdate time) than the currently presisted record). --- autopilot/prefattach_test.go | 2 +- graph/builder_test.go | 72 ++++++++++++++++++++++-------------- graph/db/graph_test.go | 4 +- graph/db/kv_store.go | 31 ---------------- graph/notifications_test.go | 4 +- peer/test_utils.go | 2 +- 6 files changed, 50 insertions(+), 65 deletions(-) diff --git a/autopilot/prefattach_test.go b/autopilot/prefattach_test.go index 4e1bbf7b1..b6793fa5f 100644 --- a/autopilot/prefattach_test.go +++ b/autopilot/prefattach_test.go @@ -36,7 +36,7 @@ type testDBGraph struct { } func newDiskChanGraph(t *testing.T) (testGraph, error) { - graphDB := graphdb.MakeTestGraph(t) + graphDB := graphdb.MakeTestGraphNew(t) require.NoError(t, graphDB.Start()) t.Cleanup(func() { require.NoError(t, graphDB.Stop()) diff --git a/graph/builder_test.go b/graph/builder_test.go index 3abb345e0..048e1e22b 100644 --- a/graph/builder_test.go +++ b/graph/builder_test.go @@ -892,15 +892,27 @@ func TestPruneChannelGraphDoubleDisabled(t *testing.T) { } func testPruneChannelGraphDoubleDisabled(t *testing.T, assumeValid bool) { + timestamp := time.Now() + + // nextTimeStamp is a helper closure that will return a new + // timestamp each time it's called, this helps us create channel updates + // with new timestamps so that we don't run into our SQL DB constraint + // which only allows an update to a channel edge if the last update + // timestamp is greater than the previous one. + nextTimeStamp := func() time.Time { + timestamp = timestamp.Add(time.Second) + + return timestamp + } + // We'll create the following test graph so that only the last channel // is pruned. We'll use a fresh timestamp to ensure they're not pruned // according to that heuristic. - timestamp := time.Now() testChannels := []*testChannel{ // Channel from self shouldn't be pruned. symmetricTestChannel( "self", "a", 100000, &testChannelPolicy{ - LastUpdate: timestamp, + LastUpdate: nextTimeStamp(), Disabled: true, }, 99, ), @@ -918,7 +930,7 @@ func testPruneChannelGraphDoubleDisabled(t *testing.T, assumeValid bool) { Node1: &testChannelEnd{ Alias: "a", testChannelPolicy: &testChannelPolicy{ - LastUpdate: timestamp, + LastUpdate: nextTimeStamp(), Disabled: true, }, }, @@ -932,7 +944,7 @@ func testPruneChannelGraphDoubleDisabled(t *testing.T, assumeValid bool) { Node1: &testChannelEnd{ Alias: "a", testChannelPolicy: &testChannelPolicy{ - LastUpdate: timestamp, + LastUpdate: nextTimeStamp(), Disabled: false, }, }, @@ -946,14 +958,14 @@ func testPruneChannelGraphDoubleDisabled(t *testing.T, assumeValid bool) { Node1: &testChannelEnd{ Alias: "a", testChannelPolicy: &testChannelPolicy{ - LastUpdate: timestamp, + LastUpdate: nextTimeStamp(), Disabled: true, }, }, Node2: &testChannelEnd{ Alias: "b", testChannelPolicy: &testChannelPolicy{ - LastUpdate: timestamp, + LastUpdate: nextTimeStamp(), Disabled: false, }, }, @@ -963,13 +975,13 @@ func testPruneChannelGraphDoubleDisabled(t *testing.T, assumeValid bool) { // Both edges enabled. symmetricTestChannel("c", "d", 100000, &testChannelPolicy{ - LastUpdate: timestamp, + LastUpdate: nextTimeStamp(), Disabled: false, }, 2), // Both edges disabled, only one pruned. symmetricTestChannel("e", "f", 100000, &testChannelPolicy{ - LastUpdate: timestamp, + LastUpdate: nextTimeStamp(), Disabled: true, }, 3), } @@ -1363,7 +1375,9 @@ func parseTestGraph(t *testing.T, useCache bool, path string) ( testAddrs = append(testAddrs, testAddr) // Next, create a temporary graph database for usage within the test. - graph := graphdb.MakeTestGraph(t, graphdb.WithUseGraphCache(useCache)) + graph := graphdb.MakeTestGraphNew( + t, graphdb.WithUseGraphCache(useCache), + ) aliasMap := make(map[string]route.Vertex) privKeyMap := make(map[string]*btcec.PrivateKey) @@ -1441,6 +1455,13 @@ func parseTestGraph(t *testing.T, useCache bool, path string) ( } source = dbNode + + // Set the selected source node. + if err := graph.SetSourceNode(ctx, source); err != nil { + return nil, err + } + + continue } // With the node fully parsed, add it as a vertex within the @@ -1450,13 +1471,6 @@ func parseTestGraph(t *testing.T, useCache bool, path string) ( } } - if source != nil { - // Set the selected source node - if err := graph.SetSourceNode(ctx, source); err != nil { - return nil, err - } - } - // With all the vertexes inserted, we can now insert the edges into the // test graph. for _, edge := range g.Edges { @@ -1739,14 +1753,16 @@ func createTestGraphFromChannels(t *testing.T, useCache bool, testAddrs = append(testAddrs, testAddr) // Next, create a temporary graph database for usage within the test. - graph := graphdb.MakeTestGraph(t, graphdb.WithUseGraphCache(useCache)) + graph := graphdb.MakeTestGraphNew( + t, graphdb.WithUseGraphCache(useCache), + ) aliasMap := make(map[string]route.Vertex) privKeyMap := make(map[string]*btcec.PrivateKey) nodeIndex := byte(0) - addNodeWithAlias := func(alias string, features *lnwire.FeatureVector) ( - *models.LightningNode, error) { + addNodeWithAlias := func(alias string, + features *lnwire.FeatureVector) error { keyBytes := []byte{ 0, 0, 0, 0, 0, 0, 0, 0, @@ -1776,26 +1792,26 @@ func createTestGraphFromChannels(t *testing.T, useCache bool, // With the node fully parsed, add it as a vertex within the // graph. - if err := graph.AddLightningNode(ctx, dbNode); err != nil { - return nil, err + if alias == source { + err = graph.SetSourceNode(ctx, dbNode) + require.NoError(t, err) + } else { + err := graph.AddLightningNode(ctx, dbNode) + require.NoError(t, err) } aliasMap[alias] = dbNode.PubKeyBytes nodeIndex++ - return dbNode, nil + return nil } // Add the source node. - dbNode, err := addNodeWithAlias(source, lnwire.EmptyFeatureVector()) + err = addNodeWithAlias(source, lnwire.EmptyFeatureVector()) if err != nil { return nil, err } - if err = graph.SetSourceNode(ctx, dbNode); err != nil { - return nil, err - } - // Initialize variable that keeps track of the next channel id to assign // if none is specified. nextUnassignedChannelID := uint64(100000) @@ -1813,7 +1829,7 @@ func createTestGraphFromChannels(t *testing.T, useCache bool, features = node.testChannelPolicy.Features } - _, err := addNodeWithAlias( + err := addNodeWithAlias( node.Alias, features, ) if err != nil { diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index 97da79dbd..7d995355d 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -3147,7 +3147,7 @@ func TestChannelEdgePruningUpdateIndexDeletion(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraph(t) + graph := MakeTestGraphNew(t) // The update index only applies to the bbolt graph. boltStore, ok := graph.V1Store.(*KVStore) @@ -3681,7 +3681,7 @@ func TestEdgePolicyMissingMaxHtcl(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraph(t) + graph := MakeTestGraphNew(t) // This test currently directly edits the bytes stored in the bbolt DB. boltStore, ok := graph.V1Store.(*KVStore) diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go index 632ace84b..66fb93ba6 100644 --- a/graph/db/kv_store.go +++ b/graph/db/kv_store.go @@ -12,7 +12,6 @@ import ( "net" "sort" "sync" - "testing" "time" "github.com/btcsuite/btcd/btcec/v2" @@ -28,7 +27,6 @@ import ( "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" - "github.com/stretchr/testify/require" ) var ( @@ -4869,32 +4867,3 @@ func (c *chanGraphNodeTx) ForEachChannel(f func(*models.ChannelEdgeInfo, }, ) } - -// MakeTestGraph creates a new instance of the ChannelGraph for testing -// purposes. -// -// NOTE: this helper currently creates a ChannelGraph that is only ever backed -// by the `KVStore` of the `V1Store` interface. -func MakeTestGraph(t testing.TB, opts ...ChanGraphOption) *ChannelGraph { - t.Helper() - - // Next, create KVStore for the first time. - backend, backendCleanup, err := kvdb.GetTestBackend(t.TempDir(), "cgr") - t.Cleanup(backendCleanup) - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, backend.Close()) - }) - - graphStore, err := NewKVStore(backend) - require.NoError(t, err) - - graph, err := NewChannelGraph(graphStore, opts...) - require.NoError(t, err) - require.NoError(t, graph.Start()) - t.Cleanup(func() { - require.NoError(t, graph.Stop()) - }) - - return graph -} diff --git a/graph/notifications_test.go b/graph/notifications_test.go index 7b4fa69cb..4f99a517c 100644 --- a/graph/notifications_test.go +++ b/graph/notifications_test.go @@ -763,7 +763,7 @@ func TestNodeUpdateNotification(t *testing.T) { // then it should trigger a new notification. // TODO(roasbeef): assume monotonic time. nodeUpdateAnn := *node1 - nodeUpdateAnn.LastUpdate = node1.LastUpdate.Add(300 * time.Millisecond) + nodeUpdateAnn.LastUpdate = node1.LastUpdate.Add(time.Second) // Add new node topology update to the channel router. if err := ctx.builder.AddNode(ctxb, &nodeUpdateAnn); err != nil { @@ -1059,7 +1059,7 @@ type testCtx struct { func createTestCtxSingleNode(t *testing.T, startingHeight uint32) *testCtx { - graph := graphdb.MakeTestGraph(t) + graph := graphdb.MakeTestGraphNew(t) sourceNode := createTestNode(t) require.NoError(t, diff --git a/peer/test_utils.go b/peer/test_utils.go index 673eceed8..37ffc6e30 100644 --- a/peer/test_utils.go +++ b/peer/test_utils.go @@ -602,7 +602,7 @@ func createTestPeer(t *testing.T) *peerTestCtx { const chanActiveTimeout = time.Minute - dbAliceGraph := graphdb.MakeTestGraph(t) + dbAliceGraph := graphdb.MakeTestGraphNew(t) require.NoError(t, dbAliceGraph.Start()) t.Cleanup(func() { require.NoError(t, dbAliceGraph.Stop()) From 3ccaacd16f2f7a63add0a68df3a71c73820d8dcc Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 11 Jun 2025 18:08:52 +0200 Subject: [PATCH 3/6] multi: rename MakeTestGraphNew to MakeTestGraph --- autopilot/prefattach_test.go | 2 +- graph/builder_test.go | 4 +- graph/db/graph.go | 4 +- graph/db/graph_test.go | 76 ++++++++++++++++++------------------ graph/notifications_test.go | 2 +- peer/test_utils.go | 2 +- routing/pathfind_test.go | 2 +- 7 files changed, 46 insertions(+), 46 deletions(-) diff --git a/autopilot/prefattach_test.go b/autopilot/prefattach_test.go index b6793fa5f..4e1bbf7b1 100644 --- a/autopilot/prefattach_test.go +++ b/autopilot/prefattach_test.go @@ -36,7 +36,7 @@ type testDBGraph struct { } func newDiskChanGraph(t *testing.T) (testGraph, error) { - graphDB := graphdb.MakeTestGraphNew(t) + graphDB := graphdb.MakeTestGraph(t) require.NoError(t, graphDB.Start()) t.Cleanup(func() { require.NoError(t, graphDB.Stop()) diff --git a/graph/builder_test.go b/graph/builder_test.go index 048e1e22b..a27605269 100644 --- a/graph/builder_test.go +++ b/graph/builder_test.go @@ -1375,7 +1375,7 @@ func parseTestGraph(t *testing.T, useCache bool, path string) ( testAddrs = append(testAddrs, testAddr) // Next, create a temporary graph database for usage within the test. - graph := graphdb.MakeTestGraphNew( + graph := graphdb.MakeTestGraph( t, graphdb.WithUseGraphCache(useCache), ) @@ -1753,7 +1753,7 @@ func createTestGraphFromChannels(t *testing.T, useCache bool, testAddrs = append(testAddrs, testAddr) // Next, create a temporary graph database for usage within the test. - graph := graphdb.MakeTestGraphNew( + graph := graphdb.MakeTestGraph( t, graphdb.WithUseGraphCache(useCache), ) diff --git a/graph/db/graph.go b/graph/db/graph.go index 692406096..922adef53 100644 --- a/graph/db/graph.go +++ b/graph/db/graph.go @@ -594,7 +594,7 @@ func (c *ChannelGraph) UpdateEdgePolicy(ctx context.Context, return nil } -// MakeTestGraphNew creates a new instance of the ChannelGraph for testing +// MakeTestGraph creates a new instance of the ChannelGraph for testing // purposes. The backing V1Store implementation depends on the version of // NewTestDB included in the current build. // @@ -603,7 +603,7 @@ func (c *ChannelGraph) UpdateEdgePolicy(ctx context.Context, // implemented, unit tests will be switched to use this function instead of // the existing MakeTestGraph helper. Once only this function is used, the // existing MakeTestGraph function will be removed and this one will be renamed. -func MakeTestGraphNew(t testing.TB, +func MakeTestGraph(t testing.TB, opts ...ChanGraphOption) *ChannelGraph { t.Helper() diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index 7d995355d..d86a2f65c 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -101,7 +101,7 @@ func TestNodeInsertionAndDeletion(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'd like to test basic insertion/deletion for vertexes from the // graph, so we'll create a test vertex to start with. @@ -264,7 +264,7 @@ func TestPartialNode(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // To insert a partial node, we need to add a channel edge that has // node keys for nodes we are not yet aware @@ -332,7 +332,7 @@ func TestAliasLookup(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'd like to test the alias index within the database, so first // create a new test node. @@ -363,7 +363,7 @@ func TestSourceNode(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'd like to test the setting/getting of the source node, so we // first create a fake node to use within the test. @@ -390,7 +390,7 @@ func TestEdgeInsertionDeletion(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'd like to test the insertion/deletion of edges, so we create two // vertexes to connect. @@ -513,7 +513,7 @@ func TestDisconnectBlockAtHeight(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) sourceNode := createTestVertex(t) if err := graph.SetSourceNode(ctx, sourceNode); err != nil { @@ -810,7 +810,7 @@ func TestEdgeInfoUpdates(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'd like to test the update of edges inserted into the database, so // we create two vertexes to connect. @@ -1128,7 +1128,7 @@ func TestAddEdgeProof(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // Add an edge with no proof. node1 := createTestVertex(t) @@ -1187,7 +1187,7 @@ func TestForEachSourceNodeChannel(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // Create a source node (A) and set it as such in the DB. nodeA := createTestVertex(t) @@ -1275,7 +1275,7 @@ func TestForEachSourceNodeChannel(t *testing.T) { func TestGraphTraversal(t *testing.T) { t.Parallel() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'd like to test some of the graph traversal capabilities within // the DB, so we'll create a series of fake nodes to insert into the @@ -1370,7 +1370,7 @@ func TestGraphTraversal(t *testing.T) { func TestGraphTraversalCacheable(t *testing.T) { t.Parallel() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'd like to test some of the graph traversal capabilities within // the DB, so we'll create a series of fake nodes to insert into the @@ -1440,7 +1440,7 @@ func TestGraphTraversalCacheable(t *testing.T) { func TestGraphCacheTraversal(t *testing.T) { t.Parallel() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'd like to test some of the graph traversal capabilities within // the DB, so we'll create a series of fake nodes to insert into the @@ -1685,7 +1685,7 @@ func TestGraphPruning(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) sourceNode := createTestVertex(t) if err := graph.SetSourceNode(ctx, sourceNode); err != nil { @@ -1875,7 +1875,7 @@ func TestHighestChanID(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // If we don't yet have any channels in the database, then we should // get a channel ID of zero if we ask for the highest channel ID. @@ -1935,7 +1935,7 @@ func TestChanUpdatesInHorizon(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // If we issue an arbitrary query before any channel updates are // inserted in the database, we should get zero results. @@ -2093,7 +2093,7 @@ func TestNodeUpdatesInHorizon(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) startTime := time.Unix(1234, 0) endTime := startTime @@ -2196,7 +2196,7 @@ func TestNodeUpdatesInHorizon(t *testing.T) { func TestFilterKnownChanIDsZombieRevival(t *testing.T) { t.Parallel() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) var ( scid1 = lnwire.ShortChannelID{BlockHeight: 1} @@ -2262,7 +2262,7 @@ func TestFilterKnownChanIDs(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) isZombieUpdate := func(updateTime1 time.Time, updateTime2 time.Time) bool { @@ -2440,7 +2440,7 @@ func TestStressTestChannelGraphAPI(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) node1 := createTestVertex(t) require.NoError(t, graph.AddLightningNode(ctx, node1)) @@ -2729,7 +2729,7 @@ func TestFilterChannelRange(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'll first populate our graph with two nodes. All channels created // below will be made between these two nodes. @@ -2948,7 +2948,7 @@ func TestFetchChanInfos(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'll first populate our graph with two nodes. All channels created // below will be made between these two nodes. @@ -3051,7 +3051,7 @@ func TestIncompleteChannelPolicies(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // Create two nodes. node1 := createTestVertex(t) @@ -3147,7 +3147,7 @@ func TestChannelEdgePruningUpdateIndexDeletion(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // The update index only applies to the bbolt graph. boltStore, ok := graph.V1Store.(*KVStore) @@ -3295,7 +3295,7 @@ func TestPruneGraphNodes(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'll start off by inserting our source node, to ensure that it's // the only node left after we prune the graph. @@ -3361,7 +3361,7 @@ func TestAddChannelEdgeShellNodes(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // To start, we'll create two nodes, and only add one of them to the // channel graph. @@ -3400,7 +3400,7 @@ func TestNodePruningUpdateIndexDeletion(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'll first populate our graph with a single node that will be // removed shortly. @@ -3469,19 +3469,19 @@ func TestNodeIsPublic(t *testing.T) { // We'll need to create a separate database and channel graph for each // participant to replicate real-world scenarios (private edges being in // some graphs but not others, etc.). - aliceGraph := MakeTestGraphNew(t) + aliceGraph := MakeTestGraph(t) aliceNode := createTestVertex(t) if err := aliceGraph.SetSourceNode(ctx, aliceNode); err != nil { t.Fatalf("unable to set source node: %v", err) } - bobGraph := MakeTestGraphNew(t) + bobGraph := MakeTestGraph(t) bobNode := createTestVertex(t) if err := bobGraph.SetSourceNode(ctx, bobNode); err != nil { t.Fatalf("unable to set source node: %v", err) } - carolGraph := MakeTestGraphNew(t) + carolGraph := MakeTestGraph(t) carolNode := createTestVertex(t) if err := carolGraph.SetSourceNode(ctx, carolNode); err != nil { t.Fatalf("unable to set source node: %v", err) @@ -3596,7 +3596,7 @@ func TestDisabledChannelIDs(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // Create first node and add it to the graph. node1 := createTestVertex(t) @@ -3681,7 +3681,7 @@ func TestEdgePolicyMissingMaxHtcl(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // This test currently directly edits the bytes stored in the bbolt DB. boltStore, ok := graph.V1Store.(*KVStore) @@ -3828,7 +3828,7 @@ func TestGraphZombieIndex(t *testing.T) { ctx := context.Background() // We'll start by creating our test graph along with a test edge. - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) node1 := createTestVertex(t) node2 := createTestVertex(t) @@ -4010,7 +4010,7 @@ func TestBatchedAddChannelEdge(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) sourceNode := createTestVertex(t) require.Nil(t, graph.SetSourceNode(ctx, sourceNode)) @@ -4087,7 +4087,7 @@ func TestBatchedUpdateEdgePolicy(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // We'd like to test the update of edges inserted into the database, so // we create two vertexes to connect. @@ -4137,7 +4137,7 @@ func TestBatchedUpdateEdgePolicy(t *testing.T) { // BenchmarkForEachChannel is a benchmark test that measures the number of // allocations and the total memory consumed by the full graph traversal. func BenchmarkForEachChannel(b *testing.B) { - graph := MakeTestGraphNew(b) + graph := MakeTestGraph(b) const numNodes = 100 const numChannels = 4 @@ -4190,7 +4190,7 @@ func TestGraphCacheForEachNodeChannel(t *testing.T) { t.Parallel() ctx := context.Background() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) // Unset the channel graph cache to simulate the user running with the // option turned off. @@ -4325,7 +4325,7 @@ func TestGraphLoading(t *testing.T) { func TestClosedScid(t *testing.T) { t.Parallel() - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) scid := lnwire.ShortChannelID{} @@ -4362,7 +4362,7 @@ func TestLightningNodePersistence(t *testing.T) { ctx := context.Background() // Create a new test graph instance. - graph := MakeTestGraphNew(t) + graph := MakeTestGraph(t) nodeAnnBytes, err := hex.DecodeString(testNodeAnn) require.NoError(t, err) diff --git a/graph/notifications_test.go b/graph/notifications_test.go index 4f99a517c..e7d081c63 100644 --- a/graph/notifications_test.go +++ b/graph/notifications_test.go @@ -1059,7 +1059,7 @@ type testCtx struct { func createTestCtxSingleNode(t *testing.T, startingHeight uint32) *testCtx { - graph := graphdb.MakeTestGraphNew(t) + graph := graphdb.MakeTestGraph(t) sourceNode := createTestNode(t) require.NoError(t, diff --git a/peer/test_utils.go b/peer/test_utils.go index 37ffc6e30..673eceed8 100644 --- a/peer/test_utils.go +++ b/peer/test_utils.go @@ -602,7 +602,7 @@ func createTestPeer(t *testing.T) *peerTestCtx { const chanActiveTimeout = time.Minute - dbAliceGraph := graphdb.MakeTestGraphNew(t) + dbAliceGraph := graphdb.MakeTestGraph(t) require.NoError(t, dbAliceGraph.Start()) t.Cleanup(func() { require.NoError(t, dbAliceGraph.Stop()) diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 45faedaf1..eea1bb22e 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -160,7 +160,7 @@ func makeTestGraph(t *testing.T, useCache bool) (*graphdb.ChannelGraph, kvdb.Backend, error) { // Create channelgraph for the first time. - graph := graphdb.MakeTestGraphNew( + graph := graphdb.MakeTestGraph( t, graphdb.WithUseGraphCache(useCache), ) require.NoError(t, graph.Start()) From b4b1c4bd8d21df7bcb6ccdb4abc172392230b0e5 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 11 Jun 2025 18:17:10 +0200 Subject: [PATCH 4/6] graph/db: remove KVStore fall-back Now that SQLStore fully implements V1Store, we no longer need the KVStore fall-back. --- docs/release-notes/release-notes-0.20.0.md | 1 + graph/db/sql_store.go | 7 +------ graph/db/test_postgres.go | 11 +---------- graph/db/test_sqlite.go | 11 +---------- 4 files changed, 4 insertions(+), 26 deletions(-) diff --git a/docs/release-notes/release-notes-0.20.0.md b/docs/release-notes/release-notes-0.20.0.md index da32397a7..4fa3688d2 100644 --- a/docs/release-notes/release-notes-0.20.0.md +++ b/docs/release-notes/release-notes-0.20.0.md @@ -91,6 +91,7 @@ circuit. The indices are only available for forwarding events saved after v0.20. * [8](https://github.com/lightningnetwork/lnd/pull/9938) * [9](https://github.com/lightningnetwork/lnd/pull/9939) * [10](https://github.com/lightningnetwork/lnd/pull/9971) + * [11](https://github.com/lightningnetwork/lnd/pull/9972) ## RPC Updates diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go index 42a5d8126..c8f222b29 100644 --- a/graph/db/sql_store.go +++ b/graph/db/sql_store.go @@ -175,10 +175,6 @@ type SQLStore struct { srcNodes map[ProtocolVersion]*srcNodeInfo srcNodeMu sync.Mutex - - // Temporary fall-back to the KVStore so that we can implement the - // interface incrementally. - *KVStore } // A compile-time assertion to ensure that SQLStore implements the V1Store @@ -194,7 +190,7 @@ type SQLStoreConfig struct { // NewSQLStore creates a new SQLStore instance given an open BatchedSQLQueries // storage backend. -func NewSQLStore(cfg *SQLStoreConfig, db BatchedSQLQueries, kvStore *KVStore, +func NewSQLStore(cfg *SQLStoreConfig, db BatchedSQLQueries, options ...StoreOptionModifier) (*SQLStore, error) { opts := DefaultOptions() @@ -210,7 +206,6 @@ func NewSQLStore(cfg *SQLStoreConfig, db BatchedSQLQueries, kvStore *KVStore, s := &SQLStore{ cfg: cfg, db: db, - KVStore: kvStore, rejectCache: newRejectCache(opts.RejectCacheSize), chanCache: newChannelCache(opts.ChannelCacheSize), srcNodes: make(map[ProtocolVersion]*srcNodeInfo), diff --git a/graph/db/test_postgres.go b/graph/db/test_postgres.go index d5cd1f8ff..11144eb3b 100644 --- a/graph/db/test_postgres.go +++ b/graph/db/test_postgres.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/btcsuite/btcd/chaincfg" - "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/sqldb" "github.com/stretchr/testify/require" ) @@ -16,14 +15,6 @@ import ( // database for testing. At the moment, it embeds a KVStore but once the // SQLStore fully implements the V1Store interface, the KVStore will be removed. func NewTestDB(t testing.TB) V1Store { - backend, backendCleanup, err := kvdb.GetTestBackend(t.TempDir(), "cgr") - require.NoError(t, err) - - t.Cleanup(backendCleanup) - - graphStore, err := NewKVStore(backend) - require.NoError(t, err) - pgFixture := sqldb.NewTestPgFixture( t, sqldb.DefaultPostgresFixtureLifetime, ) @@ -42,7 +33,7 @@ func NewTestDB(t testing.TB) V1Store { store, err := NewSQLStore( &SQLStoreConfig{ ChainHash: *chaincfg.MainNetParams.GenesisHash, - }, executor, graphStore, + }, executor, ) require.NoError(t, err) diff --git a/graph/db/test_sqlite.go b/graph/db/test_sqlite.go index c19300ce2..f9cb5a46d 100644 --- a/graph/db/test_sqlite.go +++ b/graph/db/test_sqlite.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/btcsuite/btcd/chaincfg" - "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/sqldb" "github.com/stretchr/testify/require" ) @@ -16,14 +15,6 @@ import ( // database for testing. At the moment, it embeds a KVStore but once the // SQLStore fully implements the V1Store interface, the KVStore will be removed. func NewTestDB(t testing.TB) V1Store { - backend, backendCleanup, err := kvdb.GetTestBackend(t.TempDir(), "cgr") - require.NoError(t, err) - - t.Cleanup(backendCleanup) - - graphStore, err := NewKVStore(backend) - require.NoError(t, err) - db := sqldb.NewTestSqliteDB(t).BaseDB executor := sqldb.NewTransactionExecutor( @@ -35,7 +26,7 @@ func NewTestDB(t testing.TB) V1Store { store, err := NewSQLStore( &SQLStoreConfig{ ChainHash: *chaincfg.MainNetParams.GenesisHash, - }, executor, graphStore, + }, executor, ) require.NoError(t, err) From e38aa6d1e0f19fa335352682bc6ef87824b2d685 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Mon, 23 Jun 2025 10:05:03 +0200 Subject: [PATCH 5/6] multi: itest jobs with SQL graph backend In this commit, a new `test_native_sql` build flag is defined. If this build flag is used along with the `--use-native-sql` config option, then the SQLStore implementation of the graphdb.V1Store will be initialised. This is then used to run our itest suite against the new SQLStore graph implementation. NOTE that this only works for new nodes currently - the migration from kv-to-sql is yet to be implemeneted. --- .github/workflows/main.yml | 4 +++ config_builder.go | 52 ++++++++++++++++++++++++-------------- config_prod.go | 18 +++++++++++++ config_test_native_sql.go | 31 +++++++++++++++++++++++ sqldb/migrations_dev.go | 2 +- sqldb/migrations_prod.go | 2 +- 6 files changed, 88 insertions(+), 21 deletions(-) create mode 100644 config_prod.go create mode 100644 config_test_native_sql.go diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 951637def..0b59eb985 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -368,10 +368,14 @@ jobs: args: backend=bitcoind dbbackend=sqlite - name: bitcoind-sqlite-nativesql args: backend=bitcoind dbbackend=sqlite nativesql=true + - name: bitcoind-sqlite=nativesql-experiment + args: backend=bitcoind dbbackend=sqlite nativesql=true tags=test_native_sql - name: bitcoind-postgres args: backend=bitcoind dbbackend=postgres - name: bitcoind-postgres-nativesql args: backend=bitcoind dbbackend=postgres nativesql=true + - name: bitcoind-postgres-nativesql-experiment + args: backend=bitcoind dbbackend=postgres nativesql=true tags=test_native_sql steps: - name: git checkout uses: actions/checkout@v4 diff --git a/config_builder.go b/config_builder.go index 95155b007..e47b29b7d 100644 --- a/config_builder.go +++ b/config_builder.go @@ -1046,23 +1046,6 @@ func (d *DefaultDatabaseBuilder) BuildDatabase( ) } - graphStore, err := graphdb.NewKVStore( - databaseBackends.GraphDB, graphDBOptions..., - ) - if err != nil { - return nil, nil, err - } - - dbs.GraphDB, err = graphdb.NewChannelGraph(graphStore, chanGraphOpts...) - if err != nil { - cleanUp() - - err = fmt.Errorf("unable to open graph DB: %w", err) - d.logger.Error(err) - - return nil, nil, err - } - dbOptions := []channeldb.OptionModifier{ channeldb.OptionDryRunMigration(cfg.DryRunMigration), channeldb.OptionKeepFailedPaymentAttempts( @@ -1098,6 +1081,10 @@ func (d *DefaultDatabaseBuilder) BuildDatabase( return nil, nil, err } + // The graph store implementation we will use depends on whether + // native SQL is enabled or not. + var graphStore graphdb.V1Store + // Instantiate a native SQL store if the flag is set. if d.cfg.DB.UseNativeSQL { migrations := sqldb.GetMigrations() @@ -1156,17 +1143,27 @@ func (d *DefaultDatabaseBuilder) BuildDatabase( // the base DB and transaction executor for the native SQL // invoice store. baseDB := dbs.NativeSQLStore.GetBaseDB() - executor := sqldb.NewTransactionExecutor( + invoiceExecutor := sqldb.NewTransactionExecutor( baseDB, func(tx *sql.Tx) invoices.SQLInvoiceQueries { return baseDB.WithTx(tx) }, ) sqlInvoiceDB := invoices.NewSQLStore( - executor, clock.NewDefaultClock(), + invoiceExecutor, clock.NewDefaultClock(), ) dbs.InvoiceDB = sqlInvoiceDB + + graphStore, err = d.getGraphStore( + baseDB, databaseBackends.GraphDB, graphDBOptions..., + ) + if err != nil { + err = fmt.Errorf("unable to get graph store: %w", err) + d.logger.Error(err) + + return nil, nil, err + } } else { // Check if the invoice bucket tombstone is set. If it is, we // need to return and ask the user switch back to using the @@ -1188,6 +1185,23 @@ func (d *DefaultDatabaseBuilder) BuildDatabase( } dbs.InvoiceDB = dbs.ChanStateDB + + graphStore, err = graphdb.NewKVStore( + databaseBackends.GraphDB, graphDBOptions..., + ) + if err != nil { + return nil, nil, err + } + } + + dbs.GraphDB, err = graphdb.NewChannelGraph(graphStore, chanGraphOpts...) + if err != nil { + cleanUp() + + err = fmt.Errorf("unable to open channel graph DB: %w", err) + d.logger.Error(err) + + return nil, nil, err } // Wrap the watchtower client DB and make sure we clean up. diff --git a/config_prod.go b/config_prod.go new file mode 100644 index 000000000..b480c2bb8 --- /dev/null +++ b/config_prod.go @@ -0,0 +1,18 @@ +//go:build !test_native_sql + +package lnd + +import ( + graphdb "github.com/lightningnetwork/lnd/graph/db" + "github.com/lightningnetwork/lnd/kvdb" + "github.com/lightningnetwork/lnd/sqldb" +) + +// getGraphStore returns a graphdb.V1Store backed by a graphdb.KVStore +// implementation. +func (d *DefaultDatabaseBuilder) getGraphStore(_ *sqldb.BaseDB, + kvBackend kvdb.Backend, + opts ...graphdb.StoreOptionModifier) (graphdb.V1Store, error) { + + return graphdb.NewKVStore(kvBackend, opts...) +} diff --git a/config_test_native_sql.go b/config_test_native_sql.go new file mode 100644 index 000000000..4a9469deb --- /dev/null +++ b/config_test_native_sql.go @@ -0,0 +1,31 @@ +//go:build test_native_sql + +package lnd + +import ( + "database/sql" + + graphdb "github.com/lightningnetwork/lnd/graph/db" + "github.com/lightningnetwork/lnd/kvdb" + "github.com/lightningnetwork/lnd/sqldb" +) + +// getGraphStore returns a graphdb.V1Store backed by a graphdb.SQLStore +// implementation. +func (d *DefaultDatabaseBuilder) getGraphStore(baseDB *sqldb.BaseDB, + _ kvdb.Backend, opts ...graphdb.StoreOptionModifier) (graphdb.V1Store, + error) { + + graphExecutor := sqldb.NewTransactionExecutor( + baseDB, func(tx *sql.Tx) graphdb.SQLQueries { + return baseDB.WithTx(tx) + }, + ) + + return graphdb.NewSQLStore( + &graphdb.SQLStoreConfig{ + ChainHash: *d.cfg.ActiveNetParams.GenesisHash, + }, + graphExecutor, opts..., + ) +} diff --git a/sqldb/migrations_dev.go b/sqldb/migrations_dev.go index 112e68dd0..b8721f927 100644 --- a/sqldb/migrations_dev.go +++ b/sqldb/migrations_dev.go @@ -1,4 +1,4 @@ -//go:build test_db_postgres || test_db_sqlite +//go:build test_db_postgres || test_db_sqlite || test_native_sql package sqldb diff --git a/sqldb/migrations_prod.go b/sqldb/migrations_prod.go index 551cf1fbb..faa41f419 100644 --- a/sqldb/migrations_prod.go +++ b/sqldb/migrations_prod.go @@ -1,4 +1,4 @@ -//go:build !test_db_postgres && !test_db_sqlite +//go:build !test_db_postgres && !test_db_sqlite && !test_native_sql package sqldb From cc0be0bca2b8fc9158cf55ad1d6565232bf16222 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 25 Jun 2025 11:33:36 +0200 Subject: [PATCH 6/6] graph/db: let IsZombieEdge return an error --- graph/db/errors.go | 5 +++++ graph/db/graph_test.go | 21 ++++++++++++++------- graph/db/interfaces.go | 2 +- graph/db/kv_store.go | 9 ++++++--- graph/db/sql_store.go | 12 +++++++----- 5 files changed, 33 insertions(+), 16 deletions(-) diff --git a/graph/db/errors.go b/graph/db/errors.go index 8201980aa..433592153 100644 --- a/graph/db/errors.go +++ b/graph/db/errors.go @@ -66,6 +66,11 @@ var ( // ErrUnknownAddressType is returned when a node's addressType is not // an expected value. ErrUnknownAddressType = fmt.Errorf("address type cannot be resolved") + + // ErrCantCheckIfZombieEdgeStr is an error returned when we + // attempt to check if an edge is a zombie but encounter an error. + ErrCantCheckIfZombieEdgeStr = fmt.Errorf("unable to check if edge " + + "is a zombie") ) // ErrTooManyExtraOpaqueBytes creates an error which should be returned if the diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index d86a2f65c..918369a9c 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -453,7 +453,8 @@ func TestEdgeInsertionDeletion(t *testing.T) { require.ErrorIs(t, err, ErrEdgeNotFound) _, _, _, err = graph.FetchChannelEdgesByID(chanID) require.ErrorIs(t, err, ErrZombieEdge) - isZombie, _, _ := graph.IsZombieEdge(chanID) + isZombie, _, _, err := graph.IsZombieEdge(chanID) + require.NoError(t, err) require.True(t, isZombie) // Finally, attempt to delete a (now) non-existent edge within the @@ -2205,7 +2206,9 @@ func TestFilterKnownChanIDsZombieRevival(t *testing.T) { ) isZombie := func(scid lnwire.ShortChannelID) bool { - zombie, _, _ := graph.IsZombieEdge(scid.ToUint64()) + zombie, _, _, err := graph.IsZombieEdge(scid.ToUint64()) + require.NoError(t, err) + return zombie } @@ -3844,15 +3847,17 @@ func TestGraphZombieIndex(t *testing.T) { // Since the edge is known the graph and it isn't a zombie, IsZombieEdge // should not report the channel as a zombie. - isZombie, _, _ := graph.IsZombieEdge(edge.ChannelID) + isZombie, _, _, err := graph.IsZombieEdge(edge.ChannelID) + require.NoError(t, err) require.False(t, isZombie) assertNumZombies(t, graph, 0) // If we delete the edge and mark it as a zombie, then we should expect // to see it within the index. - err := graph.DeleteChannelEdges(false, true, edge.ChannelID) + err = graph.DeleteChannelEdges(false, true, edge.ChannelID) require.NoError(t, err, "unable to mark edge as zombie") - isZombie, pubKey1, pubKey2 := graph.IsZombieEdge(edge.ChannelID) + isZombie, pubKey1, pubKey2, err := graph.IsZombieEdge(edge.ChannelID) + require.NoError(t, err) require.True(t, isZombie) require.Equal(t, node1.PubKeyBytes, pubKey1) require.Equal(t, node2.PubKeyBytes, pubKey2) @@ -3868,7 +3873,8 @@ func TestGraphZombieIndex(t *testing.T) { t, graph.MarkEdgeLive(edge.ChannelID), ErrZombieEdgeNotFound, ) - isZombie, _, _ = graph.IsZombieEdge(edge.ChannelID) + isZombie, _, _, err = graph.IsZombieEdge(edge.ChannelID) + require.NoError(t, err) require.False(t, isZombie) assertNumZombies(t, graph, 0) @@ -3880,7 +3886,8 @@ func TestGraphZombieIndex(t *testing.T) { ) require.NoError(t, err, "unable to mark edge as zombie") - isZombie, _, _ = graph.IsZombieEdge(edge.ChannelID) + isZombie, _, _, err = graph.IsZombieEdge(edge.ChannelID) + require.NoError(t, err) require.True(t, isZombie) assertNumZombies(t, graph, 1) } diff --git a/graph/db/interfaces.go b/graph/db/interfaces.go index 0759637fd..7140553b7 100644 --- a/graph/db/interfaces.go +++ b/graph/db/interfaces.go @@ -308,7 +308,7 @@ type V1Store interface { //nolint:interfacebloat // IsZombieEdge returns whether the edge is considered zombie. If it is // a zombie, then the two node public keys corresponding to this edge // are also returned. - IsZombieEdge(chanID uint64) (bool, [33]byte, [33]byte) + IsZombieEdge(chanID uint64) (bool, [33]byte, [33]byte, error) // NumZombies returns the current number of zombie channels in the // graph. diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go index 66fb93ba6..87651e461 100644 --- a/graph/db/kv_store.go +++ b/graph/db/kv_store.go @@ -3792,7 +3792,9 @@ func (c *KVStore) markEdgeLiveUnsafe(tx kvdb.RwTx, chanID uint64) error { // IsZombieEdge returns whether the edge is considered zombie. If it is a // zombie, then the two node public keys corresponding to this edge are also // returned. -func (c *KVStore) IsZombieEdge(chanID uint64) (bool, [33]byte, [33]byte) { +func (c *KVStore) IsZombieEdge(chanID uint64) (bool, [33]byte, [33]byte, + error) { + var ( isZombie bool pubKey1, pubKey2 [33]byte @@ -3817,10 +3819,11 @@ func (c *KVStore) IsZombieEdge(chanID uint64) (bool, [33]byte, [33]byte) { pubKey2 = [33]byte{} }) if err != nil { - return false, [33]byte{}, [33]byte{} + return false, [33]byte{}, [33]byte{}, fmt.Errorf("%w: %w "+ + "(chanID=%d)", ErrCantCheckIfZombieEdgeStr, err, chanID) } - return isZombie, pubKey1, pubKey2 + return isZombie, pubKey1, pubKey2, nil } // isZombieEdge returns whether an entry exists for the given channel in the diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go index c8f222b29..698c47771 100644 --- a/graph/db/sql_store.go +++ b/graph/db/sql_store.go @@ -1636,7 +1636,9 @@ func (s *SQLStore) MarkEdgeLive(chanID uint64) error { // returned. // // NOTE: part of the V1Store interface. -func (s *SQLStore) IsZombieEdge(chanID uint64) (bool, [33]byte, [33]byte) { +func (s *SQLStore) IsZombieEdge(chanID uint64) (bool, [33]byte, [33]byte, + error) { + var ( ctx = context.TODO() isZombie bool @@ -1666,12 +1668,12 @@ func (s *SQLStore) IsZombieEdge(chanID uint64) (bool, [33]byte, [33]byte) { return nil }, sqldb.NoOpReset) if err != nil { - // TODO(elle): update the IsZombieEdge method to return an - // error. - return false, route.Vertex{}, route.Vertex{} + return false, route.Vertex{}, route.Vertex{}, + fmt.Errorf("%w: %w (chanID=%d)", + ErrCantCheckIfZombieEdgeStr, err, chanID) } - return isZombie, pubKey1, pubKey2 + return isZombie, pubKey1, pubKey2, nil } // NumZombies returns the current number of zombie channels in the graph.