refactor: Encapsulate NetworkTopology

rs/canonical_state/src/lazy_tree_conversion.rs

@@ -426,7 +426,7 @@ pub fn replicated_state_as_lazy_tree(state: &ReplicatedState, height: Height) ->
     );
     let own_subnet_id = state.metadata.own_subnet_id;
     let inverted_routing_table = Arc::new(invert_routing_table(
-        &state.metadata.network_topology.routing_table,
+        &state.metadata.network_topology.routing_table(),
     ));
     let split_routing_table = Arc::new(split_inverted_routing_table(
         &inverted_routing_table,
@@ -456,7 +456,7 @@ pub fn replicated_state_as_lazy_tree(state: &ReplicatedState, height: Height) ->
             )
             .with("subnet", move || {
                 subnets_as_tree(
-                    &state.metadata.network_topology.subnets,
+                    state.metadata.network_topology.subnets(),
                     own_subnet_id,
                     &state.metadata.node_public_keys,
                     inverted_routing_table.clone(),
@@ -473,7 +473,7 @@ pub fn replicated_state_as_lazy_tree(state: &ReplicatedState, height: Height) ->
                 "canister_ranges",
                 move || {
                     canister_ranges_as_tree(
-                        &state.metadata.network_topology.subnets,
+                        state.metadata.network_topology.subnets(),
                         Arc::clone(&split_routing_table),
                         certification_version,
                     )

rs/canonical_state/src/traversal.rs

@@ -64,7 +64,7 @@ mod tests {
             ExecutionState, ExportedFunctions, NumWasmPages,
             execution_state::{CustomSection, CustomSectionType, WasmBinary, WasmMetadata},
         },
-        metadata_state::{ApiBoundaryNodeEntry, SubnetTopology},
+        metadata_state::{ApiBoundaryNodeEntry, SubnetTopology, testing::NetworkTopologyTesting},
         page_map::PageMap,
         testing::{ReplicatedStateTesting, StreamTesting},
     };
@@ -82,7 +82,6 @@ mod tests {
     use maplit::btreemap;
     use std::collections::{BTreeSet, VecDeque};
     use std::convert::TryFrom;
-    use std::sync::Arc;
     use std::time::Duration;
 
     const INITIAL_CYCLES: Cycles = Cycles::new(1 << 36);
@@ -768,7 +767,7 @@ mod tests {
     fn test_traverse_subnet() {
         let mut state = ReplicatedState::new(subnet_test_id(1), SubnetType::Application);
 
-        state.metadata.network_topology.subnets = btreemap! {
+        state.metadata.network_topology.set_subnets(btreemap! {
             subnet_test_id(0) => SubnetTopology {
                 public_key: vec![1, 2, 3, 4],
                 nodes: BTreeSet::new(),
@@ -785,8 +784,8 @@ mod tests {
                 chain_keys_held: BTreeSet::new(),
                 cost_schedule: CanisterCyclesCostSchedule::Normal,
             }
-        };
-        state.metadata.network_topology.routing_table = Arc::new(
+        });
+        state.metadata.network_topology.set_routing_table(
             RoutingTable::try_from(btreemap! {
                 id_range(0, 10) => subnet_test_id(0),
                 id_range(11, 20) => subnet_test_id(1),
@@ -933,7 +932,7 @@ mod tests {
     fn test_traverse_large_or_empty_routing_table() {
         let mut state = ReplicatedState::new(subnet_test_id(1), SubnetType::Application);
 
-        state.metadata.network_topology.subnets = btreemap! {
+        state.metadata.network_topology.set_subnets(btreemap! {
             subnet_test_id(0) => SubnetTopology {
                 public_key: vec![1, 2, 3, 4],
                 nodes: BTreeSet::new(),
@@ -950,8 +949,8 @@ mod tests {
                 chain_keys_held: BTreeSet::new(),
                 cost_schedule: CanisterCyclesCostSchedule::Normal,
             }
-        };
-        state.metadata.network_topology.routing_table = Arc::new(
+        });
+        state.metadata.network_topology.set_routing_table(
             RoutingTable::try_from(btreemap! {
                 id_range(0, 10) => subnet_test_id(0),
                 id_range(21, 30) => subnet_test_id(0),

rs/embedders/src/wasmtime_embedder/system_api/routing.rs

@@ -390,7 +390,7 @@ fn route_chain_key_message(
     }
 
     match requested_subnet {
-        Some(subnet_id) => match network_topology.subnets.get(subnet_id) {
+        Some(subnet_id) => match network_topology.subnets().get(subnet_id) {
             None => Err(ResolveDestinationError::ChainKeyError(format!(
                 "Requested threshold key {key_id} from unknown subnet {subnet_id}"
             ))),
@@ -437,7 +437,7 @@ fn route_chain_key_message(
                 }
                 ChainKeySubnetKind::OnlyHoldsKey => {
                     let mut keys = BTreeSet::new();
-                    for (subnet_id, topology) in &network_topology.subnets {
+                    for (subnet_id, topology) in network_topology.subnets() {
                         if topology.chain_keys_held.contains(key_id) {
                             return Ok((*subnet_id).get());
                         }
@@ -496,7 +496,7 @@ mod tests {
         DerivationPath, EcdsaCurve, EcdsaKeyId, SchnorrAlgorithm, SchnorrKeyId, SignWithECDSAArgs,
         VetKdCurve, VetKdKeyId,
     };
-    use ic_replicated_state::SubnetTopology;
+    use ic_replicated_state::{SubnetTopology, metadata_state::testing::NetworkTopologyTesting};
     use ic_test_utilities_types::ids::{canister_test_id, node_test_id, subnet_test_id};
     use maplit::btreemap;
     use serde_bytes::ByteBuf;
@@ -543,26 +543,24 @@ mod tests {
         key_id2: MasterPublicKeyId,
     ) -> NetworkTopology {
         let subnet_id0 = subnet_test_id(0);
-        NetworkTopology {
-            // The first key is enabled only on subnet 0.
-            chain_key_enabled_subnets: btreemap! {
-                key_id1.clone() => vec![subnet_id0],
+        let mut network_topology = NetworkTopology::default();
+        network_topology.chain_key_enabled_subnets = btreemap! {
+            key_id1.clone() => vec![subnet_id0],
+        };
+        network_topology.set_subnets(btreemap! {
+            // Subnet 0 holds both keys
+            subnet_id0 => SubnetTopology {
+                chain_keys_held: vec![key_id1.clone(), key_id2].into_iter().collect(),
+                ..SubnetTopology::default()
             },
-            subnets: btreemap! {
-                // Subnet 0 holds both keys
-                subnet_id0 => SubnetTopology {
-                    chain_keys_held: vec![key_id1.clone(), key_id2].into_iter().collect(),
-                    ..SubnetTopology::default()
-                },
-                // Subnet 1 holds only the first key.
-                subnet_test_id(1) => SubnetTopology {
-                    chain_keys_held: vec![key_id1].into_iter().collect(),
-                    ..SubnetTopology::default()
-                },
-                subnet_test_id(2) => SubnetTopology::default(),
+            // Subnet 1 holds only the first key.
+            subnet_test_id(1) => SubnetTopology {
+                chain_keys_held: vec![key_id1].into_iter().collect(),
+                ..SubnetTopology::default()
             },
-            ..NetworkTopology::default()
-        }
+            subnet_test_id(2) => SubnetTopology::default(),
+        });
+        network_topology
     }
 
     fn network_with_ecdsa_subnets() -> NetworkTopology {

rs/embedders/src/wasmtime_embedder/system_api/sandbox_safe_system_state.rs

@@ -398,7 +398,7 @@ impl SystemStateModifications {
         let mut callback_changes = BTreeMap::new();
         let nns_subnet_id = network_topology.nns_subnet_id;
         let subnet_ids: BTreeSet<PrincipalId> =
-            network_topology.subnets.keys().map(|s| s.get()).collect();
+            network_topology.subnets().keys().map(|s| s.get()).collect();
         for mut msg in self.requests {
             if msg.receiver == IC_00 {
                 match Self::validate_sender_canister_version(&msg, system_state.canister_version())
@@ -784,7 +784,7 @@ impl SandboxSafeSystemState {
         // slots across any queue to a subnet explicitly, the bitcoin canisters or
         // IC_00 itself.
         let mut ic00_aliases: BTreeSet<CanisterId> = network_topology
-            .subnets
+            .subnets()
             .keys()
             .map(|id| CanisterId::unchecked_from_principal(id.get()))
             .collect();
@@ -1410,7 +1410,7 @@ impl SandboxSafeSystemState {
     pub fn get_root_key(&self) -> Vec<u8> {
         let root_subnet_id = self.network_topology.nns_subnet_id;
         self.network_topology
-            .subnets
+            .subnets()
             .get(&root_subnet_id)
             .map(|subnet_topology| subnet_topology.public_key.clone())
             .unwrap_or(IC_ROOT_KEY.to_vec())
@@ -1439,7 +1439,7 @@ impl SandboxSafeSystemState {
                     // unwraps: we got the subnet_id from the same collection
                     self.network_topology.get_subnet_size(subnet_id).unwrap(),
                     self.network_topology
-                        .subnets
+                        .subnets()
                         .get(subnet_id)
                         .unwrap()
                         .cost_schedule,

rs/embedders/tests/common/mod.rs

@@ -1,4 +1,4 @@
-use std::{convert::TryFrom, rc::Rc, sync::Arc};
+use std::rc::Rc;
 
 use ic_base_types::{CanisterId, NumBytes, SubnetId};
 use ic_config::{embedders::Config as EmbeddersConfig, subnet_config::SchedulerConfig};
@@ -12,8 +12,9 @@ use ic_interfaces::execution_environment::{
 };
 use ic_logger::replica_logger::no_op_logger;
 use ic_nns_constants::CYCLES_MINTING_CANISTER_ID;
-use ic_registry_routing_table::{CanisterIdRange, RoutingTable};
+use ic_registry_routing_table::CanisterIdRange;
 use ic_registry_subnet_type::SubnetType;
+use ic_replicated_state::metadata_state::testing::NetworkTopologyTesting;
 use ic_replicated_state::testing::SystemStateTesting;
 use ic_replicated_state::{
     CallOrigin, Memory, NetworkTopology, NumWasmPages, SubnetTopology, SystemState,
@@ -29,7 +30,6 @@ use ic_types::{
     methods::SystemMethod,
     time::UNIX_EPOCH,
 };
-use maplit::btreemap;
 use std::collections::BTreeMap;
 
 pub const CANISTER_CURRENT_MEMORY_USAGE: NumBytes = NumBytes::new(0);
@@ -54,19 +54,24 @@ pub fn execution_parameters(execution_mode: ExecutionMode) -> ExecutionParameter
 }
 
 fn make_network_topology(own_subnet_id: SubnetId, own_subnet_type: SubnetType) -> NetworkTopology {
-    let routing_table = Arc::new(RoutingTable::try_from(btreemap! {
-            CanisterIdRange{ start: CanisterId::from(0), end: CanisterId::from(0xff) } => own_subnet_id,
-        }).unwrap());
-    NetworkTopology {
-        routing_table,
-        subnets: btreemap! {
-            own_subnet_id => SubnetTopology {
-                subnet_type: own_subnet_type,
-                ..SubnetTopology::default()
-            }
+    let mut topo = NetworkTopology::default();
+    topo.routing_table_mut()
+        .insert(
+            CanisterIdRange {
+                start: CanisterId::from(0),
+                end: CanisterId::from(0xff),
+            },
+            own_subnet_id,
+        )
+        .unwrap();
+    topo.subnets_mut().insert(
+        own_subnet_id,
+        SubnetTopology {
+            subnet_type: own_subnet_type,
+            ..SubnetTopology::default()
         },
-        ..NetworkTopology::default()
-    }
+    );
+    topo
 }
 
 // Not used in all test crates

rs/embedders/tests/sandbox_safe_system_state.rs

@@ -13,6 +13,7 @@ use ic_nns_constants::CYCLES_MINTING_CANISTER_ID;
 use ic_registry_routing_table::CanisterIdRange;
 use ic_registry_subnet_type::SubnetType;
 use ic_replicated_state::canister_state::system_state::CyclesUseCase;
+use ic_replicated_state::metadata_state::testing::NetworkTopologyTesting;
 use ic_replicated_state::testing::SystemStateTesting;
 use ic_replicated_state::{NetworkTopology, SystemState};
 use ic_test_utilities::cycles_account_manager::CyclesAccountManagerBuilder;
@@ -30,7 +31,6 @@ use ic_types::{ComputeAllocation, Cycles, NumInstructions};
 use prometheus::IntCounter;
 use std::collections::BTreeSet;
 use std::convert::From;
-use std::sync::Arc;
 
 mod common;
 use common::*;
@@ -635,24 +635,23 @@ fn two_subnet_topology(
     test_subnet_id: SubnetId,
     test_canister_id: CanisterId,
 ) -> NetworkTopology {
-    let mut topo = NetworkTopology {
-        nns_subnet_id,
-        ..Default::default()
-    };
-    topo.subnets.insert(nns_subnet_id, Default::default());
-    topo.subnets.insert(test_subnet_id, Default::default());
+    let mut topo = NetworkTopology::default();
+    topo.nns_subnet_id = nns_subnet_id;
+    topo.subnets_mut().insert(nns_subnet_id, Default::default());
+    topo.subnets_mut()
+        .insert(test_subnet_id, Default::default());
     let nns_canister_range = CanisterIdRange {
         start: nns_canister_id,
         end: nns_canister_id,
     };
-    Arc::make_mut(&mut topo.routing_table)
+    topo.routing_table_mut()
         .insert(nns_canister_range, nns_subnet_id)
         .unwrap();
     let test_canister_range = CanisterIdRange {
         start: test_canister_id,
         end: test_canister_id,
     };
-    Arc::make_mut(&mut topo.routing_table)
+    topo.routing_table_mut()
         .insert(test_canister_range, test_subnet_id)
         .unwrap();
     topo

rs/execution_environment/src/canister_manager/tests.rs

@@ -58,7 +58,9 @@ use ic_replicated_state::{
         CyclesUseCase,
         wasm_chunk_store::{self, ChunkValidationResult},
     },
-    metadata_state::subnet_call_context_manager::InstallCodeCallId,
+    metadata_state::{
+        subnet_call_context_manager::InstallCodeCallId, testing::NetworkTopologyTesting,
+    },
     page_map::TestPageAllocatorFileDescriptorImpl,
     testing::{CanisterQueuesTesting, SystemStateTesting},
 };
@@ -340,18 +342,18 @@ fn canister_manager_config(
 fn initial_state(subnet_id: SubnetId, use_specified_ids_routing_table: bool) -> ReplicatedState {
     let mut state = ReplicatedState::new(subnet_id, SubnetType::Application);
 
-    state.metadata.network_topology.routing_table = if use_specified_ids_routing_table {
-        let routing_table =
-            get_routing_table_with_specified_ids_allocation_range(subnet_id).unwrap();
-        Arc::new(routing_table)
+    let routing_table = if use_specified_ids_routing_table {
+        get_routing_table_with_specified_ids_allocation_range(subnet_id).unwrap()
     } else {
-        Arc::new(
-            RoutingTable::try_from(btreemap! {
-                CanisterIdRange{ start: CanisterId::from(0), end: CanisterId::from(CANISTER_IDS_PER_SUBNET - 1) } => subnet_id,
-            })
-            .unwrap(),
-        )
+        RoutingTable::try_from(btreemap! {
+            CanisterIdRange{ start: CanisterId::from(0), end: CanisterId::from(CANISTER_IDS_PER_SUBNET - 1) } => subnet_id,
+        })
+        .unwrap()
     };
+    state
+        .metadata
+        .network_topology
+        .set_routing_table(routing_table);
 
     state.metadata.network_topology.nns_subnet_id = subnet_id;
     state.metadata.init_allocation_ranges_if_empty().unwrap();