feat: Add subnet_metrics management canister endpoint - #11032
Open
Dfinity-Bjoern wants to merge 29 commits into
Open
feat: Add subnet_metrics management canister endpoint#11032Dfinity-Bjoern wants to merge 29 commits into
Dfinity-Bjoern wants to merge 29 commits into
Conversation
Implements the EXPERIMENTAL `subnet_metrics` endpoint from dfinity/developer-docs#333: given a subnet ID, returns that subnet's current block height, canister count, total canister state size, total consumed cycles, and total processed transactions. Canister-callable only; not reachable via ingress. Four of the five values were previously readable only by external users via the certified state tree at /subnet/<subnet_id>/metrics, which canisters cannot read. `block_height` is new: it is `current_round`, already deterministic at execution time (the same value `vetkd_derive_key` already commits to replicated state). `subnet_id` may name any subnet. Routing delivers the call to the named subnet, so the `args.subnet_id == own_subnet_id` check in the handler mirrors `node_metrics_history` and does not block cross-subnet calls; it guards the NNS direct-subnet-addressing path, where a call can reach subnet A while naming subnet B. Notes for future readers, since these are easy to "fix" back: * The instruction charge is keyed on `hot_len()`, NOT `num_canisters()`. The fold in `total_consumed_cycles()` visits hot canisters only, and `hot_len() << len()` is the steady state. Keying on the total over-charges ~41x at 100k canisters, which does not protect the subnet — it lets ~61 calls/round pin the whole shared subnet-message budget and defer install_code/snapshot traffic. Priced against the already enabled `fetch_canister_logs` (2.4 cycles per round-instruction of budget), hot-keyed `subnet_metrics` costs an attacker 3.6. `subnet_metrics_charge_ignores_cold_canisters` fails if this regresses. * `hot_len()` is the first partition-cardinality input to execution, so the unconditional `repartition_canister_states()` call in `commit_and_certify` is now a correctness requirement, not an optimisation. Moving it inside the `CertificationScope::Metadata` branch would diverge the charge across replicas. `hot_cold_partition_is_canonical_after_every_commit` guards this. * `canister_state_bytes` is read from the stored `subnet_metrics` field and must not be recomputed live: the stored value refreshes only every 10 rounds by design, so recomputing would disagree with the certified state tree on 9 rounds out of 10. * `validate_cold_stats()` alerts; it does not enforce. `validate_eq_checkpoint` discards the error and the checkpoint still finalizes. Describe it as detection, not prevention. * The system tests in general_execution_tests/api_tests.rs are Linux-only and could not be compiled locally. CI is their first real check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the experimental subnet_metrics management canister endpoint across the IC execution stack (routing, permissions, metering, and state access), plus adds unit/system tests and benchmarks to pin determinism and cost-model behavior.
Changes:
- Adds
SubnetMetricsas a new management canister method with canister-callable-only enforcement and composite-query rejection. - Implements execution logic returning subnet metrics (including
block_height) with round-instruction charging keyed tohot_len(). - Adds broad regression coverage (execution env, state manager/checkpoint validation, routing tests, candid fixtures, and criterion benches).
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rs/types/types/src/messages/inter_canister.rs | Treats SubnetMetrics as having no effective canister id for canister calls. |
| rs/types/types/src/messages/ingress_messages.rs | Ensures SubnetMetrics is treated as a subnet method (not allowed for ingress). |
| rs/types/management_canister_types/tests/ic.did | Adds Candid fixture types + service method for subnet_metrics. |
| rs/types/management_canister_types/tests/candid_equality.rs | Extends candid-equality coverage with subnet_metrics stub. |
| rs/types/management_canister_types/src/lib.rs | Adds Method::SubnetMetrics and public arg/response Rust types. |
| rs/tests/execution/general_execution_tests/api_tests.rs | Adds end-to-end system tests for own-subnet, cross-subnet, and query/composite-query failure modes. |
| rs/tests/execution/general_execution_test.rs | Registers the new subnet_metrics system tests in the execution test group. |
| rs/test_utilities/execution_environment/src/lib.rs | Exposes and plumbs current_round in the test harness; updates round-instruction special-casing. |
| rs/state_manager/tests/state_manager.rs | Adds a regression test pinning unconditional repartitioning for deterministic hot_len()-keyed charging. |
| rs/state_manager/src/lib.rs | Documents why repartition_canister_states() must remain unconditional. |
| rs/state_manager/src/checkpoint.rs | Adds advisory cold_stats aggregate validation during checkpoint validation and preserves per-canister diagnostics. |
| rs/replicated_state/src/replicated_state.rs | Strengthens documentation for repartitioning as a correctness requirement (not just an optimization). |
| rs/replicated_state/src/canister_states/tests.rs | Adds tests for total_consumed_cycles correctness and validate_cold_stats behavior. |
| rs/replicated_state/src/canister_states.rs | Adds validate_cold_stats() API and documents its advisory validation role. |
| rs/execution_environment/tests/execution_test.rs | Adds instruction-accounting tests for subnet_metrics (budget respect + hot-vs-total scaling + block height progression). |
| rs/execution_environment/src/scheduler.rs | Special-cases SubnetMetrics like ListCanisters for round-limit gating and adds instruction-limit selection entry. |
| rs/execution_environment/src/ic00_permissions.rs | Documents/records that SubnetMetrics bypasses effective-canister-id permission path. |
| rs/execution_environment/src/execution_environment.rs | Implements SubnetMetrics handler, response construction, and hot-keyed round-instruction charge function. |
| rs/execution_environment/src/execution_environment_metrics.rs | Extends metrics labeling to include SubnetMetrics. |
| rs/execution_environment/src/canister_manager/tests.rs | Adds focused unit tests for canister-only access, block height, foreign-subnet rejection, payload decoding, etc. |
| rs/execution_environment/src/canister_manager.rs | Adds SubnetMetrics to ingress-filter rejection arm for defense in depth. |
| rs/execution_environment/benches/management_canister/test_canister/src/main.rs | Adds a test-canister helper method to call subnet_metrics for benchmarks. |
| rs/execution_environment/benches/management_canister/test_canister/candid.did | Updates the benchmark test canister Candid interface with subnet_metrics. |
| rs/execution_environment/benches/management_canister/subnet_metrics.rs | New benchmarks measuring end-to-end and hot-fold costs for subnet_metrics. |
| rs/execution_environment/benches/management_canister/main.rs | Registers the new subnet_metrics benchmark module. |
| rs/embedders/src/wasmtime_embedder/system_api/sandbox_safe_system_state.rs | Allows SubnetMetrics through the system-state-modifications method allowlist. |
| rs/embedders/src/wasmtime_embedder/system_api/routing.rs | Adds routing for SubnetMetrics (payload subnet) and explicit composite-query rejection + tests. |
| rs/canonical_state/src/encoding/tests/subnet_metrics.rs | New cross-check ensuring consumed_cycles_total matches canonical encoding at V29. |
| rs/canonical_state/src/encoding.rs | Registers the new canonical-state subnet-metrics test module. |
| packages/ic-management-canister-types/tests/ic.did | Updates public-package Candid fixture with subnet_metrics types + service method. |
| packages/ic-management-canister-types/tests/candid_equality.rs | Adds candid-equality stub method for subnet_metrics. |
| packages/ic-management-canister-types/src/lib.rs | Adds exported SubnetMetricsArgs / SubnetMetricsResult types (public bindings). |
| packages/ic-management-canister-types/CHANGELOG.md | Notes addition of subnet_metrics types in the package changelog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…reshness Addresses the CI failure and the Copilot review. No production logic changed; this is test code and doc comments only. **Composite-query system test.** `subnet_metrics_composite_query_fails` asserted that the routing rejection's message reaches the caller. It does not: `reject_subnet_message_routing`'s synthesized response is never delivered on the query path, so the universal canister never replies and the outer query fails `CanisterError` / "did not produce a response". This is established platform behaviour of the composite-query arm in `resolve_destination`, not something this change introduced. A control experiment showed `fetch_canister_logs` — which has the identical arm and ships enabled — behaves identically, while `canister_status`, which has no such arm, does deliver its reject (no arm means the request is created and `QueryContext::handle_request`'s reject is delivered normally). The test now asserts the real behaviour and says plainly that this makes it weak: it cannot distinguish the arm from any other failure to reply, and would pass against a stub. The method-specific assertion lives in `resolve_subnet_metrics_rejects_composite_query` in `routing.rs`, which tests `resolve_destination` directly. The division of labour is: the unit test proves the arm, the system test documents user-visible behaviour. The now-inert `.on_reject(...)` is kept deliberately, so that if the platform ever does deliver the reject, the test fails loudly rather than quietly continuing to assert the swallowed behaviour. All five `subnet_metrics` system tests now pass, verified by execution on a Linux host rather than by inspection — including the cross-subnet attribution test, which is the first genuine cross-subnet management-call test in the repo. **Field freshness docs.** Per review, the Rust doc comments described values as "current" when four of the five lag: only `block_height` is current, the other four are as of end-of-previous-round, and `canister_state_bytes` is refreshed only every 10 rounds (so it reads 0 early in a subnet's life). Documented on both `SubnetMetricsResult` and `SubnetMetricsResponse`. The review also asked for the same wording change in the two `ic.did` fixtures. Deliberately not done: those must stay byte-identical to the upstream spec's `public/references/ic.did`. That wording fix belongs in dfinity/developer-docs#333, which already carries an open item on imprecise gauge-vs-counter wording. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three simplifications, no behaviour change. Net -296 insertions, -4 files. **Share the consumed-cycles formula instead of testing for drift.** `SubnetMetrics::consumed_cycles_total_including_canisters()` is now called by both the canonical-state encoder and the `subnet_metrics` handler, so the invariant is structural rather than pinned by a cross-check test. That test (78 lines) and its reciprocal keep-in-sync comments are deleted. The method lives on `SubnetMetrics` rather than `ReplicatedState` because `SubnetMetrics::from` — where the state tree does the addition — has no `ReplicatedState` in scope. Only the `>= V29` branch is rerouted through the new method; the `<= V28` branch still calls `consumed_cycles_total_v28()` untouched, so no state hash at any existing certification version moves. **Drop the `end_to_end` benchmark group.** It was never successfully measured, and the per-canister constant came from `bench_consumed_cycles_fold` instead. Its test-canister plumbing goes with it, restoring `benches/management_canister/test_canister/` to its previous state. The base-cost doc comment now states plainly that the base is estimated from the handler's fixed work and was never measured end to end, rather than pointing at a benchmark that no longer exists. **Move `validate_cold_stats()` out to its own change.** It is hardening for pre-existing code, not a requirement of this endpoint: `ColdStats` is already consensus-critical today via `canister_state_bytes`, with no check at all. The determinism argument for reading `hot_len` does not depend on it — it rests on `is_cold()` being time-independent, the partition never being serialized, unconditional repartitioning at commit, and all four state acquisition paths agreeing. `rs/state_manager/src/checkpoint.rs` and `rs/replicated_state/src/canister_states.rs` are byte-identical to master again. What deliberately stays, because it guards a coupling *this* change introduces rather than the removed check: `hot_cold_partition_is_canonical_after_every_commit`, the `repartition_canister_states` doc comment, and the test that `total_consumed_cycles()` equals a direct fold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
✅ No security or compliance issues detected. Reviewed everything up to 99b7d3a. Security Overview
Detected Code Changes
|
Per review feedback. Behaviour-neutral: `counts_toward_round_limit` is read only in `Ic00MethodPermissions::can_be_executed`, whose single call site in the repo is `scheduler.rs:1847` — and `can_execute_subnet_msg` returns earlier, at the `ListCanisters | SubnetMetrics` special case, so the flag is unreachable for this method. Both affected test targets pass unchanged. The flag now records that the method does consume round instructions. Two comments had to change to keep the tree self-consistent: * The note in `ic00_permissions.rs` no longer says the flag is unset because it is not consulted. It states that the flag is not consulted, and warns that the deferral comes from the dedicated special case in `can_execute_subnet_msg`, which must not be removed on the strength of this flag. * The doc on `check_consumes_round_instructions_without_effective_canister_id` said such methods "cannot use `Ic00MethodPermissions::counts_toward_round_limit`", which is no longer accurate for `subnet_metrics`. It now says the flag is never consulted for them and so cannot identify them whatever its value — true for both entries. `ListCanisters` is in the identical position and remains `false`. Aligning it would be more consistent but changes pre-existing configuration outside this change's scope; the asymmetry is noted in the comment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Master removed the `is_composite_query` parameter from `resolve_destination` and deleted the `FetchCanisterLogs` composite-query arm that `subnet_metrics` was mirroring, so the merge left this arm referencing a parameter that no longer exists. Deleted rather than adapted. Composite-query calls to ic00 no longer reach `resolve_destination` at all: `apply_changes` short-circuits them to the own subnet, where the query handler rejects any method absent from `QueryMethod`. `subnet_metrics` is deliberately absent from that allowlist, so the guarantee the arm provided is now enforced centrally, and a hand-rolled duplicate would be worse than none. The reason the arm existed is preserved as a note on `resolve_subnet_metrics_routes_to_named_subnet`: the query path has no round-instruction accounting, so adding `subnet_metrics` to `QueryMethod` would run the `O(|hot canisters|)` fold unmetered on query threads. `QueryMethod` is now the single place that decision is made. Also removes the obsolete `resolve_subnet_metrics_rejects_composite_query` test and the dropped argument from its sibling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two system tests failed on the merged tree, for unrelated reasons. **Composite query.** Master's refactor changed the mechanism *and improved the result*: composite-query calls to ic00 are short-circuited to the own subnet, where the query handler rejects anything absent from `QueryMethod`. So instead of a swallowed routing reject surfacing as `CanisterError` / "did not produce a response", the caller now gets `CanisterReject` / "Query method subnet_metrics not found." — a method-specific message. That retires the platform wart noted earlier: the reject is no longer lost, and `fetch_canister_logs` benefits identically. The test is no longer weak, so the caveat saying it would pass against a stub is gone. The `on_reject` that was kept in case the platform ever delivered the message is now what carries it. **`canister_state_bytes > 0`.** This assertion was flaky and is now fixed properly. The field refreshes only on batches that are multiples of 10, so whether the first read lands before or after a refresh is a race — it passed on two earlier runs and failed here. It now re-reads until the field is populated, bounded at 30 rounds, so it keeps the coverage rather than dropping it and fails loudly if the field never refreshes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Conflicts resolved: - packages/ic-management-canister-types/CHANGELOG.md: both sides appended independent entries under the same `### Added` heading of 0.9.0; kept both. - rs/state_manager/tests/state_manager.rs: overlapping `ic_replicated_state` import list; took the union of `CanisterStates` (ours) and `ValidatedSnapshotMetadata` (theirs). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`SubnetMetrics::consumed_cycles_total` reports all cycles removed from
circulation on the subnet: a subnet-level aggregate, plus the cycles consumed by
the canisters that currently exist. The subnet-level half is a stored field,
while the canisters' half was folded out of `CanisterStates` by each consumer on
demand -- `O(|hot canisters|)` per read, and evaluated at a different point in
the round than the half it is added to.
Hold the canisters' half in a new `SubnetMetrics::consumed_cycles_by_canisters`,
refreshed once per round, and add `consumed_cycles_total_including_canisters()`
as the single `O(1)` definition of the total. The canonical state tree at
`/subnet/<subnet_id>/metrics` reads that, so `encode_subnet_metrics` no longer
takes the canisters' contribution as a parameter and the `/subnet` subtree does
not need `CanisterStates` at all.
The refresh lives in `StateManagerImpl::commit_and_certify`, beside
`repartition_canister_states()`. That is the one choke point every committed
state passes through, with nothing able to mutate a canister between the refresh
and the hash; refreshing earlier (in message routing or the scheduler) would go
stale, because canisters are still charged afterwards -- HTTP spend refunds,
stream-builder rejects, message shedding -- and because drivers such as
`StateMachine` tests never run message routing at all. Like the repartitioning,
it must therefore stay unconditional.
The field is transient: it is not persisted, is marked `#[validate_eq(Ignore)]`
so that `validate_eq_checkpoint` does not compare it against a checkpoint that
does not carry it, and is derived from the loaded canisters by
`ReplicatedState::new_from_checkpoint`. `SubnetMetrics` derives `ValidateEq` and
`SystemMetadata::subnet_metrics` is compared with `CompareWithValidateEq` so the
exemption covers this field alone and the rest of the struct stays validated
field by field. This follows `SystemMetadata::subnet_ids_at_last_reject_generation`
and `RefundPool::{amounts, total}`.
Deriving at load is what makes a replica restarting from a checkpoint agree with
one that kept running, and hence certify the same
`/subnet/<subnet_id>/metrics` leaf; `consumed_cycles_by_canisters_is_rederived_at_restart`
pins it. The certified encoding is unchanged, as the untouched expected bytes in
`encoding/tests/compatibility.rs` show, and so is the state hash.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ReplicatedStateMetrics::observe` reconstructed the total consumed cycles by folding over the canisters and adding the subnet-level fields itself, duplicating the accounting rules that `SubnetMetrics::consumed_cycles_total_including_canisters()` already encodes for the certified state tree. Set the gauge from that aggregate instead, so the Prometheus value cannot drift from the certified one when a use case changes. The per-use-case breakdowns keep their folds, as no aggregate holds them. `consumed_cycles_by_canisters` is refreshed in `StateManagerImpl::commit_and_certify` right before the observed state is handed off to the metrics thread, so the canisters' half is up to date. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`consumed_cycles_by_canisters_is_rederived_at_restart` compared the hashes returned by `wait_for_checkpoint`, which are manifest hashes over the checkpoint's on-disk files. The aggregate is deliberately not persisted, so the manifest cannot observe it and the comparison did not pin the property the test claimed to. Comparing certified state hashes instead would not help today either: `commit_and_certify` overwrites `certification_version` with `CURRENT_CERTIFICATION_VERSION` (V28), and the `/subnet/<subnet_id>/metrics` leaf only includes `consumed_cycles_by_canisters` from V29 on. Drop the test for now; it can return, comparing the hashes from `list_state_hashes_to_certify`, once `CURRENT_CERTIFICATION_VERSION` is V29. That the leaf covers the field at V29 stays pinned by the traversal test in `rs/canonical_state/src/traversal.rs`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Alin Sinpalean <58422065+alin-at-dfinity@users.noreply.github.com>
Address review feedback: the doc comment on `refresh_consumed_cycles_by_canisters` explained what its caller does and when, which belongs in (and was already duplicated by) the inline comment in `commit_and_certify`. Keep the doc comment to what the method is, and make the caller's comment carry the "must stay unconditional" reasoning on its own. Also drop the observation about ordering against `repartition_canister_states`, which is irrelevant there. Trim the same verbosity from the two neighbours it applies to: the `consumed_cycles_by_canisters` field doc and the gauge comment in `ReplicatedStateMetrics::observe`. Comments only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-metrics-endpoint `SubnetMetrics::consumed_cycles_total_including_canisters()` now reads the stored `consumed_cycles_by_canisters` field instead of taking the canisters' contribution as a parameter, so the `subnet_metrics` management canister method reads it too rather than folding over `CanisterStates`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`subnet_metrics` now reads `SubnetMetrics::consumed_cycles_by_canisters` instead of folding over `CanisterStates`, and `ExecutionTest` never runs `commit_and_certify`, so the field stayed zero. Stand in for that refresh where production performs it. `subnet_metrics_is_partition_independent` refreshes on both sides of the repartitioning: the fold inside the refresh is now the only quantity whose partition-independence is at stake, so refreshing once would compare a stored field against itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every field of the response is a constant-time read of a `SubnetMetrics` field: `consumed_cycles_total_including_canisters()` reads the stored `consumed_cycles_by_canisters` rather than folding over `CanisterStates`, so there is no work left to price. Drop `subnet_metrics_instructions` and its cost model, record `SubnetMetrics` as `counts_toward_round_limit: false` alongside `SubnetInfo`, and remove it from the `can_execute_subnet_msg` special case that defers instruction-consuming methods without an effective canister ID. `list_canisters` keeps that arm. Remove the tests and benchmarks that priced the fold: `subnet_metrics_respects_round_instruction_limit`, `subnet_metrics_charge_ignores_cold_canisters` and the `subnet_metrics_consumed_cycles_fold` benchmark group. The block-height assertion bundled into `subnet_metrics_charges_round_instructions` is not about charging and is not covered elsewhere, so it survives as `subnet_metrics_block_height_tracks_block_height`. Dropping `subnet_metrics` from `check_consumes_round_instructions_without_effective_canister_id` makes the existing tests assert the absence of a charge, as the harness now requires `slice_instructions_used == 0` for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The own-subnet, query and composite-query cases are covered by the unit
tests in `rs/execution_environment/src/canister_manager/tests.rs`:
`subnet_metrics_canister_call_succeeds` and
`subnet_metrics_reflects_subnet_metrics_state` for the reply,
`subnet_metrics_block_height_{matches_current_round,is_non_decreasing}`
for the height, and `subnet_metrics_ingress_query_fails` for the
`QueryMethod` allowlist -- which asserts the same method-specific message
the composite-query test did. `subnet_metrics_query_fails` asserted a
method-agnostic message and said so itself.
Two things the unit tests do not cover, so they move into the surviving
cross-subnet test rather than being dropped with it:
- `canister_state_bytes`, which the unit tests set by hand. Only a running
subnet exercises the message-routing refresh that writes it, and that
refresh lands only on batches that are a multiple of 10, hence the
re-read loop.
- `num_canisters` and `update_transactions_total` being non-zero, which the
existing strict comparisons against a non-negative `before` already
imply.
The no-route rejection that `subnet_metrics_non_existing_subnet_fails`
exercised is *not* covered: `subnet_metrics_foreign_subnet_id_is_rejected`
injects into the subnet queue and so bypasses routing. It is
method-agnostic message-routing behaviour.
Inline `decode_subnet_metrics` into its single remaining caller.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three sites argued that `repartition_canister_states()` must run on every commit because `subnet_metrics` charges round instructions proportional to `CanisterStates::hot_len()`. That charge is gone, so the argument no longer holds -- and one of them named the deleted `subnet_metrics_instructions` by function name. The requirement that survives is checkpoint validation: `CanisterStates::validate_strict_split()`, run from `rs/state_manager/src/checkpoint.rs`, rejects a canister left in `hot` that satisfies `is_cold()`. Repartitioning unconditionally meets it without depending on `batch_summary` to predict which rounds checkpoint. State the negative explicitly, since it is what the old text got wrong: no execution result depends on where the split lies. Every consumer is `fold(hot) + cold aggregate` and so partition-independent, and `hot_len()` is read only by the `hot_canisters_count` metric. A skipped repartition could not diverge state; it would only leave a stale partition for the next checkpoint to reject. `hot_cold_partition_is_canonical_after_every_commit` therefore guards an implementation choice, not a divergence, and its doc says so instead of claiming otherwise. It is kept because it is cheap and would catch a future consumer keying an execution result on `hot_len()`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `subnet_metrics` endpoint reads stored `SubnetMetrics` fields and folds over nothing, so it no longer depends on the hot/cold partition and has no reason to carry tests for it: - `hot_cold_partition_is_canonical_after_every_commit` (and its `derived_hot_len` helper) asserted a property whose only stated consumer was the removed `hot_len()`-keyed charge. - `total_consumed_cycles_equals_direct_fold` and `for_each_mut_keeps_cold_stats_consumed_cycles_in_sync` (and their `consume_cycles` / `direct_consumed_cycles_fold` helpers) covered the `cold_stats` aggregate. - `subnet_metrics_is_partition_independent` covered a fold the handler no longer performs. `total_consumed_cycles_combines_hot_and_cold` and the `validate_strict_split` tests are untouched: they predate this branch. Both test files are now identical to their pre-branch state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With the endpoint charging no instructions and folding over nothing, several edits this branch made outside it no longer carry their weight: - `replicated_state.rs` and `state_manager/src/lib.rs` are reverted to their pre-branch state. Their added commentary justified the unconditional `repartition_canister_states()` call, which this branch no longer has any stake in; the explanation that was already there stands on its own. - `check_consumes_round_instructions_without_effective_canister_id` goes back to `check_is_list_canisters`, along with its doc comment and the local variable. The general name was introduced when `subnet_metrics` was a second member of that set; it is not one now, and the body tests `list_canisters` alone, so the name described a category with a single member. - `ExecutionTest::current_round()` is removed: it has no callers. An unused `pub fn` in a library crate draws no warning, so this needed a grep rather than the compiler. `set_current_round` / `with_current_round` are used by the block-height tests and stay. - The `can_execute_subnet_msg` comment in `scheduler.rs` is restored to its previous wording; the edit had only rewrapped identical text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oint `consumed_cycles_total_including_canisters` conflicted only in its doc comment: master landed #11297 (the same work this branch had merged from `mraszyk/store-consumed-cycles-by-canisters`) listing two consumers of the definition, while this branch lists three. Kept ours -- the `subnet_metrics` management canister method is the third consumer, and this branch is what adds it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Address review feedback on the `subnet_metrics` endpoint. Tests. The five aggregate/height assertions that `ExecutionTest` could only make against hand-set fields now live in one `StateMachine` test, `subnet_metrics_reports_the_subnets_metrics`, grown out of the former block-height test: it pins `block_height` to the exact round the call is processed in, `num_canisters` in both directions (create *and* delete), `canister_state_bytes` against an independently computed `total_canister_memory_usage()` -- idling to a multiple-of-10 height first, so the stored value is the quiescent one and no refresh lands mid-read -- and `update_transactions_total` to the snapshot plus the one ingress executed since. `consumed_cycles_total` gets an ordering assertion only: while the call is in flight the caller's response prepayment counts as consumed, so the reported total is *above* the post-call total, and an upper bound would be wrong. That makes `subnet_metrics_canister_call_succeeds`, `subnet_metrics_block_height_matches_current_round`, `subnet_metrics_block_height_is_non_decreasing` and `subnet_metrics_reflects_subnet_metrics_state` redundant, so they go, and with them the `with_current_round`/`set_current_round` hooks in `rs/test_utilities/execution_environment` -- that file is back to master. `resolve_subnet_metrics_routes_to_named_subnet` goes too, there being no analogous `node_metrics_history` test in `routing.rs`. What remains in `canister_manager/tests.rs` is the rejection paths, with `subnet_metrics_raw_call` now asserting that the response is the only message crossing the subnet boundary. The cross-subnet system test moves to a target of its own, `//rs/tests/execution:subnet_metrics_test`, so it has an IC to itself and nothing else creates or deletes canisters on the remote subnet meanwhile; `num_canisters` is therefore pinned with `assert_eq!` rather than a strict inequality. It creates the remote canister with the retry-free `create_and_install`, as `UniversalCanister::new_with_retries` retries creation and installation together and a failed install would leave a canister behind. Docs. Both `ic.did` copies now carry identical `subnet_metrics_args` and `subnet_metrics_result` blocks whose comments say what the Rust doc comments in both `management_canister_types` crates say: the one-round lag, the 10-round `canister_state_bytes` refresh, and that it reads as 0 for the first rounds after the subnet is created. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The entry went into the `[0.9.0]` section, which the release commit 0b55412 dated and closed out on 2026-08-13, so adding to it retroactively edits a published changelog. Put it under `[Unreleased]`, which was sitting empty right above, and use the two-level form the file already uses for the same kind of entry (`Types for list_canisters:` and `Types for canister_metrics:` in 0.8.0). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`subnet_metrics` recomputed the consumed-cycles total while executing, which made it disagree with the certified state tree it is supposed to mirror. The canisters' part of the total is only refreshed when a state is committed, so recomputing mid-round mixes that stale part with subnet-level accumulators that move as the round proceeds. Concretely, a `delete_canister` drained earlier in the same round adds the deleted canister's consumption to `consumed_cycles_by_deleted_canisters` at once, while the canisters' part still counts it -- the endpoint reported that canister twice. Replace the transient `SubnetMetrics::consumed_cycles_by_canisters` with a single transient `consumed_cycles_total_including_canisters` holding the whole total, subnet plus canisters, as of the last committed state. `ReplicatedState::refresh_consumed_cycles` (renamed from `refresh_consumed_cycles_by_canisters`) computes it on every `commit_and_certify`, and `new_from_checkpoint` re-derives it, so a replica restarting from a checkpoint agrees with one that keeps running. Every consumer of the full total now reads that one field -- the certified tree at `/subnet/<subnet_id>/metrics` from certification version `V29`, the `subnet_metrics` method, and the `replicated_state_consumed_cycles_since_replica_started` gauge -- so they cannot drift, and the method that used to compute the sum is gone. The certified encoding is unchanged: the byte-exact expectations in `encoding/tests/compatibility.rs` and the `V29` hash in `state_manager/src/tree_hash.rs` still hold, with their fixtures now setting the stored aggregate instead of the canisters' part. The gauge does now depend on the refresh having run, which in production it has, since `commit_and_certify` enqueues the observation afterwards; the scheduler metrics tests, which never commit a state, refresh explicitly. `subnet_metrics_consumed_cycles_total_is_the_committed_aggregate` pins the fix: one update issues `delete_canister` and then `subnet_metrics`, so both requests sit in the caller's output queue to `ic00` in that order and are drained in the same round, the deletion first. Reverted to a recomputation the test fails, reporting the victim's consumption twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Master landed #11335, the upstream version of this branch's consumed-cycles refactor, which makes `SubnetMetrics::consumed_cycles_total_including_canisters` a private field behind a getter written only by `SubnetMetrics::refresh_consumed_cycles`. Every conflict is between that and this branch's own variant of the same change, so master's version wins throughout; the `subnet_metrics` endpoint and its tests now read the total through the getter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
schneiderstefan
approved these changes
Aug 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a
subnet_metricsmanagement canister method, so that a canister can read asubnet's aggregate metrics with an ordinary inter-canister call instead of a
read_statecall and a certificate to verify:The four aggregates come from the same
SystemMetadata::subnet_metricsthecertified state tree at
/subnet/<subnet_id>/metricsencodes, so the two reportthe same numbers -- with the caveat that
consumed_cycles_totalis the correctedV29total, which the tree only reports once subnets certify atV29(they areat
V28today).block_height, which has noread_statecounterpart, is theheight of the block in whose execution the call is processed; it alone is
current, as the aggregates are written at the end of a round, and
canister_state_bytesis only recomputed every 10 rounds. The types and bothic.didfixtures document that staleness, and mark the method EXPERIMENTAL.The method is callable by canisters only and answers only for the subnet
executing it, but it is routed by the
subnet_idin its argument, so a canistercan read another subnet's metrics. Like the other read-only subnet queries it is
free: no cycles charged, no round instructions consumed.
Types are added for the replica (
rs/types/management_canister_types) and in thepublic
ic-management-canister-typespackage, with candid-equality testscovering both. Tests cover the rejections (ingress, query, foreign
subnet_id,malformed payload), the semantics of every field on a running subnet, and the
cross-subnet path as a system test.