Fix spurious `debug_assert` in UTXO gossip dedup check
What the developer wrote
Authored by Elias Rohrer
Fix spurious `debug_assert` in UTXO gossip dedup check
`check_replace_previous_entry` hit `debug_assert!(false)` when
`channel_announce` was `None` on a still-live `UtxoMessages`. The
comment claimed this was unreachable because `channel_announce` is set
under the same lock as the channel map entry. However, there is a
legitimate race:
1. A channel announcement arrives, an async UTXO lookup starts, and
`pending_channels[scid]` is set with a `Weak` to the
`UtxoMessages`.
2. The lookup resolves. `resolve_single_future` takes both
`channel_announce` and `complete` via `.take()`, but the
`Arc<Mutex<UtxoMessages>>` is still alive on the stack of
`check_resolved_futures`.
3. A duplicate announcement for the same SCID arrives during this
window. `check_replace_previous_entry` upgrades the `Weak`,
finds `channel_announce` is `None`, and hits the assert.
Replace the unconditional `debug_assert!(false)` with a targeted check
that `complete` has also been taken (confirming the future resolved),
which would catch a genuinely unexpected state where
`channel_announce` is `None` but `complete` is still pending.
Co-Authored-By: HAL 9000
What changed, and why it matters
This commit fixes a harmless but incorrect internal sanity check (a debug-only assertion) in the Lightning Dev Kit's code that handles duplicate routing announcements. The assertion could fire in a legitimate race condition even though nothing was wrong. The fix replaces the overly broad assertion with a more precise one that only complains if the program state is genuinely inconsistent. It is not a security vulnerability and cannot be exploited by an attacker.
No security action required. Treat as a normal code-quality/debugging fix. If running debug builds, update to avoid spurious assertion failures during duplicate channel announcements.
Security signals we found
debug_assert only affects debug builds, not production
Race condition is benign and already handled by falling through to non-matching behavior
No memory safety issue, no panic in release builds, no authentication bypass
Commit explicitly describes the scenario as legitimate and not a bug in logic
Evidence from the diff
In lightning/src/routing/utxo.rs, check_replace_previous_entry previously called debug_assert!(false) whenever it upgraded a live Weak
Changed components
lightning/src/routing/utxo.rsUtxoMessages::check_replace_previous_entryUTXO gossip deduplication pathInspect captured patch +12 / −8
diff --git a/lightning/src/routing/utxo.rs b/lightning/src/routing/utxo.rs
index 466b941..6b2f296 100644
--- a/lightning/src/routing/utxo.rs
+++ b/lightning/src/routing/utxo.rs
@@ -308,23 +308,27 @@ impl PendingChecks {
// This may be called with the mutex held on a different UtxoMessages
// struct, however in that case we have a global lockorder of new messages
// -> old messages, which makes this safe.
- let pending_matches = match &pending_msgs
- .unsafe_well_ordered_double_lock_self()
- .channel_announce
- {
+ let pending_state = pending_msgs.unsafe_well_ordered_double_lock_self();
+ let pending_matches = match &pending_state.channel_announce {
Some(ChannelAnnouncement::Full(pending_msg)) => {
Some(pending_msg) == full_msg
},
Some(ChannelAnnouncement::Unsigned(pending_msg)) => pending_msg == msg,
None => {
- // This shouldn't actually be reachable. We set the
- // `channel_announce` field under the same lock as setting the
- // channel map entry. Still, we can just treat it as
+ // This can be reached if `resolve_single_future` has already
+ // consumed `channel_announce` via `.take()` while the
+ // `Arc<Mutex<UtxoMessages>>` is still alive (e.g. held on
+ // the stack of `check_resolved_futures`). In that case,
+ // `complete` should also have been taken. Treat it as
// non-matching and let the new request fly.
- debug_assert!(false);
+ debug_assert!(
+ pending_state.complete.is_none(),
+ "channel_announce is None but complete is still pending"
+ );
false
},
};
+ drop(pending_state);
if pending_matches {
return Err(LightningError {
err: "Channel announcement is already being checked".to_owned(),
Why this scored 25/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.