From 222463858194b2f610bc70e3558dfa9de67c8c2b Mon Sep 17 00:00:00 2001 From: Leo Eichhorn Date: Mon, 5 Oct 2026 11:34:35 +0000 Subject: [PATCH] test(consensus): cover secp256r1 ECDSA keys in system tests #11573 added `EcdsaCurve::Secp256r1` but left the system tests and the CUP compatibility test on secp256k1. A secp256r1 key could not be decoded yet by the mainnet registry canister, which the default variant of every `system_test_nns` test installs, nor by the mainnet replica, which the upgrade/downgrade, backup and CUP compatibility tests run. Both now can: the mainnet registry canister is at 98c898f (#11757) and the mainnet replica at d26cd031, and both contain #11573. * `make_key_ids_for_all_schemes` and `make_key_ids_for_all_idkg_schemes` also return a secp256r1 key. It shares its name with the secp256k1 key, as keys of both curves would on mainnet, so code that tells ECDSA keys apart by name alone fails the tests. `make_ecdsa_key_id` becomes `make_ecdsa_secp256k1_key_id`, next to `make_ecdsa_secp256r1_key_id`. * `tecdsa_signature_life_cycle_test` enables the secp256r1 key later, on a subnet that already holds the secp256k1 key of the same name. * `ExhaustiveSet for EcdsaCurve` returns all curves again. Co-Authored-By: Claude Opus 5.5 --- .../tecdsa/tecdsa_performance_test_template.rs | 4 +++- .../tecdsa/tecdsa_signature_life_cycle_test.rs | 9 +++++---- rs/tests/consensus/tecdsa/utils/src/lib.rs | 17 ++++++++++++++--- rs/types/types/src/exhaustive.rs | 4 +--- 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/rs/tests/consensus/tecdsa/tecdsa_performance_test_template.rs b/rs/tests/consensus/tecdsa/tecdsa_performance_test_template.rs index bd1e26da2bc6..33ebdc333e83 100644 --- a/rs/tests/consensus/tecdsa/tecdsa_performance_test_template.rs +++ b/rs/tests/consensus/tecdsa/tecdsa_performance_test_template.rs @@ -163,7 +163,9 @@ fn make_key_ids() -> Vec { result.push(ic_consensus_threshold_sig_system_test_utils::make_eddsa_key_id()); } "ecdsa_secp256k1" => { - result.push(ic_consensus_threshold_sig_system_test_utils::make_ecdsa_key_id()); + result.push( + ic_consensus_threshold_sig_system_test_utils::make_ecdsa_secp256k1_key_id(), + ); } "vetkd_bls12_381_g2" => { result.push(ic_consensus_threshold_sig_system_test_utils::make_vetkd_key_id()); diff --git a/rs/tests/consensus/tecdsa/tecdsa_signature_life_cycle_test.rs b/rs/tests/consensus/tecdsa/tecdsa_signature_life_cycle_test.rs index b3c70d2d0417..cb243650608a 100644 --- a/rs/tests/consensus/tecdsa/tecdsa_signature_life_cycle_test.rs +++ b/rs/tests/consensus/tecdsa/tecdsa_signature_life_cycle_test.rs @@ -9,8 +9,8 @@ use ic_config::subnet_config::ECDSA_SIGNATURE_FEE; use ic_consensus_threshold_sig_system_test_utils::{ create_new_subnet_with_keys, empty_subnet_update, enable_chain_key_signing, execute_update_subnet_proposal, get_public_key_and_test_signature, get_public_key_with_retries, - get_signature_with_logger, make_bip340_key_id, make_ecdsa_key_id, make_eddsa_key_id, - scale_cycles, setup_without_ecdsa_on_nns, + get_signature_with_logger, make_bip340_key_id, make_ecdsa_secp256k1_key_id, + make_ecdsa_secp256r1_key_id, make_eddsa_key_id, scale_cycles, setup_without_ecdsa_on_nns, }; use ic_management_canister_types_private::{MasterPublicKeyId, VetKdCurve, VetKdKeyId}; use ic_nns_constants::GOVERNANCE_CANISTER_ID; @@ -75,18 +75,19 @@ fn test(env: TestEnv) { let nns_node = topology_snapshot.root_subnet().nodes().next().unwrap(); let nns_agent = nns_node.build_default_agent(); block_on(async move { - let key_id1 = make_ecdsa_key_id(); + let key_id1 = make_ecdsa_secp256k1_key_id(); let key_id2 = make_eddsa_key_id(); let key_id3 = make_bip340_key_id(); let key_id4 = make_vetkd_key_id("initial"); let key_id5 = make_vetkd_key_id("later"); + let key_id6 = make_ecdsa_secp256r1_key_id(); let nns = runtime_from_url(nns_node.get_public_url(), nns_node.effective_canister_id()); let governance = Canister::new(&nns, GOVERNANCE_CANISTER_ID); // Keys that are deployed to the subnet from the start let initial_key_ids = vec![key_id1.clone(), key_id2.clone(), key_id4.clone()]; let initial_key_ids_as_string = format!("[{key_id1}, {key_id2}, {key_id4}]"); // Keys that will be added later - let later_key_ids = vec![key_id3.clone(), key_id5.clone()]; + let later_key_ids = vec![key_id3.clone(), key_id5.clone(), key_id6.clone()]; // All key IDs let mut all_key_ids = later_key_ids.clone(); all_key_ids.extend(initial_key_ids.iter().cloned()); diff --git a/rs/tests/consensus/tecdsa/utils/src/lib.rs b/rs/tests/consensus/tecdsa/utils/src/lib.rs index 5554623d02e5..3723784ed3ff 100644 --- a/rs/tests/consensus/tecdsa/utils/src/lib.rs +++ b/rs/tests/consensus/tecdsa/utils/src/lib.rs @@ -71,13 +71,22 @@ pub fn make_key(name: &str) -> EcdsaKeyId { } } -pub fn make_ecdsa_key_id() -> MasterPublicKeyId { +pub fn make_ecdsa_secp256k1_key_id() -> MasterPublicKeyId { MasterPublicKeyId::Ecdsa(EcdsaKeyId { curve: EcdsaCurve::Secp256k1, name: "some_ecdsa_key".to_string(), }) } +/// Shares its name with the secp256k1 key, as keys of both curves would on +/// mainnet, so that tests catch code telling ECDSA keys apart by name alone. +pub fn make_ecdsa_secp256r1_key_id() -> MasterPublicKeyId { + MasterPublicKeyId::Ecdsa(EcdsaKeyId { + curve: EcdsaCurve::Secp256r1, + name: "some_ecdsa_key".to_string(), + }) +} + pub fn make_eddsa_key_id() -> MasterPublicKeyId { MasterPublicKeyId::Schnorr(SchnorrKeyId { algorithm: SchnorrAlgorithm::Ed25519, @@ -101,7 +110,8 @@ pub fn make_vetkd_key_id() -> MasterPublicKeyId { pub fn make_key_ids_for_all_schemes() -> Vec { vec![ - make_ecdsa_key_id(), + make_ecdsa_secp256k1_key_id(), + make_ecdsa_secp256r1_key_id(), make_bip340_key_id(), make_eddsa_key_id(), make_vetkd_key_id(), @@ -110,7 +120,8 @@ pub fn make_key_ids_for_all_schemes() -> Vec { pub fn make_key_ids_for_all_idkg_schemes() -> Vec { vec![ - make_ecdsa_key_id(), + make_ecdsa_secp256k1_key_id(), + make_ecdsa_secp256r1_key_id(), make_bip340_key_id(), make_eddsa_key_id(), ] diff --git a/rs/types/types/src/exhaustive.rs b/rs/types/types/src/exhaustive.rs index 2b7eca9b6338..8f177e0fedff 100644 --- a/rs/types/types/src/exhaustive.rs +++ b/rs/types/types/src/exhaustive.rs @@ -368,9 +368,7 @@ impl ExhaustiveSet for ReplicaVersion { impl ExhaustiveSet for EcdsaCurve { fn exhaustive_set(_: &mut R) -> Vec { - // A replica on the old version cannot decode a CUP naming Secp256r1. - // Restore `EcdsaCurve::iter()` once an r1 key exists in the registry. - vec![EcdsaCurve::Secp256k1] + EcdsaCurve::iter().collect() } }