From 9068ffcd8b73336c331b92cd9bd58072e0dfe40a Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Fri, 8 Nov 2024 15:49:45 +0200 Subject: [PATCH 1/8] graph: let FetchNodeFeatures take an optional read tx For consistency in the graphsessoin.graph interface, we let the FetchNodeFeatures method take a read transaction just like the ForEachNodeDirectedChannel. This is nice because then all calls in the same pathfinding transaction use the same read transaction. --- graph/db/graph.go | 18 ++++++++++++------ graph/graphsession/graph_session.go | 8 ++++++-- routing/integrated_routing_context_test.go | 2 +- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/graph/db/graph.go b/graph/db/graph.go index 8004a2a5d..a7d3cf299 100644 --- a/graph/db/graph.go +++ b/graph/db/graph.go @@ -503,9 +503,12 @@ func (c *ChannelGraph) ForEachChannel(cb func(*models.ChannelEdgeInfo, // 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 -// is halted with the error propagated back up to the caller. +// is halted with the error propagated back up to the caller. An optional read +// transaction may be provided. If none is provided, a new one will be created. // // Unknown policies are passed into the callback as nil values. +// +// NOTE: this is part of the graphsession.graph interface. func (c *ChannelGraph) ForEachNodeDirectedChannel(tx kvdb.RTx, node route.Vertex, cb func(channel *DirectedChannel) error) error { @@ -517,7 +520,7 @@ func (c *ChannelGraph) ForEachNodeDirectedChannel(tx kvdb.RTx, toNodeCallback := func() route.Vertex { return node } - toNodeFeatures, err := c.FetchNodeFeatures(node) + toNodeFeatures, err := c.FetchNodeFeatures(tx, node) if err != nil { return err } @@ -562,8 +565,11 @@ func (c *ChannelGraph) ForEachNodeDirectedChannel(tx kvdb.RTx, } // FetchNodeFeatures returns the features of a given node. If no features are -// known for the node, an empty feature vector is returned. -func (c *ChannelGraph) FetchNodeFeatures( +// known for the node, an empty feature vector is returned. An optional read +// transaction may be provided. If none is provided, a new one will be created. +// +// NOTE: this is part of the graphsession.graph interface. +func (c *ChannelGraph) FetchNodeFeatures(tx kvdb.RTx, node route.Vertex) (*lnwire.FeatureVector, error) { if c.graphCache != nil { @@ -571,7 +577,7 @@ func (c *ChannelGraph) FetchNodeFeatures( } // Fallback that uses the database. - targetNode, err := c.FetchLightningNode(node) + targetNode, err := c.FetchLightningNodeTx(tx, node) switch err { // If the node exists and has features, return them directly. case nil: @@ -618,7 +624,7 @@ func (c *ChannelGraph) ForEachNodeCached(cb func(node route.Vertex, return node.PubKeyBytes } toNodeFeatures, err := c.FetchNodeFeatures( - node.PubKeyBytes, + tx, node.PubKeyBytes, ) if err != nil { return err diff --git a/graph/graphsession/graph_session.go b/graph/graphsession/graph_session.go index 6976fad79..c555e4b1a 100644 --- a/graph/graphsession/graph_session.go +++ b/graph/graphsession/graph_session.go @@ -96,7 +96,7 @@ func (g *session) ForEachNodeChannel(nodePub route.Vertex, func (g *session) FetchNodeFeatures(nodePub route.Vertex) ( *lnwire.FeatureVector, error) { - return g.graph.FetchNodeFeatures(nodePub) + return g.graph.FetchNodeFeatures(g.tx, nodePub) } // A compile-time check to ensure that *session implements the @@ -133,7 +133,11 @@ type graph interface { // FetchNodeFeatures returns the features of a given node. If no // features are known for the node, an empty feature vector is returned. - FetchNodeFeatures(node route.Vertex) (*lnwire.FeatureVector, error) + // + // NOTE: if a nil tx is provided, then it is expected that the + // implementation create a read only tx. + FetchNodeFeatures(tx kvdb.RTx, node route.Vertex) ( + *lnwire.FeatureVector, error) } // A compile-time check to ensure that *channeldb.ChannelGraph implements the diff --git a/routing/integrated_routing_context_test.go b/routing/integrated_routing_context_test.go index e4241dac5..5acae868d 100644 --- a/routing/integrated_routing_context_test.go +++ b/routing/integrated_routing_context_test.go @@ -400,5 +400,5 @@ func (g *mockGraphSessionChanDB) ForEachNodeChannel(nodePub route.Vertex, func (g *mockGraphSessionChanDB) FetchNodeFeatures(nodePub route.Vertex) ( *lnwire.FeatureVector, error) { - return g.graph.FetchNodeFeatures(nodePub) + return g.graph.FetchNodeFeatures(g.tx, nodePub) } From 971832c792c07418daf9b92ed035b8812218d6c2 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 12 Feb 2025 17:18:40 +0200 Subject: [PATCH 2/8] graph: temporarily rename some ChannelGraph methods Add the `Tx` suffix to both ForEachNodeDirectedChannelTx and FetchNodeFeatures temporarily so that we free up the original names for other use. The renamed methods will be removed or unexported in an upcoming commit. The aim is to have no exported methods on the ChannelGraph that accept a kvdb.RTx as a parameter. --- graph/db/graph.go | 12 ++++++------ graph/db/graph_test.go | 4 ++-- graph/graphsession/graph_session.go | 12 ++++++------ routing/integrated_routing_context_test.go | 4 ++-- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/graph/db/graph.go b/graph/db/graph.go index a7d3cf299..49c24441b 100644 --- a/graph/db/graph.go +++ b/graph/db/graph.go @@ -500,7 +500,7 @@ func (c *ChannelGraph) ForEachChannel(cb func(*models.ChannelEdgeInfo, }, func() {}) } -// ForEachNodeDirectedChannel iterates through all channels of a given node, +// ForEachNodeDirectedChannelTx 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 // is halted with the error propagated back up to the caller. An optional read @@ -509,7 +509,7 @@ func (c *ChannelGraph) ForEachChannel(cb func(*models.ChannelEdgeInfo, // Unknown policies are passed into the callback as nil values. // // NOTE: this is part of the graphsession.graph interface. -func (c *ChannelGraph) ForEachNodeDirectedChannel(tx kvdb.RTx, +func (c *ChannelGraph) ForEachNodeDirectedChannelTx(tx kvdb.RTx, node route.Vertex, cb func(channel *DirectedChannel) error) error { if c.graphCache != nil { @@ -520,7 +520,7 @@ func (c *ChannelGraph) ForEachNodeDirectedChannel(tx kvdb.RTx, toNodeCallback := func() route.Vertex { return node } - toNodeFeatures, err := c.FetchNodeFeatures(tx, node) + toNodeFeatures, err := c.FetchNodeFeaturesTx(tx, node) if err != nil { return err } @@ -564,12 +564,12 @@ func (c *ChannelGraph) ForEachNodeDirectedChannel(tx kvdb.RTx, return nodeTraversal(tx, node[:], c.db, dbCallback) } -// FetchNodeFeatures returns the features of a given node. If no features are +// FetchNodeFeaturesTx returns the features of a given node. If no features are // known for the node, an empty feature vector is returned. An optional read // transaction may be provided. If none is provided, a new one will be created. // // NOTE: this is part of the graphsession.graph interface. -func (c *ChannelGraph) FetchNodeFeatures(tx kvdb.RTx, +func (c *ChannelGraph) FetchNodeFeaturesTx(tx kvdb.RTx, node route.Vertex) (*lnwire.FeatureVector, error) { if c.graphCache != nil { @@ -623,7 +623,7 @@ func (c *ChannelGraph) ForEachNodeCached(cb func(node route.Vertex, toNodeCallback := func() route.Vertex { return node.PubKeyBytes } - toNodeFeatures, err := c.FetchNodeFeatures( + toNodeFeatures, err := c.FetchNodeFeaturesTx( tx, node.PubKeyBytes, ) if err != nil { diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index eb658e2f1..b4731fbd8 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -3915,7 +3915,7 @@ func BenchmarkForEachChannel(b *testing.B) { } } -// TestGraphCacheForEachNodeChannel tests that the ForEachNodeDirectedChannel +// TestGraphCacheForEachNodeChannel tests that the ForEachNodeDirectedChannelTx // method works as expected, and is able to handle nil self edges. func TestGraphCacheForEachNodeChannel(t *testing.T) { graph, err := MakeTestGraph(t) @@ -3952,7 +3952,7 @@ func TestGraphCacheForEachNodeChannel(t *testing.T) { getSingleChannel := func() *DirectedChannel { var ch *DirectedChannel - err = graph.ForEachNodeDirectedChannel(nil, node1.PubKeyBytes, + err = graph.ForEachNodeDirectedChannelTx(nil, node1.PubKeyBytes, func(c *DirectedChannel) error { require.Nil(t, ch) ch = c diff --git a/graph/graphsession/graph_session.go b/graph/graphsession/graph_session.go index c555e4b1a..786303d2e 100644 --- a/graph/graphsession/graph_session.go +++ b/graph/graphsession/graph_session.go @@ -86,7 +86,7 @@ func (g *session) close() error { func (g *session) ForEachNodeChannel(nodePub route.Vertex, cb func(channel *graphdb.DirectedChannel) error) error { - return g.graph.ForEachNodeDirectedChannel(g.tx, nodePub, cb) + return g.graph.ForEachNodeDirectedChannelTx(g.tx, nodePub, cb) } // FetchNodeFeatures returns the features of the given node. If the node is @@ -96,7 +96,7 @@ func (g *session) ForEachNodeChannel(nodePub route.Vertex, func (g *session) FetchNodeFeatures(nodePub route.Vertex) ( *lnwire.FeatureVector, error) { - return g.graph.FetchNodeFeatures(g.tx, nodePub) + return g.graph.FetchNodeFeaturesTx(g.tx, nodePub) } // A compile-time check to ensure that *session implements the @@ -118,7 +118,7 @@ type ReadOnlyGraph interface { // database implementation, like channeldb.ChannelGraph, in order to be used by // the Router for pathfinding. type graph interface { - // ForEachNodeDirectedChannel iterates through all channels of a given + // ForEachNodeDirectedChannelTx 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 is halted with the error propagated back @@ -128,15 +128,15 @@ type graph interface { // // NOTE: if a nil tx is provided, then it is expected that the // implementation create a read only tx. - ForEachNodeDirectedChannel(tx kvdb.RTx, node route.Vertex, + ForEachNodeDirectedChannelTx(tx kvdb.RTx, node route.Vertex, cb func(channel *graphdb.DirectedChannel) error) error - // FetchNodeFeatures returns the features of a given node. If no + // FetchNodeFeaturesTx returns the features of a given node. If no // features are known for the node, an empty feature vector is returned. // // NOTE: if a nil tx is provided, then it is expected that the // implementation create a read only tx. - FetchNodeFeatures(tx kvdb.RTx, node route.Vertex) ( + FetchNodeFeaturesTx(tx kvdb.RTx, node route.Vertex) ( *lnwire.FeatureVector, error) } diff --git a/routing/integrated_routing_context_test.go b/routing/integrated_routing_context_test.go index 5acae868d..c11b055d9 100644 --- a/routing/integrated_routing_context_test.go +++ b/routing/integrated_routing_context_test.go @@ -394,11 +394,11 @@ func (g *mockGraphSessionChanDB) close() error { func (g *mockGraphSessionChanDB) ForEachNodeChannel(nodePub route.Vertex, cb func(channel *graphdb.DirectedChannel) error) error { - return g.graph.ForEachNodeDirectedChannel(g.tx, nodePub, cb) + return g.graph.ForEachNodeDirectedChannelTx(g.tx, nodePub, cb) } func (g *mockGraphSessionChanDB) FetchNodeFeatures(nodePub route.Vertex) ( *lnwire.FeatureVector, error) { - return g.graph.FetchNodeFeatures(g.tx, nodePub) + return g.graph.FetchNodeFeaturesTx(g.tx, nodePub) } From 5d5cfe36c7bb892b3ae1d61ce8e854fe893fa963 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 13 Feb 2025 08:35:56 +0200 Subject: [PATCH 3/8] routing: rename routing Graph method In preparation for having the ChannelGraph directly implement the `routing.Graph` interface, we rename the `ForEachNodeChannel` method to `ForEachNodeDirectedChannel` since the ChannelGraph already uses the `ForEachNodeChannel` name and the new name is more appropriate since the ChannelGraph currently has a `ForEachNodeDirectedChannelTx` method which passes the same DirectedChannel type to the given call-back. --- graph/graphsession/graph_session.go | 5 +++-- routing/bandwidth.go | 2 +- routing/graph.go | 6 +++--- routing/integrated_routing_context_test.go | 2 +- routing/mock_graph_test.go | 2 +- routing/pathfind.go | 4 ++-- routing/unified_edges.go | 2 +- 7 files changed, 12 insertions(+), 11 deletions(-) diff --git a/graph/graphsession/graph_session.go b/graph/graphsession/graph_session.go index 786303d2e..31265ff56 100644 --- a/graph/graphsession/graph_session.go +++ b/graph/graphsession/graph_session.go @@ -80,10 +80,11 @@ func (g *session) close() error { return nil } -// ForEachNodeChannel calls the callback for every channel of the given node. +// ForEachNodeDirectedChannel calls the callback for every channel of the given +// node. // // NOTE: Part of the routing.Graph interface. -func (g *session) ForEachNodeChannel(nodePub route.Vertex, +func (g *session) ForEachNodeDirectedChannel(nodePub route.Vertex, cb func(channel *graphdb.DirectedChannel) error) error { return g.graph.ForEachNodeDirectedChannelTx(g.tx, nodePub, cb) diff --git a/routing/bandwidth.go b/routing/bandwidth.go index a552628c7..833782616 100644 --- a/routing/bandwidth.go +++ b/routing/bandwidth.go @@ -63,7 +63,7 @@ func newBandwidthManager(graph Graph, sourceNode route.Vertex, // First, we'll collect the set of outbound edges from the target // source node and add them to our bandwidth manager's map of channels. - err := graph.ForEachNodeChannel(sourceNode, + err := graph.ForEachNodeDirectedChannel(sourceNode, func(channel *graphdb.DirectedChannel) error { shortID := lnwire.NewShortChanIDFromInt( channel.ChannelID, diff --git a/routing/graph.go b/routing/graph.go index 7608ee92b..fa1e7eb1d 100644 --- a/routing/graph.go +++ b/routing/graph.go @@ -12,9 +12,9 @@ import ( // Graph is an abstract interface that provides information about nodes and // edges to pathfinding. type Graph interface { - // ForEachNodeChannel calls the callback for every channel of the given - // node. - ForEachNodeChannel(nodePub route.Vertex, + // ForEachNodeDirectedChannel calls the callback for every channel of + // the given node. + ForEachNodeDirectedChannel(nodePub route.Vertex, cb func(channel *graphdb.DirectedChannel) error) error // FetchNodeFeatures returns the features of the given node. diff --git a/routing/integrated_routing_context_test.go b/routing/integrated_routing_context_test.go index c11b055d9..1402ce9e4 100644 --- a/routing/integrated_routing_context_test.go +++ b/routing/integrated_routing_context_test.go @@ -391,7 +391,7 @@ func (g *mockGraphSessionChanDB) close() error { return nil } -func (g *mockGraphSessionChanDB) ForEachNodeChannel(nodePub route.Vertex, +func (g *mockGraphSessionChanDB) ForEachNodeDirectedChannel(nodePub route.Vertex, cb func(channel *graphdb.DirectedChannel) error) error { return g.graph.ForEachNodeDirectedChannelTx(g.tx, nodePub, cb) diff --git a/routing/mock_graph_test.go b/routing/mock_graph_test.go index cab7c9726..68dc1e80a 100644 --- a/routing/mock_graph_test.go +++ b/routing/mock_graph_test.go @@ -165,7 +165,7 @@ func (m *mockGraph) addChannel(id uint64, node1id, node2id byte, // forEachNodeChannel calls the callback for every channel of the given node. // // NOTE: Part of the Graph interface. -func (m *mockGraph) ForEachNodeChannel(nodePub route.Vertex, +func (m *mockGraph) ForEachNodeDirectedChannel(nodePub route.Vertex, cb func(channel *graphdb.DirectedChannel) error) error { // Look up the mock node. diff --git a/routing/pathfind.go b/routing/pathfind.go index 3f1d0ba09..c14e32e8b 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -557,7 +557,7 @@ func getOutgoingBalance(node route.Vertex, outgoingChans map[uint64]struct{}, } // Iterate over all channels of the to node. - err := g.ForEachNodeChannel(node, cb) + err := g.ForEachNodeDirectedChannel(node, cb) if err != nil { return 0, 0, err } @@ -1325,7 +1325,7 @@ func processNodeForBlindedPath(g Graph, node route.Vertex, // Now, iterate over the node's channels in search for paths to this // node that can be used for blinded paths - err = g.ForEachNodeChannel(node, + err = g.ForEachNodeDirectedChannel(node, func(channel *graphdb.DirectedChannel) error { // Keep track of how many incoming channels this node // has. We only use a node as an introduction node if it diff --git a/routing/unified_edges.go b/routing/unified_edges.go index b92010da0..a18547485 100644 --- a/routing/unified_edges.go +++ b/routing/unified_edges.go @@ -125,7 +125,7 @@ func (u *nodeEdgeUnifier) addGraphPolicies(g Graph) error { } // Iterate over all channels of the to node. - return g.ForEachNodeChannel(u.toNode, cb) + return g.ForEachNodeDirectedChannel(u.toNode, cb) } // unifiedEdge is the individual channel data that is kept inside an edgeUnifier From 8ec08fbfa434adc3415623104c497e204730a64c Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 13 Feb 2025 08:40:42 +0200 Subject: [PATCH 4/8] multi: remove the need for NewRoutingGraph The `graphsession.NewRoutingGraph` method was used to create a RoutingGraph instance with no consistent read transaction across calls. But now that the ChannelGraph directly implements this, we can remove The NewRoutingGraph method. --- graph/db/graph.go | 24 ++++++++++++++++++++++++ graph/graphsession/graph_session.go | 9 --------- rpcserver.go | 5 ++--- server.go | 2 +- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/graph/db/graph.go b/graph/db/graph.go index 49c24441b..cb53ae547 100644 --- a/graph/db/graph.go +++ b/graph/db/graph.go @@ -594,6 +594,30 @@ func (c *ChannelGraph) FetchNodeFeaturesTx(tx kvdb.RTx, } } +// 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 +// is halted with the error propagated back up to the caller. If the graphCache +// is available, then it will be used to retrieve the node's channels instead +// of the database. +// +// Unknown policies are passed into the callback as nil values. +func (c *ChannelGraph) ForEachNodeDirectedChannel(nodePub route.Vertex, + cb func(channel *DirectedChannel) error) error { + + return c.ForEachNodeDirectedChannelTx(nil, nodePub, cb) +} + +// FetchNodeFeatures returns the features of the given node. If no features are +// known for the node, an empty feature vector is returned. +// If the graphCache is available, then it will be used to retrieve the node's +// features instead of the database. +func (c *ChannelGraph) FetchNodeFeatures(nodePub route.Vertex) ( + *lnwire.FeatureVector, error) { + + return c.FetchNodeFeaturesTx(nil, nodePub) +} + // ForEachNodeCached is similar to forEachNode, but it utilizes the channel // graph cache instead. Note that this doesn't return all the information the // regular forEachNode method does. diff --git a/graph/graphsession/graph_session.go b/graph/graphsession/graph_session.go index 31265ff56..535a8beb5 100644 --- a/graph/graphsession/graph_session.go +++ b/graph/graphsession/graph_session.go @@ -56,15 +56,6 @@ type session struct { tx kvdb.RTx } -// NewRoutingGraph constructs a session that which does not first start a -// read-only transaction and so each call on the routing.Graph will create a -// new transaction. -func NewRoutingGraph(graph ReadOnlyGraph) routing.Graph { - return &session{ - graph: graph, - } -} - // close closes the read-only transaction being used to access the backing // graph. If no transaction was started then this is a no-op. func (g *session) close() error { diff --git a/rpcserver.go b/rpcserver.go index e096d31f8..2ed695ff0 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -51,7 +51,6 @@ import ( "github.com/lightningnetwork/lnd/graph" graphdb "github.com/lightningnetwork/lnd/graph/db" "github.com/lightningnetwork/lnd/graph/db/models" - "github.com/lightningnetwork/lnd/graph/graphsession" "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/htlcswitch/hop" "github.com/lightningnetwork/lnd/input" @@ -711,8 +710,8 @@ func (r *rpcServer) addDeps(s *server, macService *macaroons.Service, amount lnwire.MilliSatoshi) (btcutil.Amount, error) { return routing.FetchAmountPairCapacity( - graphsession.NewRoutingGraph(graph), - selfNode.PubKeyBytes, nodeFrom, nodeTo, amount, + graph, selfNode.PubKeyBytes, nodeFrom, nodeTo, + amount, ) }, FetchChannelEndpoints: func(chanID uint64) (route.Vertex, diff --git a/server.go b/server.go index f85d60f52..0e370d293 100644 --- a/server.go +++ b/server.go @@ -1073,7 +1073,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr, s.chanRouter, err = routing.New(routing.Config{ SelfNode: selfNode.PubKeyBytes, - RoutingGraph: graphsession.NewRoutingGraph(dbs.GraphDB), + RoutingGraph: dbs.GraphDB, Chain: cc.ChainIO, Payer: s.htlcSwitch, Control: s.controlTower, From 99c944052091e4af36f74b8b13edec16f932e512 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 13 Feb 2025 08:50:19 +0200 Subject: [PATCH 5/8] graph/db: define a NodeTraverser interface Which describes methods that will use the graph cache if it is available for fast read-only calls. --- graph/db/graph.go | 4 ++++ graph/db/interfaces.go | 14 ++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/graph/db/graph.go b/graph/db/graph.go index cb53ae547..ad9ad2ad4 100644 --- a/graph/db/graph.go +++ b/graph/db/graph.go @@ -602,6 +602,8 @@ func (c *ChannelGraph) FetchNodeFeaturesTx(tx kvdb.RTx, // of the database. // // Unknown policies are passed into the callback as nil values. +// +// NOTE: this is part of the graphdb.NodeTraverser interface. func (c *ChannelGraph) ForEachNodeDirectedChannel(nodePub route.Vertex, cb func(channel *DirectedChannel) error) error { @@ -612,6 +614,8 @@ func (c *ChannelGraph) ForEachNodeDirectedChannel(nodePub route.Vertex, // known for the node, an empty feature vector is returned. // If the graphCache is available, then it will be used to retrieve the node's // features instead of the database. +// +// NOTE: this is part of the graphdb.NodeTraverser interface. func (c *ChannelGraph) FetchNodeFeatures(nodePub route.Vertex) ( *lnwire.FeatureVector, error) { diff --git a/graph/db/interfaces.go b/graph/db/interfaces.go index f44a9ff8b..f5dce71ca 100644 --- a/graph/db/interfaces.go +++ b/graph/db/interfaces.go @@ -2,6 +2,7 @@ package graphdb import ( "github.com/lightningnetwork/lnd/graph/db/models" + "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" ) @@ -23,3 +24,16 @@ type NodeRTx interface { // the same transaction. FetchNode(node route.Vertex) (NodeRTx, error) } + +// 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. +type NodeTraverser interface { + // ForEachNodeDirectedChannel calls the callback for every channel of + // the given node. + ForEachNodeDirectedChannel(nodePub route.Vertex, + cb func(channel *DirectedChannel) error) error + + // FetchNodeFeatures returns the features of the given node. + FetchNodeFeatures(nodePub route.Vertex) (*lnwire.FeatureVector, error) +} From dfe2314a2a36d45fd72c805a27cd0340f9b0d826 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 13 Feb 2025 09:22:09 +0200 Subject: [PATCH 6/8] routing: refactor pathfinding loop In preparation for the next commit where we will start hiding underlying graph details such as that a graph session needs to be "closed" after pathfinding is done with it, we refactor things here so that the main pathfinding logic is done in a call-back. --- routing/payment_session.go | 69 +++++++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 19 deletions(-) diff --git a/routing/payment_session.go b/routing/payment_session.go index 0afdf822f..b21d40998 100644 --- a/routing/payment_session.go +++ b/routing/payment_session.go @@ -6,6 +6,7 @@ import ( "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btclog/v2" "github.com/lightningnetwork/lnd/channeldb" + graphdb "github.com/lightningnetwork/lnd/graph/db" "github.com/lightningnetwork/lnd/graph/db/models" "github.com/lightningnetwork/lnd/lnutils" "github.com/lightningnetwork/lnd/lnwire" @@ -235,6 +236,17 @@ func newPaymentSession(p *LightningPayment, selfNode route.Vertex, }, nil } +// pathFindingError is a wrapper error type that is used to distinguish path +// finding errors from other errors in path finding loop. +type pathFindingError struct { + error +} + +// Unwrap returns the underlying error. +func (e *pathFindingError) Unwrap() error { + return e.error +} + // RequestRoute returns a route which is likely to be capable for successfully // routing the specified HTLC payment to the target node. Initially the first // set of paths returned from this method may encounter routing failure along @@ -295,13 +307,8 @@ func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi, maxAmt = *p.payment.MaxShardAmt } - for { - // Get a routing graph session. - graph, closeGraph, err := p.graphSessFactory.NewGraphSession() - if err != nil { - return nil, err - } - + var path []*unifiedEdge + findPath := func(graph graphdb.NodeTraverser) error { // We'll also obtain a set of bandwidthHints from the lower // layer for each of our outbound channels. This will allow the // path finding to skip any links that aren't active or just @@ -310,19 +317,13 @@ func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi, // attempt, because concurrent payments may change balances. bandwidthHints, err := p.getBandwidthHints(graph) if err != nil { - // Close routing graph session. - if graphErr := closeGraph(); graphErr != nil { - log.Errorf("could not close graph session: %v", - graphErr) - } - - return nil, err + return err } p.log.Debugf("pathfinding for amt=%v", maxAmt) // Find a route for the current amount. - path, _, err := p.pathFinder( + path, _, err = p.pathFinder( &graphParams{ additionalEdges: p.additionalEdges, bandwidthHints: bandwidthHints, @@ -332,12 +333,42 @@ func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi, p.selfNode, p.selfNode, p.payment.Target, maxAmt, p.payment.TimePref, finalHtlcExpiry, ) - - // Close routing graph session. - if err := closeGraph(); err != nil { - log.Errorf("could not close graph session: %v", err) + if err != nil { + // Wrap the error to distinguish path finding errors + // from other errors in this closure. + return &pathFindingError{err} } + return nil + } + + for { + // Get a routing graph session. + graph, closeGraph, err := p.graphSessFactory.NewGraphSession() + if err != nil { + return nil, err + } + + err = findPath(graph) + // First, close routing graph session. + // NOTE: this will be removed in an upcoming commit. + if graphErr := closeGraph(); graphErr != nil { + log.Errorf("could not close graph session: %v", + graphErr) + } + // If there is an error, and it is not a path finding error, we + // return it immediately. + if err != nil && !lnutils.ErrorAs[*pathFindingError](err) { + return nil, err + } else if err != nil { + // If the error is a path finding error, we'll unwrap it + // to check the underlying error. + // + //nolint:errorlint + err = err.(*pathFindingError).Unwrap() + } + + // Otherwise, we'll switch on the path finding error. switch { case err == errNoPathFound: // Don't split if this is a legacy payment without mpp From e004447da60c2599f1ad50979e28728bfc601fea Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 13 Feb 2025 09:44:14 +0200 Subject: [PATCH 7/8] multi: remove the need for the graphsession package In this commit, we add a `GraphSession` method to the `ChannelGraph`. This method provides a caller with access to a `NodeTraverser`. This is used by pathfinding to create a graph "session" overwhich to perform a set of queries for a pathfinding attempt. With this refactor, we hide details such as DB transaction creation and transaction commits from the caller. So with this, pathfinding does not need to remember to "close the graph session". With this commit, the `graphsession` package may be completely removed. --- graph/db/graph.go | 68 ++++++++-- graph/graphsession/graph_session.go | 137 --------------------- routing/graph.go | 17 ++- routing/integrated_routing_context_test.go | 88 +------------ routing/integrated_routing_test.go | 2 +- routing/mock_graph_test.go | 11 ++ routing/pathfind_test.go | 36 +++--- routing/payment_session.go | 17 +-- routing/payment_session_test.go | 11 +- routing/router_test.go | 14 +-- server.go | 13 +- 11 files changed, 117 insertions(+), 297 deletions(-) delete mode 100644 graph/graphsession/graph_session.go diff --git a/graph/db/graph.go b/graph/db/graph.go index ad9ad2ad4..315651a0d 100644 --- a/graph/db/graph.go +++ b/graph/db/graph.go @@ -403,16 +403,6 @@ func initChannelGraph(db kvdb.Backend) error { return nil } -// NewPathFindTx returns a new read transaction that can be used for a single -// path finding session. Will return nil if the graph cache is enabled. -func (c *ChannelGraph) NewPathFindTx() (kvdb.RTx, error) { - if c.graphCache != nil { - return nil, nil - } - - return c.db.BeginReadTx() -} - // AddrsForNode returns all known addresses for the target node public key that // the graph DB is aware of. The returned boolean indicates if the given node is // unknown to the graph DB or not. @@ -3907,6 +3897,64 @@ func (c *ChannelGraph) IsClosedScid(scid lnwire.ShortChannelID) (bool, error) { return isClosed, nil } +// GraphSession will provide the call-back with access to a NodeTraverser +// instance which can be used to perform queries against the channel graph. If +// the graph cache is not enabled, then the call-back will be provided with +// access to the graph via a consistent read-only transaction. +func (c *ChannelGraph) GraphSession(cb func(graph NodeTraverser) error) error { + var ( + tx kvdb.RTx + err error + commit = func() {} + ) + if c.graphCache == nil { + tx, err = c.db.BeginReadTx() + if err != nil { + return err + } + + commit = func() { + if err := tx.Rollback(); err != nil { + log.Errorf("Unable to rollback tx: %v", err) + } + } + } + defer commit() + + return cb(&nodeTraverserSession{ + db: c, + tx: tx, + }) +} + +// nodeTraverserSession implements the NodeTraverser interface but with a +// backing read only transaction for a consistent view of the graph in the case +// where the graph Cache has not been enabled. +type nodeTraverserSession struct { + tx kvdb.RTx + db *ChannelGraph +} + +// ForEachNodeDirectedChannel calls the callback for every channel of the given +// node. +// +// NOTE: Part of the NodeTraverser interface. +func (c *nodeTraverserSession) ForEachNodeDirectedChannel(nodePub route.Vertex, + cb func(channel *DirectedChannel) error) error { + + return c.db.ForEachNodeDirectedChannelTx(c.tx, nodePub, cb) +} + +// FetchNodeFeatures returns the features of the given node. If the node is +// unknown, assume no additional features are supported. +// +// NOTE: Part of the NodeTraverser interface. +func (c *nodeTraverserSession) FetchNodeFeatures(nodePub route.Vertex) ( + *lnwire.FeatureVector, error) { + + return c.db.FetchNodeFeaturesTx(c.tx, nodePub) +} + func putLightningNode(nodeBucket kvdb.RwBucket, aliasBucket kvdb.RwBucket, // nolint:dupl updateIndex kvdb.RwBucket, node *models.LightningNode) error { diff --git a/graph/graphsession/graph_session.go b/graph/graphsession/graph_session.go deleted file mode 100644 index 535a8beb5..000000000 --- a/graph/graphsession/graph_session.go +++ /dev/null @@ -1,137 +0,0 @@ -package graphsession - -import ( - "fmt" - - graphdb "github.com/lightningnetwork/lnd/graph/db" - "github.com/lightningnetwork/lnd/kvdb" - "github.com/lightningnetwork/lnd/lnwire" - "github.com/lightningnetwork/lnd/routing" - "github.com/lightningnetwork/lnd/routing/route" -) - -// Factory implements the routing.GraphSessionFactory and can be used to start -// a session with a ReadOnlyGraph. -type Factory struct { - graph ReadOnlyGraph -} - -// NewGraphSessionFactory constructs a new Factory which can then be used to -// start a new session. -func NewGraphSessionFactory(graph ReadOnlyGraph) routing.GraphSessionFactory { - return &Factory{ - graph: graph, - } -} - -// NewGraphSession will produce a new Graph to use for a path-finding session. -// It returns the Graph along with a call-back that must be called once Graph -// access is complete. This call-back will close any read-only transaction that -// was created at Graph construction time. -// -// NOTE: This is part of the routing.GraphSessionFactory interface. -func (g *Factory) NewGraphSession() (routing.Graph, func() error, error) { - tx, err := g.graph.NewPathFindTx() - if err != nil { - return nil, nil, err - } - - session := &session{ - graph: g.graph, - tx: tx, - } - - return session, session.close, nil -} - -// A compile-time check to ensure that Factory implements the -// routing.GraphSessionFactory interface. -var _ routing.GraphSessionFactory = (*Factory)(nil) - -// session is an implementation of the routing.Graph interface where the same -// read-only transaction is held across calls to the graph and can be used to -// access the backing channel graph. -type session struct { - graph graph - tx kvdb.RTx -} - -// close closes the read-only transaction being used to access the backing -// graph. If no transaction was started then this is a no-op. -func (g *session) close() error { - if g.tx == nil { - return nil - } - - err := g.tx.Rollback() - if err != nil { - return fmt.Errorf("error closing db tx: %w", err) - } - - return nil -} - -// ForEachNodeDirectedChannel calls the callback for every channel of the given -// node. -// -// NOTE: Part of the routing.Graph interface. -func (g *session) ForEachNodeDirectedChannel(nodePub route.Vertex, - cb func(channel *graphdb.DirectedChannel) error) error { - - return g.graph.ForEachNodeDirectedChannelTx(g.tx, nodePub, cb) -} - -// FetchNodeFeatures returns the features of the given node. If the node is -// unknown, assume no additional features are supported. -// -// NOTE: Part of the routing.Graph interface. -func (g *session) FetchNodeFeatures(nodePub route.Vertex) ( - *lnwire.FeatureVector, error) { - - return g.graph.FetchNodeFeaturesTx(g.tx, nodePub) -} - -// A compile-time check to ensure that *session implements the -// routing.Graph interface. -var _ routing.Graph = (*session)(nil) - -// ReadOnlyGraph is a graph extended with a call to create a new read-only -// transaction that can then be used to make further queries to the graph. -type ReadOnlyGraph interface { - // NewPathFindTx returns a new read transaction that can be used for a - // single path finding session. Will return nil if the graph cache is - // enabled. - NewPathFindTx() (kvdb.RTx, error) - - graph -} - -// graph describes the API necessary for a graph source to have access to on a -// database implementation, like channeldb.ChannelGraph, in order to be used by -// the Router for pathfinding. -type graph interface { - // ForEachNodeDirectedChannelTx 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 is halted with the error propagated back - // up to the caller. - // - // Unknown policies are passed into the callback as nil values. - // - // NOTE: if a nil tx is provided, then it is expected that the - // implementation create a read only tx. - ForEachNodeDirectedChannelTx(tx kvdb.RTx, node route.Vertex, - cb func(channel *graphdb.DirectedChannel) error) error - - // FetchNodeFeaturesTx returns the features of a given node. If no - // features are known for the node, an empty feature vector is returned. - // - // NOTE: if a nil tx is provided, then it is expected that the - // implementation create a read only tx. - FetchNodeFeaturesTx(tx kvdb.RTx, node route.Vertex) ( - *lnwire.FeatureVector, error) -} - -// A compile-time check to ensure that *channeldb.ChannelGraph implements the -// graph interface. -var _ graph = (*graphdb.ChannelGraph)(nil) diff --git a/routing/graph.go b/routing/graph.go index fa1e7eb1d..115f2daf1 100644 --- a/routing/graph.go +++ b/routing/graph.go @@ -21,16 +21,15 @@ type Graph interface { FetchNodeFeatures(nodePub route.Vertex) (*lnwire.FeatureVector, error) } -// GraphSessionFactory can be used to produce a new Graph instance which can -// then be used for a path-finding session. Depending on the implementation, -// the Graph session will represent a DB connection where a read-lock is being -// held across calls to the backing Graph. +// GraphSessionFactory can be used to gain access to a graphdb.NodeTraverser +// instance which can then be used for a path-finding session. Depending on the +// implementation, the session will represent a DB connection where a read-lock +// is being held across calls to the backing graph. type GraphSessionFactory interface { - // NewGraphSession will produce a new Graph to use for a path-finding - // session. It returns the Graph along with a call-back that must be - // called once Graph access is complete. This call-back will close any - // read-only transaction that was created at Graph construction time. - NewGraphSession() (Graph, func() error, error) + // GraphSession will provide the call-back with access to a + // graphdb.NodeTraverser instance which can be used to perform queries + // against the channel graph. + GraphSession(cb func(graph graphdb.NodeTraverser) error) error } // FetchAmountPairCapacity determines the maximal public capacity between two diff --git a/routing/integrated_routing_context_test.go b/routing/integrated_routing_context_test.go index 1402ce9e4..7270b6f6d 100644 --- a/routing/integrated_routing_context_test.go +++ b/routing/integrated_routing_context_test.go @@ -8,7 +8,6 @@ import ( "time" "github.com/lightningnetwork/lnd/fn/v2" - graphdb "github.com/lightningnetwork/lnd/graph/db" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" @@ -211,7 +210,7 @@ func (c *integratedRoutingContext) testPayment(maxParts uint32, session, err := newPaymentSession( &payment, c.graph.source.pubkey, getBandwidthHints, - newMockGraphSessionFactory(c.graph), mc, c.pathFindingCfg, + c.graph, mc, c.pathFindingCfg, ) if err != nil { c.t.Fatal(err) @@ -317,88 +316,3 @@ func getNodeIndex(route *route.Route, failureSource route.Vertex) *int { } return nil } - -type mockGraphSessionFactory struct { - Graph -} - -func newMockGraphSessionFactory(graph Graph) GraphSessionFactory { - return &mockGraphSessionFactory{Graph: graph} -} - -func (m *mockGraphSessionFactory) NewGraphSession() (Graph, func() error, - error) { - - return m, func() error { - return nil - }, nil -} - -var _ GraphSessionFactory = (*mockGraphSessionFactory)(nil) -var _ Graph = (*mockGraphSessionFactory)(nil) - -type mockGraphSessionFactoryChanDB struct { - graph *graphdb.ChannelGraph -} - -func newMockGraphSessionFactoryFromChanDB( - graph *graphdb.ChannelGraph) *mockGraphSessionFactoryChanDB { - - return &mockGraphSessionFactoryChanDB{ - graph: graph, - } -} - -func (g *mockGraphSessionFactoryChanDB) NewGraphSession() (Graph, func() error, - error) { - - tx, err := g.graph.NewPathFindTx() - if err != nil { - return nil, nil, err - } - - session := &mockGraphSessionChanDB{ - graph: g.graph, - tx: tx, - } - - return session, session.close, nil -} - -var _ GraphSessionFactory = (*mockGraphSessionFactoryChanDB)(nil) - -type mockGraphSessionChanDB struct { - graph *graphdb.ChannelGraph - tx kvdb.RTx -} - -func newMockGraphSessionChanDB(graph *graphdb.ChannelGraph) Graph { - return &mockGraphSessionChanDB{ - graph: graph, - } -} - -func (g *mockGraphSessionChanDB) close() error { - if g.tx == nil { - return nil - } - - err := g.tx.Rollback() - if err != nil { - return fmt.Errorf("error closing db tx: %w", err) - } - - return nil -} - -func (g *mockGraphSessionChanDB) ForEachNodeDirectedChannel(nodePub route.Vertex, - cb func(channel *graphdb.DirectedChannel) error) error { - - return g.graph.ForEachNodeDirectedChannelTx(g.tx, nodePub, cb) -} - -func (g *mockGraphSessionChanDB) FetchNodeFeatures(nodePub route.Vertex) ( - *lnwire.FeatureVector, error) { - - return g.graph.FetchNodeFeaturesTx(g.tx, nodePub) -} diff --git a/routing/integrated_routing_test.go b/routing/integrated_routing_test.go index 4a2447b48..9636b10f7 100644 --- a/routing/integrated_routing_test.go +++ b/routing/integrated_routing_test.go @@ -404,5 +404,5 @@ func TestPaymentAddrOnlyNoSplit(t *testing.T) { // The payment should have failed since we need to split in order to // route a payment to the destination, but they don't actually support // MPP. - require.Equal(t, err.Error(), errNoPathFound.Error()) + require.ErrorIs(t, err, errNoPathFound) } diff --git a/routing/mock_graph_test.go b/routing/mock_graph_test.go index 68dc1e80a..cb5d07f3a 100644 --- a/routing/mock_graph_test.go +++ b/routing/mock_graph_test.go @@ -227,6 +227,17 @@ func (m *mockGraph) FetchNodeFeatures(nodePub route.Vertex) ( return lnwire.EmptyFeatureVector(), nil } +// GraphSession will provide the call-back with access to a +// graphdb.NodeTraverser instance which can be used to perform queries against +// the channel graph. +// +// NOTE: Part of the GraphSessionFactory interface. +func (m *mockGraph) GraphSession( + cb func(graph graphdb.NodeTraverser) error) error { + + return cb(m) +} + // htlcResult describes the resolution of an htlc. If failure is nil, the htlc // was settled. type htlcResult struct { diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index f91d95d6b..fd92ed934 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -3221,30 +3221,25 @@ func dbFindPath(graph *graphdb.ChannelGraph, return nil, err } - graphSessFactory := newMockGraphSessionFactoryFromChanDB(graph) + var route []*unifiedEdge + err = graph.GraphSession(func(graph graphdb.NodeTraverser) error { + route, _, err = findPath( + &graphParams{ + additionalEdges: additionalEdges, + bandwidthHints: bandwidthHints, + graph: graph, + }, + r, cfg, sourceNode.PubKeyBytes, source, target, amt, + timePref, finalHtlcExpiry, + ) - graphSess, closeGraphSess, err := graphSessFactory.NewGraphSession() + return err + }) if err != nil { return nil, err } - defer func() { - if err := closeGraphSess(); err != nil { - log.Errorf("Error closing graph session: %v", err) - } - }() - - route, _, err := findPath( - &graphParams{ - additionalEdges: additionalEdges, - bandwidthHints: bandwidthHints, - graph: graphSess, - }, - r, cfg, sourceNode.PubKeyBytes, source, target, amt, timePref, - finalHtlcExpiry, - ) - - return route, err + return route, nil } // dbFindBlindedPaths calls findBlindedPaths after getting a db transaction from @@ -3258,8 +3253,7 @@ func dbFindBlindedPaths(graph *graphdb.ChannelGraph, } return findBlindedPaths( - newMockGraphSessionChanDB(graph), sourceNode.PubKeyBytes, - restrictions, + graph, sourceNode.PubKeyBytes, restrictions, ) } diff --git a/routing/payment_session.go b/routing/payment_session.go index b21d40998..607c10443 100644 --- a/routing/payment_session.go +++ b/routing/payment_session.go @@ -343,19 +343,7 @@ func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi, } for { - // Get a routing graph session. - graph, closeGraph, err := p.graphSessFactory.NewGraphSession() - if err != nil { - return nil, err - } - - err = findPath(graph) - // First, close routing graph session. - // NOTE: this will be removed in an upcoming commit. - if graphErr := closeGraph(); graphErr != nil { - log.Errorf("could not close graph session: %v", - graphErr) - } + err := p.graphSessFactory.GraphSession(findPath) // If there is an error, and it is not a path finding error, we // return it immediately. if err != nil && !lnutils.ErrorAs[*pathFindingError](err) { @@ -365,7 +353,8 @@ func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi, // to check the underlying error. // //nolint:errorlint - err = err.(*pathFindingError).Unwrap() + pErr, _ := err.(*pathFindingError) + err = pErr.Unwrap() } // Otherwise, we'll switch on the path finding error. diff --git a/routing/payment_session_test.go b/routing/payment_session_test.go index 278e09044..51eeda425 100644 --- a/routing/payment_session_test.go +++ b/routing/payment_session_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + graphdb "github.com/lightningnetwork/lnd/graph/db" "github.com/lightningnetwork/lnd/graph/db/models" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" @@ -118,7 +119,7 @@ func TestUpdateAdditionalEdge(t *testing.T) { func(Graph) (bandwidthHints, error) { return &mockBandwidthHints{}, nil }, - newMockGraphSessionFactory(&sessionGraph{}), + &sessionGraph{}, &MissionControl{}, PathFindingConfig{}, ) @@ -196,7 +197,7 @@ func TestRequestRoute(t *testing.T) { func(Graph) (bandwidthHints, error) { return &mockBandwidthHints{}, nil }, - newMockGraphSessionFactory(&sessionGraph{}), + &sessionGraph{}, &MissionControl{}, PathFindingConfig{}, ) @@ -257,3 +258,9 @@ type sessionGraph struct { func (g *sessionGraph) sourceNode() route.Vertex { return route.Vertex{} } + +func (g *sessionGraph) GraphSession( + cb func(graph graphdb.NodeTraverser) error) error { + + return cb(g) +} diff --git a/routing/router_test.go b/routing/router_test.go index 543f3b000..6efc75df2 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -135,20 +135,18 @@ func createTestCtxFromGraphInstanceAssumeValid(t *testing.T, sourceNode, err := graphInstance.graph.SourceNode() require.NoError(t, err) sessionSource := &SessionSource{ - GraphSessionFactory: newMockGraphSessionFactoryFromChanDB( - graphInstance.graph, - ), - SourceNode: sourceNode, - GetLink: graphInstance.getLink, - PathFindingConfig: pathFindingConfig, - MissionControl: mc, + GraphSessionFactory: graphInstance.graph, + SourceNode: sourceNode, + GetLink: graphInstance.getLink, + PathFindingConfig: pathFindingConfig, + MissionControl: mc, } graphBuilder := newMockGraphBuilder(graphInstance.graph) router, err := New(Config{ SelfNode: sourceNode.PubKeyBytes, - RoutingGraph: newMockGraphSessionChanDB(graphInstance.graph), + RoutingGraph: graphInstance.graph, Chain: chain, Payer: &mockPaymentAttemptDispatcherOld{}, Control: makeMockControlTower(), diff --git a/server.go b/server.go index 0e370d293..ecb3eceae 100644 --- a/server.go +++ b/server.go @@ -45,7 +45,6 @@ import ( "github.com/lightningnetwork/lnd/graph" graphdb "github.com/lightningnetwork/lnd/graph/db" "github.com/lightningnetwork/lnd/graph/db/models" - "github.com/lightningnetwork/lnd/graph/graphsession" "github.com/lightningnetwork/lnd/healthcheck" "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/htlcswitch/hop" @@ -1038,13 +1037,11 @@ func newServer(cfg *Config, listenAddrs []net.Addr, return nil, fmt.Errorf("error getting source node: %w", err) } paymentSessionSource := &routing.SessionSource{ - GraphSessionFactory: graphsession.NewGraphSessionFactory( - dbs.GraphDB, - ), - SourceNode: sourceNode, - MissionControl: s.defaultMC, - GetLink: s.htlcSwitch.GetLinkByShortID, - PathFindingConfig: pathFindingConfig, + GraphSessionFactory: dbs.GraphDB, + SourceNode: sourceNode, + MissionControl: s.defaultMC, + GetLink: s.htlcSwitch.GetLinkByShortID, + PathFindingConfig: pathFindingConfig, } paymentControl := channeldb.NewPaymentControl(dbs.ChanStateDB) From f3805002ff8ff0f8f9a6e978c9eab8e996ae57dc Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 13 Feb 2025 09:47:16 +0200 Subject: [PATCH 8/8] graph/db: unexport methods that take a transaction Unexport and rename the methods that were previously used by the graphsession package. --- docs/release-notes/release-notes-0.19.0.md | 5 ++++- graph/db/graph.go | 24 +++++++++------------- graph/db/graph_test.go | 4 ++-- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index acd47780b..b664800fd 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -248,13 +248,16 @@ The underlying functionality between those two options remain the same. * Graph abstraction work: - [Abstract autopilot access](https://github.com/lightningnetwork/lnd/pull/9480) - [Abstract invoicerpc server access](https://github.com/lightningnetwork/lnd/pull/9516) + - [Refactor to hide DB transactions](https://github.com/lightningnetwork/lnd/pull/9513) + +* [Golang was updated to + `v1.22.11`](https://github.com/lightningnetwork/lnd/pull/9462). * Move funding transaction validation to the gossiper [1](https://github.com/lightningnetwork/lnd/pull/9476) [2](https://github.com/lightningnetwork/lnd/pull/9477) [3](https://github.com/lightningnetwork/lnd/pull/9478). - ## Breaking Changes ## Performance Improvements diff --git a/graph/db/graph.go b/graph/db/graph.go index 315651a0d..8ee9e86fd 100644 --- a/graph/db/graph.go +++ b/graph/db/graph.go @@ -490,16 +490,14 @@ func (c *ChannelGraph) ForEachChannel(cb func(*models.ChannelEdgeInfo, }, func() {}) } -// ForEachNodeDirectedChannelTx iterates through all channels of a given 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 // is halted with the error propagated back up to the caller. An optional read // transaction may be provided. If none is provided, a new one will be created. // // Unknown policies are passed into the callback as nil values. -// -// NOTE: this is part of the graphsession.graph interface. -func (c *ChannelGraph) ForEachNodeDirectedChannelTx(tx kvdb.RTx, +func (c *ChannelGraph) forEachNodeDirectedChannel(tx kvdb.RTx, node route.Vertex, cb func(channel *DirectedChannel) error) error { if c.graphCache != nil { @@ -510,7 +508,7 @@ func (c *ChannelGraph) ForEachNodeDirectedChannelTx(tx kvdb.RTx, toNodeCallback := func() route.Vertex { return node } - toNodeFeatures, err := c.FetchNodeFeaturesTx(tx, node) + toNodeFeatures, err := c.fetchNodeFeatures(tx, node) if err != nil { return err } @@ -554,12 +552,10 @@ func (c *ChannelGraph) ForEachNodeDirectedChannelTx(tx kvdb.RTx, return nodeTraversal(tx, node[:], c.db, dbCallback) } -// FetchNodeFeaturesTx returns the features of a given node. If no features are +// fetchNodeFeatures returns the features of a given node. If no features are // known for the node, an empty feature vector is returned. An optional read // transaction may be provided. If none is provided, a new one will be created. -// -// NOTE: this is part of the graphsession.graph interface. -func (c *ChannelGraph) FetchNodeFeaturesTx(tx kvdb.RTx, +func (c *ChannelGraph) fetchNodeFeatures(tx kvdb.RTx, node route.Vertex) (*lnwire.FeatureVector, error) { if c.graphCache != nil { @@ -597,7 +593,7 @@ func (c *ChannelGraph) FetchNodeFeaturesTx(tx kvdb.RTx, func (c *ChannelGraph) ForEachNodeDirectedChannel(nodePub route.Vertex, cb func(channel *DirectedChannel) error) error { - return c.ForEachNodeDirectedChannelTx(nil, nodePub, cb) + return c.forEachNodeDirectedChannel(nil, nodePub, cb) } // FetchNodeFeatures returns the features of the given node. If no features are @@ -609,7 +605,7 @@ func (c *ChannelGraph) ForEachNodeDirectedChannel(nodePub route.Vertex, func (c *ChannelGraph) FetchNodeFeatures(nodePub route.Vertex) ( *lnwire.FeatureVector, error) { - return c.FetchNodeFeaturesTx(nil, nodePub) + return c.fetchNodeFeatures(nil, nodePub) } // ForEachNodeCached is similar to forEachNode, but it utilizes the channel @@ -641,7 +637,7 @@ func (c *ChannelGraph) ForEachNodeCached(cb func(node route.Vertex, toNodeCallback := func() route.Vertex { return node.PubKeyBytes } - toNodeFeatures, err := c.FetchNodeFeaturesTx( + toNodeFeatures, err := c.fetchNodeFeatures( tx, node.PubKeyBytes, ) if err != nil { @@ -3942,7 +3938,7 @@ type nodeTraverserSession struct { func (c *nodeTraverserSession) ForEachNodeDirectedChannel(nodePub route.Vertex, cb func(channel *DirectedChannel) error) error { - return c.db.ForEachNodeDirectedChannelTx(c.tx, nodePub, cb) + return c.db.forEachNodeDirectedChannel(c.tx, nodePub, cb) } // FetchNodeFeatures returns the features of the given node. If the node is @@ -3952,7 +3948,7 @@ func (c *nodeTraverserSession) ForEachNodeDirectedChannel(nodePub route.Vertex, func (c *nodeTraverserSession) FetchNodeFeatures(nodePub route.Vertex) ( *lnwire.FeatureVector, error) { - return c.db.FetchNodeFeaturesTx(c.tx, nodePub) + return c.db.fetchNodeFeatures(c.tx, nodePub) } func putLightningNode(nodeBucket kvdb.RwBucket, aliasBucket kvdb.RwBucket, // nolint:dupl diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index b4731fbd8..62b9cd4e1 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -3915,7 +3915,7 @@ func BenchmarkForEachChannel(b *testing.B) { } } -// TestGraphCacheForEachNodeChannel tests that the ForEachNodeDirectedChannelTx +// TestGraphCacheForEachNodeChannel tests that the forEachNodeDirectedChannel // method works as expected, and is able to handle nil self edges. func TestGraphCacheForEachNodeChannel(t *testing.T) { graph, err := MakeTestGraph(t) @@ -3952,7 +3952,7 @@ func TestGraphCacheForEachNodeChannel(t *testing.T) { getSingleChannel := func() *DirectedChannel { var ch *DirectedChannel - err = graph.ForEachNodeDirectedChannelTx(nil, node1.PubKeyBytes, + err = graph.forEachNodeDirectedChannel(nil, node1.PubKeyBytes, func(c *DirectedChannel) error { require.Nil(t, ch) ch = c