diff --git a/packages/ic-management-canister-types/CHANGELOG.md b/packages/ic-management-canister-types/CHANGELOG.md index 5612358588c3..442cd5f1b474 100644 --- a/packages/ic-management-canister-types/CHANGELOG.md +++ b/packages/ic-management-canister-types/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Added the `PATCH` variant to the `HttpMethod` enum used by canister HTTPS outcalls (`http_request`). The variant is plumbed through the type but not yet enabled on replicated subnets. +- Types for `subnet_metrics`: added the types `SubnetMetricsArgs` and `SubnetMetricsResult`. ## [0.8.0] - 2026-05-13 diff --git a/packages/ic-management-canister-types/src/lib.rs b/packages/ic-management-canister-types/src/lib.rs index 0518cf7cfc78..d42343dd240e 100644 --- a/packages/ic-management-canister-types/src/lib.rs +++ b/packages/ic-management-canister-types/src/lib.rs @@ -1372,6 +1372,64 @@ pub struct SubnetInfoResult { pub registry_version: u64, } +/// # Subnet Metrics Args. +/// +/// Argument type of [`subnet_metrics`](https://docs.internetcomputer.org/references/management-canister/#subnet_metrics). +#[derive( + CandidType, Serialize, Deserialize, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, +)] +pub struct SubnetMetricsArgs { + /// Subnet ID. + pub subnet_id: Principal, +} + +/// # Subnet Metrics Result. +/// +/// Result type of [`subnet_metrics`](https://docs.internetcomputer.org/references/management-canister/#subnet_metrics). +/// +/// This API is EXPERIMENTAL and may evolve in a non-backward-compatible way. +/// +/// # Freshness +/// +/// Only `block_height` is current as of the block in which the call is executed. +/// The other four are read from the subnet's aggregated metrics, which the replica +/// updates at the *end* of a round, so they describe the state as of an earlier +/// block: +/// +/// - `num_canisters`, `update_transactions_total` and `consumed_cycles_total` are +/// as of the end of the previous round. +/// - `canister_state_bytes` is recomputed only every 10 rounds, because summing it +/// over every canister is expensive and it does not need to be exact. It can +/// therefore be up to ten rounds stale, and reads as `0` for the first rounds +/// after a subnet's first canister appears. +/// +/// These are the same values, with the same staleness, that `read_state` returns +/// for the `/subnet//metrics` path, so the two agree. +#[derive( + CandidType, Serialize, Deserialize, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, +)] +pub struct SubnetMetricsResult { + /// Current block height of the subnet, i.e. the height of the block in whose + /// execution the call is processed. Monotonically non-decreasing for a given + /// subnet; heights of different subnets are unrelated. + pub block_height: Nat, + /// Number of canisters on the subnet, as of the end of the previous round. + pub num_canisters: Nat, + /// Total size in bytes of the state taken by canisters on the subnet. + /// + /// Refreshed only every 10 rounds, so this can be up to ten rounds stale (and + /// reads as `0` for the first rounds of a subnet's life). See the type-level + /// "Freshness" note. + pub canister_state_bytes: Nat, + /// Total cycles removed from circulation on the subnet by all current and + /// deleted canisters, as of the end of the previous round. + pub consumed_cycles_total: Nat, + /// Total number of transactions processed on the subnet, i.e. the total + /// number of messages executed in replicated mode, as of the end of the + /// previous round. + pub update_transactions_total: Nat, +} + /// # Canister ID Range. /// /// A closed range of canister IDs, both endpoints inclusive. diff --git a/packages/ic-management-canister-types/tests/candid_equality.rs b/packages/ic-management-canister-types/tests/candid_equality.rs index 5fe869aa6901..6b7c13565fc2 100644 --- a/packages/ic-management-canister-types/tests/candid_equality.rs +++ b/packages/ic-management-canister-types/tests/candid_equality.rs @@ -132,6 +132,11 @@ fn node_metrics_history(_: NodeMetricsHistoryArgs) -> NodeMetricsHistoryResult { unimplemented!() } +#[candid_method(update)] +fn subnet_metrics(_: SubnetMetricsArgs) -> SubnetMetricsResult { + unimplemented!() +} + #[candid_method(update)] fn provisional_create_canister_with_cycles( _: ProvisionalCreateCanisterWithCyclesArgs, diff --git a/packages/ic-management-canister-types/tests/ic.did b/packages/ic-management-canister-types/tests/ic.did index 773adfbc6d61..43b36866bc7c 100644 --- a/packages/ic-management-canister-types/tests/ic.did +++ b/packages/ic-management-canister-types/tests/ic.did @@ -439,6 +439,25 @@ type node_metrics_history_result = vec record { node_metrics : vec node_metrics; }; +type subnet_metrics_args = record { + subnet_id : principal; +}; + +type subnet_metrics_result = record { + // Current block height of the subnet, i.e. the height of the block in + // whose execution this call is processed. + block_height : nat; + // Current number of canisters on the subnet. + num_canisters : nat; + // Current total size in bytes of the state taken by canisters on the subnet. + canister_state_bytes : nat; + // Total cycles removed from circulation on the subnet by all current and + // deleted canisters. + consumed_cycles_total : nat; + // Total number of transactions processed on the subnet. + update_transactions_total : nat; +}; + type subnet_info_args = record { subnet_id : principal; }; @@ -701,6 +720,7 @@ service ic : { // metrics interface node_metrics_history : (node_metrics_history_args) -> (node_metrics_history_result); + subnet_metrics : (subnet_metrics_args) -> (subnet_metrics_result); // subnet info subnet_info : (subnet_info_args) -> (subnet_info_result); diff --git a/rs/canonical_state/src/encoding/types.rs b/rs/canonical_state/src/encoding/types.rs index cee7d4eeaad8..bda192d6278c 100644 --- a/rs/canonical_state/src/encoding/types.rs +++ b/rs/canonical_state/src/encoding/types.rs @@ -744,7 +744,7 @@ impl // `consumed_cycles_total` (which no longer double counts deleted // canisters) plus the cycles consumed by all non-deleted canisters. let consumed_cycles_total = if certification_version >= CertificationVersion::V29 { - metrics.consumed_cycles_total() + consumed_cycles_by_canisters + metrics.consumed_cycles_total_including_canisters(consumed_cycles_by_canisters) } else { metrics.consumed_cycles_total_v28() }; diff --git a/rs/embedders/src/wasmtime_embedder/system_api/routing.rs b/rs/embedders/src/wasmtime_embedder/system_api/routing.rs index def610efd866..817ad2f2a68b 100644 --- a/rs/embedders/src/wasmtime_embedder/system_api/routing.rs +++ b/rs/embedders/src/wasmtime_embedder/system_api/routing.rs @@ -14,9 +14,9 @@ use ic_management_canister_types_private::{ NodeMetricsHistoryArgs, Payload, ProvisionalTopUpCanisterArgs, ReadCanisterSnapshotDataArgs, ReadCanisterSnapshotMetadataArgs, RenameCanisterArgs, ReshareChainKeyArgs, SchnorrPublicKeyArgs, SetupInitialDKGArgs, SignWithECDSAArgs, SignWithSchnorrArgs, - StoredChunksArgs, SubnetInfoArgs, TakeCanisterSnapshotArgs, UninstallCodeArgs, - UpdateSettingsArgs, UploadCanisterSnapshotDataArgs, UploadCanisterSnapshotMetadataArgs, - UploadChunkArgs, VetKdDeriveKeyArgs, VetKdPublicKeyArgs, + StoredChunksArgs, SubnetInfoArgs, SubnetMetricsArgs, TakeCanisterSnapshotArgs, + UninstallCodeArgs, UpdateSettingsArgs, UploadCanisterSnapshotDataArgs, + UploadCanisterSnapshotMetadataArgs, UploadChunkArgs, VetKdDeriveKeyArgs, VetKdPublicKeyArgs, }; use ic_replicated_state::NetworkTopology; use itertools::Itertools; @@ -201,6 +201,28 @@ pub(super) fn resolve_destination( Ok(Ic00Method::NodeMetricsHistory) => { Ok(NodeMetricsHistoryArgs::decode(payload)?.subnet_id) } + Ok(Ic00Method::SubnetMetrics) => { + // Rejected explicitly, mirroring `FetchCanisterLogs` below, rather than + // relying on the composite-query path failing later in + // `QueryContext::handle_request` (where `get_active_canister` cannot + // resolve a subnet principal). That indirect guarantee holds today, but + // it would evaporate the moment `subnet_metrics` were added to + // `QueryMethod`: the query path has no round-instruction accounting, so + // the `O(|hot canisters|)` fold would run unmetered on query threads + // against a different state snapshot. Keeping the rejection here makes + // that a compile-time-visible decision rather than an accident. + if is_composite_query { + Err(ResolveDestinationError::UserError(UserError::new( + ic_error_types::ErrorCode::CanisterRejectedMessage, + format!( + "{} API cannot be called from a composite query", + Ic00Method::SubnetMetrics + ), + ))) + } else { + Ok(SubnetMetricsArgs::decode(payload)?.subnet_id) + } + } Ok(Ic00Method::SubnetInfo) => Ok(SubnetInfoArgs::decode(payload)?.subnet_id), Ok(Ic00Method::FetchCanisterLogs) => { let canister_id = FetchCanisterLogsRequest::decode(payload)?.get_canister_id(); @@ -1180,4 +1202,67 @@ mod tests { }; } } + + /// `subnet_metrics` names its target subnet in the payload, so an ordinary + /// (non-composite-query) call routes there. + #[test] + fn resolve_subnet_metrics_routes_to_named_subnet() { + let logger = no_op_logger(); + let target_subnet = subnet_test_id(1); + assert_eq!( + resolve_destination( + &network_with_ecdsa_subnets(), + &Ic00Method::SubnetMetrics.to_string(), + &Encode!(&SubnetMetricsArgs { + subnet_id: target_subnet.get() + }) + .unwrap(), + subnet_test_id(2), + canister_test_id(1), + false, + &logger, + ) + .unwrap(), + target_subnet.get() + ); + } + + /// ...but a composite query is rejected outright, mirroring + /// `fetch_canister_logs`. + /// + /// The composite-query path has no round-instruction accounting, so the + /// `O(|hot canisters|)` fold that `subnet_metrics` performs must never run on + /// query threads. This is the in-process guard for that arm; the system test + /// `subnet_metrics_composite_query_fails` asserts the same thing end to end but + /// is Linux-only. + #[test] + fn resolve_subnet_metrics_rejects_composite_query() { + let logger = no_op_logger(); + let err = resolve_destination( + &network_with_ecdsa_subnets(), + &Ic00Method::SubnetMetrics.to_string(), + &Encode!(&SubnetMetricsArgs { + subnet_id: subnet_test_id(1).get() + }) + .unwrap(), + subnet_test_id(2), + canister_test_id(1), + true, + &logger, + ) + .unwrap_err(); + match err { + ResolveDestinationError::UserError(err) => { + assert_eq!( + err.code(), + ic_error_types::ErrorCode::CanisterRejectedMessage + ); + assert_eq!( + err.description(), + "subnet_metrics API cannot be called from a composite query" + ); + } + other => panic!("Unexpected error: {other:?}"), + } + } } 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 050d720e590c..db281f7d14bd 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 @@ -338,6 +338,7 @@ impl SystemStateModifications { | Ok(Ic00Method::BitcoinSendTransaction) | Ok(Ic00Method::BitcoinGetCurrentFeePercentiles) | Ok(Ic00Method::NodeMetricsHistory) + | Ok(Ic00Method::SubnetMetrics) | Ok(Ic00Method::SubnetInfo) | Ok(Ic00Method::FetchCanisterLogs) | Ok(Ic00Method::UploadChunk) diff --git a/rs/execution_environment/benches/management_canister/main.rs b/rs/execution_environment/benches/management_canister/main.rs index 403120c5f122..c2858b62de1a 100644 --- a/rs/execution_environment/benches/management_canister/main.rs +++ b/rs/execution_environment/benches/management_canister/main.rs @@ -6,6 +6,7 @@ mod ecdsa; mod http_request; mod install_code; mod list_canisters; +mod subnet_metrics; mod update_settings; mod utils; @@ -20,6 +21,7 @@ fn all_benchmarks(c: &mut Criterion) { http_request::http_request_benchmark(c); install_code::install_code_benchmark(c); list_canisters::list_canisters_benchmark(c); + subnet_metrics::subnet_metrics_benchmark(c); update_settings::update_settings_benchmark(c); } diff --git a/rs/execution_environment/benches/management_canister/subnet_metrics.rs b/rs/execution_environment/benches/management_canister/subnet_metrics.rs new file mode 100644 index 000000000000..1dc0897b9e90 --- /dev/null +++ b/rs/execution_environment/benches/management_canister/subnet_metrics.rs @@ -0,0 +1,135 @@ +use criterion::{BenchmarkGroup, Criterion, criterion_group, criterion_main}; +use ic_base_types::{NumBytes, NumSeconds}; +use ic_replicated_state::canister_state::canister_snapshots::CanisterSnapshots; +use ic_replicated_state::canister_state::system_state::SystemState; +use ic_replicated_state::{CanisterState, CanisterStates, SchedulerState}; +use ic_test_utilities_types::ids::canister_test_id; +use ic_types_cycles::{CanisterCyclesCostSchedule, CompoundCycles, Cycles, Instructions}; +use std::sync::Arc; + +/// Builds one hot canister with non-zero consumed cycles. +/// +/// A non-zero `heap_delta_debit` keeps a canister out of the cold pool +/// (`CanisterState::is_cold`), which is what makes a fully hot pool the worst case +/// for `CanisterStates::total_consumed_cycles()`: the fold is `O(|hot|)`, the cold +/// pool being a precomputed aggregate. +fn hot_canister(id: u64) -> Arc { + let mut system_state = SystemState::new_running_for_testing( + canister_test_id(id), + canister_test_id(u64::MAX).get(), + Cycles::new(1 << 60), + NumSeconds::new(100_000), + ); + system_state.consume_cycles(CompoundCycles::::new( + Cycles::new(1_000 + id as u128), + CanisterCyclesCostSchedule::Normal, + )); + Arc::new(CanisterState::new( + system_state, + None, + SchedulerState { + heap_delta_debit: NumBytes::new(1), + ..SchedulerState::default() + }, + CanisterSnapshots::default(), + )) +} + +/// Builds a `CanisterStates` holding `canisters_number` hot canisters, allocated +/// and inserted in ascending canister-ID order. +/// +/// This is the *favourable* memory layout: the `BTreeMap` nodes and the `Arc` +/// payloads are laid out in the order the fold visits them. +fn hot_canister_states(canisters_number: u64) -> CanisterStates { + let mut states = CanisterStates::default(); + for id in 0..canisters_number { + states.insert(hot_canister(id)); + } + assert_eq!(states.hot_len() as u64, canisters_number); + states +} + +/// As [`hot_canister_states`], but with the allocation and insertion order +/// scrambled and with allocator churn interleaved, so the `BTreeMap` nodes and the +/// `Arc` payloads are scattered rather than laid out in visit +/// order. +/// +/// This is the adversarial-locality variant, and it is the one +/// `INSTRUCTIONS_PER_HOT_CANISTER` is justified against: a production hot pool is +/// built up over a long period from independently allocated, long-lived canisters, +/// not in one tight loop. In practice it measures only ~13% above the favourable +/// layout, because `size_of::()` is ~2.5KB, so at 100k canisters the +/// pool is ~254MB and the fold is DRAM-bound either way. +fn shuffled_hot_canister_states(canisters_number: u64) -> CanisterStates { + /// Deterministic pseudo-random value, so the benchmark needs no RNG + /// dependency and is reproducible run to run. + fn scramble(i: u64) -> u64 { + let mut x = i.wrapping_mul(0x9E37_79B9_7F4A_7C15); + x ^= x >> 31; + x.wrapping_mul(0xBF58_476D_1CE4_E5B9) + } + + // Fisher-Yates over `0..n`, rather than rejection-sampling a scrambled index + // until every residue has been hit: this is `O(n)` with a static termination + // bound, where the rejection loop terminates only in expectation (~12n + // iterations by coupon collector, and in principle never). + let mut order: Vec = (0..canisters_number).collect(); + for i in (1..order.len()).rev() { + order.swap(i, (scramble(i as u64) % (i as u64 + 1)) as usize); + } + + let mut states = CanisterStates::default(); + let mut ballast: Vec> = Vec::new(); + for (inserted, id) in order.into_iter().enumerate() { + // Churn: allocate, keep some, free some, so canister allocations are + // interleaved with unrelated live objects. + ballast.push(vec![0_u8; 4096]); + if ballast.len() > 64 { + let victim = inserted % ballast.len(); + ballast.swap_remove(victim); + } + states.insert(hot_canister(id)); + } + // Drop the ballast, leaving holes in the heap. + drop(ballast); + assert_eq!(states.hot_len() as u64, canisters_number); + states +} + +/// Measures `CanisterStates::total_consumed_cycles()` over a fully hot pool. +/// The slope of this measurement is what `INSTRUCTIONS_PER_HOT_CANISTER` in +/// `subnet_metrics_instructions` must cover. +fn bench_consumed_cycles_fold( + group: &mut BenchmarkGroup, + bench_name: &str, + states: CanisterStates, +) { + group.bench_function(bench_name, |b| { + b.iter(|| std::hint::black_box(states.total_consumed_cycles())); + }); +} + +pub fn subnet_metrics_benchmark(c: &mut Criterion) { + let mut group = c.benchmark_group("subnet_metrics_consumed_cycles_fold"); + for n in [0_u64, 1_000, 10_000, 100_000] { + let label = match n { + 0 => "0".to_string(), + n if n % 1_000 == 0 => format!("{}k", n / 1_000), + n => n.to_string(), + }; + bench_consumed_cycles_fold( + &mut group, + &format!("hot/{label}/sequential"), + hot_canister_states(n), + ); + bench_consumed_cycles_fold( + &mut group, + &format!("hot/{label}/shuffled"), + shuffled_hot_canister_states(n), + ); + } + group.finish(); +} + +criterion_group!(benchmarks, subnet_metrics_benchmark); +criterion_main!(benchmarks); diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index 06d5c1fc8795..c03052b0979d 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -161,6 +161,10 @@ impl CanisterManager { | Ok(Ic00Method::BitcoinSendTransactionInternal) | Ok(Ic00Method::BitcoinGetCurrentFeePercentiles) | Ok(Ic00Method::NodeMetricsHistory) + // Unreachable for `SubnetMetrics`: `extract_effective_canister_id` + // rejects it earlier, at the ingress filter. Listed for exhaustiveness + // and as defence in depth. + | Ok(Ic00Method::SubnetMetrics) | Ok(Ic00Method::SubnetInfo) // `RenameCanister` can only be called from the NNS subnet. | Ok(Ic00Method::RenameCanister) => Err(UserError::new( diff --git a/rs/execution_environment/src/canister_manager/tests.rs b/rs/execution_environment/src/canister_manager/tests.rs index e1a741922ae8..cda9408262e9 100644 --- a/rs/execution_environment/src/canister_manager/tests.rs +++ b/rs/execution_environment/src/canister_manager/tests.rs @@ -45,8 +45,8 @@ use ic_management_canister_types_private::{ InstallCodeArgsV2, Method, NodeMetricsHistoryArgs, NodeMetricsHistoryResponse, OnLowWasmMemoryHookStatus, Payload, ProvisionalCreateCanisterWithCyclesArgs, RenameCanisterArgs, RenameToArgs, StoredChunksArgs, StoredChunksReply, SubnetInfoArgs, - SubnetInfoResponse, TakeCanisterSnapshotArgs, UpdateSettingsArgs, UploadChunkArgs, - UploadChunkReply, WasmMemoryPersistence, + SubnetInfoResponse, SubnetMetricsArgs, SubnetMetricsResponse, TakeCanisterSnapshotArgs, + UpdateSettingsArgs, UploadChunkArgs, UploadChunkReply, WasmMemoryPersistence, }; use ic_metrics::MetricsRegistry; use ic_registry_provisional_whitelist::ProvisionalWhitelist; @@ -6206,6 +6206,298 @@ fn subnet_info_ingress_fails() { ); } +/// Sends the given payload to `subnet_metrics` as an inter-canister call from a +/// canister on a remote subnet, executes it, and returns the decoded response or +/// the reject. +fn subnet_metrics_raw_call( + test: &mut ExecutionTest, + payload: Vec, +) -> Result { + test.inject_call_to_ic00(Method::SubnetMetrics, payload, Cycles::zero()); + test.execute_subnet_message(); + // Route the response back towards the caller (on a different subnet) so that + // it can be inspected via `xnet_messages`. + test.induct_messages(); + let index = test.xnet_messages().len() - 1; + match &test.get_xnet_response(index).response_payload { + ic_types::messages::Payload::Data(bytes) => { + Ok(Decode!(bytes, SubnetMetricsResponse).unwrap()) + } + ic_types::messages::Payload::Reject(context) => { + Err((context.code(), context.message().to_string())) + } + } +} + +/// As [`subnet_metrics_raw_call`], with a well-formed payload naming `subnet_id`. +fn subnet_metrics_call( + test: &mut ExecutionTest, + subnet_id: PrincipalId, +) -> Result { + subnet_metrics_raw_call(test, SubnetMetricsArgs { subnet_id }.encode()) +} + +#[test] +fn subnet_metrics_canister_call_succeeds() { + let own_subnet_id = subnet_test_id(1); + let mut test = ExecutionTestBuilder::new() + .with_own_subnet_id(own_subnet_id) + .build(); + let uni_canister = test + .universal_canister_with_cycles(Cycles::new(1_000_000_000_000)) + .unwrap(); + let payload = SubnetMetricsArgs { + subnet_id: own_subnet_id.get(), + } + .encode(); + let uc_call = wasm() + .call_simple( + CanisterId::ic_00(), + Method::SubnetMetrics, + call_args().other_side(payload), + ) + .build(); + let result = test.ingress(uni_canister, "update", uc_call).unwrap(); + let bytes = match result { + WasmResult::Reply(bytes) => bytes, + WasmResult::Reject(err_msg) => panic!("Unexpected reject, expected reply: {err_msg}"), + }; + let response = Decode!(&bytes, SubnetMetricsResponse).unwrap(); + // All five fields decode. `ExecutionTest` starts at round 1 and does not run + // message routing, so only `block_height` and the live cycles fold have + // non-default values here; the other fields are covered by + // `subnet_metrics_reflects_subnet_metrics_state`. + assert_eq!(response.block_height, candid::Nat::from(1_u64)); + assert_eq!( + response.num_canisters, + candid::Nat::from(test.state().metadata.subnet_metrics.num_canisters) + ); + assert_eq!( + response.canister_state_bytes, + candid::Nat::from( + test.state() + .metadata + .subnet_metrics + .canister_state_bytes + .get() + ) + ); + assert!(response.consumed_cycles_total > 0_u64); + assert_eq!( + response.update_transactions_total, + candid::Nat::from( + test.state() + .metadata + .subnet_metrics + .update_transactions_total + ) + ); +} + +#[test] +fn subnet_metrics_block_height_matches_current_round() { + let own_subnet_id = subnet_test_id(1); + let caller_canister = canister_test_id(1); + let mut test = ExecutionTestBuilder::new() + .with_own_subnet_id(own_subnet_id) + .with_caller(subnet_test_id(2), caller_canister) + .with_current_round(42) + .build(); + + let response = subnet_metrics_call(&mut test, own_subnet_id.get()).unwrap(); + assert_eq!(response.block_height, candid::Nat::from(42_u64)); +} + +#[test] +fn subnet_metrics_block_height_is_non_decreasing() { + let own_subnet_id = subnet_test_id(1); + let caller_canister = canister_test_id(1); + let mut test = ExecutionTestBuilder::new() + .with_own_subnet_id(own_subnet_id) + .with_caller(subnet_test_id(2), caller_canister) + .with_current_round(7) + .build(); + + let first = subnet_metrics_call(&mut test, own_subnet_id.get()).unwrap(); + test.set_current_round(8); + let second = subnet_metrics_call(&mut test, own_subnet_id.get()).unwrap(); + + assert_eq!(first.block_height, candid::Nat::from(7_u64)); + assert_eq!(second.block_height, candid::Nat::from(8_u64)); + assert!(second.block_height > first.block_height); +} + +#[test] +fn subnet_metrics_ingress_update_fails_at_ingress_filter() { + let own_subnet_id = subnet_test_id(1); + let mut test = ExecutionTestBuilder::new() + .with_own_subnet_id(own_subnet_id) + .build(); + let payload = SubnetMetricsArgs { + subnet_id: own_subnet_id.get(), + } + .encode(); + + let result = test.should_accept_ingress_message(IC_00, Method::SubnetMetrics, payload); + assert_eq!( + result, + Err(UserError::new( + ErrorCode::CanisterRejectedMessage, + "ic00 method subnet_metrics can not be called via ingress messages" + )) + ); +} + +#[test] +fn subnet_metrics_ingress_update_fails_at_execution() { + let own_subnet_id = subnet_test_id(1); + let mut test = ExecutionTestBuilder::new() + .with_own_subnet_id(own_subnet_id) + .build(); + let payload = SubnetMetricsArgs { + subnet_id: own_subnet_id.get(), + } + .encode(); + test.subnet_message(Method::SubnetMetrics, payload) + .unwrap_err() + .assert_contains( + ErrorCode::CanisterContractViolation, + "subnet_metrics cannot be called by a user", + ); +} + +#[test] +fn subnet_metrics_ingress_query_fails() { + let own_subnet_id = subnet_test_id(1); + let mut test = ExecutionTestBuilder::new() + .with_own_subnet_id(own_subnet_id) + .build(); + let payload = SubnetMetricsArgs { + subnet_id: own_subnet_id.get(), + } + .encode(); + test.non_replicated_query(CanisterId::ic_00(), "subnet_metrics", payload) + .unwrap_err() + .assert_contains( + ErrorCode::CanisterMethodNotFound, + "Query method subnet_metrics not found.", + ); +} + +#[test] +fn subnet_metrics_foreign_subnet_id_is_rejected() { + let own_subnet_id = subnet_test_id(1); + let other_subnet_id = subnet_test_id(3); + let caller_canister = canister_test_id(1); + let mut test = ExecutionTestBuilder::new() + .with_own_subnet_id(own_subnet_id) + .with_caller(subnet_test_id(2), caller_canister) + .build(); + + let (code, message) = subnet_metrics_call(&mut test, other_subnet_id.get()).unwrap_err(); + assert_eq!(code, RejectCode::CanisterReject); + assert!( + message.contains("does not match current subnet ID"), + "unexpected reject message: {message}" + ); +} + +#[test] +fn subnet_metrics_reflects_subnet_metrics_state() { + let own_subnet_id = subnet_test_id(1); + let caller_canister = canister_test_id(1); + let mut test = ExecutionTestBuilder::new() + .with_own_subnet_id(own_subnet_id) + .with_caller(subnet_test_id(2), caller_canister) + .build(); + // Create a canister so that the fold over canisters is non-trivial. + let canister_id = test.create_canister(Cycles::new(1_000_000_000_000)); + let cost_schedule = test.state().get_own_subnet_cycles_config().cost_schedule; + test.canister_state_mut(canister_id) + .system_state + .consume_cycles(CompoundCycles::::new( + Cycles::new(1_000_000), + cost_schedule, + )); + + let deleted_cycles = NominalCycles::new(987_654_321_u128); + { + let metrics = &mut test.state_mut().metadata.subnet_metrics; + metrics.num_canisters = 17; + metrics.canister_state_bytes = NumBytes::new(4_321); + metrics.update_transactions_total = 99; + metrics.observe_consumed_cycles_by_deleted_canisters(deleted_cycles); + } + // Computed independently of the handler: the sum over all canisters plus the + // subnet-level aggregate. + let expected_consumed_cycles = test.state().metadata.subnet_metrics.consumed_cycles_total() + + test + .state() + .canister_states() + .all_values() + .fold(NominalCycles::zero(), |acc, canister| { + acc + canister.system_state.canister_metrics().consumed_cycles() + }); + assert!(expected_consumed_cycles > deleted_cycles); + + let response = subnet_metrics_call(&mut test, own_subnet_id.get()).unwrap(); + + assert_eq!(response.num_canisters, candid::Nat::from(17_u64)); + assert_eq!(response.canister_state_bytes, candid::Nat::from(4_321_u64)); + assert_eq!( + response.update_transactions_total, + candid::Nat::from(99_u64) + ); + assert_eq!( + response.consumed_cycles_total, + candid::Nat::from(expected_consumed_cycles.get()) + ); +} + +#[test] +fn subnet_metrics_is_partition_independent() { + let own_subnet_id = subnet_test_id(1); + let caller_canister = canister_test_id(1); + let mut test = ExecutionTestBuilder::new() + .with_own_subnet_id(own_subnet_id) + .with_caller(subnet_test_id(2), caller_canister) + .build(); + let canister_id = test.create_canister(Cycles::new(1_000_000_000_000)); + let cost_schedule = test.state().get_own_subnet_cycles_config().cost_schedule; + test.canister_state_mut(canister_id) + .system_state + .consume_cycles(CompoundCycles::::new( + Cycles::new(1_000_000), + cost_schedule, + )); + + let before = subnet_metrics_call(&mut test, own_subnet_id.get()).unwrap(); + test.state_mut().repartition_canister_states(); + let after = subnet_metrics_call(&mut test, own_subnet_id.get()).unwrap(); + + assert_eq!(before.consumed_cycles_total, after.consumed_cycles_total); +} + +#[test] +fn subnet_metrics_malformed_payload_is_rejected() { + let own_subnet_id = subnet_test_id(1); + let caller_canister = canister_test_id(1); + let mut test = ExecutionTestBuilder::new() + .with_own_subnet_id(own_subnet_id) + .with_caller(subnet_test_id(2), caller_canister) + .build(); + + let (code, message) = + subnet_metrics_raw_call(&mut test, EmptyBlob.encode()).expect_err("expected a reject"); + // The Candid decode failure surfaces as `ErrorCode::InvalidManagementPayload` + // (`candid_error_to_user_error`), which maps to `RejectCode::CanisterReject`. + assert_eq!(code, RejectCode::CanisterReject); + assert!( + message.contains("Error decoding candid"), + "unexpected reject message: {message}" + ); +} + #[test] fn node_metrics_history_update_succeeds() { let own_subnet_id = subnet_test_id(1); diff --git a/rs/execution_environment/src/execution_environment.rs b/rs/execution_environment/src/execution_environment.rs index 80c9e5155cee..ff0b0eabce9b 100644 --- a/rs/execution_environment/src/execution_environment.rs +++ b/rs/execution_environment/src/execution_environment.rs @@ -42,10 +42,10 @@ use ic_management_canister_types_private::{ ReadCanisterSnapshotDataArgs, ReadCanisterSnapshotMetadataArgs, RenameCanisterArgs, ReshareChainKeyArgs, SchnorrAlgorithm, SchnorrPublicKeyArgs, SchnorrPublicKeyResponse, SetupInitialDKGArgs, SignWithECDSAArgs, SignWithSchnorrArgs, SignWithSchnorrAux, - StoredChunksArgs, SubnetInfoArgs, SubnetInfoResponse, TakeCanisterSnapshotArgs, - UninstallCodeArgs, UpdateSettingsArgs, UploadCanisterSnapshotDataArgs, - UploadCanisterSnapshotMetadataArgs, UploadChunkArgs, VetKdDeriveKeyArgs, VetKdPublicKeyArgs, - VetKdPublicKeyResult, + StoredChunksArgs, SubnetInfoArgs, SubnetInfoResponse, SubnetMetricsArgs, SubnetMetricsResponse, + TakeCanisterSnapshotArgs, UninstallCodeArgs, UpdateSettingsArgs, + UploadCanisterSnapshotDataArgs, UploadCanisterSnapshotMetadataArgs, UploadChunkArgs, + VetKdDeriveKeyArgs, VetKdPublicKeyArgs, VetKdPublicKeyResult, }; use ic_metrics::MetricsRegistry; use ic_registry_provisional_whitelist::ProvisionalWhitelist; @@ -1860,6 +1860,27 @@ impl ExecutionEnvironment { } }, + Ok(Ic00Method::SubnetMetrics) => match &msg { + CanisterCall::Ingress(_) => { + self.reject_unexpected_ingress(Ic00Method::SubnetMetrics) + } + CanisterCall::Request(_) => { + // Only deduct round instructions for building the response + // when the request is accepted; a rejected call must not + // consume round instructions. + let res = SubnetMetricsArgs::decode(payload) + .and_then(|args| self.subnet_metrics(&state, current_round, args)) + .map(|(res, instructions)| { + round_limits.instructions -= as_round_instructions(instructions); + (res, None) + }); + ExecuteSubnetMessageResult::Finished { + response: res, + refund: msg.take_cycles(), + } + } + }, + Ok(Ic00Method::SubnetInfo) => match &msg { CanisterCall::Ingress(_) => self.reject_unexpected_ingress(Ic00Method::SubnetInfo), CanisterCall::Request(_) => { @@ -3371,6 +3392,54 @@ impl ExecutionEnvironment { Ok(Encode!(&res).unwrap()) } + /// Computes the response to the `subnet_metrics` management canister method, + /// together with the number of round instructions the caller must deduct for + /// computing it. + fn subnet_metrics( + &self, + state: &ReplicatedState, + current_round: ExecutionRound, + args: SubnetMetricsArgs, + ) -> Result<(Vec, NumInstructions), UserError> { + if args.subnet_id != self.own_subnet_id.get() { + return Err(UserError::new( + ErrorCode::CanisterRejectedMessage, + format!( + "Provided target subnet ID {} does not match current subnet ID {}.", + args.subnet_id, self.own_subnet_id + ), + )); + } + let metrics = &state.metadata.subnet_metrics; + // The same function the certified state tree at `/subnet//metrics` + // uses from certification version `V29` on, so the two cannot drift. + let consumed_cycles_total = metrics.consumed_cycles_total_including_canisters( + state.canister_states().total_consumed_cycles(), + ); + let res = SubnetMetricsResponse { + // The height of the block in whose execution this call is processed. + // `ExecutionRound` is numerically the finalized consensus block + // height; see `rs/messaging/src/state_machine.rs`. + block_height: candid::Nat::from(current_round.get()), + // `num_canisters` and `update_transactions_total` are written at the + // *end* of a round (`message_routing.rs`, `scheduler.rs`), so a call + // executing in round N reports the end-of-round-(N-1) values. That + // one-round lag is what `read_state` reports for height N-1 too, so the + // two agree; it is nonetheless not literally "current". + num_canisters: candid::Nat::from(metrics.num_canisters), + // Read from the stored `SubnetMetrics` field rather than recomputed + // live, so that the value agrees with the certified state tree. Note + // that message routing only refreshes the stored field every 10 + // rounds by design (`rs/messaging/src/message_routing.rs`), so + // recomputing it here would make `subnet_metrics` disagree with + // `read_state` on 9 rounds out of 10. + canister_state_bytes: candid::Nat::from(metrics.canister_state_bytes.get()), + consumed_cycles_total: candid::Nat::from(consumed_cycles_total.get()), + update_transactions_total: candid::Nat::from(metrics.update_transactions_total), + }; + Ok((Encode!(&res).unwrap(), subnet_metrics_instructions(state))) + } + // Executes an inter-canister response. // // Returns a tuple with the result, along with a flag indicating whether or @@ -4935,6 +5004,121 @@ pub(crate) fn full_subnet_memory_capacity( ) } +/// Computes the number of round instructions consumed by executing the +/// `subnet_metrics` management method against the given state. +/// +/// The dominant cost is `CanisterStates::total_consumed_cycles()`, which folds +/// over the **hot** canister pool only; the cold pool contributes a precomputed +/// `O(1)` aggregate. The variable term is therefore keyed on +/// `CanisterStates::hot_len()`, which is exactly what the fold visits — *not* on +/// `num_canisters()`. +/// +/// Keying on the total would over-charge by the ratio `len / hot_len`, which is +/// large in the steady state: `repartition_canister_states()` runs on every +/// `commit_and_certify` (`rs/state_manager/src/lib.rs`), so at the start of a +/// round the hot pool holds only canisters that were active in the previous one. +/// On a 100k-canister subnet with a few thousand hot canisters that is a ~40x +/// over-charge — i.e. ~40x more of the shared per-round subnet-message budget +/// consumable per call than the call actually costs the subnet, which is denial +/// capacity that is not backed by any work. See the note on inflation below: this +/// is the same mistake in a different guise. +/// +/// **This makes execution depend on the *cardinality* of the hot/cold partition, +/// which is new.** Every prior consumer of the partition is +/// partition-*independent* — `total_canister_memory_usage()` and +/// `total_consumed_cycles()` are `fold(hot) + cold aggregate`, so they yield the +/// same number wherever the split lies. `hot_len()` is a raw count of one side of +/// it, so for the first time *where* the split lies changes an execution result, +/// and hence how many subnet messages fit in a round. The determinism argument is +/// therefore not the one those consumers rely on; it is: +/// +/// 1. `CanisterState::is_cold()` is a pure function of the canister +/// (`rs/replicated_state/src/canister_state.rs`). The one term that reads as +/// time-dependent is not: `has_unexpired_callbacks()` is +/// `!unexpired_callbacks.is_empty()` and takes no `now`, unlike the +/// `has_expired_callbacks(now)` defined just above it, which `is_cold()` does +/// not call. +/// 2. The partition is **never serialized**. A checkpoint stores only the flat +/// canister set; every load path goes through +/// `ReplicatedState::new_from_checkpoint` → `CanisterStates::new`, which +/// re-derives the split from `is_cold()`. So no persisted or +/// attacker-writable value can encode a non-derived partition. +/// 3. `ReplicatedState::repartition_canister_states()` runs **unconditionally** on +/// every `commit_and_certify` (`rs/state_manager/src/lib.rs`, outside the +/// `CertificationScope::Metadata` branch), so the committed partition equals +/// the one `CanisterStates::new` would derive. +/// 4. By (2) and (3) every way a replica can acquire the state for the next round +/// yields the same partition: continuing in memory, restarting from a +/// checkpoint, state sync (same load path), and the catch-up branch of +/// `take_tip`, which clones a snapshot produced by one of the former. +/// +/// Fact (3) is load-bearing and is **pinned by +/// `hot_cold_partition_is_canonical_after_every_commit`** in +/// `rs/state_manager/tests/state_manager.rs`: making that repartition conditional +/// on checkpoint rounds would diverge the charge between a replica that kept +/// running and one that restarted, and that test fails if anyone does. +/// +/// Cost model, using the conversion `2B instructions = 1 second` +/// (i.e. `2M instructions = 1 ms`): +/// - a base cost of 100K instructions (≈50us), and +/// - a variable cost of 40 instructions (≈20ns) per **hot** canister. +/// +/// The variable term is measured by the `subnet_metrics_consumed_cycles_fold` +/// group of `benches/management_canister/subnet_metrics.rs`, which folds over a +/// fully hot pool. Measured per-hot-canister cost at 100K hot canisters: 7.2ns +/// with sequential allocation, 8.2ns with shuffled insertion order and allocator +/// churn (the `hot/…/shuffled` variants), and 13.2ns worst case on a loaded +/// machine — i.e. 14 to 27 instructions. 40 is ≈1.5x the worst observation. +/// +/// Two reasons allocation order barely matters here, so the measurement is not +/// optimistic. `size_of::()` is 2544 bytes, so 100K hot canisters +/// are ≈254MB of separately allocated `Arc` payloads: the working set is +/// DRAM-resident regardless of the order they were created in, which is why +/// shuffling costs only ~13%. And the fold touches one cache line *inside* that +/// fixed-size allocation (`system_state.canister_metrics.consumed_cycles`), so a +/// canister that owns more heap elsewhere — queues, execution state, snapshots — +/// does not make the fold slower. +/// +/// An attacker can pin canisters in the hot pool cheaply (e.g. a `global_timer` +/// set far in the future keeps `is_cold()` false forever). Under this keying that +/// raises the charge in proportion to the work it creates, which is the intent; +/// it cannot be used to make the charge under-state the work. +/// +/// The base covers the per-call work that does not scale with the number of +/// canisters: the Candid decode of the argument, five field reads, and the Candid +/// encode of five `Nat`s. **It is estimated from that work and was never measured +/// end to end** — there is no benchmark for it, deliberately, since an end-to-end +/// `StateMachine` measurement is dominated by round overhead rather than by the +/// handler. The estimate is generous: that work is order 2-10us against the 50us +/// that 100K instructions represents, and the constant is 200x below +/// `list_canisters`'s 20M. Over-estimating the base is safe for the wall-clock +/// bound (fewer calls are served per round) and costs only denial headroom, which +/// is priced in the security review against `fetch_canister_logs` — a deployed +/// method of the same shape with a *larger* base of 150K and likewise no cycle fee. +/// +/// Both constants are far below `list_canisters`'s 20M / 16K. That is +/// intentional: `list_canisters` is gated to subnet admins, whereas +/// `subnet_metrics` is open to any canister with no cycle fee, so overcharging +/// here would let an unauthenticated caller exhaust the per-round subnet-message +/// instruction budget and defer unrelated subnet messages. Do not inflate these +/// to "be safe", and do not key them on a count larger than the work — either +/// widens the denial surface rather than narrowing it. +/// +/// Saturating arithmetic, unlike `list_canisters_instructions`: a release-build +/// wrap would silently produce a small charge and remove the bound this function +/// exists to provide. +// Keep in sync with `SUBNET_METRICS_BASE_INSTRUCTIONS` / +// `SUBNET_METRICS_INSTRUCTIONS_PER_HOT_CANISTER` in `execution_test.rs`. +fn subnet_metrics_instructions(state: &ReplicatedState) -> NumInstructions { + const BASE_INSTRUCTIONS: u64 = 100_000; + const INSTRUCTIONS_PER_HOT_CANISTER: u64 = 40; + let hot_canisters = state.canister_states().hot_len() as u64; + NumInstructions::new( + BASE_INSTRUCTIONS + .saturating_add(INSTRUCTIONS_PER_HOT_CANISTER.saturating_mul(hot_canisters)), + ) +} + fn get_canister( canister_id: CanisterId, state: &ReplicatedState, diff --git a/rs/execution_environment/src/execution_environment_metrics.rs b/rs/execution_environment/src/execution_environment_metrics.rs index b5b6d3a4dd32..b0dc9e965377 100644 --- a/rs/execution_environment/src/execution_environment_metrics.rs +++ b/rs/execution_environment/src/execution_environment_metrics.rs @@ -313,6 +313,7 @@ impl ExecutionEnvironmentMetrics { | ic00::Method::BitcoinSendTransaction | ic00::Method::BitcoinGetCurrentFeePercentiles | ic00::Method::NodeMetricsHistory + | ic00::Method::SubnetMetrics | ic00::Method::SubnetInfo | ic00::Method::FetchCanisterLogs | ic00::Method::ProvisionalCreateCanisterWithCycles diff --git a/rs/execution_environment/src/ic00_permissions.rs b/rs/execution_environment/src/ic00_permissions.rs index 76d39db95c7d..2701f4cd23c5 100644 --- a/rs/execution_environment/src/ic00_permissions.rs +++ b/rs/execution_environment/src/ic00_permissions.rs @@ -72,7 +72,15 @@ impl Ic00MethodPermissions { does_not_run_on_aborted_canister: false, installs_code: false, }, - Ic00Method::FetchCanisterLogs + // `SubnetMetrics` consumes round instructions, and is recorded as such + // here. Note the flag is not actually consulted for it: the method has no + // effective canister ID, so `Scheduler::can_execute_subnet_msg` returns + // before reaching `can_be_executed`. Its deferral comes from the dedicated + // special case there, which must not be removed on the strength of this + // flag. (`ListCanisters` is in the same position but is recorded as + // `false`; see the note above.) + Ic00Method::SubnetMetrics + | Ic00Method::FetchCanisterLogs | Ic00Method::ReadCanisterSnapshotMetadata | Ic00Method::ReadCanisterSnapshotData => Self { method, diff --git a/rs/execution_environment/src/scheduler.rs b/rs/execution_environment/src/scheduler.rs index 0204dc5b4d70..158162eea178 100644 --- a/rs/execution_environment/src/scheduler.rs +++ b/rs/execution_environment/src/scheduler.rs @@ -1801,10 +1801,11 @@ fn can_execute_subnet_msg( // Some heavy methods use round instructions. let instructions_reached = round_limits.instructions_reached(); - // `list_canisters` iterates over the subnet's canisters and thus consumes - // round instructions, even though it has no effective canister ID. Defer it - // to a later round if the round instruction limit has already been reached. - if let Some(Ic00Method::ListCanisters) = msg_method { + // `list_canisters` and `subnet_metrics` iterate over the subnet's canisters + // and thus consume round instructions, even though they have no effective + // canister ID. Defer them to a later round if the round instruction limit has + // already been reached. + if let Some(Ic00Method::ListCanisters | Ic00Method::SubnetMetrics) = msg_method { return !instructions_reached; } @@ -1903,6 +1904,7 @@ fn get_instruction_limits_for_subnet_message( | BitcoinGetCurrentFeePercentiles | BitcoinGetSuccessors | NodeMetricsHistory + | SubnetMetrics | SubnetInfo | FetchCanisterLogs | ProvisionalCreateCanisterWithCycles diff --git a/rs/execution_environment/tests/execution_test.rs b/rs/execution_environment/tests/execution_test.rs index 793dde956469..d90ce40f5ad8 100644 --- a/rs/execution_environment/tests/execution_test.rs +++ b/rs/execution_environment/tests/execution_test.rs @@ -14,7 +14,8 @@ use ic_management_canister_types_private::{ CanisterMetricsArgs, CanisterSettingsArgs, CanisterSettingsArgsBuilder, CanisterStatusResultV2, CreateCanisterArgs, DerivationPath, EcdsaKeyId, EmptyBlob, IC_00, InstallCodeArgsV2, ListCanistersResponse, LoadCanisterSnapshotArgs, MasterPublicKeyId, Method, Payload, - SignWithECDSAArgs, TakeCanisterSnapshotArgs, UpdateSettingsArgs, + SignWithECDSAArgs, SubnetMetricsArgs, SubnetMetricsResponse, TakeCanisterSnapshotArgs, + UpdateSettingsArgs, }; use ic_registry_resource_limits::ResourceLimits; use ic_registry_subnet_type::SubnetType; @@ -28,7 +29,9 @@ use ic_test_utilities_metrics::{ use ic_test_utilities_types::ids::user_test_id; use ic_types::ingress::{IngressState, IngressStatus}; use ic_types::messages::MessageId; -use ic_types::{CanisterId, NumBytes, Time, ingress::WasmResult, messages::NO_DEADLINE}; +use ic_types::{ + CanisterId, NumBytes, NumInstructions, Time, ingress::WasmResult, messages::NO_DEADLINE, +}; use ic_types_cycles::{CanisterCyclesCostSchedule, Cycles}; use ic_universal_canister::{UNIVERSAL_CANISTER_WASM, call_args, wasm}; use more_asserts::{assert_ge, assert_gt, assert_le, assert_lt}; @@ -2881,6 +2884,423 @@ fn list_canisters_via_inter_canister_call_rejected_for_non_admin() { assert_eq!(env.subnet_message_instructions(), instructions_baseline); } +/// Keep in sync with `subnet_metrics_instructions` in +/// `rs/execution_environment/src/execution_environment.rs`. +const SUBNET_METRICS_BASE_INSTRUCTIONS: u64 = 100_000; +/// Keep in sync with `subnet_metrics_instructions` in +/// `rs/execution_environment/src/execution_environment.rs`. Note this is per +/// **hot** canister, which is what the fold visits — not per canister. +const SUBNET_METRICS_INSTRUCTIONS_PER_HOT_CANISTER: u64 = 40; + +fn subnet_metrics_count(env: &StateMachine) -> u64 { + fetch_histogram_vec_stats( + env.metrics_registry(), + "execution_subnet_message_duration_seconds", + ) + .get(&labels(&[ + ("method_name", "ic00_subnet_metrics"), + ("outcome", "finished"), + ("status", "success"), + ("speed", "fast"), + ])) + .map_or(0, |stats| stats.count) +} + +fn subnet_metrics_payload(env: &StateMachine) -> Vec { + SubnetMetricsArgs { + subnet_id: env.get_subnet_id().get(), + } + .encode() +} + +/// Builds a `StateMachine` whose round instruction limit is small enough that +/// the derived per-round subnet-message budget +/// (`max_instructions_per_round / SUBNET_MESSAGES_LIMIT_FRACTION`) is only a +/// small multiple of the `subnet_metrics` per-call charge. +/// +/// All four instruction-limit fields must be set together. In particular +/// `max_instructions_per_install_code_slice` defaults to `2 * B`, and the +/// canister round budget is +/// `max_instructions_per_round - max(max_instructions_per_slice, max_instructions_per_install_code_slice) + 1` +/// (see `Scheduler::round_limits` in `rs/execution_environment/src/scheduler.rs`). +/// Leaving the install-code slice at its default would make that budget negative +/// (`80M - 2B + 1 < 0`, and `RoundInstructions` is a signed `i64`), so +/// `RoundInstructions::instructions_reached()` would be true from round start, the +/// inner round would break before any canister message executed, and no +/// `subnet_metrics` call would ever be made. +/// +/// Note that the *production* sizing rule documented at +/// `rs/config/src/subnet_config.rs` — round at least +/// `max(slice, install_code_slice) + 2 * B`, so that a round lasts about a second +/// — cannot hold once the round budget is shrunk below `2 * B`. It is a sizing +/// rule, not a correctness requirement; what execution actually requires is the +/// positive canister round budget asserted below. +fn subnet_metrics_env_with_round_limit(max_instructions_per_round: u64) -> StateMachine { + let slice = max_instructions_per_round / 2; + let mut subnet_config = SubnetConfig::new(SubnetType::Application); + subnet_config.scheduler_config.max_instructions_per_round = + NumInstructions::new(max_instructions_per_round); + subnet_config.scheduler_config.max_instructions_per_slice = NumInstructions::new(slice); + subnet_config.scheduler_config.max_instructions_per_message = NumInstructions::new(slice); + subnet_config + .scheduler_config + .max_instructions_per_install_code_slice = NumInstructions::new(slice); + + // Executable precondition: the canister round budget, recomputed exactly as + // `Scheduler::round_limits` does, must be positive. Otherwise + // `RoundInstructions::instructions_reached()` is true from round start, the + // inner round breaks before executing any canister message, and every test + // built on this environment would pass vacuously. + let canister_round_budget = max_instructions_per_round as i64 + - std::cmp::max( + subnet_config + .scheduler_config + .max_instructions_per_slice + .get(), + subnet_config + .scheduler_config + .max_instructions_per_install_code_slice + .get(), + ) as i64 + + 1; + assert!( + canister_round_budget > 0, + "canister round budget {canister_round_budget} is not positive: \ + max_instructions_per_round ({}) must exceed \ + max(max_instructions_per_slice ({}), max_instructions_per_install_code_slice ({}))", + max_instructions_per_round, + subnet_config.scheduler_config.max_instructions_per_slice, + subnet_config + .scheduler_config + .max_instructions_per_install_code_slice, + ); + + StateMachineBuilder::new() + .with_config(Some(StateMachineConfig::new( + subnet_config, + HypervisorConfig::default(), + ))) + .with_subnet_type(SubnetType::Application) + .with_cost_schedule(CanisterCyclesCostSchedule::Free) + .build() +} + +// `subnet_metrics` consumes round instructions according to its cost model (a +// base cost plus a per-canister cost). This test checks that the round +// instruction limit is respected: when many `subnet_metrics` calls are pending +// at once, the per-round subnet-message instruction budget only allows some of +// them to execute per round, so the rest are deferred to later rounds (i.e. not +// all calls execute in the same round). +// +// Mirrors `list_canisters_respects_round_instruction_limit`. Unlike that test it +// has to shrink the round budget, because at the default +// `max_instructions_per_round` of `4 * B` the per-round subnet-message budget of +// 250M would need thousands of concurrent `subnet_metrics` calls to saturate, +// well past the canister output queue capacity of +// `DEFAULT_QUEUE_CAPACITY = 500`. +#[test] +fn subnet_metrics_respects_round_instruction_limit() { + // Number of concurrent `subnet_metrics` calls, bounded by + // `DEFAULT_QUEUE_CAPACITY = 500`. + const NUM_CALLS: u64 = 200; + // Keep in sync with `SUBNET_MESSAGES_LIMIT_FRACTION` in + // `rs/execution_environment/src/scheduler.rs`. + const SUBNET_MESSAGES_LIMIT_FRACTION: u64 = 16; + const MAX_INSTRUCTIONS_PER_ROUND: u64 = 80_000_000; + + let env = subnet_metrics_env_with_round_limit(MAX_INSTRUCTIONS_PER_ROUND); + let caller = create_universal_canister_with_cycles( + &env, + Some(CanisterSettingsArgsBuilder::new().build()), + INITIAL_CYCLES_BALANCE, + ); + + let num_canisters = env.get_latest_state().num_canisters() as u64; + assert_eq!(num_canisters, 1); + // The charge is `BASE + 40 * hot_len`, and `hot_len` is a property of the + // round the call happens to execute in — the caller canister is hot while it + // has pending work and cold otherwise — so an exact per-call cost is not + // observable from here. With a single canister on the subnet it is bracketed + // by `hot_len ∈ {0, 1}`, which is tight enough for every assertion below. + let min_cost_per_call = SUBNET_METRICS_BASE_INSTRUCTIONS; + let max_cost_per_call = SUBNET_METRICS_BASE_INSTRUCTIONS + + SUBNET_METRICS_INSTRUCTIONS_PER_HOT_CANISTER * num_canisters; + let budget = MAX_INSTRUCTIONS_PER_ROUND / SUBNET_MESSAGES_LIMIT_FRACTION; + // Use the *minimum* charge here, so reaching the condition is guaranteed + // rather than merely likely. + assert!( + NUM_CALLS * min_cost_per_call > budget, + "test cannot reach the condition it asserts: {NUM_CALLS} calls x \ + {min_cost_per_call} instructions do not exceed the per-round \ + subnet-message budget {budget}; lower MAX_INSTRUCTIONS_PER_ROUND or \ + raise NUM_CALLS" + ); + // ...and the *maximum* charge here, for the same reason. + assert!( + budget >= 2 * max_cost_per_call, + "budget {budget} fits fewer than two calls, so the test degenerates to \ + one call per round and proves nothing about batching" + ); + + // Build an update that fires `NUM_CALLS` concurrent `subnet_metrics` + // inter-canister calls (ignoring their responses) and then replies. + let payload = subnet_metrics_payload(&env); + let mut update = wasm(); + for _ in 0..NUM_CALLS { + update = update.call_simple( + CanisterId::ic_00(), + Method::SubnetMetrics, + call_args() + .other_side(payload.clone()) + .on_reply(wasm().noop()) + .on_reject(wasm().noop()), + ); + } + let update = update.reply().build(); + + let instructions_baseline = env.subnet_message_instructions(); + let calls_baseline = subnet_metrics_count(&env); + assert_eq!(calls_baseline, 0); + env.send_ingress(PrincipalId::new_anonymous(), caller, "update", update); + + let executed_so_far = || subnet_metrics_count(&env) - calls_baseline; + let mut executed_per_round = vec![]; + for _ in 0..200 { + env.tick(); + executed_per_round.push(executed_so_far()); + if executed_so_far() == NUM_CALLS { + break; + } + } + + // Not all `subnet_metrics` calls were executed in the same round: there is a + // round after which some but not all of them had been executed. + assert!( + executed_per_round.iter().any(|&n| n > 0 && n < NUM_CALLS), + "expected subnet_metrics calls to be spread across rounds, got progression {:?}", + executed_per_round, + ); + // Eventually all of them were executed. + assert_eq!(*executed_per_round.last().unwrap(), NUM_CALLS); + // The calls were *batched*, not executed one per round: some round drained at + // least two of them. Asserting only "spread across rounds" above would also be + // satisfied by a degenerate one-call-per-round progression, which is what the + // `budget >= 2 * max_cost_per_call` precondition exists to rule out — so + // assert the consequence too, not just the precondition. + let per_round_deltas: Vec = std::iter::once(executed_per_round[0]) + .chain(executed_per_round.windows(2).map(|w| w[1] - w[0])) + .collect(); + assert!( + per_round_deltas.iter().any(|&n| n >= 2), + "expected at least one round to execute two or more calls, got per-round \ + counts {per_round_deltas:?}" + ); + // Every executed call was charged per the cost model, within the `hot_len` + // bracket established above. + let charged = env.subnet_message_instructions() - instructions_baseline; + assert!( + charged >= (NUM_CALLS * min_cost_per_call) as f64 + && charged <= (NUM_CALLS * max_cost_per_call) as f64, + "total charge {charged} outside [{}, {}] for {NUM_CALLS} calls", + NUM_CALLS * min_cost_per_call, + NUM_CALLS * max_cost_per_call, + ); +} + +// A successful `subnet_metrics` call is charged round instructions per the cost +// model; a rejected one (malformed payload, or a `subnet_id` naming a different +// subnet) is charged nothing. +#[test] +fn subnet_metrics_charges_round_instructions() { + let env = StateMachineBuilder::new() + .with_config(Some(StateMachineConfig::new( + SubnetConfig::new(SubnetType::Application), + HypervisorConfig::default(), + ))) + .with_subnet_type(SubnetType::Application) + .with_cost_schedule(CanisterCyclesCostSchedule::Free) + .build(); + let caller = create_universal_canister_with_cycles( + &env, + Some(CanisterSettingsArgsBuilder::new().build()), + INITIAL_CYCLES_BALANCE, + ); + + let num_canisters = env.get_latest_state().num_canisters() as u64; + assert_eq!(num_canisters, 1); + // See the note in `subnet_metrics_respects_round_instruction_limit`: the exact + // `hot_len` at handler time is not observable, so bracket it. + let min_cost_per_call = SUBNET_METRICS_BASE_INSTRUCTIONS; + let max_cost_per_call = SUBNET_METRICS_BASE_INSTRUCTIONS + + SUBNET_METRICS_INSTRUCTIONS_PER_HOT_CANISTER * num_canisters; + + let call = |payload: Vec| { + wasm() + .call_simple( + CanisterId::ic_00(), + Method::SubnetMetrics, + call_args() + .other_side(payload) + .on_reject(wasm().reject_message().reject()), + ) + .build() + }; + + // Success: charged per the cost model. + let baseline = env.subnet_message_instructions(); + let reply = + get_reply(env.execute_ingress(caller, "update", call(subnet_metrics_payload(&env)))); + let first = SubnetMetricsResponse::decode(&reply).unwrap(); + let charged = env.subnet_message_instructions() - baseline; + assert!( + charged >= min_cost_per_call as f64 && charged <= max_cost_per_call as f64, + "charge {charged} outside [{min_cost_per_call}, {max_cost_per_call}]" + ); + + // `block_height` tracks the *real* block height, not just whatever round + // number a harness handed the handler: after N further rounds it has advanced + // by at least N. (`subnet_metrics_block_height_matches_current_round` in + // `canister_manager/tests.rs` pins the `current_round` plumbing; this pins that + // `current_round` is the block height in a running `StateMachine`.) + const TICKS: u64 = 5; + assert!(first.block_height > 0_u64); + for _ in 0..TICKS { + env.tick(); + } + let reply = + get_reply(env.execute_ingress(caller, "update", call(subnet_metrics_payload(&env)))); + let second = SubnetMetricsResponse::decode(&reply).unwrap(); + assert!( + second.block_height >= first.block_height.clone() + candid::Nat::from(TICKS), + "block_height did not advance with the block height: {} then {} across \ + {TICKS} ticks", + first.block_height, + second.block_height, + ); + + // Malformed payload: rejected, charged nothing. + let baseline = env.subnet_message_instructions(); + let reject = get_reject(env.execute_ingress(caller, "update", call(EmptyBlob.encode()))); + assert!( + reject.contains("Error decoding candid"), + "unexpected reject: {reject}" + ); + assert_eq!(env.subnet_message_instructions(), baseline); + + // Foreign `subnet_id`: rejected, charged nothing. + // + // Note which layer rejects here. `resolve_destination` routes the call to the + // subnet named in the payload, and this single-subnet `StateMachine` has no + // route to it, so the call is rejected by message routing and the handler + // never runs. That is exactly the behaviour the interface spec relies on for + // the cross-subnet case; the handler's own-subnet check is exercised instead + // by `subnet_metrics_foreign_subnet_id_is_rejected` in + // `canister_manager/tests.rs`, which injects the request directly into the + // subnet queue. Either way, nothing is charged. + let foreign = SubnetMetricsArgs { + subnet_id: PrincipalId::new_subnet_test_id(0x1234), + } + .encode(); + let baseline = env.subnet_message_instructions(); + let reject = get_reject(env.execute_ingress(caller, "update", call(foreign))); + assert!( + reject.contains("No route to canister"), + "unexpected reject: {reject}" + ); + assert_eq!(env.subnet_message_instructions(), baseline); +} + +// The `subnet_metrics` charge must scale with the number of **hot** canisters — +// what `CanisterStates::total_consumed_cycles()` actually folds over — and not +// with the total number of canisters on the subnet. +// +// This is the regression test for a real defect: keying the charge on +// `num_canisters()` while the work is `O(|hot|)` manufactures denial capacity that +// is not backed by any work. `repartition_canister_states()` runs on every +// `commit_and_certify`, so `hot_len() << len()` is the steady state: on a +// 100k-canister subnet a `num_canisters()`-keyed charge over-states the cost by +// ~40x, meaning ~40x fewer calls suffice to pin the shared per-round +// subnet-message budget at zero and defer every `install_code` / `upload_chunk` / +// snapshot / `update_settings` on that subnet. +#[test] +fn subnet_metrics_charge_ignores_cold_canisters() { + // Enough extra canisters that a `num_canisters()`-keyed charge is + // unambiguously distinguishable from a `hot_len()`-keyed one, while keeping + // the test cheap. + const EXTRA_CANISTERS: u64 = 30; + + let env = StateMachineBuilder::new() + .with_config(Some(StateMachineConfig::new( + SubnetConfig::new(SubnetType::Application), + HypervisorConfig::default(), + ))) + .with_subnet_type(SubnetType::Application) + .with_cost_schedule(CanisterCyclesCostSchedule::Free) + .build(); + let caller = create_universal_canister_with_cycles( + &env, + Some(CanisterSettingsArgsBuilder::new().build()), + INITIAL_CYCLES_BALANCE, + ); + for _ in 0..EXTRA_CANISTERS { + env.create_canister(Some(CanisterSettingsArgsBuilder::new().build())); + } + // Let the freshly created canisters go quiet and be demoted to the cold pool. + for _ in 0..3 { + env.tick(); + } + + let state = env.get_latest_state(); + let num_canisters = state.num_canisters() as u64; + let hot_canisters = state.canister_states().hot_len() as u64; + assert_eq!(num_canisters, EXTRA_CANISTERS + 1); + // Executable precondition: the pool really is mostly cold, so the two keyings + // give different answers and the assertion below is not vacuous. + assert!( + hot_canisters * 4 < num_canisters, + "precondition failed: {hot_canisters} of {num_canisters} canisters are hot, \ + so a hot-keyed and a total-keyed charge are not distinguishable; the \ + test proves nothing" + ); + drop(state); + + let call = wasm() + .call_simple( + CanisterId::ic_00(), + Method::SubnetMetrics, + call_args() + .other_side(subnet_metrics_payload(&env)) + .on_reject(wasm().reject_message().reject()), + ) + .build(); + + let baseline = env.subnet_message_instructions(); + let reply = get_reply(env.execute_ingress(caller, "update", call)); + SubnetMetricsResponse::decode(&reply).unwrap(); + let charged = env.subnet_message_instructions() - baseline; + + // The charge is strictly below what keying on the total would give. This is + // the assertion that fails if the cost function regresses to + // `state.num_canisters()`. + let total_keyed = SUBNET_METRICS_BASE_INSTRUCTIONS + + SUBNET_METRICS_INSTRUCTIONS_PER_HOT_CANISTER * num_canisters; + assert!( + charged < total_keyed as f64, + "charge {charged} matches a total-keyed cost model ({total_keyed} for \ + {num_canisters} canisters, of which only {hot_canisters} are hot); the \ + charge must scale with the hot pool only" + ); + // And it is within the hot-keyed bracket. `hot_len` at handler time can differ + // from the value read above by the caller canister itself, hence the slack. + let hot_keyed_upper = SUBNET_METRICS_BASE_INSTRUCTIONS + + SUBNET_METRICS_INSTRUCTIONS_PER_HOT_CANISTER * (hot_canisters + 2); + assert!( + charged >= SUBNET_METRICS_BASE_INSTRUCTIONS as f64 && charged <= hot_keyed_upper as f64, + "charge {charged} outside the hot-keyed bracket \ + [{SUBNET_METRICS_BASE_INSTRUCTIONS}, {hot_keyed_upper}]" + ); +} + #[test] fn maximum_state_size() { let maximum_state_size = NumBytes::new(1 << 30); diff --git a/rs/replicated_state/src/canister_states/tests.rs b/rs/replicated_state/src/canister_states/tests.rs index fde77f4e9552..473e91197960 100644 --- a/rs/replicated_state/src/canister_states/tests.rs +++ b/rs/replicated_state/src/canister_states/tests.rs @@ -901,6 +901,75 @@ fn total_consumed_cycles_combines_hot_and_cold() { assert_eq!(states.total_consumed_cycles(), NominalCycles::new(135)); } +/// Consumes `amount` cycles on `canister`, as storage / instruction charging +/// does. Consuming cycles does not create work, so a cold canister stays cold. +fn consume_cycles(canister: &mut Arc, amount: u128) { + use ic_types_cycles::{CanisterCyclesCostSchedule, CompoundCycles, Instructions}; + + Arc::make_mut(canister) + .system_state + .consume_cycles(CompoundCycles::::new( + Cycles::new(amount), + CanisterCyclesCostSchedule::Normal, + )); +} + +/// Folds `consumed_cycles` over every canister, hot and cold, without going +/// through the `cold_stats` aggregate. +fn direct_consumed_cycles_fold(states: &CanisterStates) -> ic_types_cycles::NominalCycles { + use ic_types_cycles::NominalCycles; + + states + .all_values() + .fold(NominalCycles::zero(), |acc, canister| { + acc + canister.system_state.canister_metrics().consumed_cycles() + }) +} + +#[test] +fn total_consumed_cycles_equals_direct_fold() { + let mut states = CanisterStates::default(); + for id in 1..=4 { + let mut cold = cold_canister(id); + consume_cycles(&mut cold, 100 * id as u128); + states.insert(cold); + } + for id in 5..=7 { + let mut hot = hot_canister(id); + consume_cycles(&mut hot, 7 * id as u128); + states.insert(hot); + } + + assert_eq!(states.cold.len(), 4); + assert_eq!(states.hot.len(), 3); + assert_eq!( + states.total_consumed_cycles(), + direct_consumed_cycles_fold(&states) + ); +} + +#[test] +fn for_each_mut_keeps_cold_stats_consumed_cycles_in_sync() { + let mut states = CanisterStates::default(); + states.insert(cold_canister(1)); + states.insert(cold_canister(2)); + states.insert(hot_canister(3)); + assert_eq!(states.cold.len(), 2); + + // The path that storage charging takes: mutate every canister in place, + // including the cold ones. + states.for_each_mut(|_id, canister| consume_cycles(canister, 11)); + + // `total_consumed_cycles()` combines the hot fold with the `cold_stats` + // aggregate, so it agrees with a direct fold over every canister only if the + // sub-before / add-after bracketing around the cold-pool mutation held. That + // is the property `subnet_metrics` depends on. + assert_eq!( + states.total_consumed_cycles(), + direct_consumed_cycles_fold(&states) + ); +} + #[test] fn validate_strict_split_accepts_canonical_partition() { let mut states = CanisterStates::default(); diff --git a/rs/replicated_state/src/metadata_state.rs b/rs/replicated_state/src/metadata_state.rs index 5eb8f6e88842..929f99b11111 100644 --- a/rs/replicated_state/src/metadata_state.rs +++ b/rs/replicated_state/src/metadata_state.rs @@ -652,6 +652,28 @@ impl SubnetMetrics { total } + /// All cycles removed from circulation on the subnet, by both deleted and + /// still-existing canisters: the subnet-level aggregate + /// ([`Self::consumed_cycles_total`]) plus the cycles consumed by the canisters + /// that currently exist, which the caller obtains from + /// [`crate::CanisterStates::total_consumed_cycles`]. + /// + /// The canisters' contribution is a parameter rather than read here because + /// `SubnetMetrics` does not own the canisters, and because computing it is + /// `O(|hot canisters|)` — the certified state tree only wants it from + /// certification version `V29` onwards and passes `zero()` below that. + /// + /// **This is the single definition of the quantity, deliberately.** Two + /// consumers must agree on it bit for bit: the certified state tree at + /// `/subnet//metrics` (from `V29`) and the `subnet_metrics` + /// management canister method. Both call this, so they cannot drift. + pub fn consumed_cycles_total_including_canisters( + &self, + consumed_cycles_by_canisters: NominalCycles, + ) -> NominalCycles { + self.consumed_cycles_total() + consumed_cycles_by_canisters + } + /// Legacy computation of the total consumed cycles, used by the canonical /// state consumer for certification versions up to and including `V28`. /// diff --git a/rs/replicated_state/src/replicated_state.rs b/rs/replicated_state/src/replicated_state.rs index 56976d4a5fc0..38cd7f098e50 100644 --- a/rs/replicated_state/src/replicated_state.rs +++ b/rs/replicated_state/src/replicated_state.rs @@ -676,6 +676,17 @@ impl ReplicatedState { /// Re-establishes strict hot / cold partitioning of canister states (see /// [`CanisterStates::try_cool_all`]). + /// + /// **The caller in `commit_and_certify` must not be made conditional** (e.g. + /// "only on checkpoint rounds"). Execution reads + /// [`CanisterStates::hot_len`] — the `subnet_metrics` management method + /// charges round instructions proportional to it — so the *cardinality* of + /// the partition, not just its consistency, has to be identical on every + /// replica. Repartitioning on every commit is what makes the committed + /// partition equal the one `CanisterStates::new` derives at load, and hence + /// makes a replica that keeps running agree with one that restarts from a + /// checkpoint. `hot_cold_partition_is_canonical_after_every_commit` in + /// `rs/state_manager/tests/state_manager.rs` pins this. pub fn repartition_canister_states(&mut self) { self.canister_states.try_cool_all(); } diff --git a/rs/state_manager/src/lib.rs b/rs/state_manager/src/lib.rs index 408496253872..7c5e81d79f1b 100644 --- a/rs/state_manager/src/lib.rs +++ b/rs/state_manager/src/lib.rs @@ -3529,6 +3529,16 @@ impl StateManager for StateManagerImpl { // during the round may have left canisters that are now cold in `hot`. The // partition must be canonical at checkpoint time so that a replica continuing // through a checkpoint and one (re)starting from it agree on the partition. + // + // This call is deliberately outside the `CertificationScope::Metadata` + // branch above and must stay unconditional: execution reads + // `CanisterStates::hot_len()` (the `subnet_metrics` management method + // charges round instructions proportional to it), so a round that skipped + // the repartition would leave a continuing replica and a restarted one + // with different `hot_len()`, hence a different charge and a different + // number of subnet messages drained — a state divergence. Pinned by + // `hot_cold_partition_is_canonical_after_every_commit` in + // `tests/state_manager.rs`. self.metrics .hot_canisters_count .observe(state.canister_states().hot_len() as f64); diff --git a/rs/state_manager/tests/state_manager.rs b/rs/state_manager/tests/state_manager.rs index 25fdf37ec1ba..c09eedcb4656 100644 --- a/rs/state_manager/tests/state_manager.rs +++ b/rs/state_manager/tests/state_manager.rs @@ -24,8 +24,8 @@ use ic_registry_routing_table::{CANISTER_IDS_PER_SUBNET, CanisterIdRange, Routin use ic_registry_subnet_features::SubnetFeatures; use ic_registry_subnet_type::SubnetType; use ic_replicated_state::{ - ExecutionState, ExportedFunctions, Memory, NetworkTopology, NumWasmPages, PageMap, - ReplicatedState, Stream, SubnetTopology, + CanisterStates, ExecutionState, ExportedFunctions, Memory, NetworkTopology, NumWasmPages, + PageMap, ReplicatedState, Stream, SubnetTopology, canister_state::canister_snapshots::CanisterSnapshot, canister_state::{execution_state::WasmBinary, system_state::wasm_chunk_store::WasmChunkStore}, metadata_state::{ @@ -716,6 +716,69 @@ fn last_install_timestamp_survives_a_checkpoint() { }); } +/// `hot_len()` as `CanisterStates::new` derives it from the flat canister set, +/// i.e. the value a replica that loads this state from a checkpoint would see. +fn derived_hot_len(state: &ReplicatedState) -> usize { + let flat: BTreeMap<_, _> = state + .canister_states() + .all_iter() + .map(|(id, canister)| (*id, Arc::clone(canister))) + .collect(); + CanisterStates::new(flat).hot_len() +} + +/// The hot/cold partition must be re-canonicalised on **every** commit, not only +/// on checkpoint rounds. +/// +/// This is a correctness requirement, not a canonicalisation convenience, since +/// `subnet_metrics_instructions` in `rs/execution_environment` charges round +/// instructions proportional to `CanisterStates::hot_len()`. If +/// `repartition_canister_states()` were made conditional — the plausible +/// optimisation being "strictness is only *needed* at checkpoint time, so only do +/// it there" — a replica continuing in memory would carry quiet-but-still-hot +/// canisters into the next round while a replica that restarted from the last +/// checkpoint would load them as cold. Different `hot_len()` means a different +/// charge, which means a different number of subnet messages drained in that +/// round, which is state divergence. +/// +/// So this test commits with `CertificationScope::Metadata` — a *non-checkpoint* +/// round — and asserts the committed partition still equals the derived one. +#[test] +fn hot_cold_partition_is_canonical_after_every_commit() { + state_manager_test(|_metrics, state_manager| { + let canister_id: CanisterId = canister_test_id(100); + let (_height, mut state) = state_manager.take_tip(); + insert_dummy_canister(&mut state, canister_id); + state_manager.commit_and_certify(state, CertificationScope::Metadata, None); + + // Leave behind a stale hot entry, as a round of execution does: taking a + // mutable reference promotes the canister into the `hot` pool without + // giving it any work, so it is hot-by-position but cold-by-predicate. + let (_height, mut state) = state_manager.take_tip(); + assert!(state.canister_state_make_mut(&canister_id).is_some()); + + // Executable precondition: the partition really is stale before the + // commit, so the assertion afterwards is not vacuous. + assert_eq!(state.canister_states().hot_len(), 1); + assert_eq!(derived_hot_len(&state), 0); + + // A non-checkpoint commit. This is the round that a conditional + // repartition would skip. + state_manager.commit_and_certify(state, CertificationScope::Metadata, None); + + let (_height, state) = state_manager.take_tip(); + assert_eq!( + state.canister_states().hot_len(), + derived_hot_len(&state), + "the committed hot/cold partition differs from the one a replica \ + loading this state from a checkpoint would derive; \ + `repartition_canister_states()` must run on every commit, because \ + `subnet_metrics_instructions` charges on `hot_len()`" + ); + assert_eq!(state.canister_states().hot_len(), 0); + }); +} + #[test] fn tip_can_be_recovered_from_metadata_checkpoint() { state_manager_restart_test(|state_manager, restart_fn| { diff --git a/rs/test_utilities/execution_environment/src/lib.rs b/rs/test_utilities/execution_environment/src/lib.rs index 4fffd9477e91..73ac4643bde0 100644 --- a/rs/test_utilities/execution_environment/src/lib.rs +++ b/rs/test_utilities/execution_environment/src/lib.rs @@ -661,6 +661,16 @@ impl ExecutionTest { self.time += duration; } + pub fn current_round(&self) -> ExecutionRound { + self.current_round + } + + /// Sets the round number passed to `execute_subnet_message` and friends, + /// i.e. the block height as seen by the execution environment. + pub fn set_current_round(&mut self, round: u64) { + self.current_round = ExecutionRound::new(round); + } + pub fn ingress_status(&self, message_id: &MessageId) -> IngressStatus { self.state().get_ingress_status(message_id).clone() } @@ -1680,7 +1690,8 @@ impl ExecutionTest { }; let maybe_canister_id = get_effective_canister_id(message.clone()); let is_install_code = check_is_install_code(message.clone()); - let is_list_canisters = check_is_list_canisters(message.clone()); + let consumes_round_instructions_without_effective_canister_id = + check_consumes_round_instructions_without_effective_canister_id(message.clone()); let mut round_limits = RoundLimits { instructions: RoundInstructions::from(i64::MAX), subnet_available_memory: self.subnet_available_memory, @@ -1765,9 +1776,10 @@ impl ExecutionTest { .insert(canister_id, paused_subnet_message); } } - } else if !is_list_canisters { - // `list_canisters` has no effective canister ID but still consumes - // round instructions, so it is exempt from this assertion. + } else if !consumes_round_instructions_without_effective_canister_id { + // `list_canisters` and `subnet_metrics` have no effective canister ID + // but still consume round instructions, so they are exempt from this + // assertion. assert_eq!(slice_instructions_used.get(), 0); } self.check_invariants(); @@ -2776,6 +2788,13 @@ impl ExecutionTestBuilder { self } + /// Sets the initial round number, i.e. the block height as seen by the + /// execution environment. + pub fn with_current_round(mut self, round: u64) -> Self { + self.current_round = ExecutionRound::new(round); + self + } + pub fn with_resource_saturation_scaling(mut self, scaling: usize) -> Self { self.subnet_config.scheduler_config.scheduler_cores = scaling; // If scaling == 1, i.e. a single core is requested in the test, DTS must @@ -3243,13 +3262,20 @@ fn check_is_install_code(message: SubnetMessage) -> bool { message.method_name() == "install_code" || message.method_name() == "install_chunked_code" } -fn check_is_list_canisters(message: SubnetMessage) -> bool { +/// Whether the message is one of the management methods that consume round +/// instructions even though they have no effective canister ID. Their +/// `Ic00MethodPermissions::counts_toward_round_limit` flag is never consulted — +/// `Scheduler::can_execute_subnet_msg` returns before reaching +/// `can_be_executed` — so it cannot be used to identify them, whatever its +/// value. Keep in sync with the special case in +/// `Scheduler::can_execute_subnet_msg`. +fn check_consumes_round_instructions_without_effective_canister_id(message: SubnetMessage) -> bool { let message = match message { SubnetMessage::Response(_) => return false, SubnetMessage::Request(request) => CanisterCall::Request(request), SubnetMessage::Ingress(ingress) => CanisterCall::Ingress(ingress), }; - message.method_name() == "list_canisters" + matches!(message.method_name(), "list_canisters" | "subnet_metrics") } pub fn wat_compilation_cost(wat: &str) -> NumInstructions { diff --git a/rs/tests/execution/general_execution_test.rs b/rs/tests/execution/general_execution_test.rs index c76cb8fec81e..63558c8691df 100644 --- a/rs/tests/execution/general_execution_test.rs +++ b/rs/tests/execution/general_execution_test.rs @@ -4,6 +4,11 @@ use anyhow::Result; use general_execution_tests::api_tests::node_metrics_history_another_subnet_succeeds; use general_execution_tests::api_tests::node_metrics_history_non_existing_subnet_fails; use general_execution_tests::api_tests::node_metrics_history_query_fails; +use general_execution_tests::api_tests::subnet_metrics_another_subnet_succeeds; +use general_execution_tests::api_tests::subnet_metrics_composite_query_fails; +use general_execution_tests::api_tests::subnet_metrics_non_existing_subnet_fails; +use general_execution_tests::api_tests::subnet_metrics_own_subnet_succeeds; +use general_execution_tests::api_tests::subnet_metrics_query_fails; use general_execution_tests::api_tests::test_controller; use general_execution_tests::api_tests::test_cycles_burn; use general_execution_tests::api_tests::test_in_replicated_execution; @@ -44,6 +49,11 @@ fn main() -> Result<()> { .add_test(systest!(node_metrics_history_query_fails)) .add_test(systest!(node_metrics_history_another_subnet_succeeds)) .add_test(systest!(node_metrics_history_non_existing_subnet_fails)) + .add_test(systest!(subnet_metrics_own_subnet_succeeds)) + .add_test(systest!(subnet_metrics_another_subnet_succeeds)) + .add_test(systest!(subnet_metrics_non_existing_subnet_fails)) + .add_test(systest!(subnet_metrics_query_fails)) + .add_test(systest!(subnet_metrics_composite_query_fails)) .add_test(systest!(can_access_big_heap_and_big_stable_memory)) .add_test(systest!(can_access_big_stable_memory)) .add_test(systest!(can_handle_overflows_when_indexing_stable_memory)) diff --git a/rs/tests/execution/general_execution_tests/api_tests.rs b/rs/tests/execution/general_execution_tests/api_tests.rs index 6b230fd6ae37..9506f0de5e44 100644 --- a/rs/tests/execution/general_execution_tests/api_tests.rs +++ b/rs/tests/execution/general_execution_tests/api_tests.rs @@ -220,6 +220,284 @@ pub fn test_cycles_burn(env: TestEnv) { }) } +/// Decodes a `subnet_metrics` reply and returns it, asserting the fields are +/// plausible. +fn decode_subnet_metrics(bytes: &[u8]) -> ic00::SubnetMetricsResponse { + let response = Decode!(bytes, ic00::SubnetMetricsResponse).unwrap(); + // The subnet has processed at least the blocks that carried this call. + assert!(response.block_height > 0_u64); + response +} + +pub fn subnet_metrics_own_subnet_succeeds(env: TestEnv) { + // Arrange. + let (app_node, agent) = setup_app_node_and_agent(&env); + let logger = env.logger(); + let subnet_id = app_node.subnet_id().unwrap().get(); + block_on({ + async move { + let canister = UniversalCanister::new_with_retries( + &agent, + app_node.effective_canister_id(), + &logger, + ) + .await; + // Act. + let result = canister + .update(wasm().call_simple( + ic00::IC_00, + Method::SubnetMetrics, + call_args().other_side(ic00::SubnetMetricsArgs { subnet_id }.encode()), + )) + .await; + // Assert. + let bytes = result.expect("subnet_metrics call failed"); + let response = decode_subnet_metrics(&bytes); + // The universal canister itself is on the subnet, so all three are + // non-zero by the time this call executes. `num_canisters` and + // `update_transactions_total` are written at the end of every round, so + // they are non-zero as soon as the canister exists. + assert!(response.num_canisters > 0_u64); + assert!(response.update_transactions_total > 0_u64); + // `canister_state_bytes` is refreshed only on rounds whose batch number + // is a multiple of 10 (`rs/messaging/src/message_routing.rs`), so unlike + // the other two it legitimately reads 0 for the first rounds after a + // subnet's first canister appears — measured in-process as 0 at heights + // 5 and 9, non-zero from height 17. By the time this test runs the + // subnet is well past that, and this assertion is observed to hold; it + // is noted here as the first thing to look at should this test ever + // start flaking. + assert!(response.canister_state_bytes > 0_u64); + } + }) +} + +/// A canister on the application subnet calls `subnet_metrics` naming a +/// *different* subnet. Message routing delivers the call to that subnet, which +/// executes it and answers with **its own** metrics. +/// +/// The attribution half is what this test is really for, and asserting only that +/// a reply arrives would not test it: a subnet answering a foreign `subnet_id` +/// with its *own* metrics — exactly what the own-subnet check exists to prevent — +/// also replies successfully. So the test perturbs only the *remote* subnet, by +/// installing a canister there, and asserts the remote reading moves. Under that +/// bug the two readings would be local and a remote canister creation could not +/// move them. +/// +/// Note also: unlike `node_metrics_history_another_subnet_succeeds`, which calls +/// `get_first_healthy_application_node_snapshot()` twice and so ends up naming its +/// *own* subnet (the test group's `setup` configures a single application subnet), +/// this test names the verified-application subnet, so the call really does cross +/// a subnet boundary. +pub fn subnet_metrics_another_subnet_succeeds(env: TestEnv) { + // Arrange. + let (app_node, agent) = setup_app_node_and_agent(&env); + let other_node = env.get_first_healthy_verified_application_node_snapshot(); + let other_agent = other_node.build_default_agent(); + let logger = env.logger(); + let other_subnet_id = other_node.subnet_id().unwrap().get(); + assert_ne!(other_subnet_id, app_node.subnet_id().unwrap().get()); + block_on({ + async move { + let canister = UniversalCanister::new_with_retries( + &agent, + app_node.effective_canister_id(), + &logger, + ) + .await; + + let read_remote = || async { + let result = canister + .update( + wasm().call_simple( + ic00::IC_00, + Method::SubnetMetrics, + call_args().other_side( + ic00::SubnetMetricsArgs { + subnet_id: other_subnet_id, + } + .encode(), + ), + ), + ) + .await; + decode_subnet_metrics(&result.expect("cross-subnet subnet_metrics call failed")) + }; + + // Act. + let before = read_remote().await; + // Perturb only the remote subnet. + let _remote_canister = UniversalCanister::new_with_retries( + &other_agent, + other_node.effective_canister_id(), + &logger, + ) + .await; + let after = read_remote().await; + + // Assert: the reply reports the *target* subnet's population, so + // creating a canister there moves it. + // + // Note the direction of the assertion. The tests of this group are + // registered via `SystemTestGroup::add_parallel(SystemTestSubGroup..)` + // in `general_execution_test.rs`, and both of those compose under + // `EvalOrder::Parallel` (`rs/tests/driver/src/driver/group.rs`: + // `add_parallel` → `add_group(_, EvalOrder::Parallel)`, and + // `SystemTestSubGroup::new()` sets `ordering: EvalOrder::Parallel`, + // which `add_test` preserves). So siblings *do* run concurrently and + // can create canisters on the remote subnet meanwhile — but that can + // only make `num_canisters` larger, never smaller, so a strict `>` + // cannot fail spuriously. + assert!( + after.num_canisters > before.num_canisters, + "cross-subnet subnet_metrics did not report the target subnet's \ + canister population: num_canisters was {} before and {} after \ + creating a canister on subnet {other_subnet_id}", + before.num_canisters, + after.num_canisters, + ); + // Sanity: the counters advance on the target subnet too. + assert!(after.block_height > before.block_height); + assert!(after.update_transactions_total > before.update_transactions_total); + } + }) +} + +pub fn subnet_metrics_non_existing_subnet_fails(env: TestEnv) { + // Arrange. + let (app_node, agent) = setup_app_node_and_agent(&env); + let logger = env.logger(); + // Create non existing subnet id. + let subnet_id = PrincipalId::new_subnet_test_id(1); + block_on({ + async move { + let canister = UniversalCanister::new_with_retries( + &agent, + app_node.effective_canister_id(), + &logger, + ) + .await; + // Act. + let result = canister + .update(wasm().call_simple( + ic00::IC_00, + Method::SubnetMetrics, + call_args().other_side(ic00::SubnetMetricsArgs { subnet_id }.encode()), + )) + .await; + // Assert. The universal canister masks the inner `DestinationInvalid` + // reject as a `CanisterReject`. + assert_reject(result, RejectCode::CanisterReject); + } + }) +} + +pub fn subnet_metrics_query_fails(env: TestEnv) { + // Arrange. + let (app_node, agent) = setup_app_node_and_agent(&env); + let logger = env.logger(); + let subnet_id = app_node.subnet_id().unwrap().get(); + block_on({ + async move { + let canister = UniversalCanister::new_with_retries( + &agent, + app_node.effective_canister_id(), + &logger, + ) + .await; + // Act. + let result = canister + .query(wasm().call_simple( + ic00::IC_00, + Method::SubnetMetrics, + call_args().other_side(ic00::SubnetMetricsArgs { subnet_id }.encode()), + )) + .await; + // Assert. Note that this message comes from `ic0.call_new` being + // unavailable in a non-replicated query and is method-agnostic, so + // this test would also pass against a stub implementation. It exists + // for parity with `node_metrics_history_query_fails`; + // `subnet_metrics_composite_query_fails` is the test that actually + // exercises the new code in a query context. + assert_reject_msg( + result, + RejectCode::CanisterError, + "cannot be executed in non replicated query mode", + ); + } + }) +} + +/// Documents what a canister developer actually observes when calling +/// `subnet_metrics` from a composite query. +/// +/// **This test is weak, and deliberately so.** It asserts only that the caller gets +/// no response, which any failure to reply would also produce — it cannot +/// distinguish "stopped by the `subnet_metrics` composite-query arm in +/// `resolve_destination`" from any other reason the canister did not reply, and it +/// would pass against a stub. The test that actually proves the arm is +/// `resolve_subnet_metrics_rejects_composite_query` in +/// `rs/embedders/src/wasmtime_embedder/system_api/routing.rs`, which calls +/// `resolve_destination` directly and asserts its error code and exact message. The +/// division of labour is: **that** unit test proves the arm; **this** system test +/// documents the end-to-end user-visible behaviour. +/// +/// **Why the arm's message cannot be asserted here.** The arm makes +/// `resolve_destination` fail, so the request never becomes a message: +/// `reject_subnet_message_routing` synthesises a reject response into the calling +/// canister's system state, and on the *query* path that response is never delivered +/// back to the callback. The universal canister therefore ends its composite query +/// without replying, and the caller gets `ErrorCode::CanisterDidNotReply` → +/// `RejectCode::CanisterError`, `"Canister did not produce a response"` — +/// never `"subnet_metrics API cannot be called from a composite query"`. +/// +/// That swallowing is **pre-existing platform behaviour of this arm, not something +/// `subnet_metrics` introduced**: `fetch_canister_logs` has the identical arm, ships +/// enabled by default, and behaves the same way. It went unnoticed because there is +/// no end-to-end test of it anywhere in the repo. Improving the error a developer +/// sees would mean changing how routing rejects are delivered on the query path for +/// every ic00 method, which is a platform change and out of scope here. +pub fn subnet_metrics_composite_query_fails(env: TestEnv) { + // Arrange. + let (app_node, agent) = setup_app_node_and_agent(&env); + let logger = env.logger(); + let subnet_id = app_node.subnet_id().unwrap().get(); + block_on({ + async move { + let canister = UniversalCanister::new_with_retries( + &agent, + app_node.effective_canister_id(), + &logger, + ) + .await; + // Act. This is the only path on which `resolve_destination` runs with + // `is_composite_query == true`. + let result = canister + .composite_query( + wasm().call_simple( + ic00::IC_00, + Method::SubnetMetrics, + call_args() + .other_side(ic00::SubnetMetricsArgs { subnet_id }.encode()) + // Kept even though it never fires, so that if the platform + // ever does deliver the routing reject, this test fails + // loudly with the inner message rather than silently + // continuing to assert the swallowed behaviour. + .on_reject(wasm().reject_message().reject()), + ), + ) + .await; + // Assert. See the doc comment: the reject is swallowed, so the + // observable outcome is that the canister produced no response. + assert_reject_msg( + result, + RejectCode::CanisterError, + "did not produce a response", + ); + } + }) +} + pub fn node_metrics_history_query_fails(env: TestEnv) { // Arrange. let (app_node, agent) = setup_app_node_and_agent(&env); diff --git a/rs/types/management_canister_types/src/lib.rs b/rs/types/management_canister_types/src/lib.rs index 1d10bf0cf099..ef4afed3b521 100644 --- a/rs/types/management_canister_types/src/lib.rs +++ b/rs/types/management_canister_types/src/lib.rs @@ -117,6 +117,7 @@ pub enum Method { // Subnet information NodeMetricsHistory, + SubnetMetrics, SubnetInfo, FetchCanisterLogs, @@ -3790,6 +3791,49 @@ pub struct SubnetInfoResponse { impl Payload<'_> for SubnetInfoResponse {} +/// `CandidType` for `SubnetMetricsArgs` +/// ```text +/// record { +/// subnet_id : principal; +/// } +/// ``` +#[derive(Clone, Debug, Default, CandidType, Deserialize)] +pub struct SubnetMetricsArgs { + pub subnet_id: PrincipalId, +} + +impl Payload<'_> for SubnetMetricsArgs {} + +/// `CandidType` for `SubnetMetricsResponse` +/// ```text +/// record { +/// block_height : nat; +/// num_canisters : nat; +/// canister_state_bytes : nat; +/// consumed_cycles_total : nat; +/// update_transactions_total : nat; +/// } +/// ``` +/// +/// Freshness: only `block_height` is current as of the block in which the call is +/// executed. The other four are read from `SystemMetadata::subnet_metrics`, which is +/// written at the *end* of a round, so they are as of the end of the previous round +/// — except `canister_state_bytes`, which message routing recomputes only every 10 +/// rounds (summing it over every canister is expensive and it need not be exact), +/// so it can be up to ten rounds stale and reads as `0` for the first rounds after a +/// subnet's first canister appears. These are the same values, with the same +/// staleness, that `read_state` serves at `/subnet//metrics`. +#[derive(Clone, Debug, Deserialize, CandidType, Serialize, PartialEq)] +pub struct SubnetMetricsResponse { + pub block_height: candid::Nat, + pub num_canisters: candid::Nat, + pub canister_state_bytes: candid::Nat, + pub consumed_cycles_total: candid::Nat, + pub update_transactions_total: candid::Nat, +} + +impl Payload<'_> for SubnetMetricsResponse {} + /// `CandidType` for `NodeMetricsHistoryArgs` /// ```text /// record { diff --git a/rs/types/management_canister_types/tests/candid_equality.rs b/rs/types/management_canister_types/tests/candid_equality.rs index 76e698a4a4e5..1fcca7342ce7 100644 --- a/rs/types/management_canister_types/tests/candid_equality.rs +++ b/rs/types/management_canister_types/tests/candid_equality.rs @@ -16,6 +16,7 @@ type CanisterInfoResult = CanisterInfoResponse; type CanisterMetadataArgs = CanisterMetadataRequest; type CanisterMetadataResult = CanisterMetadataResponse; type SubnetInfoResult = SubnetInfoResponse; +type SubnetMetricsResult = SubnetMetricsResponse; type DeleteCanisterArgs = CanisterIdRecord; type DepositCyclesArgs = CanisterIdRecord; type RawRandResult = Vec; @@ -151,6 +152,11 @@ fn node_metrics_history(_: NodeMetricsHistoryArgs) -> NodeMetricsHistoryResult { unreachable!() } +#[candid_method(update)] +fn subnet_metrics(_: SubnetMetricsArgs) -> SubnetMetricsResult { + unreachable!() +} + #[candid_method(update)] fn provisional_create_canister_with_cycles( _: ProvisionalCreateCanisterWithCyclesArgs, diff --git a/rs/types/management_canister_types/tests/ic.did b/rs/types/management_canister_types/tests/ic.did index 5d242892bc83..00f026e9fc9c 100644 --- a/rs/types/management_canister_types/tests/ic.did +++ b/rs/types/management_canister_types/tests/ic.did @@ -453,6 +453,25 @@ type node_metrics_history_result = vec record { node_metrics : vec node_metrics; }; +type subnet_metrics_args = record { + subnet_id : principal; +}; + +type subnet_metrics_result = record { + // Current block height of the subnet, i.e. the height of the block in + // whose execution this call is processed. + block_height : nat; + // Current number of canisters on the subnet. + num_canisters : nat; + // Current total size in bytes of the state taken by canisters on the subnet. + canister_state_bytes : nat; + // Total cycles removed from circulation on the subnet by all current and + // deleted canisters. + consumed_cycles_total : nat; + // Total number of transactions processed on the subnet. + update_transactions_total : nat; +}; + type subnet_info_args = record { subnet_id : principal; }; @@ -694,6 +713,7 @@ service ic : { // metrics interface node_metrics_history : (node_metrics_history_args) -> (node_metrics_history_result); + subnet_metrics : (subnet_metrics_args) -> (subnet_metrics_result); // subnet info subnet_info : (subnet_info_args) -> (subnet_info_result); diff --git a/rs/types/types/src/messages/ingress_messages.rs b/rs/types/types/src/messages/ingress_messages.rs index 0685491cc8f8..19ee6f7a31e1 100644 --- a/rs/types/types/src/messages/ingress_messages.rs +++ b/rs/types/types/src/messages/ingress_messages.rs @@ -702,6 +702,7 @@ pub fn extract_effective_canister_id( | Ok(Method::BitcoinGetSuccessors) | Ok(Method::BitcoinGetCurrentFeePercentiles) | Ok(Method::NodeMetricsHistory) + | Ok(Method::SubnetMetrics) | Ok(Method::SubnetInfo) | Ok(Method::FetchCanisterLogs) => { // Subnet method not allowed for ingress. diff --git a/rs/types/types/src/messages/inter_canister.rs b/rs/types/types/src/messages/inter_canister.rs index 135635864aa2..fd57f9b31162 100644 --- a/rs/types/types/src/messages/inter_canister.rs +++ b/rs/types/types/src/messages/inter_canister.rs @@ -287,6 +287,7 @@ impl Request { | Ok(Method::BitcoinGetSuccessors) | Ok(Method::BitcoinGetCurrentFeePercentiles) | Ok(Method::NodeMetricsHistory) + | Ok(Method::SubnetMetrics) | Ok(Method::SubnetInfo) => { // No effective canister id. None