From 8647a56e64ce6d1a0d53d502186d22f99f20c224 Mon Sep 17 00:00:00 2001 From: jolah1 Date: Wed, 3 Jun 2026 08:19:19 +0100 Subject: [PATCH 1/2] node: retain peer after local force-close When we initiate a force-close via `Node::force_close_channel`, the remote peer may be unreachable or, in the case of certain LND versions, mishandle the force-close error message. Keeping the peer in the store lets the background reconnection task continue firing and gives `channel_reestablish` a chance to drive recovery once the peer is reachable again. The peer is now only dropped from `close_channel_internal` on cooperative close; users who want to forget a force-closed peer must call `Node::disconnect`. Extend `do_channel_full_cycle` to assert the new retention behavior in both the force-close and cooperative-close branches. --- src/lib.rs | 12 ++++++++++-- tests/common/mod.rs | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 614be098b..6eaaa3385 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1859,8 +1859,16 @@ impl Node { })?; } - // Check if this was the last open channel, if so, forget the peer. - if open_channels.len() == 1 { + // If this was the last open channel and we're closing cooperatively, forget the peer + // since we have no further reason to reconnect. + + // For force-closes we intentionally keep the peer in the store so the background reconnection + // task keeps firing and can drive the channel_reestablish recovery flow. + // This is especially important against LND peers, which don't always handle force-closure error messages correctly. + + // Note that this means a force-closed peer is retained until the user explicitly calls Node::disconnect. + + if open_channels.len() == 1 && !force { self.peer_store.remove_peer(&counterparty_node_id)?; } } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 30d9a4387..3e592641f 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -1592,6 +1592,20 @@ pub(crate) async fn do_channel_full_cycle( assert!(node_b.list_balances().pending_balances_from_channel_closures.is_empty()); } + if force_close { + // Peer retained after local force-close to allow channel_reestablish recovery. + assert!( + node_a.list_peers().iter().any(|p| p.node_id == node_b.node_id() && p.is_persisted), + "node_b should remain persisted in node_a peer store after locally-initiated force-close" + ); + } else { + // Peer removed after cooperative close — no further reason to reconnect. + assert!( + !node_a.list_peers().iter().any(|p| p.node_id == node_b.node_id() && p.is_persisted), + "node_b should be removed from node_a peer store after cooperative close" + ); + } + let sum_of_all_payments_sat = (push_msat + invoice_amount_1_msat + overpaid_amount_msat From 2148262f07afe32d90a576775094adc65f4af44a Mon Sep 17 00:00:00 2001 From: jolah1 Date: Wed, 3 Jun 2026 08:19:23 +0100 Subject: [PATCH 2/2] event: drop peer on counterparty or on-chain close When the counterparty drives closure or a commitment transaction confirms on chain, no further channel state can be recovered with the peer. Retaining them in the peer store would only spin the reconnection task forever. Remove the peer once their last channel with us reaches one of these terminal states: - `CounterpartyForceClosed` - `CounterpartyInitiatedCooperativeClosure` - `CommitmentTxConfirmed` `CommitmentTxConfirmed` covers the case where a remote commitment confirms while we are disconnected; LDK may also report our own commitment confirming under the same variant, but the channel is gone on chain in either case. `HolderForceClosed` is intentionally excluded so the reconnection loop can keep driving `channel_reestablish` recovery (handled in `Node::close_channel_internal`). `counterparty_node_id` is unwrapped with `expect` since LDK has always populated it on `ChannelClosed` from 0.0.117 onward, well before the minimum version this crate targets. If peer removal fails we now return `ReplayEvent` so the event is retried instead of being silently dropped. --- src/event.rs | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/src/event.rs b/src/event.rs index 7d23be99a..de8b72010 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1609,10 +1609,49 @@ where } => { log_info!(self.logger, "Channel {} closed due to: {}", channel_id, reason); + // `counterparty_node_id` has been set on every `ChannelClosed` since LDK 0.0.117. + let counterparty_node_id = counterparty_node_id + .expect("counterparty_node_id is always set since LDK 0.0.117"); + + // Drop the peer once their last channel with us has reached a terminal + // state reconnection cannot recover. `CommitmentTxConfirmed` is included + // because LDK reports a remote (or our own) commitment confirming on-chain + // via this variant, leaving nothing for `channel_reestablish` to recover. + // `HolderForceClosed` is deliberately excluded so reconnection can still + // drive recovery (see `Node::close_channel_internal`). + // We exclude `channel_id` from the count because LDK emits `ChannelClosed` + // before removing it from its internal list. + let reconnect_unneeded = matches!( + reason, + ClosureReason::CounterpartyForceClosed { .. } + | ClosureReason::CounterpartyInitiatedCooperativeClosure + | ClosureReason::CommitmentTxConfirmed + ); + + if reconnect_unneeded { + let has_other_channels = self + .channel_manager + .list_channels_with_counterparty(&counterparty_node_id) + .iter() + .any(|c| c.channel_id != channel_id); + + if !has_other_channels { + if let Err(e) = self.peer_store.remove_peer(&counterparty_node_id) { + log_error!( + self.logger, + "Failed to remove peer {} from peer store: {}", + counterparty_node_id, + e + ); + return Err(ReplayEvent()); + } + } + } + let event = Event::ChannelClosed { channel_id, user_channel_id: UserChannelId(user_channel_id), - counterparty_node_id, + counterparty_node_id: Some(counterparty_node_id), reason: Some(reason), };