From 84175f287dad6c2801daccfbbb81cb9cd5aba68d Mon Sep 17 00:00:00 2001 From: Or Ricon Date: Wed, 11 Dec 2024 16:12:44 +0100 Subject: [PATCH 1/5] feat(BOUN-1308): add manually triggered ci workflow for rate-limits canister (#3124) This is a similar PR to https://github.com/dfinity/ic/pull/3106, in this case building and publishing a release of the rate-limits canister used to adjust rate-limit rules for the API Boundary Nodes. --- .../anonymization-backend-release.yml | 2 +- .../workflows/rate-limits-backend-release.yml | 120 ++++++++++++++++++ 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/rate-limits-backend-release.yml diff --git a/.github/workflows/anonymization-backend-release.yml b/.github/workflows/anonymization-backend-release.yml index c62de4eb182..c1b1573bcc4 100644 --- a/.github/workflows/anonymization-backend-release.yml +++ b/.github/workflows/anonymization-backend-release.yml @@ -74,7 +74,7 @@ jobs: RELEASE_TAG="${{ env.NAME }}-${COMMIT_SHORT}" fi - if [[ ! "${RELEASE_TAG}" =~ "^${{ env.NAME }}" ]]; then + if [[ ! "${RELEASE_TAG}" =~ ^${{ env.NAME }} ]]; then echo "ERROR: Required tag prefix: ${{ env.NAME }}" exit 1 fi diff --git a/.github/workflows/rate-limits-backend-release.yml b/.github/workflows/rate-limits-backend-release.yml new file mode 100644 index 00000000000..4a6394bd53e --- /dev/null +++ b/.github/workflows/rate-limits-backend-release.yml @@ -0,0 +1,120 @@ +name: rate-limits-release + +on: + workflow_dispatch: + inputs: + title: + description: 'Title for the release' + required: true + type: string + + description: + description: 'Human-readable description of the release' + required: true + type: string + + tag: + description: 'Tag for the release (required format `rate-limits-*`)' + required: false + type: string + +permissions: + contents: write + +env: + NAME: rate-limits + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + +jobs: + build-and-release: + name: Build and release the rate-limits backend canister + + runs-on: + group: zh1 + labels: dind-large + + container: + image: ghcr.io/dfinity/ic-build@sha256:4fd13b47285e783c3a6f35aadd9559d097c0de162a1cf221ead66ab1598d5d45 + options: >- + -e NODE_NAME --privileged --cgroupns host -v /cache:/cache -v /var/sysimage:/var/sysimage -v /var/tmp:/var/tmp -v /ceph-s3-info:/ceph-s3-info + + steps: + - uses: actions/checkout@v4 + + - name: build + shell: bash + run: | + TARGET='//rs/boundary_node/rate_limits:rate_limit_canister' + bazel build --config=ci ${TARGET} + + OUTPUT='bazel-bin/rs/boundary_node/rate_limits/rate_limit_canister.wasm.gz' + mv ${OUTPUT} rate_limit_canister.wasm.gz + + ARTIFACTS=( + rate_limit_canister.wasm.gz + ) + + echo "ARTIFACTS=${ARTIFACTS[@]}" >> "${GITHUB_ENV}" + + - name: checksums + run: | + CHECKSUMS=$(mktemp) + + for ARTIFACT in ${ARTIFACTS[@]}; do + shasum -a256 ${ARTIFACT} >> ${CHECKSUMS} + done + + echo "CHECKSUMS=${CHECKSUMS}" >> "${GITHUB_ENV}" + + - name: tag + run: | + RELEASE_TAG='${{ inputs.tag }}' + if [[ -z "${RELEASE_TAG}" ]]; then + COMMIT_SHORT=$(git rev-parse --short HEAD) + RELEASE_TAG="${{ env.NAME }}-${COMMIT_SHORT}" + fi + + if [[ ! "${RELEASE_TAG}" =~ ^${{ env.NAME }} ]]; then + echo "ERROR: Required tag prefix: ${{ env.NAME }}" + exit 1 + fi + + git tag ${RELEASE_TAG} + git push origin tag ${RELEASE_TAG} + + echo "RELEASE_TAG=${RELEASE_TAG}" >> "${GITHUB_ENV}" + + - name: release notes + run: | + NOTES=$(mktemp) + + CODE_BLOCK='```' + + cat > ${NOTES} <> "${GITHUB_ENV}" + + - name: release + run: | + gh release create \ + ${RELEASE_TAG} ${ARTIFACTS[@]} \ + --title '${{ inputs.title }}' \ + --verify-tag \ + --latest=false \ + --notes-file ${NOTES} From 7b5227e9dd7298ed01f66deb88981e6668e36e12 Mon Sep 17 00:00:00 2001 From: Daniel Wong <97631336+daniel-wong-dfinity-org@users.noreply.github.com> Date: Wed, 11 Dec 2024 18:22:40 +0100 Subject: [PATCH 2/5] feat(cmc): Limiter metrics. (#3118) These will be used [for alerting]. [for alerting]: https://github.com/dfinity-ops/k8s/pull/604 --- rs/nns/cmc/src/main.rs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/rs/nns/cmc/src/main.rs b/rs/nns/cmc/src/main.rs index 6b8b9f66a02..c821d224ad7 100644 --- a/rs/nns/cmc/src/main.rs +++ b/rs/nns/cmc/src/main.rs @@ -41,7 +41,7 @@ use on_wire::{FromWire, IntoWire, NewType}; use rand::{rngs::StdRng, seq::SliceRandom, SeedableRng}; use serde::{Deserialize, Serialize}; use std::{ - cell::RefCell, + cell::{Cell, RefCell}, collections::{btree_map::Entry, BTreeMap, BTreeSet}, convert::TryInto, thread::LocalKey, @@ -74,6 +74,7 @@ const CREATE_CANISTER_MIN_CYCLES: u64 = 100_000_000_000; thread_local! { static STATE: RefCell> = const { RefCell::new(None) }; + static LIMITER_REJECT_COUNT: Cell = const { Cell::new(0_u64) }; } fn with_state(f: impl FnOnce(&State) -> R) -> R { @@ -2179,6 +2180,10 @@ fn ensure_balance(cycles: Cycles) -> Result<(), String> { let count = state.limiter.get_count(); if count + cycles_to_mint > state.cycles_limit { + LIMITER_REJECT_COUNT.with(|count| { + count.set(count.get().saturating_add(1)); + }); + return Err(format!( "More than {} cycles have been minted in the last {} seconds, please try again later.", state.cycles_limit, @@ -2382,6 +2387,30 @@ fn encode_metrics(w: &mut ic_metrics_encoder::MetricsEncoder>) -> std::i u8::from(state.update_exchange_rate_canister_state.as_ref().unwrap()) as f64, "The current state of the CMC calling the exchange rate canister.", )?; + + w.encode_gauge( + "cmc_limiter_reject_count", + LIMITER_REJECT_COUNT.with(|count| count.get()) as f64, + "The number of times that the limiter has blocked a minting request \ + (since the last upgrade of this canister, or when it was first \ + installed).", + )?; + w.encode_gauge( + "cmc_limiter_cycles", + state.limiter.get_count().get() as f64, + "The amount of cycles minted in the recent past. If someone tries \ + to mint N cycles, but N + the value of this metric exceeds \ + cmc_cycles_limit, then the request will be rejected.", + )?; + w.encode_gauge( + "cmc_cycles_limit", + state.cycles_limit.get() as f64, + "The maximum amount of cycles that can be minted in the recent past. \ + More precisely, if someone tries to mint N cycles, and \ + N + cmc_limiter_cycles > cmc_cycles_limit, then the request will \ + be rejected.", + )?; + Ok(()) }) } From cf0c854ffc810f558809886f3080ceb9491a3f49 Mon Sep 17 00:00:00 2001 From: Andre Popovitch Date: Wed, 11 Dec 2024 11:32:32 -0600 Subject: [PATCH 3/5] test(sns): Do release-qualification for swap upgrades (#3128) This was disabled until we updated SNS Gov on mainnet (as previously SNS Swap did not upgrade). Closes [NNS1-3433](https://dfinity.atlassian.net/browse/NNS1-3433) #2981 already did this for the legacy (UpgradeSnsToNextVersion) upgrade qualifications, this PR does it for AvanceSnsTargetVersion upgrade qualifications as well. [NNS1-3433]: https://dfinity.atlassian.net/browse/NNS1-3433?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Note: This PR also cleans up a comment in `sns_release_qualification_legacy.rs` --- .../integration_tests/tests/sns_release_qualification.rs | 2 -- .../tests/sns_release_qualification_legacy.rs | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/rs/nervous_system/integration_tests/tests/sns_release_qualification.rs b/rs/nervous_system/integration_tests/tests/sns_release_qualification.rs index e6acae37821..c6900c233af 100644 --- a/rs/nervous_system/integration_tests/tests/sns_release_qualification.rs +++ b/rs/nervous_system/integration_tests/tests/sns_release_qualification.rs @@ -84,8 +84,6 @@ async fn test_deployment_swap_upgrade() { } /// Upgrade Tests - -#[ignore] #[tokio::test] async fn test_upgrade_swap() { test_sns_upgrade(vec![SnsCanisterType::Swap]).await; diff --git a/rs/nervous_system/integration_tests/tests/sns_release_qualification_legacy.rs b/rs/nervous_system/integration_tests/tests/sns_release_qualification_legacy.rs index 2445eae7617..0832d0d3690 100644 --- a/rs/nervous_system/integration_tests/tests/sns_release_qualification_legacy.rs +++ b/rs/nervous_system/integration_tests/tests/sns_release_qualification_legacy.rs @@ -1,10 +1,10 @@ +//! Legacy upgrade release-qualification tests + use ic_sns_wasm::pb::v1::SnsCanisterType; mod sns_upgrade_test_utils_legacy; use sns_upgrade_test_utils_legacy::test_sns_upgrade_legacy; -/// Legacy upgrade Tests -/// #[tokio::test] async fn test_upgrade_swap() { test_sns_upgrade_legacy(vec![SnsCanisterType::Swap]).await; From df6536952965e3317f998723ddeb85b59ff53b9d Mon Sep 17 00:00:00 2001 From: Carly Gundy <47304080+cgundy@users.noreply.github.com> Date: Wed, 11 Dec 2024 18:58:56 +0100 Subject: [PATCH 4/5] revert: "feat: [EXC-1753] Add mint_cycles128 API" (#3125) Reverts dfinity/ic#2589 --- rs/cycles_account_manager/src/lib.rs | 6 +- rs/embedders/src/wasm_utils/validation.rs | 10 --- .../src/wasmtime_embedder/system_api.rs | 12 --- .../benches/system_api/diff-old-vs-new.sh | 5 -- .../benches/system_api/execute_update.rs | 26 +----- .../src/query_handler/query_cache/tests.rs | 1 - rs/execution_environment/tests/hypervisor.rs | 66 -------------- rs/interfaces/src/execution_environment.rs | 22 +---- rs/system_api/src/lib.rs | 43 +--------- .../src/sandbox_safe_system_state.rs | 2 +- .../tests/sandbox_safe_system_state.rs | 53 ------------ rs/system_api/tests/system_api.rs | 18 +--- rs/tests/execution/general_execution_test.rs | 4 - .../general_execution_tests/nns_shielding.rs | 85 ++----------------- rs/universal_canister/impl/src/api.rs | 8 -- rs/universal_canister/impl/src/lib.rs | 1 - rs/universal_canister/impl/src/main.rs | 5 -- rs/universal_canister/lib/src/lib.rs | 8 -- 18 files changed, 23 insertions(+), 352 deletions(-) diff --git a/rs/cycles_account_manager/src/lib.rs b/rs/cycles_account_manager/src/lib.rs index 18c5e8e2c45..64583fdc160 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 { + ) -> Result<(), CyclesAccountManagerError> { if canister_id != CYCLES_MINTING_CANISTER_ID { let error_str = format!( "ic0.mint_cycles cannot be executed on non Cycles Minting Canister: {} != {}", @@ -1023,10 +1023,8 @@ impl CyclesAccountManager { ); 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(()) } } diff --git a/rs/embedders/src/wasm_utils/validation.rs b/rs/embedders/src/wasm_utils/validation.rs index f2517c87ba3..f6cf898a876 100644 --- a/rs/embedders/src/wasm_utils/validation.rs +++ b/rs/embedders/src/wasm_utils/validation.rs @@ -527,16 +527,6 @@ 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(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| { 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 049b784e8c3..f6405fd9f2a 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,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 diff --git a/rs/execution_environment/benches/system_api/execute_update.rs b/rs/execution_environment/benches/system_api/execute_update.rs index af476e7d48d..dadd339da0a 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 - 529004006, + 529001006, ), common::Benchmark( "wasm32/ic0_debug_print()/1B".into(), @@ -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(), @@ -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( @@ -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, ), 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 009c6beb9d9..73a65d05c39 100644 --- a/rs/execution_environment/src/query_handler/query_cache/tests.rs +++ b/rs/execution_environment/src/query_handler/query_cache/tests.rs @@ -1488,7 +1488,6 @@ fn query_cache_future_proof_test() { | SystemApiCallId::InReplicatedExecution | SystemApiCallId::IsController | SystemApiCallId::MintCycles - | SystemApiCallId::MintCycles128 | SystemApiCallId::MsgArgDataCopy | SystemApiCallId::MsgArgDataSize | SystemApiCallId::MsgCallerCopy diff --git a/rs/execution_environment/tests/hypervisor.rs b/rs/execution_environment/tests/hypervisor.rs index 7f9f649b104..e8f78482208 100644 --- a/rs/execution_environment/tests/hypervisor.rs +++ b/rs/execution_environment/tests/hypervisor.rs @@ -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(); diff --git a/rs/interfaces/src/execution_environment.rs b/rs/interfaces/src/execution_environment.rs index 970adef0fd3..a89b81ae2e9 100644 --- a/rs/interfaces/src/execution_environment.rs +++ b/rs/interfaces/src/execution_environment.rs @@ -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()` @@ -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; - /// 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. diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 88b69568043..dc29d477041 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -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 { let result = match self.api_type { ApiType::Start { .. } @@ -3188,12 +3187,9 @@ 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) } } }; @@ -3201,39 +3197,6 @@ impl SystemApi for SystemApiImpl { 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); diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index 27ad59f8c39..50bbfe3b3fc 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/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<()> { let mut new_balance = self.cycles_balance(); let result = self .cycles_account_manager diff --git a/rs/system_api/tests/sandbox_safe_system_state.rs b/rs/system_api/tests/sandbox_safe_system_state.rs index 2307aa6285f..8945641c532 100644 --- a/rs/system_api/tests/sandbox_safe_system_state.rs +++ b/rs/system_api/tests/sandbox_safe_system_state.rs @@ -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(); diff --git a/rs/system_api/tests/system_api.rs b/rs/system_api/tests/system_api.rs index 5cb55e539c1..923c2ef4340 100644 --- a/rs/system_api/tests/system_api.rs +++ b/rs/system_api/tests/system_api.rs @@ -293,8 +293,7 @@ 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::MintCycles128 => vec!["U", "Ry", "Rt", "T"] + SystemApiCallId::MintCycles => vec!["U", "Ry", "Rt", "T"] }; // the semantics of "*" is to cover all modes except for "s" matrix.get(&api_type).unwrap().contains(&context) @@ -746,11 +745,6 @@ 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(Cycles::zero(), 0, &mut [0u8; 16])); - } SystemApiCallId::IsController => { assert_api_availability( |api| api.ic0_is_controller(0, 0, &[42; 128]), @@ -828,7 +822,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, ic0.mint_cycles128 API availability for CMC + // check ic0.mint_cycles API availability for CMC let cmc_system_state = get_cmc_system_state(); assert_api_availability( |mut api| api.ic0_mint_cycles(0), @@ -838,14 +832,6 @@ fn system_api_availability() { SystemApiCallId::MintCycles, context, ); - assert_api_availability( - |mut api| api.ic0_mint_cycles128(Cycles::zero(), 0, &mut [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() { diff --git a/rs/tests/execution/general_execution_test.rs b/rs/tests/execution/general_execution_test.rs index a317c029208..0768a864174 100644 --- a/rs/tests/execution/general_execution_test.rs +++ b/rs/tests/execution/general_execution_test.rs @@ -88,10 +88,6 @@ 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 1c1c69422be..7aab49d2083 100644 --- a/rs/tests/execution/general_execution_tests/nns_shielding.rs +++ b/rs/tests/execution/general_execution_tests/nns_shielding.rs @@ -9,14 +9,14 @@ 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_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 = @@ -64,7 +64,11 @@ 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_eq!(INITIAL_CYCLES.get(), before_balance); + assert_balance_equals( + *INITIAL_CYCLES, + Cycles::from(before_balance), + BALANCE_EPSILON, + ); let res = nns_agent .update(&nns_canister_id, "test") @@ -130,86 +134,13 @@ 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 ); }); } -fn setup_ucan_and_try_mint128(node: IcNodeSnapshot) -> (AgentError, u128, u128, String) { - let agent = node.build_default_agent(); - let effective_canister_id = node.get_last_canister_id_in_allocation_ranges(); - block_on(async move { - 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") - .with_arg( - wasm() - .mint_cycles128(Cycles::from(10_000_000_000u128)) - .reply_data_append() - .reply() - .build(), - ) - .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()) - }) -} - -pub fn mint_cycles128_supported_only_on_cycles_minting_canister(env: TestEnv) { - let nns_node = env.get_first_healthy_nns_node_snapshot(); - 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 app_node = env.get_first_healthy_application_node_snapshot(); - 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) { let logger = env.logger(); let nns_node = env.get_first_healthy_nns_node_snapshot(); diff --git a/rs/universal_canister/impl/src/api.rs b/rs/universal_canister/impl/src/api.rs index e93dcb5a68c..fc28d59d9ee 100644 --- a/rs/universal_canister/impl/src/api.rs +++ b/rs/universal_canister/impl/src/api.rs @@ -71,7 +71,6 @@ mod ic0 { pub fn canister_version() -> u64; pub fn mint_cycles(amount: u64) -> u64; - 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,13 +404,6 @@ 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 39f3f03c2d0..0ec29c5dffe 100644 --- a/rs/universal_canister/impl/src/lib.rs +++ b/rs/universal_canister/impl/src/lib.rs @@ -112,6 +112,5 @@ try_from_u8!( CallWithBestEffortResponse = 82, MsgDeadline = 83, MemorySizeIsAtLeast = 84, - MintCycles128 = 85, } ); diff --git a/rs/universal_canister/impl/src/main.rs b/rs/universal_canister/impl/src/main.rs index 2341354f56b..d6635d6fe70 100644 --- a/rs/universal_canister/impl/src/main.rs +++ b/rs/universal_canister/impl/src/main.rs @@ -379,11 +379,6 @@ fn eval(ops_bytes: OpsBytes) { let amount = stack.pop_int64(); stack.push_int64(api::mint_cycles(amount)); } - Ops::MintCycles128 => { - let amount_low = stack.pop_int64(); - let amount_high = stack.pop_int64(); - stack.push_blob(api::mint_cycles128(amount_high, amount_low)) - } Ops::OneWayCallNew => { // pop in reverse order! let method = stack.pop_blob(); diff --git a/rs/universal_canister/lib/src/lib.rs b/rs/universal_canister/lib/src/lib.rs index 7b7e7a2172f..d45544ec6e2 100644 --- a/rs/universal_canister/lib/src/lib.rs +++ b/rs/universal_canister/lib/src/lib.rs @@ -516,14 +516,6 @@ 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 00d999c34899d2c31bc6205302dde0e9cbcdbb98 Mon Sep 17 00:00:00 2001 From: Arshavir Ter-Gabrielyan Date: Wed, 11 Dec 2024 20:15:37 +0100 Subject: [PATCH 5/5] chore: Fix typo in the name of an internal bazel target (#3101) This PR fixes a typo in the name of an internal bazel target. --- Cargo.lock | 2 +- Cargo.toml | 2 +- .../BUILD.bazel | 2 +- .../Cargo.toml | 4 ++-- .../src/main.rs | 0 5 files changed, 5 insertions(+), 5 deletions(-) rename rs/nervous_system/tools/{sync-with-released-nevous-system-wasms => sync-with-released-nervous-system-wasms}/BUILD.bazel (95%) rename rs/nervous_system/tools/{sync-with-released-nevous-system-wasms => sync-with-released-nervous-system-wasms}/Cargo.toml (89%) rename rs/nervous_system/tools/{sync-with-released-nevous-system-wasms => sync-with-released-nervous-system-wasms}/src/main.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index 604edc80b6f..f292c7ed923 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -20410,7 +20410,7 @@ dependencies = [ ] [[package]] -name = "sync-with-released-nevous-system-wasms" +name = "sync-with-released-nervous-system-wasms" version = "0.9.0" dependencies = [ "anyhow", diff --git a/Cargo.toml b/Cargo.toml index f49e9114450..f1f909b2881 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -197,7 +197,7 @@ members = [ "rs/nervous_system/runtime", "rs/nervous_system/string", "rs/nervous_system/temporary", - "rs/nervous_system/tools/sync-with-released-nevous-system-wasms", + "rs/nervous_system/tools/sync-with-released-nervous-system-wasms", "rs/nns/constants", "rs/nns/common", "rs/nns/common/protobuf_generator", diff --git a/rs/nervous_system/tools/sync-with-released-nevous-system-wasms/BUILD.bazel b/rs/nervous_system/tools/sync-with-released-nervous-system-wasms/BUILD.bazel similarity index 95% rename from rs/nervous_system/tools/sync-with-released-nevous-system-wasms/BUILD.bazel rename to rs/nervous_system/tools/sync-with-released-nervous-system-wasms/BUILD.bazel index e6ef1f3a035..bd454fc819f 100644 --- a/rs/nervous_system/tools/sync-with-released-nevous-system-wasms/BUILD.bazel +++ b/rs/nervous_system/tools/sync-with-released-nervous-system-wasms/BUILD.bazel @@ -25,7 +25,7 @@ DEPENDENCIES = [ ] rust_binary( - name = "sync-with-released-nevous-system-wasms", + name = "sync-with-released-nervous-system-wasms", srcs = [ "src/main.rs", ], diff --git a/rs/nervous_system/tools/sync-with-released-nevous-system-wasms/Cargo.toml b/rs/nervous_system/tools/sync-with-released-nervous-system-wasms/Cargo.toml similarity index 89% rename from rs/nervous_system/tools/sync-with-released-nevous-system-wasms/Cargo.toml rename to rs/nervous_system/tools/sync-with-released-nervous-system-wasms/Cargo.toml index 07b8699fb93..446b04da5d2 100644 --- a/rs/nervous_system/tools/sync-with-released-nevous-system-wasms/Cargo.toml +++ b/rs/nervous_system/tools/sync-with-released-nervous-system-wasms/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "sync-with-released-nevous-system-wasms" +name = "sync-with-released-nervous-system-wasms" version.workspace = true authors.workspace = true edition.workspace = true @@ -7,7 +7,7 @@ description.workspace = true documentation.workspace = true [[bin]] -name = "sync-with-released-nevous-system-wasms" +name = "sync-with-released-nervous-system-wasms" path = "src/main.rs" [dependencies] diff --git a/rs/nervous_system/tools/sync-with-released-nevous-system-wasms/src/main.rs b/rs/nervous_system/tools/sync-with-released-nervous-system-wasms/src/main.rs similarity index 100% rename from rs/nervous_system/tools/sync-with-released-nevous-system-wasms/src/main.rs rename to rs/nervous_system/tools/sync-with-released-nervous-system-wasms/src/main.rs