graph/db/models: fix race condition in Node.PubKey
What the developer wrote
Authored by Elle Mouton
graph/db/models: fix race condition in Node.PubKey
The PubKey method had a race condition where concurrent calls could
all pass the nil check and race to write to the cached pubKey field.
This is a classic check-then-act race.
Remove the caching entirely to fix the race. The overhead of parsing
a public key is minimal and doesn't justify the added complexity and
race risk of caching.
What changed, and why it matters
This commit fixes a subtle multi-threading bug in how LND retrieves a node's public key. Multiple parts of the program could call the same method at the same time, and because the method cached the parsed key in a shared field, several threads could try to write that cache simultaneously. That kind of race can corrupt memory or cause crashes in Go. The fix removes the cache entirely and simply parses the key on demand, which is fast enough that caching isn't worth the risk.
Apply the patch. It is a clean, low-risk removal of a racy cache. After applying, run the package tests with the Go race detector enabled (go test -race ./graph/db/models/...) to confirm no remaining races in this code path.
Security signals we found
Data race on shared mutable state (check-then-act)
Potential memory corruption or panic due to unsynchronized concurrent writes
Fix removes caching rather than adding synchronization, indicating low cost of safe alternative
Evidence from the diff
The Node.PubKey() method in graph/db/models/node.go previously used a check-then-act pattern: it checked whether n.pubKey was nil, and if so parsed and stored the result. Under concurrent callers this is a data race on the unguarded pubKey field. The patch removes the pubKey field and the lazy-cache logic, returning a freshly parsed public key each call. This eliminates the race with minimal performance cost because secp256k1 public-key parsing is cheap.
Changed components
graph/db/models/node.goNode.PubKey() methodInspect captured patch +1 / −15
diff --git a/graph/db/models/node.go b/graph/db/models/node.go
index 8bbd837..9a045d5 100644
--- a/graph/db/models/node.go
+++ b/graph/db/models/node.go
@@ -22,7 +22,6 @@ type Node struct {
// PubKeyBytes is the raw bytes of the public key of the target node.
PubKeyBytes [33]byte
- pubKey *btcec.PublicKey
// LastUpdate is the last time the vertex information for this node has
// been updated.
@@ -129,21 +128,8 @@ func (n *Node) HaveAnnouncement() bool {
// PubKey is the node's long-term identity public key. This key will be used to
// authenticated any advertisements/updates sent by the node.
-//
-// NOTE: By having this method to access an attribute, we ensure we only need
-// to fully deserialize the pubkey if absolutely necessary.
func (n *Node) PubKey() (*btcec.PublicKey, error) {
- if n.pubKey != nil {
- return n.pubKey, nil
- }
-
- key, err := btcec.ParsePubKey(n.PubKeyBytes[:])
- if err != nil {
- return nil, err
- }
- n.pubKey = key
-
- return key, nil
+ return btcec.ParsePubKey(n.PubKeyBytes[:])
}
// NodeAnnouncement retrieves the latest node announcement of the node.
Why this scored 37/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.