From 4641decd31ca47fffafde2f39e1b26384fa7c189 Mon Sep 17 00:00:00 2001 From: Stefan Schneider Date: Thu, 12 Feb 2026 12:05:59 +0000 Subject: [PATCH 1/2] refactor: Encapsulate NetworkTopology This commit makes NetworkTopology::subnets and NetworkTopology::routing_table private and provides a public getter as well as a test-only setter and mutable getter. The goal is to encapsulate these fields with an eye of supporting engines, where depending on the call site engines have to be handled differently. Encapsulation makes it easier to keep an eye on all relevant call sites and decisions wrt engines. Other fields in the network topology remain public for simplicity. They can be encapsulated in a similar fashion when a need arises. --- .../src/lazy_tree_conversion.rs | 6 +- rs/canonical_state/src/traversal.rs | 15 ++-- .../wasmtime_embedder/system_api/routing.rs | 40 +++++----- .../system_api/sandbox_safe_system_state.rs | 8 +- rs/embedders/tests/common/mod.rs | 32 ++++---- .../tests/sandbox_safe_system_state.rs | 16 ++-- .../src/canister_manager/tests.rs | 22 +++--- .../src/execution/install_code/tests.rs | 22 +++--- .../src/execution_environment.rs | 8 +- .../src/execution_environment/tests.rs | 10 +-- .../src/scheduler/test_utilities.rs | 17 ++--- .../src/scheduler/tests.rs | 17 +++-- rs/messaging/src/message_routing.rs | 14 ++-- rs/messaging/src/message_routing/tests.rs | 10 +-- rs/messaging/src/routing/stream_builder.rs | 2 +- .../src/routing/stream_builder/tests.rs | 51 ++++++++----- .../src/routing/stream_handler/tests.rs | 35 +++++---- rs/messaging/src/state_machine/tests.rs | 41 +++++----- rs/replicated_state/src/metadata_state.rs | 62 ++++++++++++++- rs/replicated_state/src/replicated_state.rs | 8 +- rs/replicated_state/tests/replicated_state.rs | 31 ++++---- rs/state_manager/src/tree_hash.rs | 8 +- rs/state_manager/tests/state_manager.rs | 22 +++--- .../execution_environment/src/lib.rs | 76 +++++++++++-------- rs/test_utilities/state/src/lib.rs | 15 +++- rs/xnet/payload_builder/src/lib.rs | 2 +- 26 files changed, 337 insertions(+), 253 deletions(-) diff --git a/rs/canonical_state/src/lazy_tree_conversion.rs b/rs/canonical_state/src/lazy_tree_conversion.rs index 99bf968b9469..5d9cd7079035 100644 --- a/rs/canonical_state/src/lazy_tree_conversion.rs +++ b/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, ) diff --git a/rs/canonical_state/src/traversal.rs b/rs/canonical_state/src/traversal.rs index e2e3f605dcc4..d2824bd43ebd 100644 --- a/rs/canonical_state/src/traversal.rs +++ b/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), diff --git a/rs/embedders/src/wasmtime_embedder/system_api/routing.rs b/rs/embedders/src/wasmtime_embedder/system_api/routing.rs index d327c7193f74..8035a436ebf3 100644 --- a/rs/embedders/src/wasmtime_embedder/system_api/routing.rs +++ b/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 { diff --git a/rs/embedders/src/wasmtime_embedder/system_api/sandbox_safe_system_state.rs b/rs/embedders/src/wasmtime_embedder/system_api/sandbox_safe_system_state.rs index 7e55715f04a3..385a6a4836ad 100644 --- a/rs/embedders/src/wasmtime_embedder/system_api/sandbox_safe_system_state.rs +++ b/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 = - 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 = 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 { 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, diff --git a/rs/embedders/tests/common/mod.rs b/rs/embedders/tests/common/mod.rs index a9ee8ff541af..28ac116c0b2d 100644 --- a/rs/embedders/tests/common/mod.rs +++ b/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,21 @@ 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 diff --git a/rs/embedders/tests/sandbox_safe_system_state.rs b/rs/embedders/tests/sandbox_safe_system_state.rs index a8c0056b2e33..51ab3e558f62 100644 --- a/rs/embedders/tests/sandbox_safe_system_state.rs +++ b/rs/embedders/tests/sandbox_safe_system_state.rs @@ -14,6 +14,7 @@ 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::testing::SystemStateTesting; +use ic_replicated_state::metadata_state::testing::NetworkTopologyTesting; use ic_replicated_state::{NetworkTopology, SystemState}; use ic_test_utilities::cycles_account_manager::CyclesAccountManagerBuilder; use ic_test_utilities_state::SystemStateBuilder; @@ -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,22 @@ 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 diff --git a/rs/execution_environment/src/canister_manager/tests.rs b/rs/execution_environment/src/canister_manager/tests.rs index cfed3938b334..9a4cce4d556d 100644 --- a/rs/execution_environment/src/canister_manager/tests.rs +++ b/rs/execution_environment/src/canister_manager/tests.rs @@ -58,7 +58,10 @@ 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 +343,15 @@ 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(); diff --git a/rs/execution_environment/src/execution/install_code/tests.rs b/rs/execution_environment/src/execution/install_code/tests.rs index b0fc0a631df5..2fef0616be11 100644 --- a/rs/execution_environment/src/execution/install_code/tests.rs +++ b/rs/execution_environment/src/execution/install_code/tests.rs @@ -8,10 +8,15 @@ use ic_management_canister_types_private::{ InstallCodeArgs, InstallCodeArgsV2, Method, Payload, UploadChunkArgs, UploadChunkReply, }; use ic_registry_routing_table::{CanisterIdRange, RoutingTable}; -use ic_replicated_state::canister_state::NextExecution; -use ic_replicated_state::canister_state::execution_state::WasmExecutionMode; -use ic_replicated_state::canister_state::system_state::wasm_chunk_store; -use ic_replicated_state::{ExecutionTask, ReplicatedState}; +use ic_replicated_state::{ + ExecutionTask, ReplicatedState, + canister_state::{ + NextExecution, + execution_state::WasmExecutionMode, + system_state::wasm_chunk_store, + }, + metadata_state::testing::NetworkTopologyTesting, +}; use ic_test_utilities_execution_environment::{ ExecutionTest, ExecutionTestBuilder, check_ingress_status, get_reply, }; @@ -27,7 +32,6 @@ use ic_universal_canister::{UNIVERSAL_CANISTER_WASM, call_args, wasm}; use maplit::btreemap; use more_asserts::assert_le; use std::mem::size_of; -use std::sync::Arc; const WASM_EXECUTION_MODE: WasmExecutionMode = WasmExecutionMode::Wasm32; @@ -1179,9 +1183,9 @@ fn subnet_split_cleans_in_progress_install_code_calls() { assert_ne!(own_subnet_id, other_subnet_id); // A no-op subnet split (no canisters migrated). - Arc::make_mut(&mut test.state_mut().metadata.network_topology.routing_table) + test.state_mut().metadata.network_topology.routing_table_mut() .assign_canister(canister_id_1, own_subnet_id); - Arc::make_mut(&mut test.state_mut().metadata.network_topology.routing_table) + test.state_mut().metadata.network_topology.routing_table_mut() .assign_canister(canister_id_2, own_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); @@ -1196,7 +1200,7 @@ fn subnet_split_cleans_in_progress_install_code_calls() { assert!(!test.state().subnet_queues().has_output()); // Simulate a subnet split that migrates canister 1 to another subnet. - Arc::make_mut(&mut test.state_mut().metadata.network_topology.routing_table) + test.state_mut().metadata.network_topology.routing_table_mut() .assign_canister(canister_id_1, other_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); @@ -1262,7 +1266,7 @@ fn subnet_split_cleans_in_progress_install_code_calls() { ); // Simulate a subnet split that migrates canister 2 to another subnet. - Arc::make_mut(&mut test.state_mut().metadata.network_topology.routing_table) + test.state_mut().metadata.network_topology.routing_table_mut() .assign_canister(canister_id_2, other_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); diff --git a/rs/execution_environment/src/execution_environment.rs b/rs/execution_environment/src/execution_environment.rs index d2424d806a37..241859edba6f 100644 --- a/rs/execution_environment/src/execution_environment.rs +++ b/rs/execution_environment/src/execution_environment.rs @@ -3441,10 +3441,10 @@ impl ExecutionEnvironment { } } - let topology = &state.metadata.network_topology; // If the request isn't from the NNS, then we need to charge for it. - let source_subnet = topology.route(request.sender.get()); - if source_subnet != Some(state.metadata.network_topology.nns_subnet_id) { + let source_subnet = state.metadata.network_topology.route(request.sender.get()); + let nns_subnet_id = state.metadata.network_topology.nns_subnet_id; + if source_subnet != Some(nns_subnet_id) { let cost_schedule = state.get_own_cost_schedule(); let signature_fee = self.calculate_signature_fee(&args, subnet_size, cost_schedule); if request.payment < signature_fee { @@ -3477,7 +3477,7 @@ impl ExecutionEnvironment { let threshold_key = args.key_id(); // Check if the key is enabled. - if !topology + if !state.metadata.network_topology .chain_key_enabled_subnets(&threshold_key) .contains(&state.metadata.own_subnet_id) { diff --git a/rs/execution_environment/src/execution_environment/tests.rs b/rs/execution_environment/src/execution_environment/tests.rs index d1588ab8efa3..896fd2357365 100644 --- a/rs/execution_environment/src/execution_environment/tests.rs +++ b/rs/execution_environment/src/execution_environment/tests.rs @@ -19,6 +19,7 @@ use ic_replicated_state::{ canister_state::{ DEFAULT_QUEUE_CAPACITY, WASM_PAGE_SIZE_IN_BYTES, system_state::CyclesUseCase, }, + metadata_state::testing::NetworkTopologyTesting, testing::{CanisterQueuesTesting, SystemStateTesting}, }; use ic_test_utilities::assert_utils::assert_balance_equals; @@ -44,7 +45,6 @@ use ic_universal_canister::{CallArgs, UNIVERSAL_CANISTER_WASM, call_args, wasm}; use maplit::btreemap; use more_asserts::{assert_ge, assert_gt, assert_le, assert_lt}; use std::mem::size_of; -use std::sync::Arc; #[cfg(test)] mod canister_task; @@ -1634,9 +1634,9 @@ fn subnet_split_cleans_in_progress_stop_canister_calls() { assert_ne!(own_subnet_id, other_subnet_id); // A no-op subnet split (no canisters migrated). - Arc::make_mut(&mut test.state_mut().metadata.network_topology.routing_table) + test.state_mut().metadata.network_topology.routing_table_mut() .assign_canister(canister_id_1, own_subnet_id); - Arc::make_mut(&mut test.state_mut().metadata.network_topology.routing_table) + test.state_mut().metadata.network_topology.routing_table_mut() .assign_canister(canister_id_2, own_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); @@ -1651,7 +1651,7 @@ fn subnet_split_cleans_in_progress_stop_canister_calls() { assert!(!test.state().subnet_queues().has_output()); // Simulate a subnet split that migrates canister 1 to another subnet. - Arc::make_mut(&mut test.state_mut().metadata.network_topology.routing_table) + test.state_mut().metadata.network_topology.routing_table_mut() .assign_canister(canister_id_1, other_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); @@ -1706,7 +1706,7 @@ fn subnet_split_cleans_in_progress_stop_canister_calls() { ); // Simulate a subnet split that migrates canister 2 to another subnet. - Arc::make_mut(&mut test.state_mut().metadata.network_topology.routing_table) + test.state_mut().metadata.network_topology.routing_table_mut() .assign_canister(canister_id_2, other_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); diff --git a/rs/execution_environment/src/scheduler/test_utilities.rs b/rs/execution_environment/src/scheduler/test_utilities.rs index 3ea7a8323aee..c5eea2e20843 100644 --- a/rs/execution_environment/src/scheduler/test_utilities.rs +++ b/rs/execution_environment/src/scheduler/test_utilities.rs @@ -40,6 +40,7 @@ use ic_registry_subnet_type::SubnetType; use ic_replicated_state::{ CanisterState, ExecutionState, ExportedFunctions, InputQueueType, Memory, ReplicatedState, canister_state::execution_state::{self, WasmExecutionMode, WasmMetadata}, + metadata_state::testing::NetworkTopologyTesting, page_map::TestPageAllocatorFileDescriptorImpl, testing::{CanisterQueuesTesting, ReplicatedStateTesting}, }; @@ -834,17 +835,15 @@ impl SchedulerTestBuilder { pub fn build(self) -> SchedulerTest { let first_xnet_canister = u64::MAX / 2; - let routing_table = Arc::new( - RoutingTable::try_from(btreemap! { - CanisterIdRange { start: CanisterId::from(0x0), end: CanisterId::from(first_xnet_canister) } => self.own_subnet_id, - }).unwrap() - ); + let routing_table = RoutingTable::try_from(btreemap! { + CanisterIdRange { start: CanisterId::from(0x0), end: CanisterId::from(first_xnet_canister) } => self.own_subnet_id, + }).unwrap(); let mut state = ReplicatedState::new(self.own_subnet_id, self.subnet_type); let mut registry_settings = self.registry_settings; - state.metadata.network_topology.subnets = generate_subnets( + state.metadata.network_topology.set_subnets(generate_subnets( vec![self.own_subnet_id, self.nns_subnet_id], self.nns_subnet_id, None, @@ -852,8 +851,8 @@ impl SchedulerTestBuilder { self.subnet_type, registry_settings.subnet_size, self.cost_schedule, - ); - state.metadata.network_topology.routing_table = routing_table; + )); + state.metadata.network_topology.set_routing_table(routing_table); state.metadata.network_topology.nns_subnet_id = self.nns_subnet_id; state.metadata.batch_time = self.batch_time; @@ -867,7 +866,7 @@ impl SchedulerTestBuilder { state .metadata .network_topology - .subnets + .subnets_mut() .get_mut(&self.own_subnet_id) .unwrap() .chain_keys_held diff --git a/rs/execution_environment/src/scheduler/tests.rs b/rs/execution_environment/src/scheduler/tests.rs index 4dba50a4b0ff..31621e98b83a 100644 --- a/rs/execution_environment/src/scheduler/tests.rs +++ b/rs/execution_environment/src/scheduler/tests.rs @@ -20,10 +20,13 @@ use ic_management_canister_types_private::{ }; use ic_registry_routing_table::CanisterIdRange; use ic_registry_subnet_type::SubnetType; -use ic_replicated_state::testing::{CanisterQueuesTesting, SystemStateTesting}; use ic_replicated_state::{ canister_state::system_state::{CyclesUseCase, PausedExecutionId}, - metadata_state::subnet_call_context_manager::EcdsaMatchedPreSignature, + metadata_state::{ + subnet_call_context_manager::EcdsaMatchedPreSignature, + testing::NetworkTopologyTesting, + }, + testing::{CanisterQueuesTesting, SystemStateTesting}, }; use ic_state_machine_tests::{PayloadBuilder, StateMachineBuilder}; use ic_test_utilities_consensus::idkg::{key_transcript_for_tests, pre_signature_for_tests}; @@ -3587,8 +3590,7 @@ fn replicated_state_metrics_all_canisters_in_routing_table() { state.put_canister_state(get_running_canister(canister_test_id(1))); state.put_canister_state(get_running_canister(canister_test_id(2))); - let routing_table = Arc::make_mut(&mut state.metadata.network_topology.routing_table); - routing_table + state.metadata.network_topology.routing_table_mut() .insert( CanisterIdRange { start: canister_test_id(0), @@ -3660,8 +3662,7 @@ fn replicated_state_metrics_some_canisters_not_in_routing_table() { state.put_canister_state(get_running_canister(canister_test_id(2))); state.put_canister_state(get_running_canister(canister_test_id(100))); - let routing_table = Arc::make_mut(&mut state.metadata.network_topology.routing_table); - routing_table + state.metadata.network_topology.routing_table_mut() .insert( CanisterIdRange { start: canister_test_id(0), @@ -6286,7 +6287,7 @@ fn subnet_split_cleans_in_progress_raw_rand_requests() { assert_ne!(own_subnet_id, other_subnet_id); // A no-op subnet split (no canisters migrated). - Arc::make_mut(&mut test.state_mut().metadata.network_topology.routing_table) + test.state_mut().metadata.network_topology.routing_table_mut() .assign_canister(canister_id, own_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); @@ -6302,7 +6303,7 @@ fn subnet_split_cleans_in_progress_raw_rand_requests() { assert!(!test.state().subnet_queues().has_output()); // Simulate a subnet split that migrates the canister to another subnet. - Arc::make_mut(&mut test.state_mut().metadata.network_topology.routing_table) + test.state_mut().metadata.network_topology.routing_table_mut() .assign_canister(canister_id, other_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); diff --git a/rs/messaging/src/message_routing.rs b/rs/messaging/src/message_routing.rs index 14e54c991415..54cc0c59ac4c 100644 --- a/rs/messaging/src/message_routing.rs +++ b/rs/messaging/src/message_routing.rs @@ -934,7 +934,7 @@ impl BatchProcessorImpl { .set(subnet_record.max_block_payload_size as i64); self.metrics .canister_ranges_count - .set(network_topology.routing_table.ranges(own_subnet_id).len() as i64); + .set(network_topology.routing_table().ranges(own_subnet_id).len() as i64); // Please export any new features via the `subnet_features` metric below. let SubnetFeatures { canister_sandboxing, @@ -1108,15 +1108,15 @@ impl BatchProcessorImpl { .map_err(|err| registry_error("chain key signing subnets", None, err))? .unwrap_or_default(); - Ok(NetworkTopology { + Ok(NetworkTopology::new( subnets, - routing_table: Arc::new(routing_table), + Arc::new(routing_table), + Arc::new(canister_migrations), nns_subnet_id, - canister_migrations: Arc::new(canister_migrations), chain_key_enabled_subnets, - bitcoin_testnet_canister_id: self.bitcoin_config.testnet_canister_id, - bitcoin_mainnet_canister_id: self.bitcoin_config.mainnet_canister_id, - }) + self.bitcoin_config.testnet_canister_id, + self.bitcoin_config.mainnet_canister_id, + )) } /// Tries to populate node public keys from the registry at a specific version. diff --git a/rs/messaging/src/message_routing/tests.rs b/rs/messaging/src/message_routing/tests.rs index 648576b7e6d2..c97b98a1fec6 100644 --- a/rs/messaging/src/message_routing/tests.rs +++ b/rs/messaging/src/message_routing/tests.rs @@ -891,12 +891,12 @@ fn try_read_registry_succeeds_with_fully_specified_registry_records() { assert_eq!(metrics.critical_error_missing_subnet_size.get(), 0); // Check network topology. - assert_eq!(network_topology.subnets.len(), 2); + assert_eq!(network_topology.subnets().len(), 2); for (subnet_id, subnet_record, transcript) in [ (own_subnet_id, &own_subnet_record, &own_transcript), (other_subnet_id, &other_subnet_record, &other_transcript), ] { - let subnet_topology = network_topology.subnets.get(&subnet_id).unwrap(); + let subnet_topology = network_topology.subnets().get(&subnet_id).unwrap(); assert_eq!( ic_crypto_utils_threshold_sig_der::public_key_to_der( &ThresholdSigPublicKey::try_from(transcript) @@ -935,7 +935,7 @@ fn try_read_registry_succeeds_with_fully_specified_registry_records() { .map(|(key, val)| (key.clone(), Valid(val.clone()))) .collect::>() ); - assert_eq!(routing_table, *network_topology.routing_table); + assert_eq!(&routing_table, network_topology.routing_table().as_ref()); assert_eq!(canister_migrations, *network_topology.canister_migrations); // Check registry execution settings. @@ -1017,7 +1017,7 @@ fn try_read_registry_succeeds_with_fully_specified_registry_records() { // defined above). Additionally check the `registry_execution_settings` are also passed // correctly (they are stored in the internal `Arc` of the fake state machine itself). let latest_state = state_manager.get_latest_state().take(); - assert_ne!(network_topology, latest_state.metadata.network_topology); + assert_ne!(&network_topology, &latest_state.metadata.network_topology); assert_ne!( own_subnet_features, latest_state.metadata.own_subnet_features @@ -1042,7 +1042,7 @@ fn try_read_registry_succeeds_with_fully_specified_registry_records() { replica_version: ReplicaVersion::default(), }); let latest_state = state_manager.get_latest_state().take(); - assert_eq!(network_topology, latest_state.metadata.network_topology); + assert_eq!(&network_topology, &latest_state.metadata.network_topology); assert_eq!( own_subnet_features, latest_state.metadata.own_subnet_features diff --git a/rs/messaging/src/routing/stream_builder.rs b/rs/messaging/src/routing/stream_builder.rs index eeb861d29f83..5367a59bbb8b 100644 --- a/rs/messaging/src/routing/stream_builder.rs +++ b/rs/messaging/src/routing/stream_builder.rs @@ -469,7 +469,7 @@ impl StreamBuilderImpl { && is_at_limit( &dst_stream_entry, network_topology - .subnets + .subnets() .get(&dst_subnet_id) .map_or(SubnetType::Application, |topology| topology.subnet_type), ) diff --git a/rs/messaging/src/routing/stream_builder/tests.rs b/rs/messaging/src/routing/stream_builder/tests.rs index 303663ba1110..71733fe177a6 100644 --- a/rs/messaging/src/routing/stream_builder/tests.rs +++ b/rs/messaging/src/routing/stream_builder/tests.rs @@ -8,10 +8,13 @@ use ic_limits::SYSTEM_SUBNET_STREAM_MSG_LIMIT; use ic_management_canister_types_private::Method; use ic_registry_routing_table::{CanisterIdRange, CanisterMigrations, RoutingTable}; use ic_registry_subnet_type::SubnetType; -use ic_replicated_state::testing::{ - CanisterQueuesTesting, ReplicatedStateTesting, StreamTesting, SystemStateTesting, +use ic_replicated_state::{ + CanisterState, InputQueueType, ReplicatedState, Stream, SubnetTopology, + metadata_state::testing::NetworkTopologyTesting, + testing::{ + CanisterQueuesTesting, ReplicatedStateTesting, StreamTesting, SystemStateTesting, + }, }; -use ic_replicated_state::{CanisterState, InputQueueType, ReplicatedState, Stream, SubnetTopology}; use ic_test_utilities_logger::with_test_replica_logger; use ic_test_utilities_metrics::{ MetricVec, fetch_histogram_stats, fetch_int_counter_vec, fetch_int_gauge_vec, metric_vec, @@ -166,7 +169,7 @@ fn reject_local_request() { fn build_streams_success() { with_test_replica_logger(|log| { let (stream_builder, mut provided_state, metrics_registry) = new_fixture(&log); - provided_state.metadata.network_topology.routing_table = Arc::new(RoutingTable::try_from( + provided_state.metadata.network_topology.set_routing_table(RoutingTable::try_from( btreemap! { CanisterIdRange{ start: CanisterId::from(0), end: CanisterId::from(0xfff) } => REMOTE_SUBNET, }, @@ -285,10 +288,13 @@ fn build_streams_local_canisters() { provided_state.put_canister_states(provided_canister_states); // Ensure the routing table knows about the `LOCAL_SUBNET`. - let routing_table = Arc::new(RoutingTable::try_from(btreemap! { + let routing_table = RoutingTable::try_from(btreemap! { CanisterIdRange{ start: CanisterId::from(0), end: CanisterId::from(0xfff) } => LOCAL_SUBNET, - }).unwrap()); - provided_state.metadata.network_topology.routing_table = Arc::clone(&routing_table); + }).unwrap(); + provided_state + .metadata + .network_topology + .set_routing_table(routing_table.clone()); // Set up the expected Stream from the messages. let expected_stream = Stream::new( @@ -312,7 +318,10 @@ fn build_streams_local_canisters() { streams.insert(LOCAL_SUBNET, expected_stream); }); - expected_state.metadata.network_topology.routing_table = routing_table; + expected_state + .metadata + .network_topology + .set_routing_table(routing_table.clone()); let result_state = stream_builder.build_streams(provided_state); @@ -369,7 +378,7 @@ fn build_streams_at_limit_leaves_state_untouched_impl( target_stream_size_bytes, SYSTEM_SUBNET_STREAM_MSG_LIMIT, ); - provided_state.metadata.network_topology.routing_table = Arc::new(RoutingTable::try_from( + provided_state.metadata.network_topology.set_routing_table(RoutingTable::try_from( btreemap! { CanisterIdRange{ start: CanisterId::from(0), end: CanisterId::from(0xfff) } => REMOTE_SUBNET, }, @@ -458,7 +467,7 @@ fn build_streams_respects_limits( target_stream_size_bytes, SYSTEM_SUBNET_STREAM_MSG_LIMIT, ); - provided_state.metadata.network_topology.routing_table = Arc::new(RoutingTable::try_from( + provided_state.metadata.network_topology.set_routing_table(RoutingTable::try_from( btreemap! { CanisterIdRange{ start: CanisterId::from(0), end: CanisterId::from(0xfff) } => REMOTE_SUBNET, }, @@ -625,7 +634,7 @@ fn build_streams_with_messages_targeted_to_other_subnets() { let (stream_builder, mut provided_state, metrics_registry) = new_fixture(&log); // Ensure the routing table knows about the `REMOTE_SUBNET`. - provided_state.metadata.network_topology.routing_table = Arc::new(RoutingTable::try_from( + provided_state.metadata.network_topology.set_routing_table(RoutingTable::try_from( btreemap! { CanisterIdRange{ start: CanisterId::from(0), end: CanisterId::from(0xfff) } => REMOTE_SUBNET, }, @@ -633,7 +642,7 @@ fn build_streams_with_messages_targeted_to_other_subnets() { provided_state .metadata .network_topology - .subnets + .subnets_mut() .insert(REMOTE_SUBNET, Default::default()); // Set up the provided_canister_states. @@ -719,12 +728,12 @@ fn build_streams_with_best_effort_messages_impl( let (stream_builder, mut provided_state, _) = new_fixture(&log); // Set the subnet types of the local and remote subnets. - provided_state.metadata.network_topology.subnets = btreemap! { + provided_state.metadata.network_topology.set_subnets(btreemap! { LOCAL_SUBNET => SubnetTopology {subnet_type: local_subnet_type, ..Default::default()}, REMOTE_SUBNET => SubnetTopology {subnet_type: remote_subnet_type, ..Default::default()}, - }; + }); // Ensure that the routing table knows about `LOCAL_SUBNET` and `REMOTE_SUBNET`. - provided_state.metadata.network_topology.routing_table = Arc::new(RoutingTable::try_from( + provided_state.metadata.network_topology.set_routing_table(RoutingTable::try_from( btreemap! { CanisterIdRange{ start: local_canister_id, end: local_canister_id } => LOCAL_SUBNET, CanisterIdRange{ start: remote_canister_id, end: remote_canister_id } => REMOTE_SUBNET, @@ -859,13 +868,13 @@ fn build_streams_with_refunds( ); // Set the type of both subnets. - provided_state.metadata.network_topology.subnets = btreemap! { + provided_state.metadata.network_topology.set_subnets(btreemap! { LOCAL_SUBNET => SubnetTopology {subnet_type, ..Default::default()}, REMOTE_SUBNET => SubnetTopology {subnet_type, ..Default::default()}, - }; + }); // Map local canisters to `LOCAL_SUBNET`, remote canisters to `REMOTE_SUBNET`. - provided_state.metadata.network_topology.routing_table = Arc::new(RoutingTable::try_from( + provided_state.metadata.network_topology.set_routing_table(RoutingTable::try_from( btreemap! { CanisterIdRange{ start: first_local_canister, end: last_local_canister } => LOCAL_SUBNET, CanisterIdRange{ start: first_remote_canister, end: last_remote_canister } => REMOTE_SUBNET, @@ -1154,7 +1163,7 @@ fn build_streams_with_oversized_payloads() { let (stream_builder, mut provided_state, metrics_registry) = new_fixture(&log); // Map local canister to `LOCAL_SUBNET` and remote canister to `REMOTE_SUBNET`. - provided_state.metadata.network_topology.routing_table = Arc::new( + provided_state.metadata.network_topology.set_routing_table( RoutingTable::try_from(btreemap! { CanisterIdRange{ start: local_canister, end: local_canister } => LOCAL_SUBNET, CanisterIdRange{ start: remote_canister, end: remote_canister } => REMOTE_SUBNET, @@ -1265,7 +1274,7 @@ fn test_observe_misrouted_messages_on_splitting_subnet() { // Routing table: `migrating_canister` is still hosted by the local subnet; // `migrated_canister` has already migrated from the local subnet to B. - state.metadata.network_topology.routing_table = Arc::new( + state.metadata.network_topology.set_routing_table( RoutingTable::try_from(btreemap! { CanisterIdRange{ start: local_canister, end: local_canister } => LOCAL_SUBNET, CanisterIdRange{ start: migrating_canister, end: migrating_canister } => LOCAL_SUBNET, @@ -1376,7 +1385,7 @@ fn test_observe_misrouted_messages_on_third_party_subnet() { // Routing table: `migrating_canister` is still hosted by subnet A; // `migrated_canister` has already migrated from A to B. - state.metadata.network_topology.routing_table = Arc::new( + state.metadata.network_topology.set_routing_table( RoutingTable::try_from(btreemap! { CanisterIdRange{ start: local_canister, end: local_canister } => LOCAL_SUBNET, CanisterIdRange{ start: canister_on_a, end: canister_on_a } => REMOTE_SUBNET_A, diff --git a/rs/messaging/src/routing/stream_handler/tests.rs b/rs/messaging/src/routing/stream_handler/tests.rs index 935d5740da14..c3a26bb232c6 100644 --- a/rs/messaging/src/routing/stream_handler/tests.rs +++ b/rs/messaging/src/routing/stream_handler/tests.rs @@ -9,11 +9,13 @@ use ic_interfaces::messaging::LABEL_VALUE_CANISTER_NOT_FOUND; use ic_metrics::MetricsRegistry; use ic_registry_routing_table::{CanisterIdRange, CanisterIdRanges, RoutingTable}; use ic_registry_subnet_type::SubnetType; -use ic_replicated_state::canister_state::system_state::CyclesUseCase::DroppedMessages; -use ic_replicated_state::metadata_state::StreamMap; -use ic_replicated_state::replicated_state::LABEL_VALUE_OUT_OF_MEMORY; -use ic_replicated_state::testing::{ReplicatedStateTesting, StreamTesting, SystemStateTesting}; -use ic_replicated_state::{CanisterStatus, ReplicatedState, Stream}; +use ic_replicated_state::{ + CanisterStatus, ReplicatedState, Stream, + canister_state::system_state::CyclesUseCase::DroppedMessages, + metadata_state::{StreamMap, testing::NetworkTopologyTesting}, + replicated_state::LABEL_VALUE_OUT_OF_MEMORY, + testing::{ReplicatedStateTesting, StreamTesting, SystemStateTesting}, +}; use ic_test_utilities_logger::with_test_replica_logger; use ic_test_utilities_metrics::{ HistogramStats, MetricVec, fetch_histogram_stats, fetch_histogram_vec_count, fetch_int_counter, @@ -3252,13 +3254,11 @@ fn with_test_setup_and_config( start: CanisterId::from(0x100), end: CanisterId::from(0x1ff), }; - let routing_table = Arc::new( - RoutingTable::try_from(btreemap! { - local_range => LOCAL_SUBNET, - remote_range => REMOTE_SUBNET, - }) - .unwrap(), - ); + let routing_table = RoutingTable::try_from(btreemap! { + local_range => LOCAL_SUBNET, + remote_range => REMOTE_SUBNET, + }) + .unwrap(); assert_eq!( Some((local_range, LOCAL_SUBNET)), routing_table.lookup_entry(*LOCAL_CANISTER) @@ -3268,12 +3268,15 @@ fn with_test_setup_and_config( routing_table.lookup_entry(*REMOTE_CANISTER) ); assert!(routing_table.lookup_entry(*UNKNOWN_CANISTER).is_none()); - state.metadata.network_topology.routing_table = routing_table; + state + .metadata + .network_topology + .set_routing_table(routing_table); for subnet in [LOCAL_SUBNET, REMOTE_SUBNET] { state .metadata .network_topology - .subnets + .subnets_mut() .insert(subnet, Default::default()); } @@ -3842,11 +3845,11 @@ fn complete_canister_migration( }]) .unwrap(); - let mut routing_table = (*state.metadata.network_topology.routing_table).clone(); + let mut routing_table = state.metadata.network_topology.routing_table().as_ref().clone(); routing_table .assign_ranges(canister_id_ranges, destination) .unwrap(); - state.metadata.network_topology.routing_table = Arc::new(routing_table); + state.metadata.network_topology.set_routing_table(routing_table); state } diff --git a/rs/messaging/src/state_machine/tests.rs b/rs/messaging/src/state_machine/tests.rs index 5501c1a583f9..1f4b1bccc7d1 100644 --- a/rs/messaging/src/state_machine/tests.rs +++ b/rs/messaging/src/state_machine/tests.rs @@ -8,7 +8,7 @@ use ic_metrics::MetricsRegistry; use ic_registry_routing_table::{CANISTER_IDS_PER_SUBNET, CanisterIdRange, RoutingTable}; use ic_registry_subnet_features::SubnetFeatures; use ic_registry_subnet_type::SubnetType; -use ic_replicated_state::{ReplicatedState, SubnetTopology}; +use ic_replicated_state::{ReplicatedState, SubnetTopology, metadata_state::testing::NetworkTopologyTesting}; use ic_test_utilities_execution_environment::test_registry_settings; use ic_test_utilities_logger::with_test_replica_logger; use ic_test_utilities_metrics::{fetch_int_counter_vec, nonzero_values}; @@ -24,7 +24,6 @@ use ic_types::{ use maplit::btreemap; use mockall::{Sequence, mock, predicate::*}; use std::collections::{BTreeMap, BTreeSet}; -use std::sync::Arc; use std::time::Duration; mock! { @@ -131,11 +130,9 @@ fn test_fixture(provided_batch: &Batch) -> StateMachineTestFixture { }, ); - let network_topology = NetworkTopology { - subnets, - nns_subnet_id: SUBNET_0, - ..Default::default() - }; + let mut network_topology = NetworkTopology::default(); + network_topology.nns_subnet_id = SUBNET_0; + network_topology.set_subnets(subnets); StateMachineTestFixture { scheduler, @@ -172,8 +169,8 @@ fn state_machine_populates_network_topology() { )); assert_ne!( - fixture.initial_state.metadata.network_topology, - fixture.network_topology.clone() + &fixture.initial_state.metadata.network_topology, + &fixture.network_topology ); let state = state_machine.execute_round( @@ -186,7 +183,7 @@ fn state_machine_populates_network_topology() { Default::default(), ); - assert_eq!(state.metadata.network_topology, fixture.network_topology); + assert_eq!(&state.metadata.network_topology, &fixture.network_topology); }); } @@ -296,19 +293,17 @@ fn split_fixture() -> StateMachineTestFixture { SUBNET_A => SubnetTopology::default(), SUBNET_B => SubnetTopology::default(), }; - let network_topology = NetworkTopology { - subnets, - nns_subnet_id: NNS_SUBNET_ID, - routing_table: Arc::new( - RoutingTable::try_from(btreemap! { - CANISTER_RANGE_NNS => NNS_SUBNET_ID, - CANISTER_RANGE_A => SUBNET_A, - CANISTER_RANGE_B => SUBNET_B, - }) - .unwrap(), - ), - ..Default::default() - }; + let mut network_topology = NetworkTopology::default(); + network_topology.nns_subnet_id = NNS_SUBNET_ID; + network_topology.set_subnets(subnets); + network_topology.set_routing_table( + RoutingTable::try_from(btreemap! { + CANISTER_RANGE_NNS => NNS_SUBNET_ID, + CANISTER_RANGE_A => SUBNET_A, + CANISTER_RANGE_B => SUBNET_B, + }) + .unwrap(), + ); let metrics_registry = MetricsRegistry::new(); let metrics = MessageRoutingMetrics::new(&metrics_registry); diff --git a/rs/replicated_state/src/metadata_state.rs b/rs/replicated_state/src/metadata_state.rs index 56544b99a940..9584c4537137 100644 --- a/rs/replicated_state/src/metadata_state.rs +++ b/rs/replicated_state/src/metadata_state.rs @@ -187,10 +187,10 @@ pub struct SystemMetadata { /// use. #[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)] pub struct NetworkTopology { - pub subnets: BTreeMap, + subnets: BTreeMap, #[serde(serialize_with = "ic_utils::serde_arc::serialize_arc")] #[serde(deserialize_with = "ic_utils::serde_arc::deserialize_arc")] - pub routing_table: Arc, + routing_table: Arc, #[serde(serialize_with = "ic_utils::serde_arc::serialize_arc")] #[serde(deserialize_with = "ic_utils::serde_arc::deserialize_arc")] pub canister_migrations: Arc, @@ -237,6 +237,37 @@ impl Default for NetworkTopology { } impl NetworkTopology { + #[allow(clippy::too_many_arguments)] + pub fn new( + subnets: BTreeMap, + routing_table: Arc, + canister_migrations: Arc, + nns_subnet_id: SubnetId, + chain_key_enabled_subnets: BTreeMap>, + bitcoin_testnet_canister_id: Option, + bitcoin_mainnet_canister_id: Option, + ) -> Self { + Self { + subnets, + routing_table, + canister_migrations, + nns_subnet_id, + chain_key_enabled_subnets, + bitcoin_testnet_canister_id, + bitcoin_mainnet_canister_id, + } + } + + /// Returns a reference to the subnets map. + pub fn subnets(&self) -> &BTreeMap { + &self.subnets + } + + /// Returns a reference to the routing table. + pub fn routing_table(&self) -> &Arc { + &self.routing_table + } + /// Returns a list of subnets where the chain key feature is enabled. pub fn chain_key_enabled_subnets(&self, key_id: &MasterPublicKeyId) -> &[SubnetId] { self.chain_key_enabled_subnets @@ -1862,6 +1893,33 @@ impl UnflushedCheckpointOps { pub mod testing { use super::*; + /// Exposes `NetworkTopology` internals for use in tests. + pub trait NetworkTopologyTesting { + /// Returns a mutable reference to the subnets map. + fn subnets_mut(&mut self) -> &mut BTreeMap; + /// Sets the subnets map. + fn set_subnets(&mut self, subnets: BTreeMap); + /// Returns a mutable reference to the routing table. + fn routing_table_mut(&mut self) -> &mut RoutingTable; + /// Sets the routing table. + fn set_routing_table(&mut self, routing_table: RoutingTable); + } + + impl NetworkTopologyTesting for NetworkTopology { + fn subnets_mut(&mut self) -> &mut BTreeMap { + &mut self.subnets + } + fn set_subnets(&mut self, subnets: BTreeMap) { + self.subnets = subnets; + } + fn routing_table_mut(&mut self) -> &mut RoutingTable { + Arc::make_mut(&mut self.routing_table) + } + fn set_routing_table(&mut self, routing_table: RoutingTable) { + self.routing_table = Arc::new(routing_table); + } + } + pub trait StreamTesting { /// Creates a new `Stream` with the given `messages` and `signals_end`. #[allow(clippy::new_ret_no_self)] diff --git a/rs/replicated_state/src/replicated_state.rs b/rs/replicated_state/src/replicated_state.rs index bbb374b03310..12e579d64fb2 100644 --- a/rs/replicated_state/src/replicated_state.rs +++ b/rs/replicated_state/src/replicated_state.rs @@ -678,7 +678,7 @@ impl ReplicatedState { } pub fn routing_table(&self) -> Arc { - Arc::clone(&self.metadata.network_topology.routing_table) + Arc::clone(&self.metadata.network_topology.routing_table()) } /// Returns the cost schedule of this subnet. @@ -686,7 +686,7 @@ impl ReplicatedState { let subnet_id = self.metadata.own_subnet_id; self.metadata .network_topology - .subnets + .subnets() .get(&subnet_id) .map(|x| x.cost_schedule) .unwrap_or_default() @@ -696,7 +696,7 @@ impl ReplicatedState { pub fn get_cost_schedule(&self, subnet_id: SubnetId) -> Option { self.metadata .network_topology - .subnets + .subnets() .get(&subnet_id) .map(|x| x.cost_schedule) } @@ -1543,7 +1543,7 @@ impl ReplicatedState { // All of the canisters in `canister_states` must be hosted by either // `new_subnet_id` or `other_subnet_id`. - let routing_table = metadata.network_topology.routing_table.clone(); + let routing_table = metadata.network_topology.routing_table().clone(); let lookup_subnet = |canister_id: &CanisterId| { routing_table .lookup_entry(*canister_id) diff --git a/rs/replicated_state/tests/replicated_state.rs b/rs/replicated_state/tests/replicated_state.rs index 12cce00ae6ad..7cb41258fc46 100644 --- a/rs/replicated_state/tests/replicated_state.rs +++ b/rs/replicated_state/tests/replicated_state.rs @@ -11,24 +11,23 @@ use ic_management_canister_types_private::{ }; use ic_registry_routing_table::{CANISTER_IDS_PER_SUBNET, CanisterIdRange, RoutingTable}; use ic_registry_subnet_type::SubnetType; -use ic_replicated_state::canister_snapshots::CanisterSnapshot; -use ic_replicated_state::canister_state::execution_state::{ - CustomSection, CustomSectionType, WasmMetadata, -}; -use ic_replicated_state::metadata_state::subnet_call_context_manager::{ - BitcoinGetSuccessorsContext, BitcoinSendTransactionInternalContext, InstallCodeCallId, - SubnetCallContext, -}; -use ic_replicated_state::replicated_state::testing::ReplicatedStateTesting; -use ic_replicated_state::replicated_state::{ - MemoryTaken, PeekableOutputIterator, ReplicatedStateMessageRouting, -}; -use ic_replicated_state::testing::{ - CanisterQueuesTesting, FakeDropMessageMetrics, SystemStateTesting, -}; use ic_replicated_state::{ CanisterState, ExecutionTask, IngressHistoryState, InputSource, ReplicatedState, SchedulerState, StateError, SystemState, + canister_snapshots::CanisterSnapshot, + canister_state::execution_state::{CustomSection, CustomSectionType, WasmMetadata}, + metadata_state::{ + subnet_call_context_manager::{ + BitcoinGetSuccessorsContext, BitcoinSendTransactionInternalContext, InstallCodeCallId, + SubnetCallContext, + }, + testing::NetworkTopologyTesting, + }, + replicated_state::{ + MemoryTaken, PeekableOutputIterator, ReplicatedStateMessageRouting, + testing::ReplicatedStateTesting, + }, + testing::{CanisterQueuesTesting, FakeDropMessageMetrics, SystemStateTesting}, }; use ic_test_utilities_state::{ExecutionStateBuilder, arb_replicated_state_with_output_queues}; use ic_test_utilities_types::ids::{SUBNET_1, canister_test_id, message_test_id, user_test_id}; @@ -1268,7 +1267,7 @@ fn online_split() { CanisterIdRange {start: CanisterId::from_u64(CANISTER_2_U64 + 1), end: CanisterId::from_u64(CANISTER_IDS_PER_SUBNET - 1)} => SUBNET_A, }) .unwrap(); - fixture.state.metadata.network_topology.routing_table = Arc::new(routing_table.clone()); + fixture.state.metadata.network_topology.set_routing_table(routing_table.clone()); // Stream with a couple of requests. The details don't matter, should be // retained unmodified on subnet A' only. diff --git a/rs/state_manager/src/tree_hash.rs b/rs/state_manager/src/tree_hash.rs index f603349f3ced..870ecef46739 100644 --- a/rs/state_manager/src/tree_hash.rs +++ b/rs/state_manager/src/tree_hash.rs @@ -80,7 +80,7 @@ mod tests { execution_state::{CustomSection, CustomSectionType, WasmBinary, WasmMetadata}, system_state::CyclesUseCase, }, - metadata_state::{ApiBoundaryNodeEntry, Stream, SubnetMetrics}, + metadata_state::{ApiBoundaryNodeEntry, Stream, SubnetMetrics, testing::NetworkTopologyTesting}, page_map::{PAGE_SIZE, PageIndex}, testing::{ReplicatedStateTesting, StreamTesting}, }; @@ -321,11 +321,11 @@ mod tests { }) .unwrap(); - state.metadata.network_topology.subnets = btreemap! { + state.metadata.network_topology.set_subnets(btreemap! { own_subnet_id => Default::default(), other_subnet_id => Default::default(), - }; - state.metadata.network_topology.routing_table = Arc::new(routing_table); + }); + state.metadata.network_topology.set_routing_table(routing_table); state.metadata.prev_state_hash = Some(CryptoHashOfPartialState::new(CryptoHash(vec![3, 2, 1]))); diff --git a/rs/state_manager/tests/state_manager.rs b/rs/state_manager/tests/state_manager.rs index 4f92790dd6db..a25ac7dbe7b4 100644 --- a/rs/state_manager/tests/state_manager.rs +++ b/rs/state_manager/tests/state_manager.rs @@ -20,14 +20,14 @@ use ic_metrics::MetricsRegistry; use ic_registry_routing_table::{CANISTER_IDS_PER_SUBNET, CanisterIdRange, RoutingTable}; use ic_registry_subnet_features::SubnetFeatures; use ic_registry_subnet_type::SubnetType; -use ic_replicated_state::testing::{ReplicatedStateTesting, StreamTesting, SystemStateTesting}; use ic_replicated_state::{ ExecutionState, ExportedFunctions, Memory, NetworkTopology, NumWasmPages, PageMap, ReplicatedState, Stream, SubnetTopology, canister_snapshots::CanisterSnapshot, canister_state::{execution_state::WasmBinary, system_state::wasm_chunk_store::WasmChunkStore}, - metadata_state::ApiBoundaryNodeEntry, + metadata_state::{ApiBoundaryNodeEntry, testing::NetworkTopologyTesting}, page_map::{PageIndex, Shard, StorageLayout}, + testing::{ReplicatedStateTesting, StreamTesting, SystemStateTesting}, }; use ic_state_layout::{ CANISTER_FILE, CheckpointLayout, ReadOnly, SYSTEM_METADATA_FILE, StateLayout, WASM_FILE, @@ -5552,11 +5552,9 @@ fn certified_read_can_certify_node_public_keys_since_v12() { }, ); - let network_topology = NetworkTopology { - subnets, - nns_subnet_id: subnet_test_id(42), - ..Default::default() - }; + let mut network_topology = NetworkTopology::default(); + network_topology.nns_subnet_id = subnet_test_id(42); + network_topology.set_subnets(subnets); state.metadata.network_topology = network_topology; state.metadata.node_public_keys = node_public_keys; @@ -5962,11 +5960,9 @@ fn certified_read_can_exclude_canister_ranges() { .unwrap(); } - let network_topology = NetworkTopology { - subnets, - routing_table: Arc::new(routing_table), - ..Default::default() - }; + let mut network_topology = NetworkTopology::default(); + network_topology.set_subnets(subnets); + network_topology.set_routing_table(routing_table); state.metadata.network_topology = network_topology; @@ -8143,7 +8139,7 @@ fn can_split_with_inflight_restore_snapshot() { CanisterIdRange {start: CANISTER_3, end: CanisterId::from_u64(CANISTER_IDS_PER_SUBNET - 1)} => SUBNET_A, }) .unwrap(); - state.metadata.network_topology.routing_table = Arc::new(routing_table.clone()); + state.metadata.network_topology.set_routing_table(routing_table.clone()); // Expected state after splitting. let mut expected = state.clone(); diff --git a/rs/test_utilities/execution_environment/src/lib.rs b/rs/test_utilities/execution_environment/src/lib.rs index 30d38b4bb9ba..0b169a0bc1e8 100644 --- a/rs/test_utilities/execution_environment/src/lib.rs +++ b/rs/test_utilities/execution_environment/src/lib.rs @@ -49,6 +49,7 @@ use ic_replicated_state::{ NextExecution, execution_state::SandboxMemory, execution_state::WasmExecutionMode, system_state::CyclesUseCase, }, + metadata_state::testing::NetworkTopologyTesting, page_map::{ PAGE_SIZE, PageMap, TestPageAllocatorFileDescriptorImpl, test_utils::base_only_storage_layout, @@ -170,19 +171,21 @@ pub fn generate_network_topology( routing_table: Option, own_subnet_cost_schedule: CanisterCyclesCostSchedule, ) -> NetworkTopology { - NetworkTopology { - nns_subnet_id, - subnets: generate_subnets(subnets, nns_subnet_id, None, own_subnet_id, own_subnet_type, subnet_size, own_subnet_cost_schedule), - routing_table: match routing_table { - Some(routing_table) => Arc::new(routing_table), - None => { - Arc::new(RoutingTable::try_from(btreemap! { - CanisterIdRange { start: CanisterId::from(0), end: CanisterId::from(CANISTER_IDS_PER_SUBNET - 1) } => own_subnet_id, - }).unwrap()) - } - }, - ..Default::default() + let mut topo = NetworkTopology::default(); + topo.nns_subnet_id = nns_subnet_id; + topo.set_subnets(generate_subnets(subnets, nns_subnet_id, None, own_subnet_id, own_subnet_type, subnet_size, own_subnet_cost_schedule)); + match routing_table { + Some(rt) => topo.set_routing_table(rt), + None => { + topo.routing_table_mut() + .insert( + CanisterIdRange { start: CanisterId::from(0), end: CanisterId::from(CANISTER_IDS_PER_SUBNET - 1) }, + own_subnet_id, + ) + .unwrap(); + } } + topo } pub fn test_registry_settings() -> RegistryExecutionSettings { @@ -1993,6 +1996,7 @@ pub struct ExecutionTestBuilder { replica_version: ReplicaVersion, precompiled_universal_canister: bool, cost_schedule: CanisterCyclesCostSchedule, + network_topology: Option, } impl Default for ExecutionTestBuilder { @@ -2028,6 +2032,7 @@ impl Default for ExecutionTestBuilder { replica_version: ReplicaVersion::default(), precompiled_universal_canister: true, cost_schedule: CanisterCyclesCostSchedule::Normal, + network_topology: None, } } } @@ -2341,7 +2346,7 @@ impl ExecutionTestBuilder { pub fn build_with_routing_table_for_specified_ids(self) -> ExecutionTest { let routing_table = get_routing_table_with_specified_ids_allocation_range(self.own_subnet_id).unwrap(); - self.build_common(Arc::new(routing_table)) + self.build_common(routing_table) } pub fn with_stable_memory_dirty_page_limit( @@ -2468,13 +2473,18 @@ impl ExecutionTestBuilder { self } + pub fn with_network_topology(mut self, network_topology: NetworkTopology) -> Self { + self.network_topology = Some(network_topology); + self + } + pub fn build(self) -> ExecutionTest { let own_range = CanisterIdRange { start: CanisterId::from(CANISTER_IDS_PER_SUBNET), end: CanisterId::from(2 * CANISTER_IDS_PER_SUBNET - 1), }; - let routing_table = Arc::new(match self.caller_canister_id { + let routing_table = match self.caller_canister_id { None => RoutingTable::try_from(btreemap! { CanisterIdRange { start: CanisterId::from(0), end: CanisterId::from(CANISTER_IDS_PER_SUBNET - 1) } => self.own_subnet_id, }).unwrap(), @@ -2482,27 +2492,33 @@ impl ExecutionTestBuilder { CanisterIdRange { start: caller_canister, end: caller_canister } => self.caller_subnet_id.unwrap(), own_range => self.own_subnet_id, }).unwrap_or_else(|_| panic!("Unable to create routing table - sender canister {caller_canister} is in the range {own_range:?}")), - }); + }; self.build_common(routing_table) } - fn build_common(mut self, routing_table: Arc) -> ExecutionTest { + fn build_common(mut self, routing_table: RoutingTable) -> ExecutionTest { let mut state = ReplicatedState::new(self.own_subnet_id, self.subnet_type); - let mut subnets = vec![self.own_subnet_id, self.nns_subnet_id]; - subnets.extend(self.caller_subnet_id.iter().copied()); - state.metadata.network_topology.subnets = generate_subnets( - subnets, - self.nns_subnet_id, - self.root_key, - self.own_subnet_id, - self.subnet_type, - self.registry_settings.subnet_size, - self.cost_schedule, - ); - state.metadata.network_topology.routing_table = routing_table; - state.metadata.network_topology.nns_subnet_id = self.nns_subnet_id; + if let Some(network_topology) = self.network_topology.take() { + state.metadata.network_topology = network_topology; + } else { + let mut subnets = vec![self.own_subnet_id, self.nns_subnet_id]; + subnets.extend(self.caller_subnet_id.iter().copied()); + let mut network_topology = NetworkTopology::default(); + network_topology.nns_subnet_id = self.nns_subnet_id; + network_topology.set_subnets(generate_subnets( + subnets, + self.nns_subnet_id, + self.root_key.clone(), + self.own_subnet_id, + self.subnet_type, + self.registry_settings.subnet_size, + self.cost_schedule, + )); + network_topology.set_routing_table(routing_table); + state.metadata.network_topology = network_topology; + } state.metadata.init_allocation_ranges_if_empty().unwrap(); state.metadata.bitcoin_get_successors_follow_up_responses = self.bitcoin_get_successors_follow_up_responses; @@ -2570,7 +2586,7 @@ impl ExecutionTestBuilder { state .metadata .network_topology - .subnets + .subnets_mut() .get_mut(&self.own_subnet_id) .unwrap() .chain_keys_held diff --git a/rs/test_utilities/state/src/lib.rs b/rs/test_utilities/state/src/lib.rs index 64bbe83514f2..dccc7dff9622 100644 --- a/rs/test_utilities/state/src/lib.rs +++ b/rs/test_utilities/state/src/lib.rs @@ -20,6 +20,7 @@ use ic_replicated_state::{ subnet_call_context_manager::{ BitcoinGetSuccessorsContext, BitcoinSendTransactionInternalContext, SubnetCallContext, }, + testing::NetworkTopologyTesting, }, page_map::PageMap, testing::{CanisterQueuesTesting, ReplicatedStateTesting, StreamTesting, SystemStateTesting}, @@ -143,8 +144,11 @@ impl ReplicatedStateBuilder { ) .unwrap(); - state.metadata.network_topology.routing_table = Arc::new(routing_table); - state.metadata.network_topology.subnets.insert( + state + .metadata + .network_topology + .set_routing_table(routing_table); + state.metadata.network_topology.subnets_mut().insert( self.subnet_id, SubnetTopology { public_key: vec![], @@ -757,7 +761,7 @@ pub fn get_initial_state_with_balance( state.put_canister_state(canister_state_builder.build()); } - state.metadata.network_topology.routing_table = Arc::new({ + state.metadata.network_topology.set_routing_table({ let mut rt = ic_registry_routing_table::RoutingTable::new(); rt.insert( ic_registry_routing_table::CanisterIdRange { @@ -1191,7 +1195,10 @@ fn new_replicated_state_with_output_queues( own_subnet_id, ) .unwrap(); - replicated_state.metadata.network_topology.routing_table = Arc::new(routing_table); + replicated_state + .metadata + .network_topology + .set_routing_table(routing_table); replicated_state.put_canister_states(canister_states); if let Some(subnet_queues) = subnet_queues { diff --git a/rs/xnet/payload_builder/src/lib.rs b/rs/xnet/payload_builder/src/lib.rs index 5fb38b7905d7..12c4420b836f 100644 --- a/rs/xnet/payload_builder/src/lib.rs +++ b/rs/xnet/payload_builder/src/lib.rs @@ -995,7 +995,7 @@ pub fn get_msg_limit(subnet_id: SubnetId, state: &ReplicatedState) -> Option Date: Thu, 12 Feb 2026 16:34:09 +0000 Subject: [PATCH 2/2] Automatically fixing code for linting and formatting issues --- rs/embedders/tests/common/mod.rs | 5 +++- .../tests/sandbox_safe_system_state.rs | 5 ++-- .../src/canister_manager/tests.rs | 8 +++--- .../src/execution/install_code/tests.rs | 24 ++++++++++++----- .../src/execution_environment.rs | 4 ++- .../src/execution_environment/tests.rs | 20 +++++++++++--- .../src/scheduler/test_utilities.rs | 26 ++++++++++++------- .../src/scheduler/tests.rs | 23 +++++++++++----- .../src/routing/stream_builder/tests.rs | 15 ++++++----- .../src/routing/stream_handler/tests.rs | 12 +++++++-- rs/messaging/src/state_machine/tests.rs | 4 ++- rs/replicated_state/tests/replicated_state.rs | 6 ++++- rs/state_manager/src/tree_hash.rs | 9 +++++-- rs/state_manager/tests/state_manager.rs | 5 +++- .../execution_environment/src/lib.rs | 15 +++++++++-- 15 files changed, 131 insertions(+), 50 deletions(-) diff --git a/rs/embedders/tests/common/mod.rs b/rs/embedders/tests/common/mod.rs index 28ac116c0b2d..189361311e10 100644 --- a/rs/embedders/tests/common/mod.rs +++ b/rs/embedders/tests/common/mod.rs @@ -57,7 +57,10 @@ fn make_network_topology(own_subnet_id: SubnetId, own_subnet_type: SubnetType) - let mut topo = NetworkTopology::default(); topo.routing_table_mut() .insert( - CanisterIdRange { start: CanisterId::from(0), end: CanisterId::from(0xff) }, + CanisterIdRange { + start: CanisterId::from(0), + end: CanisterId::from(0xff), + }, own_subnet_id, ) .unwrap(); diff --git a/rs/embedders/tests/sandbox_safe_system_state.rs b/rs/embedders/tests/sandbox_safe_system_state.rs index 51ab3e558f62..2cb022bb9a1a 100644 --- a/rs/embedders/tests/sandbox_safe_system_state.rs +++ b/rs/embedders/tests/sandbox_safe_system_state.rs @@ -13,8 +13,8 @@ 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::testing::SystemStateTesting; 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; use ic_test_utilities_state::SystemStateBuilder; @@ -638,7 +638,8 @@ fn two_subnet_topology( 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()); + topo.subnets_mut() + .insert(test_subnet_id, Default::default()); let nns_canister_range = CanisterIdRange { start: nns_canister_id, end: nns_canister_id, diff --git a/rs/execution_environment/src/canister_manager/tests.rs b/rs/execution_environment/src/canister_manager/tests.rs index 9a4cce4d556d..258416ef5f9e 100644 --- a/rs/execution_environment/src/canister_manager/tests.rs +++ b/rs/execution_environment/src/canister_manager/tests.rs @@ -59,8 +59,7 @@ use ic_replicated_state::{ wasm_chunk_store::{self, ChunkValidationResult}, }, metadata_state::{ - subnet_call_context_manager::InstallCodeCallId, - testing::NetworkTopologyTesting, + subnet_call_context_manager::InstallCodeCallId, testing::NetworkTopologyTesting, }, page_map::TestPageAllocatorFileDescriptorImpl, testing::{CanisterQueuesTesting, SystemStateTesting}, @@ -351,7 +350,10 @@ fn initial_state(subnet_id: SubnetId, use_specified_ids_routing_table: bool) -> }) .unwrap() }; - state.metadata.network_topology.set_routing_table(routing_table); + 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(); diff --git a/rs/execution_environment/src/execution/install_code/tests.rs b/rs/execution_environment/src/execution/install_code/tests.rs index 2fef0616be11..1206c738b82a 100644 --- a/rs/execution_environment/src/execution/install_code/tests.rs +++ b/rs/execution_environment/src/execution/install_code/tests.rs @@ -11,9 +11,7 @@ use ic_registry_routing_table::{CanisterIdRange, RoutingTable}; use ic_replicated_state::{ ExecutionTask, ReplicatedState, canister_state::{ - NextExecution, - execution_state::WasmExecutionMode, - system_state::wasm_chunk_store, + NextExecution, execution_state::WasmExecutionMode, system_state::wasm_chunk_store, }, metadata_state::testing::NetworkTopologyTesting, }; @@ -1183,9 +1181,15 @@ fn subnet_split_cleans_in_progress_install_code_calls() { assert_ne!(own_subnet_id, other_subnet_id); // A no-op subnet split (no canisters migrated). - test.state_mut().metadata.network_topology.routing_table_mut() + test.state_mut() + .metadata + .network_topology + .routing_table_mut() .assign_canister(canister_id_1, own_subnet_id); - test.state_mut().metadata.network_topology.routing_table_mut() + test.state_mut() + .metadata + .network_topology + .routing_table_mut() .assign_canister(canister_id_2, own_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); @@ -1200,7 +1204,10 @@ fn subnet_split_cleans_in_progress_install_code_calls() { assert!(!test.state().subnet_queues().has_output()); // Simulate a subnet split that migrates canister 1 to another subnet. - test.state_mut().metadata.network_topology.routing_table_mut() + test.state_mut() + .metadata + .network_topology + .routing_table_mut() .assign_canister(canister_id_1, other_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); @@ -1266,7 +1273,10 @@ fn subnet_split_cleans_in_progress_install_code_calls() { ); // Simulate a subnet split that migrates canister 2 to another subnet. - test.state_mut().metadata.network_topology.routing_table_mut() + test.state_mut() + .metadata + .network_topology + .routing_table_mut() .assign_canister(canister_id_2, other_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); diff --git a/rs/execution_environment/src/execution_environment.rs b/rs/execution_environment/src/execution_environment.rs index 241859edba6f..91e0a3fc9460 100644 --- a/rs/execution_environment/src/execution_environment.rs +++ b/rs/execution_environment/src/execution_environment.rs @@ -3477,7 +3477,9 @@ impl ExecutionEnvironment { let threshold_key = args.key_id(); // Check if the key is enabled. - if !state.metadata.network_topology + if !state + .metadata + .network_topology .chain_key_enabled_subnets(&threshold_key) .contains(&state.metadata.own_subnet_id) { diff --git a/rs/execution_environment/src/execution_environment/tests.rs b/rs/execution_environment/src/execution_environment/tests.rs index 896fd2357365..1f3d6f7a3b92 100644 --- a/rs/execution_environment/src/execution_environment/tests.rs +++ b/rs/execution_environment/src/execution_environment/tests.rs @@ -1634,9 +1634,15 @@ fn subnet_split_cleans_in_progress_stop_canister_calls() { assert_ne!(own_subnet_id, other_subnet_id); // A no-op subnet split (no canisters migrated). - test.state_mut().metadata.network_topology.routing_table_mut() + test.state_mut() + .metadata + .network_topology + .routing_table_mut() .assign_canister(canister_id_1, own_subnet_id); - test.state_mut().metadata.network_topology.routing_table_mut() + test.state_mut() + .metadata + .network_topology + .routing_table_mut() .assign_canister(canister_id_2, own_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); @@ -1651,7 +1657,10 @@ fn subnet_split_cleans_in_progress_stop_canister_calls() { assert!(!test.state().subnet_queues().has_output()); // Simulate a subnet split that migrates canister 1 to another subnet. - test.state_mut().metadata.network_topology.routing_table_mut() + test.state_mut() + .metadata + .network_topology + .routing_table_mut() .assign_canister(canister_id_1, other_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); @@ -1706,7 +1715,10 @@ fn subnet_split_cleans_in_progress_stop_canister_calls() { ); // Simulate a subnet split that migrates canister 2 to another subnet. - test.state_mut().metadata.network_topology.routing_table_mut() + test.state_mut() + .metadata + .network_topology + .routing_table_mut() .assign_canister(canister_id_2, other_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); diff --git a/rs/execution_environment/src/scheduler/test_utilities.rs b/rs/execution_environment/src/scheduler/test_utilities.rs index c5eea2e20843..43ca89ed7936 100644 --- a/rs/execution_environment/src/scheduler/test_utilities.rs +++ b/rs/execution_environment/src/scheduler/test_utilities.rs @@ -843,16 +843,22 @@ impl SchedulerTestBuilder { let mut registry_settings = self.registry_settings; - state.metadata.network_topology.set_subnets(generate_subnets( - vec![self.own_subnet_id, self.nns_subnet_id], - self.nns_subnet_id, - None, - self.own_subnet_id, - self.subnet_type, - registry_settings.subnet_size, - self.cost_schedule, - )); - state.metadata.network_topology.set_routing_table(routing_table); + state + .metadata + .network_topology + .set_subnets(generate_subnets( + vec![self.own_subnet_id, self.nns_subnet_id], + self.nns_subnet_id, + None, + self.own_subnet_id, + self.subnet_type, + registry_settings.subnet_size, + self.cost_schedule, + )); + state + .metadata + .network_topology + .set_routing_table(routing_table); state.metadata.network_topology.nns_subnet_id = self.nns_subnet_id; state.metadata.batch_time = self.batch_time; diff --git a/rs/execution_environment/src/scheduler/tests.rs b/rs/execution_environment/src/scheduler/tests.rs index 31621e98b83a..655af5709519 100644 --- a/rs/execution_environment/src/scheduler/tests.rs +++ b/rs/execution_environment/src/scheduler/tests.rs @@ -23,8 +23,7 @@ use ic_registry_subnet_type::SubnetType; use ic_replicated_state::{ canister_state::system_state::{CyclesUseCase, PausedExecutionId}, metadata_state::{ - subnet_call_context_manager::EcdsaMatchedPreSignature, - testing::NetworkTopologyTesting, + subnet_call_context_manager::EcdsaMatchedPreSignature, testing::NetworkTopologyTesting, }, testing::{CanisterQueuesTesting, SystemStateTesting}, }; @@ -3590,7 +3589,10 @@ fn replicated_state_metrics_all_canisters_in_routing_table() { state.put_canister_state(get_running_canister(canister_test_id(1))); state.put_canister_state(get_running_canister(canister_test_id(2))); - state.metadata.network_topology.routing_table_mut() + state + .metadata + .network_topology + .routing_table_mut() .insert( CanisterIdRange { start: canister_test_id(0), @@ -3662,7 +3664,10 @@ fn replicated_state_metrics_some_canisters_not_in_routing_table() { state.put_canister_state(get_running_canister(canister_test_id(2))); state.put_canister_state(get_running_canister(canister_test_id(100))); - state.metadata.network_topology.routing_table_mut() + state + .metadata + .network_topology + .routing_table_mut() .insert( CanisterIdRange { start: canister_test_id(0), @@ -6287,7 +6292,10 @@ fn subnet_split_cleans_in_progress_raw_rand_requests() { assert_ne!(own_subnet_id, other_subnet_id); // A no-op subnet split (no canisters migrated). - test.state_mut().metadata.network_topology.routing_table_mut() + test.state_mut() + .metadata + .network_topology + .routing_table_mut() .assign_canister(canister_id, own_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); @@ -6303,7 +6311,10 @@ fn subnet_split_cleans_in_progress_raw_rand_requests() { assert!(!test.state().subnet_queues().has_output()); // Simulate a subnet split that migrates the canister to another subnet. - test.state_mut().metadata.network_topology.routing_table_mut() + test.state_mut() + .metadata + .network_topology + .routing_table_mut() .assign_canister(canister_id, other_subnet_id); test.online_split_state(own_subnet_id, other_subnet_id); diff --git a/rs/messaging/src/routing/stream_builder/tests.rs b/rs/messaging/src/routing/stream_builder/tests.rs index 71733fe177a6..74610cc05d52 100644 --- a/rs/messaging/src/routing/stream_builder/tests.rs +++ b/rs/messaging/src/routing/stream_builder/tests.rs @@ -11,9 +11,7 @@ use ic_registry_subnet_type::SubnetType; use ic_replicated_state::{ CanisterState, InputQueueType, ReplicatedState, Stream, SubnetTopology, metadata_state::testing::NetworkTopologyTesting, - testing::{ - CanisterQueuesTesting, ReplicatedStateTesting, StreamTesting, SystemStateTesting, - }, + testing::{CanisterQueuesTesting, ReplicatedStateTesting, StreamTesting, SystemStateTesting}, }; use ic_test_utilities_logger::with_test_replica_logger; use ic_test_utilities_metrics::{ @@ -868,10 +866,13 @@ fn build_streams_with_refunds( ); // Set the type of both subnets. - provided_state.metadata.network_topology.set_subnets(btreemap! { - LOCAL_SUBNET => SubnetTopology {subnet_type, ..Default::default()}, - REMOTE_SUBNET => SubnetTopology {subnet_type, ..Default::default()}, - }); + provided_state + .metadata + .network_topology + .set_subnets(btreemap! { + LOCAL_SUBNET => SubnetTopology {subnet_type, ..Default::default()}, + REMOTE_SUBNET => SubnetTopology {subnet_type, ..Default::default()}, + }); // Map local canisters to `LOCAL_SUBNET`, remote canisters to `REMOTE_SUBNET`. provided_state.metadata.network_topology.set_routing_table(RoutingTable::try_from( diff --git a/rs/messaging/src/routing/stream_handler/tests.rs b/rs/messaging/src/routing/stream_handler/tests.rs index c3a26bb232c6..a8a84fbfbb21 100644 --- a/rs/messaging/src/routing/stream_handler/tests.rs +++ b/rs/messaging/src/routing/stream_handler/tests.rs @@ -3845,11 +3845,19 @@ fn complete_canister_migration( }]) .unwrap(); - let mut routing_table = state.metadata.network_topology.routing_table().as_ref().clone(); + let mut routing_table = state + .metadata + .network_topology + .routing_table() + .as_ref() + .clone(); routing_table .assign_ranges(canister_id_ranges, destination) .unwrap(); - state.metadata.network_topology.set_routing_table(routing_table); + state + .metadata + .network_topology + .set_routing_table(routing_table); state } diff --git a/rs/messaging/src/state_machine/tests.rs b/rs/messaging/src/state_machine/tests.rs index 1f4b1bccc7d1..014ef23935bd 100644 --- a/rs/messaging/src/state_machine/tests.rs +++ b/rs/messaging/src/state_machine/tests.rs @@ -8,7 +8,9 @@ use ic_metrics::MetricsRegistry; use ic_registry_routing_table::{CANISTER_IDS_PER_SUBNET, CanisterIdRange, RoutingTable}; use ic_registry_subnet_features::SubnetFeatures; use ic_registry_subnet_type::SubnetType; -use ic_replicated_state::{ReplicatedState, SubnetTopology, metadata_state::testing::NetworkTopologyTesting}; +use ic_replicated_state::{ + ReplicatedState, SubnetTopology, metadata_state::testing::NetworkTopologyTesting, +}; use ic_test_utilities_execution_environment::test_registry_settings; use ic_test_utilities_logger::with_test_replica_logger; use ic_test_utilities_metrics::{fetch_int_counter_vec, nonzero_values}; diff --git a/rs/replicated_state/tests/replicated_state.rs b/rs/replicated_state/tests/replicated_state.rs index 7cb41258fc46..5994f96a05dd 100644 --- a/rs/replicated_state/tests/replicated_state.rs +++ b/rs/replicated_state/tests/replicated_state.rs @@ -1267,7 +1267,11 @@ fn online_split() { CanisterIdRange {start: CanisterId::from_u64(CANISTER_2_U64 + 1), end: CanisterId::from_u64(CANISTER_IDS_PER_SUBNET - 1)} => SUBNET_A, }) .unwrap(); - fixture.state.metadata.network_topology.set_routing_table(routing_table.clone()); + fixture + .state + .metadata + .network_topology + .set_routing_table(routing_table.clone()); // Stream with a couple of requests. The details don't matter, should be // retained unmodified on subnet A' only. diff --git a/rs/state_manager/src/tree_hash.rs b/rs/state_manager/src/tree_hash.rs index 870ecef46739..b111553de5bf 100644 --- a/rs/state_manager/src/tree_hash.rs +++ b/rs/state_manager/src/tree_hash.rs @@ -80,7 +80,9 @@ mod tests { execution_state::{CustomSection, CustomSectionType, WasmBinary, WasmMetadata}, system_state::CyclesUseCase, }, - metadata_state::{ApiBoundaryNodeEntry, Stream, SubnetMetrics, testing::NetworkTopologyTesting}, + metadata_state::{ + ApiBoundaryNodeEntry, Stream, SubnetMetrics, testing::NetworkTopologyTesting, + }, page_map::{PAGE_SIZE, PageIndex}, testing::{ReplicatedStateTesting, StreamTesting}, }; @@ -325,7 +327,10 @@ mod tests { own_subnet_id => Default::default(), other_subnet_id => Default::default(), }); - state.metadata.network_topology.set_routing_table(routing_table); + state + .metadata + .network_topology + .set_routing_table(routing_table); state.metadata.prev_state_hash = Some(CryptoHashOfPartialState::new(CryptoHash(vec![3, 2, 1]))); diff --git a/rs/state_manager/tests/state_manager.rs b/rs/state_manager/tests/state_manager.rs index a25ac7dbe7b4..d301f0b561c8 100644 --- a/rs/state_manager/tests/state_manager.rs +++ b/rs/state_manager/tests/state_manager.rs @@ -8139,7 +8139,10 @@ fn can_split_with_inflight_restore_snapshot() { CanisterIdRange {start: CANISTER_3, end: CanisterId::from_u64(CANISTER_IDS_PER_SUBNET - 1)} => SUBNET_A, }) .unwrap(); - state.metadata.network_topology.set_routing_table(routing_table.clone()); + state + .metadata + .network_topology + .set_routing_table(routing_table.clone()); // Expected state after splitting. let mut expected = state.clone(); diff --git a/rs/test_utilities/execution_environment/src/lib.rs b/rs/test_utilities/execution_environment/src/lib.rs index 0b169a0bc1e8..d0394d3248ca 100644 --- a/rs/test_utilities/execution_environment/src/lib.rs +++ b/rs/test_utilities/execution_environment/src/lib.rs @@ -173,13 +173,24 @@ pub fn generate_network_topology( ) -> NetworkTopology { let mut topo = NetworkTopology::default(); topo.nns_subnet_id = nns_subnet_id; - topo.set_subnets(generate_subnets(subnets, nns_subnet_id, None, own_subnet_id, own_subnet_type, subnet_size, own_subnet_cost_schedule)); + topo.set_subnets(generate_subnets( + subnets, + nns_subnet_id, + None, + own_subnet_id, + own_subnet_type, + subnet_size, + own_subnet_cost_schedule, + )); match routing_table { Some(rt) => topo.set_routing_table(rt), None => { topo.routing_table_mut() .insert( - CanisterIdRange { start: CanisterId::from(0), end: CanisterId::from(CANISTER_IDS_PER_SUBNET - 1) }, + CanisterIdRange { + start: CanisterId::from(0), + end: CanisterId::from(CANISTER_IDS_PER_SUBNET - 1), + }, own_subnet_id, ) .unwrap();