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..189361311e10 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,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 diff --git a/rs/embedders/tests/sandbox_safe_system_state.rs b/rs/embedders/tests/sandbox_safe_system_state.rs index a8c0056b2e33..2cb022bb9a1a 100644 --- a/rs/embedders/tests/sandbox_safe_system_state.rs +++ b/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 diff --git a/rs/execution_environment/src/canister_manager/tests.rs b/rs/execution_environment/src/canister_manager/tests.rs index cfed3938b334..258416ef5f9e 100644 --- a/rs/execution_environment/src/canister_manager/tests.rs +++ b/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(); diff --git a/rs/execution_environment/src/execution/install_code/tests.rs b/rs/execution_environment/src/execution/install_code/tests.rs index b0fc0a631df5..1206c738b82a 100644 --- a/rs/execution_environment/src/execution/install_code/tests.rs +++ b/rs/execution_environment/src/execution/install_code/tests.rs @@ -8,10 +8,13 @@ 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 +30,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 +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). - 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 +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. - 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 +1273,10 @@ 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..91e0a3fc9460 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,9 @@ 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..1f3d6f7a3b92 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,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). - 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 +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. - 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 +1715,10 @@ 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..43ca89ed7936 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,26 +835,30 @@ 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( - 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.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; @@ -867,7 +872,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..655af5709519 100644 --- a/rs/execution_environment/src/scheduler/tests.rs +++ b/rs/execution_environment/src/scheduler/tests.rs @@ -20,10 +20,12 @@ 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 +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))); - 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 +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))); - 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 +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). - 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 +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. - 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..74610cc05d52 100644 --- a/rs/messaging/src/routing/stream_builder/tests.rs +++ b/rs/messaging/src/routing/stream_builder/tests.rs @@ -8,10 +8,11 @@ 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 +167,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 +286,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 +316,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 +376,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 +465,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 +632,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 +640,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 +726,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 +866,16 @@ fn build_streams_with_refunds( ); // Set the type of both subnets. - provided_state.metadata.network_topology.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.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 +1164,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 +1275,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 +1386,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..a8a84fbfbb21 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,19 @@ 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..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}; +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 +26,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 +132,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 +171,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 +185,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 +295,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..5994f96a05dd 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,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.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..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}, + metadata_state::{ + ApiBoundaryNodeEntry, Stream, SubnetMetrics, testing::NetworkTopologyTesting, + }, page_map::{PAGE_SIZE, PageIndex}, testing::{ReplicatedStateTesting, StreamTesting}, }; @@ -321,11 +323,14 @@ 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..d301f0b561c8 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,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.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..d0394d3248ca 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,32 @@ pub fn generate_network_topology( routing_table: Option, own_subnet_cost_schedule: CanisterCyclesCostSchedule, ) -> NetworkTopology { - NetworkTopology { + let mut topo = NetworkTopology::default(); + topo.nns_subnet_id = nns_subnet_id; + topo.set_subnets(generate_subnets( + subnets, 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() + 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 +2007,7 @@ pub struct ExecutionTestBuilder { replica_version: ReplicaVersion, precompiled_universal_canister: bool, cost_schedule: CanisterCyclesCostSchedule, + network_topology: Option, } impl Default for ExecutionTestBuilder { @@ -2028,6 +2043,7 @@ impl Default for ExecutionTestBuilder { replica_version: ReplicaVersion::default(), precompiled_universal_canister: true, cost_schedule: CanisterCyclesCostSchedule::Normal, + network_topology: None, } } } @@ -2341,7 +2357,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 +2484,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 +2503,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 +2597,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