Skip to content

Fix peer store management on channel closure#895

Open
Jolah1 wants to merge 4 commits intolightningdevkit:mainfrom
Jolah1:peer-handling-on-force-close
Open

Fix peer store management on channel closure#895
Jolah1 wants to merge 4 commits intolightningdevkit:mainfrom
Jolah1:peer-handling-on-force-close

Conversation

@Jolah1
Copy link
Copy Markdown
Contributor

@Jolah1 Jolah1 commented May 1, 2026

Summary

This PR resolves both issues described in #745 related to peer store management on channel closure.

Fixed behaviors

  • Local force-close → peer is retained to allow channel_reestablish recovery
  • Counterparty force-closes last channel → peer is removed to avoid unnecessary reconnections

Problems

1. Local force-close removes peer too early

When we force-closed a channel in close_channel_internal, we immediately removed the peer from the store regardless of closure type:

// Before — peer removed for BOTH cooperative and force-close
if open_channels.len() == 1 {
    self.peer_store.remove_peer(&counterparty_node_id)?;
}

This broke recovery. The background reconnection task only reconnects to peers in the store. Removing the peer immediately guaranteed we would never reconnect, so channel_reestablish could never complete. This is especially problematic for LND peers, which may not handle force-closure error messages correctly and rely on us reconnecting to recover.

2. Counterparty force-close never cleans up peer

When a counterparty force-closed their last channel with us, the ChannelClosed event handler did nothing with the peer store. The peer remained persisted indefinitely and the node kept attempting to reconnect on every background tick. wasted resources and misleading peer state.


Solution

1. Retain peer on local force-close (src/lib.rs)

In close_channel_internal, peers are now only removed for cooperative
closes:

if open_channels.len() == 1 && !force {
    self.peer_store.remove_peer(&counterparty_node_id)?;
}

For force-closes, the peer is retained in the store so the background reconnection task can fire, channel_reestablish can complete, and peer cleanup is deferred to the ChannelClosed event handler.

2. Remove peer on counterparty-initiated closure (src/event.rs)

In the LdkEvent::ChannelClosed handler, we now check:

  • Is this the last channel with the peer?
  • Did the counterparty initiate the closure?

If both conditions are true, the peer is removed from the store.


Implementation Details

LDK timing behavior

LDK emits ChannelClosed before removing the channel from its internal state. A naive call to list_channels_with_counterparty() inside the handler would still return the closing channel, making the "last channel" check always wrong. We fix this by excluding the closing channel explicitly:

let has_other_channels = self
    .channel_manager
    .list_channels_with_counterparty(&counterparty_node_id)
    .iter()
    .any(|c| c.channel_id != channel_id); // exclude the channel that just closed

Ordering to avoid race conditions

Peer removal is performed before calling add_event. This ensures consumers of next_event_async() always observe a consistent peer store when they react to ChannelClosed — avoiding a race where the event is visible before the removal completes.


Tests

Added two integration tests in tests/integration_tests_rust.rs:

test_peer_removed_on_counterparty_force_close
Verifies that after the counterparty force-closes the last channel, the peer is no longer persisted in the local peer store.

test_peer_retained_on_local_force_close
Verifies that after a locally initiated force-close, the peer remains persisted in the local peer store to allow background reconnection and channel_reestablish recovery.

Both tests assert on is_persisted rather than peer presence to correctly distinguish stored peers from transient TCP connections that may still appear in list_peers() during teardown.


Results

test test_peer_removed_on_counterparty_force_close ... ok
test test_peer_retained_on_local_force_close ... ok
test result: ok. 41 passed; 0 failed; 0 ignored; 0 measured
cargo build → no errors
cargo clippy → no new warnings
cargo fmt → applied

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented May 1, 2026

I've assigned @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-reviews-bot ldk-reviews-bot requested a review from tnull May 1, 2026 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants