discovery: let reject cache use gossip version in key
What the developer wrote
Authored by Elle Mouton
discovery: let reject cache use gossip version in key
Since the gossip protocols are completely disjoint, we need to treat
messages on the two protocols completely separately and should not let
rejections on one protocol affect how we treat messages on the other.
What changed, and why it matters
This change fixes how LND's gossip message handler keeps track of recently rejected messages. Previously, the reject cache only used the sender's public key and channel ID as its lookup key. Because Lightning now has two separate gossip protocols (versions 1 and 2), a rejection on one protocol could incorrectly block or affect the same message on the other protocol. The patch adds the gossip protocol version to the cache key so the two protocols are isolated. It also makes the recently-rejected check only apply to actual gossip messages. This is a correctness fix in protocol handling; it does not by itself appear to allow direct theft of funds, but it could let invalid or stale gossip propagate or persist longer than intended across protocol boundaries.
Review whether the cross-protocol cache collision could have allowed a peer to bypass rejection logic or replay rejected announcements between gossip versions. Consider adding regression tests that specifically exercise rejection isolation across lnwire.GossipVersion values, and verify that all production call sites now supply a gossip version. No immediate emergency response is indicated by the diff alone.
Security signals we found
Cross-protocol state sharing in a security-relevant cache
Cache key namespace isolation fix
Type guard added to restrict cache lookups to gossip messages only
Error comparison hardened with errors.Is
Evidence from the diff
The rejectCacheKey struct in discovery/gossiper.go gains a gossipVersion field of type lnwire.GossipVersion, and newRejectCacheKey now requires a GossipVersion argument. All call sites in handleChanAnnouncement, handleChanUpdate, and the test are updated to pass ann.GossipVersion() or upd.GossipVersion(). isRecentlyRejectedMsg now type-asserts the input to lnwire.GossipMessage and uses gMsg.GossipVersion() when building the key, returning false for non-gossip messages. The error check is also changed from err != cache.ErrElementNotFound to !errors.Is(err, cache.ErrElementNotFound). The stated intent is to keep the two disjoint gossip protocols from interfering with each other’s rejection state.
Changed components
discovery/gossiper.godiscovery/gossiper_test.goAuthenticatedGossiper reject cacheisRecentlyRejectedMsghandleChanAnnouncementhandleChanUpdateInspect captured patch +34 / −8
diff --git a/discovery/gossiper.go b/discovery/gossiper.go
index 50dd3a5..96a8c04 100644
--- a/discovery/gossiper.go
+++ b/discovery/gossiper.go
@@ -437,15 +437,19 @@ func (c *cachedNetworkMsg) Size() (uint64, error) {
// rejectCacheKey is the cache key that we'll use to track announcements we've
// recently rejected.
type rejectCacheKey struct {
- pubkey [33]byte
- chanID uint64
+ gossipVersion lnwire.GossipVersion
+ pubkey [33]byte
+ chanID uint64
}
// newRejectCacheKey returns a new cache key for the reject cache.
-func newRejectCacheKey(cid uint64, pub [33]byte) rejectCacheKey {
+func newRejectCacheKey(v lnwire.GossipVersion, cid uint64,
+ pub [33]byte) rejectCacheKey {
+
k := rejectCacheKey{
- chanID: cid,
- pubkey: pub,
+ gossipVersion: v,
+ chanID: cid,
+ pubkey: pub,
}
return k
@@ -1688,8 +1692,15 @@ func (d *AuthenticatedGossiper) PruneSyncState(peer route.Vertex) {
func (d *AuthenticatedGossiper) isRecentlyRejectedMsg(msg lnwire.Message,
peerPub [33]byte) bool {
+ // We only cache rejections for gossip messages. So if it is not
+ // a gossip message, we return false.
+ gMsg, ok := msg.(lnwire.GossipMessage)
+ if !ok {
+ return false
+ }
+
var scid uint64
- switch m := msg.(type) {
+ switch m := gMsg.(type) {
case *lnwire.ChannelUpdate1:
scid = m.ShortChannelID.ToUint64()
@@ -1700,8 +1711,11 @@ func (d *AuthenticatedGossiper) isRecentlyRejectedMsg(msg lnwire.Message,
return false
}
- _, err := d.recentRejects.Get(newRejectCacheKey(scid, peerPub))
- return err != cache.ErrElementNotFound
+ _, err := d.recentRejects.Get(newRejectCacheKey(
+ gMsg.GossipVersion(), scid, peerPub,
+ ))
+
+ return !errors.Is(err, cache.ErrElementNotFound)
}
// retransmitStaleAnns examines all outgoing channels that the source node is
@@ -2571,6 +2585,7 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(ctx context.Context,
log.Errorf(err.Error())
key := newRejectCacheKey(
+ ann.GossipVersion(),
scid.ToUint64(),
sourceToPub(nMsg.source),
)
@@ -2588,6 +2603,7 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(ctx context.Context,
log.Errorf(err.Error())
key := newRejectCacheKey(
+ ann.GossipVersion(),
scid.ToUint64(),
sourceToPub(nMsg.source),
)
@@ -2662,6 +2678,7 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(ctx context.Context,
"%v", err)
key := newRejectCacheKey(
+ ann.GossipVersion(),
scid.ToUint64(),
sourceToPub(nMsg.source),
)
@@ -2741,6 +2758,7 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(ctx context.Context,
errors.Is(err, ErrInvalidFundingOutput):
key := newRejectCacheKey(
+ ann.GossipVersion(),
scid.ToUint64(),
sourceToPub(nMsg.source),
)
@@ -2750,6 +2768,7 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(ctx context.Context,
case errors.Is(err, ErrChannelSpent):
key := newRejectCacheKey(
+ ann.GossipVersion(),
scid.ToUint64(),
sourceToPub(nMsg.source),
)
@@ -2776,6 +2795,7 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(ctx context.Context,
// edge. We won't increase the ban score for the
// remote peer.
key := newRejectCacheKey(
+ ann.GossipVersion(),
scid.ToUint64(),
sourceToPub(nMsg.source),
)
@@ -2839,6 +2859,7 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(ctx context.Context,
anns, rErr := d.processRejectedEdge(ctx, ann, proof)
if rErr != nil {
key := newRejectCacheKey(
+ ann.GossipVersion(),
scid.ToUint64(),
sourceToPub(nMsg.source),
)
@@ -2866,6 +2887,7 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(ctx context.Context,
// Otherwise, this is just a regular rejected edge.
key := newRejectCacheKey(
+ ann.GossipVersion(),
scid.ToUint64(),
sourceToPub(nMsg.source),
)
@@ -2998,6 +3020,7 @@ func (d *AuthenticatedGossiper) handleChanUpdate(ctx context.Context,
log.Errorf(err.Error())
key := newRejectCacheKey(
+ upd.GossipVersion(),
upd.ShortChannelID.ToUint64(),
sourceToPub(nMsg.source),
)
@@ -3178,6 +3201,7 @@ func (d *AuthenticatedGossiper) handleChanUpdate(ctx context.Context,
nMsg.err <- err
key := newRejectCacheKey(
+ upd.GossipVersion(),
upd.ShortChannelID.ToUint64(),
sourceToPub(nMsg.source),
)
@@ -3307,6 +3331,7 @@ func (d *AuthenticatedGossiper) handleChanUpdate(ctx context.Context,
// Since we know the stored SCID in the graph, we'll
// cache that SCID.
key := newRejectCacheKey(
+ upd.GossipVersion(),
chanInfo.ChannelID,
sourceToPub(nMsg.source),
)
diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go
index 521e089..01faf55 100644
--- a/discovery/gossiper_test.go
+++ b/discovery/gossiper_test.go
@@ -4717,6 +4717,7 @@ func TestChanAnnBanningNonChanPeer(t *testing.T) {
// Remove the scid from the reject cache.
key := newRejectCacheKey(
+ ca.GossipVersion(),
ca.ShortChannelID.ToUint64(),
sourceToPub(nodePeer2.IdentityKey()),
)
Why this scored 49/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.