Repository navigation
feat(crypto): add secp256r1 variant to EcdsaCurve - #11573
Merged
Merged
Conversation
### Why * The threshold crypto layer already implements ECDSA on NIST P-256. `EccCurveType` has a `P256` variant, `AlgorithmId` has `ThresholdEcdsaSecp256r1`, and the IDkg code paths are curve-generic, with no `todo!` or `unimplemented!` left on a P256 arm. * The public `EcdsaCurve` enum has a single variant, so a `secp256r1` key id cannot be named in the management canister API or represented in the registry. The curve is unreachable from outside the crypto layer. * P-256 is the curve the web platform standardised on: WebAuthn, TLS, VAPID web push (ES256) and OAuth DPoP. A canister cannot produce any of those signatures today, so each one needs a key held off-chain. ### What * `EcdsaCurve` gains `Secp256r1` in `rs/types/management_canister_types` and in the published `packages/ic-management-canister-types`, together with the Candid doc comment, the `u32` conversions that decode the `ic0.cost_sign_with_ecdsa` argument, and `FromStr`, which is what NNS proposals parse. * `From<EcdsaCurve> for AlgorithmId` in `rs/types/types/src/crypto.rs` maps the new variant to `AlgorithmId::ThresholdEcdsaSecp256r1`. This is the single conversion that hands a key id to the crypto layer. * `types.proto` gains `ECDSA_CURVE_SECP256R1 = 2`, so an `EcdsaKeyId` round-trips through the registry, plus the regenerated files. * Both `tests/ic.did` fixtures gain the variant. That is what the Candid equality tests compare the Rust enum against. Two values need confirming before this merges: `u32` = 1 and protobuf = 2. Each is the next free value in its own scheme, but the `u32` is normative because it is the System API ABI in the interface spec. The variant has to land in one commit. Adding it to either the proto or a types crate on its own breaks the other's exhaustive match, so no split compiles. This generates no key and makes no key id available. Enabling a key is a separate registry and governance step. `rs/tests/consensus/tecdsa` is deliberately not extended. `cast_ecdsa_key_id` casts into the published `ic-management-canister-types` through cdk-rs, so r1 system tests need that crate released first.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Exhaustive matches and hand-maintained Candid interfaces remain stale, causing compilation and interface-equality test failures.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds public secp256r1/P-256 support to ECDSA key identifiers and connects it to threshold crypto.
Changes:
- Adds
Secp256r1to public/private management types and Candid fixtures. - Maps the curve to its System API, protobuf, and crypto identifiers.
- Regenerates protobuf bindings and updates orchestrator test utilities.
File summaries
| File | Description |
|---|---|
rs/types/types/src/crypto.rs |
Maps r1 to its threshold algorithm. |
rs/types/management_canister_types/tests/ic.did |
Updates private Candid fixture. |
rs/types/management_canister_types/src/lib.rs |
Defines conversions and parsing. |
rs/protobuf/def/types/v1/types.proto |
Adds protobuf enum value. |
rs/protobuf/src/gen/types/types.v1.rs |
Regenerates types binding. |
rs/protobuf/src/gen/state/types.v1.rs |
Regenerates state binding. |
rs/protobuf/src/gen/registry/types.v1.rs |
Regenerates registry binding. |
rs/protobuf/src/gen/messaging/types.v1.rs |
Regenerates messaging binding. |
rs/protobuf/src/gen/crypto/types.v1.rs |
Regenerates crypto binding. |
rs/orchestrator/src/upgrade.rs |
Generates r1 test transcripts. |
packages/ic-management-canister-types/tests/ic.did |
Updates published Candid fixture. |
packages/ic-management-canister-types/src/lib.rs |
Exposes r1 and its ABI value. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The variant commit missed every consumer outside the type crates, because the sweep that scoped it excluded test files and searched only the lowercase `ecdsa_curve` spelling. `verify_signature` and `cast_ecdsa_key_id` in the tecdsa system-test utils match on `ic_management_canister_types_private::EcdsaCurve`, so both became non-exhaustive and that library stopped compiling. `verify_signature` gains a P-256 verifier mirroring the k1 one. `cast_ecdsa_key_id` cannot be completed: it casts into `ic_cdk_management_canister::EcdsaCurve`, which re-exports `ic-management-canister-types` 0.7.1 from crates.io and is secp256k1-only, so the arm is `unimplemented!` naming that blocker. `UpdateSubnetPayload` carries `Option<Vec<MasterPublicKeyId>>`, so the new variant reaches the Candid of the registry canister and of the engine controller, which forwards that payload. Their hand-maintained `.did` files are checked by `service_equal`, which fails with "Left side is missing secp256r1" until they declare it. `registry_test.did` too. `should_fail_if_unkown_ecdsa_curve` used protobuf value 2 as its unknown curve. That is now `ECDSA_CURVE_SECP256R1`, so the conversion succeeds and the test no longer panics. The sentinel moves to 3, which is how the Schnorr sibling already picks the first value past the last valid one. The published crate's changelog records the addition under `Unreleased`. `signer.did` and the management-canister bench `candid.did` are deliberately unchanged. Both canisters type their arguments with the crates.io `ic_cdk_management_canister`, so their interfaces stay secp256k1-only until that crate is released.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The public ABI value still requires external specification confirmation, and a newly added runtime path intentionally panics until a dependency release is available.
Review details
- Files reviewed: 20/21 changed files
- Comments generated: 0 new
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
`cup_compatibility_test` failed on the variant commit, in two independent ways. Both are fixed here, and the second is the interesting one. `derive(Hash)` omits the discriminant while an enum has a single variant and starts writing it as soon as a second one exists. `EcdsaCurve` reaches `payload_hash` through `IDkgPayload`, so adding the variant silently changed the hash of every block payload that names an ECDSA key, even though the serialized payload was byte-identical. A replica on the old version recomputes the old hash and rejects the block, which is a divergence during a rollout and not only a rollback problem. `EcdsaCurve` is therefore hashed by hand, with `Secp256k1` contributing nothing exactly as it did before. Separately, `ExhaustiveSet for EcdsaCurve` iterated the variants, so the generated CUP artifacts named `secp256r1` and the old version could not decode them at all. The variant is withheld until the rollout that generates an r1 key. Both were needed. Withholding the variant alone still failed the integrity check 5 times out of 5, and preserving the hash alone failed to decode with `Unable to convert 2 to an EcdsaCurve`. Verified against the parent commit's `types_test` binary: artifacts from this tree are byte-identical to the parent's under a pinned seed, and the parent binary deserializes them in 5 of 5 random-seed runs. The hand-written `Hash` needs the crypto team's sign-off, since `crypto_hash` asks for that on anything it hashes. It is also easy to undo by accident: a later `derive(Hash)` here would move consensus hashes with nothing to catch it except this test.
Until now the only thing catching a `derive(Hash)` put back on `EcdsaCurve` was `cup_compatibility_test`, a Bazel system test that needs the mainnet binaries and takes minutes. This pins the property directly: hashing `Secp256k1` writes no bytes, hashing `Secp256r1` writes some. Against the regression it fails immediately with `left: 8, right: 0`, the eight bytes being the derived discriminant.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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
|
✅ No security or compliance issues detected. Reviewed everything up to a962a02. Security OverviewDetected Code Changes
|
|
✅ No security or compliance issues detected. Reviewed everything up to a962a02. Security OverviewDetected Code Changes
|
ci/scripts/rust-lint.sh denies clippy::unseparated_literal_suffix.
eichhorl
reviewed
Sep 16, 2026
The previous marker byte was forgeable. `1_u8` for `Secp256r1` is
indistinguishable from a `name` whose first byte is `0x01`, so these two key
ids fed the hasher the identical stream `[01, 6b, 65, 79, 5f, 31, ff]`:
EcdsaKeyId { curve: Secp256r1, name: "key_1" }
EcdsaKeyId { curve: Secp256k1, name: "\u{1}key_1" }
That is a collision inside a type reaching `payload_hash`, so it goes.
No standalone `impl` on `EcdsaCurve` can fix this, because whatever byte it
picks the enclosing struct's next field can reproduce. Only the enclosing type
knows what follows the curve, so `EcdsaKeyId` hashes it and `EcdsaCurve`
implements no `Hash` at all, with the reasoning recorded on both. The marker is
now `0xfe`, which never begins a UTF-8 sequence and so cannot come out of a
`str`. The impl destructures `Self` exhaustively and binds `name` as `&str`, so
a new field or a `name` that stops being text fails to compile rather than
quietly weakening that argument.
`Secp256k1` still writes nothing, so existing hashes are unchanged: artifacts
are byte-identical to the pre-variant binary under a pinned seed, 48 of 48, and
round-trip through it on 5 of 5 random seeds.
Deriving `Hash` over both variants remains the right end state and needs its own
staged rollout.
`get_ecdsa_public_key_with_retries` validated every returned key as secp256k1, so an r1 key would have failed the check on a point that is perfectly valid on P-256. It already takes the `EcdsaKeyId`, so it now branches on the curve. Both verifiers are also named after their curve now, since `verify_ecdsa_signature` sitting next to `verify_ecdsa_secp256r1_signature` read as if it covered both.
`fake_master_public_key_ids_for_all_idkg_algorithms` skipped `ThresholdEcdsaSecp256r1`, so every consensus test built on it exercised one ECDSA curve only. Add the arm, with a `_with_curve` variant of each fake key id helper behind it; the existing no-argument helpers keep their signature and their secp256k1 result, so their 56 callers are untouched. The fourth key id is a second ECDSA one, and an ECDSA pre-signature costs two transcripts of payload capacity against a Schnorr one's single transcript. A round of the priority queue therefore went from 4 capacity to 6, and `test_pre_signatures_are_started_up_to_payload_capacity` asserted an even split across keys at a hardcoded capacity of 20. That is 5 whole rounds for three keys but 3 rounds and a remainder for four, and the remainder lands on one arbitrary key. Derive the capacity from the key set so the even split is a property of the allocator rather than of the constant.
Same facts, fewer words. 26 lines of comment become 15.
eichhorl
approved these changes
Sep 16, 2026
pietrodimarco-dfinity
approved these changes
Sep 22, 2026
daniel-wong-dfinity-org-twin
approved these changes
Sep 22, 2026
MRmarioruci
dismissed
github-actions[bot]’s stale review
September 22, 2026 11:02
Done. rs/registry/canister/unreleased_changelog.md has an entry under Added.
Not breaking. didc check passes against master. The curve does reach a response type through get_subnet → SubnetRecord.chain_key_config, but every enclosing field is opt, so a client on the old interface decodes an r1 key_id as null rather than failing, and only once an r1 key exists in a subnet record.
No. No stored format changes, nothing to rewrite.
Done. One hazard found and fixed. derive(Hash) omits an enum's discriminant while it has a single variant and starts writing it once a second exists, so adding the variant moved payload_hash for every block naming an ECDSA key, and replicas either side of the upgrade would reject each other's CUPs. Caught by cup_compatibility_test, fixed in ff7d234 by hashing the curve from EcdsaKeyId, and secp256r1 is withheld from ExhaustiveSet until an r1 key exists in the registry.
claude Bot
pushed a commit
that referenced
this pull request
Sep 22, 2026
One conflict, in `rs/engine_controller/engine_controller.did`: master's #11573 added the `secp256r1` variant to `EcdsaCurve` on the line directly below the doc comment this branch rewrote. Kept both — master's `variant { secp256k1; secp256r1 }` verbatim, and this branch's comment listing `cooling_down` among the settable fields. `rs/registry/canister/unreleased_changelog.md` auto-merged: master's `secp256r1` entry under "Added", this branch's `cooling_down` entry under "Changed". Master added no field to `UpdateSubnetPayload`, so the two exhaustively destructuring allowlists are unaffected: all 32 fields are still bound exactly once, with `subnet_id`, `subnet_admins`, `is_halted` and `cooling_down` allowed and the other 28 rejected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpdqCjCv7jjrGQRwgcqpAV
alin-at-dfinity
pushed a commit
that referenced
this pull request
Sep 22, 2026
### Why * The threshold crypto layer already implements ECDSA on NIST P-256. [`EccCurveType`](https://github.com/dfinity/ic/blob/master/rs/crypto/internal/crypto_lib/threshold_sig/canister_threshold_sig/src/utils/group.rs#L25) has a `P256` variant, `AlgorithmId` has `ThresholdEcdsaSecp256r1`, and the IDkg code paths are curve-generic, with no `todo!` or `unimplemented!` left on a P256 arm. * The public [`EcdsaCurve`](https://github.com/dfinity/ic/blob/master/rs/types/management_canister_types/src/lib.rs#L2892) enum has a single variant, so a `secp256r1` key id cannot be named in the management canister API or represented in the registry. The curve is unreachable from outside the crypto layer. * P-256 is the curve the web platform standardised on: WebAuthn, TLS, VAPID web push (ES256) and OAuth DPoP. A canister cannot produce any of those signatures today, so each one needs a key held off-chain. For Internet Identity's web push work that key is the blocker. ### What `EcdsaCurve` gains `Secp256r1` in `rs/types/management_canister_types` and in the published `packages/ic-management-canister-types`, together with the Candid doc comment, the `u32` conversions that decode the `ic0.cost_sign_with_ecdsa` argument, and `FromStr`, which is what NNS proposals parse. [`From<EcdsaCurve> for AlgorithmId`](https://github.com/dfinity/ic/blob/master/rs/types/types/src/crypto.rs#L277) maps the new variant to `AlgorithmId::ThresholdEcdsaSecp256r1`; that is the single conversion handing a key id to the crypto layer. `types.proto` gains `ECDSA_CURVE_SECP256R1 = 2` so an `EcdsaKeyId` round-trips through the registry, plus the regenerated files. Both `tests/ic.did` fixtures gain the variant, which is what the Candid equality tests compare the Rust enum against. Two values need confirming before this merges: `u32` = 1 and protobuf = 2. Each is the next free value in its own scheme, but the `u32` is normative because it is the System API ABI in the interface spec. The matching spec change is dfinity/developer-docs#394. The variant has to land in one commit. Adding it to either the proto or a types crate on its own breaks the other's exhaustive match, so no split compiles. This generates no key and makes no key id available. Enabling a key is a separate registry and governance step. `rs/tests/consensus/tecdsa` is deliberately not extended. `cast_ecdsa_key_id` casts into the published `ic-management-canister-types` through cdk-rs, so r1 system tests need that crate released first, and `verify_signature` needs a P-256 verifier. Happy to follow up with that once the crate is out. ### Tests `cargo check --tests`, the 39 unit tests and both `candid_equality_test`s pass on `ic-management-canister-types`, `ic-management-canister-types-private`, `ic-types` and `ic-protobuf`. `cargo run -p ic-protobuf-generator` reproduces the five generated files exactly. Two gaps CI needs to cover, since neither is reachable on macOS: `rs/orchestrator` does not build there (`libc::gettid`, `SYS_futex`, `libcryptsetup-rs`), so its test arm is unverified, and a match in any other Linux-only crate would not have shown up locally. Unrelated, but worth flagging: running the protobuf generator also rewrites doc-comment escaping in six generated files it does not otherwise touch (`\<=` becomes `<=`, list renumbering). That looks like prost-build 0.14.3 disagreeing with whatever version produced the committed files. Reverted here, but the next regen will hit it.
This was referenced Sep 25, 2026
valorjesson-commits
pushed a commit
to valorjesson-commits/ic
that referenced
this pull request
Oct 3, 2026
Bump the crate version to 0.11.0 and close out the CHANGELOG's [Unreleased] section, which collects since v0.10.0: - the `Secp256r1` variant of `EcdsaCurve`, selecting threshold ECDSA on the NIST P-256 curve (dfinity#11573) - the `million_round_instructions_total` field of `SubnetMetricsResult` (dfinity#11586) It also records, as a doc change, how far back the `CyclesConsumed` amounts served by `canister_metrics` reach (dfinity#11585). Neither `EcdsaCurve` nor `SubnetMetricsResult` is `#[non_exhaustive]`, so the new variant breaks exhaustive matches downstream and the new field breaks struct literals, hence the minor bump. --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
valorjesson-commits
pushed a commit
to valorjesson-commits/ic
that referenced
this pull request
Oct 5, 2026
…11741) ### Why dfinity#11573 added `EcdsaCurve::Secp256r1`, but PocketIC cannot sign with a secp256r1 key: * its subnets only hold secp256k1 ECDSA keys, and * the StateMachine's `MasterPublicKeyId::Ecdsa(id)` arm ignores `id.curve`, so a secp256r1 key id would silently get a secp256k1 key. The execution side already works: `ecdsa_public_key` derives secp256r1 keys through `derive_threshold_public_key`, which handles `ThresholdEcdsaSecp256r1`. ### What * **StateMachine** (`rs/state_machine_tests`): a secp256r1 key id gets a P-256 key. Like the secp256k1 keys, it is derived from a fixed seed and the key name, so all PocketIC subnets share it. `sign_with_ecdsa` signs with it and normalizes `s` to the lower half of the range. The IC does this for all curves (and its verifier rejects a high `s`), but `ic-secp256r1` does not. * **PocketIC server**: every existing ECDSA key name also gets a secp256r1 key: `key_1` on the II and fiduciary subnets, and `test_key_1` and `dfx_test_key` on the test threshold keys subnet. This keeps the HOWTO's "holds keys … for all algorithms" true. * CHANGELOG and HOWTO updated. No secp256r1 keys with names `key_1` or `test_key_1` exist on mainnet yet, so PocketIC holds these keys before mainnet does. The CHANGELOG entry says so. ### Tests `test_ecdsa` now also covers secp256r1 for `key_1`, `test_key_1` and `dfx_test_key`. For each key, the test fetches the public key, signs 4 message hashes, verifies each signature with the `p256` crate and asserts that `s` is normalized. Verifying against the public key from `ecdsa_public_key` also checks that the StateMachine's signing key matches the key derived by execution. The test canister's typed ECDSA endpoints cannot name secp256r1, because the published `ic-cdk-management-canister` does not have the variant yet. The test therefore calls the management canister through the canister's existing `proxy_call` endpoint, and the test canister's wasm is unchanged. The test fails if the `s` normalization is removed. It signs 4 hashes per key because with a single hash per key, all three signatures already had a low `s` and the assertion did not catch the removal. Locally, all 213 bazel tests that depend on the changed files at depth 2 pass, except for 5 golden-state tests. Those fail before their test logic runs because they cannot scp the state snapshot without credentials. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Rachit2323
pushed a commit
to Rachit2323/ic
that referenced
this pull request
Oct 5, 2026
…11762) ### Why dfinity#11573 added `EcdsaCurve::Secp256r1` but left the system tests and the CUP compatibility test on secp256k1, because a secp256r1 key could not yet be decoded by the mainnet versions they run: * The default (non-`_head_nns`) variant of every `system_test_nns` test installs the **mainnet NNS canisters**. The mainnet registry canister (237a41f) rejected protobuf curve `2` ("Unable to convert 2 to an EcdsaCurve"), so any proposal adding a secp256r1 key failed. * `upgrade_downgrade_*_app_subnet_test`, `backup_manager_{up,down}grade_test` and `cup_compatibility_test` run the **mainnet replica** (or its `ic-replay` / `types-test` binaries). Both now support it: dfinity#11757 moved the mainnet registry canister to 98c898f, and every mainnet subnet in `mainnet-icos-revisions.json` runs d26cd03. Both contain dfinity#11573. ### What * `make_key_ids_for_all_schemes()` and `make_key_ids_for_all_idkg_schemes()` also return a secp256r1 key. The tests that use them now cover it too: the tecdsa tests, subnet recovery, backup, upgrade/downgrade and `rotate_ecdsa_idkg_key_test`. * The secp256r1 key shares its name (`some_ecdsa_key`) with the secp256k1 key, as keys of both curves would on mainnet. Code that tells ECDSA keys apart by name alone therefore fails the tests. * `make_ecdsa_key_id` is renamed to `make_ecdsa_secp256k1_key_id`, next to the new `make_ecdsa_secp256r1_key_id`. * `tecdsa_signature_life_cycle_test` adds the secp256r1 key to the keys enabled later, on a subnet that already holds the secp256k1 key of the same name. This is how it would be rolled out on mainnet. The test checks that requests for the key are rejected before it is enabled, signs with it, reshares it to a new subnet and disables it. * `ExhaustiveSet for EcdsaCurve` returns all curves again. `cup_compatibility_test` therefore checks that the mainnet replica decodes CUPs naming a secp256r1 key. ### Not in this PR * The secp256r1 arm of `cast_ecdsa_key_id` stays `unimplemented!`. * The signer canister's `signer.did` and the management canister bench's `candid.did` stay secp256k1-only. Both canisters type their ECDSA arguments with `ic-cdk-management-canister`. Its latest release (0.2.0) still requires `ic-management-canister-types ^0.10`, and cdk-rs `main` pins `0.10`. Version 0.11.0, the first with `Secp256r1`, was published only today. The only test that reaches this path is the manual `tecdsa_performance_test_template`, through `large_ecdsa_method_and_payload`. ### Tests Run locally in the dev container: * `cargo check`, `cargo fmt` and `ci/scripts/rust-lint.sh` are clean. * The CLAUDE.md depth-2 affected-test query (412 targets) misses most of the system tests: they reach the changed lib through their `_bin` target, one hop further, and `cup_compatibility_test` only uses `types_test` as data. So I added the `_farm` variants of every system test that calls the changed helpers or reads `exhaustive.rs`: 56 targets, covering both the mainnet-NNS and the `_head_nns` variant. Of the 466 targets in one run, 456 passed, including all 58 system-test targets: * every tecdsa test * `backup_manager_{up,down}grade_test` * `rotate_ecdsa_idkg_key_test` * both `upgrade_downgrade_*app_subnet_test` targets * all `sr_app_*` subnet recovery tests * `cup_compatibility_test` * all four `tecdsa_signature_life_cycle_test` variants The 10 failures are unrelated: * The 2 golden-state tests can't fetch the state snapshot without the SSH key they need. * 7 tests (4 timeouts, 3 failures) had run next to 55 Farm system tests, and passed when rerun alone with `--local_test_jobs=2`. * `//rs/pocket_ic_server:external_types_tests` also fails on master e8fd3a6: Internet Identity's candid gained fields with the release bump in dfinity#11757. Serializing the exhaustive CUP set here and on master shows 22 ECDSA key ids switching from curve 1 to curve 2. So `cup_compatibility_test` now has the mainnet `types-test` decode CUPs naming secp256r1 keys. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
mraszyk
added a commit
to dfinity/developer-docs
that referenced
this pull request
Oct 8, 2026
…t canister reference The changelog entry sat under 0.68.0 (2026-09-14), but dfinity/ic#11573 merged on 2026-09-22, so it belongs in the first section after that, 0.69.0 (2026-09-28). docs/references/management-canister.md stated three interface facts for secp256k1 only: the supported curves, the derivation scheme, and the 64-byte signature. Each now covers secp256r1 as well. The curve list links to the deployed-keys table, so it does not read as a claim that a P-256 key exists on mainnet.
mraszyk
added a commit
to dfinity/developer-docs
that referenced
this pull request
Oct 8, 2026
The entry now opens its own section, 0.70.0 (2026-10-05). That is the day the last subnet, the NNS subnet, upgraded to a replica version that contains dfinity/ic#11573 (proposal 144203). 0.69.0 (2026-09-28) predates the full rollout.
mraszyk
added a commit
to dfinity/developer-docs
that referenced
this pull request
Oct 8, 2026
New variant `secp256r1` of `ecdsa_curve`, selecting threshold ECDSA on the NIST P-256 curve in `ecdsa_public_key` and `sign_with_ecdsa`. P-256 is the curve the web platform standardised on: WebAuthn, TLS, VAPID web push (ES256) and OAuth DPoP. A canister cannot produce any of those signatures today, so each one needs a key held off-chain. ## Changes Key derivation for the new curve is the same generalization of BIP32 instantiated over P-256, which is SLIP-10. [`signing/key_derivation.rs:210`](https://github.com/dfinity/ic/blob/master/rs/crypto/internal/crypto_lib/threshold_sig/canister_threshold_sig/src/signing/key_derivation.rs#L210) dispatches `Ed25519 => eddsa_ckd, _ => bip32_ckdpub`, so K256 and P256 run the same code, and `bip32_ckdpub` documents itself as following SLIP-0010 with BIP32 identical for K256. The derivation paragraph is split into one bullet per curve, matching how `schnorr_public_key` already presents its two algorithms. Signatures are encoded as for `secp256k1`, because `EccCurveType::scalar_bytes` is 32 on both curves, so r and s are 32 bytes each. The `ecdsa_curve` argument of `ic0.cost_sign_with_ecdsa` accepts `1`, matching `TryFrom<u32> for EcdsaCurve` in dfinity/ic#11573. The changelog bullet opens a new section, `0.70.0 (2026-10-05)`. That is the day dfinity/ic#11573 finished rolling out: the last subnet (the NNS subnet, proposal 144203) upgraded to a replica version containing it. `docs/references/management-canister.md` gets the same three facts: the supported curves, the SLIP-10 derivation, and the 64-byte signature. Its curve list links to the deployed-keys table, so it does not read as a claim that a P-256 key exists on mainnet. This describes the interface, not availability. Whether a given `key_id` exists depends on the implementation, so the pages that list the keys deployed on mainnet are unchanged. Those should change when a key is generated. Paired with dfinity/ic#11573, which adds the variant to the implementation. It merged on 2026-09-22 and has been on every subnet since 2026-10-05. ## Tests `didc check public/references/ic.did` passes, and `node scripts/validate.js` passes on the four changed markdown files, and the production build passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Martin Raszyk <martin.raszyk@dfinity.org>
3 tasks done
This file contains hidden or 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
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.
Why
EccCurveTypehas aP256variant,AlgorithmIdhasThresholdEcdsaSecp256r1, and the IDkg code paths are curve-generic, with notodo!orunimplemented!left on a P256 arm.EcdsaCurveenum has a single variant, so asecp256r1key id cannot be named in the management canister API or represented in the registry. The curve is unreachable from outside the crypto layer.What
EcdsaCurvegainsSecp256r1inrs/types/management_canister_typesand in the publishedpackages/ic-management-canister-types, together with the Candid doc comment, theu32conversions that decode theic0.cost_sign_with_ecdsaargument, andFromStr, which is what NNS proposals parse.From<EcdsaCurve> for AlgorithmIdmaps the new variant toAlgorithmId::ThresholdEcdsaSecp256r1; that is the single conversion handing a key id to the crypto layer.types.protogainsECDSA_CURVE_SECP256R1 = 2so anEcdsaKeyIdround-trips through the registry, plus the regenerated files. Bothtests/ic.didfixtures gain the variant, which is what the Candid equality tests compare the Rust enum against.Two values need confirming before this merges:
u32= 1 and protobuf = 2. Each is the next free value in its own scheme, but theu32is normative because it is the System API ABI in the interface spec. The matching spec change is dfinity/developer-docs#394.The variant has to land in one commit. Adding it to either the proto or a types crate on its own breaks the other's exhaustive match, so no split compiles.
This generates no key and makes no key id available. Enabling a key is a separate registry and governance step.
rs/tests/consensus/tecdsais deliberately not extended.cast_ecdsa_key_idcasts into the publishedic-management-canister-typesthrough cdk-rs, so r1 system tests need that crate released first, andverify_signatureneeds a P-256 verifier. Happy to follow up with that once the crate is out.Tests
cargo check --tests, the 39 unit tests and bothcandid_equality_tests pass onic-management-canister-types,ic-management-canister-types-private,ic-typesandic-protobuf.cargo run -p ic-protobuf-generatorreproduces the five generated files exactly.Two gaps CI needs to cover, since neither is reachable on macOS:
rs/orchestratordoes not build there (libc::gettid,SYS_futex,libcryptsetup-rs), so its test arm is unverified, and a match in any other Linux-only crate would not have shown up locally.Unrelated, but worth flagging: running the protobuf generator also rewrites doc-comment escaping in six generated files it does not otherwise touch (
\<=becomes<=, list renumbering). That looks like prost-build 0.14.3 disagreeing with whatever version produced the committed files. Reverted here, but the next regen will hit it.