diff --git a/rs/registry/canister/src/invariants/checks.rs b/rs/registry/canister/src/invariants/checks.rs index 01a1b84f37f..17389bf1dd2 100644 --- a/rs/registry/canister/src/invariants/checks.rs +++ b/rs/registry/canister/src/invariants/checks.rs @@ -949,6 +949,7 @@ mod tests { |subnet_id, snapshot| { vec![ apply_to_subnet_record(snapshot, subnet_id, |record| { + record.chain_key_config = Some(ChainKeyConfig::from(config.clone())); record.ecdsa_config = Some(config); }), upsert_ck(ck_key_id.clone(), &[subnet_id]), @@ -988,9 +989,11 @@ mod tests { |subnet_id, snapshot| { vec![ apply_to_subnet_record(snapshot, subnet_id, |record| { + record.chain_key_config = Some(ChainKeyConfig::from(config.clone())); record.ecdsa_config = Some(config.clone()); }), apply_to_subnet_record(snapshot, subnet_test_id(1), |record| { + record.chain_key_config = Some(ChainKeyConfig::from(config.clone())); record.ecdsa_config = Some(config); }), upsert_ecdsa(ecdsa_key_id.clone(), &[subnet_test_id(1)]), diff --git a/rs/registry/canister/src/invariants/crypto.rs b/rs/registry/canister/src/invariants/crypto.rs index 092abf555f0..ec5fb6c6dda 100644 --- a/rs/registry/canister/src/invariants/crypto.rs +++ b/rs/registry/canister/src/invariants/crypto.rs @@ -9,10 +9,11 @@ use crate::invariants::{ }; use ic_base_types::{subnet_id_try_from_protobuf, NodeId, SubnetId}; use ic_crypto_utils_ni_dkg::extract_subnet_threshold_sig_public_key; -use ic_protobuf::registry::crypto::v1::{PublicKey, X509PublicKeyCert}; +use ic_protobuf::registry::crypto::v1::{MasterPublicKeyId, PublicKey, X509PublicKeyCert}; use ic_protobuf::registry::subnet::v1::{CatchUpPackageContents, SubnetRecord}; use ic_registry_keys::{ - get_ecdsa_key_id_from_signing_subnet_list_key, make_catch_up_package_contents_key, + get_ecdsa_key_id_from_signing_subnet_list_key, + get_master_public_key_id_from_signing_subnet_list_key, make_catch_up_package_contents_key, make_crypto_threshold_signing_pubkey_key, make_node_record_key, make_subnet_record_key, maybe_parse_crypto_node_key, maybe_parse_crypto_tls_cert_key, CRYPTO_RECORD_KEY_PREFIX, CRYPTO_TLS_CERT_KEY_PREFIX, NODE_RECORD_KEY_PREFIX, @@ -27,6 +28,8 @@ use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use dfn_core::println; use ic_crypto_utils_basic_sig::conversions::derive_node_id; +use super::common::get_all_chain_key_signing_subnet_list_records; + #[cfg(test)] mod tests; @@ -65,6 +68,7 @@ pub(crate) fn check_node_crypto_keys_invariants( check_no_orphaned_node_crypto_records(snapshot)?; check_chain_key_configs(snapshot)?; check_ecdsa_signing_subnet_lists(snapshot)?; + check_chain_key_signing_subnet_lists(snapshot)?; check_high_threshold_public_key_matches_the_one_in_cup(snapshot)?; Ok(()) } @@ -315,7 +319,7 @@ fn check_chain_key_configs(snapshot: &RegistrySnapshot) -> Result<(), InvariantC Ok(()) } -//TODO(NNS1-3039): extend to check chain key signing subnet lists +// TODO[NNS1-2986]: Remove this function after the migration has been performed. fn check_ecdsa_signing_subnet_lists( snapshot: &RegistrySnapshot, ) -> Result<(), InvariantCheckError> { @@ -373,6 +377,71 @@ fn check_ecdsa_signing_subnet_lists( }) } +/// Checks that the chain key signing subnet list is consistent with the chain key configurations +/// +/// In particular, this function checks: +/// - That every subnet refered to by the signing subnet list exists +/// - That the subnet has a chain key configuration that contains the corresponding key +fn check_chain_key_signing_subnet_lists( + snapshot: &RegistrySnapshot, +) -> Result<(), InvariantCheckError> { + let subnet_records_map = get_subnet_records_map(snapshot); + + get_all_chain_key_signing_subnet_list_records(snapshot) + .iter() + .try_for_each(|(key_id, chain_key_signing_subnet_list)| { + let master_key_id = get_master_public_key_id_from_signing_subnet_list_key(key_id) + .map_err(|err| InvariantCheckError { + msg: format!( + "Registry key_id {} could not be converted to an MasterPublicKeyId", + key_id, + ), + source: Some(Box::new(err)), + })?; + + chain_key_signing_subnet_list + .subnets + .iter() + .try_for_each(|subnet_id_bytes| { + let subnet_id = subnet_id_try_from_protobuf(subnet_id_bytes.clone()) + .map_err(|err| InvariantCheckError { + msg: "Failed to deserialize subnet id from protobuf".to_string(), + source: Some(Box::new(err)), + })?; + + if subnet_records_map + .get(&make_subnet_record_key(subnet_id).into_bytes()) + .ok_or(InvariantCheckError { + msg: format!( + "A non-existent subnet {} was set as the holder of a key_id {}", + subnet_id, key_id + ), + source: None, + })? + .chain_key_config + .as_ref() + .ok_or(InvariantCheckError { + msg: format!("The subnet {} does not have a ChainKeyConfig", subnet_id), + source: None, + })? + .key_configs + .iter() + .filter_map(|config| config.key_id.clone()) + .any(|key| key == MasterPublicKeyId::from(&master_key_id)) { + Ok(()) + } else { + Err(InvariantCheckError { + msg: format!( + "The subnet {} does not have the key with {} in its chain key configurations", + subnet_id, key_id + ), + source: None, + }) + } + }) + }) +} + fn check_no_orphaned_node_crypto_records( snapshot: &RegistrySnapshot, ) -> Result<(), InvariantCheckError> { diff --git a/rs/registry/canister/tests/common/test_helpers.rs b/rs/registry/canister/tests/common/test_helpers.rs index d3ec5256caf..77ea7695054 100644 --- a/rs/registry/canister/tests/common/test_helpers.rs +++ b/rs/registry/canister/tests/common/test_helpers.rs @@ -20,7 +20,7 @@ use ic_registry_keys::{ }; use ic_registry_proto_data_provider::ProtoRegistryDataProvider; use ic_registry_routing_table::RoutingTable; -use ic_registry_subnet_features::{EcdsaConfig, DEFAULT_ECDSA_MAX_QUEUE_SIZE}; +use ic_registry_subnet_features::{ChainKeyConfig, EcdsaConfig, DEFAULT_ECDSA_MAX_QUEUE_SIZE}; use ic_registry_transport::pb::v1::RegistryAtomicMutateRequest; use ic_types::ReplicaVersion; use registry_canister::init::RegistryCanisterInitPayloadBuilder; @@ -59,16 +59,16 @@ pub fn get_subnet_holding_ecdsa_keys( ..Default::default() } .into(); - record.ecdsa_config = Some( - EcdsaConfig { - quadruples_to_create_in_advance: 1, - key_ids: ecdsa_key_ids.to_vec(), - max_queue_size: Some(DEFAULT_ECDSA_MAX_QUEUE_SIZE), - signature_request_timeout_ns: None, - idkg_key_rotation_period_ms: None, - } - .into(), - ); + + let ecdsa_config = EcdsaConfig { + quadruples_to_create_in_advance: 1, + key_ids: ecdsa_key_ids.to_vec(), + max_queue_size: Some(DEFAULT_ECDSA_MAX_QUEUE_SIZE), + signature_request_timeout_ns: None, + idkg_key_rotation_period_ms: None, + }; + record.chain_key_config = Some(ChainKeyConfig::from(ecdsa_config.clone()).into()); + record.ecdsa_config = Some(ecdsa_config.into()); record }