Decommission signer registration with declarative PoolId #653
Conversation
This file contains 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
jpraynaud
force-pushed
the
jpraynaud/621-decommission-unverified-signer-registration
branch
from
b074d03
to
90061ff
Compare
jpraynaud
temporarily deployed
to
testing-preview
GitHub Actions
Inactive
iquerejeta
reviewed
Dec 14, 2022
| @@ -252,7 +252,7 @@ impl KeyRegWrapper { | |||
| if cfg!(not(feature = "allow_skip_signer_certification")) { | |||
| Err(ProtocolRegistrationErrorWrapper::OpCertMissing)? | |||
| } | |||
| println!("WARNING: Uncertified signer regsitration by providing a Pool Id is deprecated and will be removed soon! (Pool Id: {:?})", party_id); | |||
| println!("WARNING: Uncertified signer regsitration by providing a Pool Id is decommissionned and must be used for tests only! (Pool Id: {:?})", party_id); | |||
There was a problem hiding this comment.
Should we put this warning directly when the signers are initialised? So, rather than warn users only when they've tried to register, if they already run the node with the "allow_skip_signer_certification", then this warning should appear, saying they won't be able to register. WDYT?
There was a problem hiding this comment.
It's difficult to know that at signer initialization if the signer will be certified or not because the feature flag allow_skip_signer_certification is applied to mithril-common. But I agree with you that this could be a good option 🤔
There was a problem hiding this comment.
ah, good point. We could put the warning with StmInitializerWrapper::setup() then. WDYT?
There was a problem hiding this comment.
Yes, I moved the warning in StmInitializerWrapper::setup() in b83300e
iquerejeta
approved these changes
Dec 14, 2022
In 'mithril-common' and 'mithril-end-to-end'.
jpraynaud
force-pushed
the
jpraynaud/621-decommission-unverified-signer-registration
branch
from
90061ff
to
b00a5cc
Compare
jpraynaud
temporarily deployed
to
testing-preview
GitHub Actions
Inactive
jpraynaud
temporarily deployed
to
testing-preview
GitHub Actions
Inactive
jpraynaud
temporarily deployed
to
testing-preview
GitHub Actions
Inactive
Moved to 'StmInitializerWrapper::setup()' to warn earlier in the process.
To make all tests pass in signer registerer of aggregator. This is a temporary fix. This issue will be resolved in #663.
jpraynaud
force-pushed
the
jpraynaud/621-decommission-unverified-signer-registration
branch
from
b83300e
to
fcd16bb
Compare
Avoid unwanted usage of features when building the artifacts from the workspace. Split in 2 phases: tooling first and then distribution.
jpraynaud
force-pushed
the
jpraynaud/621-decommission-unverified-signer-registration
branch
from
fcd16bb
to
16cf8fe
Compare
jpraynaud
temporarily deployed
to
testing-preview
GitHub Actions
Inactive
Also remove the 'allow_skip_signer_certification' feature that was misused. Use directly 'mithril-stm' as dependency in this crate to retrieve protocol types.
Alenar
approved these changes
Dec 20, 2022
jpraynaud
temporarily deployed
to
testing-preview
GitHub Actions
Inactive
ghubertpalo
approved these changes
Dec 20, 2022
jpraynaud
deleted the
jpraynaud/621-decommission-unverified-signer-registration
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Content
This PR decommissions the declarative
PoolIdsigner registration mode. Once this PR is merged SPOs will be required to provide theirOperational CertificateandKES Secret Keyto certify that they are the owner of aPoolId.Pre-submit checklist
Issue(s)
Closes #621