fix: check the cycles balance and charge executed instructions on DTS resume - #11039
Merged
Conversation
… resume `InstallCodeHelper` records the cycles balance of the clean canister state it was built from and compares it when resuming a paused `install_code` execution, analogously to `ResponseHelper` and `CallOrTaskHelper`. Moreover, if resuming a paused execution fails, then the instructions already executed by the paused Wasm execution are charged now: those instructions consumed round instructions, but the instruction limits are only updated when the Wasm execution finishes and hence such instructions used to be free. For `install_code`, only the instructions of the steps preceding the paused Wasm execution (e.g., compilation) used to be charged; for update calls, replicated queries, canister tasks, and response callbacks, no instructions used to be charged at all; for cleanup callbacks, the instructions of the preceding response callback used to be charged, but not those of the cleanup callback itself. Finally, `dts_replicated_execution_resume_fails_due_to_cycles_change` moves to the general execution tests as `dts_resume_fails_due_to_cycles_decrease`, decreases the cycles balance using `remove_cycles`, and covers response and cleanup callbacks and the heartbeat in addition to update calls and replicated queries; the new test `dts_install_code_resume_fails_due_to_cycles_decrease` covers `install_code`. The tests assert that the instructions reported by the failed execution match the round instructions consumed by its slices and that the canister is charged exactly the cost of those instructions, for which `ExecutionTest` tracks the round instructions consumed per canister. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Ensures failed DTS resumes detect cycle-balance changes and charge instructions already consumed by paused Wasm execution.
Changes:
- Tracks paused Wasm instructions across install, upgrade, call, task, response, and cleanup execution.
- Adds install-code cycle-balance validation.
- Expands accounting utilities and DTS resume tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
rs/test_utilities/execution_environment/src/lib.rs |
Tracks round instructions per canister. |
rs/execution_environment/src/execution/upgrade.rs |
Preserves paused upgrade instruction counts. |
rs/execution_environment/src/execution/response.rs |
Charges failed response and cleanup resumes. |
rs/execution_environment/src/execution/install.rs |
Preserves paused install instruction counts. |
rs/execution_environment/src/execution/install_code/tests.rs |
Tests install resume after cycle reduction. |
rs/execution_environment/src/execution/install_code.rs |
Validates balances and accounts paused instructions. |
rs/execution_environment/src/execution/call_or_task/tests.rs |
Removes superseded narrow DTS test. |
rs/execution_environment/src/execution/call_or_task.rs |
Charges failed call and task resumes. |
rs/execution_environment/src/execution_environment/tests.rs |
Adds broader DTS accounting coverage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…s_decrease The test now asserts, analogously to `dts_resume_fails_due_to_cycles_decrease`, that the cycles for the whole install code instruction limit are prepaid while the execution is paused and that, after the failed resume, the consumed cycles metric and the cycles balance account for exactly the cost of the instructions consumed by the slices of the failed execution, i.e., the rest of the prepaid cycles is refunded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`CallOrTaskHelper` and `ResponseHelper` do not derive the instructions charged for a message from their instruction limits, but from `num_instructions_left` of a finished Wasm execution, so the comments now refer to that instead. Moreover, the resume sites no longer say that the paused Wasm execution is "aborted", which could be confused with aborting a DTS execution: the latter does not charge the instructions of the aborted slices as the message is re-executed from scratch on the preserved prepaid execution cycles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`InstallCodeHelper` reset `executed_wasm_instructions` in-line while `ResponseHelper` used `start_new_wasm_execution`. Both now use the method `reset_executed_wasm_instructions`, whose name and doc state the rule shared by the two call sites: the instructions of the finished Wasm execution are reflected in the instruction limits that have just been updated, so the next Wasm execution starts counting from zero. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ions `ExecutionAccounting` snapshots the counter metric instead of the gauge metric of the cycles consumed for instructions: the counter is only bumped when the prepaid execution cycles are refunded at the end of a message execution, while the gauge is bumped already by the prepayment. Hence, a single snapshot taken right before the first slice of the paused execution suffices, even for a response callback, whose execution is prepaid already when the caller performs the call, and `resume_paused_execution_after_cycles_decrease` takes a single snapshot instead of two. Moreover, a comment clarifies why `dts_install_code_resume_fails_due_to_cycles_decrease` does not compare the executed instructions against the round instructions consumed by the slices. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
✅ No security or compliance issues detected. Reviewed everything up to 607e38f. Security Overview
Detected Code Changes
|
alin-at-dfinity
left a comment
alin-at-dfinity
left a comment
Contributor
There was a problem hiding this comment.
This is taking me quite a bit longer than expected, so here's a first batch of (not hugely consequential) comments.
Each `reset_executed_wasm_instructions` helper had a single call site, so inline the assignment there and fold the rationale from the doc comment into the comment already present at that site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The parameter holds the instructions executed by the slice of the Wasm execution that is being paused (all call sites pass `slice.executed_instructions`), so name it accordingly and spell out in the doc comment that it is added to the instructions executed by the earlier slices of that same Wasm execution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Growing the stable memory after spinning to the slice instruction limit meant that the state change was never even attempted before the resume failed. Doing it first makes the assertion on the stable memory size actually prove that the state changes are dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
alin-at-dfinity
approved these changes
Aug 13, 2026
alin-at-dfinity
left a comment
alin-at-dfinity
left a comment
Contributor
There was a problem hiding this comment.
Turns out I didn't have all that much left to say anyway, except "the same comments as in result.rs, where applicable, if you agree with them".
LGTM
`ExecutionTest` derives the execution cost of a canister from the very same executed instructions that the tests already check, so asserting that the cost matches `execution_cost(executed_instructions)` only restates the test harness bookkeeping. The assertions on the cycles consumed for instructions, which are recorded by the production code, remain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every call site took the snapshot and executed the first slice right before calling the helper, so do both inside it and simplify the API to just the test and the canister. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The counter is bumped only when a message execution completes, which is what distinguishes it from the round instructions tracked next to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Round instructions" did not convey that the counter also accumulates the instructions executed by slices of message executions that have not completed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ExecutionTest` tracked the instructions executed by canister messages and tasks only once their execution finished, which is why a separate counter for the instructions executed by all slices had to be added. Accumulate `executed_instructions` for every executed slice instead and drop that counter: the execution cost, whose base fee must not be charged per slice, keeps being accumulated when an execution finishes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restructure the field comments along the reviewer's suggestion: state what the field holds, then why it is needed, then when it is reset. Also move the comment about charging the instructions of a paused Wasm execution next to the computation of the instructions left on a failed resume. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There is no reason to defer accounting for the instructions of an install_code slice until the execution finishes: only the execution cost must be accumulated per message, so that its base fee is charged exactly once, and it is the cost that is capped at the message instruction limit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restore the backward compatibility note at both guards, phrased in terms of what each of them skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cycles-accounting
…nity/ic into mraszyk/dts-resume-cycles-accounting
mraszyk
enabled auto-merge
August 14, 2026 07:53
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.
InstallCodeHelperrecords the cycles balance of the clean canister state it was built from and compares it when resuming a pausedinstall_codeexecution, analogously toResponseHelperandCallOrTaskHelper.Moreover, if resuming a paused execution fails, then the instructions already executed by the paused Wasm execution are charged now: those instructions consumed round instructions, but the instruction limits are only updated when the Wasm execution finishes and hence such instructions used to be free. For
install_code, only the instructions of the steps preceding the paused Wasm execution (e.g., compilation) used to be charged; for update calls, replicated queries, canister tasks, and response callbacks, no instructions used to be charged at all; for cleanup callbacks, the instructions of the preceding response callback used to be charged, but not those of the cleanup callback itself.Finally,
dts_replicated_execution_resume_fails_due_to_cycles_changemoves to the general execution tests asdts_resume_fails_due_to_cycles_decrease, decreases the cycles balance usingremove_cycles, and covers response and cleanup callbacks and the heartbeat in addition to update calls and replicated queries; the new testdts_install_code_resume_fails_due_to_cycles_decreasecoversinstall_code. The tests assert that the canister is charged exactly the cost of all the executed instructions.