From c3d439a329f66d7cec9936fe1bc99755af837904 Mon Sep 17 00:00:00 2001 From: Frederik Gartenmeister Date: Mon, 4 Mar 2024 13:39:58 +0100 Subject: [PATCH 1/4] fix: error out on failed evm exectuon in transct pallet --- pallets/ethereum-transaction/Cargo.toml | 2 +- pallets/ethereum-transaction/src/lib.rs | 40 +++++++++++++++++++++-- pallets/ethereum-transaction/src/tests.rs | 29 +++++++--------- 3 files changed, 51 insertions(+), 20 deletions(-) diff --git a/pallets/ethereum-transaction/Cargo.toml b/pallets/ethereum-transaction/Cargo.toml index 4bf63ca27d..c40f63e1ed 100644 --- a/pallets/ethereum-transaction/Cargo.toml +++ b/pallets/ethereum-transaction/Cargo.toml @@ -40,7 +40,7 @@ cfg-traits = { workspace = true } sp-core = { workspace = true } sp-io = { workspace = true } -pallet-balances = { workspace = true } +pallet-balances = { workspace = true , default-features = true } pallet-evm-precompile-simple = { workspace = true, default-features = true } pallet-timestamp = { workspace = true, default-features = true } diff --git a/pallets/ethereum-transaction/src/lib.rs b/pallets/ethereum-transaction/src/lib.rs index 5f2d900261..dec5419c40 100644 --- a/pallets/ethereum-transaction/src/lib.rs +++ b/pallets/ethereum-transaction/src/lib.rs @@ -29,6 +29,7 @@ mod tests; #[frame_support::pallet] pub mod pallet { + use ethereum::ReceiptV3; use frame_system::pallet_prelude::OriginFor; use super::*; @@ -64,6 +65,27 @@ pub mod pallet { } } + #[pallet::error] + pub enum Error { + EvmExecutionFailed, + } + + impl Pallet + where + OriginFor: + From + Into>>, + { + fn valid_code(receipt: &ReceiptV3) -> bool { + let code = match receipt { + ReceiptV3::Legacy(inner) + | ReceiptV3::EIP2930(inner) + | ReceiptV3::EIP1559(inner) => inner.status_code, + }; + + code == 1 + } + } + impl EthereumTransactor for Pallet where OriginFor: @@ -107,7 +129,7 @@ pub mod pallet { Nonce::::put(nonce.saturating_add(U256::one())); - pallet_ethereum::Pallet::::transact( + let info = pallet_ethereum::Pallet::::transact( pallet_ethereum::Origin::EthereumTransaction(from).into(), transaction, ) @@ -129,7 +151,21 @@ pub mod pallet { .map_or(Some(read_weight), |weight| { Some(weight.saturating_add(read_weight)) }), - }) + })?; + + // NOTE: The Ethereuem side of things never returns a DispatchError + // if the execution failed. But we can check that manually by + // querying the `Pending` storage of the pallet-ethereum. + let pending = pallet_ethereum::Pending::::get(); + let last = pending.last().ok_or(DispatchError::Other( + "Ethereuem not adding pending storage. Unexpected.", + ))?; + + if Pallet::::valid_code(&last.2) { + Ok(info) + } else { + Err(Error::::EvmExecutionFailed.into()) + } } } } diff --git a/pallets/ethereum-transaction/src/tests.rs b/pallets/ethereum-transaction/src/tests.rs index e7752611f3..c737173525 100644 --- a/pallets/ethereum-transaction/src/tests.rs +++ b/pallets/ethereum-transaction/src/tests.rs @@ -4,7 +4,7 @@ use pallet_evm::{AddressMapping, Error::BalanceLow}; use sp_core::{crypto::AccountId32, H160, U256}; use super::mock::*; -use crate::pallet::Nonce; +use crate::{pallet::Nonce, Error}; mod utils { use super::*; @@ -101,22 +101,17 @@ mod call { assert_eq!(Nonce::::get(), U256::from(0)); - // NOTE: We can not check for errors as the internal `pallet-ethereum` logic - // does not transform an EVM error into an Substrate error and return - // `Ok(..)` in theses cases. - // - // We can also not mimic the `pallet-ethereum::Pallet::::transact(..)` - // code path as some needed parts are private. - assert_ok!(::call( - sender, - to, - data.as_slice(), - value, - gas_price, - gas_limit, - )); - - assert_eq!(Nonce::::get(), U256::from(1)); + assert_eq!( + ::call( + sender, + to, + data.as_slice(), + value, + gas_price, + gas_limit, + ), + Err(Error::::EvmExecutionFailed.into()) + ); }); } } From 561d27cb6d90e184d29b38f122e6fad83ee6e0a4 Mon Sep 17 00:00:00 2001 From: Frederik Gartenmeister Date: Mon, 4 Mar 2024 15:19:46 +0100 Subject: [PATCH 2/4] fix: tapl --- pallets/ethereum-transaction/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/ethereum-transaction/Cargo.toml b/pallets/ethereum-transaction/Cargo.toml index c40f63e1ed..bc71905386 100644 --- a/pallets/ethereum-transaction/Cargo.toml +++ b/pallets/ethereum-transaction/Cargo.toml @@ -40,7 +40,7 @@ cfg-traits = { workspace = true } sp-core = { workspace = true } sp-io = { workspace = true } -pallet-balances = { workspace = true , default-features = true } +pallet-balances = { workspace = true, default-features = true } pallet-evm-precompile-simple = { workspace = true, default-features = true } pallet-timestamp = { workspace = true, default-features = true } From 48931d53d072fbe6430d7c432825eb7db373d946 Mon Sep 17 00:00:00 2001 From: Frederik Gartenmeister Date: Mon, 4 Mar 2024 15:21:11 +0100 Subject: [PATCH 3/4] chore: destructure tuple --- pallets/ethereum-transaction/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pallets/ethereum-transaction/src/lib.rs b/pallets/ethereum-transaction/src/lib.rs index dec5419c40..e5673e87f1 100644 --- a/pallets/ethereum-transaction/src/lib.rs +++ b/pallets/ethereum-transaction/src/lib.rs @@ -157,11 +157,11 @@ pub mod pallet { // if the execution failed. But we can check that manually by // querying the `Pending` storage of the pallet-ethereum. let pending = pallet_ethereum::Pending::::get(); - let last = pending.last().ok_or(DispatchError::Other( + let (_, _, receipt) = pending.last().ok_or(DispatchError::Other( "Ethereuem not adding pending storage. Unexpected.", ))?; - if Pallet::::valid_code(&last.2) { + if Pallet::::valid_code(receipt) { Ok(info) } else { Err(Error::::EvmExecutionFailed.into()) From 10344dc4ea4ccba6166f02e352568a2dc80a92a0 Mon Sep 17 00:00:00 2001 From: Cosmin Damian <17934949+cdamian@users.noreply.github.com> Date: Tue, 5 Mar 2024 16:32:20 +0200 Subject: [PATCH 4/4] lp-gateway-routers: Remove value from test data (#1760) --- pallets/liquidity-pools-gateway/routers/src/tests.rs | 4 ++-- runtime/integration-tests/src/evm/ethereum_transaction.rs | 3 +-- .../integration-tests/src/generic/cases/liquidity_pools.rs | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/pallets/liquidity-pools-gateway/routers/src/tests.rs b/pallets/liquidity-pools-gateway/routers/src/tests.rs index 7c268d8db4..57d1672428 100644 --- a/pallets/liquidity-pools-gateway/routers/src/tests.rs +++ b/pallets/liquidity-pools-gateway/routers/src/tests.rs @@ -55,7 +55,7 @@ mod evm_router { target_contract_address: test_contract_address, target_contract_hash: test_contract_hash, fee_values: FeeValues { - value: U256::from(10), + value: U256::from(0), gas_limit: U256::from(10), gas_price: U256::from(10), }, @@ -337,7 +337,7 @@ mod axelar_evm { target_contract_address: axelar_contract_address, target_contract_hash: axelar_contract_hash, fee_values: FeeValues { - value: U256::from(10), + value: U256::from(0), gas_limit: U256::from(10), gas_price: U256::from(10), }, diff --git a/runtime/integration-tests/src/evm/ethereum_transaction.rs b/runtime/integration-tests/src/evm/ethereum_transaction.rs index ad783c1d48..28d9ddfd0d 100644 --- a/runtime/integration-tests/src/evm/ethereum_transaction.rs +++ b/runtime/integration-tests/src/evm/ethereum_transaction.rs @@ -147,8 +147,7 @@ async fn call() { U256::from(0x100000), ); - // NOTE: WE CAN NOTE CHECK WHETHER THE EVM ERRORS OUT - assert!(res.is_ok()); + assert!(res.is_err()); }) .unwrap(); } diff --git a/runtime/integration-tests/src/generic/cases/liquidity_pools.rs b/runtime/integration-tests/src/generic/cases/liquidity_pools.rs index 71a8e586af..b6cfc907fa 100644 --- a/runtime/integration-tests/src/generic/cases/liquidity_pools.rs +++ b/runtime/integration-tests/src/generic/cases/liquidity_pools.rs @@ -4854,8 +4854,8 @@ mod development { target_contract_address: axelar_contract_address, target_contract_hash: axelar_contract_hash, fee_values: FeeValues { - value: U256::from(10), - gas_limit: U256::from(transaction_call_cost + 10_000), + value: U256::from(0), + gas_limit: U256::from(transaction_call_cost + 1_000_000), gas_price: U256::from(10), }, };