Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

revert: "feat: [EXC-1753] Add mint_cycles128 API" #3125

Merged
merged 1 commit into from
Dec 11, 2024
Merged

revert: "feat: [EXC-1753] Add mint_cycles128 API" #3125

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions rs/cycles_account_manager/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1015,18 +1015,16 @@ impl CyclesAccountManager {
canister_id: CanisterId,
cycles_balance: &mut Cycles,
amount_to_mint: Cycles,
) -> Result<Cycles, CyclesAccountManagerError> {
) -> Result<(), 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;
// equal to amount_to_mint, except when the addition saturated
Ok(*cycles_balance - before_balance)
Ok(())
}
}

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

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

linker
.func_wrap("ic0", "cycles_burn128", {
move |mut caller: Caller<'_, StoreData>, amount_high: u64, amount_low: u64, dst: I| {
Expand Down
5 changes: 0 additions & 5 deletions rs/execution_environment/benches/system_api/diff-old-vs-new.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ set -ue
## | update/ic0_canister_status() | 1.27G | 1.34G | +5% | 3.73s |
## | inspect/ic0_msg_method_name_size() | - | 1.28G | - | 23.92s |

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

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

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

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

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

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

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

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

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

/// Checks whether the principal identified by src/size is one of the
/// controllers of the canister. If yes, then a value of 1 is returned,
/// otherwise a 0 is returned. It can be called multiple times.
Expand Down
43 changes: 3 additions & 40 deletions rs/system_api/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3168,8 +3168,7 @@ impl SystemApi for SystemApiImpl {
trace_syscall!(self, CanisterStatus, result);
result
}
// TODO(EXC-1806): This can be removed (in favour of ic0_mint_cycles128) once the CMC is upgraded, so it
// doesn't make sense to deduplicate the shared code.

fn ic0_mint_cycles(&mut self, amount: u64) -> HypervisorResult<u64> {
let result = match self.api_type {
ApiType::Start { .. }
Expand All @@ -3188,52 +3187,16 @@ impl SystemApi for SystemApiImpl {
// Access to this syscall not permitted.
Err(self.error_for("ic0_mint_cycles"))
} else {
let actually_minted = self
.sandbox_safe_system_state
self.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_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())
Ok(amount)
}
}
};
trace_syscall!(self, MintCycles, result, amount);
result
}

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

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

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

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

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

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

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

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

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