graph/db: gracefully handle duplicate node announcements
What the developer wrote
Authored by Elle Mouton
graph/db: gracefully handle duplicate node announcements
It can happen that we are handling 2 of the same node announcements in
the same batch transaction. In that case, our `UpsertNode` conflict
assertion may fail. We need to handle this gracefully.
What changed, and why it matters
This change fixes a database error that could occur when the same Lightning Network node announcement was processed twice in quick succession. Instead of crashing or returning an error, the code now ignores the duplicate. It is a robustness fix rather than a vulnerability that allows theft or remote takeover.
Treat as a normal bug-fix/robustness patch. Reviewers should verify that ignoring sql.ErrNoRows here does not mask other genuine failures, and that the conflict condition only triggers on exact duplicate timestamps, not on stale or malicious announcements.
Security signals we found
Duplicate database constraint violation converted to silent success
Batch-scheduled graph update race condition
sql.ErrNoRows used as duplicate-detection signal
Regression test added for duplicate node announcement handling
Evidence from the diff
In LND’s graph/db SQL store, AddNode uses a batch scheduler. If two identical node announcements arrive in the same batch, the upsertNode call can return sql.ErrNoRows because the DB constraint requires last_update to be strictly greater than the existing value. The patch catches sql.ErrNoRows inside the batch Do function and converts it to nil, gracefully swallowing the duplicate. A regression test is added to TestNodeInsertionAndDeletion.
Changed components
graph/db/sql_store.go AddNode methodgraph/db batch node upsert logicLightning Network graph database layerInspect captured patch +19 / −0
diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go
index 482c1aa..08fa36f 100644
--- a/graph/db/graph_test.go
+++ b/graph/db/graph_test.go
@@ -125,6 +125,14 @@ func TestNodeInsertionAndDeletion(t *testing.T) {
require.NoError(t, graph.AddNode(ctx, node))
assertNodeInCache(t, graph, node, testFeatures)
+ // Our AddNode implementation uses the batcher meaning that it is
+ // possible that two updates for the same node announcement may be
+ // processed in the same batch. So to avoid the conflict error (since we
+ // require at the DB level that the new timestamp is strictly
+ // greater than the previous one), we need to gracefully handle the
+ // case where the exact same node announcement is added twice.
+ require.NoError(t, graph.AddNode(ctx, node))
+
// Next, fetch the node from the database to ensure everything was
// serialized properly.
dbNode, err := graph.FetchNode(ctx, testPub)
diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go
index f134087..a942405 100644
--- a/graph/db/sql_store.go
+++ b/graph/db/sql_store.go
@@ -255,6 +255,17 @@ func (s *SQLStore) AddNode(ctx context.Context,
Opts: batch.NewSchedulerOptions(opts...),
Do: func(queries SQLQueries) error {
_, err := upsertNode(ctx, queries, node)
+
+ // It is possible that two of the same node
+ // announcements are both being processed in the same
+ // batch. This may case the UpsertNode conflict to
+ // be hit since we require at the db layer that the
+ // new last_update is greater than the existing
+ // last_update. We need to gracefully handle this here.
+ if errors.Is(err, sql.ErrNoRows) {
+ return nil
+ }
+
return err
},
}
Why this scored 34/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.