feat: Do not route subnet messages to a cooling down subnet - #11230
Open
mraszyk wants to merge 6 commits into
Open
feat: Do not route subnet messages to a cooling down subnet#11230mraszyk wants to merge 6 commits into
mraszyk wants to merge 6 commits into
Conversation
Messages in the subnet's own output queues (only ever responses, as the management canister makes no calls of its own) are no longer routed into streams to a cooling down subnet, as long as this subnet is not cooling down itself. They are retained in their output queue until the destination subnet stops cooling down, rather than being rejected or dropped; and the skipped queue is counted in `mr_cooling_down_skipped_queues`, exactly like a canister output queue. The subnet's own output queues stay exempt from the source side of the check, so that a cooling down subnet can still respond to the calls it has already accepted: while this subnet is cooling down, its own output responses are routed regardless of whether the destination subnet is cooling down, the loopback stream included. Also renames `is_from_subnet_queues` to `is_subnet_output_response`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mraszyk
force-pushed
the
mraszyk/no-subnet-messages-to-cooling-down-subnet
branch
from
August 20, 2026 09:44
fafe9eb to
35e8496
Compare
Corrects the comment on the cooling down check: the subnet's own output responses are exempt for as long as this subnet is cooling down, whether or not the destination subnet is cooling down; they are only held back if this subnet is not cooling down but the destination subnet is. The previous wording claimed they were held back whenever the destination subnet was cooling down, which does not hold. On the test side: drops the `mark_cooling_down()` helper in favor of the existing `new_local_cooling_down_fixture()`; flattens `cooling_down_subnet_message_matrix()` into a list of `(deadline, refund)` pairs; and drops the cases with a management canister as the response originator, as the management canister makes no calls of its own and can therefore never be one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`own_subnet_as_canister_id` is explained by the comment right above it, which does not apply to `own_subnet_is_cooling_down`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`build_streams_routes_subnet_messages_while_cooling_down()` only covered a destination subnet that was not cooling down and the loopback stream. Restore the `mark_cooling_down()` helper and switch back to `new_cooling_down_fixture()` so that a response addressed to a canister on a remote cooling down subnet is covered as well: it is routed all the same, as `LOCAL_SUBNET` is cooling down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents routing subnet-generated responses to cooling-down destinations unless the source subnet is also cooling down.
Changes:
- Adds cooling-down routing logic for subnet output queues.
- Adds response-routing and retention tests.
- Updates metric documentation and naming.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
rs/messaging/src/routing/stream_builder.rs |
Implements subnet-response cooling-down behavior. |
rs/messaging/src/routing/stream_builder/tests.rs |
Adds routing matrix tests and helpers. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
✅ No security or compliance issues detected. Reviewed everything up to b84c239. Security Overview
Detected Code Changes
|
alin-at-dfinity
approved these changes
Aug 21, 2026
| let msg = msg.clone(); | ||
|
|
||
| let is_from_subnet_queues = msg.sender() == own_subnet_as_canister_id; | ||
| let is_subnet_output_response = msg.sender() == own_subnet_as_canister_id; |
Contributor
There was a problem hiding this comment.
Ultimate nitpicking: You could also say msg.sender().get() == self.subnet_id.get(). Then you wouldn't need an explicit own_subnet_as_canister_id.
And regardless, you should probably move own_subnet_as_canister_id's comment here, as it's more relevant here.
Comment on lines
+495
to
+499
| // The subnet's own output responses are exempt for as long as this | ||
| // subnet is cooling down, so that it can still respond to the calls it | ||
| // has already accepted; whether or not the destination subnet is cooling | ||
| // down, the loopback stream included. They are only held back if this | ||
| // subnet is not cooling down but the destination subnet is. |
Contributor
There was a problem hiding this comment.
Suggested change
| // The subnet's own output responses are exempt for as long as this | |
| // subnet is cooling down, so that it can still respond to the calls it | |
| // has already accepted; whether or not the destination subnet is cooling | |
| // down, the loopback stream included. They are only held back if this | |
| // subnet is not cooling down but the destination subnet is. | |
| // Subnet output queues of cooling down subnets (only holding responses) are | |
| // exempt, so they can deliver responses to all the calls they have already | |
| // accepted. |
You can optionally add "... deliver responses (before the subnet is deleted) to all ...".
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.
Messages in the subnet's own output queues (only ever responses, as the
management canister makes no calls of its own) are no longer routed into
streams to a cooling down subnet, as long as this subnet is not cooling down
itself. They are retained in their output queue until the destination subnet
stops cooling down, rather than being rejected or dropped; and the skipped
queue is counted in
mr_cooling_down_skipped_queues, exactly like a canisteroutput queue.
The subnet's own output queues stay exempt from the source side of the check,
so that a cooling down subnet can still respond to the calls it has already
accepted: while this subnet is cooling down, its own output responses are
routed regardless of whether the destination subnet is cooling down, the
loopback stream included.
Also renames
is_from_subnet_queuestois_subnet_output_response.