From 61635e75c1bfe913196823e17742f22e8d617b50 Mon Sep 17 00:00:00 2001
From: Wei Tang <hi@that.world>
Date: Sat, 4 Jul 2020 11:57:50 +0200
Subject: [PATCH] pallet-evm: return Ok(()) when EVM execution fails (#6493)
* pallet-evm: return Ok(()) when EVM execution fails
* Bump spec version
* Init test module
* Add fail_call_return_ok test
* Fix tests and use full match pattern
Co-authored-by: Gav Wood <gavin@parity.io>
---
substrate/frame/evm/src/lib.rs | 73 +++++++------
substrate/frame/evm/src/tests.rs | 169 +++++++++++++++++++++++++++++++
2 files changed, 213 insertions(+), 29 deletions(-)
create mode 100644 substrate/frame/evm/src/tests.rs
diff --git a/substrate/frame/evm/src/lib.rs b/substrate/frame/evm/src/lib.rs
index eebdc66b38f..f7aa51e9ffa 100644
--- a/substrate/frame/evm/src/lib.rs
+++ b/substrate/frame/evm/src/lib.rs
@@ -21,6 +21,7 @@
#![cfg_attr(not(feature = "std"), no_std)]
mod backend;
+mod tests;
pub use crate::backend::{Account, Log, Vicinity, Backend};
@@ -144,7 +145,7 @@ pub trait Trait: frame_system::Trait + pallet_timestamp::Trait {
/// Precompiles associated with this EVM engine.
type Precompiles: Precompiles;
/// Chain ID of EVM.
- type ChainId: Get<U256>;
+ type ChainId: Get<u64>;
/// EVM config used in the module.
fn config() -> &'static Config {
@@ -201,6 +202,12 @@ decl_event! {
Log(Log),
/// A contract has been created at given address.
Created(H160),
+ /// A contract was attempted to be created, but the execution failed.
+ CreatedFailed(H160),
+ /// A contract has been executed successfully with states applied.
+ Executed(H160),
+ /// A contract has been executed with errors. States are reverted with only gas fees applied.
+ ExecutedFailed(H160),
/// A deposit has been made at a given address.
BalanceDeposit(AccountId, H160, U256),
/// A withdrawal has been made from a given address.
@@ -220,12 +227,6 @@ decl_error! {
WithdrawFailed,
/// Gas price is too low.
GasPriceTooLow,
- /// Call failed
- ExitReasonFailed,
- /// Call reverted
- ExitReasonRevert,
- /// Call returned VM fatal error
- ExitReasonFatal,
/// Nonce is invalid
InvalidNonce,
}
@@ -300,7 +301,7 @@ decl_module! {
let sender = ensure_signed(origin)?;
let source = T::ConvertAccountId::convert_account_id(&sender);
- Self::execute_call(
+ match Self::execute_call(
source,
target,
input,
@@ -308,7 +309,16 @@ decl_module! {
gas_limit,
gas_price,
nonce,
- ).map_err(Into::into)
+ )? {
+ ExitReason::Succeed(_) => {
+ Module::<T>::deposit_event(Event::<T>::Executed(target));
+ },
+ ExitReason::Error(_) | ExitReason::Revert(_) | ExitReason::Fatal(_) => {
+ Module::<T>::deposit_event(Event::<T>::ExecutedFailed(target));
+ },
+ }
+
+ Ok(())
}
/// Issue an EVM create operation. This is similar to a contract creation transaction in
@@ -327,16 +337,22 @@ decl_module! {
let sender = ensure_signed(origin)?;
let source = T::ConvertAccountId::convert_account_id(&sender);
- let create_address = Self::execute_create(
+ match Self::execute_create(
source,
init,
value,
gas_limit,
gas_price,
nonce
- )?;
+ )? {
+ (create_address, ExitReason::Succeed(_)) => {
+ Module::<T>::deposit_event(Event::<T>::Created(create_address));
+ },
+ (create_address, _) => {
+ Module::<T>::deposit_event(Event::<T>::CreatedFailed(create_address));
+ },
+ }
- Module::<T>::deposit_event(Event::<T>::Created(create_address));
Ok(())
}
@@ -356,7 +372,7 @@ decl_module! {
let sender = ensure_signed(origin)?;
let source = T::ConvertAccountId::convert_account_id(&sender);
- let create_address = Self::execute_create2(
+ match Self::execute_create2(
source,
init,
salt,
@@ -364,9 +380,15 @@ decl_module! {
gas_limit,
gas_price,
nonce
- )?;
+ )? {
+ (create_address, ExitReason::Succeed(_)) => {
+ Module::<T>::deposit_event(Event::<T>::Created(create_address));
+ },
+ (create_address, _) => {
+ Module::<T>::deposit_event(Event::<T>::CreatedFailed(create_address));
+ },
+ }
- Module::<T>::deposit_event(Event::<T>::Created(create_address));
Ok(())
}
}
@@ -413,7 +435,7 @@ impl<T: Trait> Module<T> {
gas_limit: u32,
gas_price: U256,
nonce: Option<U256>
- ) -> Result<H160, Error<T>> {
+ ) -> Result<(H160, ExitReason), Error<T>> {
Self::execute_evm(
source,
value,
@@ -442,7 +464,7 @@ impl<T: Trait> Module<T> {
gas_limit: u32,
gas_price: U256,
nonce: Option<U256>
- ) -> Result<H160, Error<T>> {
+ ) -> Result<(H160, ExitReason), Error<T>> {
let code_hash = H256::from_slice(Keccak256::digest(&init).as_slice());
Self::execute_evm(
source,
@@ -473,8 +495,8 @@ impl<T: Trait> Module<T> {
gas_limit: u32,
gas_price: U256,
nonce: Option<U256>,
- ) -> Result<(), Error<T>> {
- Self::execute_evm(
+ ) -> Result<ExitReason, Error<T>> {
+ Ok(Self::execute_evm(
source,
value,
gas_limit,
@@ -487,7 +509,7 @@ impl<T: Trait> Module<T> {
input,
gas_limit as usize,
)),
- )
+ )?.1)
}
/// Execute an EVM operation.
@@ -498,7 +520,7 @@ impl<T: Trait> Module<T> {
gas_price: U256,
nonce: Option<U256>,
f: F,
- ) -> Result<R, Error<T>> where
+ ) -> Result<(R, ExitReason), Error<T>> where
F: FnOnce(&mut StackExecutor<Backend<T>>) -> (R, ExitReason),
{
let vicinity = Vicinity {
@@ -527,19 +549,12 @@ impl<T: Trait> Module<T> {
let (retv, reason) = f(&mut executor);
- let ret = match reason {
- ExitReason::Succeed(_) => Ok(retv),
- ExitReason::Error(_) => Err(Error::<T>::ExitReasonFailed),
- ExitReason::Revert(_) => Err(Error::<T>::ExitReasonRevert),
- ExitReason::Fatal(_) => Err(Error::<T>::ExitReasonFatal),
- };
-
let actual_fee = executor.fee(gas_price);
executor.deposit(source, total_fee.saturating_sub(actual_fee));
let (values, logs) = executor.deconstruct();
backend.apply(values, logs, true);
- ret
+ Ok((retv, reason))
}
}
diff --git a/substrate/frame/evm/src/tests.rs b/substrate/frame/evm/src/tests.rs
new file mode 100644
index 00000000000..b1f65e10e18
--- /dev/null
+++ b/substrate/frame/evm/src/tests.rs
@@ -0,0 +1,169 @@
+#![cfg(test)]
+
+use super::*;
+
+use std::{str::FromStr, collections::BTreeMap};
+use frame_support::{
+ assert_ok, impl_outer_origin, parameter_types, impl_outer_dispatch,
+};
+use sp_core::H256;
+// The testing primitives are very useful for avoiding having to work with signatures
+// or public keys. `u64` is used as the `AccountId` and no `Signature`s are required.
+use sp_runtime::{
+ Perbill,
+ testing::Header,
+ traits::{BlakeTwo256, IdentityLookup},
+};
+
+impl_outer_origin! {
+ pub enum Origin for Test where system = frame_system {}
+}
+
+impl_outer_dispatch! {
+ pub enum OuterCall for Test where origin: Origin {
+ self::EVM,
+ }
+}
+
+// For testing the pallet, we construct most of a mock runtime. This means
+// first constructing a configuration type (`Test`) which `impl`s each of the
+// configuration traits of pallets we want to use.
+#[derive(Clone, Eq, PartialEq)]
+pub struct Test;
+parameter_types! {
+ pub const BlockHashCount: u64 = 250;
+ pub const MaximumBlockWeight: Weight = 1024;
+ pub const MaximumBlockLength: u32 = 2 * 1024;
+ pub const AvailableBlockRatio: Perbill = Perbill::one();
+}
+impl frame_system::Trait for Test {
+ type BaseCallFilter = ();
+ type Origin = Origin;
+ type Index = u64;
+ type BlockNumber = u64;
+ type Hash = H256;
+ type Call = OuterCall;
+ type Hashing = BlakeTwo256;
+ type AccountId = H256;
+ type Lookup = IdentityLookup<Self::AccountId>;
+ type Header = Header;
+ type Event = ();
+ type BlockHashCount = BlockHashCount;
+ type MaximumBlockWeight = MaximumBlockWeight;
+ type DbWeight = ();
+ type BlockExecutionWeight = ();
+ type ExtrinsicBaseWeight = ();
+ type MaximumExtrinsicWeight = MaximumBlockWeight;
+ type MaximumBlockLength = MaximumBlockLength;
+ type AvailableBlockRatio = AvailableBlockRatio;
+ type Version = ();
+ type ModuleToIndex = ();
+ type AccountData = pallet_balances::AccountData<u64>;
+ type OnNewAccount = ();
+ type OnKilledAccount = ();
+}
+
+parameter_types! {
+ pub const ExistentialDeposit: u64 = 1;
+}
+impl pallet_balances::Trait for Test {
+ type Balance = u64;
+ type DustRemoval = ();
+ type Event = ();
+ type ExistentialDeposit = ExistentialDeposit;
+ type AccountStore = System;
+}
+
+parameter_types! {
+ pub const MinimumPeriod: u64 = 1000;
+}
+impl pallet_timestamp::Trait for Test {
+ type Moment = u64;
+ type OnTimestampSet = ();
+ type MinimumPeriod = MinimumPeriod;
+}
+
+/// Fixed gas price of `0`.
+pub struct FixedGasPrice;
+impl FeeCalculator for FixedGasPrice {
+ fn min_gas_price() -> U256 {
+ // Gas price is always one token per gas.
+ 0.into()
+ }
+}
+parameter_types! {
+ pub const EVMModuleId: ModuleId = ModuleId(*b"py/evmpa");
+}
+impl Trait for Test {
+ type ChainId = SystemChainId;
+ type ModuleId = EVMModuleId;
+ type FeeCalculator = FixedGasPrice;
+ type ConvertAccountId = HashTruncateConvertAccountId<BlakeTwo256>;
+ type Currency = Balances;
+ type Event = Event<Test>;
+ type Precompiles = ();
+}
+
+type System = frame_system::Module<Test>;
+type Balances = pallet_balances::Module<Test>;
+type EVM = Module<Test>;
+
+// This function basically just builds a genesis storage key/value store according to
+// our desired mockup.
+pub fn new_test_ext() -> sp_io::TestExternalities {
+ let mut t = frame_system::GenesisConfig::default().build_storage::<Test>().unwrap();
+
+ let mut accounts = BTreeMap::new();
+ accounts.insert(
+ H160::from_str("1000000000000000000000000000000000000001").unwrap(),
+ GenesisAccount {
+ nonce: U256::from(1),
+ balance: U256::from(1000000),
+ storage: Default::default(),
+ code: vec![
+ 0x00, // STOP
+ ],
+ }
+ );
+ accounts.insert(
+ H160::from_str("1000000000000000000000000000000000000002").unwrap(),
+ GenesisAccount {
+ nonce: U256::from(1),
+ balance: U256::from(1000000),
+ storage: Default::default(),
+ code: vec![
+ 0xff, // INVALID
+ ],
+ }
+ );
+
+ // We use default for brevity, but you can configure as desired if needed.
+ pallet_balances::GenesisConfig::<Test>::default().assimilate_storage(&mut t).unwrap();
+ GenesisConfig { accounts }.assimilate_storage(&mut t).unwrap();
+ t.into()
+}
+
+#[test]
+fn fail_call_return_ok() {
+ new_test_ext().execute_with(|| {
+ assert_ok!(EVM::call(
+ Origin::signed(H256::default()),
+ H160::from_str("1000000000000000000000000000000000000001").unwrap(),
+ Vec::new(),
+ U256::default(),
+ 1000000,
+ U256::default(),
+ None,
+ ));
+
+ assert_ok!(EVM::call(
+ Origin::signed(H256::default()),
+ H160::from_str("1000000000000000000000000000000000000002").unwrap(),
+ Vec::new(),
+ U256::default(),
+ 1000000,
+ U256::default(),
+ None,
+ ));
+ });
+}
--
GitLab