What the developer wrote
Authored by Wilmer Paulino
Fix legacy SCID pruning
We relied on `position` giving us the last index we need to prune, but
this may return `None` when all known legacy SCIDs need to be pruned. In
such cases, we ended up not pruning any of the legacy SCIDs at all.
Rewritten by: Matt Corallo <git@bluematt.me>
What changed, and why it matters
This commit fixes a bug in how the Lightning node removes outdated short channel identifiers (legacy SCIDs). Previously, when every historical SCID was old enough to be removed, the code mistakenly removed none of them. This could leave stale routing identifiers in place longer than intended, potentially causing routing confusion or failed payments.
Review whether retained stale SCIDs could cause routing or channel-state issues in deployed nodes, and consider whether a follow-up migration or advisory is warranted if nodes have accumulated unpruned legacy SCIDs.
Security signals we found
Logic error in pruning boundary condition
Use of `position`/`unwrap_or` pattern that conflates 'not found' with 'none match'
State accumulation of stale routing identifiers
Potential for routing ambiguity or payment failure due to retained legacy SCIDs
Evidence from the diff
The pruning logic used Iterator::position(|retain_scid| retain_scid) to find the first SCID to keep, intending to drop everything before it. However, position returns None when no element satisfies the predicate, i.e., when all known legacy SCIDs should be pruned. The code then treated None as ‘nothing to prune’ via unwrap_or(0), so it dropped zero entries. The fix replaces position with filter(...drop_scid...).count(), directly counting how many entries should be removed. This is a correctness fix for state pruning, not a memory-safety or cryptographic bug.
Changed components
lightning/src/ln/channel.rsLegacy short_channel_id pruning routineInspect captured patch +7 / −7
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 8f74dc2..c342415 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -13233,18 +13233,18 @@ where
let end = self
.funding
.get_short_channel_id()
- .and_then(|current_scid| {
+ .map(|current_scid| {
let historical_scids = &self.context.historical_scids;
historical_scids
.iter()
.zip(historical_scids.iter().skip(1).chain(core::iter::once(¤t_scid)))
- .map(|(_, next_scid)| {
- let funding_height = block_from_scid(*next_scid);
- let retain_scid =
- funding_height + CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY - 1 > height;
- retain_scid
+ .filter(|(_, next_scid)| {
+ let funding_height = block_from_scid(**next_scid);
+ let drop_scid =
+ funding_height + CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY - 1 <= height;
+ drop_scid
})
- .position(|retain_scid| retain_scid)
+ .count()
})
.unwrap_or(0);
Why this scored 33/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.