-
Notifications
You must be signed in to change notification settings - Fork 747
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
add possibility to inject non-authorities session-keys in genesis #5078
add possibility to inject non-authorities session-keys in genesis #5078
Conversation
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.
Technically looks good
substrate/frame/session/src/lib.rs
Outdated
/// List of (AccountId, ValidatorId, Keys) that will be registered at genesis, but not as active validators. | ||
pub non_authority_keys: Vec<(T::AccountId, T::ValidatorId, T::Keys)>, |
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.
IMO is worth being explicit that these keys (together with keys
) will end up in the NextKeys
set. I.e. keys that are registered as candidates for session after next session (session # 2).
And technically these are still authorities (well, authority candidates). So maybe a more appropriate name? Don't know registered_candidates
? But naming is hard so I may be biased :-D
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 disagree, non_authority_keys
are not necessarily authorities, whether they will be authorities it will be decided by SessionManager
(even in the session # 2). Just for having keys registered they will not be authorities, unless they SessionManager
decides to select them.
As for the naming, I am fine with anything as I am not too biased around 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.
Well.
- I agree that these are not necessarily actual authorities, indeed I put in bold that are candidates.
- This is not my opinion, if I've not overlooked something, this is just a fact. The
inner_set_keys
is called, which callsput_keys
, which stores yournon_authority_keys
inNextKeys
set, which are the candidates registered
So I still think is worth being explicit about what is the usage of this non_authority_keys
list. Otherwise as a user the doc is not super clear.
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.
ah got it. Yeah I can add more clarity on that for sure!
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.
Alright so I added a comment saying that these keys are valid at least until session 2 (maybe valid is not the best word here?), but basically my intention is to say that if there is a session-key change, the first session that would pick these new keys is session number 3
@girazoki CI is not happy. |
will check |
Head branch was pushed to by a user without write access
@girazoki also clippy was failing: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6772111 |
@girazoki CI is still not happy :D |
I am really trying xD. Let's see this time |
Head branch was pushed to by a user without write access
@bkchr I think I fixed everything but I am not sure about what the process is here, let me know if there is anything else I can do for the CI to be happy |
The CI pipeline was cancelled due to failure one of the required jobs. |
Head branch was pushed to by a user without write access
Also |
That should be easy I think 😄 |
Or maybe not 🙈, why is semver telling me the crates are not listed in the pr-doc when they are? |
Ahh yeah there was a bug with |
d6f5987
…ritytech#5078) Add the possibility of injecting session-keys in genesis for non-validators. Currently all keys injected in genesis were considered as part of the initial validators set, this PR allows to inject a new vector with non-authority keys --------- Co-authored-by: Bastian Köcher <[email protected]>
…ritytech#5078) Add the possibility of injecting session-keys in genesis for non-validators. Currently all keys injected in genesis were considered as part of the initial validators set, this PR allows to inject a new vector with non-authority keys --------- Co-authored-by: Bastian Köcher <[email protected]>
Add the possibility of injecting session-keys in genesis for non-validators. Currently all keys injected in genesis were considered as part of the initial validators set, this PR allows to inject a new vector with non-authority keys