refactor(libp2p): use precise connection cleanup in handshake error handling#5330
Merged
Merged
Conversation
janos
approved these changes
Jan 24, 2026
martinconic
approved these changes
Jan 26, 2026
akrem-chabchoub
approved these changes
Jan 26, 2026
janos
approved these changes
Jul 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Description
Problem
In the handshake error paths, we called
s.host.Network().ClosePeer(peerID), which terminates all libp2p connections to that peer, not just the one that failed. With NAT'd peers and simultaneous dials, duplicate connection attempts are common: a failed handshake on a new, redundant connection would tear down an existing healthy session, notify kademlia, depeer the neighbor, and force a full reconnect cycle. Worst case was the duplicate-peer path inConnect(): a mereFullClose()failure on a redundant handshake stream calleds.Disconnect(overlay, ...), actively removing a healthy peer from the topology.Fix
Close only the connection that carried the failed handshake (
stream.Conn().Close()) and leave established sessions untouched:stream.Conn().Close()instead ofClosePeer()in the handshake error paths ofhandleIncomingandConnectConnect, close only the new redundant connection instead of disconnecting the peerbuildFullMAsfallback error path inConnect(was missed in the first pass)Why this is safe
addIfNotExiststracks every connection, and theDisconnectednotifiee only removes the peer when its last connection drops. Closing a single connection is handled correctly end to end.handleIncoming) already usedstream.Conn().Close()on master, this PR makes the outbound path and remaining error paths consistent with it.ClosePeer(): for those we do want to sever everything.Expected impact
Lower peer churn and a more stable topology: fewer spurious
bee_kademlia_total_inbound_disconnections, fewer reconnect attempts, longer peer session lifetimes. Being an error-path change, the effect is most visible in NAT-heavy environments; we'll verify with a canary comparing disconnection rate and session lifetime split by bee version.Open API Spec Version Changes (if applicable)
N/A
Motivation and Context (Optional)
Prevents disrupting healthy concurrent connections when rejecting redundant or failed connection attempts, reducing unnecessary kademlia topology churn.
Related Issue (Optional)
Screenshots (if appropriate):