client fix: filter buggy transaction not included in the ethereum block

client/rpc/debug/src/lib.rs

@@ -26,8 +26,10 @@ use ethereum_types::H256;
 use fc_rpc::{frontier_backend_client, internal_err};
 use fc_storage::StorageOverride;
 use fp_rpc::EthereumRuntimeRPCApi;
+use moonbeam_client_evm_tracing::formatters::call_tracer::CallTracerInner;
 use moonbeam_client_evm_tracing::types::block;
 use moonbeam_client_evm_tracing::types::block::BlockTransactionTrace;
+use moonbeam_client_evm_tracing::types::single::TransactionTrace;
 use moonbeam_client_evm_tracing::{formatters::ResponseFormatter, types::single};
 use moonbeam_rpc_core_types::{RequestBlockId, RequestBlockTag};
 use moonbeam_rpc_primitives_debug::{DebugRuntimeApi, TracerInput};
@@ -38,6 +40,7 @@ use sp_block_builder::BlockBuilder;
 use sp_blockchain::{
 	Backend as BlockchainBackend, Error as BlockChainError, HeaderBackend, HeaderMetadata,
 };
+use sp_core::H160;
 use sp_runtime::{
 	generic::BlockId,
 	traits::{BlakeTwo256, Block as BlockT, Header as HeaderT, UniqueSaturatedInto},
@@ -422,13 +425,32 @@ where
 			.current_transaction_statuses(hash)
 			.unwrap_or_default();
 
+		// Partial ethereum transaction data to check if a trace match an ethereum transaction
+		struct EthTxPartial {
+			transaction_hash: H256,
+			from: H160,
+			to: Option<H160>,
+		}
+
 		// Known ethereum transaction hashes.
-		let eth_transactions_by_index: BTreeMap<u32, H256> = statuses
+		let eth_transactions_by_index: BTreeMap<u32, EthTxPartial> = statuses
 			.iter()
-			.map(|t| (t.transaction_index, t.transaction_hash))
+			.map(|status| {
+				(
+					status.transaction_index,
+					EthTxPartial {
+						transaction_hash: status.transaction_hash,
+						from: status.from,
+						to: status.to,
+					},
+				)
+			})
 			.collect();
 
-		let eth_tx_hashes: Vec<_> = eth_transactions_by_index.values().cloned().collect();
+		let eth_tx_hashes: Vec<_> = eth_transactions_by_index
+			.values()
+			.map(|tx| tx.transaction_hash)
+			.collect();
 
 		// If there are no ethereum transactions in the block return empty trace right away.
 		if eth_tx_hashes.is_empty() {
@@ -504,6 +526,9 @@ where
 			Ok(moonbeam_rpc_primitives_debug::Response::Block)
 		};
 
+		// Offset to account for old buggy transactions that are in trace not in the ethereum block
+		let mut tx_position_offset = 0;
+
 		return match trace_type {
 			single::TraceType::CallList => {
 				let mut proxy = moonbeam_client_evm_tracing::listeners::CallList::default();
@@ -517,13 +542,58 @@ where
 								.ok_or("Trace result is empty.")
 								.map_err(|e| internal_err(format!("{:?}", e)))?
 								.into_iter()
-								.map(|mut trace| {
-									if let Some(transaction_hash) =
-										eth_transactions_by_index.get(&trace.tx_position)
+								.filter_map(|mut trace: BlockTransactionTrace| {
+									if let Some(EthTxPartial {
+										transaction_hash,
+										from,
+										to,
+									}) = eth_transactions_by_index
+										.get(&(trace.tx_position - tx_position_offset))
 									{
-										trace.tx_hash = *transaction_hash;
+										// verify that the trace matches the ethereum transaction
+										let (trace_from, trace_to) = match trace.result {
+											TransactionTrace::Raw { .. } => {
+												(Default::default(), None)
+											}
+											TransactionTrace::CallList(_) => {
+												(Default::default(), None)
+											}
+											TransactionTrace::CallListNested(ref call) => {
+												match call {
+													single::Call::Blockscout(_) => {
+														(Default::default(), None)
+													}
+													single::Call::CallTracer(call) => (
+														call.from,
+														match call.inner {
+															CallTracerInner::Call {
+																to, ..
+															} => Some(to),
+															CallTracerInner::Create { .. } => None,
+															CallTracerInner::SelfDestruct {
+																..
+															} => None,
+														},
+													),
+												}
+											}
+										};
+										if trace_from == *from && trace_to == *to {
+											trace.tx_hash = *transaction_hash;
+											Some(trace)
+										} else {
+											// if the trace does not match the ethereum transaction
+											// it means that the trace is about a buggy transaction that is not in the block
+											// we need to offset the tx_position
+											tx_position_offset += 1;
+											None
+										}
+									} else {
+										// If the transaction is not in the ethereum block
+										// it should not appear in the block trace
+										tx_position_offset += 1;
+										None
 									}
-									trace
 								})
 								.collect::<Vec<BlockTransactionTrace>>();
 

pallets/erc20-xcm-bridge/src/lib.rs

@@ -170,18 +170,20 @@ pub mod pallet {
 				match erc20s_origins.drain(contract_address, amount) {
 					// We perform the evm transfers in a storage transaction to ensure that if one
 					// of them fails all the changes of the previous evm calls are rolled back.
-					Ok(tokens_to_transfer) => tokens_to_transfer
-						.into_iter()
-						.try_for_each(|(from, subamount)| {
-							Self::erc20_transfer(
-								contract_address,
-								from,
-								beneficiary,
-								subamount,
-								gas_limit,
-							)
-						})
-						.map_err(Into::into),
+					Ok(tokens_to_transfer) => frame_support::storage::with_storage_layer(|| {
+						tokens_to_transfer
+							.into_iter()
+							.try_for_each(|(from, subamount)| {
+								Self::erc20_transfer(
+									contract_address,
+									from,
+									beneficiary,
+									subamount,
+									gas_limit,
+								)
+							})
+					})
+					.map_err(Into::into),
 					Err(DrainError::AssetNotFound) => Err(XcmError::AssetNotFound),
 					Err(DrainError::NotEnoughFounds) => Err(XcmError::FailedToTransactAsset(
 						"not enough founds in xcm holding",
@@ -213,7 +215,11 @@ pub mod pallet {
 
 			let gas_limit = Self::gas_limit_of_erc20_transfer(&asset.id);
 
-			Self::erc20_transfer(contract_address, from, to, amount, gas_limit)?;
+			// We perform the evm transfers in a storage transaction to ensure that if it fail
+			// any contract storage changes are rolled back.
+			frame_support::storage::with_storage_layer(|| {
+				Self::erc20_transfer(contract_address, from, to, amount, gas_limit)
+			})?;
 
 			Ok(asset.clone().into())
 		}

pallets/ethereum-xcm/Cargo.toml

@@ -35,18 +35,15 @@ fp-evm = { workspace = true }
 fp-rpc = { workspace = true }
 fp-self-contained = { workspace = true }
 pallet-evm = { workspace = true, features = [ "forbid-evm-reentrancy" ] }
-pallet-ethereum = { workspace = true, features = [ "forbid-evm-reentrancy" ] }
 xcm-primitives = { workspace = true }
 
-# Polkadot
-xcm = { workspace = true }
-xcm-executor = { workspace = true }
-
 # Benchmarks
 frame-benchmarking = { workspace = true, optional = true }
 
 [dev-dependencies]
 pallet-evm-precompile-proxy = { workspace = true, features = [ "std" ] }
+
+pallet-ethereum = { workspace = true, features = [ "forbid-evm-reentrancy", "std" ] }
 pallet-evm = { workspace = true, features = [ "forbid-evm-reentrancy", "std" ] }
 pallet-proxy = { workspace = true, features = [ "std" ] }
 
@@ -71,7 +68,6 @@ std = [
 	# Substrate FRAME
 	"frame-support/std",
 	"frame-system/std",
-	"pallet-ethereum/std",
 	"pallet-evm/std",
 	"pallet-timestamp/std",
 	# Parity
@@ -85,17 +81,14 @@ std = [
 	"sp-runtime/std",
 	"sp-std/std",
 	"xcm-primitives/std",
-	# Polkadot
-	"xcm/std",
-	"xcm-executor/std",
 ]
 runtime-benchmarks = [
 	"frame-benchmarking",
 	"frame-benchmarking/runtime-benchmarks",
 	"frame-support/runtime-benchmarks",
 	"frame-system/runtime-benchmarks",
 	"pallet-evm/runtime-benchmarks", 
-	"pallet-ethereum/runtime-benchmarks",
 	"xcm-primitives/runtime-benchmarks",
+	"pallet-ethereum/runtime-benchmarks"
 ]
 try-runtime = [ "frame-support/try-runtime" ]

pallets/ethereum-xcm/src/lib.rs

@@ -26,7 +26,6 @@
 mod mock;
 #[cfg(all(feature = "std", test))]
 mod tests;
-mod transactional_processor;
 
 use ethereum_types::{H160, H256, U256};
 use fp_ethereum::{TransactionData, ValidatedTransaction};
@@ -43,7 +42,6 @@ use scale_info::TypeInfo;
 use sp_runtime::{traits::UniqueSaturatedInto, DispatchErrorWithPostInfo, RuntimeDebug};
 use sp_std::{marker::PhantomData, prelude::*};
 
-pub use self::transactional_processor::XcmEthTransactionalProcessor;
 pub use ethereum::{
 	AccessListItem, BlockV2 as Block, LegacyTransactionMessage, Log, ReceiptV3 as Receipt,
 	TransactionAction, TransactionV2 as Transaction,
@@ -374,23 +372,9 @@ impl<T: Config> Pallet<T> {
 			// transaction on chain - we increase the global nonce.
 			<Nonce<T>>::put(current_nonce.saturating_add(U256::one()));
 
-			let (dispatch_info, execution_info) =
+			let (dispatch_info, _) =
 				T::ValidatedTransaction::apply(source, transaction, maybe_force_create_address)?;
 
-			// If the transaction reverted, signal it to XCM Transactional Processor
-			match execution_info {
-				fp_evm::CallOrCreateInfo::Call(info) => {
-					if let fp_evm::ExitReason::Revert(_) = info.exit_reason {
-						XcmEthTransactionalProcessor::signal_evm_revert();
-					}
-				}
-				fp_evm::CallOrCreateInfo::Create(info) => {
-					if let fp_evm::ExitReason::Revert(_) = info.exit_reason {
-						XcmEthTransactionalProcessor::signal_evm_revert();
-					}
-				}
-			}
-
 			XCM_MESSAGE_HASH::with(|xcm_msg_hash| {
 				Self::deposit_event(Event::ExecutedFromXcm {
 					xcm_msg_hash: *xcm_msg_hash,