diff --git a/Cargo.lock b/Cargo.lock index 7880ac5f52c..3692d766812 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6807,6 +6807,7 @@ dependencies = [ "ic-interfaces-registry", "ic-interfaces-registry-mocks", "ic-logger", + "ic-management-canister-types", "ic-metrics", "ic-protobuf", "ic-registry-client", @@ -7148,6 +7149,7 @@ dependencies = [ "ic-crypto-utils-basic-sig", "ic-interfaces", "ic-logger", + "ic-management-canister-types", "ic-metrics", "ic-protobuf", "ic-sys", diff --git a/rs/consensus/src/consensus/batch_delivery.rs b/rs/consensus/src/consensus/batch_delivery.rs index bf88a935c69..5bb79ad829b 100644 --- a/rs/consensus/src/consensus/batch_delivery.rs +++ b/rs/consensus/src/consensus/batch_delivery.rs @@ -308,7 +308,7 @@ pub fn generate_responses_to_setup_initial_dkg_calls( for (id, callback_id, transcript) in transcripts_for_remote_subnets.iter() { let add_transcript = |transcript_results: &mut TranscriptResults| { let value = Some(transcript.clone()); - match id.dkg_tag { + match &id.dkg_tag { NiDkgTag::LowThreshold => { if transcript_results.low_threshold.is_some() { error!( @@ -327,6 +327,12 @@ pub fn generate_responses_to_setup_initial_dkg_calls( } transcript_results.high_threshold = value; } + NiDkgTag::HighThresholdForKey(master_public_key_id) => { + error!( + log, + "Implementation error: NiDkgTag::HighThresholdForKey({master_public_key_id}) used in SetupInitialDKG for callback ID {callback_id}", + ); + } } }; match transcripts.get_mut(callback_id) { diff --git a/rs/consensus/src/consensus/dkg_key_manager.rs b/rs/consensus/src/consensus/dkg_key_manager.rs index bb3e10aa4b4..4d95ead7b60 100644 --- a/rs/consensus/src/consensus/dkg_key_manager.rs +++ b/rs/consensus/src/consensus/dkg_key_manager.rs @@ -535,9 +535,12 @@ impl Drop for DkgKeyManager { /// Print the information about a [`NiDkgId`] in a concise way for logging fn dkg_id_log_msg(id: &NiDkgId) -> String { - let tag = match id.dkg_tag { - NiDkgTag::LowThreshold => "low", - NiDkgTag::HighThreshold => "high", + let tag = match &id.dkg_tag { + NiDkgTag::LowThreshold => "low".to_string(), + NiDkgTag::HighThreshold => "high".to_string(), + NiDkgTag::HighThresholdForKey(master_public_key_id) => { + format!("highForKey({master_public_key_id})") + } }; // If the target is local (which it is usually), we don't log the target diff --git a/rs/consensus/src/dkg/payload_builder.rs b/rs/consensus/src/dkg/payload_builder.rs index 55dd17c790e..f04f875717b 100644 --- a/rs/consensus/src/dkg/payload_builder.rs +++ b/rs/consensus/src/dkg/payload_builder.rs @@ -507,6 +507,21 @@ pub(crate) fn get_configs_for_local_transcripts( resharing_transcript.cloned(), ) } + //////////////////////////////////////////////////////////////////////////////// + // TODO: how to behave here? The code below is copied+adapted from the HighThreshold case. However, + // this code currently won't be executed because we iterate over TAGS, which is a const that does not + // and cannot contain an NiDkgTag::HighThresholdForKey entry + /////////////////////////////////////////////////////////////////////////////// + NiDkgTag::HighThresholdForKey(master_public_key_id) => { + let resharing_transcript = reshared_transcripts + .get(&NiDkgTag::HighThresholdForKey(master_public_key_id.clone())); + ( + resharing_transcript + .map(|transcript| transcript.committee.get().clone()) + .unwrap_or_else(|| node_ids.clone()), + resharing_transcript.cloned(), + ) + } }; let threshold = NumberOfNodes::from(tag.threshold_for_subnet_of_size(node_ids.len()) as u32); @@ -1206,9 +1221,13 @@ mod tests { assert_eq!(conf.max_corrupt_dealers().get(), 2); assert_eq!( conf.threshold().get().get(), - match *tag { + match tag { NiDkgTag::LowThreshold => 3, NiDkgTag::HighThreshold => 5, + ///////////////////////////////////////// + // TODO: check with Consensus team + NiDkgTag::HighThresholdForKey(_) => panic!("not applicable"), + ///////////////////////////////////////// } ); } @@ -1311,9 +1330,13 @@ mod tests { assert_eq!(conf.max_corrupt_dealers().get(), 2); assert_eq!( conf.threshold().get().get(), - match *tag { + match tag { NiDkgTag::LowThreshold => 3, NiDkgTag::HighThreshold => 5, + ///////////////////////////////////////// + // TODO: check with Consensus team + NiDkgTag::HighThresholdForKey(_) => panic!("not applicable"), + ///////////////////////////////////////// } ); diff --git a/rs/crypto/BUILD.bazel b/rs/crypto/BUILD.bazel index db9188c218f..411812b64ae 100644 --- a/rs/crypto/BUILD.bazel +++ b/rs/crypto/BUILD.bazel @@ -37,6 +37,7 @@ DEPENDENCIES = [ "//rs/registry/helpers", "//rs/registry/keys", "//rs/types/base_types", + "//rs/types/management_canister_types", "//rs/types/types", "@crate_index//:bincode", "@crate_index//:hex", diff --git a/rs/crypto/Cargo.toml b/rs/crypto/Cargo.toml index c7cec3750f3..a28c51b347b 100644 --- a/rs/crypto/Cargo.toml +++ b/rs/crypto/Cargo.toml @@ -31,6 +31,7 @@ ic-crypto-tls-cert-validation = { path = "node_key_validation/tls_cert_validatio ic-crypto-tls-interfaces = { path = "tls_interfaces" } ic-crypto-utils-basic-sig = { path = "utils/basic_sig" } ic-crypto-utils-tls = { path = "utils/tls" } +ic-management-canister-types = { path = "../types/management_canister_types" } ic-interfaces = { path = "../interfaces" } ic-interfaces-registry = { path = "../interfaces/registry" } ic-logger = { path = "../monitoring/logger" } diff --git a/rs/crypto/benches/ni_dkg.rs b/rs/crypto/benches/ni_dkg.rs index f0625f7ee80..52fabed5d8d 100644 --- a/rs/crypto/benches/ni_dkg.rs +++ b/rs/crypto/benches/ni_dkg.rs @@ -258,6 +258,7 @@ impl TestCase { let tag_name = match self.dkg_tag { NiDkgTag::LowThreshold => "low", NiDkgTag::HighThreshold => "high", + NiDkgTag::HighThresholdForKey(_) => unimplemented!(), }; format!( "crypto_nidkg_{}_nodes_{}_dealers_{}", @@ -281,6 +282,7 @@ impl TestCase { num_of_dealers: num_of_nodes, dkg_tag, }, + NiDkgTag::HighThresholdForKey(_) => unimplemented!(), } } } diff --git a/rs/crypto/internal/crypto_service_provider/BUILD.bazel b/rs/crypto/internal/crypto_service_provider/BUILD.bazel index 18f657f8daf..8272c5460bd 100644 --- a/rs/crypto/internal/crypto_service_provider/BUILD.bazel +++ b/rs/crypto/internal/crypto_service_provider/BUILD.bazel @@ -80,6 +80,7 @@ DEV_DEPENDENCIES = [ "//rs/crypto/utils/basic_sig", "//rs/test_utilities/in_memory_logger", "//rs/test_utilities/time", + "//rs/types/management_canister_types", "//rs/types/types_test_utils", "@crate_index//:assert_matches", "@crate_index//:lazy_static", diff --git a/rs/crypto/internal/crypto_service_provider/Cargo.toml b/rs/crypto/internal/crypto_service_provider/Cargo.toml index 7d1a9c71db7..0b5f2b1e1f4 100644 --- a/rs/crypto/internal/crypto_service_provider/Cargo.toml +++ b/rs/crypto/internal/crypto_service_provider/Cargo.toml @@ -76,6 +76,7 @@ ic-crypto-test-utils-local-csp-vault = { path = "../../../crypto/test_utils/loca ic-crypto-test-utils-metrics = { path = "../../../crypto/test_utils/metrics" } ic-crypto-test-utils-reproducible-rng = { path = "../../../crypto/test_utils/reproducible_rng" } ic-crypto-utils-basic-sig = { path = "../../utils/basic_sig" } +ic-management-canister-types = { path = "../../../types/management_canister_types" } ic-test-utilities-compare-dirs = { path = "../../../test_utilities/compare_dirs" } ic-test-utilities-in-memory-logger = { path = "../../../test_utilities/in_memory_logger" } ic-test-utilities-time = { path = "../../../test_utilities/time" } diff --git a/rs/crypto/internal/crypto_service_provider/src/vault/test_utils/ni_dkg/fixtures.rs b/rs/crypto/internal/crypto_service_provider/src/vault/test_utils/ni_dkg/fixtures.rs index ac5d81840e7..290130b7110 100644 --- a/rs/crypto/internal/crypto_service_provider/src/vault/test_utils/ni_dkg/fixtures.rs +++ b/rs/crypto/internal/crypto_service_provider/src/vault/test_utils/ni_dkg/fixtures.rs @@ -10,6 +10,10 @@ use ic_crypto_internal_types::sign::threshold_sig::ni_dkg::{ CspNiDkgDealing, CspNiDkgTranscript, Epoch, }; use ic_crypto_internal_types::sign::threshold_sig::public_key::CspThresholdSigPublicKey; +use ic_management_canister_types::{ + EcdsaCurve, EcdsaKeyId, MasterPublicKeyId, SchnorrAlgorithm, SchnorrKeyId, VetKdCurve, + VetKdKeyId, +}; use ic_types::crypto::threshold_sig::ni_dkg::config::dealers::NiDkgDealers; use ic_types::crypto::threshold_sig::ni_dkg::config::receivers::NiDkgReceivers; use ic_types::crypto::threshold_sig::ni_dkg::config::NiDkgThreshold; @@ -22,7 +26,7 @@ use rand::Rng; use rand_chacha::ChaCha20Rng; use std::collections::BTreeMap; use std::sync::Arc; -use strum::IntoEnumIterator; +use strum::{EnumCount, IntoEnumIterator}; // Generate random data structures: // Alternatively we could implement Distribution for all of these types. @@ -33,10 +37,45 @@ pub fn random_height(rng: &mut ChaCha20Rng) -> Height { pub fn random_subnet_id(rng: &mut ChaCha20Rng) -> SubnetId { subnet_test_id(rng.gen::()) } +pub fn random_master_public_key_id(rng: &mut ChaCha20Rng) -> MasterPublicKeyId { + assert_eq!(MasterPublicKeyId::COUNT, 3); + assert_eq!(EcdsaCurve::iter().count(), 1); + assert_eq!(SchnorrAlgorithm::iter().count(), 2); + assert_eq!(VetKdCurve::iter().count(), 1); + + use rand::prelude::SliceRandom; + [ + MasterPublicKeyId::Ecdsa(EcdsaKeyId { + curve: EcdsaCurve::Secp256k1, + name: "some key".to_string(), + }), + MasterPublicKeyId::Schnorr(SchnorrKeyId { + algorithm: SchnorrAlgorithm::Bip340Secp256k1, + name: "some key".to_string(), + }), + MasterPublicKeyId::Schnorr(SchnorrKeyId { + algorithm: SchnorrAlgorithm::Ed25519, + name: "some key".to_string(), + }), + MasterPublicKeyId::VetKd(VetKdKeyId { + curve: VetKdCurve::Bls12_381_G2, + name: "some key".to_string(), + }), + ] + .choose(rng) + .cloned() + .expect("Could not choose a MasterPublicKeyId") +} pub fn random_ni_dkg_tag(rng: &mut ChaCha20Rng) -> NiDkgTag { - NiDkgTag::iter() - .choose(rng) - .expect("Could not choose a NiDkgTag") + use rand::prelude::SliceRandom; + [ + NiDkgTag::LowThreshold, + NiDkgTag::HighThreshold, + NiDkgTag::HighThresholdForKey(random_master_public_key_id(rng)), + ] + .choose(rng) + .cloned() + .expect("Could not choose a NiDkgTag") } pub fn random_ni_dkg_id(rng: &mut ChaCha20Rng) -> NiDkgId { NiDkgId { diff --git a/rs/crypto/src/sign/threshold_sig.rs b/rs/crypto/src/sign/threshold_sig.rs index 689f493de1e..47225cc1e63 100644 --- a/rs/crypto/src/sign/threshold_sig.rs +++ b/rs/crypto/src/sign/threshold_sig.rs @@ -22,6 +22,9 @@ mod tests; pub struct ThresholdSignerInternal {} impl ThresholdSignerInternal { + //////////////////////////////////////// + #[allow(clippy::result_large_err)] + //////////////////////////////////////// pub fn sign_threshold( lockable_threshold_sig_data_store: &LockableThresholdSigDataStore, threshold_sig_csp_client: &C, @@ -70,6 +73,9 @@ fn sig_data_not_found_error(dkg_id: NiDkgId) -> ThresholdSigDataNotFoundError { ThresholdSigDataNotFoundError::ThresholdSigDataNotFound { dkg_id } } +//////////////////////////////////////// +#[allow(clippy::result_large_err)] +//////////////////////////////////////// fn threshold_sig_share_or_panic( csp_signature: CspSignature, ) -> Result, ThresholdSignError> { @@ -441,12 +447,8 @@ impl ThresholdSigVerifierInternal { H: Signable, { let csp_signature = CspSignature::try_from(signature)?; - let transcript = initial_ni_dkg_transcript_from_registry( - registry, - subnet_id, - version, - NiDkgTag::HighThreshold, - )?; + let transcript = + initial_high_threshold_ni_dkg_transcript_from_registry(registry, subnet_id, version)?; let csp_pub_coeffs = CspPublicCoefficients::from(&transcript); threshold_sig_csp_client .threshold_verify_combined_signature( @@ -459,20 +461,16 @@ impl ThresholdSigVerifierInternal { } } -fn initial_ni_dkg_transcript_from_registry( +fn initial_high_threshold_ni_dkg_transcript_from_registry( registry: &dyn RegistryClient, subnet_id: SubnetId, registry_version: RegistryVersion, - dkg_tag: NiDkgTag, ) -> CryptoResult { let maybe_transcripts = registry .get_initial_dkg_transcripts(subnet_id, registry_version) .map_err(CryptoError::RegistryClient)?; match maybe_transcripts.value { - Some(transcripts) => Ok(match dkg_tag { - NiDkgTag::LowThreshold => transcripts.low_threshold, - NiDkgTag::HighThreshold => transcripts.high_threshold, - }), + Some(transcripts) => Ok(transcripts.high_threshold), None => Err(CryptoError::DkgTranscriptNotFound { subnet_id, registry_version, diff --git a/rs/crypto/src/sign/threshold_sig/store.rs b/rs/crypto/src/sign/threshold_sig/store.rs index 3a78f843d4a..7d51813a37b 100644 --- a/rs/crypto/src/sign/threshold_sig/store.rs +++ b/rs/crypto/src/sign/threshold_sig/store.rs @@ -1,4 +1,5 @@ use super::*; +use ic_management_canister_types::MasterPublicKeyId; use ic_types::crypto::threshold_sig::ni_dkg::NiDkgId; use std::collections::VecDeque; @@ -79,15 +80,24 @@ impl TranscriptData { /// data was inserted _first_ is removed, that is, DKG IDs are purged in /// insertion order. /// -/// The maximum number of threshold signature data stored per tag is defined by -/// `CAPACITY_PER_TAG`. For the moment there are two tags, `LowThreshold` and `HighThreshold`, -/// meaning that the total capacity of the threshold signature data store is `2 * CAPACITY_PER_TAG`. +/// The maximum number of threshold signature data stored per tag or key is defined by +/// `CAPACITY_PER_TAG_OR_KEY`. For the moment there are three tags: +/// * `LowThreshold` +/// * `HighThreshold` +/// * `HighThresholdForKey(MasterPublicKeyId)` +/// +/// and the total capacity of the threshold signature data store is +/// `2*CAPACITY_PER_TAG_OR_KEY + K*CAPACITY_PER_TAG_OR_KEY` where `K` is +/// the number of different `MasterPublicKeyId`s that are stored on the +/// subnet. In production, currently at most 3 keys are stored per subnet +/// (1 ECDSA key, 2 Schnorr keys). pub struct ThresholdSigDataStoreImpl { store: BTreeMap, - max_num_of_dkg_ids_per_tag: usize, + max_num_of_dkg_ids_per_tag_or_key: usize, // VecDeque used as queue: `push_back` to add, `pop_front` to remove low_threshold_dkg_id_insertion_order: VecDeque, high_threshold_dkg_id_insertion_order: VecDeque, + high_threshold_for_key_dkg_id_insertion_order: BTreeMap>, } #[derive(Default)] @@ -115,20 +125,21 @@ impl ThresholdSigDataStoreImpl { /// /// # Panics /// If `max_num_of_dkg_ids_per_tag` is smaller than 1. - fn new_with_max_size(max_num_of_dkg_ids_per_tag: usize) -> Self { + fn new_with_max_size(max_num_of_dkg_ids_per_tag_or_key: usize) -> Self { assert!( - max_num_of_dkg_ids_per_tag >= 1, + max_num_of_dkg_ids_per_tag_or_key >= 1, "The maximum size per tag must be at least 1" ); ThresholdSigDataStoreImpl { store: BTreeMap::new(), - max_num_of_dkg_ids_per_tag, + max_num_of_dkg_ids_per_tag_or_key, low_threshold_dkg_id_insertion_order: VecDeque::with_capacity( - max_num_of_dkg_ids_per_tag, + max_num_of_dkg_ids_per_tag_or_key, ), high_threshold_dkg_id_insertion_order: VecDeque::with_capacity( - max_num_of_dkg_ids_per_tag, + max_num_of_dkg_ids_per_tag_or_key, ), + high_threshold_for_key_dkg_id_insertion_order: BTreeMap::new(), } } @@ -137,7 +148,7 @@ impl ThresholdSigDataStoreImpl { if !self.store.contains_key(dkg_id) { self.store .insert(dkg_id.clone(), ThresholdSigData::default()); - match dkg_id.dkg_tag { + match &dkg_id.dkg_tag { NiDkgTag::LowThreshold => { self.low_threshold_dkg_id_insertion_order .push_back(dkg_id.clone()); @@ -146,6 +157,21 @@ impl ThresholdSigDataStoreImpl { self.high_threshold_dkg_id_insertion_order .push_back(dkg_id.clone()); } + NiDkgTag::HighThresholdForKey(master_public_key_id) => { + match self + .high_threshold_for_key_dkg_id_insertion_order + .get_mut(master_public_key_id) + { + Some(insertion_order) => insertion_order.push_back(dkg_id.clone()), + None => { + let mut buf = + VecDeque::with_capacity(self.max_num_of_dkg_ids_per_tag_or_key); + buf.push_back(dkg_id.clone()); + self.high_threshold_for_key_dkg_id_insertion_order + .insert(master_public_key_id.clone(), buf); + } + } + } } } self.store @@ -155,24 +181,34 @@ impl ThresholdSigDataStoreImpl { fn purge_entry_for_oldest_dkg_id_if_necessary(&mut self, tag: &NiDkgTag) { let dkg_id_insertion_order = match tag { - NiDkgTag::LowThreshold => &mut self.low_threshold_dkg_id_insertion_order, - NiDkgTag::HighThreshold => &mut self.high_threshold_dkg_id_insertion_order, + NiDkgTag::LowThreshold => Some(&mut self.low_threshold_dkg_id_insertion_order), + NiDkgTag::HighThreshold => Some(&mut self.high_threshold_dkg_id_insertion_order), + NiDkgTag::HighThresholdForKey(master_public_key_id) => self + .high_threshold_for_key_dkg_id_insertion_order + .get_mut(master_public_key_id), }; - if dkg_id_insertion_order.len() > self.max_num_of_dkg_ids_per_tag { - let oldest_dkg_id = dkg_id_insertion_order - .pop_front() - .expect("dkg store unexpectedly empty"); - self.store.remove(&oldest_dkg_id); + if let Some(insertion_order) = dkg_id_insertion_order { + if insertion_order.len() > self.max_num_of_dkg_ids_per_tag_or_key { + let oldest_dkg_id = insertion_order + .pop_front() + .expect("dkg store unexpectedly empty"); + self.store.remove(&oldest_dkg_id); + } } } fn assert_length_invariant(&self) { + let high_threshold_for_key_id_dkg_id_insertion_order_len: usize = self + .high_threshold_for_key_dkg_id_insertion_order + .values() + .map(|v| v.len()) + .sum(); assert_eq!( self.store.len(), self.low_threshold_dkg_id_insertion_order.len() - + self.high_threshold_dkg_id_insertion_order.len(), - "The combined length of the queues maintaining DKG ID insertion order must be the \ - same as that of the map containing the DKG ID data." + + self.high_threshold_dkg_id_insertion_order.len() + + high_threshold_for_key_id_dkg_id_insertion_order_len, + "ThresholdSigDataStore length invariant violated" ); } } diff --git a/rs/crypto/src/sign/threshold_sig/store/tests.rs b/rs/crypto/src/sign/threshold_sig/store/tests.rs index 542543e426a..2e063813c15 100644 --- a/rs/crypto/src/sign/threshold_sig/store/tests.rs +++ b/rs/crypto/src/sign/threshold_sig/store/tests.rs @@ -1,42 +1,62 @@ use super::*; -use crate::sign::threshold_sig::tests::{NI_DKG_ID_1, NI_DKG_ID_2}; use ic_crypto_internal_types::sign::threshold_sig::ni_dkg::ni_dkg_groth20_bls12_381::PublicCoefficientsBytes; use ic_crypto_internal_types::sign::threshold_sig::public_key::bls12_381::PublicKeyBytes; use ic_crypto_internal_types::sign::threshold_sig::public_key::CspThresholdSigPublicKey; -use ic_types::crypto::threshold_sig::ni_dkg::NiDkgId; +use ic_management_canister_types::{ + EcdsaCurve, EcdsaKeyId, SchnorrAlgorithm, SchnorrKeyId, VetKdCurve, VetKdKeyId, +}; +use ic_types::crypto::threshold_sig::ni_dkg::{NiDkgId, NiDkgTargetId, NiDkgTargetSubnet}; use ic_types::Height; use ic_types_test_utils::ids::{node_test_id, SUBNET_1}; +use strum::{EnumCount, IntoEnumIterator}; const NODE_1: u64 = 1; const NODE_2: u64 = 2; const NODE_1_INDEX: NodeIndex = 1; const NODE_2_INDEX: NodeIndex = 2; +pub const NI_DKG_ID_HIGH_T: NiDkgId = NiDkgId { + start_block_height: Height::new(3), + dealer_subnet: SUBNET_1, + dkg_tag: NiDkgTag::HighThreshold, + target_subnet: NiDkgTargetSubnet::Remote(NiDkgTargetId::new([42; 32])), +}; + +pub const NI_DKG_ID_LOW_T: NiDkgId = NiDkgId { + start_block_height: Height::new(3), + dealer_subnet: SUBNET_1, + dkg_tag: NiDkgTag::LowThreshold, + target_subnet: NiDkgTargetSubnet::Remote(NiDkgTargetId::new([42; 32])), +}; + +pub const NI_DKG_ID_1: NiDkgId = NI_DKG_ID_HIGH_T; +pub const NI_DKG_ID_2: NiDkgId = NI_DKG_ID_LOW_T; + #[test] fn should_contain_transcript_data_after_insertion_with_nidkg_id() { - should_contain_transcript_data_after_insertion_with_dkg_id(&NI_DKG_ID_1); -} + for tag in all_tags() { + let mut store = ThresholdSigDataStoreImpl::new(); + let indices = indices_with(vec![ + (node_test_id(NODE_1), NODE_1_INDEX), + (node_test_id(NODE_2), NODE_2_INDEX), + ]); + let public_coeffs = public_coeffs(); -fn should_contain_transcript_data_after_insertion_with_dkg_id(dkg_id: &NiDkgId) { - let mut store = ThresholdSigDataStoreImpl::new(); - let indices = indices_with(vec![ - (node_test_id(NODE_1), NODE_1_INDEX), - (node_test_id(NODE_2), NODE_2_INDEX), - ]); - let public_coeffs = public_coeffs(); + let dkg_id = ni_dkg_id_with_tag(tag.clone(), 42); - store.insert_transcript_data(dkg_id, public_coeffs.clone(), indices); + store.insert_transcript_data(&dkg_id, public_coeffs.clone(), indices); - let transcript_data = store.transcript_data(dkg_id).unwrap(); - assert_eq!(transcript_data.public_coefficients(), &public_coeffs); - assert_eq!( - transcript_data.index(node_test_id(NODE_1)), - Some(&NODE_1_INDEX) - ); - assert_eq!( - transcript_data.index(node_test_id(NODE_2)), - Some(&NODE_2_INDEX) - ); + let transcript_data = store.transcript_data(&dkg_id).unwrap(); + assert_eq!(transcript_data.public_coefficients(), &public_coeffs); + assert_eq!( + transcript_data.index(node_test_id(NODE_1)), + Some(&NODE_1_INDEX) + ); + assert_eq!( + transcript_data.index(node_test_id(NODE_2)), + Some(&NODE_2_INDEX) + ); + } } #[test] @@ -48,41 +68,47 @@ fn should_not_contain_nonexistent_transcript_data() { #[test] fn should_contain_individual_public_keys_after_insertion_with_nidkg_id() { - let dkg_id = NI_DKG_ID_1; - let mut store = ThresholdSigDataStoreImpl::new(); - let csp_pubkey = csp_public_key(); + for tag in all_tags() { + let dkg_id = ni_dkg_id_with_tag(tag.clone(), 1); + let mut store = ThresholdSigDataStoreImpl::new(); + let csp_pubkey = csp_public_key(); - store.insert_individual_public_key(&dkg_id, node_test_id(NODE_1), csp_pubkey); + store.insert_individual_public_key(&dkg_id, node_test_id(NODE_1), csp_pubkey); - assert_eq!( - store.individual_public_key(&dkg_id, node_test_id(NODE_1)), - Some(&csp_pubkey) - ); + assert_eq!( + store.individual_public_key(&dkg_id, node_test_id(NODE_1)), + Some(&csp_pubkey) + ); + } } #[test] fn should_insert_multiple_individual_public_keys() { - let mut store = ThresholdSigDataStoreImpl::new(); - let csp_pubkey_1 = csp_public_key(); - let csp_pubkey_2 = csp_public_key(); + for tag in all_tags() { + let mut store = ThresholdSigDataStoreImpl::new(); + let csp_pubkey_1 = csp_public_key(); + let csp_pubkey_2 = other_csp_public_key(); + let ni_dkg_id_1 = ni_dkg_id_with_tag(tag.clone(), 1); + let ni_dkg_id_2 = ni_dkg_id_with_tag(tag.clone(), 2); + + assert_ne!(ni_dkg_id_1, NI_DKG_ID_2); + store.insert_individual_public_key(&ni_dkg_id_1, node_test_id(NODE_1), csp_pubkey_1); + store.insert_individual_public_key(&ni_dkg_id_1, node_test_id(NODE_2), csp_pubkey_2); + store.insert_individual_public_key(&ni_dkg_id_2, node_test_id(NODE_1), csp_pubkey_2); - assert_ne!(NI_DKG_ID_1, NI_DKG_ID_2); - store.insert_individual_public_key(&NI_DKG_ID_1, node_test_id(NODE_1), csp_pubkey_1); - store.insert_individual_public_key(&NI_DKG_ID_1, node_test_id(NODE_2), csp_pubkey_2); - store.insert_individual_public_key(&NI_DKG_ID_2, node_test_id(NODE_1), csp_pubkey_2); - - assert_eq!( - store.individual_public_key(&NI_DKG_ID_1, node_test_id(NODE_1)), - Some(&csp_pubkey_1) - ); - assert_eq!( - store.individual_public_key(&NI_DKG_ID_1, node_test_id(NODE_2)), - Some(&csp_pubkey_2) - ); - assert_eq!( - store.individual_public_key(&NI_DKG_ID_2, node_test_id(NODE_1)), - Some(&csp_pubkey_2) - ); + assert_eq!( + store.individual_public_key(&ni_dkg_id_1, node_test_id(NODE_1)), + Some(&csp_pubkey_1) + ); + assert_eq!( + store.individual_public_key(&ni_dkg_id_1, node_test_id(NODE_2)), + Some(&csp_pubkey_2) + ); + assert_eq!( + store.individual_public_key(&ni_dkg_id_2, node_test_id(NODE_1)), + Some(&csp_pubkey_2) + ); + } } #[test] @@ -97,49 +123,58 @@ fn should_not_contain_nonexistent_individual_public_key() { #[test] fn should_overwrite_existing_public_coefficients() { - let mut store = ThresholdSigDataStoreImpl::new(); - let (public_coeffs_1, public_coeffs_2) = (public_coeffs_1(), public_coeffs_2()); - assert_ne!(public_coeffs_1, public_coeffs_2); + for tag in all_tags() { + let mut store = ThresholdSigDataStoreImpl::new(); + let (public_coeffs_1, public_coeffs_2) = (public_coeffs_1(), public_coeffs_2()); + assert_ne!(public_coeffs_1, public_coeffs_2); + let ni_dkg_id = ni_dkg_id_with_tag(tag.clone(), 1); - store.insert_transcript_data(&NI_DKG_ID_1, public_coeffs_1, BTreeMap::new()); - store.insert_transcript_data(&NI_DKG_ID_1, public_coeffs_2.clone(), BTreeMap::new()); + store.insert_transcript_data(&ni_dkg_id, public_coeffs_1, BTreeMap::new()); + store.insert_transcript_data(&ni_dkg_id, public_coeffs_2.clone(), BTreeMap::new()); - let transcript_data = store.transcript_data(&NI_DKG_ID_1).unwrap(); - assert_eq!(transcript_data.public_coefficients(), &public_coeffs_2); + let transcript_data = store.transcript_data(&ni_dkg_id).unwrap(); + assert_eq!(transcript_data.public_coefficients(), &public_coeffs_2); + } } #[test] fn should_overwrite_existing_indices() { - let mut store = ThresholdSigDataStoreImpl::new(); - let indices_1 = indices_with(vec![(node_test_id(NODE_1), NODE_1_INDEX)]); - let indices_2 = indices_with(vec![(node_test_id(NODE_2), NODE_2_INDEX)]); - let public_coeffs = public_coeffs(); - - store.insert_transcript_data(&NI_DKG_ID_1, public_coeffs.clone(), indices_1); - store.insert_transcript_data(&NI_DKG_ID_1, public_coeffs, indices_2); - - let transcript_data = store.transcript_data(&NI_DKG_ID_1).unwrap(); - assert_eq!(transcript_data.index(node_test_id(NODE_1)), None); - assert_eq!( - transcript_data.index(node_test_id(NODE_2)), - Some(&NODE_2_INDEX) - ); + for tag in all_tags() { + let mut store = ThresholdSigDataStoreImpl::new(); + let indices_1 = indices_with(vec![(node_test_id(NODE_1), NODE_1_INDEX)]); + let indices_2 = indices_with(vec![(node_test_id(NODE_2), NODE_2_INDEX)]); + let public_coeffs = public_coeffs(); + let ni_dkg_id = ni_dkg_id_with_tag(tag.clone(), 1); + + store.insert_transcript_data(&ni_dkg_id, public_coeffs.clone(), indices_1); + store.insert_transcript_data(&ni_dkg_id, public_coeffs, indices_2); + + let transcript_data = store.transcript_data(&ni_dkg_id).unwrap(); + assert_eq!(transcript_data.index(node_test_id(NODE_1)), None); + assert_eq!( + transcript_data.index(node_test_id(NODE_2)), + Some(&NODE_2_INDEX) + ); + } } #[test] fn should_overwrite_existing_individual_public_keys() { - let mut store = ThresholdSigDataStoreImpl::new(); - let csp_pubkey_1 = csp_public_key(); - let csp_pubkey_2 = other_csp_public_key(); - assert_ne!(csp_pubkey_1, csp_pubkey_2); + for tag in all_tags() { + let mut store = ThresholdSigDataStoreImpl::new(); + let csp_pubkey_1 = csp_public_key(); + let csp_pubkey_2 = other_csp_public_key(); + assert_ne!(csp_pubkey_1, csp_pubkey_2); + let ni_dkg_id = ni_dkg_id_with_tag(tag.clone(), 1); - store.insert_individual_public_key(&NI_DKG_ID_1, node_test_id(NODE_1), csp_pubkey_1); - store.insert_individual_public_key(&NI_DKG_ID_1, node_test_id(NODE_1), csp_pubkey_2); + store.insert_individual_public_key(&ni_dkg_id, node_test_id(NODE_1), csp_pubkey_1); + store.insert_individual_public_key(&ni_dkg_id, node_test_id(NODE_1), csp_pubkey_2); - assert_eq!( - store.individual_public_key(&NI_DKG_ID_1, node_test_id(NODE_1)), - Some(&csp_pubkey_2) - ); + assert_eq!( + store.individual_public_key(&ni_dkg_id, node_test_id(NODE_1)), + Some(&csp_pubkey_2) + ); + } } #[test] @@ -149,70 +184,105 @@ fn should_have_capacity_per_tag_of_9() { #[test] fn should_not_purge_data_on_inserting_coeffs_and_indices_if_capacity_not_exceeded() { - let mut store = ThresholdSigDataStoreImpl::new(); + for tag in all_tags() { + let mut store = ThresholdSigDataStoreImpl::new(); + + for i in 1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG { + store.insert_transcript_data( + &ni_dkg_id_with_tag(tag.clone(), i), + public_coeffs(), + BTreeMap::new(), + ); + } - for i in 1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG { - store.insert_transcript_data(&ni_dkg_id(i), public_coeffs(), BTreeMap::new()); + assert_eq!( + store.store.len(), + ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + ); } - - assert_eq!( - store.store.len(), - ThresholdSigDataStoreImpl::CAPACITY_PER_TAG - ); } #[test] fn should_not_purge_data_on_inserting_pubkeys_if_capacity_not_exceeded() { - let mut store = ThresholdSigDataStoreImpl::new(); + for tag in all_tags() { + let mut store = ThresholdSigDataStoreImpl::new(); + + for i in 1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG { + store.insert_individual_public_key( + &ni_dkg_id_with_tag(tag.clone(), i), + node_test_id(NODE_1), + csp_public_key(), + ); + } - for i in 1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG { - store.insert_individual_public_key(&ni_dkg_id(i), node_test_id(NODE_1), csp_public_key()); + assert_eq!( + store.store.len(), + ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + ); } - - assert_eq!( - store.store.len(), - ThresholdSigDataStoreImpl::CAPACITY_PER_TAG - ); } #[test] fn should_purge_data_on_inserting_coeffs_and_indices_if_capacity_exceeded() { - let mut store = ThresholdSigDataStoreImpl::new(); - let pub_coeffs = public_coeffs(); - - for i in 1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1 { - store.insert_transcript_data(&ni_dkg_id(i), pub_coeffs.clone(), BTreeMap::new()); - } + for tag in all_tags() { + let mut store = ThresholdSigDataStoreImpl::new(); + let pub_coeffs = public_coeffs(); + + for i in 1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1 { + store.insert_transcript_data( + &ni_dkg_id_with_tag(tag.clone(), i), + pub_coeffs.clone(), + BTreeMap::new(), + ); + } - assert_eq!( - store.store.len(), - ThresholdSigDataStoreImpl::CAPACITY_PER_TAG - ); - assert!(store.transcript_data(&ni_dkg_id(1)).is_none()); - for i in 2..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1 { - assert_eq!(pub_coeffs_from_store(&store, ni_dkg_id(i)), pub_coeffs); + assert_eq!( + store.store.len(), + ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + ); + assert!(store + .transcript_data(&ni_dkg_id_with_tag(tag.clone(), 1)) + .is_none()); + for i in 2..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1 { + assert_eq!( + pub_coeffs_from_store(&store, ni_dkg_id_with_tag(tag.clone(), i)), + pub_coeffs + ); + } } } #[test] fn should_purge_data_in_insertion_order_on_inserting_coeffs_and_indices_if_capacity_exceeded() { - let mut store = ThresholdSigDataStoreImpl::new(); - let pub_coeffs = public_coeffs(); - - for i in (1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1).rev() { - store.insert_transcript_data(&ni_dkg_id(i), pub_coeffs.clone(), BTreeMap::new()); - } + for tag in all_tags() { + let mut store = ThresholdSigDataStoreImpl::new(); + let pub_coeffs = public_coeffs(); + + for i in (1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1).rev() { + store.insert_transcript_data( + &ni_dkg_id_with_tag(tag.clone(), i), + pub_coeffs.clone(), + BTreeMap::new(), + ); + } - assert_eq!( - store.store.len(), - ThresholdSigDataStoreImpl::CAPACITY_PER_TAG - ); - for i in 1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG { - assert_eq!(pub_coeffs_from_store(&store, ni_dkg_id(i)), pub_coeffs); + assert_eq!( + store.store.len(), + ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + ); + for i in 1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG { + assert_eq!( + pub_coeffs_from_store(&store, ni_dkg_id_with_tag(tag.clone(), i)), + pub_coeffs + ); + } + assert!(store + .transcript_data(&ni_dkg_id_with_tag( + tag.clone(), + ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1 + )) + .is_none()); } - assert!(store - .transcript_data(&ni_dkg_id(ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1)) - .is_none()); } fn should_not_purge_all_transcripts_of_certain_threshold_if_capacity_exceeded( @@ -239,11 +309,7 @@ fn should_not_purge_all_transcripts_of_certain_threshold_if_capacity_exceeded( store.store.len(), ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1 ); - assert_eq!( - store.store.len(), - store.low_threshold_dkg_id_insertion_order.len() - + store.high_threshold_dkg_id_insertion_order.len(), - ); + assert_store_length_invariant(&store); // verify there is at least one high threshold transcript and at least one low threshold transcript let mut found_single_threshold = false; @@ -263,130 +329,117 @@ fn should_not_purge_all_transcripts_of_certain_threshold_if_capacity_exceeded( } #[test] -fn should_not_purge_only_low_threshold_transcript_if_capacity_exceeded() { - should_not_purge_all_transcripts_of_certain_threshold_if_capacity_exceeded( - NiDkgTag::LowThreshold, - NiDkgTag::HighThreshold, - ); -} - -#[test] -fn should_not_purge_only_high_threshold_transcript_if_capacity_exceeded() { - should_not_purge_all_transcripts_of_certain_threshold_if_capacity_exceeded( - NiDkgTag::HighThreshold, - NiDkgTag::LowThreshold, - ); +fn should_not_purge_only_transcripts_for_some_tag_if_capacity_exceeded() { + for tag in all_tags() { + for other_tag in all_tags() { + if tag != other_tag { + should_not_purge_all_transcripts_of_certain_threshold_if_capacity_exceeded( + tag.clone(), + other_tag, + ) + } + } + } } #[test] fn should_purge_data_on_inserting_pubkeys_if_capacity_exceeded() { - let mut store = ThresholdSigDataStoreImpl::new(); - let csp_pubkey = csp_public_key(); - let node_id = node_test_id(NODE_1); - - for i in 1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1 { - store.insert_individual_public_key(&ni_dkg_id(i), node_id, csp_pubkey); - } + for tag in all_tags() { + let mut store = ThresholdSigDataStoreImpl::new(); + let csp_pubkey = csp_public_key(); + let node_id = node_test_id(NODE_1); + + for i in 1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1 { + store.insert_individual_public_key( + &ni_dkg_id_with_tag(tag.clone(), i), + node_id, + csp_pubkey, + ); + } - assert_eq!( - store.store.len(), - ThresholdSigDataStoreImpl::CAPACITY_PER_TAG - ); - for i in 2..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1 { assert_eq!( - store.individual_public_key(&ni_dkg_id(i), node_id), - Some(&csp_pubkey) + store.store.len(), + ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + ); + for i in 2..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1 { + assert_eq!( + store.individual_public_key(&ni_dkg_id_with_tag(tag.clone(), i), node_id), + Some(&csp_pubkey) + ); + } + assert_eq!( + store.individual_public_key(&ni_dkg_id_with_tag(tag.clone(), 1), node_id), + None ); } - assert_eq!(store.individual_public_key(&ni_dkg_id(1), node_id), None); } #[test] fn should_purge_data_in_insertion_order_on_inserting_pubkeys_if_max_size_exceeded() { - let mut store = ThresholdSigDataStoreImpl::new(); - let csp_pubkey = csp_public_key(); - let node_id = node_test_id(NODE_1); - - for i in (1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1).rev() { - store.insert_individual_public_key(&ni_dkg_id(i), node_id, csp_pubkey); - } + for tag in all_tags() { + let mut store = ThresholdSigDataStoreImpl::new(); + let csp_pubkey = csp_public_key(); + let node_id = node_test_id(NODE_1); + + for i in (1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1).rev() { + store.insert_individual_public_key( + &ni_dkg_id_with_tag(tag.clone(), i), + node_id, + csp_pubkey, + ); + } - assert_eq!( - store.store.len(), - ThresholdSigDataStoreImpl::CAPACITY_PER_TAG - ); - for i in 1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG { assert_eq!( - store.individual_public_key(&ni_dkg_id(i), node_id), - Some(&csp_pubkey) + store.store.len(), + ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + ); + for i in 1..=ThresholdSigDataStoreImpl::CAPACITY_PER_TAG { + assert_eq!( + store.individual_public_key(&ni_dkg_id_with_tag(tag.clone(), i), node_id), + Some(&csp_pubkey) + ); + } + assert_eq!( + store.individual_public_key( + &ni_dkg_id_with_tag(tag.clone(), ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1), + node_id + ), + None ); } - assert_eq!( - store.individual_public_key( - &ni_dkg_id(ThresholdSigDataStoreImpl::CAPACITY_PER_TAG + 1), - node_id - ), - None - ); } #[test] -fn should_store_up_to_capacity_per_tag_for_both_tags() { +fn should_store_up_to_capacity_per_tag_for_all_tags() { let mut store = ThresholdSigDataStoreImpl::new(); let pub_coeffs = public_coeffs(); for i in 0..ThresholdSigDataStoreImpl::CAPACITY_PER_TAG { - store.insert_transcript_data( - &ni_dkg_id_with_tag(NiDkgTag::LowThreshold, i), - pub_coeffs.clone(), - BTreeMap::new(), - ); - store.insert_transcript_data( - &ni_dkg_id_with_tag(NiDkgTag::HighThreshold, i), - pub_coeffs.clone(), - BTreeMap::new(), - ); + for tag in all_tags() { + store.insert_transcript_data( + &ni_dkg_id_with_tag(tag.clone(), i), + pub_coeffs.clone(), + BTreeMap::new(), + ); + } } // Verify we have exactly the max capacity stored - assert_eq!( - store.store.len(), - ThresholdSigDataStoreImpl::CAPACITY_PER_TAG * 2 - ); - assert_eq!( - store.store.len(), - store.low_threshold_dkg_id_insertion_order.len() - + store.high_threshold_dkg_id_insertion_order.len(), - ); + assert_max_store_capacity(&store); + assert_store_length_invariant(&store); // Insert one more transcript per tag - store.insert_transcript_data( - &ni_dkg_id_with_tag( - NiDkgTag::LowThreshold, - ThresholdSigDataStoreImpl::CAPACITY_PER_TAG, - ), - pub_coeffs.clone(), - BTreeMap::new(), - ); - store.insert_transcript_data( - &ni_dkg_id_with_tag( - NiDkgTag::HighThreshold, - ThresholdSigDataStoreImpl::CAPACITY_PER_TAG, - ), - pub_coeffs.clone(), - BTreeMap::new(), - ); + for tag in all_tags() { + store.insert_transcript_data( + &ni_dkg_id_with_tag(tag, ThresholdSigDataStoreImpl::CAPACITY_PER_TAG), + pub_coeffs.clone(), + BTreeMap::new(), + ); + } // Verify that we still have exactly the max capacity stored (since one of each tag was purged) - assert_eq!( - store.store.len(), - ThresholdSigDataStoreImpl::CAPACITY_PER_TAG * 2 - ); - assert_eq!( - store.store.len(), - store.low_threshold_dkg_id_insertion_order.len() - + store.high_threshold_dkg_id_insertion_order.len(), - ); + assert_max_store_capacity(&store); + assert_store_length_invariant(&store); } fn indices_with(mappings: Vec<(NodeId, NodeIndex)>) -> BTreeMap { @@ -447,3 +500,69 @@ fn ni_dkg_id_with_tag(ni_dkg_tag: NiDkgTag, height: usize) -> NiDkgId { ..ni_dkg_id(height) } } + +fn all_tags() -> Vec { + assert_eq!(MasterPublicKeyId::COUNT, 3); + assert_eq!(EcdsaCurve::iter().count(), 1); + assert_eq!(SchnorrAlgorithm::iter().count(), 2); + assert_eq!(VetKdCurve::iter().count(), 1); + vec![ + NiDkgTag::LowThreshold, + NiDkgTag::HighThreshold, + NiDkgTag::HighThresholdForKey(ecdsa_secp256k1_master_public_key_id()), + NiDkgTag::HighThresholdForKey(schnorr_bip340_master_public_key_id()), + NiDkgTag::HighThresholdForKey(schnorr_ed25519_master_public_key_id()), + NiDkgTag::HighThresholdForKey(vetkd_master_public_key_id()), + ] +} + +fn ecdsa_secp256k1_master_public_key_id() -> MasterPublicKeyId { + MasterPublicKeyId::Ecdsa(EcdsaKeyId { + curve: EcdsaCurve::Secp256k1, + name: "ecdsa_secp256k1_key".to_string(), + }) +} + +fn schnorr_bip340_master_public_key_id() -> MasterPublicKeyId { + MasterPublicKeyId::Schnorr(SchnorrKeyId { + algorithm: SchnorrAlgorithm::Bip340Secp256k1, + name: "schnorr_bip340_secp256k1_key".to_string(), + }) +} + +fn schnorr_ed25519_master_public_key_id() -> MasterPublicKeyId { + MasterPublicKeyId::Schnorr(SchnorrKeyId { + algorithm: SchnorrAlgorithm::Ed25519, + name: "schnorr_ed25519_key".to_string(), + }) +} + +fn vetkd_master_public_key_id() -> MasterPublicKeyId { + MasterPublicKeyId::VetKd(VetKdKeyId { + curve: VetKdCurve::Bls12_381_G2, + name: "vetkd_bls12_381_g2_key".to_string(), + }) +} + +fn assert_max_store_capacity(store: &ThresholdSigDataStoreImpl) { + assert_eq!( + store.store.len(), + ThresholdSigDataStoreImpl::CAPACITY_PER_TAG * 2 + + ThresholdSigDataStoreImpl::CAPACITY_PER_TAG * (all_tags().len() - 2) + ); +} + +fn assert_store_length_invariant(store: &ThresholdSigDataStoreImpl) { + let high_threshold_for_key_id_dkg_id_insertion_order_len: usize = store + .high_threshold_for_key_dkg_id_insertion_order + .values() + .map(|v| v.len()) + .sum(); + assert_eq!( + store.store.len(), + store.low_threshold_dkg_id_insertion_order.len() + + store.high_threshold_dkg_id_insertion_order.len() + + high_threshold_for_key_id_dkg_id_insertion_order_len, + "ThresholdSigDataStore length invariant violated" + ); +} diff --git a/rs/crypto/src/sign/threshold_sig/tests.rs b/rs/crypto/src/sign/threshold_sig/tests.rs index 10d3f8126ba..a0a5d52d37b 100644 --- a/rs/crypto/src/sign/threshold_sig/tests.rs +++ b/rs/crypto/src/sign/threshold_sig/tests.rs @@ -21,7 +21,7 @@ use ic_types::crypto::threshold_sig::ni_dkg::{ use ic_types::crypto::{CombinedThresholdSig, SignableMock, ThresholdSigShare}; use ic_types::Height; use ic_types::SubnetId; -use ic_types_test_utils::ids::{NODE_1, SUBNET_0, SUBNET_1}; +use ic_types_test_utils::ids::{NODE_1, SUBNET_1}; pub const NODE_ID: NodeId = NODE_1; @@ -32,13 +32,6 @@ pub const NI_DKG_ID_1: NiDkgId = NiDkgId { target_subnet: NiDkgTargetSubnet::Remote(NiDkgTargetId::new([42; 32])), }; -pub const NI_DKG_ID_2: NiDkgId = NiDkgId { - start_block_height: Height::new(2), - dealer_subnet: SUBNET_0, - dkg_tag: NiDkgTag::HighThreshold, - target_subnet: NiDkgTargetSubnet::Remote(NiDkgTargetId::new([23; 32])), -}; - mod sign_threshold { use super::*; use ic_crypto_internal_csp::key_id::KeyId; diff --git a/rs/crypto/test_utils/ni-dkg/src/initial_config/tests.rs b/rs/crypto/test_utils/ni-dkg/src/initial_config/tests.rs index 2c48e47db8c..85d1e1e5454 100644 --- a/rs/crypto/test_utils/ni-dkg/src/initial_config/tests.rs +++ b/rs/crypto/test_utils/ni-dkg/src/initial_config/tests.rs @@ -294,6 +294,9 @@ fn registry_with_ni_dkg_transcript( cup_contents.initial_ni_dkg_transcript_low_threshold = Some(InitialNiDkgTranscriptRecord::from(transcript())); } + NiDkgTag::HighThresholdForKey(_master_public_key_id) => { + unimplemented!("not supported currently") + } } let registry_data = ProtoRegistryDataProvider::new(); registry_data diff --git a/rs/crypto/test_utils/ni-dkg/src/lib.rs b/rs/crypto/test_utils/ni-dkg/src/lib.rs index 52aa8754cbf..4cb89c34c68 100644 --- a/rs/crypto/test_utils/ni-dkg/src/lib.rs +++ b/rs/crypto/test_utils/ni-dkg/src/lib.rs @@ -461,9 +461,10 @@ impl RandomNiDkgConfig { /// is possible to perform tests where a single subnet has both /// high and low threshold transcripts pub fn new_with_inverted_threshold(&self, rng: &mut R) -> Self { - let dkg_tag = match self.0.dkg_id().dkg_tag { + let dkg_tag = match &self.0.dkg_id().dkg_tag { NiDkgTag::LowThreshold => NiDkgTag::HighThreshold, NiDkgTag::HighThreshold => NiDkgTag::LowThreshold, + NiDkgTag::HighThresholdForKey(_) => unimplemented!("not supported/needed currently"), }; let subnet_size = self.0.receivers().get().len(); diff --git a/rs/protobuf/def/types/v1/dkg.proto b/rs/protobuf/def/types/v1/dkg.proto index 5f44eae6bae..c6279bfd114 100644 --- a/rs/protobuf/def/types/v1/dkg.proto +++ b/rs/protobuf/def/types/v1/dkg.proto @@ -41,6 +41,7 @@ message Summary { message TaggedNiDkgTranscript { NiDkgTranscript transcript = 1; NiDkgTag tag = 2; + optional MasterPublicKeyId key_id = 3; } message CallbackIdedNiDkgTranscript { diff --git a/rs/protobuf/def/types/v1/types.proto b/rs/protobuf/def/types/v1/types.proto index c90b4c6da99..e6ec885e72c 100644 --- a/rs/protobuf/def/types/v1/types.proto +++ b/rs/protobuf/def/types/v1/types.proto @@ -32,6 +32,7 @@ message NiDkgId { bytes dealer_subnet = 2; NiDkgTag dkg_tag = 4; google.protobuf.BytesValue remote_target_id = 5; + optional MasterPublicKeyId key_id = 6; } // A non-interactive distributed key generation (NI-DKG) tag. @@ -39,6 +40,7 @@ enum NiDkgTag { NI_DKG_TAG_UNSPECIFIED = 0; NI_DKG_TAG_LOW_THRESHOLD = 1; NI_DKG_TAG_HIGH_THRESHOLD = 2; + NI_DKG_TAG_HIGH_THRESHOLD_FOR_KEY = 3; } message NominalCycles { diff --git a/rs/protobuf/generator/src/lib.rs b/rs/protobuf/generator/src/lib.rs index cb752ed8930..6fa62fa3b2a 100644 --- a/rs/protobuf/generator/src/lib.rs +++ b/rs/protobuf/generator/src/lib.rs @@ -351,7 +351,10 @@ fn build_types_proto(def: &Path, out: &Path) { config.type_attribute(".types.v1.SubnetId", "#[derive(Eq, Hash)]"); config.type_attribute(".types.v1.NiDkgId", "#[derive(Eq, Hash)]"); config.type_attribute(".types.v1.PrincipalId", "#[derive(Eq, Hash)]"); - config.type_attribute(".types.v1.EcdsaKeyId", "#[derive(Eq)]"); + config.type_attribute(".types.v1.MasterPublicKeyId", "#[derive(Eq, Hash)]"); + config.type_attribute(".types.v1.EcdsaKeyId", "#[derive(Eq, Hash)]"); + config.type_attribute(".types.v1.SchnorrKeyId", "#[derive(Eq, Hash)]"); + config.type_attribute(".types.v1.VetKdKeyId", "#[derive(Eq, Hash)]"); config.type_attribute(".types.v1.EcdsaCurve", "#[derive(candid::CandidType)]"); config.type_attribute(".types.v1.EcdsaKeyId", "#[derive(candid::CandidType)]"); config.type_attribute( diff --git a/rs/protobuf/src/gen/crypto/types.v1.rs b/rs/protobuf/src/gen/crypto/types.v1.rs index 19efff53939..89a98b27763 100644 --- a/rs/protobuf/src/gen/crypto/types.v1.rs +++ b/rs/protobuf/src/gen/crypto/types.v1.rs @@ -35,6 +35,8 @@ pub struct NiDkgId { pub dkg_tag: i32, #[prost(message, optional, tag = "5")] pub remote_target_id: ::core::option::Option<::prost::alloc::vec::Vec>, + #[prost(message, optional, tag = "6")] + pub key_id: ::core::option::Option, } #[derive(serde::Serialize, serde::Deserialize, Clone, Copy, PartialEq, ::prost::Message)] pub struct NominalCycles { @@ -100,6 +102,7 @@ pub enum NiDkgTag { Unspecified = 0, LowThreshold = 1, HighThreshold = 2, + HighThresholdForKey = 3, } impl NiDkgTag { /// String value of the enum field names used in the ProtoBuf definition. @@ -111,6 +114,7 @@ impl NiDkgTag { Self::Unspecified => "NI_DKG_TAG_UNSPECIFIED", Self::LowThreshold => "NI_DKG_TAG_LOW_THRESHOLD", Self::HighThreshold => "NI_DKG_TAG_HIGH_THRESHOLD", + Self::HighThresholdForKey => "NI_DKG_TAG_HIGH_THRESHOLD_FOR_KEY", } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -119,6 +123,7 @@ impl NiDkgTag { "NI_DKG_TAG_UNSPECIFIED" => Some(Self::Unspecified), "NI_DKG_TAG_LOW_THRESHOLD" => Some(Self::LowThreshold), "NI_DKG_TAG_HIGH_THRESHOLD" => Some(Self::HighThreshold), + "NI_DKG_TAG_HIGH_THRESHOLD_FOR_KEY" => Some(Self::HighThresholdForKey), _ => None, } } diff --git a/rs/protobuf/src/gen/messaging/types.v1.rs b/rs/protobuf/src/gen/messaging/types.v1.rs index a49c3e2e36f..59f003f321c 100644 --- a/rs/protobuf/src/gen/messaging/types.v1.rs +++ b/rs/protobuf/src/gen/messaging/types.v1.rs @@ -35,6 +35,8 @@ pub struct NiDkgId { pub dkg_tag: i32, #[prost(message, optional, tag = "5")] pub remote_target_id: ::core::option::Option<::prost::alloc::vec::Vec>, + #[prost(message, optional, tag = "6")] + pub key_id: ::core::option::Option, } #[derive(serde::Serialize, serde::Deserialize, Clone, Copy, PartialEq, ::prost::Message)] pub struct NominalCycles { @@ -100,6 +102,7 @@ pub enum NiDkgTag { Unspecified = 0, LowThreshold = 1, HighThreshold = 2, + HighThresholdForKey = 3, } impl NiDkgTag { /// String value of the enum field names used in the ProtoBuf definition. @@ -111,6 +114,7 @@ impl NiDkgTag { Self::Unspecified => "NI_DKG_TAG_UNSPECIFIED", Self::LowThreshold => "NI_DKG_TAG_LOW_THRESHOLD", Self::HighThreshold => "NI_DKG_TAG_HIGH_THRESHOLD", + Self::HighThresholdForKey => "NI_DKG_TAG_HIGH_THRESHOLD_FOR_KEY", } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -119,6 +123,7 @@ impl NiDkgTag { "NI_DKG_TAG_UNSPECIFIED" => Some(Self::Unspecified), "NI_DKG_TAG_LOW_THRESHOLD" => Some(Self::LowThreshold), "NI_DKG_TAG_HIGH_THRESHOLD" => Some(Self::HighThreshold), + "NI_DKG_TAG_HIGH_THRESHOLD_FOR_KEY" => Some(Self::HighThresholdForKey), _ => None, } } diff --git a/rs/protobuf/src/gen/registry/types.v1.rs b/rs/protobuf/src/gen/registry/types.v1.rs index a4772d33102..69525148912 100644 --- a/rs/protobuf/src/gen/registry/types.v1.rs +++ b/rs/protobuf/src/gen/registry/types.v1.rs @@ -35,6 +35,8 @@ pub struct NiDkgId { pub dkg_tag: i32, #[prost(message, optional, tag = "5")] pub remote_target_id: ::core::option::Option<::prost::alloc::vec::Vec>, + #[prost(message, optional, tag = "6")] + pub key_id: ::core::option::Option, } #[derive(Clone, Copy, PartialEq, ::prost::Message)] pub struct NominalCycles { @@ -88,6 +90,7 @@ pub enum NiDkgTag { Unspecified = 0, LowThreshold = 1, HighThreshold = 2, + HighThresholdForKey = 3, } impl NiDkgTag { /// String value of the enum field names used in the ProtoBuf definition. @@ -99,6 +102,7 @@ impl NiDkgTag { Self::Unspecified => "NI_DKG_TAG_UNSPECIFIED", Self::LowThreshold => "NI_DKG_TAG_LOW_THRESHOLD", Self::HighThreshold => "NI_DKG_TAG_HIGH_THRESHOLD", + Self::HighThresholdForKey => "NI_DKG_TAG_HIGH_THRESHOLD_FOR_KEY", } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -107,6 +111,7 @@ impl NiDkgTag { "NI_DKG_TAG_UNSPECIFIED" => Some(Self::Unspecified), "NI_DKG_TAG_LOW_THRESHOLD" => Some(Self::LowThreshold), "NI_DKG_TAG_HIGH_THRESHOLD" => Some(Self::HighThreshold), + "NI_DKG_TAG_HIGH_THRESHOLD_FOR_KEY" => Some(Self::HighThresholdForKey), _ => None, } } diff --git a/rs/protobuf/src/gen/state/types.v1.rs b/rs/protobuf/src/gen/state/types.v1.rs index 24d72856531..7ed42c11b5e 100644 --- a/rs/protobuf/src/gen/state/types.v1.rs +++ b/rs/protobuf/src/gen/state/types.v1.rs @@ -35,6 +35,8 @@ pub struct NiDkgId { pub dkg_tag: i32, #[prost(message, optional, tag = "5")] pub remote_target_id: ::core::option::Option<::prost::alloc::vec::Vec>, + #[prost(message, optional, tag = "6")] + pub key_id: ::core::option::Option, } #[derive(Clone, Copy, PartialEq, ::prost::Message)] pub struct NominalCycles { @@ -88,6 +90,7 @@ pub enum NiDkgTag { Unspecified = 0, LowThreshold = 1, HighThreshold = 2, + HighThresholdForKey = 3, } impl NiDkgTag { /// String value of the enum field names used in the ProtoBuf definition. @@ -99,6 +102,7 @@ impl NiDkgTag { Self::Unspecified => "NI_DKG_TAG_UNSPECIFIED", Self::LowThreshold => "NI_DKG_TAG_LOW_THRESHOLD", Self::HighThreshold => "NI_DKG_TAG_HIGH_THRESHOLD", + Self::HighThresholdForKey => "NI_DKG_TAG_HIGH_THRESHOLD_FOR_KEY", } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -107,6 +111,7 @@ impl NiDkgTag { "NI_DKG_TAG_UNSPECIFIED" => Some(Self::Unspecified), "NI_DKG_TAG_LOW_THRESHOLD" => Some(Self::LowThreshold), "NI_DKG_TAG_HIGH_THRESHOLD" => Some(Self::HighThreshold), + "NI_DKG_TAG_HIGH_THRESHOLD_FOR_KEY" => Some(Self::HighThresholdForKey), _ => None, } } diff --git a/rs/protobuf/src/gen/types/types.v1.rs b/rs/protobuf/src/gen/types/types.v1.rs index 70b5c346d04..c6b224d0946 100644 --- a/rs/protobuf/src/gen/types/types.v1.rs +++ b/rs/protobuf/src/gen/types/types.v1.rs @@ -122,6 +122,8 @@ pub struct NiDkgId { pub dkg_tag: i32, #[prost(message, optional, tag = "5")] pub remote_target_id: ::core::option::Option<::prost::alloc::vec::Vec>, + #[prost(message, optional, tag = "6")] + pub key_id: ::core::option::Option, } #[derive(Clone, Copy, PartialEq, ::prost::Message)] pub struct NominalCycles { @@ -131,7 +133,14 @@ pub struct NominalCycles { pub low: u64, } #[derive( - serde::Serialize, serde::Deserialize, Eq, candid::CandidType, Clone, PartialEq, ::prost::Message, + serde::Serialize, + serde::Deserialize, + Eq, + Hash, + candid::CandidType, + Clone, + PartialEq, + ::prost::Message, )] pub struct EcdsaKeyId { #[prost(enumeration = "EcdsaCurve", tag = "1")] @@ -139,28 +148,28 @@ pub struct EcdsaKeyId { #[prost(string, tag = "2")] pub name: ::prost::alloc::string::String, } -#[derive(serde::Serialize, serde::Deserialize, Clone, PartialEq, ::prost::Message)] +#[derive(serde::Serialize, serde::Deserialize, Eq, Hash, Clone, PartialEq, ::prost::Message)] pub struct SchnorrKeyId { #[prost(enumeration = "SchnorrAlgorithm", tag = "1")] pub algorithm: i32, #[prost(string, tag = "2")] pub name: ::prost::alloc::string::String, } -#[derive(serde::Serialize, serde::Deserialize, Clone, PartialEq, ::prost::Message)] +#[derive(serde::Serialize, serde::Deserialize, Eq, Hash, Clone, PartialEq, ::prost::Message)] pub struct VetKdKeyId { #[prost(enumeration = "VetKdCurve", tag = "1")] pub curve: i32, #[prost(string, tag = "2")] pub name: ::prost::alloc::string::String, } -#[derive(serde::Serialize, serde::Deserialize, Clone, PartialEq, ::prost::Message)] +#[derive(serde::Serialize, serde::Deserialize, Eq, Hash, Clone, PartialEq, ::prost::Message)] pub struct MasterPublicKeyId { #[prost(oneof = "master_public_key_id::KeyId", tags = "1, 2, 3")] pub key_id: ::core::option::Option, } /// Nested message and enum types in `MasterPublicKeyId`. pub mod master_public_key_id { - #[derive(serde::Serialize, serde::Deserialize, Clone, PartialEq, ::prost::Oneof)] + #[derive(serde::Serialize, serde::Deserialize, Eq, Hash, Clone, PartialEq, ::prost::Oneof)] pub enum KeyId { #[prost(message, tag = "1")] Ecdsa(super::EcdsaKeyId), @@ -177,6 +186,7 @@ pub enum NiDkgTag { Unspecified = 0, LowThreshold = 1, HighThreshold = 2, + HighThresholdForKey = 3, } impl NiDkgTag { /// String value of the enum field names used in the ProtoBuf definition. @@ -188,6 +198,7 @@ impl NiDkgTag { Self::Unspecified => "NI_DKG_TAG_UNSPECIFIED", Self::LowThreshold => "NI_DKG_TAG_LOW_THRESHOLD", Self::HighThreshold => "NI_DKG_TAG_HIGH_THRESHOLD", + Self::HighThresholdForKey => "NI_DKG_TAG_HIGH_THRESHOLD_FOR_KEY", } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -196,6 +207,7 @@ impl NiDkgTag { "NI_DKG_TAG_UNSPECIFIED" => Some(Self::Unspecified), "NI_DKG_TAG_LOW_THRESHOLD" => Some(Self::LowThreshold), "NI_DKG_TAG_HIGH_THRESHOLD" => Some(Self::HighThreshold), + "NI_DKG_TAG_HIGH_THRESHOLD_FOR_KEY" => Some(Self::HighThresholdForKey), _ => None, } } @@ -382,6 +394,8 @@ pub struct TaggedNiDkgTranscript { pub transcript: ::core::option::Option, #[prost(enumeration = "NiDkgTag", tag = "2")] pub tag: i32, + #[prost(message, optional, tag = "3")] + pub key_id: ::core::option::Option, } #[derive(Clone, PartialEq, ::prost::Message)] pub struct CallbackIdedNiDkgTranscript { diff --git a/rs/types/management_canister_types/src/lib.rs b/rs/types/management_canister_types/src/lib.rs index e9fdf516511..563f1a08a57 100644 --- a/rs/types/management_canister_types/src/lib.rs +++ b/rs/types/management_canister_types/src/lib.rs @@ -35,7 +35,7 @@ use serde::Serialize; use serde_bytes::ByteBuf; use std::mem::size_of; use std::{collections::BTreeSet, convert::TryFrom, error::Error, fmt, slice::Iter, str::FromStr}; -use strum_macros::{Display, EnumIter, EnumString}; +use strum_macros::{Display, EnumCount, EnumIter, EnumString}; /// The id of the management canister. pub const IC_00: CanisterId = CanisterId::ic_00(); @@ -2388,7 +2388,17 @@ impl FromStr for VetKdKeyId { /// (variant { EcdsaKeyId; SchnorrKeyId }) /// ``` #[derive( - Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, CandidType, Deserialize, Serialize, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + Hash, + Debug, + CandidType, + Deserialize, + EnumCount, + Serialize, )] pub enum MasterPublicKeyId { Ecdsa(EcdsaKeyId), diff --git a/rs/types/types/src/consensus/dkg.rs b/rs/types/types/src/consensus/dkg.rs index cb32c6c7aa4..49db861e46b 100644 --- a/rs/types/types/src/consensus/dkg.rs +++ b/rs/types/types/src/consensus/dkg.rs @@ -9,6 +9,7 @@ use crate::{ messages::CallbackId, ReplicaVersion, }; +use ic_management_canister_types::MasterPublicKeyId; use ic_protobuf::types::v1 as pb; use serde_with::serde_as; use std::collections::BTreeMap; @@ -300,6 +301,10 @@ fn build_tagged_transcripts_vec( .map(|(tag, transcript)| pb::TaggedNiDkgTranscript { tag: pb::NiDkgTag::from(tag) as i32, transcript: Some(pb::NiDkgTranscript::from(transcript)), + key_id: match tag { + NiDkgTag::HighThresholdForKey(k) => Some(pb::MasterPublicKeyId::from(k)), + _ => None, + }, }) .collect() } @@ -375,12 +380,55 @@ fn build_tagged_transcripts_map( .ok_or_else(|| ProxyDecodeError::MissingField("TaggedNiDkgTranscript::transcript")) .and_then(|t| { Ok(( - NiDkgTag::try_from(tagged_transcript.tag).map_err(|e| { - ProxyDecodeError::Other(format!( - "Failed to convert NiDkgTag of transcript: {:?}", - e - )) - })?, + ///////////////////////////////////////////////// + // TODO: ideally we extend existing tests where build_tagged_transcripts_map + // is used (e.g., tests for `impl TryFrom for Summary`) to test + // this new behavior, but I cannot find such tests: are there no such tests + // currently, or am I overlooking them? + ///////////////////////////////////////////////// + match tagged_transcript.tag { + 1 => Ok(NiDkgTag::LowThreshold), + 2 => Ok(NiDkgTag::HighThreshold), + 3 => { + let mpkid_proto = + tagged_transcript.key_id.as_ref().ok_or_else(|| { + ProxyDecodeError::Other( + "Failed to convert NiDkgTag of transcript: \ + Invalid DkgTag: missing key ID" + .to_string(), + ) + })?; + //////////////////////////////////////////////////////// + // TODO: Consensus team to decide if we should rather + // not extend the TaggedNiDkgTranscript with a key ID and + // instead use the key ID from the tagged_transcript's + // transcripts's NIDkgId as follows: + // let mpkid_proto = tagged_transcript + // .transcript + // .as_ref() + // .ok_or_else(|| ProxyDecodeError::Other("TODO".to_string()))? + // .dkg_id + // .as_ref() + // .ok_or_else(|| ProxyDecodeError::Other("TODO".to_string()))? + // .key_id + // .as_ref() + // .ok_or_else(|| ProxyDecodeError::Other("TODO".to_string()))?; + //////////////////////////////////////////////////////// + + let mpkid = MasterPublicKeyId::try_from(mpkid_proto.clone()) + .map_err(|e| { + ProxyDecodeError::Other(format!( + "Failed to convert NiDkgTag of transcript: \ + Invalid MasterPublicKeyId: {e}" + )) + })?; + Ok(NiDkgTag::HighThresholdForKey(mpkid)) + } + _ => Err(ProxyDecodeError::Other( + "Failed to convert NiDkgTag of transcript: Invalid DKG tag" + .to_string(), + )), + }?, NiDkgTranscript::try_from(t).map_err(ProxyDecodeError::Other)?, )) }) @@ -535,7 +583,7 @@ impl NiDkgTag { let f = crate::consensus::get_faults_tolerated(committee_size); match self { NiDkgTag::LowThreshold => f + 1, - NiDkgTag::HighThreshold => committee_size - f, + NiDkgTag::HighThreshold | NiDkgTag::HighThresholdForKey(_) => committee_size - f, } } } @@ -585,7 +633,14 @@ impl TryFrom for Payload { #[cfg(test)] mod tests { + use super::*; + use ic_management_canister_types::{ + EcdsaCurve, EcdsaKeyId, MasterPublicKeyId, SchnorrAlgorithm, SchnorrKeyId, VetKdCurve, + VetKdKeyId, + }; + use strum::EnumCount; + use strum::IntoEnumIterator; #[test] fn should_correctly_calculate_threshold_for_ni_dkg_tag_low_threshold() { @@ -600,6 +655,7 @@ mod tests { assert_eq!(low_threshold_tag.threshold_for_subnet_of_size(28), 10); assert_eq!(low_threshold_tag.threshold_for_subnet_of_size(64), 22); } + #[test] fn should_correctly_calculate_threshold_for_ni_dkg_tag_high_threshold() { let high_threshold_tag = NiDkgTag::HighThreshold; @@ -614,6 +670,44 @@ mod tests { assert_eq!(high_threshold_tag.threshold_for_subnet_of_size(64), 43); } + #[test] + fn should_correctly_calculate_threshold_for_ni_dkg_tag_high_threshold_for_key() { + for master_public_key_id in [ + MasterPublicKeyId::Ecdsa(EcdsaKeyId { + curve: EcdsaCurve::Secp256k1, + name: "some key".to_string(), + }), + MasterPublicKeyId::Schnorr(SchnorrKeyId { + algorithm: SchnorrAlgorithm::Bip340Secp256k1, + name: "some key".to_string(), + }), + MasterPublicKeyId::Schnorr(SchnorrKeyId { + algorithm: SchnorrAlgorithm::Ed25519, + name: "some key".to_string(), + }), + MasterPublicKeyId::VetKd(VetKdKeyId { + curve: VetKdCurve::Bls12_381_G2, + name: "some key".to_string(), + }), + ] { + let tag = NiDkgTag::HighThresholdForKey(master_public_key_id); + + assert_eq!(tag.threshold_for_subnet_of_size(0), 1); + assert_eq!(tag.threshold_for_subnet_of_size(1), 1); + assert_eq!(tag.threshold_for_subnet_of_size(2), 1); + assert_eq!(tag.threshold_for_subnet_of_size(3), 1); + assert_eq!(tag.threshold_for_subnet_of_size(4), 3); + assert_eq!(tag.threshold_for_subnet_of_size(5), 3); + assert_eq!(tag.threshold_for_subnet_of_size(6), 3); + assert_eq!(tag.threshold_for_subnet_of_size(28), 19); + assert_eq!(tag.threshold_for_subnet_of_size(64), 43); + } + assert_eq!(MasterPublicKeyId::COUNT, 3); + assert_eq!(EcdsaCurve::iter().count(), 1); + assert_eq!(SchnorrAlgorithm::iter().count(), 2); + assert_eq!(VetKdCurve::iter().count(), 1); + } + #[test] fn should_correctly_calculate_faults_tolerated_for_committee_of_size() { use crate::consensus::get_faults_tolerated; diff --git a/rs/types/types/src/crypto/threshold_sig/ni_dkg.rs b/rs/types/types/src/crypto/threshold_sig/ni_dkg.rs index 05c6831af45..1170844574c 100644 --- a/rs/types/types/src/crypto/threshold_sig/ni_dkg.rs +++ b/rs/types/types/src/crypto/threshold_sig/ni_dkg.rs @@ -9,12 +9,13 @@ use core::fmt; use ic_crypto_internal_types::sign::threshold_sig::ni_dkg::{CspNiDkgDealing, CspNiDkgTranscript}; #[cfg(test)] use ic_exhaustive_derive::ExhaustiveSet; +use ic_management_canister_types::MasterPublicKeyId; use ic_protobuf::types::v1 as pb; use ic_protobuf::types::v1::NiDkgId as NiDkgIdProto; use serde::{Deserialize, Serialize}; use std::collections::BTreeSet; use std::convert::TryFrom; -use strum_macros::EnumIter; +use strum_macros::EnumCount; use thiserror::Error; pub mod config; @@ -31,11 +32,12 @@ mod tests; /// Allows to distinguish protocol executions in high and low threshold /// settings. -#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Deserialize, EnumIter, Serialize)] -#[cfg_attr(test, derive(ExhaustiveSet))] +#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Deserialize, EnumCount, Serialize)] +#[repr(isize)] pub enum NiDkgTag { LowThreshold = 1, HighThreshold = 2, + HighThresholdForKey(MasterPublicKeyId) = 3, } impl From<&NiDkgTag> for pb::NiDkgTag { @@ -43,6 +45,9 @@ impl From<&NiDkgTag> for pb::NiDkgTag { match tag { NiDkgTag::LowThreshold => pb::NiDkgTag::LowThreshold, NiDkgTag::HighThreshold => pb::NiDkgTag::HighThreshold, + NiDkgTag::HighThresholdForKey(_master_public_key_id) => { + pb::NiDkgTag::HighThresholdForKey + } } } } @@ -100,18 +105,6 @@ impl fmt::Display for NiDkgTargetSubnet { } } -impl TryFrom for NiDkgTag { - type Error = (); - - fn try_from(ni_dkg_tag: i32) -> Result { - match ni_dkg_tag { - 1 => Ok(NiDkgTag::LowThreshold), - 2 => Ok(NiDkgTag::HighThreshold), - _ => Err(()), - } - } -} - /// A dealer's contribution (called dealing) to distributed key generation. #[derive(Clone, Eq, PartialEq, Hash, Debug, Deserialize, Serialize)] pub struct NiDkgDealing { diff --git a/rs/types/types/src/crypto/threshold_sig/ni_dkg/id.rs b/rs/types/types/src/crypto/threshold_sig/ni_dkg/id.rs index 66985cd50a4..1248c9e9227 100644 --- a/rs/types/types/src/crypto/threshold_sig/ni_dkg/id.rs +++ b/rs/types/types/src/crypto/threshold_sig/ni_dkg/id.rs @@ -47,11 +47,15 @@ impl From for NiDkgIdProto { NiDkgIdProto { start_block_height: ni_dkg_id.start_block_height.get(), dealer_subnet: ni_dkg_id.dealer_subnet.get().into_vec(), - dkg_tag: ni_dkg_id.dkg_tag as i32, + dkg_tag: pb::NiDkgTag::from(&ni_dkg_id.dkg_tag) as i32, remote_target_id: match ni_dkg_id.target_subnet { NiDkgTargetSubnet::Remote(target_id) => Some(target_id.0.to_vec()), NiDkgTargetSubnet::Local => None, }, + key_id: match ni_dkg_id.dkg_tag { + NiDkgTag::HighThresholdForKey(k) => Some(pb::MasterPublicKeyId::from(&k)), + _ => None, + }, } } } @@ -66,8 +70,22 @@ impl TryFrom for NiDkgId { PrincipalId::try_from(ni_dkg_id_proto.dealer_subnet.as_slice()) .map_err(NiDkgIdFromProtoError::InvalidPrincipalId)?, ), - dkg_tag: NiDkgTag::try_from(ni_dkg_id_proto.dkg_tag) - .map_err(|_| NiDkgIdFromProtoError::InvalidDkgTag)?, + dkg_tag: { + match ni_dkg_id_proto.dkg_tag { + 1 => Ok(NiDkgTag::LowThreshold), + 2 => Ok(NiDkgTag::HighThreshold), + 3 => { + let mpkid_proto = ni_dkg_id_proto + .key_id + .ok_or(NiDkgIdFromProtoError::InvalidDkgTagMissingKeyId)?; + let mpkid = MasterPublicKeyId::try_from(mpkid_proto).map_err(|e| { + NiDkgIdFromProtoError::InvalidMasterPublicKeyId(format!("{e}")) + })?; + Ok(NiDkgTag::HighThresholdForKey(mpkid)) + } + _ => Err(NiDkgIdFromProtoError::InvalidDkgTag), + }? + }, target_subnet: match ni_dkg_id_proto.remote_target_id { None => NiDkgTargetSubnet::Local, // Note that empty bytes (which are different from None) will lead to an error. @@ -103,7 +121,9 @@ pub fn ni_dkg_target_id(data: &[u8]) -> Result for ic_protobuf::proxy::ProxyDecodeError { @@ -112,9 +132,15 @@ impl From for ic_protobuf::proxy::ProxyDecodeError { match error { InvalidPrincipalId(err) => Self::InvalidPrincipalId(Box::new(err)), InvalidDkgTag => Self::Other("Invalid DKG tag.".to_string()), + InvalidDkgTagMissingKeyId => { + Self::Other("Invalid DKG tag: missing the mandatory key ID.".to_string()) + } InvalidRemoteTargetIdSize(_) => { Self::Other("Invalid remote target Id size.".to_string()) } + InvalidMasterPublicKeyId(e) => { + Self::Other(format!("Invalid master public key for NiDkgTag: {e}.")) + } } } } diff --git a/rs/types/types/src/crypto/threshold_sig/ni_dkg/id/tests.rs b/rs/types/types/src/crypto/threshold_sig/ni_dkg/id/tests.rs index 1541705abe3..3fa606c9ddd 100644 --- a/rs/types/types/src/crypto/threshold_sig/ni_dkg/id/tests.rs +++ b/rs/types/types/src/crypto/threshold_sig/ni_dkg/id/tests.rs @@ -1,3 +1,6 @@ +use assert_matches::assert_matches; +use ic_management_canister_types::{EcdsaCurve, EcdsaKeyId}; + use super::*; #[test] @@ -21,6 +24,37 @@ fn should_convert_ni_dkg_id_to_proto() { dealer_subnet: principal_id.into_vec(), remote_target_id: Some(target_id.to_vec()), dkg_tag: 2, + key_id: None, + } + ) +} + +#[test] +fn should_convert_ni_dkg_id_with_key_id_to_proto() { + let principal_id = PrincipalId::new_subnet_test_id(42); + let target_id = [42; NiDkgTargetId::SIZE]; + let height = 7; + let master_public_key_id = MasterPublicKeyId::Ecdsa(EcdsaKeyId { + curve: EcdsaCurve::Secp256k1, + name: "key".to_string(), + }); + let id = NiDkgId { + start_block_height: Height::new(7), + dealer_subnet: SubnetId::from(principal_id), + dkg_tag: NiDkgTag::HighThresholdForKey(master_public_key_id.clone()), + target_subnet: NiDkgTargetSubnet::Remote(NiDkgTargetId::new(target_id)), + }; + + let proto = NiDkgIdProto::from(id); + + assert_eq!( + proto, + NiDkgIdProto { + start_block_height: height, + dealer_subnet: principal_id.into_vec(), + remote_target_id: Some(target_id.to_vec()), + dkg_tag: pb::NiDkgTag::HighThresholdForKey as i32, + key_id: Some(pb::MasterPublicKeyId::from(&master_public_key_id)), } ) } @@ -37,6 +71,7 @@ fn should_parse_valid_proto_as_ni_dkg_id() { dealer_subnet: principal_id_blob.clone(), remote_target_id: val.clone(), dkg_tag: 2, + key_id: None, }; let id = NiDkgId::try_from(proto).unwrap(); @@ -58,6 +93,44 @@ fn should_parse_valid_proto_as_ni_dkg_id() { } } +#[test] +fn should_parse_valid_proto_as_ni_dkg_id_with_key_id() { + let principal_id_blob = vec![42; PrincipalId::MAX_LENGTH_IN_BYTES]; + let target_id = [42; NiDkgTargetId::SIZE]; + let height = 7; + let master_public_key_id = MasterPublicKeyId::Ecdsa(EcdsaKeyId { + curve: EcdsaCurve::Secp256k1, + name: "key".to_string(), + }); + + for val in [None, Some(target_id.to_vec())].iter() { + let proto = NiDkgIdProto { + start_block_height: height, + dealer_subnet: principal_id_blob.clone(), + remote_target_id: val.clone(), + dkg_tag: pb::NiDkgTag::HighThresholdForKey as i32, + key_id: Some(pb::MasterPublicKeyId::from(&master_public_key_id)), + }; + + let id = NiDkgId::try_from(proto).unwrap(); + + assert_eq!( + id, + NiDkgId { + start_block_height: Height::new(height), + dealer_subnet: SubnetId::from( + PrincipalId::try_from(principal_id_blob.as_slice()).unwrap() + ), + dkg_tag: NiDkgTag::HighThresholdForKey(master_public_key_id.clone()), + target_subnet: match val { + None => NiDkgTargetSubnet::Local, + Some(_) => NiDkgTargetSubnet::Remote(NiDkgTargetId::new(target_id)), + }, + } + ); + } +} + #[test] fn should_return_error_if_remote_target_id_invalid_when_parsing_proto() { let target_id_size = NiDkgTargetId::SIZE - 2; @@ -66,6 +139,7 @@ fn should_return_error_if_remote_target_id_invalid_when_parsing_proto() { dealer_subnet: vec![42; PrincipalId::MAX_LENGTH_IN_BYTES], remote_target_id: Some(vec![42; target_id_size]), dkg_tag: 1, + key_id: None, }; let result = NiDkgId::try_from(proto); @@ -76,14 +150,57 @@ fn should_return_error_if_remote_target_id_invalid_when_parsing_proto() { ); } +#[test] +fn should_return_error_if_ni_dkg_tag_invalid_with_missing_keyid_when_parsing_proto() { + let dkg_tag_requiring_some_key_id = 3; + let proto = NiDkgIdProto { + start_block_height: 7, + dealer_subnet: vec![42; PrincipalId::MAX_LENGTH_IN_BYTES], + remote_target_id: Some(vec![42; NiDkgTargetId::SIZE]), + dkg_tag: dkg_tag_requiring_some_key_id, + key_id: None, + }; + + let result = NiDkgId::try_from(proto); + + assert_eq!( + result.unwrap_err(), + NiDkgIdFromProtoError::InvalidDkgTagMissingKeyId + ); +} + +#[test] +fn should_return_error_if_ni_dkg_tag_invalid_with_invalid_master_public_key_when_parsing_proto() { + let proto = NiDkgIdProto { + start_block_height: 7, + dealer_subnet: vec![42; PrincipalId::MAX_LENGTH_IN_BYTES], + remote_target_id: Some(vec![42; NiDkgTargetId::SIZE]), + dkg_tag: pb::NiDkgTag::HighThresholdForKey as i32, + key_id: Some(pb::MasterPublicKeyId { + key_id: Some(pb::master_public_key_id::KeyId::Ecdsa(pb::EcdsaKeyId { + curve: 99, + name: "invalid_curve".to_string(), + })), + }), + }; + + let result = NiDkgId::try_from(proto); + + assert_matches!( + result.unwrap_err(), + NiDkgIdFromProtoError::InvalidMasterPublicKeyId(_) + ); +} + #[test] fn should_return_error_if_ni_dkg_tag_invalid_when_parsing_proto() { - let invalid_dkg_tag = 3; + let invalid_dkg_tag = 4; let proto = NiDkgIdProto { start_block_height: 7, dealer_subnet: vec![42; PrincipalId::MAX_LENGTH_IN_BYTES], remote_target_id: Some(vec![42; NiDkgTargetId::SIZE]), dkg_tag: invalid_dkg_tag, + key_id: None, }; let result = NiDkgId::try_from(proto); @@ -99,6 +216,7 @@ fn should_return_error_if_dealer_subnet_id_invalid_when_parsing_proto() { dealer_subnet: vec![42; invalid_principal_length], remote_target_id: Some(vec![42; NiDkgTargetId::SIZE]), dkg_tag: 2, + key_id: None, }; let result = NiDkgId::try_from(proto); diff --git a/rs/types/types/src/crypto/threshold_sig/ni_dkg/tests.rs b/rs/types/types/src/crypto/threshold_sig/ni_dkg/tests.rs index e2508479f3d..48953e59d3c 100644 --- a/rs/types/types/src/crypto/threshold_sig/ni_dkg/tests.rs +++ b/rs/types/types/src/crypto/threshold_sig/ni_dkg/tests.rs @@ -301,18 +301,3 @@ fn empty_ni_csp_dkg_transcript() -> CspNiDkgTranscript { receiver_data: Default::default(), }) } - -#[test] -fn should_correctly_convert_i32_to_ni_dkg_tag() { - assert!(NiDkgTag::try_from(-1).is_err()); - assert!(NiDkgTag::try_from(0).is_err()); - assert_eq!(NiDkgTag::try_from(1), Ok(NiDkgTag::LowThreshold)); - assert_eq!(NiDkgTag::try_from(2), Ok(NiDkgTag::HighThreshold)); - assert!(NiDkgTag::try_from(3).is_err()); -} - -#[test] -fn should_correctly_convert_ni_dkg_tag_to_i32() { - assert_eq!(NiDkgTag::LowThreshold as i32, 1); - assert_eq!(NiDkgTag::HighThreshold as i32, 2); -} diff --git a/rs/types/types/src/exhaustive.rs b/rs/types/types/src/exhaustive.rs index d494659d24b..0b2abb4eeb9 100644 --- a/rs/types/types/src/exhaustive.rs +++ b/rs/types/types/src/exhaustive.rs @@ -631,6 +631,13 @@ impl ExhaustiveSet for Signed> { } } +// TODO(CON-1433): Remove once NiDkgTag::HighThresholdForKey variant is supported by the mainnet version +impl ExhaustiveSet for NiDkgTag { + fn exhaustive_set(_: &mut R) -> Vec { + vec![NiDkgTag::LowThreshold, NiDkgTag::HighThreshold] + } +} + impl ExhaustiveSet for NiDkgConfig { fn exhaustive_set(_: &mut R) -> Vec { vec![NiDkgConfig::new(valid_dkg_config_data()).unwrap()]