node: consolidate peer-store cleanup into ChannelClosed handler

Peer-store cleanup on channel closure was split across two places:
close_channel_internal removed the peer on a cooperative close, while the
ChannelClosed handler removed it for an allowlist of counterparty/on-chain
reasons. That allowlist missed terminal cases such as a channel closing
before funding (CounterpartyCoopClosedUnfundedChannel), leaving those
peers in the store and the reconnection loop retrying them indefinitely.

Make the ChannelClosed handler the single owner of the decision: retain
the peer only for HolderForceClosed -- where we deliberately keep
reconnecting so channel_reestablish can drive recovery, important against
LND peers that don't always handle force-closure error messages -- and
drop it for every other terminal reason once no other channel with the
peer remains. close_channel_internal no longer touches the peer store.

Add an integration test for the counterparty force-close path and keep
the retain/remove assertions in do_channel_full_cycle.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: Jolah1

src/event.rs

@@ -1613,10 +1613,45 @@ 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 its last channel with us has reached a terminal state
+				// that reconnection cannot recover. Every closure reason is terminal except
+				// `HolderForceClosed`: when *we* force-close, we keep reconnecting so that
+				// `channel_reestablish` can drive recovery (see `Node::close_channel_internal`).
+				// This also cleans up peers persisted for a channel that closed before funding
+				// (e.g. `CounterpartyCoopClosedUnfundedChannel`), which would otherwise be
+				// retried forever.
+				// We exclude `channel_id` from the count because LDK emits `ChannelClosed`
+				// before removing it from its internal list.
+				let dont_reconnect = !matches!(reason, ClosureReason::HolderForceClosed { .. });
+
+				if dont_reconnect {
+					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),
 				};
 

src/lib.rs

@@ -1919,10 +1919,13 @@ impl Node {
 					})?;
 			}
 
-			// Check if this was the last open channel, if so, forget the peer.
-			if open_channels.len() == 1 {
-				self.runtime.block_on(self.peer_store.remove_peer(&counterparty_node_id))?;
-			}
+			// Peer store cleanup is handled centrally in the `ChannelClosed` event handler,
+			// which drops the peer once its last channel reaches a terminal state that
+			// reconnection cannot recover. We intentionally do nothing here so that a
+			// force-closed peer is retained, letting the background reconnection task keep
+			// firing and drive the `channel_reestablish` recovery flow. This is especially
+			// important against LND peers, which don't always handle force-closure error
+			// messages correctly.
 		}
 
 		Ok(())

tests/common/mod.rs

@@ -1597,6 +1597,20 @@ pub(crate) async fn do_channel_full_cycle<E: ElectrumApi>(
 		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

tests/integration_tests_rust.rs

@@ -97,6 +97,56 @@ async fn channel_full_cycle_force_close_trusted_no_reserve() {
 	.await;
 }
 
+#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+async fn peer_removed_when_counterparty_force_closes_last_channel() {
+	// When we open a channel outbound, we persist the counterparty so the background
+	// reconnection task can reach them. If the counterparty then force-closes what turns out
+	// to be their last channel with us, the channel is terminal and there is nothing left for
+	// `channel_reestablish` to recover, so the peer should be dropped from the store rather
+	// than reconnected to forever.
+	let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
+	let chain_source = random_chain_source(&bitcoind, &electrsd);
+	let (node_a, node_b) = setup_two_nodes(&chain_source, false, true, false);
+
+	let address_a = node_a.onchain_payment().new_address().unwrap();
+	let premine_amount_sat = 5_000_000;
+	premine_and_distribute_funds(
+		&bitcoind.client,
+		&electrsd.client,
+		vec![address_a],
+		Amount::from_sat(premine_amount_sat),
+	)
+	.await;
+	node_a.sync_wallets().unwrap();
+
+	// node_a opens the channel, so node_a persists node_b in its peer store.
+	open_channel(&node_a, &node_b, 4_000_000, false, &electrsd).await;
+	generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await;
+	node_a.sync_wallets().unwrap();
+	node_b.sync_wallets().unwrap();
+
+	let _user_channel_id_a = expect_channel_ready_event!(node_a, node_b.node_id());
+	let user_channel_id_b = expect_channel_ready_event!(node_b, node_a.node_id());
+
+	assert!(
+		node_a.list_peers().iter().any(|p| p.node_id == node_b.node_id() && p.is_persisted),
+		"node_a should persist node_b after opening a channel to it"
+	);
+
+	// The counterparty force-closes their last channel with us.
+	node_b.force_close_channel(&user_channel_id_b, node_a.node_id(), None).unwrap();
+
+	expect_event!(node_a, ChannelClosed);
+	expect_event!(node_b, ChannelClosed);
+
+	// node_a should have dropped node_b from its peer store. We assert on `is_persisted` rather
+	// than peer presence so a lingering transient TCP connection doesn't mask the removal.
+	assert!(
+		!node_a.list_peers().iter().any(|p| p.node_id == node_b.node_id() && p.is_persisted),
+		"node_a should drop node_b from its peer store after node_b force-closed the last channel"
+	);
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
 async fn channel_full_cycle_0conf() {
 	let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();