-
Notifications
You must be signed in to change notification settings - Fork 621
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
Use owners instead of public keys. #3160
Conversation
3d6b992
to
f4c5f3f
Compare
f4c5f3f
to
0d6d990
Compare
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.
Very nice!
pub fn verify_owner(&self, proposal: &BlockProposal) -> Option<PublicKey> { | ||
if let Some(public_key) = self.ownership.get().super_owners.get(&proposal.owner) { | ||
return Some(*public_key); | ||
pub fn verify_owner(&self, proposal: &BlockProposal) -> bool { |
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.
Need to update method description here?
@@ -124,6 +124,8 @@ BlockProposal: | |||
TYPENAME: ProposalContent | |||
- owner: | |||
TYPENAME: Owner | |||
- public_key: |
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.
LGTM!
Motivation
For #3162 we need to stop using
ChainOwnership
as a lookup table for public keys.In general, we are using
PublicKey
in several places where we should be using theOwner
type.Proposal
Remove public keys from
ChainOwnership
. Add the signer's public key directly to theBlockProposal
instead.When creating key pairs, assigning chains and changing chain ownership via CLI commands, node service mutations or system API calls, use
Owner
instead ofPublicKey
.Test Plan
Several tests have been updated.
Release Plan
Links
PublicKey
fromChainOwnership
. #3165.