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), }; 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