-
Notifications
You must be signed in to change notification settings - Fork 49
Add a Statement Gossip Subsystem section to the implementors guide #89
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
Conversation
| ```rust | ||
| struct StatementGossipSubsystem<GS: GossipService, SP: Spawn> { | ||
| /// This trait comes from the legacy networking code, so it is likely to be changed at some point. | ||
| gossip_service: GS, |
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.
cc @mxinden I'd expect that we'd do things the way they're laid out here, where the subsystem owns its entire gossip service.
However with the gossip-service as-is we need to write a GossipValidator that handles incoming messages and checks signatures and controls what messages we send to peers. Since the validator set can change from block to block, we'll want to:
- only accept messages related to the relay-parents of jobs that we have active
- fetch the validator set for a parent-hash as part of the job-starting process - either by having overseer provide this in
StartWorkor by calling out to anotherRuntimeApisubsystem to avoid shared ownership of the runtime API. - communicate to our peers which messages we are willing to receive and only send messages to peers they are expecting to receive. this is done with "neighbor packets" in the existing gossip code, which are special messages used to establish data-dependency, as @infinity0 frames it.
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.
I would imagine the StatementGossipSubsystem to own a GossipEngine as well as some means of communication to a custom implementation of the Validator trait. I don't have an opinion whether the GossipEngine should be wrapped in a GossipService.
- communicate to our peers which messages we are willing to receive and only send messages to peers they are expecting to receive. this is done with "neighbor packets" in the existing gossip code, which are special messages used to establish data-dependency, as Ximin frames it.
Sounds good to me. Only thing that would be important to me is that we don't misuse the notion of a Validator to send out messages like we do in substrate/client/finality-grandpa.
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.
As I understand the gossip model, it's the responsibility of the validator to re-broadcast messages that have been received. Without doing that, messages will not propagate. It's necessary to do that at the gossip-validator level so we can distinguish between neighbor packets and gossip messages. So without changing the lower-level gossip APIs substantially, we have to send messages from the gossip-validator. Did you mean something different than that?
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.
As I understand the gossip model, it's the responsibility of the validator to re-broadcast messages that have been received. Without doing that, messages will not propagate.
Yes, even though I would like to be a bit nit picky and rephrase it as: "A validator validates gossip messages and decides whether a message (if and only if valid) should be further propagated (via the ValidationResult)."
A gossip validator only reacts to messages but is never proactive as in sending new messages (not like what we do today with new neighbor messages in grandpa). Sending new messages (e.g. neighbor messages or statements) should happen through the GossipEngine.
Co-authored-by: Robert Habermeier <[email protected]>
| async fn statement_gossip_job<GS: GossipService>( | ||
| gossip_service: GS, | ||
| // The relay parent that this job is running for. | ||
| relay_parent: Hash, |
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.
I'd expect: A backing attestation would've relay_parent matching the candidate receipt I guess. An availability attestation would've relay patent when the parablock gets declared backed, so well after the relay_parent of the candidate receipt. An approval/validity attestation would've relay patent when the parablock gets declared available, even further after the relay_parent of the candidate receipt.
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.
Yes on the backing statements, although this module is not responsible at the moment for availability or secondary check gossip in any way. Availability bitfields could reasonably be merged with this module, but secondary check results probably will use a separate mechanism as they are likely to just be transactions circulated by the transaction queue gossip.
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.
I see, so those happen elsewhere probably
| /// A statement that we've received from a validator. | ||
| StatementReceived { relay_parent: Hash, statement: SignedStatement } | ||
| } | ||
| ``` |
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.
Approval VRF gossiping can sign its gossip messages with their own VRFs. Adds some code strangeness, but saves 64 bytes and 35ish us per gossip messages, maybe not worth the trouble.
|
Can you add sections with information on:
Also, and this is beyond the scope of the section, but we should provide some context and describe the components that make up a gossip protocol. We're going to have a few different sections that each define gossip protocols, so having some common reference point for what we mean is important for readability. |
No description provided.