← Watch feed
Move is_chan_closed check into loop
Public commit record
60/100 · Adequate
What the developer wrote
Authored by Valentine Wallace
Move is_chan_closed check into loop
Necessary for the next commit and makes it easier to read.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides an explanatory body
What changed, and why it matters
This commit is a pure code restructure: it moves an existing `if is_channel_closed` check inside the loops that process HTLCs, rather than wrapping the loops from the outside. The commit message explicitly says it is 'necessary for the next commit and makes it easier to read.' No behavior changes are visible in the diff, and there is no security fix here.
Recommended action
No security action needed. Review the follow-up commit referenced in the message to assess any actual security relevance.
Security signals we found
No strong security signals were identified.
Evidence from the diff
In lightning/src/ln/channelmanager.rs, the patch inverts the control flow of an existing closure check. Previously, the code branched if is_channel_closed { for ... { ... } }. Now it iterates first and continues when !is_channel_closed. The bodies of the two match arms (PreviousHopData and OutboundRoute) are unchanged except for indentation and the added continue. A third loop (get_onchain_failed_outbound_htlcs) is moved outside the former if block but is itself unconditional and unchanged. The is_channel_closed boolean is still set exactly once before these loops, so the effective behavior is identical.
Changed components
lightning/src/ln/channelmanager.rsInspect captured patch +147 / −138
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 0b6cc07..b774467 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -18005,156 +18005,165 @@ where
is_channel_closed = !peer_state.channel_by_id.contains_key(channel_id);
}
- if is_channel_closed {
- for (htlc_source, (htlc, preimage_opt)) in
- monitor.get_all_current_outbound_htlcs()
- {
- let logger = WithChannelMonitor::from(
- &args.logger,
- monitor,
- Some(htlc.payment_hash),
- );
- let htlc_id = SentHTLCId::from_source(&htlc_source);
- match htlc_source {
- HTLCSource::PreviousHopData(prev_hop_data) => {
- let pending_forward_matches_htlc = |info: &PendingAddHTLCInfo| {
- info.prev_funding_outpoint == prev_hop_data.outpoint
- && info.prev_htlc_id == prev_hop_data.htlc_id
- };
- // The ChannelMonitor is now responsible for this HTLC's
- // failure/success and will let us know what its outcome is. If we
- // still have an entry for this HTLC in `forward_htlcs`,
- // `pending_intercepted_htlcs`, or `decode_update_add_htlcs`, we were apparently not
- // persisted after the monitor was when forwarding the payment.
- dedup_decode_update_add_htlcs(
- &mut decode_update_add_htlcs,
- &prev_hop_data,
- "HTLC was forwarded to the closed channel",
- &args.logger,
- );
- dedup_decode_update_add_htlcs(
- &mut decode_update_add_htlcs_legacy,
- &prev_hop_data,
- "HTLC was forwarded to the closed channel",
- &args.logger,
- );
- forward_htlcs_legacy.retain(|_, forwards| {
- forwards.retain(|forward| {
- if let HTLCForwardInfo::AddHTLC(htlc_info) = forward {
- if pending_forward_matches_htlc(&htlc_info) {
- log_info!(logger, "Removing pending to-forward HTLC with hash {} as it was forwarded to the closed channel {}",
- &htlc.payment_hash, &monitor.channel_id());
- false
- } else { true }
+ for (htlc_source, (htlc, preimage_opt)) in monitor.get_all_current_outbound_htlcs()
+ {
+ let logger =
+ WithChannelMonitor::from(&args.logger, monitor, Some(htlc.payment_hash));
+ let htlc_id = SentHTLCId::from_source(&htlc_source);
+ match htlc_source {
+ HTLCSource::PreviousHopData(prev_hop_data) => {
+ let pending_forward_matches_htlc = |info: &PendingAddHTLCInfo| {
+ info.prev_funding_outpoint == prev_hop_data.outpoint
+ && info.prev_htlc_id == prev_hop_data.htlc_id
+ };
+ if !is_channel_closed {
+ continue;
+ }
+ // The ChannelMonitor is now responsible for this HTLC's
+ // failure/success and will let us know what its outcome is. If we
+ // still have an entry for this HTLC in `forward_htlcs`,
+ // `pending_intercepted_htlcs`, or `decode_update_add_htlcs`, we were apparently not
+ // persisted after the monitor was when forwarding the payment.
+ dedup_decode_update_add_htlcs(
+ &mut decode_update_add_htlcs,
+ &prev_hop_data,
+ "HTLC was forwarded to the closed channel",
+ &args.logger,
+ );
+ dedup_decode_update_add_htlcs(
+ &mut decode_update_add_htlcs_legacy,
+ &prev_hop_data,
+ "HTLC was forwarded to the closed channel",
+ &args.logger,
+ );
+ forward_htlcs_legacy.retain(|_, forwards| {
+ forwards.retain(|forward| {
+ if let HTLCForwardInfo::AddHTLC(htlc_info) = forward {
+ if pending_forward_matches_htlc(&htlc_info) {
+ log_info!(logger, "Removing pending to-forward HTLC with hash {} as it was forwarded to the closed channel {}",
+ &htlc.payment_hash, &monitor.channel_id());
+ false
} else { true }
- });
- !forwards.is_empty()
- });
- pending_intercepted_htlcs_legacy.retain(|intercepted_id, htlc_info| {
- if pending_forward_matches_htlc(&htlc_info) {
- log_info!(logger, "Removing pending intercepted HTLC with hash {} as it was forwarded to the closed channel {}",
- &htlc.payment_hash, &monitor.channel_id());
- pending_events_read.retain(|(event, _)| {
- if let Event::HTLCIntercepted { intercept_id: ev_id, .. } = event {
- intercepted_id != ev_id
- } else { true }
- });
- false
} else { true }
});
- },
- HTLCSource::OutboundRoute {
- payment_id,
- session_priv,
- path,
- bolt12_invoice,
- ..
- } => {
- if let Some(preimage) = preimage_opt {
- let pending_events = Mutex::new(pending_events_read);
- let update = PaymentCompleteUpdate {
- counterparty_node_id: monitor.get_counterparty_node_id(),
- channel_funding_outpoint: monitor.get_funding_txo(),
- channel_id: monitor.channel_id(),
- htlc_id,
- };
- let mut compl_action = Some(
- EventCompletionAction::ReleasePaymentCompleteChannelMonitorUpdate(update)
- );
- pending_outbounds.claim_htlc(
- payment_id,
- preimage,
- bolt12_invoice,
- session_priv,
- path,
- true,
- &mut compl_action,
- &pending_events,
- );
- // If the completion action was not consumed, then there was no
- // payment to claim, and we need to tell the `ChannelMonitor`
- // we don't need to hear about the HTLC again, at least as long
- // as the PaymentSent event isn't still sitting around in our
- // event queue.
- let have_action = if compl_action.is_some() {
- let pending_events = pending_events.lock().unwrap();
- pending_events.iter().any(|(_, act)| *act == compl_action)
- } else {
- false
- };
- if !have_action && compl_action.is_some() {
- let mut peer_state = per_peer_state
- .get(&counterparty_node_id)
- .map(|state| state.lock().unwrap())
- .expect("Channels originating a preimage must have peer state");
- let update_id = peer_state
- .closed_channel_monitor_update_ids
- .get_mut(channel_id)
- .expect("Channels originating a preimage must have a monitor");
- // Note that for channels closed pre-0.1, the latest
- // update_id is `u64::MAX`.
- *update_id = update_id.saturating_add(1);
-
- pending_background_events.push(BackgroundEvent::MonitorUpdateRegeneratedOnStartup {
- counterparty_node_id: monitor.get_counterparty_node_id(),
+ !forwards.is_empty()
+ });
+ pending_intercepted_htlcs_legacy.retain(|intercepted_id, htlc_info| {
+ if pending_forward_matches_htlc(&htlc_info) {
+ log_info!(logger, "Removing pending intercepted HTLC with hash {} as it was forwarded to the closed channel {}",
+ &htlc.payment_hash, &monitor.channel_id());
+ pending_events_read.retain(|(event, _)| {
+ if let Event::HTLCIntercepted { intercept_id: ev_id, .. } = event {
+ intercepted_id != ev_id
+ } else { true }
+ });
+ false
+ } else { true }
+ });
+ },
+ HTLCSource::OutboundRoute {
+ payment_id,
+ session_priv,
+ path,
+ bolt12_invoice,
+ ..
+ } => {
+ if !is_channel_closed {
+ continue;
+ }
+ if let Some(preimage) = preimage_opt {
+ let pending_events = Mutex::new(pending_events_read);
+ let update = PaymentCompleteUpdate {
+ counterparty_node_id: monitor.get_counterparty_node_id(),
+ channel_funding_outpoint: monitor.get_funding_txo(),
+ channel_id: monitor.channel_id(),
+ htlc_id,
+ };
+ let mut compl_action = Some(
+ EventCompletionAction::ReleasePaymentCompleteChannelMonitorUpdate(update)
+ );
+ pending_outbounds.claim_htlc(
+ payment_id,
+ preimage,
+ bolt12_invoice,
+ session_priv,
+ path,
+ true,
+ &mut compl_action,
+ &pending_events,
+ );
+ // If the completion action was not consumed, then there was no
+ // payment to claim, and we need to tell the `ChannelMonitor`
+ // we don't need to hear about the HTLC again, at least as long
+ // as the PaymentSent event isn't still sitting around in our
+ // event queue.
+ let have_action = if compl_action.is_some() {
+ let pending_events = pending_events.lock().unwrap();
+ pending_events.iter().any(|(_, act)| *act == compl_action)
+ } else {
+ false
+ };
+ if !have_action && compl_action.is_some() {
+ let mut peer_state = per_peer_state
+ .get(&counterparty_node_id)
+ .map(|state| state.lock().unwrap())
+ .expect(
+ "Channels originating a preimage must have peer state",
+ );
+ let update_id = peer_state
+ .closed_channel_monitor_update_ids
+ .get_mut(channel_id)
+ .expect(
+ "Channels originating a preimage must have a monitor",
+ );
+ // Note that for channels closed pre-0.1, the latest
+ // update_id is `u64::MAX`.
+ *update_id = update_id.saturating_add(1);
+
+ pending_background_events.push(
+ BackgroundEvent::MonitorUpdateRegeneratedOnStartup {
+ counterparty_node_id: monitor
+ .get_counterparty_node_id(),
funding_txo: monitor.get_funding_txo(),
channel_id: monitor.channel_id(),
update: ChannelMonitorUpdate {
update_id: *update_id,
channel_id: Some(monitor.channel_id()),
- updates: vec![ChannelMonitorUpdateStep::ReleasePaymentComplete {
- htlc: htlc_id,
- }],
+ updates: vec![
+ ChannelMonitorUpdateStep::ReleasePaymentComplete {
+ htlc: htlc_id,
+ },
+ ],
},
- });
- }
- pending_events_read = pending_events.into_inner().unwrap();
+ },
+ );
}
- },
- }
+ pending_events_read = pending_events.into_inner().unwrap();
+ }
+ },
}
- for (htlc_source, payment_hash) in monitor.get_onchain_failed_outbound_htlcs() {
- log_info!(
- args.logger,
- "Failing HTLC with payment hash {} as it was resolved on-chain.",
- payment_hash
- );
- let completion_action = Some(PaymentCompleteUpdate {
- counterparty_node_id: monitor.get_counterparty_node_id(),
- channel_funding_outpoint: monitor.get_funding_txo(),
- channel_id: monitor.channel_id(),
- htlc_id: SentHTLCId::from_source(&htlc_source),
- });
+ }
+ for (htlc_source, payment_hash) in monitor.get_onchain_failed_outbound_htlcs() {
+ log_info!(
+ args.logger,
+ "Failing HTLC with payment hash {} as it was resolved on-chain.",
+ payment_hash
+ );
+ let completion_action = Some(PaymentCompleteUpdate {
+ counterparty_node_id: monitor.get_counterparty_node_id(),
+ channel_funding_outpoint: monitor.get_funding_txo(),
+ channel_id: monitor.channel_id(),
+ htlc_id: SentHTLCId::from_source(&htlc_source),
+ });
- failed_htlcs.push((
- htlc_source,
- payment_hash,
- monitor.get_counterparty_node_id(),
- monitor.channel_id(),
- LocalHTLCFailureReason::OnChainTimeout,
- completion_action,
- ));
- }
+ failed_htlcs.push((
+ htlc_source,
+ payment_hash,
+ monitor.get_counterparty_node_id(),
+ monitor.channel_id(),
+ LocalHTLCFailureReason::OnChainTimeout,
+ completion_action,
+ ));
}
// Whether the downstream channel was closed or not, try to re-apply any payment
Risk score
Our methodology →Why this scored 11/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.