Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: abstract RPC error over HaltReason #14104

Merged
merged 4 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion crates/ethereum/evm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use reth_primitives_traits::transaction::execute::FillTxEnv;
use reth_revm::{inspector_handle_register, EvmBuilder};
use revm_primitives::{
AnalysisKind, BlobExcessGasAndPrice, BlockEnv, Bytes, CfgEnv, CfgEnvWithHandlerCfg, EVMError,
HandlerCfg, ResultAndState, SpecId, TxEnv, TxKind,
HaltReason, HandlerCfg, ResultAndState, SpecId, TxEnv, TxKind,
};

mod config;
Expand All @@ -53,6 +53,7 @@ impl<EXT, DB: Database> Evm for EthEvm<'_, EXT, DB> {
type DB = DB;
type Tx = TxEnv;
type Error = EVMError<DB::Error>;
type HaltReason = HaltReason;

fn block(&self) -> &BlockEnv {
self.0.block()
Expand Down Expand Up @@ -238,6 +239,7 @@ impl ConfigureEvmEnv for EthEvmConfig {
impl ConfigureEvm for EthEvmConfig {
type Evm<'a, DB: Database + 'a, I: 'a> = EthEvm<'a, I, DB>;
type EvmError<DBError: core::error::Error + Send + Sync + 'static> = EVMError<DBError>;
type HaltReason = HaltReason;

fn evm_with_env<DB: Database>(&self, db: DB, evm_env: EvmEnv) -> Self::Evm<'_, DB, ()> {
let cfg_env_with_handler_cfg = CfgEnvWithHandlerCfg {
Expand Down
7 changes: 7 additions & 0 deletions crates/evm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ pub trait Evm {
type Tx;
/// Error type.
type Error;
/// Halt reason.
type HaltReason;
klkvr marked this conversation as resolved.
Show resolved Hide resolved

/// Reference to [`BlockEnv`].
fn block(&self) -> &BlockEnv;
Expand Down Expand Up @@ -96,11 +98,15 @@ pub trait ConfigureEvm: ConfigureEvmEnv {
Tx = Self::TxEnv,
DB = DB,
Error = Self::EvmError<DB::Error>,
HaltReason = Self::HaltReason,
>;

/// The error type returned by the EVM.
type EvmError<DBError: core::error::Error + Send + Sync + 'static>: EvmError;

/// Halt reason type returned by the EVM.
type HaltReason;

/// Returns a new EVM with the given database configured with the given environment settings,
/// including the spec id and transaction environment.
///
Expand Down Expand Up @@ -147,6 +153,7 @@ where
{
type Evm<'a, DB: Database + 'a, I: 'a> = T::Evm<'a, DB, I>;
type EvmError<DBError: core::error::Error + Send + Sync + 'static> = T::EvmError<DBError>;
type HaltReason = T::HaltReason;

fn evm_for_block<DB: Database>(&self, db: DB, header: &Self::Header) -> Self::Evm<'_, DB, ()> {
(*self).evm_for_block(db, header)
Expand Down
6 changes: 4 additions & 2 deletions crates/optimism/evm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ pub use receipts::*;
mod error;
pub use error::OpBlockExecutionError;
use revm_primitives::{
BlobExcessGasAndPrice, BlockEnv, Bytes, CfgEnv, EVMError, HandlerCfg, OptimismFields,
ResultAndState, SpecId, TxKind,
BlobExcessGasAndPrice, BlockEnv, Bytes, CfgEnv, EVMError, HaltReason, HandlerCfg,
OptimismFields, ResultAndState, SpecId, TxKind,
};

/// OP EVM implementation.
Expand All @@ -53,6 +53,7 @@ impl<EXT, DB: Database> Evm for OpEvm<'_, EXT, DB> {
type DB = DB;
type Tx = TxEnv;
type Error = EVMError<DB::Error>;
type HaltReason = HaltReason;

fn block(&self) -> &BlockEnv {
self.0.block()
Expand Down Expand Up @@ -225,6 +226,7 @@ impl ConfigureEvmEnv for OpEvmConfig {
impl ConfigureEvm for OpEvmConfig {
type Evm<'a, DB: Database + 'a, I: 'a> = OpEvm<'a, I, DB>;
type EvmError<DBError: core::error::Error + Send + Sync + 'static> = EVMError<DBError>;
type HaltReason = HaltReason;

fn evm_with_env<DB: Database>(&self, db: DB, evm_env: EvmEnv) -> Self::Evm<'_, DB, ()> {
let cfg_env_with_handler_cfg = CfgEnvWithHandlerCfg {
Expand Down
10 changes: 8 additions & 2 deletions crates/optimism/rpc/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use alloy_rpc_types_eth::{error::EthRpcErrorCode, BlockError};
use jsonrpsee_types::error::INTERNAL_ERROR_CODE;
use reth_optimism_evm::OpBlockExecutionError;
use reth_rpc_eth_api::AsEthApiError;
use reth_rpc_eth_types::EthApiError;
use reth_rpc_eth_types::{error::api::FromEvmHalt, EthApiError};
use reth_rpc_server_types::result::{internal_rpc_err, rpc_err};
use revm::primitives::{EVMError, InvalidTransaction, OptimismInvalidTransaction};
use revm::primitives::{EVMError, HaltReason, InvalidTransaction, OptimismInvalidTransaction};

/// Optimism specific errors, that extend [`EthApiError`].
#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -128,3 +128,9 @@ where
Self::Eth(error.into())
}
}

impl FromEvmHalt for OpEthApiError {
fn from_evm_halt(halt: HaltReason, gas_limit: u64) -> Self {
EthApiError::from_evm_halt(halt, gas_limit).into()
}
}
6 changes: 3 additions & 3 deletions crates/rpc/rpc-eth-api/src/helpers/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use reth_revm::{
};
use reth_rpc_eth_types::{
cache::db::{StateCacheDbRefMutWrapper, StateProviderTraitObjWrapper},
error::ensure_success,
error::{api::FromEvmHalt, ensure_success},
revm_utils::{
apply_block_overrides, apply_state_overrides, caller_gas_allowance, get_precompiles,
},
Expand Down Expand Up @@ -441,7 +441,7 @@ pub trait EthCall: EstimateCall + Call + LoadPendingBlock + LoadBlock + FullEthA
match result.result {
ExecutionResult::Halt { reason, gas_used } => {
let error =
Some(RpcInvalidTransactionError::halt(reason, tx_env.gas_limit()).to_string());
Some(Self::Error::from_evm_halt(reason, tx_env.gas_limit()).to_string());
return Ok(AccessListResult { access_list, gas_used: U256::from(gas_used), error })
}
ExecutionResult::Revert { output, gas_used } => {
Expand All @@ -456,7 +456,7 @@ pub trait EthCall: EstimateCall + Call + LoadPendingBlock + LoadBlock + FullEthA
let res = match result.result {
ExecutionResult::Halt { reason, gas_used } => {
let error =
Some(RpcInvalidTransactionError::halt(reason, tx_env.gas_limit()).to_string());
Some(Self::Error::from_evm_halt(reason, tx_env.gas_limit()).to_string());
AccessListResult { access_list, gas_used: U256::from(gas_used), error }
}
ExecutionResult::Revert { output, gas_used } => {
Expand Down
38 changes: 7 additions & 31 deletions crates/rpc/rpc-eth-api/src/helpers/estimate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,9 @@ use reth_chainspec::MIN_TRANSACTION_GAS;
use reth_errors::ProviderError;
use reth_evm::{env::EvmEnv, ConfigureEvmEnv, Database, 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::{
error::api::FromEvmHalt,
revm_utils::{apply_state_overrides, caller_gas_allowance},
EthApiError, RevertError, RpcInvalidTransactionError,
};
Expand Down Expand Up @@ -148,10 +145,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(Self::Error::from_evm_halt(reason, tx_env.gas_limit()))
}
ExecutionResult::Revert { output, .. } => {
// if price or limit was included in the request then we can execute the request
Expand Down Expand Up @@ -330,32 +327,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 with a higher gas limit before, so any failure
// means that we need to increase it.
Comment on lines +330 to +332
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, this now exempts all haltreasons, I am not sure if this is okay.
judging from the docs it might because we don't expect those.
so this seems fine,
I'd like to keep the docs+context here

*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:
// <https://github.com/OpenZeppelin/openzeppelin-contracts/blob/94697be8a3f0dfcd95dfb13ffbd39b5973f5c65d/contracts/metatx/ERC2771Forwarder.sol#L360-L367>

// 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.
Comment on lines -340 to -354
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we keep these docs, why we don't return an error on halt here?

return Err(RpcInvalidTransactionError::EvmHalt(err).into_eth_err())
}
}
}
};

Ok(())
Expand Down
26 changes: 20 additions & 6 deletions crates/rpc/rpc-eth-types/src/error/api.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//! Helper traits to wrap generic l1 errors, in network specific error type configured in
//! `reth_rpc_eth_api::EthApiTypes`.

use crate::{EthApiError, RpcInvalidTransactionError};
use reth_errors::ProviderError;
use reth_evm::ConfigureEvm;

use crate::EthApiError;
use revm_primitives::HaltReason;

/// Helper trait to wrap core [`EthApiError`].
pub trait FromEthApiError: From<EthApiError> {
Expand Down Expand Up @@ -53,7 +53,7 @@ pub trait AsEthApiError {
fn as_err(&self) -> Option<&EthApiError>;

/// Returns `true` if error is
/// [`RpcInvalidTransactionError::GasTooHigh`](crate::RpcInvalidTransactionError::GasTooHigh).
/// [`RpcInvalidTransactionError::GasTooHigh`].
fn is_gas_too_high(&self) -> bool {
if let Some(err) = self.as_err() {
return err.is_gas_too_high()
Expand All @@ -63,7 +63,7 @@ pub trait AsEthApiError {
}

/// Returns `true` if error is
/// [`RpcInvalidTransactionError::GasTooLow`](crate::RpcInvalidTransactionError::GasTooLow).
/// [`RpcInvalidTransactionError::GasTooLow`].
fn is_gas_too_low(&self) -> bool {
if let Some(err) = self.as_err() {
return err.is_gas_too_low()
Expand All @@ -80,7 +80,9 @@ impl AsEthApiError for EthApiError {
}

/// Helper trait to convert from revm errors.
pub trait FromEvmError<Evm: ConfigureEvm>: From<Evm::EvmError<ProviderError>> {
pub trait FromEvmError<Evm: ConfigureEvm>:
From<Evm::EvmError<ProviderError>> + FromEvmHalt
{
/// Converts from EVM error to this type.
fn from_evm_err(err: Evm::EvmError<ProviderError>) -> Self {
err.into()
Expand All @@ -89,7 +91,19 @@ pub trait FromEvmError<Evm: ConfigureEvm>: From<Evm::EvmError<ProviderError>> {

impl<T, Evm> FromEvmError<Evm> for T
where
T: From<Evm::EvmError<ProviderError>>,
T: From<Evm::EvmError<ProviderError>> + FromEvmHalt,
Evm: ConfigureEvm,
{
}

/// Helper trait to convert from revm errors.
pub trait FromEvmHalt<Halt = HaltReason> {
/// Converts from EVM halt to this type.
fn from_evm_halt(halt: Halt, gas_limit: u64) -> Self;
}

impl FromEvmHalt for EthApiError {
fn from_evm_halt(halt: HaltReason, gas_limit: u64) -> Self {
RpcInvalidTransactionError::halt(halt, gas_limit).into()
}
}
13 changes: 8 additions & 5 deletions crates/rpc/rpc-eth-types/src/simulate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@ use revm::Database;
use revm_primitives::{Address, Bytes, ExecutionResult, TxKind};

use crate::{
error::{api::FromEthApiError, ToRpcError},
EthApiError, RevertError, RpcInvalidTransactionError,
error::{
api::{FromEthApiError, FromEvmHalt},
ToRpcError,
},
EthApiError, RevertError,
};

/// Errors which may occur during `eth_simulateV1` execution.
Expand Down Expand Up @@ -118,7 +121,7 @@ pub fn build_simulated_block<T, B>(
block: B,
) -> Result<SimulatedBlock<Block<T::Transaction, Header<B::Header>>>, T::Error>
where
T: TransactionCompat<BlockTx<B>, Error: FromEthApiError>,
T: TransactionCompat<BlockTx<B>, Error: FromEthApiError + FromEvmHalt>,
B: reth_primitives_traits::Block,
{
let mut calls: Vec<SimCallResult> = Vec::with_capacity(results.len());
Expand All @@ -127,12 +130,12 @@ 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 = T::Error::from_evm_halt(*reason, tx.gas_limit());
SimCallResult {
return_data: Bytes::new(),
error: Some(SimulateError {
code: error.error_code(),
message: error.to_string(),
code: error.into().code(),
}),
gas_used: *gas_used,
logs: Vec::new(),
Expand Down
4 changes: 3 additions & 1 deletion examples/custom-evm/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ use reth::{
inspector_handle_register,
precompile::{Precompile, PrecompileOutput, PrecompileSpecId},
primitives::{
CfgEnvWithHandlerCfg, EVMError, Env, HandlerCfg, PrecompileResult, SpecId, TxEnv,
CfgEnvWithHandlerCfg, EVMError, Env, HaltReason, HandlerCfg, PrecompileResult, SpecId,
TxEnv,
},
ContextPrecompiles, EvmBuilder, GetInspector,
},
Expand Down Expand Up @@ -112,6 +113,7 @@ impl ConfigureEvmEnv for MyEvmConfig {
impl ConfigureEvm for MyEvmConfig {
type Evm<'a, DB: Database + 'a, I: 'a> = EthEvm<'a, I, DB>;
type EvmError<DBError: core::error::Error + Send + Sync + 'static> = EVMError<DBError>;
type HaltReason = HaltReason;

fn evm_with_env<DB: Database>(&self, db: DB, evm_env: EvmEnv) -> Self::Evm<'_, DB, ()> {
let cfg_env_with_handler_cfg = CfgEnvWithHandlerCfg {
Expand Down
3 changes: 2 additions & 1 deletion examples/stateful-precompile/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use reth::{
inspector_handle_register,
precompile::{Precompile, PrecompileSpecId},
primitives::{
CfgEnvWithHandlerCfg, EVMError, Env, HandlerCfg, PrecompileResult, SpecId,
CfgEnvWithHandlerCfg, EVMError, Env, HaltReason, HandlerCfg, PrecompileResult, SpecId,
StatefulPrecompileMut, TxEnv,
},
ContextPrecompile, ContextPrecompiles, EvmBuilder, GetInspector,
Expand Down Expand Up @@ -174,6 +174,7 @@ impl ConfigureEvmEnv for MyEvmConfig {
impl ConfigureEvm for MyEvmConfig {
type Evm<'a, DB: Database + 'a, I: 'a> = EthEvm<'a, I, DB>;
type EvmError<DBError: core::error::Error + Send + Sync + 'static> = EVMError<DBError>;
type HaltReason = HaltReason;

fn evm_with_env<DB: Database>(&self, db: DB, evm_env: EvmEnv) -> Self::Evm<'_, DB, ()> {
let cfg_env_with_handler_cfg = CfgEnvWithHandlerCfg {
Expand Down
Loading