graph/db: add test for SetSourceNode same timestamp behavior
What the developer wrote
Authored by Elle Mouton
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.
What changed, and why it matters
This commit only adds a new test to document an existing inconsistency between two database backends in the Lightning Network Daemon (LND). It does not change production code. The test shows that when LND updates its own node information with the same timestamp but different details, the SQL database backend rejects it with an error, while the older bbolt backend accepts it silently. The commit message says this will be fixed later. So by itself, this commit is not a security fix and does not introduce a vulnerability, but it documents a real behavioral quirk that could affect how LND stores its own node announcements.
Treat this commit as a documentation/test addition, not a security patch. Monitor the follow-up commit referenced in the message that will fix the inconsistent behavior. If running the SQL backend, be aware that same-timestamp source node updates currently fail, which could delay propagation of updated node announcements but is not directly exploitable by a remote attacker.
Security signals we found
Behavioral inconsistency between SQL and bbolt storage backends documented
Same-timestamp node self-announcement may be rejected or ignored depending on backend
No production code change; test-only commit
Commit message indicates a fix is planned in a subsequent commit
Evidence from the diff
The diff adds TestSetSourceNodeSameTimestamp in graph/db/graph_test.go. The test calls SetSourceNode twice with the same LastUpdate timestamp but different alias/color. For the SQL backend, the underlying upsert’s UPDATE clause only fires when the new timestamp is strictly greater, so sql.ErrNoRows is returned. For bbolt, stale updates are silently ignored and no error is returned. The commit message explicitly states this behavior will be fixed in a future commit. No production code is modified.
Changed components
graph/db/graph_test.gograph/db SetSourceNode (behavior under test)SQLStore upsert logic for source nodebbolt KV store source node update pathInspect captured patch +57 / −0
diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go
index e4a8c7f..2fd7e50 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()
Why this scored 27/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.