From d2fd2e9087f63cef77a3dfe89217841fd8b919bb Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Fri, 26 Apr 2024 19:49:55 +0530 Subject: [PATCH 1/6] update evm runtimes to account for nonce non updates during the pre_dispatch --- Cargo.lock | 13 +++ domains/pallets/evm_nonce_tracker/Cargo.toml | 30 +++++ domains/pallets/evm_nonce_tracker/src/lib.rs | 63 +++++++++++ domains/runtime/evm/Cargo.toml | 2 + domains/runtime/evm/src/lib.rs | 112 +++++++++++++++++-- domains/test/runtime/evm/Cargo.toml | 2 + domains/test/runtime/evm/src/lib.rs | 112 +++++++++++++++++-- 7 files changed, 316 insertions(+), 18 deletions(-) create mode 100644 domains/pallets/evm_nonce_tracker/Cargo.toml create mode 100644 domains/pallets/evm_nonce_tracker/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index dd70002b95..d094f9fcb5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3344,6 +3344,7 @@ dependencies = [ "pallet-ethereum", "pallet-evm", "pallet-evm-chain-id", + "pallet-evm-nonce-tracker", "pallet-evm-precompile-modexp", "pallet-evm-precompile-sha3fips", "pallet-evm-precompile-simple", @@ -3397,6 +3398,7 @@ dependencies = [ "pallet-ethereum", "pallet-evm", "pallet-evm-chain-id", + "pallet-evm-nonce-tracker", "pallet-evm-precompile-modexp", "pallet-evm-precompile-sha3fips", "pallet-evm-precompile-simple", @@ -7565,6 +7567,17 @@ dependencies = [ "scale-info", ] +[[package]] +name = "pallet-evm-nonce-tracker" +version = "0.1.0" +dependencies = [ + "frame-support", + "frame-system", + "parity-scale-codec", + "scale-info", + "sp-core", +] + [[package]] name = "pallet-evm-precompile-modexp" version = "2.0.0-dev" diff --git a/domains/pallets/evm_nonce_tracker/Cargo.toml b/domains/pallets/evm_nonce_tracker/Cargo.toml new file mode 100644 index 0000000000..0129b8bbb6 --- /dev/null +++ b/domains/pallets/evm_nonce_tracker/Cargo.toml @@ -0,0 +1,30 @@ +[package] +name = "pallet-evm-nonce-tracker" +version = "0.1.0" +authors = ["Subspace Labs "] +edition = "2021" +license = "Apache-2.0" +homepage = "https://subspace.network" +repository = "https://github.com/subspace/subspace" +description = "Subspace node pallet for EVM account nonce tracker." +include = [ + "/src", + "/Cargo.toml", +] + +[dependencies] +codec = { package = "parity-scale-codec", version = "3.6.5", default-features = false, features = ["derive"] } +frame-support = { default-features = false, git = "https://github.com/subspace/polkadot-sdk", rev = "44d742b90e7852aed1f08ab5299d5d88cfa1c6ed" } +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" } + +[features] +default = ["std"] +std = [ + "codec/std", + "frame-support/std", + "frame-system/std", + "scale-info/std", + "sp-core/std", +] diff --git a/domains/pallets/evm_nonce_tracker/src/lib.rs b/domains/pallets/evm_nonce_tracker/src/lib.rs new file mode 100644 index 0000000000..1d959c97cf --- /dev/null +++ b/domains/pallets/evm_nonce_tracker/src/lib.rs @@ -0,0 +1,63 @@ +// Copyright (C) 2023 Subspace Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Pallet EVM Account nonce tracker + +#![cfg_attr(not(feature = "std"), no_std)] + +pub use pallet::*; +use sp_core::{H160, U256}; + +#[frame_support::pallet] +mod pallet { + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::BlockNumberFor; + use sp_core::{H160, U256}; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + /// Storage to hold evm account nonces. + /// 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>; + + /// Pallet EVM account nonce tracker. + #[pallet::pallet] + #[pallet::without_storage_info] + pub struct Pallet(_); + + #[pallet::hooks] + impl Hooks> for Pallet { + fn on_finalize(_now: BlockNumberFor) { + // clear the storage since we would start with updated nonce + // during the pre_dispatch in next block + let _ = AccountNonce::::clear(u32::MAX, None); + } + } +} + +impl Pallet { + /// Returns current nonce for the given account. + pub fn account_nonce(account: H160) -> Option { + AccountNonce::::get(account) + } + + /// Set nonce to the account. + pub fn set_account_nonce(account: H160, nonce: U256) { + AccountNonce::::set(account, Some(nonce)) + } +} diff --git a/domains/runtime/evm/Cargo.toml b/domains/runtime/evm/Cargo.toml index 2ceb68f494..c37a638fef 100644 --- a/domains/runtime/evm/Cargo.toml +++ b/domains/runtime/evm/Cargo.toml @@ -35,6 +35,7 @@ pallet-domain-id = { version = "0.1.0", path = "../../pallets/domain-id", defaul pallet-ethereum = { default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" } pallet-evm = { version = "6.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" } pallet-evm-chain-id = { version = "1.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" } +pallet-evm-nonce-tracker = { version = "0.1.0", path = "../../pallets/evm_nonce_tracker", default-features = false } pallet-evm-precompile-modexp = { version = "2.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" } pallet-evm-precompile-sha3fips = { version = "2.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" } pallet-evm-precompile-simple = { version = "2.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" } @@ -93,6 +94,7 @@ std = [ "pallet-ethereum/std", "pallet-evm/std", "pallet-evm-chain-id/std", + "pallet-evm-nonce-tracker/std", "pallet-evm-precompile-modexp/std", "pallet-evm-precompile-sha3fips/std", "pallet-evm-precompile-simple/std", diff --git a/domains/runtime/evm/src/lib.rs b/domains/runtime/evm/src/lib.rs index 56f32e2b64..1627ea2ab7 100644 --- a/domains/runtime/evm/src/lib.rs +++ b/domains/runtime/evm/src/lib.rs @@ -38,9 +38,12 @@ use frame_support::weights::constants::{ParityDbWeight, WEIGHT_REF_TIME_PER_SECO use frame_support::weights::{ConstantMultiplier, IdentityFee, Weight}; use frame_support::{construct_runtime, parameter_types}; use frame_system::limits::{BlockLength, BlockWeights}; +use frame_system::CheckWeight; use pallet_block_fees::fees::OnChargeDomainTransaction; use pallet_ethereum::Call::transact; -use pallet_ethereum::{PostLogContent, Transaction as EthereumTransaction, TransactionStatus}; +use pallet_ethereum::{ + Call, PostLogContent, Transaction as EthereumTransaction, TransactionData, TransactionStatus, +}; use pallet_evm::{ Account as EVMAccount, EnsureAddressNever, EnsureAddressRoot, FeeCalculator, IdentityAddressMapping, Runner, @@ -70,6 +73,7 @@ use sp_runtime::{ ExtrinsicInclusionMode, }; pub use sp_runtime::{MultiAddress, Perbill, Permill}; +use sp_std::cmp::{max, Ordering}; use sp_std::marker::PhantomData; use sp_std::prelude::*; use sp_subspace_mmr::domain_mmr_runtime_interface::verify_mmr_proof; @@ -80,6 +84,9 @@ use subspace_runtime_primitives::{ SlowAdjustingFeeUpdate, SSC, }; +/// Custom error when nonce overflow occurs +const ERR_NONCE_OVERFLOW: u8 = 100; + /// Alias to 512-bit hash when used in the context of a transaction signature on the chain. pub type Signature = EthereumSignature; @@ -641,6 +648,8 @@ impl pallet_evm::Config for Runtime { type WeightInfo = pallet_evm::weights::SubstrateWeight; } +impl pallet_evm_nonce_tracker::Config for Runtime {} + parameter_types! { pub const PostOnlyBlockHash: PostLogContent = PostLogContent::OnlyBlockHash; } @@ -712,6 +721,7 @@ construct_runtime!( EVM: pallet_evm = 81, EVMChainId: pallet_evm_chain_id = 82, BaseFee: pallet_base_fee = 83, + EVMNoncetracker: pallet_evm_nonce_tracker = 84, // domain instance stuff SelfDomainId: pallet_domain_id = 90, @@ -817,6 +827,75 @@ mod benches { ); } +/// Custom pre_dispatch for extrinsic verification. +/// Most of the logic is same as `pre_dispatch_self_contained` except +/// - we use `validate_self_contained` instead `pre_dispatch_self_contained` +/// since the nonce is not incremented in `pre_dispatch_self_contained` +/// - Manually track the account nonce to check either Stale or Future nonce. +fn pre_dispatch_evm_transaction( + account_id: H160, + call: RuntimeCall, + dispatch_info: &DispatchInfoOf, + len: usize, +) -> Result<(), TransactionValidityError> { + match call { + RuntimeCall::Ethereum(call) => { + // Withdraw the consensus chain storage fee from the caller and record + // it in the `BlockFees` + let consensus_storage_fee = + BlockFees::consensus_chain_byte_fee() * Balance::from(len as u32); + match >::withdraw_fee( + &account_id, + consensus_storage_fee.into(), + ) { + Ok(None) => {} + Ok(Some(paid_consensus_storage_fee)) => { + BlockFees::note_consensus_storage_fee(paid_consensus_storage_fee.peek()) + } + Err(_) => return Err(InvalidTransaction::Payment.into()), + } + + if let Some(transaction_validity) = + call.validate_self_contained(&account_id, dispatch_info, len) + { + transaction_validity.map(|_| ())?; + + if let Call::transact { transaction } = call { + CheckWeight::::do_pre_dispatch(dispatch_info, len)?; + + let transaction_data: TransactionData = (&transaction).into(); + let transaction_nonce = transaction_data.nonce; + // if this the first transaction from this sender, then use the + // transaction nonce as first nonce. + // if the current account nonce is more the tracked nonce, then + // pick the highest nonce + let account_nonce = { + let tracked_nonce = + EVMNoncetracker::account_nonce(account_id).unwrap_or(transaction_nonce); + let account_nonce = EVM::account_basic(&account_id).0.nonce; + max(tracked_nonce, account_nonce) + }; + + match transaction_nonce.cmp(&account_nonce) { + Ordering::Less => return Err(InvalidTransaction::Stale.into()), + Ordering::Greater => return Err(InvalidTransaction::Future.into()), + Ordering::Equal => {} + } + + let next_nonce = account_nonce + .checked_add(U256::one()) + .ok_or(InvalidTransaction::Custom(ERR_NONCE_OVERFLOW))?; + + EVMNoncetracker::set_account_nonce(account_id, next_nonce); + } + } + + Ok(()) + } + _ => Err(InvalidTransaction::Call.into()), + } +} + fn check_transaction_and_do_pre_dispatch_inner( uxt: &::Extrinsic, ) -> Result<(), TransactionValidityError> { @@ -837,19 +916,34 @@ fn check_transaction_and_do_pre_dispatch_inner( // which would help to maintain context across multiple transaction validity check against same // runtime instance. match xt.signed { - CheckedSignature::Signed(account_id, extra) => extra - .pre_dispatch(&account_id, &xt.function, &dispatch_info, encoded_len) - .map(|_| ()), + CheckedSignature::Signed(account_id, extra) => { + // if a sender sends a one 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, + // since substrate updates nonce in pre_dispatch, evm transaction will be rejected. + if let Some(tracked_nonce) = EVMNoncetracker::account_nonce(H160::from(account_id.0)) { + let account_nonce = U256::from(System::account_nonce(account_id)); + let current_nonce = max(tracked_nonce, account_nonce); + let transaction_nonce = U256::from(extra.5 .0); + match transaction_nonce.cmp(¤t_nonce) { + Ordering::Less => return Err(InvalidTransaction::Stale.into()), + Ordering::Greater => return Err(InvalidTransaction::Future.into()), + Ordering::Equal => {} + } + } + + extra + .pre_dispatch(&account_id, &xt.function, &dispatch_info, encoded_len) + .map(|_| ()) + } CheckedSignature::Unsigned => { Runtime::pre_dispatch(&xt.function).map(|_| ())?; SignedExtra::pre_dispatch_unsigned(&xt.function, &dispatch_info, encoded_len) .map(|_| ()) } - CheckedSignature::SelfContained(self_contained_signing_info) => xt - .function - .pre_dispatch_self_contained(&self_contained_signing_info, &dispatch_info, encoded_len) - .ok_or(TransactionValidityError::Invalid(InvalidTransaction::Call)) - .map(|_| ()), + CheckedSignature::SelfContained(account_id) => { + pre_dispatch_evm_transaction(account_id, xt.function, &dispatch_info, encoded_len) + } } } diff --git a/domains/test/runtime/evm/Cargo.toml b/domains/test/runtime/evm/Cargo.toml index 7fc9af3f9b..18c0de79f3 100644 --- a/domains/test/runtime/evm/Cargo.toml +++ b/domains/test/runtime/evm/Cargo.toml @@ -34,6 +34,7 @@ pallet-domain-id = { version = "0.1.0", path = "../../../pallets/domain-id", def pallet-ethereum = { default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" } pallet-evm = { version = "6.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" } pallet-evm-chain-id = { version = "1.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" } +pallet-evm-nonce-tracker = { version = "0.1.0", path = "../../../pallets/evm_nonce_tracker", default-features = false } pallet-evm-precompile-modexp = { version = "2.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" } pallet-evm-precompile-sha3fips = { version = "2.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" } pallet-evm-precompile-simple = { version = "2.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" } @@ -88,6 +89,7 @@ std = [ "pallet-ethereum/std", "pallet-evm/std", "pallet-evm-chain-id/std", + "pallet-evm-nonce-tracker/std", "pallet-evm-precompile-modexp/std", "pallet-evm-precompile-sha3fips/std", "pallet-evm-precompile-simple/std", diff --git a/domains/test/runtime/evm/src/lib.rs b/domains/test/runtime/evm/src/lib.rs index 0fc44271d5..654bda420f 100644 --- a/domains/test/runtime/evm/src/lib.rs +++ b/domains/test/runtime/evm/src/lib.rs @@ -36,9 +36,12 @@ use frame_support::weights::constants::{ParityDbWeight, WEIGHT_REF_TIME_PER_SECO use frame_support::weights::{ConstantMultiplier, IdentityFee, Weight}; use frame_support::{construct_runtime, parameter_types}; use frame_system::limits::{BlockLength, BlockWeights}; +use frame_system::CheckWeight; use pallet_block_fees::fees::OnChargeDomainTransaction; use pallet_ethereum::Call::transact; -use pallet_ethereum::{PostLogContent, Transaction as EthereumTransaction, TransactionStatus}; +use pallet_ethereum::{ + Call, PostLogContent, Transaction as EthereumTransaction, TransactionData, TransactionStatus, +}; use pallet_evm::{ Account as EVMAccount, EnsureAddressNever, EnsureAddressRoot, FeeCalculator, IdentityAddressMapping, Runner, @@ -68,6 +71,7 @@ use sp_runtime::{ ExtrinsicInclusionMode, }; pub use sp_runtime::{MultiAddress, Perbill, Permill}; +use sp_std::cmp::{max, Ordering}; use sp_std::marker::PhantomData; use sp_std::prelude::*; use sp_subspace_mmr::domain_mmr_runtime_interface::verify_mmr_proof; @@ -77,6 +81,9 @@ use subspace_runtime_primitives::{ BlockNumber as ConsensusBlockNumber, Hash as ConsensusBlockHash, Moment, SSC, }; +/// Custom error when nonce overflow occurs +const ERR_NONCE_OVERFLOW: u8 = 100; + /// Alias to 512-bit hash when used in the context of a transaction signature on the chain. pub type Signature = EthereumSignature; @@ -597,6 +604,8 @@ impl pallet_evm::OnChargeEVMTransaction for EVMCurrencyAdapter { } } +impl pallet_evm_nonce_tracker::Config for Runtime {} + impl pallet_evm::Config for Runtime { type FeeCalculator = BaseFee; type GasWeightMapping = pallet_evm::FixedGasWeightMapping; @@ -689,6 +698,7 @@ construct_runtime!( EVM: pallet_evm = 81, EVMChainId: pallet_evm_chain_id = 82, BaseFee: pallet_base_fee = 83, + EVMNoncetracker: pallet_evm_nonce_tracker = 84, // domain instance stuff SelfDomainId: pallet_domain_id = 90, @@ -785,6 +795,75 @@ fn extrinsic_era(extrinsic: &::Extrinsic) -> Option { .map(|(_, _, extra)| extra.4 .0) } +/// Custom pre_dispatch for extrinsic verification. +/// Most of the logic is same as `pre_dispatch_self_contained` except +/// - we use `validate_self_contained` instead `pre_dispatch_self_contained` +/// since the nonce is not incremented in `pre_dispatch_self_contained` +/// - Manually track the account nonce to check either Stale or Future nonce. +fn pre_dispatch_evm_transaction( + account_id: H160, + call: RuntimeCall, + dispatch_info: &DispatchInfoOf, + len: usize, +) -> Result<(), TransactionValidityError> { + match call { + RuntimeCall::Ethereum(call) => { + // Withdraw the consensus chain storage fee from the caller and record + // it in the `BlockFees` + let consensus_storage_fee = + BlockFees::consensus_chain_byte_fee() * Balance::from(len as u32); + match >::withdraw_fee( + &account_id, + consensus_storage_fee.into(), + ) { + Ok(None) => {} + Ok(Some(paid_consensus_storage_fee)) => { + BlockFees::note_consensus_storage_fee(paid_consensus_storage_fee.peek()) + } + Err(_) => return Err(InvalidTransaction::Payment.into()), + } + + if let Some(transaction_validity) = + call.validate_self_contained(&account_id, dispatch_info, len) + { + transaction_validity.map(|_| ())?; + + if let Call::transact { transaction } = call { + CheckWeight::::do_pre_dispatch(dispatch_info, len)?; + + let transaction_data: TransactionData = (&transaction).into(); + let transaction_nonce = transaction_data.nonce; + // if this the first transaction from this sender, then use the + // transaction nonce as first nonce. + // if the current account nonce is more the tracked nonce, then + // pick the highest nonce + let account_nonce = { + let tracked_nonce = + EVMNoncetracker::account_nonce(account_id).unwrap_or(transaction_nonce); + let account_nonce = EVM::account_basic(&account_id).0.nonce; + max(tracked_nonce, account_nonce) + }; + + match transaction_nonce.cmp(&account_nonce) { + Ordering::Less => return Err(InvalidTransaction::Stale.into()), + Ordering::Greater => return Err(InvalidTransaction::Future.into()), + Ordering::Equal => {} + } + + let next_nonce = account_nonce + .checked_add(U256::one()) + .ok_or(InvalidTransaction::Custom(ERR_NONCE_OVERFLOW))?; + + EVMNoncetracker::set_account_nonce(account_id, next_nonce); + } + } + + Ok(()) + } + _ => Err(InvalidTransaction::Call.into()), + } +} + fn check_transaction_and_do_pre_dispatch_inner( uxt: &::Extrinsic, ) -> Result<(), TransactionValidityError> { @@ -805,19 +884,34 @@ fn check_transaction_and_do_pre_dispatch_inner( // which would help to maintain context across multiple transaction validity check against same // runtime instance. match xt.signed { - CheckedSignature::Signed(account_id, extra) => extra - .pre_dispatch(&account_id, &xt.function, &dispatch_info, encoded_len) - .map(|_| ()), + CheckedSignature::Signed(account_id, extra) => { + // if a sender sends a one 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, + // since substrate updates nonce in pre_dispatch, evm transaction will be rejected. + if let Some(tracked_nonce) = EVMNoncetracker::account_nonce(H160::from(account_id.0)) { + let account_nonce = U256::from(System::account_nonce(account_id)); + let current_nonce = max(tracked_nonce, account_nonce); + let transaction_nonce = U256::from(extra.5 .0); + match transaction_nonce.cmp(¤t_nonce) { + Ordering::Less => return Err(InvalidTransaction::Stale.into()), + Ordering::Greater => return Err(InvalidTransaction::Future.into()), + Ordering::Equal => {} + } + } + + extra + .pre_dispatch(&account_id, &xt.function, &dispatch_info, encoded_len) + .map(|_| ()) + } CheckedSignature::Unsigned => { Runtime::pre_dispatch(&xt.function).map(|_| ())?; SignedExtra::pre_dispatch_unsigned(&xt.function, &dispatch_info, encoded_len) .map(|_| ()) } - CheckedSignature::SelfContained(self_contained_signing_info) => xt - .function - .pre_dispatch_self_contained(&self_contained_signing_info, &dispatch_info, encoded_len) - .ok_or(TransactionValidityError::Invalid(InvalidTransaction::Call)) - .map(|_| ()), + CheckedSignature::SelfContained(account_id) => { + pre_dispatch_evm_transaction(account_id, xt.function, &dispatch_info, encoded_len) + } } } From 895d9be95af8749f0161c37f85ae359b88059cca Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Mon, 29 Apr 2024 19:02:41 +0530 Subject: [PATCH 2/6] use safe arithematic while computing consensus storage fee --- crates/pallet-domains/src/tests.rs | 8 +++++++- domains/pallets/executive/src/lib.rs | 7 +++++-- domains/pallets/executive/src/mock.rs | 8 +++++++- domains/primitives/runtime/src/lib.rs | 5 +++++ domains/runtime/auto-id/src/lib.rs | 12 +++++++++--- domains/runtime/evm/src/lib.rs | 25 +++++++++++++++---------- domains/test/runtime/evm/src/lib.rs | 22 +++++++++++++++------- 7 files changed, 63 insertions(+), 24 deletions(-) diff --git a/crates/pallet-domains/src/tests.rs b/crates/pallet-domains/src/tests.rs index 2f3388346d..b275a7c8d8 100644 --- a/crates/pallet-domains/src/tests.rs +++ b/crates/pallet-domains/src/tests.rs @@ -40,6 +40,7 @@ use sp_domains_fraud_proof::{ use sp_runtime::traits::{ AccountIdConversion, BlakeTwo256, BlockNumberProvider, Hash as HashT, IdentityLookup, One, }; +use sp_runtime::transaction_validity::TransactionValidityError; use sp_runtime::{BuildStorage, Digest, OpaqueExtrinsic, Saturating}; use sp_state_machine::backend::AsTrieBackend; use sp_state_machine::{prove_read, Backend, TrieBackendBuilder}; @@ -299,7 +300,12 @@ impl domain_pallet_executive::ExtrinsicStorageFees for ExtrinsicStorageFee (None, DispatchInfo::default()) } - fn on_storage_fees_charged(_charged_fees: Balance, _tx_size: u32) {} + fn on_storage_fees_charged( + _charged_fees: Balance, + _tx_size: u32, + ) -> Result<(), TransactionValidityError> { + Ok(()) + } } impl domain_pallet_executive::Config for Test { diff --git a/domains/pallets/executive/src/lib.rs b/domains/pallets/executive/src/lib.rs index 9586664b4f..5e09ea6d93 100644 --- a/domains/pallets/executive/src/lib.rs +++ b/domains/pallets/executive/src/lib.rs @@ -68,7 +68,10 @@ pub trait ExtrinsicStorageFees { /// Extracts signer from given extrinsic and its dispatch info. fn extract_signer(xt: ExtrinsicOf) -> (Option>, DispatchInfo); /// Hook to note operator rewards for charged storage fees. - fn on_storage_fees_charged(charged_fees: BalanceOf, tx_size: u32); + fn on_storage_fees_charged( + charged_fees: BalanceOf, + tx_size: u32, + ) -> Result<(), TransactionValidityError>; } type AccountIdOf = ::AccountId; @@ -501,7 +504,7 @@ where ExecutiveConfig::ExtrinsicStorageFees::on_storage_fees_charged( charged_fees, encoded.len() as u32, - ) + )?; } } diff --git a/domains/pallets/executive/src/mock.rs b/domains/pallets/executive/src/mock.rs index 40150c6e06..f4be3a348b 100644 --- a/domains/pallets/executive/src/mock.rs +++ b/domains/pallets/executive/src/mock.rs @@ -5,6 +5,7 @@ use frame_support::weights::IdentityFee; use frame_support::{derive_impl, parameter_types}; use frame_system::mocking::MockUncheckedExtrinsic; use pallet_balances::AccountData; +use sp_runtime::transaction_validity::TransactionValidityError; use sp_runtime::BuildStorage; type Block = frame_system::mocking::MockBlock; @@ -46,7 +47,12 @@ impl crate::ExtrinsicStorageFees for ExtrinsicStorageFees { (None, DispatchInfo::default()) } - fn on_storage_fees_charged(_charged_fees: Balance, _tx_size: u32) {} + fn on_storage_fees_charged( + _charged_fees: Balance, + _tx_size: u32, + ) -> Result<(), TransactionValidityError> { + Ok(()) + } } impl Config for MockRuntime { diff --git a/domains/primitives/runtime/src/lib.rs b/domains/primitives/runtime/src/lib.rs index 1c50fd4e01..2689d6676e 100644 --- a/domains/primitives/runtime/src/lib.rs +++ b/domains/primitives/runtime/src/lib.rs @@ -72,6 +72,11 @@ pub const MAXIMUM_MANDATORY_BLOCK_LENGTH: u32 = 5 * 1024 * 1024; /// Maximum block length for operational and normal dispatches. pub const MAXIMUM_OPERATIONAL_AND_NORMAL_BLOCK_LENGTH: u32 = u32::MAX; +/// Custom error when nonce overflow occurs. +pub const ERR_NONCE_OVERFLOW: u8 = 100; +/// Custom error when balance overflow occurs. +pub const ERR_BALANCE_OVERFLOW: u8 = 200; + /// Maximum block length for all dispatches. pub fn maximum_block_length() -> BlockLength { BlockLength { diff --git a/domains/runtime/auto-id/src/lib.rs b/domains/runtime/auto-id/src/lib.rs index 513e935318..72abce5b03 100644 --- a/domains/runtime/auto-id/src/lib.rs +++ b/domains/runtime/auto-id/src/lib.rs @@ -20,7 +20,7 @@ pub use domain_runtime_primitives::{ }; use domain_runtime_primitives::{ AccountId, Address, CheckExtrinsicsValidityError, DecodeExtrinsicError, Signature, - SLOT_DURATION, + ERR_BALANCE_OVERFLOW, SLOT_DURATION, }; use frame_support::dispatch::{DispatchClass, DispatchInfo, GetDispatchInfo}; use frame_support::genesis_builder_helper::{build_config, create_default_config}; @@ -269,8 +269,13 @@ impl domain_pallet_executive::ExtrinsicStorageFees for ExtrinsicStorage (maybe_signer, dispatch_info) } - fn on_storage_fees_charged(charged_fees: Balance, tx_size: u32) { - let consensus_storage_fee = BlockFees::consensus_chain_byte_fee() * Balance::from(tx_size); + fn on_storage_fees_charged( + charged_fees: Balance, + tx_size: u32, + ) -> Result<(), TransactionValidityError> { + let consensus_storage_fee = BlockFees::consensus_chain_byte_fee() + .checked_mul(Balance::from(tx_size)) + .ok_or(InvalidTransaction::Custom(ERR_BALANCE_OVERFLOW))?; let (paid_consensus_storage_fee, paid_domain_fee) = if charged_fees <= consensus_storage_fee { @@ -281,6 +286,7 @@ impl domain_pallet_executive::ExtrinsicStorageFees for ExtrinsicStorage BlockFees::note_consensus_storage_fee(paid_consensus_storage_fee); BlockFees::note_domain_execution_fee(paid_domain_fee); + Ok(()) } } diff --git a/domains/runtime/evm/src/lib.rs b/domains/runtime/evm/src/lib.rs index 1627ea2ab7..c81a01ec2f 100644 --- a/domains/runtime/evm/src/lib.rs +++ b/domains/runtime/evm/src/lib.rs @@ -21,7 +21,8 @@ pub use domain_runtime_primitives::{ EXISTENTIAL_DEPOSIT, MAXIMUM_BLOCK_WEIGHT, }; use domain_runtime_primitives::{ - CheckExtrinsicsValidityError, DecodeExtrinsicError, SLOT_DURATION, + CheckExtrinsicsValidityError, DecodeExtrinsicError, ERR_BALANCE_OVERFLOW, ERR_NONCE_OVERFLOW, + SLOT_DURATION, }; use fp_account::EthereumSignature; use fp_self_contained::{CheckedSignature, SelfContainedCall}; @@ -84,9 +85,6 @@ use subspace_runtime_primitives::{ SlowAdjustingFeeUpdate, SSC, }; -/// Custom error when nonce overflow occurs -const ERR_NONCE_OVERFLOW: u8 = 100; - /// Alias to 512-bit hash when used in the context of a transaction signature on the chain. pub type Signature = EthereumSignature; @@ -164,7 +162,7 @@ impl fp_self_contained::SelfContainedCall for RuntimeCall { RuntimeCall::Ethereum(call) => { // Ensure the caller can pay for the consensus chain storage fee let consensus_storage_fee = - BlockFees::consensus_chain_byte_fee() * Balance::from(len as u32); + BlockFees::consensus_chain_byte_fee().checked_mul(Balance::from(len as u32))?; let withdraw_res = >::withdraw_fee(info, consensus_storage_fee.into()); @@ -189,7 +187,7 @@ impl fp_self_contained::SelfContainedCall for RuntimeCall { // Withdraw the consensus chain storage fee from the caller and record // it in the `BlockFees` let consensus_storage_fee = - BlockFees::consensus_chain_byte_fee() * Balance::from(len as u32); + BlockFees::consensus_chain_byte_fee().checked_mul(Balance::from(len as u32))?; match >::withdraw_fee( info, consensus_storage_fee.into(), @@ -388,8 +386,13 @@ impl domain_pallet_executive::ExtrinsicStorageFees for ExtrinsicStorage (maybe_signer, dispatch_info) } - fn on_storage_fees_charged(charged_fees: Balance, tx_size: u32) { - let consensus_storage_fee = BlockFees::consensus_chain_byte_fee() * Balance::from(tx_size); + fn on_storage_fees_charged( + charged_fees: Balance, + tx_size: u32, + ) -> Result<(), TransactionValidityError> { + let consensus_storage_fee = BlockFees::consensus_chain_byte_fee() + .checked_mul(Balance::from(tx_size)) + .ok_or(InvalidTransaction::Custom(ERR_BALANCE_OVERFLOW))?; let (paid_consensus_storage_fee, paid_domain_fee) = if charged_fees <= consensus_storage_fee { @@ -400,6 +403,7 @@ impl domain_pallet_executive::ExtrinsicStorageFees for ExtrinsicStorage BlockFees::note_consensus_storage_fee(paid_consensus_storage_fee); BlockFees::note_domain_execution_fee(paid_domain_fee); + Ok(()) } } @@ -842,8 +846,9 @@ fn pre_dispatch_evm_transaction( RuntimeCall::Ethereum(call) => { // Withdraw the consensus chain storage fee from the caller and record // it in the `BlockFees` - let consensus_storage_fee = - BlockFees::consensus_chain_byte_fee() * Balance::from(len as u32); + let consensus_storage_fee = BlockFees::consensus_chain_byte_fee() + .checked_mul(Balance::from(len as u32)) + .ok_or(InvalidTransaction::Custom(ERR_BALANCE_OVERFLOW))?; match >::withdraw_fee( &account_id, consensus_storage_fee.into(), diff --git a/domains/test/runtime/evm/src/lib.rs b/domains/test/runtime/evm/src/lib.rs index 654bda420f..bf877e1c94 100644 --- a/domains/test/runtime/evm/src/lib.rs +++ b/domains/test/runtime/evm/src/lib.rs @@ -17,7 +17,8 @@ use alloc::format; use codec::{Decode, Encode, MaxEncodedLen}; pub use domain_runtime_primitives::opaque::Header; use domain_runtime_primitives::{ - block_weights, maximum_block_length, EXISTENTIAL_DEPOSIT, MAXIMUM_BLOCK_WEIGHT, SLOT_DURATION, + block_weights, maximum_block_length, ERR_BALANCE_OVERFLOW, EXISTENTIAL_DEPOSIT, + MAXIMUM_BLOCK_WEIGHT, SLOT_DURATION, }; pub use domain_runtime_primitives::{ opaque, Balance, BlockNumber, CheckExtrinsicsValidityError, DecodeExtrinsicError, Hash, Nonce, @@ -161,7 +162,7 @@ impl fp_self_contained::SelfContainedCall for RuntimeCall { RuntimeCall::Ethereum(call) => { // Ensure the caller can pay the consensus chain storage fee let consensus_storage_fee = - BlockFees::consensus_chain_byte_fee() * Balance::from(len as u32); + BlockFees::consensus_chain_byte_fee().checked_mul(Balance::from(len as u32))?; let withdraw_res = >::withdraw_fee(info, consensus_storage_fee.into()); @@ -186,7 +187,7 @@ impl fp_self_contained::SelfContainedCall for RuntimeCall { // Withdraw the consensus chain storage fee from the caller and record // it in the `BlockFees` let consensus_storage_fee = - BlockFees::consensus_chain_byte_fee() * Balance::from(len as u32); + BlockFees::consensus_chain_byte_fee().checked_mul(Balance::from(len as u32))?; match >::withdraw_fee( info, consensus_storage_fee.into(), @@ -374,8 +375,13 @@ impl domain_pallet_executive::ExtrinsicStorageFees for ExtrinsicStorage (maybe_signer, dispatch_info) } - fn on_storage_fees_charged(charged_fees: Balance, tx_size: u32) { - let consensus_storage_fee = BlockFees::consensus_chain_byte_fee() * Balance::from(tx_size); + fn on_storage_fees_charged( + charged_fees: Balance, + tx_size: u32, + ) -> Result<(), TransactionValidityError> { + let consensus_storage_fee = BlockFees::consensus_chain_byte_fee() + .checked_mul(Balance::from(tx_size)) + .ok_or(InvalidTransaction::Custom(ERR_BALANCE_OVERFLOW))?; let (paid_consensus_storage_fee, paid_domain_fee) = if charged_fees <= consensus_storage_fee { @@ -386,6 +392,7 @@ impl domain_pallet_executive::ExtrinsicStorageFees for ExtrinsicStorage BlockFees::note_consensus_storage_fee(paid_consensus_storage_fee); BlockFees::note_domain_execution_fee(paid_domain_fee); + Ok(()) } } @@ -810,8 +817,9 @@ fn pre_dispatch_evm_transaction( RuntimeCall::Ethereum(call) => { // Withdraw the consensus chain storage fee from the caller and record // it in the `BlockFees` - let consensus_storage_fee = - BlockFees::consensus_chain_byte_fee() * Balance::from(len as u32); + let consensus_storage_fee = BlockFees::consensus_chain_byte_fee() + .checked_mul(Balance::from(len as u32)) + .ok_or(InvalidTransaction::Custom(ERR_BALANCE_OVERFLOW))?; match >::withdraw_fee( &account_id, consensus_storage_fee.into(), From 8455a8a5309754296c040d5fee6db6fab80a095e Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Mon, 29 Apr 2024 19:26:11 +0530 Subject: [PATCH 3/6] update comments with more descriptive notes for future reference --- domains/runtime/evm/src/lib.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/domains/runtime/evm/src/lib.rs b/domains/runtime/evm/src/lib.rs index c81a01ec2f..8f640cd7c8 100644 --- a/domains/runtime/evm/src/lib.rs +++ b/domains/runtime/evm/src/lib.rs @@ -863,16 +863,19 @@ fn pre_dispatch_evm_transaction( if let Some(transaction_validity) = call.validate_self_contained(&account_id, dispatch_info, len) { - transaction_validity.map(|_| ())?; + let _ = transaction_validity?; if let Call::transact { transaction } = call { CheckWeight::::do_pre_dispatch(dispatch_info, len)?; let transaction_data: TransactionData = (&transaction).into(); let transaction_nonce = transaction_data.nonce; - // if this the first transaction from this sender, then use the - // transaction nonce as first nonce. - // if the current account nonce is more the tracked nonce, then + // If this the first transaction from this sender for this bundle, + // then use the transaction nonce as first nonce. + // We do this to ensure all the ordered transactions coming from + // tx_pool is included in all the successive bundles within the same consensus block. + // Once the Domain block is imported, the nonce will be updated and as a result, + // if the current account nonce is greater than the tracked nonce, then // pick the highest nonce let account_nonce = { let tracked_nonce = @@ -922,10 +925,10 @@ fn check_transaction_and_do_pre_dispatch_inner( // runtime instance. match xt.signed { CheckedSignature::Signed(account_id, extra) => { - // if a sender sends a one evm transaction first and substrate transaction + // 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, - // since substrate updates nonce in pre_dispatch, evm transaction will be rejected. + // evm transaction will be rejected, since substrate updates nonce in pre_dispatch. if let Some(tracked_nonce) = EVMNoncetracker::account_nonce(H160::from(account_id.0)) { let account_nonce = U256::from(System::account_nonce(account_id)); let current_nonce = max(tracked_nonce, account_nonce); From e8145695bfed6a300fc84b6b39e1a7778f18b915 Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Tue, 30 Apr 2024 10:10:14 +0530 Subject: [PATCH 4/6] define custom SignedExtra with custom Nonce check --- domains/runtime/evm/src/check_nonce.rs | 119 ++++++++++++++++++++ domains/runtime/evm/src/lib.rs | 43 ++++--- domains/test/runtime/evm/src/check_nonce.rs | 119 ++++++++++++++++++++ domains/test/runtime/evm/src/lib.rs | 50 ++++---- 4 files changed, 292 insertions(+), 39 deletions(-) create mode 100644 domains/runtime/evm/src/check_nonce.rs create mode 100644 domains/test/runtime/evm/src/check_nonce.rs diff --git a/domains/runtime/evm/src/check_nonce.rs b/domains/runtime/evm/src/check_nonce.rs new file mode 100644 index 0000000000..c0b5889e16 --- /dev/null +++ b/domains/runtime/evm/src/check_nonce.rs @@ -0,0 +1,119 @@ +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/runtime/evm/src/lib.rs b/domains/runtime/evm/src/lib.rs index 8f640cd7c8..f8ea70012e 100644 --- a/domains/runtime/evm/src/lib.rs +++ b/domains/runtime/evm/src/lib.rs @@ -3,6 +3,7 @@ // `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. @@ -39,7 +40,6 @@ use frame_support::weights::constants::{ParityDbWeight, WEIGHT_REF_TIME_PER_SECO use frame_support::weights::{ConstantMultiplier, IdentityFee, Weight}; use frame_support::{construct_runtime, parameter_types}; use frame_system::limits::{BlockLength, BlockWeights}; -use frame_system::CheckWeight; use pallet_block_fees::fees::OnChargeDomainTransaction; use pallet_ethereum::Call::transact; use pallet_ethereum::{ @@ -119,6 +119,19 @@ pub type SignedExtra = ( pallet_transaction_payment::ChargeTransactionPayment, ); +/// Custom signed extra for check_and_pre_dispatch. +/// Only Nonce check is updated and rest remains same +type CustomSignedExtra = ( + frame_system::CheckNonZeroSender, + frame_system::CheckSpecVersion, + frame_system::CheckTxVersion, + frame_system::CheckGenesis, + frame_system::CheckMortality, + check_nonce::CheckNonce, + frame_system::CheckWeight, + pallet_transaction_payment::ChargeTransactionPayment, +); + /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = fp_self_contained::UncheckedExtrinsic; @@ -866,7 +879,7 @@ fn pre_dispatch_evm_transaction( let _ = transaction_validity?; if let Call::transact { transaction } = call { - CheckWeight::::do_pre_dispatch(dispatch_info, len)?; + frame_system::CheckWeight::::do_pre_dispatch(dispatch_info, len)?; let transaction_data: TransactionData = (&transaction).into(); let transaction_nonce = transaction_data.nonce; @@ -925,22 +938,18 @@ fn check_transaction_and_do_pre_dispatch_inner( // runtime instance. match xt.signed { CheckedSignature::Signed(account_id, extra) => { - // 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. - if let Some(tracked_nonce) = EVMNoncetracker::account_nonce(H160::from(account_id.0)) { - let account_nonce = U256::from(System::account_nonce(account_id)); - let current_nonce = max(tracked_nonce, account_nonce); - let transaction_nonce = U256::from(extra.5 .0); - match transaction_nonce.cmp(¤t_nonce) { - Ordering::Less => return Err(InvalidTransaction::Stale.into()), - Ordering::Greater => return Err(InvalidTransaction::Future.into()), - Ordering::Equal => {} - } - } + let custom_extra: CustomSignedExtra = ( + extra.0, + extra.1, + extra.2, + extra.3, + extra.4, + check_nonce::CheckNonce::from(extra.5 .0), + extra.6, + extra.7, + ); - extra + custom_extra .pre_dispatch(&account_id, &xt.function, &dispatch_info, encoded_len) .map(|_| ()) } diff --git a/domains/test/runtime/evm/src/check_nonce.rs b/domains/test/runtime/evm/src/check_nonce.rs new file mode 100644 index 0000000000..c0b5889e16 --- /dev/null +++ b/domains/test/runtime/evm/src/check_nonce.rs @@ -0,0 +1,119 @@ +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 bf877e1c94..0b1cf97e5e 100644 --- a/domains/test/runtime/evm/src/lib.rs +++ b/domains/test/runtime/evm/src/lib.rs @@ -3,6 +3,7 @@ // `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. @@ -17,8 +18,8 @@ use alloc::format; use codec::{Decode, Encode, MaxEncodedLen}; pub use domain_runtime_primitives::opaque::Header; use domain_runtime_primitives::{ - block_weights, maximum_block_length, ERR_BALANCE_OVERFLOW, EXISTENTIAL_DEPOSIT, - MAXIMUM_BLOCK_WEIGHT, SLOT_DURATION, + block_weights, maximum_block_length, ERR_BALANCE_OVERFLOW, ERR_NONCE_OVERFLOW, + EXISTENTIAL_DEPOSIT, MAXIMUM_BLOCK_WEIGHT, SLOT_DURATION, }; pub use domain_runtime_primitives::{ opaque, Balance, BlockNumber, CheckExtrinsicsValidityError, DecodeExtrinsicError, Hash, Nonce, @@ -37,7 +38,6 @@ use frame_support::weights::constants::{ParityDbWeight, WEIGHT_REF_TIME_PER_SECO use frame_support::weights::{ConstantMultiplier, IdentityFee, Weight}; use frame_support::{construct_runtime, parameter_types}; use frame_system::limits::{BlockLength, BlockWeights}; -use frame_system::CheckWeight; use pallet_block_fees::fees::OnChargeDomainTransaction; use pallet_ethereum::Call::transact; use pallet_ethereum::{ @@ -82,9 +82,6 @@ use subspace_runtime_primitives::{ BlockNumber as ConsensusBlockNumber, Hash as ConsensusBlockHash, Moment, SSC, }; -/// Custom error when nonce overflow occurs -const ERR_NONCE_OVERFLOW: u8 = 100; - /// Alias to 512-bit hash when used in the context of a transaction signature on the chain. pub type Signature = EthereumSignature; @@ -119,6 +116,19 @@ pub type SignedExtra = ( pallet_transaction_payment::ChargeTransactionPayment, ); +/// Custom signed extra for check_and_pre_dispatch. +/// Only Nonce check is updated and rest remains same +type CustomSignedExtra = ( + frame_system::CheckNonZeroSender, + frame_system::CheckSpecVersion, + frame_system::CheckTxVersion, + frame_system::CheckGenesis, + frame_system::CheckMortality, + check_nonce::CheckNonce, + frame_system::CheckWeight, + pallet_transaction_payment::ChargeTransactionPayment, +); + /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = fp_self_contained::UncheckedExtrinsic; @@ -837,7 +847,7 @@ fn pre_dispatch_evm_transaction( transaction_validity.map(|_| ())?; if let Call::transact { transaction } = call { - CheckWeight::::do_pre_dispatch(dispatch_info, len)?; + frame_system::CheckWeight::::do_pre_dispatch(dispatch_info, len)?; let transaction_data: TransactionData = (&transaction).into(); let transaction_nonce = transaction_data.nonce; @@ -893,22 +903,18 @@ fn check_transaction_and_do_pre_dispatch_inner( // runtime instance. match xt.signed { CheckedSignature::Signed(account_id, extra) => { - // if a sender sends a one 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, - // since substrate updates nonce in pre_dispatch, evm transaction will be rejected. - if let Some(tracked_nonce) = EVMNoncetracker::account_nonce(H160::from(account_id.0)) { - let account_nonce = U256::from(System::account_nonce(account_id)); - let current_nonce = max(tracked_nonce, account_nonce); - let transaction_nonce = U256::from(extra.5 .0); - match transaction_nonce.cmp(¤t_nonce) { - Ordering::Less => return Err(InvalidTransaction::Stale.into()), - Ordering::Greater => return Err(InvalidTransaction::Future.into()), - Ordering::Equal => {} - } - } + let custom_extra: CustomSignedExtra = ( + extra.0, + extra.1, + extra.2, + extra.3, + extra.4, + check_nonce::CheckNonce::from(extra.5 .0), + extra.6, + extra.7, + ); - extra + custom_extra .pre_dispatch(&account_id, &xt.function, &dispatch_info, encoded_len) .map(|_| ()) } From 1bad28c7488a07fcf8032403720e65d8e1bb603a Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Tue, 30 Apr 2024 14:35:23 +0530 Subject: [PATCH 5/6] ensure the furture ready transactions cannot be included in the bundle within same consensus block --- domains/runtime/evm/src/lib.rs | 9 ++------- domains/test/runtime/evm/src/lib.rs | 4 +--- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/domains/runtime/evm/src/lib.rs b/domains/runtime/evm/src/lib.rs index f8ea70012e..4699638d6d 100644 --- a/domains/runtime/evm/src/lib.rs +++ b/domains/runtime/evm/src/lib.rs @@ -883,16 +883,11 @@ fn pre_dispatch_evm_transaction( let transaction_data: TransactionData = (&transaction).into(); let transaction_nonce = transaction_data.nonce; - // If this the first transaction from this sender for this bundle, - // then use the transaction nonce as first nonce. - // We do this to ensure all the ordered transactions coming from - // tx_pool is included in all the successive bundles within the same consensus block. - // Once the Domain block is imported, the nonce will be updated and as a result, - // if the current account nonce is greater than the tracked nonce, then + // If the current account nonce is greater than the tracked nonce, then // pick the highest nonce let account_nonce = { let tracked_nonce = - EVMNoncetracker::account_nonce(account_id).unwrap_or(transaction_nonce); + EVMNoncetracker::account_nonce(account_id).unwrap_or(U256::zero()); let account_nonce = EVM::account_basic(&account_id).0.nonce; max(tracked_nonce, account_nonce) }; diff --git a/domains/test/runtime/evm/src/lib.rs b/domains/test/runtime/evm/src/lib.rs index 0b1cf97e5e..f6e404ea19 100644 --- a/domains/test/runtime/evm/src/lib.rs +++ b/domains/test/runtime/evm/src/lib.rs @@ -851,13 +851,11 @@ fn pre_dispatch_evm_transaction( let transaction_data: TransactionData = (&transaction).into(); let transaction_nonce = transaction_data.nonce; - // if this the first transaction from this sender, then use the - // transaction nonce as first nonce. // if the current account nonce is more the tracked nonce, then // pick the highest nonce let account_nonce = { let tracked_nonce = - EVMNoncetracker::account_nonce(account_id).unwrap_or(transaction_nonce); + EVMNoncetracker::account_nonce(account_id).unwrap_or(U256::zero()); let account_nonce = EVM::account_basic(&account_id).0.nonce; max(tracked_nonce, account_nonce) }; From 65d6a640c9e2523f090f497e37e9774b22d2fc1b Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Tue, 30 Apr 2024 19:57:23 +0530 Subject: [PATCH 6/6] 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, );