graph/db: remove NodeRTx ForEachChannel method
What the developer wrote
Authored by Elle Mouton
graph/db: remove NodeRTx ForEachChannel method
What changed, and why it matters
This commit removes an unused method called ForEachChannel from the graph database interfaces and both of its implementations (one for the older key-value store and one for the newer SQL store). It also updates one integration test to use a different, already-existing method that provides the same information in a cached form. There is no indication this fixes a security bug; it appears to be routine code cleanup to simplify the interface and remove duplicated functionality.
No security action required. Treat as normal refactoring. If reviewing, confirm that all callers of the removed ForEachChannel method have been updated and that the replacement ForEachNodeCached behavior is equivalent for the test's purposes.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch deletes the NodeRTx.ForEachChannel method from graph/db/interfaces.go and its concrete implementations in graph/db/kv_store.go (chanGraphNodeTx.ForEachChannel) and graph/db/sql_store.go (sqlGraphNodeTx.ForEachChannel). The removed method iterated over a node’s channels within the same read transaction used to fetch the node. The itest/lnd_graph_migration_test.go test is updated to use ForEachNodeCached instead, which supplies a map of DirectedChannel values and avoids the need for a per-node channel iterator. The change is purely subtractive and migratory, with no added validation, no bounds checks, and no privilege or trust-boundary changes visible in the diff.
Changed components
graph/db/interfaces.gograph/db/kv_store.gograph/db/sql_store.goitest/lnd_graph_migration_test.goInspect captured patch +11 / −49
diff --git a/graph/db/interfaces.go b/graph/db/interfaces.go
index 3dadd14..5ad086d 100644
--- a/graph/db/interfaces.go
+++ b/graph/db/interfaces.go
@@ -20,11 +20,6 @@ import (
type NodeRTx interface {
// Node returns the raw information of the node.
Node() *models.LightningNode
-
- // ForEachChannel can be used to iterate over the node's channels under
- // the same transaction used to fetch the node.
- ForEachChannel(func(*models.ChannelEdgeInfo, *models.ChannelEdgePolicy,
- *models.ChannelEdgePolicy) error) error
}
// NodeTraverser is an abstract read only interface that provides information
diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go
index df761cf..cf49b0d 100644
--- a/graph/db/kv_store.go
+++ b/graph/db/kv_store.go
@@ -4906,24 +4906,3 @@ func newChanGraphNodeTx(tx kvdb.RTx, db *KVStore,
func (c *chanGraphNodeTx) Node() *models.LightningNode {
return c.node
}
-
-// ForEachChannel can be used to iterate over the node's channels under
-// the same transaction used to fetch the node.
-//
-// NOTE: This is a part of the NodeRTx interface.
-func (c *chanGraphNodeTx) ForEachChannel(f func(*models.ChannelEdgeInfo,
- *models.ChannelEdgePolicy, *models.ChannelEdgePolicy) error) error {
-
- return c.db.forEachNodeChannelTx(
- c.tx, c.node.PubKeyBytes,
- func(_ kvdb.RTx, info *models.ChannelEdgeInfo, policy1,
- policy2 *models.ChannelEdgePolicy) error {
-
- return f(info, policy1, policy2)
- },
- // NOTE: We don't need to reset anything here as the caller is
- // expected to pass in the reset function to the ForEachNode
- // method that constructed the chanGraphNodeTx.
- func() {},
- )
-}
diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go
index dc1c1c4..f6430ba 100644
--- a/graph/db/sql_store.go
+++ b/graph/db/sql_store.go
@@ -853,18 +853,6 @@ func (s *sqlGraphNodeTx) Node() *models.LightningNode {
return s.node
}
-// ForEachChannel can be used to iterate over the node's channels under the same
-// transaction used to fetch the node.
-//
-// NOTE: This is a part of the NodeRTx interface.
-func (s *sqlGraphNodeTx) ForEachChannel(cb func(*models.ChannelEdgeInfo,
- *models.ChannelEdgePolicy, *models.ChannelEdgePolicy) error) error {
-
- ctx := context.TODO()
-
- return forEachNodeChannel(ctx, s.db, s.cfg, s.id, cb)
-}
-
// ForEachNodeDirectedChannel iterates through all channels of a given node,
// executing the passed callback on the directed edge representing the channel
// and its incoming policy. If the callback returns an error, then the iteration
diff --git a/itest/lnd_graph_migration_test.go b/itest/lnd_graph_migration_test.go
index fe3e6b1..07abc3b 100644
--- a/itest/lnd_graph_migration_test.go
+++ b/itest/lnd_graph_migration_test.go
@@ -3,12 +3,13 @@ package itest
import (
"context"
"database/sql"
+ "net"
"github.com/lightningnetwork/lnd"
graphdb "github.com/lightningnetwork/lnd/graph/db"
- "github.com/lightningnetwork/lnd/graph/db/models"
"github.com/lightningnetwork/lnd/lntest"
"github.com/lightningnetwork/lnd/lntest/node"
+ "github.com/lightningnetwork/lnd/routing/route"
"github.com/lightningnetwork/lnd/sqldb"
"github.com/stretchr/testify/require"
)
@@ -69,19 +70,18 @@ func testGraphMigration(ht *lntest.HarnessTest) {
numNodes int
edges = make(map[uint64]bool)
)
- err := db.ForEachNode(ctx, func(tx graphdb.NodeRTx) error {
+ err := db.ForEachNodeCached(ctx, false, func(_ context.Context,
+ _ route.Vertex, _ []net.Addr,
+ chans map[uint64]*graphdb.DirectedChannel) error {
+
numNodes++
// For each node, also count the number of edges.
- return tx.ForEachChannel(
- func(info *models.ChannelEdgeInfo,
- _ *models.ChannelEdgePolicy,
- _ *models.ChannelEdgePolicy) error {
-
- edges[info.ChannelID] = true
- return nil
- },
- )
+ for _, ch := range chans {
+ edges[ch.ChannelID] = true
+ }
+
+ return nil
}, func() {
clear(edges)
numNodes = 0
Why this scored 12/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.