Skip to content
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): Obsolete legacy ECDSA configs #3709

Merged
merged 9 commits into from
Feb 5, 2025
Merged

Conversation

aterga
Copy link
Member

@aterga aterga commented Jan 31, 2025

This PR is a follow-up to the chain-key config generalization worked that took place last year. Now that chain key has worked on the mainnet for a few months, the clients are ready, we simplify the API.

Notes on changed tests:

  • Most tests were generalized from legacy ECDSA to chain key in scope of the original work, so the remaining legacy ones are deleted in this PR, not ported (those were marked TODO[NNS1-3102]: Remove this test.).
  • Since we no longer need legacy ECDSA <--> chain key conversions, the corresponding tests were removed.
  • Tests that covered legacy ECDSA and chain key configs being specified together were removed, as not legacy ECDSA is always forbidden.
  • In one instance, I de-duplicate tests to avoid obvious redundancy (test_disallow_chain_key_signing_disable_and_enable_for_same_key and enable_and_disable_signing_lists_should_not_have_same_keys_in_single_request)

@github-actions github-actions bot added the feat label Jan 31, 2025
@aterga aterga force-pushed the arshavir/NNS1-3022 branch from ddf7002 to 29f7e20 Compare February 4, 2025 13:40
@aterga aterga marked this pull request as ready for review February 4, 2025 21:47
@aterga aterga requested review from a team as code owners February 4, 2025 21:47
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this pull request affects the behavior of any canister owned by
the Governance team, remember to update the corresponding
unreleased_changes.md file(s).

To acknowldge this reminder (and unblock the PR), dismiss this
code review by going to the bottom of the pull request page, and
supply one of the following reasons:

  1. Done.

  2. No canister behavior changes.

@aterga aterga enabled auto-merge February 5, 2025 16:54
@aterga aterga added this pull request to the merge queue Feb 5, 2025
Merged via the queue into master with commit b803bf0 Feb 5, 2025
30 checks passed
@aterga aterga deleted the arshavir/NNS1-3022 branch February 5, 2025 18:00
marko-k0 added a commit that referenced this pull request Feb 6, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
This reverts commit b803bf0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants