← Watch feed
graph/db: remove address loading from cached node iteration
Public commit record
73/100 · Adequate
What the developer wrote
Authored by Elle Mouton
graph/db: remove address loading from cached node iteration
ForEachNodeCached is now only used for topology-oriented traversal,
so the address-loading option forced one autopilot scoring path to
bypass the in-memory graph cache for data it did not consume. Remove
the withAddrs parameter and the associated SQL/KV address plumbing
so cached node iteration can consistently use the graph cache when
it is loaded.
Autopilot still requires peer addresses before opening channels.
That filtering remains in Agent.openChans via ForEachNode, where the
selected candidates' addresses are collected for ConnectToPeer. The
trade-off is that ForEachNodesChannels no longer excludes
addressless nodes from graph-wide scoring inputs such as median
channel size or centrality, which also feed lncli getnetworkinfo
statistics like graph diameter. In practice the only addressless
nodes our local view tends to know about are nodes with no public
channels (e.g. our own node or peers we share only private channels
with), so the impact on the reported stats should be negligible.
Active channel candidates remain address-filtered before dialing.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This is a performance and code-simplification cleanup, not a security fix. It removes an optional address-loading path from a cached graph-traversal function so the cache can be used more consistently. The only behavioral change is that autopilot's graph-wide scoring now includes nodes without advertised addresses when computing statistics such as median channel size and centrality. Those nodes were previously skipped. Active channel candidates are still filtered by address before any peer connection is attempted, so the practical security impact is minimal.
Recommended action
No security action required. Treat as a normal refactoring/performance change. If desired, verify that autopilot's later address filter in Agent.openChans still prevents opening channels to unreachable peers.
Security signals we found
01
Behavioral change: addressless nodes now included in graph-wide scoring/statistics
02
No input validation, cryptographic, or authorization changes
03
No memory safety, concurrency, or resource exhaustion fixes
04
No vendor disclosure of security relevance
Evidence from the diff
The commit refactors ForEachNodeCached across graph/db and autopilot to drop the withAddrs parameter and the associated SQL/KV address plumbing. In autopilot/graph.go, ForEachNodesChannels no longer skips nodes with len(addrs)==0, meaning addressless nodes are included in topology scoring inputs. Address filtering for actual channel opening remains in Agent.openChans via ForEachNode. The change also affects GetNetworkInfo graph statistics, but the commit message states the impact should be negligible because addressless nodes in the local view are typically those with no public channels.
Changed components
autopilot/graph.goautopilot/interface.gograph/db/graph.gograph/db/interfaces.gograph/db/kv_store.gograph/db/sql_store.gorpcserver.go GetNetworkInfoInspect captured patch +32 / −100
diff --git a/autopilot/graph.go b/autopilot/graph.go
index be64015..d82cdd3 100644
--- a/autopilot/graph.go
+++ b/autopilot/graph.go
@@ -111,18 +111,12 @@ func (d *databaseChannelGraph) ForEachNodesChannels(ctx context.Context,
cb func(context.Context, Node, []*ChannelEdge) error,
reset func()) error {
+ // The channel-scoring callers only need topology data here. Address
+ // filtering happens through ForEachNode before connecting to peers.
return d.db.ForEachNodeCached(
- ctx, true, func(ctx context.Context, node route.Vertex,
- addrs []net.Addr,
+ ctx, func(ctx context.Context, node route.Vertex,
chans map[uint64]*graphdb.DirectedChannel) error {
- // We'll skip over any node that doesn't have any
- // advertised addresses. As we won't be able to reach
- // them to actually open any channels.
- if len(addrs) == 0 {
- return nil
- }
-
edges := make([]*ChannelEdge, 0, len(chans))
for _, channel := range chans {
edges = append(edges, &ChannelEdge{
@@ -135,8 +129,7 @@ func (d *databaseChannelGraph) ForEachNodesChannels(ctx context.Context,
}
return cb(ctx, &dbNode{
- pub: node,
- addrs: addrs,
+ pub: node,
}, edges)
}, reset,
)
@@ -196,8 +189,8 @@ func (nc dbNodeCached) Addrs() []net.Addr {
func (dc *databaseChannelGraphCached) ForEachNode(ctx context.Context,
cb func(context.Context, Node) error, reset func()) error {
- return dc.db.ForEachNodeCached(ctx, false, func(ctx context.Context,
- n route.Vertex, _ []net.Addr,
+ return dc.db.ForEachNodeCached(ctx, func(ctx context.Context,
+ n route.Vertex,
channels map[uint64]*graphdb.DirectedChannel) error {
if len(channels) > 0 {
@@ -223,8 +216,8 @@ func (dc *databaseChannelGraphCached) ForEachNodesChannels(ctx context.Context,
cb func(context.Context, Node, []*ChannelEdge) error,
reset func()) error {
- return dc.db.ForEachNodeCached(ctx, false, func(ctx context.Context,
- n route.Vertex, _ []net.Addr,
+ return dc.db.ForEachNodeCached(ctx, func(ctx context.Context,
+ n route.Vertex,
channels map[uint64]*graphdb.DirectedChannel) error {
edges := make([]*ChannelEdge, 0, len(channels))
diff --git a/autopilot/interface.go b/autopilot/interface.go
index 215f920..b3fc3de 100644
--- a/autopilot/interface.go
+++ b/autopilot/interface.go
@@ -237,9 +237,8 @@ type GraphSource interface {
// channel graph cache if one is available. It is less consistent than
// ForEachNode since any further calls are made across multiple
// transactions.
- ForEachNodeCached(ctx context.Context, withAddrs bool,
+ ForEachNodeCached(ctx context.Context,
cb func(ctx context.Context, node route.Vertex,
- addrs []net.Addr,
chans map[uint64]*graphdb.DirectedChannel) error,
reset func()) error
}
diff --git a/graph/db/benchmark_test.go b/graph/db/benchmark_test.go
index d076444..ff245d0 100644
--- a/graph/db/benchmark_test.go
+++ b/graph/db/benchmark_test.go
@@ -5,7 +5,6 @@ import (
"database/sql"
"errors"
"fmt"
- "net"
"path"
"sync"
"testing"
@@ -698,10 +697,9 @@ func BenchmarkGraphReadMethods(b *testing.B) {
fn: func(b testing.TB, store Store) {
//nolint:ll
err := store.ForEachNodeCached(
- ctx, lnwire.GossipVersion1, false,
+ ctx, lnwire.GossipVersion1,
func(context.Context,
route.Vertex,
- []net.Addr,
map[uint64]*DirectedChannel) error {
// Increment the counter to
diff --git a/graph/db/graph.go b/graph/db/graph.go
index 9e72cad..a63b9d0 100644
--- a/graph/db/graph.go
+++ b/graph/db/graph.go
@@ -342,21 +342,21 @@ func (c *ChannelGraph) GraphSession(ctx context.Context,
//
// NOTE: The callback contents MUST not be modified.
func (c *ChannelGraph) ForEachNodeCached(ctx context.Context,
- v lnwire.GossipVersion, withAddrs bool,
- cb func(ctx context.Context, node route.Vertex, addrs []net.Addr,
+ v lnwire.GossipVersion,
+ cb func(ctx context.Context, node route.Vertex,
chans map[uint64]*DirectedChannel) error, reset func()) error {
- if !withAddrs && c.cache != nil && c.cache.isLoaded() {
+ if c.cache != nil && c.cache.isLoaded() {
return c.cache.graphCache.ForEachNode(
func(node route.Vertex,
channels map[uint64]*DirectedChannel) error {
- return cb(ctx, node, nil, channels)
+ return cb(ctx, node, channels)
},
)
}
- return c.db.ForEachNodeCached(ctx, v, withAddrs, cb, reset)
+ return c.db.ForEachNodeCached(ctx, v, cb, reset)
}
// AddNode adds a vertex/node to the graph database. If the node is not
@@ -919,12 +919,11 @@ func (c *VersionedGraph) ForEachNodeDirectedChannel(ctx context.Context,
// ForEachNodeCached iterates through all stored vertices/nodes in the graph,
// delegating to the embedded ChannelGraph.
func (c *VersionedGraph) ForEachNodeCached(ctx context.Context,
- withAddrs bool, cb func(ctx context.Context, node route.Vertex,
- addrs []net.Addr,
+ cb func(ctx context.Context, node route.Vertex,
chans map[uint64]*DirectedChannel) error,
reset func()) error {
- return c.ChannelGraph.ForEachNodeCached(ctx, c.v, withAddrs, cb, reset)
+ return c.ChannelGraph.ForEachNodeCached(ctx, c.v, cb, reset)
}
// ForEachNode iterates through all stored vertices/nodes in the graph.
diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go
index 6521a84..3d0156f 100644
--- a/graph/db/graph_test.go
+++ b/graph/db/graph_test.go
@@ -449,24 +449,6 @@ func testNodeInsertionAndDeletion(t *testing.T, v lnwire.GossipVersion) {
dbNode, err = graph.FetchNode(ctx, testPub)
require.NoError(t, err)
require.Equal(t, expAddrs, dbNode.Addresses)
-
- // Also check that the withAddr param of ForEachNodeCached correctly
- // returns the addresses we expect for this node.
- err = graph.ForEachNodeCached(
- ctx, true, func(ctx context.Context, node route.Vertex,
- addrs []net.Addr,
- chans map[uint64]*DirectedChannel) error {
-
- if node != dbNode.PubKeyBytes {
- return nil
- }
-
- require.Equal(t, expAddrs, addrs)
-
- return nil
- }, func() {},
- )
- require.NoError(t, err)
}
// testPartialNode tests that partial/shell nodes are correctly created when
@@ -1799,8 +1781,8 @@ func TestGraphTraversal(t *testing.T) {
nodeIndex[node.PubKeyBytes] = struct{}{}
}
- err := graph.ForEachNodeCached(ctx, lnwire.GossipVersion1, false,
- func(_ context.Context, node route.Vertex, _ []net.Addr,
+ err := graph.ForEachNodeCached(ctx, lnwire.GossipVersion1,
+ func(_ context.Context, node route.Vertex,
chans map[uint64]*DirectedChannel) error {
if _, ok := nodeIndex[node]; !ok {
@@ -6026,9 +6008,8 @@ func TestAsyncGraphCache(t *testing.T) {
// assert that we get the expected number of nodes and
// channels.
err := graph.ForEachNodeCached(
- ctx, lnwire.GossipVersion1, false,
+ ctx, lnwire.GossipVersion1,
func(_ context.Context, node route.Vertex,
- _ []net.Addr,
chans map[uint64]*DirectedChannel) error {
numNodes++
diff --git a/graph/db/interfaces.go b/graph/db/interfaces.go
index 0230300..c126be0 100644
--- a/graph/db/interfaces.go
+++ b/graph/db/interfaces.go
@@ -81,17 +81,11 @@ type Store interface { //nolint:interfacebloat
*models.ChannelEdgePolicy) error, reset func()) error
// ForEachNodeCached is similar to forEachNode, but it returns
- // DirectedChannel data to the call-back. If withAddrs is true, then
- // the call-back will also be provided with the addresses associated
- // with the node. The address retrieval will likely result in an
- // additional round-trip to the database, so it should only be used if
- // the addresses are actually needed.
+ // DirectedChannel data to the call-back.
//
// NOTE: The callback contents MUST not be modified.
ForEachNodeCached(ctx context.Context, v lnwire.GossipVersion,
- withAddrs bool,
cb func(ctx context.Context, node route.Vertex,
- addrs []net.Addr,
chans map[uint64]*DirectedChannel) error,
reset func()) error
diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go
index ed6d0e0..47c478c 100644
--- a/graph/db/kv_store.go
+++ b/graph/db/kv_store.go
@@ -703,8 +703,8 @@ func (c *KVStore) FetchNodeFeatures(_ context.Context, v lnwire.GossipVersion,
//
// NOTE: The callback contents MUST not be modified.
func (c *KVStore) ForEachNodeCached(ctx context.Context,
- v lnwire.GossipVersion, withAddrs bool,
- cb func(ctx context.Context, node route.Vertex, addrs []net.Addr,
+ v lnwire.GossipVersion,
+ cb func(ctx context.Context, node route.Vertex,
chans map[uint64]*DirectedChannel) error, reset func()) error {
if v != lnwire.GossipVersion1 {
@@ -769,12 +769,7 @@ func (c *KVStore) ForEachNodeCached(ctx context.Context,
return err
}
- var addrs []net.Addr
- if withAddrs {
- addrs = node.Addresses
- }
-
- return cb(ctx, node.PubKeyBytes, addrs, channels)
+ return cb(ctx, node.PubKeyBytes, channels)
}, reset)
}
diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go
index f4824d7..e705468 100644
--- a/graph/db/sql_store.go
+++ b/graph/db/sql_store.go
@@ -1719,20 +1719,16 @@ func (s *SQLStore) chanUpdatesInHorizonV2(ctx context.Context,
}
// ForEachNodeCached is similar to forEachNode, but it returns DirectedChannel
-// data to the call-back. If withAddrs is true, then the call-back will also be
-// provided with the addresses associated with the node. The address retrieval
-// result in an additional round-trip to the database, so it should only be used
-// if the addresses are actually needed.
+// data to the call-back.
//
// NOTE: part of the Store interface.
func (s *SQLStore) ForEachNodeCached(ctx context.Context,
- v lnwire.GossipVersion, withAddrs bool,
- cb func(ctx context.Context, node route.Vertex, addrs []net.Addr,
+ v lnwire.GossipVersion,
+ cb func(ctx context.Context, node route.Vertex,
chans map[uint64]*DirectedChannel) error, reset func()) error {
type nodeCachedBatchData struct {
features map[int64][]int
- addrs map[int64][]nodeAddress
chanBatchData *batchChannelData
chanMap map[int64][]sqlc.ListChannelsForNodeIDsRow
}
@@ -1765,19 +1761,6 @@ func (s *SQLStore) ForEachNodeCached(ctx context.Context,
"node features: %w", err)
}
- // Maybe fetch the node's addresses if requested.
- var nodeAddrs map[int64][]nodeAddress
- if withAddrs {
- nodeAddrs, err = batchLoadNodeAddressesHelper(
- ctx, s.cfg.QueryCfg, db, nodeIDs,
- )
- if err != nil {
- return nil, fmt.Errorf("unable to "+
- "batch load node "+
- "addresses: %w", err)
- }
- }
-
// Batch load ALL unique channels for ALL nodes in this
// page.
allChannels, err := db.ListChannelsForNodeIDs(
@@ -1866,7 +1849,6 @@ func (s *SQLStore) ForEachNodeCached(ctx context.Context,
return &nodeCachedBatchData{
features: nodeFeatures,
- addrs: nodeAddrs,
chanBatchData: channelBatchData,
chanMap: nodeChannelMap,
}, nil
@@ -1910,15 +1892,7 @@ func (s *SQLStore) ForEachNodeCached(ctx context.Context,
channels[directedChan.ChannelID] = directedChan
}
- addrs, err := buildNodeAddresses(
- batchData.addrs[nodeData.ID],
- )
- if err != nil {
- return fmt.Errorf("unable to build node "+
- "addresses: %w", err)
- }
-
- return cb(ctx, nodePub, addrs, channels)
+ return cb(ctx, nodePub, channels)
}
return sqldb.ExecuteCollectAndBatchWithSharedDataQuery(
diff --git a/itest/lnd_graph_migration_test.go b/itest/lnd_graph_migration_test.go
index c8ae229..5dc7491 100644
--- a/itest/lnd_graph_migration_test.go
+++ b/itest/lnd_graph_migration_test.go
@@ -3,7 +3,6 @@ package itest
import (
"context"
"database/sql"
- "net"
graphdb "github.com/lightningnetwork/lnd/graph/db"
"github.com/lightningnetwork/lnd/lntest"
@@ -66,9 +65,9 @@ func testGraphMigration(ht *lntest.HarnessTest) {
numNodes int
edges = make(map[uint64]bool)
)
- err := db.ForEachNodeCached(ctx, lnwire.GossipVersion1, false,
+ err := db.ForEachNodeCached(ctx, lnwire.GossipVersion1,
func(_ context.Context,
- _ route.Vertex, _ []net.Addr,
+ _ route.Vertex,
chans map[uint64]*graphdb.DirectedChannel,
) error {
diff --git a/rpcserver.go b/rpcserver.go
index d4a9ce5..491bd8a 100644
--- a/rpcserver.go
+++ b/rpcserver.go
@@ -7439,8 +7439,8 @@ func (r *rpcServer) GetNetworkInfo(ctx context.Context,
// network, tallying up the total number of nodes, and also gathering
// each node so we can measure the graph diameter and degree stats
// below.
- err := graph.ForEachNodeCached(ctx, false, func(ctx context.Context,
- node route.Vertex, _ []net.Addr,
+ err := graph.ForEachNodeCached(ctx, func(ctx context.Context,
+ node route.Vertex,
edges map[uint64]*graphdb.DirectedChannel) error {
// Increment the total number of nodes with each iteration.
Risk score
Our methodology →Why this scored 18/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.