Merge branch 'rumenov/rmnnssubnetit' into 'master'

fix: ICSUP-3716 don't expose publicly the production NNS_SUBNET_ID People are tempted to use the value directly resulting in incidents like ICSUP-3716. Instead of the NNS_SUBNET_ID, maybe use the subnet index in the list of subnet records. NNS should be index 0. Closes ICSUP-3716 See merge request dfinity-lab/public/ic!15653
dfinity · Oct 25, 2023 · 687d783 · 687d783
2 parents f3f69df + 79a6204
commit 687d783
Show file tree

Hide file tree

Showing 8 changed files with 17 additions and 20 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/rs/nns/constants/BUILD.bazel b/rs/nns/constants/BUILD.bazel
@@ -4,7 +4,6 @@ package(default_visibility = ["//visibility:public"])
 
 DEPENDENCIES = [
     "//rs/types/base_types",
-    "@crate_index//:lazy_static",
 ]
 
 rust_library(

diff --git a/rs/nns/constants/Cargo.toml b/rs/nns/constants/Cargo.toml
@@ -5,4 +5,3 @@ edition = "2021"
 
 [dependencies]
 ic-base-types = { path = "../../types/base_types"}
-lazy_static = "1.4.0"
diff --git a/rs/nns/constants/src/lib.rs b/rs/nns/constants/src/lib.rs
@@ -1,6 +1,4 @@
-use ic_base_types::{CanisterId, PrincipalId, SubnetId};
-use lazy_static::lazy_static;
-use std::str::FromStr;
+use ic_base_types::CanisterId;
 
 // WARNING: The NNS canisters MUST be installed in the NNS subnet,
 // in the following order, otherwise they won't be able to find
@@ -45,13 +43,6 @@ pub const NNS_CANISTER_WASMS: [&str; 13] = [
     "ic-ckbtc-minter",
 ];
 
-lazy_static! {
-    pub static ref NNS_SUBNET_ID: SubnetId = SubnetId::new(
-        PrincipalId::from_str("tdb26-jop6k-aogll-7ltgs-eruif-6kk7m-qpktf-gdiqx-mxtrf-vb5e6-eqe")
-            .unwrap()
-    );
-}
-
 pub const NUM_NNS_CANISTERS: usize = ALL_NNS_CANISTER_IDS.len();
 
 pub const REGISTRY_CANISTER_ID: CanisterId =

diff --git a/rs/registry/canister/BUILD.bazel b/rs/registry/canister/BUILD.bazel
@@ -38,6 +38,7 @@ DEPENDENCIES = [
     "@crate_index//:ic-cdk",
     "@crate_index//:ic-certified-map",
     "@crate_index//:ic-metrics-encoder",
+    "@crate_index//:lazy_static",
     "@crate_index//:ipnet",
     "@crate_index//:leb128",
     "@crate_index//:prost",
@@ -49,7 +50,6 @@ DEPENDENCIES = [
 DEV_DEPENDENCIES = [
     "@crate_index//:assert_matches",
     "@crate_index//:itertools",
-    "@crate_index//:lazy_static",
     "@crate_index//:maplit",
     "@crate_index//:rand_0_8_4",
     "@crate_index//:rand_distr_0_4",

diff --git a/rs/registry/canister/Cargo.toml b/rs/registry/canister/Cargo.toml
@@ -36,8 +36,9 @@ ic-registry-subnet-features = { path = "../../registry/subnet_features" }
 ic-registry-subnet-type = { path = "../../registry/subnet_type" }
 ic-registry-transport = { path = "../transport" }
 ic-types = { path = "../../types/types" }
-ipnet = "2.5.0"
+lazy_static = "1.4.0"
 leb128 = "0.2.4"
+ipnet = "2.5.0"
 on_wire = { path = "../../rust_canisters/on_wire" }
 prost = { workspace = true }
 serde = { version = "1.0", features = ["derive"] }

diff --git a/rs/registry/canister/src/mutations/do_delete_subnet.rs b/rs/registry/canister/src/mutations/do_delete_subnet.rs
@@ -10,7 +10,7 @@ use candid::{CandidType, Deserialize};
 use cycles_minting_canister::RemoveSubnetFromAuthorizedSubnetListArgs;
 use dfn_core::call;
 use ic_base_types::{PrincipalId, SubnetId};
-use ic_nns_constants::{CYCLES_MINTING_CANISTER_ID, GOVERNANCE_CANISTER_ID, NNS_SUBNET_ID};
+use ic_nns_constants::{CYCLES_MINTING_CANISTER_ID, GOVERNANCE_CANISTER_ID};
 use ic_protobuf::registry::routing_table::v1::RoutingTable as RoutingTablePb;
 use ic_registry_keys::{
     make_catch_up_package_contents_key, make_crypto_threshold_signing_pubkey_key,
@@ -19,6 +19,15 @@ use ic_registry_keys::{
 };
 use ic_registry_routing_table::RoutingTable;
 use ic_registry_transport::{delete, update};
+use lazy_static::lazy_static;
+use std::str::FromStr;
+
+lazy_static! {
+    pub static ref NNS_SUBNET_ID: SubnetId = SubnetId::new(
+        PrincipalId::from_str("tdb26-jop6k-aogll-7ltgs-eruif-6kk7m-qpktf-gdiqx-mxtrf-vb5e6-eqe")
+            .unwrap()
+    );
+}
 
 impl Registry {
     /// Delete an existing Subnet from the Registry.

diff --git a/rs/registry/canister/tests/delete_subnet.rs b/rs/registry/canister/tests/delete_subnet.rs
@@ -2,9 +2,7 @@ use candid::Encode;
 use cycles_minting_canister::CyclesCanisterInitPayload;
 use ic_base_types::{PrincipalId, SubnetId};
 use ic_nns_common::registry::encode_or_panic;
-use ic_nns_constants::{
-    CYCLES_MINTING_CANISTER_ID, GOVERNANCE_CANISTER_ID, LEDGER_CANISTER_ID, NNS_SUBNET_ID,
-};
+use ic_nns_constants::{CYCLES_MINTING_CANISTER_ID, GOVERNANCE_CANISTER_ID, LEDGER_CANISTER_ID};
 use ic_nns_test_utils::registry::new_node_keys_and_node_id;
 use ic_nns_test_utils::{
     itest_helpers::{
@@ -28,7 +26,8 @@ use ic_test_utilities::types::ids::user_test_id;
 use ic_types::{p2p::build_default_gossip_config, ReplicaVersion};
 use registry_canister::mutations::node_management::common::make_add_node_registry_mutations;
 use registry_canister::{
-    init::RegistryCanisterInitPayloadBuilder, mutations::do_delete_subnet::DeleteSubnetPayload,
+    init::RegistryCanisterInitPayloadBuilder,
+    mutations::do_delete_subnet::{DeleteSubnetPayload, NNS_SUBNET_ID},
 };
 
 #[test]
Original file line number	Diff line number	Diff line change
Expand Up		@@ -5,4 +5,3 @@ edition = "2021"

		[dependencies]
		ic-base-types = { path = "../../types/base_types"}
		lazy_static = "1.4.0"