graph/db/models: fix race conditions in ChannelEdgeInfo
What the developer wrote
Authored by Elle Mouton
graph/db/models: fix race conditions in ChannelEdgeInfo
Both NodeKey1 and NodeKey2 methods had the same race condition as the
Node.PubKey method, where concurrent calls could race to write to the
cached fields.
Remove the caching for the same reasons: parsing overhead is minimal
and doesn't justify the complexity and race risk.
What changed, and why it matters
This commit fixes a race condition in LND's graph database model. Two methods that return public keys for channel partners were caching parsed keys in struct fields without any synchronization. If multiple goroutines called these methods at the same time, they could write to the same memory field simultaneously, leading to undefined behavior or crashes. The fix removes the caching entirely and simply parses the key each time, because parsing is cheap and the cache wasn't worth the risk.
Apply the patch. After applying, run the Go race detector (go test -race) on graph/db/models tests and any code paths that concurrently access ChannelEdgeInfo. Consider auditing other lazy-cached accessors in the codebase for similar patterns.
Security signals we found
Data race on shared mutable struct fields
Unsynchronized lazy initialization (read-modify-write)
Fix removes caching rather than adding locks, citing low parsing overhead
Reference to prior similar fix in Node.PubKey suggests pattern of race-prone caching
Evidence from the diff
ChannelEdgeInfo.NodeKey1() and NodeKey2() previously lazily cached parsed *btcec.PublicKey values in unexported struct fields (nodeKey1, nodeKey2). With concurrent callers, multiple goroutines could execute the read-check-write sequence on these shared fields without synchronization, creating a data race. The patch removes the cached fields and the lazy initialization logic, returning a freshly parsed public key on every call. This mirrors a prior fix for Node.PubKey mentioned in the commit message.
Changed components
graph/db/models/channel_edge_info.goChannelEdgeInfo.NodeKey1()ChannelEdgeInfo.NodeKey2()Inspect captured patch +2 / −30
diff --git a/graph/db/models/channel_edge_info.go b/graph/db/models/channel_edge_info.go
index b86c140..0d6cd04 100644
--- a/graph/db/models/channel_edge_info.go
+++ b/graph/db/models/channel_edge_info.go
@@ -30,11 +30,9 @@ type ChannelEdgeInfo struct {
// NodeKey1Bytes is the raw public key of the first node.
NodeKey1Bytes [33]byte
- nodeKey1 *btcec.PublicKey
// NodeKey2Bytes is the raw public key of the first node.
NodeKey2Bytes [33]byte
- nodeKey2 *btcec.PublicKey
// BitcoinKey1Bytes is the raw public key of the first node.
BitcoinKey1Bytes [33]byte
@@ -78,42 +76,16 @@ type ChannelEdgeInfo struct {
// the creation of this channel. A node is considered "first" if the
// lexicographical ordering the its serialized public key is "smaller" than
// that of the other node involved in channel creation.
-//
-// NOTE: By having this method to access an attribute, we ensure we only need
-// to fully deserialize the pubkey if absolutely necessary.
func (c *ChannelEdgeInfo) NodeKey1() (*btcec.PublicKey, error) {
- if c.nodeKey1 != nil {
- return c.nodeKey1, nil
- }
-
- key, err := btcec.ParsePubKey(c.NodeKey1Bytes[:])
- if err != nil {
- return nil, err
- }
- c.nodeKey1 = key
-
- return key, nil
+ return btcec.ParsePubKey(c.NodeKey1Bytes[:])
}
// NodeKey2 is the identity public key of the "second" node that was involved in
// the creation of this channel. A node is considered "second" if the
// lexicographical ordering the its serialized public key is "larger" than that
// of the other node involved in channel creation.
-//
-// NOTE: By having this method to access an attribute, we ensure we only need
-// to fully deserialize the pubkey if absolutely necessary.
func (c *ChannelEdgeInfo) NodeKey2() (*btcec.PublicKey, error) {
- if c.nodeKey2 != nil {
- return c.nodeKey2, nil
- }
-
- key, err := btcec.ParsePubKey(c.NodeKey2Bytes[:])
- if err != nil {
- return nil, err
- }
- c.nodeKey2 = key
-
- return key, nil
+ return btcec.ParsePubKey(c.NodeKey2Bytes[:])
}
// OtherNodeKeyBytes returns the node key bytes of the other end of the channel.
Why this scored 44/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.