Rebuild pending payments list before replaying pending claims/fails
What the developer wrote
Authored by Matt Corallo
Rebuild pending payments list before replaying pending claims/fails
On `ChannelManager` reload we rebuild the pending outbound payments
list by looking for any missing payments in `ChannelMonitor`s.
However, in the same loop over `ChannelMonitor`s, we also re-claim
any pending payments which we see we have a payment preimage for.
If we send an MPP payment across different chanels, the result may
be that we'll iterate the loop, and in each iteration add a
pending payment with only one known path, then claim/fail it and
remove the pending apyment (at least for the claim case). This may
result in spurious extra events, or even both a `PaymentFailed` and
`PaymentSent` event on startup for the same payment.
What changed, and why it matters
This patch fixes a startup bug in the Lightning Dev Kit's channel manager. When the software restarts, it scans past payment data to rebuild its list of pending payments and to finalize any that already have a result. Previously, these two steps were mixed together in one loop, which could cause a multi-path payment to be partially rebuilt and then finalized before all paths were seen. This could produce duplicate or contradictory events, such as reporting the same payment as both failed and sent. The fix separates the work into two loops: first rebuild all pending payments, then finalize them. There is no direct security exploit here, but the inconsistent state could confuse downstream software or users.
Apply the patch. After upgrade, monitor for any anomalous duplicate PaymentSent/PaymentFailed events during node restart, and ensure downstream event consumers handle idempotently. No immediate incident response is indicated because the issue is a local state-replay bug, not an externally exploitable vulnerability.
Security signals we found
State inconsistency on reload leading to duplicate/conflicting payment events
Multi-path payment (MPP) handling edge case
No input validation or memory-safety issue visible in diff
Evidence from the diff
In ChannelManager::read, the existing single loop over channel_monitors both populated pending_outbound_payments and immediately replayed claims/fails for any HTLC with a known preimage or failure. For MPP payments spread across multiple ChannelMonitors, this interleaving meant the pending payment entry could be created from one path, claimed/failed, and removed before other paths were processed in subsequent iterations. The patch splits this into two passes: the first pass rebuilds the pending payments map, and the second pass re-claims/re-fails HTLCs. This ensures all paths are accounted for before any finalization action is taken, preventing spurious or conflicting PaymentSent/PaymentFailed events on reload.
Changed components
lightning/src/ln/channelmanager.rsChannelManager reload/deserialization pathpending outbound payments trackingMPP payment event generationInspect captured patch +15 / −0
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index af82f86..ec26f7a 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -16451,6 +16451,9 @@ where
// payments which are still in-flight via their on-chain state.
// We only rebuild the pending payments map if we were most recently serialized by
// 0.0.102+
+ //
+ // First we rebuild all pending payments, then separately re-claim and re-fail pending
+ // payments. This avoids edge-cases around MPP payments resulting in redundant actions.
for (channel_id, monitor) in args.channel_monitors.iter() {
let mut is_channel_closed = false;
let counterparty_node_id = monitor.get_counterparty_node_id();
@@ -16489,6 +16492,18 @@ where
);
}
}
+ }
+ }
+ for (channel_id, monitor) in args.channel_monitors.iter() {
+ let mut is_channel_closed = false;
+ let counterparty_node_id = monitor.get_counterparty_node_id();
+ if let Some(peer_state_mtx) = per_peer_state.get(&counterparty_node_id) {
+ let mut peer_state_lock = peer_state_mtx.lock().unwrap();
+ let peer_state = &mut *peer_state_lock;
+ 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()
{
Why this scored 34/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.