Skip to content

Commit

Permalink
inc nonce if evm-tx fails (#2547)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ermalkaleci
ermalkaleci authored May 31, 2023
1 parent cf7aa23 commit 5c10543
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 13 deletions.
52 changes: 44 additions & 8 deletions modules/evm/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::*};

Expand Down Expand Up @@ -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,
Expand All @@ -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::<T>::deposit_event(Event::<T>::ExecutedFailed {
from: source,
Expand Down Expand Up @@ -817,15 +821,19 @@ 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,
gas_limit,
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::<T>::deposit_event(Event::<T>::CreatedFailed {
from: source,
Expand Down Expand Up @@ -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,
Expand All @@ -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::<T>::deposit_event(Event::<T>::CreatedFailed {
from: source,
Expand Down Expand Up @@ -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::<T>(used_gas)),
Expand Down Expand Up @@ -1868,6 +1879,31 @@ impl<T: Config> Pallet<T> {

Ok(())
}

fn inc_nonce_if_needed<Output>(origin: &H160, outcome: &Result<ExecutionInfo<Output>, DispatchError>) {
if matches!(
outcome,
Ok(ExecutionInfo {
exit_reason: ExitReason::Succeed(_),
..
})
) {
// nonce increased by EVM
return;
}

// EVM changes reverted, increase nonce by ourselves
Accounts::<T>::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<T: Config> EVMTrait<T::AccountId> for Pallet<T> {
Expand Down
64 changes: 59 additions & 5 deletions modules/evm/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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::<Runtime>::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::<Runtime>::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::<Runtime>::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::<Runtime>::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::<Runtime>::inc_nonce_if_needed::<H160>(&alice(), &Err(Error::<Runtime>::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));
});
}

Expand Down

0 comments on commit 5c10543

Please sign in to comment.