Skip to content

Commit da050af

Browse files
mraszykclaude
andcommitted
docs: state leaving the state unchanged on failure as a requirement
The function's doc comment presented it as a consequence of performing the withdrawal before the refund, and did not mention it at all where the error is documented. State the requirement first and the order as the means, in both the function's and the test's doc comment, without spelling out what the callers do with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0b61445 commit da050af

2 files changed

Lines changed: 8 additions & 16 deletions

File tree

rs/cycles_account_manager/src/cycles_account_manager.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -890,12 +890,14 @@ impl CyclesAccountManager {
890890
/// it ends up with exactly the prepayment that the cost schedule in effect at
891891
/// response time requires.
892892
///
893-
/// The withdrawal is performed before the refund so that a failing withdrawal
894-
/// leaves the canister state unchanged, as the callers expect.
893+
/// A failing adjustment must leave the canister state unchanged: neither the
894+
/// balance nor the consumed cycles metrics may move. The withdrawal is therefore
895+
/// performed before the refund, which cannot fail.
895896
///
896897
/// Returns the prepayment matching the cycles required for executing the response
897898
/// in the given Wasm execution mode, or a `CanisterOutOfCyclesError` if the
898-
/// canister's balance does not cover the additional prepayment.
899+
/// canister's balance does not cover the additional prepayment, in which case
900+
/// the canister state is left unchanged.
899901
pub fn adjust_prepayment_for_response_execution(
900902
&self,
901903
system_state: &mut SystemState,

rs/cycles_account_manager/tests/cycles_account_manager.rs

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -555,19 +555,9 @@ fn response_execution_cycles_match_response_execution_setting() {
555555
}
556556

557557
/// If the canister's balance does not cover the cycles missing from the
558-
/// prepayment, then the adjustment fails and leaves the canister state unchanged.
559-
///
560-
/// Both of its call sites in `rs/execution_environment/src/execution/response.rs`
561-
/// rely on that, since both go on to settle the *unadjusted* prepayment: the
562-
/// `ResponseHelper` method wrapping the adjustment records the adjusted prepayment
563-
/// only on success, so what a failure leaves to be settled is the prepayment
564-
/// recorded in the callback. `execute_response` rejects the response without
565-
/// executing the callback, settling that prepayment in
566-
/// `settle_prepayment_for_unexecuted_response`; `ResponseHelper::resume` turns the
567-
/// failure of replaying the adjustment on the clean canister state into a Wasm
568-
/// execution error, which ends up settling it in `refund_unused_execution_cycles`.
569-
/// Either way, an excess that a failed adjustment had already refunded would be
570-
/// refunded a second time.
558+
/// prepayment, then the adjustment fails and leaves the canister state unchanged:
559+
/// neither the balance nor the consumed cycles metrics move. That is a requirement
560+
/// of `adjust_prepayment_for_response_execution` which its callers rely on.
571561
///
572562
/// In the second setting below the prepayment falls short of the requirement in
573563
/// the real part while exceeding it in the nominal one, so that the excess to be

0 commit comments

Comments
 (0)