From 5c1054391030e55c68f0159f8d9d359fd4b136e8 Mon Sep 17 00:00:00 2001 From: Ermal Kaleci Date: Wed, 31 May 2023 09:39:17 +0200 Subject: [PATCH] inc nonce if evm-tx fails (#2547) --- modules/evm/src/lib.rs | 52 +++++++++++++++++++++++++++----- modules/evm/src/tests.rs | 64 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 103 insertions(+), 13 deletions(-) diff --git a/modules/evm/src/lib.rs b/modules/evm/src/lib.rs index 707f961134..92567fbfaa 100644 --- a/modules/evm/src/lib.rs +++ b/modules/evm/src/lib.rs @@ -73,7 +73,7 @@ use sp_core::{H160, H256, U256}; use sp_runtime::{ traits::{Convert, DispatchInfoOf, One, PostDispatchInfoOf, SignedExtension, UniqueSaturatedInto, Zero}, transaction_validity::TransactionValidityError, - Either, SaturatedConversion, TransactionOutcome, + Either, SaturatedConversion, Saturating, TransactionOutcome, }; use sp_std::{cmp, collections::btree_map::BTreeMap, fmt::Debug, marker::PhantomData, prelude::*}; @@ -634,7 +634,7 @@ pub mod module { let who = ensure_signed(origin)?; let source = T::AddressMapping::get_or_create_evm_address(&who); - match T::Runner::call( + let outcome = T::Runner::call( source, source, target, @@ -644,7 +644,11 @@ pub mod module { storage_limit, access_list.into_iter().map(|v| (v.address, v.storage_keys)).collect(), T::config(), - ) { + ); + + Self::inc_nonce_if_needed(&source, &outcome); + + match outcome { Err(e) => { Pallet::::deposit_event(Event::::ExecutedFailed { from: source, @@ -817,7 +821,7 @@ pub mod module { let who = ensure_signed(origin)?; let source = T::AddressMapping::get_or_create_evm_address(&who); - match T::Runner::create( + let outcome = T::Runner::create( source, input, value, @@ -825,7 +829,11 @@ pub mod module { storage_limit, access_list.into_iter().map(|v| (v.address, v.storage_keys)).collect(), T::config(), - ) { + ); + + Self::inc_nonce_if_needed(&source, &outcome); + + match outcome { Err(e) => { Pallet::::deposit_event(Event::::CreatedFailed { from: source, @@ -891,7 +899,7 @@ pub mod module { let who = ensure_signed(origin)?; let source = T::AddressMapping::get_or_create_evm_address(&who); - match T::Runner::create2( + let outcome = T::Runner::create2( source, input, salt, @@ -900,7 +908,11 @@ pub mod module { storage_limit, access_list.into_iter().map(|v| (v.address, v.storage_keys)).collect(), T::config(), - ) { + ); + + Self::inc_nonce_if_needed(&source, &outcome); + + match outcome { Err(e) => { Pallet::::deposit_event(Event::::CreatedFailed { from: source, @@ -1299,7 +1311,6 @@ pub mod module { "batch_call failed: [from: {:?}, contract: {:?}, exit_reason: {:?}, output: {:?}, logs: {:?}, used_gas: {:?}]", source, target, info.exit_reason, info.value, info.logs, used_gas ); - Err(DispatchErrorWithPostInfo { post_info: PostDispatchInfo { actual_weight: Some(call_weight::(used_gas)), @@ -1868,6 +1879,31 @@ impl Pallet { Ok(()) } + + fn inc_nonce_if_needed(origin: &H160, outcome: &Result, DispatchError>) { + if matches!( + outcome, + Ok(ExecutionInfo { + exit_reason: ExitReason::Succeed(_), + .. + }) + ) { + // nonce increased by EVM + return; + } + + // EVM changes reverted, increase nonce by ourselves + Accounts::::mutate(origin, |account| { + if let Some(info) = account.as_mut() { + info.nonce = info.nonce.saturating_add(T::Index::one()); + } else { + *account = Some(AccountInfo { + nonce: T::Index::one(), + contract_info: None, + }); + } + }); + } } impl EVMTrait for Pallet { diff --git a/modules/evm/src/tests.rs b/modules/evm/src/tests.rs index 35ae69340a..c9d91dc649 100644 --- a/modules/evm/src/tests.rs +++ b/modules/evm/src/tests.rs @@ -26,7 +26,7 @@ use crate::runner::{ state::{StackExecutor, StackState, StackSubstateMetadata}, }; use frame_support::{assert_noop, assert_ok, dispatch::DispatchErrorWithPostInfo}; -use module_support::AddressMapping; +use module_support::{mocks::MockAddressMapping, AddressMapping}; use sp_core::{ bytes::{from_hex, to_hex}, H160, @@ -35,23 +35,77 @@ use sp_runtime::{traits::BadOrigin, AccountId32}; use std::str::FromStr; #[test] -fn fail_call_return_ok() { +fn inc_nonce_if_needed() { + new_test_ext().execute_with(|| { + assert_eq!(EVM::account_basic(&alice()).nonce, U256::from(1)); + + let mut call_info = CallInfo { + exit_reason: ExitReason::Succeed(ExitSucceed::Returned), + value: vec![], + used_gas: Default::default(), + used_storage: 0, + logs: vec![], + }; + + // succeed call won't inc nonce + Pallet::::inc_nonce_if_needed(&alice(), &Ok(call_info.clone())); + assert_eq!(EVM::account_basic(&alice()).nonce, U256::from(1)); + + call_info.exit_reason = ExitReason::Revert(ExitRevert::Reverted); + // revert call will inc nonce + Pallet::::inc_nonce_if_needed(&alice(), &Ok(call_info.clone())); + assert_eq!(EVM::account_basic(&alice()).nonce, U256::from(2)); + + call_info.exit_reason = ExitReason::Fatal(ExitFatal::NotSupported); + // fatal call will inc nonce + Pallet::::inc_nonce_if_needed(&alice(), &Ok(call_info.clone())); + assert_eq!(EVM::account_basic(&alice()).nonce, U256::from(3)); + + call_info.exit_reason = ExitReason::Error(ExitError::OutOfGas); + // error call will inc nonce + Pallet::::inc_nonce_if_needed(&alice(), &Ok(call_info.clone())); + assert_eq!(EVM::account_basic(&alice()).nonce, U256::from(4)); + + // dispatch error will inc nonce + Pallet::::inc_nonce_if_needed::(&alice(), &Err(Error::::InvalidDecimals.into())); + assert_eq!(EVM::account_basic(&alice()).nonce, U256::from(5)); + }); +} + +#[test] +fn fail_call_return_ok_and_inc_nonce() { new_test_ext().execute_with(|| { let mut data = [0u8; 32]; data[0..4].copy_from_slice(b"evm:"); let signer: AccountId32 = AccountId32::from(data); - + let alice = MockAddressMapping::get_or_create_evm_address(&signer); let origin = RuntimeOrigin::signed(signer); + + // nonce 0 + assert_eq!(EVM::account_basic(&alice).nonce, U256::zero()); + + // out of gas + assert_ok!(EVM::call(origin.clone(), contract_a(), Vec::new(), 0, 100, 0, vec![])); + // nonce inc by 1 + assert_eq!(EVM::account_basic(&alice).nonce, U256::from(1)); + + // success call assert_ok!(EVM::call( origin.clone(), - contract_a(), + contract_b(), Vec::new(), 0, 1000000, 0, vec![] )); - assert_ok!(EVM::call(origin, contract_b(), Vec::new(), 0, 1000000, 0, vec![])); + // nonce inc by 1 + assert_eq!(EVM::account_basic(&alice).nonce, U256::from(2)); + + // invalid decimals + assert_ok!(EVM::call(origin, contract_b(), Vec::new(), 1111, 1000000, 0, vec![])); + // nonce inc by 1 + assert_eq!(EVM::account_basic(&alice).nonce, U256::from(3)); }); }