feat: allow composite queries to call the management canister query methods - #10987
Open
mraszyk wants to merge 8 commits into
Open
feat: allow composite queries to call the management canister query methods#10987mraszyk wants to merge 8 commits into
mraszyk wants to merge 8 commits into
Conversation
…ery methods Composite queries can now call the management canister methods that can be served from a state snapshot in the non-replicated mode, i.e. the methods listed in `QueryMethod`: `fetch_canister_logs`, `canister_status`, `list_canisters` and `canister_metrics`. The calls are executed against the state of the own subnet on behalf of the calling canister, so they can only target canisters hosted by the own subnet and they are subject to the same access control as the corresponding queries sent by end users. The feature is gated behind the (disabled by default) config flag `composite_query_ic00_calls` to enable a gradual rollout. Implementation notes: - The execution of the management canister query methods is factored out of the user query path into the new `subnet_query` module which is shared by both the user query path and the new composite query path. - Requests to ic:00 made by a composite query in the non-replicated mode are no longer routed based on the method and the payload, but resolved to the own subnet and handled by the query handler. Consequently, the flag telling `resolve_destination` about composite queries only refers to non-replicated composite queries now and is thus renamed accordingly. Composite queries executed in the replicated mode route their calls to ic:00 like any other replicated message, which also means that `fetch_canister_logs` is now accessible to them subject to `replicated_inter_canister_log_fetch`. - The instructions consumed while producing the reply (`list_canisters` is linear in the number of canisters on the subnet) are charged towards the instruction limit of the whole call graph, in addition to the base overhead charged for every query call. - Query contexts calling the management canister are never cached: their results may depend on parts of the state that the query cache does not track, e.g. the cycles balance of a canister other than the queried one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Requests to the management canister made by a composite query are now resolved to the own subnet at the call site of `resolve_destination` instead of inside it: `resolve_destination` no longer takes a flag telling it about composite queries and is not called for composite queries at all. Also fix comments to refer to the management canister instead of ic:00 and to state that composite queries are always executed in the non-replicated mode (a composite query called in the replicated mode is rejected before its execution), rather than claiming that replicated composite queries route their calls to the management canister like any other replicated message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…mposite_query Every composite query is executed in the non-replicated mode, so the qualifier is redundant: the flag is now only used to decide whether a request to the management canister is resolved to the own subnet instead of being routed based on the method and the payload. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e management canister - Charge the base overhead of a query call to the management canister only after rejecting calls to methods that are not management canister query methods, in the same way as calls to non-existing methods of a canister are not charged. - Reject calls to management canister methods that cannot be executed in the non-replicated mode with the error of `parse_query_method`, i.e. the same error as for such a query sent by an end user to the management canister. - Charge the same number of instructions for `fetch_canister_logs` in the non-replicated mode as in the replicated mode by introducing the shared helper `fetch_canister_logs_reply` (analogously to `list_canisters`). - Test that the result of a composite query is not cached if the management canister is called from a reply callback of a nested composite query call. - Do not test `list_canisters` itself in the composite query test and tighten the instruction limit test to the number of calls the limit is set up for. - Fix comments: query methods can also be executed in the replicated mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…omposite query call are not cached Complements the test for a call to the management canister made from a reply callback: here the call is made by the callee of a nested composite query call, i.e. one level down in the call graph of the query context whose result must not be cached. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…call to the management canister Assert the exact reject message including the own subnet ID instead of just checking that the message contains "not found": the latter would also pass if the call failed because the canister whose logs are fetched was not found. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ry methods The instructions consumed while producing the reply of a management canister query method are still charged towards the instruction limit of the whole call graph, but they are no longer added to the instruction observation and the measurement scope: - the instruction observation is only used to charge the instructions of a canister http transform, and a composite query cannot be used as a transform (see `QueryContext::run`), so the management canister cannot be called on that path at all; - the measurement scope tracks the executions of canister messages, while the execution of a management canister query method is already observed by `observe_subnet_query_message`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enables composite queries to call selected management canister query methods (fetch_canister_logs, canister_status, list_canisters, canister_metrics) by routing such calls to the caller’s own subnet and executing them against local subnet state, gated behind the disabled-by-default composite_query_ic00_calls flag.
Changes:
- Factor management-canister query execution into a shared
subnet_querymodule and reuse it from both end-user query handling and composite-query handling. - Route composite-query
ic:00calls to the local subnet and execute them in the query handler, including instruction accounting and metrics. - Ensure composite-query contexts that call the management canister are never cached; add/extend tests and metrics to cover the new behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| rs/test_utilities/execution_environment/src/lib.rs | Adds a test-builder helper to enable the composite_query_ic00_calls feature flag. |
| rs/execution_environment/tests/canister_logging.rs | Updates state-machine tests for fetch_canister_logs behavior when composite-query ic00 calls are enabled/disabled. |
| rs/execution_environment/src/query_handler/tests.rs | Adds query-handler tests covering composite-query calls into management canister query methods and instruction charging. |
| rs/execution_environment/src/query_handler/subnet_query.rs | Introduces a shared implementation for executing management canister query methods against a local subnet state. |
| rs/execution_environment/src/query_handler/query_context.rs | Adds composite-query handling for routed-to-subnet management calls, instruction charging, and tracks ic00_calls for cache bypass. |
| rs/execution_environment/src/query_handler/query_cache/tests.rs | Adds tests asserting composite-query results are never cached when management canister is called (including nested/callback cases). |
| rs/execution_environment/src/query_handler/query_cache.rs | Skips caching when ic00_calls > 0 and adds a dedicated invalidation metric. |
| rs/execution_environment/src/query_handler.rs | Refactors management-canister query handling to use subnet_query and wires ic00_calls into cache insertion. |
| rs/execution_environment/src/metrics.rs | Adjusts subnet-query observation to accept encoded replies (Vec<u8>) rather than WasmResult. |
| rs/execution_environment/src/execution/response.rs | Removes composite-query origin checks in replicated response handling (composite queries handled via query handler). |
| rs/execution_environment/src/execution/call_or_task.rs | Removes composite-query detection in replicated execution paths (composite queries are non-replicated). |
| rs/execution_environment/src/canister_logs.rs | Factors out fetch_canister_logs_reply to share reply generation + instruction accounting across replicated/non-replicated paths. |
| rs/embedders/src/wasmtime_embedder/system_api/sandbox_safe_system_state.rs | Routes composite-query management-canister requests to the local subnet instead of payload-based routing. |
| rs/embedders/src/wasmtime_embedder/system_api/routing.rs | Simplifies management-canister routing API and removes composite-query special-casing from routing logic. |
| rs/config/src/execution_environment.rs | Adds the composite_query_ic00_calls config flag (disabled by default) and documents it. |
Suppressed comments (1)
rs/execution_environment/tests/canister_logging.rs:487
- Typo in comment: "relatioship" → "relationship".
// There are 3 actors with the following controller relatioship: user -> canister_a -> canister_b.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
✅ No security or compliance issues detected. Reviewed everything up to ad9c3bf. Security Overview
Detected Code Changes
|
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.
Composite queries can now call the management canister methods that can be invoked by users as non-replicated query calls, i.e. the methods listed in
QueryMethod:fetch_canister_logs,canister_status,list_canistersandcanister_metrics. The calls are executed against the state of the own subnet on behalf of the calling canister, so they can only target canisters hosted by the own subnet and they are subject to the same access control as the corresponding queries sent by end users.The feature is gated behind the (disabled by default) config flag
composite_query_ic00_callsto enable a gradual rollout.Implementation notes:
subnet_querymodule which is shared by both the user query path and the new composite query path.fetch_canister_logsandlist_canisters) are charged towards the instruction limit of the whole call graph, in addition to the base overhead charged for every query call.