-
Notifications
You must be signed in to change notification settings - Fork 372
feat(registry): CRP-2618 allow omitting pre_signatures_to_create_in_advance for keys that don't require it #7859
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
feat(registry): CRP-2618 allow omitting pre_signatures_to_create_in_advance for keys that don't require it #7859
Conversation
…ance for keys that don't require it
This reverts commit c8d9758.
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.
This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):
-
Update
unreleased_changelog.md(if there are behavior changes, even if they are
non-breaking). -
Are there BREAKING changes?
-
Is a data migration needed?
-
Security review?
How to Satisfy This Automatic Review
-
Go to the bottom of the pull request page.
-
Look for where it says this bot is requesting changes.
-
Click the three dots to the right.
-
Select "Dismiss review".
-
In the text entry box, respond to each of the numbered items in the previous
section, declare one of the following:
-
Done.
-
$REASON_WHY_NO_NEED. E.g. for
unreleased_changelog.md, "No
canister behavior changes.", or for item 2, "Existing APIs
behave as before.".
Brief Guide to "Externally Visible" Changes
"Externally visible behavior change" is very often due to some NEW canister API.
Changes to EXISTING APIs are more likely to be "breaking".
If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.
If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.
Reference(s)
For a more comprehensive checklist, see here.
GOVERNANCE_CHECKLIST_REMINDER_DEDUP
- Done (updated unreleased_changelog.md).
- No breaking changes.
- No data migration needed.
- No security review needed as not security critical.
…vance for vetKD (#7981) Adapts the registry canister and ic-admin so that for {Create|Update|Recover}SubnetPayload the `KeyConfig`'s `pre_signatures_to_create_in_advance` field is no longer needed/allowed for vetKD keys (and generally for keys that don't have pre-signatures). So far, this field was always required, and for vetKD keys this was confusing because vetKD doesn't have the concept of pre-signatures. The protobuf representation in the registry is unchanged, because there the `pre_signatures_to_create_in_advance` field was already optional. For existing registry entries for vetKD keys, the currently set value of 0 remains unchanged, while new entries that are created in the future will not have the field set any more. This change is possible now that the registry canister was recently adapted (see Proposal [139679](https://dashboard.internetcomputer.org/proposal/139679)) to allow omitting `pre_signatures_to_create_in_advance` for keys that don't require it ([#7859](#7859)). An alternative to this approach here would be to turn the `ChainKeyConfig`'s `KeyConfig` into a protobuf enum. However, this would require an involved multi-step migration of registry data structures.
Adapts the registry canister so that during subnet creation, subnet update, and subnet recovery, the
KeyConfig's fieldpre_signatures_to_create_in_advancecan be omitted for keys that do not have pre-signatures. Currently the only keys that do not require pre-signatures are VetKD keys (as opposed to Ecdsa/Schnorr keys). When the field is omitted, it is automatically set to zero. So far, because this field was always required, it was set to 0 manually for vetKD keys as part of proposal creation.Note that there is a registry invariant that requires this field to be non-zero for keys that do have pre-signatures. For keys that do not have pre-signatures, any value is permitted (i.e., there is no invariant). The replica simply ignores this field for vetKD.
This PR is a first step towards making it mandatory to omit the
pre_signatures_to_create_in_advancefield for vetKD keys. This step is necessary because our CI runs the system tests both agains the head NNS and the mainnet NNS, and directly omitting this field in the replica would make the system tests that run agains mainnet NNS fail.