-
Notifications
You must be signed in to change notification settings - Fork 524
network: consensus traffic via pubsub #6501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6501 +/- ##
==========================================
- Coverage 47.87% 47.67% -0.20%
==========================================
Files 662 655 -7
Lines 87934 87983 +49
==========================================
- Hits 42095 41947 -148
- Misses 43076 43253 +177
- Partials 2763 2783 +20 ☔ View full report in Codecov by Sentry. |
1. make syncCh unbuffered 2. handle first writing into it 3. handlre remaining writings as diect calls
bd870a2 to
b1ea1fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces consensus traffic routing via PubSub by adding support for agreement votes, proposal payloads, and vote bundles to the P2P gossip network. The implementation extends the existing transaction-focused PubSub infrastructure to handle consensus messages.
Key changes:
- Added three new PubSub topics (AV, PP, VB) for consensus messages alongside the existing TX topic
- Implemented validator handlers for synchronous message validation with forwarding policy support
- Disabled vote compression tests for P2P transport pending future implementation
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| network/p2p/pubsub.go | Defined new topic names and updated PubSub configuration to support consensus message topics |
| network/p2pNetwork.go | Added consensus topic handling loop and extended validator to support multiple message types |
| network/metrics.go | Added metrics tracking for the new consensus message topics |
| network/p2pNetwork_test.go | Updated tests to use validator handlers and added topic subscription checks |
| network/msgCompressor_test.go | Disabled P2P vote compression tests temporarily |
| agreement/gossip/network.go | Implemented synchronous validation handlers with forwarding policy support |
| agreement/gossip/network_test.go | Added validator handler registration methods to test network |
| agreement/fuzzer/networkFacade_test.go | Added stub validator handler methods for fuzzer facade |
| agreement/abstractions.go | Added Ignore method to Network interface |
| agreement/actions.go | Implemented Ignore action handling |
| agreement/proposalManager.go | Fixed typo in comment (delievered → delivered) |
| agreement/service_test.go | Added stub Ignore method to test endpoint |
| network/mesh_test.go | Updated mock P2P service to properly handle subscriptions and closures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Summary
Route AV, PP, VB messages to pubsub via
TaggedMessageValidatorHandler. It is quite similar toTxHandlerimplementation except the following:net.Relay/net.Ignorefor the second response from agreement state machine. AlsopeerID + seqNomessage ID solves re-transmission issue (all messages have new ID so that propagated).What is missing: votes compression
Test Plan
TestNodeHybridOrP2PTopologyto check both P2P and Hybrid networks advanceagreement/fuzzer/networkFacade_test.go