From 8c1e08cfe0ab26488fa678d524bed0723bfaadc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20Tomi=C4=87?= Date: Thu, 13 Feb 2025 12:11:59 +0100 Subject: [PATCH 1/4] test: disable node subnet replacement tests ### Summary - Disabled tests related to node replacement in subnet. ### Details - Marked `should_add_node_and_replace_existing_node_in_subnet` as ignored in `do_add_node.rs` pending Consensus team support. - Marked `should_replace_node_in_subnet_and_update_allowance` as ignored in `do_remove_node_directly.rs` pending Consensus team support. ### Additional Changes - Introduced `replacements_of_nodes_in_subnets_enabled` flag set to `false` in `do_remove_node_directly.rs` to conditionally control node replacement logic. --- .../canister/src/mutations/node_management/do_add_node.rs | 2 ++ .../mutations/node_management/do_remove_node_directly.rs | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/rs/registry/canister/src/mutations/node_management/do_add_node.rs b/rs/registry/canister/src/mutations/node_management/do_add_node.rs index 65f4c4d8e31..df4540972a8 100644 --- a/rs/registry/canister/src/mutations/node_management/do_add_node.rs +++ b/rs/registry/canister/src/mutations/node_management/do_add_node.rs @@ -769,6 +769,8 @@ mod tests { assert!(e.contains("do_add_node: There is already another node with the same IPv4 address")); } + // This test is disabled until the Consensus team is ready to support direct replacement of nodes that are active in a subnet. + #[ignore] #[test] fn should_add_node_and_replace_existing_node_in_subnet() { // This test verifies that adding a new node replaces an existing node in a subnet diff --git a/rs/registry/canister/src/mutations/node_management/do_remove_node_directly.rs b/rs/registry/canister/src/mutations/node_management/do_remove_node_directly.rs index eba4f411c38..678bdc77040 100644 --- a/rs/registry/canister/src/mutations/node_management/do_remove_node_directly.rs +++ b/rs/registry/canister/src/mutations/node_management/do_remove_node_directly.rs @@ -149,8 +149,10 @@ impl Registry { // 4. Check if node is in a subnet, and if so, replace it in the subnet by updating the membership in the subnet record. let subnet_list_record = get_subnet_list_record(self); let is_node_in_subnet = find_subnet_for_node(self, payload.node_id, &subnet_list_record); + // Disabled until the Consensus team is ready to support direct replacement of nodes that are active in a subnet. + let replacements_of_nodes_in_subnets_enabled = false; if let Some(subnet_id) = is_node_in_subnet { - if new_node_id.is_some() { + if new_node_id.is_some() && replacements_of_nodes_in_subnets_enabled { // The node is in a subnet and is being replaced with a new node. // Update the subnet record with the new node membership. let mut subnet_record = self.get_subnet_or_panic(subnet_id); @@ -626,6 +628,8 @@ mod tests { registry.do_remove_node(payload, node_operator_id); } + // This test is disabled until the Consensus team is ready to support direct replacement of nodes that are active in a subnet. + #[ignore] #[test] fn should_replace_node_in_subnet_and_update_allowance() { let mut registry = invariant_compliant_registry(0); From c36c922377eaf63f0b474389734c26286e5ca4be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20Tomi=C4=87?= Date: Thu, 13 Feb 2025 13:22:56 +0100 Subject: [PATCH 2/4] Update comments --- .../canister/src/mutations/node_management/do_add_node.rs | 2 +- .../src/mutations/node_management/do_remove_node_directly.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rs/registry/canister/src/mutations/node_management/do_add_node.rs b/rs/registry/canister/src/mutations/node_management/do_add_node.rs index df4540972a8..f709b3c50d8 100644 --- a/rs/registry/canister/src/mutations/node_management/do_add_node.rs +++ b/rs/registry/canister/src/mutations/node_management/do_add_node.rs @@ -769,7 +769,7 @@ mod tests { assert!(e.contains("do_add_node: There is already another node with the same IPv4 address")); } - // This test is disabled until the Consensus team is ready to support direct replacement of nodes that are active in a subnet. + // This test is disabled until it becomes possible to directly replace nodes that are active in a subnet. #[ignore] #[test] fn should_add_node_and_replace_existing_node_in_subnet() { diff --git a/rs/registry/canister/src/mutations/node_management/do_remove_node_directly.rs b/rs/registry/canister/src/mutations/node_management/do_remove_node_directly.rs index 678bdc77040..3d4529f1d02 100644 --- a/rs/registry/canister/src/mutations/node_management/do_remove_node_directly.rs +++ b/rs/registry/canister/src/mutations/node_management/do_remove_node_directly.rs @@ -149,7 +149,7 @@ impl Registry { // 4. Check if node is in a subnet, and if so, replace it in the subnet by updating the membership in the subnet record. let subnet_list_record = get_subnet_list_record(self); let is_node_in_subnet = find_subnet_for_node(self, payload.node_id, &subnet_list_record); - // Disabled until the Consensus team is ready to support direct replacement of nodes that are active in a subnet. + // Disabled until the direct replacement of nodes that are active in a subnet is possible. let replacements_of_nodes_in_subnets_enabled = false; if let Some(subnet_id) = is_node_in_subnet { if new_node_id.is_some() && replacements_of_nodes_in_subnets_enabled { @@ -628,7 +628,7 @@ mod tests { registry.do_remove_node(payload, node_operator_id); } - // This test is disabled until the Consensus team is ready to support direct replacement of nodes that are active in a subnet. + // This test is disabled until it becomes possible to directly replace nodes that are active in a subnet. #[ignore] #[test] fn should_replace_node_in_subnet_and_update_allowance() { From b8ce3a50c72cab56de870381535f3e4aa597614f Mon Sep 17 00:00:00 2001 From: CI Automation Date: Thu, 13 Feb 2025 12:18:06 +0000 Subject: [PATCH 3/4] chore: Update Mainnet IC revisions canisters file --- mainnet-canister-revisions.json | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/mainnet-canister-revisions.json b/mainnet-canister-revisions.json index 99030805ea8..27e9437111f 100644 --- a/mainnet-canister-revisions.json +++ b/mainnet-canister-revisions.json @@ -60,16 +60,16 @@ "sha256": "9637743e1215a4db376a62ee807a0986faf20833be2b332df09b3d5dbdd7339e" }, "cycles-minting": { - "rev": "b5192581ccd35b67fe5a1f795ead9cbcd25956d6", - "sha256": "11c8dedd11741f05990498c90f925e9e37ad60647a65ef47caa59cdba234be6f" + "rev": "2d4bcba47ea10520ff21ce54a8acb9cdb0629317", + "sha256": "4e4a6f907a3b4720bfeb4c45c5027d9b1379deb840e4ba0217876df9a4c5b193" }, "genesis-token": { "rev": "4bed17bfc82cddc5691743db6228992cdc2740f4", "sha256": "fd25a4e2e283b498c3be1aaf63cc9b2726264d78a12b12f43ad453ceeb575e7c" }, "governance": { - "rev": "c5e098e8cc8e62249e6d7f4ed09e6c2ed87fc800", - "sha256": "bd821399f16d4bdac8cef06b81d8ac72cf8e36a7ab766efccb2a0925be6f388f" + "rev": "345790def6dfa93bcc317171c9733851d205802e", + "sha256": "15c9b8db9e54963d4f7075e1fd3bd6a0055dbe6981a2f6a54dd64114fdb0629f" }, "index": { "rev": "7c6309cb5bec7ab28ed657ac7672af08a59fc1ba", @@ -96,8 +96,8 @@ "sha256": "8c8eb285de53ca5609abd7dc41ba3ec8eeb67708b81469311fd670e6738d7d0a" }, "registry": { - "rev": "a5878586e47536d4cd47f0aadb66b73df8131d2b", - "sha256": "f0fb8fa545b2cc68f030b040e1182a8d004c4d4f4bb4341c9f1b432642c85bef" + "rev": "2d4bcba47ea10520ff21ce54a8acb9cdb0629317", + "sha256": "f8b2aba5d7217cc07a84d59a18af825c2a8813dd38073b96456b2e83d1e78e37" }, "root": { "rev": "c5e098e8cc8e62249e6d7f4ed09e6c2ed87fc800", @@ -112,8 +112,8 @@ "sha256": "2b0970a84976bc2eb9591b68d44501566937994fa5594972f8aac9c8b058672f" }, "sns_governance": { - "rev": "c5e098e8cc8e62249e6d7f4ed09e6c2ed87fc800", - "sha256": "487940ab827c929471dbd128fcb1c7d5dfe2f3f56d9e489e0f3b68122b56a94f" + "rev": "2d4bcba47ea10520ff21ce54a8acb9cdb0629317", + "sha256": "82edbcf511304b53e519965b4899266ea75593c3e731524f0b010a1fc4e974ad" }, "sns_index": { "rev": "c741e349451edf0c9792149ad439bb32a0161371", @@ -128,11 +128,11 @@ "sha256": "3d808fa63a3d8ebd4510c0400aa078e99a31afaa0515f0b68778f929ce4b2a46" }, "sns_root": { - "rev": "c5e098e8cc8e62249e6d7f4ed09e6c2ed87fc800", - "sha256": "9c129437f868e54a2dc3c72438cd7977925d089f9982c35d0370eaa71d5e3a6d" + "rev": "2d4bcba47ea10520ff21ce54a8acb9cdb0629317", + "sha256": "cbbc1ffdd911c58692409e942fafbddcf3e3445e1df99b26d83ae8ed5383af9a" }, "swap": { "rev": "a5878586e47536d4cd47f0aadb66b73df8131d2b", "sha256": "45408ed654561dfb17c84b86948dda9498aa0ba8ee669ae774e5faca830c4c24" } -} +} \ No newline at end of file From c962d7eb782558f85b4342a56667f424cbfa5321 Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Thu, 13 Feb 2025 13:21:38 +0000 Subject: [PATCH 4/4] Add changelog note --- rs/registry/canister/unreleased_changelog.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rs/registry/canister/unreleased_changelog.md b/rs/registry/canister/unreleased_changelog.md index 94126a0ff42..d613a35c384 100644 --- a/rs/registry/canister/unreleased_changelog.md +++ b/rs/registry/canister/unreleased_changelog.md @@ -17,4 +17,9 @@ on the process that this file is part of, see ## Fixed +### Disable replacement of nodes that are active in subnets + +Direct node replacements of nodes that are active in a subnet may result in unexpected behavior and potential problems in the current Consensus code. +So to be on the safe side we need to disable the functionality on the Registry side until the rest of the core protocol can handle it safely. + ## Security