diff --git a/rs/nns/governance/api/src/types.rs b/rs/nns/governance/api/src/types.rs index 404695879fac..6326ba6bd436 100644 --- a/rs/nns/governance/api/src/types.rs +++ b/rs/nns/governance/api/src/types.rs @@ -4335,6 +4335,9 @@ pub enum NnsFunction { /// `SetupInitialDKG` requests without an explicit subnet id are routed to the /// calling subnet (NNS). SetDefaultInitialDkgSubnet = 58, + /// Merge a subnet into another subnet: in the routing table, reassigns all + /// canister ranges hosted by the source subnet to the destination subnet. + MergeSubnets = 59, } impl NnsFunction { /// String value of the enum field names used in the ProtoBuf definition. @@ -4423,6 +4426,7 @@ impl NnsFunction { NnsFunction::SetDefaultInitialDkgSubnet => { "NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET" } + NnsFunction::MergeSubnets => "NNS_FUNCTION_MERGE_SUBNETS", } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -4506,6 +4510,7 @@ impl NnsFunction { "NNS_FUNCTION_SPLIT_SUBNET" => Some(Self::SplitSubnet), "NNS_FUNCTION_DELETE_SUBNET" => Some(Self::DeleteSubnet), "NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET" => Some(Self::SetDefaultInitialDkgSubnet), + "NNS_FUNCTION_MERGE_SUBNETS" => Some(Self::MergeSubnets), _ => None, } } diff --git a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto index 7a792cafbcd6..fea066dfed65 100644 --- a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto +++ b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto @@ -513,6 +513,10 @@ enum NnsFunction { // `SetupInitialDKG` requests without an explicit subnet id are routed to the // calling subnet (NNS). NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET = 58; + + // Merge a subnet into another subnet: in the routing table, reassigns all + // canister ranges hosted by the source subnet to the destination subnet. + NNS_FUNCTION_MERGE_SUBNETS = 59; } // Payload of a proposal that calls a function on another NNS diff --git a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs index d7e10ba02e15..c798f330d7d7 100644 --- a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs +++ b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs @@ -5550,6 +5550,9 @@ pub enum NnsFunction { /// `SetupInitialDKG` requests without an explicit subnet id are routed to the /// calling subnet (NNS). SetDefaultInitialDkgSubnet = 58, + /// Merge a subnet into another subnet: in the routing table, reassigns all + /// canister ranges hosted by the source subnet to the destination subnet. + MergeSubnets = 59, } impl NnsFunction { /// String value of the enum field names used in the ProtoBuf definition. @@ -5626,6 +5629,7 @@ impl NnsFunction { Self::SplitSubnet => "NNS_FUNCTION_SPLIT_SUBNET", Self::DeleteSubnet => "NNS_FUNCTION_DELETE_SUBNET", Self::SetDefaultInitialDkgSubnet => "NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET", + Self::MergeSubnets => "NNS_FUNCTION_MERGE_SUBNETS", } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -5709,6 +5713,7 @@ impl NnsFunction { "NNS_FUNCTION_SPLIT_SUBNET" => Some(Self::SplitSubnet), "NNS_FUNCTION_DELETE_SUBNET" => Some(Self::DeleteSubnet), "NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET" => Some(Self::SetDefaultInitialDkgSubnet), + "NNS_FUNCTION_MERGE_SUBNETS" => Some(Self::MergeSubnets), _ => None, } } diff --git a/rs/nns/governance/src/pb/conversions/mod.rs b/rs/nns/governance/src/pb/conversions/mod.rs index 6c2a48848eb6..32fddbf50877 100644 --- a/rs/nns/governance/src/pb/conversions/mod.rs +++ b/rs/nns/governance/src/pb/conversions/mod.rs @@ -3942,6 +3942,7 @@ impl From for api::NnsFunction { api::NnsFunction::SetSubnetOperationalLevel } pb::NnsFunction::SplitSubnet => api::NnsFunction::SplitSubnet, + pb::NnsFunction::MergeSubnets => api::NnsFunction::MergeSubnets, pb::NnsFunction::DeleteSubnet => api::NnsFunction::DeleteSubnet, pb::NnsFunction::SetDefaultInitialDkgSubnet => { api::NnsFunction::SetDefaultInitialDkgSubnet @@ -4042,6 +4043,7 @@ impl From for pb::NnsFunction { pb::NnsFunction::SetSubnetOperationalLevel } api::NnsFunction::SplitSubnet => pb::NnsFunction::SplitSubnet, + api::NnsFunction::MergeSubnets => pb::NnsFunction::MergeSubnets, api::NnsFunction::DeleteSubnet => pb::NnsFunction::DeleteSubnet, api::NnsFunction::SetDefaultInitialDkgSubnet => { pb::NnsFunction::SetDefaultInitialDkgSubnet diff --git a/rs/nns/governance/src/proposals/execute_nns_function.rs b/rs/nns/governance/src/proposals/execute_nns_function.rs index 7aba6eaa7d39..7b7b8a13a64d 100644 --- a/rs/nns/governance/src/proposals/execute_nns_function.rs +++ b/rs/nns/governance/src/proposals/execute_nns_function.rs @@ -459,6 +459,7 @@ pub enum ValidNnsFunction { SplitSubnet, DeleteSubnet, SetDefaultInitialDkgSubnet, + MergeSubnets, } impl ValidNnsFunction { @@ -592,6 +593,7 @@ impl ValidNnsFunction { ValidNnsFunction::SetDefaultInitialDkgSubnet => { (REGISTRY_CANISTER_ID, "set_default_initial_dkg_subnet") } + ValidNnsFunction::MergeSubnets => (REGISTRY_CANISTER_ID, "merge_subnets"), } } @@ -623,7 +625,8 @@ impl ValidNnsFunction { | ValidNnsFunction::SetSubnetOperationalLevel | ValidNnsFunction::SplitSubnet | ValidNnsFunction::DeleteSubnet - | ValidNnsFunction::SetDefaultInitialDkgSubnet => Topic::SubnetManagement, + | ValidNnsFunction::SetDefaultInitialDkgSubnet + | ValidNnsFunction::MergeSubnets => Topic::SubnetManagement, ValidNnsFunction::ReviseElectedGuestosVersions | ValidNnsFunction::ReviseElectedHostosVersions => Topic::IcOsVersionElection, @@ -714,6 +717,7 @@ impl ValidNnsFunction { ValidNnsFunction::SplitSubnet => "Split subnet", ValidNnsFunction::DeleteSubnet => "Delete Subnet", ValidNnsFunction::SetDefaultInitialDkgSubnet => "Set Default Initial DKG Subnet", + ValidNnsFunction::MergeSubnets => "Merge subnets", } } @@ -944,6 +948,11 @@ impl ValidNnsFunction { calls are routed when no subnet is specified explicitly in the request. If unset, \ such requests are routed to the calling subnet (NNS)." } + ValidNnsFunction::MergeSubnets => { + "Merge a subnet into another subnet: in the routing table, reassigns all \ + canister ranges hosted by the source subnet to the destination subnet. The source \ + subnet is not deleted." + } } } } @@ -1033,6 +1042,7 @@ impl TryFrom for ValidNnsFunction { Ok(ValidNnsFunction::SetSubnetOperationalLevel) } NnsFunction::SplitSubnet => Ok(ValidNnsFunction::SplitSubnet), + NnsFunction::MergeSubnets => Ok(ValidNnsFunction::MergeSubnets), NnsFunction::DeleteSubnet => Ok(ValidNnsFunction::DeleteSubnet), NnsFunction::SetDefaultInitialDkgSubnet => { Ok(ValidNnsFunction::SetDefaultInitialDkgSubnet) diff --git a/rs/nns/governance/unreleased_changelog.md b/rs/nns/governance/unreleased_changelog.md index 94126a0ff421..e0e0916c569c 100644 --- a/rs/nns/governance/unreleased_changelog.md +++ b/rs/nns/governance/unreleased_changelog.md @@ -9,6 +9,11 @@ on the process that this file is part of, see ## Added +* Added a new `NnsFunction` variant `MergeSubnets`, which proposes to merge a + subnet into another subnet: in the routing table, reassigns all canister + ranges hosted by the source subnet to the destination subnet. The source + subnet is not deleted. + ## Changed ## Deprecated diff --git a/rs/registry/canister/canister/canister.rs b/rs/registry/canister/canister/canister.rs index 7112869b1b17..01568e96f077 100644 --- a/rs/registry/canister/canister/canister.rs +++ b/rs/registry/canister/canister/canister.rs @@ -79,6 +79,7 @@ use registry_canister::{ firewall::{ AddFirewallRulesPayload, RemoveFirewallRulesPayload, UpdateFirewallRulesPayload, }, + merge_subnets::MergeSubnetsPayload, node_management::{ do_remove_node_directly::RemoveNodeDirectlyPayload, do_remove_nodes::RemoveNodesPayload, @@ -1085,6 +1086,24 @@ fn reroute_canister_ranges_(payload: RerouteCanisterRangesPayload) { recertify_registry(); } +#[unsafe(export_name = "canister_update merge_subnets")] +fn merge_subnets() { + check_caller_is_governance_and_log("merge_subnets"); + over(candid_one, merge_subnets_); +} + +#[candid_method(update, rename = "merge_subnets")] +fn merge_subnets_(payload: MergeSubnetsPayload) { + registry_mut() + .merge_subnets(payload) + .unwrap_or_else(|error_message| { + trap_with(&format!( + "{LOG_PREFIX} Merge subnets failed: {error_message}" + )) + }); + recertify_registry(); +} + #[unsafe(export_name = "canister_update split_subnet")] fn split_subnet() { check_caller_is_governance_and_log("split_subnet"); diff --git a/rs/registry/canister/canister/registry.did b/rs/registry/canister/canister/registry.did index f0baf2ac5600..a3759d9ffb20 100644 --- a/rs/registry/canister/canister/registry.did +++ b/rs/registry/canister/canister/registry.did @@ -308,6 +308,11 @@ type IPv4Config = record { ip_addr : text; }; +type MergeSubnetsPayload = record { + source_subnet : principal; + destination_subnet : principal; +}; + type MigrateCanistersPayload = record { canister_ids : vec principal; target_subnet_id : principal; @@ -646,6 +651,7 @@ service : { get_node_providers_monthly_xdr_rewards : (opt GetNodeProvidersMonthlyXdrRewardsRequest) -> (GetNodeProvidersMonthlyXdrRewardsResponse) query; get_subnet : (GetSubnetRequest) -> (GetSubnetResponse) query; get_subnet_for_canister : (GetSubnetForCanisterRequest) -> (GetSubnetForCanisterResponse) query; + merge_subnets : (MergeSubnetsPayload) -> (); migrate_canisters: (MigrateCanistersPayload) -> (MigrateCanistersResponse); migrate_node_operator_directly : (MigrateNodeOperatorPayload) -> (); prepare_canister_migration : (PrepareCanisterMigrationPayload) -> (); diff --git a/rs/registry/canister/canister/registry_test.did b/rs/registry/canister/canister/registry_test.did index d635c42ecf3b..7493ae134411 100644 --- a/rs/registry/canister/canister/registry_test.did +++ b/rs/registry/canister/canister/registry_test.did @@ -308,6 +308,11 @@ type IPv4Config = record { ip_addr : text; }; +type MergeSubnetsPayload = record { + source_subnet : principal; + destination_subnet : principal; +}; + type MigrateCanistersPayload = record { canister_ids : vec principal; target_subnet_id : principal; @@ -646,6 +651,7 @@ service : { get_node_providers_monthly_xdr_rewards : (opt GetNodeProvidersMonthlyXdrRewardsRequest) -> (GetNodeProvidersMonthlyXdrRewardsResponse) query; get_subnet : (GetSubnetRequest) -> (GetSubnetResponse) query; get_subnet_for_canister : (GetSubnetForCanisterRequest) -> (GetSubnetForCanisterResponse) query; + merge_subnets : (MergeSubnetsPayload) -> (); migrate_canisters: (MigrateCanistersPayload) -> (MigrateCanistersResponse); migrate_node_operator_directly : (MigrateNodeOperatorPayload) -> (); prepare_canister_migration : (PrepareCanisterMigrationPayload) -> (); diff --git a/rs/registry/canister/src/mutations/merge_subnets.rs b/rs/registry/canister/src/mutations/merge_subnets.rs new file mode 100644 index 000000000000..d32bfdb33c58 --- /dev/null +++ b/rs/registry/canister/src/mutations/merge_subnets.rs @@ -0,0 +1,357 @@ +use crate::{common::LOG_PREFIX, registry::Registry}; +use candid::CandidType; +#[cfg(target_arch = "wasm32")] +use dfn_core::println; +use ic_base_types::SubnetId; +use ic_registry_keys::make_subnet_record_key; +use ic_registry_routing_table::are_disjoint; +use serde::{Deserialize, Serialize}; + +impl Registry { + /// Merges the canister ID ranges of the source subnet into the canister ID + /// range set of the destination subnet. + /// + /// This is one step of the subnet merging process: by the time this mutation + /// is applied, both subnets are halted and all streams to and from the source + /// subnet are empty. The destination subnet is only unhalted after this + /// registry change has been applied. + /// + /// After this operation, all canisters that used to be hosted by the source + /// subnet are routed to the destination subnet and the source subnet does + /// not host any canister ID range anymore. + /// + /// Note that only the routing table is updated: neither subnet record is + /// modified and, in particular, the source subnet is not deleted. + pub fn merge_subnets(&mut self, payload: MergeSubnetsPayload) -> Result<(), String> { + println!("{LOG_PREFIX}merge_subnets: {payload:?}"); + + let MergeSubnetsPayload { + source_subnet, + destination_subnet, + } = payload; + + // The subnets must be distinct. + if source_subnet == destination_subnet { + return Err(format!( + "source subnet {source_subnet} and destination subnet {destination_subnet} must be different subnets" + )); + } + + let version = self.latest_version(); + + // The subnets must exist. + self.get(&make_subnet_record_key(source_subnet).into_bytes(), version) + .ok_or_else(|| format!("source {source_subnet} is not a known subnet"))?; + self.get( + &make_subnet_record_key(destination_subnet).into_bytes(), + version, + ) + .ok_or_else(|| format!("destination {destination_subnet} is not a known subnet"))?; + + // The source subnet must be nonempty. + let routing_table = self.get_routing_table_or_panic(version); + let source_ranges = routing_table.ranges(source_subnet); + if source_ranges.is_empty() { + return Err(format!( + "source subnet {source_subnet} does not host any canister ID range" + )); + } + + // Separate checks before the merge should ensure that this never triggers: rerouting the + // canister ID ranges of the source subnet would break any ongoing canister migration out + // of those ranges, since the migrated ranges would end up being hosted by the destination + // subnet, which is not on the recorded migration trace. + if let Some(canister_migrations) = self.get_canister_migrations(version) + && !are_disjoint(canister_migrations.ranges(), source_ranges.iter()) + { + return Err(format!( + "source subnet {source_subnet} hosts canister ID ranges with ongoing canister migrations" + )); + } + + self.maybe_apply_mutation_internal(self.merge_subnets_mutation( + version, + source_subnet, + destination_subnet, + )); + + Ok(()) + } +} + +/// The argument for the `merge_subnets` update call. +#[derive(Clone, Eq, PartialEq, Debug, CandidType, Deserialize, Serialize)] +pub struct MergeSubnetsPayload { + /// The subnet whose canister ID ranges are merged into the canister ID range + /// set of `destination_subnet`. + pub source_subnet: SubnetId, + /// The subnet that hosts the canister ID ranges of `source_subnet` after the + /// merge. + pub destination_subnet: SubnetId, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + common::test_helpers::{ + add_fake_subnet, get_invariant_compliant_subnet_record, invariant_compliant_registry, + prepare_registry_with_nodes, + }, + mutations::{ + prepare_canister_migration::PrepareCanisterMigrationPayload, + routing_table::routing_table_into_registry_mutation, + }, + }; + use ic_registry_routing_table::{CanisterIdRange, RoutingTable}; + use ic_types::CanisterId; + use ic_types_test_utils::ids::{SUBNET_1, SUBNET_2, SUBNET_3}; + use lazy_static::lazy_static; + use maplit::btreemap; + + const fn range(start: u64, end: u64) -> CanisterIdRange { + CanisterIdRange { + start: CanisterId::from_u64(start), + end: CanisterId::from_u64(end), + } + } + + /// The routing table of `TWO_SUBNETS_REGISTRY`. + const FIXTURE_ROUTING_TABLE_ENTRIES: [(CanisterIdRange, SubnetId); 3] = [ + (range(10, 19), SUBNET_1), + (range(20, 29), SUBNET_2), + (range(30, 39), SUBNET_1), + ]; + + lazy_static! { + /// A registry with two subnets, `SUBNET_1` and `SUBNET_2`, where `SUBNET_1` + /// hosts the canister ID ranges `[10, 19]` and `[30, 39]` and `SUBNET_2` + /// hosts the canister ID range `[20, 29]`. + static ref TWO_SUBNETS_REGISTRY: Registry = { + let mut registry = invariant_compliant_registry(0); + + let (mutate_request, node_ids_and_dkg_pks) = prepare_registry_with_nodes( + 1, // start_mutation_id + 2, // nodes + ); + registry.maybe_apply_mutation_internal(mutate_request.mutations); + + let mut subnet_list_record = registry.get_subnet_list_record(); + let subnet_ids_and_nodes = [SUBNET_1, SUBNET_2].into_iter().zip(node_ids_and_dkg_pks); + + for (subnet_id, (node_id, dkg_pk)) in subnet_ids_and_nodes { + let subnet_record = get_invariant_compliant_subnet_record(vec![node_id]); + let subnet_mutations = add_fake_subnet( + subnet_id, + &mut subnet_list_record, + subnet_record, + &btreemap! { node_id => dkg_pk }, + ); + registry.maybe_apply_mutation_internal(subnet_mutations); + } + + let mut routing_table = RoutingTable::new(); + for (canister_id_range, subnet_id) in FIXTURE_ROUTING_TABLE_ENTRIES { + routing_table.insert(canister_id_range, subnet_id).unwrap(); + } + registry.maybe_apply_mutation_internal(routing_table_into_registry_mutation( + ®istry, + routing_table, + )); + + registry + }; + } + + fn get_routing_table_entries(registry: &Registry) -> Vec<(CanisterIdRange, SubnetId)> { + registry + .get_routing_table_or_panic(registry.latest_version()) + .into_iter() + .collect::>() + } + + #[test] + fn test_merge_subnets() { + // Step 1: Prepare the world. + let mut registry = TWO_SUBNETS_REGISTRY.clone(); + + // Step 2: Run the code under test. + let result = registry.merge_subnets(MergeSubnetsPayload { + source_subnet: SUBNET_1, + destination_subnet: SUBNET_2, + }); + + // Step 3: Verify results. + + // Step 3.1: Inspect the return value. + assert_eq!(result, Ok(())); + + // Step 3.2: The canister ID ranges of both subnets are now hosted by the + // destination subnet, and the three adjacent ranges got merged into one. + assert_eq!( + get_routing_table_entries(®istry), + vec![(range(10, 39), SUBNET_2)], + ); + } + + #[test] + fn test_merge_subnets_into_subnet_without_canister_id_ranges() { + // Step 1: Prepare the world: let the destination subnet host no canister ID + // range at all, so that the merge has to add the destination subnet to the + // routing table. The two ranges of the source subnet are not adjacent, so + // they must stay two separate entries. A subnet hosting no canister ID range + // is not a realistic scenario; this only exercises that code path. + let mut registry = TWO_SUBNETS_REGISTRY.clone(); + let mut routing_table = RoutingTable::new(); + routing_table.insert(range(10, 19), SUBNET_1).unwrap(); + routing_table.insert(range(30, 39), SUBNET_1).unwrap(); + registry.maybe_apply_mutation_internal(routing_table_into_registry_mutation( + ®istry, + routing_table, + )); + + // Step 2: Run the code under test. + let result = registry.merge_subnets(MergeSubnetsPayload { + source_subnet: SUBNET_1, + destination_subnet: SUBNET_2, + }); + + // Step 3: Verify results. Both ranges are hosted by the destination subnet + // and, not being adjacent, did not get merged into a single entry. + assert_eq!(result, Ok(())); + assert_eq!( + get_routing_table_entries(®istry), + vec![(range(10, 19), SUBNET_2), (range(30, 39), SUBNET_2)], + ); + } + + #[test] + fn test_merge_subnets_fails_when_subnets_are_equal() { + // Step 1: Prepare the world. + let mut registry = TWO_SUBNETS_REGISTRY.clone(); + + // Step 2: Run the code under test. + let result = registry.merge_subnets(MergeSubnetsPayload { + source_subnet: SUBNET_1, + destination_subnet: SUBNET_1, + }); + + // Step 3: Verify results. + let error_message = result.unwrap_err(); + assert!( + error_message.contains("must be different subnets"), + "{error_message}" + ); + assert_eq!( + get_routing_table_entries(®istry), + FIXTURE_ROUTING_TABLE_ENTRIES.to_vec(), + ); + } + + #[test] + fn test_merge_subnets_fails_when_source_subnet_is_unknown() { + // Step 1: Prepare the world. + let mut registry = TWO_SUBNETS_REGISTRY.clone(); + + // Step 2: Run the code under test. + let result = registry.merge_subnets(MergeSubnetsPayload { + source_subnet: SUBNET_3, + destination_subnet: SUBNET_2, + }); + + // Step 3: Verify results. + let error_message = result.unwrap_err(); + assert!( + error_message.contains(&format!("source {SUBNET_3} is not a known subnet")), + "{error_message}" + ); + assert_eq!( + get_routing_table_entries(®istry), + FIXTURE_ROUTING_TABLE_ENTRIES.to_vec(), + ); + } + + #[test] + fn test_merge_subnets_fails_when_destination_subnet_is_unknown() { + // Step 1: Prepare the world. + let mut registry = TWO_SUBNETS_REGISTRY.clone(); + + // Step 2: Run the code under test. + let result = registry.merge_subnets(MergeSubnetsPayload { + source_subnet: SUBNET_1, + destination_subnet: SUBNET_3, + }); + + // Step 3: Verify results. + let error_message = result.unwrap_err(); + assert!( + error_message.contains(&format!("destination {SUBNET_3} is not a known subnet")), + "{error_message}" + ); + assert_eq!( + get_routing_table_entries(®istry), + FIXTURE_ROUTING_TABLE_ENTRIES.to_vec(), + ); + } + + #[test] + fn test_merge_subnets_fails_when_source_subnet_hosts_no_canister_id_range() { + // Step 1: Prepare the world: only the destination subnet hosts a canister + // ID range. + let mut registry = TWO_SUBNETS_REGISTRY.clone(); + let mut routing_table = RoutingTable::new(); + routing_table.insert(range(20, 29), SUBNET_2).unwrap(); + registry.maybe_apply_mutation_internal(routing_table_into_registry_mutation( + ®istry, + routing_table, + )); + + // Step 2: Run the code under test. + let result = registry.merge_subnets(MergeSubnetsPayload { + source_subnet: SUBNET_1, + destination_subnet: SUBNET_2, + }); + + // Step 3: Verify results. + let error_message = result.unwrap_err(); + assert!( + error_message.contains("does not host any canister ID range"), + "{error_message}" + ); + assert_eq!( + get_routing_table_entries(®istry), + vec![(range(20, 29), SUBNET_2)], + ); + } + + #[test] + fn test_merge_subnets_fails_with_ongoing_canister_migration() { + // Step 1: Prepare the world: start migrating a canister ID range away from + // the source subnet. + let mut registry = TWO_SUBNETS_REGISTRY.clone(); + registry + .prepare_canister_migration(PrepareCanisterMigrationPayload { + canister_id_ranges: vec![range(10, 11)], + source_subnet: SUBNET_1, + destination_subnet: SUBNET_2, + }) + .unwrap(); + + // Step 2: Run the code under test. + let result = registry.merge_subnets(MergeSubnetsPayload { + source_subnet: SUBNET_1, + destination_subnet: SUBNET_2, + }); + + // Step 3: Verify results. + let error_message = result.unwrap_err(); + assert!( + error_message.contains("ongoing canister migrations"), + "{error_message}" + ); + assert_eq!( + get_routing_table_entries(®istry), + FIXTURE_ROUTING_TABLE_ENTRIES.to_vec(), + ); + } +} diff --git a/rs/registry/canister/src/mutations/mod.rs b/rs/registry/canister/src/mutations/mod.rs index 846fdbac93cf..73de4d92300a 100644 --- a/rs/registry/canister/src/mutations/mod.rs +++ b/rs/registry/canister/src/mutations/mod.rs @@ -37,6 +37,7 @@ pub mod do_update_subnet; pub mod do_update_subnet_admins; pub mod do_update_unassigned_nodes_config; pub mod firewall; +pub mod merge_subnets; mod node; pub mod node_management; pub mod prepare_canister_migration; diff --git a/rs/registry/canister/src/mutations/routing_table.rs b/rs/registry/canister/src/mutations/routing_table.rs index cbb4734ce976..d026e4b6f6dc 100644 --- a/rs/registry/canister/src/mutations/routing_table.rs +++ b/rs/registry/canister/src/mutations/routing_table.rs @@ -378,6 +378,24 @@ impl Registry { }) } + /// Makes a registry mutation that merges all canister ID ranges currently + /// assigned to the `source` subnet into the canister ID range set of the + /// `destination` subnet. After the mutation, `source` does not host any + /// canister ID range anymore. + pub fn merge_subnets_mutation( + &self, + version: u64, + source: SubnetId, + destination: SubnetId, + ) -> Vec { + self.modify_routing_table(version, |routing_table| { + let source_ranges = routing_table.ranges(source); + routing_table + .assign_ranges(source_ranges, destination) + .unwrap(); + }) + } + /// Retrieves the canister migrations if the key exists. pub fn get_canister_migrations(&self, version: u64) -> Option { self.get(make_canister_migrations_record_key().as_bytes(), version) diff --git a/rs/registry/canister/tests/merge_subnets.rs b/rs/registry/canister/tests/merge_subnets.rs new file mode 100644 index 000000000000..5d4ffc016c83 --- /dev/null +++ b/rs/registry/canister/tests/merge_subnets.rs @@ -0,0 +1,137 @@ +use candid::Encode; +use ic_nns_test_utils::{ + itest_helpers::{ + set_up_registry_canister, set_up_universal_canister, state_machine_test_on_nns_subnet, + try_call_via_universal_canister, + }, + registry::{initial_routing_table_mutations, prepare_registry_with_two_node_sets}, +}; +use ic_registry_routing_table::{CanisterIdRange, RoutingTable}; +use ic_registry_transport::pb::v1::RegistryAtomicMutateRequest; +use ic_types::{CanisterId, SubnetId}; +use registry_canister::{ + init::RegistryCanisterInitPayloadBuilder, + mutations::merge_subnets::MergeSubnetsPayload, + pb::v1::{GetSubnetForCanisterRequest, SubnetForCanister}, +}; + +mod common; +use common::test_helpers::{check_error_message, check_subnet_for_canisters}; + +/// Exercises the `merge_subnets` endpoint end to end. The payload validation +/// itself is covered by the unit tests of `Registry::merge_subnets`, so this test +/// only covers what those cannot: that the endpoint is reachable with a Candid +/// encoded payload, that only governance may call it, and that the resulting +/// routing table is visible through the canister's query API. +#[test] +fn test_merge_subnets() { + state_machine_test_on_nns_subnet(|runtime| { + async move { + // Step 1: Prepare the world: two subnets where subnet 2 hosts the canister + // ID range [0, 255] and subnet 1 hosts [256, 511]. + let (subnet_1_mutation, subnet_id_1, subnet_id_2_option, _, _) = + prepare_registry_with_two_node_sets( + 4, // num_nodes_in_subnet1 + 4, // num_nodes_in_subnet2 + true, // assign_nodes_to_subnet2 + ); + let subnet_id_2 = subnet_id_2_option.unwrap(); + let routing_table_mutation = { + fn range(start: u64, end: u64) -> CanisterIdRange { + CanisterIdRange { + start: CanisterId::from(start), + end: CanisterId::from(end), + } + } + + let mut routing_table = RoutingTable::new(); + routing_table + .insert(range(0, 255), subnet_id_2) + .expect("failed to update the routing table"); + routing_table + .insert(range(256, 511), subnet_id_1) + .expect("failed to update the routing table"); + + RegistryAtomicMutateRequest { + mutations: initial_routing_table_mutations(&routing_table), + preconditions: vec![], + } + }; + + let registry = set_up_registry_canister( + &runtime, + RegistryCanisterInitPayloadBuilder::new() + .push_init_mutate_request(subnet_1_mutation) + .push_init_mutate_request(routing_table_mutation) + .build(), + ) + .await; + + let governance_fake = set_up_universal_canister(&runtime).await; + assert_eq!( + governance_fake.canister_id(), + ic_nns_constants::GOVERNANCE_CANISTER_ID + ); + + // Step 2: Run the code under test. + + // Step 2.1: The sad case: caller is not the Governance canister. + check_error_message( + registry + .update_( + "merge_subnets", + dfn_candid::candid_one, + MergeSubnetsPayload { + source_subnet: subnet_id_1, + destination_subnet: subnet_id_2, + }, + ) + .await as Result<(), String>, + "not authorized", + ); + + // Step 2.2: The happy case: merging subnet 1 into subnet 2 succeeds, + // because Governance is the caller. + try_call_via_universal_canister( + &governance_fake, + ®istry, + "merge_subnets", + Encode!(&MergeSubnetsPayload { + source_subnet: subnet_id_1, + destination_subnet: subnet_id_2, + }) + .unwrap(), + ) + .await + .unwrap(); + + // Step 3: Verify results. + + // Step 3.1: The canisters formerly hosted by subnet 1 are now hosted by + // subnet 2, and the canisters of subnet 2 stay put. + check_subnet_for_canisters( + ®istry, + (0..=511_u64) + .map(|canister_id| (CanisterId::from(canister_id), subnet_id_2)) + .collect::>(), + ) + .await; + + // Step 3.2: Canister IDs outside the merged ranges are still not routed + // to any subnet. + let subnet_for_canister: Result = registry + .query_( + "get_subnet_for_canister", + dfn_candid::candid_one, + GetSubnetForCanisterRequest { + principal: Some(CanisterId::from(512).get()), + }, + ) + .await + .unwrap(); + check_error_message(subnet_for_canister, "not assigned to any subnet"); + + Ok(()) + } + }); +} diff --git a/rs/registry/canister/unreleased_changelog.md b/rs/registry/canister/unreleased_changelog.md index 333bf8c79edb..6b3d4f99dfc0 100644 --- a/rs/registry/canister/unreleased_changelog.md +++ b/rs/registry/canister/unreleased_changelog.md @@ -21,6 +21,11 @@ on the process that this file is part of, see `HostosVersion` accept, so until now, it was possible to elect a version that consumers could not read back out of the Registry. +* `merge_subnets` endpoint, callable through a `MergeSubnets` proposal. It merges a subnet into + another subnet: in the routing table, reassigns all canister ranges hosted by the source subnet + to the destination subnet. Only the routing table is updated: neither subnet record is modified + and the source subnet is not deleted. + ## Changed ## Deprecated diff --git a/rs/state_machine_tests/src/lib.rs b/rs/state_machine_tests/src/lib.rs index 306f853b1689..0698c52a90d9 100644 --- a/rs/state_machine_tests/src/lib.rs +++ b/rs/state_machine_tests/src/lib.rs @@ -2003,8 +2003,8 @@ impl StateMachine { let mut low_threshold_transcript_record = ni_dkg_transcript; low_threshold_transcript_record.dkg_id.dkg_tag = NiDkgTag::LowThreshold; let initial_transcript_records = SetupInitialDKGResponse { - low_threshold_transcript_record: high_threshold_transcript_record.into(), - high_threshold_transcript_record: low_threshold_transcript_record.into(), + low_threshold_transcript_record: low_threshold_transcript_record.into(), + high_threshold_transcript_record: high_threshold_transcript_record.into(), fresh_subnet_id: subnet_id, subnet_threshold_public_key: public_key.into(), };