From 68b7ae9bd40cdb217e68a49c3272fbe8819b7466 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 15:42:47 +0200 Subject: [PATCH 01/21] fixtures and simple tests --- substrate/frame/revive/fixtures/build.rs | 1 - .../revive/fixtures/contracts/balance.rs | 8 +- .../frame/revive/fixtures/contracts/call.rs | 8 +- .../fixtures/contracts/call_return_code.rs | 10 +-- .../contracts/call_runtime_and_call.rs | 8 +- .../contracts/call_with_flags_and_value.rs | 10 +-- .../fixtures/contracts/call_with_limit.rs | 4 +- .../fixtures/contracts/caller_contract.rs | 4 +- .../contracts/chain_extension_temp_storage.rs | 8 +- .../fixtures/contracts/common/src/lib.rs | 27 +++++++ .../contracts/create_storage_and_call.rs | 4 +- .../create_storage_and_instantiate.rs | 6 +- .../create_transient_storage_and_call.rs | 2 +- .../fixtures/contracts/delegate_call_lib.rs | 7 +- .../contracts/destroy_and_transfer.rs | 4 +- .../frame/revive/fixtures/contracts/drain.rs | 11 +-- .../contracts/instantiate_return_code.rs | 6 +- .../fixtures/contracts/read_only_call.rs | 8 +- .../revive/fixtures/contracts/recurse.rs | 8 +- .../fixtures/contracts/self_destruct.rs | 8 +- .../contracts/transfer_return_code.rs | 4 +- substrate/frame/revive/src/exec.rs | 16 ++-- substrate/frame/revive/src/tests.rs | 30 +++++--- substrate/frame/revive/src/wasm/runtime.rs | 45 +++-------- substrate/frame/revive/uapi/src/host.rs | 45 ++++------- .../frame/revive/uapi/src/host/riscv32.rs | 77 +++++++------------ 26 files changed, 167 insertions(+), 202 deletions(-) diff --git a/substrate/frame/revive/fixtures/build.rs b/substrate/frame/revive/fixtures/build.rs index 944ae246c1b8..04ac6abd6718 100644 --- a/substrate/frame/revive/fixtures/build.rs +++ b/substrate/frame/revive/fixtures/build.rs @@ -95,7 +95,6 @@ mod build { }; set_dep("uapi", "../uapi")?; set_dep("common", "./contracts/common")?; - cargo_toml["bin"] = toml::Value::Array( entries .map(|entry| { diff --git a/substrate/frame/revive/fixtures/contracts/balance.rs b/substrate/frame/revive/fixtures/contracts/balance.rs index 4011b8379cbf..2db38a3cd5ca 100644 --- a/substrate/frame/revive/fixtures/contracts/balance.rs +++ b/substrate/frame/revive/fixtures/contracts/balance.rs @@ -18,7 +18,7 @@ #![no_std] #![no_main] -use common::output; +use common::u64_output; use uapi::{HostFn, HostFnImpl as api}; #[no_mangle] @@ -29,8 +29,6 @@ pub extern "C" fn deploy() {} #[polkavm_derive::polkavm_export] pub extern "C" fn call() { // Initialize buffer with 1s so that we can check that it is overwritten. - output!(balance, [1u8; 8], api::balance,); - - // Assert that the balance is 0. - assert_eq!(&[0u8; 8], balance); + let balance = u64_output!(api::balance,); + assert_eq!(balance, 0); } diff --git a/substrate/frame/revive/fixtures/contracts/call.rs b/substrate/frame/revive/fixtures/contracts/call.rs index 93687441fa50..ee51548879d9 100644 --- a/substrate/frame/revive/fixtures/contracts/call.rs +++ b/substrate/frame/revive/fixtures/contracts/call.rs @@ -38,10 +38,10 @@ pub extern "C" fn call() { api::call( uapi::CallFlags::empty(), callee_addr, - 0u64, // How much ref_time to devote for the execution. 0 = all. - 0u64, // How much proof_size to devote for the execution. 0 = all. - None, // No deposit limit. - &0u64.to_le_bytes(), // Value transferred to the contract. + 0u64, // How much ref_time to devote for the execution. 0 = all. + 0u64, // How much proof_size to devote for the execution. 0 = all. + None, // No deposit limit. + &[0u8; 32], // Value transferred to the contract. callee_input, None, ) diff --git a/substrate/frame/revive/fixtures/contracts/call_return_code.rs b/substrate/frame/revive/fixtures/contracts/call_return_code.rs index 29b77c343fe9..25370459acb4 100644 --- a/substrate/frame/revive/fixtures/contracts/call_return_code.rs +++ b/substrate/frame/revive/fixtures/contracts/call_return_code.rs @@ -21,7 +21,7 @@ #![no_std] #![no_main] -use common::input; +use common::{input, u256_bytes}; use uapi::{HostFn, HostFnImpl as api}; #[no_mangle] @@ -41,10 +41,10 @@ pub extern "C" fn call() { let err_code = match api::call( uapi::CallFlags::empty(), callee_addr, - 0u64, // How much ref_time to devote for the execution. 0 = all. - 0u64, // How much proof_size to devote for the execution. 0 = all. - None, // No deposit limit. - &100u64.to_le_bytes(), // Value transferred to the contract. + 0u64, // How much ref_time to devote for the execution. 0 = all. + 0u64, // How much proof_size to devote for the execution. 0 = all. + None, // No deposit limit. + &u256_bytes(100u64), // Value transferred to the contract. input, None, ) { diff --git a/substrate/frame/revive/fixtures/contracts/call_runtime_and_call.rs b/substrate/frame/revive/fixtures/contracts/call_runtime_and_call.rs index 7cd46849655f..8c8aee962849 100644 --- a/substrate/frame/revive/fixtures/contracts/call_runtime_and_call.rs +++ b/substrate/frame/revive/fixtures/contracts/call_runtime_and_call.rs @@ -42,10 +42,10 @@ pub extern "C" fn call() { api::call( uapi::CallFlags::empty(), callee_addr, - 0u64, // How much ref_time to devote for the execution. 0 = all. - 0u64, // How much proof_size to devote for the execution. 0 = all. - None, // No deposit limit. - &0u64.to_le_bytes(), // Value transferred to the contract. + 0u64, // How much ref_time to devote for the execution. 0 = all. + 0u64, // How much proof_size to devote for the execution. 0 = all. + None, // No deposit limit. + &[0u8; 32], // Value transferred to the contract. callee_input, None, ) diff --git a/substrate/frame/revive/fixtures/contracts/call_with_flags_and_value.rs b/substrate/frame/revive/fixtures/contracts/call_with_flags_and_value.rs index c3204c29281c..330393e706e9 100644 --- a/substrate/frame/revive/fixtures/contracts/call_with_flags_and_value.rs +++ b/substrate/frame/revive/fixtures/contracts/call_with_flags_and_value.rs @@ -19,7 +19,7 @@ #![no_std] #![no_main] -use common::input; +use common::{input, u256_bytes}; use uapi::{HostFn, HostFnImpl as api}; #[no_mangle] @@ -40,10 +40,10 @@ pub extern "C" fn call() { api::call( uapi::CallFlags::from_bits(flags).unwrap(), callee_addr, - 0u64, // How much ref_time to devote for the execution. 0 = all. - 0u64, // How much proof_size to devote for the execution. 0 = all. - None, // No deposit limit. - &value.to_le_bytes(), // Value transferred to the contract. + 0u64, // How much ref_time to devote for the execution. 0 = all. + 0u64, // How much proof_size to devote for the execution. 0 = all. + None, // No deposit limit. + &u256_bytes(value), // Value transferred to the contract. forwarded_input, None, ) diff --git a/substrate/frame/revive/fixtures/contracts/call_with_limit.rs b/substrate/frame/revive/fixtures/contracts/call_with_limit.rs index a941aa9a3421..6ab892a6b7ae 100644 --- a/substrate/frame/revive/fixtures/contracts/call_with_limit.rs +++ b/substrate/frame/revive/fixtures/contracts/call_with_limit.rs @@ -43,8 +43,8 @@ pub extern "C" fn call() { callee_addr, ref_time, proof_size, - None, // No deposit limit. - &0u64.to_le_bytes(), // value transferred to the contract. + None, // No deposit limit. + &[0u8; 32], // value transferred to the contract. forwarded_input, None, ) diff --git a/substrate/frame/revive/fixtures/contracts/caller_contract.rs b/substrate/frame/revive/fixtures/contracts/caller_contract.rs index 3b83f208d623..eb29fca87c15 100644 --- a/substrate/frame/revive/fixtures/contracts/caller_contract.rs +++ b/substrate/frame/revive/fixtures/contracts/caller_contract.rs @@ -18,7 +18,7 @@ #![no_std] #![no_main] -use common::input; +use common::{input, u256_bytes}; use uapi::{HostFn, HostFnImpl as api, ReturnErrorCode}; #[no_mangle] @@ -32,7 +32,7 @@ pub extern "C" fn call() { // The value to transfer on instantiation and calls. Chosen to be greater than existential // deposit. - let value = 32768u64.to_le_bytes(); + let value = u256_bytes(32768u64); let salt = [0u8; 32]; // Callee will use the first 4 bytes of the input to return an exit status. diff --git a/substrate/frame/revive/fixtures/contracts/chain_extension_temp_storage.rs b/substrate/frame/revive/fixtures/contracts/chain_extension_temp_storage.rs index bb5c1ccbc1d6..22d6c5b548d8 100644 --- a/substrate/frame/revive/fixtures/contracts/chain_extension_temp_storage.rs +++ b/substrate/frame/revive/fixtures/contracts/chain_extension_temp_storage.rs @@ -54,10 +54,10 @@ pub extern "C" fn call() { api::call( uapi::CallFlags::ALLOW_REENTRY, &addr, - 0u64, // How much ref_time to devote for the execution. 0 = all. - 0u64, // How much proof_size to devote for the execution. 0 = all. - None, // No deposit limit. - &0u64.to_le_bytes(), // Value transferred to the contract. + 0u64, // How much ref_time to devote for the execution. 0 = all. + 0u64, // How much proof_size to devote for the execution. 0 = all. + None, // No deposit limit. + &[0u8; 32], // Value transferred to the contract. input, None, ) diff --git a/substrate/frame/revive/fixtures/contracts/common/src/lib.rs b/substrate/frame/revive/fixtures/contracts/common/src/lib.rs index 947247e9cf74..02c7d62c58fd 100644 --- a/substrate/frame/revive/fixtures/contracts/common/src/lib.rs +++ b/substrate/frame/revive/fixtures/contracts/common/src/lib.rs @@ -167,3 +167,30 @@ macro_rules! unwrap_output { $host_fn($($arg,)* $output).unwrap(); }; } + +/// Call the host function and convert the [u8; 32] output to u64. +#[macro_export] +macro_rules! u64_output { + ($host_fn:path, $($arg:expr),*) => {{ + let mut buffer = [1u8; 32]; + $host_fn($($arg,)* &mut buffer); + assert!(buffer[8..].iter().all(|&x| x == 0)); + u64::from_le_bytes(buffer[..8].try_into().unwrap()) + }}; +} + +/// Convert a u* into a [u8; 32]. +pub const fn u256_bytes(value: u64) -> [u8; 32] { + let mut buffer = [0u8; 32]; + let bytes = value.to_le_bytes(); + + buffer[0] = bytes[0]; + buffer[1] = bytes[1]; + buffer[2] = bytes[2]; + buffer[3] = bytes[3]; + buffer[4] = bytes[4]; + buffer[5] = bytes[5]; + buffer[6] = bytes[6]; + buffer[7] = bytes[7]; + buffer +} diff --git a/substrate/frame/revive/fixtures/contracts/create_storage_and_call.rs b/substrate/frame/revive/fixtures/contracts/create_storage_and_call.rs index 28d161791e5b..4fa2db0c8c1c 100644 --- a/substrate/frame/revive/fixtures/contracts/create_storage_and_call.rs +++ b/substrate/frame/revive/fixtures/contracts/create_storage_and_call.rs @@ -33,7 +33,7 @@ pub extern "C" fn call() { buffer, input: [u8; 4], callee: &[u8; 20], - deposit_limit: [u8; 8], + deposit_limit: &[u8; 32], ); // create 4 byte of storage before calling @@ -46,7 +46,7 @@ pub extern "C" fn call() { 0u64, // How much ref_time weight to devote for the execution. 0 = all. 0u64, // How much proof_size weight to devote for the execution. 0 = all. Some(deposit_limit), - &0u64.to_le_bytes(), // Value transferred to the contract. + &[0u8; 32], // Value transferred to the contract. input, None, ) diff --git a/substrate/frame/revive/fixtures/contracts/create_storage_and_instantiate.rs b/substrate/frame/revive/fixtures/contracts/create_storage_and_instantiate.rs index d87c2e8cd35a..e1372e2eb8b6 100644 --- a/substrate/frame/revive/fixtures/contracts/create_storage_and_instantiate.rs +++ b/substrate/frame/revive/fixtures/contracts/create_storage_and_instantiate.rs @@ -19,7 +19,7 @@ #![no_std] #![no_main] -use common::input; +use common::{input, u256_bytes}; use uapi::{HostFn, HostFnImpl as api}; #[no_mangle] @@ -32,10 +32,10 @@ pub extern "C" fn call() { input!( input: [u8; 4], code_hash: &[u8; 32], - deposit_limit: [u8; 8], + deposit_limit: &[u8; 32], ); - let value = 10_000u64.to_le_bytes(); + let value = u256_bytes(10_000u64); let salt = [0u8; 32]; let mut address = [0u8; 20]; diff --git a/substrate/frame/revive/fixtures/contracts/create_transient_storage_and_call.rs b/substrate/frame/revive/fixtures/contracts/create_transient_storage_and_call.rs index 753490cf26b7..d2efb26e5ceb 100644 --- a/substrate/frame/revive/fixtures/contracts/create_transient_storage_and_call.rs +++ b/substrate/frame/revive/fixtures/contracts/create_transient_storage_and_call.rs @@ -52,7 +52,7 @@ pub extern "C" fn call() { 0u64, // How much ref_time weight to devote for the execution. 0 = all. 0u64, // How much proof_size weight to devote for the execution. 0 = all. None, - &0u64.to_le_bytes(), // Value transferred to the contract. + &[0u8; 32], // Value transferred to the contract. input, None, ) diff --git a/substrate/frame/revive/fixtures/contracts/delegate_call_lib.rs b/substrate/frame/revive/fixtures/contracts/delegate_call_lib.rs index c5525423a9ee..95c1bd2aa6cd 100644 --- a/substrate/frame/revive/fixtures/contracts/delegate_call_lib.rs +++ b/substrate/frame/revive/fixtures/contracts/delegate_call_lib.rs @@ -18,7 +18,7 @@ #![no_std] #![no_main] -use common::output; +use common::u64_output; use uapi::{HostFn, HostFnImpl as api, StorageFlags}; #[no_mangle] @@ -39,9 +39,8 @@ pub extern "C" fn call() { // Assert that `value_transferred` is equal to the value // passed to the `caller` contract: 1337. - output!(value_transferred, [0u8; 8], api::value_transferred,); - let value_transferred = u64::from_le_bytes(value_transferred[..].try_into().unwrap()); - assert_eq!(value_transferred, 1337); + let value = u64_output!(api::value_transferred,); + assert_eq!(value, 1337); // Assert that ALICE is the caller of the contract. let mut caller = [0u8; 20]; diff --git a/substrate/frame/revive/fixtures/contracts/destroy_and_transfer.rs b/substrate/frame/revive/fixtures/contracts/destroy_and_transfer.rs index 4959a5e2e0ce..d381db8e398f 100644 --- a/substrate/frame/revive/fixtures/contracts/destroy_and_transfer.rs +++ b/substrate/frame/revive/fixtures/contracts/destroy_and_transfer.rs @@ -18,11 +18,11 @@ #![no_std] #![no_main] -use common::input; +use common::{input, u256_bytes}; use uapi::{HostFn, HostFnImpl as api, StorageFlags}; const ADDRESS_KEY: [u8; 32] = [0u8; 32]; -const VALUE: [u8; 8] = [0, 0, 1u8, 0, 0, 0, 0, 0]; +const VALUE: [u8; 32] = u256_bytes(65536); #[no_mangle] #[polkavm_derive::polkavm_export] diff --git a/substrate/frame/revive/fixtures/contracts/drain.rs b/substrate/frame/revive/fixtures/contracts/drain.rs index b46d4f7c8418..0d644a4238c4 100644 --- a/substrate/frame/revive/fixtures/contracts/drain.rs +++ b/substrate/frame/revive/fixtures/contracts/drain.rs @@ -18,7 +18,7 @@ #![no_std] #![no_main] -use common::output; +use common::{u256_bytes, u64_output}; use uapi::{HostFn, HostFnImpl as api}; #[no_mangle] @@ -28,17 +28,14 @@ pub extern "C" fn deploy() {} #[no_mangle] #[polkavm_derive::polkavm_export] pub extern "C" fn call() { - output!(balance, [0u8; 8], api::balance,); - let balance = u64::from_le_bytes(balance[..].try_into().unwrap()); - - output!(minimum_balance, [0u8; 8], api::minimum_balance,); - let minimum_balance = u64::from_le_bytes(minimum_balance[..].try_into().unwrap()); + let balance = u64_output!(api::balance,); + let minimum_balance = u64_output!(api::minimum_balance,); // Make the transferred value exceed the balance by adding the minimum balance. let balance = balance + minimum_balance; // Try to self-destruct by sending more balance to the 0 address. // The call will fail because a contract transfer has a keep alive requirement. - let res = api::transfer(&[0u8; 20], &balance.to_le_bytes()); + let res = api::transfer(&[0u8; 20], &u256_bytes(balance)); assert!(matches!(res, Err(uapi::ReturnErrorCode::TransferFailed))); } diff --git a/substrate/frame/revive/fixtures/contracts/instantiate_return_code.rs b/substrate/frame/revive/fixtures/contracts/instantiate_return_code.rs index a81ffea943d4..c5736850960a 100644 --- a/substrate/frame/revive/fixtures/contracts/instantiate_return_code.rs +++ b/substrate/frame/revive/fixtures/contracts/instantiate_return_code.rs @@ -18,7 +18,7 @@ #![no_std] #![no_main] -use common::input; +use common::{input, u256_bytes}; use uapi::{HostFn, HostFnImpl as api}; #[no_mangle] @@ -36,8 +36,8 @@ pub extern "C" fn call() { 0u64, // How much ref_time weight to devote for the execution. 0 = all. 0u64, /* How much proof_size weight to devote for the execution. 0 = * all. */ - None, // No deposit limit. - &10_000u64.to_le_bytes(), // Value to transfer. + None, // No deposit limit. + &u256_bytes(10_000u64), // Value to transfer. input, None, None, diff --git a/substrate/frame/revive/fixtures/contracts/read_only_call.rs b/substrate/frame/revive/fixtures/contracts/read_only_call.rs index 7476b7a8366d..ea74d56867f5 100644 --- a/substrate/frame/revive/fixtures/contracts/read_only_call.rs +++ b/substrate/frame/revive/fixtures/contracts/read_only_call.rs @@ -39,10 +39,10 @@ pub extern "C" fn call() { api::call( uapi::CallFlags::READ_ONLY, callee_addr, - 0u64, // How much ref_time to devote for the execution. 0 = all. - 0u64, // How much proof_size to devote for the execution. 0 = all. - None, // No deposit limit. - &0u64.to_le_bytes(), // Value transferred to the contract. + 0u64, // How much ref_time to devote for the execution. 0 = all. + 0u64, // How much proof_size to devote for the execution. 0 = all. + None, // No deposit limit. + &[0u8; 32], // Value transferred to the contract. callee_input, None, ) diff --git a/substrate/frame/revive/fixtures/contracts/recurse.rs b/substrate/frame/revive/fixtures/contracts/recurse.rs index c15784b7f245..2e70d67d8c73 100644 --- a/substrate/frame/revive/fixtures/contracts/recurse.rs +++ b/substrate/frame/revive/fixtures/contracts/recurse.rs @@ -43,10 +43,10 @@ pub extern "C" fn call() { api::call( uapi::CallFlags::ALLOW_REENTRY, &addr, - 0u64, // How much ref_time to devote for the execution. 0 = all. - 0u64, // How much deposit_limit to devote for the execution. 0 = all. - None, // No deposit limit. - &0u64.to_le_bytes(), // Value transferred to the contract. + 0u64, // How much ref_time to devote for the execution. 0 = all. + 0u64, // How much deposit_limit to devote for the execution. 0 = all. + None, // No deposit limit. + &[0u8; 32], // Value transferred to the contract. &(calls_left - 1).to_le_bytes(), None, ) diff --git a/substrate/frame/revive/fixtures/contracts/self_destruct.rs b/substrate/frame/revive/fixtures/contracts/self_destruct.rs index 0e1e4d30e6f3..524979991ec7 100644 --- a/substrate/frame/revive/fixtures/contracts/self_destruct.rs +++ b/substrate/frame/revive/fixtures/contracts/self_destruct.rs @@ -42,10 +42,10 @@ pub extern "C" fn call() { api::call( uapi::CallFlags::ALLOW_REENTRY, &addr, - 0u64, // How much ref_time to devote for the execution. 0 = all. - 0u64, // How much proof_size to devote for the execution. 0 = all. - None, // No deposit limit. - &0u64.to_le_bytes(), // Value to transfer. + 0u64, // How much ref_time to devote for the execution. 0 = all. + 0u64, // How much proof_size to devote for the execution. 0 = all. + None, // No deposit limit. + &[0u8; 32], // Value to transfer. &[0u8; 0], None, ) diff --git a/substrate/frame/revive/fixtures/contracts/transfer_return_code.rs b/substrate/frame/revive/fixtures/contracts/transfer_return_code.rs index 3e1f2757c27a..bfeca9b8b4a4 100644 --- a/substrate/frame/revive/fixtures/contracts/transfer_return_code.rs +++ b/substrate/frame/revive/fixtures/contracts/transfer_return_code.rs @@ -18,7 +18,7 @@ #![no_std] #![no_main] -extern crate common; +use common::u256_bytes; use uapi::{HostFn, HostFnImpl as api}; #[no_mangle] @@ -28,7 +28,7 @@ pub extern "C" fn deploy() {} #[no_mangle] #[polkavm_derive::polkavm_export] pub extern "C" fn call() { - let ret_code = match api::transfer(&[0u8; 20], &100u64.to_le_bytes()) { + let ret_code = match api::transfer(&[0u8; 20], &u256_bytes(100u64)) { Ok(_) => 0u32, Err(code) => code as u32, }; diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index 649479f7790f..dd3e97b21e4f 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -49,7 +49,7 @@ use frame_system::{ use sp_core::{ ecdsa::Public as ECDSAPublic, sr25519::{Public as SR25519Public, Signature as SR25519Signature}, - ConstU32, Get, H160, H256, + ConstU32, Get, H160, H256, U256, }; use sp_io::{crypto::secp256k1_ecdsa_recover_compressed, hashing::blake2_256}; use sp_runtime::{ @@ -304,16 +304,16 @@ pub trait Ext: sealing::Sealed { /// Returns the balance of the current contract. /// /// The `value_transferred` is already added. - fn balance(&self) -> BalanceOf; + fn balance(&self) -> U256; /// Returns the value transferred along with this call. - fn value_transferred(&self) -> BalanceOf; + fn value_transferred(&self) -> U256; - /// Returns a reference to the timestamp of the current block - fn now(&self) -> &MomentOf; + /// Returns the timestamp of the current block + fn now(&self) -> U256; /// Returns the minimum balance that is required for creating an account. - fn minimum_balance(&self) -> BalanceOf; + fn minimum_balance(&self) -> U256; /// Deposit an event with the given topics. /// @@ -321,13 +321,13 @@ pub trait Ext: sealing::Sealed { fn deposit_event(&mut self, topics: Vec>, data: Vec); /// Returns the current block number. - fn block_number(&self) -> BlockNumberFor; + fn block_number(&self) -> U256; /// Returns the maximum allowed size of a storage item. fn max_value_size(&self) -> u32; /// Returns the price for the specified amount of weight. - fn get_weight_price(&self, weight: Weight) -> BalanceOf; + fn get_weight_price(&self, weight: Weight) -> U256; /// Get an immutable reference to the nested gas meter. fn gas_meter(&self) -> &GasMeter; diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index 447d55f0dd8d..717d020a5813 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -3402,7 +3402,7 @@ mod run_tests { assert_err_ignore_postinfo!( builder::call(addr_caller) .storage_deposit_limit(13) - .data((100u32, &addr_callee, 0u64).encode()) + .data((100u32, &addr_callee, U256::from(0u64)).encode()) .build(), >::StorageDepositLimitExhausted, ); @@ -3416,7 +3416,7 @@ mod run_tests { assert_err_ignore_postinfo!( builder::call(addr_caller) .storage_deposit_limit(14) - .data((101u32, &addr_callee, 0u64).encode()) + .data((101u32, &addr_callee, U256::from(0u64)).encode()) .build(), >::StorageDepositLimitExhausted, ); @@ -3429,7 +3429,7 @@ mod run_tests { assert_err_ignore_postinfo!( builder::call(addr_caller) .storage_deposit_limit(16) - .data((102u32, &addr_callee, 1u64).encode()) + .data((102u32, &addr_callee, U256::from(1u64)).encode()) .build(), >::StorageDepositLimitExhausted, ); @@ -3440,7 +3440,7 @@ mod run_tests { assert_err_ignore_postinfo!( builder::call(addr_caller) .storage_deposit_limit(0) - .data((87u32, &addr_callee, 0u64).encode()) + .data((87u32, &addr_callee, U256::from(0u64)).encode()) .build(), >::StorageDepositLimitExhausted, ); @@ -3450,7 +3450,9 @@ mod run_tests { // Require more than the sender's balance. // We don't set a special limit for the nested call. assert_err_ignore_postinfo!( - builder::call(addr_caller).data((512u32, &addr_callee, 1u64).encode()).build(), + builder::call(addr_caller) + .data((512u32, &addr_callee, U256::from(1u64)).encode()) + .build(), >::StorageDepositLimitExhausted, ); @@ -3459,7 +3461,7 @@ mod run_tests { // enforced as callee frees up storage. This should pass. assert_ok!(builder::call(addr_caller) .storage_deposit_limit(1) - .data((87u32, &addr_callee, 1u64).encode()) + .data((87u32, &addr_callee, U256::from(1u64)).encode()) .build()); }); } @@ -3500,7 +3502,7 @@ mod run_tests { builder::call(addr_caller) .origin(RuntimeOrigin::signed(BOB)) .storage_deposit_limit(callee_info_len + 2 + ED + 1) - .data((0u32, &code_hash_callee, 0u64).encode()) + .data((0u32, &code_hash_callee, U256::from(0u64)).encode()) .build(), >::StorageDepositLimitExhausted, ); @@ -3514,7 +3516,7 @@ mod run_tests { builder::call(addr_caller) .origin(RuntimeOrigin::signed(BOB)) .storage_deposit_limit(callee_info_len + 2 + ED + 2) - .data((1u32, &code_hash_callee, 0u64).encode()) + .data((1u32, &code_hash_callee, U256::from(0u64)).encode()) .build(), >::StorageDepositLimitExhausted, ); @@ -3528,7 +3530,10 @@ mod run_tests { builder::call(addr_caller) .origin(RuntimeOrigin::signed(BOB)) .storage_deposit_limit(callee_info_len + 2 + ED + 2) - .data((0u32, &code_hash_callee, callee_info_len + 2 + ED + 1).encode()) + .data( + (0u32, &code_hash_callee, U256::from(callee_info_len + 2 + ED + 1)) + .encode() + ) .build(), >::StorageDepositLimitExhausted, ); @@ -3543,7 +3548,10 @@ mod run_tests { builder::call(addr_caller) .origin(RuntimeOrigin::signed(BOB)) .storage_deposit_limit(callee_info_len + 2 + ED + 3) // enough parent limit - .data((1u32, &code_hash_callee, callee_info_len + 2 + ED + 2).encode()) + .data( + (1u32, &code_hash_callee, U256::from(callee_info_len + 2 + ED + 2)) + .encode() + ) .build(), >::StorageDepositLimitExhausted, ); @@ -3554,7 +3562,7 @@ mod run_tests { let result = builder::bare_call(addr_caller) .origin(RuntimeOrigin::signed(BOB)) .storage_deposit_limit(callee_info_len + 2 + ED + 4) - .data((1u32, &code_hash_callee, callee_info_len + 2 + ED + 3).encode()) + .data((1u32, &code_hash_callee, U256::from(callee_info_len + 2 + ED + 3)).encode()) .build(); let returned = result.result.unwrap(); diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index 51c723493847..241a01c61241 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -1477,17 +1477,11 @@ pub mod env { /// Stores the *free* balance of the current account into the supplied buffer. /// See [`pallet_revive_uapi::HostFn::balance`]. #[api_version(0)] - fn balance( - &mut self, - memory: &mut M, - out_ptr: u32, - out_len_ptr: u32, - ) -> Result<(), TrapReason> { + fn balance(&mut self, memory: &mut M, out_ptr: u32) -> Result<(), TrapReason> { self.charge_gas(RuntimeCosts::Balance)?; - Ok(self.write_sandbox_output( + Ok(self.write_fixed_sandbox_output( memory, out_ptr, - out_len_ptr, &self.ext.balance().encode(), false, already_charged, @@ -1497,17 +1491,11 @@ pub mod env { /// Stores the value transferred along with this call/instantiate into the supplied buffer. /// See [`pallet_revive_uapi::HostFn::value_transferred`]. #[api_version(0)] - fn value_transferred( - &mut self, - memory: &mut M, - out_ptr: u32, - out_len_ptr: u32, - ) -> Result<(), TrapReason> { + fn value_transferred(&mut self, memory: &mut M, out_ptr: u32) -> Result<(), TrapReason> { self.charge_gas(RuntimeCosts::ValueTransferred)?; - Ok(self.write_sandbox_output( + Ok(self.write_fixed_sandbox_output( memory, out_ptr, - out_len_ptr, &self.ext.value_transferred().encode(), false, already_charged, @@ -1517,12 +1505,11 @@ pub mod env { /// Load the latest block timestamp into the supplied buffer /// See [`pallet_revive_uapi::HostFn::now`]. #[api_version(0)] - fn now(&mut self, memory: &mut M, out_ptr: u32, out_len_ptr: u32) -> Result<(), TrapReason> { + fn now(&mut self, memory: &mut M, out_ptr: u32) -> Result<(), TrapReason> { self.charge_gas(RuntimeCosts::Now)?; - Ok(self.write_sandbox_output( + Ok(self.write_fixed_sandbox_output( memory, out_ptr, - out_len_ptr, &self.ext.now().encode(), false, already_charged, @@ -1532,17 +1519,11 @@ pub mod env { /// Stores the minimum balance (a.k.a. existential deposit) into the supplied buffer. /// See [`pallet_revive_uapi::HostFn::minimum_balance`]. #[api_version(0)] - fn minimum_balance( - &mut self, - memory: &mut M, - out_ptr: u32, - out_len_ptr: u32, - ) -> Result<(), TrapReason> { + fn minimum_balance(&mut self, memory: &mut M, out_ptr: u32) -> Result<(), TrapReason> { self.charge_gas(RuntimeCosts::MinimumBalance)?; - Ok(self.write_sandbox_output( + Ok(self.write_fixed_sandbox_output( memory, out_ptr, - out_len_ptr, &self.ext.minimum_balance().encode(), false, already_charged, @@ -1589,17 +1570,11 @@ pub mod env { /// Stores the current block number of the current contract into the supplied buffer. /// See [`pallet_revive_uapi::HostFn::block_number`]. #[api_version(0)] - fn block_number( - &mut self, - memory: &mut M, - out_ptr: u32, - out_len_ptr: u32, - ) -> Result<(), TrapReason> { + fn block_number(&mut self, memory: &mut M, out_ptr: u32) -> Result<(), TrapReason> { self.charge_gas(RuntimeCosts::BlockNumber)?; - Ok(self.write_sandbox_output( + Ok(self.write_fixed_sandbox_output( memory, out_ptr, - out_len_ptr, &self.ext.block_number().encode(), false, already_charged, diff --git a/substrate/frame/revive/uapi/src/host.rs b/substrate/frame/revive/uapi/src/host.rs index f52ea9574025..fc59e6dc423f 100644 --- a/substrate/frame/revive/uapi/src/host.rs +++ b/substrate/frame/revive/uapi/src/host.rs @@ -58,21 +58,17 @@ pub trait HostFn: private::Sealed { /// Stores the *free* balance of the current account into the supplied buffer. /// - /// If the available space in `output` is less than the size of the value a trap is triggered. - /// /// # Parameters /// /// - `output`: A reference to the output data buffer to write the balance. - fn balance(output: &mut &mut [u8]); + fn balance(output: &mut [u8; 32]); /// Stores the current block number of the current contract into the supplied buffer. /// - /// If the available space in `output` is less than the size of the value a trap is triggered. - /// /// # Parameters /// /// - `output`: A reference to the output data buffer to write the block number. - fn block_number(output: &mut &mut [u8]); + fn block_number(output: &mut [u8; 32]); /// Call (possibly transferring some amount of funds) into the specified account. /// @@ -86,8 +82,7 @@ pub trait HostFn: private::Sealed { /// - `deposit`: The storage deposit limit for instantiation. Should be decodable as a /// `Option`. Traps otherwise. Passing `None` means setting no specific limit for /// the call, which implies storage usage up to the limit of the parent call. - /// - `value`: The value to transfer into the contract. Should be decodable as a `T::Balance`. - /// Traps otherwise. + /// - `value`: The U256 value to transfer into the contract. /// - `input`: The input data buffer used to call the contract. /// - `output`: A reference to the output data buffer to write the call output buffer. If `None` /// is provided then the output buffer is not copied. @@ -106,8 +101,8 @@ pub trait HostFn: private::Sealed { callee: &[u8; 20], ref_time_limit: u64, proof_size_limit: u64, - deposit: Option<&[u8]>, - value: &[u8], + deposit: Option<&[u8; 32]>, + value: &[u8; 32], input_data: &[u8], output: Option<&mut &mut [u8]>, ) -> Result; @@ -377,8 +372,7 @@ pub trait HostFn: private::Sealed { /// - `deposit`: The storage deposit limit for instantiation. Should be decodable as a /// `Option`. Traps otherwise. Passing `None` means setting no specific limit for /// the call, which implies storage usage up to the limit of the parent call. - /// - `value`: The value to transfer into the contract. Should be decodable as a `T::Balance`. - /// Traps otherwise. + /// - `value`: The U256 value to transfer into the contract. /// - `input`: The input data buffer. /// - `address`: A reference to the address buffer to write the address of the contract. If /// `None` is provided then the output buffer is not copied. @@ -402,8 +396,8 @@ pub trait HostFn: private::Sealed { code_hash: &[u8; 32], ref_time_limit: u64, proof_size_limit: u64, - deposit: Option<&[u8]>, - value: &[u8], + deposit: Option<&[u8; 32]>, + value: &[u8; 32], input: &[u8], address: Option<&mut [u8; 20]>, output: Option<&mut &mut [u8]>, @@ -422,14 +416,11 @@ pub trait HostFn: private::Sealed { fn is_contract(address: &[u8; 20]) -> bool; /// Stores the minimum balance (a.k.a. existential deposit) into the supplied buffer. - /// The data is encoded as `T::Balance`. - /// - /// If the available space in `output` is less than the size of the value a trap is triggered. /// /// # Parameters /// /// - `output`: A reference to the output data buffer to write the minimum balance. - fn minimum_balance(output: &mut &mut [u8]); + fn minimum_balance(output: &mut [u8; 32]); /// Retrieve the code hash of the currently executing contract. /// @@ -440,12 +431,10 @@ pub trait HostFn: private::Sealed { /// Load the latest block timestamp into the supplied buffer /// - /// If the available space in `output` is less than the size of the value a trap is triggered. - /// /// # Parameters /// /// - `output`: A reference to the output data buffer to write the timestamp. - fn now(output: &mut &mut [u8]); + fn now(output: &mut [u8; 32]); /// Removes the delegate dependency from the contract. /// @@ -548,12 +537,12 @@ pub trait HostFn: private::Sealed { /// # Parameters /// /// - `address`: The address of the account to transfer funds to. - /// - `value`: The value to transfer. Should be decodable as a `T::Balance`. Traps otherwise. + /// - `value`: The U256 value to transfer. /// /// # Errors /// /// - [TransferFailed][`crate::ReturnErrorCode::TransferFailed] - fn transfer(address: &[u8; 20], value: &[u8]) -> Result; + fn transfer(address: &[u8; 20], value: &[u8; 32]) -> Result; /// Remove the calling account and transfer remaining **free** balance. /// @@ -573,26 +562,20 @@ pub trait HostFn: private::Sealed { fn terminate(beneficiary: &[u8; 20]) -> !; /// Stores the value transferred along with this call/instantiate into the supplied buffer. - /// The data is encoded as `T::Balance`. - /// - /// If the available space in `output` is less than the size of the value a trap is triggered. /// /// # Parameters /// /// - `output`: A reference to the output data buffer to write the transferred value. - fn value_transferred(output: &mut &mut [u8]); + fn value_transferred(output: &mut [u8; 32]); /// Stores the price for the specified amount of gas into the supplied buffer. - /// The data is encoded as `T::Balance`. - /// - /// If the available space in `output` is less than the size of the value a trap is triggered. /// /// # Parameters /// /// - `ref_time_limit`: The *ref_time* Weight limit to query the price for. /// - `proof_size_limit`: The *proof_size* Weight limit to query the price for. /// - `output`: A reference to the output data buffer to write the price. - fn weight_to_fee(ref_time_limit: u64, proof_size_limit: u64, output: &mut &mut [u8]); + fn weight_to_fee(ref_time_limit: u64, proof_size_limit: u64, output: &mut [u8; 32]); /// Execute an XCM program locally, using the contract's address as the origin. /// This is equivalent to dispatching `pallet_xcm::execute` through call_runtime, except that diff --git a/substrate/frame/revive/uapi/src/host/riscv32.rs b/substrate/frame/revive/uapi/src/host/riscv32.rs index c8218bb8f737..592b142ca00f 100644 --- a/substrate/frame/revive/uapi/src/host/riscv32.rs +++ b/substrate/frame/revive/uapi/src/host/riscv32.rs @@ -79,12 +79,7 @@ mod sys { pub fn caller_is_origin() -> ReturnCode; pub fn caller_is_root() -> ReturnCode; pub fn address(out_ptr: *mut u8); - pub fn weight_to_fee( - ref_time: u64, - proof_size: u64, - out_ptr: *mut u8, - out_len_ptr: *mut u32, - ); + pub fn weight_to_fee(ref_time: u64, proof_size: u64, out_ptr: *mut u8); pub fn weight_left(out_ptr: *mut u8, out_len_ptr: *mut u32); pub fn balance(out_ptr: *mut u8, out_len_ptr: *mut u32); pub fn value_transferred(out_ptr: *mut u8, out_len_ptr: *mut u32); @@ -136,21 +131,19 @@ mod sys { } } +/// A macro to implement all Host functions with a signature of `fn(&mut [u8; n])`. macro_rules! impl_wrapper_for { - ( $( $name:ident, )* ) => { - $( - fn $name(output: &mut &mut [u8]) { - let mut output_len = output.len() as u32; - unsafe { - sys::$name( - output.as_mut_ptr(), - &mut output_len, - ) - } - extract_from_slice(output, output_len as usize) - } - )* - } + (@impl_fn $name:ident, $n: literal) => { + fn $name(_output: &mut [u8; $n]) { + } + }; + + () => {}; + + ([u8; $n: literal] => $($name:ident),*; $($tail:tt)*) => { + $(impl_wrapper_for!(@impl_fn $name, $n);)* + impl_wrapper_for!($($tail)*); + }; } macro_rules! impl_hash_fn { @@ -185,7 +178,7 @@ fn ptr_len_or_sentinel(data: &mut Option<&mut &mut [u8]>) -> (*mut u8, u32) { } #[inline(always)] -fn ptr_or_sentinel(data: &Option<&[u8]>) -> *const u8 { +fn ptr_or_sentinel(data: &Option<&[u8; 32]>) -> *const u8 { match data { Some(ref data) => data.as_ptr(), None => crate::SENTINEL as _, @@ -197,8 +190,8 @@ impl HostFn for HostFnImpl { code_hash: &[u8; 32], ref_time_limit: u64, proof_size_limit: u64, - deposit_limit: Option<&[u8]>, - value: &[u8], + deposit_limit: Option<&[u8; 32]>, + value: &[u8; 32], input: &[u8], mut address: Option<&mut [u8; 20]>, mut output: Option<&mut &mut [u8]>, @@ -253,8 +246,8 @@ impl HostFn for HostFnImpl { callee: &[u8; 20], ref_time_limit: u64, proof_size_limit: u64, - deposit_limit: Option<&[u8]>, - value: &[u8], + deposit_limit: Option<&[u8; 32]>, + value: &[u8; 32], input: &[u8], mut output: Option<&mut &mut [u8]>, ) -> Result { @@ -327,7 +320,7 @@ impl HostFn for HostFnImpl { ret_code.into() } - fn transfer(address: &[u8; 20], value: &[u8]) -> Result { + fn transfer(address: &[u8; 20], value: &[u8; 32]) -> Result { let ret_code = unsafe { sys::transfer(address.as_ptr(), value.as_ptr()) }; ret_code.into() } @@ -449,33 +442,19 @@ impl HostFn for HostFnImpl { ret_code.into() } - fn address(output: &mut [u8; 20]) { - unsafe { sys::address(output.as_mut_ptr()) } - } - - fn caller(output: &mut [u8; 20]) { - unsafe { sys::caller(output.as_mut_ptr()) } - } - impl_wrapper_for! { - block_number, balance, - value_transferred,now, minimum_balance, - weight_left, + [u8; 32] => block_number, balance, value_transferred, now, minimum_balance; + [u8; 20] => address, caller; } - fn weight_to_fee(ref_time_limit: u64, proof_size_limit: u64, output: &mut &mut [u8]) { + fn weight_left(output: &mut &mut [u8]) { let mut output_len = output.len() as u32; - { - unsafe { - sys::weight_to_fee( - ref_time_limit, - proof_size_limit, - output.as_mut_ptr(), - &mut output_len, - ) - }; - } - extract_from_slice(output, output_len as usize); + unsafe { sys::weight_left(output.as_mut_ptr(), &mut output_len) } + extract_from_slice(output, output_len as usize) + } + + fn weight_to_fee(ref_time_limit: u64, proof_size_limit: u64, output: &mut [u8; 32]) { + unsafe { sys::weight_to_fee(ref_time_limit, proof_size_limit, output.as_mut_ptr()) }; } impl_hash_fn!(sha2_256, 32); From d4eee2de6e8450ce37ae58587fa08567493fb45a Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 17:13:23 +0200 Subject: [PATCH 02/21] tests pass --- .../revive/fixtures/contracts/balance.rs | 1 - .../revive/src/benchmarking/call_builder.rs | 9 ++- .../frame/revive/src/benchmarking/mod.rs | 67 +++++++------------ substrate/frame/revive/src/exec.rs | 59 +++++++++------- substrate/frame/revive/src/lib.rs | 57 ++++++++++------ .../frame/revive/src/test_utils/builder.rs | 3 + substrate/frame/revive/src/tests.rs | 1 + substrate/frame/revive/src/wasm/mod.rs | 20 ++++-- substrate/frame/revive/src/wasm/runtime.rs | 15 ++--- .../frame/revive/uapi/src/host/riscv32.rs | 13 ++-- 10 files changed, 137 insertions(+), 108 deletions(-) diff --git a/substrate/frame/revive/fixtures/contracts/balance.rs b/substrate/frame/revive/fixtures/contracts/balance.rs index 2db38a3cd5ca..4606135d9807 100644 --- a/substrate/frame/revive/fixtures/contracts/balance.rs +++ b/substrate/frame/revive/fixtures/contracts/balance.rs @@ -28,7 +28,6 @@ pub extern "C" fn deploy() {} #[no_mangle] #[polkavm_derive::polkavm_export] pub extern "C" fn call() { - // Initialize buffer with 1s so that we can check that it is overwritten. let balance = u64_output!(api::balance,); assert_eq!(balance, 0); } diff --git a/substrate/frame/revive/src/benchmarking/call_builder.rs b/substrate/frame/revive/src/benchmarking/call_builder.rs index c000817a8a39..914110041da0 100644 --- a/substrate/frame/revive/src/benchmarking/call_builder.rs +++ b/substrate/frame/revive/src/benchmarking/call_builder.rs @@ -22,12 +22,13 @@ use crate::{ storage::meter::Meter, transient_storage::MeterEntry, wasm::{ApiVersion, PreparedCall, Runtime}, - BalanceOf, Config, DebugBuffer, Error, GasMeter, Origin, TypeInfo, WasmBlob, Weight, + BalanceOf, Config, DebugBuffer, Error, GasMeter, MomentOf, Origin, TypeInfo, WasmBlob, Weight, }; use alloc::{vec, vec::Vec}; use codec::{Encode, HasCompact}; use core::fmt::Debug; use frame_benchmarking::benchmarking; +use sp_core::U256; type StackExt<'a, T> = Stack<'a, T, WasmBlob>; @@ -48,6 +49,9 @@ impl Default for CallSetup where T: Config + pallet_balances::Config, as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, + BalanceOf: Into, + BalanceOf: TryFrom, + MomentOf: Into, { fn default() -> Self { Self::new(WasmModule::dummy()) @@ -58,6 +62,9 @@ impl CallSetup where T: Config + pallet_balances::Config, as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, + BalanceOf: Into, + BalanceOf: TryFrom, + MomentOf: Into, { /// Setup a new call for the given module. pub fn new(module: WasmModule) -> Self { diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index 2c5285622843..e2f4088070d4 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -72,6 +72,9 @@ impl Contract where T: Config + pallet_balances::Config, as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, + BalanceOf: Into, + BalanceOf: TryFrom, + MomentOf: Into, { /// Create new contract and use a default account id as instantiator. fn new(module: WasmModule, data: Vec) -> Result, &'static str> { @@ -219,6 +222,10 @@ fn default_deposit_limit() -> BalanceOf { as codec::HasCompact>::Type: Clone + Eq + PartialEq + core::fmt::Debug + scale_info::TypeInfo + codec::Encode, T: Config + pallet_balances::Config, BalanceOf: From< as Currency>::Balance>, + as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, + BalanceOf: Into, + BalanceOf: TryFrom, + MomentOf: Into, as Currency>::Balance: From>, )] mod benchmarks { @@ -661,85 +668,67 @@ mod benchmarks { #[benchmark(pov_mode = Measured)] fn seal_balance() { - let len = ::max_encoded_len() as u32; - build_runtime!(runtime, memory: [len.to_le_bytes(), vec![0u8; len as _], ]); + build_runtime!(runtime, memory: [[0u8;32], ]); let result; #[block] { - result = runtime.bench_balance(memory.as_mut_slice(), 4, 0); + result = runtime.bench_balance(memory.as_mut_slice(), 0); } assert_ok!(result); - assert_eq!( - ::decode(&mut &memory[4..]).unwrap(), - runtime.ext().balance().into() - ); + assert_eq!(U256::from_little_endian(&memory[..]), runtime.ext().balance()); } #[benchmark(pov_mode = Measured)] fn seal_value_transferred() { - let len = ::max_encoded_len() as u32; - build_runtime!(runtime, memory: [len.to_le_bytes(), vec![0u8; len as _], ]); + build_runtime!(runtime, memory: [[0u8;32], ]); let result; #[block] { - result = runtime.bench_value_transferred(memory.as_mut_slice(), 4, 0); + result = runtime.bench_value_transferred(memory.as_mut_slice(), 0); } assert_ok!(result); - assert_eq!( - ::decode(&mut &memory[4..]).unwrap(), - runtime.ext().value_transferred().into() - ); + assert_eq!(U256::from_little_endian(&memory[..]), runtime.ext().value_transferred()); } #[benchmark(pov_mode = Measured)] fn seal_minimum_balance() { - let len = ::max_encoded_len() as u32; - build_runtime!(runtime, memory: [len.to_le_bytes(), vec![0u8; len as _], ]); + build_runtime!(runtime, memory: [[0u8;32], ]); let result; #[block] { - result = runtime.bench_minimum_balance(memory.as_mut_slice(), 4, 0); + result = runtime.bench_minimum_balance(memory.as_mut_slice(), 0); } assert_ok!(result); - assert_eq!( - ::decode(&mut &memory[4..]).unwrap(), - runtime.ext().minimum_balance().into() - ); + assert_eq!(U256::from_little_endian(&memory[..]), runtime.ext().minimum_balance()); } #[benchmark(pov_mode = Measured)] fn seal_block_number() { - let len = as MaxEncodedLen>::max_encoded_len() as u32; - build_runtime!(runtime, memory: [len.to_le_bytes(), vec![0u8; len as _], ]); + build_runtime!(runtime, memory: [[0u8;32], ]); let result; #[block] { - result = runtime.bench_block_number(memory.as_mut_slice(), 4, 0); + result = runtime.bench_block_number(memory.as_mut_slice(), 0); } assert_ok!(result); - assert_eq!( - >::decode(&mut &memory[4..]).unwrap(), - runtime.ext().block_number() - ); + assert_eq!(U256::from_little_endian(&memory[..]), runtime.ext().block_number()); } #[benchmark(pov_mode = Measured)] fn seal_now() { - let len = as MaxEncodedLen>::max_encoded_len() as u32; - build_runtime!(runtime, memory: [len.to_le_bytes(), vec![0u8; len as _], ]); + build_runtime!(runtime, memory: [[0u8;32], ]); let result; #[block] { - result = runtime.bench_now(memory.as_mut_slice(), 4, 0); + result = runtime.bench_now(memory.as_mut_slice(), 0); } assert_ok!(result); - assert_eq!(>::decode(&mut &memory[4..]).unwrap(), *runtime.ext().now()); + assert_eq!(U256::from_little_endian(&memory[..]), runtime.ext().now()); } #[benchmark(pov_mode = Measured)] fn seal_weight_to_fee() { - let len = ::max_encoded_len() as u32; - build_runtime!(runtime, memory: [len.to_le_bytes(), vec![0u8; len as _], ]); + build_runtime!(runtime, memory: [[0u8;32], ]); let weight = Weight::from_parts(500_000, 300_000); let result; #[block] @@ -748,15 +737,11 @@ mod benchmarks { memory.as_mut_slice(), weight.ref_time(), weight.proof_size(), - 4, 0, ); } assert_ok!(result); - assert_eq!( - >::decode(&mut &memory[4..]).unwrap(), - runtime.ext().get_weight_price(weight) - ); + assert_eq!(U256::from_little_endian(&memory[..]), runtime.ext().get_weight_price(weight)); } #[benchmark(pov_mode = Measured)] @@ -1536,11 +1521,11 @@ mod benchmarks { let hash_len = hash_bytes.len() as u32; let value: BalanceOf = 1u32.into(); - let value_bytes = value.encode(); + let value_bytes = Into::::into(value).encode(); let value_len = value_bytes.len() as u32; let deposit: BalanceOf = 0u32.into(); - let deposit_bytes = deposit.encode(); + let deposit_bytes = Into::::into(deposit).encode(); let deposit_len = deposit_bytes.len() as u32; let mut setup = CallSetup::::default(); diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index dd3e97b21e4f..d1c24affd54e 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -209,9 +209,9 @@ pub trait Ext: sealing::Sealed { fn instantiate( &mut self, gas_limit: Weight, - deposit_limit: BalanceOf, + deposit_limit: U256, code: H256, - value: BalanceOf, + value: U256, input_data: Vec, salt: Option<&[u8; 32]>, ) -> Result<(H160, ExecReturnValue), ExecError>; @@ -697,6 +697,9 @@ impl CachedContract { impl<'a, T, E> Stack<'a, T, E> where T: Config, + BalanceOf: Into, + BalanceOf: TryFrom, + MomentOf: Into, E: Executable, { /// Create and run a new call stack by calling into `dest`. @@ -1240,6 +1243,9 @@ impl<'a, T, E> Ext for Stack<'a, T, E> where T: Config, E: Executable, + BalanceOf: Into, + BalanceOf: TryFrom, + MomentOf: Into, { type T = T; @@ -1322,9 +1328,9 @@ where fn instantiate( &mut self, gas_limit: Weight, - deposit_limit: BalanceOf, + deposit_limit: U256, code_hash: H256, - value: BalanceOf, + value: U256, input_data: Vec, salt: Option<&[u8; 32]>, ) -> Result<(H160, ExecReturnValue), ExecError> { @@ -1337,9 +1343,9 @@ where salt, input_data: input_data.as_ref(), }, - value, + value.try_into().map_err(|_| Error::::ConversionFailed)?, gas_limit, - deposit_limit, + deposit_limit.try_into().map_err(|_| Error::::ConversionFailed)?, self.is_read_only(), )?; let address = T::AddressMapper::to_address(&self.top_frame().account_id); @@ -1462,24 +1468,25 @@ where self.caller_is_origin() && self.origin == Origin::Root } - fn balance(&self) -> BalanceOf { + fn balance(&self) -> U256 { T::Currency::reducible_balance( &self.top_frame().account_id, Preservation::Preserve, Fortitude::Polite, ) + .into() } - fn value_transferred(&self) -> BalanceOf { - self.top_frame().value_transferred + fn value_transferred(&self) -> U256 { + self.top_frame().value_transferred.into() } - fn now(&self) -> &MomentOf { - &self.timestamp + fn now(&self) -> U256 { + self.timestamp.clone().into() } - fn minimum_balance(&self) -> BalanceOf { - T::Currency::minimum_balance() + fn minimum_balance(&self) -> U256 { + T::Currency::minimum_balance().into() } fn deposit_event(&mut self, topics: Vec, data: Vec) { @@ -1492,16 +1499,16 @@ where ); } - fn block_number(&self) -> BlockNumberFor { - self.block_number + fn block_number(&self) -> U256 { + self.block_number.into() } fn max_value_size(&self) -> u32 { limits::PAYLOAD_BYTES } - fn get_weight_price(&self, weight: Weight) -> BalanceOf { - T::WeightPrice::convert(weight) + fn get_weight_price(&self, weight: Weight) -> U256 { + T::WeightPrice::convert(weight).into() } fn gas_meter(&self) -> &GasMeter { @@ -1864,7 +1871,7 @@ mod tests { let value = 55; let success_ch = MockLoader::insert(Call, move |ctx, _| { - assert_eq!(ctx.ext.value_transferred(), value); + assert_eq!(ctx.ext.value_transferred(), U256::from(value)); Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }) }); @@ -1896,12 +1903,12 @@ mod tests { let value = 35; let success_ch = MockLoader::insert(Call, move |ctx, _| { - assert_eq!(ctx.ext.value_transferred(), value); + assert_eq!(ctx.ext.value_transferred(), U256::from(value)); Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }) }); let delegate_ch = MockLoader::insert(Call, move |ctx, _| { - assert_eq!(ctx.ext.value_transferred(), value); + assert_eq!(ctx.ext.value_transferred(), U256::from(value)); let _ = ctx.ext.delegate_call(success_ch, Vec::new())?; Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }) }); @@ -2621,9 +2628,9 @@ mod tests { .ext .instantiate( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), dummy_ch, - ::Currency::minimum_balance(), + ::Currency::minimum_balance().into(), vec![], Some(&[48; 32]), ) @@ -2699,9 +2706,9 @@ mod tests { assert_matches!( ctx.ext.instantiate( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), dummy_ch, - ::Currency::minimum_balance(), + ::Currency::minimum_balance().into(), vec![], Some(&[0; 32]), ), @@ -3251,7 +3258,7 @@ mod tests { ctx.ext .instantiate( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), fail_code, ctx.ext.minimum_balance() * 100, vec![], @@ -3268,7 +3275,7 @@ mod tests { .ext .instantiate( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), success_code, ctx.ext.minimum_balance() * 100, vec![], diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs index 393acc8c9852..267b66cb7145 100644 --- a/substrate/frame/revive/src/lib.rs +++ b/substrate/frame/revive/src/lib.rs @@ -27,7 +27,9 @@ mod benchmarking_dummy; mod exec; mod gas; mod primitives; +use crate::exec::MomentOf; pub use primitives::*; +use sp_core::U256; mod limits; mod storage; @@ -129,6 +131,7 @@ pub mod pallet { use crate::debug::Debugger; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; + use sp_core::U256; use sp_runtime::Perbill; /// The in-code storage version. @@ -569,6 +572,8 @@ pub mod pallet { InvalidStorageFlags, /// PolkaVM failed during code execution. Probably due to a malformed program. ExecutionFailed, + /// Failed to convert a U256 to a Balance. + ConversionFailed, } /// A reason for the pallet contracts placing a hold on funds. @@ -772,6 +777,9 @@ pub mod pallet { impl Pallet where as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, + BalanceOf: Into, + BalanceOf: TryFrom, + MomentOf: Into, { /// Makes a call to an account, optionally transferring some balance. /// @@ -1053,7 +1061,13 @@ fn dispatch_result( .map_err(|e| DispatchErrorWithPostInfo { post_info, error: e }) } -impl Pallet { +impl Pallet +where + T: Config, + BalanceOf: Into, + BalanceOf: TryFrom, + MomentOf: Into, +{ /// A generalized version of [`Self::call`]. /// /// Identical to [`Self::call`] but tailored towards being called by other code within the @@ -1226,24 +1240,6 @@ impl Pallet { Ok((module, deposit)) } - /// Deposit a pallet contracts event. - fn deposit_event(event: Event) { - >::deposit_event(::RuntimeEvent::from(event)) - } - - /// Deposit a pallet contracts indexed event. - fn deposit_indexed_event(topics: Vec, event: Event) { - >::deposit_event_indexed( - &topics, - ::RuntimeEvent::from(event).into(), - ) - } - - /// Return the existential deposit of [`Config::Currency`]. - fn min_balance() -> BalanceOf { - >>::minimum_balance() - } - /// Run the supplied function `f` if no other instance of this pallet is on the stack. fn run_guarded Result>(f: F) -> Result { executing_contract::using_once(&mut false, || { @@ -1264,6 +1260,29 @@ impl Pallet { } } +impl Pallet +where + T: Config, +{ + /// Return the existential deposit of [`Config::Currency`]. + fn min_balance() -> BalanceOf { + >>::minimum_balance() + } + + /// Deposit a pallet contracts event. + fn deposit_event(event: Event) { + >::deposit_event(::RuntimeEvent::from(event)) + } + + /// Deposit a pallet contracts indexed event. + fn deposit_indexed_event(topics: Vec, event: Event) { + >::deposit_event_indexed( + &topics, + ::RuntimeEvent::from(event).into(), + ) + } +} + // Set up a global reference to the boolean flag used for the re-entrancy guard. environmental!(executing_contract: bool); diff --git a/substrate/frame/revive/src/test_utils/builder.rs b/substrate/frame/revive/src/test_utils/builder.rs index b17067769c05..1e09e2837f2c 100644 --- a/substrate/frame/revive/src/test_utils/builder.rs +++ b/substrate/frame/revive/src/test_utils/builder.rs @@ -54,6 +54,9 @@ macro_rules! builder { impl $name where as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, + BalanceOf: Into, + BalanceOf: TryFrom, + crate::MomentOf: Into, { $( #[doc = concat!("Set the ", stringify!($field))] diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index 717d020a5813..d0b231c6fff5 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -593,6 +593,7 @@ impl Default for Origin { mod run_tests { use super::*; use pretty_assertions::{assert_eq, assert_ne}; + use sp_core::U256; // Perform a call to a plain account. // The actual transfer fails because we can only call contracts. diff --git a/substrate/frame/revive/src/wasm/mod.rs b/substrate/frame/revive/src/wasm/mod.rs index 9024390fd24f..d6b5eb56b738 100644 --- a/substrate/frame/revive/src/wasm/mod.rs +++ b/substrate/frame/revive/src/wasm/mod.rs @@ -47,7 +47,7 @@ use frame_support::{ ensure, traits::{fungible::MutateHold, tokens::Precision::BestEffort}, }; -use sp_core::Get; +use sp_core::{Get, U256}; use sp_runtime::DispatchError; /// Validated Wasm module ready for execution. @@ -123,7 +123,11 @@ impl Token for CodeLoadToken { } } -impl WasmBlob { +impl WasmBlob +where + BalanceOf: Into, + BalanceOf: TryFrom, +{ /// We only check for size and nothing else when the code is uploaded. pub fn from_code( code: Vec, @@ -251,7 +255,11 @@ pub struct PreparedCall<'a, E: Ext> { api_version: ApiVersion, } -impl<'a, E: Ext> PreparedCall<'a, E> { +impl<'a, E: Ext> PreparedCall<'a, E> +where + BalanceOf: Into, + BalanceOf: TryFrom, +{ pub fn call(mut self) -> ExecResult { let exec_result = loop { let interrupt = self.instance.run(); @@ -315,7 +323,11 @@ impl WasmBlob { } } -impl Executable for WasmBlob { +impl Executable for WasmBlob +where + BalanceOf: Into, + BalanceOf: TryFrom, +{ fn from_storage( code_hash: sp_core::H256, gas_meter: &mut GasMeter, diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index 241a01c61241..dfdba0218b2c 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -35,7 +35,7 @@ use frame_support::{ }; use pallet_revive_proc_macro::define_env; use pallet_revive_uapi::{CallFlags, ReturnErrorCode, ReturnFlags, StorageFlags}; -use sp_core::{H160, H256}; +use sp_core::{H160, H256, U256}; use sp_io::hashing::{blake2_128, blake2_256, keccak_256, sha2_256}; use sp_runtime::{traits::Zero, DispatchError, RuntimeDebug}; @@ -1036,12 +1036,9 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { salt_ptr: u32, ) -> Result { self.charge_gas(RuntimeCosts::Instantiate { input_data_len })?; - let deposit_limit: BalanceOf<::T> = if deposit_ptr == SENTINEL { - BalanceOf::<::T>::zero() - } else { - memory.read_as(deposit_ptr)? - }; - let value: BalanceOf<::T> = memory.read_as(value_ptr)?; + let deposit_limit: U256 = + if deposit_ptr == SENTINEL { U256::zero() } else { memory.read_as(deposit_ptr)? }; + let value: U256 = memory.read_as(value_ptr)?; let code_hash: H256 = memory.read_as(code_hash_ptr)?; let input_data = memory.read(input_data_ptr, input_data_len)?; let salt = if salt_ptr == SENTINEL { @@ -1439,14 +1436,12 @@ pub mod env { ref_time_limit: u64, proof_size_limit: u64, out_ptr: u32, - out_len_ptr: u32, ) -> Result<(), TrapReason> { let weight = Weight::from_parts(ref_time_limit, proof_size_limit); self.charge_gas(RuntimeCosts::WeightToFee)?; - Ok(self.write_sandbox_output( + Ok(self.write_fixed_sandbox_output( memory, out_ptr, - out_len_ptr, &self.ext.get_weight_price(weight).encode(), false, already_charged, diff --git a/substrate/frame/revive/uapi/src/host/riscv32.rs b/substrate/frame/revive/uapi/src/host/riscv32.rs index 592b142ca00f..6fafdcb677a6 100644 --- a/substrate/frame/revive/uapi/src/host/riscv32.rs +++ b/substrate/frame/revive/uapi/src/host/riscv32.rs @@ -81,17 +81,17 @@ mod sys { pub fn address(out_ptr: *mut u8); pub fn weight_to_fee(ref_time: u64, proof_size: u64, out_ptr: *mut u8); pub fn weight_left(out_ptr: *mut u8, out_len_ptr: *mut u32); - pub fn balance(out_ptr: *mut u8, out_len_ptr: *mut u32); - pub fn value_transferred(out_ptr: *mut u8, out_len_ptr: *mut u32); - pub fn now(out_ptr: *mut u8, out_len_ptr: *mut u32); - pub fn minimum_balance(out_ptr: *mut u8, out_len_ptr: *mut u32); + pub fn balance(out_ptr: *mut u8); + pub fn value_transferred(out_ptr: *mut u8); + pub fn now(out_ptr: *mut u8); + pub fn minimum_balance(out_ptr: *mut u8); pub fn deposit_event( topics_ptr: *const u8, topics_len: u32, data_ptr: *const u8, data_len: u32, ); - pub fn block_number(out_ptr: *mut u8, out_len_ptr: *mut u32); + pub fn block_number(out_ptr: *mut u8); pub fn hash_sha2_256(input_ptr: *const u8, input_len: u32, out_ptr: *mut u8); pub fn hash_keccak_256(input_ptr: *const u8, input_len: u32, out_ptr: *mut u8); pub fn hash_blake2_256(input_ptr: *const u8, input_len: u32, out_ptr: *mut u8); @@ -134,7 +134,8 @@ mod sys { /// A macro to implement all Host functions with a signature of `fn(&mut [u8; n])`. macro_rules! impl_wrapper_for { (@impl_fn $name:ident, $n: literal) => { - fn $name(_output: &mut [u8; $n]) { + fn $name(output: &mut [u8; $n]) { + unsafe { sys::$name(output.as_mut_ptr()) } } }; From 1c0d4475ac16540d10e5654637a1db118e020404 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 17:16:03 +0200 Subject: [PATCH 03/21] fix comments --- substrate/frame/revive/uapi/src/host.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/substrate/frame/revive/uapi/src/host.rs b/substrate/frame/revive/uapi/src/host.rs index fc59e6dc423f..281e90e64bcb 100644 --- a/substrate/frame/revive/uapi/src/host.rs +++ b/substrate/frame/revive/uapi/src/host.rs @@ -79,10 +79,10 @@ pub trait HostFn: private::Sealed { /// otherwise. /// - `ref_time_limit`: how much *ref_time* Weight to devote to the execution. /// - `proof_size_limit`: how much *proof_size* Weight to devote to the execution. - /// - `deposit`: The storage deposit limit for instantiation. Should be decodable as a - /// `Option`. Traps otherwise. Passing `None` means setting no specific limit for - /// the call, which implies storage usage up to the limit of the parent call. - /// - `value`: The U256 value to transfer into the contract. + /// - `deposit`: The storage deposit limit for instantiation. Passing `None` means setting no + /// specific limit for the call, which implies storage usage up to the limit of the parent + /// call. + /// - `value`: The value to transfer into the contract. /// - `input`: The input data buffer used to call the contract. /// - `output`: A reference to the output data buffer to write the call output buffer. If `None` /// is provided then the output buffer is not copied. @@ -369,10 +369,10 @@ pub trait HostFn: private::Sealed { /// - `code_hash`: The hash of the code to be instantiated. /// - `ref_time_limit`: how much *ref_time* Weight to devote to the execution. /// - `proof_size_limit`: how much *proof_size* Weight to devote to the execution. - /// - `deposit`: The storage deposit limit for instantiation. Should be decodable as a - /// `Option`. Traps otherwise. Passing `None` means setting no specific limit for - /// the call, which implies storage usage up to the limit of the parent call. - /// - `value`: The U256 value to transfer into the contract. + /// - `deposit`: The storage deposit limit for instantiation. Passing `None` means setting no + /// specific limit for the call, which implies storage usage up to the limit of the parent + /// call. + /// - `value`: The value to transfer into the contract. /// - `input`: The input data buffer. /// - `address`: A reference to the address buffer to write the address of the contract. If /// `None` is provided then the output buffer is not copied. From cc57520247f49c4248648ec809440f8cd830bf72 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 17:25:44 +0200 Subject: [PATCH 04/21] nit --- substrate/frame/revive/fixtures/build.rs | 1 + substrate/frame/revive/fixtures/contracts/common/src/lib.rs | 2 +- substrate/frame/revive/src/exec.rs | 4 ++-- substrate/frame/revive/src/lib.rs | 2 +- substrate/frame/revive/uapi/src/host.rs | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/substrate/frame/revive/fixtures/build.rs b/substrate/frame/revive/fixtures/build.rs index 04ac6abd6718..944ae246c1b8 100644 --- a/substrate/frame/revive/fixtures/build.rs +++ b/substrate/frame/revive/fixtures/build.rs @@ -95,6 +95,7 @@ mod build { }; set_dep("uapi", "../uapi")?; set_dep("common", "./contracts/common")?; + cargo_toml["bin"] = toml::Value::Array( entries .map(|entry| { diff --git a/substrate/frame/revive/fixtures/contracts/common/src/lib.rs b/substrate/frame/revive/fixtures/contracts/common/src/lib.rs index 02c7d62c58fd..abfba282bec1 100644 --- a/substrate/frame/revive/fixtures/contracts/common/src/lib.rs +++ b/substrate/frame/revive/fixtures/contracts/common/src/lib.rs @@ -179,7 +179,7 @@ macro_rules! u64_output { }}; } -/// Convert a u* into a [u8; 32]. +/// Convert a u64 into a [u8; 32]. pub const fn u256_bytes(value: u64) -> [u8; 32] { let mut buffer = [0u8; 32]; let bytes = value.to_le_bytes(); diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index d1c24affd54e..cfca502b162c 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -1343,9 +1343,9 @@ where salt, input_data: input_data.as_ref(), }, - value.try_into().map_err(|_| Error::::ConversionFailed)?, + value.try_into().map_err(|_| Error::::BalanceConversionFailed)?, gas_limit, - deposit_limit.try_into().map_err(|_| Error::::ConversionFailed)?, + deposit_limit.try_into().map_err(|_| Error::::BalanceConversionFailed)?, self.is_read_only(), )?; let address = T::AddressMapper::to_address(&self.top_frame().account_id); diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs index 267b66cb7145..783da878c0f8 100644 --- a/substrate/frame/revive/src/lib.rs +++ b/substrate/frame/revive/src/lib.rs @@ -573,7 +573,7 @@ pub mod pallet { /// PolkaVM failed during code execution. Probably due to a malformed program. ExecutionFailed, /// Failed to convert a U256 to a Balance. - ConversionFailed, + BalanceConversionFailed, } /// A reason for the pallet contracts placing a hold on funds. diff --git a/substrate/frame/revive/uapi/src/host.rs b/substrate/frame/revive/uapi/src/host.rs index 281e90e64bcb..d27586269aa4 100644 --- a/substrate/frame/revive/uapi/src/host.rs +++ b/substrate/frame/revive/uapi/src/host.rs @@ -537,7 +537,7 @@ pub trait HostFn: private::Sealed { /// # Parameters /// /// - `address`: The address of the account to transfer funds to. - /// - `value`: The U256 value to transfer. + /// - `value`: The value to transfer. /// /// # Errors /// From 659515f32564506db226e162ef1a750ee70f3a15 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 17:30:47 +0200 Subject: [PATCH 05/21] Add prdoc --- prdoc/pr_5608.prdoc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 prdoc/pr_5608.prdoc diff --git a/prdoc/pr_5608.prdoc b/prdoc/pr_5608.prdoc new file mode 100644 index 000000000000..865b21b44ca4 --- /dev/null +++ b/prdoc/pr_5608.prdoc @@ -0,0 +1,16 @@ +title: "[pallet-revive] update runtime types" + +doc: + - audience: Runtime Dev + description: | + Refactor the Ext trait to use U256 instead of BalanceOf or MomentOf + +crates: + - name: pallet-revive + bump: patch + - name: pallet-revive-uapi + bump: patch + - name: pallet-revive-fixtures + bump: patch + + From 62178a80a3e7ec5aef19e8490d74b7f7235419e2 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 17:30:55 +0200 Subject: [PATCH 06/21] nit --- substrate/frame/revive/uapi/src/host.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/revive/uapi/src/host.rs b/substrate/frame/revive/uapi/src/host.rs index d27586269aa4..281e90e64bcb 100644 --- a/substrate/frame/revive/uapi/src/host.rs +++ b/substrate/frame/revive/uapi/src/host.rs @@ -537,7 +537,7 @@ pub trait HostFn: private::Sealed { /// # Parameters /// /// - `address`: The address of the account to transfer funds to. - /// - `value`: The value to transfer. + /// - `value`: The U256 value to transfer. /// /// # Errors /// From f4b6c1ae25e27240a710487ab8e736b8df250ece Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 17:41:48 +0200 Subject: [PATCH 07/21] Missing U256 update --- .../frame/revive/src/benchmarking/mod.rs | 4 +- substrate/frame/revive/src/exec.rs | 88 +++++++------------ substrate/frame/revive/src/wasm/runtime.rs | 8 +- 3 files changed, 38 insertions(+), 62 deletions(-) diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index e2f4088070d4..eab585a33fd9 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -1446,10 +1446,10 @@ mod benchmarks { let callee_len = callee_bytes.len() as u32; let value: BalanceOf = t.into(); - let value_bytes = value.encode(); + let value_bytes = Into::::into(value).encode(); let deposit: BalanceOf = (u32::MAX - 100).into(); - let deposit_bytes = deposit.encode(); + let deposit_bytes = Into::::into(deposit).encode(); let deposit_len = deposit_bytes.len() as u32; let mut setup = CallSetup::::default(); diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index cfca502b162c..fbc3e0be310f 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -184,9 +184,9 @@ pub trait Ext: sealing::Sealed { fn call( &mut self, gas_limit: Weight, - deposit_limit: BalanceOf, + deposit_limit: U256, to: &H160, - value: BalanceOf, + value: U256, input_data: Vec, allows_reentry: bool, read_only: bool, @@ -1252,9 +1252,9 @@ where fn call( &mut self, gas_limit: Weight, - deposit_limit: BalanceOf, + deposit_limit: U256, dest: &H160, - value: BalanceOf, + value: U256, input_data: Vec, allows_reentry: bool, read_only: bool, @@ -1283,9 +1283,9 @@ where }); let executable = self.push_frame( FrameArgs::Call { dest, cached_info, delegated_call: None }, - value, + value.try_into().map_err(|_| Error::::BalanceConversionFailed)?, gas_limit, - deposit_limit, + deposit_limit.try_into().map_err(|_| Error::::BalanceConversionFailed)?, // Enable read-only access if requested; cannot disable it if already set. read_only || self.is_read_only(), )?; @@ -2119,9 +2119,9 @@ mod tests { // Try to call into yourself. let r = ctx.ext.call( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), &BOB_ADDR, - 0, + U256::zero(), vec![], true, false, @@ -2182,9 +2182,9 @@ mod tests { assert_matches!( ctx.ext.call( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), &CHARLIE_ADDR, - 0, + U256::zero(), vec![], true, false @@ -2323,9 +2323,9 @@ mod tests { // BOB calls CHARLIE ctx.ext.call( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), &CHARLIE_ADDR, - 0, + U256::zero(), vec![], true, false, @@ -2417,9 +2417,9 @@ mod tests { // BOB calls CHARLIE. ctx.ext.call( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), &CHARLIE_ADDR, - 0, + U256::zero(), vec![], true, false, @@ -2455,9 +2455,9 @@ mod tests { assert_matches!( ctx.ext.call( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), &CHARLIE_ADDR, - 0, + U256::zero(), vec![], true, false @@ -2811,9 +2811,9 @@ mod tests { assert_eq!( ctx.ext.call( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), &CHARLIE_ADDR, - 0, + U256::zero(), vec![], true, false @@ -2827,15 +2827,7 @@ mod tests { let code_charlie = MockLoader::insert(Call, |ctx, _| { assert!(ctx .ext - .call( - Weight::zero(), - BalanceOf::::zero(), - &BOB_ADDR, - 0, - vec![99], - true, - false - ) + .call(Weight::zero(), U256::zero(), &BOB_ADDR, U256::zero(), vec![99], true, false) .is_ok()); exec_trapped() }); @@ -2867,7 +2859,7 @@ mod tests { let addr = ::AddressMapper::to_address(&account_id); assert_matches!( - ctx.ext.call(Weight::zero(), BalanceOf::::zero(), &addr, 0, vec![], + ctx.ext.call(Weight::zero(), U256::zero(), &addr, U256::zero(), vec![], true, false), Err(ExecError{error, ..}) if error == >::ContractNotFound.into() ); exec_success() @@ -3005,7 +2997,7 @@ mod tests { let code_bob = MockLoader::insert(Call, |ctx, _| { let dest = H160::from_slice(ctx.input_data.as_ref()); ctx.ext - .call(Weight::zero(), BalanceOf::::zero(), &dest, 0, vec![], false, false) + .call(Weight::zero(), U256::zero(), &dest, U256::zero(), vec![], false, false) }); let code_charlie = MockLoader::insert(Call, |_, _| exec_success()); @@ -3050,9 +3042,9 @@ mod tests { if ctx.input_data[0] == 0 { ctx.ext.call( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), &CHARLIE_ADDR, - 0, + U256::zero(), vec![], false, false, @@ -3066,9 +3058,9 @@ mod tests { let code_charlie = MockLoader::insert(Call, |ctx, _| { ctx.ext.call( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), &BOB_ADDR, - 0, + U256::zero(), vec![1], true, false, @@ -3291,7 +3283,7 @@ mod tests { // a plain call should not influence the account counter ctx.ext - .call(Weight::zero(), BalanceOf::::zero(), &addr, 0, vec![], false, false) + .call(Weight::zero(), U256::zero(), &addr, U256::zero(), vec![], false, false) .unwrap(); assert_eq!(System::account_nonce(ALICE), alice_nonce); @@ -3829,9 +3821,9 @@ mod tests { assert_eq!( ctx.ext.call( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), &CHARLIE_ADDR, - 0, + U256::zero(), vec![], true, false, @@ -3856,15 +3848,7 @@ mod tests { let code_charlie = MockLoader::insert(Call, |ctx, _| { assert!(ctx .ext - .call( - Weight::zero(), - BalanceOf::::zero(), - &BOB_ADDR, - 0, - vec![99], - true, - false - ) + .call(Weight::zero(), U256::zero(), &BOB_ADDR, U256::zero(), vec![99], true, false) .is_ok()); // CHARLIE can not read BOB`s storage. assert_eq!(ctx.ext.get_transient_storage(storage_key_1), None); @@ -3941,9 +3925,9 @@ mod tests { assert_eq!( ctx.ext.call( Weight::zero(), - BalanceOf::::zero(), + U256::zero(), &CHARLIE_ADDR, - 0, + U256::zero(), vec![], true, false @@ -3964,15 +3948,7 @@ mod tests { let code_charlie = MockLoader::insert(Call, |ctx, _| { assert!(ctx .ext - .call( - Weight::zero(), - BalanceOf::::zero(), - &BOB_ADDR, - 0, - vec![99], - true, - false - ) + .call(Weight::zero(), U256::zero(), &BOB_ADDR, U256::zero(), vec![99], true, false) .is_ok()); exec_trapped() }); diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index dfdba0218b2c..aa6c4f66e16e 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -37,7 +37,7 @@ use pallet_revive_proc_macro::define_env; use pallet_revive_uapi::{CallFlags, ReturnErrorCode, ReturnFlags, StorageFlags}; use sp_core::{H160, H256, U256}; use sp_io::hashing::{blake2_128, blake2_256, keccak_256, sha2_256}; -use sp_runtime::{traits::Zero, DispatchError, RuntimeDebug}; +use sp_runtime::{DispatchError, RuntimeDebug}; type CallOf = ::RuntimeCall; @@ -963,13 +963,13 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { CallType::Call { callee_ptr, value_ptr, deposit_ptr, weight } => { let mut callee = H160::zero(); memory.read_into_buf(callee_ptr, callee.as_bytes_mut())?; - let deposit_limit: BalanceOf<::T> = if deposit_ptr == SENTINEL { - BalanceOf::<::T>::zero() + let deposit_limit: U256 = if deposit_ptr == SENTINEL { + U256::zero() } else { memory.read_as(deposit_ptr)? }; let read_only = flags.contains(CallFlags::READ_ONLY); - let value: BalanceOf<::T> = memory.read_as(value_ptr)?; + let value: U256 = memory.read_as(value_ptr)?; if value > 0u32.into() { // If the call value is non-zero and state change is not allowed, issue an // error. From e1c13bfc3e91d6875eb71e168e22a012244eb953 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 17:46:30 +0200 Subject: [PATCH 08/21] missing --- substrate/frame/revive/src/benchmarking/mod.rs | 2 +- substrate/frame/revive/src/exec.rs | 6 +++--- substrate/frame/revive/src/wasm/runtime.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index eab585a33fd9..823e062f959b 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -1420,7 +1420,7 @@ mod benchmarks { let account_bytes = account.encode(); let account_len = account_bytes.len() as u32; - let value_bytes = value.encode(); + let value_bytes = Into::::into(value).encode(); let mut memory = memory!(account_bytes, value_bytes,); let result; diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index fbc3e0be310f..b04d06d5e769 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -226,7 +226,7 @@ pub trait Ext: sealing::Sealed { fn terminate(&mut self, beneficiary: &H160) -> DispatchResult; /// Transfer some amount of funds into the specified account. - fn transfer(&mut self, to: &H160, value: BalanceOf) -> DispatchResult; + fn transfer(&mut self, to: &H160, value: U256) -> DispatchResult; /// Returns the storage entry of the executing account by the given `key`. /// @@ -1380,12 +1380,12 @@ where Ok(()) } - fn transfer(&mut self, to: &H160, value: BalanceOf) -> DispatchResult { + fn transfer(&mut self, to: &H160, value: U256) -> DispatchResult { Self::transfer( Preservation::Preserve, &self.top_frame().account_id, &T::AddressMapper::to_account_id(to), - value, + value.try_into().map_err(|_| Error::::BalanceConversionFailed)?, ) } diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index aa6c4f66e16e..e2ddcca6947c 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -24,7 +24,7 @@ use crate::{ limits, primitives::ExecReturnValue, weights::WeightInfo, - BalanceOf, Config, Error, LOG_TARGET, SENTINEL, + Config, Error, LOG_TARGET, SENTINEL, }; use alloc::{boxed::Box, vec, vec::Vec}; use codec::{Decode, DecodeLimit, Encode, MaxEncodedLen}; @@ -1191,7 +1191,7 @@ pub mod env { self.charge_gas(RuntimeCosts::Transfer)?; let mut callee = H160::zero(); memory.read_into_buf(address_ptr, callee.as_bytes_mut())?; - let value: BalanceOf<::T> = memory.read_as(value_ptr)?; + let value: U256 = memory.read_as(value_ptr)?; let result = self.ext.transfer(&callee, value); match result { Ok(()) => Ok(ReturnErrorCode::Success), From a12320313485ef3da4b0531da7379e376bcc32d6 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 18:23:47 +0200 Subject: [PATCH 09/21] Force T::Hash -> H256 --- .../revive/src/benchmarking/call_builder.rs | 5 +++- .../frame/revive/src/benchmarking/mod.rs | 2 ++ substrate/frame/revive/src/exec.rs | 8 ++++--- substrate/frame/revive/src/lib.rs | 24 ++++++++++++------- substrate/frame/revive/src/storage.rs | 8 +++++-- substrate/frame/revive/src/storage/meter.rs | 10 +++++--- .../frame/revive/src/test_utils/builder.rs | 1 + substrate/frame/revive/src/wasm/mod.rs | 5 ++-- substrate/frame/revive/src/wasm/runtime.rs | 2 +- 9 files changed, 44 insertions(+), 21 deletions(-) diff --git a/substrate/frame/revive/src/benchmarking/call_builder.rs b/substrate/frame/revive/src/benchmarking/call_builder.rs index 914110041da0..7a423a633f69 100644 --- a/substrate/frame/revive/src/benchmarking/call_builder.rs +++ b/substrate/frame/revive/src/benchmarking/call_builder.rs @@ -28,7 +28,8 @@ use alloc::{vec, vec::Vec}; use codec::{Encode, HasCompact}; use core::fmt::Debug; use frame_benchmarking::benchmarking; -use sp_core::U256; +use frame_support::traits::IsType; +use sp_core::{H256, U256}; type StackExt<'a, T> = Stack<'a, T, WasmBlob>; @@ -51,6 +52,7 @@ where as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, BalanceOf: Into, BalanceOf: TryFrom, + T::Hash: IsType, MomentOf: Into, { fn default() -> Self { @@ -61,6 +63,7 @@ where impl CallSetup where T: Config + pallet_balances::Config, + T::Hash: IsType, as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, BalanceOf: Into, BalanceOf: TryFrom, diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index 823e062f959b..edf578a9e0ae 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -75,6 +75,7 @@ where BalanceOf: Into, BalanceOf: TryFrom, MomentOf: Into, + T::Hash: IsType, { /// Create new contract and use a default account id as instantiator. fn new(module: WasmModule, data: Vec) -> Result, &'static str> { @@ -226,6 +227,7 @@ fn default_deposit_limit() -> BalanceOf { BalanceOf: Into, BalanceOf: TryFrom, MomentOf: Into, + T::Hash: IsType, as Currency>::Balance: From>, )] mod benchmarks { diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index b04d06d5e769..630f8d5e3daa 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -37,7 +37,7 @@ use frame_support::{ traits::{ fungible::{Inspect, Mutate}, tokens::{Fortitude, Preservation}, - Contains, OriginTrait, Time, + Contains, IsType, OriginTrait, Time, }, weights::Weight, Blake2_128Concat, BoundedVec, StorageHasher, @@ -318,7 +318,7 @@ pub trait Ext: sealing::Sealed { /// Deposit an event with the given topics. /// /// There should not be any duplicates in `topics`. - fn deposit_event(&mut self, topics: Vec>, data: Vec); + fn deposit_event(&mut self, topics: Vec, data: Vec); /// Returns the current block number. fn block_number(&self) -> U256; @@ -697,6 +697,7 @@ impl CachedContract { impl<'a, T, E> Stack<'a, T, E> where T: Config, + T::Hash: IsType, BalanceOf: Into, BalanceOf: TryFrom, MomentOf: Into, @@ -1242,6 +1243,7 @@ where impl<'a, T, E> Ext for Stack<'a, T, E> where T: Config, + T::Hash: IsType, E: Executable, BalanceOf: Into, BalanceOf: TryFrom, @@ -1489,7 +1491,7 @@ where T::Currency::minimum_balance().into() } - fn deposit_event(&mut self, topics: Vec, data: Vec) { + fn deposit_event(&mut self, topics: Vec, data: Vec) { Contracts::::deposit_indexed_event( topics, Event::ContractEmitted { diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs index 783da878c0f8..6b74dc069988 100644 --- a/substrate/frame/revive/src/lib.rs +++ b/substrate/frame/revive/src/lib.rs @@ -28,6 +28,7 @@ mod exec; mod gas; mod primitives; use crate::exec::MomentOf; +use frame_support::traits::IsType; pub use primitives::*; use sp_core::U256; @@ -209,7 +210,7 @@ pub mod pallet { /// /// # Note /// - /// It is safe to chage this value on a live chain as all refunds are pro rata. + /// It is safe to change this value on a live chain as all refunds are pro rata. #[pallet::constant] #[pallet::no_default_bounds] type DepositPerByte: Get>; @@ -218,7 +219,7 @@ pub mod pallet { /// /// # Note /// - /// It is safe to chage this value on a live chain as all refunds are pro rata. + /// It is safe to change this value on a live chain as all refunds are pro rata. #[pallet::constant] #[pallet::no_default_bounds] type DepositPerItem: Get>; @@ -308,13 +309,13 @@ pub mod pallet { BlockNumberFor, >; - /// The amount of memory in bytes that parachain nodes alot to the runtime. + /// The amount of memory in bytes that parachain nodes a lot to the runtime. /// /// This is used in [`Pallet::integrity_test`] to make sure that the runtime has enough /// memory to support this pallet if set to the correct value. type RuntimeMemory: Get; - /// The amount of memory in bytes that relay chain validators alot to the PoV. + /// The amount of memory in bytes that relay chain validators a lot to the PoV. /// /// This is used in [`Pallet::integrity_test`] to make sure that the runtime has enough /// memory to support this pallet if set to the correct value. @@ -625,7 +626,10 @@ pub mod pallet { } #[pallet::hooks] - impl Hooks> for Pallet { + impl Hooks> for Pallet + where + T::Hash: IsType, + { fn on_idle(_block: BlockNumberFor, limit: Weight) -> Weight { use migration::MigrateResult::*; let mut meter = WeightMeter::with_limit(limit); @@ -776,6 +780,7 @@ pub mod pallet { #[pallet::call] impl Pallet where + T::Hash: IsType, as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, BalanceOf: Into, BalanceOf: TryFrom, @@ -1061,12 +1066,12 @@ fn dispatch_result( .map_err(|e| DispatchErrorWithPostInfo { post_info, error: e }) } -impl Pallet +impl Pallet where - T: Config, BalanceOf: Into, BalanceOf: TryFrom, MomentOf: Into, + T::Hash: IsType, { /// A generalized version of [`Self::call`]. /// @@ -1263,6 +1268,7 @@ where impl Pallet where T: Config, + T::Hash: IsType, { /// Return the existential deposit of [`Config::Currency`]. fn min_balance() -> BalanceOf { @@ -1275,9 +1281,9 @@ where } /// Deposit a pallet contracts indexed event. - fn deposit_indexed_event(topics: Vec, event: Event) { + fn deposit_indexed_event(topics: Vec, event: Event) { >::deposit_event_indexed( - &topics, + &topics.into_iter().map(Into::into).collect::>(), ::RuntimeEvent::from(event).into(), ) } diff --git a/substrate/frame/revive/src/storage.rs b/substrate/frame/revive/src/storage.rs index ef7ce2db32cf..9939de1dfd19 100644 --- a/substrate/frame/revive/src/storage.rs +++ b/substrate/frame/revive/src/storage.rs @@ -33,11 +33,12 @@ use codec::{Decode, Encode, MaxEncodedLen}; use core::marker::PhantomData; use frame_support::{ storage::child::{self, ChildInfo}, + traits::IsType, weights::{Weight, WeightMeter}, CloneNoBound, DefaultNoBound, }; use scale_info::TypeInfo; -use sp_core::{ConstU32, Get, H160}; +use sp_core::{ConstU32, Get, H160, H256}; use sp_io::KillStorageResult; use sp_runtime::{ traits::{Hash, Saturating, Zero}, @@ -77,7 +78,10 @@ pub struct ContractInfo { delegate_dependencies: DelegateDependencyMap, } -impl ContractInfo { +impl ContractInfo +where + T::Hash: IsType, +{ /// Constructs a new contract info **without** writing it to storage. /// /// This returns an `Err` if an contract with the supplied `account` already exists diff --git a/substrate/frame/revive/src/storage/meter.rs b/substrate/frame/revive/src/storage/meter.rs index f6ad4c5fc346..9d70ddf85870 100644 --- a/substrate/frame/revive/src/storage/meter.rs +++ b/substrate/frame/revive/src/storage/meter.rs @@ -21,17 +21,17 @@ use crate::{ address::AddressMapper, storage::ContractInfo, AccountIdOf, BalanceOf, CodeInfo, Config, Error, Event, HoldReason, Inspect, Origin, Pallet, StorageDeposit as Deposit, System, LOG_TARGET, }; - use alloc::vec::Vec; use core::{fmt::Debug, marker::PhantomData}; use frame_support::{ traits::{ fungible::{Mutate, MutateHold}, tokens::{Fortitude, Fortitude::Polite, Precision, Preservation, Restriction}, - Get, + Get, IsType, }, DefaultNoBound, RuntimeDebugNoBound, }; +use sp_core::H256; use sp_runtime::{ traits::{Saturating, Zero}, DispatchError, FixedPointNumber, FixedU128, @@ -400,6 +400,7 @@ where impl RawMeter where T: Config, + T::Hash: IsType, E: Ext, { /// Charges `diff` from the meter. @@ -503,7 +504,10 @@ where } } -impl Ext for ReservingExt { +impl Ext for ReservingExt +where + T::Hash: IsType, +{ fn check_limit( origin: &T::AccountId, limit: BalanceOf, diff --git a/substrate/frame/revive/src/test_utils/builder.rs b/substrate/frame/revive/src/test_utils/builder.rs index 1e09e2837f2c..7d5f3be8cca6 100644 --- a/substrate/frame/revive/src/test_utils/builder.rs +++ b/substrate/frame/revive/src/test_utils/builder.rs @@ -57,6 +57,7 @@ macro_rules! builder { BalanceOf: Into, BalanceOf: TryFrom, crate::MomentOf: Into, + T::Hash: frame_support::traits::IsType, { $( #[doc = concat!("Set the ", stringify!($field))] diff --git a/substrate/frame/revive/src/wasm/mod.rs b/substrate/frame/revive/src/wasm/mod.rs index d6b5eb56b738..a5c406ecd6a8 100644 --- a/substrate/frame/revive/src/wasm/mod.rs +++ b/substrate/frame/revive/src/wasm/mod.rs @@ -45,9 +45,9 @@ use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ dispatch::DispatchResult, ensure, - traits::{fungible::MutateHold, tokens::Precision::BestEffort}, + traits::{fungible::MutateHold, tokens::Precision::BestEffort, IsType}, }; -use sp_core::{Get, U256}; +use sp_core::{Get, H256, U256}; use sp_runtime::DispatchError; /// Validated Wasm module ready for execution. @@ -125,6 +125,7 @@ impl Token for CodeLoadToken { impl WasmBlob where + T::Hash: IsType, BalanceOf: Into, BalanceOf: TryFrom, { diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index e2ddcca6947c..bd528d3e671f 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -1545,7 +1545,7 @@ pub mod env { return Err(Error::::ValueTooLarge.into()); } - let topics: Vec::T>> = match topics_len { + let topics: Vec = match topics_len { 0 => Vec::new(), _ => memory.read_as_unbounded(topics_ptr, topics_len)?, }; From ee9a0c1cc284185da2740f8fb001bc3f8e96fc1d Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 22:17:12 +0200 Subject: [PATCH 10/21] Fix benchmark Contract --- substrate/frame/revive/src/benchmarking/mod.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index edf578a9e0ae..6f4239b5558d 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -65,7 +65,6 @@ const UNBALANCED_TRIE_LAYERS: u32 = 20; struct Contract { caller: T::AccountId, account_id: T::AccountId, - addr: T::AccountId, } impl Contract @@ -114,7 +113,7 @@ where let address = outcome.result?.addr; let account_id = T::AddressMapper::to_account_id_contract(&address); - let result = Contract { caller, account_id: account_id.clone(), addr: account_id }; + let result = Contract { caller, account_id: account_id.clone() }; ContractInfoOf::::insert(&address, result.info()?); @@ -335,7 +334,7 @@ mod benchmarks { let instance = Contract::::with_caller(whitelisted_caller(), WasmModule::sized(c), vec![])?; let value = Pallet::::min_balance(); - let callee = T::AddressMapper::to_address(&instance.addr); + let callee = T::AddressMapper::to_address(&instance.account_id); let storage_deposit = default_deposit_limit::(); #[extrinsic_call] @@ -443,7 +442,7 @@ mod benchmarks { Contract::::with_caller(whitelisted_caller(), WasmModule::dummy(), vec![])?; let value = Pallet::::min_balance(); let origin = RawOrigin::Signed(instance.caller.clone()); - let callee = T::AddressMapper::to_address(&instance.addr); + let callee = T::AddressMapper::to_address(&instance.account_id); let before = T::Currency::balance(&instance.account_id); let storage_deposit = default_deposit_limit::(); #[extrinsic_call] @@ -519,7 +518,7 @@ mod benchmarks { let storage_deposit = default_deposit_limit::(); let hash = >::bare_upload_code(origin.into(), code, storage_deposit)?.code_hash; - let callee = T::AddressMapper::to_address(&instance.addr); + let callee = T::AddressMapper::to_address(&instance.account_id); assert_ne!(instance.info()?.code_hash, hash); #[extrinsic_call] _(RawOrigin::Root, callee, hash); From 939bdc43af205f0d196e50294717c2b9fa27236a Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 22:20:17 +0200 Subject: [PATCH 11/21] Fix clippy --- substrate/frame/revive/src/exec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index 630f8d5e3daa..d3215b0b81fd 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -1484,7 +1484,7 @@ where } fn now(&self) -> U256 { - self.timestamp.clone().into() + self.timestamp.into() } fn minimum_balance(&self) -> U256 { From 3656177c1283fb0991e8003eebd19dc352c8424a Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 22:29:49 +0200 Subject: [PATCH 12/21] Join lines --- substrate/frame/revive/src/benchmarking/call_builder.rs | 6 ++---- substrate/frame/revive/src/benchmarking/mod.rs | 7 ++----- substrate/frame/revive/src/exec.rs | 6 ++---- substrate/frame/revive/src/lib.rs | 6 ++---- substrate/frame/revive/src/test_utils/builder.rs | 3 +-- substrate/frame/revive/src/wasm/mod.rs | 6 ++---- 6 files changed, 11 insertions(+), 23 deletions(-) diff --git a/substrate/frame/revive/src/benchmarking/call_builder.rs b/substrate/frame/revive/src/benchmarking/call_builder.rs index 7a423a633f69..020a578c3a3a 100644 --- a/substrate/frame/revive/src/benchmarking/call_builder.rs +++ b/substrate/frame/revive/src/benchmarking/call_builder.rs @@ -50,8 +50,7 @@ impl Default for CallSetup where T: Config + pallet_balances::Config, as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, - BalanceOf: Into, - BalanceOf: TryFrom, + BalanceOf: Into + TryFrom, T::Hash: IsType, MomentOf: Into, { @@ -65,8 +64,7 @@ where T: Config + pallet_balances::Config, T::Hash: IsType, as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, - BalanceOf: Into, - BalanceOf: TryFrom, + BalanceOf: Into + TryFrom, MomentOf: Into, { /// Setup a new call for the given module. diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index 6f4239b5558d..2a7567ea47cc 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -71,8 +71,7 @@ impl Contract where T: Config + pallet_balances::Config, as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, - BalanceOf: Into, - BalanceOf: TryFrom, + BalanceOf: Into + TryFrom, MomentOf: Into, T::Hash: IsType, { @@ -221,10 +220,8 @@ fn default_deposit_limit() -> BalanceOf { where as codec::HasCompact>::Type: Clone + Eq + PartialEq + core::fmt::Debug + scale_info::TypeInfo + codec::Encode, T: Config + pallet_balances::Config, - BalanceOf: From< as Currency>::Balance>, + BalanceOf: From< as Currency>::Balance> + Into + TryFrom, as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, - BalanceOf: Into, - BalanceOf: TryFrom, MomentOf: Into, T::Hash: IsType, as Currency>::Balance: From>, diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index d3215b0b81fd..e1a74c1c639a 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -698,8 +698,7 @@ impl<'a, T, E> Stack<'a, T, E> where T: Config, T::Hash: IsType, - BalanceOf: Into, - BalanceOf: TryFrom, + BalanceOf: Into + TryFrom, MomentOf: Into, E: Executable, { @@ -1245,8 +1244,7 @@ where T: Config, T::Hash: IsType, E: Executable, - BalanceOf: Into, - BalanceOf: TryFrom, + BalanceOf: Into + TryFrom, MomentOf: Into, { type T = T; diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs index 6b74dc069988..c64afbf560f5 100644 --- a/substrate/frame/revive/src/lib.rs +++ b/substrate/frame/revive/src/lib.rs @@ -782,8 +782,7 @@ pub mod pallet { where T::Hash: IsType, as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, - BalanceOf: Into, - BalanceOf: TryFrom, + BalanceOf: Into + TryFrom, MomentOf: Into, { /// Makes a call to an account, optionally transferring some balance. @@ -1068,8 +1067,7 @@ fn dispatch_result( impl Pallet where - BalanceOf: Into, - BalanceOf: TryFrom, + BalanceOf: Into + TryFrom, MomentOf: Into, T::Hash: IsType, { diff --git a/substrate/frame/revive/src/test_utils/builder.rs b/substrate/frame/revive/src/test_utils/builder.rs index 7d5f3be8cca6..b17d7628fb80 100644 --- a/substrate/frame/revive/src/test_utils/builder.rs +++ b/substrate/frame/revive/src/test_utils/builder.rs @@ -54,8 +54,7 @@ macro_rules! builder { impl $name where as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, - BalanceOf: Into, - BalanceOf: TryFrom, + BalanceOf: Into + TryFrom, crate::MomentOf: Into, T::Hash: frame_support::traits::IsType, { diff --git a/substrate/frame/revive/src/wasm/mod.rs b/substrate/frame/revive/src/wasm/mod.rs index a5c406ecd6a8..5813903326bf 100644 --- a/substrate/frame/revive/src/wasm/mod.rs +++ b/substrate/frame/revive/src/wasm/mod.rs @@ -126,8 +126,7 @@ impl Token for CodeLoadToken { impl WasmBlob where T::Hash: IsType, - BalanceOf: Into, - BalanceOf: TryFrom, + BalanceOf: Into + TryFrom, { /// We only check for size and nothing else when the code is uploaded. pub fn from_code( @@ -326,8 +325,7 @@ impl WasmBlob { impl Executable for WasmBlob where - BalanceOf: Into, - BalanceOf: TryFrom, + BalanceOf: Into + TryFrom, { fn from_storage( code_hash: sp_core::H256, From 98771ff631da969390e621ae1d169320150be9fa Mon Sep 17 00:00:00 2001 From: pgherveou Date: Thu, 5 Sep 2024 23:26:25 +0200 Subject: [PATCH 13/21] Fix --- prdoc/pr_5608.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_5608.prdoc b/prdoc/pr_5608.prdoc index 865b21b44ca4..9a0748e46bab 100644 --- a/prdoc/pr_5608.prdoc +++ b/prdoc/pr_5608.prdoc @@ -7,7 +7,7 @@ doc: crates: - name: pallet-revive - bump: patch + bump: major - name: pallet-revive-uapi bump: patch - name: pallet-revive-fixtures From bc41883ec8c0e5c113544a479b5c13ede723d61b Mon Sep 17 00:00:00 2001 From: pgherveou Date: Fri, 6 Sep 2024 09:26:30 +0200 Subject: [PATCH 14/21] rm TopicOf type --- substrate/frame/revive/src/exec.rs | 3 --- substrate/frame/revive/src/wasm/runtime.rs | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index e1a74c1c639a..016bdec37afd 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -61,9 +61,6 @@ pub type AccountIdOf = ::AccountId; pub type MomentOf = <::Time as Time>::Moment; pub type ExecResult = Result; -/// A type that represents a topic of an event. At the moment a hash is used. -pub type TopicOf = ::Hash; - /// Type for variable sized storage key. Used for transparent hashing. type VarSizedKey = BoundedVec>; diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index bd528d3e671f..da5d1684f275 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -19,7 +19,7 @@ use crate::{ address::AddressMapper, - exec::{ExecError, ExecResult, Ext, Key, TopicOf}, + exec::{ExecError, ExecResult, Ext, Key}, gas::{ChargedAmount, Token}, limits, primitives::ExecReturnValue, @@ -1538,7 +1538,7 @@ pub mod env { data_len: u32, ) -> Result<(), TrapReason> { let num_topic = topics_len - .checked_div(core::mem::size_of::>() as u32) + .checked_div(core::mem::size_of::() as u32) .ok_or("Zero sized topics are not allowed")?; self.charge_gas(RuntimeCosts::DepositEvent { num_topic, len: data_len })?; if data_len > self.ext.max_value_size() { From 696c4610ee7b97054fd93f6034bddb2ca5efe8fd Mon Sep 17 00:00:00 2001 From: pgherveou Date: Fri, 6 Sep 2024 11:43:10 +0200 Subject: [PATCH 15/21] Fixing topics --- .../contracts/event_and_return_on_deploy.rs | 3 +- .../revive/fixtures/contracts/event_size.rs | 3 +- .../frame/revive/src/benchmarking/mod.rs | 5 ++++ substrate/frame/revive/src/wasm/runtime.rs | 29 ++++++++++--------- substrate/frame/revive/uapi/src/host.rs | 4 +-- .../frame/revive/uapi/src/host/riscv32.rs | 6 ++-- 6 files changed, 30 insertions(+), 20 deletions(-) diff --git a/substrate/frame/revive/fixtures/contracts/event_and_return_on_deploy.rs b/substrate/frame/revive/fixtures/contracts/event_and_return_on_deploy.rs index 9186835d2911..e4b106c5e2d0 100644 --- a/substrate/frame/revive/fixtures/contracts/event_and_return_on_deploy.rs +++ b/substrate/frame/revive/fixtures/contracts/event_and_return_on_deploy.rs @@ -25,7 +25,8 @@ use uapi::{HostFn, HostFnImpl as api}; #[polkavm_derive::polkavm_export] pub extern "C" fn deploy() { let buffer = [1u8, 2, 3, 4]; - api::deposit_event(&[0u8; 0], &buffer); + let topics = [[0u8; 32]; 0]; + api::deposit_event(&topics, &buffer); api::return_value(uapi::ReturnFlags::empty(), &buffer); } diff --git a/substrate/frame/revive/fixtures/contracts/event_size.rs b/substrate/frame/revive/fixtures/contracts/event_size.rs index 2b56de4bd3fd..7f04ae42765a 100644 --- a/substrate/frame/revive/fixtures/contracts/event_size.rs +++ b/substrate/frame/revive/fixtures/contracts/event_size.rs @@ -33,6 +33,7 @@ pub extern "C" fn call() { input!(len: u32,); let data = &BUFFER[..len as usize]; + let topics = [[0u8; 32]; 0]; - api::deposit_event(&[0u8; 0], data); + api::deposit_event(&topics, data); } diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index 2a7567ea47cc..b098d84da65f 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -75,6 +75,11 @@ where MomentOf: Into, T::Hash: IsType, { + /// The address of the contract. + fn address(&self) -> H160 { + T::AddressMapper::to_address(&self.account_id) + } + /// Create new contract and use a default account id as instantiator. fn new(module: WasmModule, data: Vec) -> Result, &'static str> { Self::with_index(0, module, data) diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index da5d1684f275..53b0c999931f 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -1533,32 +1533,35 @@ pub mod env { &mut self, memory: &mut M, topics_ptr: u32, - topics_len: u32, + num_topic: u32, data_ptr: u32, data_len: u32, ) -> Result<(), TrapReason> { - let num_topic = topics_len - .checked_div(core::mem::size_of::() as u32) - .ok_or("Zero sized topics are not allowed")?; self.charge_gas(RuntimeCosts::DepositEvent { num_topic, len: data_len })?; + + if num_topic > limits::NUM_EVENT_TOPICS { + return Err(Error::::TooManyTopics.into()); + } + if data_len > self.ext.max_value_size() { return Err(Error::::ValueTooLarge.into()); } - let topics: Vec = match topics_len { + let topics: Vec = match num_topic { 0 => Vec::new(), - _ => memory.read_as_unbounded(topics_ptr, topics_len)?, + _ => { + let mut v = Vec::with_capacity(num_topic as usize); + let topics_len = num_topic * core::mem::size_of::() as u32; + let buf = memory.read(topics_ptr, topics_len)?; + for chunk in buf.chunks_exact(core::mem::size_of::()) { + v.push(H256::from_slice(chunk)); + } + v + }, }; - // If there are more than `event_topics`, then trap. - if topics.len() as u32 > limits::NUM_EVENT_TOPICS { - return Err(Error::::TooManyTopics.into()); - } - let event_data = memory.read(data_ptr, data_len)?; - self.ext.deposit_event(topics, event_data); - Ok(()) } diff --git a/substrate/frame/revive/uapi/src/host.rs b/substrate/frame/revive/uapi/src/host.rs index 281e90e64bcb..101ae9aca465 100644 --- a/substrate/frame/revive/uapi/src/host.rs +++ b/substrate/frame/revive/uapi/src/host.rs @@ -282,8 +282,8 @@ pub trait HostFn: private::Sealed { /// /// # Parameters /// - /// - `topics`: The topics list encoded as `Vec`. It can't contain duplicates. - fn deposit_event(topics: &[u8], data: &[u8]); + /// - `topics`: The topics list. It can't contain duplicates. + fn deposit_event(topics: &[[u8; 32]], data: &[u8]); /// Recovers the ECDSA public key from the given message hash and signature. /// diff --git a/substrate/frame/revive/uapi/src/host/riscv32.rs b/substrate/frame/revive/uapi/src/host/riscv32.rs index 6fafdcb677a6..b7b660c40837 100644 --- a/substrate/frame/revive/uapi/src/host/riscv32.rs +++ b/substrate/frame/revive/uapi/src/host/riscv32.rs @@ -86,8 +86,8 @@ mod sys { pub fn now(out_ptr: *mut u8); pub fn minimum_balance(out_ptr: *mut u8); pub fn deposit_event( - topics_ptr: *const u8, - topics_len: u32, + topics_ptr: *const [u8; 32], + num_topic: u32, data_ptr: *const u8, data_len: u32, ); @@ -326,7 +326,7 @@ impl HostFn for HostFnImpl { ret_code.into() } - fn deposit_event(topics: &[u8], data: &[u8]) { + fn deposit_event(topics: &[[u8; 32]], data: &[u8]) { unsafe { sys::deposit_event( topics.as_ptr(), From d13a8bb4850d3d3e381143d106ed23905b247626 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Fri, 6 Sep 2024 11:54:42 +0200 Subject: [PATCH 16/21] fix bench --- .../frame/revive/src/benchmarking/mod.rs | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index b098d84da65f..971923d68341 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -75,11 +75,6 @@ where MomentOf: Into, T::Hash: IsType, { - /// The address of the contract. - fn address(&self) -> H160 { - T::AddressMapper::to_address(&self.account_id) - } - /// Create new contract and use a default account id as instantiator. fn new(module: WasmModule, data: Vec) -> Result, &'static str> { Self::with_index(0, module, data) @@ -816,28 +811,31 @@ mod benchmarks { t: Linear<0, { limits::NUM_EVENT_TOPICS as u32 }>, n: Linear<0, { limits::PAYLOAD_BYTES }>, ) { - let topics = (0..t).map(|i| T::Hashing::hash_of(&i)).collect::>().encode(); - let topics_len = topics.len() as u32; - - build_runtime!(runtime, memory: [ - n.to_le_bytes(), - topics, - vec![0u8; n as _], - ]); + let num_topic = t as u32; + let topics = (0..t).map(|i| H256::repeat_byte(i as u8)).collect::>(); + let topics_data = + topics.iter().flat_map(|hash| hash.as_bytes().to_vec()).collect::>(); + let data = vec![42u8; n as _]; + build_runtime!(runtime, memory: [ topics_data, data, ]); let result; #[block] { result = runtime.bench_deposit_event( memory.as_mut_slice(), - 4, // topics_ptr - topics_len, // topics_len - 4 + topics_len, // data_ptr - 0, // data_len + 0, // topics_ptr + num_topic, + topics_data.len() as u32, // data_ptr + n, // data_len ); } - assert_ok!(result); + + let event = System::::events().into_iter().next().unwrap(); + assert_eq!( + topics, + event.topics.iter().map(|t| H256::from_slice(t.as_ref())).collect::>() + ); } // Benchmark debug_message call From e72a4af8f1686b9949413a56ae3638a1169e6f92 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Fri, 6 Sep 2024 12:16:49 +0200 Subject: [PATCH 17/21] Test some topics --- .../revive/fixtures/contracts/event_and_return_on_deploy.rs | 2 +- substrate/frame/revive/src/tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/revive/fixtures/contracts/event_and_return_on_deploy.rs b/substrate/frame/revive/fixtures/contracts/event_and_return_on_deploy.rs index e4b106c5e2d0..5c438c1a75a1 100644 --- a/substrate/frame/revive/fixtures/contracts/event_and_return_on_deploy.rs +++ b/substrate/frame/revive/fixtures/contracts/event_and_return_on_deploy.rs @@ -25,7 +25,7 @@ use uapi::{HostFn, HostFnImpl as api}; #[polkavm_derive::polkavm_export] pub extern "C" fn deploy() { let buffer = [1u8, 2, 3, 4]; - let topics = [[0u8; 32]; 0]; + let topics = [[42u8; 32]; 1]; api::deposit_event(&topics, &buffer); api::return_value(uapi::ReturnFlags::empty(), &buffer); } diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index d0b231c6fff5..fb168022013c 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -746,7 +746,7 @@ mod run_tests { contract: addr, data: vec![1, 2, 3, 4] }), - topics: vec![], + topics: vec![H256::repeat_byte(42)], }, EventRecord { phase: Phase::Initialization, From 563eaded6569a38034744d3717d7a4a8e3f9257c Mon Sep 17 00:00:00 2001 From: pgherveou Date: Fri, 6 Sep 2024 15:50:25 +0200 Subject: [PATCH 18/21] Improve test --- .../frame/revive/src/benchmarking/mod.rs | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index 971923d68341..2e2dbadfa036 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -75,6 +75,11 @@ where MomentOf: Into, T::Hash: IsType, { + /// Returns the address of the contract. + fn address(&self) -> H160 { + T::AddressMapper::to_address(&self.account_id) + } + /// Create new contract and use a default account id as instantiator. fn new(module: WasmModule, data: Vec) -> Result, &'static str> { Self::with_index(0, module, data) @@ -218,12 +223,12 @@ fn default_deposit_limit() -> BalanceOf { #[benchmarks( where - as codec::HasCompact>::Type: Clone + Eq + PartialEq + core::fmt::Debug + scale_info::TypeInfo + codec::Encode, - T: Config + pallet_balances::Config, - BalanceOf: From< as Currency>::Balance> + Into + TryFrom, as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode, + BalanceOf: From< as Currency>::Balance> + Into + TryFrom, + T: Config + pallet_balances::Config, MomentOf: Into, - T::Hash: IsType, + ::RuntimeEvent: From>, + T::Hash: IsType, as Currency>::Balance: From>, )] mod benchmarks { @@ -816,7 +821,7 @@ mod benchmarks { let topics_data = topics.iter().flat_map(|hash| hash.as_bytes().to_vec()).collect::>(); let data = vec![42u8; n as _]; - build_runtime!(runtime, memory: [ topics_data, data, ]); + build_runtime!(runtime, instance, memory: [ topics_data, data, ]); let result; #[block] @@ -831,10 +836,16 @@ mod benchmarks { } assert_ok!(result); - let event = System::::events().into_iter().next().unwrap(); + let events = System::::events(); + let record = &events[events.len() - 1]; + + assert_eq!( + record.event, + crate::Event::ContractEmitted { contract: instance.address(), data }.into(), + ); assert_eq!( + record.topics.iter().map(|t| H256::from_slice(t.as_ref())).collect::>(), topics, - event.topics.iter().map(|t| H256::from_slice(t.as_ref())).collect::>() ); } From 1b2ed74ac4ad085219bd4e7373e9e392b32b8e09 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Sun, 8 Sep 2024 19:41:24 +0200 Subject: [PATCH 19/21] Remove SCALE encoding in runtime --- substrate/frame/revive/src/wasm/runtime.rs | 81 +++++++++++++++------- 1 file changed, 57 insertions(+), 24 deletions(-) diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index 53b0c999931f..528b0ababfa0 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -44,6 +44,13 @@ type CallOf = ::RuntimeCall; /// The maximum nesting depth a contract can use when encoding types. const MAX_DECODE_NESTING: u32 = 256; +/// Encode a `U256` into a 32 byte buffer. +fn as_bytes(u: U256) -> [u8; 32] { + let mut bytes = [0u8; 32]; + u.to_little_endian(&mut bytes); + bytes +} + #[derive(Clone, Copy)] pub enum ApiVersion { /// Expose all APIs even unversioned ones. Only used for testing and benchmarking. @@ -84,6 +91,32 @@ pub trait Memory { Ok(buf) } + /// Same as `read` but reads into a fixed size buffer. + fn read_array(&self, ptr: u32) -> Result<[u8; N], DispatchError> { + let mut buf = [0u8; N]; + self.read_into_buf(ptr, &mut buf)?; + Ok(buf) + } + + /// Read a `u32` from the sandbox memory. + fn read_u32(&self, ptr: u32) -> Result { + let buf: [u8; 4] = self.read_array(ptr)?; + Ok(u32::from_le_bytes(buf)) + } + + /// Read a `U256` from the sandbox memory. + fn read_u256(&self, ptr: u32) -> Result { + let buf: [u8; 32] = self.read_array(ptr)?; + Ok(U256::from_little_endian(&buf)) + } + + /// Read a `H256` from the sandbox memory. + fn read_h256(&self, ptr: u32) -> Result { + let mut code_hash = H256::default(); + self.read_into_buf(ptr, code_hash.as_bytes_mut())?; + Ok(code_hash) + } + /// Read designated chunk from the sandbox memory and attempt to decode into the specified type. /// /// Returns `Err` if one of the following conditions occurs: @@ -647,7 +680,7 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { } let buf_len = buf.len() as u32; - let len: u32 = memory.read_as(out_len_ptr)?; + let len = memory.read_u32(out_len_ptr)?; if len < buf_len { return Err(Error::::OutputBufferTooSmall.into()) @@ -963,13 +996,13 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { CallType::Call { callee_ptr, value_ptr, deposit_ptr, weight } => { let mut callee = H160::zero(); memory.read_into_buf(callee_ptr, callee.as_bytes_mut())?; - let deposit_limit: U256 = if deposit_ptr == SENTINEL { + let deposit_limit = if deposit_ptr == SENTINEL { U256::zero() } else { - memory.read_as(deposit_ptr)? + memory.read_u256(deposit_ptr)? }; let read_only = flags.contains(CallFlags::READ_ONLY); - let value: U256 = memory.read_as(value_ptr)?; + let value = memory.read_u256(value_ptr)?; if value > 0u32.into() { // If the call value is non-zero and state change is not allowed, issue an // error. @@ -992,7 +1025,8 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { if flags.intersects(CallFlags::ALLOW_REENTRY | CallFlags::READ_ONLY) { return Err(Error::::InvalidCallFlags.into()) } - let code_hash = memory.read_as(code_hash_ptr)?; + + let code_hash = memory.read_h256(code_hash_ptr)?; self.ext.delegate_call(code_hash, input_data) }, }; @@ -1037,15 +1071,14 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { ) -> Result { self.charge_gas(RuntimeCosts::Instantiate { input_data_len })?; let deposit_limit: U256 = - if deposit_ptr == SENTINEL { U256::zero() } else { memory.read_as(deposit_ptr)? }; - let value: U256 = memory.read_as(value_ptr)?; - let code_hash: H256 = memory.read_as(code_hash_ptr)?; + if deposit_ptr == SENTINEL { U256::zero() } else { memory.read_u256(deposit_ptr)? }; + let value = memory.read_u256(value_ptr)?; + let code_hash = memory.read_h256(code_hash_ptr)?; let input_data = memory.read(input_data_ptr, input_data_len)?; let salt = if salt_ptr == SENTINEL { None } else { - let mut salt = [0u8; 32]; - memory.read_into_buf(salt_ptr, salt.as_mut_slice())?; + let salt: [u8; 32] = memory.read_array(salt_ptr)?; Some(salt) }; let instantiate_outcome = self.ext.instantiate( @@ -1191,7 +1224,7 @@ pub mod env { self.charge_gas(RuntimeCosts::Transfer)?; let mut callee = H160::zero(); memory.read_into_buf(address_ptr, callee.as_bytes_mut())?; - let value: U256 = memory.read_as(value_ptr)?; + let value: U256 = memory.read_u256(value_ptr)?; let result = self.ext.transfer(&callee, value); match result { Ok(()) => Ok(ReturnErrorCode::Success), @@ -1371,7 +1404,7 @@ pub mod env { self.write_fixed_sandbox_output( memory, out_ptr, - &value.encode(), + &value.as_bytes(), false, already_charged, )?; @@ -1386,11 +1419,11 @@ pub mod env { #[api_version(0)] fn own_code_hash(&mut self, memory: &mut M, out_ptr: u32) -> Result<(), TrapReason> { self.charge_gas(RuntimeCosts::OwnCodeHash)?; - let code_hash_encoded = &self.ext.own_code_hash().encode(); + let code_hash = *self.ext.own_code_hash(); Ok(self.write_fixed_sandbox_output( memory, out_ptr, - code_hash_encoded, + code_hash.as_bytes(), false, already_charged, )?) @@ -1477,7 +1510,7 @@ pub mod env { Ok(self.write_fixed_sandbox_output( memory, out_ptr, - &self.ext.balance().encode(), + &as_bytes(self.ext.balance()), false, already_charged, )?) @@ -1491,7 +1524,7 @@ pub mod env { Ok(self.write_fixed_sandbox_output( memory, out_ptr, - &self.ext.value_transferred().encode(), + &as_bytes(self.ext.value_transferred()), false, already_charged, )?) @@ -1505,7 +1538,7 @@ pub mod env { Ok(self.write_fixed_sandbox_output( memory, out_ptr, - &self.ext.now().encode(), + &as_bytes(self.ext.now()), false, already_charged, )?) @@ -1519,7 +1552,7 @@ pub mod env { Ok(self.write_fixed_sandbox_output( memory, out_ptr, - &self.ext.minimum_balance().encode(), + &as_bytes(self.ext.minimum_balance()), false, already_charged, )?) @@ -1551,9 +1584,9 @@ pub mod env { 0 => Vec::new(), _ => { let mut v = Vec::with_capacity(num_topic as usize); - let topics_len = num_topic * core::mem::size_of::() as u32; + let topics_len = num_topic * H256::len_bytes() as u32; let buf = memory.read(topics_ptr, topics_len)?; - for chunk in buf.chunks_exact(core::mem::size_of::()) { + for chunk in buf.chunks_exact(H256::len_bytes()) { v.push(H256::from_slice(chunk)); } v @@ -1573,7 +1606,7 @@ pub mod env { Ok(self.write_fixed_sandbox_output( memory, out_ptr, - &self.ext.block_number().encode(), + &as_bytes(self.ext.block_number()), false, already_charged, )?) @@ -1857,7 +1890,7 @@ pub mod env { code_hash_ptr: u32, ) -> Result { self.charge_gas(RuntimeCosts::SetCodeHash)?; - let code_hash: H256 = memory.read_as(code_hash_ptr)?; + let code_hash: H256 = memory.read_h256(code_hash_ptr)?; match self.ext.set_code_hash(code_hash) { Err(err) => { let code = Self::err_into_return_code(err)?; @@ -1899,7 +1932,7 @@ pub mod env { code_hash_ptr: u32, ) -> Result<(), TrapReason> { self.charge_gas(RuntimeCosts::LockDelegateDependency)?; - let code_hash = memory.read_as(code_hash_ptr)?; + let code_hash = memory.read_h256(code_hash_ptr)?; self.ext.lock_delegate_dependency(code_hash)?; Ok(()) } @@ -1914,7 +1947,7 @@ pub mod env { code_hash_ptr: u32, ) -> Result<(), TrapReason> { self.charge_gas(RuntimeCosts::UnlockDelegateDependency)?; - let code_hash = memory.read_as(code_hash_ptr)?; + let code_hash = memory.read_h256(code_hash_ptr)?; self.ext.unlock_delegate_dependency(&code_hash)?; Ok(()) } From f1299fb2b30414b2193189429c40f2eea0951114 Mon Sep 17 00:00:00 2001 From: PG Herveou Date: Sun, 8 Sep 2024 21:49:51 +0200 Subject: [PATCH 20/21] [pallet-revive] remove custom migration framework (#5622) Fixes https://github.com/paritytech/polkadot-sdk/issues/5621 --- substrate/bin/node/runtime/src/lib.rs | 4 - substrate/frame/revive/Cargo.toml | 1 - substrate/frame/revive/build.rs | 78 --- .../frame/revive/src/benchmarking/mod.rs | 69 -- substrate/frame/revive/src/lib.rs | 93 +-- substrate/frame/revive/src/migration.rs | 650 ------------------ substrate/frame/revive/src/primitives.rs | 2 - substrate/frame/revive/src/tests.rs | 70 +- substrate/frame/revive/src/weights.rs | 365 ---------- 9 files changed, 3 insertions(+), 1329 deletions(-) delete mode 100644 substrate/frame/revive/build.rs delete mode 100644 substrate/frame/revive/src/migration.rs diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 001b2273c9b2..5288c9de8e57 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1418,10 +1418,6 @@ impl pallet_revive::Config for Runtime { type UploadOrigin = EnsureSigned; type InstantiateOrigin = EnsureSigned; type RuntimeHoldReason = RuntimeHoldReason; - #[cfg(not(feature = "runtime-benchmarks"))] - type Migrations = (); - #[cfg(feature = "runtime-benchmarks")] - type Migrations = pallet_revive::migration::codegen::BenchMigrations; type CodeHashLockupDepositPercent = CodeHashLockupDepositPercent; type Debug = (); type Xcm = (); diff --git a/substrate/frame/revive/Cargo.toml b/substrate/frame/revive/Cargo.toml index 6b7542e89202..667328ac2d0d 100644 --- a/substrate/frame/revive/Cargo.toml +++ b/substrate/frame/revive/Cargo.toml @@ -3,7 +3,6 @@ name = "pallet-revive" version = "0.1.0" authors.workspace = true edition.workspace = true -build = "build.rs" license = "Apache-2.0" homepage.workspace = true repository.workspace = true diff --git a/substrate/frame/revive/build.rs b/substrate/frame/revive/build.rs deleted file mode 100644 index ca8e62df6047..000000000000 --- a/substrate/frame/revive/build.rs +++ /dev/null @@ -1,78 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -use std::io::Write; - -/// We start with version 2 instead of 0 when adding the pallet. -/// -/// Because otherwise we can't test any migrations since they require the storage version -/// to be lower than the pallet version in order to be triggerd. With the pallet version -/// at the minimum (0) this would not work. -const LOWEST_STORAGE_VERSION: u16 = 2; - -/// Get the latest migration version. -/// -/// Find the highest version number from the available migration files. -/// Each migration file should follow the naming convention `vXX.rs`, where `XX` is the version -/// number. -fn get_latest_version() -> u16 { - let Ok(dir) = std::fs::read_dir("src/migration") else { return LOWEST_STORAGE_VERSION }; - dir.filter_map(|entry| { - let file_name = entry.as_ref().ok()?.file_name(); - let file_name = file_name.to_str()?; - if file_name.starts_with('v') && file_name.ends_with(".rs") { - let version = &file_name[1..&file_name.len() - 3]; - let version = version.parse::().ok()?; - - // Ensure that the version matches the one defined in the file. - let path = entry.unwrap().path(); - let file_content = std::fs::read_to_string(&path).ok()?; - assert!( - file_content.contains(&format!("const VERSION: u16 = {}", version)), - "Invalid MigrationStep::VERSION in {:?}", - path - ); - - return Some(version) - } - None - }) - .max() - .unwrap_or(LOWEST_STORAGE_VERSION) -} - -/// Generates a module that exposes the latest migration version, and the benchmark migrations type. -fn main() -> Result<(), Box> { - let out_dir = std::env::var("OUT_DIR")?; - let path = std::path::Path::new(&out_dir).join("migration_codegen.rs"); - let mut f = std::fs::File::create(path)?; - let version = get_latest_version(); - write!( - f, - " - pub mod codegen {{ - use crate::NoopMigration; - /// The latest migration version, pulled from the latest migration file. - pub const LATEST_MIGRATION_VERSION: u16 = {version}; - /// The Migration Steps used for benchmarking the migration framework. - pub type BenchMigrations = (NoopMigration<{}>, NoopMigration<{version}>); - }}", - version - 1, - )?; - - Ok(()) -} diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index 2e2dbadfa036..8cdd7da5db9d 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -25,7 +25,6 @@ use self::{call_builder::CallSetup, code::WasmModule}; use crate::{ exec::{Key, MomentOf}, limits, - migration::codegen::LATEST_MIGRATION_VERSION, storage::WriteOutcome, Pallet as Contracts, *, }; @@ -34,7 +33,6 @@ use codec::{Encode, MaxEncodedLen}; use frame_benchmarking::v2::*; use frame_support::{ self, assert_ok, - pallet_prelude::StorageVersion, storage::child, traits::{fungible::InspectHold, Currency}, weights::{Weight, WeightMeter}, @@ -256,73 +254,6 @@ mod benchmarks { Ok(()) } - // This benchmarks the weight of executing Migration::migrate to execute a noop migration. - #[benchmark(pov_mode = Measured)] - fn migration_noop() { - let version = LATEST_MIGRATION_VERSION; - StorageVersion::new(version).put::>(); - #[block] - { - Migration::::migrate(&mut WeightMeter::new()); - } - assert_eq!(StorageVersion::get::>(), version); - } - - // This benchmarks the weight of dispatching migrate to execute 1 `NoopMigration` - #[benchmark(pov_mode = Measured)] - fn migrate() { - let latest_version = LATEST_MIGRATION_VERSION; - StorageVersion::new(latest_version - 2).put::>(); - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade(); - - #[extrinsic_call] - _(RawOrigin::Signed(whitelisted_caller()), Weight::MAX); - - assert_eq!(StorageVersion::get::>(), latest_version - 1); - } - - // This benchmarks the weight of running on_runtime_upgrade when there are no migration in - // progress. - #[benchmark(pov_mode = Measured)] - fn on_runtime_upgrade_noop() { - let latest_version = LATEST_MIGRATION_VERSION; - StorageVersion::new(latest_version).put::>(); - #[block] - { - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade(); - } - assert!(MigrationInProgress::::get().is_none()); - } - - // This benchmarks the weight of running on_runtime_upgrade when there is a migration in - // progress. - #[benchmark(pov_mode = Measured)] - fn on_runtime_upgrade_in_progress() { - let latest_version = LATEST_MIGRATION_VERSION; - StorageVersion::new(latest_version - 2).put::>(); - let v = vec![42u8].try_into().ok(); - MigrationInProgress::::set(v.clone()); - #[block] - { - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade(); - } - assert!(MigrationInProgress::::get().is_some()); - assert_eq!(MigrationInProgress::::get(), v); - } - - // This benchmarks the weight of running on_runtime_upgrade when there is a migration to - // process. - #[benchmark(pov_mode = Measured)] - fn on_runtime_upgrade() { - let latest_version = LATEST_MIGRATION_VERSION; - StorageVersion::new(latest_version - 2).put::>(); - #[block] - { - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade(); - } - assert!(MigrationInProgress::::get().is_some()); - } - // This benchmarks the overhead of loading a code of size `c` byte from storage and into // the execution engine. This does **not** include the actual execution for which the gas meter // is responsible. This is achieved by generating all code to the `deploy` function diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs index c64afbf560f5..4c6e5cd26a11 100644 --- a/substrate/frame/revive/src/lib.rs +++ b/substrate/frame/revive/src/lib.rs @@ -39,7 +39,6 @@ mod wasm; pub mod chain_extension; pub mod debug; -pub mod migration; pub mod test_utils; pub mod weights; @@ -57,7 +56,7 @@ use environmental::*; use frame_support::{ dispatch::{ DispatchErrorWithPostInfo, DispatchResultWithPostInfo, GetDispatchInfo, Pays, - PostDispatchInfo, RawOrigin, WithPostDispatchInfo, + PostDispatchInfo, RawOrigin, }, ensure, traits::{ @@ -82,7 +81,6 @@ use sp_runtime::{ pub use crate::{ address::{AddressMapper, DefaultAddressMapper}, debug::Tracing, - migration::{MigrateSequence, Migration, NoopMigration}, pallet::*, }; pub use weights::WeightInfo; @@ -275,25 +273,6 @@ pub mod pallet { #[pallet::no_default_bounds] type InstantiateOrigin: EnsureOrigin; - /// The sequence of migration steps that will be applied during a migration. - /// - /// # Examples - /// ```ignore - /// use pallet_revive::migration::{v10, v11}; - /// # struct Runtime {}; - /// # struct Currency {}; - /// type Migrations = (v10::Migration, v11::Migration); - /// ``` - /// - /// If you have a single migration step, you can use a tuple with a single element: - /// ```ignore - /// use pallet_revive::migration::v10; - /// # struct Runtime {}; - /// # struct Currency {}; - /// type Migrations = (v10::Migration,); - /// ``` - type Migrations: MigrateSequence; - /// For most production chains, it's recommended to use the `()` implementation of this /// trait. This implementation offers additional logging when the log target /// "runtime::revive" is set to trace. @@ -386,7 +365,6 @@ pub mod pallet { type DepositPerByte = DepositPerByte; type DepositPerItem = DepositPerItem; type MaxCodeLen = ConstU32<{ 123 * 1024 }>; - type Migrations = (); type Time = Self; type UnsafeUnstableInterface = ConstBool; type UploadOrigin = EnsureSigned; @@ -553,10 +531,6 @@ pub mod pallet { /// A more detailed error can be found on the node console if debug messages are enabled /// by supplying `-lruntime::revive=debug`. CodeRejected, - /// A pending migration needs to complete before the extrinsic can be called. - MigrationInProgress, - /// Migrate dispatch call was attempted but no migration was performed. - NoMigrationPerformed, /// The contract has reached its maximum number of delegate dependencies. MaxDelegateDependenciesReached, /// The dependency was not found in the contract's delegate dependencies. @@ -611,12 +585,6 @@ pub mod pallet { pub(crate) type DeletionQueueCounter = StorageValue<_, DeletionQueueManager, ValueQuery>; - /// A migration can span across multiple blocks. This storage defines a cursor to track the - /// progress of the migration, enabling us to resume from the last completed position. - #[pallet::storage] - pub(crate) type MigrationInProgress = - StorageValue<_, migration::Cursor, OptionQuery>; - #[pallet::extra_constants] impl Pallet { #[pallet::constant_name(ApiVersion)] @@ -631,29 +599,12 @@ pub mod pallet { T::Hash: IsType, { fn on_idle(_block: BlockNumberFor, limit: Weight) -> Weight { - use migration::MigrateResult::*; let mut meter = WeightMeter::with_limit(limit); - - loop { - match Migration::::migrate(&mut meter) { - // There is not enough weight to perform a migration. - // We can't do anything more, so we return the used weight. - NoMigrationPerformed | InProgress { steps_done: 0 } => return meter.consumed(), - // Migration is still in progress, we can start the next step. - InProgress { .. } => continue, - // Either no migration is in progress, or we are done with all migrations, we - // can do some more other work with the remaining weight. - Completed | NoMigrationInProgress => break, - } - } - ContractInfo::::process_deletion_queue_batch(&mut meter); meter.consumed() } fn integrity_test() { - Migration::::integrity_test(); - // Total runtime memory limit let max_runtime_mem: u32 = T::RuntimeMemory::get(); // Memory limits for a single contract: @@ -969,7 +920,6 @@ pub mod pallet { origin: OriginFor, code_hash: sp_core::H256, ) -> DispatchResultWithPostInfo { - Migration::::ensure_migrated()?; let origin = ensure_signed(origin)?; >::remove(&origin, code_hash)?; // we waive the fee because removing unused code is beneficial @@ -993,7 +943,6 @@ pub mod pallet { dest: H160, code_hash: sp_core::H256, ) -> DispatchResult { - Migration::::ensure_migrated()?; ensure_root(origin)?; >::try_mutate(&dest, |contract| { let contract = if let Some(contract) = contract { @@ -1012,40 +961,6 @@ pub mod pallet { Ok(()) }) } - - /// When a migration is in progress, this dispatchable can be used to run migration steps. - /// Calls that contribute to advancing the migration have their fees waived, as it's helpful - /// for the chain. Note that while the migration is in progress, the pallet will also - /// leverage the `on_idle` hooks to run migration steps. - #[pallet::call_index(6)] - #[pallet::weight(T::WeightInfo::migrate().saturating_add(*weight_limit))] - pub fn migrate(origin: OriginFor, weight_limit: Weight) -> DispatchResultWithPostInfo { - use migration::MigrateResult::*; - ensure_signed(origin)?; - - let weight_limit = weight_limit.saturating_add(T::WeightInfo::migrate()); - let mut meter = WeightMeter::with_limit(weight_limit); - let result = Migration::::migrate(&mut meter); - - match result { - Completed => Ok(PostDispatchInfo { - actual_weight: Some(meter.consumed()), - pays_fee: Pays::No, - }), - InProgress { steps_done, .. } if steps_done > 0 => Ok(PostDispatchInfo { - actual_weight: Some(meter.consumed()), - pays_fee: Pays::No, - }), - InProgress { .. } => Ok(PostDispatchInfo { - actual_weight: Some(meter.consumed()), - pays_fee: Pays::Yes, - }), - NoMigrationInProgress | NoMigrationPerformed => { - let err: DispatchError = >::NoMigrationPerformed.into(); - Err(err.with_weight(meter.consumed())) - }, - } - } } } @@ -1095,7 +1010,6 @@ where None }; let try_call = || { - Migration::::ensure_migrated()?; let origin = Origin::from_runtime_origin(origin)?; let mut storage_meter = StorageMeter::new(&origin, storage_deposit_limit, value)?; let result = ExecStack::>::run_call( @@ -1148,7 +1062,6 @@ where let mut debug_message = if debug == DebugInfo::UnsafeDebug { Some(DebugBuffer::default()) } else { None }; let try_instantiate = || { - Migration::::ensure_migrated()?; let instantiate_account = T::InstantiateOrigin::ensure_origin(origin.clone())?; let (executable, upload_deposit) = match code { Code::Upload(code) => { @@ -1209,7 +1122,6 @@ where code: Vec, storage_deposit_limit: BalanceOf, ) -> CodeUploadResult> { - Migration::::ensure_migrated()?; let origin = T::UploadOrigin::ensure_origin(origin)?; let (module, deposit) = Self::try_upload_code(origin, code, storage_deposit_limit, None)?; Ok(CodeUploadReturnValue { code_hash: *module.code_hash(), deposit }) @@ -1217,9 +1129,6 @@ where /// Query storage of a specified contract under a specified key. pub fn get_storage(address: H160, key: [u8; 32]) -> GetStorageResult { - if Migration::::in_progress() { - return Err(ContractAccessError::MigrationInProgress) - } let contract_info = ContractInfoOf::::get(&address).ok_or(ContractAccessError::DoesntExist)?; diff --git a/substrate/frame/revive/src/migration.rs b/substrate/frame/revive/src/migration.rs deleted file mode 100644 index b67467b322f5..000000000000 --- a/substrate/frame/revive/src/migration.rs +++ /dev/null @@ -1,650 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! Multi-block Migration framework for pallet-revive. -//! -//! This module allows us to define a migration as a sequence of [`MigrationStep`]s that can be -//! executed across multiple blocks. -//! -//! # Usage -//! -//! A migration step is defined under `src/migration/vX.rs`, where `X` is the version number. -//! For example, `vX.rs` defines a migration from version `X - 1` to version `X`. -//! -//! ## Example: -//! -//! To configure a migration to `v11` for a runtime using `v10` of pallet-revive on the chain, -//! you would set the `Migrations` type as follows: -//! -//! ```ignore -//! use pallet_revive::migration::{v10, v11}; -//! # pub enum Runtime {}; -//! # struct Currency; -//! type Migrations = (v10::Migration, v11::Migration); -//! ``` -//! -//! ## Notes: -//! -//! - Migrations should always be tested with `try-runtime` before being deployed. -//! - By testing with `try-runtime` against a live network, you ensure that all migration steps work -//! and that you have included the required steps. -//! -//! ## Low Level / Implementation Details -//! -//! When a migration starts and [`OnRuntimeUpgrade::on_runtime_upgrade`] is called, instead of -//! performing the actual migration, we set a custom storage item [`MigrationInProgress`]. -//! This storage item defines a [`Cursor`] for the current migration. -//! -//! If the [`MigrationInProgress`] storage item exists, it means a migration is in progress, and its -//! value holds a cursor for the current migration step. These migration steps are executed during -//! [`Hooks::on_idle`] or when the [`Pallet::migrate`] dispatchable is -//! called. -//! -//! While the migration is in progress, all dispatchables except `migrate`, are blocked, and returns -//! a `MigrationInProgress` error. - -include!(concat!(env!("OUT_DIR"), "/migration_codegen.rs")); - -use crate::{weights::WeightInfo, Config, Error, MigrationInProgress, Pallet, Weight, LOG_TARGET}; -use codec::{Codec, Decode}; -use core::marker::PhantomData; -use frame_support::{ - pallet_prelude::*, - traits::{ConstU32, OnRuntimeUpgrade}, - weights::WeightMeter, -}; -use sp_runtime::Saturating; - -#[cfg(feature = "try-runtime")] -use alloc::vec::Vec; -#[cfg(feature = "try-runtime")] -use sp_runtime::TryRuntimeError; - -const PROOF_ENCODE: &str = "Tuple::max_encoded_len() < Cursor::max_encoded_len()` is verified in `Self::integrity_test()`; qed"; -const PROOF_DECODE: &str = - "We encode to the same type in this trait only. No other code touches this item; qed"; - -fn invalid_version(version: StorageVersion) -> ! { - panic!("Required migration {version:?} not supported by this runtime. This is a bug."); -} - -/// The cursor used to encode the position (usually the last iterated key) of the current migration -/// step. -pub type Cursor = BoundedVec>; - -/// IsFinished describes whether a migration is finished or not. -pub enum IsFinished { - Yes, - No, -} - -/// A trait that allows to migrate storage from one version to another. -/// -/// The migration is done in steps. The migration is finished when -/// `step()` returns `IsFinished::Yes`. -pub trait MigrationStep: Codec + MaxEncodedLen + Default { - /// Returns the version of the migration. - const VERSION: u16; - - /// Returns the maximum weight that can be consumed in a single step. - fn max_step_weight() -> Weight; - - /// Process one step of the migration. - /// - /// Returns whether the migration is finished. - fn step(&mut self, meter: &mut WeightMeter) -> IsFinished; - - /// Verify that the migration step fits into `Cursor`, and that `max_step_weight` is not greater - /// than `max_block_weight`. - fn integrity_test(max_block_weight: Weight) { - if Self::max_step_weight().any_gt(max_block_weight) { - panic!( - "Invalid max_step_weight for Migration {}. Value should be lower than {}", - Self::VERSION, - max_block_weight - ); - } - - let len = ::max_encoded_len(); - let max = Cursor::bound(); - if len > max { - panic!( - "Migration {} has size {} which is bigger than the maximum of {}", - Self::VERSION, - len, - max, - ); - } - } - - /// Execute some pre-checks prior to running the first step of this migration. - #[cfg(feature = "try-runtime")] - fn pre_upgrade_step() -> Result, TryRuntimeError> { - Ok(Vec::new()) - } - - /// Execute some post-checks after running the last step of this migration. - #[cfg(feature = "try-runtime")] - fn post_upgrade_step(_state: Vec) -> Result<(), TryRuntimeError> { - Ok(()) - } -} - -/// A noop migration that can be used when there is no migration to be done for a given version. -#[doc(hidden)] -#[derive(frame_support::DefaultNoBound, Encode, Decode, MaxEncodedLen)] -pub struct NoopMigration; - -impl MigrationStep for NoopMigration { - const VERSION: u16 = N; - fn max_step_weight() -> Weight { - Weight::zero() - } - fn step(&mut self, _meter: &mut WeightMeter) -> IsFinished { - log::debug!(target: LOG_TARGET, "Noop migration for version {}", N); - IsFinished::Yes - } -} - -mod private { - use crate::migration::MigrationStep; - pub trait Sealed {} - #[impl_trait_for_tuples::impl_for_tuples(10)] - #[tuple_types_custom_trait_bound(MigrationStep)] - impl Sealed for Tuple {} -} - -/// Defines a sequence of migrations. -/// -/// The sequence must be defined by a tuple of migrations, each of which must implement the -/// `MigrationStep` trait. Migrations must be ordered by their versions with no gaps. -pub trait MigrateSequence: private::Sealed { - /// Returns the range of versions that this migrations sequence can handle. - /// Migrations must be ordered by their versions with no gaps. - /// - /// The following code will fail to compile: - /// - /// ```compile_fail - /// # use pallet_revive::{NoopMigration, MigrateSequence}; - /// let _ = <(NoopMigration<1>, NoopMigration<3>)>::VERSION_RANGE; - /// ``` - /// The following code will compile: - /// ``` - /// # use pallet_revive::{NoopMigration, MigrateSequence}; - /// let _ = <(NoopMigration<1>, NoopMigration<2>)>::VERSION_RANGE; - /// ``` - const VERSION_RANGE: (u16, u16); - - /// Returns the default cursor for the given version. - fn new(version: StorageVersion) -> Cursor; - - #[cfg(feature = "try-runtime")] - fn pre_upgrade_step(_version: StorageVersion) -> Result, TryRuntimeError> { - Ok(Vec::new()) - } - - #[cfg(feature = "try-runtime")] - fn post_upgrade_step(_version: StorageVersion, _state: Vec) -> Result<(), TryRuntimeError> { - Ok(()) - } - - /// Execute the migration step until the available weight is consumed. - fn steps(version: StorageVersion, cursor: &[u8], meter: &mut WeightMeter) -> StepResult; - - /// Verify that the migration step fits into `Cursor`, and that `max_step_weight` is not greater - /// than `max_block_weight`. - fn integrity_test(max_block_weight: Weight); - - /// Returns whether migrating from `in_storage` to `target` is supported. - /// - /// A migration is supported if `VERSION_RANGE` is (in_storage + 1, target). - fn is_upgrade_supported(in_storage: StorageVersion, target: StorageVersion) -> bool { - let (low, high) = Self::VERSION_RANGE; - target == high && in_storage + 1 == low - } -} - -/// Performs all necessary migrations based on `StorageVersion`. -/// -/// If `TEST_ALL_STEPS == true` and `try-runtime` is enabled, this will run all the migrations -/// inside `on_runtime_upgrade`. This should be set to false in tests that want to ensure the step -/// by step migration works. -pub struct Migration(PhantomData); - -#[cfg(feature = "try-runtime")] -impl Migration { - fn run_all_steps() -> Result<(), TryRuntimeError> { - let mut meter = &mut WeightMeter::new(); - let name = >::name(); - loop { - let in_progress_version = >::on_chain_storage_version() + 1; - let state = T::Migrations::pre_upgrade_step(in_progress_version)?; - let before = meter.consumed(); - let status = Self::migrate(&mut meter); - log::info!( - target: LOG_TARGET, - "{name}: Migration step {:?} weight = {}", - in_progress_version, - meter.consumed() - before - ); - T::Migrations::post_upgrade_step(in_progress_version, state)?; - if matches!(status, MigrateResult::Completed) { - break - } - } - - let name = >::name(); - log::info!(target: LOG_TARGET, "{name}: Migration steps weight = {}", meter.consumed()); - Ok(()) - } -} - -impl OnRuntimeUpgrade for Migration { - fn on_runtime_upgrade() -> Weight { - let name = >::name(); - let in_code_version = >::in_code_storage_version(); - let on_chain_version = >::on_chain_storage_version(); - - if on_chain_version == in_code_version { - log::warn!( - target: LOG_TARGET, - "{name}: No Migration performed storage_version = latest_version = {:?}", - &on_chain_version - ); - return T::WeightInfo::on_runtime_upgrade_noop() - } - - // In case a migration is already in progress we create the next migration - // (if any) right when the current one finishes. - if Self::in_progress() { - log::warn!( - target: LOG_TARGET, - "{name}: Migration already in progress {:?}", - &on_chain_version - ); - - return T::WeightInfo::on_runtime_upgrade_in_progress() - } - - log::info!( - target: LOG_TARGET, - "{name}: Upgrading storage from {on_chain_version:?} to {in_code_version:?}.", - ); - - let cursor = T::Migrations::new(on_chain_version + 1); - MigrationInProgress::::set(Some(cursor)); - - #[cfg(feature = "try-runtime")] - if TEST_ALL_STEPS { - Self::run_all_steps().unwrap(); - } - - T::WeightInfo::on_runtime_upgrade() - } - - #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, TryRuntimeError> { - // We can't really do much here as our migrations do not happen during the runtime upgrade. - // Instead, we call the migrations `pre_upgrade` and `post_upgrade` hooks when we iterate - // over our migrations. - let on_chain_version = >::on_chain_storage_version(); - let in_code_version = >::in_code_storage_version(); - - if on_chain_version == in_code_version { - return Ok(Default::default()) - } - - log::debug!( - target: LOG_TARGET, - "Requested migration of {} from {:?}(on-chain storage version) to {:?}(in-code storage version)", - >::name(), on_chain_version, in_code_version - ); - - ensure!( - T::Migrations::is_upgrade_supported(on_chain_version, in_code_version), - "Unsupported upgrade: VERSION_RANGE should be (on-chain storage version + 1, in-code storage version)" - ); - - Ok(Default::default()) - } - - #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), TryRuntimeError> { - if !TEST_ALL_STEPS { - return Ok(()) - } - - log::info!(target: LOG_TARGET, "=== POST UPGRADE CHECKS ==="); - - // Ensure that the hashing algorithm is correct for each storage map. - if let Some(hash) = crate::CodeInfoOf::::iter_keys().next() { - crate::CodeInfoOf::::get(hash).expect("CodeInfo exists for hash; qed"); - } - if let Some(hash) = crate::PristineCode::::iter_keys().next() { - crate::PristineCode::::get(hash).expect("PristineCode exists for hash; qed"); - } - if let Some(account_id) = crate::ContractInfoOf::::iter_keys().next() { - crate::ContractInfoOf::::get(account_id) - .expect("ContractInfo exists for account_id; qed"); - } - if let Some(nonce) = crate::DeletionQueue::::iter_keys().next() { - crate::DeletionQueue::::get(nonce).expect("DeletionQueue exists for nonce; qed"); - } - - Ok(()) - } -} - -/// The result of running the migration. -#[derive(Debug, PartialEq)] -pub enum MigrateResult { - /// No migration was performed - NoMigrationPerformed, - /// No migration currently in progress - NoMigrationInProgress, - /// A migration is in progress - InProgress { steps_done: u32 }, - /// All migrations are completed - Completed, -} - -/// The result of running a migration step. -#[derive(Debug, PartialEq)] -pub enum StepResult { - InProgress { cursor: Cursor, steps_done: u32 }, - Completed { steps_done: u32 }, -} - -impl Migration { - /// Verify that each migration's step of the [`Config::Migrations`] sequence fits into - /// `Cursor`. - pub(crate) fn integrity_test() { - let max_weight = ::BlockWeights::get().max_block; - T::Migrations::integrity_test(max_weight) - } - - /// Execute the multi-step migration. - /// Returns whether or not a migration is in progress - pub(crate) fn migrate(mut meter: &mut WeightMeter) -> MigrateResult { - let name = >::name(); - - if meter.try_consume(T::WeightInfo::migrate()).is_err() { - return MigrateResult::NoMigrationPerformed - } - - MigrationInProgress::::mutate_exists(|progress| { - let Some(cursor_before) = progress.as_mut() else { - meter.consume(T::WeightInfo::migration_noop()); - return MigrateResult::NoMigrationInProgress - }; - - // if a migration is running it is always upgrading to the next version - let storage_version = >::on_chain_storage_version(); - let in_progress_version = storage_version + 1; - - log::info!( - target: LOG_TARGET, - "{name}: Migrating from {:?} to {:?},", - storage_version, - in_progress_version, - ); - - let result = - match T::Migrations::steps(in_progress_version, cursor_before.as_ref(), &mut meter) - { - StepResult::InProgress { cursor, steps_done } => { - *progress = Some(cursor); - MigrateResult::InProgress { steps_done } - }, - StepResult::Completed { steps_done } => { - in_progress_version.put::>(); - if >::in_code_storage_version() != in_progress_version { - log::info!( - target: LOG_TARGET, - "{name}: Next migration is {:?},", - in_progress_version + 1 - ); - *progress = Some(T::Migrations::new(in_progress_version + 1)); - MigrateResult::InProgress { steps_done } - } else { - log::info!( - target: LOG_TARGET, - "{name}: All migrations done. At version {:?},", - in_progress_version - ); - *progress = None; - MigrateResult::Completed - } - }, - }; - - result - }) - } - - pub(crate) fn ensure_migrated() -> DispatchResult { - if Self::in_progress() { - Err(Error::::MigrationInProgress.into()) - } else { - Ok(()) - } - } - - pub(crate) fn in_progress() -> bool { - MigrationInProgress::::exists() - } -} - -#[impl_trait_for_tuples::impl_for_tuples(10)] -#[tuple_types_custom_trait_bound(MigrationStep)] -impl MigrateSequence for Tuple { - const VERSION_RANGE: (u16, u16) = { - let mut versions: (u16, u16) = (0, 0); - for_tuples!( - #( - match versions { - (0, 0) => { - versions = (Tuple::VERSION, Tuple::VERSION); - }, - (min_version, last_version) if Tuple::VERSION == last_version + 1 => { - versions = (min_version, Tuple::VERSION); - }, - _ => panic!("Migrations must be ordered by their versions with no gaps.") - } - )* - ); - versions - }; - - fn new(version: StorageVersion) -> Cursor { - for_tuples!( - #( - if version == Tuple::VERSION { - return Tuple::default().encode().try_into().expect(PROOF_ENCODE) - } - )* - ); - invalid_version(version) - } - - #[cfg(feature = "try-runtime")] - /// Execute the pre-checks of the step associated with this version. - fn pre_upgrade_step(version: StorageVersion) -> Result, TryRuntimeError> { - for_tuples!( - #( - if version == Tuple::VERSION { - return Tuple::pre_upgrade_step() - } - )* - ); - invalid_version(version) - } - - #[cfg(feature = "try-runtime")] - /// Execute the post-checks of the step associated with this version. - fn post_upgrade_step(version: StorageVersion, state: Vec) -> Result<(), TryRuntimeError> { - for_tuples!( - #( - if version == Tuple::VERSION { - return Tuple::post_upgrade_step(state) - } - )* - ); - invalid_version(version) - } - - fn steps(version: StorageVersion, mut cursor: &[u8], meter: &mut WeightMeter) -> StepResult { - for_tuples!( - #( - if version == Tuple::VERSION { - let mut migration = ::decode(&mut cursor) - .expect(PROOF_DECODE); - let max_weight = Tuple::max_step_weight(); - let mut steps_done = 0; - while meter.can_consume(max_weight) { - steps_done.saturating_accrue(1); - if matches!(migration.step(meter), IsFinished::Yes) { - return StepResult::Completed{ steps_done } - } - } - return StepResult::InProgress{cursor: migration.encode().try_into().expect(PROOF_ENCODE), steps_done } - } - )* - ); - invalid_version(version) - } - - fn integrity_test(max_block_weight: Weight) { - for_tuples!( - #( - Tuple::integrity_test(max_block_weight); - )* - ); - } -} - -#[cfg(test)] -mod test { - use super::*; - use crate::{ - migration::codegen::LATEST_MIGRATION_VERSION, - tests::{ExtBuilder, Test}, - }; - - #[derive(Default, Encode, Decode, MaxEncodedLen)] - struct MockMigration { - // MockMigration needs `N` steps to finish - count: u16, - } - - impl MigrationStep for MockMigration { - const VERSION: u16 = N; - fn max_step_weight() -> Weight { - Weight::from_all(1) - } - fn step(&mut self, meter: &mut WeightMeter) -> IsFinished { - assert!(self.count != N); - self.count += 1; - meter.consume(Weight::from_all(1)); - if self.count == N { - IsFinished::Yes - } else { - IsFinished::No - } - } - } - - #[test] - fn test_storage_version_matches_last_migration_file() { - assert_eq!(StorageVersion::new(LATEST_MIGRATION_VERSION), crate::pallet::STORAGE_VERSION); - } - - #[test] - fn version_range_works() { - let range = <(MockMigration<1>, MockMigration<2>)>::VERSION_RANGE; - assert_eq!(range, (1, 2)); - } - - #[test] - fn is_upgrade_supported_works() { - type Migrations = (MockMigration<9>, MockMigration<10>, MockMigration<11>); - assert!(Migrations::is_upgrade_supported(StorageVersion::new(8), StorageVersion::new(11))); - assert!(!Migrations::is_upgrade_supported(StorageVersion::new(9), StorageVersion::new(11))); - assert!(!Migrations::is_upgrade_supported(StorageVersion::new(8), StorageVersion::new(12))); - } - - #[test] - fn steps_works() { - type Migrations = (MockMigration<2>, MockMigration<3>); - let version = StorageVersion::new(2); - let mut cursor = Migrations::new(version); - - let mut meter = WeightMeter::with_limit(Weight::from_all(1)); - let result = Migrations::steps(version, &cursor, &mut meter); - cursor = alloc::vec![1u8, 0].try_into().unwrap(); - assert_eq!(result, StepResult::InProgress { cursor: cursor.clone(), steps_done: 1 }); - assert_eq!(meter.consumed(), Weight::from_all(1)); - - let mut meter = WeightMeter::with_limit(Weight::from_all(1)); - assert_eq!( - Migrations::steps(version, &cursor, &mut meter), - StepResult::Completed { steps_done: 1 } - ); - } - - #[test] - fn no_migration_in_progress_works() { - type TestMigration = Migration; - - ExtBuilder::default().build().execute_with(|| { - assert_eq!(StorageVersion::get::>(), LATEST_MIGRATION_VERSION); - assert_eq!( - TestMigration::migrate(&mut WeightMeter::new()), - MigrateResult::NoMigrationInProgress - ) - }); - } - - #[test] - fn migration_works() { - type TestMigration = Migration; - - ExtBuilder::default() - .set_storage_version(LATEST_MIGRATION_VERSION - 2) - .build() - .execute_with(|| { - assert_eq!(StorageVersion::get::>(), LATEST_MIGRATION_VERSION - 2); - TestMigration::on_runtime_upgrade(); - for (version, status) in [ - (LATEST_MIGRATION_VERSION - 1, MigrateResult::InProgress { steps_done: 1 }), - (LATEST_MIGRATION_VERSION, MigrateResult::Completed), - ] { - assert_eq!(TestMigration::migrate(&mut WeightMeter::new()), status); - assert_eq!( - >::on_chain_storage_version(), - StorageVersion::new(version) - ); - } - - assert_eq!( - TestMigration::migrate(&mut WeightMeter::new()), - MigrateResult::NoMigrationInProgress - ); - assert_eq!(StorageVersion::get::>(), LATEST_MIGRATION_VERSION); - }); - } -} diff --git a/substrate/frame/revive/src/primitives.rs b/substrate/frame/revive/src/primitives.rs index 98e8879457bf..1b48527d23d7 100644 --- a/substrate/frame/revive/src/primitives.rs +++ b/substrate/frame/revive/src/primitives.rs @@ -103,8 +103,6 @@ pub enum ContractAccessError { DoesntExist, /// Storage key cannot be decoded from the provided input data. KeyDecodingFailed, - /// Storage is migrating. Try again later. - MigrationInProgress, } /// Output of a contract call or instantiation which ran to completion. diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index fb168022013c..f2944c7932a6 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -33,7 +33,6 @@ use crate::{ }, exec::Key, limits, - migration::codegen::LATEST_MIGRATION_VERSION, primitives::CodeUploadReturnValue, storage::DeletionQueueManager, test_utils::*, @@ -41,8 +40,8 @@ use crate::{ wasm::Memory, weights::WeightInfo, BalanceOf, Code, CodeInfoOf, CollectEvents, Config, ContractInfo, ContractInfoOf, DebugInfo, - DefaultAddressMapper, DeletionQueueCounter, Error, HoldReason, MigrationInProgress, Origin, - Pallet, PristineCode, H160, + DefaultAddressMapper, DeletionQueueCounter, Error, HoldReason, Origin, Pallet, PristineCode, + H160, }; use crate::test_utils::builder::Contract; @@ -490,7 +489,6 @@ impl Config for Test { type UnsafeUnstableInterface = UnstableInterface; type UploadOrigin = EnsureAccount; type InstantiateOrigin = EnsureAccount; - type Migrations = crate::migration::codegen::BenchMigrations; type CodeHashLockupDepositPercent = CodeHashLockupDepositPercent; type Debug = TestDebug; } @@ -523,10 +521,6 @@ impl ExtBuilder { pub fn set_associated_consts(&self) { EXISTENTIAL_DEPOSIT.with(|v| *v.borrow_mut() = self.existential_deposit); } - pub fn set_storage_version(mut self, version: u16) -> Self { - self.storage_version = Some(StorageVersion::new(version)); - self - } pub fn build(self) -> sp_io::TestExternalities { sp_tracing::try_init_simple(); self.set_associated_consts(); @@ -617,66 +611,6 @@ mod run_tests { }); } - #[test] - fn migration_on_idle_hooks_works() { - // Defines expectations of how many migration steps can be done given the weight limit. - let tests = [ - (Weight::zero(), LATEST_MIGRATION_VERSION - 2), - (::WeightInfo::migrate() + 1.into(), LATEST_MIGRATION_VERSION - 1), - (Weight::MAX, LATEST_MIGRATION_VERSION), - ]; - - for (weight, expected_version) in tests { - ExtBuilder::default() - .set_storage_version(LATEST_MIGRATION_VERSION - 2) - .build() - .execute_with(|| { - MigrationInProgress::::set(Some(Default::default())); - Contracts::on_idle(System::block_number(), weight); - assert_eq!(StorageVersion::get::>(), expected_version); - }); - } - } - - #[test] - fn migration_in_progress_works() { - let (wasm, code_hash) = compile_module("dummy").unwrap(); - - ExtBuilder::default().existential_deposit(1).build().execute_with(|| { - let _ = ::Currency::set_balance(&ALICE, 1_000_000); - MigrationInProgress::::set(Some(Default::default())); - - assert_err!( - Contracts::upload_code( - RuntimeOrigin::signed(ALICE), - vec![], - deposit_limit::(), - ), - Error::::MigrationInProgress, - ); - assert_err!( - Contracts::remove_code(RuntimeOrigin::signed(ALICE), code_hash), - Error::::MigrationInProgress, - ); - assert_err!( - Contracts::set_code(RuntimeOrigin::signed(ALICE), BOB_ADDR, code_hash), - Error::::MigrationInProgress, - ); - assert_err_ignore_postinfo!( - builder::call(BOB_ADDR).build(), - Error::::MigrationInProgress - ); - assert_err_ignore_postinfo!( - builder::instantiate_with_code(wasm).value(100_000).build(), - Error::::MigrationInProgress, - ); - assert_err_ignore_postinfo!( - builder::instantiate(code_hash).value(100_000).build(), - Error::::MigrationInProgress, - ); - }); - } - #[test] fn instantiate_and_call_and_deposit_event() { let (wasm, code_hash) = compile_module("event_and_return_on_deploy").unwrap(); diff --git a/substrate/frame/revive/src/weights.rs b/substrate/frame/revive/src/weights.rs index 7974cc1260e4..8913592c13bb 100644 --- a/substrate/frame/revive/src/weights.rs +++ b/substrate/frame/revive/src/weights.rs @@ -51,19 +51,6 @@ use core::marker::PhantomData; pub trait WeightInfo { fn on_process_deletion_queue_batch() -> Weight; fn on_initialize_per_trie_key(k: u32, ) -> Weight; - fn v9_migration_step(c: u32, ) -> Weight; - fn v10_migration_step() -> Weight; - fn v11_migration_step(k: u32, ) -> Weight; - fn v12_migration_step(c: u32, ) -> Weight; - fn v13_migration_step() -> Weight; - fn v14_migration_step() -> Weight; - fn v15_migration_step() -> Weight; - fn v16_migration_step() -> Weight; - fn migration_noop() -> Weight; - fn migrate() -> Weight; - fn on_runtime_upgrade_noop() -> Weight; - fn on_runtime_upgrade_in_progress() -> Weight; - fn on_runtime_upgrade() -> Weight; fn call_with_code_per_byte(c: u32, ) -> Weight; fn instantiate_with_code(c: u32, i: u32) -> Weight; fn instantiate(i: u32) -> Weight; @@ -162,182 +149,6 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(k.into()))) .saturating_add(Weight::from_parts(0, 70).saturating_mul(k.into())) } - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc553022fca90611ba8b7942f8bdb3b97f6580` (r:2 w:1) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc553022fca90611ba8b7942f8bdb3b97f6580` (r:2 w:1) - /// The range of component `c` is `[0, 125952]`. - fn v9_migration_step(c: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `211 + c * (1 ±0)` - // Estimated: `6149 + c * (1 ±0)` - // Minimum execution time: 7_783_000 picoseconds. - Weight::from_parts(4_462_075, 6149) - // Standard Error: 5 - .saturating_add(Weight::from_parts(1_634, 0).saturating_mul(c.into())) - .saturating_add(T::DbWeight::get().reads(2_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) - .saturating_add(Weight::from_parts(0, 1).saturating_mul(c.into())) - } - /// Storage: `Contracts::ContractInfoOf` (r:2 w:1) - /// Proof: `Contracts::ContractInfoOf` (`max_values`: None, `max_size`: Some(1795), added: 4270, mode: `Measured`) - /// Storage: `System::Account` (r:1 w:0) - /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `Measured`) - fn v10_migration_step() -> Weight { - // Proof Size summary in bytes: - // Measured: `510` - // Estimated: `6450` - // Minimum execution time: 15_971_000 picoseconds. - Weight::from_parts(16_730_000, 6450) - .saturating_add(T::DbWeight::get().reads(3_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) - } - /// Storage: `Contracts::DeletionQueue` (r:1 w:1025) - /// Proof: `Contracts::DeletionQueue` (`max_values`: None, `max_size`: Some(142), added: 2617, mode: `Measured`) - /// Storage: `Contracts::DeletionQueueCounter` (r:0 w:1) - /// Proof: `Contracts::DeletionQueueCounter` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `Measured`) - /// The range of component `k` is `[0, 1024]`. - fn v11_migration_step(k: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `171 + k * (1 ±0)` - // Estimated: `3635 + k * (1 ±0)` - // Minimum execution time: 3_149_000 picoseconds. - Weight::from_parts(3_264_000, 3635) - // Standard Error: 559 - .saturating_add(Weight::from_parts(1_111_209, 0).saturating_mul(k.into())) - .saturating_add(T::DbWeight::get().reads(1_u64)) - .saturating_add(T::DbWeight::get().writes(2_u64)) - .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(k.into()))) - .saturating_add(Weight::from_parts(0, 1).saturating_mul(k.into())) - } - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc553053f13fd319a03c211337c76e0fe776df` (r:2 w:0) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc553053f13fd319a03c211337c76e0fe776df` (r:2 w:0) - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc553022fca90611ba8b7942f8bdb3b97f6580` (r:1 w:1) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc553022fca90611ba8b7942f8bdb3b97f6580` (r:1 w:1) - /// Storage: `System::Account` (r:1 w:0) - /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `Measured`) - /// Storage: `Contracts::CodeInfoOf` (r:0 w:1) - /// Proof: `Contracts::CodeInfoOf` (`max_values`: None, `max_size`: Some(93), added: 2568, mode: `Measured`) - /// The range of component `c` is `[0, 125952]`. - fn v12_migration_step(c: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `325 + c * (1 ±0)` - // Estimated: `6263 + c * (1 ±0)` - // Minimum execution time: 15_072_000 picoseconds. - Weight::from_parts(15_721_891, 6263) - // Standard Error: 2 - .saturating_add(Weight::from_parts(428, 0).saturating_mul(c.into())) - .saturating_add(T::DbWeight::get().reads(4_u64)) - .saturating_add(T::DbWeight::get().writes(2_u64)) - .saturating_add(Weight::from_parts(0, 1).saturating_mul(c.into())) - } - /// Storage: `Contracts::ContractInfoOf` (r:2 w:1) - /// Proof: `Contracts::ContractInfoOf` (`max_values`: None, `max_size`: Some(1795), added: 4270, mode: `Measured`) - fn v13_migration_step() -> Weight { - // Proof Size summary in bytes: - // Measured: `440` - // Estimated: `6380` - // Minimum execution time: 12_047_000 picoseconds. - Weight::from_parts(12_500_000, 6380) - .saturating_add(T::DbWeight::get().reads(2_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) - } - /// Storage: `Contracts::CodeInfoOf` (r:2 w:0) - /// Proof: `Contracts::CodeInfoOf` (`max_values`: None, `max_size`: Some(93), added: 2568, mode: `Measured`) - /// Storage: `System::Account` (r:1 w:1) - /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `Measured`) - /// Storage: `Balances::Holds` (r:1 w:0) - /// Proof: `Balances::Holds` (`max_values`: None, `max_size`: Some(193), added: 2668, mode: `Measured`) - fn v14_migration_step() -> Weight { - // Proof Size summary in bytes: - // Measured: `352` - // Estimated: `6292` - // Minimum execution time: 47_488_000 picoseconds. - Weight::from_parts(48_482_000, 6292) - .saturating_add(T::DbWeight::get().reads(4_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) - } - /// Storage: `Contracts::ContractInfoOf` (r:2 w:1) - /// Proof: `Contracts::ContractInfoOf` (`max_values`: None, `max_size`: Some(1795), added: 4270, mode: `Measured`) - /// Storage: `System::Account` (r:2 w:1) - /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `Measured`) - fn v15_migration_step() -> Weight { - // Proof Size summary in bytes: - // Measured: `594` - // Estimated: `6534` - // Minimum execution time: 52_801_000 picoseconds. - Weight::from_parts(54_230_000, 6534) - .saturating_add(T::DbWeight::get().reads(4_u64)) - .saturating_add(T::DbWeight::get().writes(2_u64)) - } - /// Storage: `Contracts::ContractInfoOf` (r:2 w:1) - /// Proof: `Contracts::ContractInfoOf` (`max_values`: None, `max_size`: Some(1795), added: 4270, mode: `Measured`) - fn v16_migration_step() -> Weight { - // Proof Size summary in bytes: - // Measured: `409` - // Estimated: `6349` - // Minimum execution time: 11_618_000 picoseconds. - Weight::from_parts(12_068_000, 6349) - .saturating_add(T::DbWeight::get().reads(2_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) - } - /// Storage: `Contracts::MigrationInProgress` (r:1 w:1) - /// Proof: `Contracts::MigrationInProgress` (`max_values`: Some(1), `max_size`: Some(1026), added: 1521, mode: `Measured`) - fn migration_noop() -> Weight { - // Proof Size summary in bytes: - // Measured: `142` - // Estimated: `1627` - // Minimum execution time: 2_131_000 picoseconds. - Weight::from_parts(2_255_000, 1627) - .saturating_add(T::DbWeight::get().reads(1_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) - } - /// Storage: `Contracts::MigrationInProgress` (r:1 w:1) - /// Proof: `Contracts::MigrationInProgress` (`max_values`: Some(1), `max_size`: Some(1026), added: 1521, mode: `Measured`) - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:1) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:1) - fn migrate() -> Weight { - // Proof Size summary in bytes: - // Measured: `166` - // Estimated: `3631` - // Minimum execution time: 10_773_000 picoseconds. - Weight::from_parts(11_118_000, 3631) - .saturating_add(T::DbWeight::get().reads(2_u64)) - .saturating_add(T::DbWeight::get().writes(2_u64)) - } - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:0) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:0) - fn on_runtime_upgrade_noop() -> Weight { - // Proof Size summary in bytes: - // Measured: `142` - // Estimated: `3607` - // Minimum execution time: 4_371_000 picoseconds. - Weight::from_parts(4_624_000, 3607) - .saturating_add(T::DbWeight::get().reads(1_u64)) - } - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:0) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:0) - /// Storage: `Contracts::MigrationInProgress` (r:1 w:0) - /// Proof: `Contracts::MigrationInProgress` (`max_values`: Some(1), `max_size`: Some(1026), added: 1521, mode: `Measured`) - fn on_runtime_upgrade_in_progress() -> Weight { - // Proof Size summary in bytes: - // Measured: `167` - // Estimated: `3632` - // Minimum execution time: 5_612_000 picoseconds. - Weight::from_parts(5_838_000, 3632) - .saturating_add(T::DbWeight::get().reads(2_u64)) - } - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:0) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:0) - /// Storage: `Contracts::MigrationInProgress` (r:1 w:1) - /// Proof: `Contracts::MigrationInProgress` (`max_values`: Some(1), `max_size`: Some(1026), added: 1521, mode: `Measured`) - fn on_runtime_upgrade() -> Weight { - // Proof Size summary in bytes: - // Measured: `142` - // Estimated: `3607` - // Minimum execution time: 5_487_000 picoseconds. - Weight::from_parts(5_693_000, 3607) - .saturating_add(T::DbWeight::get().reads(2_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) - } /// Storage: `Contracts::MigrationInProgress` (r:1 w:0) /// Proof: `Contracts::MigrationInProgress` (`max_values`: Some(1), `max_size`: Some(1026), added: 1521, mode: `Measured`) /// Storage: `Contracts::ContractInfoOf` (r:1 w:1) @@ -1152,182 +963,6 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(k.into()))) .saturating_add(Weight::from_parts(0, 70).saturating_mul(k.into())) } - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc553022fca90611ba8b7942f8bdb3b97f6580` (r:2 w:1) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc553022fca90611ba8b7942f8bdb3b97f6580` (r:2 w:1) - /// The range of component `c` is `[0, 125952]`. - fn v9_migration_step(c: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `211 + c * (1 ±0)` - // Estimated: `6149 + c * (1 ±0)` - // Minimum execution time: 7_783_000 picoseconds. - Weight::from_parts(4_462_075, 6149) - // Standard Error: 5 - .saturating_add(Weight::from_parts(1_634, 0).saturating_mul(c.into())) - .saturating_add(RocksDbWeight::get().reads(2_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) - .saturating_add(Weight::from_parts(0, 1).saturating_mul(c.into())) - } - /// Storage: `Contracts::ContractInfoOf` (r:2 w:1) - /// Proof: `Contracts::ContractInfoOf` (`max_values`: None, `max_size`: Some(1795), added: 4270, mode: `Measured`) - /// Storage: `System::Account` (r:1 w:0) - /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `Measured`) - fn v10_migration_step() -> Weight { - // Proof Size summary in bytes: - // Measured: `510` - // Estimated: `6450` - // Minimum execution time: 15_971_000 picoseconds. - Weight::from_parts(16_730_000, 6450) - .saturating_add(RocksDbWeight::get().reads(3_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) - } - /// Storage: `Contracts::DeletionQueue` (r:1 w:1025) - /// Proof: `Contracts::DeletionQueue` (`max_values`: None, `max_size`: Some(142), added: 2617, mode: `Measured`) - /// Storage: `Contracts::DeletionQueueCounter` (r:0 w:1) - /// Proof: `Contracts::DeletionQueueCounter` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `Measured`) - /// The range of component `k` is `[0, 1024]`. - fn v11_migration_step(k: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `171 + k * (1 ±0)` - // Estimated: `3635 + k * (1 ±0)` - // Minimum execution time: 3_149_000 picoseconds. - Weight::from_parts(3_264_000, 3635) - // Standard Error: 559 - .saturating_add(Weight::from_parts(1_111_209, 0).saturating_mul(k.into())) - .saturating_add(RocksDbWeight::get().reads(1_u64)) - .saturating_add(RocksDbWeight::get().writes(2_u64)) - .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(k.into()))) - .saturating_add(Weight::from_parts(0, 1).saturating_mul(k.into())) - } - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc553053f13fd319a03c211337c76e0fe776df` (r:2 w:0) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc553053f13fd319a03c211337c76e0fe776df` (r:2 w:0) - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc553022fca90611ba8b7942f8bdb3b97f6580` (r:1 w:1) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc553022fca90611ba8b7942f8bdb3b97f6580` (r:1 w:1) - /// Storage: `System::Account` (r:1 w:0) - /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `Measured`) - /// Storage: `Contracts::CodeInfoOf` (r:0 w:1) - /// Proof: `Contracts::CodeInfoOf` (`max_values`: None, `max_size`: Some(93), added: 2568, mode: `Measured`) - /// The range of component `c` is `[0, 125952]`. - fn v12_migration_step(c: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `325 + c * (1 ±0)` - // Estimated: `6263 + c * (1 ±0)` - // Minimum execution time: 15_072_000 picoseconds. - Weight::from_parts(15_721_891, 6263) - // Standard Error: 2 - .saturating_add(Weight::from_parts(428, 0).saturating_mul(c.into())) - .saturating_add(RocksDbWeight::get().reads(4_u64)) - .saturating_add(RocksDbWeight::get().writes(2_u64)) - .saturating_add(Weight::from_parts(0, 1).saturating_mul(c.into())) - } - /// Storage: `Contracts::ContractInfoOf` (r:2 w:1) - /// Proof: `Contracts::ContractInfoOf` (`max_values`: None, `max_size`: Some(1795), added: 4270, mode: `Measured`) - fn v13_migration_step() -> Weight { - // Proof Size summary in bytes: - // Measured: `440` - // Estimated: `6380` - // Minimum execution time: 12_047_000 picoseconds. - Weight::from_parts(12_500_000, 6380) - .saturating_add(RocksDbWeight::get().reads(2_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) - } - /// Storage: `Contracts::CodeInfoOf` (r:2 w:0) - /// Proof: `Contracts::CodeInfoOf` (`max_values`: None, `max_size`: Some(93), added: 2568, mode: `Measured`) - /// Storage: `System::Account` (r:1 w:1) - /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `Measured`) - /// Storage: `Balances::Holds` (r:1 w:0) - /// Proof: `Balances::Holds` (`max_values`: None, `max_size`: Some(193), added: 2668, mode: `Measured`) - fn v14_migration_step() -> Weight { - // Proof Size summary in bytes: - // Measured: `352` - // Estimated: `6292` - // Minimum execution time: 47_488_000 picoseconds. - Weight::from_parts(48_482_000, 6292) - .saturating_add(RocksDbWeight::get().reads(4_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) - } - /// Storage: `Contracts::ContractInfoOf` (r:2 w:1) - /// Proof: `Contracts::ContractInfoOf` (`max_values`: None, `max_size`: Some(1795), added: 4270, mode: `Measured`) - /// Storage: `System::Account` (r:2 w:1) - /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `Measured`) - fn v15_migration_step() -> Weight { - // Proof Size summary in bytes: - // Measured: `594` - // Estimated: `6534` - // Minimum execution time: 52_801_000 picoseconds. - Weight::from_parts(54_230_000, 6534) - .saturating_add(RocksDbWeight::get().reads(4_u64)) - .saturating_add(RocksDbWeight::get().writes(2_u64)) - } - /// Storage: `Contracts::ContractInfoOf` (r:2 w:1) - /// Proof: `Contracts::ContractInfoOf` (`max_values`: None, `max_size`: Some(1795), added: 4270, mode: `Measured`) - fn v16_migration_step() -> Weight { - // Proof Size summary in bytes: - // Measured: `409` - // Estimated: `6349` - // Minimum execution time: 11_618_000 picoseconds. - Weight::from_parts(12_068_000, 6349) - .saturating_add(RocksDbWeight::get().reads(2_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) - } - /// Storage: `Contracts::MigrationInProgress` (r:1 w:1) - /// Proof: `Contracts::MigrationInProgress` (`max_values`: Some(1), `max_size`: Some(1026), added: 1521, mode: `Measured`) - fn migration_noop() -> Weight { - // Proof Size summary in bytes: - // Measured: `142` - // Estimated: `1627` - // Minimum execution time: 2_131_000 picoseconds. - Weight::from_parts(2_255_000, 1627) - .saturating_add(RocksDbWeight::get().reads(1_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) - } - /// Storage: `Contracts::MigrationInProgress` (r:1 w:1) - /// Proof: `Contracts::MigrationInProgress` (`max_values`: Some(1), `max_size`: Some(1026), added: 1521, mode: `Measured`) - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:1) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:1) - fn migrate() -> Weight { - // Proof Size summary in bytes: - // Measured: `166` - // Estimated: `3631` - // Minimum execution time: 10_773_000 picoseconds. - Weight::from_parts(11_118_000, 3631) - .saturating_add(RocksDbWeight::get().reads(2_u64)) - .saturating_add(RocksDbWeight::get().writes(2_u64)) - } - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:0) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:0) - fn on_runtime_upgrade_noop() -> Weight { - // Proof Size summary in bytes: - // Measured: `142` - // Estimated: `3607` - // Minimum execution time: 4_371_000 picoseconds. - Weight::from_parts(4_624_000, 3607) - .saturating_add(RocksDbWeight::get().reads(1_u64)) - } - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:0) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:0) - /// Storage: `Contracts::MigrationInProgress` (r:1 w:0) - /// Proof: `Contracts::MigrationInProgress` (`max_values`: Some(1), `max_size`: Some(1026), added: 1521, mode: `Measured`) - fn on_runtime_upgrade_in_progress() -> Weight { - // Proof Size summary in bytes: - // Measured: `167` - // Estimated: `3632` - // Minimum execution time: 5_612_000 picoseconds. - Weight::from_parts(5_838_000, 3632) - .saturating_add(RocksDbWeight::get().reads(2_u64)) - } - /// Storage: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:0) - /// Proof: UNKNOWN KEY `0x4342193e496fab7ec59d615ed0dc55304e7b9012096b41c4eb3aaf947f6ea429` (r:1 w:0) - /// Storage: `Contracts::MigrationInProgress` (r:1 w:1) - /// Proof: `Contracts::MigrationInProgress` (`max_values`: Some(1), `max_size`: Some(1026), added: 1521, mode: `Measured`) - fn on_runtime_upgrade() -> Weight { - // Proof Size summary in bytes: - // Measured: `142` - // Estimated: `3607` - // Minimum execution time: 5_487_000 picoseconds. - Weight::from_parts(5_693_000, 3607) - .saturating_add(RocksDbWeight::get().reads(2_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) - } /// Storage: `Contracts::MigrationInProgress` (r:1 w:0) /// Proof: `Contracts::MigrationInProgress` (`max_values`: Some(1), `max_size`: Some(1026), added: 1521, mode: `Measured`) /// Storage: `Contracts::ContractInfoOf` (r:1 w:1) From c8ae6cac93ff775f143f6e11ebf62a9588f711be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Sun, 8 Sep 2024 22:26:13 +0200 Subject: [PATCH 21/21] Remove revive migration from runtime --- substrate/bin/node/runtime/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index f18e446f9701..caebd63408db 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -2588,7 +2588,6 @@ type Migrations = ( pallet_nomination_pools::migration::versioned::V6ToV7, pallet_alliance::migration::Migration, pallet_contracts::Migration, - pallet_revive::Migration, pallet_identity::migration::versioned::V0ToV1, );