From 1b8210300f0d472199d22c6ad786c9f759f8b1f0 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 34fa7f54d..0a7b5a571 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1919,8 +1919,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.runtime.block_on(self.peer_store.remove_peer(&counterparty_node_id))?; } } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index adeb327bf..bde783456 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -1597,6 +1597,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 1af7e7c47ec8eaa8735b5efeb23fe6598b53bd68 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 80acd0690..474def7d6 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1613,10 +1613,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).await { + 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), };