diff --git a/bus-mapping/src/circuit_input_builder/input_state_ref.rs b/bus-mapping/src/circuit_input_builder/input_state_ref.rs index 251e536c09..ab8479a68a 100644 --- a/bus-mapping/src/circuit_input_builder/input_state_ref.rs +++ b/bus-mapping/src/circuit_input_builder/input_state_ref.rs @@ -1510,7 +1510,7 @@ impl<'a> CircuitInputStateRef<'a> { return Ok(Some(ExecError::InvalidOpcode)); } - if let Some(error) = &step.error { + if let Some(error) = step.error { return Ok(Some(get_step_reported_error(&step.op, error))); } diff --git a/bus-mapping/src/circuit_input_builder/tracer_tests.rs b/bus-mapping/src/circuit_input_builder/tracer_tests.rs index 3d6a73bc44..fa2f8698f8 100644 --- a/bus-mapping/src/circuit_input_builder/tracer_tests.rs +++ b/bus-mapping/src/circuit_input_builder/tracer_tests.rs @@ -4,10 +4,6 @@ use crate::{ error::{ ContractAddressCollisionError, DepthError, ExecError, InsufficientBalanceError, OogError, }, - geth_errors::{ - GETH_ERR_GAS_UINT_OVERFLOW, GETH_ERR_OUT_OF_GAS, GETH_ERR_STACK_OVERFLOW, - GETH_ERR_STACK_UNDERFLOW, - }, operation::RWCounter, state_db::Account, }; @@ -15,7 +11,7 @@ use eth_types::{ address, bytecode, evm_types::{stack::Stack, Gas, Memory, OpcodeId}, geth_types::GethData, - word, Bytecode, Hash, ToAddress, ToWord, Word, + word, Bytecode, GethExecError, Hash, ToAddress, ToWord, Word, }; use lazy_static::lazy_static; use mock::{ @@ -1503,7 +1499,7 @@ fn tracer_err_gas_uint_overflow() { let step = &block.geth_traces[0].struct_logs[index]; let next_step = block.geth_traces[0].struct_logs.get(index + 1); assert_eq!(step.op, OpcodeId::MSTORE); - assert_eq!(step.error, Some(GETH_ERR_GAS_UINT_OVERFLOW.to_string())); + assert_eq!(step.error, Some(GethExecError::GasUintOverflow)); let mut builder = CircuitInputBuilderTx::new(&block, step); assert_eq!( @@ -1674,7 +1670,7 @@ fn tracer_err_out_of_gas() { .into(); let struct_logs = &block.geth_traces[0].struct_logs; - assert_eq!(struct_logs[1].error, Some(GETH_ERR_OUT_OF_GAS.to_string())); + assert_eq!(struct_logs[1].error, Some(GethExecError::OutOfGas)); } #[test] @@ -1697,10 +1693,13 @@ fn tracer_err_stack_overflow() { let index = block.geth_traces[0].struct_logs.len() - 1; // PUSH2 let step = &block.geth_traces[0].struct_logs[index]; let next_step = block.geth_traces[0].struct_logs.get(index + 1); - assert_eq!( + assert!(matches!( step.error, - Some(format!("{GETH_ERR_STACK_OVERFLOW} 1024 (1023)")) - ); + Some(GethExecError::StackOverflow { + stack_len: 1024, + limit: 1023, + }) + )); let mut builder = CircuitInputBuilderTx::new(&block, step); assert_eq!( @@ -1728,10 +1727,13 @@ fn tracer_err_stack_underflow() { let index = 0; // SWAP5 let step = &block.geth_traces[0].struct_logs[index]; let next_step = block.geth_traces[0].struct_logs.get(index + 1); - assert_eq!( + assert!(matches!( step.error, - Some(format!("{GETH_ERR_STACK_UNDERFLOW} (0 <=> 6)",)) - ); + Some(GethExecError::StackUnderflow { + stack_len: 0, + required: 6, + }) + )); let mut builder = CircuitInputBuilderTx::new(&block, step); assert_eq!( diff --git a/bus-mapping/src/error.rs b/bus-mapping/src/error.rs index bf41228243..e62c18621b 100644 --- a/bus-mapping/src/error.rs +++ b/bus-mapping/src/error.rs @@ -1,15 +1,10 @@ //! Error module for the bus-mapping crate use core::fmt::{Display, Formatter, Result as FmtResult}; -use eth_types::{evm_types::OpcodeId, Address, GethExecStep, Word, H256}; +use eth_types::{evm_types::OpcodeId, Address, GethExecError, GethExecStep, Word, H256}; use ethers_providers::ProviderError; use std::error::Error as StdError; -use crate::geth_errors::{ - GETH_ERR_GAS_UINT_OVERFLOW, GETH_ERR_OUT_OF_GAS, GETH_ERR_STACK_OVERFLOW, - GETH_ERR_STACK_UNDERFLOW, GETH_ERR_WRITE_PROTECTION, -}; - /// Error type for any BusMapping related failure. #[derive(Debug)] pub enum Error { @@ -185,42 +180,43 @@ pub enum ExecError { } // TODO: Move to impl block. -pub(crate) fn get_step_reported_error(op: &OpcodeId, error: &str) -> ExecError { - if [GETH_ERR_OUT_OF_GAS, GETH_ERR_GAS_UINT_OVERFLOW].contains(&error) { - // NOTE: We report a GasUintOverflow error as an OutOfGas error - let oog_err = match op { - OpcodeId::MLOAD | OpcodeId::MSTORE | OpcodeId::MSTORE8 => { - OogError::StaticMemoryExpansion - } - OpcodeId::RETURN | OpcodeId::REVERT => OogError::DynamicMemoryExpansion, - OpcodeId::CALLDATACOPY - | OpcodeId::CODECOPY - | OpcodeId::EXTCODECOPY - | OpcodeId::RETURNDATACOPY => OogError::MemoryCopy, - OpcodeId::BALANCE | OpcodeId::EXTCODESIZE | OpcodeId::EXTCODEHASH => { - OogError::AccountAccess - } - OpcodeId::LOG0 | OpcodeId::LOG1 | OpcodeId::LOG2 | OpcodeId::LOG3 | OpcodeId::LOG4 => { - OogError::Log - } - OpcodeId::EXP => OogError::Exp, - OpcodeId::SHA3 => OogError::Sha3, - OpcodeId::CALL | OpcodeId::CALLCODE | OpcodeId::DELEGATECALL | OpcodeId::STATICCALL => { - OogError::Call - } - OpcodeId::SLOAD | OpcodeId::SSTORE => OogError::SloadSstore, - OpcodeId::CREATE | OpcodeId::CREATE2 => OogError::Create, - OpcodeId::SELFDESTRUCT => OogError::SelfDestruct, - _ => OogError::Constant, - }; - ExecError::OutOfGas(oog_err) - } else if error.starts_with(GETH_ERR_STACK_OVERFLOW) { - ExecError::StackOverflow - } else if error.starts_with(GETH_ERR_STACK_UNDERFLOW) { - ExecError::StackUnderflow - } else if error.starts_with(GETH_ERR_WRITE_PROTECTION) { - ExecError::WriteProtection - } else { - panic!("Unknown GethExecStep.error: {error}"); +pub(crate) fn get_step_reported_error(op: &OpcodeId, error: GethExecError) -> ExecError { + match error { + GethExecError::OutOfGas | GethExecError::GasUintOverflow => { + // NOTE: We report a GasUintOverflow error as an OutOfGas error + let oog_err = match op { + OpcodeId::MLOAD | OpcodeId::MSTORE | OpcodeId::MSTORE8 => { + OogError::StaticMemoryExpansion + } + OpcodeId::RETURN | OpcodeId::REVERT => OogError::DynamicMemoryExpansion, + OpcodeId::CALLDATACOPY + | OpcodeId::CODECOPY + | OpcodeId::EXTCODECOPY + | OpcodeId::RETURNDATACOPY => OogError::MemoryCopy, + OpcodeId::BALANCE | OpcodeId::EXTCODESIZE | OpcodeId::EXTCODEHASH => { + OogError::AccountAccess + } + OpcodeId::LOG0 + | OpcodeId::LOG1 + | OpcodeId::LOG2 + | OpcodeId::LOG3 + | OpcodeId::LOG4 => OogError::Log, + OpcodeId::EXP => OogError::Exp, + OpcodeId::SHA3 => OogError::Sha3, + OpcodeId::CALL + | OpcodeId::CALLCODE + | OpcodeId::DELEGATECALL + | OpcodeId::STATICCALL => OogError::Call, + OpcodeId::SLOAD | OpcodeId::SSTORE => OogError::SloadSstore, + OpcodeId::CREATE | OpcodeId::CREATE2 => OogError::Create, + OpcodeId::SELFDESTRUCT => OogError::SelfDestruct, + _ => OogError::Constant, + }; + ExecError::OutOfGas(oog_err) + } + GethExecError::StackOverflow { .. } => ExecError::StackOverflow, + GethExecError::StackUnderflow { .. } => ExecError::StackUnderflow, + GethExecError::WriteProtection => ExecError::WriteProtection, + _ => panic!("Unknown GethExecStep.error: {error}"), } } diff --git a/bus-mapping/src/geth_errors.rs b/bus-mapping/src/geth_errors.rs deleted file mode 100644 index b10b95bd22..0000000000 --- a/bus-mapping/src/geth_errors.rs +++ /dev/null @@ -1,10 +0,0 @@ -/// Geth error message for stack overflow -pub const GETH_ERR_STACK_OVERFLOW: &str = "stack limit reached"; -/// Geth error message for stack underflow -pub const GETH_ERR_STACK_UNDERFLOW: &str = "stack underflow"; -/// Geth error message for out of gas -pub const GETH_ERR_OUT_OF_GAS: &str = "out of gas"; -/// Geth error message for gas uint64 overflow -pub const GETH_ERR_GAS_UINT_OVERFLOW: &str = "gas uint64 overflow"; -/// Geth error message for write protection -pub const GETH_ERR_WRITE_PROTECTION: &str = "write protection"; diff --git a/bus-mapping/src/lib.rs b/bus-mapping/src/lib.rs index 2a8cd256cd..982504f7cb 100644 --- a/bus-mapping/src/lib.rs +++ b/bus-mapping/src/lib.rs @@ -234,7 +234,6 @@ pub mod circuit_input_builder; pub mod error; pub mod evm; pub mod exec_trace; -pub(crate) mod geth_errors; pub mod l2_predeployed; pub mod mock; pub mod operation; diff --git a/eth-types/src/l2_types.rs b/eth-types/src/l2_types.rs index c3ab407354..66dc88dd55 100644 --- a/eth-types/src/l2_types.rs +++ b/eth-types/src/l2_types.rs @@ -2,7 +2,7 @@ use crate::{ evm_types::{Gas, GasCost, Memory, OpcodeId, ProgramCounter, Stack, Storage}, - Block, GethExecStep, GethExecTrace, Hash, Transaction, Word, H256, + Block, GethExecError, GethExecStep, GethExecTrace, Hash, Transaction, Word, H256, }; use ethers_core::types::{Address, Bytes, U256, U64}; use serde::{Deserialize, Serialize}; @@ -225,7 +225,7 @@ pub struct ExecStep { #[serde(default)] pub refund: u64, pub depth: isize, - pub error: Option, + pub error: Option, pub stack: Option>, pub memory: Option>, pub storage: Option>, diff --git a/eth-types/src/lib.rs b/eth-types/src/lib.rs index a318d613b5..174965904f 100644 --- a/eth-types/src/lib.rs +++ b/eth-types/src/lib.rs @@ -44,7 +44,12 @@ pub use ethers_core::{ use once_cell::sync::Lazy; use serde::{de, Deserialize, Deserializer, Serialize}; -use std::{collections::HashMap, fmt, str::FromStr}; +use std::{ + collections::HashMap, + fmt, + fmt::{Display, Formatter}, + str::FromStr, +}; /// Trait used to reduce verbosity with the declaration of the [`FieldExt`] /// trait and its repr. @@ -350,7 +355,7 @@ struct GethExecStepInternal { #[serde(rename = "gasCost")] gas_cost: GasCost, depth: u16, - error: Option, + error: Option, // stack is in hex 0x prefixed #[serde(default)] stack: Vec, @@ -373,7 +378,7 @@ pub struct GethExecStep { pub gas_cost: GasCost, pub refund: Gas, pub depth: u16, - pub error: Option, + pub error: Option, // stack is in hex 0x prefixed pub stack: Stack, // memory is in chunks of 32 bytes, in hex @@ -382,6 +387,178 @@ pub struct GethExecStep { pub storage: Storage, } +/// Errors of StructLogger Result from Geth +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +pub enum GethExecError { + /// out of gas + OutOfGas, + /// contract creation code storage out of gas + CodeStoreOutOfGas, + /// max call depth exceeded + Depth, + /// insufficient balance for transfer + InsufficientBalance, + /// contract address collision + ContractAddressCollision, + /// execution reverted + ExecutionReverted, + /// max initcode size exceeded + MaxInitCodeSizeExceeded, + /// max code size exceeded + MaxCodeSizeExceeded, + /// invalid jump destination + InvalidJump, + /// write protection + WriteProtection, + /// return data out of bounds + ReturnDataOutOfBounds, + /// gas uint64 overflow + GasUintOverflow, + /// invalid code: must not begin with 0xef + InvalidCode, + /// nonce uint64 overflow + NonceUintOverflow, + /// stack underflow + StackUnderflow { + /// stack length + stack_len: u64, + /// required length + required: u64, + }, + /// stack limit reached + StackOverflow { + /// stack length + stack_len: u64, + /// stack limit + limit: u64, + }, + /// invalid opcode + InvalidOpcode(OpcodeId), +} + +impl GethExecError { + /// Returns the error as a string constant. + pub fn error(self) -> &'static str { + match self { + GethExecError::OutOfGas => "out of gas", + GethExecError::CodeStoreOutOfGas => "contract creation code storage out of gas", + GethExecError::Depth => "max call depth exceeded", + GethExecError::InsufficientBalance => "insufficient balance for transfer", + GethExecError::ContractAddressCollision => "contract address collision", + GethExecError::ExecutionReverted => "execution reverted", + GethExecError::MaxInitCodeSizeExceeded => "max initcode size exceeded", + GethExecError::MaxCodeSizeExceeded => "max code size exceeded", + GethExecError::InvalidJump => "invalid jump destination", + GethExecError::WriteProtection => "write protection", + GethExecError::ReturnDataOutOfBounds => "return data out of bounds", + GethExecError::GasUintOverflow => "gas uint64 overflow", + GethExecError::InvalidCode => "invalid code: must not begin with 0xef", + GethExecError::NonceUintOverflow => "nonce uint64 overflow", + GethExecError::StackUnderflow { .. } => "stack underflow", + GethExecError::StackOverflow { .. } => "stack limit reached", + GethExecError::InvalidOpcode(_) => "invalid opcode", + } + } +} + +impl Display for GethExecError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + GethExecError::StackUnderflow { + stack_len, + required, + } => { + write!(f, "stack underflow ({stack_len} <=> {required})") + } + GethExecError::StackOverflow { stack_len, limit } => { + write!(f, "stack limit reached {stack_len} ({limit})") + } + GethExecError::InvalidOpcode(op) => { + write!(f, "invalid opcode: {op}") + } + _ => f.write_str(self.error()), + } + } +} + +impl Serialize for GethExecError { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + // We serialize the error as a string constant. + serializer.serialize_str(self.error()) + } +} + +impl<'de> Deserialize<'de> for GethExecError { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct GethExecErrorVisitor; + + static STACK_UNDERFLOW_RE: Lazy = + Lazy::new(|| regex::Regex::new(r"^stack underflow \((\d+) <=> (\d+)\)$").unwrap()); + static STACK_OVERFLOW_RE: Lazy = + Lazy::new(|| regex::Regex::new(r"^stack limit reached (\d+) \((\d+)\)$").unwrap()); + + impl<'de> de::Visitor<'de> for GethExecErrorVisitor { + type Value = GethExecError; + + fn expecting(&self, formatter: &mut Formatter) -> fmt::Result { + write!(formatter, "a geth struct logger error string constant") + } + + fn visit_str(self, v: &str) -> Result + where + E: de::Error, + { + let e = match v { + "out of gas" => GethExecError::OutOfGas, + "contract creation code storage out of gas" => GethExecError::CodeStoreOutOfGas, + "max call depth exceeded" => GethExecError::Depth, + "insufficient balance for transfer" => GethExecError::InsufficientBalance, + "contract address collision" => GethExecError::ContractAddressCollision, + "execution reverted" => GethExecError::ExecutionReverted, + "max initcode size exceeded" => GethExecError::MaxInitCodeSizeExceeded, + "max code size exceeded" => GethExecError::MaxCodeSizeExceeded, + "invalid jump destination" => GethExecError::InvalidJump, + "write protection" => GethExecError::WriteProtection, + "return data out of bounds" => GethExecError::ReturnDataOutOfBounds, + "gas uint64 overflow" => GethExecError::GasUintOverflow, + "invalid code: must not begin with 0xef" => GethExecError::InvalidCode, + "nonce uint64 overflow" => GethExecError::NonceUintOverflow, + _ if v.starts_with("stack underflow") => { + let caps = STACK_UNDERFLOW_RE.captures(v).unwrap(); + let stack_len = caps.get(1).unwrap().as_str().parse::().unwrap(); + let required = caps.get(2).unwrap().as_str().parse::().unwrap(); + GethExecError::StackUnderflow { + stack_len, + required, + } + } + _ if v.starts_with("stack limit reached") => { + let caps = STACK_OVERFLOW_RE.captures(v).unwrap(); + let stack_len = caps.get(1).unwrap().as_str().parse::().unwrap(); + let limit = caps.get(2).unwrap().as_str().parse::().unwrap(); + GethExecError::StackOverflow { stack_len, limit } + } + _ if v.starts_with("invalid opcode") => v + .strip_prefix("invalid opcode: ") + .map(|s| OpcodeId::from_str(s).unwrap()) + .map(GethExecError::InvalidOpcode) + .unwrap(), + _ => return Err(E::invalid_value(de::Unexpected::Str(v), &self)), + }; + Ok(e) + } + } + + deserializer.deserialize_str(GethExecErrorVisitor) + } +} + // Wrapper over u8 that provides formats the byte in hex for [`fmt::Debug`]. pub(crate) struct DebugByte(pub(crate) u8); diff --git a/testool/src/utils.rs b/testool/src/utils.rs index 4358dff10f..3101f7f703 100644 --- a/testool/src/utils.rs +++ b/testool/src/utils.rs @@ -141,7 +141,7 @@ pub fn print_trace(trace: GethExecTrace) -> Result<()> { format!("{}", step.gas.0), format!("{}", step.gas_cost.0), format!("{}", step.depth), - step.error.unwrap_or_default(), + step.error.map(|e| e.error()).unwrap_or(""), split(step.stack.0.iter().map(u256_to_str).collect(), 30), split(step.memory.0.iter().map(ToString::to_string).collect(), 30), split(kv(step.storage.0), 30)