From 4d8442d83ab4584675bd5b1e521b03a33b642966 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Wed, 29 Jan 2025 19:10:19 +0400 Subject: [PATCH] refactor: do not match on HaltReason manually --- crates/rpc/rpc-eth-api/src/helpers/call.rs | 8 ++-- .../rpc/rpc-eth-api/src/helpers/estimate.rs | 39 ++++--------------- crates/rpc/rpc-eth-types/src/error/mod.rs | 39 ++++--------------- crates/rpc/rpc-eth-types/src/simulate.rs | 4 +- 4 files changed, 19 insertions(+), 71 deletions(-) diff --git a/crates/rpc/rpc-eth-api/src/helpers/call.rs b/crates/rpc/rpc-eth-api/src/helpers/call.rs index 49604c10481f..c799473d0815 100644 --- a/crates/rpc/rpc-eth-api/src/helpers/call.rs +++ b/crates/rpc/rpc-eth-api/src/helpers/call.rs @@ -436,8 +436,7 @@ pub trait EthCall: EstimateCall + Call + LoadPendingBlock + LoadBlock + FullEthA tx_env.set_access_list(access_list.clone()); match result.result { ExecutionResult::Halt { reason, gas_used } => { - let error = - Some(RpcInvalidTransactionError::halt(reason, tx_env.gas_limit()).to_string()); + let error = Some(RpcInvalidTransactionError::halt(reason).to_string()); return Ok(AccessListResult { access_list, gas_used: U256::from(gas_used), error }) } ExecutionResult::Revert { output, gas_used } => { @@ -448,11 +447,10 @@ pub trait EthCall: EstimateCall + Call + LoadPendingBlock + LoadBlock + FullEthA }; // transact again to get the exact gas used - let (result, (_, tx_env)) = self.transact(&mut db, evm_env, tx_env)?; + let (result, _) = self.transact(&mut db, evm_env, tx_env)?; let res = match result.result { ExecutionResult::Halt { reason, gas_used } => { - let error = - Some(RpcInvalidTransactionError::halt(reason, tx_env.gas_limit()).to_string()); + let error = Some(RpcInvalidTransactionError::halt(reason).to_string()); AccessListResult { access_list, gas_used: U256::from(gas_used), error } } ExecutionResult::Revert { output, gas_used } => { diff --git a/crates/rpc/rpc-eth-api/src/helpers/estimate.rs b/crates/rpc/rpc-eth-api/src/helpers/estimate.rs index b64edaf16e03..4dc6fe46cd00 100644 --- a/crates/rpc/rpc-eth-api/src/helpers/estimate.rs +++ b/crates/rpc/rpc-eth-api/src/helpers/estimate.rs @@ -8,11 +8,7 @@ use futures::Future; use reth_chainspec::MIN_TRANSACTION_GAS; use reth_evm::{env::EvmEnv, ConfigureEvmEnv, TransactionEnv}; use reth_provider::StateProvider; -use reth_revm::{ - database::StateProviderDatabase, - db::CacheDB, - primitives::{ExecutionResult, HaltReason}, -}; +use reth_revm::{database::StateProviderDatabase, db::CacheDB, primitives::ExecutionResult}; use reth_rpc_eth_types::{ revm_utils::{apply_state_overrides, caller_gas_allowance}, EthApiError, RevertError, RpcInvalidTransactionError, @@ -147,10 +143,10 @@ pub trait EstimateCall: Call { let gas_refund = match res.result { ExecutionResult::Success { gas_refunded, .. } => gas_refunded, - ExecutionResult::Halt { reason, gas_used } => { + ExecutionResult::Halt { reason, .. } => { // here we don't check for invalid opcode because already executed with highest gas // limit - return Err(RpcInvalidTransactionError::halt(reason, gas_used).into_eth_err()) + return Err(RpcInvalidTransactionError::halt(reason).into_eth_err()) } ExecutionResult::Revert { output, .. } => { // if price or limit was included in the request then we can execute the request @@ -306,7 +302,7 @@ pub trait EstimateCall: Call { RpcInvalidTransactionError::Revert(RevertError::new(output)).into_eth_err() } ExecutionResult::Halt { reason, .. } => { - RpcInvalidTransactionError::EvmHalt(reason).into_eth_err() + RpcInvalidTransactionError::halt(reason).into_eth_err() } } } @@ -329,32 +325,11 @@ pub fn update_estimated_gas_range( // Cap the highest gas limit with the succeeding gas limit. *highest_gas_limit = tx_gas_limit; } - ExecutionResult::Revert { .. } => { - // Increase the lowest gas limit. + ExecutionResult::Revert { .. } | ExecutionResult::Halt { .. } => { + // We know that transaction succeeded before, so any failure means that we need to + // increase gas limit. *lowest_gas_limit = tx_gas_limit; } - ExecutionResult::Halt { reason, .. } => { - match reason { - HaltReason::OutOfGas(_) | HaltReason::InvalidFEOpcode => { - // Both `OutOfGas` and `InvalidEFOpcode` can occur dynamically if the gas - // left is too low. Treat this as an out of gas - // condition, knowing that the call succeeds with a - // higher gas limit. - // - // Common usage of invalid opcode in OpenZeppelin: - // - - // Increase the lowest gas limit. - *lowest_gas_limit = tx_gas_limit; - } - err => { - // These cases should be unreachable because we know the transaction - // succeeds, but if they occur, treat them as an - // error. - return Err(RpcInvalidTransactionError::EvmHalt(err).into_eth_err()) - } - } - } }; Ok(()) diff --git a/crates/rpc/rpc-eth-types/src/error/mod.rs b/crates/rpc/rpc-eth-types/src/error/mod.rs index d798002f33cd..d5bbd1b13691 100644 --- a/crates/rpc/rpc-eth-types/src/error/mod.rs +++ b/crates/rpc/rpc-eth-types/src/error/mod.rs @@ -4,6 +4,7 @@ pub mod api; pub use api::{AsEthApiError, FromEthApiError, FromEvmError, IntoEthApiError}; use core::time::Duration; +use std::fmt::Debug; use alloy_eips::BlockId; use alloy_primitives::{Address, Bytes, U256}; @@ -18,7 +19,7 @@ use reth_transaction_pool::error::{ Eip4844PoolTransactionError, Eip7702PoolTransactionError, InvalidPoolTransactionError, PoolError, PoolErrorKind, PoolTransactionError, }; -use revm::primitives::{EVMError, ExecutionResult, HaltReason, InvalidTransaction, OutOfGasError}; +use revm::primitives::{EVMError, ExecutionResult, InvalidTransaction}; use revm_inspectors::tracing::MuxError; use revm_primitives::InvalidHeader; use tracing::error; @@ -371,24 +372,12 @@ pub enum RpcInvalidTransactionError { /// Contains the gas limit. #[error("out of gas: gas required exceeds: {0}")] BasicOutOfGas(u64), - /// Gas limit was exceeded during memory expansion. - /// Contains the gas limit. - #[error("out of gas: gas exhausted during memory expansion: {0}")] - MemoryOutOfGas(u64), - /// Gas limit was exceeded during precompile execution. - /// Contains the gas limit. - #[error("out of gas: gas exhausted during precompiled contract execution: {0}")] - PrecompileOutOfGas(u64), - /// An operand to an opcode was invalid or out of range. - /// Contains the gas limit. - #[error("out of gas: invalid operand to an opcode: {0}")] - InvalidOperandOutOfGas(u64), /// Thrown if executing a transaction failed during estimate/call #[error(transparent)] Revert(RevertError), /// Unspecific EVM halt error. #[error("EVM error: {0:?}")] - EvmHalt(HaltReason), + EvmHalt(Box), /// Invalid chain id set for the transaction. #[error("invalid chain ID")] InvalidChainId, @@ -459,22 +448,8 @@ impl RpcInvalidTransactionError { /// Converts the halt error /// /// Takes the configured gas limit of the transaction which is attached to the error - pub const fn halt(reason: HaltReason, gas_limit: u64) -> Self { - match reason { - HaltReason::OutOfGas(err) => Self::out_of_gas(err, gas_limit), - HaltReason::NonceOverflow => Self::NonceMaxValue, - err => Self::EvmHalt(err), - } - } - - /// Converts the out of gas error - pub const fn out_of_gas(reason: OutOfGasError, gas_limit: u64) -> Self { - match reason { - OutOfGasError::Basic => Self::BasicOutOfGas(gas_limit), - OutOfGasError::Memory | OutOfGasError::MemoryLimit => Self::MemoryOutOfGas(gas_limit), - OutOfGasError::Precompile => Self::PrecompileOutOfGas(gas_limit), - OutOfGasError::InvalidOperand => Self::InvalidOperandOutOfGas(gas_limit), - } + pub fn halt(reason: impl Debug + Send + Sync + 'static) -> Self { + Self::EvmHalt(Box::new(reason)) } } @@ -760,8 +735,8 @@ pub fn ensure_success(result: ExecutionResult) -> EthResult { ExecutionResult::Revert { output, .. } => { Err(RpcInvalidTransactionError::Revert(RevertError::new(output)).into()) } - ExecutionResult::Halt { reason, gas_used } => { - Err(RpcInvalidTransactionError::halt(reason, gas_used).into()) + ExecutionResult::Halt { reason, .. } => { + Err(RpcInvalidTransactionError::halt(reason).into()) } } } diff --git a/crates/rpc/rpc-eth-types/src/simulate.rs b/crates/rpc/rpc-eth-types/src/simulate.rs index 0782d62ac200..5eeefe7ee852 100644 --- a/crates/rpc/rpc-eth-types/src/simulate.rs +++ b/crates/rpc/rpc-eth-types/src/simulate.rs @@ -1,6 +1,6 @@ //! Utilities for serving `eth_simulateV1` -use alloy_consensus::{BlockHeader, Transaction as _, TxType}; +use alloy_consensus::{BlockHeader, TxType}; use alloy_rpc_types_eth::{ simulate::{SimCallResult, SimulateError, SimulatedBlock}, transaction::TransactionRequest, @@ -127,7 +127,7 @@ where for (index, (result, tx)) in results.iter().zip(block.body().transactions()).enumerate() { let call = match result { ExecutionResult::Halt { reason, gas_used } => { - let error = RpcInvalidTransactionError::halt(*reason, tx.gas_limit()); + let error = RpcInvalidTransactionError::halt(*reason); SimCallResult { return_data: Bytes::new(), error: Some(SimulateError {