From 2837fb6b9ab6b7849523e723d856dcdf9e0dd6c0 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Fri, 19 May 2023 19:35:05 +0200 Subject: [PATCH] chore: properly convert invalid transaction errors (#2748) --- crates/primitives/src/transaction/error.rs | 2 + crates/rpc/rpc/src/eth/api/call.rs | 31 +++-- crates/rpc/rpc/src/eth/api/fees.rs | 4 +- crates/rpc/rpc/src/eth/api/state.rs | 4 +- crates/rpc/rpc/src/eth/error.rs | 151 +++++++++++++++------ crates/rpc/rpc/src/eth/gas_oracle.rs | 4 +- crates/rpc/rpc/src/eth/revm_utils.rs | 10 +- 7 files changed, 141 insertions(+), 65 deletions(-) diff --git a/crates/primitives/src/transaction/error.rs b/crates/primitives/src/transaction/error.rs index 44360f02d8..e3860ab3af 100644 --- a/crates/primitives/src/transaction/error.rs +++ b/crates/primitives/src/transaction/error.rs @@ -9,6 +9,8 @@ pub enum InvalidTransactionError { #[error("Sender does not have enough funds ({available_funds:?}) to cover transaction fees: {cost:?}.")] InsufficientFunds { cost: U256, available_funds: U256 }, /// The nonce is lower than the account's nonce, or there is a nonce gap present. + /// + /// This is a consensus error. #[error("Transaction nonce is not consistent.")] NonceNotConsistent, /// The transaction is before Spurious Dragon and has a chain ID diff --git a/crates/rpc/rpc/src/eth/api/call.rs b/crates/rpc/rpc/src/eth/api/call.rs index 7daca66c68..d857b42ad7 100644 --- a/crates/rpc/rpc/src/eth/api/call.rs +++ b/crates/rpc/rpc/src/eth/api/call.rs @@ -2,7 +2,7 @@ use crate::{ eth::{ - error::{ensure_success, EthApiError, EthResult, InvalidTransactionError, RevertError}, + error::{ensure_success, EthApiError, EthResult, RevertError, RpcInvalidTransactionError}, revm_utils::{ build_call_evm_env, cap_tx_gas_limit_with_caller_allowance, get_precompiles, inspect, transact, @@ -111,7 +111,9 @@ where let available_funds = db.basic(env.tx.caller)?.map(|acc| acc.balance).unwrap_or_default(); if env.tx.value > available_funds { - return Err(InvalidTransactionError::InsufficientFundsForTransfer.into()) + return Err( + RpcInvalidTransactionError::InsufficientFundsForTransfer.into() + ) } return Ok(U256::from(MIN_TRANSACTION_GAS)) } @@ -125,7 +127,7 @@ where let mut available_funds = db.basic(env.tx.caller)?.map(|acc| acc.balance).unwrap_or_default(); if env.tx.value > available_funds { - return Err(InvalidTransactionError::InsufficientFunds.into()) + return Err(RpcInvalidTransactionError::InsufficientFunds.into()) } // subtract transferred value from available funds // SAFETY: value < available_funds, checked above @@ -151,7 +153,8 @@ where // Exceptional case: init used too much gas, we need to increase the gas limit and try // again - if let Err(EthApiError::InvalidTransaction(InvalidTransactionError::GasTooHigh)) = ethres { + if let Err(EthApiError::InvalidTransaction(RpcInvalidTransactionError::GasTooHigh)) = ethres + { // if price or limit was included in the request then we can execute the request // again with the block's gas limit to check if revert is gas related or not if request_gas.is_some() || request_gas_price.is_some() { @@ -165,7 +168,7 @@ where // succeeded } ExecutionResult::Halt { reason, gas_used } => { - return Err(InvalidTransactionError::halt(reason, gas_used).into()) + return Err(RpcInvalidTransactionError::halt(reason, gas_used).into()) } ExecutionResult::Revert { output, .. } => { // if price or limit was included in the request then we can execute the request @@ -174,7 +177,7 @@ where Err(map_out_of_gas_err(env_gas_limit, env, &mut db)) } else { // the transaction did revert - Err(InvalidTransactionError::Revert(RevertError::new(output)).into()) + Err(RpcInvalidTransactionError::Revert(RevertError::new(output)).into()) } } } @@ -204,7 +207,7 @@ where // Exceptional case: init used too much gas, we need to increase the gas limit and try // again - if let Err(EthApiError::InvalidTransaction(InvalidTransactionError::GasTooHigh)) = + if let Err(EthApiError::InvalidTransaction(RpcInvalidTransactionError::GasTooHigh)) = ethres { // increase the lowest gas limit @@ -234,7 +237,7 @@ where err => { // these should be unreachable because we know the transaction succeeds, // but we consider these cases an error - return Err(InvalidTransactionError::EvmHalt(err).into()) + return Err(RpcInvalidTransactionError::EvmHalt(err).into()) } } } @@ -289,11 +292,11 @@ where match result.result { ExecutionResult::Halt { reason, .. } => Err(match reason { - Halt::NonceOverflow => InvalidTransactionError::NonceMaxValue, - halt => InvalidTransactionError::EvmHalt(halt), + Halt::NonceOverflow => RpcInvalidTransactionError::NonceMaxValue, + halt => RpcInvalidTransactionError::EvmHalt(halt), }), ExecutionResult::Revert { output, .. } => { - Err(InvalidTransactionError::Revert(RevertError::new(output))) + Err(RpcInvalidTransactionError::Revert(RevertError::new(output))) } ExecutionResult::Success { .. } => Ok(()), }?; @@ -322,12 +325,12 @@ where ExecutionResult::Success { .. } => { // transaction succeeded by manually increasing the gas limit to // highest, which means the caller lacks funds to pay for the tx - InvalidTransactionError::BasicOutOfGas(U256::from(req_gas_limit)).into() + RpcInvalidTransactionError::BasicOutOfGas(U256::from(req_gas_limit)).into() } ExecutionResult::Revert { output, .. } => { // reverted again after bumping the limit - InvalidTransactionError::Revert(RevertError::new(output)).into() + RpcInvalidTransactionError::Revert(RevertError::new(output)).into() } - ExecutionResult::Halt { reason, .. } => InvalidTransactionError::EvmHalt(reason).into(), + ExecutionResult::Halt { reason, .. } => RpcInvalidTransactionError::EvmHalt(reason).into(), } } diff --git a/crates/rpc/rpc/src/eth/api/fees.rs b/crates/rpc/rpc/src/eth/api/fees.rs index 746eaf6676..980961e738 100644 --- a/crates/rpc/rpc/src/eth/api/fees.rs +++ b/crates/rpc/rpc/src/eth/api/fees.rs @@ -1,7 +1,7 @@ //! Contains RPC handler implementations for fee history. use crate::{ - eth::error::{EthApiError, EthResult, InvalidTransactionError}, + eth::error::{EthApiError, EthResult, RpcInvalidTransactionError}, EthApi, }; use reth_network_api::NetworkInfo; @@ -129,7 +129,7 @@ where for transaction in transactions.iter() { let reward = transaction .effective_gas_tip(header.base_fee_per_gas) - .ok_or(InvalidTransactionError::FeeCapTooLow)?; + .ok_or(RpcInvalidTransactionError::FeeCapTooLow)?; sorter.push(TxGasAndReward { gas_used: header.gas_used as u128, reward }) } diff --git a/crates/rpc/rpc/src/eth/api/state.rs b/crates/rpc/rpc/src/eth/api/state.rs index 42d9a285af..92f1caa9a5 100644 --- a/crates/rpc/rpc/src/eth/api/state.rs +++ b/crates/rpc/rpc/src/eth/api/state.rs @@ -1,7 +1,7 @@ //! Contains RPC handler implementations specific to state. use crate::{ - eth::error::{EthApiError, EthResult, InvalidTransactionError}, + eth::error::{EthApiError, EthResult, RpcInvalidTransactionError}, EthApi, }; use reth_primitives::{ @@ -62,7 +62,7 @@ where .transaction .nonce() .checked_add(1) - .ok_or(InvalidTransactionError::NonceMaxValue)?; + .ok_or(RpcInvalidTransactionError::NonceMaxValue)?; return Ok(U256::from(tx_count)) } } diff --git a/crates/rpc/rpc/src/eth/error.rs b/crates/rpc/rpc/src/eth/error.rs index ebea4c6754..4f57f5848a 100644 --- a/crates/rpc/rpc/src/eth/error.rs +++ b/crates/rpc/rpc/src/eth/error.rs @@ -37,7 +37,7 @@ pub enum EthApiError { #[error("both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified")] ConflictingFeeFieldsInRequest, #[error(transparent)] - InvalidTransaction(#[from] InvalidTransactionError), + InvalidTransaction(#[from] RpcInvalidTransactionError), /// Thrown when constructing an RPC block from a primitive block data failed. #[error(transparent)] InvalidBlockData(#[from] BlockError), @@ -83,7 +83,7 @@ impl From for ErrorObject<'static> { EthApiError::BothStateAndStateDiffInOverride(_) | EthApiError::InvalidTracerConfig => invalid_params_rpc_err(error.to_string()), EthApiError::InvalidTransaction(err) => err.into(), - EthApiError::PoolError(_) | + EthApiError::PoolError(err) => err.into(), EthApiError::PrevrandaoNotSet | EthApiError::InvalidBlockData(_) | EthApiError::Internal(_) | @@ -111,7 +111,7 @@ where { fn from(err: EVMError) -> Self { match err { - EVMError::Transaction(err) => InvalidTransactionError::from(err).into(), + EVMError::Transaction(err) => RpcInvalidTransactionError::from(err).into(), EVMError::PrevrandaoNotSet => EthApiError::PrevrandaoNotSet, EVMError::Database(err) => err.into(), } @@ -120,23 +120,26 @@ where /// An error due to invalid transaction. /// -/// This adds compatibility with geth. +/// The only reason this exists is to maintain compatibility with other clients de-facto standard +/// error messages. /// /// These error variants can be thrown when the transaction is checked prior to execution. /// +/// These variants also cover all errors that can be thrown by revm. +/// /// ## Nomenclature /// /// This type is explicitly modeled after geth's error variants and uses /// `fee cap` for `max_fee_per_gas` /// `tip` for `max_priority_fee_per_gas` #[derive(thiserror::Error, Debug)] -pub enum InvalidTransactionError { +pub enum RpcInvalidTransactionError { /// returned if the nonce of a transaction is lower than the one present in the local chain. #[error("nonce too low")] NonceTooLow, /// returned if the nonce of a transaction is higher than the next one expected based on the /// local chain. - #[error("Nonce too high")] + #[error("nonce too high")] NonceTooHigh, /// Returned if the nonce of a transaction is too high /// Incrementing the nonce would lead to invalid state (overflow) @@ -149,7 +152,7 @@ pub enum InvalidTransactionError { #[error("max initcode size exceeded")] MaxInitCodeSizeExceeded, /// Represents the inability to cover max cost + value (account balance too low). - #[error("Insufficient funds for gas * price + value")] + #[error("insufficient funds for gas * price + value")] InsufficientFunds, /// Thrown when calculating gas usage #[error("gas uint64 overflow")] @@ -201,16 +204,19 @@ pub enum InvalidTransactionError { /// Invalid chain id set for the transaction. #[error("Invalid chain id")] InvalidChainId, + /// The transaction is before Spurious Dragon and has a chain ID + #[error("Transactions before Spurious Dragon should not have a chain ID.")] + OldLegacyChainId, } -impl InvalidTransactionError { +impl RpcInvalidTransactionError { /// Returns the rpc error code for this error. fn error_code(&self) -> i32 { match self { - InvalidTransactionError::InvalidChainId | - InvalidTransactionError::GasTooLow | - InvalidTransactionError::GasTooHigh => EthRpcErrorCode::InvalidInput.code(), - InvalidTransactionError::Revert(_) => EthRpcErrorCode::ExecutionError.code(), + RpcInvalidTransactionError::InvalidChainId | + RpcInvalidTransactionError::GasTooLow | + RpcInvalidTransactionError::GasTooHigh => EthRpcErrorCode::InvalidInput.code(), + RpcInvalidTransactionError::Revert(_) => EthRpcErrorCode::ExecutionError.code(), _ => EthRpcErrorCode::TransactionRejected.code(), } } @@ -220,9 +226,9 @@ impl InvalidTransactionError { /// Takes the configured gas limit of the transaction which is attached to the error pub(crate) fn halt(reason: Halt, gas_limit: u64) -> Self { match reason { - Halt::OutOfGas(err) => InvalidTransactionError::out_of_gas(err, gas_limit), - Halt::NonceOverflow => InvalidTransactionError::NonceMaxValue, - err => InvalidTransactionError::EvmHalt(err), + Halt::OutOfGas(err) => RpcInvalidTransactionError::out_of_gas(err, gas_limit), + Halt::NonceOverflow => RpcInvalidTransactionError::NonceMaxValue, + err => RpcInvalidTransactionError::EvmHalt(err), } } @@ -230,21 +236,21 @@ impl InvalidTransactionError { pub(crate) fn out_of_gas(reason: OutOfGasError, gas_limit: u64) -> Self { let gas_limit = U256::from(gas_limit); match reason { - OutOfGasError::BasicOutOfGas => InvalidTransactionError::BasicOutOfGas(gas_limit), - OutOfGasError::Memory => InvalidTransactionError::MemoryOutOfGas(gas_limit), - OutOfGasError::Precompile => InvalidTransactionError::PrecompileOutOfGas(gas_limit), + OutOfGasError::BasicOutOfGas => RpcInvalidTransactionError::BasicOutOfGas(gas_limit), + OutOfGasError::Memory => RpcInvalidTransactionError::MemoryOutOfGas(gas_limit), + OutOfGasError::Precompile => RpcInvalidTransactionError::PrecompileOutOfGas(gas_limit), OutOfGasError::InvalidOperand => { - InvalidTransactionError::InvalidOperandOutOfGas(gas_limit) + RpcInvalidTransactionError::InvalidOperandOutOfGas(gas_limit) } - OutOfGasError::MemoryLimit => InvalidTransactionError::MemoryOutOfGas(gas_limit), + OutOfGasError::MemoryLimit => RpcInvalidTransactionError::MemoryOutOfGas(gas_limit), } } } -impl From for ErrorObject<'static> { - fn from(err: InvalidTransactionError) -> Self { +impl From for ErrorObject<'static> { + fn from(err: RpcInvalidTransactionError) -> Self { match err { - InvalidTransactionError::Revert(revert) => { + RpcInvalidTransactionError::Revert(revert) => { // include out data if some rpc_err( revert.error_code(), @@ -257,32 +263,72 @@ impl From for ErrorObject<'static> { } } -impl From for InvalidTransactionError { +impl From for RpcInvalidTransactionError { fn from(err: revm::primitives::InvalidTransaction) -> Self { use revm::primitives::InvalidTransaction; match err { - InvalidTransaction::InvalidChainId => InvalidTransactionError::InvalidChainId, + InvalidTransaction::InvalidChainId => RpcInvalidTransactionError::InvalidChainId, InvalidTransaction::GasMaxFeeGreaterThanPriorityFee => { - InvalidTransactionError::TipAboveFeeCap + RpcInvalidTransactionError::TipAboveFeeCap } - InvalidTransaction::GasPriceLessThanBasefee => InvalidTransactionError::FeeCapTooLow, - InvalidTransaction::CallerGasLimitMoreThanBlock => InvalidTransactionError::GasTooHigh, - InvalidTransaction::CallGasCostMoreThanGasLimit => InvalidTransactionError::GasTooHigh, - InvalidTransaction::RejectCallerWithCode => InvalidTransactionError::SenderNoEOA, + InvalidTransaction::GasPriceLessThanBasefee => RpcInvalidTransactionError::FeeCapTooLow, + InvalidTransaction::CallerGasLimitMoreThanBlock => { + RpcInvalidTransactionError::GasTooHigh + } + InvalidTransaction::CallGasCostMoreThanGasLimit => { + RpcInvalidTransactionError::GasTooHigh + } + InvalidTransaction::RejectCallerWithCode => RpcInvalidTransactionError::SenderNoEOA, InvalidTransaction::LackOfFundForGasLimit { .. } => { - InvalidTransactionError::InsufficientFunds + RpcInvalidTransactionError::InsufficientFunds } InvalidTransaction::OverflowPaymentInTransaction => { - InvalidTransactionError::GasUintOverflow + RpcInvalidTransactionError::GasUintOverflow } InvalidTransaction::NonceOverflowInTransaction => { - InvalidTransactionError::NonceMaxValue + RpcInvalidTransactionError::NonceMaxValue } InvalidTransaction::CreateInitcodeSizeLimit => { - InvalidTransactionError::MaxInitCodeSizeExceeded + RpcInvalidTransactionError::MaxInitCodeSizeExceeded + } + InvalidTransaction::NonceTooHigh { .. } => RpcInvalidTransactionError::NonceTooHigh, + InvalidTransaction::NonceTooLow { .. } => RpcInvalidTransactionError::NonceTooLow, + } + } +} + +impl From for RpcInvalidTransactionError { + fn from(err: reth_primitives::InvalidTransactionError) -> Self { + use reth_primitives::InvalidTransactionError; + // This conversion is used to convert any transaction errors that could occur inside the + // txpool (e.g. `eth_sendRawTransaction`) to their corresponding RPC + match err { + InvalidTransactionError::InsufficientFunds { .. } => { + RpcInvalidTransactionError::InsufficientFunds + } + InvalidTransactionError::NonceNotConsistent => RpcInvalidTransactionError::NonceTooLow, + InvalidTransactionError::OldLegacyChainId => { + // Note: this should be unreachable since Spurious Dragon now enabled + RpcInvalidTransactionError::OldLegacyChainId + } + InvalidTransactionError::ChainIdMismatch => RpcInvalidTransactionError::InvalidChainId, + InvalidTransactionError::Eip2930Disabled => { + RpcInvalidTransactionError::TxTypeNotSupported + } + InvalidTransactionError::Eip1559Disabled => { + RpcInvalidTransactionError::TxTypeNotSupported + } + InvalidTransactionError::TxTypeNotSupported => { + RpcInvalidTransactionError::TxTypeNotSupported + } + InvalidTransactionError::GasUintOverflow => RpcInvalidTransactionError::GasUintOverflow, + InvalidTransactionError::GasTooLow => RpcInvalidTransactionError::GasTooLow, + InvalidTransactionError::GasTooHigh => RpcInvalidTransactionError::GasTooHigh, + InvalidTransactionError::TipAboveFeeCap => RpcInvalidTransactionError::TipAboveFeeCap, + InvalidTransactionError::FeeCapTooLow => RpcInvalidTransactionError::FeeCapTooLow, + InvalidTransactionError::SignerAccountHasBytecode => { + RpcInvalidTransactionError::SenderNoEOA } - InvalidTransaction::NonceTooHigh { .. } => InvalidTransactionError::NonceTooHigh, - InvalidTransaction::NonceTooLow { .. } => InvalidTransactionError::NonceTooLow, } } } @@ -344,17 +390,28 @@ pub enum RpcPoolError { #[error("replacement transaction underpriced")] ReplaceUnderpriced, #[error("exceeds block gas limit")] - GasLimit, + ExceedsGasLimit, #[error("negative value")] NegativeValue, #[error("oversized data")] OversizedData, + #[error("max initcode size exceeded")] + ExceedsMaxInitCodeSize, #[error(transparent)] - Invalid(#[from] InvalidPoolTransactionError), + Invalid(#[from] RpcInvalidTransactionError), #[error(transparent)] Other(Box), } +impl From for ErrorObject<'static> { + fn from(error: RpcPoolError) -> Self { + match error { + RpcPoolError::Invalid(err) => err.into(), + error => internal_rpc_err(error.to_string()), + } + } +} + impl From for RpcPoolError { fn from(err: PoolError) -> RpcPoolError { match err { @@ -369,6 +426,20 @@ impl From for RpcPoolError { } } +impl From for RpcPoolError { + fn from(err: InvalidPoolTransactionError) -> RpcPoolError { + match err { + InvalidPoolTransactionError::Consensus(err) => RpcPoolError::Invalid(err.into()), + InvalidPoolTransactionError::ExceedsGasLimit(_, _) => RpcPoolError::ExceedsGasLimit, + InvalidPoolTransactionError::ExceedsMaxInitCodeSize(_, _) => { + RpcPoolError::ExceedsMaxInitCodeSize + } + InvalidPoolTransactionError::OversizedData(_, _) => RpcPoolError::OversizedData, + InvalidPoolTransactionError::Underpriced => RpcPoolError::Underpriced, + } + } +} + impl From for EthApiError { fn from(err: PoolError) -> Self { EthApiError::PoolError(RpcPoolError::from(err)) @@ -398,10 +469,10 @@ pub(crate) fn ensure_success(result: ExecutionResult) -> EthResult { match result { ExecutionResult::Success { output, .. } => Ok(output.into_data().into()), ExecutionResult::Revert { output, .. } => { - Err(InvalidTransactionError::Revert(RevertError::new(output)).into()) + Err(RpcInvalidTransactionError::Revert(RevertError::new(output)).into()) } ExecutionResult::Halt { reason, gas_used } => { - Err(InvalidTransactionError::halt(reason, gas_used).into()) + Err(RpcInvalidTransactionError::halt(reason, gas_used).into()) } } } diff --git a/crates/rpc/rpc/src/eth/gas_oracle.rs b/crates/rpc/rpc/src/eth/gas_oracle.rs index 9d18493e14..d46ce8f80d 100644 --- a/crates/rpc/rpc/src/eth/gas_oracle.rs +++ b/crates/rpc/rpc/src/eth/gas_oracle.rs @@ -2,7 +2,7 @@ //! previous blocks. use crate::eth::{ cache::EthStateCache, - error::{EthApiError, EthResult, InvalidTransactionError}, + error::{EthApiError, EthResult, RpcInvalidTransactionError}, }; use reth_primitives::{constants::GWEI_TO_WEI, BlockNumberOrTag, H256, U256}; use reth_provider::BlockProviderIdExt; @@ -236,7 +236,7 @@ where for tx in txs.iter().take(limit) { // a `None` effective_gas_tip represents a transaction where the max_fee_per_gas is // less than the base fee - let effective_tip = tx.ok_or(InvalidTransactionError::FeeCapTooLow)?; + let effective_tip = tx.ok_or(RpcInvalidTransactionError::FeeCapTooLow)?; final_result.push(U256::from(effective_tip)); } diff --git a/crates/rpc/rpc/src/eth/revm_utils.rs b/crates/rpc/rpc/src/eth/revm_utils.rs index fe79aba323..3d47939ef9 100644 --- a/crates/rpc/rpc/src/eth/revm_utils.rs +++ b/crates/rpc/rpc/src/eth/revm_utils.rs @@ -1,6 +1,6 @@ //! utilities for working with revm -use crate::eth::error::{EthApiError, EthResult, InvalidTransactionError}; +use crate::eth::error::{EthApiError, EthResult, RpcInvalidTransactionError}; use reth_primitives::{ AccessList, Address, TransactionSigned, TransactionSignedEcRecovered, TxHash, H256, U256, }; @@ -224,9 +224,9 @@ pub(crate) fn create_txn_env(block_env: &BlockEnv, request: CallRequest) -> EthR let gas_limit = gas.unwrap_or(block_env.gas_limit.min(U256::from(u64::MAX))); let env = TxEnv { - gas_limit: gas_limit.try_into().map_err(|_| InvalidTransactionError::GasUintOverflow)?, + gas_limit: gas_limit.try_into().map_err(|_| RpcInvalidTransactionError::GasUintOverflow)?, nonce: nonce - .map(|n| n.try_into().map_err(|_| InvalidTransactionError::NonceTooHigh)) + .map(|n| n.try_into().map_err(|_| RpcInvalidTransactionError::NonceTooHigh)) .transpose()?, caller: from.unwrap_or_default(), gas_price, @@ -257,7 +257,7 @@ where // subtract transferred value allowance = allowance .checked_sub(env.value) - .ok_or_else(|| InvalidTransactionError::InsufficientFunds)?; + .ok_or_else(|| RpcInvalidTransactionError::InsufficientFunds)?; // cap the gas limit if let Ok(gas_limit) = allowance.checked_div(env.gas_price).unwrap_or_default().try_into() { @@ -313,7 +313,7 @@ impl CallFees { // Fail early return Err( // `max_priority_fee_per_gas` is greater than the `max_fee_per_gas` - InvalidTransactionError::TipAboveFeeCap.into(), + RpcInvalidTransactionError::TipAboveFeeCap.into(), ) } }