This repository was archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Split the Roles in three types #5520
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…o compile on my machine
andresilva
approved these changes
Apr 3, 2020
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reopening #5492 so that the CI scripts works when picking the Polkadot companion.
---- Original description ----
This PR splits the
Rolesbitfield in three:The existing
Roleshas been moved untouched into thesc_network::protocol::messagemodule, where it continues to represent the data that is transferred on the wire.sc_network::confignow contains a newRole(note the lack of 's') enum whose variants areFull,Light,SentryandAuthority. It is more robust to misuse, as one can only pass a list of sentry nodes when usingAuthority, and a valiator when usingSentry.The gossiping API now uses
ObservedRole, which is an enum that can be one ofFull,Light,OurSentry(for when you're a validator connecting to your sentry),OurGuardedValidator(for when you're a sentry connecting to your validator), orAuthority(for third-party validators).GrandPa has been adjusted to always gossip messages to nodes whose role is
OurSentryorOurGuardedValidator. This fixes some issues with validators not receiving GrandPa messages from their sentries. This change is the original reason for this PR.Additionally, the
--sentryCLI option now accepts a list of multiaddress as parameter, representing the addresses of the validators we're protecting. This is a CLI breaking change.While modifying the CLI code, I encountered some issues with how to properly build a
Role::Authoritywhen the parameter of--sentryis aString.To fix that, I changed various
Stringsin the structopt structs insc_clito instead contain aMultiaddrorMultiaddrWithPeerId(a new type introduced in this PR).Some other minor changes:
client/rpc/api/system::NodeRolenow contains aSentryvariant. In other words, querying the node role from the RPC endpoint will now return "sentry" for sentries.authority_discoverynow accepts aVec<MultiaddrWithPeerId>instead of aVec<String>in accordance with the changes in the CLI.node_rolePrometheus metric will now contain the value3for sentries, rather than4(4 meaning "authority").polkadot companion: paritytech/polkadot#960