chore: Revert "revert: "feat: [EXC-1753] Add mint_cycles128 API"" (#3154

)

#2589 caused a problem in a backup
test in the hourly pipeline, so it was reverted in #3125.

#3147 addressed the underlying problem
in the backup test and was merged.
#3151 is 2589+3147 and shows that the
mitigation works, but instead of merging that, it's nicer to revert the
revert:

This PR reverts #3125

rs/cycles_account_manager/src/lib.rs

@@ -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)
         }
     }
 

rs/embedders/src/wasm_utils/validation.rs

@@ -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![(

rs/embedders/src/wasmtime_embedder/system_api.rs

@@ -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| {

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

rs/execution_environment/benches/system_api/execute_update.rs

@@ -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(
@@ -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,
             ),

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

rs/execution_environment/tests/hypervisor.rs

@@ -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();

rs/interfaces/src/execution_environment.rs

@@ -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()`
@@ -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.

rs/system_api/src/lib.rs

@@ -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 { .. }
@@ -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);

rs/system_api/src/sandbox_safe_system_state.rs

@@ -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

rs/system_api/tests/sandbox_safe_system_state.rs

@@ -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();