[Networking] Add a hook for application messages #3759
Conversation
| )) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| // See whether or not we should be DA | ||
| let is_da = node_id < config.da_staked_committee_size as u64; |
There was a problem hiding this comment.
is it possible to take this from config.known_da_nodes instead?
I feel like it would be better to have config.known_da_nodes initialized to the first da_staked_committee_size nodes, and take directly from that here (otherwise we have da nodes getting set independently in two different places, which might lead to surprising behaviour down the line)
There was a problem hiding this comment.
Yeah we should do this. However the code is like this all over the test suites, I'll create an issue for this one and follow up since it might be a bigger change.
| @@ -160,11 +163,14 @@ impl<TYPES: NodeType> NetworkMessageTaskState<TYPES> { | |||
|
|
|||
| // Handle external messages | |||
| MessageKind::External(data) => { | |||
| if sender == self.public_key { | |||
There was a problem hiding this comment.
Does this essentially prevent loops with processing messages that we sent?
There was a problem hiding this comment.
It's basically so we don't broadcast messages to ourselves. I feel like it's unexpected behavior from the applications perspective to get it's own message
What is the external network in this case? The change is allow the sequencer to communicate through the hotshot layer because it's much more flexible and efficient. For example if a sequencer wanted to get a DA block from another node it sends a web request to a configured url. If non of the configured query nodes have it then it's hosed. Instead the sequencer could use this api to send a request to any HS public key it knows is on the DA committee to get the info. |
This PR:
This adds a way for the application layer to send arbitrary messages to other participants via the HotShot networking layer. It's not perfect as it'll do double de/serialization and all messages are fire and forget within hotshot.
This PR does not:
Key places to review:
Changes to
SystemContextHandleTest added to the newtorking tests