From 3c63dba94e21ad78e0548cadc6d0dd4d1f5b9ff2 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Mon, 15 Jun 2026 09:20:55 +0000 Subject: [PATCH 1/8] chore: support canister migration from/to deleted subnet in registry --- .../src/mutations/do_migrate_canisters.rs | 162 +++++++++++++++++- .../canister/src/mutations/routing_table.rs | 6 + 2 files changed, 164 insertions(+), 4 deletions(-) diff --git a/rs/registry/canister/src/mutations/do_migrate_canisters.rs b/rs/registry/canister/src/mutations/do_migrate_canisters.rs index 5f690b3c8fe6..4eb2145139c6 100644 --- a/rs/registry/canister/src/mutations/do_migrate_canisters.rs +++ b/rs/registry/canister/src/mutations/do_migrate_canisters.rs @@ -40,7 +40,6 @@ impl Registry { .collect::, _>>()?; let target_subnet_id = SubnetId::new(target_subnet_id); - self.get_subnet(target_subnet_id, self.latest_version())?; Ok((canister_ids, target_subnet_id)) } @@ -88,6 +87,8 @@ mod test { let target_subnet_id = registry_create_subnet_with_nodes(&mut registry, &node_ids_and_dkg_pks, &[0, 1, 2, 3]); + // The target subnet must already appear in the routing table for migration to proceed; + // give it a dedicated range (256–256) in addition to the system subnet's range. let mut initial_routing_table = RoutingTable::new(); initial_routing_table .insert( @@ -98,6 +99,15 @@ mod test { system_subnet.into(), ) .unwrap(); + initial_routing_table + .insert( + CanisterIdRange { + start: CanisterId::from_u64(256), + end: CanisterId::from_u64(256), + }, + target_subnet_id, + ) + .unwrap(); registry.apply_mutations_for_test(routing_table_into_registry_mutation( ®istry, @@ -114,7 +124,6 @@ mod test { let response = registry.do_migrate_canisters(request); - // currently just empty response. assert_eq!( MigrateCanistersResponse { registry_version: registry.latest_version() @@ -124,8 +133,8 @@ mod test { let updated_routing_table = registry.get_routing_table_or_panic(registry.latest_version()); - // We expect the range to be split into 3 ranges, with the middle one being a single-canister-id range - // and the others pointing at the system subnet as before. + // We expect the system subnet's range to be split around canister 100, with canister 100 + // now assigned to the target subnet alongside its pre-existing range 256–256. let mut expected_routing_table = RoutingTable::new(); expected_routing_table .insert( @@ -154,6 +163,151 @@ mod test { system_subnet.into(), ) .unwrap(); + expected_routing_table + .insert( + CanisterIdRange { + start: CanisterId::from_u64(256), + end: CanisterId::from_u64(256), + }, + target_subnet_id, + ) + .unwrap(); + assert_eq!(updated_routing_table, expected_routing_table); + } + + #[test] + fn test_migrate_canisters_succeeds_if_source_subnet_deleted() { + // Verify that migrate_canisters succeeds when the source subnet has been deleted + // (has no key in subnet_list and no ranges in routing_table). + // + // We simulate this by using a subnet ID that is not in the routing table to begin with: + // the source subnet for canister 100 simply has no routing_table ranges (gap at 100). + + let mut registry = invariant_compliant_registry(0); + let system_subnet_id = + PrincipalId::try_from(registry.get_subnet_list_record().subnets.first().unwrap()) + .unwrap(); + + // Set up routing table: system subnet covers 0–99 and 101–255; canister 100 is + // unrouted, as if its source subnet has been deleted. + let mut initial_routing_table = RoutingTable::new(); + initial_routing_table + .insert( + CanisterIdRange { + start: CanisterId::from_u64(0), + end: CanisterId::from_u64(99), + }, + system_subnet_id.into(), + ) + .unwrap(); + initial_routing_table + .insert( + CanisterIdRange { + start: CanisterId::from_u64(101), + end: CanisterId::from_u64(255), + }, + system_subnet_id.into(), + ) + .unwrap(); + registry.apply_mutations_for_test(routing_table_into_registry_mutation( + ®istry, + initial_routing_table, + )); + + // Migrate canister 100 (not in routing table) to the system subnet. + // The system subnet is in the routing table, so the endpoint must succeed. + let response = registry.do_migrate_canisters(MigrateCanistersPayload { + canister_ids: vec![PrincipalId::from(CanisterId::from_u64(100))], + target_subnet_id: system_subnet_id, + }); + assert_eq!( + MigrateCanistersResponse { + registry_version: registry.latest_version() + }, + response + ); + + // Canister 100 was unrouted (gap); assigning it to the system subnet fills the gap, + // and optimize() merges 0–99, 100–100, 101–255 into a single range. + let updated_routing_table = registry.get_routing_table_or_panic(registry.latest_version()); + let mut expected_routing_table = RoutingTable::new(); + expected_routing_table + .insert( + CanisterIdRange { + start: CanisterId::from_u64(0), + end: CanisterId::from_u64(255), + }, + system_subnet_id.into(), + ) + .unwrap(); + assert_eq!(updated_routing_table, expected_routing_table); + } + + #[test] + fn test_migrate_canisters_succeeds_if_target_subnet_does_not_exist() { + // Verify that migrate_canisters succeeds when the target subnet does not exist + // (has no key in subnet_list and no ranges in routing_table). + // + // We simulate this by using a subnet ID that is not in the routing table to begin with. + + let mut registry = invariant_compliant_registry(0); + let system_subnet_id = + PrincipalId::try_from(registry.get_subnet_list_record().subnets.first().unwrap()) + .unwrap(); + + // Set up routing table: system subnet covers 0–255. + let mut initial_routing_table = RoutingTable::new(); + initial_routing_table + .insert( + CanisterIdRange { + start: CanisterId::from_u64(0), + end: CanisterId::from_u64(255), + }, + system_subnet_id.into(), + ) + .unwrap(); + registry.apply_mutations_for_test(routing_table_into_registry_mutation( + ®istry, + initial_routing_table, + )); + + // Migrate canister 100 to a non-existent target subnet. + // The target subnet is not in the routing table, so the endpoint must succeed + // without adding any range for the target subnet. + let non_existent_subnet_id = PrincipalId::new_user_test_id(42); + let response = registry.do_migrate_canisters(MigrateCanistersPayload { + canister_ids: vec![PrincipalId::from(CanisterId::from_u64(100))], + target_subnet_id: non_existent_subnet_id, + }); + assert_eq!( + MigrateCanistersResponse { + registry_version: registry.latest_version() + }, + response + ); + + // Canister 100 was removed from the system subnet's range and not assigned to + // any other subnet (target absent from routing table), leaving a gap at 100. + let updated_routing_table = registry.get_routing_table_or_panic(registry.latest_version()); + let mut expected_routing_table = RoutingTable::new(); + expected_routing_table + .insert( + CanisterIdRange { + start: CanisterId::from_u64(0), + end: CanisterId::from_u64(99), + }, + system_subnet_id.into(), + ) + .unwrap(); + expected_routing_table + .insert( + CanisterIdRange { + start: CanisterId::from_u64(101), + end: CanisterId::from_u64(255), + }, + system_subnet_id.into(), + ) + .unwrap(); assert_eq!(updated_routing_table, expected_routing_table); } } diff --git a/rs/registry/canister/src/mutations/routing_table.rs b/rs/registry/canister/src/mutations/routing_table.rs index 35ce7a10d225..1c71c051c210 100644 --- a/rs/registry/canister/src/mutations/routing_table.rs +++ b/rs/registry/canister/src/mutations/routing_table.rs @@ -303,9 +303,15 @@ impl Registry { subnet_id: SubnetId, ) -> Vec { self.modify_routing_table(version, |routing_table| { + let target_in_routing_table = !routing_table.ranges(subnet_id).is_empty(); for canister_id in canister_ids { routing_table.assign_canister(canister_id, subnet_id); } + if !target_in_routing_table { + // Target subnet has no canister ranges in the routing table: remove + // any canister ranges just assigned to it, keeping source subnets split. + routing_table.remove_subnet(subnet_id); + } routing_table.optimize(); }) } From a924162a829e6481a7d7a5996be1091cb9650045 Mon Sep 17 00:00:00 2001 From: mraszyk <31483726+mraszyk@users.noreply.github.com> Date: Mon, 15 Jun 2026 12:18:18 +0200 Subject: [PATCH 2/8] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- rs/registry/canister/src/mutations/routing_table.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/registry/canister/src/mutations/routing_table.rs b/rs/registry/canister/src/mutations/routing_table.rs index 1c71c051c210..1ac1cc862eb0 100644 --- a/rs/registry/canister/src/mutations/routing_table.rs +++ b/rs/registry/canister/src/mutations/routing_table.rs @@ -303,7 +303,7 @@ impl Registry { subnet_id: SubnetId, ) -> Vec { self.modify_routing_table(version, |routing_table| { - let target_in_routing_table = !routing_table.ranges(subnet_id).is_empty(); + let target_in_routing_table = routing_table.iter().any(|(_range, sid)| *sid == subnet_id); for canister_id in canister_ids { routing_table.assign_canister(canister_id, subnet_id); } From 531789917fec5ef038dcfb43f0502569fa2029bb Mon Sep 17 00:00:00 2001 From: mraszyk <31483726+mraszyk@users.noreply.github.com> Date: Mon, 15 Jun 2026 12:18:32 +0200 Subject: [PATCH 3/8] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- rs/registry/canister/src/mutations/do_migrate_canisters.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/registry/canister/src/mutations/do_migrate_canisters.rs b/rs/registry/canister/src/mutations/do_migrate_canisters.rs index 4eb2145139c6..bb00aac7e5dd 100644 --- a/rs/registry/canister/src/mutations/do_migrate_canisters.rs +++ b/rs/registry/canister/src/mutations/do_migrate_canisters.rs @@ -87,7 +87,7 @@ mod test { let target_subnet_id = registry_create_subnet_with_nodes(&mut registry, &node_ids_and_dkg_pks, &[0, 1, 2, 3]); - // The target subnet must already appear in the routing table for migration to proceed; + // The target subnet must already appear in the routing table for the migration to keep it assigned; // give it a dedicated range (256–256) in addition to the system subnet's range. let mut initial_routing_table = RoutingTable::new(); initial_routing_table From 4dcfe931e55166e3e7babe2aa834f28950242810 Mon Sep 17 00:00:00 2001 From: mraszyk <31483726+mraszyk@users.noreply.github.com> Date: Mon, 15 Jun 2026 12:20:17 +0200 Subject: [PATCH 4/8] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- rs/registry/canister/src/mutations/do_migrate_canisters.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs/registry/canister/src/mutations/do_migrate_canisters.rs b/rs/registry/canister/src/mutations/do_migrate_canisters.rs index bb00aac7e5dd..559b10dbe7fd 100644 --- a/rs/registry/canister/src/mutations/do_migrate_canisters.rs +++ b/rs/registry/canister/src/mutations/do_migrate_canisters.rs @@ -39,6 +39,8 @@ impl Registry { }) .collect::, _>>()?; + // Intentionally do not validate that the target subnet exists: the migration orchestrator + // may still attempt to migrate canisters to/from a subnet that has been deleted. let target_subnet_id = SubnetId::new(target_subnet_id); Ok((canister_ids, target_subnet_id)) From 7e5ff24dbfdf0b893e44b1fa4d4ffc2079783d00 Mon Sep 17 00:00:00 2001 From: IDX GitHub Automation Date: Mon, 15 Jun 2026 10:26:07 +0000 Subject: [PATCH 5/8] Automatically fixing code for linting and formatting issues --- rs/registry/canister/src/mutations/routing_table.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rs/registry/canister/src/mutations/routing_table.rs b/rs/registry/canister/src/mutations/routing_table.rs index 1ac1cc862eb0..df0425f82d34 100644 --- a/rs/registry/canister/src/mutations/routing_table.rs +++ b/rs/registry/canister/src/mutations/routing_table.rs @@ -303,7 +303,8 @@ impl Registry { subnet_id: SubnetId, ) -> Vec { self.modify_routing_table(version, |routing_table| { - let target_in_routing_table = routing_table.iter().any(|(_range, sid)| *sid == subnet_id); + let target_in_routing_table = + routing_table.iter().any(|(_range, sid)| *sid == subnet_id); for canister_id in canister_ids { routing_table.assign_canister(canister_id, subnet_id); } From 4f6b9bf905531a31f85516b944a15aa78f214c15 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 18 Jun 2026 12:58:29 +0000 Subject: [PATCH 6/8] comment --- rs/registry/canister/src/mutations/do_migrate_canisters.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rs/registry/canister/src/mutations/do_migrate_canisters.rs b/rs/registry/canister/src/mutations/do_migrate_canisters.rs index 559b10dbe7fd..eacb22419a5b 100644 --- a/rs/registry/canister/src/mutations/do_migrate_canisters.rs +++ b/rs/registry/canister/src/mutations/do_migrate_canisters.rs @@ -40,7 +40,10 @@ impl Registry { .collect::, _>>()?; // Intentionally do not validate that the target subnet exists: the migration orchestrator - // may still attempt to migrate canisters to/from a subnet that has been deleted. + // may still attempt to migrate canisters to a subnet that has been deleted. + // In this case, the outcome should be as if the migration completed before the subnet + // got deleted, i.e., the canister should be removed from the source subnet's canister + // ranges and not added to the (deleted) target subnet's canister ranges. let target_subnet_id = SubnetId::new(target_subnet_id); Ok((canister_ids, target_subnet_id)) From 183afe325d0ebc9bfe1c117c3bebbb4f8f2af836 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 18 Jun 2026 13:01:15 +0000 Subject: [PATCH 7/8] comment --- .../src/mutations/do_migrate_canisters.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/rs/registry/canister/src/mutations/do_migrate_canisters.rs b/rs/registry/canister/src/mutations/do_migrate_canisters.rs index eacb22419a5b..ffdb3a081b0a 100644 --- a/rs/registry/canister/src/mutations/do_migrate_canisters.rs +++ b/rs/registry/canister/src/mutations/do_migrate_canisters.rs @@ -248,19 +248,17 @@ mod test { assert_eq!(updated_routing_table, expected_routing_table); } + // Verify that migrate_canisters succeeds when the target subnet does not exist + // (has no key in subnet_list and no ranges in routing_table), e.g., + // after the target subnet has been deleted. #[test] fn test_migrate_canisters_succeeds_if_target_subnet_does_not_exist() { - // Verify that migrate_canisters succeeds when the target subnet does not exist - // (has no key in subnet_list and no ranges in routing_table). - // - // We simulate this by using a subnet ID that is not in the routing table to begin with. - let mut registry = invariant_compliant_registry(0); let system_subnet_id = PrincipalId::try_from(registry.get_subnet_list_record().subnets.first().unwrap()) .unwrap(); - // Set up routing table: system subnet covers 0–255. + // Set up routing table: system subnet covers canister 0–255. let mut initial_routing_table = RoutingTable::new(); initial_routing_table .insert( @@ -276,10 +274,13 @@ mod test { initial_routing_table, )); + // We simulate a deleted subnet by using a subnet ID + // that is not in the routing table to begin with. + let non_existent_subnet_id = PrincipalId::new_user_test_id(42); + // Migrate canister 100 to a non-existent target subnet. // The target subnet is not in the routing table, so the endpoint must succeed // without adding any range for the target subnet. - let non_existent_subnet_id = PrincipalId::new_user_test_id(42); let response = registry.do_migrate_canisters(MigrateCanistersPayload { canister_ids: vec![PrincipalId::from(CanisterId::from_u64(100))], target_subnet_id: non_existent_subnet_id, @@ -292,7 +293,7 @@ mod test { ); // Canister 100 was removed from the system subnet's range and not assigned to - // any other subnet (target absent from routing table), leaving a gap at 100. + // any other subnet (as target subnet is absent from routing table), leaving a gap at 100. let updated_routing_table = registry.get_routing_table_or_panic(registry.latest_version()); let mut expected_routing_table = RoutingTable::new(); expected_routing_table From ba435704ff0698225abcf27f8e8fc25ed07fbfaa Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Tue, 23 Jun 2026 13:01:07 +0000 Subject: [PATCH 8/8] comments and variable names --- .../src/mutations/do_migrate_canisters.rs | 23 +++++++++------ .../canister/src/mutations/routing_table.rs | 28 +++++++++++++------ 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/rs/registry/canister/src/mutations/do_migrate_canisters.rs b/rs/registry/canister/src/mutations/do_migrate_canisters.rs index ffdb3a081b0a..9ac62eac9166 100644 --- a/rs/registry/canister/src/mutations/do_migrate_canisters.rs +++ b/rs/registry/canister/src/mutations/do_migrate_canisters.rs @@ -39,11 +39,16 @@ impl Registry { }) .collect::, _>>()?; - // Intentionally do not validate that the target subnet exists: the migration orchestrator - // may still attempt to migrate canisters to a subnet that has been deleted. - // In this case, the outcome should be as if the migration completed before the subnet - // got deleted, i.e., the canister should be removed from the source subnet's canister - // ranges and not added to the (deleted) target subnet's canister ranges. + // Subnet deletion is (currently) a manual process and we don't expect to actually delete + // a subnet while a canister migration is ongoing (instead we expect to disable the + // canister migration API first and only delete a subnet once there's no ongoing canister + // migration). However, e.g., if a subnet is permanently stuck, it might be impossible to + // successfully complete an ongoing canister migration and in this case, we'd like to still + // be able to delete such a subnet. In this case, the outcome should be as if the migration + // completed before the subnet got deleted, i.e., the canister should be removed from the + // source subnet's canister ranges and not added to the (deleted) target subnet's canister + // ranges. + // Hence, we intentionally do not validate that the target subnet exists here. let target_subnet_id = SubnetId::new(target_subnet_id); Ok((canister_ids, target_subnet_id)) @@ -79,7 +84,8 @@ mod test { #[test] fn test_basic_migrate_canisters() { // We create an invariant compliant registry, then we migrate a single canister - // to a new subnet, and we check that the Routing table has the correct ranges at that point. + // to a new subnet, and we check that the Routing table has the correct ranges + // at that point. let mut registry = invariant_compliant_registry(0); let system_subnet = @@ -92,8 +98,9 @@ mod test { let target_subnet_id = registry_create_subnet_with_nodes(&mut registry, &node_ids_and_dkg_pks, &[0, 1, 2, 3]); - // The target subnet must already appear in the routing table for the migration to keep it assigned; - // give it a dedicated range (256–256) in addition to the system subnet's range. + // The target subnet must already appear in the routing table for the migration to keep + // it assigned; give it a dedicated range (256–256) in addition to the system subnet's + // range. let mut initial_routing_table = RoutingTable::new(); initial_routing_table .insert( diff --git a/rs/registry/canister/src/mutations/routing_table.rs b/rs/registry/canister/src/mutations/routing_table.rs index df0425f82d34..ab3f20d6b54d 100644 --- a/rs/registry/canister/src/mutations/routing_table.rs +++ b/rs/registry/canister/src/mutations/routing_table.rs @@ -296,22 +296,34 @@ impl Registry { routing_table_into_registry_mutation(self, routing_table) } + /// Subnet deletion is (currently) a manual process and we don't expect to actually delete + /// a subnet while a canister migration is ongoing (instead we expect to disable the + /// canister migration API first and only delete a subnet once there's no ongoing canister + /// migration). However, e.g., if a subnet is permanently stuck, it might be impossible to + /// successfully complete an ongoing canister migration and in this case, we'd like to still + /// be able to delete such a subnet. In this case, the outcome should be as if the migration + /// completed before the subnet got deleted. pub fn migrate_canisters_to_subnet( &self, version: u64, canister_ids: Vec, - subnet_id: SubnetId, + target_subnet_id: SubnetId, ) -> Vec { self.modify_routing_table(version, |routing_table| { - let target_in_routing_table = - routing_table.iter().any(|(_range, sid)| *sid == subnet_id); + let target_subnet_in_routing_table = routing_table + .iter() + .any(|(_range, sid)| *sid == target_subnet_id); + // If the source subnet has been deleted, assign_canister still + // successfully removes the canister from the source subnet's canister ranges. + // If the target subnet has been deleted, assign_canister introduces a new + // canister range for it, which is cleaned up below. for canister_id in canister_ids { - routing_table.assign_canister(canister_id, subnet_id); + routing_table.assign_canister(canister_id, target_subnet_id); } - if !target_in_routing_table { - // Target subnet has no canister ranges in the routing table: remove - // any canister ranges just assigned to it, keeping source subnets split. - routing_table.remove_subnet(subnet_id); + if !target_subnet_in_routing_table { + // Target subnet has no canister ranges in the routing table (e.g., it has + // been deleted): remove any canister ranges just assigned to it. + routing_table.remove_subnet(target_subnet_id); } routing_table.optimize(); })