Public commit record
68/100 · Adequate
What the developer wrote
Authored by Elle Mouton
graph/db: move gossip versions up one layer
Update some of the node related graph CRUD methods to take a version
rather than hardcoding them in the SQLStore layer. Move the version up
one layer instead. This will make it easier to make it configurable
later on.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit is a straightforward internal code refactor in LND's channel graph database. It moves the choice of which gossip protocol version to use from deep inside the database layer up to the caller layer, but still hardcodes version 1 everywhere. There is no security fix or vulnerability here; it is preparation for possible future configurability.
Recommended action
No security action required. Treat as normal maintenance/refactoring. Reviewers may verify that all call sites pass GossipVersion1 and that KVStore's new non-V1 error paths cannot be triggered by untrusted input.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change updates the graph/db Store interface and its KVStore/SQLStore implementations so that methods such as FetchNode, HasNode, DeleteNode, AddrsForNode, LookupAlias, SourceNode, and FetchNodeFeatures accept a lnwire.GossipVersion parameter. The higher-level ChannelGraph wrapper passes lnwire.GossipVersion1 explicitly. KVStore rejects non-V1 versions with a new ErrVersionNotSupportedForKVDB error, while SQLStore now uses the passed version in its queries instead of an internal hardcoded constant. The commit message explicitly frames this as making the version ‘configurable later on’.
Changed components
graph/db/graph.gograph/db/interfaces.gograph/db/kv_store.gograph/db/sql_store.gograph/db/graph_test.goInspect captured patch +111 / −53
diff --git a/graph/db/graph.go b/graph/db/graph.go
index e9bd4aa..47df50e 100644
--- a/graph/db/graph.go
+++ b/graph/db/graph.go
@@ -226,7 +226,7 @@ func (c *ChannelGraph) FetchNodeFeatures(node route.Vertex) (
return c.graphCache.GetFeatures(node), nil
}
- return c.db.FetchNodeFeatures(node)
+ return c.db.FetchNodeFeatures(lnwire.GossipVersion1, node)
}
// GraphSession will provide the call-back with access to a NodeTraverser
@@ -298,7 +298,7 @@ func (c *ChannelGraph) AddNode(ctx context.Context,
func (c *ChannelGraph) DeleteNode(ctx context.Context,
nodePub route.Vertex) error {
- err := c.db.DeleteNode(ctx, nodePub)
+ err := c.db.DeleteNode(ctx, lnwire.GossipVersion1, nodePub)
if err != nil {
return err
}
@@ -607,7 +607,7 @@ func (c *ChannelGraph) UpdateEdgePolicy(ctx context.Context,
func (c *ChannelGraph) AddrsForNode(ctx context.Context,
nodePub *btcec.PublicKey) (bool, []net.Addr, error) {
- return c.db.AddrsForNode(ctx, nodePub)
+ return c.db.AddrsForNode(ctx, lnwire.GossipVersion1, nodePub)
}
// ForEachSourceNodeChannel iterates through all channels of the source node.
@@ -646,7 +646,7 @@ func (c *ChannelGraph) ForEachNodeCacheable(ctx context.Context,
func (c *ChannelGraph) LookupAlias(ctx context.Context,
pub *btcec.PublicKey) (string, error) {
- return c.db.LookupAlias(ctx, pub)
+ return c.db.LookupAlias(ctx, lnwire.GossipVersion1, pub)
}
// NodeUpdatesInHorizon returns all known lightning nodes with updates in the
@@ -661,7 +661,7 @@ func (c *ChannelGraph) NodeUpdatesInHorizon(startTime, endTime time.Time,
func (c *ChannelGraph) FetchNode(ctx context.Context,
nodePub route.Vertex) (*models.Node, error) {
- return c.db.FetchNode(ctx, nodePub)
+ return c.db.FetchNode(ctx, lnwire.GossipVersion1, nodePub)
}
// HasV1Node determines if the graph has a vertex identified by the target node
@@ -677,7 +677,7 @@ func (c *ChannelGraph) HasV1Node(ctx context.Context,
func (c *ChannelGraph) HasNode(ctx context.Context, nodePub [33]byte) (bool,
error) {
- return c.db.HasNode(ctx, nodePub)
+ return c.db.HasNode(ctx, lnwire.GossipVersion1, nodePub)
}
// IsPublicNode determines whether the node is seen as public in the graph.
@@ -796,7 +796,7 @@ func (c *ChannelGraph) IsClosedScid(scid lnwire.ShortChannelID) (bool, error) {
// SourceNode returns the source node of the graph.
func (c *ChannelGraph) SourceNode(ctx context.Context) (*models.Node, error) {
- return c.db.SourceNode(ctx)
+ return c.db.SourceNode(ctx, lnwire.GossipVersion1)
}
// SetSourceNode sets the source node within the graph database.
diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go
index c147b09..16b0879 100644
--- a/graph/db/graph_test.go
+++ b/graph/db/graph_test.go
@@ -153,7 +153,9 @@ func TestNodeInsertionAndDeletion(t *testing.T) {
// Check that the node's features are fetched correctly. This check
// will check the database directly.
- features, err = graph.db.FetchNodeFeatures(node.PubKeyBytes)
+ features, err = graph.db.FetchNodeFeatures(
+ lnwire.GossipVersion1, node.PubKeyBytes,
+ )
require.NoError(t, err)
require.Equal(t, testFeatures, features)
diff --git a/graph/db/interfaces.go b/graph/db/interfaces.go
index 5917283..b6a8c10 100644
--- a/graph/db/interfaces.go
+++ b/graph/db/interfaces.go
@@ -31,7 +31,14 @@ type NodeTraverser interface {
// Store represents the main interface for the channel graph database for all
// channels and nodes gossiped via the V1 gossip protocol as defined in BOLT 7.
type Store interface { //nolint:interfacebloat
- NodeTraverser
+ // ForEachNodeDirectedChannel calls the callback for every channel of
+ // the given node.
+ ForEachNodeDirectedChannel(nodePub route.Vertex,
+ cb func(channel *DirectedChannel) error, reset func()) error
+
+ // FetchNodeFeatures returns the features of the given node.
+ FetchNodeFeatures(v lnwire.GossipVersion,
+ nodePub route.Vertex) (*lnwire.FeatureVector, error)
// AddNode adds a vertex/node to the graph database. If the
// node is not in the database from before, this will add a new,
@@ -45,7 +52,7 @@ type Store interface { //nolint:interfacebloat
// 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.
- AddrsForNode(ctx context.Context,
+ AddrsForNode(ctx context.Context, v lnwire.GossipVersion,
nodePub *btcec.PublicKey) (bool, []net.Addr, error)
// ForEachSourceNodeChannel iterates through all channels of the source
@@ -100,11 +107,13 @@ type Store interface { //nolint:interfacebloat
// LookupAlias attempts to return the alias as advertised by the target
// node.
- LookupAlias(ctx context.Context, pub *btcec.PublicKey) (string, error)
+ LookupAlias(ctx context.Context, v lnwire.GossipVersion,
+ pub *btcec.PublicKey) (string, error)
// DeleteNode starts a new database transaction to remove a
// vertex/node from the database according to the node's public key.
- DeleteNode(ctx context.Context, nodePub route.Vertex) error
+ DeleteNode(ctx context.Context, v lnwire.GossipVersion,
+ nodePub route.Vertex) error
// NodeUpdatesInHorizon returns all the known lightning node which have
// an update timestamp within the passed range. This method can be used
@@ -116,8 +125,8 @@ type Store interface { //nolint:interfacebloat
// FetchNode attempts to look up a target node by its identity
// public key. If the node isn't found in the database, then
// ErrGraphNodeNotFound is returned.
- FetchNode(ctx context.Context, nodePub route.Vertex) (*models.Node,
- error)
+ FetchNode(ctx context.Context, v lnwire.GossipVersion,
+ nodePub route.Vertex) (*models.Node, error)
// HasV1Node determines if the graph has a vertex identified by
// the target node identity public key in the V1 graph. If the node
@@ -131,7 +140,8 @@ type Store interface { //nolint:interfacebloat
// HasNode determines if the graph has a vertex identified by
// the target node identity public key.
- HasNode(ctx context.Context, nodePub [33]byte) (bool, error)
+ HasNode(ctx context.Context, v lnwire.GossipVersion,
+ nodePub [33]byte) (bool, error)
// IsPublicNode is a helper method that determines whether the node with
// the given public key is seen as a public node in the graph from the
@@ -334,7 +344,8 @@ type Store interface { //nolint:interfacebloat
// treated as the center node within a star-graph. This method may be
// used to kick off a path finding algorithm in order to explore the
// reachability of another node based off the source node.
- SourceNode(ctx context.Context) (*models.Node, error)
+ SourceNode(ctx context.Context, v lnwire.GossipVersion) (*models.Node,
+ error)
// SetSourceNode sets the source node within the graph database. The
// source node is to be used as the center of a star-graph within path
diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go
index a582fd5..021f4a3 100644
--- a/graph/db/kv_store.go
+++ b/graph/db/kv_store.go
@@ -164,6 +164,11 @@ var (
//
// maps: scid -> []byte{}
closedScidBucket = []byte("closed-scid")
+
+ // ErrVersionNotSupportedForKVDB is returned with KVStore queries are
+ // made using a gossip version other than V1.
+ ErrVersionNotSupportedForKVDB = errors.New("only gossip v1 is " +
+ "supported for kvdb graph store")
)
const (
@@ -376,7 +381,7 @@ func initKVStore(db kvdb.Backend) error {
// unknown to the graph DB or not.
//
// NOTE: this is part of the channeldb.AddrSource interface.
-func (c *KVStore) AddrsForNode(ctx context.Context,
+func (c *KVStore) AddrsForNode(ctx context.Context, v lnwire.GossipVersion,
nodePub *btcec.PublicKey) (bool, []net.Addr, error) {
pubKey, err := route.NewVertexFromBytes(nodePub.SerializeCompressed())
@@ -384,7 +389,7 @@ func (c *KVStore) AddrsForNode(ctx context.Context,
return false, nil, err
}
- node, err := c.FetchNode(ctx, pubKey)
+ node, err := c.FetchNode(ctx, v, pubKey)
// We don't consider it an error if the graph is unaware of the node.
switch {
case err != nil && !errors.Is(err, ErrGraphNodeNotFound):
@@ -666,8 +671,12 @@ func (c *KVStore) ForEachNodeDirectedChannel(nodePub route.Vertex,
// known for the node, an empty feature vector is returned.
//
// NOTE: this is part of the graphdb.NodeTraverser interface.
-func (c *KVStore) FetchNodeFeatures(nodePub route.Vertex) (
- *lnwire.FeatureVector, error) {
+func (c *KVStore) FetchNodeFeatures(v lnwire.GossipVersion,
+ nodePub route.Vertex) (*lnwire.FeatureVector, error) {
+
+ if v != lnwire.GossipVersion1 {
+ return nil, ErrVersionNotSupportedForKVDB
+ }
return c.fetchNodeFeatures(nil, nodePub)
}
@@ -901,7 +910,13 @@ func (c *KVStore) ForEachNodeCacheable(_ context.Context,
// as the center node within a star-graph. This method may be used to kick off
// a path finding algorithm in order to explore the reachability of another
// node based off the source node.
-func (c *KVStore) SourceNode(_ context.Context) (*models.Node, error) {
+func (c *KVStore) SourceNode(_ context.Context,
+ v lnwire.GossipVersion) (*models.Node, error) {
+
+ if v != lnwire.GossipVersion1 {
+ return nil, ErrVersionNotSupportedForKVDB
+ }
+
return sourceNode(c.db)
}
@@ -955,6 +970,10 @@ func sourceNodeWithTx(nodes kvdb.RBucket) (*models.Node, error) {
func (c *KVStore) SetSourceNode(_ context.Context,
node *models.Node) error {
+ if node.Version != lnwire.GossipVersion1 {
+ return ErrVersionNotSupportedForKVDB
+ }
+
nodePubBytes := node.PubKeyBytes[:]
return kvdb.Update(c.db, func(tx kvdb.RwTx) error {
@@ -1021,9 +1040,13 @@ func addLightningNode(tx kvdb.RwTx, node *models.Node) error {
// LookupAlias attempts to return the alias as advertised by the target node.
// TODO(roasbeef): currently assumes that aliases are unique...
-func (c *KVStore) LookupAlias(_ context.Context,
+func (c *KVStore) LookupAlias(_ context.Context, v lnwire.GossipVersion,
pub *btcec.PublicKey) (string, error) {
+ if v != lnwire.GossipVersion1 {
+ return "", ErrVersionNotSupportedForKVDB
+ }
+
var alias string
err := kvdb.View(c.db, func(tx kvdb.RTx) error {
@@ -1060,9 +1083,13 @@ func (c *KVStore) LookupAlias(_ context.Context,
// DeleteNode starts a new database transaction to remove a vertex/node
// from the database according to the node's public key.
-func (c *KVStore) DeleteNode(_ context.Context,
+func (c *KVStore) DeleteNode(_ context.Context, v lnwire.GossipVersion,
nodePub route.Vertex) error {
+ if v != lnwire.GossipVersion1 {
+ return ErrVersionNotSupportedForKVDB
+ }
+
// TODO(roasbeef): ensure dangling edges are removed...
return kvdb.Update(c.db, func(tx kvdb.RwTx) error {
nodes := tx.ReadWriteBucket(nodeBucket)
@@ -3370,9 +3397,13 @@ func (c *KVStore) fetchNodeTx(tx kvdb.RTx, nodePub route.Vertex) (*models.Node,
// FetchNode attempts to look up a target node by its identity public
// key. If the node isn't found in the database, then ErrGraphNodeNotFound is
// returned.
-func (c *KVStore) FetchNode(_ context.Context,
+func (c *KVStore) FetchNode(_ context.Context, v lnwire.GossipVersion,
nodePub route.Vertex) (*models.Node, error) {
+ if v != lnwire.GossipVersion1 {
+ return nil, ErrVersionNotSupportedForKVDB
+ }
+
return c.fetchLightningNode(nil, nodePub)
}
@@ -3487,7 +3518,13 @@ func (c *KVStore) HasV1Node(_ context.Context,
// HasNode determines if the graph has a vertex identified by the target node
// identity public key.
-func (c *KVStore) HasNode(_ context.Context, nodePub [33]byte) (bool, error) {
+func (c *KVStore) HasNode(_ context.Context, v lnwire.GossipVersion,
+ nodePub [33]byte) (bool, error) {
+
+ if v != lnwire.GossipVersion1 {
+ return false, ErrVersionNotSupportedForKVDB
+ }
+
var exists bool
err := kvdb.View(c.db, func(tx kvdb.RTx) error {
// First grab the nodes bucket which stores the mapping from
@@ -4366,6 +4403,10 @@ func (c *nodeTraverserSession) FetchNodeFeatures(nodePub route.Vertex) (
func putLightningNode(nodeBucket, aliasBucket, updateIndex kvdb.RwBucket,
node *models.Node) error {
+ if node.Version != lnwire.GossipVersion1 {
+ return ErrVersionNotSupportedForKVDB
+ }
+
var (
scratch [16]byte
b bytes.Buffer
diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go
index d9fca51..0df0985 100644
--- a/graph/db/sql_store.go
+++ b/graph/db/sql_store.go
@@ -267,13 +267,15 @@ func (s *SQLStore) AddNode(ctx context.Context,
// returned.
//
// NOTE: part of the Store interface.
-func (s *SQLStore) FetchNode(ctx context.Context,
+func (s *SQLStore) FetchNode(ctx context.Context, v lnwire.GossipVersion,
pubKey route.Vertex) (*models.Node, error) {
var node *models.Node
err := s.db.ExecTx(ctx, sqldb.ReadTxOpt(), func(db SQLQueries) error {
var err error
- _, node, err = getNodeByPubKey(ctx, s.cfg.QueryCfg, db, pubKey)
+ _, node, err = getNodeByPubKey(
+ ctx, s.cfg.QueryCfg, db, v, pubKey,
+ )
return err
}, sqldb.NoOpReset)
@@ -331,11 +333,10 @@ func (s *SQLStore) HasV1Node(ctx context.Context,
// target node identity public key.
//
// NOTE: part of the Store interface.
-func (s *SQLStore) HasNode(ctx context.Context, pubKey [33]byte) (bool, error) {
- var (
- v = lnwire.GossipVersion1
- exists bool
- )
+func (s *SQLStore) HasNode(ctx context.Context, v lnwire.GossipVersion,
+ pubKey [33]byte) (bool, error) {
+
+ var exists bool
err := s.db.ExecTx(ctx, sqldb.ReadTxOpt(), func(db SQLQueries) error {
var err error
exists, err = db.NodeExists(ctx, sqlc.NodeExistsParams{
@@ -358,7 +359,7 @@ func (s *SQLStore) HasNode(ctx context.Context, pubKey [33]byte) (bool, error) {
// given node is unknown to the graph DB or not.
//
// NOTE: part of the Store interface.
-func (s *SQLStore) AddrsForNode(ctx context.Context,
+func (s *SQLStore) AddrsForNode(ctx context.Context, v lnwire.GossipVersion,
nodePub *btcec.PublicKey) (bool, []net.Addr, error) {
var (
@@ -370,7 +371,7 @@ func (s *SQLStore) AddrsForNode(ctx context.Context,
// does.
dbID, err := db.GetNodeIDByPubKey(
ctx, sqlc.GetNodeIDByPubKeyParams{
- Version: int16(lnwire.GossipVersion1),
+ Version: int16(v),
PubKey: nodePub.SerializeCompressed(),
},
)
@@ -400,13 +401,13 @@ func (s *SQLStore) AddrsForNode(ctx context.Context,
// from the database according to the node's public key.
//
// NOTE: part of the Store interface.
-func (s *SQLStore) DeleteNode(ctx context.Context,
+func (s *SQLStore) DeleteNode(ctx context.Context, v lnwire.GossipVersion,
pubKey route.Vertex) error {
err := s.db.ExecTx(ctx, sqldb.WriteTxOpt(), func(db SQLQueries) error {
res, err := db.DeleteNodeByPubKey(
ctx, sqlc.DeleteNodeByPubKeyParams{
- Version: int16(lnwire.GossipVersion1),
+ Version: int16(v),
PubKey: pubKey[:],
},
)
@@ -438,12 +439,12 @@ func (s *SQLStore) DeleteNode(ctx context.Context,
// known for the node, an empty feature vector is returned.
//
// NOTE: this is part of the graphdb.NodeTraverser interface.
-func (s *SQLStore) FetchNodeFeatures(nodePub route.Vertex) (
- *lnwire.FeatureVector, error) {
+func (s *SQLStore) FetchNodeFeatures(v lnwire.GossipVersion,
+ nodePub route.Vertex) (*lnwire.FeatureVector, error) {
ctx := context.TODO()
- return fetchNodeFeatures(ctx, s.db, nodePub)
+ return fetchNodeFeatures(ctx, s.db, v, nodePub)
}
// DisabledChannelIDs returns the channel ids of disabled channels.
@@ -478,14 +479,14 @@ func (s *SQLStore) DisabledChannelIDs() ([]uint64, error) {
// LookupAlias attempts to return the alias as advertised by the target node.
//
// NOTE: part of the Store interface.
-func (s *SQLStore) LookupAlias(ctx context.Context,
+func (s *SQLStore) LookupAlias(ctx context.Context, v lnwire.GossipVersion,
pub *btcec.PublicKey) (string, error) {
var alias string
err := s.db.ExecTx(ctx, sqldb.ReadTxOpt(), func(db SQLQueries) error {
dbNode, err := db.GetNodeByPubKey(
ctx, sqlc.GetNodeByPubKeyParams{
- Version: int16(lnwire.GossipVersion1),
+ Version: int16(v),
PubKey: pub.SerializeCompressed(),
},
)
@@ -516,20 +517,20 @@ func (s *SQLStore) LookupAlias(ctx context.Context,
// node based off the source node.
//
// NOTE: part of the Store interface.
-func (s *SQLStore) SourceNode(ctx context.Context) (*models.Node,
- error) {
+func (s *SQLStore) SourceNode(ctx context.Context,
+ v lnwire.GossipVersion) (*models.Node, error) {
var node *models.Node
err := s.db.ExecTx(ctx, sqldb.ReadTxOpt(), func(db SQLQueries) error {
- _, nodePub, err := s.getSourceNode(
- ctx, db, lnwire.GossipVersion1,
- )
+ _, nodePub, err := s.getSourceNode(ctx, db, v)
if err != nil {
return fmt.Errorf("unable to fetch V1 source node: %w",
err)
}
- _, node, err = getNodeByPubKey(ctx, s.cfg.QueryCfg, db, nodePub)
+ _, node, err = getNodeByPubKey(
+ ctx, s.cfg.QueryCfg, db, v, nodePub,
+ )
return err
}, sqldb.NoOpReset)
@@ -919,7 +920,8 @@ func (s *SQLStore) ForEachSourceNodeChannel(ctx context.Context,
}
_, otherNode, err := getNodeByPubKey(
- ctx, s.cfg.QueryCfg, db, otherNodePub,
+ ctx, s.cfg.QueryCfg, db,
+ lnwire.GossipVersion1, otherNodePub,
)
if err != nil {
return fmt.Errorf("unable to fetch "+
@@ -3104,7 +3106,7 @@ func (s *sqlNodeTraverser) FetchNodeFeatures(nodePub route.Vertex) (
ctx := context.TODO()
- return fetchNodeFeatures(ctx, s.db, nodePub)
+ return fetchNodeFeatures(ctx, s.db, lnwire.GossipVersion1, nodePub)
}
// forEachNodeDirectedChannel iterates through all channels of a given
@@ -3466,11 +3468,12 @@ func updateChanEdgePolicy(ctx context.Context, tx SQLQueries,
// getNodeByPubKey attempts to look up a target node by its public key.
func getNodeByPubKey(ctx context.Context, cfg *sqldb.QueryConfig, db SQLQueries,
- pubKey route.Vertex) (int64, *models.Node, error) {
+ v lnwire.GossipVersion, pubKey route.Vertex) (int64, *models.Node,
+ error) {
dbNode, err := db.GetNodeByPubKey(
ctx, sqlc.GetNodeByPubKeyParams{
- Version: int16(lnwire.GossipVersion1),
+ Version: int16(v),
PubKey: pubKey[:],
},
)
@@ -3926,12 +3929,13 @@ func upsertNodeFeatures(ctx context.Context, db SQLQueries, nodeID int64,
// fetchNodeFeatures fetches the features for a node with the given public key.
func fetchNodeFeatures(ctx context.Context, queries SQLQueries,
- nodePub route.Vertex) (*lnwire.FeatureVector, error) {
+ v lnwire.GossipVersion, nodePub route.Vertex) (*lnwire.FeatureVector,
+ error) {
rows, err := queries.GetNodeFeaturesByPubKey(
ctx, sqlc.GetNodeFeaturesByPubKeyParams{
PubKey: nodePub[:],
- Version: int16(lnwire.GossipVersion1),
+ Version: int16(v),
},
)
if err != nil {
Risk score
Our methodology →Why this scored 13/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.