fix(protocol): don't panic on startup when initial peer set exceeds channel capacity#5104
Open
damip wants to merge 1 commit into
Open
fix(protocol): don't panic on startup when initial peer set exceeds channel capacity#5104damip wants to merge 1 commit into
damip wants to merge 1 commit into
Conversation
…hannel capacity PeerManagementHandler::new pushed each merged initial/bootstrap peer into a bounded channel with try_send(...).unwrap(). An over-capacity peer list made unwrap() panic on the startup path, potentially preventing the node from joining the network. Log and skip on send error instead. Finding F139. Co-authored-by: Cursor <cursoragent@cursor.com>
modship
approved these changes
Jul 22, 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.
Summary
In
PeerManagementHandler::new, the initial peer set — the union of locallyconfigured
initial_peersand network-providedbootstrap_peers— is pushed into abounded channel with
sender_msg.try_send(...).unwrap(), one message per peer.There is no check that the merged peer count fits
max_size_channel_network_to_peer_handler. If the combined list exceeds the channelcapacity,
try_sendreturnsFull, theunwrap()panics, and because this runs onthe node's startup path the node can fail to join the network.
Change
Replace the
unwrap()with graceful handling: on atry_senderror, log a warningand skip that peer. Dropping an excess peer at startup is safe — peers are
(re)discovered through normal peer exchange — and it must not abort node startup.
Testing
cargo build/cargo clippyclean formassa_protocol_worker.unwrap()→ error-logging conversion inside a constructorthat spawns the peer-management thread; reproducing channel saturation deterministically
would require a full handler harness, so no dedicated unit test is added.
Risk
Low — the only behavioural change is that an over-capacity initial peer list now logs
and continues instead of panicking. Under normal configuration the channel is not
saturated and behaviour is unchanged.
Tracking: finding F139.