From 65d6a640c9e2523f090f497e37e9774b22d2fc1b Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Tue, 30 Apr 2024 19:57:23 +0530 Subject: [PATCH] move check_nonce to pallet-evm-nonce-tracker --- Cargo.lock | 1 + domains/pallets/evm_nonce_tracker/Cargo.toml | 2 + .../evm_nonce_tracker}/src/check_nonce.rs | 70 ++++++----- domains/pallets/evm_nonce_tracker/src/lib.rs | 16 ++- domains/runtime/evm/src/lib.rs | 10 +- domains/test/runtime/evm/src/check_nonce.rs | 119 ------------------ domains/test/runtime/evm/src/lib.rs | 10 +- 7 files changed, 62 insertions(+), 166 deletions(-) rename domains/{runtime/evm => pallets/evm_nonce_tracker}/src/check_nonce.rs (63%) delete mode 100644 domains/test/runtime/evm/src/check_nonce.rs diff --git a/Cargo.lock b/Cargo.lock index d094f9fcb5..9864f50c69 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7576,6 +7576,7 @@ dependencies = [ "parity-scale-codec", "scale-info", "sp-core", + "sp-runtime", ] [[package]] diff --git a/domains/pallets/evm_nonce_tracker/Cargo.toml b/domains/pallets/evm_nonce_tracker/Cargo.toml index 0129b8bbb6..bee446dc52 100644 --- a/domains/pallets/evm_nonce_tracker/Cargo.toml +++ b/domains/pallets/evm_nonce_tracker/Cargo.toml @@ -18,6 +18,7 @@ frame-support = { default-features = false, git = "https://github.com/subspace/p frame-system = { default-features = false, git = "https://github.com/subspace/polkadot-sdk", rev = "44d742b90e7852aed1f08ab5299d5d88cfa1c6ed" } scale-info = { version = "2.11.1", default-features = false, features = ["derive"] } sp-core = { default-features = false, git = "https://github.com/subspace/polkadot-sdk", rev = "44d742b90e7852aed1f08ab5299d5d88cfa1c6ed" } +sp-runtime = { default-features = false, git = "https://github.com/subspace/polkadot-sdk", rev = "44d742b90e7852aed1f08ab5299d5d88cfa1c6ed" } [features] default = ["std"] @@ -27,4 +28,5 @@ std = [ "frame-system/std", "scale-info/std", "sp-core/std", + "sp-runtime/std", ] diff --git a/domains/runtime/evm/src/check_nonce.rs b/domains/pallets/evm_nonce_tracker/src/check_nonce.rs similarity index 63% rename from domains/runtime/evm/src/check_nonce.rs rename to domains/pallets/evm_nonce_tracker/src/check_nonce.rs index c0b5889e16..0c92ee2d4c 100644 --- a/domains/runtime/evm/src/check_nonce.rs +++ b/domains/pallets/evm_nonce_tracker/src/check_nonce.rs @@ -1,44 +1,53 @@ -use crate::{AccountId, EVMNoncetracker, Runtime, RuntimeCall}; +use crate::Config; +#[cfg(not(feature = "std"))] +use alloc::fmt; +#[cfg(not(feature = "std"))] +use alloc::vec; use codec::{Decode, Encode}; -use domain_runtime_primitives::Nonce; +use core::cmp::max; +use core::result::Result; +use frame_support::dispatch::DispatchInfo; use frame_support::pallet_prelude::{ InvalidTransaction, TransactionLongevity, TransactionValidity, TransactionValidityError, TypeInfo, ValidTransaction, }; -use frame_support::sp_runtime::traits::{DispatchInfoOf, One, SignedExtension, Zero}; -use sp_core::{H160, U256}; -use sp_std::cmp::max; -use sp_std::vec; +use frame_support::sp_runtime::traits::{DispatchInfoOf, One, SignedExtension}; +use sp_runtime::traits::{Dispatchable, Zero}; +#[cfg(feature = "std")] +use std::fmt; +#[cfg(feature = "std")] +use std::vec; -/// Check nonce is a fork of frame_system::CheckNonce with change to pre_dispatch function -/// where this fork uses EVMNonceTracker to track the nonce since EVM pre_dispatch does not -/// increment the nonce unlike the Substrate pre_dispatch #[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] -pub struct CheckNonce(#[codec(compact)] pub Nonce); +#[scale_info(skip_type_params(T))] +pub struct CheckNonce(#[codec(compact)] pub T::Nonce); -impl CheckNonce { +impl CheckNonce { /// utility constructor. Used only in client/factory code. - pub fn from(nonce: Nonce) -> Self { + pub fn from(nonce: T::Nonce) -> Self { Self(nonce) } } -impl sp_std::fmt::Debug for CheckNonce { +impl fmt::Debug for CheckNonce { #[cfg(feature = "std")] - fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "CheckNonce({})", self.0) } #[cfg(not(feature = "std"))] - fn fmt(&self, _: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { + fn fmt(&self, _: &mut fmt::Formatter) -> fmt::Result { Ok(()) } } -impl SignedExtension for CheckNonce { +impl SignedExtension for CheckNonce +where + T::RuntimeCall: Dispatchable, +{ const IDENTIFIER: &'static str = "CheckNonce"; - type AccountId = AccountId; - type Call = RuntimeCall; + type AccountId = T::AccountId; + type Call = T::RuntimeCall; type AdditionalSigned = (); type Pre = (); @@ -53,7 +62,7 @@ impl SignedExtension for CheckNonce { _info: &DispatchInfoOf, _len: usize, ) -> TransactionValidity { - let account = frame_system::Account::::get(who); + let account = frame_system::Account::::get(who); if account.providers.is_zero() && account.sufficients.is_zero() { // Nonce storage not paid for return InvalidTransaction::Payment.into(); @@ -64,7 +73,7 @@ impl SignedExtension for CheckNonce { let provides = vec![Encode::encode(&(who, self.0))]; let requires = if account.nonce < self.0 { - vec![Encode::encode(&(who, self.0 - Nonce::one()))] + vec![Encode::encode(&(who, self.0 - One::one()))] } else { vec![] }; @@ -85,24 +94,21 @@ impl SignedExtension for CheckNonce { _info: &DispatchInfoOf, _len: usize, ) -> Result<(), TransactionValidityError> { - let mut account = frame_system::Account::::get(who); + let mut account = frame_system::Account::::get(who); if account.providers.is_zero() && account.sufficients.is_zero() { // Nonce storage not paid for return Err(InvalidTransaction::Payment.into()); } - // if a sender sends an evm transaction first and substrate transaction // after with same nonce, then reject the second transaction // if sender reverse the transaction types, substrate first and evm second, // evm transaction will be rejected, since substrate updates nonce in pre_dispatch. - let account_nonce = - if let Some(tracked_nonce) = EVMNoncetracker::account_nonce(H160::from(*who)) { - let account_nonce = U256::from(account.nonce); - let current_nonce = max(tracked_nonce, account_nonce); - current_nonce.as_u32() - } else { - account.nonce - }; + let account_nonce = if let Some(tracked_nonce) = crate::AccountNonce::::get(who.clone()) + { + max(tracked_nonce.as_u32().into(), account.nonce) + } else { + account.nonce + }; if self.0 != account_nonce { return Err(if self.0 < account.nonce { @@ -112,8 +118,8 @@ impl SignedExtension for CheckNonce { } .into()); } - account.nonce += Nonce::one(); - frame_system::Account::::insert(who, account); + account.nonce += T::Nonce::one(); + frame_system::Account::::insert(who, account); Ok(()) } } diff --git a/domains/pallets/evm_nonce_tracker/src/lib.rs b/domains/pallets/evm_nonce_tracker/src/lib.rs index 1d959c97cf..4033115be8 100644 --- a/domains/pallets/evm_nonce_tracker/src/lib.rs +++ b/domains/pallets/evm_nonce_tracker/src/lib.rs @@ -17,14 +17,19 @@ #![cfg_attr(not(feature = "std"), no_std)] +#[cfg(not(feature = "std"))] +extern crate alloc; + +mod check_nonce; +pub use check_nonce::CheckNonce; pub use pallet::*; -use sp_core::{H160, U256}; +use sp_core::U256; #[frame_support::pallet] mod pallet { use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::BlockNumberFor; - use sp_core::{H160, U256}; + use sp_core::U256; #[pallet::config] pub trait Config: frame_system::Config {} @@ -33,7 +38,8 @@ mod pallet { /// This is only used for pre_dispatch since EVM pre_dispatch does not /// increment account nonce. #[pallet::storage] - pub(super) type AccountNonce = StorageMap<_, Identity, H160, U256, OptionQuery>; + pub(super) type AccountNonce = + StorageMap<_, Identity, T::AccountId, U256, OptionQuery>; /// Pallet EVM account nonce tracker. #[pallet::pallet] @@ -52,12 +58,12 @@ mod pallet { impl Pallet { /// Returns current nonce for the given account. - pub fn account_nonce(account: H160) -> Option { + pub fn account_nonce(account: T::AccountId) -> Option { AccountNonce::::get(account) } /// Set nonce to the account. - pub fn set_account_nonce(account: H160, nonce: U256) { + pub fn set_account_nonce(account: T::AccountId, nonce: U256) { AccountNonce::::set(account, Some(nonce)) } } diff --git a/domains/runtime/evm/src/lib.rs b/domains/runtime/evm/src/lib.rs index 4699638d6d..93dbe9fd56 100644 --- a/domains/runtime/evm/src/lib.rs +++ b/domains/runtime/evm/src/lib.rs @@ -3,7 +3,6 @@ // `construct_runtime!` does a lot of recursion and requires us to increase the limit to 256. #![recursion_limit = "256"] -mod check_nonce; mod precompiles; // Make the WASM binary available. @@ -127,7 +126,7 @@ type CustomSignedExtra = ( frame_system::CheckTxVersion, frame_system::CheckGenesis, frame_system::CheckMortality, - check_nonce::CheckNonce, + pallet_evm_nonce_tracker::CheckNonce, frame_system::CheckWeight, pallet_transaction_payment::ChargeTransactionPayment, ); @@ -887,7 +886,8 @@ fn pre_dispatch_evm_transaction( // pick the highest nonce let account_nonce = { let tracked_nonce = - EVMNoncetracker::account_nonce(account_id).unwrap_or(U256::zero()); + EVMNoncetracker::account_nonce(AccountId::from(account_id)) + .unwrap_or(U256::zero()); let account_nonce = EVM::account_basic(&account_id).0.nonce; max(tracked_nonce, account_nonce) }; @@ -902,7 +902,7 @@ fn pre_dispatch_evm_transaction( .checked_add(U256::one()) .ok_or(InvalidTransaction::Custom(ERR_NONCE_OVERFLOW))?; - EVMNoncetracker::set_account_nonce(account_id, next_nonce); + EVMNoncetracker::set_account_nonce(AccountId::from(account_id), next_nonce); } } @@ -939,7 +939,7 @@ fn check_transaction_and_do_pre_dispatch_inner( extra.2, extra.3, extra.4, - check_nonce::CheckNonce::from(extra.5 .0), + pallet_evm_nonce_tracker::CheckNonce::from(extra.5 .0), extra.6, extra.7, ); diff --git a/domains/test/runtime/evm/src/check_nonce.rs b/domains/test/runtime/evm/src/check_nonce.rs deleted file mode 100644 index c0b5889e16..0000000000 --- a/domains/test/runtime/evm/src/check_nonce.rs +++ /dev/null @@ -1,119 +0,0 @@ -use crate::{AccountId, EVMNoncetracker, Runtime, RuntimeCall}; -use codec::{Decode, Encode}; -use domain_runtime_primitives::Nonce; -use frame_support::pallet_prelude::{ - InvalidTransaction, TransactionLongevity, TransactionValidity, TransactionValidityError, - TypeInfo, ValidTransaction, -}; -use frame_support::sp_runtime::traits::{DispatchInfoOf, One, SignedExtension, Zero}; -use sp_core::{H160, U256}; -use sp_std::cmp::max; -use sp_std::vec; - -/// Check nonce is a fork of frame_system::CheckNonce with change to pre_dispatch function -/// where this fork uses EVMNonceTracker to track the nonce since EVM pre_dispatch does not -/// increment the nonce unlike the Substrate pre_dispatch -#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] -pub struct CheckNonce(#[codec(compact)] pub Nonce); - -impl CheckNonce { - /// utility constructor. Used only in client/factory code. - pub fn from(nonce: Nonce) -> Self { - Self(nonce) - } -} - -impl sp_std::fmt::Debug for CheckNonce { - #[cfg(feature = "std")] - fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { - write!(f, "CheckNonce({})", self.0) - } - - #[cfg(not(feature = "std"))] - fn fmt(&self, _: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { - Ok(()) - } -} - -impl SignedExtension for CheckNonce { - const IDENTIFIER: &'static str = "CheckNonce"; - type AccountId = AccountId; - type Call = RuntimeCall; - type AdditionalSigned = (); - type Pre = (); - - fn additional_signed(&self) -> Result<(), TransactionValidityError> { - Ok(()) - } - - fn validate( - &self, - who: &Self::AccountId, - _call: &Self::Call, - _info: &DispatchInfoOf, - _len: usize, - ) -> TransactionValidity { - let account = frame_system::Account::::get(who); - if account.providers.is_zero() && account.sufficients.is_zero() { - // Nonce storage not paid for - return InvalidTransaction::Payment.into(); - } - if self.0 < account.nonce { - return InvalidTransaction::Stale.into(); - } - - let provides = vec![Encode::encode(&(who, self.0))]; - let requires = if account.nonce < self.0 { - vec![Encode::encode(&(who, self.0 - Nonce::one()))] - } else { - vec![] - }; - - Ok(ValidTransaction { - priority: 0, - requires, - provides, - longevity: TransactionLongevity::MAX, - propagate: true, - }) - } - - fn pre_dispatch( - self, - who: &Self::AccountId, - _call: &Self::Call, - _info: &DispatchInfoOf, - _len: usize, - ) -> Result<(), TransactionValidityError> { - let mut account = frame_system::Account::::get(who); - if account.providers.is_zero() && account.sufficients.is_zero() { - // Nonce storage not paid for - return Err(InvalidTransaction::Payment.into()); - } - - // if a sender sends an evm transaction first and substrate transaction - // after with same nonce, then reject the second transaction - // if sender reverse the transaction types, substrate first and evm second, - // evm transaction will be rejected, since substrate updates nonce in pre_dispatch. - let account_nonce = - if let Some(tracked_nonce) = EVMNoncetracker::account_nonce(H160::from(*who)) { - let account_nonce = U256::from(account.nonce); - let current_nonce = max(tracked_nonce, account_nonce); - current_nonce.as_u32() - } else { - account.nonce - }; - - if self.0 != account_nonce { - return Err(if self.0 < account.nonce { - InvalidTransaction::Stale - } else { - InvalidTransaction::Future - } - .into()); - } - account.nonce += Nonce::one(); - frame_system::Account::::insert(who, account); - Ok(()) - } -} diff --git a/domains/test/runtime/evm/src/lib.rs b/domains/test/runtime/evm/src/lib.rs index f6e404ea19..31ab789d82 100644 --- a/domains/test/runtime/evm/src/lib.rs +++ b/domains/test/runtime/evm/src/lib.rs @@ -3,7 +3,6 @@ // `construct_runtime!` does a lot of recursion and requires us to increase the limit to 256. #![recursion_limit = "256"] -mod check_nonce; mod precompiles; // Make the WASM binary available. @@ -124,7 +123,7 @@ type CustomSignedExtra = ( frame_system::CheckTxVersion, frame_system::CheckGenesis, frame_system::CheckMortality, - check_nonce::CheckNonce, + pallet_evm_nonce_tracker::CheckNonce, frame_system::CheckWeight, pallet_transaction_payment::ChargeTransactionPayment, ); @@ -855,7 +854,8 @@ fn pre_dispatch_evm_transaction( // pick the highest nonce let account_nonce = { let tracked_nonce = - EVMNoncetracker::account_nonce(account_id).unwrap_or(U256::zero()); + EVMNoncetracker::account_nonce(AccountId::from(account_id)) + .unwrap_or(U256::zero()); let account_nonce = EVM::account_basic(&account_id).0.nonce; max(tracked_nonce, account_nonce) }; @@ -870,7 +870,7 @@ fn pre_dispatch_evm_transaction( .checked_add(U256::one()) .ok_or(InvalidTransaction::Custom(ERR_NONCE_OVERFLOW))?; - EVMNoncetracker::set_account_nonce(account_id, next_nonce); + EVMNoncetracker::set_account_nonce(AccountId::from(account_id), next_nonce); } } @@ -907,7 +907,7 @@ fn check_transaction_and_do_pre_dispatch_inner( extra.2, extra.3, extra.4, - check_nonce::CheckNonce::from(extra.5 .0), + pallet_evm_nonce_tracker::CheckNonce::from(extra.5 .0), extra.6, extra.7, );