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.

Author: ellemouton

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 {
 

graph/graphsession/graph_session.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

routing/graph.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)
-}

routing/integrated_routing_context_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)
 }

routing/integrated_routing_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 {