From c06036d2b43ab09186f1730ad181ba3984fd5ba3 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Mon, 26 May 2025 13:44:22 +0200 Subject: [PATCH] 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())