fix(network): allow Disconnect message on T3 connections#15405
Conversation
darioush
changed the title
There was a problem hiding this comment.
Pull request overview
This PR fixes a tier-mismatch in the networking layer where Disconnect messages were rejected on T3 connections, even though PeerActor::stop_actor sends Disconnect on shutdown for all tiers. It also improves test observability of non-T2 connection lifecycles.
Changes:
- Allow
PeerMessage::Disconnectto be received on all TCP tiers intcp::Tier::is_allowed_receive. - Ensure
NetworkState::unregisteremits theConnectionClosedtest event for all tiers (by avoiding the early return for non-T2). - Add a regression test that sends
Disconnectover a T3 connection and asserts it is handled cleanly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
chain/network/src/peer_manager/connection/mod.rs |
Updates tier message-allowlist to accept Disconnect on any tier. |
chain/network/src/peer_manager/network_state/mod.rs |
Refactors unregister to keep T2-only routing/store logic gated while still emitting ConnectionClosed test events for all tiers. |
chain/network/src/peer_manager/tests/connection_pool.rs |
Adds t3_disconnect regression test validating T3 Disconnect is accepted and produces the expected closing reason. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #15405 +/- ##
==========================================
- Coverage 69.02% 69.01% -0.02%
==========================================
Files 934 934
Lines 209214 209873 +659
Branches 209214 209873 +659
==========================================
+ Hits 144405 144834 +429
- Misses 58823 59049 +226
- Partials 5986 5990 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Disconnectwas only allowed on T2 inis_allowed_receive, butPeerActor::stopping(peer_actor.rs) sends it unconditionally on all tiers when a peer connection closes. This caused every T3 connection closure to log a spurious warning and forcibly drop the connection asDisallowedMessageinstead of processing the disconnect cleanly.Disconnecton all tiers (likeHandshakeFailureandLastEdge)ConnectionClosedtest event for non-T2 tiers inunregisterso tests can observe T3 connection lifecyclet3_disconnecttest that sends aDisconnectover T3 and asserts it's not rejected