From 41615f74a50c0f605f425aaf57a9020c62df3740 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Fri, 14 Nov 2025 10:45:58 +0200 Subject: [PATCH] graph/db: add test for SetSourceNode same timestamp behavior This commit adds TestSetSourceNodeSameTimestamp to demonstrate the current behavior when SetSourceNode is called with the same last update timestamp. The test reveals a difference between the SQL and bbolt implementations: - SQL store returns sql.ErrNoRows when attempting to update with the same timestamp, as the upsert query's UPDATE clause requires the new timestamp to be strictly greater than the existing one - bbolt store silently ignores stale updates and returns no error This behavior is important to document because our own node announcements may change quickly with the same timestamp, unlike announcements from other nodes where same timestamp typically means identical parameters. --- graph/db/graph_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index e4a8c7fae..2fd7e508b 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "crypto/sha256" + "database/sql" "encoding/hex" "errors" "fmt" @@ -396,6 +397,62 @@ func TestSourceNode(t *testing.T) { compareNodes(t, testNode, sourceNode) } +// TestSetSourceNodeSameTimestamp demonstrates that SetSourceNode can return an +// error when called with the same last update timestamp. Calling SetSourceNode +// with the same timestamp should be allowed (unlike AddNode), as it is +// possible that our own node announcement may change quickly. This will be +// fixed in an upcoming commit. +func TestSetSourceNodeSameTimestamp(t *testing.T) { + t.Parallel() + ctx := t.Context() + + graph := MakeTestGraph(t) + + _, isSQLStore := graph.V1Store.(*SQLStore) + + // Create and set the initial source node. + testNode := createTestVertex(t) + require.NoError(t, graph.SetSourceNode(ctx, testNode)) + + // Verify the source node was set correctly. + sourceNode, err := graph.SourceNode(ctx) + require.NoError(t, err) + compareNodes(t, testNode, sourceNode) + + // Create a modified version of the node with the same timestamp but + // different parameters (e.g., different alias and color). This + // could well be the case for our own node announcement (unlike other + // announcements where same timestamp means same parameters). + modifiedNode := models.NewV1Node( + testNode.PubKeyBytes, &models.NodeV1Fields{ + // Same timestamp. + LastUpdate: testNode.LastUpdate, + // Different alias. + Alias: "different-alias", + Color: color.RGBA{R: 100, G: 200, B: 50, A: 0}, + Addresses: testNode.Addresses, + Features: testNode.Features.RawFeatureVector, + AuthSigBytes: testNode.AuthSigBytes, + }, + ) + + // Attempt to set the source node with the same timestamp but + // different parameters. + err = graph.SetSourceNode(ctx, modifiedNode) + + // The SQL store will return sql.ErrNoRows because the UPDATE clause + // in the upsert query requires the new timestamp to be strictly + // greater than the existing one. When this condition is not met, no + // rows are updated and the SQL query returns ErrNoRows. The bbolt KV + // store, on the other hand, silently ignores stale updates and returns + // no error. + if isSQLStore { + require.ErrorIs(t, err, sql.ErrNoRows) + } else { + require.NoError(t, err) + } +} + // TestEdgeInsertionDeletion tests the basic CRUD operations for channel edges. func TestEdgeInsertionDeletion(t *testing.T) { t.Parallel()