diff --git a/rs/registry/canister/src/invariants/crypto/tests.rs b/rs/registry/canister/src/invariants/crypto/tests.rs index 513a244373b4..e5aa12cbe7aa 100644 --- a/rs/registry/canister/src/invariants/crypto/tests.rs +++ b/rs/registry/canister/src/invariants/crypto/tests.rs @@ -682,7 +682,7 @@ mod chain_key_enabled_subnet_lists { #[should_panic( expected = "Missing required struct field: KeyConfig::pre_signatures_to_create_in_advance" )] - fn should_fail_if_missing_pre_signatures() { + fn should_fail_if_missing_pre_signatures_for_key_that_requires_pre_signatures() { let mut config = invariant_compliant_chain_key_config(); config.key_configs[1].pre_signatures_to_create_in_advance = None; check_chain_key_config_invariant(config); @@ -692,12 +692,36 @@ mod chain_key_enabled_subnet_lists { #[should_panic( expected = "`pre_signatures_to_create_in_advance` for key ecdsa:Secp256k1:ecdsa_key of subnet ya35z-hhham-aaaaa-aaaap-yai cannot be zero." )] - fn should_fail_if_pre_signatures_is_zero() { + fn should_fail_if_pre_signatures_is_zero_for_key_that_requires_pre_signatures() { let mut config = invariant_compliant_chain_key_config(); config.key_configs[0].pre_signatures_to_create_in_advance = Some(0); check_chain_key_config_invariant(config); } + #[test] + fn should_succeed_if_pre_signatures_is_missing_for_key_that_does_not_require_pre_signatures() { + let mut config = invariant_compliant_chain_key_config(); + let key_config = &mut config.key_configs[2]; + assert!(matches!( + key_config.key_id.as_ref().unwrap().key_id, + Some(master_public_key_id::KeyId::Vetkd(_)) + ),); + key_config.pre_signatures_to_create_in_advance = None; + check_chain_key_config_invariant(config); + } + + #[test] + fn should_succeed_if_pre_signatures_is_zero_for_key_that_does_not_require_pre_signatures() { + let mut config = invariant_compliant_chain_key_config(); + let key_config = &mut config.key_configs[2]; + assert!(matches!( + key_config.key_id.as_ref().unwrap().key_id, + Some(master_public_key_id::KeyId::Vetkd(_)) + ),); + key_config.pre_signatures_to_create_in_advance = Some(0); + check_chain_key_config_invariant(config); + } + #[test] #[should_panic(expected = "Missing required struct field: KeyConfig::max_queue_size")] fn should_fail_if_missing_queue_size() { diff --git a/rs/registry/canister/src/mutations/do_create_subnet.rs b/rs/registry/canister/src/mutations/do_create_subnet.rs index a689fe407854..6a2c3aafed72 100644 --- a/rs/registry/canister/src/mutations/do_create_subnet.rs +++ b/rs/registry/canister/src/mutations/do_create_subnet.rs @@ -414,10 +414,12 @@ impl TryFrom for KeyConfigInternal { return Err("KeyConfig.key_id must be specified.".to_string()); }; - let Some(pre_signatures_to_create_in_advance) = pre_signatures_to_create_in_advance else { - return Err( - "KeyConfig.pre_signatures_to_create_in_advance must be specified.".to_string(), - ); + // Ensure presence of `pre_signatures_to_create_in_advance` for keys that require pre-signatures. + // Note that an invariant ensures that this field is not zero for keys that require pre-signatures. + if key_id.requires_pre_signatures() && pre_signatures_to_create_in_advance.is_none() { + return Err(format!( + "KeyConfig.pre_signatures_to_create_in_advance must be specified for key {key_id}." + )); }; let Some(max_queue_size) = max_queue_size else { @@ -426,7 +428,7 @@ impl TryFrom for KeyConfigInternal { Ok(Self { key_id, - pre_signatures_to_create_in_advance, + pre_signatures_to_create_in_advance: pre_signatures_to_create_in_advance.unwrap_or(0), max_queue_size, }) } diff --git a/rs/registry/canister/src/mutations/do_recover_subnet.rs b/rs/registry/canister/src/mutations/do_recover_subnet.rs index 53a5291ab41c..b936eae813a7 100644 --- a/rs/registry/canister/src/mutations/do_recover_subnet.rs +++ b/rs/registry/canister/src/mutations/do_recover_subnet.rs @@ -378,10 +378,12 @@ impl TryFrom for KeyConfigInternal { return Err("KeyConfig.key_id must be specified.".to_string()); }; - let Some(pre_signatures_to_create_in_advance) = pre_signatures_to_create_in_advance else { - return Err( - "KeyConfig.pre_signatures_to_create_in_advance must be specified.".to_string(), - ); + // Ensure presence of `pre_signatures_to_create_in_advance` for keys that require pre-signatures. + // Note that an invariant ensures that this field is not zero for keys that require pre-signatures. + if key_id.requires_pre_signatures() && pre_signatures_to_create_in_advance.is_none() { + return Err(format!( + "KeyConfig.pre_signatures_to_create_in_advance must be specified for key {key_id}." + )); }; let Some(max_queue_size) = max_queue_size else { @@ -390,7 +392,7 @@ impl TryFrom for KeyConfigInternal { Ok(Self { key_id, - pre_signatures_to_create_in_advance, + pre_signatures_to_create_in_advance: pre_signatures_to_create_in_advance.unwrap_or(0), max_queue_size, }) } diff --git a/rs/registry/canister/src/mutations/do_update_subnet.rs b/rs/registry/canister/src/mutations/do_update_subnet.rs index 67db8665e04b..0501330f6ed3 100644 --- a/rs/registry/canister/src/mutations/do_update_subnet.rs +++ b/rs/registry/canister/src/mutations/do_update_subnet.rs @@ -368,18 +368,22 @@ impl TryFrom for KeyConfigInternal { let Some(key_id) = key_id else { return Err("KeyConfig.key_id must be specified.".to_string()); }; - let Some(pre_signatures_to_create_in_advance) = pre_signatures_to_create_in_advance else { - return Err( - "KeyConfig.pre_signatures_to_create_in_advance must be specified.".to_string(), - ); + + // Ensure presence of `pre_signatures_to_create_in_advance` for keys that require pre-signatures. + // Note that an invariant ensures that this field is not zero for keys that require pre-signatures. + if key_id.requires_pre_signatures() && pre_signatures_to_create_in_advance.is_none() { + return Err(format!( + "KeyConfig.pre_signatures_to_create_in_advance must be specified for key {key_id}." + )); }; + let Some(max_queue_size) = max_queue_size else { return Err("KeyConfig.max_queue_size must be specified.".to_string()); }; Ok(Self { key_id, - pre_signatures_to_create_in_advance, + pre_signatures_to_create_in_advance: pre_signatures_to_create_in_advance.unwrap_or(0), max_queue_size, }) } diff --git a/rs/registry/canister/unreleased_changelog.md b/rs/registry/canister/unreleased_changelog.md index fa4cd939dc82..81916ac46361 100644 --- a/rs/registry/canister/unreleased_changelog.md +++ b/rs/registry/canister/unreleased_changelog.md @@ -20,6 +20,8 @@ All node operators can now swap nodes on non-system subnets; later, swapping wil ## Changed +* When performing subnet creation, subnet update, or subnet recovery, it is now allowed to omit the `KeyConfig`'s `pre_signatures_to_create_in_advance` field for keys that do not have pre-signatures. Currently only vetKD keys do not have pre-signatures (unlike Ecdsa/Schnorr keys). When the field is omitted, it is automatically set to zero. + ## Deprecated ## Removed diff --git a/rs/registry/subnet_features/src/lib.rs b/rs/registry/subnet_features/src/lib.rs index 3f2babda5457..8cd2126c8172 100644 --- a/rs/registry/subnet_features/src/lib.rs +++ b/rs/registry/subnet_features/src/lib.rs @@ -119,12 +119,17 @@ impl TryFrom for KeyConfig { type Error = ProxyDecodeError; fn try_from(value: pb::KeyConfig) -> Result { - Ok(KeyConfig { - pre_signatures_to_create_in_advance: try_from_option_field( - value.pre_signatures_to_create_in_advance, + let key_id: MasterPublicKeyId = try_from_option_field(value.key_id, "KeyConfig::key_id")?; + if key_id.requires_pre_signatures() && value.pre_signatures_to_create_in_advance.is_none() { + return Err(ProxyDecodeError::MissingField( "KeyConfig::pre_signatures_to_create_in_advance", - )?, - key_id: try_from_option_field(value.key_id, "KeyConfig::key_id")?, + )); + } + Ok(KeyConfig { + pre_signatures_to_create_in_advance: value + .pre_signatures_to_create_in_advance + .unwrap_or(0), + key_id, max_queue_size: try_from_option_field( value.max_queue_size, "KeyConfig::max_queue_size",