From 2b0b6c63af507c7544521d10cbc3df325c60094d Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Wed, 13 Nov 2024 09:17:23 +0000 Subject: [PATCH 01/33] add test --- .../src/hypervisor/tests.rs | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/rs/execution_environment/src/hypervisor/tests.rs b/rs/execution_environment/src/hypervisor/tests.rs index 305852af23b..78a3da7eadf 100644 --- a/rs/execution_environment/src/hypervisor/tests.rs +++ b/rs/execution_environment/src/hypervisor/tests.rs @@ -2272,6 +2272,53 @@ fn ic0_mint_cycles_succeeds_on_cmc() { ); } +// TODO: change amount of cycles minted +// TODO: change signature of mint_cycles to two u64 args +const MINT_CYCLES128: &str = r#" + (module + (import "ic0" "msg_reply_data_append" + (func $msg_reply_data_append (param i32) (param i32)) + ) + (import "ic0" "mint_cycles128" + (func $mint_cycles128 (param i64) (result i64)) + ) + (import "ic0" "msg_reply" (func $ic0_msg_reply)) + + (func (export "canister_update test") + (i64.store + ;; store at the beginning of the heap + (i32.const 0) ;; store at the beginning of the heap + (call $mint_cycles128 (i64.const 10000000000)) + ) + (call $msg_reply_data_append (i32.const 0) (i32.const 8)) + (call $ic0_msg_reply) + ) + (memory 1 1) + )"#; + +#[test] +fn ic0_mint_cycles128_succeeds_on_cmc() { + let mut test = ExecutionTestBuilder::new() + .with_subnet_type(SubnetType::System) + .build(); + let mut canister_id = test.canister_from_wat(MINT_CYCLES128).unwrap(); + // This loop should finish after four iterations. + while canister_id != CYCLES_MINTING_CANISTER_ID { + canister_id = test.canister_from_wat(MINT_CYCLES128).unwrap(); + } + let initial_cycles = test.canister_state(canister_id).system_state.balance(); + let result = test.ingress(canister_id, "test", vec![]).unwrap(); + // ic0_mint() returns the minted amount: hex(10_000_000_000) = 0x2_54_0b_e4_00. + assert_eq!(WasmResult::Reply(vec![0, 228, 11, 84, 2, 0, 0, 0]), 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(10_000_000_000), + canister_state.system_state.balance(), + BALANCE_EPSILON, + ); +} + #[test] fn ic0_call_enqueues_request() { let mut test = ExecutionTestBuilder::new().build(); From 4a4e1c0210a8bcc6b912ad6f6d226a54a3b40278 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Wed, 13 Nov 2024 12:35:42 +0000 Subject: [PATCH 02/33] implement ic0_mint_cycles128 --- rs/embedders/src/wasm_utils/validation.rs | 10 +++++ .../src/wasmtime_embedder/system_api.rs | 12 ++++++ .../src/hypervisor/tests.rs | 23 ++++++----- rs/interfaces/src/execution_environment.rs | 20 ++++++++++ rs/system_api/src/lib.rs | 38 ++++++++++++++++++- 5 files changed, 90 insertions(+), 13 deletions(-) diff --git a/rs/embedders/src/wasm_utils/validation.rs b/rs/embedders/src/wasm_utils/validation.rs index ea3120560e7..7288ec36d8b 100644 --- a/rs/embedders/src/wasm_utils/validation.rs +++ b/rs/embedders/src/wasm_utils/validation.rs @@ -527,6 +527,16 @@ fn get_valid_system_apis_common(I: ValType) -> HashMap, 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(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| { diff --git a/rs/execution_environment/src/hypervisor/tests.rs b/rs/execution_environment/src/hypervisor/tests.rs index 78a3da7eadf..feebba6a7b7 100644 --- a/rs/execution_environment/src/hypervisor/tests.rs +++ b/rs/execution_environment/src/hypervisor/tests.rs @@ -2272,25 +2272,20 @@ fn ic0_mint_cycles_succeeds_on_cmc() { ); } -// TODO: change amount of cycles minted -// TODO: change signature of mint_cycles to two u64 args const MINT_CYCLES128: &str = r#" (module (import "ic0" "msg_reply_data_append" (func $msg_reply_data_append (param i32) (param i32)) ) (import "ic0" "mint_cycles128" - (func $mint_cycles128 (param i64) (result i64)) + (func $mint_cycles128 (param i64) (param i64) (param i32)) ) (import "ic0" "msg_reply" (func $ic0_msg_reply)) (func (export "canister_update test") - (i64.store - ;; store at the beginning of the heap - (i32.const 0) ;; store at the beginning of the heap - (call $mint_cycles128 (i64.const 10000000000)) - ) - (call $msg_reply_data_append (i32.const 0) (i32.const 8)) + ;; store at the beginning of the heap + (call $mint_cycles128 (i64.const 1) (i64.const 2) (i32.const 0)) + (call $msg_reply_data_append (i32.const 0) (i32.const 16)) (call $ic0_msg_reply) ) (memory 1 1) @@ -2308,12 +2303,16 @@ fn ic0_mint_cycles128_succeeds_on_cmc() { } let initial_cycles = test.canister_state(canister_id).system_state.balance(); let result = test.ingress(canister_id, "test", vec![]).unwrap(); - // ic0_mint() returns the minted amount: hex(10_000_000_000) = 0x2_54_0b_e4_00. - assert_eq!(WasmResult::Reply(vec![0, 228, 11, 84, 2, 0, 0, 0]), result); + // (high=1, low=2) => 2^64 + 2^1 as in the MINT_CYCLES128 wasm module + assert_eq!( + WasmResult::Reply(vec![2, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0]), + 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(10_000_000_000), + initial_cycles + Cycles::new(18446744073709551618), canister_state.system_state.balance(), BALANCE_EPSILON, ); diff --git a/rs/interfaces/src/execution_environment.rs b/rs/interfaces/src/execution_environment.rs index 5e7297f5fae..8706492e0df 100644 --- a/rs/interfaces/src/execution_environment.rs +++ b/rs/interfaces/src/execution_environment.rs @@ -1132,6 +1132,26 @@ pub trait SystemApi { /// Returns the amount of cycles added to the canister's balance. fn ic0_mint_cycles(&mut self, amount: u64) -> HypervisorResult; + /// Mints the `amount` cycles + /// Adds cycles to the canister's balance. + /// + /// 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. + /// + /// 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 starting at the location `dst`. + fn ic0_mint_cycles128( + &mut self, + amount_high: u64, + amount_low: u64, + 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. diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 46ac54b6ed3..041cdbbd41d 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -3167,7 +3167,8 @@ impl SystemApi for SystemApiImpl { trace_syscall!(self, CanisterStatus, result); result } - + // TODO: 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 { let result = match self.api_type { ApiType::Start { .. } @@ -3196,6 +3197,41 @@ impl SystemApi for SystemApiImpl { result } + fn ic0_mint_cycles128( + &mut self, + amount_high: u64, + amount_low: u64, + dst: usize, + heap: &mut [u8], + ) -> HypervisorResult<()> { + let cycles = Cycles::from_parts(amount_high, amount_low); + 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 { + self.sandbox_safe_system_state.mint_cycles(cycles)?; + copy_cycles_to_heap(cycles, dst, heap, "ic0_mint_cycles_128")?; + Ok(()) + } + } + }; + trace_syscall!(self, MintCycles, result, cycles); + 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); From 941694465f16e1a7f2f5a8a3990ff2b786200235 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Wed, 13 Nov 2024 16:15:07 +0000 Subject: [PATCH 03/33] tests --- rs/embedders/fuzz/src/imports.rs | 1 + .../src/hypervisor/tests.rs | 40 ++++++++++++++++++ .../tests/sandbox_safe_system_state.rs | 42 +++++++++++++++++++ 3 files changed, 83 insertions(+) diff --git a/rs/embedders/fuzz/src/imports.rs b/rs/embedders/fuzz/src/imports.rs index ef1489e60dc..7c10e5957e7 100644 --- a/rs/embedders/fuzz/src/imports.rs +++ b/rs/embedders/fuzz/src/imports.rs @@ -90,5 +90,6 @@ pub(crate) const SYSTEM_API_IMPORTS: &str = r#" (import "ic0" "is_controller" (func (;51;) (type 7))) (import "ic0" "in_replicated_execution" (func (;52;) (type 1))) (import "ic0" "cycles_burn128" (func (;53;) (type 31))) + (import "ic0" "mint_cycles128" (func (;54;) (type 31))) ) "#; diff --git a/rs/execution_environment/src/hypervisor/tests.rs b/rs/execution_environment/src/hypervisor/tests.rs index feebba6a7b7..4d52e52c123 100644 --- a/rs/execution_environment/src/hypervisor/tests.rs +++ b/rs/execution_environment/src/hypervisor/tests.rs @@ -2291,6 +2291,46 @@ const MINT_CYCLES128: &str = r#" (memory 1 1) )"#; +#[test] +fn ic0_mint_cycles128_fails_on_application_subnet() { + let mut test = ExecutionTestBuilder::new().build(); + let canister_id = test.canister_from_wat(MINT_CYCLES128).unwrap(); + let initial_cycles = test.canister_state(canister_id).system_state.balance(); + let err = test.ingress(canister_id, "test", vec![]).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_system_subnet_non_cmc() { + let mut test = ExecutionTestBuilder::new() + .with_subnet_type(SubnetType::System) + .build(); + let canister_id = test.canister_from_wat(MINT_CYCLES128).unwrap(); + let initial_cycles = test.canister_state(canister_id).system_state.balance(); + let err = test.ingress(canister_id, "test", vec![]).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_succeeds_on_cmc() { let mut test = ExecutionTestBuilder::new() diff --git a/rs/system_api/tests/sandbox_safe_system_state.rs b/rs/system_api/tests/sandbox_safe_system_state.rs index fb0a0fd1a29..471873d2a6f 100644 --- a/rs/system_api/tests/sandbox_safe_system_state.rs +++ b/rs/system_api/tests/sandbox_safe_system_state.rs @@ -344,6 +344,48 @@ fn mint_cycles_fails_caller_not_on_nns() { ); } +#[test] +fn mint_cycles_very_large_value() { + let cycles_account_manager = CyclesAccountManagerBuilder::new() + .with_subnet_type(SubnetType::System) + .build(); + let mut system_state = SystemStateBuilder::new() + .canister_id(CYCLES_MINTING_CANISTER_ID) + .build(); + + system_state.add_cycles( + Cycles::from(1_000_000_000_000_000_u128), + CyclesUseCase::NonConsumed, + ); + + 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); + + let amount_high = u64::MAX; + let amount_low = 50; + let mut heap = [0u8; 16]; + // Canisters on the System subnet can hold any amount of cycles + api.ic0_mint_cycles128(amount_high, amount_low, 0, &mut heap) + .unwrap(); + let cycles_minted = u128::from_le_bytes(heap); + assert_eq!( + cycles_minted, + Cycles::from_parts(amount_high, amount_low).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, + Cycles::from_parts(amount_high, amount_low).get() + ); +} + #[test] fn is_controller_test() { let mut system_state = SystemStateBuilder::default().build(); From c778b0927e0b5dc91f8fe871bfefffbec0220629 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Tue, 19 Nov 2024 09:38:01 +0000 Subject: [PATCH 04/33] todo markers --- rs/execution_environment/benches/system_api/execute_update.rs | 1 + rs/interfaces/src/execution_environment.rs | 1 + rs/universal_canister/impl/src/api.rs | 1 + 3 files changed, 3 insertions(+) diff --git a/rs/execution_environment/benches/system_api/execute_update.rs b/rs/execution_environment/benches/system_api/execute_update.rs index dadd339da0a..4aa83b98797 100644 --- a/rs/execution_environment/benches/system_api/execute_update.rs +++ b/rs/execution_environment/benches/system_api/execute_update.rs @@ -878,6 +878,7 @@ pub fn execute_update_bench(c: &mut Criterion) { Module::Test.from_ic0("mint_cycles", Param1(1_i64), Result::I64, Wasm64::Enabled), 18000006, ), + // TODO? common::Benchmark( "wasm32/ic0_is_controller()".into(), Module::Test.from_ic0( diff --git a/rs/interfaces/src/execution_environment.rs b/rs/interfaces/src/execution_environment.rs index 8706492e0df..805795d070c 100644 --- a/rs/interfaces/src/execution_environment.rs +++ b/rs/interfaces/src/execution_environment.rs @@ -175,6 +175,7 @@ pub enum SystemApiCallId { IsController, /// Tracker for `ic0.mint_cycles()` MintCycles, + /// // TODO? /// Tracker for `ic0.msg_arg_data_copy()` MsgArgDataCopy, /// Tracker for `ic0.msg_arg_data_size()` diff --git a/rs/universal_canister/impl/src/api.rs b/rs/universal_canister/impl/src/api.rs index fc28d59d9ee..891278a5d22 100644 --- a/rs/universal_canister/impl/src/api.rs +++ b/rs/universal_canister/impl/src/api.rs @@ -71,6 +71,7 @@ mod ic0 { pub fn canister_version() -> u64; pub fn mint_cycles(amount: u64) -> u64; + // TODO? pub fn is_controller(src: u32, size: u32) -> u32; pub fn in_replicated_execution() -> u32; From 795835c9c21728abb534b1626b722eb209df2d9b Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Tue, 19 Nov 2024 16:08:27 +0000 Subject: [PATCH 05/33] uni canister --- rs/universal_canister/impl/src/api.rs | 9 ++++++++- rs/universal_canister/impl/src/lib.rs | 1 + rs/universal_canister/impl/src/main.rs | 5 +++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/rs/universal_canister/impl/src/api.rs b/rs/universal_canister/impl/src/api.rs index 891278a5d22..e93dcb5a68c 100644 --- a/rs/universal_canister/impl/src/api.rs +++ b/rs/universal_canister/impl/src/api.rs @@ -71,7 +71,7 @@ mod ic0 { pub fn canister_version() -> u64; pub fn mint_cycles(amount: u64) -> u64; - // TODO? + pub fn mint_cycles128(amount_high: u64, amount_low: u64, dst: u32) -> (); pub fn is_controller(src: u32, size: u32) -> u32; pub fn in_replicated_execution() -> u32; @@ -405,6 +405,13 @@ pub fn mint_cycles(amount: u64) -> u64 { unsafe { ic0::mint_cycles(amount) } } +/// Mint cycles (only works on CMC). +pub fn mint_cycles128(amount_high: u64, amount_low: u64) -> Vec { + let mut result_bytes = vec![0u8; CYCLES_SIZE]; + unsafe { ic0::mint_cycles128(amount_high, amount_low, result_bytes.as_mut_ptr() as u32) } + result_bytes +} + pub fn is_controller(data: &[u8]) -> u32 { unsafe { ic0::is_controller(data.as_ptr() as u32, data.len() as u32) } } diff --git a/rs/universal_canister/impl/src/lib.rs b/rs/universal_canister/impl/src/lib.rs index 0ec29c5dffe..b31a82baaeb 100644 --- a/rs/universal_canister/impl/src/lib.rs +++ b/rs/universal_canister/impl/src/lib.rs @@ -112,5 +112,6 @@ try_from_u8!( CallWithBestEffortResponse = 82, MsgDeadline = 83, MemorySizeIsAtLeast = 84, + MintCycles128, } ); diff --git a/rs/universal_canister/impl/src/main.rs b/rs/universal_canister/impl/src/main.rs index d6635d6fe70..e33d6d849e4 100644 --- a/rs/universal_canister/impl/src/main.rs +++ b/rs/universal_canister/impl/src/main.rs @@ -379,6 +379,11 @@ fn eval(ops_bytes: OpsBytes) { let amount = stack.pop_int64(); stack.push_int64(api::mint_cycles(amount)); } + Ops::MintCycles128 => { + let amount_high = stack.pop_int64(); + let amount_low = stack.pop_int64(); + stack.push_blob(api::mint_cycles128(amount_high, amount_low)) + } Ops::OneWayCallNew => { // pop in reverse order! let method = stack.pop_blob(); From db9ed21a60667cd7c7351d543402fda5639846bc Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Wed, 20 Nov 2024 09:06:08 +0000 Subject: [PATCH 06/33] added SystemApiCallId --- .../src/query_handler/query_cache/tests.rs | 1 + rs/interfaces/src/execution_environment.rs | 3 ++- rs/system_api/tests/system_api.rs | 18 ++++++++++++++++-- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/rs/execution_environment/src/query_handler/query_cache/tests.rs b/rs/execution_environment/src/query_handler/query_cache/tests.rs index 73a65d05c39..009c6beb9d9 100644 --- a/rs/execution_environment/src/query_handler/query_cache/tests.rs +++ b/rs/execution_environment/src/query_handler/query_cache/tests.rs @@ -1488,6 +1488,7 @@ fn query_cache_future_proof_test() { | SystemApiCallId::InReplicatedExecution | SystemApiCallId::IsController | SystemApiCallId::MintCycles + | SystemApiCallId::MintCycles128 | SystemApiCallId::MsgArgDataCopy | SystemApiCallId::MsgArgDataSize | SystemApiCallId::MsgCallerCopy diff --git a/rs/interfaces/src/execution_environment.rs b/rs/interfaces/src/execution_environment.rs index 805795d070c..b9470422ba6 100644 --- a/rs/interfaces/src/execution_environment.rs +++ b/rs/interfaces/src/execution_environment.rs @@ -175,7 +175,8 @@ pub enum SystemApiCallId { IsController, /// Tracker for `ic0.mint_cycles()` MintCycles, - /// // TODO? + /// Tracker for `ic0.mint_cycles128()` + MintCycles128, /// Tracker for `ic0.msg_arg_data_copy()` MsgArgDataCopy, /// Tracker for `ic0.msg_arg_data_size()` diff --git a/rs/system_api/tests/system_api.rs b/rs/system_api/tests/system_api.rs index 6fedaf9f8bf..44258767651 100644 --- a/rs/system_api/tests/system_api.rs +++ b/rs/system_api/tests/system_api.rs @@ -293,7 +293,8 @@ fn is_supported(api_type: SystemApiCallId, context: &str) -> bool { SystemApiCallId::InReplicatedExecution => vec!["*", "s"], SystemApiCallId::DebugPrint => vec!["*", "s"], SystemApiCallId::Trap => vec!["*", "s"], - SystemApiCallId::MintCycles => vec!["U", "Ry", "Rt", "T"] + SystemApiCallId::MintCycles => vec!["U", "Ry", "Rt", "T"], + SystemApiCallId::MintCycles128 => vec!["U", "Ry", "Rt", "T"] }; // the semantics of "*" is to cover all modes except for "s" matrix.get(&api_type).unwrap().contains(&context) @@ -745,6 +746,11 @@ fn api_availability_test( let mut api = get_system_api(api_type, &system_state, cycles_account_manager); assert_api_not_supported(api.ic0_mint_cycles(0)); } + SystemApiCallId::MintCycles128 => { + // ic0.mint_cycles128 is only supported for CMC which is tested separately + let mut api = get_system_api(api_type, &system_state, cycles_account_manager); + assert_api_not_supported(api.ic0_mint_cycles128(0, 0, 0, &mut vec![0u8; 16])); + } SystemApiCallId::IsController => { assert_api_availability( |api| api.ic0_is_controller(0, 0, &[42; 128]), @@ -822,7 +828,7 @@ fn system_api_availability() { let api = get_system_api(api_type.clone(), &system_state, cycles_account_manager); check_stable_apis_support(api); - // check ic0.mint_cycles API availability for CMC + // check ic0.mint_cycles, ic0.mint_cycles128 API availability for CMC let cmc_system_state = get_cmc_system_state(); assert_api_availability( |mut api| api.ic0_mint_cycles(0), @@ -832,6 +838,14 @@ fn system_api_availability() { SystemApiCallId::MintCycles, context, ); + assert_api_availability( + |mut api| api.ic0_mint_cycles128(0, 0, 0, &mut vec![0u8; 16]), + api_type.clone(), + &cmc_system_state, + cycles_account_manager, + SystemApiCallId::MintCycles128, + context, + ); // now check all other API availability for non-CMC for api_type_enum in SystemApiCallId::iter() { From 9c5d9acb04cec5844a2f7188f63b5dbc478049b3 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Thu, 21 Nov 2024 14:02:58 +0000 Subject: [PATCH 07/33] uni canister public endpoint --- rs/universal_canister/impl/src/main.rs | 2 +- rs/universal_canister/lib/src/lib.rs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/rs/universal_canister/impl/src/main.rs b/rs/universal_canister/impl/src/main.rs index e33d6d849e4..2341354f56b 100644 --- a/rs/universal_canister/impl/src/main.rs +++ b/rs/universal_canister/impl/src/main.rs @@ -380,8 +380,8 @@ fn eval(ops_bytes: OpsBytes) { stack.push_int64(api::mint_cycles(amount)); } Ops::MintCycles128 => { - let amount_high = stack.pop_int64(); let amount_low = stack.pop_int64(); + let amount_high = stack.pop_int64(); stack.push_blob(api::mint_cycles128(amount_high, amount_low)) } Ops::OneWayCallNew => { diff --git a/rs/universal_canister/lib/src/lib.rs b/rs/universal_canister/lib/src/lib.rs index d690677ded9..d0cb61e892e 100644 --- a/rs/universal_canister/lib/src/lib.rs +++ b/rs/universal_canister/lib/src/lib.rs @@ -507,6 +507,14 @@ impl PayloadBuilder { self } + pub fn mint_cycles128(mut self, amount: Cycles) -> Self { + let (amount_high, amount_low) = amount.into_parts(); + self = self.push_int64(amount_high); + self = self.push_int64(amount_low); + self.0.push(Ops::MintCycles128 as u8); + self + } + pub fn cycles_burn128(mut self, amount: Cycles) -> Self { let (amount_high, amount_low) = amount.into_parts(); self = self.push_int64(amount_high); From feee57f1bbfd38a81dee37bd0b9c1372e7e951ba Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Thu, 21 Nov 2024 14:44:14 +0000 Subject: [PATCH 08/33] use uni canister in mint_cycles128 tests --- .../src/hypervisor/tests.rs | 48 +++++++++---------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/rs/execution_environment/src/hypervisor/tests.rs b/rs/execution_environment/src/hypervisor/tests.rs index b9ef633522f..101721ecdbd 100644 --- a/rs/execution_environment/src/hypervisor/tests.rs +++ b/rs/execution_environment/src/hypervisor/tests.rs @@ -2272,31 +2272,17 @@ fn ic0_mint_cycles_succeeds_on_cmc() { ); } -const MINT_CYCLES128: &str = r#" - (module - (import "ic0" "msg_reply_data_append" - (func $msg_reply_data_append (param i32) (param i32)) - ) - (import "ic0" "mint_cycles128" - (func $mint_cycles128 (param i64) (param i64) (param i32)) - ) - (import "ic0" "msg_reply" (func $ic0_msg_reply)) - - (func (export "canister_update test") - ;; store at the beginning of the heap - (call $mint_cycles128 (i64.const 1) (i64.const 2) (i32.const 0)) - (call $msg_reply_data_append (i32.const 0) (i32.const 16)) - (call $ic0_msg_reply) - ) - (memory 1 1) - )"#; - #[test] fn ic0_mint_cycles128_fails_on_application_subnet() { let mut test = ExecutionTestBuilder::new().build(); - let canister_id = test.canister_from_wat(MINT_CYCLES128).unwrap(); + let canister_id = test.universal_canister().unwrap(); let initial_cycles = test.canister_state(canister_id).system_state.balance(); - let err = test.ingress(canister_id, "test", vec![]).unwrap_err(); + let payload = wasm() + .mint_cycles128(Cycles::from((1u128 << 64) + 2u128)) + .reply_data_append() + .reply() + .build(); + let err = test.ingress(canister_id, "update", payload).unwrap_err(); assert_eq!(ErrorCode::CanisterContractViolation, err.code()); assert!(err .description() @@ -2315,9 +2301,14 @@ fn ic0_mint_cycles128_fails_on_system_subnet_non_cmc() { let mut test = ExecutionTestBuilder::new() .with_subnet_type(SubnetType::System) .build(); - let canister_id = test.canister_from_wat(MINT_CYCLES128).unwrap(); + let canister_id = test.universal_canister().unwrap(); let initial_cycles = test.canister_state(canister_id).system_state.balance(); - let err = test.ingress(canister_id, "test", vec![]).unwrap_err(); + let payload = wasm() + .mint_cycles128(Cycles::from((1u128 << 64) + 2u128)) + .reply_data_append() + .reply() + .build(); + let err = test.ingress(canister_id, "update", payload).unwrap_err(); assert_eq!(ErrorCode::CanisterContractViolation, err.code()); assert!(err .description() @@ -2336,13 +2327,18 @@ fn ic0_mint_cycles128_succeeds_on_cmc() { let mut test = ExecutionTestBuilder::new() .with_subnet_type(SubnetType::System) .build(); - let mut canister_id = test.canister_from_wat(MINT_CYCLES128).unwrap(); + let mut canister_id = test.universal_canister().unwrap(); // This loop should finish after four iterations. while canister_id != CYCLES_MINTING_CANISTER_ID { - canister_id = test.canister_from_wat(MINT_CYCLES128).unwrap(); + canister_id = test.universal_canister().unwrap(); } let initial_cycles = test.canister_state(canister_id).system_state.balance(); - let result = test.ingress(canister_id, "test", vec![]).unwrap(); + let payload = wasm() + .mint_cycles128(Cycles::from((1u128 << 64) + 2u128)) + .reply_data_append() + .reply() + .build(); + let result = test.ingress(canister_id, "update", payload).unwrap(); // (high=1, low=2) => 2^64 + 2^1 as in the MINT_CYCLES128 wasm module assert_eq!( WasmResult::Reply(vec![2, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0]), From 8676bb37534cf7d50863cd65b00ba3c8aa50e0a7 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Thu, 21 Nov 2024 14:46:59 +0000 Subject: [PATCH 09/33] comment --- rs/execution_environment/src/hypervisor/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/execution_environment/src/hypervisor/tests.rs b/rs/execution_environment/src/hypervisor/tests.rs index 101721ecdbd..e13ad7157dd 100644 --- a/rs/execution_environment/src/hypervisor/tests.rs +++ b/rs/execution_environment/src/hypervisor/tests.rs @@ -2339,7 +2339,7 @@ fn ic0_mint_cycles128_succeeds_on_cmc() { .reply() .build(); let result = test.ingress(canister_id, "update", payload).unwrap(); - // (high=1, low=2) => 2^64 + 2^1 as in the MINT_CYCLES128 wasm module + // (high=1, low=2) => 2^64 + 2^1 assert_eq!( WasmResult::Reply(vec![2, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0]), result From 481cf48564ce1098f5efded4b10fc70d55d91446 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Thu, 21 Nov 2024 14:59:22 +0000 Subject: [PATCH 10/33] add benchmark code --- .../benches/system_api/execute_update.rs | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/rs/execution_environment/benches/system_api/execute_update.rs b/rs/execution_environment/benches/system_api/execute_update.rs index 4aa83b98797..1a85f57cd83 100644 --- a/rs/execution_environment/benches/system_api/execute_update.rs +++ b/rs/execution_environment/benches/system_api/execute_update.rs @@ -878,7 +878,26 @@ pub fn execute_update_bench(c: &mut Criterion) { Module::Test.from_ic0("mint_cycles", Param1(1_i64), Result::I64, Wasm64::Enabled), 18000006, ), - // TODO? + common::Benchmark( + "wasm32/ic0_mint_cycles128()".into(), + Module::Test.from_ic0( + "mint_cycles128", + Params3(1_i64, 2_i64, 3_i32), + Result::No, + Wasm64::Disabled, + ), + 18000006, + ), + common::Benchmark( + "wasm64/ic0_mint_cycles128()".into(), + Module::Test.from_ic0( + "mint_cycles128", + Params3(1_i64, 2_i64, 3_i32), + Result::No, + Wasm64::Enabled, + ), + 18000006, + ), common::Benchmark( "wasm32/ic0_is_controller()".into(), Module::Test.from_ic0( @@ -923,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_i64), Result::No, Wasm64::Disabled, ), From 177913987fdd1ceed76c1d894af8ef39b2006ec2 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Thu, 21 Nov 2024 15:29:20 +0000 Subject: [PATCH 11/33] benchmark costs --- .../benches/system_api/diff-old-vs-new.sh | 5 +++++ .../benches/system_api/execute_update.rs | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/rs/execution_environment/benches/system_api/diff-old-vs-new.sh b/rs/execution_environment/benches/system_api/diff-old-vs-new.sh index f6405fd9f2a..049b784e8c3 100755 --- a/rs/execution_environment/benches/system_api/diff-old-vs-new.sh +++ b/rs/execution_environment/benches/system_api/diff-old-vs-new.sh @@ -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 diff --git a/rs/execution_environment/benches/system_api/execute_update.rs b/rs/execution_environment/benches/system_api/execute_update.rs index 1a85f57cd83..73f72f9f13b 100644 --- a/rs/execution_environment/benches/system_api/execute_update.rs +++ b/rs/execution_environment/benches/system_api/execute_update.rs @@ -886,7 +886,7 @@ pub fn execute_update_bench(c: &mut Criterion) { Result::No, Wasm64::Disabled, ), - 18000006, + 19001006, ), common::Benchmark( "wasm64/ic0_mint_cycles128()".into(), @@ -896,7 +896,7 @@ pub fn execute_update_bench(c: &mut Criterion) { Result::No, Wasm64::Enabled, ), - 18000006, + 19001006, ), common::Benchmark( "wasm32/ic0_is_controller()".into(), From fb8709227eaedd89d155213095704468778fc22a Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Fri, 22 Nov 2024 09:11:00 +0000 Subject: [PATCH 12/33] benchmark types --- rs/embedders/src/wasm_utils/validation.rs | 2 +- rs/execution_environment/benches/system_api/execute_update.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rs/embedders/src/wasm_utils/validation.rs b/rs/embedders/src/wasm_utils/validation.rs index 7288ec36d8b..54ec9755d57 100644 --- a/rs/embedders/src/wasm_utils/validation.rs +++ b/rs/embedders/src/wasm_utils/validation.rs @@ -532,7 +532,7 @@ fn get_valid_system_apis_common(I: ValType) -> HashMap Date: Fri, 22 Nov 2024 11:09:41 +0000 Subject: [PATCH 13/33] update benchmark numbers --- .../benches/system_api/execute_update.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rs/execution_environment/benches/system_api/execute_update.rs b/rs/execution_environment/benches/system_api/execute_update.rs index 78371ac740f..af476e7d48d 100644 --- a/rs/execution_environment/benches/system_api/execute_update.rs +++ b/rs/execution_environment/benches/system_api/execute_update.rs @@ -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(), @@ -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(), @@ -886,7 +886,7 @@ pub fn execute_update_bench(c: &mut Criterion) { Result::No, Wasm64::Disabled, ), - 19004006, + 19001006, ), common::Benchmark( "wasm64/ic0_mint_cycles128()".into(), @@ -896,7 +896,7 @@ pub fn execute_update_bench(c: &mut Criterion) { Result::No, Wasm64::Enabled, ), - 19001006, + 19004006, ), common::Benchmark( "wasm32/ic0_is_controller()".into(), @@ -942,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_i64), + Params3(1_i64, 2_i64, 3_i32), Result::No, Wasm64::Disabled, ), From 9f1154207c348f872d980174eeae23da4efa706f Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Fri, 22 Nov 2024 13:46:37 +0000 Subject: [PATCH 14/33] syscall --- rs/system_api/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 799c63acc89..097b7b0e7e5 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -3229,7 +3229,7 @@ impl SystemApi for SystemApiImpl { } } }; - trace_syscall!(self, MintCycles, result, cycles); + trace_syscall!(self, MintCycles128, result, cycles); result } From c38b8ceb2a4927646d36562245bfc249648b2e90 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Fri, 22 Nov 2024 14:07:27 +0000 Subject: [PATCH 15/33] clippy --- rs/system_api/tests/system_api.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/system_api/tests/system_api.rs b/rs/system_api/tests/system_api.rs index 44258767651..290b96a2c63 100644 --- a/rs/system_api/tests/system_api.rs +++ b/rs/system_api/tests/system_api.rs @@ -749,7 +749,7 @@ fn api_availability_test( SystemApiCallId::MintCycles128 => { // ic0.mint_cycles128 is only supported for CMC which is tested separately let mut api = get_system_api(api_type, &system_state, cycles_account_manager); - assert_api_not_supported(api.ic0_mint_cycles128(0, 0, 0, &mut vec![0u8; 16])); + assert_api_not_supported(api.ic0_mint_cycles128(0, 0, 0, &mut [0u8; 16])); } SystemApiCallId::IsController => { assert_api_availability( @@ -839,7 +839,7 @@ fn system_api_availability() { context, ); assert_api_availability( - |mut api| api.ic0_mint_cycles128(0, 0, 0, &mut vec![0u8; 16]), + |mut api| api.ic0_mint_cycles128(0, 0, 0, &mut [0u8; 16]), api_type.clone(), &cmc_system_state, cycles_account_manager, From 72e6b5e2449d5b67e887a3e6c427b2be361467fb Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Mon, 25 Nov 2024 13:40:20 +0000 Subject: [PATCH 16/33] systests --- rs/tests/execution/general_execution_test.rs | 4 + .../general_execution_tests/nns_shielding.rs | 100 ++++++++++++++++++ 2 files changed, 104 insertions(+) diff --git a/rs/tests/execution/general_execution_test.rs b/rs/tests/execution/general_execution_test.rs index 0768a864174..a317c029208 100644 --- a/rs/tests/execution/general_execution_test.rs +++ b/rs/tests/execution/general_execution_test.rs @@ -88,6 +88,10 @@ fn main() -> Result<()> { mint_cycles_supported_only_on_cycles_minting_canister )) .add_test(systest!(mint_cycles_not_supported_on_application_subnet)) + .add_test(systest!( + mint_cycles128_supported_only_on_cycles_minting_canister + )) + .add_test(systest!(mint_cycles128_not_supported_on_application_subnet)) .add_test(systest!(no_cycle_balance_limit_on_nns_subnet)) .add_test(systest!(app_canister_attempt_initiating_dkg_fails)) .add_test(systest!(canister_heartbeat_is_called_at_regular_intervals)) diff --git a/rs/tests/execution/general_execution_tests/nns_shielding.rs b/rs/tests/execution/general_execution_tests/nns_shielding.rs index 7aab49d2083..ffd3d419ee1 100644 --- a/rs/tests/execution/general_execution_tests/nns_shielding.rs +++ b/rs/tests/execution/general_execution_tests/nns_shielding.rs @@ -14,6 +14,7 @@ use ic_system_test_driver::driver::test_env_api::{GetFirstHealthyNodeSnapshot, H use ic_system_test_driver::{util::CYCLES_LIMIT_PER_CANISTER, util::*}; use ic_types::Cycles; use ic_types_test_utils::ids::node_test_id; +use ic_universal_canister::wasm; use lazy_static::lazy_static; const BALANCE_EPSILON: Cycles = Cycles::new(10_000_000); @@ -141,6 +142,105 @@ pub fn mint_cycles_not_supported_on_application_subnet(env: TestEnv) { }); } +pub fn mint_cycles128_supported_only_on_cycles_minting_canister(env: TestEnv) { + let nns_node = env.get_first_healthy_nns_node_snapshot(); + let specified_id = nns_node.get_last_canister_id_in_allocation_ranges(); + // Check that 'specified_id' is not 'CYCLES_MINTING_CANISTER_ID'. + assert_ne!(specified_id, CYCLES_MINTING_CANISTER_ID.into()); + let nns_agent = nns_node.build_default_agent(); + block_on(async move { + let not_cmc = UniversalCanister::new_with_cycles(&nns_agent, specified_id, *INITIAL_CYCLES) + .await + .unwrap() + .canister_id(); + + let before_balance = get_balance(¬_cmc, &nns_agent).await; + assert_balance_equals( + *INITIAL_CYCLES, + Cycles::from(before_balance), + BALANCE_EPSILON, + ); + + let res = nns_agent + .update(¬_cmc, "update") + .with_arg( + wasm() + .mint_cycles128(Cycles::from((1u128 << 64) + 2u128)) + .reply_data_append() + .reply() + .build(), + ) + .call_and_wait() + .await + .expect_err("should not succeed"); + + assert_eq!( + res, + AgentError::CertifiedReject( + RejectResponse { + reject_code: RejectCode::CanisterError, + reject_message: format!( + "Error from Canister {}: Canister violated contract: ic0.mint_cycles cannot be executed on non Cycles Minting Canister: {} != {}.\nThis is likely an error with the compiler/CDK toolchain being used to build the canister. Please report the error to IC devs on the forum: https://forum.dfinity.org and include which language/CDK was used to create the canister.", + not_cmc, not_cmc, + CYCLES_MINTING_CANISTER_ID), + error_code: None}) + ); + + let after_balance = get_balance(¬_cmc, &nns_agent).await; + assert!( + after_balance == before_balance, + "expected {} == {}", + after_balance, + before_balance + ); + }); +} + +pub fn mint_cycles128_not_supported_on_application_subnet(env: TestEnv) { + let initial_cycles = CANISTER_FREEZE_BALANCE_RESERVE + Cycles::new(5_000_000_000_000); + let app_node = env.get_first_healthy_application_node_snapshot(); + let agent = app_node.build_default_agent(); + block_on(async move { + let wasm = wat::parse_str(MINT_CYCLES).unwrap(); + + let canister_id = UniversalCanister::new_with_cycles( + &agent, + app_node.effective_canister_id(), + initial_cycles * 3u64, + ) + .await + .unwrap() + .canister_id(); + + let before_balance = get_balance(&canister_id, &agent).await; + assert!( + Cycles::from(before_balance) > initial_cycles * 2u64, + "expected {} > {}", + before_balance, + initial_cycles * 2u64 + ); + assert!( + Cycles::from(before_balance) <= initial_cycles * 3u64, + "expected {} <= {}", + before_balance, + initial_cycles * 3u64 + ); + + // The test function on the wasm module will call the mint_cycles system + // call. + let res = agent.update(&canister_id, "test").call_and_wait().await; + + assert_reject(res, RejectCode::CanisterError); + let after_balance = get_balance(&canister_id, &agent).await; + assert!( + after_balance < before_balance, + "expected {} < expected {}", + after_balance, + before_balance + ); + }); +} + pub fn no_cycle_balance_limit_on_nns_subnet(env: TestEnv) { let logger = env.logger(); let nns_node = env.get_first_healthy_nns_node_snapshot(); From 86cd708ea3e68a76e97d9025125cd6aca5972e1a Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Mon, 25 Nov 2024 13:56:30 +0000 Subject: [PATCH 17/33] fix --- rs/system_api/src/lib.rs | 2 +- rs/tests/execution/general_execution_tests/nns_shielding.rs | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 097b7b0e7e5..5a3b9fefabb 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -3168,7 +3168,7 @@ impl SystemApi for SystemApiImpl { trace_syscall!(self, CanisterStatus, result); result } - // TODO: This can be removed (in favour of ic0_mint_cycles128) once the CMC is upgraded, so it + // 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 { let result = match self.api_type { diff --git a/rs/tests/execution/general_execution_tests/nns_shielding.rs b/rs/tests/execution/general_execution_tests/nns_shielding.rs index ffd3d419ee1..faab0bcefa5 100644 --- a/rs/tests/execution/general_execution_tests/nns_shielding.rs +++ b/rs/tests/execution/general_execution_tests/nns_shielding.rs @@ -201,8 +201,6 @@ pub fn mint_cycles128_not_supported_on_application_subnet(env: TestEnv) { let app_node = env.get_first_healthy_application_node_snapshot(); let agent = app_node.build_default_agent(); block_on(async move { - let wasm = wat::parse_str(MINT_CYCLES).unwrap(); - let canister_id = UniversalCanister::new_with_cycles( &agent, app_node.effective_canister_id(), From a2afe58089772e020265ca4eb5648dab1b5cc857 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Mon, 9 Dec 2024 09:34:14 +0000 Subject: [PATCH 18/33] change interface to cycles --- rs/embedders/src/wasmtime_embedder/system_api.rs | 2 +- rs/interfaces/src/execution_environment.rs | 5 ++--- rs/system_api/src/lib.rs | 10 ++++------ rs/system_api/tests/sandbox_safe_system_state.rs | 2 +- rs/system_api/tests/system_api.rs | 4 ++-- 5 files changed, 10 insertions(+), 13 deletions(-) diff --git a/rs/embedders/src/wasmtime_embedder/system_api.rs b/rs/embedders/src/wasmtime_embedder/system_api.rs index 0f3eaf405a5..5a88a07db14 100644 --- a/rs/embedders/src/wasmtime_embedder/system_api.rs +++ b/rs/embedders/src/wasmtime_embedder/system_api.rs @@ -1195,7 +1195,7 @@ pub fn syscalls< 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(amount_high, amount_low, dst, memory) + 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))) } diff --git a/rs/interfaces/src/execution_environment.rs b/rs/interfaces/src/execution_environment.rs index b9470422ba6..bac5f65070e 100644 --- a/rs/interfaces/src/execution_environment.rs +++ b/rs/interfaces/src/execution_environment.rs @@ -1145,11 +1145,10 @@ pub trait SystemApi { /// /// 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 starting at the location `dst`. + /// memory starting at the location `dst`. fn ic0_mint_cycles128( &mut self, - amount_high: u64, - amount_low: u64, + amount: Cycles, dst: usize, heap: &mut [u8], ) -> HypervisorResult<()>; diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 5a3b9fefabb..51b82008557 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -3200,12 +3200,10 @@ impl SystemApi for SystemApiImpl { fn ic0_mint_cycles128( &mut self, - amount_high: u64, - amount_low: u64, + amount: Cycles, dst: usize, heap: &mut [u8], ) -> HypervisorResult<()> { - let cycles = Cycles::from_parts(amount_high, amount_low); let result = match self.api_type { ApiType::Start { .. } | ApiType::Init { .. } @@ -3223,13 +3221,13 @@ impl SystemApi for SystemApiImpl { // Access to this syscall not permitted. Err(self.error_for("ic0_mint_cycles128")) } else { - self.sandbox_safe_system_state.mint_cycles(cycles)?; - copy_cycles_to_heap(cycles, dst, heap, "ic0_mint_cycles_128")?; + self.sandbox_safe_system_state.mint_cycles(amount)?; + copy_cycles_to_heap(amount, dst, heap, "ic0_mint_cycles_128")?; Ok(()) } } }; - trace_syscall!(self, MintCycles128, result, cycles); + trace_syscall!(self, MintCycles128, result, amount); result } diff --git a/rs/system_api/tests/sandbox_safe_system_state.rs b/rs/system_api/tests/sandbox_safe_system_state.rs index 7de30a16c4b..3b4fe086aa4 100644 --- a/rs/system_api/tests/sandbox_safe_system_state.rs +++ b/rs/system_api/tests/sandbox_safe_system_state.rs @@ -374,7 +374,7 @@ fn mint_cycles_very_large_value() { let amount_low = 50; let mut heap = [0u8; 16]; // Canisters on the System subnet can hold any amount of cycles - api.ic0_mint_cycles128(amount_high, amount_low, 0, &mut heap) + api.ic0_mint_cycles128(Cycles::from_parts(amount_high, amount_low), 0, &mut heap) .unwrap(); let cycles_minted = u128::from_le_bytes(heap); assert_eq!( diff --git a/rs/system_api/tests/system_api.rs b/rs/system_api/tests/system_api.rs index 290b96a2c63..450ee70dc92 100644 --- a/rs/system_api/tests/system_api.rs +++ b/rs/system_api/tests/system_api.rs @@ -749,7 +749,7 @@ fn api_availability_test( SystemApiCallId::MintCycles128 => { // ic0.mint_cycles128 is only supported for CMC which is tested separately let mut api = get_system_api(api_type, &system_state, cycles_account_manager); - assert_api_not_supported(api.ic0_mint_cycles128(0, 0, 0, &mut [0u8; 16])); + assert_api_not_supported(api.ic0_mint_cycles128(Cycles::zero(), 0, &mut [0u8; 16])); } SystemApiCallId::IsController => { assert_api_availability( @@ -839,7 +839,7 @@ fn system_api_availability() { context, ); assert_api_availability( - |mut api| api.ic0_mint_cycles128(0, 0, 0, &mut [0u8; 16]), + |mut api| api.ic0_mint_cycles128(Cycles::zero(), 0, &mut [0u8; 16]), api_type.clone(), &cmc_system_state, cycles_account_manager, From 54240a6822c5dcbce5f67b751362a2c4586f234e Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Mon, 9 Dec 2024 09:40:51 +0000 Subject: [PATCH 19/33] removed doc line --- rs/interfaces/src/execution_environment.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/rs/interfaces/src/execution_environment.rs b/rs/interfaces/src/execution_environment.rs index bac5f65070e..2efe36023c8 100644 --- a/rs/interfaces/src/execution_environment.rs +++ b/rs/interfaces/src/execution_environment.rs @@ -1127,10 +1127,6 @@ 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; @@ -1139,10 +1135,6 @@ 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. - /// /// 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`. From 0ac0940d880221597afa72731dbbed817eabf4c9 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Mon, 9 Dec 2024 10:09:19 +0000 Subject: [PATCH 20/33] small fixes --- rs/tests/execution/general_execution_tests/nns_shielding.rs | 4 ++-- rs/universal_canister/impl/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rs/tests/execution/general_execution_tests/nns_shielding.rs b/rs/tests/execution/general_execution_tests/nns_shielding.rs index faab0bcefa5..4d1eaebb715 100644 --- a/rs/tests/execution/general_execution_tests/nns_shielding.rs +++ b/rs/tests/execution/general_execution_tests/nns_shielding.rs @@ -135,7 +135,7 @@ pub fn mint_cycles_not_supported_on_application_subnet(env: TestEnv) { let after_balance = get_balance(&canister_id, &agent).await; assert!( after_balance < before_balance, - "expected {} < expected {}", + "expected {} < {}", after_balance, before_balance ); @@ -232,7 +232,7 @@ pub fn mint_cycles128_not_supported_on_application_subnet(env: TestEnv) { let after_balance = get_balance(&canister_id, &agent).await; assert!( after_balance < before_balance, - "expected {} < expected {}", + "expected {} < {}", after_balance, before_balance ); diff --git a/rs/universal_canister/impl/src/lib.rs b/rs/universal_canister/impl/src/lib.rs index b31a82baaeb..39f3f03c2d0 100644 --- a/rs/universal_canister/impl/src/lib.rs +++ b/rs/universal_canister/impl/src/lib.rs @@ -112,6 +112,6 @@ try_from_u8!( CallWithBestEffortResponse = 82, MsgDeadline = 83, MemorySizeIsAtLeast = 84, - MintCycles128, + MintCycles128 = 85, } ); From 0ab93bc1c7ce630a8104067c3a3c2974ff32659a Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Mon, 9 Dec 2024 10:52:24 +0000 Subject: [PATCH 21/33] return actually minted cycles --- rs/interfaces/src/execution_environment.rs | 3 ++- rs/system_api/src/lib.rs | 10 ++++++---- rs/system_api/src/sandbox_safe_system_state.rs | 11 ++++++----- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/rs/interfaces/src/execution_environment.rs b/rs/interfaces/src/execution_environment.rs index 2efe36023c8..16733e5d2c0 100644 --- a/rs/interfaces/src/execution_environment.rs +++ b/rs/interfaces/src/execution_environment.rs @@ -1133,7 +1133,8 @@ pub trait SystemApi { /// Mints the `amount` cycles /// Adds cycles to the canister's balance. /// - /// Adds no more cycles than `amount`. + /// 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 diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 51b82008557..2f3140761cf 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -3188,9 +3188,11 @@ 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. + Ok(actually_minted.low64()) } } }; @@ -3221,8 +3223,8 @@ impl SystemApi for SystemApiImpl { // Access to this syscall not permitted. Err(self.error_for("ic0_mint_cycles128")) } else { - self.sandbox_safe_system_state.mint_cycles(amount)?; - copy_cycles_to_heap(amount, dst, heap, "ic0_mint_cycles_128")?; + let actually_minted = self.sandbox_safe_system_state.mint_cycles(amount)?; + copy_cycles_to_heap(actually_minted, dst, heap, "ic0_mint_cycles_128")?; Ok(()) } } diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index 88b4ff9a43f..167a9fff15b 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -881,16 +881,17 @@ 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 { + let old_balance = self.cycles_balance(); let mut new_balance = self.cycles_balance(); - let result = self - .cycles_account_manager + self.cycles_account_manager .mint_cycles(self.canister_id, &mut new_balance, amount_to_mint) .map_err(|CyclesAccountManagerError::ContractViolation(msg)| { HypervisorError::ToolchainContractViolation { error: msg } - }); + })?; self.update_balance_change(new_balance); - result + // equal to amount_to_mint, except when the Cycles addition saturated + Ok(new_balance - old_balance) } /// Burns min(balance - freezing_treshold, amount_to_burn) cycles from the canister's From 16b74f8db2770757aa7eeb5304dc5947659f7fba Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Mon, 9 Dec 2024 12:22:16 +0000 Subject: [PATCH 22/33] comments --- rs/cycles_account_manager/src/lib.rs | 6 ++++-- rs/system_api/src/lib.rs | 3 ++- rs/system_api/src/sandbox_safe_system_state.rs | 9 ++++----- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/rs/cycles_account_manager/src/lib.rs b/rs/cycles_account_manager/src/lib.rs index 64583fdc160..84106cdfe91 100644 --- a/rs/cycles_account_manager/src/lib.rs +++ b/rs/cycles_account_manager/src/lib.rs @@ -1015,7 +1015,7 @@ impl CyclesAccountManager { canister_id: CanisterId, cycles_balance: &mut Cycles, amount_to_mint: Cycles, - ) -> Result<(), CyclesAccountManagerError> { + ) -> Result { if canister_id != CYCLES_MINTING_CANISTER_ID { let error_str = format!( "ic0.mint_cycles cannot be executed on non Cycles Minting Canister: {} != {}", @@ -1023,8 +1023,10 @@ impl CyclesAccountManager { ); Err(CyclesAccountManagerError::ContractViolation(error_str)) } else { + let before_balance = cycles_balance.clone(); *cycles_balance += amount_to_mint; - Ok(()) + // equal to amount_to_mint, except when the addition saturated + Ok(*cycles_balance - before_balance) } } diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 2f3140761cf..01292fb1b03 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -3191,7 +3191,8 @@ impl SystemApi for SystemApiImpl { let actually_minted = self .sandbox_safe_system_state .mint_cycles(Cycles::from(amount))?; - // the actually minted amount cannot be larger than the argument which is a u64. + // the actually minted amount cannot be larger than the argument, which is a u64. + debug_assert!(actually_minted.high64(), 0); Ok(actually_minted.low64()) } } diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index 167a9fff15b..15be906285f 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -882,16 +882,15 @@ impl SandboxSafeSystemState { } pub(super) fn mint_cycles(&mut self, amount_to_mint: Cycles) -> HypervisorResult { - let old_balance = self.cycles_balance(); let mut new_balance = self.cycles_balance(); - self.cycles_account_manager + let result = self + .cycles_account_manager .mint_cycles(self.canister_id, &mut new_balance, amount_to_mint) .map_err(|CyclesAccountManagerError::ContractViolation(msg)| { HypervisorError::ToolchainContractViolation { error: msg } - })?; + }); self.update_balance_change(new_balance); - // equal to amount_to_mint, except when the Cycles addition saturated - Ok(new_balance - old_balance) + result } /// Burns min(balance - freezing_treshold, amount_to_burn) cycles from the canister's From 5f814aec1fc676d640b28c59fdba67b07b429311 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Mon, 9 Dec 2024 12:31:00 +0000 Subject: [PATCH 23/33] clippy --- rs/cycles_account_manager/src/lib.rs | 2 +- rs/system_api/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/cycles_account_manager/src/lib.rs b/rs/cycles_account_manager/src/lib.rs index 84106cdfe91..18c5e8e2c45 100644 --- a/rs/cycles_account_manager/src/lib.rs +++ b/rs/cycles_account_manager/src/lib.rs @@ -1023,7 +1023,7 @@ impl CyclesAccountManager { ); Err(CyclesAccountManagerError::ContractViolation(error_str)) } else { - let before_balance = cycles_balance.clone(); + let before_balance = *cycles_balance; *cycles_balance += amount_to_mint; // equal to amount_to_mint, except when the addition saturated Ok(*cycles_balance - before_balance) diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 01292fb1b03..88b69568043 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -3192,7 +3192,7 @@ impl SystemApi for SystemApiImpl { .sandbox_safe_system_state .mint_cycles(Cycles::from(amount))?; // the actually minted amount cannot be larger than the argument, which is a u64. - debug_assert!(actually_minted.high64(), 0); + 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()) } } From 345761cff29b5fd505e332ceef34db292ed47e07 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Mon, 9 Dec 2024 14:21:17 +0000 Subject: [PATCH 24/33] add saturation tests --- .../tests/sandbox_safe_system_state.rs | 85 ++++++++++++++++--- .../general_execution_tests/nns_shielding.rs | 13 ++- 2 files changed, 78 insertions(+), 20 deletions(-) diff --git a/rs/system_api/tests/sandbox_safe_system_state.rs b/rs/system_api/tests/sandbox_safe_system_state.rs index 3b4fe086aa4..4ff7d6e046a 100644 --- a/rs/system_api/tests/sandbox_safe_system_state.rs +++ b/rs/system_api/tests/sandbox_safe_system_state.rs @@ -354,14 +354,41 @@ fn mint_cycles_very_large_value() { let cycles_account_manager = CyclesAccountManagerBuilder::new() .with_subnet_type(SubnetType::System) .build(); - let mut system_state = SystemStateBuilder::new() + let system_state = SystemStateBuilder::new() .canister_id(CYCLES_MINTING_CANISTER_ID) .build(); - system_state.add_cycles( - Cycles::from(1_000_000_000_000_000_u128), - CyclesUseCase::NonConsumed, - ); + 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 amount_high = u64::MAX; + let amount_low = 50; + let cycles_to_mint = Cycles::from_parts(amount_high, amount_low); + 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, cycles_to_mint.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, cycles_to_mint.get()); +} + +#[test] +fn mint_cycles_max() { + let cycles_account_manager = CyclesAccountManagerBuilder::new() + .with_subnet_type(SubnetType::System) + .build(); + let system_state = SystemStateBuilder::new() + .initial_cycles(Cycles::zero()) + .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); @@ -369,25 +396,57 @@ fn mint_cycles_very_large_value() { api.ic0_canister_cycle_balance128(0, &mut balance_before) .unwrap(); let balance_before = u128::from_le_bytes(balance_before); + assert_eq!(balance_before, 0); + let amount_high = u64::MAX; + let amount_low = u64::MAX; + let cycles_to_mint = Cycles::from_parts(amount_high, amount_low); + assert_eq!(cycles_to_mint, u128::MAX.into()); + 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, cycles_to_mint.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, cycles_to_mint.get()); +} +#[test] +fn mint_cycles_saturate() { + let cycles_account_manager = CyclesAccountManagerBuilder::new() + .with_subnet_type(SubnetType::System) + .build(); + let initial_amount: u128 = 5; + let system_state = SystemStateBuilder::new() + .initial_cycles(initial_amount.into()) + .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_amount); let amount_high = u64::MAX; - let amount_low = 50; + let amount_low = u64::MAX; + let cycles_to_mint = Cycles::from_parts(amount_high, amount_low); + assert_eq!(cycles_to_mint, u128::MAX.into()); let mut heap = [0u8; 16]; - // Canisters on the System subnet can hold any amount of cycles - api.ic0_mint_cycles128(Cycles::from_parts(amount_high, amount_low), 0, &mut heap) + api.ic0_mint_cycles128(cycles_to_mint, 0, &mut heap) .unwrap(); let cycles_minted = u128::from_le_bytes(heap); - assert_eq!( - cycles_minted, - Cycles::from_parts(amount_high, amount_low).get() - ); + assert_eq!(cycles_minted, cycles_to_mint.get() - initial_amount); 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, - Cycles::from_parts(amount_high, amount_low).get() + cycles_to_mint.get() - initial_amount ); } diff --git a/rs/tests/execution/general_execution_tests/nns_shielding.rs b/rs/tests/execution/general_execution_tests/nns_shielding.rs index 4d1eaebb715..e09ee06204e 100644 --- a/rs/tests/execution/general_execution_tests/nns_shielding.rs +++ b/rs/tests/execution/general_execution_tests/nns_shielding.rs @@ -17,7 +17,6 @@ use ic_types_test_utils::ids::node_test_id; use ic_universal_canister::wasm; use lazy_static::lazy_static; -const BALANCE_EPSILON: Cycles = Cycles::new(10_000_000); const CANISTER_FREEZE_BALANCE_RESERVE: Cycles = Cycles::new(5_000_000_000_000); lazy_static! { static ref INITIAL_CYCLES: Cycles = @@ -68,7 +67,7 @@ pub fn mint_cycles_supported_only_on_cycles_minting_canister(env: TestEnv) { assert_balance_equals( *INITIAL_CYCLES, Cycles::from(before_balance), - BALANCE_EPSILON, + Cycles::zero(), ); let res = nns_agent @@ -144,12 +143,12 @@ pub fn mint_cycles_not_supported_on_application_subnet(env: TestEnv) { pub fn mint_cycles128_supported_only_on_cycles_minting_canister(env: TestEnv) { let nns_node = env.get_first_healthy_nns_node_snapshot(); - let specified_id = nns_node.get_last_canister_id_in_allocation_ranges(); + let canister_id = nns_node.get_last_canister_id_in_allocation_ranges(); // Check that 'specified_id' is not 'CYCLES_MINTING_CANISTER_ID'. - assert_ne!(specified_id, CYCLES_MINTING_CANISTER_ID.into()); + assert_ne!(canister_id, CYCLES_MINTING_CANISTER_ID.into()); let nns_agent = nns_node.build_default_agent(); block_on(async move { - let not_cmc = UniversalCanister::new_with_cycles(&nns_agent, specified_id, *INITIAL_CYCLES) + let not_cmc = UniversalCanister::new_with_cycles(&nns_agent, canister_id, *INITIAL_CYCLES) .await .unwrap() .canister_id(); @@ -158,14 +157,14 @@ pub fn mint_cycles128_supported_only_on_cycles_minting_canister(env: TestEnv) { assert_balance_equals( *INITIAL_CYCLES, Cycles::from(before_balance), - BALANCE_EPSILON, + Cycles::zero(), ); let res = nns_agent .update(¬_cmc, "update") .with_arg( wasm() - .mint_cycles128(Cycles::from((1u128 << 64) + 2u128)) + .mint_cycles128(Cycles::from(2u128)) .reply_data_append() .reply() .build(), From 33cbf1a881623e91e7d68e5ca3a3f0a2a3144d70 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Mon, 9 Dec 2024 20:32:52 +0000 Subject: [PATCH 25/33] comment --- rs/tests/execution/general_execution_tests/nns_shielding.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/tests/execution/general_execution_tests/nns_shielding.rs b/rs/tests/execution/general_execution_tests/nns_shielding.rs index e09ee06204e..75577953e8d 100644 --- a/rs/tests/execution/general_execution_tests/nns_shielding.rs +++ b/rs/tests/execution/general_execution_tests/nns_shielding.rs @@ -144,7 +144,7 @@ pub fn mint_cycles_not_supported_on_application_subnet(env: TestEnv) { pub fn mint_cycles128_supported_only_on_cycles_minting_canister(env: TestEnv) { let nns_node = env.get_first_healthy_nns_node_snapshot(); let canister_id = nns_node.get_last_canister_id_in_allocation_ranges(); - // Check that 'specified_id' is not 'CYCLES_MINTING_CANISTER_ID'. + // Check that 'canister_id' is not 'CYCLES_MINTING_CANISTER_ID'. assert_ne!(canister_id, CYCLES_MINTING_CANISTER_ID.into()); let nns_agent = nns_node.build_default_agent(); block_on(async move { From 254f006ca8b09b4bffed2bf39efad7d9d032fc3a Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Mon, 9 Dec 2024 20:50:30 +0000 Subject: [PATCH 26/33] factor out shared code --- .../src/hypervisor/tests.rs | 33 ++++++------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/rs/execution_environment/src/hypervisor/tests.rs b/rs/execution_environment/src/hypervisor/tests.rs index e13ad7157dd..6cea60431a4 100644 --- a/rs/execution_environment/src/hypervisor/tests.rs +++ b/rs/execution_environment/src/hypervisor/tests.rs @@ -2272,9 +2272,8 @@ fn ic0_mint_cycles_succeeds_on_cmc() { ); } -#[test] -fn ic0_mint_cycles128_fails_on_application_subnet() { - let mut test = ExecutionTestBuilder::new().build(); +// 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() @@ -2296,30 +2295,18 @@ fn ic0_mint_cycles128_fails_on_application_subnet() { ); } +#[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 mut test = ExecutionTestBuilder::new() + let test = ExecutionTestBuilder::new() .with_subnet_type(SubnetType::System) .build(); - 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((1u128 << 64) + 2u128)) - .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, - ); + verify_error_and_no_effect(test); } #[test] From 59739999c4aa81371d28d2f653104662e6775285 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Mon, 9 Dec 2024 20:58:51 +0000 Subject: [PATCH 27/33] comments --- rs/execution_environment/src/hypervisor/tests.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/rs/execution_environment/src/hypervisor/tests.rs b/rs/execution_environment/src/hypervisor/tests.rs index 6cea60431a4..cb3f6f7aebb 100644 --- a/rs/execution_environment/src/hypervisor/tests.rs +++ b/rs/execution_environment/src/hypervisor/tests.rs @@ -2315,27 +2315,24 @@ fn ic0_mint_cycles128_succeeds_on_cmc() { .with_subnet_type(SubnetType::System) .build(); let mut canister_id = test.universal_canister().unwrap(); - // This loop should finish after four iterations. - while canister_id != CYCLES_MINTING_CANISTER_ID { + 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((1u128 << 64) + 2u128)) + .mint_cycles128(Cycles::from(amount)) .reply_data_append() .reply() .build(); let result = test.ingress(canister_id, "update", payload).unwrap(); - // (high=1, low=2) => 2^64 + 2^1 - assert_eq!( - WasmResult::Reply(vec![2, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0]), - result - ); + 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(18446744073709551618), + initial_cycles + Cycles::new(amount), canister_state.system_state.balance(), BALANCE_EPSILON, ); From 340e2a57a6d0dd2d86bcb3118ca5d7527efeb455 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Tue, 10 Dec 2024 10:28:23 +0000 Subject: [PATCH 28/33] test --- .../general_execution_tests/nns_shielding.rs | 140 +++++++----------- 1 file changed, 56 insertions(+), 84 deletions(-) diff --git a/rs/tests/execution/general_execution_tests/nns_shielding.rs b/rs/tests/execution/general_execution_tests/nns_shielding.rs index 75577953e8d..f508fa8a95b 100644 --- a/rs/tests/execution/general_execution_tests/nns_shielding.rs +++ b/rs/tests/execution/general_execution_tests/nns_shielding.rs @@ -9,10 +9,10 @@ use ic_agent::{ use ic_base_types::RegistryVersion; use ic_management_canister_types::SetupInitialDKGArgs; use ic_nns_constants::CYCLES_MINTING_CANISTER_ID; -use ic_system_test_driver::driver::test_env::TestEnv; use ic_system_test_driver::driver::test_env_api::{GetFirstHealthyNodeSnapshot, HasPublicApiUrl}; +use ic_system_test_driver::driver::{test_env::TestEnv, test_env_api::IcNodeSnapshot}; use ic_system_test_driver::{util::CYCLES_LIMIT_PER_CANISTER, util::*}; -use ic_types::Cycles; +use ic_types::{Cycles, PrincipalId}; use ic_types_test_utils::ids::node_test_id; use ic_universal_canister::wasm; use lazy_static::lazy_static; @@ -64,11 +64,7 @@ pub fn mint_cycles_supported_only_on_cycles_minting_canister(env: TestEnv) { .await; let before_balance = get_balance(&nns_canister_id, &nns_agent).await; - assert_balance_equals( - *INITIAL_CYCLES, - Cycles::from(before_balance), - Cycles::zero(), - ); + assert_eq!(INITIAL_CYCLES.get(), before_balance); let res = nns_agent .update(&nns_canister_id, "test") @@ -141,30 +137,20 @@ pub fn mint_cycles_not_supported_on_application_subnet(env: TestEnv) { }); } -pub fn mint_cycles128_supported_only_on_cycles_minting_canister(env: TestEnv) { - let nns_node = env.get_first_healthy_nns_node_snapshot(); - let canister_id = nns_node.get_last_canister_id_in_allocation_ranges(); - // Check that 'canister_id' is not 'CYCLES_MINTING_CANISTER_ID'. - assert_ne!(canister_id, CYCLES_MINTING_CANISTER_ID.into()); - let nns_agent = nns_node.build_default_agent(); +fn setup_ucan_and_try_mint128(node: IcNodeSnapshot) -> (AgentError, u128, u128, String) { + let agent = node.build_default_agent(); + let canister_id = node.get_last_canister_id_in_allocation_ranges(); block_on(async move { - let not_cmc = UniversalCanister::new_with_cycles(&nns_agent, canister_id, *INITIAL_CYCLES) + let canister_id = UniversalCanister::new_with_cycles(&agent, canister_id, *INITIAL_CYCLES) .await .unwrap() .canister_id(); - - let before_balance = get_balance(¬_cmc, &nns_agent).await; - assert_balance_equals( - *INITIAL_CYCLES, - Cycles::from(before_balance), - Cycles::zero(), - ); - - let res = nns_agent - .update(¬_cmc, "update") + let before_balance = get_balance(&canister_id, &agent).await; + let res = agent + .update(&canister_id, "update") .with_arg( wasm() - .mint_cycles128(Cycles::from(2u128)) + .mint_cycles128(Cycles::from(10_000_000_000u128)) .reply_data_append() .reply() .build(), @@ -172,70 +158,56 @@ pub fn mint_cycles128_supported_only_on_cycles_minting_canister(env: TestEnv) { .call_and_wait() .await .expect_err("should not succeed"); + let after_balance = get_balance(&canister_id, &agent).await; + (res, before_balance, after_balance, canister_id.to_string()) + }) +} - assert_eq!( - res, - AgentError::CertifiedReject( - RejectResponse { - reject_code: RejectCode::CanisterError, - reject_message: format!( - "Error from Canister {}: Canister violated contract: ic0.mint_cycles cannot be executed on non Cycles Minting Canister: {} != {}.\nThis is likely an error with the compiler/CDK toolchain being used to build the canister. Please report the error to IC devs on the forum: https://forum.dfinity.org and include which language/CDK was used to create the canister.", - not_cmc, not_cmc, - CYCLES_MINTING_CANISTER_ID), - error_code: None}) - ); - - let after_balance = get_balance(¬_cmc, &nns_agent).await; - assert!( - after_balance == before_balance, - "expected {} == {}", - after_balance, - before_balance - ); - }); +pub fn mint_cycles128_supported_only_on_cycles_minting_canister(env: TestEnv) { + let nns_node = env.get_first_healthy_nns_node_snapshot(); + let canister_id = nns_node.get_last_canister_id_in_allocation_ranges(); + // Check that 'canister_id' is not 'CYCLES_MINTING_CANISTER_ID'. + assert_ne!(canister_id, CYCLES_MINTING_CANISTER_ID.into()); + let (res, before_balance, after_balance, canister_id) = setup_ucan_and_try_mint128(nns_node); + assert_eq!( + res, + AgentError::CertifiedReject( + RejectResponse { + reject_code: RejectCode::CanisterError, + reject_message: format!( + "Error from Canister {}: Canister violated contract: ic0.mint_cycles cannot be executed on non Cycles Minting Canister: {} != {}.\nThis is likely an error with the compiler/CDK toolchain being used to build the canister. Please report the error to IC devs on the forum: https://forum.dfinity.org and include which language/CDK was used to create the canister.", + canister_id, canister_id, + CYCLES_MINTING_CANISTER_ID), + error_code: None}) + ); + assert!( + after_balance == before_balance, + "expected {} == {}", + after_balance, + before_balance + ); } pub fn mint_cycles128_not_supported_on_application_subnet(env: TestEnv) { - let initial_cycles = CANISTER_FREEZE_BALANCE_RESERVE + Cycles::new(5_000_000_000_000); let app_node = env.get_first_healthy_application_node_snapshot(); - let agent = app_node.build_default_agent(); - block_on(async move { - let canister_id = UniversalCanister::new_with_cycles( - &agent, - app_node.effective_canister_id(), - initial_cycles * 3u64, - ) - .await - .unwrap() - .canister_id(); - - let before_balance = get_balance(&canister_id, &agent).await; - assert!( - Cycles::from(before_balance) > initial_cycles * 2u64, - "expected {} > {}", - before_balance, - initial_cycles * 2u64 - ); - assert!( - Cycles::from(before_balance) <= initial_cycles * 3u64, - "expected {} <= {}", - before_balance, - initial_cycles * 3u64 - ); - - // The test function on the wasm module will call the mint_cycles system - // call. - let res = agent.update(&canister_id, "test").call_and_wait().await; - - assert_reject(res, RejectCode::CanisterError); - let after_balance = get_balance(&canister_id, &agent).await; - assert!( - after_balance < before_balance, - "expected {} < {}", - after_balance, - before_balance - ); - }); + let (res, before_balance, after_balance, canister_id) = setup_ucan_and_try_mint128(app_node); + assert_eq!( + res, + AgentError::CertifiedReject( + RejectResponse { + reject_code: RejectCode::CanisterError, + reject_message: format!( + "Error from Canister {}: Canister violated contract: ic0.mint_cycles cannot be executed on non Cycles Minting Canister: {} != {}.\nThis is likely an error with the compiler/CDK toolchain being used to build the canister. Please report the error to IC devs on the forum: https://forum.dfinity.org and include which language/CDK was used to create the canister.", + canister_id, canister_id, + CYCLES_MINTING_CANISTER_ID), + error_code: None}) + ); + assert!( + after_balance <= before_balance, + "expected {} <= {}", + after_balance, + before_balance + ); } pub fn no_cycle_balance_limit_on_nns_subnet(env: TestEnv) { From 2f53076e285fb3eca4ec8e686814b3899de6c612 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Tue, 10 Dec 2024 10:30:53 +0000 Subject: [PATCH 29/33] clippy --- rs/tests/execution/general_execution_tests/nns_shielding.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/tests/execution/general_execution_tests/nns_shielding.rs b/rs/tests/execution/general_execution_tests/nns_shielding.rs index f508fa8a95b..88959335b03 100644 --- a/rs/tests/execution/general_execution_tests/nns_shielding.rs +++ b/rs/tests/execution/general_execution_tests/nns_shielding.rs @@ -12,7 +12,7 @@ use ic_nns_constants::CYCLES_MINTING_CANISTER_ID; use ic_system_test_driver::driver::test_env_api::{GetFirstHealthyNodeSnapshot, HasPublicApiUrl}; use ic_system_test_driver::driver::{test_env::TestEnv, test_env_api::IcNodeSnapshot}; use ic_system_test_driver::{util::CYCLES_LIMIT_PER_CANISTER, util::*}; -use ic_types::{Cycles, PrincipalId}; +use ic_types::Cycles; use ic_types_test_utils::ids::node_test_id; use ic_universal_canister::wasm; use lazy_static::lazy_static; From 01bc80860ee30ef0c0f4caa45417a92f30aac6e6 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Wed, 11 Dec 2024 09:11:40 +0000 Subject: [PATCH 30/33] comments --- .../general_execution_tests/nns_shielding.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/rs/tests/execution/general_execution_tests/nns_shielding.rs b/rs/tests/execution/general_execution_tests/nns_shielding.rs index 88959335b03..0e340ab7799 100644 --- a/rs/tests/execution/general_execution_tests/nns_shielding.rs +++ b/rs/tests/execution/general_execution_tests/nns_shielding.rs @@ -139,12 +139,15 @@ pub fn mint_cycles_not_supported_on_application_subnet(env: TestEnv) { fn setup_ucan_and_try_mint128(node: IcNodeSnapshot) -> (AgentError, u128, u128, String) { let agent = node.build_default_agent(); - let canister_id = node.get_last_canister_id_in_allocation_ranges(); + let effective_canister_id = node.get_last_canister_id_in_allocation_ranges(); block_on(async move { - let canister_id = UniversalCanister::new_with_cycles(&agent, canister_id, *INITIAL_CYCLES) - .await - .unwrap() - .canister_id(); + let canister_id = + UniversalCanister::new_with_cycles(&agent, effective_canister_id, *INITIAL_CYCLES) + .await + .unwrap() + .canister_id(); + // Check that 'canister_id' is not 'CYCLES_MINTING_CANISTER_ID'. + assert_ne!(canister_id, CYCLES_MINTING_CANISTER_ID.into()); let before_balance = get_balance(&canister_id, &agent).await; let res = agent .update(&canister_id, "update") @@ -166,8 +169,6 @@ fn setup_ucan_and_try_mint128(node: IcNodeSnapshot) -> (AgentError, u128, u128, pub fn mint_cycles128_supported_only_on_cycles_minting_canister(env: TestEnv) { let nns_node = env.get_first_healthy_nns_node_snapshot(); let canister_id = nns_node.get_last_canister_id_in_allocation_ranges(); - // Check that 'canister_id' is not 'CYCLES_MINTING_CANISTER_ID'. - assert_ne!(canister_id, CYCLES_MINTING_CANISTER_ID.into()); let (res, before_balance, after_balance, canister_id) = setup_ucan_and_try_mint128(nns_node); assert_eq!( res, From 047d0b4b2a72b143ad9f0289ae2bc19c860c9624 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Wed, 11 Dec 2024 09:45:47 +0000 Subject: [PATCH 31/33] comments --- rs/execution_environment/tests/hypervisor.rs | 2 +- .../tests/sandbox_safe_system_state.rs | 92 +++++-------------- 2 files changed, 23 insertions(+), 71 deletions(-) diff --git a/rs/execution_environment/tests/hypervisor.rs b/rs/execution_environment/tests/hypervisor.rs index ff1ef365699..ce9f30ab810 100644 --- a/rs/execution_environment/tests/hypervisor.rs +++ b/rs/execution_environment/tests/hypervisor.rs @@ -2285,7 +2285,7 @@ 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((1u128 << 64) + 2u128)) + .mint_cycles128(Cycles::from(10_000_000_000_u128) .reply_data_append() .reply() .build(); diff --git a/rs/system_api/tests/sandbox_safe_system_state.rs b/rs/system_api/tests/sandbox_safe_system_state.rs index 0952c44efa6..2307aa6285f 100644 --- a/rs/system_api/tests/sandbox_safe_system_state.rs +++ b/rs/system_api/tests/sandbox_safe_system_state.rs @@ -349,12 +349,16 @@ fn mint_cycles_fails_caller_not_on_nns() { ); } -#[test] -fn mint_cycles_very_large_value() { +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(); @@ -364,90 +368,38 @@ fn mint_cycles_very_large_value() { 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 amount_high = u64::MAX; - let amount_low = 50; - let cycles_to_mint = Cycles::from_parts(amount_high, amount_low); + 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, cycles_to_mint.get()); + 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, cycles_to_mint.get()); + assert_eq!( + balance_after - balance_before, + expected_actually_minted.get() + ); } #[test] -fn mint_cycles_max() { - let cycles_account_manager = CyclesAccountManagerBuilder::new() - .with_subnet_type(SubnetType::System) - .build(); - let system_state = SystemStateBuilder::new() - .initial_cycles(Cycles::zero()) - .canister_id(CYCLES_MINTING_CANISTER_ID) - .build(); +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); +} - 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, 0); - let amount_high = u64::MAX; - let amount_low = u64::MAX; - let cycles_to_mint = Cycles::from_parts(amount_high, amount_low); - assert_eq!(cycles_to_mint, u128::MAX.into()); - 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, cycles_to_mint.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, cycles_to_mint.get()); +#[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 cycles_account_manager = CyclesAccountManagerBuilder::new() - .with_subnet_type(SubnetType::System) - .build(); - let initial_amount: u128 = 5; - let system_state = SystemStateBuilder::new() - .initial_cycles(initial_amount.into()) - .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_amount); - let amount_high = u64::MAX; - let amount_low = u64::MAX; - let cycles_to_mint = Cycles::from_parts(amount_high, amount_low); - assert_eq!(cycles_to_mint, u128::MAX.into()); - 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, cycles_to_mint.get() - initial_amount); - 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, - cycles_to_mint.get() - initial_amount - ); + let to_mint = Cycles::from_parts(u64::MAX, u64::MAX); + common_mint_cycles_128(INITIAL_CYCLES, to_mint, to_mint - INITIAL_CYCLES); } #[test] From b5a37828ff1434ddaae6892142658950ada8127b Mon Sep 17 00:00:00 2001 From: michael-weigelt <122277901+michael-weigelt@users.noreply.github.com> Date: Wed, 11 Dec 2024 10:54:13 +0100 Subject: [PATCH 32/33] Update rs/tests/execution/general_execution_tests/nns_shielding.rs Co-authored-by: mraszyk <31483726+mraszyk@users.noreply.github.com> --- rs/tests/execution/general_execution_tests/nns_shielding.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/rs/tests/execution/general_execution_tests/nns_shielding.rs b/rs/tests/execution/general_execution_tests/nns_shielding.rs index 0e340ab7799..1c1c69422be 100644 --- a/rs/tests/execution/general_execution_tests/nns_shielding.rs +++ b/rs/tests/execution/general_execution_tests/nns_shielding.rs @@ -168,7 +168,6 @@ fn setup_ucan_and_try_mint128(node: IcNodeSnapshot) -> (AgentError, u128, u128, pub fn mint_cycles128_supported_only_on_cycles_minting_canister(env: TestEnv) { let nns_node = env.get_first_healthy_nns_node_snapshot(); - let canister_id = nns_node.get_last_canister_id_in_allocation_ranges(); let (res, before_balance, after_balance, canister_id) = setup_ucan_and_try_mint128(nns_node); assert_eq!( res, From 19823383337a975981f9e93c6f7065f1eb880b27 Mon Sep 17 00:00:00 2001 From: Michael Weigelt Date: Wed, 11 Dec 2024 10:29:29 +0000 Subject: [PATCH 33/33] bracket --- rs/execution_environment/tests/hypervisor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/execution_environment/tests/hypervisor.rs b/rs/execution_environment/tests/hypervisor.rs index ce9f30ab810..7f9f649b104 100644 --- a/rs/execution_environment/tests/hypervisor.rs +++ b/rs/execution_environment/tests/hypervisor.rs @@ -2285,7 +2285,7 @@ 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) + .mint_cycles128(Cycles::from(10_000_000_000_u128)) .reply_data_append() .reply() .build();