graph/db: move sanity check out of insertChannel
What the developer wrote
Authored by Elle Mouton
graph/db: move sanity check out of insertChannel
We do this so that this lookup is only done in the situation it is
actually needed. During a migration, we dont need to special case this
AlreadyExists error since we will terminate the transaction either way.
So there is no need for the extra lookup during the migration.
A timing analysis showed that this query was significantly impacting the
performance of the migration when run with a postgres backend.
What changed, and why it matters
This change is a performance optimization for LND's database code. It moves a 'does this channel already exist?' check from inside a helper function to the caller, so the check is skipped during database migrations where it isn't needed. The commit message says this significantly speeds up migrations on PostgreSQL. There is no direct evidence this fixes a security vulnerability.
No security action required. Treat as a routine performance refactor. If reviewing for a release, verify that the duplicate-channel behavior remains correct in `AddChannelEdge` and that migration paths no longer perform the unnecessary lookup.
Security signals we found
No security-relevant signal in commit message or diff
Change is described as performance optimization, not security fix
Functional duplicate-channel check is preserved in caller
No input validation, authentication, authorization, or cryptographic changes
Evidence from the diff
The patch refactors insertChannel in graph/db/sql_store.go by extracting the pre-insert GetChannelBySCID lookup into AddChannelEdge. This avoids the lookup during migrations while preserving the explicit duplicate-check behavior for normal AddChannelEdge operations. The functional behavior of AddChannelEdge remains unchanged: it still checks for existing channels, still silences ErrEdgeAlreadyExist, and still returns other errors. The change is framed purely as a performance improvement for PostgreSQL-backed migrations.
Changed components
graph/db/sql_store.goinsertChannel functionAddChannelEdge functionLND graph database channel edge insertionInspect captured patch +20 / −24
diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go
index 1c913b7..40d5525 100644
--- a/graph/db/sql_store.go
+++ b/graph/db/sql_store.go
@@ -591,15 +591,29 @@ func (s *SQLStore) AddChannelEdge(ctx context.Context,
alreadyExists = false
},
Do: func(tx SQLQueries) error {
- _, err := insertChannel(ctx, tx, edge)
-
- // Silence ErrEdgeAlreadyExist so that the batch can
- // succeed, but propagate the error via local state.
- if errors.Is(err, ErrEdgeAlreadyExist) {
+ chanIDB := channelIDToBytes(edge.ChannelID)
+
+ // Make sure that the channel doesn't already exist. We
+ // do this explicitly instead of relying on catching a
+ // unique constraint error because relying on SQL to
+ // throw that error would abort the entire batch of
+ // transactions.
+ _, err := tx.GetChannelBySCID(
+ ctx, sqlc.GetChannelBySCIDParams{
+ Scid: chanIDB,
+ Version: int16(ProtocolV1),
+ },
+ )
+ if err == nil {
alreadyExists = true
return nil
+ } else if !errors.Is(err, sql.ErrNoRows) {
+ return fmt.Errorf("unable to fetch channel: %w",
+ err)
}
+ _, err = insertChannel(ctx, tx, edge)
+
return err
},
OnCommit: func(err error) error {
@@ -3767,24 +3781,6 @@ type dbChanInfo struct {
func insertChannel(ctx context.Context, db SQLQueries,
edge *models.ChannelEdgeInfo) (*dbChanInfo, error) {
- chanIDB := channelIDToBytes(edge.ChannelID)
-
- // Make sure that the channel doesn't already exist. We do this
- // explicitly instead of relying on catching a unique constraint error
- // because relying on SQL to throw that error would abort the entire
- // batch of transactions.
- _, err := db.GetChannelBySCID(
- ctx, sqlc.GetChannelBySCIDParams{
- Scid: chanIDB,
- Version: int16(ProtocolV1),
- },
- )
- if err == nil {
- return nil, ErrEdgeAlreadyExist
- } else if !errors.Is(err, sql.ErrNoRows) {
- return nil, fmt.Errorf("unable to fetch channel: %w", err)
- }
-
// Make sure that at least a "shell" entry for each node is present in
// the nodes table.
node1DBID, err := maybeCreateShellNode(ctx, db, edge.NodeKey1Bytes)
@@ -3804,7 +3800,7 @@ func insertChannel(ctx context.Context, db SQLQueries,
createParams := sqlc.CreateChannelParams{
Version: int16(ProtocolV1),
- Scid: chanIDB,
+ Scid: channelIDToBytes(edge.ChannelID),
NodeID1: node1DBID,
NodeID2: node2DBID,
Outpoint: edge.ChannelPoint.String(),
Why this scored 16/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.