Prune HTLC sources from pending funding scopes
What the developer wrote
Authored by Wilmer Paulino
Prune HTLC sources from pending funding scopes
We only need to track the HTLC sources for the previous and current
counterparty commitments.
What changed, and why it matters
This change updates how the Lightning node cleans up old payment source information when a new commitment transaction is received. Previously, only the active funding scope was pruned; now pending funding scopes are also pruned. The commit message frames this as a cleanup to avoid tracking unnecessary data. There is no direct evidence in the commit that this fixes an exploitable vulnerability, but incomplete cleanup could theoretically lead to stale state or incorrect event generation in multi-funding-scope channels.
Review whether failure to prune pending funding scopes could cause duplicate or incorrect HTLC fulfill/fail events, especially in splicing or dual-funding scenarios. Consider adding tests covering pending funding scope transitions. No immediate emergency action is indicated by the diff alone.
Security signals we found
State cleanup extended to pending funding scopes
Potential for stale HTLC source data in multi-funding channels
Shared mutable state guarded by boolean flag across iterations
No explicit security framing in commit message
Evidence from the diff
The patch refactors HTLC source pruning in ChannelMonitorImpl. It introduces a closure prune_htlc_sources that operates on any FundingScope, and applies it to both self.funding and all entries in self.pending_funding. It also adds a guard removed_fulfilled_htlcs to ensure the shared counterparty_fulfilled_htlcs set is only modified once across all scopes. The change ensures HTLC sources are cleared from previous counterparty commitments in pending funding scopes, not just the current one.
Changed components
lightning/src/chain/channelmonitor.rsChannelMonitorImplFundingScope HTLC source trackingcounterparty_claimable_outpointscounterparty_fulfilled_htlcsInspect captured patch +25 / −16
diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index 863439b..9ff58ab 100644
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -3399,26 +3399,35 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
// Prune HTLCs from the previous counterparty commitment tx so we don't generate failure/fulfill
// events for now-revoked/fulfilled HTLCs.
- if let Some(txid) = self.funding.prev_counterparty_commitment_txid.take() {
- if self.funding.current_counterparty_commitment_txid.unwrap() != txid {
- let cur_claimables = self.funding.counterparty_claimable_outpoints.get(
- &self.funding.current_counterparty_commitment_txid.unwrap()).unwrap();
- for (_, ref source_opt) in self.funding.counterparty_claimable_outpoints.get(&txid).unwrap() {
- if let Some(source) = source_opt {
- if !cur_claimables.iter()
- .any(|(_, cur_source_opt)| cur_source_opt == source_opt)
- {
- self.counterparty_fulfilled_htlcs.remove(&SentHTLCId::from_source(source));
+ let mut removed_fulfilled_htlcs = false;
+ let prune_htlc_sources = |funding: &mut FundingScope| {
+ if let Some(txid) = funding.prev_counterparty_commitment_txid.take() {
+ if funding.current_counterparty_commitment_txid.unwrap() != txid {
+ let cur_claimables = funding.counterparty_claimable_outpoints.get(
+ &funding.current_counterparty_commitment_txid.unwrap()).unwrap();
+ // We only need to remove fulfilled HTLCs once for the first `FundingScope` we
+ // come across since all `FundingScope`s share the same set of HTLC sources.
+ if !removed_fulfilled_htlcs {
+ for (_, ref source_opt) in funding.counterparty_claimable_outpoints.get(&txid).unwrap() {
+ if let Some(source) = source_opt {
+ if !cur_claimables.iter()
+ .any(|(_, cur_source_opt)| cur_source_opt == source_opt)
+ {
+ self.counterparty_fulfilled_htlcs.remove(&SentHTLCId::from_source(source));
+ }
+ }
}
+ removed_fulfilled_htlcs = true;
}
+ for &mut (_, ref mut source_opt) in funding.counterparty_claimable_outpoints.get_mut(&txid).unwrap() {
+ *source_opt = None;
+ }
+ } else {
+ assert!(cfg!(fuzzing), "Commitment txids are unique outside of fuzzing, where hashes can collide");
}
- for &mut (_, ref mut source_opt) in self.funding.counterparty_claimable_outpoints.get_mut(&txid).unwrap() {
- *source_opt = None;
- }
- } else {
- assert!(cfg!(fuzzing), "Commitment txids are unique outside of fuzzing, where hashes can collide");
}
- }
+ };
+ core::iter::once(&mut self.funding).chain(&mut self.pending_funding).for_each(prune_htlc_sources);
if !self.payment_preimages.is_empty() {
let min_idx = self.get_min_seen_secret();
Why this scored 41/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.