Skip to content

chore: Testing out AI instructions for shorter comments #11438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
alin-at-dfinity wants to merge 1 commit into
base: master
Choose a base branch
from
+42 −102
Draft

chore: Testing out AI instructions for shorter comments #11438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 22 additions & 47 deletions rs/cycles_account_manager/src/cycles_account_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -849,30 +849,19 @@ impl CyclesAccountManager {
)
}

/// Adjusts the cycles prepaid for the execution of a response so that they match
/// exactly the cycles required for executing the response in the given Wasm
/// execution mode.
/// Adjusts the cycles prepaid for a response execution to those required in the
/// given Wasm execution mode and returns them, withdrawing any shortfall from
/// the canister or refunding it any excess.
///
/// The cycles for a response execution are prepaid when the corresponding call
/// is performed, i.e., using the instruction costs of the Wasm execution mode
/// of the calling canister at that time. The canister might have been upgraded
/// to a different Wasm execution mode before the response arrives:
/// The cycles were prepaid when the corresponding call was performed, at the
/// instruction costs of the Wasm execution mode the canister had back then; an
/// upgrade since may have changed that mode. Adjusting makes the canister pay
/// for the instructions it executes at the costs of the mode it executes them
/// in.
///
/// - if the prepayment falls short of the requirement (the canister was upgraded
/// to a more expensive Wasm execution mode), then the missing cycles are
/// withdrawn from the canister's balance. No freezing threshold is applied:
/// the canister already committed to executing the response when it performed
/// the corresponding call;
/// - if the prepayment exceeds the requirement (the canister was upgraded to a
/// cheaper Wasm execution mode), then the excess is refunded immediately.
///
/// Matching the prepayment to the requirement lets the canister pay exactly for
/// the instructions it executed, at the instruction costs of the Wasm execution
/// mode it executed them in.
///
/// Returns the prepayment matching the cycles required for executing the response
/// in the given Wasm execution mode, or a `CanisterOutOfCyclesError` if the
/// canister's balance does not cover the additional prepayment.
/// The withdrawal ignores the freezing threshold, as the canister committed to
/// executing the response when it performed the call. It leaves the canister
/// untouched and fails if the balance does not cover the shortfall.
pub fn adjust_prepayment_for_response_execution(
&self,
system_state: &mut SystemState,
Expand All @@ -884,7 +873,6 @@ impl CyclesAccountManager {
let required = self.prepayment_for_response_execution(subnet_cycles_config, execution_mode);
let prepaid = prepayment_for_response_execution;
if prepaid.real() < required.real() {
// No freezing threshold is applied, i.e., the threshold is zero.
self.consume_with_threshold_impl(
system_state,
required - prepaid,
Expand All @@ -901,13 +889,8 @@ impl CyclesAccountManager {
))
}

/// Refunds the part of the cycles prepaid for the execution of a response that
/// exceeds the cycles required for executing the response in the given Wasm
/// execution mode and returns the remaining prepayment.
///
/// Unlike `adjust_prepayment_for_response_execution`, which uses this function to
/// handle an excessive prepayment, this never withdraws cycles from the canister's
/// balance and hence it cannot fail.
/// Refunds the part of the cycles prepaid for a response execution that exceeds
/// the cycles required in the given Wasm execution mode and returns the rest.
fn refund_excess_prepayment_for_response_execution(
&self,
system_state: &mut SystemState,
Expand All @@ -919,41 +902,33 @@ impl CyclesAccountManager {
if prepayment_for_response_execution.real() <= required.real() {
return prepayment_for_response_execution;
}
// The excess part of the prepayment is refunded in full and hence it does not
// contribute to the consumed cycles of the canister.
// Refunded in full, so the excess never counts as consumed by the canister.
let excess = prepayment_for_response_execution - required;
system_state.refund_cycles(excess, excess);
required
}

/// Settles the cycles prepaid for the execution of a response whose callback is
/// not executed at all: the canister is charged the fixed per-message execution
/// fee and the rest of the prepayment is refunded to it.
///
/// Since no instructions are executed, the fixed per-message execution fee is all
/// that is due, no matter which Wasm execution mode the cycles were prepaid for
/// and which one the canister has now. In particular, the canister keeps the rest
/// of its prepayment even if the prepayment falls short of the cycles that
/// executing the response in its current Wasm execution mode would require.
/// Settles the cycles prepaid for a response whose callback is not executed at
/// all, charging the fixed per-message execution fee and refunding the rest.
///
/// Note that the prepayment is never topped up for such a response: the additional
/// cycles would be refunded right away and, unlike this refund, the withdrawal
/// could fail.
/// With no instructions executed, that fee is all that is due, whichever Wasm
/// execution mode the cycles were prepaid for and whichever one the canister
/// has now. The prepayment is deliberately not adjusted first: topping it up
/// would refund the additional cycles right away and, unlike this refund, the
/// withdrawal could fail.
pub fn settle_prepayment_for_unexecuted_response(
&self,
system_state: &mut SystemState,
prepayment_for_response_execution: CompoundCycles<Instructions>,
subnet_cycles_config: CyclesAccountManagerSubnetConfig,
execution_mode: WasmExecutionMode,
) {
// Executing no instructions costs the fixed per-message execution fee only.
let base_fee = self.execution_cost(
NumInstructions::from(0),
subnet_cycles_config,
execution_mode,
);
// The prepayment covers the fixed per-message execution fee, but clamp the
// charge to it so that no more than the prepayment is ever charged.
// A prepayment made in a cheaper mode may fall short of this mode's fee.
let charge = base_fee.min(prepayment_for_response_execution);
system_state.refund_cycles(
prepayment_for_response_execution,
Expand Down
75 changes: 20 additions & 55 deletions rs/execution_environment/src/execution/response.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,9 @@ struct ResponseHelper {
prepayment_for_response_transmission: CompoundCycles<RequestAndResponseTransmission>,
prepayment_for_call_transmission: CompoundCycles<RequestAndResponseTransmission>,
refund_for_response_transmission: CompoundCycles<RequestAndResponseTransmission>,
/// Cycles prepaid for the execution of this response.
///
/// Initially the prepayment recorded in the callback when the corresponding call
/// was performed, i.e., the cycles required for executing the response in the
/// Wasm execution mode the canister had at that time. Before the callback is
/// executed, `adjust_prepayment_for_response_execution()` replaces this with the
/// cycles required for executing the response in the canister's current Wasm
/// execution mode; for responses whose callback is not executed at all,
/// `early_finish()` settles the prepayment in full instead.
/// Cycles prepaid for the execution of this response, taken from the callback
/// and adjusted to the canister's current Wasm execution mode before the
/// callback runs.
prepayment_for_response_execution: CompoundCycles<Instructions>,
initial_cycles_balance: Cycles,
response_sender: CanisterId,
Expand Down Expand Up @@ -255,29 +249,18 @@ impl ResponseHelper {
);
}

/// Returns the Wasm execution mode of the canister executing this response.
///
/// This is the mode of the code that actually runs the callback, which is not
/// necessarily the mode the canister had when it performed the corresponding
/// call: the canister might have been upgraded in the meantime.
/// Returns the Wasm execution mode of the code that runs the callback, which is
/// not necessarily the one the canister had when it performed the call.
fn wasm_execution_mode(&self) -> WasmExecutionMode {
self.canister
.execution_state
.as_ref()
.map_or(WasmExecutionMode::Wasm32, |state| state.wasm_execution_mode)
}

/// Adjusts the cycles prepaid for the execution of this response to the cycles
/// required for executing it in the canister's current Wasm execution mode.
///
/// The cycles were prepaid when the corresponding call was performed, i.e.,
/// using the instruction costs of the Wasm execution mode of the canister at
/// that time. If the canister has been upgraded since then, then the missing
/// cycles are withdrawn from the canister's balance (without applying the
/// freezing threshold) or the excess cycles are refunded to it.
///
/// Returns an error if the canister's balance does not cover the additional
/// prepayment, in which case the canister state is left unchanged.
/// Adjusts `prepayment_for_response_execution` to the canister's current Wasm
/// execution mode, see
/// `CyclesAccountManager::adjust_prepayment_for_response_execution()`.
fn adjust_prepayment_for_response_execution(
&mut self,
original: &OriginalContext,
Expand Down Expand Up @@ -456,21 +439,11 @@ impl ResponseHelper {
return Err((helper, err));
}

// Replay the adjustment of the prepayment for the response execution: the
// initial steps are replayed on the clean canister state, which does not
// contain the changes of the ongoing DTS execution.
//
// The required prepayment is the same as in `execute_response()`: the Wasm
// module, and hence its Wasm execution mode, cannot change while a DTS
// execution of this canister is in progress because installing code is not
// executed for a canister that has a paused or an aborted execution (see
// `can_execute_subnet_msg()`), i.e., it is deferred to a later round. Should
// the paused execution be aborted nevertheless, e.g., before a checkpoint,
// then the response execution starts over in `execute_response()`, where the
// prepayment is adjusted afresh for whatever module is installed by then.
//
// Together with the check above that the cycles balance of the clean canister
// has not decreased, this replay is therefore expected to succeed.
// The adjustment is not persisted in the callback, so it must be replayed on
// the clean canister state. It is expected to succeed: the balance has not
// decreased (checked above) and the Wasm module cannot change while a DTS
// execution is in progress, as installing code is deferred for a canister
// with a paused or an aborted execution (see `can_execute_subnet_msg()`).
if let Err(err) = helper.adjust_prepayment_for_response_execution(original, round) {
let msg = format!("Failed to prepay for resuming a response call: {err}");
let err = HypervisorError::WasmEngineError(FailedToApplySystemChanges(msg));
Expand Down Expand Up @@ -735,15 +708,9 @@ impl ResponseHelper {
/// Completes execution of the response and cleanup callbacks without
/// consuming any instructions and without producing any heap delta.
///
/// No Wasm code is executed at all, hence the fixed per-message execution fee is
/// all that is due: it is charged here and the rest of the cycles prepaid for the
/// response execution is refunded, independently of the Wasm execution mode the
/// cycles were prepaid for and of the one the canister has now. In particular,
/// the canister keeps the rest of its prepayment even if the prepayment falls
/// short of what its current Wasm execution mode requires.
///
/// Note that the prepayment is never topped up here: the additional cycles would
/// be refunded right away and, unlike this refund, the withdrawal could fail.
/// The prepayment for the response execution is settled here instead of being
/// adjusted, see
/// `CyclesAccountManager::settle_prepayment_for_unexecuted_response()`.
fn early_finish(
mut self,
result: Result<Option<WasmResult>, HypervisorError>,
Expand All @@ -760,8 +727,7 @@ impl ResponseHelper {
original.subnet_cycles_config,
execution_mode,
);
// The prepayment has been settled in full, hence there is nothing left for
// `finish()` to settle.
// Settled in full, so `finish()` has nothing left to refund.
self.prepayment_for_response_execution =
CompoundCycles::new(Cycles::zero(), original.subnet_cycles_config.cost_schedule);

Expand Down Expand Up @@ -1150,10 +1116,9 @@ pub fn execute_response(
}
};

// The cycles prepaid for this response execution when the corresponding call was
// performed might not match the cost of executing it in the canister's current
// Wasm execution mode. If the canister cannot pay a shortfall, then the response
// is rejected without executing the callback.
// An upgrade since the call was performed may have made the response execution
// more expensive; if the canister cannot pay the difference, the response is
// rejected without executing the callback.
if let Err(err) = helper.adjust_prepayment_for_response_execution(&original, &round) {
info!(
round.log,
Expand Down
Loading