From f4d3cbc92ca1cc636d3e70e819d7899345c7e0b2 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 27 Apr 2023 15:52:01 +0300 Subject: [PATCH 01/23] Use AccountId passed through pallet-evm config --- frame/evm/precompile/dispatch/src/lib.rs | 2 +- frame/evm/src/lib.rs | 78 ++++++++++++++++++------ frame/evm/src/runner/stack.rs | 2 +- template/runtime/src/lib.rs | 2 + 4 files changed, 64 insertions(+), 20 deletions(-) diff --git a/frame/evm/precompile/dispatch/src/lib.rs b/frame/evm/precompile/dispatch/src/lib.rs index 958a4a86cc..a212add8f0 100644 --- a/frame/evm/precompile/dispatch/src/lib.rs +++ b/frame/evm/precompile/dispatch/src/lib.rs @@ -49,7 +49,7 @@ impl Precompile for Dispatch where T: pallet_evm::Config, T::RuntimeCall: Dispatchable + GetDispatchInfo + Decode, - ::RuntimeOrigin: From>, + ::RuntimeOrigin: From::AccountId>>, DecodeLimit: Get, { fn execute(handle: &mut impl PrecompileHandle) -> PrecompileResult { diff --git a/frame/evm/src/lib.rs b/frame/evm/src/lib.rs index a6b1406e2d..a5a5228ecc 100644 --- a/frame/evm/src/lib.rs +++ b/frame/evm/src/lib.rs @@ -76,7 +76,7 @@ use frame_system::RawOrigin; use impl_trait_for_tuples::impl_for_tuples; use sp_core::{Hasher, H160, H256, U256}; use sp_runtime::{ - traits::{BadOrigin, Saturating, UniqueSaturatedInto, Zero}, + traits::{BadOrigin, Saturating, UniqueSaturatedInto, Zero, One}, AccountId32, DispatchErrorWithPostInfo, }; use sp_std::{cmp::min, vec::Vec}; @@ -102,6 +102,8 @@ pub mod pallet { use super::*; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; + use sp_runtime::traits::{MaybeDisplay, AtLeast32Bit}; + use sp_std::fmt::Debug; #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] @@ -110,6 +112,27 @@ pub mod pallet { #[pallet::config] pub trait Config: frame_system::Config + pallet_timestamp::Config { + /// 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; + /// Calculator for current gas price. type FeeCalculator: FeeCalculator; @@ -125,12 +148,12 @@ pub mod pallet { /// Allow the origin to call on behalf of given address. type CallOrigin: EnsureAddressOrigin; /// Allow the origin to withdraw on behalf of given address. - type WithdrawOrigin: EnsureAddressOrigin; + type WithdrawOrigin: EnsureAddressOrigin::AccountId>; /// Mapping from address to account id. - type AddressMapping: AddressMapping; + type AddressMapping: AddressMapping<::AccountId>; /// Currency type for withdraw and balance storage. - type Currency: Currency + Inspect; + type Currency: Currency<::AccountId> + Inspect<::AccountId>; /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -486,7 +509,7 @@ pub mod pallet { MAX_ACCOUNT_NONCE, UniqueSaturatedInto::::unique_saturated_into(account.nonce), ) { - frame_system::Pallet::::inc_account_nonce(&account_id); + Pallet::::inc_account_nonce(&account_id); } T::Currency::deposit_creating(&account_id, account.balance.unique_saturated_into()); @@ -508,15 +531,32 @@ pub mod pallet { #[pallet::getter(fn account_storages)] pub type AccountStorages = StorageDoubleMap<_, Blake2_128Concat, H160, Blake2_128Concat, H256, H256, ValueQuery>; + + #[pallet::storage] + #[pallet::getter(fn account_nonces)] + pub type AccountNonces = + StorageMap<_, Blake2_128Concat, ::AccountId, ::Index, ValueQuery>; +} + +impl Pallet { + /// Retrieve the account transaction counter from storage. + pub fn account_nonce(who: &::AccountId) -> ::Index { + AccountNonces::::get(who) + } + + /// Increment a particular account's nonce by 1. + pub fn inc_account_nonce(who: &::AccountId) { + AccountNonces::::mutate(who, |a| *a += ::Index::one()); + } } /// Type alias for currency balance. pub type BalanceOf = - <::Currency as Currency<::AccountId>>::Balance; + <::Currency as Currency<::AccountId>>::Balance; /// Type alias for negative imbalance during fees type NegativeImbalanceOf = - ::AccountId>>::NegativeImbalance; + ::AccountId>>::NegativeImbalance; pub trait EnsureAddressOrigin { /// Success return type. @@ -690,7 +730,8 @@ impl Pallet { pub fn remove_account(address: &H160) { if >::contains_key(address) { let account_id = T::AddressMapping::into_account_id(*address); - let _ = frame_system::Pallet::::dec_sufficients(&account_id); + // ATTENTION. CHECK. + // let _ = frame_system::Pallet::::dec_sufficients(&account_id); } >::remove(address); @@ -706,7 +747,8 @@ impl Pallet { if !>::contains_key(address) { let account_id = T::AddressMapping::into_account_id(address); - let _ = frame_system::Pallet::::inc_sufficients(&account_id); + // ATTENTION. CHECK. + // let _ = frame_system::Pallet::::inc_sufficients(&account_id); } >::insert(address, code); @@ -716,7 +758,7 @@ impl Pallet { pub fn account_basic(address: &H160) -> (Account, frame_support::weights::Weight) { let account_id = T::AddressMapping::into_account_id(*address); - let nonce = frame_system::Pallet::::account_nonce(&account_id); + let nonce = Pallet::::account_nonce(&account_id); // keepalive `true` takes into account ExistentialDeposit as part of what's considered liquid balance. let balance = T::Currency::reducible_balance(&account_id, true); @@ -772,17 +814,17 @@ pub struct EVMCurrencyAdapter(sp_std::marker::PhantomData<(C, OU)>); impl OnChargeEVMTransaction for EVMCurrencyAdapter where T: Config, - C: Currency<::AccountId>, + C: Currency<::AccountId>, C::PositiveImbalance: Imbalance< - ::AccountId>>::Balance, + ::AccountId>>::Balance, Opposite = C::NegativeImbalance, >, C::NegativeImbalance: Imbalance< - ::AccountId>>::Balance, + ::AccountId>>::Balance, Opposite = C::PositiveImbalance, >, OU: OnUnbalanced>, - U256: UniqueSaturatedInto<::AccountId>>::Balance>, + U256: UniqueSaturatedInto<::AccountId>>::Balance>, { // Kept type as Option to satisfy bound of Default type LiquidityInfo = Option>; @@ -866,10 +908,10 @@ where impl OnChargeEVMTransaction for () where T: Config, - ::AccountId>>::PositiveImbalance: - Imbalance<::AccountId>>::Balance, Opposite = ::AccountId>>::NegativeImbalance>, - ::AccountId>>::NegativeImbalance: -Imbalance<::AccountId>>::Balance, Opposite = ::AccountId>>::PositiveImbalance>, + ::AccountId>>::PositiveImbalance: + Imbalance<::AccountId>>::Balance, Opposite = ::AccountId>>::NegativeImbalance>, + ::AccountId>>::NegativeImbalance: +Imbalance<::AccountId>>::Balance, Opposite = ::AccountId>>::PositiveImbalance>, U256: UniqueSaturatedInto>, { diff --git a/frame/evm/src/runner/stack.rs b/frame/evm/src/runner/stack.rs index be39912a48..cbe18e6cb6 100644 --- a/frame/evm/src/runner/stack.rs +++ b/frame/evm/src/runner/stack.rs @@ -721,7 +721,7 @@ where fn inc_nonce(&mut self, address: H160) { let account_id = T::AddressMapping::into_account_id(address); - frame_system::Pallet::::inc_account_nonce(&account_id); + Pallet::::inc_account_nonce(&account_id); } fn set_storage(&mut self, address: H160, index: H256, value: H256) { diff --git a/template/runtime/src/lib.rs b/template/runtime/src/lib.rs index 4b0aec6759..c4c35b9b7f 100644 --- a/template/runtime/src/lib.rs +++ b/template/runtime/src/lib.rs @@ -326,6 +326,8 @@ parameter_types! { } impl pallet_evm::Config for Runtime { + type AccountId = AccountId; + type Index = Index; type FeeCalculator = BaseFee; type GasWeightMapping = pallet_evm::FixedGasWeightMapping; type WeightPerGas = WeightPerGas; From 813c7c93aa05e54bab1cd288d4b6fcffd887a6f6 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 27 Apr 2023 15:52:50 +0300 Subject: [PATCH 02/23] Fix mocking with external account id usage --- frame/ethereum/src/mock.rs | 2 ++ frame/evm/precompile/dispatch/src/mock.rs | 6 ++++-- frame/evm/src/mock.rs | 6 ++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/frame/ethereum/src/mock.rs b/frame/ethereum/src/mock.rs index 5e41224e9d..490bfb6697 100644 --- a/frame/ethereum/src/mock.rs +++ b/frame/ethereum/src/mock.rs @@ -152,6 +152,8 @@ impl AddressMapping for HashedAddressMapping { } impl pallet_evm::Config for Test { + type AccountId = AccountId32; + type Index = u64; type FeeCalculator = FixedGasPrice; type GasWeightMapping = pallet_evm::FixedGasWeightMapping; type WeightPerGas = WeightPerGas; diff --git a/frame/evm/precompile/dispatch/src/mock.rs b/frame/evm/precompile/dispatch/src/mock.rs index 173091f4f6..ee2a053e56 100644 --- a/frame/evm/precompile/dispatch/src/mock.rs +++ b/frame/evm/precompile/dispatch/src/mock.rs @@ -139,14 +139,16 @@ parameter_types! { pub WeightPerGas: Weight = Weight::from_ref_time(20_000); } impl pallet_evm::Config for Test { + type AccountId = H160; + type Index = u64; type FeeCalculator = FixedGasPrice; type GasWeightMapping = pallet_evm::FixedGasWeightMapping; type WeightPerGas = WeightPerGas; type BlockHashMapping = pallet_evm::SubstrateBlockHashMapping; - type CallOrigin = EnsureAddressRoot; + type CallOrigin = EnsureAddressRoot<::AccountId>; - type WithdrawOrigin = EnsureAddressNever; + type WithdrawOrigin = EnsureAddressNever<::AccountId>; type AddressMapping = IdentityAddressMapping; type Currency = Balances; diff --git a/frame/evm/src/mock.rs b/frame/evm/src/mock.rs index a82721fe5e..2410b47cbd 100644 --- a/frame/evm/src/mock.rs +++ b/frame/evm/src/mock.rs @@ -132,14 +132,16 @@ parameter_types! { pub MockPrecompiles: MockPrecompileSet = MockPrecompileSet; } impl crate::Config for Test { + type AccountId = H160; + type Index = u64; type FeeCalculator = FixedGasPrice; type GasWeightMapping = crate::FixedGasWeightMapping; type WeightPerGas = WeightPerGas; type BlockHashMapping = crate::SubstrateBlockHashMapping; - type CallOrigin = EnsureAddressRoot; + type CallOrigin = EnsureAddressRoot<::AccountId>; - type WithdrawOrigin = EnsureAddressNever; + type WithdrawOrigin = EnsureAddressNever<::AccountId>; type AddressMapping = IdentityAddressMapping; type Currency = Balances; From b4295c2aecbdb5776ebf0c4ee32e9b09d82e7e16 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Mon, 1 May 2023 17:25:06 +0300 Subject: [PATCH 03/23] Little fix with sufficients to make all tests working --- frame/evm/src/lib.rs | 55 +++++++++++++++++++++++++++++++++++++++--- frame/evm/src/tests.rs | 14 +++++------ 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/frame/evm/src/lib.rs b/frame/evm/src/lib.rs index a5a5228ecc..8229e754dc 100644 --- a/frame/evm/src/lib.rs +++ b/frame/evm/src/lib.rs @@ -536,6 +536,11 @@ pub mod pallet { #[pallet::getter(fn account_nonces)] pub type AccountNonces = StorageMap<_, Blake2_128Concat, ::AccountId, ::Index, ValueQuery>; + + #[pallet::storage] + #[pallet::getter(fn account_sufficients)] + pub type AccountSufficients = + StorageMap<_, Blake2_128Concat, ::AccountId, u32, ValueQuery>; } impl Pallet { @@ -548,6 +553,50 @@ impl Pallet { pub fn inc_account_nonce(who: &::AccountId) { AccountNonces::::mutate(who, |a| *a += ::Index::one()); } + + /// Increment the self-sufficient reference counter on an account. + pub fn inc_sufficients(who: &::AccountId) { + AccountSufficients::::mutate(who, |sufficients| { + if *sufficients == 0 { + // Account is being created. + *sufficients = 1; + // Self::on_created_account(who.clone(), a); + } else { + *sufficients = sufficients.saturating_add(1); + } + }) + } + + /// Decrement the sufficients reference counter on an account. + /// + /// This *MUST* only be done once for every time you called `inc_sufficients` on `who`. + pub fn dec_sufficients(who: &::AccountId) { + AccountSufficients::::mutate_exists(who, |maybe_sufficients| { + if let Some(mut sufficients) = maybe_sufficients.take() { + if sufficients == 0 { + // Logic error - cannot decrement beyond zero. + log::error!( + target: "frame-evm", + "Logic error: Unexpected underflow in reducing sufficients", + ); + } + match sufficients { + 1 => { + // Pallet::::on_killed_account(who.clone()); + }, + x => { + sufficients = x - 1; + *maybe_sufficients = Some(sufficients); + }, + } + } else { + log::error!( + target: "frame-evm", + "Logic error: Account already dead when reducing provider", + ); + } + }) + } } /// Type alias for currency balance. @@ -730,8 +779,7 @@ impl Pallet { pub fn remove_account(address: &H160) { if >::contains_key(address) { let account_id = T::AddressMapping::into_account_id(*address); - // ATTENTION. CHECK. - // let _ = frame_system::Pallet::::dec_sufficients(&account_id); + Self::dec_sufficients(&account_id); } >::remove(address); @@ -747,8 +795,7 @@ impl Pallet { if !>::contains_key(address) { let account_id = T::AddressMapping::into_account_id(address); - // ATTENTION. CHECK. - // let _ = frame_system::Pallet::::inc_sufficients(&account_id); + Self::inc_sufficients(&account_id); } >::insert(address, code); diff --git a/frame/evm/src/tests.rs b/frame/evm/src/tests.rs index 1e8202ea43..840dfd37ce 100644 --- a/frame/evm/src/tests.rs +++ b/frame/evm/src/tests.rs @@ -414,24 +414,22 @@ fn handle_sufficient_reference() { new_test_ext().execute_with(|| { let addr = H160::from_str("1230000000000000000000000000000000000001").unwrap(); let addr_2 = H160::from_str("1234000000000000000000000000000000000001").unwrap(); - let substrate_addr = ::AddressMapping::into_account_id(addr); - let substrate_addr_2 = ::AddressMapping::into_account_id(addr_2); // Sufficients should increase when creating EVM accounts. let _ = >::insert(addr, &vec![0]); - let account = frame_system::Account::::get(substrate_addr); + let sufficients = >::get(addr); // Using storage is not correct as it leads to a sufficient reference mismatch. - assert_eq!(account.sufficients, 0); + assert_eq!(sufficients, 0); // Using the create / remove account functions is the correct way to handle it. EVM::create_account(addr_2, vec![1, 2, 3]); - let account_2 = frame_system::Account::::get(substrate_addr_2); + let sufficients = >::get(addr_2); // We increased the sufficient reference by 1. - assert_eq!(account_2.sufficients, 1); + assert_eq!(sufficients, 1); EVM::remove_account(&addr_2); - let account_2 = frame_system::Account::::get(substrate_addr_2); + let sufficients = >::get(addr_2); // We decreased the sufficient reference by 1 on removing the account. - assert_eq!(account_2.sufficients, 0); + assert_eq!(sufficients, 0); }); } From 7d1501c536d481169e0770aa0442ab98b6ce755d Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 4 May 2023 18:38:50 +0300 Subject: [PATCH 04/23] Introduce pallet-evm-system --- Cargo.lock | 25 +++++ Cargo.toml | 2 + frame/evm-system/Cargo.toml | 74 +++++++++++++++ frame/evm-system/src/lib.rs | 179 ++++++++++++++++++++++++++++++++++++ rust-toolchain.toml | 5 + 5 files changed, 285 insertions(+) create mode 100644 frame/evm-system/Cargo.toml create mode 100644 frame/evm-system/src/lib.rs create mode 100644 rust-toolchain.toml diff --git a/Cargo.lock b/Cargo.lock index 24d8a09dc7..5d6f9018d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4929,6 +4929,31 @@ dependencies = [ "sp-io", ] +[[package]] +name = "pallet-evm-system" +version = "6.0.0-dev" +dependencies = [ + "environmental", + "evm", + "fp-evm", + "frame-benchmarking", + "frame-support", + "frame-system", + "hex", + "impl-trait-for-tuples", + "log", + "pallet-balances", + "pallet-evm-precompile-simple", + "pallet-timestamp", + "parity-scale-codec", + "rlp", + "scale-info", + "sp-core", + "sp-io", + "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..123fff06a5 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 = "6.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..116561a435 --- /dev/null +++ b/frame/evm-system/Cargo.toml @@ -0,0 +1,74 @@ +[package] +name = "pallet-evm-system" +version = "6.0.0-dev" +license = "Apache-2.0" +readme = "README.md" +description = "FRAME EVM contracts pallet." +authors = { workspace = true } +edition = { workspace = true } +repository = { workspace = true } + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +environmental = { workspace = true, optional = true } +evm = { workspace = true, features = ["with-codec"] } +hex = { version = "0.4.3", default-features = false, features = ["alloc"] } +impl-trait-for-tuples = "0.2.2" +log = { version = "0.4.17", default-features = false } +rlp = { workspace = true } +scale-codec = { package = "parity-scale-codec", workspace = true } +scale-info = { workspace = true } +# Substrate +frame-benchmarking = { workspace = true, optional = true } +frame-support = { workspace = true } +frame-system = { workspace = true } +pallet-timestamp = { workspace = true } +sp-core = { workspace = true } +sp-io = { workspace = true } +sp-runtime = { workspace = true } +sp-std = { workspace = true } +# Frontier +fp-evm = { workspace = true } + +[dev-dependencies] +# Substrate +pallet-balances = { workspace = true, features = ["default"] } +pallet-evm-precompile-simple = { workspace = true } + +[features] +default = ["std"] +std = [ + "environmental?/std", + "evm/std", + "evm/with-serde", + "hex/std", + "log/std", + "rlp/std", + "scale-codec/std", + "scale-info/std", + # Substrate + "frame-benchmarking/std", + "frame-support/std", + "frame-system/std", + "pallet-timestamp/std", + "sp-core/std", + "sp-io/std", + "sp-runtime/std", + "sp-std/std", + # Frontier + "fp-evm/std", +] +runtime-benchmarks = [ + "frame-benchmarking/runtime-benchmarks", + "frame-support/runtime-benchmarks", + "frame-system/runtime-benchmarks", + "pallet-timestamp/runtime-benchmarks", +] +try-runtime = [ + "frame-support/try-runtime", + "frame-system/try-runtime", + "pallet-timestamp/try-runtime", +] +forbid-evm-reentrancy = ["dep:environmental"] diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs new file mode 100644 index 0000000000..738fcd63c8 --- /dev/null +++ b/frame/evm-system/src/lib.rs @@ -0,0 +1,179 @@ +// 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; + +pub use evm::{ + Config as EvmConfig, Context, ExitError, ExitFatal, ExitReason, ExitRevert, ExitSucceed, +}; +pub use fp_evm::{ + Account, CallInfo, CreateInfo, ExecutionInfo, FeeCalculator, InvalidEvmTransactionError, + LinearCostPrecompile, Log, Precompile, PrecompileFailure, PrecompileHandle, PrecompileOutput, + PrecompileResult, PrecompileSet, Vicinity, +}; + +pub use pallet::*; + +#[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; + + /// 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>; + } + + #[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 }, + } + + #[pallet::storage] + #[pallet::getter(fn account_nonces)] + pub type AccountNonces = + StorageMap<_, Blake2_128Concat, ::AccountId, ::Index, ValueQuery>; + + #[pallet::storage] + #[pallet::getter(fn account_sufficients)] + pub type AccountSufficients = + StorageMap<_, Blake2_128Concat, ::AccountId, u32, ValueQuery>; +} + +impl Pallet { + /// An account is being created. + pub 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 { + AccountNonces::::get(who) + } + + /// Increment a particular account's nonce by 1. + pub fn inc_account_nonce(who: &::AccountId) { + AccountNonces::::mutate(who, |a| *a += ::Index::one()); + } + + /// Increment the self-sufficient reference counter on an account. + pub fn inc_sufficients(who: &::AccountId) { + AccountSufficients::::mutate(who, |sufficients| { + if *sufficients == 0 { + // Account is being created. + *sufficients = 1; + Self::on_created_account(who.clone()); + } else { + *sufficients = sufficients.saturating_add(1); + } + }) + } + + /// Decrement the sufficients reference counter on an account. + /// + /// This *MUST* only be done once for every time you called `inc_sufficients` on `who`. + pub fn dec_sufficients(who: &::AccountId) { + AccountSufficients::::mutate_exists(who, |maybe_sufficients| { + if let Some(mut sufficients) = maybe_sufficients.take() { + if sufficients == 0 { + // Logic error - cannot decrement beyond zero. + log::error!( + target: "frame-evm", + "Logic error: Unexpected underflow in reducing sufficients", + ); + } + match sufficients { + 1 => { + Pallet::::on_killed_account(who.clone()); + }, + x => { + sufficients = x - 1; + *maybe_sufficients = Some(sufficients); + }, + } + } else { + log::error!( + target: "frame-evm", + "Logic error: Account already dead when reducing provider", + ); + } + }) + } +} + +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); +} diff --git a/rust-toolchain.toml b/rust-toolchain.toml new file mode 100644 index 0000000000..420568123b --- /dev/null +++ b/rust-toolchain.toml @@ -0,0 +1,5 @@ +[toolchain] +channel = "nightly-2023-01-11" +components = ["rustfmt", "clippy"] +targets = ["wasm32-unknown-unknown"] +profile = "minimal" From 364f78c1e373d6c30db8dab06ea5486b2cdd6c6a Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 4 May 2023 18:49:24 +0300 Subject: [PATCH 05/23] Use full info account representation --- frame/evm-system/src/lib.rs | 104 +++++++++++++++++++++++++----------- 1 file changed, 74 insertions(+), 30 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 738fcd63c8..018ecc07ea 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -20,8 +20,9 @@ // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] -use sp_runtime::traits::One; - +use sp_runtime::{traits::One, RuntimeDebug}; +use scale_codec::{Encode, Decode, MaxEncodedLen}; +use scale_info::TypeInfo; pub use evm::{ Config as EvmConfig, Context, ExitError, ExitFatal, ExitReason, ExitRevert, ExitSucceed, }; @@ -33,6 +34,43 @@ pub use fp_evm::{ pub use pallet::*; +/// Type used to encode the number of references an account has. +pub type RefCount = u32; + +/// Some resultant status relevant to incrementing a provider/self-sufficient reference. +#[derive(Eq, PartialEq, RuntimeDebug)] +pub enum IncRefStatus { + /// Account was created. + Created, + /// Account already existed. + Existed, +} + +/// Some resultant status relevant to decrementing a provider/self-sufficient reference. +#[derive(Eq, PartialEq, RuntimeDebug)] +pub enum DecRefStatus { + /// Account was destroyed. + Reaped, + /// Account still exists. + Exists, +} + +/// Information of an account. +#[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 number of other modules that currently depend on this account's existence. The account + /// cannot be reaped until this is zero. + pub consumers: RefCount, + /// The number of other modules that allow this account to exist. The account may not be reaped + /// until this and `sufficients` are both zero. + pub providers: RefCount, + /// The number of modules that allow this account to exist for their own purposes only. The + /// account may not be reaped until this and `providers` are both zero. + pub sufficients: RefCount, +} + #[frame_support::pallet] pub mod pallet { use super::*; @@ -80,6 +118,17 @@ pub mod pallet { 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>, + ValueQuery, + >; + #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { @@ -88,16 +137,6 @@ pub mod pallet { /// An account was reaped. KilledAccount { account: ::AccountId }, } - - #[pallet::storage] - #[pallet::getter(fn account_nonces)] - pub type AccountNonces = - StorageMap<_, Blake2_128Concat, ::AccountId, ::Index, ValueQuery>; - - #[pallet::storage] - #[pallet::getter(fn account_sufficients)] - pub type AccountSufficients = - StorageMap<_, Blake2_128Concat, ::AccountId, u32, ValueQuery>; } impl Pallet { @@ -115,23 +154,25 @@ impl Pallet { /// Retrieve the account transaction counter from storage. pub fn account_nonce(who: &::AccountId) -> ::Index { - AccountNonces::::get(who) + FullAccount::::get(who).nonce } /// Increment a particular account's nonce by 1. pub fn inc_account_nonce(who: &::AccountId) { - AccountNonces::::mutate(who, |a| *a += ::Index::one()); + FullAccount::::mutate(who, |a| a.nonce += ::Index::one()); } /// Increment the self-sufficient reference counter on an account. - pub fn inc_sufficients(who: &::AccountId) { - AccountSufficients::::mutate(who, |sufficients| { - if *sufficients == 0 { + pub fn inc_sufficients(who: &::AccountId) -> IncRefStatus { + FullAccount::::mutate(who, |a| { + if a.providers + a.sufficients == 0 { // Account is being created. - *sufficients = 1; + a.sufficients = 1; Self::on_created_account(who.clone()); + IncRefStatus::Created } else { - *sufficients = sufficients.saturating_add(1); + a.sufficients = a.sufficients.saturating_add(1); + IncRefStatus::Existed } }) } @@ -139,30 +180,33 @@ impl Pallet { /// Decrement the sufficients reference counter on an account. /// /// This *MUST* only be done once for every time you called `inc_sufficients` on `who`. - pub fn dec_sufficients(who: &::AccountId) { - AccountSufficients::::mutate_exists(who, |maybe_sufficients| { - if let Some(mut sufficients) = maybe_sufficients.take() { - if sufficients == 0 { + pub fn dec_sufficients(who: &::AccountId) -> DecRefStatus { + FullAccount::::mutate_exists(who, |maybe_account| { + if let Some(mut account) = maybe_account.take() { + if account.sufficients == 0 { // Logic error - cannot decrement beyond zero. log::error!( - target: "frame-evm", + target: "frame-evm-system", "Logic error: Unexpected underflow in reducing sufficients", ); } - match sufficients { - 1 => { + match (account.sufficients, account.providers) { + (0, 0) | (1, 0) => { Pallet::::on_killed_account(who.clone()); + DecRefStatus::Reaped }, - x => { - sufficients = x - 1; - *maybe_sufficients = Some(sufficients); + (x, _) => { + account.sufficients = x - 1; + *maybe_account = Some(account); + DecRefStatus::Exists }, } } else { log::error!( - target: "frame-evm", + target: "frame-evm-system", "Logic error: Account already dead when reducing provider", ); + DecRefStatus::Reaped } }) } From 531917b7ca9688fb865d41908ad0ba0f1b36c960 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 4 May 2023 19:23:42 +0300 Subject: [PATCH 06/23] Use AccountProvider for frame-evm --- frame/evm/src/lib.rs | 135 ++++++++++--------------------------------- 1 file changed, 29 insertions(+), 106 deletions(-) diff --git a/frame/evm/src/lib.rs b/frame/evm/src/lib.rs index 8229e754dc..6934b39676 100644 --- a/frame/evm/src/lib.rs +++ b/frame/evm/src/lib.rs @@ -76,7 +76,7 @@ use frame_system::RawOrigin; use impl_trait_for_tuples::impl_for_tuples; use sp_core::{Hasher, H160, H256, U256}; use sp_runtime::{ - traits::{BadOrigin, Saturating, UniqueSaturatedInto, Zero, One}, + traits::{BadOrigin, Saturating, UniqueSaturatedInto, Zero}, AccountId32, DispatchErrorWithPostInfo, }; use sp_std::{cmp::min, vec::Vec}; @@ -102,8 +102,6 @@ pub mod pallet { use super::*; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; - use sp_runtime::traits::{MaybeDisplay, AtLeast32Bit}; - use sp_std::fmt::Debug; #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] @@ -112,26 +110,8 @@ pub mod pallet { #[pallet::config] pub trait Config: frame_system::Config + pallet_timestamp::Config { - /// 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; + /// Account info provider. + type AccountProvider: AccountProvider; /// Calculator for current gas price. type FeeCalculator: FeeCalculator; @@ -148,12 +128,12 @@ pub mod pallet { /// Allow the origin to call on behalf of given address. type CallOrigin: EnsureAddressOrigin; /// Allow the origin to withdraw on behalf of given address. - type WithdrawOrigin: EnsureAddressOrigin::AccountId>; + type WithdrawOrigin: EnsureAddressOrigin::AccountId>; /// Mapping from address to account id. - type AddressMapping: AddressMapping<::AccountId>; + type AddressMapping: AddressMapping<::AccountId>; /// Currency type for withdraw and balance storage. - type Currency: Currency<::AccountId> + Inspect<::AccountId>; + type Currency: Currency<::AccountId> + Inspect<::AccountId>; /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -509,7 +489,7 @@ pub mod pallet { MAX_ACCOUNT_NONCE, UniqueSaturatedInto::::unique_saturated_into(account.nonce), ) { - Pallet::::inc_account_nonce(&account_id); + T::AccountProvider::inc_account_nonce(&account_id); } T::Currency::deposit_creating(&account_id, account.balance.unique_saturated_into()); @@ -531,81 +511,15 @@ pub mod pallet { #[pallet::getter(fn account_storages)] pub type AccountStorages = StorageDoubleMap<_, Blake2_128Concat, H160, Blake2_128Concat, H256, H256, ValueQuery>; - - #[pallet::storage] - #[pallet::getter(fn account_nonces)] - pub type AccountNonces = - StorageMap<_, Blake2_128Concat, ::AccountId, ::Index, ValueQuery>; - - #[pallet::storage] - #[pallet::getter(fn account_sufficients)] - pub type AccountSufficients = - StorageMap<_, Blake2_128Concat, ::AccountId, u32, ValueQuery>; -} - -impl Pallet { - /// Retrieve the account transaction counter from storage. - pub fn account_nonce(who: &::AccountId) -> ::Index { - AccountNonces::::get(who) - } - - /// Increment a particular account's nonce by 1. - pub fn inc_account_nonce(who: &::AccountId) { - AccountNonces::::mutate(who, |a| *a += ::Index::one()); - } - - /// Increment the self-sufficient reference counter on an account. - pub fn inc_sufficients(who: &::AccountId) { - AccountSufficients::::mutate(who, |sufficients| { - if *sufficients == 0 { - // Account is being created. - *sufficients = 1; - // Self::on_created_account(who.clone(), a); - } else { - *sufficients = sufficients.saturating_add(1); - } - }) - } - - /// Decrement the sufficients reference counter on an account. - /// - /// This *MUST* only be done once for every time you called `inc_sufficients` on `who`. - pub fn dec_sufficients(who: &::AccountId) { - AccountSufficients::::mutate_exists(who, |maybe_sufficients| { - if let Some(mut sufficients) = maybe_sufficients.take() { - if sufficients == 0 { - // Logic error - cannot decrement beyond zero. - log::error!( - target: "frame-evm", - "Logic error: Unexpected underflow in reducing sufficients", - ); - } - match sufficients { - 1 => { - // Pallet::::on_killed_account(who.clone()); - }, - x => { - sufficients = x - 1; - *maybe_sufficients = Some(sufficients); - }, - } - } else { - log::error!( - target: "frame-evm", - "Logic error: Account already dead when reducing provider", - ); - } - }) - } } /// Type alias for currency balance. pub type BalanceOf = - <::Currency as Currency<::AccountId>>::Balance; + <::Currency as Currency<<::AccountProvider as AccountProvider>::AccountId>>::Balance; /// Type alias for negative imbalance during fees type NegativeImbalanceOf = - ::AccountId>>::NegativeImbalance; + ::AccountProvider as AccountProvider>::AccountId>>::NegativeImbalance; pub trait EnsureAddressOrigin { /// Success return type. @@ -779,7 +693,7 @@ impl Pallet { pub fn remove_account(address: &H160) { if >::contains_key(address) { let account_id = T::AddressMapping::into_account_id(*address); - Self::dec_sufficients(&account_id); + T::AccountProvider::dec_sufficients(&account_id); } >::remove(address); @@ -795,7 +709,7 @@ impl Pallet { if !>::contains_key(address) { let account_id = T::AddressMapping::into_account_id(address); - Self::inc_sufficients(&account_id); + T::AccountProvider::inc_sufficients(&account_id); } >::insert(address, code); @@ -805,7 +719,7 @@ impl Pallet { pub fn account_basic(address: &H160) -> (Account, frame_support::weights::Weight) { let account_id = T::AddressMapping::into_account_id(*address); - let nonce = Pallet::::account_nonce(&account_id); + let nonce = T::AccountProvider::account_nonce(&account_id); // keepalive `true` takes into account ExistentialDeposit as part of what's considered liquid balance. let balance = T::Currency::reducible_balance(&account_id, true); @@ -861,17 +775,17 @@ pub struct EVMCurrencyAdapter(sp_std::marker::PhantomData<(C, OU)>); impl OnChargeEVMTransaction for EVMCurrencyAdapter where T: Config, - C: Currency<::AccountId>, + C: Currency<<::AccountProvider as AccountProvider>::AccountId>, C::PositiveImbalance: Imbalance< - ::AccountId>>::Balance, + ::AccountProvider as AccountProvider>::AccountId>>::Balance, Opposite = C::NegativeImbalance, >, C::NegativeImbalance: Imbalance< - ::AccountId>>::Balance, + ::AccountProvider as AccountProvider>::AccountId>>::Balance, Opposite = C::PositiveImbalance, >, OU: OnUnbalanced>, - U256: UniqueSaturatedInto<::AccountId>>::Balance>, + U256: UniqueSaturatedInto<::AccountProvider as AccountProvider>::AccountId>>::Balance>, { // Kept type as Option to satisfy bound of Default type LiquidityInfo = Option>; @@ -955,10 +869,10 @@ where impl OnChargeEVMTransaction for () where T: Config, - ::AccountId>>::PositiveImbalance: - Imbalance<::AccountId>>::Balance, Opposite = ::AccountId>>::NegativeImbalance>, - ::AccountId>>::NegativeImbalance: -Imbalance<::AccountId>>::Balance, Opposite = ::AccountId>>::PositiveImbalance>, + ::AccountProvider as AccountProvider>::AccountId>>::PositiveImbalance: + Imbalance<::AccountProvider as AccountProvider>::AccountId>>::Balance, Opposite = ::AccountProvider as AccountProvider>::AccountId>>::NegativeImbalance>, + ::AccountProvider as AccountProvider>::AccountId>>::NegativeImbalance: +Imbalance<::AccountProvider as AccountProvider>::AccountId>>::Balance, Opposite = ::AccountProvider as AccountProvider>::AccountId>>::PositiveImbalance>, U256: UniqueSaturatedInto>, { @@ -1002,3 +916,12 @@ impl OnCreate for Tuple { )*) } } + +pub trait AccountProvider { + type AccountId; + + fn account_nonce(who: &Self::AccountId) -> u128; + fn inc_account_nonce(who: &Self::AccountId); + fn inc_sufficients(who: &Self::AccountId); + fn dec_sufficients(who: &Self::AccountId); +} From 4f74dc0e3156d18905142ad76102bd410f0e8974 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 4 May 2023 19:42:53 +0300 Subject: [PATCH 07/23] Add NativeSystemAccountProvider default implementation --- frame/evm/src/lib.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/frame/evm/src/lib.rs b/frame/evm/src/lib.rs index 6934b39676..a07ee2fde9 100644 --- a/frame/evm/src/lib.rs +++ b/frame/evm/src/lib.rs @@ -919,9 +919,32 @@ impl OnCreate for Tuple { pub trait AccountProvider { type AccountId; + type Index; - fn account_nonce(who: &Self::AccountId) -> u128; + fn account_nonce(who: &Self::AccountId) -> Self::Index; fn inc_account_nonce(who: &Self::AccountId); fn inc_sufficients(who: &Self::AccountId); fn dec_sufficients(who: &Self::AccountId); } + +pub struct NativeSystemAccountProvider(sp_std::marker::PhantomData); + +impl AccountProvider for NativeSystemAccountProvider { + type AccountId = ::AccountId; + type Index = ::Index; + + fn account_nonce(who: &Self::AccountId) -> Self::Index { + frame_system::Pallet::::account_nonce(&who) + } + + fn inc_account_nonce(who: &Self::AccountId) { + frame_system::Pallet::::inc_account_nonce(&who) + } + + fn inc_sufficients(who: &Self::AccountId) { + let _ = frame_system::Pallet::::inc_sufficients(&who); + } + fn dec_sufficients(who: &Self::AccountId) { + let _ = frame_system::Pallet::::dec_sufficients(&who); + } +} From cc5db1708e5c86e5f8825407251718a115fbc7a3 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 4 May 2023 20:02:12 +0300 Subject: [PATCH 08/23] Restrict Index type --- frame/evm/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/evm/src/lib.rs b/frame/evm/src/lib.rs index a07ee2fde9..dba79e5cf5 100644 --- a/frame/evm/src/lib.rs +++ b/frame/evm/src/lib.rs @@ -76,7 +76,7 @@ use frame_system::RawOrigin; use impl_trait_for_tuples::impl_for_tuples; use sp_core::{Hasher, H160, H256, U256}; use sp_runtime::{ - traits::{BadOrigin, Saturating, UniqueSaturatedInto, Zero}, + traits::{BadOrigin, Saturating, UniqueSaturatedInto, AtLeast32Bit, Zero}, AccountId32, DispatchErrorWithPostInfo, }; use sp_std::{cmp::min, vec::Vec}; @@ -919,7 +919,7 @@ impl OnCreate for Tuple { pub trait AccountProvider { type AccountId; - type Index; + type Index: AtLeast32Bit; fn account_nonce(who: &Self::AccountId) -> Self::Index; fn inc_account_nonce(who: &Self::AccountId); From e8e6d50bf5b05c6ee837efe3d5ac4055e5dcc364 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 4 May 2023 20:05:24 +0300 Subject: [PATCH 09/23] Fix frame evm mock --- frame/evm/src/mock.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/frame/evm/src/mock.rs b/frame/evm/src/mock.rs index 2410b47cbd..ad886616db 100644 --- a/frame/evm/src/mock.rs +++ b/frame/evm/src/mock.rs @@ -132,16 +132,15 @@ parameter_types! { pub MockPrecompiles: MockPrecompileSet = MockPrecompileSet; } impl crate::Config for Test { - type AccountId = H160; - type Index = u64; + type AccountProvider = crate::NativeSystemAccountProvider; type FeeCalculator = FixedGasPrice; type GasWeightMapping = crate::FixedGasWeightMapping; type WeightPerGas = WeightPerGas; type BlockHashMapping = crate::SubstrateBlockHashMapping; - type CallOrigin = EnsureAddressRoot<::AccountId>; + type CallOrigin = EnsureAddressRoot<::AccountId>; - type WithdrawOrigin = EnsureAddressNever<::AccountId>; + type WithdrawOrigin = EnsureAddressNever<::AccountId>; type AddressMapping = IdentityAddressMapping; type Currency = Balances; From ef440b07f20e0b62d8de48d21d1f899778c03aa8 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 4 May 2023 20:07:24 +0300 Subject: [PATCH 10/23] Apply changes to runner stack logic --- frame/evm/src/runner/stack.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/evm/src/runner/stack.rs b/frame/evm/src/runner/stack.rs index cbe18e6cb6..02d4848956 100644 --- a/frame/evm/src/runner/stack.rs +++ b/frame/evm/src/runner/stack.rs @@ -20,7 +20,7 @@ use crate::{ runner::Runner as RunnerT, AccountCodes, AccountStorages, AddressMapping, BalanceOf, BlockHashMapping, Config, Error, Event, FeeCalculator, OnChargeEVMTransaction, OnCreate, - Pallet, RunnerError, + Pallet, RunnerError, AccountProvider }; use evm::{ backend::Backend as BackendT, @@ -721,7 +721,7 @@ where fn inc_nonce(&mut self, address: H160) { let account_id = T::AddressMapping::into_account_id(address); - Pallet::::inc_account_nonce(&account_id); + T::AccountProvider::inc_account_nonce(&account_id); } fn set_storage(&mut self, address: H160, index: H256, value: H256) { From 54ad15d1638cee03763cbded6f3ab8aa548fbe89 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 4 May 2023 20:10:15 +0300 Subject: [PATCH 11/23] Revert changes applied to test with sufficients --- frame/evm/src/tests.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/frame/evm/src/tests.rs b/frame/evm/src/tests.rs index 840dfd37ce..1e8202ea43 100644 --- a/frame/evm/src/tests.rs +++ b/frame/evm/src/tests.rs @@ -414,22 +414,24 @@ fn handle_sufficient_reference() { new_test_ext().execute_with(|| { let addr = H160::from_str("1230000000000000000000000000000000000001").unwrap(); let addr_2 = H160::from_str("1234000000000000000000000000000000000001").unwrap(); + let substrate_addr = ::AddressMapping::into_account_id(addr); + let substrate_addr_2 = ::AddressMapping::into_account_id(addr_2); // Sufficients should increase when creating EVM accounts. let _ = >::insert(addr, &vec![0]); - let sufficients = >::get(addr); + let account = frame_system::Account::::get(substrate_addr); // Using storage is not correct as it leads to a sufficient reference mismatch. - assert_eq!(sufficients, 0); + assert_eq!(account.sufficients, 0); // Using the create / remove account functions is the correct way to handle it. EVM::create_account(addr_2, vec![1, 2, 3]); - let sufficients = >::get(addr_2); + let account_2 = frame_system::Account::::get(substrate_addr_2); // We increased the sufficient reference by 1. - assert_eq!(sufficients, 1); + assert_eq!(account_2.sufficients, 1); EVM::remove_account(&addr_2); - let sufficients = >::get(addr_2); + let account_2 = frame_system::Account::::get(substrate_addr_2); // We decreased the sufficient reference by 1 on removing the account. - assert_eq!(sufficients, 0); + assert_eq!(account_2.sufficients, 0); }); } From ff023372dfbbd805b99c396430ca7d0469c15f11 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 4 May 2023 20:13:07 +0300 Subject: [PATCH 12/23] Apply account provider logic at dispatch evm precompile --- frame/evm/precompile/dispatch/src/lib.rs | 2 +- frame/evm/precompile/dispatch/src/mock.rs | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/frame/evm/precompile/dispatch/src/lib.rs b/frame/evm/precompile/dispatch/src/lib.rs index a212add8f0..d6f9cd2b42 100644 --- a/frame/evm/precompile/dispatch/src/lib.rs +++ b/frame/evm/precompile/dispatch/src/lib.rs @@ -49,7 +49,7 @@ impl Precompile for Dispatch where T: pallet_evm::Config, T::RuntimeCall: Dispatchable + GetDispatchInfo + Decode, - ::RuntimeOrigin: From::AccountId>>, + ::RuntimeOrigin: From::AccountProvider as pallet_evm::AccountProvider>::AccountId>>, DecodeLimit: Get, { fn execute(handle: &mut impl PrecompileHandle) -> PrecompileResult { diff --git a/frame/evm/precompile/dispatch/src/mock.rs b/frame/evm/precompile/dispatch/src/mock.rs index ee2a053e56..7163d8ea2d 100644 --- a/frame/evm/precompile/dispatch/src/mock.rs +++ b/frame/evm/precompile/dispatch/src/mock.rs @@ -139,16 +139,15 @@ parameter_types! { pub WeightPerGas: Weight = Weight::from_ref_time(20_000); } impl pallet_evm::Config for Test { - type AccountId = H160; - type Index = u64; + type AccountProvider = pallet_evm::NativeSystemAccountProvider; type FeeCalculator = FixedGasPrice; type GasWeightMapping = pallet_evm::FixedGasWeightMapping; type WeightPerGas = WeightPerGas; type BlockHashMapping = pallet_evm::SubstrateBlockHashMapping; - type CallOrigin = EnsureAddressRoot<::AccountId>; + type CallOrigin = EnsureAddressRoot<::AccountId>; - type WithdrawOrigin = EnsureAddressNever<::AccountId>; + type WithdrawOrigin = EnsureAddressNever<::AccountId>; type AddressMapping = IdentityAddressMapping; type Currency = Balances; From 9503d2a3a548f0c50e0564c2548de757e541a63f Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 4 May 2023 20:13:51 +0300 Subject: [PATCH 13/23] Fix mock ethereum --- frame/ethereum/src/mock.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/ethereum/src/mock.rs b/frame/ethereum/src/mock.rs index 490bfb6697..9614fb8a0c 100644 --- a/frame/ethereum/src/mock.rs +++ b/frame/ethereum/src/mock.rs @@ -152,8 +152,7 @@ impl AddressMapping for HashedAddressMapping { } impl pallet_evm::Config for Test { - type AccountId = AccountId32; - type Index = u64; + type AccountProvider = pallet_evm::NativeSystemAccountProvider; type FeeCalculator = FixedGasPrice; type GasWeightMapping = pallet_evm::FixedGasWeightMapping; type WeightPerGas = WeightPerGas; From d155faabb34cda1a7c92012bd718a88fc8489a3c Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 4 May 2023 20:14:43 +0300 Subject: [PATCH 14/23] Fix frontier template --- template/runtime/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/template/runtime/src/lib.rs b/template/runtime/src/lib.rs index c4c35b9b7f..2316298271 100644 --- a/template/runtime/src/lib.rs +++ b/template/runtime/src/lib.rs @@ -326,8 +326,7 @@ parameter_types! { } impl pallet_evm::Config for Runtime { - type AccountId = AccountId; - type Index = Index; + type AccountProvider = pallet_evm::NativeSystemAccountProvider; type FeeCalculator = BaseFee; type GasWeightMapping = pallet_evm::FixedGasWeightMapping; type WeightPerGas = WeightPerGas; From b1600055092ac47fe798be8796f48ede9f344f81 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Thu, 4 May 2023 20:16:03 +0300 Subject: [PATCH 15/23] Rename description --- 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 116561a435..c72540a2b5 100644 --- a/frame/evm-system/Cargo.toml +++ b/frame/evm-system/Cargo.toml @@ -3,7 +3,7 @@ name = "pallet-evm-system" version = "6.0.0-dev" license = "Apache-2.0" readme = "README.md" -description = "FRAME EVM contracts pallet." +description = "FRAME EVM SYSTEM accounts pallet." authors = { workspace = true } edition = { workspace = true } repository = { workspace = true } From 3f6105d233aa8cc7f6502599e68a319a1f35497a Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Fri, 5 May 2023 17:58:45 +0300 Subject: [PATCH 16/23] Fix deps --- Cargo.lock | 14 +------------ Cargo.toml | 2 +- frame/evm-system/Cargo.toml | 42 +------------------------------------ frame/evm-system/src/lib.rs | 8 ------- 4 files changed, 3 insertions(+), 63 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d6f9018d6..e18e0f74bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4931,25 +4931,13 @@ dependencies = [ [[package]] name = "pallet-evm-system" -version = "6.0.0-dev" +version = "1.0.0-dev" dependencies = [ - "environmental", - "evm", - "fp-evm", - "frame-benchmarking", "frame-support", "frame-system", - "hex", - "impl-trait-for-tuples", "log", - "pallet-balances", - "pallet-evm-precompile-simple", - "pallet-timestamp", "parity-scale-codec", - "rlp", "scale-info", - "sp-core", - "sp-io", "sp-runtime", "sp-std", ] diff --git a/Cargo.toml b/Cargo.toml index 123fff06a5..8ebea92164 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -138,7 +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 = "6.0.0-dev", path = "frame/evm-system", 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 index c72540a2b5..c6c9188fdf 100644 --- a/frame/evm-system/Cargo.toml +++ b/frame/evm-system/Cargo.toml @@ -1,8 +1,7 @@ [package] name = "pallet-evm-system" -version = "6.0.0-dev" +version = "1.0.0-dev" license = "Apache-2.0" -readme = "README.md" description = "FRAME EVM SYSTEM accounts pallet." authors = { workspace = true } edition = { workspace = true } @@ -12,63 +11,24 @@ repository = { workspace = true } targets = ["x86_64-unknown-linux-gnu"] [dependencies] -environmental = { workspace = true, optional = true } -evm = { workspace = true, features = ["with-codec"] } -hex = { version = "0.4.3", default-features = false, features = ["alloc"] } -impl-trait-for-tuples = "0.2.2" log = { version = "0.4.17", default-features = false } -rlp = { workspace = true } scale-codec = { package = "parity-scale-codec", workspace = true } scale-info = { workspace = true } # Substrate -frame-benchmarking = { workspace = true, optional = true } frame-support = { workspace = true } frame-system = { workspace = true } -pallet-timestamp = { workspace = true } -sp-core = { workspace = true } -sp-io = { workspace = true } sp-runtime = { workspace = true } sp-std = { workspace = true } -# Frontier -fp-evm = { workspace = true } - -[dev-dependencies] -# Substrate -pallet-balances = { workspace = true, features = ["default"] } -pallet-evm-precompile-simple = { workspace = true } [features] default = ["std"] std = [ - "environmental?/std", - "evm/std", - "evm/with-serde", - "hex/std", "log/std", - "rlp/std", "scale-codec/std", "scale-info/std", # Substrate - "frame-benchmarking/std", "frame-support/std", "frame-system/std", - "pallet-timestamp/std", - "sp-core/std", - "sp-io/std", "sp-runtime/std", "sp-std/std", - # Frontier - "fp-evm/std", -] -runtime-benchmarks = [ - "frame-benchmarking/runtime-benchmarks", - "frame-support/runtime-benchmarks", - "frame-system/runtime-benchmarks", - "pallet-timestamp/runtime-benchmarks", -] -try-runtime = [ - "frame-support/try-runtime", - "frame-system/try-runtime", - "pallet-timestamp/try-runtime", ] -forbid-evm-reentrancy = ["dep:environmental"] diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 018ecc07ea..5514cb3073 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -23,14 +23,6 @@ use sp_runtime::{traits::One, RuntimeDebug}; use scale_codec::{Encode, Decode, MaxEncodedLen}; use scale_info::TypeInfo; -pub use evm::{ - Config as EvmConfig, Context, ExitError, ExitFatal, ExitReason, ExitRevert, ExitSucceed, -}; -pub use fp_evm::{ - Account, CallInfo, CreateInfo, ExecutionInfo, FeeCalculator, InvalidEvmTransactionError, - LinearCostPrecompile, Log, Precompile, PrecompileFailure, PrecompileHandle, PrecompileOutput, - PrecompileResult, PrecompileSet, Vicinity, -}; pub use pallet::*; From 5b3325f9410e4fc0373f50701e7be41a2ca816ac Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Fri, 5 May 2023 18:18:56 +0300 Subject: [PATCH 17/23] Use create and remove accounts logic at account provider --- frame/evm/src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/frame/evm/src/lib.rs b/frame/evm/src/lib.rs index dba79e5cf5..f0531ffb15 100644 --- a/frame/evm/src/lib.rs +++ b/frame/evm/src/lib.rs @@ -693,7 +693,7 @@ impl Pallet { pub fn remove_account(address: &H160) { if >::contains_key(address) { let account_id = T::AddressMapping::into_account_id(*address); - T::AccountProvider::dec_sufficients(&account_id); + T::AccountProvider::remove_account(&account_id); } >::remove(address); @@ -709,7 +709,7 @@ impl Pallet { if !>::contains_key(address) { let account_id = T::AddressMapping::into_account_id(address); - T::AccountProvider::inc_sufficients(&account_id); + T::AccountProvider::create_account(&account_id); } >::insert(address, code); @@ -921,10 +921,10 @@ pub trait AccountProvider { type AccountId; type Index: AtLeast32Bit; + fn create_account(who: &Self::AccountId); + fn remove_account(who: &Self::AccountId); fn account_nonce(who: &Self::AccountId) -> Self::Index; fn inc_account_nonce(who: &Self::AccountId); - fn inc_sufficients(who: &Self::AccountId); - fn dec_sufficients(who: &Self::AccountId); } pub struct NativeSystemAccountProvider(sp_std::marker::PhantomData); @@ -941,10 +941,10 @@ impl AccountProvider for NativeSystemAccountProvider { frame_system::Pallet::::inc_account_nonce(&who) } - fn inc_sufficients(who: &Self::AccountId) { + fn create_account(who: &Self::AccountId) { let _ = frame_system::Pallet::::inc_sufficients(&who); } - fn dec_sufficients(who: &Self::AccountId) { + fn remove_account(who: &Self::AccountId) { let _ = frame_system::Pallet::::dec_sufficients(&who); } } From 0dc1774d87b8c69bb7e5f16d7fc5b77ebca59dad Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Fri, 5 May 2023 18:46:34 +0300 Subject: [PATCH 18/23] Remove redundant providers and sufficients --- frame/evm-system/src/lib.rs | 105 +++++++++++------------------------- 1 file changed, 31 insertions(+), 74 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 5514cb3073..4e2011b5e1 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -20,47 +20,27 @@ // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] -use sp_runtime::{traits::One, RuntimeDebug}; -use scale_codec::{Encode, Decode, MaxEncodedLen}; -use scale_info::TypeInfo; +use sp_runtime::{traits::{One, Zero}, RuntimeDebug}; +use scale_codec::MaxEncodedLen; pub use pallet::*; -/// Type used to encode the number of references an account has. -pub type RefCount = u32; - -/// Some resultant status relevant to incrementing a provider/self-sufficient reference. +/// Account creation result status. #[derive(Eq, PartialEq, RuntimeDebug)] -pub enum IncRefStatus { +pub enum AccountCreationStatus { /// Account was created. Created, /// Account already existed. Existed, } -/// Some resultant status relevant to decrementing a provider/self-sufficient reference. +/// Account removing result status. #[derive(Eq, PartialEq, RuntimeDebug)] -pub enum DecRefStatus { - /// Account was destroyed. - Reaped, - /// Account still exists. - Exists, -} - -/// Information of an account. -#[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 number of other modules that currently depend on this account's existence. The account - /// cannot be reaped until this is zero. - pub consumers: RefCount, - /// The number of other modules that allow this account to exist. The account may not be reaped - /// until this and `sufficients` are both zero. - pub providers: RefCount, - /// The number of modules that allow this account to exist for their own purposes only. The - /// account may not be reaped until this and `providers` are both zero. - pub sufficients: RefCount, +pub enum AccountRemovingStatus { + /// Account was removed. + Removed, + /// Account doesn't exist. + NotExist, } #[frame_support::pallet] @@ -117,7 +97,7 @@ pub mod pallet { _, Blake2_128Concat, ::AccountId, - AccountInfo<::Index>, + ::Index, ValueQuery, >; @@ -133,7 +113,7 @@ pub mod pallet { impl Pallet { /// An account is being created. - pub fn on_created_account(who: ::AccountId) { + fn on_created_account(who: ::AccountId) { ::OnNewAccount::on_new_account(&who); Self::deposit_event(Event::NewAccount { account: who }); } @@ -146,61 +126,38 @@ impl Pallet { /// Retrieve the account transaction counter from storage. pub fn account_nonce(who: &::AccountId) -> ::Index { - FullAccount::::get(who).nonce + FullAccount::::get(who) } /// Increment a particular account's nonce by 1. pub fn inc_account_nonce(who: &::AccountId) { - FullAccount::::mutate(who, |a| a.nonce += ::Index::one()); + FullAccount::::mutate(who, |nonce| *nonce += ::Index::one()); } - /// Increment the self-sufficient reference counter on an account. - pub fn inc_sufficients(who: &::AccountId) -> IncRefStatus { - FullAccount::::mutate(who, |a| { - if a.providers + a.sufficients == 0 { + /// Create an account. + pub fn create_account(who: &::AccountId) -> AccountCreationStatus { + FullAccount::::mutate(who, |nonce| { + if *nonce == Zero::zero() { // Account is being created. - a.sufficients = 1; Self::on_created_account(who.clone()); - IncRefStatus::Created + *nonce = One::one(); + AccountCreationStatus::Created } else { - a.sufficients = a.sufficients.saturating_add(1); - IncRefStatus::Existed + AccountCreationStatus::Existed } }) } - /// Decrement the sufficients reference counter on an account. - /// - /// This *MUST* only be done once for every time you called `inc_sufficients` on `who`. - pub fn dec_sufficients(who: &::AccountId) -> DecRefStatus { - FullAccount::::mutate_exists(who, |maybe_account| { - if let Some(mut account) = maybe_account.take() { - if account.sufficients == 0 { - // Logic error - cannot decrement beyond zero. - log::error!( - target: "frame-evm-system", - "Logic error: Unexpected underflow in reducing sufficients", - ); - } - match (account.sufficients, account.providers) { - (0, 0) | (1, 0) => { - Pallet::::on_killed_account(who.clone()); - DecRefStatus::Reaped - }, - (x, _) => { - account.sufficients = x - 1; - *maybe_account = Some(account); - DecRefStatus::Exists - }, - } - } else { - log::error!( - target: "frame-evm-system", - "Logic error: Account already dead when reducing provider", - ); - DecRefStatus::Reaped - } - }) + /// Remove an account. + pub fn remove_account(who: &::AccountId) -> AccountRemovingStatus { + let nonce = FullAccount::::take(who); + + if nonce == Zero::zero() { + return AccountRemovingStatus::NotExist; + } + + Self::on_killed_account(who.clone()); + AccountRemovingStatus::Removed } } From 3155c301a6ac13e2a6af16291aa816f487a1400a Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Fri, 5 May 2023 18:49:05 +0300 Subject: [PATCH 19/23] Remove toolchain --- rust-toolchain.toml | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 rust-toolchain.toml diff --git a/rust-toolchain.toml b/rust-toolchain.toml deleted file mode 100644 index 420568123b..0000000000 --- a/rust-toolchain.toml +++ /dev/null @@ -1,5 +0,0 @@ -[toolchain] -channel = "nightly-2023-01-11" -components = ["rustfmt", "clippy"] -targets = ["wasm32-unknown-unknown"] -profile = "minimal" From 0997a686265917014417ca7e6b8db150ccb59ecc Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Mon, 8 May 2023 16:08:36 +0300 Subject: [PATCH 20/23] Use generic for account data --- frame/evm-system/src/lib.rs | 31 +++++++++++++++++++++++-------- rust-toolchain.toml | 5 +++++ 2 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 rust-toolchain.toml diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 4e2011b5e1..959cd7d6bc 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -21,10 +21,21 @@ #![cfg_attr(not(feature = "std"), no_std)] use sp_runtime::{traits::{One, Zero}, RuntimeDebug}; -use scale_codec::MaxEncodedLen; +use scale_codec::{Encode, Decode, MaxEncodedLen, FullCodec}; +use scale_info::TypeInfo; pub use pallet::*; +/// Information of an account. +#[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 { @@ -81,6 +92,10 @@ pub mod pallet { + 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>; @@ -97,7 +112,7 @@ pub mod pallet { _, Blake2_128Concat, ::AccountId, - ::Index, + AccountInfo<::Index, ::AccountData>, ValueQuery, >; @@ -126,21 +141,21 @@ impl Pallet { /// Retrieve the account transaction counter from storage. pub fn account_nonce(who: &::AccountId) -> ::Index { - FullAccount::::get(who) + FullAccount::::get(who).nonce } /// Increment a particular account's nonce by 1. pub fn inc_account_nonce(who: &::AccountId) { - FullAccount::::mutate(who, |nonce| *nonce += ::Index::one()); + FullAccount::::mutate(who, |a| a.nonce += ::Index::one()); } /// Create an account. pub fn create_account(who: &::AccountId) -> AccountCreationStatus { - FullAccount::::mutate(who, |nonce| { - if *nonce == Zero::zero() { + FullAccount::::mutate(who, |a| { + if a.nonce == Zero::zero() { // Account is being created. Self::on_created_account(who.clone()); - *nonce = One::one(); + a.nonce = One::one(); AccountCreationStatus::Created } else { AccountCreationStatus::Existed @@ -150,7 +165,7 @@ impl Pallet { /// Remove an account. pub fn remove_account(who: &::AccountId) -> AccountRemovingStatus { - let nonce = FullAccount::::take(who); + let nonce = FullAccount::::take(who).nonce; if nonce == Zero::zero() { return AccountRemovingStatus::NotExist; diff --git a/rust-toolchain.toml b/rust-toolchain.toml new file mode 100644 index 0000000000..420568123b --- /dev/null +++ b/rust-toolchain.toml @@ -0,0 +1,5 @@ +[toolchain] +channel = "nightly-2023-01-11" +components = ["rustfmt", "clippy"] +targets = ["wasm32-unknown-unknown"] +profile = "minimal" From 27632949cc1b22c10319ad1190b26271ed202f75 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Mon, 8 May 2023 16:23:02 +0300 Subject: [PATCH 21/23] Edit description --- 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 c6c9188fdf..9697c1ccd8 100644 --- a/frame/evm-system/Cargo.toml +++ b/frame/evm-system/Cargo.toml @@ -2,7 +2,7 @@ name = "pallet-evm-system" version = "1.0.0-dev" license = "Apache-2.0" -description = "FRAME EVM SYSTEM accounts pallet." +description = "FRAME EVM SYSTEM pallet." authors = { workspace = true } edition = { workspace = true } repository = { workspace = true } From b687ed443d832f933f55b3c914c7cdacad84d9b1 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Mon, 8 May 2023 16:35:38 +0300 Subject: [PATCH 22/23] Introduce evm-balances --- Cargo.lock | 13 + Cargo.toml | 2 + frame/evm-balances/Cargo.toml | 34 + frame/evm-balances/src/imbalances.rs | 172 ++++++ frame/evm-balances/src/lib.rs | 894 +++++++++++++++++++++++++++ 5 files changed, 1115 insertions(+) create mode 100644 frame/evm-balances/Cargo.toml create mode 100644 frame/evm-balances/src/imbalances.rs create mode 100644 frame/evm-balances/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index e18e0f74bb..4519398d85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4838,6 +4838,19 @@ dependencies = [ "sp-std", ] +[[package]] +name = "pallet-evm-balances" +version = "1.0.0-dev" +dependencies = [ + "frame-support", + "frame-system", + "log", + "parity-scale-codec", + "scale-info", + "sp-runtime", + "sp-std", +] + [[package]] name = "pallet-evm-chain-id" version = "1.0.0-dev" diff --git a/Cargo.toml b/Cargo.toml index 8ebea92164..65d02c527c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,7 @@ members = [ "frame/dynamic-fee", "frame/ethereum", "frame/evm", + "frame/evm-balances", "frame/evm-chain-id", "frame/evm-system", "frame/hotfix-sufficients", @@ -137,6 +138,7 @@ pallet-base-fee = { version = "1.0.0", path = "frame/base-fee", default-features pallet-dynamic-fee = { version = "4.0.0-dev", path = "frame/dynamic-fee", default-features = false } 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-balances = { version = "6.0.0-dev", path = "frame/evm-balances", 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 } diff --git a/frame/evm-balances/Cargo.toml b/frame/evm-balances/Cargo.toml new file mode 100644 index 0000000000..2a77c0af41 --- /dev/null +++ b/frame/evm-balances/Cargo.toml @@ -0,0 +1,34 @@ +[package] +name = "pallet-evm-balances" +version = "1.0.0-dev" +license = "Apache-2.0" +description = "FRAME EVM BALANCES 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", +] diff --git a/frame/evm-balances/src/imbalances.rs b/frame/evm-balances/src/imbalances.rs new file mode 100644 index 0000000000..5378a79821 --- /dev/null +++ b/frame/evm-balances/src/imbalances.rs @@ -0,0 +1,172 @@ +//! Imbalances implementation. + +use frame_support::traits::{SameOrOther, TryDrop}; +use sp_std::{cmp::Ordering, mem}; + +use super::*; + +/// Opaque, move-only struct with private fields that serves as a token denoting that +/// funds have been created without any equal and opposite accounting. +#[must_use] +#[derive(RuntimeDebug, PartialEq, Eq)] +pub struct PositiveImbalance, I: 'static = ()>(T::Balance); + +impl, I: 'static> PositiveImbalance { + /// Create a new positive imbalance from a balance. + pub fn new(amount: T::Balance) -> Self { + PositiveImbalance(amount) + } +} + +/// Opaque, move-only struct with private fields that serves as a token denoting that +/// funds have been destroyed without any equal and opposite accounting. +#[must_use] +#[derive(RuntimeDebug, PartialEq, Eq)] +pub struct NegativeImbalance, I: 'static = ()>(T::Balance); + +impl, I: 'static> NegativeImbalance { + /// Create a new negative imbalance from a balance. + pub fn new(amount: T::Balance) -> Self { + NegativeImbalance(amount) + } +} + +impl, I: 'static> TryDrop for PositiveImbalance { + fn try_drop(self) -> result::Result<(), Self> { + self.drop_zero() + } +} + +impl, I: 'static> Default for PositiveImbalance { + fn default() -> Self { + Self::zero() + } +} + +impl, I: 'static> Imbalance for PositiveImbalance { + type Opposite = NegativeImbalance; + + fn zero() -> Self { + Self(Zero::zero()) + } + + fn drop_zero(self) -> result::Result<(), Self> { + if self.0.is_zero() { + Ok(()) + } else { + Err(self) + } + } + + fn split(self, amount: T::Balance) -> (Self, Self) { + let first = self.0.min(amount); + let second = self.0 - first; + + mem::forget(self); + (Self(first), Self(second)) + } + + fn merge(mut self, other: Self) -> Self { + self.0 = self.0.saturating_add(other.0); + mem::forget(other); + + self + } + + fn subsume(&mut self, other: Self) { + self.0 = self.0.saturating_add(other.0); + mem::forget(other); + } + + fn offset(self, other: Self::Opposite) -> SameOrOther { + let (a, b) = (self.0, other.0); + mem::forget((self, other)); + + match a.cmp(&b) { + Ordering::Greater => SameOrOther::Same(Self(a - b)), + Ordering::Less => SameOrOther::Other(NegativeImbalance::new(b - a)), + Ordering::Equal => SameOrOther::None, + } + } + + fn peek(&self) -> T::Balance { + self.0 + } +} + +impl, I: 'static> TryDrop for NegativeImbalance { + fn try_drop(self) -> result::Result<(), Self> { + self.drop_zero() + } +} + +impl, I: 'static> Default for NegativeImbalance { + fn default() -> Self { + Self::zero() + } +} + +impl, I: 'static> Imbalance for NegativeImbalance { + type Opposite = PositiveImbalance; + + fn zero() -> Self { + Self(Zero::zero()) + } + + fn drop_zero(self) -> result::Result<(), Self> { + if self.0.is_zero() { + Ok(()) + } else { + Err(self) + } + } + + fn split(self, amount: T::Balance) -> (Self, Self) { + let first = self.0.min(amount); + let second = self.0 - first; + + mem::forget(self); + (Self(first), Self(second)) + } + + fn merge(mut self, other: Self) -> Self { + self.0 = self.0.saturating_add(other.0); + mem::forget(other); + + self + } + + fn subsume(&mut self, other: Self) { + self.0 = self.0.saturating_add(other.0); + mem::forget(other); + } + + fn offset(self, other: Self::Opposite) -> SameOrOther { + let (a, b) = (self.0, other.0); + mem::forget((self, other)); + + match a.cmp(&b) { + Ordering::Greater => SameOrOther::Same(Self(a - b)), + Ordering::Less => SameOrOther::Other(PositiveImbalance::new(b - a)), + Ordering::Equal => SameOrOther::None, + } + } + + fn peek(&self) -> T::Balance { + self.0 + } +} + +impl, I: 'static> Drop for PositiveImbalance { + /// Basic drop handler will just square up the total issuance. + fn drop(&mut self) { + >::mutate(|v| *v = v.saturating_add(self.0)); + } +} + +impl, I: 'static> Drop for NegativeImbalance { + /// Basic drop handler will just square up the total issuance. + fn drop(&mut self) { + >::mutate(|v| *v = v.saturating_sub(self.0)); + } +} diff --git a/frame/evm-balances/src/lib.rs b/frame/evm-balances/src/lib.rs new file mode 100644 index 0000000000..4088406a42 --- /dev/null +++ b/frame/evm-balances/src/lib.rs @@ -0,0 +1,894 @@ +// 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 Balances Pallet + +// Ensure we're `no_std` when compiling for Wasm. +#![cfg_attr(not(feature = "std"), no_std)] + +use sp_runtime::{ + traits::{Bounded, CheckedAdd, CheckedSub, MaybeSerializeDeserialize, Zero}, + ArithmeticError, DispatchError, DispatchResult, RuntimeDebug, Saturating, +}; +use scale_codec::{Codec, Decode, Encode, MaxEncodedLen}; +use frame_support::{ + ensure, + traits::{ + fungible, + tokens::{DepositConsequence, WithdrawConsequence}, + Currency, ExistenceRequirement, + ExistenceRequirement::AllowDeath, + Get, Imbalance, OnUnbalanced, SignedImbalance, StorageVersion, WithdrawReasons, + }, +}; +use scale_info::TypeInfo; +use sp_std::{cmp, fmt::Debug, result}; + +mod imbalances; +pub use imbalances::{NegativeImbalance, PositiveImbalance}; + +pub use pallet::*; + + +/// The current storage version. +const STORAGE_VERSION: StorageVersion = StorageVersion::new(0); + +/// Simplified reasons for withdrawing balance. +#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, MaxEncodedLen, TypeInfo)] +pub enum Reasons { + /// Paying system transaction fees. + Fee = 0, + /// Any reason other than paying system transaction fees. + Misc = 1, + /// Any reason at all. + All = 2, +} + +impl From for Reasons { + fn from(r: WithdrawReasons) -> Reasons { + if r == WithdrawReasons::TRANSACTION_PAYMENT { + Reasons::Fee + } else if r.contains(WithdrawReasons::TRANSACTION_PAYMENT) { + Reasons::All + } else { + Reasons::Misc + } + } +} + +/// All balance information for an account. +#[derive(Encode, Decode, Clone, PartialEq, Eq, Default, RuntimeDebug, MaxEncodedLen, TypeInfo)] +pub struct AccountData { + /// Non-reserved part of the balance. There may still be restrictions on this, but it is the + /// total pool what may in principle be transferred, reserved and used for tipping. + /// + /// This is the only balance that matters in terms of most operations on tokens. It + /// alone is used to determine the balance when in the contract execution environment. + pub free: Balance, + /// Balance which is reserved and may not be used at all. + /// + /// This can still get slashed, but gets slashed last of all. + /// + /// This balance is a 'reserve' balance that other subsystems use in order to set aside tokens + /// that are still 'owned' by the account holder, but which are suspendable. + /// This includes named reserve and unnamed reserve. + pub reserved: Balance, + /// The amount that `free` may not drop below when withdrawing for *anything except transaction + /// fee payment*. + pub misc_frozen: Balance, + /// The amount that `free` may not drop below when withdrawing specifically for transaction + /// fee payment. + pub fee_frozen: Balance, +} + +impl AccountData { + /// The amount that this account's free balance may not be reduced beyond for the given + /// `reasons`. + fn frozen(&self, reasons: Reasons) -> Balance { + match reasons { + Reasons::All => self.misc_frozen.max(self.fee_frozen), + Reasons::Misc => self.misc_frozen, + Reasons::Fee => self.fee_frozen, + } + } + + /// The total balance in this account including any that is reserved and ignoring any frozen. + fn total(&self) -> Balance { + self.free.saturating_add(self.reserved) + } +} + +// We have to temporarily allow some clippy lints. Later on we'll send patches to substrate to +// fix them at their end. +#[allow( + missing_docs, + clippy::missing_docs_in_private_items, + clippy::unused_unit +)] +#[frame_support::pallet] +pub mod pallet { + use frame_support::pallet_prelude::*; + use sp_runtime::{ + traits::{AtLeast32BitUnsigned, MaybeDisplay}, + FixedPointOperand, + }; + use sp_std::fmt::Debug; + + use super::*; + + /// Configuration trait of this pallet. + #[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; + + /// The balance of an account. + type Balance: Parameter + + Member + + AtLeast32BitUnsigned + + Codec + + Default + + Copy + + MaybeSerializeDeserialize + + Debug + + MaxEncodedLen + + TypeInfo + + FixedPointOperand; + + /// The minimum amount required to keep an account open. + #[pallet::constant] + type ExistentialDeposit: Get; + + /// Handler for the unbalanced reduction when removing a dust account. + type DustRemoval: OnUnbalanced>; + } + + #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(PhantomData<(T, I)>); + + /// The total units issued. + #[pallet::storage] + #[pallet::getter(fn total_issuance)] + #[pallet::whitelist_storage] + pub type TotalIssuance, I: 'static = ()> = StorageValue<_, T::Balance, ValueQuery>; + + /// The total units of outstanding deactivated balance. + #[pallet::storage] + #[pallet::getter(fn inactive_issuance)] + #[pallet::whitelist_storage] + pub type InactiveIssuance, I: 'static = ()> = + StorageValue<_, T::Balance, ValueQuery>; + + /// The full account balance information for a particular account ID. + #[pallet::storage] + #[pallet::getter(fn account_store)] + pub type AccountStore, I: 'static = ()> = StorageMap< + _, + Blake2_128Concat, + >::AccountId, + AccountData, + ValueQuery, + >; + + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event, I: 'static = ()> { + /// An account was created with some free balance. + Endowed { + account: >::AccountId, + free_balance: T::Balance, + }, + /// An account was removed whose balance was non-zero but below ExistentialDeposit, + /// resulting in an outright loss. + DustLost { + account: >::AccountId, + amount: T::Balance, + }, + /// Transfer succeeded. + Transfer { + from: >::AccountId, + to: >::AccountId, + amount: T::Balance, + }, + /// A balance was set by root. + BalanceSet { + who: >::AccountId, + free: T::Balance, + reserved: T::Balance, + }, + /// Some balance was reserved (moved from free to reserved). + Reserved { + who: >::AccountId, + amount: T::Balance, + }, + /// Some amount was deposited (e.g. for transaction fees). + Deposit { + who: >::AccountId, + amount: T::Balance, + }, + /// Some amount was withdrawn from the account (e.g. for transaction fees). + Withdraw { + who: >::AccountId, + amount: T::Balance, + }, + /// Some amount was removed from the account (e.g. for misbehavior). + Slashed { + who: >::AccountId, + amount: T::Balance, + }, + } + + #[pallet::error] + pub enum Error { + /// Account liquidity restrictions prevent withdrawal + LiquidityRestrictions, + /// Balance too low to send value. + InsufficientBalance, + /// Value too low to create account due to existential deposit + ExistentialDeposit, + /// Transfer/payment would kill account + KeepAlive, + /// A vesting schedule already exists for this account + ExistingVestingSchedule, + /// Beneficiary account must pre-exist + DeadAccount, + /// Number of named reserves exceed MaxReserves + TooManyReserves, + } +} + +/// Removes a dust account whose balance was non-zero but below `ExistentialDeposit`. +pub struct DustCleaner, I: 'static = ()>( + Option<(>::AccountId, NegativeImbalance)>, +); + +impl, I: 'static> Drop for DustCleaner { + fn drop(&mut self) { + if let Some((who, dust)) = self.0.take() { + Pallet::::deposit_event(Event::DustLost { + account: who, + amount: dust.peek(), + }); + T::DustRemoval::on_unbalanced(dust); + } + } +} + +impl, I: 'static> Pallet { + /// Get all balance information for an account. + fn account(who: &>::AccountId) -> AccountData { + >::get(who) + } + + /// Mutate an account to some new value, or delete it entirely with `None`. Will enforce + /// `ExistentialDeposit` law, annulling the account as needed. This will do nothing if the + /// result of `f` is an `Err`. + /// + /// NOTE: Doesn't do any preparatory work for creating a new account, so should only be used + /// when it is known that the account already exists. + /// + /// NOTE: LOW-LEVEL: This will not attempt to maintain total issuance. It is expected that + /// the caller will do this. + fn try_mutate_account>( + who: &>::AccountId, + f: impl FnOnce(&mut AccountData, bool) -> Result, + ) -> Result { + Self::try_mutate_account_with_dust(who, f).map(|(result, dust_cleaner)| { + drop(dust_cleaner); + result + }) + } + + /// Mutate an account to some new value, or delete it entirely with `None`. Will enforce + /// `ExistentialDeposit` law, annulling the account as needed. This will do nothing if the + /// result of `f` is an `Err`. + /// + /// It returns both the result from the closure, and an optional `DustCleaner` instance which + /// should be dropped once it is known that all nested mutates that could affect storage items + /// what the dust handler touches have completed. + /// + /// NOTE: Doesn't do any preparatory work for creating a new account, so should only be used + /// when it is known that the account already exists. + /// + /// NOTE: LOW-LEVEL: This will not attempt to maintain total issuance. It is expected that + /// the caller will do this. + fn try_mutate_account_with_dust>( + who: &>::AccountId, + f: impl FnOnce(&mut AccountData, bool) -> Result, + ) -> Result<(R, DustCleaner), E> { + let result = >::try_mutate_exists(who, |maybe_account| { + let is_new = maybe_account.is_none(); + let mut account = maybe_account.take().unwrap_or_default(); + f(&mut account, is_new).map(move |result| { + let maybe_endowed = if is_new { Some(account.free) } else { None }; + let maybe_account_maybe_dust = Self::post_mutation(who, account); + *maybe_account = maybe_account_maybe_dust.0; + (maybe_endowed, maybe_account_maybe_dust.1, result) + }) + }); + result.map(|(maybe_endowed, maybe_dust, result)| { + if let Some(endowed) = maybe_endowed { + Self::deposit_event(Event::Endowed { + account: who.clone(), + free_balance: endowed, + }); + } + let dust_cleaner = DustCleaner(maybe_dust.map(|dust| (who.clone(), dust))); + (result, dust_cleaner) + }) + } + + /// Handles any steps needed after mutating an account. + /// + /// This includes `DustRemoval` unbalancing, in the case than the `new` account's total balance + /// is non-zero but below ED. + /// + /// Returns two values: + /// - `Some` containing the the `new` account, iff the account has sufficient balance. + /// - `Some` containing the dust to be dropped, iff some dust should be dropped. + fn post_mutation( + _who: &>::AccountId, + new: AccountData, + ) -> ( + Option>, + Option>, + ) { + let total = new.total(); + if total < T::ExistentialDeposit::get() { + if total.is_zero() { + (None, None) + } else { + (None, Some(NegativeImbalance::new(total))) + } + } else { + (Some(new), None) + } + } + + fn deposit_consequence( + _who: &>::AccountId, + amount: T::Balance, + account: &AccountData, + mint: bool, + ) -> DepositConsequence { + if amount.is_zero() { + return DepositConsequence::Success; + } + + if mint && TotalIssuance::::get().checked_add(&amount).is_none() { + return DepositConsequence::Overflow; + } + + let new_total_balance = match account.total().checked_add(&amount) { + Some(x) => x, + None => return DepositConsequence::Overflow, + }; + + if new_total_balance < T::ExistentialDeposit::get() { + return DepositConsequence::BelowMinimum; + } + + // NOTE: We assume that we are a provider, so don't need to do any checks in the + // case of account creation. + + DepositConsequence::Success + } + + fn withdraw_consequence( + _who: &>::AccountId, + amount: T::Balance, + account: &AccountData, + ) -> WithdrawConsequence { + if amount.is_zero() { + return WithdrawConsequence::Success; + } + + if TotalIssuance::::get().checked_sub(&amount).is_none() { + return WithdrawConsequence::Underflow; + } + + let new_total_balance = match account.total().checked_sub(&amount) { + Some(x) => x, + None => return WithdrawConsequence::NoFunds, + }; + + // Provider restriction - total account balance cannot be reduced to zero if it cannot + // sustain the loss of a provider reference. + // NOTE: This assumes that the pallet is a provider (which is true). Is this ever changes, + // then this will need to adapt accordingly. + let ed = T::ExistentialDeposit::get(); + let success = if new_total_balance < ed { + // ATTENTION. CHECK. + // if frame_system::Pallet::::can_dec_provider(who) { + // WithdrawConsequence::ReducedToZero(new_total_balance) + // } else { + // return WithdrawConsequence::WouldDie; + // } + WithdrawConsequence::ReducedToZero(new_total_balance) + } else { + WithdrawConsequence::Success + }; + + // Enough free funds to have them be reduced. + let new_free_balance = match account.free.checked_sub(&amount) { + Some(b) => b, + None => return WithdrawConsequence::NoFunds, + }; + + // Eventual free funds must be no less than the frozen balance. + let min_balance = account.frozen(Reasons::All); + if new_free_balance < min_balance { + return WithdrawConsequence::Frozen; + } + + success + } +} + +impl, I: 'static> Currency<>::AccountId> for Pallet +where + T::Balance: MaybeSerializeDeserialize + Debug, +{ + type Balance = T::Balance; + type PositiveImbalance = PositiveImbalance; + type NegativeImbalance = NegativeImbalance; + + fn total_balance(who: &>::AccountId) -> Self::Balance { + Self::account(who).total() + } + + // Check if `value` amount of free balance can be slashed from `who`. + fn can_slash(who: &>::AccountId, value: Self::Balance) -> bool { + if value.is_zero() { + return true; + } + Self::free_balance(who) >= value + } + + fn total_issuance() -> Self::Balance { + TotalIssuance::::get() + } + + fn active_issuance() -> Self::Balance { + >::AccountId>>::active_issuance() + } + + fn deactivate(amount: Self::Balance) { + InactiveIssuance::::mutate(|b| b.saturating_accrue(amount)); + } + + fn reactivate(amount: Self::Balance) { + InactiveIssuance::::mutate(|b| b.saturating_reduce(amount)); + } + + fn minimum_balance() -> Self::Balance { + T::ExistentialDeposit::get() + } + + // Burn funds from the total issuance, returning a positive imbalance for the amount burned. + // Is a no-op if amount to be burned is zero. + fn burn(mut amount: Self::Balance) -> Self::PositiveImbalance { + if amount.is_zero() { + return PositiveImbalance::zero(); + } + >::mutate(|issued| { + *issued = issued.checked_sub(&amount).unwrap_or_else(|| { + amount = *issued; + Zero::zero() + }); + }); + PositiveImbalance::new(amount) + } + + // Create new funds into the total issuance, returning a negative imbalance + // for the amount issued. + // Is a no-op if amount to be issued it zero. + fn issue(mut amount: Self::Balance) -> Self::NegativeImbalance { + if amount.is_zero() { + return NegativeImbalance::zero(); + } + >::mutate(|issued| { + *issued = issued.checked_add(&amount).unwrap_or_else(|| { + amount = Self::Balance::max_value() - *issued; + Self::Balance::max_value() + }) + }); + NegativeImbalance::new(amount) + } + + fn free_balance(who: &>::AccountId) -> Self::Balance { + Self::account(who).free + } + + // Ensure that an account can withdraw from their free balance given any existing withdrawal + // restrictions like locks and vesting balance. + // Is a no-op if amount to be withdrawn is zero. + // + // # + // Despite iterating over a list of locks, they are limited by the number of + // lock IDs, which means the number of runtime pallets that intend to use and create locks. + // # + fn ensure_can_withdraw( + who: &>::AccountId, + amount: T::Balance, + reasons: WithdrawReasons, + new_balance: T::Balance, + ) -> DispatchResult { + if amount.is_zero() { + return Ok(()); + } + let min_balance = Self::account(who).frozen(reasons.into()); + ensure!( + new_balance >= min_balance, + Error::::LiquidityRestrictions + ); + Ok(()) + } + + // Transfer some free balance from `transactor` to `dest`, respecting existence requirements. + // Is a no-op if value to be transferred is zero or the `transactor` is the same as `dest`. + fn transfer( + transactor: &>::AccountId, + dest: &>::AccountId, + value: Self::Balance, + existence_requirement: ExistenceRequirement, + ) -> DispatchResult { + if value.is_zero() || transactor == dest { + return Ok(()); + } + + Self::try_mutate_account_with_dust( + dest, + |to_account, _| -> Result, DispatchError> { + Self::try_mutate_account_with_dust( + transactor, + |from_account, _| -> DispatchResult { + from_account.free = from_account + .free + .checked_sub(&value) + .ok_or(Error::::InsufficientBalance)?; + + // NOTE: total stake being stored in the same type means that this could + // never overflow but better to be safe than sorry. + to_account.free = to_account + .free + .checked_add(&value) + .ok_or(ArithmeticError::Overflow)?; + + let ed = T::ExistentialDeposit::get(); + ensure!(to_account.total() >= ed, Error::::ExistentialDeposit); + + Self::ensure_can_withdraw( + transactor, + value, + WithdrawReasons::TRANSFER, + from_account.free, + ) + .map_err(|_| Error::::LiquidityRestrictions)?; + + // TODO: This is over-conservative. There may now be other providers, and + // this pallet may not even be a provider. + let allow_death = existence_requirement == ExistenceRequirement::AllowDeath; + // ATTENTION. CHECK. + // let allow_death = + // allow_death && system::Pallet::::can_dec_provider(transactor); + ensure!( + allow_death || from_account.total() >= ed, + Error::::KeepAlive + ); + + Ok(()) + }, + ) + .map(|(_, maybe_dust_cleaner)| maybe_dust_cleaner) + }, + )?; + + // Emit transfer event. + Self::deposit_event(Event::Transfer { + from: transactor.clone(), + to: dest.clone(), + amount: value, + }); + + Ok(()) + } + + /// Slash a target account `who`, returning the negative imbalance created and any left over + /// amount that could not be slashed. + /// + /// Is a no-op if `value` to be slashed is zero or the account does not exist. + /// + /// NOTE: `slash()` prefers free balance, but assumes that reserve balance can be drawn + /// from in extreme circumstances. `can_slash()` should be used prior to `slash()` to avoid + /// having to draw from reserved funds, however we err on the side of punishment if things are + /// inconsistent or `can_slash` wasn't used appropriately. + fn slash( + who: &>::AccountId, + value: Self::Balance, + ) -> (Self::NegativeImbalance, Self::Balance) { + if value.is_zero() { + return (NegativeImbalance::zero(), Zero::zero()); + } + if Self::total_balance(who).is_zero() { + return (NegativeImbalance::zero(), value); + } + + for attempt in 0..2 { + match Self::try_mutate_account( + who, + |account, + _is_new| + -> Result<(Self::NegativeImbalance, Self::Balance), DispatchError> { + // Best value is the most amount we can slash following liveness rules. + let best_value = match attempt { + // First attempt we try to slash the full amount, and see if liveness issues + // happen. + 0 => value, + // If acting as a critical provider (i.e. first attempt failed), then slash + // as much as possible while leaving at least at ED. + _ => value.min( + (account.free + account.reserved) + .saturating_sub(T::ExistentialDeposit::get()), + ), + }; + + let free_slash = cmp::min(account.free, best_value); + account.free -= free_slash; // Safe because of above check + let remaining_slash = best_value - free_slash; // Safe because of above check + + if !remaining_slash.is_zero() { + // If we have remaining slash, take it from reserved balance. + let reserved_slash = cmp::min(account.reserved, remaining_slash); + account.reserved -= reserved_slash; // Safe because of above check + Ok(( + NegativeImbalance::new(free_slash + reserved_slash), + value - free_slash - reserved_slash, /* Safe because value is gt or + * eq total slashed */ + )) + } else { + // Else we are done! + Ok(( + NegativeImbalance::new(free_slash), + value - free_slash, // Safe because value is gt or eq to total slashed + )) + } + }, + ) { + Ok((imbalance, not_slashed)) => { + Self::deposit_event(Event::Slashed { + who: who.clone(), + amount: value.saturating_sub(not_slashed), + }); + return (imbalance, not_slashed) + }, + Err(_) => (), + } + } + + // Should never get here. But we'll be defensive anyway. + (Self::NegativeImbalance::zero(), value) + } + + /// Deposit some `value` into the free balance of an existing target account `who`. + /// + /// Is a no-op if the `value` to be deposited is zero. + fn deposit_into_existing( + who: &>::AccountId, + value: Self::Balance, + ) -> Result { + if value.is_zero() { + return Ok(PositiveImbalance::zero()); + } + + Self::try_mutate_account( + who, + |account, is_new| -> Result { + ensure!(!is_new, Error::::DeadAccount); + account.free = account + .free + .checked_add(&value) + .ok_or(ArithmeticError::Overflow)?; + Self::deposit_event(Event::Deposit { + who: who.clone(), + amount: value, + }); + Ok(PositiveImbalance::new(value)) + }, + ) + } + + /// Deposit some `value` into the free balance of `who`, possibly creating a new account. + /// + /// This function is a no-op if: + /// - the `value` to be deposited is zero; or + /// - the `value` to be deposited is less than the required ED and the account does not yet + /// exist; or + /// - the deposit would necessitate the account to exist and there are no provider references; + /// or + /// - `value` is so large it would cause the balance of `who` to overflow. + fn deposit_creating( + who: &>::AccountId, + value: Self::Balance, + ) -> Self::PositiveImbalance { + if value.is_zero() { + return Self::PositiveImbalance::zero(); + } + + Self::try_mutate_account( + who, + |account, is_new| -> Result { + let ed = T::ExistentialDeposit::get(); + ensure!(value >= ed || !is_new, Error::::ExistentialDeposit); + + // defensive only: overflow should never happen, however in case it does, then this + // operation is a no-op. + account.free = match account.free.checked_add(&value) { + Some(x) => x, + None => return Ok(Self::PositiveImbalance::zero()), + }; + + Self::deposit_event(Event::Deposit { + who: who.clone(), + amount: value, + }); + Ok(PositiveImbalance::new(value)) + }, + ) + .unwrap_or_else(|_| Self::PositiveImbalance::zero()) + } + + /// Withdraw some free balance from an account, respecting existence requirements. + /// + /// Is a no-op if value to be withdrawn is zero. + fn withdraw( + who: &>::AccountId, + value: Self::Balance, + reasons: WithdrawReasons, + liveness: ExistenceRequirement, + ) -> result::Result { + if value.is_zero() { + return Ok(NegativeImbalance::zero()); + } + + Self::try_mutate_account( + who, + |account, _| -> Result { + let new_free_account = account + .free + .checked_sub(&value) + .ok_or(Error::::InsufficientBalance)?; + + // bail if we need to keep the account alive and this would kill it. + let ed = T::ExistentialDeposit::get(); + let would_be_dead = new_free_account + account.reserved < ed; + let would_kill = would_be_dead && account.free + account.reserved >= ed; + ensure!( + liveness == AllowDeath || !would_kill, + Error::::KeepAlive + ); + + Self::ensure_can_withdraw(who, value, reasons, new_free_account)?; + + account.free = new_free_account; + + Self::deposit_event(Event::Withdraw { + who: who.clone(), + amount: value, + }); + Ok(NegativeImbalance::new(value)) + }, + ) + } + + /// Force the new free balance of a target account `who` to some new value `balance`. + fn make_free_balance_be( + who: &>::AccountId, + value: Self::Balance, + ) -> SignedImbalance { + Self::try_mutate_account( + who, + |account, + is_new| + -> Result, DispatchError> { + let ed = T::ExistentialDeposit::get(); + let total = value.saturating_add(account.reserved); + // If we're attempting to set an existing account to less than ED, then + // bypass the entire operation. It's a no-op if you follow it through, but + // since this is an instance where we might account for a negative imbalance + // (in the dust cleaner of set_account) before we account for its actual + // equal and opposite cause (returned as an Imbalance), then in the + // instance that there's no other accounts on the system at all, we might + // underflow the issuance and our arithmetic will be off. + ensure!(total >= ed || !is_new, Error::::ExistentialDeposit); + + let imbalance = if account.free <= value { + SignedImbalance::Positive(PositiveImbalance::new(value - account.free)) + } else { + SignedImbalance::Negative(NegativeImbalance::new(account.free - value)) + }; + account.free = value; + Self::deposit_event(Event::BalanceSet { + who: who.clone(), + free: account.free, + reserved: account.reserved, + }); + Ok(imbalance) + }, + ) + .unwrap_or_else(|_| SignedImbalance::Positive(Self::PositiveImbalance::zero())) + } +} + +impl, I: 'static> fungible::Inspect<>::AccountId> for Pallet { + type Balance = T::Balance; + + fn total_issuance() -> Self::Balance { + TotalIssuance::::get() + } + fn active_issuance() -> Self::Balance { + TotalIssuance::::get().saturating_sub(InactiveIssuance::::get()) + } + fn minimum_balance() -> Self::Balance { + T::ExistentialDeposit::get() + } + fn balance(who: &>::AccountId) -> Self::Balance { + Self::account(who).total() + } + fn reducible_balance(who: &>::AccountId, keep_alive: bool) -> Self::Balance { + let a = Self::account(who); + // Liquid balance is what is neither reserved nor locked/frozen. + let liquid = a.free.saturating_sub(a.fee_frozen.max(a.misc_frozen)); + // ATTENTION. CHECK. + // if frame_system::Pallet::::can_dec_provider(who) && !keep_alive { + // liquid + if !keep_alive { + liquid + } else { + // `must_remain_to_exist` is the part of liquid balance which must remain to keep total + // over ED. + let must_remain_to_exist = + T::ExistentialDeposit::get().saturating_sub(a.total() - liquid); + liquid.saturating_sub(must_remain_to_exist) + } + } + fn can_deposit( + who: &>::AccountId, + amount: Self::Balance, + mint: bool, + ) -> DepositConsequence { + Self::deposit_consequence(who, amount, &Self::account(who), mint) + } + fn can_withdraw( + who: &>::AccountId, + amount: Self::Balance, + ) -> WithdrawConsequence { + Self::withdraw_consequence(who, amount, &Self::account(who)) + } +} From 112dca3ef31c366966dcca3e202b0517d7eb91e1 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Mon, 8 May 2023 16:53:07 +0300 Subject: [PATCH 23/23] Use AccountStore type to manage account data --- frame/evm-balances/src/account_data.rs | 50 ++++++++++++++ frame/evm-balances/src/lib.rs | 93 ++++++++++---------------- 2 files changed, 87 insertions(+), 56 deletions(-) create mode 100644 frame/evm-balances/src/account_data.rs diff --git a/frame/evm-balances/src/account_data.rs b/frame/evm-balances/src/account_data.rs new file mode 100644 index 0000000000..37bba31b24 --- /dev/null +++ b/frame/evm-balances/src/account_data.rs @@ -0,0 +1,50 @@ +//! Account balances logic. + +use super::*; + +/// All balance information for an account. +#[derive(Encode, Decode, Clone, PartialEq, Eq, Default, RuntimeDebug, MaxEncodedLen, TypeInfo)] +pub struct AccountData { + /// Non-reserved part of the balance. There may still be restrictions on this, but it is the + /// total pool what may in principle be transferred, reserved and used for tipping. + /// + /// This is the only balance that matters in terms of most operations on tokens. It + /// alone is used to determine the balance when in the contract execution environment. + pub free: Balance, + /// Balance which is reserved and may not be used at all. + /// + /// This can still get slashed, but gets slashed last of all. + /// + /// This balance is a 'reserve' balance that other subsystems use in order to set aside tokens + /// that are still 'owned' by the account holder, but which are suspendable. + /// This includes named reserve and unnamed reserve. + pub reserved: Balance, + /// The amount that `free` may not drop below when withdrawing for *anything except transaction + /// fee payment*. + pub misc_frozen: Balance, + /// The amount that `free` may not drop below when withdrawing specifically for transaction + /// fee payment. + pub fee_frozen: Balance, +} + +impl AccountData { + /// How much this account's balance can be reduced for the given `reasons`. + pub(crate) fn usable(&self, reasons: Reasons) -> Balance { + self.free.saturating_sub(self.frozen(reasons)) + } + + /// The amount that this account's free balance may not be reduced beyond for the given + /// `reasons`. + pub(crate) fn frozen(&self, reasons: Reasons) -> Balance { + match reasons { + Reasons::All => self.misc_frozen.max(self.fee_frozen), + Reasons::Misc => self.misc_frozen, + Reasons::Fee => self.fee_frozen, + } + } + + /// The total balance in this account including any that is reserved and ignoring any frozen. + pub(crate) fn total(&self) -> Balance { + self.free.saturating_add(self.reserved) + } +} diff --git a/frame/evm-balances/src/lib.rs b/frame/evm-balances/src/lib.rs index 4088406a42..fe2416c47d 100644 --- a/frame/evm-balances/src/lib.rs +++ b/frame/evm-balances/src/lib.rs @@ -32,12 +32,15 @@ use frame_support::{ tokens::{DepositConsequence, WithdrawConsequence}, Currency, ExistenceRequirement, ExistenceRequirement::AllowDeath, - Get, Imbalance, OnUnbalanced, SignedImbalance, StorageVersion, WithdrawReasons, + Get, Imbalance, OnUnbalanced, SignedImbalance, StorageVersion, WithdrawReasons, StoredMap }, }; use scale_info::TypeInfo; use sp_std::{cmp, fmt::Debug, result}; +pub mod account_data; +use account_data::AccountData; + mod imbalances; pub use imbalances::{NegativeImbalance, PositiveImbalance}; @@ -70,48 +73,6 @@ impl From for Reasons { } } -/// All balance information for an account. -#[derive(Encode, Decode, Clone, PartialEq, Eq, Default, RuntimeDebug, MaxEncodedLen, TypeInfo)] -pub struct AccountData { - /// Non-reserved part of the balance. There may still be restrictions on this, but it is the - /// total pool what may in principle be transferred, reserved and used for tipping. - /// - /// This is the only balance that matters in terms of most operations on tokens. It - /// alone is used to determine the balance when in the contract execution environment. - pub free: Balance, - /// Balance which is reserved and may not be used at all. - /// - /// This can still get slashed, but gets slashed last of all. - /// - /// This balance is a 'reserve' balance that other subsystems use in order to set aside tokens - /// that are still 'owned' by the account holder, but which are suspendable. - /// This includes named reserve and unnamed reserve. - pub reserved: Balance, - /// The amount that `free` may not drop below when withdrawing for *anything except transaction - /// fee payment*. - pub misc_frozen: Balance, - /// The amount that `free` may not drop below when withdrawing specifically for transaction - /// fee payment. - pub fee_frozen: Balance, -} - -impl AccountData { - /// The amount that this account's free balance may not be reduced beyond for the given - /// `reasons`. - fn frozen(&self, reasons: Reasons) -> Balance { - match reasons { - Reasons::All => self.misc_frozen.max(self.fee_frozen), - Reasons::Misc => self.misc_frozen, - Reasons::Fee => self.fee_frozen, - } - } - - /// The total balance in this account including any that is reserved and ignoring any frozen. - fn total(&self) -> Balance { - self.free.saturating_add(self.reserved) - } -} - // We have to temporarily allow some clippy lints. Later on we'll send patches to substrate to // fix them at their end. #[allow( @@ -163,6 +124,9 @@ pub mod pallet { #[pallet::constant] type ExistentialDeposit: Get; + /// The means of storing the balances of an account. + type AccountStore: StoredMap<>::AccountId, AccountData>; + /// Handler for the unbalanced reduction when removing a dust account. type DustRemoval: OnUnbalanced>; } @@ -185,17 +149,6 @@ pub mod pallet { pub type InactiveIssuance, I: 'static = ()> = StorageValue<_, T::Balance, ValueQuery>; - /// The full account balance information for a particular account ID. - #[pallet::storage] - #[pallet::getter(fn account_store)] - pub type AccountStore, I: 'static = ()> = StorageMap< - _, - Blake2_128Concat, - >::AccountId, - AccountData, - ValueQuery, - >; - #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event, I: 'static = ()> { @@ -281,9 +234,31 @@ impl, I: 'static> Drop for DustCleaner { } impl, I: 'static> Pallet { + /// Get the free balance of an account. + pub fn free_balance(who: impl sp_std::borrow::Borrow<>::AccountId>) -> T::Balance { + Self::account(who.borrow()).free + } + + /// Get the balance of an account that can be used for transfers, reservations, or any other + /// non-locking, non-transaction-fee activity. Will be at most `free_balance`. + pub fn usable_balance(who: impl sp_std::borrow::Borrow<>::AccountId>) -> T::Balance { + Self::account(who.borrow()).usable(Reasons::Misc) + } + + /// Get the balance of an account that can be used for paying transaction fees (not tipping, + /// or any other kind of fees, though). Will be at most `free_balance`. + pub fn usable_balance_for_fees(who: impl sp_std::borrow::Borrow<>::AccountId>) -> T::Balance { + Self::account(who.borrow()).usable(Reasons::Fee) + } + + /// Get the reserved balance of an account. + pub fn reserved_balance(who: impl sp_std::borrow::Borrow<>::AccountId>) -> T::Balance { + Self::account(who.borrow()).reserved + } + /// Get all balance information for an account. fn account(who: &>::AccountId) -> AccountData { - >::get(who) + T::AccountStore::get(who) } /// Mutate an account to some new value, or delete it entirely with `None`. Will enforce @@ -322,7 +297,7 @@ impl, I: 'static> Pallet { who: &>::AccountId, f: impl FnOnce(&mut AccountData, bool) -> Result, ) -> Result<(R, DustCleaner), E> { - let result = >::try_mutate_exists(who, |maybe_account| { + let result = T::AccountStore::try_mutate_exists(who, |maybe_account| { let is_new = maybe_account.is_none(); let mut account = maybe_account.take().unwrap_or_default(); f(&mut account, is_new).map(move |result| { @@ -852,15 +827,19 @@ impl, I: 'static> fungible::Inspect<>::AccountId> fo fn total_issuance() -> Self::Balance { TotalIssuance::::get() } + fn active_issuance() -> Self::Balance { TotalIssuance::::get().saturating_sub(InactiveIssuance::::get()) } + fn minimum_balance() -> Self::Balance { T::ExistentialDeposit::get() } + fn balance(who: &>::AccountId) -> Self::Balance { Self::account(who).total() } + fn reducible_balance(who: &>::AccountId, keep_alive: bool) -> Self::Balance { let a = Self::account(who); // Liquid balance is what is neither reserved nor locked/frozen. @@ -878,6 +857,7 @@ impl, I: 'static> fungible::Inspect<>::AccountId> fo liquid.saturating_sub(must_remain_to_exist) } } + fn can_deposit( who: &>::AccountId, amount: Self::Balance, @@ -885,6 +865,7 @@ impl, I: 'static> fungible::Inspect<>::AccountId> fo ) -> DepositConsequence { Self::deposit_consequence(who, amount, &Self::account(who), mint) } + fn can_withdraw( who: &>::AccountId, amount: Self::Balance,