Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
};

Expand Down
11 changes: 7 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
14 changes: 14 additions & 0 deletions tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 50 additions & 0 deletions tests/integration_tests_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading