From 406d782fab86135a0f4c6de00137ccf3696a7a42 Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Thu, 28 Oct 2021 15:25:55 +0800 Subject: [PATCH 01/13] update comment --- modules/idle-scheduler/src/lib.rs | 5 ++--- modules/idle-scheduler/src/mock.rs | 2 +- modules/idle-scheduler/src/tests.rs | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/modules/idle-scheduler/src/lib.rs b/modules/idle-scheduler/src/lib.rs index ca9923b4e..2ae64520a 100644 --- a/modules/idle-scheduler/src/lib.rs +++ b/modules/idle-scheduler/src/lib.rs @@ -16,10 +16,9 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -//! # Example Module +//! # Idle scheduler Module //! -//! A simple example of a FRAME pallet demonstrating -//! concepts, APIs and structures common to most FRAME runtimes. +//! Allow pallets and chain maintainer to schedule a task to be dispatched when chain is idle. #![cfg_attr(not(feature = "std"), no_std)] #![allow(clippy::unused_unit)] diff --git a/modules/idle-scheduler/src/mock.rs b/modules/idle-scheduler/src/mock.rs index 8a215153e..668bab401 100644 --- a/modules/idle-scheduler/src/mock.rs +++ b/modules/idle-scheduler/src/mock.rs @@ -16,7 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -//! Mocks for example module. +//! Mocks for idle-scheduler module. #![cfg(test)] diff --git a/modules/idle-scheduler/src/tests.rs b/modules/idle-scheduler/src/tests.rs index e7728bcb6..e6072eb18 100644 --- a/modules/idle-scheduler/src/tests.rs +++ b/modules/idle-scheduler/src/tests.rs @@ -16,7 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -//! Unit tests for example module. +//! Unit tests for idle-scheduler module. #![cfg(test)] From 501dd53f8f854bce77678cd7daf4f9193b02a586 Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Thu, 28 Oct 2021 15:56:38 +0800 Subject: [PATCH 02/13] check task id overflow --- modules/idle-scheduler/src/lib.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/modules/idle-scheduler/src/lib.rs b/modules/idle-scheduler/src/lib.rs index 2ae64520a..31eeeaf2a 100644 --- a/modules/idle-scheduler/src/lib.rs +++ b/modules/idle-scheduler/src/lib.rs @@ -31,7 +31,10 @@ use codec::{Codec, EncodeLike}; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; use scale_info::TypeInfo; -use sp_runtime::traits::{One, Zero}; +use sp_runtime::{ + traits::{One, Zero}, + ArithmeticError, +}; use sp_std::{cmp::PartialEq, fmt::Debug, prelude::*}; mod mock; @@ -91,25 +94,25 @@ pub mod module { #[pallet::weight(< T as Config >::WeightInfo::schedule_task())] pub fn schedule_task(origin: OriginFor, task: T::Task) -> DispatchResult { ensure_root(origin)?; - Self::do_schedule_task(task); - Ok(()) + Self::do_schedule_task(task) } } } impl Pallet { /// Add the task to the queue to be dispatched later - fn do_schedule_task(task: T::Task) { - let id = Self::get_next_task_id(); + fn do_schedule_task(task: T::Task) -> DispatchResult { + let id = Self::get_next_task_id()?; Tasks::::insert(id, task); + Ok(()) } /// Retrieves the next task ID from storage, and increment it by one. - fn get_next_task_id() -> Nonce { - NextTaskId::::mutate(|current| { + fn get_next_task_id() -> Result { + NextTaskId::::mutate(|current| -> Result { let id = *current; - *current = current.saturating_add(One::one()); - id + *current = current.checked_add(One::one()).ok_or(ArithmeticError::Overflow)?; + Ok(id) }) } From 2b8523c93ffb84496be306b10ad87a82a88254f8 Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Wed, 3 Nov 2021 11:30:57 +0800 Subject: [PATCH 03/13] use idle-scheduler to remove contracts --- Cargo.lock | 1 + modules/evm/Cargo.toml | 1 + modules/evm/src/lib.rs | 177 +++++++++++++++++++++------- modules/evm/src/mock.rs | 23 +++- modules/evm/src/runner/mod.rs | 1 + modules/evm/src/runner/stack.rs | 24 ++-- modules/evm/src/runner/state.rs | 9 +- modules/idle-scheduler/src/lib.rs | 24 ++-- modules/idle-scheduler/src/mock.rs | 6 +- modules/idle-scheduler/src/tests.rs | 2 +- primitives/src/task.rs | 23 ++-- runtime/mandala/src/lib.rs | 7 +- 12 files changed, 217 insertions(+), 81 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5357a479e..2458013d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5394,6 +5394,7 @@ dependencies = [ "hex-literal 0.3.3", "impl-trait-for-tuples 0.1.3", "module-evm-utiltity", + "module-idle-scheduler", "module-support", "orml-currencies", "orml-tokens", diff --git a/modules/evm/Cargo.toml b/modules/evm/Cargo.toml index b4359d686..071f85871 100644 --- a/modules/evm/Cargo.toml +++ b/modules/evm/Cargo.toml @@ -35,6 +35,7 @@ env_logger = "0.7" pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.12" } orml-currencies = { path = "../../orml/currencies" } orml-tokens = { path = "../../orml/tokens" } +module-idle-scheduler = { path = "../idle-scheduler" } [features] default = ["std"] diff --git a/modules/evm/src/lib.rs b/modules/evm/src/lib.rs index adc632041..5a4832b34 100644 --- a/modules/evm/src/lib.rs +++ b/modules/evm/src/lib.rs @@ -31,7 +31,7 @@ pub use crate::{ Runner, }, }; -use codec::{Decode, Encode, MaxEncodedLen}; +use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use frame_support::{ dispatch::{DispatchError, DispatchResult, DispatchResultWithPostInfo}, ensure, @@ -61,6 +61,7 @@ pub use orml_traits::currency::TransferAll; use primitive_types::{H160, H256, U256}; pub use primitives::{ evm::{CallInfo, CreateInfo, EvmAddress, ExecutionInfo, Vicinity}, + task::{DispatchableTask, IdleScheduler, TaskResult}, ReserveIdentifier, H160_PREFIX_DEXSHARE, H160_PREFIX_TOKEN, MIRRORED_NFT_ADDRESS_START, PRECOMPILE_ADDRESS_START, SYSTEM_CONTRACT_ADDRESS_PREFIX, }; @@ -68,6 +69,7 @@ use scale_info::TypeInfo; #[cfg(feature = "std")] use serde::{Deserialize, Serialize}; use sha3::{Digest, Keccak256}; +use sp_io::KillStorageResult::{AllRemoved, SomeRemaining}; use sp_runtime::{ traits::{ Convert, DispatchInfoOf, One, PostDispatchInfoOf, Saturating, SignedExtension, UniqueSaturatedInto, Zero, @@ -75,7 +77,14 @@ use sp_runtime::{ transaction_validity::TransactionValidityError, Either, TransactionOutcome, }; -use sp_std::{collections::btree_map::BTreeMap, convert::TryInto, fmt::Write, marker::PhantomData, prelude::*}; +use sp_std::{ + cmp, + collections::btree_map::BTreeMap, + convert::TryInto, + fmt::{Debug, Write}, + marker::PhantomData, + prelude::*, +}; pub mod precompiles; pub mod runner; @@ -220,6 +229,12 @@ pub mod module { /// Find author for the current block. type FindAuthor: FindAuthor; + /// Dispatchable tasks + type Task: DispatchableTask + FullCodec + Debug + Clone + PartialEq + TypeInfo + From>; + + /// Idle scheduler for the evm task. + type IdleScheduler: IdleScheduler; + /// Weight information for the extrinsics in this module. type WeightInfo: WeightInfo; } @@ -832,8 +847,8 @@ pub mod module { #[transactional] pub fn selfdestruct(origin: OriginFor, contract: EvmAddress) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; - let maintainer = T::AddressMapping::get_evm_address(&who).ok_or(Error::::AddressNotMapped)?; - Self::do_selfdestruct(who, &maintainer, contract)?; + let caller = T::AddressMapping::get_evm_address(&who).ok_or(Error::::AddressNotMapped)?; + Self::do_selfdestruct(&caller, &contract)?; Pallet::::deposit_event(Event::::ContractSelfdestructed(contract)); @@ -867,15 +882,15 @@ impl Pallet { } #[transactional] - pub fn remove_contract(address: &EvmAddress) -> Result { + pub fn remove_contract(caller: &EvmAddress, address: &EvmAddress) -> DispatchResult { let address_account = T::AddressMapping::get_account_id(address); - let size = Accounts::::try_mutate_exists(address, |account_info| -> Result { - let account_info = account_info.as_mut().ok_or(Error::::ContractNotFound)?; - let contract_info = account_info.contract_info.take().ok_or(Error::::ContractNotFound)?; - - let maintainer_account = T::AddressMapping::get_account_id(&contract_info.maintainer); - T::TransferAll::transfer_all(&address_account, &maintainer_account)?; + Accounts::::try_mutate_exists(address, |account_info| -> DispatchResult { + let account_info = account_info.take().ok_or(Error::::ContractNotFound)?; + let contract_info = account_info + .contract_info + .as_ref() + .ok_or(Error::::ContractNotFound)?; CodeInfos::::mutate_exists(&contract_info.code_hash, |maybe_code_info| { if let Some(code_info) = maybe_code_info.as_mut() { @@ -890,18 +905,21 @@ impl Pallet { } }); - AccountStorages::::remove_prefix(address, None); - - let size = ContractStorageSizes::::take(address); - - Ok(size) + T::IdleScheduler::schedule( + EvmTask::Remove { + caller: *caller, + contract: *address, + maintainer: contract_info.maintainer, + } + .into(), + ) })?; // this should happen after `Accounts` is updated because this could trigger another updates on // `Accounts` frame_system::Pallet::::dec_providers(&address_account)?; - Ok(size) + Ok(()) } /// Removes an account from Accounts and AccountStorages. @@ -969,11 +987,6 @@ impl Pallet { deployed: false, }; - Self::update_contract_storage_size( - &address, - code_size.saturating_add(T::NewContractExtraBytes::get()) as i32, - ); - CodeInfos::::mutate_exists(&code_hash, |maybe_code_info| { if let Some(code_info) = maybe_code_info.as_mut() { code_info.ref_count = code_info.ref_count.saturating_add(1); @@ -1179,31 +1192,17 @@ impl Pallet { } /// Selfdestruct a contract at a given address. - fn do_selfdestruct(who: T::AccountId, maintainer: &EvmAddress, contract: EvmAddress) -> DispatchResult { + fn do_selfdestruct(caller: &EvmAddress, contract: &EvmAddress) -> DispatchResult { let account_info = Self::accounts(contract).ok_or(Error::::ContractNotFound)?; let contract_info = account_info .contract_info .as_ref() .ok_or(Error::::ContractNotFound)?; - ensure!(contract_info.maintainer == *maintainer, Error::::NoPermission); + ensure!(contract_info.maintainer == *caller, Error::::NoPermission); ensure!(!contract_info.deployed, Error::::ContractAlreadyDeployed); - let storage = Self::remove_contract(&contract)?; - - let contract_account = T::AddressMapping::get_account_id(&contract); - - let amount = T::StorageDepositPerByte::get().saturating_mul(storage.into()); - let val = T::Currency::repatriate_reserved_named( - &RESERVE_ID_STORAGE_DEPOSIT, - &contract_account, - &who, - amount, - BalanceStatus::Free, - )?; - debug_assert!(val.is_zero()); - - Ok(()) + Self::remove_contract(caller, contract) } fn ensure_root_or_signed(o: T::Origin) -> Result, BadOrigin> { @@ -1308,13 +1307,40 @@ impl Pallet { &contract_acc, &user, amount, - BalanceStatus::Reserved, + BalanceStatus::Free, )?; debug_assert!(val.is_zero()); }; Ok(()) } + + fn refund_storage(caller: &H160, contract: &H160, maintainer: &H160) -> DispatchResult { + let user = T::AddressMapping::get_account_id(caller); + let contract_acc = T::AddressMapping::get_account_id(contract); + let maintainer_acc = T::AddressMapping::get_account_id(maintainer); + let amount = T::Currency::reserved_balance_named(&RESERVE_ID_STORAGE_DEPOSIT, &contract_acc); + + log::debug!( + target: "evm", + "refund_storage: [from: {:?}, account: {:?}, contract: {:?}, contract_acc: {:?}, maintainer: {:?}, maintainer_acc: {:?}, amount: {:?}]", + caller, user, contract, contract_acc, maintainer, maintainer_acc, amount + ); + + // user can't be a dead account + let val = T::Currency::repatriate_reserved_named( + &RESERVE_ID_STORAGE_DEPOSIT, + &contract_acc, + &user, + amount, + BalanceStatus::Reserved, + )?; + debug_assert!(val.is_zero()); + + T::TransferAll::transfer_all(&contract_acc, &maintainer_acc)?; + + Ok(()) + } } impl EVMTrait for Pallet { @@ -1494,3 +1520,74 @@ impl SignedExtension for SetEvmOrigin { Ok(()) } } + +#[derive(Clone, RuntimeDebug, PartialEq, Encode, Decode, TypeInfo)] +pub enum EvmTask { + Schedule { + from: EvmAddress, + target: EvmAddress, + input: Vec, + value: BalanceOf, + gas_limit: u64, + storage_limit: u32, + }, + Remove { + caller: EvmAddress, + contract: EvmAddress, + maintainer: EvmAddress, + }, +} + +impl DispatchableTask for EvmTask { + fn dispatch(self, weight: Weight) -> TaskResult { + match self { + // TODO + EvmTask::Schedule { .. } => { + // check weight and call `scheduled_call` + TaskResult { + result: Ok(()), + used_weight: 0, + finished: false, + } + } + EvmTask::Remove { + caller, + contract, + maintainer, + } => { + // default remove 100 + let limit = cmp::min( + weight + .checked_div(::DbWeight::get().write) + .unwrap_or(100), + 100, + ) as u32; + + match >::remove_prefix(contract, Some(limit)) { + AllRemoved(count) => { + let res = Pallet::::refund_storage(&caller, &contract, &maintainer); + if res.is_err() { + log::error!( + target: "evm", + "refund_storage failed: [from: {:?}, contract: {:?}, maintainer: {:?}], error: {:?}", + caller, contract, maintainer, res + ); + } + ContractStorageSizes::::take(contract); + + TaskResult { + result: res, + used_weight: ::DbWeight::get().write * count as u64, + finished: true, + } + } + SomeRemaining(count) => TaskResult { + result: Ok(()), + used_weight: ::DbWeight::get().write * count as u64, + finished: false, + }, + } + } + } + } +} diff --git a/modules/evm/src/mock.rs b/modules/evm/src/mock.rs index c6f2ca1ae..b2065a184 100644 --- a/modules/evm/src/mock.rs +++ b/modules/evm/src/mock.rs @@ -28,7 +28,7 @@ use frame_support::{ use frame_system::EnsureSignedBy; use module_support::mocks::MockAddressMapping; use orml_traits::parameter_type_with_key; -use primitives::{Amount, BlockNumber, CurrencyId, ReserveIdentifier, TokenSymbol}; +use primitives::{define_combined_task, Amount, BlockNumber, CurrencyId, ReserveIdentifier, TokenSymbol}; use sp_core::{H160, H256}; use sp_runtime::{ testing::Header, @@ -128,6 +128,24 @@ impl orml_currencies::Config for Runtime { } pub type AdaptedBasicCurrency = orml_currencies::BasicCurrencyAdapter; +define_combined_task! { + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] + pub enum ScheduledTasks { + EvmTask(EvmTask), + } +} + +parameter_types!( + pub MinimumWeightRemainInBlock: Weight = u64::MAX; +); + +impl module_idle_scheduler::Config for Runtime { + type Event = Event; + type WeightInfo = (); + type Task = ScheduledTasks; + type MinimumWeightRemainInBlock = MinimumWeightRemainInBlock; +} + pub struct GasToWeight; impl Convert for GasToWeight { @@ -183,6 +201,8 @@ impl Config for Runtime { type Runner = crate::runner::stack::Runner; type FindAuthor = AuthorGiven; + type Task = ScheduledTasks; + type IdleScheduler = IdleScheduler; type WeightInfo = (); } @@ -200,6 +220,7 @@ construct_runtime!( Tokens: orml_tokens::{Pallet, Storage, Event}, Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, Currencies: orml_currencies::{Pallet, Call, Event}, + IdleScheduler: module_idle_scheduler::{Pallet, Call, Storage, Event}, } ); diff --git a/modules/evm/src/runner/mod.rs b/modules/evm/src/runner/mod.rs index cdac2cf04..bdff3cb75 100644 --- a/modules/evm/src/runner/mod.rs +++ b/modules/evm/src/runner/mod.rs @@ -90,6 +90,7 @@ pub trait StackState<'config>: Backend { fn inc_nonce(&mut self, address: H160); fn set_storage(&mut self, address: H160, key: H256, value: H256); fn reset_storage(&mut self, address: H160); + fn storage_size(&mut self, address: H160) -> u32; fn log(&mut self, address: H160, topics: Vec, data: Vec); fn set_deleted(&mut self, address: H160); fn set_code(&mut self, address: H160, code: Vec); diff --git a/modules/evm/src/runner/stack.rs b/modules/evm/src/runner/stack.rs index dcaa59756..d7e8cd7af 100644 --- a/modules/evm/src/runner/stack.rs +++ b/modules/evm/src/runner/stack.rs @@ -44,7 +44,6 @@ pub use primitives::{ }; use sha3::{Digest, Keccak256}; use sp_core::{H160, H256, U256}; -use sp_io::KillStorageResult::{AllRemoved, SomeRemaining}; use sp_runtime::traits::UniqueSaturatedInto; use sp_std::{boxed::Box, collections::btree_set::BTreeSet, marker::PhantomData, mem, vec, vec::Vec}; @@ -163,7 +162,7 @@ impl Runner { "Deleting account at {:?}", address ); - Pallet::::remove_contract(&address).map_err(|e| { + Pallet::::remove_contract(&origin, &address).map_err(|e| { log::debug!( target: "evm", "CannotKillContract address {:?}, reason: {:?}", @@ -607,15 +606,13 @@ impl<'vicinity, 'config, T: Config> StackStateT<'config> for SubstrateStackState } } + // Unused fn reset_storage(&mut self, address: H160) { - match >::remove_prefix(address, None) { - AllRemoved(count) | SomeRemaining(count) => { - // should not happen - let storage = count.saturating_mul(STORAGE_SIZE); - Pallet::::update_contract_storage_size(&address, -(storage as i32)); - self.substate.metadata.storage_meter_mut().refund(storage); - } - } + >::remove_prefix(address, None); + } + + fn storage_size(&mut self, address: H160) -> u32 { + ContractStorageSizes::::get(address) } fn log(&mut self, address: H160, topics: Vec, data: Vec) { @@ -624,8 +621,6 @@ impl<'vicinity, 'config, T: Config> StackStateT<'config> for SubstrateStackState fn set_deleted(&mut self, address: H160) { self.substate.set_deleted(address); - let size = ContractStorageSizes::::get(address); - self.substate.metadata.storage_meter_mut().refund(size); } fn set_code(&mut self, address: H160, code: Vec) { @@ -670,7 +665,12 @@ impl<'vicinity, 'config, T: Config> StackStateT<'config> for SubstrateStackState } } + let code_size = code.len() as u32; Pallet::::create_contract(caller, address, code); + + let used_storage = code_size.saturating_add(T::NewContractExtraBytes::get()); + Pallet::::update_contract_storage_size(&address, used_storage as i32); + self.substate.metadata.storage_meter_mut().charge(used_storage); } fn transfer(&mut self, transfer: Transfer) -> Result<(), ExitError> { diff --git a/modules/evm/src/runner/state.rs b/modules/evm/src/runner/state.rs index 9ce1701d1..60aebff82 100644 --- a/modules/evm/src/runner/state.rs +++ b/modules/evm/src/runner/state.rs @@ -37,6 +37,7 @@ pub use primitives::{ SYSTEM_CONTRACT_ADDRESS_PREFIX, }; use sha3::{Digest, Keccak256}; +use sp_runtime::traits::Zero; use sp_std::{rc::Rc, vec::Vec}; macro_rules! event { @@ -576,7 +577,13 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { return Capture::Exit((ExitError::CreateCollision.into(), None, Vec::new())); } - self.state.reset_storage(address); + if !self.state.storage_size(address).is_zero() { + let _ = self.exit_substate(StackExitKind::Failed); + return Capture::Exit((ExitError::CreateCollision.into(), None, Vec::new())); + } + + // Check instead of reset. + // self.state.reset_storage(address); } let context = Context { diff --git a/modules/idle-scheduler/src/lib.rs b/modules/idle-scheduler/src/lib.rs index 31eeeaf2a..aac78b9fd 100644 --- a/modules/idle-scheduler/src/lib.rs +++ b/modules/idle-scheduler/src/lib.rs @@ -24,10 +24,10 @@ #![allow(clippy::unused_unit)] #![allow(unused_must_use)] use acala_primitives::{ - task::{DispatchableTask, IdelScheduler}, + task::{DispatchableTask, IdleScheduler, TaskResult}, Nonce, }; -use codec::{Codec, EncodeLike}; +use codec::FullCodec; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; use scale_info::TypeInfo; @@ -55,7 +55,7 @@ pub mod module { type WeightInfo: WeightInfo; /// Dispatchable tasks - type Task: DispatchableTask + Codec + EncodeLike + Debug + Clone + PartialEq + TypeInfo; + type Task: DispatchableTask + FullCodec + Debug + Clone + PartialEq + TypeInfo; /// The minimum weight that should remain before idle tasks are dispatched. #[pallet::constant] @@ -66,8 +66,8 @@ pub mod module { #[pallet::generate_deposit(pub fn deposit_event)] pub enum Event { /// A task has been dispatched on_idle. - /// \[TaskId\] - TaskDispatched(Nonce), + /// \[TaskId, DispatchResult\] + TaskDispatched(Nonce, DispatchResult), } /// Some documentation @@ -123,13 +123,13 @@ impl Pallet { return Zero::zero(); } - let mut completed_tasks: Vec = vec![]; + let mut completed_tasks: Vec<(Nonce, TaskResult)> = vec![]; for (id, task) in Tasks::::iter() { let result = task.dispatch(weight_remaining); weight_remaining = weight_remaining.saturating_sub(result.used_weight); if result.finished { - completed_tasks.push(id); + completed_tasks.push((id, result)); } // If remaining weight falls below the minimmum, break from the loop. @@ -139,8 +139,8 @@ impl Pallet { } // Deposit event and remove completed tasks. - for id in completed_tasks { - Self::deposit_event(Event::::TaskDispatched(id)); + for (id, result) in completed_tasks { + Self::deposit_event(Event::::TaskDispatched(id, result.result)); Tasks::::remove(id); } @@ -148,8 +148,8 @@ impl Pallet { } } -impl IdelScheduler for Pallet { - fn schedule(task: T::Task) { - Self::do_schedule_task(task); +impl IdleScheduler for Pallet { + fn schedule(task: T::Task) -> DispatchResult { + Self::do_schedule_task(task) } } diff --git a/modules/idle-scheduler/src/mock.rs b/modules/idle-scheduler/src/mock.rs index 668bab401..fdea185eb 100644 --- a/modules/idle-scheduler/src/mock.rs +++ b/modules/idle-scheduler/src/mock.rs @@ -105,9 +105,10 @@ impl DispatchableTask for HomaLiteTask { } define_combined_task! { + #[derive(Clone, Debug, PartialEq, Encode, Decode, TypeInfo)] pub enum ScheduledTasks { - BalancesTask, - HomaLiteTask, + BalancesTask(BalancesTask), + HomaLiteTask(HomaLiteTask), } } @@ -121,7 +122,6 @@ construct_runtime!( UncheckedExtrinsic = UncheckedExtrinsic { System: frame_system::{Pallet, Call, Event}, - // NOTE: name Example here is needed in order to have same module prefix IdleScheduler: module_idle_scheduler::{Pallet, Call, Event, Storage}, } ); diff --git a/modules/idle-scheduler/src/tests.rs b/modules/idle-scheduler/src/tests.rs index e6072eb18..8ec15c3e4 100644 --- a/modules/idle-scheduler/src/tests.rs +++ b/modules/idle-scheduler/src/tests.rs @@ -21,7 +21,7 @@ #![cfg(test)] use super::*; -use crate::mock::*; +use crate::mock::{IdleScheduler, *}; use frame_support::assert_ok; // Can schedule tasks diff --git a/primitives/src/task.rs b/primitives/src/task.rs index fc3110262..0d9c42036 100644 --- a/primitives/src/task.rs +++ b/primitives/src/task.rs @@ -16,18 +16,22 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . use frame_support::weights::Weight; +use sp_runtime::DispatchResult; #[macro_export] macro_rules! define_combined_task { ( - pub enum $combined_name:ident { - $($task:ident), *$(,)? + $(#[$meta:meta])* + $vis:vis enum $combined_name:ident { + $( + $task:ident ( $vtask:ident $(<$($generic:tt),*>)? ) + ),+ $(,)? } ) => { - #[derive(Debug, Clone, PartialEq, Encode, Decode, TypeInfo)] - pub enum $combined_name { + $(#[$meta])* + $vis enum $combined_name { $( - $task($task), + $task($vtask $(<$($generic),*>)?), )* } @@ -42,8 +46,8 @@ macro_rules! define_combined_task { } $( - impl From<$task> for $combined_name { - fn from(t: $task) -> Self{ + impl From<$vtask $(<$($generic),*>)?> for $combined_name { + fn from(t: $vtask $(<$($generic),*>)?) -> Self{ $combined_name::$task(t) } } @@ -53,6 +57,7 @@ macro_rules! define_combined_task { #[allow(dead_code)] pub struct TaskResult { + pub result: DispatchResult, pub used_weight: Weight, pub finished: bool, } @@ -61,6 +66,6 @@ pub trait DispatchableTask { fn dispatch(self, weight: Weight) -> TaskResult; } -pub trait IdelScheduler { - fn schedule(task: Task); +pub trait IdleScheduler { + fn schedule(task: Task) -> DispatchResult; } diff --git a/runtime/mandala/src/lib.rs b/runtime/mandala/src/lib.rs index e9b5a1d41..acb93b82e 100644 --- a/runtime/mandala/src/lib.rs +++ b/runtime/mandala/src/lib.rs @@ -50,8 +50,7 @@ pub use frame_support::{ use frame_system::{EnsureRoot, RawOrigin}; use hex_literal::hex; use module_currencies::{BasicCurrencyAdapter, Currency}; -use module_evm::Runner; -use module_evm::{CallInfo, CreateInfo}; +use module_evm::{CallInfo, CreateInfo, EvmTask, Runner}; use module_evm_accounts::EvmAddressMapping; pub use module_evm_manager::EvmCurrencyIdMapping; use module_relaychain::RelayChainCallBuilder; @@ -1588,6 +1587,8 @@ impl module_evm::Config for Runtime { type FreeDeploymentOrigin = EnsureRootOrHalfGeneralCouncil; type Runner = module_evm::runner::stack::Runner; type FindAuthor = pallet_session::FindAccountFromAuthorIndex; + type Task = ScheduledTasks; + type IdleScheduler = IdleScheduler; type WeightInfo = weights::module_evm::WeightInfo; #[cfg(feature = "with-ethereum-compatibility")] @@ -1939,7 +1940,9 @@ impl nutsfinance_stable_asset::Config for Runtime { } define_combined_task! { + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] pub enum ScheduledTasks { + EvmTask(EvmTask), } } From 87991dc8e04dd96f8a8aa2998bfa9844c1c1a34a Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Thu, 4 Nov 2021 01:10:00 +0800 Subject: [PATCH 04/13] fix evm tests --- Cargo.lock | 2 +- modules/evm/Cargo.toml | 2 +- modules/evm/src/lib.rs | 7 ++--- modules/evm/src/mock.rs | 2 +- modules/evm/src/runner/stack.rs | 27 ++++++++++++++-- modules/evm/src/runner/state.rs | 13 ++------ modules/evm/src/runner/storage_meter.rs | 42 +++++-------------------- modules/evm/src/tests.rs | 21 +++++++++---- 8 files changed, 56 insertions(+), 60 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2458013d6..91a9a7527 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5388,7 +5388,7 @@ name = "module-evm" version = "1.6.0" dependencies = [ "acala-primitives", - "env_logger 0.7.1", + "env_logger 0.9.0", "frame-support", "frame-system", "hex-literal 0.3.3", diff --git a/modules/evm/Cargo.toml b/modules/evm/Cargo.toml index 071f85871..a2eeccdd2 100644 --- a/modules/evm/Cargo.toml +++ b/modules/evm/Cargo.toml @@ -31,7 +31,7 @@ module-evm-utiltity = { path = "../evm-utiltity", default-features = false } primitives = { package = "acala-primitives", path = "../../primitives", default-features = false } [dev-dependencies] -env_logger = "0.7" +env_logger = "0.9.0" pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.12" } orml-currencies = { path = "../../orml/currencies" } orml-tokens = { path = "../../orml/tokens" } diff --git a/modules/evm/src/lib.rs b/modules/evm/src/lib.rs index 5a4832b34..4c92437dd 100644 --- a/modules/evm/src/lib.rs +++ b/modules/evm/src/lib.rs @@ -381,8 +381,7 @@ pub mod module { address: *address, apparent_value: Default::default(), }; - let metadata = - StackSubstateMetadata::new(210_000, 1000, T::NewContractExtraBytes::get(), T::config()); + let metadata = StackSubstateMetadata::new(210_000, 1000, T::config()); let state = SubstrateStackState::::new(&vicinity, metadata); let mut executor = StackExecutor::new(state, T::config()); @@ -1307,7 +1306,7 @@ impl Pallet { &contract_acc, &user, amount, - BalanceStatus::Free, + BalanceStatus::Reserved, )?; debug_assert!(val.is_zero()); }; @@ -1333,7 +1332,7 @@ impl Pallet { &contract_acc, &user, amount, - BalanceStatus::Reserved, + BalanceStatus::Free, )?; debug_assert!(val.is_zero()); diff --git a/modules/evm/src/mock.rs b/modules/evm/src/mock.rs index b2065a184..1fc2f93fe 100644 --- a/modules/evm/src/mock.rs +++ b/modules/evm/src/mock.rs @@ -136,7 +136,7 @@ define_combined_task! { } parameter_types!( - pub MinimumWeightRemainInBlock: Weight = u64::MAX; + pub MinimumWeightRemainInBlock: Weight = u64::MIN; ); impl module_idle_scheduler::Config for Runtime { diff --git a/modules/evm/src/runner/stack.rs b/modules/evm/src/runner/stack.rs index d7e8cd7af..69e25740a 100644 --- a/modules/evm/src/runner/stack.rs +++ b/modules/evm/src/runner/stack.rs @@ -69,7 +69,7 @@ impl Runner { let gas_price = U256::one(); let vicinity = Vicinity { gas_price, origin }; - let metadata = StackSubstateMetadata::new(gas_limit, storage_limit, T::NewContractExtraBytes::get(), config); + let metadata = StackSubstateMetadata::new(gas_limit, storage_limit, config); let state = SubstrateStackState::new(&vicinity, metadata); let mut executor = StackExecutor::new_with_precompile(state, config, T::Precompiles::execute); @@ -116,7 +116,7 @@ impl Runner { // Refund fees to the `source` account if deducted more before, // T::OnChargeTransaction::correct_and_deposit_fee(&source, actual_fee, fee)?; - let state = executor.into_state(); + let mut state = executor.into_state(); // charge storage let actual_storage = state @@ -126,7 +126,20 @@ impl Runner { .ok_or(Error::::OutOfStorage)?; let used_storage = state.metadata().storage_meter().total_used(); let refunded_storage = state.metadata().storage_meter().total_refunded(); + + // add used storage from self + let target = state.metadata().target().expect("Storage target is none"); + state + .substate + .storage_logs + .push((target, state.metadata().storage_meter().used_storage())); + let mut sum_storage: i32 = 0; for (target, storage) in &state.substate.storage_logs { + log::debug!( + target: "evm", + "Storage logs: target: {:?}, storage: {:?}", + target, storage + ); if !config.estimate { Pallet::::charge_storage(&origin, target, *storage).map_err(|e| { log::debug!( @@ -140,7 +153,17 @@ impl Runner { Error::::ChargeStorageFailed })?; } + sum_storage += storage; } + if actual_storage != sum_storage { + log::debug!( + target: "evm", + "ChargeStorageFailed [actual_storage: {:?}, sum_storage: {:?}]", + actual_storage, sum_storage + ); + return Err(Error::::ChargeStorageFailed.into()); + } + if !config.estimate { Pallet::::unreserve_storage(&origin, storage_limit, used_storage, refunded_storage).map_err(|e| { log::debug!( diff --git a/modules/evm/src/runner/state.rs b/modules/evm/src/runner/state.rs index 60aebff82..d69c5462c 100644 --- a/modules/evm/src/runner/state.rs +++ b/modules/evm/src/runner/state.rs @@ -77,10 +77,10 @@ pub struct StackSubstateMetadata<'config> { } impl<'config> StackSubstateMetadata<'config> { - pub fn new(gas_limit: u64, storage_limit: u32, extra_bytes: u32, config: &'config Config) -> Self { + pub fn new(gas_limit: u64, storage_limit: u32, config: &'config Config) -> Self { Self { gasometer: Gasometer::new(gas_limit, config), - storage_meter: StorageMeter::new(storage_limit, extra_bytes), + storage_meter: StorageMeter::new(storage_limit), is_static: false, depth: None, caller: None, @@ -111,10 +111,7 @@ impl<'config> StackSubstateMetadata<'config> { pub fn spit_child(&self, gas_limit: u64, is_static: bool) -> Self { Self { gasometer: Gasometer::new(gas_limit, self.gasometer().config()), - storage_meter: StorageMeter::new( - self.storage_meter().available_storage(), - self.storage_meter().extra_bytes(), - ), + storage_meter: StorageMeter::new(self.storage_meter().available_storage()), is_static: is_static || self.is_static, depth: match self.depth { None => Some(0), @@ -627,10 +624,6 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { match self.state.metadata_mut().gasometer_mut().record_deposit(out.len()) { Ok(()) => { - self.state - .metadata_mut() - .storage_meter_mut() - .charge_with_extra_bytes(out.len() as u32); let e = self.exit_substate(StackExitKind::Succeeded); try_or_fail!(e); self.state.set_code(address, out); diff --git a/modules/evm/src/runner/storage_meter.rs b/modules/evm/src/runner/storage_meter.rs index cfdb96f96..fd7bea0f1 100644 --- a/modules/evm/src/runner/storage_meter.rs +++ b/modules/evm/src/runner/storage_meter.rs @@ -20,7 +20,6 @@ use frame_support::log; pub struct StorageMeter { limit: u32, - extra_bytes: u32, used: u32, refunded: u32, // save storage of children @@ -30,10 +29,9 @@ pub struct StorageMeter { impl StorageMeter { /// Create a new storage_meter with given storage limit. - pub fn new(limit: u32, extra_bytes: u32) -> Self { + pub fn new(limit: u32) -> Self { Self { limit, - extra_bytes, used: 0, refunded: 0, child_used: 0, @@ -43,17 +41,13 @@ impl StorageMeter { pub fn child_meter(&mut self) -> Self { let storage = self.available_storage(); - StorageMeter::new(storage, self.extra_bytes) + StorageMeter::new(storage) } pub fn storage_limit(&self) -> u32 { self.limit } - pub fn extra_bytes(&self) -> u32 { - self.extra_bytes - } - pub fn used(&self) -> u32 { self.used } @@ -115,15 +109,6 @@ impl StorageMeter { self.used = self.used.saturating_add(storage); } - pub fn charge_with_extra_bytes(&mut self, storage: u32) { - log::trace!( - target: "evm", - "StorageMeter: charge: storage {:?}", - storage - ); - self.used = self.used.saturating_add(storage).saturating_add(self.extra_bytes); - } - pub fn uncharge(&mut self, storage: u32) { log::trace!( target: "evm", @@ -154,7 +139,7 @@ mod tests { #[test] fn test_storage_with_limit_zero() { - let mut storage_meter = StorageMeter::new(0, 0); + let mut storage_meter = StorageMeter::new(0); assert_eq!(storage_meter.available_storage(), 0); assert_eq!(storage_meter.storage_limit(), 0); @@ -183,22 +168,9 @@ mod tests { assert_eq!(storage_meter.finish(), Some(-1)); } - #[test] - fn test_with_extra_bytes() { - let mut storage_meter = StorageMeter::new(1000, 100); - assert_eq!(storage_meter.available_storage(), 1000); - assert_eq!(storage_meter.extra_bytes(), 100); - - storage_meter.charge(200); - assert_eq!(storage_meter.finish(), Some(200)); - - storage_meter.charge_with_extra_bytes(200); - assert_eq!(storage_meter.finish(), Some(500)); - } - #[test] fn test_out_of_storage() { - let mut storage_meter = StorageMeter::new(1000, 0); + let mut storage_meter = StorageMeter::new(1000); assert_eq!(storage_meter.available_storage(), 1000); storage_meter.charge(200); @@ -213,7 +185,7 @@ mod tests { #[test] fn test_high_use_and_refund() { - let mut storage_meter = StorageMeter::new(1000, 0); + let mut storage_meter = StorageMeter::new(1000); assert_eq!(storage_meter.available_storage(), 1000); storage_meter.charge(1000); @@ -233,7 +205,7 @@ mod tests { #[test] fn test_child_meter() { - let mut storage_meter = StorageMeter::new(1000, 0); + let mut storage_meter = StorageMeter::new(1000); storage_meter.charge(100); let mut child_meter = storage_meter.child_meter(); @@ -268,7 +240,7 @@ mod tests { #[test] fn test_merge() { - let mut storage_meter = StorageMeter::new(1000, 0); + let mut storage_meter = StorageMeter::new(1000); storage_meter.charge(100); let mut child_meter = storage_meter.child_meter(); diff --git a/modules/evm/src/tests.rs b/modules/evm/src/tests.rs index 5bcf0ea7c..d2e275a56 100644 --- a/modules/evm/src/tests.rs +++ b/modules/evm/src/tests.rs @@ -19,7 +19,7 @@ #![cfg(test)] use super::*; -use mock::{Event, *}; +use mock::{Event, IdleScheduler, *}; use crate::runner::{ stack::SubstrateStackState, @@ -57,7 +57,7 @@ fn should_calculate_contract_address() { gas_price: U256::one(), origin: Default::default(), }; - let metadata = StackSubstateMetadata::new(1000, 1000, NewContractExtraBytes::get(), &ACALA_CONFIG); + let metadata = StackSubstateMetadata::new(1000, 1000, &ACALA_CONFIG); let state = SubstrateStackState::::new(&vicinity, metadata); let mut executor = StackExecutor::new(state, &ACALA_CONFIG); @@ -286,7 +286,7 @@ fn should_deploy_payable_contract() { let result = ::Runner::create( alice(), - contract, + contract.clone(), amount, 1000000, 100000, @@ -513,10 +513,13 @@ fn contract_should_deploy_contracts() { alice_balance - amount - 281 * ::StorageDepositPerByte::get() ); assert_eq!(balance(factory_contract_address), amount); - assert_eq!(reserved_balance(factory_contract_address), 5950); + assert_eq!( + reserved_balance(factory_contract_address), + (467 + 281) * ::StorageDepositPerByte::get() + ); let contract_address = H160::from_str("7b8f8ca099f6e33cf1817cf67d0556429cfc54e4").unwrap(); assert_eq!(balance(contract_address), 0); - assert_eq!(reserved_balance(contract_address), 1530); + assert_eq!(reserved_balance(contract_address), 0); }); } @@ -579,7 +582,10 @@ fn contract_should_deploy_contracts_without_payable() { alice_balance - (result.used_storage as u64 * ::StorageDepositPerByte::get()) ); assert_eq!(balance(factory_contract_address), 0); - assert_eq!(reserved_balance(factory_contract_address), 5920); + assert_eq!( + reserved_balance(factory_contract_address), + (464 + 290) * ::StorageDepositPerByte::get() + ); }); } @@ -1230,6 +1236,9 @@ fn should_selfdestruct() { assert_eq!(System::providers(&contract_account_id), 2); assert_ok!(EVM::selfdestruct(Origin::signed(alice_account_id), contract_address)); + assert_eq!(System::providers(&contract_account_id), 1); + assert!(System::account_exists(&contract_account_id)); + IdleScheduler::on_idle(0, 1_000_000_000_000); assert_eq!(System::providers(&contract_account_id), 0); assert!(!System::account_exists(&contract_account_id)); assert!(!Accounts::::contains_key(&contract_address)); From 78566e18a1980e030c3d9d66f8f9c2609926bcf3 Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Thu, 4 Nov 2021 13:17:31 +0800 Subject: [PATCH 05/13] fix integration tests --- Cargo.lock | 3 ++ modules/currencies/src/mock.rs | 2 + modules/evm-bridge/src/mock.rs | 2 + modules/evm-manager/src/mock.rs | 2 + modules/evm/src/lib.rs | 46 ++++++++++++------- modules/evm/src/runner/stack.rs | 62 ++++++++++++-------------- modules/evm/src/runner/state.rs | 8 ++-- modules/evm/src/tests.rs | 10 +++-- modules/idle-scheduler/Cargo.toml | 2 + modules/idle-scheduler/src/lib.rs | 6 +-- modules/idle-scheduler/src/mock.rs | 8 ++-- modules/support/src/lib.rs | 23 ++++++++++ primitives/src/task.rs | 22 +++------ runtime/acala/src/lib.rs | 18 +++++--- runtime/common/Cargo.toml | 2 + runtime/common/src/precompile/mock.rs | 33 +++++++++++--- runtime/integration-tests/Cargo.toml | 2 +- runtime/integration-tests/src/evm.rs | 13 ++++++ runtime/integration-tests/src/setup.rs | 19 ++++---- runtime/karura/src/lib.rs | 16 ++++--- runtime/mandala/src/lib.rs | 6 +-- 21 files changed, 191 insertions(+), 114 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 91a9a7527..12a988dae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5630,6 +5630,7 @@ dependencies = [ "acala-primitives", "frame-support", "frame-system", + "module-support", "parity-scale-codec", "scale-info", "serde", @@ -9825,6 +9826,7 @@ dependencies = [ "module-evm-bridge", "module-evm-manager", "module-evm-utiltity-macro", + "module-idle-scheduler", "module-nft", "module-prices", "module-staking-pool", @@ -9873,6 +9875,7 @@ dependencies = [ "ecosystem-compound-cash", "ecosystem-renvm-bridge", "ecosystem-starport", + "env_logger 0.9.0", "frame-support", "frame-system", "hex", diff --git a/modules/currencies/src/mock.rs b/modules/currencies/src/mock.rs index 846298628..3f2c1fa7f 100644 --- a/modules/currencies/src/mock.rs +++ b/modules/currencies/src/mock.rs @@ -174,6 +174,8 @@ impl module_evm::Config for Runtime { type Runner = module_evm::runner::stack::Runner; type FindAuthor = (); + type Task = (); + type IdleScheduler = (); type WeightInfo = (); } diff --git a/modules/evm-bridge/src/mock.rs b/modules/evm-bridge/src/mock.rs index edadc492e..1c3020853 100644 --- a/modules/evm-bridge/src/mock.rs +++ b/modules/evm-bridge/src/mock.rs @@ -130,6 +130,8 @@ impl module_evm::Config for Runtime { type Runner = module_evm::runner::stack::Runner; type FindAuthor = (); + type Task = (); + type IdleScheduler = (); type WeightInfo = (); } diff --git a/modules/evm-manager/src/mock.rs b/modules/evm-manager/src/mock.rs index ec35f947a..9f11f2464 100644 --- a/modules/evm-manager/src/mock.rs +++ b/modules/evm-manager/src/mock.rs @@ -162,6 +162,8 @@ impl module_evm::Config for Runtime { type Runner = module_evm::runner::stack::Runner; type FindAuthor = (); + type Task = (); + type IdleScheduler = (); type WeightInfo = (); } diff --git a/modules/evm/src/lib.rs b/modules/evm/src/lib.rs index 4c92437dd..8ca62d9bc 100644 --- a/modules/evm/src/lib.rs +++ b/modules/evm/src/lib.rs @@ -55,13 +55,14 @@ pub use module_evm_utiltity::{ Account, }; pub use module_support::{ - AddressMapping, EVMStateRentTrait, ExecutionMode, InvokeContext, TransactionPayment, EVM as EVMTrait, + AddressMapping, DispatchableTask, EVMStateRentTrait, ExecutionMode, IdleScheduler, InvokeContext, + TransactionPayment, EVM as EVMTrait, }; pub use orml_traits::currency::TransferAll; use primitive_types::{H160, H256, U256}; pub use primitives::{ evm::{CallInfo, CreateInfo, EvmAddress, ExecutionInfo, Vicinity}, - task::{DispatchableTask, IdleScheduler, TaskResult}, + task::TaskResult, ReserveIdentifier, H160_PREFIX_DEXSHARE, H160_PREFIX_TOKEN, MIRRORED_NFT_ADDRESS_START, PRECOMPILE_ADDRESS_START, SYSTEM_CONTRACT_ADDRESS_PREFIX, }; @@ -1522,6 +1523,7 @@ impl SignedExtension for SetEvmOrigin { #[derive(Clone, RuntimeDebug, PartialEq, Encode, Decode, TypeInfo)] pub enum EvmTask { + // TODO: update Schedule { from: EvmAddress, target: EvmAddress, @@ -1540,7 +1542,7 @@ pub enum EvmTask { impl DispatchableTask for EvmTask { fn dispatch(self, weight: Weight) -> TaskResult { match self { - // TODO + // TODO: update EvmTask::Schedule { .. } => { // check weight and call `scheduled_call` TaskResult { @@ -1554,7 +1556,7 @@ impl DispatchableTask for EvmTask { contract, maintainer, } => { - // default remove 100 + // default limit 100 let limit = cmp::min( weight .checked_div(::DbWeight::get().write) @@ -1565,13 +1567,11 @@ impl DispatchableTask for EvmTask { match >::remove_prefix(contract, Some(limit)) { AllRemoved(count) => { let res = Pallet::::refund_storage(&caller, &contract, &maintainer); - if res.is_err() { - log::error!( - target: "evm", - "refund_storage failed: [from: {:?}, contract: {:?}, maintainer: {:?}], error: {:?}", - caller, contract, maintainer, res - ); - } + log::debug!( + target: "evm", + "EvmTask::Remove: [from: {:?}, contract: {:?}, maintainer: {:?}, count: {:?}, result: {:?}]", + caller, contract, maintainer, count, res + ); ContractStorageSizes::::take(contract); TaskResult { @@ -1580,13 +1580,27 @@ impl DispatchableTask for EvmTask { finished: true, } } - SomeRemaining(count) => TaskResult { - result: Ok(()), - used_weight: ::DbWeight::get().write * count as u64, - finished: false, - }, + SomeRemaining(count) => { + log::debug!( + target: "evm", + "EvmTask::Remove: [from: {:?}, contract: {:?}, maintainer: {:?}, count: {:?}]", + caller, contract, maintainer, count + ); + + TaskResult { + result: Ok(()), + used_weight: ::DbWeight::get().write * count as u64, + finished: false, + } + } } } } } } + +impl From> for () { + fn from(_task: EvmTask) -> Self { + unimplemented!() + } +} diff --git a/modules/evm/src/runner/stack.rs b/modules/evm/src/runner/stack.rs index 69e25740a..6d823d2df 100644 --- a/modules/evm/src/runner/stack.rs +++ b/modules/evm/src/runner/stack.rs @@ -116,7 +116,7 @@ impl Runner { // Refund fees to the `source` account if deducted more before, // T::OnChargeTransaction::correct_and_deposit_fee(&source, actual_fee, fee)?; - let mut state = executor.into_state(); + let state = executor.into_state(); // charge storage let actual_storage = state @@ -126,20 +126,13 @@ impl Runner { .ok_or(Error::::OutOfStorage)?; let used_storage = state.metadata().storage_meter().total_used(); let refunded_storage = state.metadata().storage_meter().total_refunded(); - - // add used storage from self - let target = state.metadata().target().expect("Storage target is none"); - state - .substate - .storage_logs - .push((target, state.metadata().storage_meter().used_storage())); + log::debug!( + target: "evm", + "Storage logs: {:?}", + state.substate.storage_logs + ); let mut sum_storage: i32 = 0; for (target, storage) in &state.substate.storage_logs { - log::debug!( - target: "evm", - "Storage logs: target: {:?}, storage: {:?}", - target, storage - ); if !config.estimate { Pallet::::charge_storage(&origin, target, *storage).map_err(|e| { log::debug!( @@ -658,36 +651,37 @@ impl<'vicinity, 'config, T: Config> StackStateT<'config> for SubstrateStackState let mut substate = &self.substate; loop { - if let Some(c) = substate.metadata().caller() { - // the caller maybe is contract and not deployed. - // get the parent's maintainer. - if Pallet::::is_account_empty(c) { - if substate.parent.is_none() { - log::error!( - target: "evm", - "get parent's maintainer failed. caller: {:?}, address: {:?}", - c, - address - ); - debug_assert!(false); - return; - } - substate = substate.parent.as_ref().expect("has checked; qed"); - } else { - caller = *c; - break; - } - } else { + // get maintainer from parent caller + // `enter_substate` will do `spit_child` + if substate.parent.is_none() { log::error!( target: "evm", - "get maintainer failed. address: {:?}", + "get parent's maintainer failed. address: {:?}", address ); debug_assert!(false); return; } + + substate = substate.parent.as_ref().expect("has checked; qed"); + + if let Some(c) = substate.metadata().caller() { + // the caller maybe is contract and not deployed. + // get the parent's maintainer. + if !Pallet::::is_account_empty(c) { + caller = *c; + break; + } + } } + log::debug!( + target: "evm", + "set_code: address: {:?}, maintainer: {:?}", + address, + caller + ); + let code_size = code.len() as u32; Pallet::::create_contract(caller, address, code); diff --git a/modules/evm/src/runner/state.rs b/modules/evm/src/runner/state.rs index d69c5462c..8d0e4b8fb 100644 --- a/modules/evm/src/runner/state.rs +++ b/modules/evm/src/runner/state.rs @@ -574,13 +574,13 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { return Capture::Exit((ExitError::CreateCollision.into(), None, Vec::new())); } + // use `storage_size` instead of `reset_storage`. + // self.state.reset_storage(address); + if !self.state.storage_size(address).is_zero() { let _ = self.exit_substate(StackExitKind::Failed); return Capture::Exit((ExitError::CreateCollision.into(), None, Vec::new())); } - - // Check instead of reset. - // self.state.reset_storage(address); } let context = Context { @@ -624,9 +624,9 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { match self.state.metadata_mut().gasometer_mut().record_deposit(out.len()) { Ok(()) => { + self.state.set_code(address, out); let e = self.exit_substate(StackExitKind::Succeeded); try_or_fail!(e); - self.state.set_code(address, out); Capture::Exit((ExitReason::Succeed(s), Some(address), Vec::new())) } Err(e) => { diff --git a/modules/evm/src/tests.rs b/modules/evm/src/tests.rs index d2e275a56..23f405c60 100644 --- a/modules/evm/src/tests.rs +++ b/modules/evm/src/tests.rs @@ -515,11 +515,14 @@ fn contract_should_deploy_contracts() { assert_eq!(balance(factory_contract_address), amount); assert_eq!( reserved_balance(factory_contract_address), - (467 + 281) * ::StorageDepositPerByte::get() + (467 + 128) * ::StorageDepositPerByte::get() ); let contract_address = H160::from_str("7b8f8ca099f6e33cf1817cf67d0556429cfc54e4").unwrap(); assert_eq!(balance(contract_address), 0); - assert_eq!(reserved_balance(contract_address), 0); + assert_eq!( + reserved_balance(contract_address), + 153 * ::StorageDepositPerByte::get() + ); }); } @@ -584,7 +587,7 @@ fn contract_should_deploy_contracts_without_payable() { assert_eq!(balance(factory_contract_address), 0); assert_eq!( reserved_balance(factory_contract_address), - (464 + 290) * ::StorageDepositPerByte::get() + (464 + 128) * ::StorageDepositPerByte::get() ); }); } @@ -753,7 +756,6 @@ fn create_predeploy_contract_works() { }); } -#[test] #[test] fn should_transfer_maintainer() { // pragma solidity ^0.5.0; diff --git a/modules/idle-scheduler/Cargo.toml b/modules/idle-scheduler/Cargo.toml index d44a60f96..0d0d7d32c 100644 --- a/modules/idle-scheduler/Cargo.toml +++ b/modules/idle-scheduler/Cargo.toml @@ -13,6 +13,7 @@ sp-std = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v frame-support = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.12", default-features = false } frame-system = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.12", default-features = false } acala-primitives = { path = "../../primitives", default-features = false } +module-support = { path = "../support", default-features = false } [dev-dependencies] sp-core = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.12" } @@ -28,5 +29,6 @@ std = [ "frame-support/std", "frame-system/std", "acala-primitives/std", + "module-support/std", ] try-runtime = ["frame-support/try-runtime"] diff --git a/modules/idle-scheduler/src/lib.rs b/modules/idle-scheduler/src/lib.rs index aac78b9fd..715614170 100644 --- a/modules/idle-scheduler/src/lib.rs +++ b/modules/idle-scheduler/src/lib.rs @@ -23,13 +23,11 @@ #![cfg_attr(not(feature = "std"), no_std)] #![allow(clippy::unused_unit)] #![allow(unused_must_use)] -use acala_primitives::{ - task::{DispatchableTask, IdleScheduler, TaskResult}, - Nonce, -}; +use acala_primitives::{task::TaskResult, Nonce}; use codec::FullCodec; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; +pub use module_support::{DispatchableTask, IdleScheduler}; use scale_info::TypeInfo; use sp_runtime::{ traits::{One, Zero}, diff --git a/modules/idle-scheduler/src/mock.rs b/modules/idle-scheduler/src/mock.rs index fdea185eb..a6cb918e4 100644 --- a/modules/idle-scheduler/src/mock.rs +++ b/modules/idle-scheduler/src/mock.rs @@ -21,12 +21,10 @@ #![cfg(test)] use crate as module_idle_scheduler; -use acala_primitives::{ - define_combined_task, - task::{DispatchableTask, TaskResult}, -}; +use acala_primitives::{define_combined_task, task::TaskResult}; use frame_support::weights::Weight; use frame_support::{construct_runtime, parameter_types, traits::Everything}; +use module_support::DispatchableTask; use codec::{Decode, Encode}; use scale_info::TypeInfo; @@ -84,6 +82,7 @@ pub enum BalancesTask { impl DispatchableTask for BalancesTask { fn dispatch(self, weight: Weight) -> TaskResult { TaskResult { + result: Ok(()), used_weight: BASE_WEIGHT, finished: weight >= BASE_WEIGHT, } @@ -98,6 +97,7 @@ pub enum HomaLiteTask { impl DispatchableTask for HomaLiteTask { fn dispatch(self, weight: Weight) -> TaskResult { TaskResult { + result: Ok(()), used_weight: BASE_WEIGHT, finished: weight >= BASE_WEIGHT, } diff --git a/modules/support/src/lib.rs b/modules/support/src/lib.rs index 818ea61fd..4763d44aa 100644 --- a/modules/support/src/lib.rs +++ b/modules/support/src/lib.rs @@ -23,6 +23,7 @@ use codec::{Decode, Encode, FullCodec, HasCompact}; use frame_support::pallet_prelude::{DispatchClass, Pays, Weight}; use primitives::{ evm::{CallInfo, EvmAddress}, + task::TaskResult, CurrencyId, }; use sp_core::H160; @@ -600,3 +601,25 @@ pub trait CallBuilder { /// - debt: the weight limit used to process the `call`. fn finalize_call_into_xcm_message(call: Self::RelayChainCall, extra_fee: Self::Balance, weight: Weight) -> Xcm<()>; } + +/// Dispatchable tasks +pub trait DispatchableTask { + fn dispatch(self, weight: Weight) -> TaskResult; +} + +/// Idle scheduler trait +pub trait IdleScheduler { + fn schedule(task: Task) -> DispatchResult; +} + +impl DispatchableTask for () { + fn dispatch(self, _weight: Weight) -> TaskResult { + unimplemented!() + } +} + +impl IdleScheduler for () { + fn schedule(_task: Task) -> DispatchResult { + unimplemented!() + } +} diff --git a/primitives/src/task.rs b/primitives/src/task.rs index 0d9c42036..03b6d614f 100644 --- a/primitives/src/task.rs +++ b/primitives/src/task.rs @@ -18,6 +18,13 @@ use frame_support::weights::Weight; use sp_runtime::DispatchResult; +#[allow(dead_code)] +pub struct TaskResult { + pub result: DispatchResult, + pub used_weight: Weight, + pub finished: bool, +} + #[macro_export] macro_rules! define_combined_task { ( @@ -54,18 +61,3 @@ macro_rules! define_combined_task { )* }; } - -#[allow(dead_code)] -pub struct TaskResult { - pub result: DispatchResult, - pub used_weight: Weight, - pub finished: bool, -} - -pub trait DispatchableTask { - fn dispatch(self, weight: Weight) -> TaskResult; -} - -pub trait IdleScheduler { - fn schedule(task: Task) -> DispatchResult; -} diff --git a/runtime/acala/src/lib.rs b/runtime/acala/src/lib.rs index 6bc706b0c..c926880ed 100644 --- a/runtime/acala/src/lib.rs +++ b/runtime/acala/src/lib.rs @@ -53,10 +53,11 @@ use sp_version::RuntimeVersion; use frame_system::{EnsureRoot, RawOrigin}; use module_currencies::BasicCurrencyAdapter; -use module_evm::{CallInfo, CreateInfo, Runner}; +use module_evm::{CallInfo, CreateInfo, EvmTask, Runner}; use module_evm_accounts::EvmAddressMapping; use module_evm_manager::EvmCurrencyIdMapping; use module_relaychain::RelayChainCallBuilder; +use module_support::DispatchableTask; use module_transaction_payment::{Multiplier, TargetedFeeAdjustment}; use orml_traits::{ create_median_value_data_provider, parameter_type_with_key, DataFeeder, DataProviderExtended, MultiCurrency, @@ -99,11 +100,9 @@ pub use sp_runtime::BuildStorage; pub use authority::AuthorityConfigImpl; pub use constants::{fee::*, time::*}; pub use primitives::{ - define_combined_task, - evm::EstimateResourcesRequest, - task::{DispatchableTask, TaskResult}, - AccountId, AccountIndex, Address, Amount, AuctionId, AuthoritysOriginId, Balance, BlockNumber, CurrencyId, - DataProviderId, EraIndex, Hash, Moment, Nonce, ReserveIdentifier, Share, Signature, TokenSymbol, TradingPair, + define_combined_task, evm::EstimateResourcesRequest, task::TaskResult, AccountId, AccountIndex, Address, Amount, + AuctionId, AuthoritysOriginId, Balance, BlockNumber, CurrencyId, DataProviderId, EraIndex, Hash, Moment, Nonce, + ReserveIdentifier, Share, Signature, TokenSymbol, TradingPair, }; pub use runtime_common::{ cent, dollar, microcent, millicent, CurveFeeModel, EnsureRootOrAllGeneralCouncil, @@ -1320,6 +1319,8 @@ impl module_evm::Config for Runtime { type FreeDeploymentOrigin = EnsureRootOrHalfGeneralCouncil; type Runner = module_evm::runner::stack::Runner; type FindAuthor = pallet_session::FindAccountFromAuthorIndex; + type Task = ScheduledTasks; + type IdleScheduler = IdleScheduler; type WeightInfo = weights::module_evm::WeightInfo; } @@ -1682,7 +1683,10 @@ impl frame_support::traits::OnRuntimeUpgrade for OnRuntimeUpgrade { } define_combined_task! { - pub enum ScheduledTasks {} + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] + pub enum ScheduledTasks { + EvmTask(EvmTask), + } } parameter_types!( diff --git a/runtime/common/Cargo.toml b/runtime/common/Cargo.toml index c592db5ae..2ad4a1586 100644 --- a/runtime/common/Cargo.toml +++ b/runtime/common/Cargo.toml @@ -30,6 +30,7 @@ module-evm = { path = "../../modules/evm", default-features = false } module-evm-utiltity-macro = { path = "../../modules/evm-utiltity/macro" } module-staking-pool = { path = "../../modules/staking-pool", default-features = false } module-support = { path = "../../modules/support", default-features = false } +module-idle-scheduler = { path = "../../modules/idle-scheduler", default-features = false } primitives = { package = "acala-primitives", path = "../../primitives", default-features = false } [dev-dependencies] @@ -77,6 +78,7 @@ std = [ "orml-traits/std", "module-evm/std", + "module-idle-scheduler/std", "module-staking-pool/std", "module-support/std", "primitives/std", diff --git a/runtime/common/src/precompile/mock.rs b/runtime/common/src/precompile/mock.rs index 4b1b2598b..35849e449 100644 --- a/runtime/common/src/precompile/mock.rs +++ b/runtime/common/src/precompile/mock.rs @@ -28,13 +28,15 @@ use frame_support::{ PalletId, RuntimeDebug, }; use frame_system::{EnsureRoot, EnsureSignedBy}; +use module_evm::EvmTask; +use module_support::DispatchableTask; use module_support::{ mocks::MockAddressMapping, AddressMapping as AddressMappingT, DEXIncentives, ExchangeRate, ExchangeRateProvider, }; use orml_traits::{parameter_type_with_key, MultiReservableCurrency}; pub use primitives::{ - evm::EvmAddress, Amount, BlockNumber, CurrencyId, DexShare, Header, Nonce, ReserveIdentifier, TokenSymbol, - TradingPair, + define_combined_task, evm::EvmAddress, task::TaskResult, Amount, BlockNumber, CurrencyId, DexShare, Header, Nonce, + ReserveIdentifier, TokenSymbol, TradingPair, }; use scale_info::TypeInfo; use sp_core::{crypto::AccountId32, H160, H256}; @@ -174,7 +176,7 @@ impl module_currencies::Config for Test { } impl module_evm_bridge::Config for Test { - type EVM = ModuleEVM; + type EVM = EVMModule; } impl module_evm_manager::Config for Test { @@ -182,6 +184,24 @@ impl module_evm_manager::Config for Test { type EVMBridge = EVMBridge; } +define_combined_task! { + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] + pub enum ScheduledTasks { + EvmTask(EvmTask), + } +} + +parameter_types!( + pub MinimumWeightRemainInBlock: Weight = u64::MIN; +); + +impl module_idle_scheduler::Config for Test { + type Event = Event; + type WeightInfo = (); + type Task = ScheduledTasks; + type MinimumWeightRemainInBlock = MinimumWeightRemainInBlock; +} + parameter_types! { pub const CreateClassDeposit: Balance = 200; pub const CreateTokenDeposit: Balance = 100; @@ -350,7 +370,7 @@ pub type MultiCurrencyPrecompile = pub type NFTPrecompile = crate::NFTPrecompile; pub type StateRentPrecompile = - crate::StateRentPrecompile; + crate::StateRentPrecompile; pub type OraclePrecompile = crate::OraclePrecompile< AccountId, MockAddressMapping, @@ -419,6 +439,8 @@ impl module_evm::Config for Test { type FreeDeploymentOrigin = EnsureSignedBy; type Runner = module_evm::runner::stack::Runner; type FindAuthor = (); + type Task = ScheduledTasks; + type IdleScheduler = IdleScheduler; type WeightInfo = (); } @@ -520,7 +542,8 @@ frame_support::construct_runtime!( Utility: pallet_utility::{Pallet, Call, Event}, Scheduler: pallet_scheduler::{Pallet, Call, Storage, Event}, DexModule: module_dex::{Pallet, Storage, Call, Event, Config}, - ModuleEVM: module_evm::{Pallet, Config, Call, Storage, Event}, + EVMModule: module_evm::{Pallet, Config, Call, Storage, Event}, + IdleScheduler: module_idle_scheduler::{Pallet, Call, Storage, Event}, } ); diff --git a/runtime/integration-tests/Cargo.toml b/runtime/integration-tests/Cargo.toml index a84628944..050e8d664 100644 --- a/runtime/integration-tests/Cargo.toml +++ b/runtime/integration-tests/Cargo.toml @@ -127,7 +127,7 @@ ecosystem-starport = { path = "../../ecosystem-modules/starport" } ecosystem-compound-cash = { path = "../../ecosystem-modules/compound-cash" } [dev-dependencies] - +env_logger = "0.9.0" sp-io = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.12" } sp-trie = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.12" } diff --git a/runtime/integration-tests/src/evm.rs b/runtime/integration-tests/src/evm.rs index 3cbca1e12..f3b2c3d21 100644 --- a/runtime/integration-tests/src/evm.rs +++ b/runtime/integration-tests/src/evm.rs @@ -591,13 +591,26 @@ fn should_not_kill_contract_on_transfer_all_tokens() { assert_eq!(Currencies::free_balance(USD_CURRENCY, &alice()), 1000 * dollar(USD_CURRENCY)); // assert the contract account is not purged + #[cfg(feature = "with-ethereum-compatibility")] + assert_eq!(System::providers(&contract_account_id), 1); + #[cfg(not(feature = "with-ethereum-compatibility"))] + assert_eq!(System::providers(&contract_account_id), 2); assert!(EVM::accounts(contract).is_some()); assert_ok!(EVM::call(Origin::signed(alice()), contract.clone(), hex_literal::hex!("41c0e1b5").to_vec(), 0, 1000000000, 100000)); + #[cfg(feature = "with-ethereum-compatibility")] assert_eq!(System::providers(&contract_account_id), 0); + #[cfg(not(feature = "with-ethereum-compatibility"))] + assert_eq!(System::providers(&contract_account_id), 1); + assert!(EVM::accounts(contract).is_none()); + // use IdleScheduler to remove contract + run_to_block(System::block_number() + 1); + + assert_eq!(System::providers(&contract_account_id), 0); + // should be gone assert!(!System::account_exists(&contract_account_id)); }); diff --git a/runtime/integration-tests/src/setup.rs b/runtime/integration-tests/src/setup.rs index 7d1549ca6..02cb1c4db 100644 --- a/runtime/integration-tests/src/setup.rs +++ b/runtime/integration-tests/src/setup.rs @@ -19,7 +19,7 @@ use acala_service::chain_spec::mandala::evm_genesis; pub use codec::Encode; use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder; -use frame_support::traits::{GenesisBuild, OnFinalize, OnInitialize}; +use frame_support::traits::{GenesisBuild, OnFinalize, OnIdle, OnInitialize}; pub use frame_support::{assert_noop, assert_ok, traits::Currency}; pub use frame_system::RawOrigin; @@ -48,8 +48,8 @@ mod mandala_imports { AuthoritysOriginId, Balance, Balances, BlockNumber, Call, CdpEngine, CdpTreasury, CreateClassDeposit, CreateTokenDeposit, Currencies, CurrencyId, CurrencyIdConvert, DataDepositPerByte, DefaultExchangeRate, Dex, EmergencyShutdown, EnabledTradingPairs, Event, EvmAccounts, ExistentialDeposits, Get, GetNativeCurrencyId, - HomaLite, Honzon, Loans, MinimumDebitValue, MultiLocation, NativeTokenExistentialDeposit, NetworkId, - NftPalletId, OneDay, Origin, OriginCaller, ParachainInfo, ParachainSystem, Proxy, ProxyType, + HomaLite, Honzon, IdleScheduler, Loans, MinimumDebitValue, MultiLocation, NativeTokenExistentialDeposit, + NetworkId, NftPalletId, OneDay, Origin, OriginCaller, ParachainInfo, ParachainSystem, Proxy, ProxyType, RelayChainSovereignSubAccount, Runtime, Scheduler, Session, SessionManager, SevenDays, System, Timestamp, TokenSymbol, Tokens, TreasuryAccount, TreasuryPalletId, Utility, Vesting, XcmConfig, XcmExecutor, XcmUnbondFee, NFT, @@ -76,7 +76,7 @@ mod karura_imports { AuctionManager, Authority, AuthoritysOriginId, Balance, Balances, BlockNumber, Call, CdpEngine, CdpTreasury, CreateClassDeposit, CreateTokenDeposit, Currencies, CurrencyId, CurrencyIdConvert, DataDepositPerByte, DefaultExchangeRate, Dex, EmergencyShutdown, Event, EvmAccounts, ExistentialDeposits, Get, GetNativeCurrencyId, - HomaLite, Honzon, KaruraFoundationAccounts, Loans, MinimumDebitValue, MultiLocation, + HomaLite, Honzon, IdleScheduler, KaruraFoundationAccounts, Loans, MinimumDebitValue, MultiLocation, NativeTokenExistentialDeposit, NetworkId, NftPalletId, OneDay, Origin, OriginCaller, ParachainAccount, ParachainInfo, ParachainSystem, Proxy, ProxyType, RelayChainBlockNumberProvider, RelayChainSovereignSubAccount, Runtime, Scheduler, Session, SessionManager, SevenDays, System, Timestamp, TokenSymbol, Tokens, @@ -114,11 +114,11 @@ mod acala_imports { AuctionManager, Authority, AuthoritysOriginId, Balance, Balances, BlockNumber, Call, CdpEngine, CdpTreasury, CreateClassDeposit, CreateTokenDeposit, Currencies, CurrencyId, CurrencyIdConvert, DataDepositPerByte, DefaultExchangeRate, Dex, EmergencyShutdown, Event, EvmAccounts, ExistentialDeposits, Get, GetNativeCurrencyId, - HomaLite, Honzon, Loans, MinimumDebitValue, MultiLocation, NativeTokenExistentialDeposit, NetworkId, - NftPalletId, OneDay, Origin, OriginCaller, ParachainAccount, ParachainInfo, ParachainSystem, Proxy, ProxyType, - RelayChainBlockNumberProvider, RelayChainSovereignSubAccount, Runtime, Scheduler, Session, SessionManager, - SevenDays, System, Timestamp, TokenSymbol, Tokens, TreasuryPalletId, Utility, Vesting, XTokens, XcmConfig, - XcmExecutor, XcmUnbondFee, NFT, + HomaLite, Honzon, IdleScheduler, Loans, MinimumDebitValue, MultiLocation, NativeTokenExistentialDeposit, + NetworkId, NftPalletId, OneDay, Origin, OriginCaller, ParachainAccount, ParachainInfo, ParachainSystem, Proxy, + ProxyType, RelayChainBlockNumberProvider, RelayChainSovereignSubAccount, Runtime, Scheduler, Session, + SessionManager, SevenDays, System, Timestamp, TokenSymbol, Tokens, TreasuryPalletId, Utility, Vesting, XTokens, + XcmConfig, XcmExecutor, XcmUnbondFee, NFT, }; pub use frame_support::parameter_types; pub use primitives::TradingPair; @@ -168,6 +168,7 @@ pub fn run_to_block(n: u32) { Scheduler::on_initialize(System::block_number()); Session::on_initialize(System::block_number()); SessionManager::on_initialize(System::block_number()); + IdleScheduler::on_idle(System::block_number(), u64::MAX); } } diff --git a/runtime/karura/src/lib.rs b/runtime/karura/src/lib.rs index 1c4fcba3d..fa979a62d 100644 --- a/runtime/karura/src/lib.rs +++ b/runtime/karura/src/lib.rs @@ -53,11 +53,11 @@ use sp_version::RuntimeVersion; use frame_system::{EnsureRoot, RawOrigin}; use module_currencies::BasicCurrencyAdapter; -use module_evm::Runner; -use module_evm::{CallInfo, CreateInfo}; +use module_evm::{CallInfo, CreateInfo, EvmTask, Runner}; use module_evm_accounts::EvmAddressMapping; use module_evm_manager::EvmCurrencyIdMapping; use module_relaychain::RelayChainCallBuilder; +use module_support::DispatchableTask; use module_transaction_payment::{Multiplier, TargetedFeeAdjustment}; use orml_traits::{ @@ -101,11 +101,9 @@ pub use sp_runtime::BuildStorage; pub use authority::AuthorityConfigImpl; pub use constants::{fee::*, parachains, time::*}; pub use primitives::{ - define_combined_task, - evm::EstimateResourcesRequest, - task::{DispatchableTask, TaskResult}, - AccountId, AccountIndex, Address, Amount, AuctionId, AuthoritysOriginId, Balance, BlockNumber, CurrencyId, - DataProviderId, EraIndex, Hash, Moment, Nonce, ReserveIdentifier, Share, Signature, TokenSymbol, TradingPair, + define_combined_task, evm::EstimateResourcesRequest, task::TaskResult, AccountId, AccountIndex, Address, Amount, + AuctionId, AuthoritysOriginId, Balance, BlockNumber, CurrencyId, DataProviderId, EraIndex, Hash, Moment, Nonce, + ReserveIdentifier, Share, Signature, TokenSymbol, TradingPair, }; pub use runtime_common::{ cent, dollar, microcent, millicent, CurveFeeModel, EnsureRootOrAllGeneralCouncil, @@ -1321,6 +1319,8 @@ impl module_evm::Config for Runtime { type FreeDeploymentOrigin = EnsureRootOrHalfGeneralCouncil; type Runner = module_evm::runner::stack::Runner; type FindAuthor = pallet_session::FindAccountFromAuthorIndex; + type Task = ScheduledTasks; + type IdleScheduler = IdleScheduler; type WeightInfo = weights::module_evm::WeightInfo; } @@ -1716,7 +1716,9 @@ impl orml_xcm::Config for Runtime { } define_combined_task! { + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] pub enum ScheduledTasks { + EvmTask(EvmTask), } } diff --git a/runtime/mandala/src/lib.rs b/runtime/mandala/src/lib.rs index acb93b82e..f4f982989 100644 --- a/runtime/mandala/src/lib.rs +++ b/runtime/mandala/src/lib.rs @@ -54,7 +54,7 @@ use module_evm::{CallInfo, CreateInfo, EvmTask, Runner}; use module_evm_accounts::EvmAddressMapping; pub use module_evm_manager::EvmCurrencyIdMapping; use module_relaychain::RelayChainCallBuilder; -use module_support::ExchangeRateProvider; +use module_support::{DispatchableTask, ExchangeRateProvider}; use module_transaction_payment::{Multiplier, TargetedFeeAdjustment}; use scale_info::TypeInfo; @@ -64,9 +64,7 @@ use orml_traits::{ }; use pallet_transaction_payment::{FeeDetails, RuntimeDispatchInfo}; use primitives::{ - define_combined_task, - evm::EthereumTransactionMessage, - task::{DispatchableTask, TaskResult}, + define_combined_task, evm::EthereumTransactionMessage, task::TaskResult, unchecked_extrinsic::AcalaUncheckedExtrinsic, }; use sp_api::impl_runtime_apis; From ce3d1366469afc52dc6db6828e5d542e33c6c120 Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Thu, 4 Nov 2021 13:38:48 +0800 Subject: [PATCH 06/13] add test cases --- modules/evm/src/tests.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/modules/evm/src/tests.rs b/modules/evm/src/tests.rs index 23f405c60..32e05b294 100644 --- a/modules/evm/src/tests.rs +++ b/modules/evm/src/tests.rs @@ -1240,14 +1240,30 @@ fn should_selfdestruct() { assert_eq!(System::providers(&contract_account_id), 1); assert!(System::account_exists(&contract_account_id)); + assert!(!Accounts::::contains_key(&contract_address)); + assert!(ContractStorageSizes::::contains_key(&contract_address)); + assert_eq!(AccountStorages::::iter_prefix(&contract_address).count(), 1); + assert!(!CodeInfos::::contains_key(&code_hash)); + assert!(!Codes::::contains_key(&code_hash)); + + assert_eq!(balance(alice()), alice_balance); + assert_eq!(balance(contract_address), 1000); + assert_eq!( + reserved_balance(contract_address), + 287 * ::StorageDepositPerByte::get() + ); + IdleScheduler::on_idle(0, 1_000_000_000_000); + + // refund storage deposit + assert_eq!(balance(alice()), alice_balance + 3870); + assert_eq!(balance(contract_address), 0); + assert_eq!(reserved_balance(contract_address), 0); + assert_eq!(System::providers(&contract_account_id), 0); assert!(!System::account_exists(&contract_account_id)); - assert!(!Accounts::::contains_key(&contract_address)); assert!(!ContractStorageSizes::::contains_key(&contract_address)); assert_eq!(AccountStorages::::iter_prefix(&contract_address).count(), 0); - assert!(!CodeInfos::::contains_key(&code_hash)); - assert!(!Codes::::contains_key(&code_hash)); }); } From 434e861743c52d6232a081d8a9970c4cd36c294c Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Thu, 4 Nov 2021 15:17:21 +0800 Subject: [PATCH 07/13] handle extra weight in remove contract. --- modules/evm/src/lib.rs | 29 ++++++++++++++++++++--------- modules/evm/src/runner/stack.rs | 4 +++- modules/evm/src/tests.rs | 4 ++++ primitives/src/evm.rs | 2 ++ 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/modules/evm/src/lib.rs b/modules/evm/src/lib.rs index 8ca62d9bc..ca2d1ca63 100644 --- a/modules/evm/src/lib.rs +++ b/modules/evm/src/lib.rs @@ -531,7 +531,7 @@ pub mod module { let used_gas: u64 = info.used_gas.unique_saturated_into(); Ok(PostDispatchInfo { - actual_weight: Some(T::GasToWeight::convert(used_gas)), + actual_weight: Some(T::GasToWeight::convert(used_gas).saturating_add(info.extra_weight)), pays_fee: Pays::Yes, }) } @@ -590,7 +590,7 @@ pub mod module { } Ok(PostDispatchInfo { - actual_weight: Some(T::GasToWeight::convert(used_gas)), + actual_weight: Some(T::GasToWeight::convert(used_gas).saturating_add(info.extra_weight)), pays_fee: Pays::Yes, }) } @@ -728,6 +728,7 @@ pub mod module { exit_reason: ExitReason::Succeed(ExitSucceed::Stopped), used_gas: 0.into(), used_storage: 0, + extra_weight: 0, logs: vec![], } } else { @@ -848,11 +849,14 @@ pub mod module { pub fn selfdestruct(origin: OriginFor, contract: EvmAddress) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let caller = T::AddressMapping::get_evm_address(&who).ok_or(Error::::AddressNotMapped)?; - Self::do_selfdestruct(&caller, &contract)?; + let extra_weight = Self::do_selfdestruct(&caller, &contract)?; Pallet::::deposit_event(Event::::ContractSelfdestructed(contract)); - Ok(().into()) + Ok(PostDispatchInfo { + actual_weight: Some(::WeightInfo::selfdestruct().saturating_add(extra_weight)), + pays_fee: Pays::Yes, + }) } } } @@ -882,7 +886,7 @@ impl Pallet { } #[transactional] - pub fn remove_contract(caller: &EvmAddress, address: &EvmAddress) -> DispatchResult { + pub fn remove_contract(caller: &EvmAddress, address: &EvmAddress) -> Result { let address_account = T::AddressMapping::get_account_id(address); Accounts::::try_mutate_exists(address, |account_info| -> DispatchResult { @@ -919,7 +923,12 @@ impl Pallet { // `Accounts` frame_system::Pallet::::dec_providers(&address_account)?; - Ok(()) + // estimate weight + let weight = (ContractStorageSizes::::get(address) + .checked_div(STORAGE_SIZE) + .unwrap_or_default() as u64) + .saturating_mul(::DbWeight::get().write); + Ok(weight) } /// Removes an account from Accounts and AccountStorages. @@ -1192,7 +1201,7 @@ impl Pallet { } /// Selfdestruct a contract at a given address. - fn do_selfdestruct(caller: &EvmAddress, contract: &EvmAddress) -> DispatchResult { + fn do_selfdestruct(caller: &EvmAddress, contract: &EvmAddress) -> Result { let account_info = Self::accounts(contract).ok_or(Error::::ContractNotFound)?; let contract_info = account_info .contract_info @@ -1576,7 +1585,8 @@ impl DispatchableTask for EvmTask { TaskResult { result: res, - used_weight: ::DbWeight::get().write * count as u64, + used_weight: ::DbWeight::get().write.saturating_mul(count) + as u64, finished: true, } } @@ -1589,7 +1599,8 @@ impl DispatchableTask for EvmTask { TaskResult { result: Ok(()), - used_weight: ::DbWeight::get().write * count as u64, + used_weight: ::DbWeight::get().write.saturating_mul(count) + as u64, finished: false, } } diff --git a/modules/evm/src/runner/stack.rs b/modules/evm/src/runner/stack.rs index 6d823d2df..081f6a702 100644 --- a/modules/evm/src/runner/stack.rs +++ b/modules/evm/src/runner/stack.rs @@ -172,13 +172,14 @@ impl Runner { })?; } + let mut extra_weight = Default::default(); for address in state.substate.deletes { log::debug!( target: "evm", "Deleting account at {:?}", address ); - Pallet::::remove_contract(&origin, &address).map_err(|e| { + extra_weight = Pallet::::remove_contract(&origin, &address).map_err(|e| { log::debug!( target: "evm", "CannotKillContract address {:?}, reason: {:?}", @@ -200,6 +201,7 @@ impl Runner { exit_reason: reason, used_gas, used_storage: actual_storage, + extra_weight, logs: state.substate.logs, }) } diff --git a/modules/evm/src/tests.rs b/modules/evm/src/tests.rs index 32e05b294..61fd8688e 100644 --- a/modules/evm/src/tests.rs +++ b/modules/evm/src/tests.rs @@ -1462,6 +1462,7 @@ fn evm_execute_mode_should_work() { value: vec![], used_gas: U256::from(139845), used_storage: 290, + extra_weight: 0, logs: vec![] } ); @@ -1485,6 +1486,7 @@ fn evm_execute_mode_should_work() { value: vec![], used_gas: U256::from(256402), used_storage: 580, + extra_weight: 0, logs: vec![] } ); @@ -1525,6 +1527,7 @@ fn evm_execute_mode_should_work() { value: vec![], used_gas: U256::from(107869), used_storage: 290, + extra_weight: 0, logs: vec![] } ); @@ -1553,6 +1556,7 @@ fn evm_execute_mode_should_work() { value: vec![], used_gas: U256::from(92869), used_storage: 290, + extra_weight: 0, logs: vec![] } ); diff --git a/primitives/src/evm.rs b/primitives/src/evm.rs index 3ab846f84..b2768ae70 100644 --- a/primitives/src/evm.rs +++ b/primitives/src/evm.rs @@ -18,6 +18,7 @@ use crate::{Balance, BlockNumber, Nonce}; use codec::{Decode, Encode}; +use frame_support::weights::Weight; use module_evm_utiltity::{ ethereum::{Log, TransactionAction}, evm::ExitReason, @@ -49,6 +50,7 @@ pub struct ExecutionInfo { pub value: T, pub used_gas: U256, pub used_storage: i32, + pub extra_weight: Weight, pub logs: Vec, } From ba2e0cee5e6d78aa06ac40ef5dfa444f169efd4d Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Thu, 4 Nov 2021 15:37:14 +0800 Subject: [PATCH 08/13] fix --- modules/evm/src/lib.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/modules/evm/src/lib.rs b/modules/evm/src/lib.rs index ca2d1ca63..f22cfdd85 100644 --- a/modules/evm/src/lib.rs +++ b/modules/evm/src/lib.rs @@ -1585,8 +1585,9 @@ impl DispatchableTask for EvmTask { TaskResult { result: res, - used_weight: ::DbWeight::get().write.saturating_mul(count) - as u64, + used_weight: ::DbWeight::get() + .write + .saturating_mul(count.into()), finished: true, } } @@ -1599,8 +1600,9 @@ impl DispatchableTask for EvmTask { TaskResult { result: Ok(()), - used_weight: ::DbWeight::get().write.saturating_mul(count) - as u64, + used_weight: ::DbWeight::get() + .write + .saturating_mul(count.into()), finished: false, } } From 2184fbff798e538abb7181dfb41df1543a7a1333 Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Thu, 4 Nov 2021 22:21:36 +0800 Subject: [PATCH 09/13] add cfg for () --- modules/evm/src/lib.rs | 1 + modules/support/src/lib.rs | 5 +++++ primitives/src/task.rs | 1 - 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/modules/evm/src/lib.rs b/modules/evm/src/lib.rs index f22cfdd85..b51c1f2b7 100644 --- a/modules/evm/src/lib.rs +++ b/modules/evm/src/lib.rs @@ -1612,6 +1612,7 @@ impl DispatchableTask for EvmTask { } } +#[cfg(feature = "std")] impl From> for () { fn from(_task: EvmTask) -> Self { unimplemented!() diff --git a/modules/support/src/lib.rs b/modules/support/src/lib.rs index 4763d44aa..8d1fa1dc8 100644 --- a/modules/support/src/lib.rs +++ b/modules/support/src/lib.rs @@ -65,6 +65,7 @@ pub trait RiskManager { fn check_debit_cap(currency_id: CurrencyId, total_debit_balance: DebitBalance) -> DispatchResult; } +#[cfg(feature = "std")] impl RiskManager for () { @@ -146,6 +147,7 @@ pub trait DEXManager { ) -> DispatchResult; } +#[cfg(feature = "std")] impl DEXManager for () where Balance: Default, @@ -305,6 +307,7 @@ pub trait DEXIncentives { fn do_withdraw_dex_share(who: &AccountId, lp_currency_id: CurrencyId, amount: Balance) -> DispatchResult; } +#[cfg(feature = "std")] impl DEXIncentives for () { fn do_deposit_dex_share(_: &AccountId, _: CurrencyId, _: Balance) -> DispatchResult { Ok(()) @@ -612,12 +615,14 @@ pub trait IdleScheduler { fn schedule(task: Task) -> DispatchResult; } +#[cfg(feature = "std")] impl DispatchableTask for () { fn dispatch(self, _weight: Weight) -> TaskResult { unimplemented!() } } +#[cfg(feature = "std")] impl IdleScheduler for () { fn schedule(_task: Task) -> DispatchResult { unimplemented!() diff --git a/primitives/src/task.rs b/primitives/src/task.rs index 03b6d614f..450f311c3 100644 --- a/primitives/src/task.rs +++ b/primitives/src/task.rs @@ -18,7 +18,6 @@ use frame_support::weights::Weight; use sp_runtime::DispatchResult; -#[allow(dead_code)] pub struct TaskResult { pub result: DispatchResult, pub used_weight: Weight, From 94f57ed058d44bf00eb0e9b4529af360a2b455cd Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Thu, 4 Nov 2021 22:50:07 +0800 Subject: [PATCH 10/13] remove extra_weight --- modules/evm/src/lib.rs | 23 +++++++---------------- modules/evm/src/runner/stack.rs | 4 +--- modules/evm/src/tests.rs | 4 ---- primitives/src/evm.rs | 2 -- primitives/src/task.rs | 7 +++++++ 5 files changed, 15 insertions(+), 25 deletions(-) diff --git a/modules/evm/src/lib.rs b/modules/evm/src/lib.rs index b51c1f2b7..8f9e4fdaf 100644 --- a/modules/evm/src/lib.rs +++ b/modules/evm/src/lib.rs @@ -531,7 +531,7 @@ pub mod module { let used_gas: u64 = info.used_gas.unique_saturated_into(); Ok(PostDispatchInfo { - actual_weight: Some(T::GasToWeight::convert(used_gas).saturating_add(info.extra_weight)), + actual_weight: Some(T::GasToWeight::convert(used_gas)), pays_fee: Pays::Yes, }) } @@ -590,7 +590,7 @@ pub mod module { } Ok(PostDispatchInfo { - actual_weight: Some(T::GasToWeight::convert(used_gas).saturating_add(info.extra_weight)), + actual_weight: Some(T::GasToWeight::convert(used_gas)), pays_fee: Pays::Yes, }) } @@ -728,7 +728,6 @@ pub mod module { exit_reason: ExitReason::Succeed(ExitSucceed::Stopped), used_gas: 0.into(), used_storage: 0, - extra_weight: 0, logs: vec![], } } else { @@ -849,14 +848,11 @@ pub mod module { pub fn selfdestruct(origin: OriginFor, contract: EvmAddress) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let caller = T::AddressMapping::get_evm_address(&who).ok_or(Error::::AddressNotMapped)?; - let extra_weight = Self::do_selfdestruct(&caller, &contract)?; + Self::do_selfdestruct(&caller, &contract)?; Pallet::::deposit_event(Event::::ContractSelfdestructed(contract)); - Ok(PostDispatchInfo { - actual_weight: Some(::WeightInfo::selfdestruct().saturating_add(extra_weight)), - pays_fee: Pays::Yes, - }) + Ok(().into()) } } } @@ -886,7 +882,7 @@ impl Pallet { } #[transactional] - pub fn remove_contract(caller: &EvmAddress, address: &EvmAddress) -> Result { + pub fn remove_contract(caller: &EvmAddress, address: &EvmAddress) -> DispatchResult { let address_account = T::AddressMapping::get_account_id(address); Accounts::::try_mutate_exists(address, |account_info| -> DispatchResult { @@ -923,12 +919,7 @@ impl Pallet { // `Accounts` frame_system::Pallet::::dec_providers(&address_account)?; - // estimate weight - let weight = (ContractStorageSizes::::get(address) - .checked_div(STORAGE_SIZE) - .unwrap_or_default() as u64) - .saturating_mul(::DbWeight::get().write); - Ok(weight) + Ok(()) } /// Removes an account from Accounts and AccountStorages. @@ -1201,7 +1192,7 @@ impl Pallet { } /// Selfdestruct a contract at a given address. - fn do_selfdestruct(caller: &EvmAddress, contract: &EvmAddress) -> Result { + fn do_selfdestruct(caller: &EvmAddress, contract: &EvmAddress) -> DispatchResult { let account_info = Self::accounts(contract).ok_or(Error::::ContractNotFound)?; let contract_info = account_info .contract_info diff --git a/modules/evm/src/runner/stack.rs b/modules/evm/src/runner/stack.rs index 081f6a702..6d823d2df 100644 --- a/modules/evm/src/runner/stack.rs +++ b/modules/evm/src/runner/stack.rs @@ -172,14 +172,13 @@ impl Runner { })?; } - let mut extra_weight = Default::default(); for address in state.substate.deletes { log::debug!( target: "evm", "Deleting account at {:?}", address ); - extra_weight = Pallet::::remove_contract(&origin, &address).map_err(|e| { + Pallet::::remove_contract(&origin, &address).map_err(|e| { log::debug!( target: "evm", "CannotKillContract address {:?}, reason: {:?}", @@ -201,7 +200,6 @@ impl Runner { exit_reason: reason, used_gas, used_storage: actual_storage, - extra_weight, logs: state.substate.logs, }) } diff --git a/modules/evm/src/tests.rs b/modules/evm/src/tests.rs index 61fd8688e..32e05b294 100644 --- a/modules/evm/src/tests.rs +++ b/modules/evm/src/tests.rs @@ -1462,7 +1462,6 @@ fn evm_execute_mode_should_work() { value: vec![], used_gas: U256::from(139845), used_storage: 290, - extra_weight: 0, logs: vec![] } ); @@ -1486,7 +1485,6 @@ fn evm_execute_mode_should_work() { value: vec![], used_gas: U256::from(256402), used_storage: 580, - extra_weight: 0, logs: vec![] } ); @@ -1527,7 +1525,6 @@ fn evm_execute_mode_should_work() { value: vec![], used_gas: U256::from(107869), used_storage: 290, - extra_weight: 0, logs: vec![] } ); @@ -1556,7 +1553,6 @@ fn evm_execute_mode_should_work() { value: vec![], used_gas: U256::from(92869), used_storage: 290, - extra_weight: 0, logs: vec![] } ); diff --git a/primitives/src/evm.rs b/primitives/src/evm.rs index b2768ae70..3ab846f84 100644 --- a/primitives/src/evm.rs +++ b/primitives/src/evm.rs @@ -18,7 +18,6 @@ use crate::{Balance, BlockNumber, Nonce}; use codec::{Decode, Encode}; -use frame_support::weights::Weight; use module_evm_utiltity::{ ethereum::{Log, TransactionAction}, evm::ExitReason, @@ -50,7 +49,6 @@ pub struct ExecutionInfo { pub value: T, pub used_gas: U256, pub used_storage: i32, - pub extra_weight: Weight, pub logs: Vec, } diff --git a/primitives/src/task.rs b/primitives/src/task.rs index 450f311c3..cd67a560b 100644 --- a/primitives/src/task.rs +++ b/primitives/src/task.rs @@ -15,9 +15,16 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . +use codec::{Decode, Encode}; use frame_support::weights::Weight; +use scale_info::TypeInfo; +#[cfg(feature = "std")] +use serde::{Deserialize, Serialize}; use sp_runtime::DispatchResult; +use sp_runtime::RuntimeDebug; +#[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] pub struct TaskResult { pub result: DispatchResult, pub used_weight: Weight, From 74e55387f7c4bfb4acc79bde7f4710e5c5b77b73 Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Thu, 4 Nov 2021 23:11:35 +0800 Subject: [PATCH 11/13] add test cases --- modules/evm/src/lib.rs | 25 +++++++++++++----------- modules/evm/src/runner/mod.rs | 1 - modules/evm/src/runner/stack.rs | 9 ++------- modules/evm/src/runner/state.rs | 11 +++-------- modules/evm/src/tests.rs | 34 ++++++++++++++++++++++++++++++--- 5 files changed, 50 insertions(+), 30 deletions(-) diff --git a/modules/evm/src/lib.rs b/modules/evm/src/lib.rs index 8f9e4fdaf..4738207a2 100644 --- a/modules/evm/src/lib.rs +++ b/modules/evm/src/lib.rs @@ -882,15 +882,14 @@ impl Pallet { } #[transactional] - pub fn remove_contract(caller: &EvmAddress, address: &EvmAddress) -> DispatchResult { - let address_account = T::AddressMapping::get_account_id(address); + pub fn remove_contract(caller: &EvmAddress, contract: &EvmAddress) -> DispatchResult { + let contract_account = T::AddressMapping::get_account_id(contract); - Accounts::::try_mutate_exists(address, |account_info| -> DispatchResult { - let account_info = account_info.take().ok_or(Error::::ContractNotFound)?; - let contract_info = account_info - .contract_info - .as_ref() - .ok_or(Error::::ContractNotFound)?; + Accounts::::try_mutate_exists(contract, |account_info| -> DispatchResult { + // We will keep the nonce until the storages are cleared. + // Only remove the `contract_info` + let account_info = account_info.as_mut().ok_or(Error::::ContractNotFound)?; + let contract_info = account_info.contract_info.take().ok_or(Error::::ContractNotFound)?; CodeInfos::::mutate_exists(&contract_info.code_hash, |maybe_code_info| { if let Some(code_info) = maybe_code_info.as_mut() { @@ -905,10 +904,12 @@ impl Pallet { } }); + ContractStorageSizes::::take(contract); + T::IdleScheduler::schedule( EvmTask::Remove { caller: *caller, - contract: *address, + contract: *contract, maintainer: contract_info.maintainer, } .into(), @@ -917,7 +918,7 @@ impl Pallet { // this should happen after `Accounts` is updated because this could trigger another updates on // `Accounts` - frame_system::Pallet::::dec_providers(&address_account)?; + frame_system::Pallet::::dec_providers(&contract_account)?; Ok(()) } @@ -1572,7 +1573,9 @@ impl DispatchableTask for EvmTask { "EvmTask::Remove: [from: {:?}, contract: {:?}, maintainer: {:?}, count: {:?}, result: {:?}]", caller, contract, maintainer, count, res ); - ContractStorageSizes::::take(contract); + + // Remove account after all of the storages are cleared. + Accounts::::take(contract); TaskResult { result: res, diff --git a/modules/evm/src/runner/mod.rs b/modules/evm/src/runner/mod.rs index bdff3cb75..cdac2cf04 100644 --- a/modules/evm/src/runner/mod.rs +++ b/modules/evm/src/runner/mod.rs @@ -90,7 +90,6 @@ pub trait StackState<'config>: Backend { fn inc_nonce(&mut self, address: H160); fn set_storage(&mut self, address: H160, key: H256, value: H256); fn reset_storage(&mut self, address: H160); - fn storage_size(&mut self, address: H160) -> u32; fn log(&mut self, address: H160, topics: Vec, data: Vec); fn set_deleted(&mut self, address: H160); fn set_code(&mut self, address: H160, code: Vec); diff --git a/modules/evm/src/runner/stack.rs b/modules/evm/src/runner/stack.rs index 6d823d2df..ebd6e1226 100644 --- a/modules/evm/src/runner/stack.rs +++ b/modules/evm/src/runner/stack.rs @@ -25,8 +25,8 @@ use crate::{ state::{StackExecutor, StackSubstateMetadata}, Runner as RunnerT, StackState as StackStateT, }, - AccountInfo, AccountStorages, Accounts, BalanceOf, CallInfo, Config, ContractStorageSizes, CreateInfo, Error, - Event, ExecutionInfo, One, Pallet, STORAGE_SIZE, + AccountInfo, AccountStorages, Accounts, BalanceOf, CallInfo, Config, CreateInfo, Error, Event, ExecutionInfo, One, + Pallet, STORAGE_SIZE, }; use frame_support::{ dispatch::DispatchError, @@ -622,15 +622,10 @@ impl<'vicinity, 'config, T: Config> StackStateT<'config> for SubstrateStackState } } - // Unused fn reset_storage(&mut self, address: H160) { >::remove_prefix(address, None); } - fn storage_size(&mut self, address: H160) -> u32 { - ContractStorageSizes::::get(address) - } - fn log(&mut self, address: H160, topics: Vec, data: Vec) { self.substate.log(address, topics, data) } diff --git a/modules/evm/src/runner/state.rs b/modules/evm/src/runner/state.rs index 8d0e4b8fb..f13fefa0d 100644 --- a/modules/evm/src/runner/state.rs +++ b/modules/evm/src/runner/state.rs @@ -37,7 +37,6 @@ pub use primitives::{ SYSTEM_CONTRACT_ADDRESS_PREFIX, }; use sha3::{Digest, Keccak256}; -use sp_runtime::traits::Zero; use sp_std::{rc::Rc, vec::Vec}; macro_rules! event { @@ -569,18 +568,14 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { return Capture::Exit((ExitError::CreateCollision.into(), None, Vec::new())); } + // We will keep the nonce until the storages are cleared. if self.nonce(address) > U256::zero() { let _ = self.exit_substate(StackExitKind::Failed); return Capture::Exit((ExitError::CreateCollision.into(), None, Vec::new())); } - // use `storage_size` instead of `reset_storage`. - // self.state.reset_storage(address); - - if !self.state.storage_size(address).is_zero() { - let _ = self.exit_substate(StackExitKind::Failed); - return Capture::Exit((ExitError::CreateCollision.into(), None, Vec::new())); - } + // Still do this, although it is superfluous. + self.state.reset_storage(address); } let context = Context { diff --git a/modules/evm/src/tests.rs b/modules/evm/src/tests.rs index 32e05b294..3935b0e45 100644 --- a/modules/evm/src/tests.rs +++ b/modules/evm/src/tests.rs @@ -1240,8 +1240,8 @@ fn should_selfdestruct() { assert_eq!(System::providers(&contract_account_id), 1); assert!(System::account_exists(&contract_account_id)); - assert!(!Accounts::::contains_key(&contract_address)); - assert!(ContractStorageSizes::::contains_key(&contract_address)); + assert!(Accounts::::contains_key(&contract_address)); + assert!(!ContractStorageSizes::::contains_key(&contract_address)); assert_eq!(AccountStorages::::iter_prefix(&contract_address).count(), 1); assert!(!CodeInfos::::contains_key(&code_hash)); assert!(!Codes::::contains_key(&code_hash)); @@ -1253,6 +1253,25 @@ fn should_selfdestruct() { 287 * ::StorageDepositPerByte::get() ); + // can't deploy at the same address until everything is wiped out + assert_noop!( + EVM::create_predeploy_contract( + Origin::signed(NetworkContractAccount::get()), + contract_address, + vec![], + 0, + 1000000, + 1000000, + ), + DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { + actual_weight: None, + pays_fee: Pays::Yes, + }, + error: Error::::ContractAlreadyExisted.into() + } + ); + IdleScheduler::on_idle(0, 1_000_000_000_000); // refund storage deposit @@ -1262,8 +1281,17 @@ fn should_selfdestruct() { assert_eq!(System::providers(&contract_account_id), 0); assert!(!System::account_exists(&contract_account_id)); - assert!(!ContractStorageSizes::::contains_key(&contract_address)); + assert!(!Accounts::::contains_key(&contract_address)); assert_eq!(AccountStorages::::iter_prefix(&contract_address).count(), 0); + + assert_ok!(EVM::create_predeploy_contract( + Origin::signed(NetworkContractAccount::get()), + contract_address, + vec![], + 0, + 1000000, + 1000000, + )); }); } From 518b1ad526f2d35429dd8a73b3a2df668ad115a9 Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Fri, 5 Nov 2021 07:25:56 +0800 Subject: [PATCH 12/13] fix integration tests --- runtime/integration-tests/src/evm.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/runtime/integration-tests/src/evm.rs b/runtime/integration-tests/src/evm.rs index f3b2c3d21..da12f8416 100644 --- a/runtime/integration-tests/src/evm.rs +++ b/runtime/integration-tests/src/evm.rs @@ -604,12 +604,13 @@ fn should_not_kill_contract_on_transfer_all_tokens() { #[cfg(not(feature = "with-ethereum-compatibility"))] assert_eq!(System::providers(&contract_account_id), 1); - assert!(EVM::accounts(contract).is_none()); + assert!(EVM::accounts(contract).is_some()); // use IdleScheduler to remove contract run_to_block(System::block_number() + 1); assert_eq!(System::providers(&contract_account_id), 0); + assert!(EVM::accounts(contract).is_none()); // should be gone assert!(!System::account_exists(&contract_account_id)); From a5dc78ee1eec676d21b5a835ef99697115b3c7ef Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Fri, 5 Nov 2021 08:50:33 +0800 Subject: [PATCH 13/13] fix integration tests --- runtime/integration-tests/src/evm.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/runtime/integration-tests/src/evm.rs b/runtime/integration-tests/src/evm.rs index da12f8416..f33ceba27 100644 --- a/runtime/integration-tests/src/evm.rs +++ b/runtime/integration-tests/src/evm.rs @@ -604,6 +604,9 @@ fn should_not_kill_contract_on_transfer_all_tokens() { #[cfg(not(feature = "with-ethereum-compatibility"))] assert_eq!(System::providers(&contract_account_id), 1); + #[cfg(feature = "with-ethereum-compatibility")] + assert!(EVM::accounts(contract).is_none()); + #[cfg(not(feature = "with-ethereum-compatibility"))] assert!(EVM::accounts(contract).is_some()); // use IdleScheduler to remove contract