From 39213a0be53cbfd3458aff3abb71a3311492c0f0 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Wed, 10 May 2023 11:57:31 +0300 Subject: [PATCH 01/15] Add initial impl of evm-system --- Cargo.lock | 13 +++ Cargo.toml | 2 + frame/evm-system/Cargo.toml | 34 +++++++ frame/evm-system/src/lib.rs | 187 ++++++++++++++++++++++++++++++++++++ 4 files changed, 236 insertions(+) create mode 100644 frame/evm-system/Cargo.toml create mode 100644 frame/evm-system/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 24d8a09dc7..e18e0f74bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4929,6 +4929,19 @@ dependencies = [ "sp-io", ] +[[package]] +name = "pallet-evm-system" +version = "1.0.0-dev" +dependencies = [ + "frame-support", + "frame-system", + "log", + "parity-scale-codec", + "scale-info", + "sp-runtime", + "sp-std", +] + [[package]] name = "pallet-evm-test-vector-support" version = "1.0.0-dev" diff --git a/Cargo.toml b/Cargo.toml index f55d5acd2e..8ebea92164 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,6 +5,7 @@ members = [ "frame/ethereum", "frame/evm", "frame/evm-chain-id", + "frame/evm-system", "frame/hotfix-sufficients", "frame/evm/precompile/sha3fips", "frame/evm/precompile/simple", @@ -137,6 +138,7 @@ pallet-dynamic-fee = { version = "4.0.0-dev", path = "frame/dynamic-fee", defaul pallet-ethereum = { version = "4.0.0-dev", path = "frame/ethereum", default-features = false } pallet-evm = { version = "6.0.0-dev", path = "frame/evm", default-features = false } pallet-evm-chain-id = { version = "1.0.0-dev", path = "frame/evm-chain-id", default-features = false } +pallet-evm-system = { version = "1.0.0-dev", path = "frame/evm-system", default-features = false } pallet-evm-precompile-modexp = { version = "2.0.0-dev", path = "frame/evm/precompile/modexp", default-features = false } pallet-evm-precompile-sha3fips = { version = "2.0.0-dev", path = "frame/evm/precompile/sha3fips", default-features = false } pallet-evm-precompile-simple = { version = "2.0.0-dev", path = "frame/evm/precompile/simple", default-features = false } diff --git a/frame/evm-system/Cargo.toml b/frame/evm-system/Cargo.toml new file mode 100644 index 0000000000..7822e94c50 --- /dev/null +++ b/frame/evm-system/Cargo.toml @@ -0,0 +1,34 @@ +[package] +name = "pallet-evm-system" +version = "1.0.0-dev" +license = "Apache-2.0" +description = "FRAME EVM SYSTEM pallet." +authors = { workspace = true } +edition = { workspace = true } +repository = { workspace = true } + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +log = { version = "0.4.17", default-features = false } +scale-codec = { package = "parity-scale-codec", workspace = true } +scale-info = { workspace = true } +# Substrate +frame-support = { workspace = true } +frame-system = { workspace = true } +sp-runtime = { workspace = true } +sp-std = { workspace = true } + +[features] +default = ["std"] +std = [ + "log/std", + "scale-codec/std", + "scale-info/std", + # Substrate + "frame-support/std", + "frame-system/std", + "sp-runtime/std", + "sp-std/std", +] \ No newline at end of file diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs new file mode 100644 index 0000000000..535140253f --- /dev/null +++ b/frame/evm-system/src/lib.rs @@ -0,0 +1,187 @@ +// SPDX-License-Identifier: Apache-2.0 +// This file is part of Frontier. +// +// Copyright (c) 2020-2022 Parity Technologies (UK) Ltd. +// +// 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. + +//! # EVM System Pallet + +// Ensure we're `no_std` when compiling for Wasm. +#![cfg_attr(not(feature = "std"), no_std)] + +use sp_runtime::{traits::{One, Zero}, RuntimeDebug}; +use scale_codec::{Encode, Decode, MaxEncodedLen, FullCodec}; +use scale_info::TypeInfo; + +pub use pallet::*; + +/// Account information. +#[derive(Clone, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode, TypeInfo, MaxEncodedLen)] +pub struct AccountInfo { + /// The number of transactions this account has sent. + pub nonce: Index, + /// The additional data that belongs to this account. Used to store the balance(s) in a lot of + /// chains. + pub data: AccountData, +} + +/// Account creation result status. +#[derive(Eq, PartialEq, RuntimeDebug)] +pub enum AccountCreationStatus { + /// Account was created. + Created, + /// Account already existed. + Existed, +} + +/// Account removing result status. +#[derive(Eq, PartialEq, RuntimeDebug)] +pub enum AccountRemovingStatus { + /// Account was removed. + Removed, + /// Account doesn't exist. + NotExist, +} + +#[frame_support::pallet] +pub mod pallet { + use super::*; + use frame_support::pallet_prelude::*; + use sp_runtime::traits::{MaybeDisplay, AtLeast32Bit}; + use sp_std::fmt::Debug; + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + #[pallet::without_storage_info] + pub struct Pallet(PhantomData); + + #[pallet::config] + pub trait Config: frame_system::Config { + /// The overarching event type. + type RuntimeEvent: From> + IsType<::RuntimeEvent>; + + /// The user account identifier type. + type AccountId: Parameter + + Member + + MaybeSerializeDeserialize + + Debug + + MaybeDisplay + + Ord + + MaxEncodedLen; + + /// Account index (aka nonce) type. This stores the number of previous transactions + /// associated with a sender account. + type Index: Parameter + + Member + + MaybeSerializeDeserialize + + Debug + + Default + + MaybeDisplay + + AtLeast32Bit + + Copy + + MaxEncodedLen; + + /// Data to be associated with an account (other than nonce/transaction counter, which this + /// pallet does regardless). + type AccountData: Member + FullCodec + Clone + Default + TypeInfo + MaxEncodedLen; + + /// Handler for when a new account has just been created. + type OnNewAccount: OnNewAccount<::AccountId>; + + /// A function that is invoked when an account has been determined to be dead. + /// + /// All resources should be cleaned up associated with the given account. + type OnKilledAccount: OnKilledAccount<::AccountId>; + } + + /// The full account information for a particular account ID. + #[pallet::storage] + #[pallet::getter(fn full_account)] + pub type FullAccount = StorageMap< + _, + Blake2_128Concat, + ::AccountId, + AccountInfo<::Index, ::AccountData>, + ValueQuery, + >; + + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + /// A new account was created. + NewAccount { account: ::AccountId }, + /// An account was reaped. + KilledAccount { account: ::AccountId }, + } +} + +impl Pallet { + /// An account is being created. + fn on_created_account(who: ::AccountId) { + ::OnNewAccount::on_new_account(&who); + Self::deposit_event(Event::NewAccount { account: who }); + } + + /// Do anything that needs to be done after an account has been killed. + fn on_killed_account(who: ::AccountId) { + ::OnKilledAccount::on_killed_account(&who); + Self::deposit_event(Event::KilledAccount { account: who }); + } + + /// Retrieve the account transaction counter from storage. + pub fn account_nonce(who: &::AccountId) -> ::Index { + FullAccount::::get(who).nonce + } + + /// Increment a particular account's nonce by 1. + pub fn inc_account_nonce(who: &::AccountId) { + FullAccount::::mutate(who, |a| a.nonce += ::Index::one()); + } + + /// Create an account. + pub fn create_account(who: &::AccountId) -> AccountCreationStatus { + FullAccount::::mutate(who, |a| { + if a.nonce == Zero::zero() { + // Account is being created. + Self::on_created_account(who.clone()); + a.nonce = One::one(); + AccountCreationStatus::Created + } else { + AccountCreationStatus::Existed + } + }) + } + + /// Remove an account. + pub fn remove_account(who: &::AccountId) -> AccountRemovingStatus { + let nonce = FullAccount::::take(who).nonce; + + if nonce == Zero::zero() { + return AccountRemovingStatus::NotExist; + } + + Self::on_killed_account(who.clone()); + AccountRemovingStatus::Removed + } +} + +pub trait OnNewAccount { + /// A new account `who` has been registered. + fn on_new_account(who: &AccountId); +} + +pub trait OnKilledAccount { + /// The account with the given id was reaped. + fn on_killed_account(who: &AccountId); +} From 26d2e24330a9caf9a56cbd2e3b7c8f3656c6920c Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Wed, 10 May 2023 12:24:28 +0300 Subject: [PATCH 02/15] Check account existence --- frame/evm-system/src/lib.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 535140253f..7c6d66e61f 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -127,6 +127,10 @@ pub mod pallet { } impl Pallet { + pub fn account_exists(who: &::AccountId) -> bool { + FullAccount::::contains_key(who) + } + /// An account is being created. fn on_created_account(who: ::AccountId) { ::OnNewAccount::on_new_account(&who); @@ -151,26 +155,24 @@ impl Pallet { /// Create an account. pub fn create_account(who: &::AccountId) -> AccountCreationStatus { - FullAccount::::mutate(who, |a| { - if a.nonce == Zero::zero() { - // Account is being created. - Self::on_created_account(who.clone()); - a.nonce = One::one(); - AccountCreationStatus::Created - } else { - AccountCreationStatus::Existed - } + if Self::account_exists(who) { + return AccountCreationStatus::Existed; + } + + FullAccount::::mutate(who, |_| { + // Account is being created. + Self::on_created_account(who.clone()); + AccountCreationStatus::Created }) } /// Remove an account. pub fn remove_account(who: &::AccountId) -> AccountRemovingStatus { - let nonce = FullAccount::::take(who).nonce; - - if nonce == Zero::zero() { + if !Self::account_exists(who) { return AccountRemovingStatus::NotExist; } + FullAccount::::remove(who); Self::on_killed_account(who.clone()); AccountRemovingStatus::Removed } From 9368df4bc162d0fee13de32c05a970bc1fa4a7d2 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Wed, 10 May 2023 12:34:36 +0300 Subject: [PATCH 03/15] Improve creation account logic --- frame/evm-system/src/lib.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 7c6d66e61f..9dc9e80985 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -20,6 +20,7 @@ // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] +use frame_support::StorageMap; use sp_runtime::{traits::{One, Zero}, RuntimeDebug}; use scale_codec::{Encode, Decode, MaxEncodedLen, FullCodec}; use scale_info::TypeInfo; @@ -159,11 +160,9 @@ impl Pallet { return AccountCreationStatus::Existed; } - FullAccount::::mutate(who, |_| { - // Account is being created. - Self::on_created_account(who.clone()); - AccountCreationStatus::Created - }) + FullAccount::::insert(who.clone(), Default::default()); + Self::on_created_account(who.clone()); + AccountCreationStatus::Created } /// Remove an account. From 433b676c9861b1322866f1573a09665cdc174f24 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Wed, 10 May 2023 12:48:03 +0300 Subject: [PATCH 04/15] Add new line --- frame/evm-system/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/evm-system/Cargo.toml b/frame/evm-system/Cargo.toml index 7822e94c50..9697c1ccd8 100644 --- a/frame/evm-system/Cargo.toml +++ b/frame/evm-system/Cargo.toml @@ -31,4 +31,4 @@ std = [ "frame-system/std", "sp-runtime/std", "sp-std/std", -] \ No newline at end of file +] From 07b8660206fd494e88c38979ac25b365b25bc2a4 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 11 May 2023 14:34:15 +0300 Subject: [PATCH 05/15] Add default implementations --- frame/evm-system/src/lib.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 9dc9e80985..7fc228d8b3 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -20,11 +20,13 @@ // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] -use frame_support::StorageMap; -use sp_runtime::{traits::{One, Zero}, RuntimeDebug}; +use sp_runtime::{traits::One, RuntimeDebug}; use scale_codec::{Encode, Decode, MaxEncodedLen, FullCodec}; use scale_info::TypeInfo; +#[cfg(test)] +mod mock; + pub use pallet::*; /// Account information. @@ -160,7 +162,7 @@ impl Pallet { return AccountCreationStatus::Existed; } - FullAccount::::insert(who.clone(), Default::default()); + FullAccount::::insert(who.clone(), AccountInfo::<_, _>::default()); Self::on_created_account(who.clone()); AccountCreationStatus::Created } @@ -182,7 +184,15 @@ pub trait OnNewAccount { fn on_new_account(who: &AccountId); } +impl OnNewAccount for () { + fn on_new_account(_who: &AccountId) {} +} + pub trait OnKilledAccount { /// The account with the given id was reaped. fn on_killed_account(who: &AccountId); } + +impl OnKilledAccount for () { + fn on_killed_account(_who: &AccountId) {} +} From 06c5049090b574152b098c61708590f9b7e0c0cb Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 11 May 2023 14:35:03 +0300 Subject: [PATCH 06/15] Add mock --- Cargo.lock | 2 + frame/evm-system/Cargo.toml | 4 ++ frame/evm-system/src/mock.rs | 93 ++++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+) create mode 100644 frame/evm-system/src/mock.rs diff --git a/Cargo.lock b/Cargo.lock index e18e0f74bb..e8a43b226f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4938,6 +4938,8 @@ dependencies = [ "log", "parity-scale-codec", "scale-info", + "sp-core", + "sp-io", "sp-runtime", "sp-std", ] diff --git a/frame/evm-system/Cargo.toml b/frame/evm-system/Cargo.toml index 9697c1ccd8..e13b29cfe3 100644 --- a/frame/evm-system/Cargo.toml +++ b/frame/evm-system/Cargo.toml @@ -20,6 +20,10 @@ frame-system = { workspace = true } sp-runtime = { workspace = true } sp-std = { workspace = true } +[dev-dependencies] +sp-core = { workspace = true } +sp-io = { workspace = true } + [features] default = ["std"] std = [ diff --git a/frame/evm-system/src/mock.rs b/frame/evm-system/src/mock.rs new file mode 100644 index 0000000000..28f0a10a68 --- /dev/null +++ b/frame/evm-system/src/mock.rs @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: Apache-2.0 +// This file is part of Frontier. +// +// Copyright (c) 2020-2022 Parity Technologies (UK) Ltd. +// +// 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. + +//! Test mock for unit tests and benchmarking + +use frame_support::{ + traits::{ConstU32, ConstU64}, +}; +use sp_core::{H160, H256}; +use sp_runtime::{ + generic, + traits::{BlakeTwo256, IdentityLookup}, BuildStorage, +}; +use sp_std::{boxed::Box, prelude::*}; + +use crate::{self as pallet_evm_system}; + +type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; +type Block = frame_system::mocking::MockBlock; + +frame_support::construct_runtime! { + pub enum Test where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic, + { + System: frame_system, + EvmSystem: pallet_evm_system, + } +} + +impl frame_system::Config for Test { + type BaseCallFilter = frame_support::traits::Everything; + type BlockWeights = (); + type BlockLength = (); + type RuntimeOrigin = RuntimeOrigin; + type RuntimeCall = RuntimeCall; + type Index = u64; + type BlockNumber = u64; + type Hash = H256; + type Hashing = BlakeTwo256; + type AccountId = H160; + type Lookup = IdentityLookup; + type Header = generic::Header; + type RuntimeEvent = RuntimeEvent; + type BlockHashCount = ConstU64<250>; + type DbWeight = (); + type Version = (); + type PalletInfo = PalletInfo; + type AccountData = (); + type OnNewAccount = (); + type OnKilledAccount = (); + type SystemWeightInfo = (); + type SS58Prefix = (); + type OnSetCode = (); + type MaxConsumers = ConstU32<16>; +} + +impl pallet_evm_system::Config for Test { + type RuntimeEvent = RuntimeEvent; + type AccountId = H160; + type Index = u64; + type AccountData = (); + type OnNewAccount = (); + type OnKilledAccount = (); +} + +/// Build test externalities from the custom genesis. +/// Using this call requires manual assertions on the genesis init logic. +pub fn new_test_ext_with() -> sp_io::TestExternalities { + // Build genesis. + let config = GenesisConfig { + ..Default::default() + }; + let storage = config.build_storage().unwrap(); + + // Make test externalities from the storage. + storage.into() +} From 25ebfda9145690c5f0875589d5bd6ef0e705fd10 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 11 May 2023 16:18:06 +0300 Subject: [PATCH 07/15] Use DispatchResult instead of custom enums --- frame/evm-system/src/lib.rs | 42 ++++++++++++++----------------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 7fc228d8b3..2fd0722a0c 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -15,17 +15,19 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! # EVM System Pallet +//! # EVM System Pallet. // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] -use sp_runtime::{traits::One, RuntimeDebug}; +use sp_runtime::{traits::One, RuntimeDebug, DispatchResult}; use scale_codec::{Encode, Decode, MaxEncodedLen, FullCodec}; use scale_info::TypeInfo; #[cfg(test)] mod mock; +#[cfg(test)] +mod tests; pub use pallet::*; @@ -39,24 +41,6 @@ pub struct AccountInfo { pub data: AccountData, } -/// Account creation result status. -#[derive(Eq, PartialEq, RuntimeDebug)] -pub enum AccountCreationStatus { - /// Account was created. - Created, - /// Account already existed. - Existed, -} - -/// Account removing result status. -#[derive(Eq, PartialEq, RuntimeDebug)] -pub enum AccountRemovingStatus { - /// Account was removed. - Removed, - /// Account doesn't exist. - NotExist, -} - #[frame_support::pallet] pub mod pallet { use super::*; @@ -127,6 +111,12 @@ pub mod pallet { /// An account was reaped. KilledAccount { account: ::AccountId }, } + + #[pallet::error] + pub enum Error { + AccountAlreadyExist, + AccountNotExist, + } } impl Pallet { @@ -157,25 +147,25 @@ impl Pallet { } /// Create an account. - pub fn create_account(who: &::AccountId) -> AccountCreationStatus { + pub fn create_account(who: &::AccountId) -> DispatchResult { if Self::account_exists(who) { - return AccountCreationStatus::Existed; + return Err(Error::::AccountAlreadyExist.into()); } FullAccount::::insert(who.clone(), AccountInfo::<_, _>::default()); Self::on_created_account(who.clone()); - AccountCreationStatus::Created + Ok(()) } /// Remove an account. - pub fn remove_account(who: &::AccountId) -> AccountRemovingStatus { + pub fn remove_account(who: &::AccountId) -> DispatchResult { if !Self::account_exists(who) { - return AccountRemovingStatus::NotExist; + return Err(Error::::AccountNotExist.into()); } FullAccount::::remove(who); Self::on_killed_account(who.clone()); - AccountRemovingStatus::Removed + Ok(()) } } From 0e88ed3c4aea762ae96ea8c9ef1fbb94a6595397 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 11 May 2023 16:18:40 +0300 Subject: [PATCH 08/15] Basic create account tests --- frame/evm-system/src/mock.rs | 4 ++-- frame/evm-system/src/tests.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 frame/evm-system/src/tests.rs diff --git a/frame/evm-system/src/mock.rs b/frame/evm-system/src/mock.rs index 28f0a10a68..e8eab93c0f 100644 --- a/frame/evm-system/src/mock.rs +++ b/frame/evm-system/src/mock.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Test mock for unit tests and benchmarking +//! Test mock for unit tests and benchmarking. use frame_support::{ traits::{ConstU32, ConstU64}, @@ -81,7 +81,7 @@ impl pallet_evm_system::Config for Test { /// Build test externalities from the custom genesis. /// Using this call requires manual assertions on the genesis init logic. -pub fn new_test_ext_with() -> sp_io::TestExternalities { +pub fn new_test_ext() -> sp_io::TestExternalities { // Build genesis. let config = GenesisConfig { ..Default::default() diff --git a/frame/evm-system/src/tests.rs b/frame/evm-system/src/tests.rs new file mode 100644 index 0000000000..70263efefe --- /dev/null +++ b/frame/evm-system/src/tests.rs @@ -0,0 +1,27 @@ +//! Unit tests. + +use core::str::FromStr; + +use frame_support::{assert_ok, assert_noop}; +use sp_core::H160; + +use crate::{mock::*, *}; + +#[test] +fn create_account_works() { + new_test_ext().execute_with(|| { + let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + + assert_ok!(EvmSystem::create_account(&account_id)); + }); +} + +#[test] +fn create_account_fails() { + new_test_ext().execute_with(|| { + let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + >::insert(account_id.clone(), AccountInfo::<_, _>::default()); + + assert_noop!(EvmSystem::create_account(&account_id), Error::::AccountAlreadyExist); + }); +} From 586bd52aedde1e1b69fdb45bd056356bb5dcec9d Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 11 May 2023 16:24:02 +0300 Subject: [PATCH 09/15] Add simple tests with remove account and nonce update --- frame/evm-system/src/tests.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/frame/evm-system/src/tests.rs b/frame/evm-system/src/tests.rs index 70263efefe..0743d97be5 100644 --- a/frame/evm-system/src/tests.rs +++ b/frame/evm-system/src/tests.rs @@ -25,3 +25,35 @@ fn create_account_fails() { assert_noop!(EvmSystem::create_account(&account_id), Error::::AccountAlreadyExist); }); } + +#[test] +fn remove_account_works() { + new_test_ext().execute_with(|| { + let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + >::insert(account_id.clone(), AccountInfo::<_, _>::default()); + + assert_ok!(EvmSystem::remove_account(&account_id)); + }); +} + +#[test] +fn remove_account_fails() { + new_test_ext().execute_with(|| { + let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + + assert_noop!(EvmSystem::remove_account(&account_id), Error::::AccountNotExist); + }); +} + +#[test] +fn nonce_update_works() { + new_test_ext().execute_with(|| { + let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + let nonce_before = EvmSystem::account_nonce(&account_id); + + EvmSystem::inc_account_nonce(&account_id); + + let nonce_after = EvmSystem::account_nonce(&account_id); + assert_eq!(nonce_after, nonce_before + 1); + }); +} From de0858a21f117c8cf40ea1f39a475a76f97a596e Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 11 May 2023 16:35:10 +0300 Subject: [PATCH 10/15] Remove default implementations for OnNewAccount and OnKilledAccount --- frame/evm-system/src/lib.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 2fd0722a0c..ecd8c63aa9 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -174,15 +174,7 @@ pub trait OnNewAccount { fn on_new_account(who: &AccountId); } -impl OnNewAccount for () { - fn on_new_account(_who: &AccountId) {} -} - pub trait OnKilledAccount { /// The account with the given id was reaped. fn on_killed_account(who: &AccountId); } - -impl OnKilledAccount for () { - fn on_killed_account(_who: &AccountId) {} -} From edf7ea596fe56d4132ef8bccd190f979a7dcf8a6 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 11 May 2023 16:36:05 +0300 Subject: [PATCH 11/15] Add mock objects for OnNewAccount and OnKilledAccount --- frame/evm-system/src/mock.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/frame/evm-system/src/mock.rs b/frame/evm-system/src/mock.rs index e8eab93c0f..1a37be492e 100644 --- a/frame/evm-system/src/mock.rs +++ b/frame/evm-system/src/mock.rs @@ -20,6 +20,7 @@ use frame_support::{ traits::{ConstU32, ConstU64}, }; +use mockall::mock; use sp_core::{H160, H256}; use sp_runtime::{ generic, @@ -27,7 +28,21 @@ use sp_runtime::{ }; use sp_std::{boxed::Box, prelude::*}; -use crate::{self as pallet_evm_system}; +use crate::{self as pallet_evm_system, *}; + +mock! { + pub DummyOnNewAccount {} + impl OnNewAccount for DummyOnNewAccount { + pub fn on_new_account(who: &H160); + } +} + +mock! { + pub DummyOnKilledAccount {} + impl OnKilledAccount for DummyOnKilledAccount { + pub fn on_killed_account(who: &H160); + } +} type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; @@ -75,8 +90,8 @@ impl pallet_evm_system::Config for Test { type AccountId = H160; type Index = u64; type AccountData = (); - type OnNewAccount = (); - type OnKilledAccount = (); + type OnNewAccount = MockDummyOnNewAccount; + type OnKilledAccount = MockDummyOnKilledAccount; } /// Build test externalities from the custom genesis. From 01eaf81859106ffac8e50ed3f8a378df617b86c3 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 11 May 2023 17:34:40 +0300 Subject: [PATCH 12/15] Use mock logic in tests --- frame/evm-system/Cargo.toml | 1 + frame/evm-system/src/mock.rs | 34 ++++++++++++++++++++++++++++++++++ frame/evm-system/src/tests.rs | 35 ++++++++++++++++++++++++++++++++++- 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/frame/evm-system/Cargo.toml b/frame/evm-system/Cargo.toml index e13b29cfe3..4a8f6ffd10 100644 --- a/frame/evm-system/Cargo.toml +++ b/frame/evm-system/Cargo.toml @@ -21,6 +21,7 @@ sp-runtime = { workspace = true } sp-std = { workspace = true } [dev-dependencies] +mockall = "0.11" sp-core = { workspace = true } sp-io = { workspace = true } diff --git a/frame/evm-system/src/mock.rs b/frame/evm-system/src/mock.rs index 1a37be492e..56c71bf7e6 100644 --- a/frame/evm-system/src/mock.rs +++ b/frame/evm-system/src/mock.rs @@ -31,14 +31,18 @@ use sp_std::{boxed::Box, prelude::*}; use crate::{self as pallet_evm_system, *}; mock! { + #[derive(Debug)] pub DummyOnNewAccount {} + impl OnNewAccount for DummyOnNewAccount { pub fn on_new_account(who: &H160); } } mock! { + #[derive(Debug)] pub DummyOnKilledAccount {} + impl OnKilledAccount for DummyOnKilledAccount { pub fn on_killed_account(who: &H160); } @@ -106,3 +110,33 @@ pub fn new_test_ext() -> sp_io::TestExternalities { // Make test externalities from the storage. storage.into() } + +pub fn runtime_lock() -> std::sync::MutexGuard<'static, ()> { + static MOCK_RUNTIME_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + // Ignore the poisoning for the tests that panic. + // We only care about concurrency here, not about the poisoning. + match MOCK_RUNTIME_MUTEX.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + } +} + +pub trait TestExternalitiesExt { + fn execute_with_ext(&mut self, execute: E) -> R + where + E: for<'e> FnOnce(&'e ()) -> R; +} + +impl TestExternalitiesExt for frame_support::sp_io::TestExternalities { + fn execute_with_ext(&mut self, execute: E) -> R + where + E: for<'e> FnOnce(&'e ()) -> R, + { + let guard = runtime_lock(); + let result = self.execute_with(|| execute(&guard)); + drop(guard); + result + } +} + diff --git a/frame/evm-system/src/tests.rs b/frame/evm-system/src/tests.rs index 0743d97be5..ec1136592a 100644 --- a/frame/evm-system/src/tests.rs +++ b/frame/evm-system/src/tests.rs @@ -3,16 +3,28 @@ use core::str::FromStr; use frame_support::{assert_ok, assert_noop}; +use mockall::predicate; use sp_core::H160; use crate::{mock::*, *}; #[test] fn create_account_works() { - new_test_ext().execute_with(|| { + new_test_ext().execute_with_ext(|_| { let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + let on_new_account_ctx = MockDummyOnNewAccount::on_new_account_context(); + on_new_account_ctx + .expect() + .once() + .with( + predicate::eq(account_id), + ) + .return_const(()); + assert_ok!(EvmSystem::create_account(&account_id)); + + on_new_account_ctx.checkpoint(); }); } @@ -22,7 +34,12 @@ fn create_account_fails() { let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); >::insert(account_id.clone(), AccountInfo::<_, _>::default()); + let on_new_account_ctx = MockDummyOnNewAccount::on_new_account_context(); + on_new_account_ctx.expect().never(); + assert_noop!(EvmSystem::create_account(&account_id), Error::::AccountAlreadyExist); + + on_new_account_ctx.checkpoint(); }); } @@ -32,7 +49,18 @@ fn remove_account_works() { let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); >::insert(account_id.clone(), AccountInfo::<_, _>::default()); + let on_killed_account_ctx = MockDummyOnKilledAccount::on_killed_account_context(); + on_killed_account_ctx + .expect() + .once() + .with( + predicate::eq(account_id), + ) + .return_const(()); + assert_ok!(EvmSystem::remove_account(&account_id)); + + on_killed_account_ctx.checkpoint(); }); } @@ -41,7 +69,12 @@ fn remove_account_fails() { new_test_ext().execute_with(|| { let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + let on_killed_account_ctx = MockDummyOnKilledAccount::on_killed_account_context(); + on_killed_account_ctx.expect().never(); + assert_noop!(EvmSystem::remove_account(&account_id), Error::::AccountNotExist); + + on_killed_account_ctx.checkpoint(); }); } From d236225c41f8926663e62c7c1d37e33c8f6ff42d Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 11 May 2023 17:45:12 +0300 Subject: [PATCH 13/15] Some tests improvements --- frame/evm-system/src/tests.rs | 36 +++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/frame/evm-system/src/tests.rs b/frame/evm-system/src/tests.rs index ec1136592a..03102d1822 100644 --- a/frame/evm-system/src/tests.rs +++ b/frame/evm-system/src/tests.rs @@ -11,8 +11,13 @@ use crate::{mock::*, *}; #[test] fn create_account_works() { new_test_ext().execute_with_ext(|_| { + // Prepare test data. let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + // Check test preconditions. + assert!(!EvmSystem::account_exists(&account_id)); + + // Set mock expectations. let on_new_account_ctx = MockDummyOnNewAccount::on_new_account_context(); on_new_account_ctx .expect() @@ -22,8 +27,13 @@ fn create_account_works() { ) .return_const(()); + // Invoke the function under test. assert_ok!(EvmSystem::create_account(&account_id)); + // Assert state changes. + assert!(EvmSystem::account_exists(&account_id)); + + // Assert mock invocations. on_new_account_ctx.checkpoint(); }); } @@ -31,24 +41,23 @@ fn create_account_works() { #[test] fn create_account_fails() { new_test_ext().execute_with(|| { + // Prepare test data. let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); >::insert(account_id.clone(), AccountInfo::<_, _>::default()); - let on_new_account_ctx = MockDummyOnNewAccount::on_new_account_context(); - on_new_account_ctx.expect().never(); - + // Invoke the function under test. assert_noop!(EvmSystem::create_account(&account_id), Error::::AccountAlreadyExist); - - on_new_account_ctx.checkpoint(); }); } #[test] fn remove_account_works() { new_test_ext().execute_with(|| { + // Prepare test data. let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); >::insert(account_id.clone(), AccountInfo::<_, _>::default()); + // Set mock expectations. let on_killed_account_ctx = MockDummyOnKilledAccount::on_killed_account_context(); on_killed_account_ctx .expect() @@ -58,8 +67,10 @@ fn remove_account_works() { ) .return_const(()); + // Invoke the function under test. assert_ok!(EvmSystem::remove_account(&account_id)); + // Assert mock invocations. on_killed_account_ctx.checkpoint(); }); } @@ -67,26 +78,27 @@ fn remove_account_works() { #[test] fn remove_account_fails() { new_test_ext().execute_with(|| { + // Prepare test data. let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); - let on_killed_account_ctx = MockDummyOnKilledAccount::on_killed_account_context(); - on_killed_account_ctx.expect().never(); - + // Invoke the function under test. assert_noop!(EvmSystem::remove_account(&account_id), Error::::AccountNotExist); - - on_killed_account_ctx.checkpoint(); }); } #[test] fn nonce_update_works() { new_test_ext().execute_with(|| { + // Prepare test data. let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + + // Check test preconditions. let nonce_before = EvmSystem::account_nonce(&account_id); + // Invoke the function under test. EvmSystem::inc_account_nonce(&account_id); - let nonce_after = EvmSystem::account_nonce(&account_id); - assert_eq!(nonce_after, nonce_before + 1); + // Assert state changes. + assert_eq!(EvmSystem::account_nonce(&account_id), nonce_before + 1); }); } From dc4b819d3403ef3c6a85c11068b4e7b9b753dd0a Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 11 May 2023 17:49:10 +0300 Subject: [PATCH 14/15] Add docs to tests --- Cargo.lock | 1 + frame/evm-system/src/tests.rs | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index e8a43b226f..87cb0af0b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4936,6 +4936,7 @@ dependencies = [ "frame-support", "frame-system", "log", + "mockall", "parity-scale-codec", "scale-info", "sp-core", diff --git a/frame/evm-system/src/tests.rs b/frame/evm-system/src/tests.rs index 03102d1822..9d73ff0be6 100644 --- a/frame/evm-system/src/tests.rs +++ b/frame/evm-system/src/tests.rs @@ -8,6 +8,7 @@ use sp_core::H160; use crate::{mock::*, *}; +/// This test verifies that creating account works in the happy path. #[test] fn create_account_works() { new_test_ext().execute_with_ext(|_| { @@ -38,6 +39,7 @@ fn create_account_works() { }); } +/// This test verifies that creating account fails when the account already exists. #[test] fn create_account_fails() { new_test_ext().execute_with(|| { @@ -50,6 +52,7 @@ fn create_account_fails() { }); } +/// This test verifies that removing account works in the happy path. #[test] fn remove_account_works() { new_test_ext().execute_with(|| { @@ -75,6 +78,7 @@ fn remove_account_works() { }); } +/// This test verifies that removing account fails when the account doesn't exist. #[test] fn remove_account_fails() { new_test_ext().execute_with(|| { @@ -86,8 +90,9 @@ fn remove_account_fails() { }); } +/// This test verifies that incrementing account nonce works in the happy path. #[test] -fn nonce_update_works() { +fn inc_account_nonce_works() { new_test_ext().execute_with(|| { // Prepare test data. let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); From 6b045e7332e89f4d60c217c335fbc157274d7a78 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 11 May 2023 17:59:16 +0300 Subject: [PATCH 15/15] Check events in tests --- frame/evm-system/src/lib.rs | 5 +++++ frame/evm-system/src/mock.rs | 14 +++++++------- frame/evm-system/src/tests.rs | 25 ++++++++++++++++++------- 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index ecd8c63aa9..b8585ca470 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -114,12 +114,15 @@ pub mod pallet { #[pallet::error] pub enum Error { + /// The account already exists in case creating it. AccountAlreadyExist, + /// The account doesn't exist in case removing it. AccountNotExist, } } impl Pallet { + /// Check the account existence. pub fn account_exists(who: &::AccountId) -> bool { FullAccount::::contains_key(who) } @@ -169,11 +172,13 @@ impl Pallet { } } +/// Interface to handle account creation. pub trait OnNewAccount { /// A new account `who` has been registered. fn on_new_account(who: &AccountId); } +/// Interface to handle account killing. pub trait OnKilledAccount { /// The account with the given id was reaped. fn on_killed_account(who: &AccountId); diff --git a/frame/evm-system/src/mock.rs b/frame/evm-system/src/mock.rs index 56c71bf7e6..b62f488515 100644 --- a/frame/evm-system/src/mock.rs +++ b/frame/evm-system/src/mock.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Test mock for unit tests and benchmarking. +//! Test mock for unit tests. use frame_support::{ traits::{ConstU32, ConstU64}, @@ -32,20 +32,20 @@ use crate::{self as pallet_evm_system, *}; mock! { #[derive(Debug)] - pub DummyOnNewAccount {} + pub DummyOnNewAccount {} impl OnNewAccount for DummyOnNewAccount { - pub fn on_new_account(who: &H160); - } + pub fn on_new_account(who: &H160); + } } mock! { #[derive(Debug)] - pub DummyOnKilledAccount {} + pub DummyOnKilledAccount {} impl OnKilledAccount for DummyOnKilledAccount { - pub fn on_killed_account(who: &H160); - } + pub fn on_killed_account(who: &H160); + } } type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; diff --git a/frame/evm-system/src/tests.rs b/frame/evm-system/src/tests.rs index 9d73ff0be6..468b83dff8 100644 --- a/frame/evm-system/src/tests.rs +++ b/frame/evm-system/src/tests.rs @@ -18,21 +18,25 @@ fn create_account_works() { // Check test preconditions. assert!(!EvmSystem::account_exists(&account_id)); + // Set block number to enable events. + System::set_block_number(1); + // Set mock expectations. let on_new_account_ctx = MockDummyOnNewAccount::on_new_account_context(); on_new_account_ctx - .expect() - .once() - .with( - predicate::eq(account_id), - ) + .expect() + .once() + .with( + predicate::eq(account_id), + ) .return_const(()); // Invoke the function under test. assert_ok!(EvmSystem::create_account(&account_id)); // Assert state changes. - assert!(EvmSystem::account_exists(&account_id)); + assert!(EvmSystem::account_exists(&account_id)); + System::assert_has_event(RuntimeEvent::EvmSystem(Event::NewAccount { account: account_id } )); // Assert mock invocations. on_new_account_ctx.checkpoint(); @@ -60,9 +64,12 @@ fn remove_account_works() { let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); >::insert(account_id.clone(), AccountInfo::<_, _>::default()); + // Set block number to enable events. + System::set_block_number(1); + // Set mock expectations. let on_killed_account_ctx = MockDummyOnKilledAccount::on_killed_account_context(); - on_killed_account_ctx + on_killed_account_ctx .expect() .once() .with( @@ -73,6 +80,10 @@ fn remove_account_works() { // Invoke the function under test. assert_ok!(EvmSystem::remove_account(&account_id)); + // Assert state changes. + assert!(!EvmSystem::account_exists(&account_id)); + System::assert_has_event(RuntimeEvent::EvmSystem(Event::KilledAccount { account: account_id } )); + // Assert mock invocations. on_killed_account_ctx.checkpoint(); });