From fbeb55b11ae8b7d51a9dd96de5852104cc99e3c7 Mon Sep 17 00:00:00 2001 From: Ruediger Birkner Date: Thu, 7 May 2026 10:12:54 +0000 Subject: [PATCH 1/2] feat: Allow changing subnet_admins via UpdateSubnetPayload Adds an optional `subnet_admins` field to `UpdateSubnetPayload` so NNS proposals can replace a subnet's admin list. Coexists with the Subnet Rental Canister's existing `update_subnet_admins` path, which is unchanged. Promotes the max-10 cap on `subnet_admins` from a local constant in `do_update_subnet_admins` to a registry invariant in `invariants::subnet`, so the cap is enforced uniformly on every mutation. --- .../integration_tests/src/subnet_handler.rs | 1 + rs/registry/admin/bin/update_subnet.rs | 15 +- rs/registry/canister/canister/registry.did | 1 + .../canister/canister/registry_test.did | 1 + rs/registry/canister/src/invariants/mod.rs | 2 +- rs/registry/canister/src/invariants/subnet.rs | 20 ++- .../canister/src/invariants/subnet/tests.rs | 37 +++++ rs/registry/canister/src/lib.rs | 2 +- .../src/mutations/do_update_subnet.rs | 147 +++++++++++++++++- .../src/mutations/do_update_subnet_admins.rs | 3 +- rs/registry/canister/tests/update_subnet.rs | 8 + rs/tests/consensus/cup_explorer_test.rs | 2 + .../tecdsa_signature_life_cycle_test.rs | 2 + .../tecdsa/tecdsa_two_signing_subnets_test.rs | 2 + rs/tests/consensus/tecdsa/utils/src/lib.rs | 4 + rs/tests/consensus/utils/src/ssh_access.rs | 1 + rs/tests/consensus/utils/src/subnet.rs | 2 + rs/tests/testnets/mainnet_nns/src/lib.rs | 1 + 18 files changed, 240 insertions(+), 11 deletions(-) diff --git a/rs/nns/integration_tests/src/subnet_handler.rs b/rs/nns/integration_tests/src/subnet_handler.rs index e841b576b052..e0f04f911a52 100644 --- a/rs/nns/integration_tests/src/subnet_handler.rs +++ b/rs/nns/integration_tests/src/subnet_handler.rs @@ -96,6 +96,7 @@ fn test_submit_and_accept_update_subnet_proposal() { max_number_of_canisters: Some(200), ssh_readonly_access: Some(vec!["pub_key_0".to_string()]), ssh_backup_access: Some(vec!["pub_key_1".to_string()]), + subnet_admins: None, chain_key_config: None, chain_key_signing_enable: None, chain_key_signing_disable: None, diff --git a/rs/registry/admin/bin/update_subnet.rs b/rs/registry/admin/bin/update_subnet.rs index ac082335d82b..4d28c332613b 100644 --- a/rs/registry/admin/bin/update_subnet.rs +++ b/rs/registry/admin/bin/update_subnet.rs @@ -13,7 +13,7 @@ use ic_nns_common::types::NeuronId; use ic_registry_nns_data_provider::registry::RegistryCanister; use ic_registry_resource_limits::ResourceLimits; use ic_registry_subnet_features::SubnetFeatures; -use ic_types::SubnetId; +use ic_types::{PrincipalId, SubnetId}; use registry_canister::mutations::do_update_subnet; use std::collections::BTreeMap; use std::{collections::HashSet, path::PathBuf}; @@ -175,6 +175,12 @@ pub(crate) struct ProposeToUpdateSubnetCmd { #[clap(long, num_args(1..))] ssh_backup_access: Option>, + /// If set, replaces the subnet's admin list with the given principals + /// (passing zero principals clears the list). When unset, the existing + /// admin list is left unchanged. + #[clap(long, num_args(0..))] + pub subnet_admins: Option>, + /// If set, the created proposal will contain a desired override of that /// field to the value set. See `ProposeToCreateSubnetCmd` for the semantic /// of this field. @@ -374,6 +380,7 @@ impl ProposeToUpdateSubnetCmd { ssh_readonly_access: self.ssh_readonly_access.clone(), ssh_backup_access: self.ssh_backup_access.clone(), + subnet_admins: self.subnet_admins.clone(), max_number_of_canisters: self.max_number_of_canisters, chain_key_config, @@ -443,6 +450,7 @@ mod tests { max_number_of_canisters: None, ssh_readonly_access: None, ssh_backup_access: None, + subnet_admins: None, chain_key_config: None, chain_key_signing_enable: None, chain_key_signing_disable: None, @@ -481,6 +489,7 @@ mod tests { resource_limits: None, ssh_readonly_access: None, ssh_backup_access: None, + subnet_admins: None, max_number_of_canisters: None, } } @@ -556,6 +565,7 @@ mod tests { assert_eq!( cmd.new_payload_for_subnet(subnet_id, subnet_record), do_update_subnet::UpdateSubnetPayload { + subnet_admins: None, chain_key_config: Some(do_update_subnet::ChainKeyConfig { // Note the order is not important so long as it is deterministic. As a matter // of fact, the order is lexicographic, as the implementation uses `BTreeSet`. @@ -666,6 +676,7 @@ mod tests { assert_eq!( cmd.new_payload_for_subnet(subnet_id, subnet_record), do_update_subnet::UpdateSubnetPayload { + subnet_admins: None, chain_key_config: Some(do_update_subnet::ChainKeyConfig { key_configs: vec![ do_update_subnet::KeyConfig { @@ -757,6 +768,7 @@ mod tests { assert_eq!( cmd.new_payload_for_subnet(subnet_id, subnet_record), do_update_subnet::UpdateSubnetPayload { + subnet_admins: None, chain_key_config: Some(do_update_subnet::ChainKeyConfig { key_configs: vec![ // Existed before, now being updated. @@ -847,6 +859,7 @@ mod tests { do_update_subnet::UpdateSubnetPayload { resource_limits: expected_resource_limits .map(|resource_limits| resource_limits.into()), + subnet_admins: None, ..make_empty_update_payload(subnet_id) }, ); diff --git a/rs/registry/canister/canister/registry.did b/rs/registry/canister/canister/registry.did index 298c5cdb0da6..caa7b7c85568 100644 --- a/rs/registry/canister/canister/registry.did +++ b/rs/registry/canister/canister/registry.did @@ -486,6 +486,7 @@ type UpdateSubnetPayload = record { max_artifact_streams_per_peer : opt nat32; subnet_type : opt SubnetType; ssh_readonly_access : opt vec text; + subnet_admins : opt vec principal; chain_key_config : opt ChainKeyConfig; chain_key_signing_enable : opt vec MasterPublicKeyId; chain_key_signing_disable : opt vec MasterPublicKeyId; diff --git a/rs/registry/canister/canister/registry_test.did b/rs/registry/canister/canister/registry_test.did index 5a8fe5c58945..9fd05ed08fff 100644 --- a/rs/registry/canister/canister/registry_test.did +++ b/rs/registry/canister/canister/registry_test.did @@ -486,6 +486,7 @@ type UpdateSubnetPayload = record { max_artifact_streams_per_peer : opt nat32; subnet_type : opt SubnetType; ssh_readonly_access : opt vec text; + subnet_admins : opt vec principal; chain_key_config : opt ChainKeyConfig; chain_key_signing_enable : opt vec MasterPublicKeyId; chain_key_signing_disable : opt vec MasterPublicKeyId; diff --git a/rs/registry/canister/src/invariants/mod.rs b/rs/registry/canister/src/invariants/mod.rs index cde989a790fe..fad8ad4863e6 100644 --- a/rs/registry/canister/src/invariants/mod.rs +++ b/rs/registry/canister/src/invariants/mod.rs @@ -10,5 +10,5 @@ mod node_operator; mod node_record; mod replica_version; mod routing_table; -mod subnet; +pub(crate) mod subnet; mod unassigned_nodes_config; diff --git a/rs/registry/canister/src/invariants/subnet.rs b/rs/registry/canister/src/invariants/subnet.rs index fcc21b28a8de..bded573e16f7 100644 --- a/rs/registry/canister/src/invariants/subnet.rs +++ b/rs/registry/canister/src/invariants/subnet.rs @@ -17,6 +17,9 @@ use ic_protobuf::registry::{ use ic_registry_keys::{SUBNET_RECORD_KEY_PREFIX, make_subnet_record_key}; use prost::Message; +/// The maximum number of subnet admins a subnet may have. +pub const MAX_SUBNET_ADMINS: usize = 10; + /// Subnet invariants hold iff: /// * Each SSH key access list does not contain more than 50 keys /// * Subnet membership contains no repetition @@ -30,7 +33,8 @@ use prost::Message; /// * consist of nodes with reward type 4 /// * Conversely, only cloud engines can have nodes with reward type 4 /// * SEV-enabled subnets consist of SEV-enabled nodes only (i.e. nodes with a chip ID in the node record) -/// * Only rented subnets can have subnet admins set to a non-empty list +/// * Only rented or cloud engine subnets can have subnet admins set to a non-empty list +/// * No subnet has more than `MAX_SUBNET_ADMINS` subnet admins pub(crate) fn check_subnet_invariants( snapshot: &RegistrySnapshot, ) -> Result<(), InvariantCheckError> { @@ -176,7 +180,8 @@ pub(crate) fn check_subnet_invariants( Ok(()) } -// Checks that only rented subnets or cloud engine subnets can have admins. +// Checks that only rented subnets or cloud engine subnets can have admins, and +// that no subnet has more than `MAX_SUBNET_ADMINS` admins. fn check_subnet_admins_invariant( subnet_record: &SubnetRecord, subnet_id: SubnetId, @@ -202,6 +207,17 @@ fn check_subnet_admins_invariant( source: None, }); } + + if subnet_record.subnet_admins.len() > MAX_SUBNET_ADMINS { + return Err(InvariantCheckError { + msg: format!( + "Subnet {subnet_id:} has {} subnet admins, which exceeds the maximum of {MAX_SUBNET_ADMINS}", + subnet_record.subnet_admins.len() + ), + source: None, + }); + } + Ok(()) } diff --git a/rs/registry/canister/src/invariants/subnet/tests.rs b/rs/registry/canister/src/invariants/subnet/tests.rs index 50660642b6ee..25acb96c5fdd 100644 --- a/rs/registry/canister/src/invariants/subnet/tests.rs +++ b/rs/registry/canister/src/invariants/subnet/tests.rs @@ -270,6 +270,43 @@ fn non_rented_application_subnets_cannot_have_subnet_admins() { ); } +#[test] +fn subnets_cannot_have_more_than_max_subnet_admins() { + let system_subnet_id = subnet_test_id(1); + let test_subnet_id = subnet_test_id(2); + let (mut snapshot, mut test_subnet_record) = + setup_minimal_registry_snapshot_for_check_subnet_invariants( + system_subnet_id, + test_subnet_id, + 1, // num_nodes_in_test_subnet + false, // with_chip_id + ); + + // Make the subnet rented so the type/cost-schedule check passes. + test_subnet_record.subnet_type = i32::from(SubnetType::Application); + test_subnet_record.canister_cycles_cost_schedule = i32::from(CanisterCyclesCostSchedule::Free); + + // Exactly `MAX_SUBNET_ADMINS` admins is allowed. + test_subnet_record.subnet_admins = (0..MAX_SUBNET_ADMINS as u64) + .map(|i| PrincipalIdPb::from(user_test_id(i).get())) + .collect(); + snapshot.insert( + make_subnet_record_key(test_subnet_id).into_bytes(), + test_subnet_record.encode_to_vec(), + ); + check_subnet_invariants(&snapshot).unwrap(); + + // One more than `MAX_SUBNET_ADMINS` is not. + test_subnet_record.subnet_admins = (0..(MAX_SUBNET_ADMINS as u64 + 1)) + .map(|i| PrincipalIdPb::from(user_test_id(i).get())) + .collect(); + snapshot.insert( + make_subnet_record_key(test_subnet_id).into_bytes(), + test_subnet_record.encode_to_vec(), + ); + assert_non_compliant_record(&snapshot, "subnet admins, which exceeds the maximum of"); +} + #[test] fn cloud_engine_subnets_must_have_type4_nodes() { let system_subnet_id = subnet_test_id(1); diff --git a/rs/registry/canister/src/lib.rs b/rs/registry/canister/src/lib.rs index f8ec9852e34c..9686844d51cb 100644 --- a/rs/registry/canister/src/lib.rs +++ b/rs/registry/canister/src/lib.rs @@ -12,5 +12,5 @@ pub mod registry; pub mod registry_lifecycle; pub mod storage; -mod invariants; +pub(crate) mod invariants; mod rate_limits; diff --git a/rs/registry/canister/src/mutations/do_update_subnet.rs b/rs/registry/canister/src/mutations/do_update_subnet.rs index 610ab57e57d2..afe6504344e1 100644 --- a/rs/registry/canister/src/mutations/do_update_subnet.rs +++ b/rs/registry/canister/src/mutations/do_update_subnet.rs @@ -1,11 +1,14 @@ use crate::{common::LOG_PREFIX, mutations::common::has_duplicates, registry::Registry}; use candid::{CandidType, Deserialize}; use dfn_core::println; -use ic_base_types::{SubnetId, subnet_id_into_protobuf}; +use ic_base_types::{PrincipalId, SubnetId, subnet_id_into_protobuf}; use ic_management_canister_types_private::MasterPublicKeyId; -use ic_protobuf::registry::subnet::v1::{ - ResourceLimits as ResourceLimitsPb, SubnetFeatures as SubnetFeaturesPb, - SubnetRecord as SubnetRecordPb, +use ic_protobuf::{ + registry::subnet::v1::{ + ResourceLimits as ResourceLimitsPb, SubnetFeatures as SubnetFeaturesPb, + SubnetRecord as SubnetRecordPb, + }, + types::v1::PrincipalId as PrincipalIdPb, }; use ic_registry_keys::{make_chain_key_enabled_subnet_list_key, make_subnet_record_key}; use ic_registry_subnet_features::{ @@ -251,6 +254,11 @@ pub struct UpdateSubnetPayload { pub ssh_readonly_access: Option>, pub ssh_backup_access: Option>, + /// When `Some`, replaces the subnet's admin list with the given principals + /// (an empty `Vec` clears the list). `None` leaves the existing list + /// unchanged. + pub subnet_admins: Option>, + // TODO(NNS1-2444): The fields below are deprecated and they are not read anywhere. pub max_artifact_streams_per_peer: Option, pub max_chunk_wait_ms: Option, @@ -458,6 +466,7 @@ fn merge_subnet_record( max_number_of_canisters, ssh_readonly_access, ssh_backup_access, + subnet_admins, // Deprecated/unused values follow max_artifact_streams_per_peer: _, max_chunk_wait_ms: _, @@ -505,6 +514,10 @@ fn merge_subnet_record( maybe_set!(subnet_record, ssh_readonly_access); maybe_set!(subnet_record, ssh_backup_access); + if let Some(subnet_admins) = subnet_admins { + subnet_record.subnet_admins = subnet_admins.into_iter().map(PrincipalIdPb::from).collect(); + } + subnet_record } @@ -552,6 +565,7 @@ mod tests { max_number_of_canisters: None, ssh_readonly_access: None, ssh_backup_access: None, + subnet_admins: None, chain_key_config: None, chain_key_signing_enable: None, chain_key_signing_disable: None, @@ -648,6 +662,7 @@ mod tests { max_number_of_canisters: Some(10), ssh_readonly_access: Some(vec!["pub_key_0".to_string()]), ssh_backup_access: Some(vec!["pub_key_1".to_string()]), + subnet_admins: None, chain_key_config: Some(chain_key_config.clone()), chain_key_signing_enable: None, chain_key_signing_disable: None, @@ -766,6 +781,7 @@ mod tests { max_number_of_canisters: Some(50), ssh_readonly_access: None, ssh_backup_access: None, + subnet_admins: None, chain_key_config: None, chain_key_signing_enable: None, chain_key_signing_disable: None, @@ -1513,4 +1529,127 @@ mod tests { }); payload } + + /// Adds a rented (Application + Free cost schedule) subnet to the registry, + /// so that setting subnet admins is permitted by the registry invariant. + fn make_registry_with_rented_subnet() -> (Registry, SubnetId) { + let mut registry = invariant_compliant_registry(0); + + let (mutate_request, node_ids_and_dkg_pks) = prepare_registry_with_nodes(1, 2); + registry.maybe_apply_mutation_internal(mutate_request.mutations); + + let mut subnet_list_record = registry.get_subnet_list_record(); + + let (first_node_id, first_dkg_pk) = node_ids_and_dkg_pks + .iter() + .next() + .expect("should contain at least one node ID and key"); + let mut subnet_record = get_invariant_compliant_subnet_record(vec![*first_node_id]); + subnet_record.subnet_type = i32::from(SubnetType::Application); + subnet_record.canister_cycles_cost_schedule = i32::from(CanisterCyclesCostSchedule::Free); + + let subnet_id = subnet_test_id(1000); + registry.maybe_apply_mutation_internal(add_fake_subnet( + subnet_id, + &mut subnet_list_record, + subnet_record, + &btreemap!(*first_node_id => first_dkg_pk.clone()), + )); + + (registry, subnet_id) + } + + #[test] + fn can_set_subnet_admins() { + let (mut registry, subnet_id) = make_registry_with_rented_subnet(); + + let user1 = *TEST_USER1_PRINCIPAL; + let user2 = *TEST_USER2_PRINCIPAL; + + let mut payload = make_empty_update_payload(subnet_id); + payload.subnet_admins = Some(vec![user1, user2]); + + registry.do_update_subnet(payload); + + assert_eq!( + registry.get_subnet_or_panic(subnet_id).subnet_admins, + vec![PrincipalIdPb::from(user1), PrincipalIdPb::from(user2)], + ); + } + + #[test] + fn can_clear_subnet_admins() { + let (mut registry, subnet_id) = make_registry_with_rented_subnet(); + + let user1 = *TEST_USER1_PRINCIPAL; + + // First set an admin. + let mut payload = make_empty_update_payload(subnet_id); + payload.subnet_admins = Some(vec![user1]); + registry.do_update_subnet(payload); + assert_eq!( + registry.get_subnet_or_panic(subnet_id).subnet_admins, + vec![PrincipalIdPb::from(user1)], + ); + + // Then clear it via Some(vec![]). + let mut payload = make_empty_update_payload(subnet_id); + payload.subnet_admins = Some(vec![]); + registry.do_update_subnet(payload); + assert_eq!( + registry.get_subnet_or_panic(subnet_id).subnet_admins, + Vec::::new(), + ); + } + + #[test] + fn none_subnet_admins_leaves_existing_admins_unchanged() { + let (mut registry, subnet_id) = make_registry_with_rented_subnet(); + + let user1 = *TEST_USER1_PRINCIPAL; + + let mut payload = make_empty_update_payload(subnet_id); + payload.subnet_admins = Some(vec![user1]); + registry.do_update_subnet(payload); + + // A subsequent update with `subnet_admins: None` must not change the list. + let payload = make_empty_update_payload(subnet_id); + registry.do_update_subnet(payload); + + assert_eq!( + registry.get_subnet_or_panic(subnet_id).subnet_admins, + vec![PrincipalIdPb::from(user1)], + ); + } + + #[test] + #[should_panic(expected = "subnet admins, which exceeds the maximum of")] + fn cannot_set_more_than_max_subnet_admins() { + let (mut registry, subnet_id) = make_registry_with_rented_subnet(); + + // 11 admins exceeds MAX_SUBNET_ADMINS (10). + let admins = (0..11u64) + .map(|i| PrincipalId::new_user_test_id(2000 + i)) + .collect::>(); + + let mut payload = make_empty_update_payload(subnet_id); + payload.subnet_admins = Some(admins); + + registry.do_update_subnet(payload); + } + + #[test] + #[should_panic( + expected = "is not a rented or cloud engine subnet but has a non-empty subnet admins list" + )] + fn cannot_set_subnet_admins_on_non_rented_subnet() { + // Use the default test fixture which produces an Application + Normal subnet + // (not rented), so setting admins must violate the invariant. + let (mut registry, subnet_id) = make_registry_for_update_subnet_tests(); + + let mut payload = make_empty_update_payload(subnet_id); + payload.subnet_admins = Some(vec![*TEST_USER1_PRINCIPAL]); + + registry.do_update_subnet(payload); + } } diff --git a/rs/registry/canister/src/mutations/do_update_subnet_admins.rs b/rs/registry/canister/src/mutations/do_update_subnet_admins.rs index e00d88199f4c..a15d37de8c34 100644 --- a/rs/registry/canister/src/mutations/do_update_subnet_admins.rs +++ b/rs/registry/canister/src/mutations/do_update_subnet_admins.rs @@ -1,4 +1,4 @@ -use crate::{common::LOG_PREFIX, registry::Registry}; +use crate::{common::LOG_PREFIX, invariants::subnet::MAX_SUBNET_ADMINS, registry::Registry}; use candid::{CandidType, Deserialize}; use ic_base_types::{PrincipalId, SubnetId}; use ic_nervous_system_time_helpers::now_system_time; @@ -17,7 +17,6 @@ use std::collections::HashSet; mod rate_limits; use rate_limits::UpdateSubnetAdminsRateLimiter; -const MAX_SUBNET_ADMINS: usize = 10; pub const MAX_SUSTAINED_SUBNET_ADMINS_PER_DAY: u64 = 10; #[derive(Clone, Eq, PartialEq, Debug, CandidType, Deserialize, Serialize)] diff --git a/rs/registry/canister/tests/update_subnet.rs b/rs/registry/canister/tests/update_subnet.rs index 7d36a0251bf9..18f9e592c4eb 100644 --- a/rs/registry/canister/tests/update_subnet.rs +++ b/rs/registry/canister/tests/update_subnet.rs @@ -75,6 +75,7 @@ fn test_the_anonymous_user_cannot_update_a_subnets_configuration() { max_number_of_canisters: Some(10), ssh_readonly_access: Some(vec!["pub_key_0".to_string()]), ssh_backup_access: Some(vec!["pub_key_1".to_string()]), + subnet_admins: None, chain_key_config: None, chain_key_signing_enable: None, chain_key_signing_disable: None, @@ -205,6 +206,7 @@ fn test_a_canister_other_than_the_governance_canister_cannot_update_a_subnets_co max_number_of_canisters: Some(100), ssh_readonly_access: None, ssh_backup_access: None, + subnet_admins: None, chain_key_config: None, chain_key_signing_enable: None, chain_key_signing_disable: None, @@ -327,6 +329,7 @@ fn test_the_governance_canister_can_update_a_subnets_configuration() { max_number_of_canisters: Some(42), ssh_readonly_access: Some(vec!["pub_key_0".to_string()]), ssh_backup_access: Some(vec!["pub_key_1".to_string()]), + subnet_admins: None, chain_key_config: None, chain_key_signing_enable: None, chain_key_signing_disable: None, @@ -519,6 +522,7 @@ fn test_subnets_configuration_chain_key_fields_are_updated_correctly(key_id: Mas // update payload message let mut payload = UpdateSubnetPayload { + subnet_admins: None, chain_key_config: Some(chain_key_config.clone()), chain_key_signing_enable: Some(vec![key_id.clone()]), ..empty_update_subnet_payload(subnet_id) @@ -548,6 +552,7 @@ fn test_subnets_configuration_chain_key_fields_are_updated_correctly(key_id: Mas // Change one field at a time in this payload payload = UpdateSubnetPayload { + subnet_admins: None, chain_key_config: None, chain_key_signing_enable: Some(vec![key_id.clone()]), ..empty_update_subnet_payload(subnet_id) @@ -587,6 +592,7 @@ fn test_subnets_configuration_chain_key_fields_are_updated_correctly(key_id: Mas // First call: { let payload_1 = UpdateSubnetPayload { + subnet_admins: None, chain_key_config: Some(chain_key_config.clone()), ..empty_update_subnet_payload(subnet_id) }; @@ -620,6 +626,7 @@ fn test_subnets_configuration_chain_key_fields_are_updated_correctly(key_id: Mas { // This update should enable signing on our subnet for the given key. let payload_2 = UpdateSubnetPayload { + subnet_admins: None, chain_key_signing_enable: Some(vec![key_id.clone()]), ..empty_update_subnet_payload(subnet_id) }; @@ -684,6 +691,7 @@ fn empty_update_subnet_payload(subnet_id: SubnetId) -> UpdateSubnetPayload { max_number_of_canisters: None, ssh_readonly_access: None, ssh_backup_access: None, + subnet_admins: None, chain_key_config: None, chain_key_signing_enable: None, chain_key_signing_disable: None, diff --git a/rs/tests/consensus/cup_explorer_test.rs b/rs/tests/consensus/cup_explorer_test.rs index aa6b2406cee0..c2b7eb32d61c 100644 --- a/rs/tests/consensus/cup_explorer_test.rs +++ b/rs/tests/consensus/cup_explorer_test.rs @@ -124,6 +124,7 @@ fn test(env: TestEnv) { let halt_at_cup_height_payload = UpdateSubnetPayload { subnet_id: app_subnet.subnet_id, halt_at_cup_height: Some(true), + subnet_admins: None, ..empty_subnet_update() }; block_on(execute_update_subnet_proposal( @@ -171,6 +172,7 @@ fn test(env: TestEnv) { let update_subnet_payload = UpdateSubnetPayload { subnet_id: app_subnet.subnet_id, dkg_interval_length: Some(14), + subnet_admins: None, ..empty_subnet_update() }; block_on(execute_update_subnet_proposal( 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 b650f2cb7b7d..a1a3aa0c621f 100644 --- a/rs/tests/consensus/tecdsa/tecdsa_signature_life_cycle_test.rs +++ b/rs/tests/consensus/tecdsa/tecdsa_signature_life_cycle_test.rs @@ -227,6 +227,7 @@ fn test(env: TestEnv) { let disable_signing_payload = UpdateSubnetPayload { subnet_id: app_subnet.subnet_id, chain_key_signing_disable: Some(all_key_ids.clone()), + subnet_admins: None, ..empty_subnet_update() }; execute_update_subnet_proposal( @@ -284,6 +285,7 @@ fn test(env: TestEnv) { let proposal_payload = UpdateSubnetPayload { subnet_id: new_subnet_id, chain_key_signing_enable: Some(all_key_ids.clone()), + subnet_admins: None, ..empty_subnet_update() }; execute_update_subnet_proposal( diff --git a/rs/tests/consensus/tecdsa/tecdsa_two_signing_subnets_test.rs b/rs/tests/consensus/tecdsa/tecdsa_two_signing_subnets_test.rs index d746bee8343d..39505ce35f28 100644 --- a/rs/tests/consensus/tecdsa/tecdsa_two_signing_subnets_test.rs +++ b/rs/tests/consensus/tecdsa/tecdsa_two_signing_subnets_test.rs @@ -77,6 +77,7 @@ fn enable_signing(governance: &Canister<'_>, subnet_id: SubnetId, logger: &Logge let enable_signing_payload = UpdateSubnetPayload { subnet_id, chain_key_signing_enable: Some(vec![MasterPublicKeyId::Ecdsa(make_key(KEY_ID1))]), + subnet_admins: None, ..empty_subnet_update() }; block_on(execute_update_subnet_proposal( @@ -91,6 +92,7 @@ fn disable_signing(governance: &Canister<'_>, subnet_id: SubnetId, logger: &Logg let disable_signing_payload = UpdateSubnetPayload { subnet_id, chain_key_signing_disable: Some(vec![MasterPublicKeyId::Ecdsa(make_key(KEY_ID1))]), + subnet_admins: None, ..empty_subnet_update() }; block_on(execute_update_subnet_proposal( diff --git a/rs/tests/consensus/tecdsa/utils/src/lib.rs b/rs/tests/consensus/tecdsa/utils/src/lib.rs index 70605a09a763..d1b1be0c7cb7 100644 --- a/rs/tests/consensus/tecdsa/utils/src/lib.rs +++ b/rs/tests/consensus/tecdsa/utils/src/lib.rs @@ -223,6 +223,7 @@ pub fn empty_subnet_update() -> UpdateSubnetPayload { max_number_of_canisters: None, ssh_readonly_access: None, ssh_backup_access: None, + subnet_admins: None, // Deprecated/unused values follow max_artifact_streams_per_peer: None, max_chunk_wait_ms: None, @@ -992,6 +993,7 @@ pub async fn add_chain_keys_with_timeout_and_rotation_period( ) { let proposal_payload = UpdateSubnetPayload { subnet_id, + subnet_admins: None, chain_key_config: Some(ChainKeyConfig { key_configs: key_ids .into_iter() @@ -1035,6 +1037,7 @@ pub async fn enable_chain_key_signing_with_timeout_and_rotation_period( let proposal_payload = UpdateSubnetPayload { subnet_id, + subnet_admins: None, chain_key_signing_enable: Some(key_ids), ..empty_subnet_update() }; @@ -1195,6 +1198,7 @@ pub async fn set_pre_signature_stash_size( ) { let proposal_payload = UpdateSubnetPayload { subnet_id, + subnet_admins: None, chain_key_config: Some(ChainKeyConfig { key_configs: key_ids .iter() diff --git a/rs/tests/consensus/utils/src/ssh_access.rs b/rs/tests/consensus/utils/src/ssh_access.rs index 49db7cc029f0..54324e625098 100644 --- a/rs/tests/consensus/utils/src/ssh_access.rs +++ b/rs/tests/consensus/utils/src/ssh_access.rs @@ -209,6 +209,7 @@ pub fn get_update_subnet_payload_with_keys( max_number_of_canisters: None, ssh_readonly_access: readonly_keys, ssh_backup_access: backup_keys, + subnet_admins: None, // Deprecated/unused values follow max_artifact_streams_per_peer: None, max_chunk_wait_ms: None, diff --git a/rs/tests/consensus/utils/src/subnet.rs b/rs/tests/consensus/utils/src/subnet.rs index 809b7cae3ab2..59a6a6f7b1ff 100644 --- a/rs/tests/consensus/utils/src/subnet.rs +++ b/rs/tests/consensus/utils/src/subnet.rs @@ -119,6 +119,7 @@ pub fn enable_chain_key_signing_on_subnet( let enable_signing_payload = UpdateSubnetPayload { subnet_id, chain_key_signing_enable: Some(key_ids.clone()), + subnet_admins: None, ..empty_subnet_update() }; block_on(execute_update_subnet_proposal( @@ -152,6 +153,7 @@ pub fn disable_chain_key_on_subnet( let disable_signing_payload = UpdateSubnetPayload { subnet_id, chain_key_signing_disable: Some(key_ids.clone()), + subnet_admins: None, ..empty_subnet_update() }; block_on(execute_update_subnet_proposal( diff --git a/rs/tests/testnets/mainnet_nns/src/lib.rs b/rs/tests/testnets/mainnet_nns/src/lib.rs index 721768c89e29..202ecb77e9c1 100644 --- a/rs/tests/testnets/mainnet_nns/src/lib.rs +++ b/rs/tests/testnets/mainnet_nns/src/lib.rs @@ -282,6 +282,7 @@ async fn setup_recovered_nns( max_number_of_canisters: None, ssh_readonly_access: None, ssh_backup_access: None, + subnet_admins: None, max_artifact_streams_per_peer: None, max_chunk_wait_ms: None, max_duplicity: None, From 7093cdef909cfdf1c3fd8466b2b0044bdad82833 Mon Sep 17 00:00:00 2001 From: Ruediger Birkner Date: Thu, 7 May 2026 10:58:41 +0000 Subject: [PATCH 2/2] fix: 11u64 -> 11_u64 --- rs/registry/canister/src/mutations/do_update_subnet.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/registry/canister/src/mutations/do_update_subnet.rs b/rs/registry/canister/src/mutations/do_update_subnet.rs index afe6504344e1..bddc444f98aa 100644 --- a/rs/registry/canister/src/mutations/do_update_subnet.rs +++ b/rs/registry/canister/src/mutations/do_update_subnet.rs @@ -1628,7 +1628,7 @@ mod tests { let (mut registry, subnet_id) = make_registry_with_rented_subnet(); // 11 admins exceeds MAX_SUBNET_ADMINS (10). - let admins = (0..11u64) + let admins = (0..11_u64) .map(|i| PrincipalId::new_user_test_id(2000 + i)) .collect::>();