Skip to content

Commit

Permalink
feat: [EXC-1753] Add mint_cycles128 API (#2589)
Browse files Browse the repository at this point in the history
This PR adds a new system API method `ic0_mint_cycles128` which, like
its predecessor `ic0_mint_cycles`, is not usable by any canister except
the CMC. It is therefore not necessary to add it to the interface spec
or the SDKs. It also means that this system call does not need an
instruction cost adjustment.

The method's signature deviates from the predecessor because 128 bit
values cannot be handled by WASM natively. Therefore, the argument is
split into two `u64`s, representing the high and low bits. The amount of
actually minted cycles cannot be returned as a WASM return value either.
Instead, the result is written to a caller-specified location in memory,
where it occupies the next 16 bytes.

---------

Co-authored-by: Michael Weigelt <[email protected]>
Co-authored-by: mraszyk <[email protected]>
  • Loading branch information
3 people authored Dec 11, 2024
1 parent b006ae9 commit 858c038
Show file tree
Hide file tree
Showing 18 changed files with 352 additions and 23 deletions.
6 changes: 4 additions & 2 deletions rs/cycles_account_manager/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1015,16 +1015,18 @@ impl CyclesAccountManager {
canister_id: CanisterId,
cycles_balance: &mut Cycles,
amount_to_mint: Cycles,
) -> Result<(), CyclesAccountManagerError> {
) -> Result<Cycles, CyclesAccountManagerError> {
if canister_id != CYCLES_MINTING_CANISTER_ID {
let error_str = format!(
"ic0.mint_cycles cannot be executed on non Cycles Minting Canister: {} != {}",
canister_id, CYCLES_MINTING_CANISTER_ID
);
Err(CyclesAccountManagerError::ContractViolation(error_str))
} else {
let before_balance = *cycles_balance;
*cycles_balance += amount_to_mint;
Ok(())
// equal to amount_to_mint, except when the addition saturated
Ok(*cycles_balance - before_balance)
}
}

Expand Down
10 changes: 10 additions & 0 deletions rs/embedders/src/wasm_utils/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,16 @@ fn get_valid_system_apis_common(I: ValType) -> HashMap<String, HashMap<String, F
},
)],
),
(
"mint_cycles128",
vec![(
API_VERSION_IC0,
FunctionSignature {
param_types: vec![ValType::I64, ValType::I64, I],
return_type: vec![],
},
)],
),
(
"call_cycles_add128",
vec![(
Expand Down
12 changes: 12 additions & 0 deletions rs/embedders/src/wasmtime_embedder/system_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1190,6 +1190,18 @@ pub fn syscalls<
})
.unwrap();

linker
.func_wrap("ic0", "mint_cycles128", {
move |mut caller: Caller<'_, StoreData>, amount_high: u64, amount_low: u64, dst: I| {
with_memory_and_system_api(&mut caller, |s, memory| {
let dst: usize = dst.try_into().expect("Failed to convert I to usize");
s.ic0_mint_cycles128(Cycles::from_parts(amount_high, amount_low), dst, memory)
})
.map_err(|e| anyhow::Error::msg(format!("ic0_mint_cycles128 failed: {}", e)))
}
})
.unwrap();

linker
.func_wrap("ic0", "cycles_burn128", {
move |mut caller: Caller<'_, StoreData>, amount_high: u64, amount_low: u64, dst: I| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ set -ue
## | update/ic0_canister_status() | 1.27G | 1.34G | +5% | 3.73s |
## | inspect/ic0_msg_method_name_size() | - | 1.28G | - | 23.92s |

if ! which bazel rg >/dev/null; then
echo "Error checking dependencies: please ensure 'bazel' and 'rg' are installed"
exit 1
fi

## To quickly assess the new changes, run benchmarks just once
QUICK=${QUICK:-}
if [ -n "${QUICK}" ]; then
Expand Down
26 changes: 23 additions & 3 deletions rs/execution_environment/benches/system_api/execute_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ pub fn execute_update_bench(c: &mut Criterion) {
Result::No,
Wasm64::Enabled,
), // 10B max
529001006,
529004006,
),
common::Benchmark(
"wasm32/ic0_debug_print()/1B".into(),
Expand Down Expand Up @@ -736,7 +736,7 @@ pub fn execute_update_bench(c: &mut Criterion) {
Result::No,
Wasm64::Enabled,
),
517001006,
517004006,
),
common::Benchmark(
"wasm32/ic0_msg_cycles_available()".into(),
Expand Down Expand Up @@ -878,6 +878,26 @@ pub fn execute_update_bench(c: &mut Criterion) {
Module::Test.from_ic0("mint_cycles", Param1(1_i64), Result::I64, Wasm64::Enabled),
18000006,
),
common::Benchmark(
"wasm32/ic0_mint_cycles128()".into(),
Module::Test.from_ic0(
"mint_cycles128",
Params3(1_i64, 2_i64, 3_i32),
Result::No,
Wasm64::Disabled,
),
19001006,
),
common::Benchmark(
"wasm64/ic0_mint_cycles128()".into(),
Module::Test.from_ic0(
"mint_cycles128",
Params3(1_i64, 2_i64, 3_i64),
Result::No,
Wasm64::Enabled,
),
19004006,
),
common::Benchmark(
"wasm32/ic0_is_controller()".into(),
Module::Test.from_ic0(
Expand Down Expand Up @@ -922,7 +942,7 @@ pub fn execute_update_bench(c: &mut Criterion) {
"wasm32/ic0_cycles_burn128()".into(),
Module::Test.from_ic0(
"cycles_burn128",
Params3(1_i64, 2_i64, 3),
Params3(1_i64, 2_i64, 3_i32),
Result::No,
Wasm64::Disabled,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1488,6 +1488,7 @@ fn query_cache_future_proof_test() {
| SystemApiCallId::InReplicatedExecution
| SystemApiCallId::IsController
| SystemApiCallId::MintCycles
| SystemApiCallId::MintCycles128
| SystemApiCallId::MsgArgDataCopy
| SystemApiCallId::MsgArgDataSize
| SystemApiCallId::MsgCallerCopy
Expand Down
66 changes: 66 additions & 0 deletions rs/execution_environment/tests/hypervisor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2280,6 +2280,72 @@ fn ic0_mint_cycles_succeeds_on_cmc() {
);
}

// helper for mint_cycles128 tests
fn verify_error_and_no_effect(mut test: ExecutionTest) {
let canister_id = test.universal_canister().unwrap();
let initial_cycles = test.canister_state(canister_id).system_state.balance();
let payload = wasm()
.mint_cycles128(Cycles::from(10_000_000_000_u128))
.reply_data_append()
.reply()
.build();
let err = test.ingress(canister_id, "update", payload).unwrap_err();
assert_eq!(ErrorCode::CanisterContractViolation, err.code());
assert!(err
.description()
.contains("ic0.mint_cycles cannot be executed"));
let canister_state = test.canister_state(canister_id);
assert_eq!(0, canister_state.system_state.queues().output_queues_len());
assert_balance_equals(
initial_cycles,
canister_state.system_state.balance(),
BALANCE_EPSILON,
);
}

#[test]
fn ic0_mint_cycles128_fails_on_application_subnet() {
let test = ExecutionTestBuilder::new().build();
verify_error_and_no_effect(test);
}

#[test]
fn ic0_mint_cycles128_fails_on_system_subnet_non_cmc() {
let test = ExecutionTestBuilder::new()
.with_subnet_type(SubnetType::System)
.build();
verify_error_and_no_effect(test);
}

#[test]
fn ic0_mint_cycles128_succeeds_on_cmc() {
let mut test = ExecutionTestBuilder::new()
.with_subnet_type(SubnetType::System)
.build();
let mut canister_id = test.universal_canister().unwrap();
for _ in 0..4 {
canister_id = test.universal_canister().unwrap();
}
assert_eq!(canister_id, CYCLES_MINTING_CANISTER_ID);
let initial_cycles = test.canister_state(canister_id).system_state.balance();
let amount: u128 = (1u128 << 64) + 2u128;
let payload = wasm()
.mint_cycles128(Cycles::from(amount))
.reply_data_append()
.reply()
.build();
let result = test.ingress(canister_id, "update", payload).unwrap();
assert_eq!(WasmResult::Reply(amount.to_le_bytes().to_vec()), result);
let canister_state = test.canister_state(canister_id);

assert_eq!(0, canister_state.system_state.queues().output_queues_len());
assert_balance_equals(
initial_cycles + Cycles::new(amount),
canister_state.system_state.balance(),
BALANCE_EPSILON,
);
}

#[test]
fn ic0_call_enqueues_request() {
let mut test = ExecutionTestBuilder::new().build();
Expand Down
22 changes: 18 additions & 4 deletions rs/interfaces/src/execution_environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ pub enum SystemApiCallId {
IsController,
/// Tracker for `ic0.mint_cycles()`
MintCycles,
/// Tracker for `ic0.mint_cycles128()`
MintCycles128,
/// Tracker for `ic0.msg_arg_data_copy()`
MsgArgDataCopy,
/// Tracker for `ic0.msg_arg_data_size()`
Expand Down Expand Up @@ -1125,13 +1127,25 @@ pub trait SystemApi {
///
/// Adds no more cycles than `amount`.
///
/// The canister balance afterwards does not exceed
/// maximum amount of cycles it can hold.
/// However, canisters on system subnets have no balance limit.
///
/// Returns the amount of cycles added to the canister's balance.
fn ic0_mint_cycles(&mut self, amount: u64) -> HypervisorResult<u64>;

/// Mints the `amount` cycles
/// Adds cycles to the canister's balance.
///
/// Adds no more cycles than `amount`. The balance afterwards cannot
/// exceed u128::MAX, so the amount added may be less than `amount`.
///
/// The amount of cycles added to the canister's balance is
/// represented by a 128-bit value and is copied in the canister
/// memory starting at the location `dst`.
fn ic0_mint_cycles128(
&mut self,
amount: Cycles,
dst: usize,
heap: &mut [u8],
) -> HypervisorResult<()>;

/// Checks whether the principal identified by src/size is one of the
/// controllers of the canister. If yes, then a value of 1 is returned,
/// otherwise a 0 is returned. It can be called multiple times.
Expand Down
43 changes: 40 additions & 3 deletions rs/system_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3168,7 +3168,8 @@ impl SystemApi for SystemApiImpl {
trace_syscall!(self, CanisterStatus, result);
result
}

// TODO(EXC-1806): This can be removed (in favour of ic0_mint_cycles128) once the CMC is upgraded, so it
// doesn't make sense to deduplicate the shared code.
fn ic0_mint_cycles(&mut self, amount: u64) -> HypervisorResult<u64> {
let result = match self.api_type {
ApiType::Start { .. }
Expand All @@ -3187,16 +3188,52 @@ impl SystemApi for SystemApiImpl {
// Access to this syscall not permitted.
Err(self.error_for("ic0_mint_cycles"))
} else {
self.sandbox_safe_system_state
let actually_minted = self
.sandbox_safe_system_state
.mint_cycles(Cycles::from(amount))?;
Ok(amount)
// the actually minted amount cannot be larger than the argument, which is a u64.
debug_assert_eq!(actually_minted.high64(), 0, "ic0_mint_cycles was called with u64 but minted more cycles than fit into 64 bit");
Ok(actually_minted.low64())
}
}
};
trace_syscall!(self, MintCycles, result, amount);
result
}

fn ic0_mint_cycles128(
&mut self,
amount: Cycles,
dst: usize,
heap: &mut [u8],
) -> HypervisorResult<()> {
let result = match self.api_type {
ApiType::Start { .. }
| ApiType::Init { .. }
| ApiType::PreUpgrade { .. }
| ApiType::Cleanup { .. }
| ApiType::ReplicatedQuery { .. }
| ApiType::NonReplicatedQuery { .. }
| ApiType::InspectMessage { .. } => Err(self.error_for("ic0_mint_cycles128")),
ApiType::Update { .. }
| ApiType::SystemTask { .. }
| ApiType::ReplyCallback { .. }
| ApiType::RejectCallback { .. } => {
if self.execution_parameters.execution_mode == ExecutionMode::NonReplicated {
// Non-replicated mode means we are handling a composite query.
// Access to this syscall not permitted.
Err(self.error_for("ic0_mint_cycles128"))
} else {
let actually_minted = self.sandbox_safe_system_state.mint_cycles(amount)?;
copy_cycles_to_heap(actually_minted, dst, heap, "ic0_mint_cycles_128")?;
Ok(())
}
}
};
trace_syscall!(self, MintCycles128, result, amount);
result
}

fn ic0_debug_print(&self, src: usize, size: usize, heap: &[u8]) -> HypervisorResult<()> {
const MAX_DEBUG_MESSAGE_SIZE: usize = 32 * 1024;
let size = size.min(MAX_DEBUG_MESSAGE_SIZE);
Expand Down
2 changes: 1 addition & 1 deletion rs/system_api/src/sandbox_safe_system_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,7 @@ impl SandboxSafeSystemState {
self.update_balance_change(new_balance);
}

pub(super) fn mint_cycles(&mut self, amount_to_mint: Cycles) -> HypervisorResult<()> {
pub(super) fn mint_cycles(&mut self, amount_to_mint: Cycles) -> HypervisorResult<Cycles> {
let mut new_balance = self.cycles_balance();
let result = self
.cycles_account_manager
Expand Down
53 changes: 53 additions & 0 deletions rs/system_api/tests/sandbox_safe_system_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,59 @@ fn mint_cycles_fails_caller_not_on_nns() {
);
}

fn common_mint_cycles_128(
initial_cycles: Cycles,
cycles_to_mint: Cycles,
expected_actually_minted: Cycles,
) {
let cycles_account_manager = CyclesAccountManagerBuilder::new()
.with_subnet_type(SubnetType::System)
.build();
let system_state = SystemStateBuilder::new()
.initial_cycles(initial_cycles)
.canister_id(CYCLES_MINTING_CANISTER_ID)
.build();

let api_type = ApiTypeBuilder::build_update_api();
let mut api = get_system_api(api_type, &system_state, cycles_account_manager);
let mut balance_before = [0u8; 16];
api.ic0_canister_cycle_balance128(0, &mut balance_before)
.unwrap();
let balance_before = u128::from_le_bytes(balance_before);
assert_eq!(balance_before, initial_cycles.get());
let mut heap = [0u8; 16];
api.ic0_mint_cycles128(cycles_to_mint, 0, &mut heap)
.unwrap();
let cycles_minted = u128::from_le_bytes(heap);
assert_eq!(cycles_minted, expected_actually_minted.get());
let mut balance_after = [0u8; 16];
api.ic0_canister_cycle_balance128(0, &mut balance_after)
.unwrap();
let balance_after = u128::from_le_bytes(balance_after);
assert_eq!(
balance_after - balance_before,
expected_actually_minted.get()
);
}

#[test]
fn mint_cycles_very_large_value() {
let to_mint = Cycles::from_parts(u64::MAX, 50);
common_mint_cycles_128(INITIAL_CYCLES, to_mint, to_mint);
}

#[test]
fn mint_cycles_max() {
let to_mint = Cycles::from_parts(u64::MAX, u64::MAX);
common_mint_cycles_128(Cycles::zero(), to_mint, to_mint);
}

#[test]
fn mint_cycles_saturate() {
let to_mint = Cycles::from_parts(u64::MAX, u64::MAX);
common_mint_cycles_128(INITIAL_CYCLES, to_mint, to_mint - INITIAL_CYCLES);
}

#[test]
fn is_controller_test() {
let mut system_state = SystemStateBuilder::default().build();
Expand Down
Loading

0 comments on commit 858c038

Please sign in to comment.