From 8aa8c7cc422bd31e502c4aad47ddeb4757b15cc7 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Sun, 3 Aug 2025 17:33:04 +0200 Subject: [PATCH] multi: remove the NodeRTx interface type --- autopilot/graph.go | 9 +++---- autopilot/interface.go | 7 +++--- autopilot/prefattach_test.go | 23 ------------------ graph/db/benchmark_test.go | 28 ++++++++++------------ graph/db/graph_test.go | 17 ++++++------- graph/db/interfaces.go | 15 ++---------- graph/db/kv_store.go | 39 ++++-------------------------- graph/db/sql_migration_test.go | 27 +++++++++++---------- graph/db/sql_store.go | 44 ++++------------------------------ rpcserver.go | 4 ++-- 10 files changed, 58 insertions(+), 155 deletions(-) diff --git a/autopilot/graph.go b/autopilot/graph.go index a2a2e02ff..5a1f8dceb 100644 --- a/autopilot/graph.go +++ b/autopilot/graph.go @@ -10,6 +10,7 @@ import ( "github.com/btcsuite/btcd/btcec/v2/ecdsa" "github.com/btcsuite/btcd/btcutil" graphdb "github.com/lightningnetwork/lnd/graph/db" + "github.com/lightningnetwork/lnd/graph/db/models" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" ) @@ -83,17 +84,17 @@ func (d *dbNode) Addrs() []net.Addr { func (d *databaseChannelGraph) ForEachNode(ctx context.Context, cb func(context.Context, Node) error, reset func()) error { - return d.db.ForEachNode(ctx, func(nodeTx graphdb.NodeRTx) error { + return d.db.ForEachNode(ctx, func(n *models.LightningNode) 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(nodeTx.Node().Addresses) == 0 { + if len(n.Addresses) == 0 { return nil } node := &dbNode{ - pub: nodeTx.Node().PubKeyBytes, - addrs: nodeTx.Node().Addresses, + pub: n.PubKeyBytes, + addrs: n.Addresses, } return cb(ctx, node) diff --git a/autopilot/interface.go b/autopilot/interface.go index 1554b03ee..9d9d0d1ca 100644 --- a/autopilot/interface.go +++ b/autopilot/interface.go @@ -8,6 +8,7 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/wire" graphdb "github.com/lightningnetwork/lnd/graph/db" + "github.com/lightningnetwork/lnd/graph/db/models" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" ) @@ -228,9 +229,9 @@ type GraphSource interface { // ForEachNode iterates through all the stored vertices/nodes in the // graph, executing the passed callback with each node encountered. If // the callback returns an error, then the transaction is aborted and - // the iteration stops early. Any operations performed on the NodeTx - // passed to the call-back are executed under the same read transaction. - ForEachNode(context.Context, func(graphdb.NodeRTx) error, func()) error + // the iteration stops early. + ForEachNode(context.Context, func(*models.LightningNode) error, + func()) error // ForEachNodeCached is similar to ForEachNode, but it utilizes the // channel graph cache if one is available. It is less consistent than diff --git a/autopilot/prefattach_test.go b/autopilot/prefattach_test.go index f0f351384..d7a578f7d 100644 --- a/autopilot/prefattach_test.go +++ b/autopilot/prefattach_test.go @@ -741,26 +741,3 @@ func (m *memChannelGraph) addRandNode() (*btcec.PublicKey, error) { return newPub, nil } - -type testNodeTx struct { - db *testDBGraph - node *models.LightningNode -} - -func (t *testNodeTx) Node() *models.LightningNode { - return t.node -} - -func (t *testNodeTx) ForEachChannel(f func(*models.ChannelEdgeInfo, - *models.ChannelEdgePolicy, *models.ChannelEdgePolicy) error) error { - - return t.db.db.ForEachNodeChannel(context.Background(), - t.node.PubKeyBytes, func(edge *models.ChannelEdgeInfo, policy1, - policy2 *models.ChannelEdgePolicy) error { - - return f(edge, policy1, policy2) - }, func() {}, - ) -} - -var _ graphdb.NodeRTx = (*testNodeTx)(nil) diff --git a/graph/db/benchmark_test.go b/graph/db/benchmark_test.go index e5fd6c256..08dba16de 100644 --- a/graph/db/benchmark_test.go +++ b/graph/db/benchmark_test.go @@ -338,12 +338,15 @@ func TestPopulateDBs(t *testing.T) { // graph. countNodes := func(graph *ChannelGraph) int { numNodes := 0 - err := graph.ForEachNode(ctx, func(tx NodeRTx) error { - numNodes++ - return nil - }, func() { - numNodes = 0 - }) + err := graph.ForEachNode( + ctx, func(node *models.LightningNode) error { + numNodes++ + + return nil + }, func() { + numNodes = 0 + }, + ) require.NoError(t, err) return numNodes @@ -487,18 +490,12 @@ func syncGraph(t *testing.T, src, dest *ChannelGraph) { } var wgNodes sync.WaitGroup - err := src.ForEachNode(ctx, func(tx NodeRTx) error { + err := src.ForEachNode(ctx, func(node *models.LightningNode) error { wgNodes.Add(1) go func() { defer wgNodes.Done() - // NOTE: even though the transaction (tx) may have - // already been aborted, it is still ok to use the - // Node() result since that is a static object that - // is not affected by the transaction state. - err := dest.AddLightningNode( - ctx, tx.Node(), batch.LazyAdd(), - ) + err := dest.AddLightningNode(ctx, node, batch.LazyAdd()) require.NoError(t, err) mu.Lock() @@ -658,7 +655,8 @@ func BenchmarkGraphReadMethods(b *testing.B) { name: "ForEachNode", fn: func(b testing.TB, store V1Store) { err := store.ForEachNode( - ctx, func(_ NodeRTx) error { + ctx, + func(_ *models.LightningNode) error { // Increment the counter to // ensure the callback is doing // something. diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index ef7ec4738..b9d972b3b 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -1469,8 +1469,8 @@ func TestGraphTraversalCacheable(t *testing.T) { // Create a map of all nodes with the iteration we know works (because // it is tested in another test). nodeMap := make(map[route.Vertex]struct{}) - err := graph.ForEachNode(ctx, func(tx NodeRTx) error { - nodeMap[tx.Node().PubKeyBytes] = struct{}{} + err := graph.ForEachNode(ctx, func(n *models.LightningNode) error { + nodeMap[n.PubKeyBytes] = struct{}{} return nil }, func() {}) @@ -1601,8 +1601,8 @@ func fillTestGraph(t testing.TB, graph *ChannelGraph, numNodes, // Iterate over each node as returned by the graph, if all nodes are // reached, then the map created above should be empty. - err := graph.ForEachNode(ctx, func(tx NodeRTx) error { - delete(nodeIndex, tx.Node().Alias) + err := graph.ForEachNode(ctx, func(n *models.LightningNode) error { + delete(nodeIndex, n.Alias) return nil }, func() {}) require.NoError(t, err) @@ -1710,11 +1710,12 @@ func assertNumChans(t *testing.T, graph *ChannelGraph, n int) { func assertNumNodes(t *testing.T, graph *ChannelGraph, n int) { numNodes := 0 - err := graph.ForEachNode(context.Background(), func(tx NodeRTx) error { - numNodes++ + err := graph.ForEachNode(context.Background(), + func(_ *models.LightningNode) error { + numNodes++ - return nil - }, func() {}) + return nil + }, func() {}) if err != nil { _, _, line, _ := runtime.Caller(1) t.Fatalf("line %v: unable to scan nodes: %v", line, err) diff --git a/graph/db/interfaces.go b/graph/db/interfaces.go index 5ad086d54..7f9b37008 100644 --- a/graph/db/interfaces.go +++ b/graph/db/interfaces.go @@ -14,14 +14,6 @@ import ( "github.com/lightningnetwork/lnd/routing/route" ) -// NodeRTx represents transaction object with an underlying node associated that -// can be used to make further queries to the graph under the same transaction. -// This is useful for consistency during graph traversal and queries. -type NodeRTx interface { - // Node returns the raw information of the node. - Node() *models.LightningNode -} - // NodeTraverser is an abstract read only interface that provides information // about nodes and their edges. The interface is about providing fast read-only // access to the graph and so if a cache is available, it should be used. @@ -94,11 +86,8 @@ type V1Store interface { //nolint:interfacebloat // ForEachNode iterates through all the stored vertices/nodes in the // graph, executing the passed callback with each node encountered. If // the callback returns an error, then the transaction is aborted and - // the iteration stops early. Any operations performed on the NodeTx - // passed to the call-back are executed under the same read transaction - // and so, methods on the NodeTx object _MUST_ only be called from - // within the call-back. - ForEachNode(ctx context.Context, cb func(tx NodeRTx) error, + // the iteration stops early. + ForEachNode(ctx context.Context, cb func(*models.LightningNode) error, reset func()) error // ForEachNodeCacheable iterates through all the stored vertices/nodes diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go index cf49b0d60..49191d7c2 100644 --- a/graph/db/kv_store.go +++ b/graph/db/kv_store.go @@ -800,16 +800,16 @@ func (c *KVStore) DisabledChannelIDs() ([]uint64, error) { // ForEachNode iterates through all the stored vertices/nodes in the graph, // executing the passed callback with each node encountered. If the callback // returns an error, then the transaction is aborted and the iteration stops -// early. Any operations performed on the NodeTx passed to the call-back are -// executed under the same read transaction and so, methods on the NodeTx object -// _MUST_ only be called from within the call-back. +// early. +// +// NOTE: this is part of the V1Store interface. func (c *KVStore) ForEachNode(_ context.Context, - cb func(tx NodeRTx) error, reset func()) error { + cb func(*models.LightningNode) error, reset func()) error { return forEachNode(c.db, func(tx kvdb.RTx, node *models.LightningNode) error { - return cb(newChanGraphNodeTx(tx, c, node)) + return cb(node) }, reset) } @@ -4877,32 +4877,3 @@ func deserializeChanEdgePolicyRaw(r io.Reader) (*models.ChannelEdgePolicy, return edge, nil } - -// chanGraphNodeTx is an implementation of the NodeRTx interface backed by the -// KVStore and a kvdb.RTx. -type chanGraphNodeTx struct { - tx kvdb.RTx - db *KVStore - node *models.LightningNode -} - -// A compile-time constraint to ensure chanGraphNodeTx implements the NodeRTx -// interface. -var _ NodeRTx = (*chanGraphNodeTx)(nil) - -func newChanGraphNodeTx(tx kvdb.RTx, db *KVStore, - node *models.LightningNode) *chanGraphNodeTx { - - return &chanGraphNodeTx{ - tx: tx, - db: db, - node: node, - } -} - -// Node returns the raw information of the node. -// -// NOTE: This is a part of the NodeRTx interface. -func (c *chanGraphNodeTx) Node() *models.LightningNode { - return c.node -} diff --git a/graph/db/sql_migration_test.go b/graph/db/sql_migration_test.go index 7d6b0e3b0..d134b2fb1 100644 --- a/graph/db/sql_migration_test.go +++ b/graph/db/sql_migration_test.go @@ -395,23 +395,24 @@ func assertInSync(t *testing.T, kvDB *KVStore, sqlDB *SQLStore, func fetchAllNodes(t *testing.T, store V1Store) []*models.LightningNode { nodes := make([]*models.LightningNode, 0) - err := store.ForEachNode(context.Background(), func(tx NodeRTx) error { - node := tx.Node() + err := store.ForEachNode(context.Background(), + func(node *models.LightningNode) error { - // Call PubKey to ensure the objects cached pubkey is set so that - // the objects can be compared as a whole. - _, err := node.PubKey() - require.NoError(t, err) + // Call PubKey to ensure the objects cached pubkey is set so that + // the objects can be compared as a whole. + _, err := node.PubKey() + require.NoError(t, err) - // Sort the addresses to ensure a consistent order. - sortAddrs(node.Addresses) + // Sort the addresses to ensure a consistent order. + sortAddrs(node.Addresses) - nodes = append(nodes, node) + nodes = append(nodes, node) - return nil - }, func() { - nodes = nil - }) + return nil + }, func() { + nodes = nil + }, + ) require.NoError(t, err) // Sort the nodes by their public key to ensure a consistent order. diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go index f6430ba43..7a7987855 100644 --- a/graph/db/sql_store.go +++ b/graph/db/sql_store.go @@ -799,60 +799,24 @@ func (s *SQLStore) ForEachSourceNodeChannel(ctx context.Context, // ForEachNode iterates through all the stored vertices/nodes in the graph, // executing the passed callback with each node encountered. If the callback // returns an error, then the transaction is aborted and the iteration stops -// early. Any operations performed on the NodeTx passed to the call-back are -// executed under the same read transaction and so, methods on the NodeTx object -// _MUST_ only be called from within the call-back. +// early. // // NOTE: part of the V1Store interface. func (s *SQLStore) ForEachNode(ctx context.Context, - cb func(tx NodeRTx) error, reset func()) error { + cb func(node *models.LightningNode) error, reset func()) error { return s.db.ExecTx(ctx, sqldb.ReadTxOpt(), func(db SQLQueries) error { return forEachNodePaginated( ctx, s.cfg.QueryCfg, db, - ProtocolV1, - func(ctx context.Context, dbNodeID int64, + ProtocolV1, func(_ context.Context, _ int64, node *models.LightningNode) error { - return cb(newSQLGraphNodeTx( - db, s.cfg, dbNodeID, node, - )) + return cb(node) }, ) }, reset) } -// sqlGraphNodeTx is an implementation of the NodeRTx interface backed by the -// SQLStore and a SQL transaction. -type sqlGraphNodeTx struct { - db SQLQueries - id int64 - node *models.LightningNode - cfg *SQLStoreConfig -} - -// A compile-time constraint to ensure sqlGraphNodeTx implements the NodeRTx -// interface. -var _ NodeRTx = (*sqlGraphNodeTx)(nil) - -func newSQLGraphNodeTx(db SQLQueries, cfg *SQLStoreConfig, - id int64, node *models.LightningNode) *sqlGraphNodeTx { - - return &sqlGraphNodeTx{ - db: db, - cfg: cfg, - id: id, - node: node, - } -} - -// Node returns the raw information of the node. -// -// NOTE: This is a part of the NodeRTx interface. -func (s *sqlGraphNodeTx) Node() *models.LightningNode { - return s.node -} - // 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/rpcserver.go b/rpcserver.go index a4e83fe8f..08f9cf565 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -6757,8 +6757,8 @@ func (r *rpcServer) DescribeGraph(ctx context.Context, // First iterate through all the known nodes (connected or unconnected // within the graph), collating their current state into the RPC // response. - err := graph.ForEachNode(ctx, func(nodeTx graphdb.NodeRTx) error { - lnNode := marshalNode(nodeTx.Node()) + err := graph.ForEachNode(ctx, func(node *models.LightningNode) error { + lnNode := marshalNode(node) resp.Nodes = append(resp.Nodes, lnNode)