Comm Component for Simplex #3998
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the communication component for Simplex by adding new fields to the Config struct and implementing a Comm type for sending and broadcasting messages among validators. Key changes include:
- Adding new Config fields (Sender and OutboundMsgBuilder) to support message construction and delivery.
- Implementing Comm with methods for SendMessage and Broadcast.
- Updating tests and mocks to validate the communication flow and BLS signing functionality.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| simplex/test_utils.go | Added test utilities for validator info and engine config |
| simplex/config.go | Extended Config struct to include Sender and MsgBuilder |
| simplex/comm_test.go | Added tests for Comm functionality and error conditions |
| simplex/comm.go | Implemented Comm with SendMessage and Broadcast methods |
| simplex/bls_test.go, simplex/bls.go | Added BLS authentication with signing and verification |
| message/simplex_outbound_msg_builder.go | Added Simplex-specific outbound message builder functions |
| message/outbound_msg_builder.go | Integrated Simplex builder into general outbound messaging |
| message/messagemock/outbound_message_builder.go | Updated mocks with Simplex message methods |
| go.mod | Updated dependency for simplex package |
| @@ -181,6 +181,8 @@ type OutboundMsgBuilder interface { | |||
| chainID ids.ID, | |||
| msg []byte, | |||
| ) (OutboundMessage, error) | |||
|
|
|||
| SimplexOutboundMessageBuilder | |||
There was a problem hiding this comment.
rather than bloating this file up, i thought it would be cleaner to separate the simplex outbound messages into their own file
Signed-off-by: Sam Liokumovich <[email protected]>
Signed-off-by: Sam Liokumovich <[email protected]>
Signed-off-by: Sam Liokumovich <[email protected]>
| @@ -181,6 +181,8 @@ type OutboundMsgBuilder interface { | |||
| chainID ids.ID, | |||
| msg []byte, | |||
| ) (OutboundMessage, error) | |||
|
|
|||
| SimplexOutboundMessageBuilder | |||
| } | |||
|
|
|||
| type outMsgBuilder struct { | |||
There was a problem hiding this comment.
Unrelated to this PR, but I have to say I am not sure I understand why this is an un-exported struct. it Makes it impossible to use anywhere in the code (for tests) and we also have only one concrete implementation of it.
@StephenButtolph do you know why?
| "github.com/ava-labs/avalanchego/utils/set" | ||
| ) | ||
|
|
||
| var errNodeNotFound = errors.New("node not found in the validator list") |
There was a problem hiding this comment.
can we emphasize that it is our node that is not found?
There was a problem hiding this comment.
added to the error where it was being returned, so this err could potentially be reused
Signed-off-by: Sam Liokumovich <[email protected]>
Why this should be merged
The
Comminterface is used by simplex to send and broadcast messages. This PR builds off pr #3993 and should be merged afterHow this works
This PR adds two new fields to the Config struct—
SenderandOutboundMsgBuilder—which enable the Simplex engine to construct and send outbound messages to other nodes.How this was tested
Using generated mocks for
ExternalSenderandOutboundMessageBuilder, I added tests in comm_test to verify that the correct messages are sent the expected number of times.Need to be documented in RELEASES.md?
No