diff --git a/src/rpc/methods/eth/trace/parity.rs b/src/rpc/methods/eth/trace/parity.rs index 865a16da15b..3889cc65891 100644 --- a/src/rpc/methods/eth/trace/parity.rs +++ b/src/rpc/methods/eth/trace/parity.rs @@ -13,7 +13,7 @@ use super::super::{decode_payload, encode_filecoin_params_as_abi, encode_filecoi use super::Environment; use super::types::{ EthCallTraceAction, EthCallTraceResult, EthCreateTraceAction, EthCreateTraceResult, EthTrace, - TraceAction, TraceResult, + TraceAction, TraceError, TraceResult, }; use super::utils::trace_to_address; use crate::eth::{EAMMethod, EVMMethod}; @@ -29,17 +29,6 @@ use fvm_ipld_blockstore::Blockstore; use num::FromPrimitive; use tracing::debug; -/// Error string used in Parity-format traces. -pub const PARITY_TRACE_REVERT_ERROR: &str = "Reverted"; -pub const PARITY_EVM_INVALID_INSTRUCTION: &str = "invalid instruction"; -pub const PARITY_EVM_UNDEFINED_INSTRUCTION: &str = "undefined instruction"; -pub const PARITY_EVM_STACK_UNDERFLOW: &str = "stack underflow"; -pub const PARITY_EVM_STACK_OVERFLOW: &str = "stack overflow"; -pub const PARITY_EVM_ILLEGAL_MEMORY_ACCESS: &str = "illegal memory access"; -pub const PARITY_EVM_BAD_JUMPDEST: &str = "invalid jump destination"; -pub const PARITY_EVM_SELFDESTRUCT_FAILED: &str = "self destruct failed"; -pub const PARITY_EVM_OUT_OF_GAS: &str = "out of gas"; - /// Returns `true` if the invoked actor is an EVM contract or the Ethereum Account Manager. fn trace_is_evm_or_eam(trace: &ExecutionTrace) -> bool { if let Some(invoked_actor) = &trace.invoked_actor { @@ -50,49 +39,45 @@ fn trace_is_evm_or_eam(trace: &ExecutionTrace) -> bool { } } -/// Converts a trace's exit code into a human-readable Parity-style error string. +/// Converts a trace's exit code into a typed [`TraceError`]. /// Returns `None` when the trace completed successfully. -fn trace_err_msg(trace: &ExecutionTrace) -> Option { +fn trace_err_msg(trace: &ExecutionTrace) -> Option { let code = trace.msg_rct.exit_code; if code.is_success() { return None; } - // EVM tools often expect this literal string. if code == ExitCode::SYS_OUT_OF_GAS { - return Some(PARITY_EVM_OUT_OF_GAS.into()); + return Some(TraceError::OutOfGas); } - // indicate when we have a "system" error. if code < ExitCode::FIRST_ACTOR_ERROR_CODE.into() { - return Some(format!("vm error: {code}")); + return Some(TraceError::VmError(code.value())); } - // handle special exit codes from the EVM/EAM. if trace_is_evm_or_eam(trace) { match code.into() { - evm12::EVM_CONTRACT_REVERTED => return Some(PARITY_TRACE_REVERT_ERROR.into()), // capitalized for compatibility + evm12::EVM_CONTRACT_REVERTED => return Some(TraceError::Reverted), evm12::EVM_CONTRACT_INVALID_INSTRUCTION => { - return Some(PARITY_EVM_INVALID_INSTRUCTION.into()); + return Some(TraceError::InvalidInstruction); } evm12::EVM_CONTRACT_UNDEFINED_INSTRUCTION => { - return Some(PARITY_EVM_UNDEFINED_INSTRUCTION.into()); + return Some(TraceError::UndefinedInstruction); } - evm12::EVM_CONTRACT_STACK_UNDERFLOW => return Some(PARITY_EVM_STACK_UNDERFLOW.into()), - evm12::EVM_CONTRACT_STACK_OVERFLOW => return Some(PARITY_EVM_STACK_OVERFLOW.into()), + evm12::EVM_CONTRACT_STACK_UNDERFLOW => return Some(TraceError::StackUnderflow), + evm12::EVM_CONTRACT_STACK_OVERFLOW => return Some(TraceError::StackOverflow), evm12::EVM_CONTRACT_ILLEGAL_MEMORY_ACCESS => { - return Some(PARITY_EVM_ILLEGAL_MEMORY_ACCESS.into()); + return Some(TraceError::IllegalMemoryAccess); } - evm12::EVM_CONTRACT_BAD_JUMPDEST => return Some(PARITY_EVM_BAD_JUMPDEST.into()), + evm12::EVM_CONTRACT_BAD_JUMPDEST => return Some(TraceError::BadJumpDest), evm12::EVM_CONTRACT_SELFDESTRUCT_FAILED => { - return Some(PARITY_EVM_SELFDESTRUCT_FAILED.into()); + return Some(TraceError::SelfDestructFailed); } _ => (), } } - // everything else... - Some(format!("actor error: {code}")) + Some(TraceError::ActorError(code.value())) } /// Recursively builds the traces for a given ExecutionTrace by walking the subcalls diff --git a/src/rpc/methods/eth/trace/types.rs b/src/rpc/methods/eth/trace/types.rs index 05a499d1109..a0b90842b71 100644 --- a/src/rpc/methods/eth/trace/types.rs +++ b/src/rpc/methods/eth/trace/types.rs @@ -9,13 +9,99 @@ use super::super::types::{EthAddress, EthAddressList, EthBytes, EthHash}; use super::super::{EthBigInt, EthUint64}; use crate::lotus_json::lotus_json_with_self; +use crate::rpc::eth::trace::GETH_TRACE_REVERT_ERROR; use crate::rpc::eth::trace::utils::extract_revert_reason; -use crate::rpc::eth::trace::{GETH_TRACE_REVERT_ERROR, PARITY_TRACE_REVERT_ERROR}; +use crate::shim::error::ExitCode; use anyhow::{Context as _, Result, bail}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; +/// Typed error for Parity-style EVM trace entries. +#[derive(Debug, Hash, Clone, PartialEq, Eq, thiserror::Error)] +pub enum TraceError { + #[error("Reverted")] + Reverted, + #[error("out of gas")] + OutOfGas, + #[error("invalid instruction")] + InvalidInstruction, + #[error("undefined instruction")] + UndefinedInstruction, + #[error("stack underflow")] + StackUnderflow, + #[error("stack overflow")] + StackOverflow, + #[error("illegal memory access")] + IllegalMemoryAccess, + #[error("invalid jump destination")] + BadJumpDest, + #[error("self destruct failed")] + SelfDestructFailed, + /// System-level VM error (exit code < FIRST_ACTOR_ERROR_CODE). + #[error("vm error: {}", ExitCode::from(*.0))] + VmError(u32), + /// Actor-level error (catch-all for unrecognised exit codes). + #[error("actor error: {}", ExitCode::from(*.0))] + ActorError(u32), +} + +impl Serialize for TraceError { + fn serialize(&self, serializer: S) -> Result { + serializer.serialize_str(&self.to_string()) + } +} + +impl<'de> Deserialize<'de> for TraceError { + fn deserialize>(deserializer: D) -> Result { + let s = String::deserialize(deserializer)?; + Ok(TraceError::from_string(&s)) + } +} + +impl TraceError { + pub fn from_string(s: &str) -> Self { + match s { + "Reverted" => Self::Reverted, + "out of gas" => Self::OutOfGas, + "invalid instruction" => Self::InvalidInstruction, + "undefined instruction" => Self::UndefinedInstruction, + "stack underflow" => Self::StackUnderflow, + "stack overflow" => Self::StackOverflow, + "illegal memory access" => Self::IllegalMemoryAccess, + "invalid jump destination" => Self::BadJumpDest, + "self destruct failed" => Self::SelfDestructFailed, + other => { + if let Some(rest) = other.strip_prefix("vm error: ") { + Self::VmError(parse_exit_code_display(rest)) + } else if let Some(rest) = other.strip_prefix("actor error: ") { + Self::ActorError(parse_exit_code_display(rest)) + } else { + Self::ActorError(0) + } + } + } + } + + /// Converts this Parity-format error to the equivalent Geth-format string. + pub fn to_geth_error_string(&self) -> String { + match self { + Self::Reverted => GETH_TRACE_REVERT_ERROR.into(), + other => other.to_string(), + } + } +} + +/// Parses `ExitCode`'s display format back to its `u32` value. +/// Handles both `"Name(N)"` (e.g., `"SysErrOutOfGas(7)"`) and plain `"N"`. +fn parse_exit_code_display(s: &str) -> u32 { + s.rsplit_once('(') + .and_then(|(_, n)| n.strip_suffix(')')) + .unwrap_or(s) + .parse() + .unwrap_or(0) +} + #[derive(Eq, Hash, PartialEq, Default, Serialize, Deserialize, Debug, Clone, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct EthCallTraceAction { @@ -419,7 +505,8 @@ pub struct EthTrace { pub action: TraceAction, pub result: TraceResult, #[serde(skip_serializing_if = "Option::is_none")] - pub error: Option, + #[schemars(with = "Option")] + pub error: Option, } impl EthTrace { @@ -428,23 +515,13 @@ impl EthTrace { } /// Returns true if the trace is a revert error. - /// - /// This is not a complete check for reverted traces (there are other possible revert reasons). pub fn is_reverted(&self) -> bool { - self.error - .as_deref() - .is_some_and(|e| e == PARITY_TRACE_REVERT_ERROR) + matches!(self.error, Some(TraceError::Reverted)) } /// Converts the Parity-format error stored in this trace to the Geth-format. pub fn to_geth_error(&self) -> Option { - self.error.as_deref().map(|error| { - if error == PARITY_TRACE_REVERT_ERROR { - GETH_TRACE_REVERT_ERROR.into() - } else { - error.to_string() - } - }) + self.error.as_ref().map(TraceError::to_geth_error_string) } /// Converts a Parity-style [`EthTrace`] into a Geth-style [`GethCallFrame`]. @@ -1131,7 +1208,7 @@ mod tests { gas_used: EthUint64(100), output: EthBytes(vec![]), }), - error: Some(PARITY_TRACE_REVERT_ERROR.into()), + error: Some(TraceError::Reverted), ..EthTrace::default() }; @@ -1240,7 +1317,7 @@ mod tests { code: EthBytes(vec![]), }), error: if result_address.is_none() { - Some("ErrForbidden".into()) + Some(TraceError::Reverted) } else { None }, @@ -1355,4 +1432,55 @@ mod tests { }; assert!(trace.match_filter_criteria(None, None).is_err()); } + + #[rstest] + #[case(TraceError::Reverted, "\"Reverted\"")] + #[case(TraceError::OutOfGas, "\"out of gas\"")] + #[case(TraceError::InvalidInstruction, "\"invalid instruction\"")] + #[case(TraceError::UndefinedInstruction, "\"undefined instruction\"")] + #[case(TraceError::StackUnderflow, "\"stack underflow\"")] + #[case(TraceError::StackOverflow, "\"stack overflow\"")] + #[case(TraceError::IllegalMemoryAccess, "\"illegal memory access\"")] + #[case(TraceError::BadJumpDest, "\"invalid jump destination\"")] + #[case(TraceError::SelfDestructFailed, "\"self destruct failed\"")] + fn test_trace_error_serialization_round_trip( + #[case] error: TraceError, + #[case] expected_json: &str, + ) { + let json = serde_json::to_string(&error).unwrap(); + assert_eq!(json, expected_json); + + let deserialized: TraceError = serde_json::from_str(&json).unwrap(); + assert_eq!(deserialized, error); + } + + #[test] + fn test_trace_error_vm_error_serialization() { + // ExitCode 7 = SYS_OUT_OF_GAS, but VmError is for other system codes + let error = TraceError::VmError(5); + let json = serde_json::to_string(&error).unwrap(); + assert!(json.contains("vm error:")); + + let deserialized: TraceError = serde_json::from_str(&json).unwrap(); + assert_eq!(deserialized, error); + } + + #[test] + fn test_trace_error_actor_error_serialization() { + let error = TraceError::ActorError(33); + let json = serde_json::to_string(&error).unwrap(); + assert!(json.contains("actor error:")); + + let deserialized: TraceError = serde_json::from_str(&json).unwrap(); + assert_eq!(deserialized, error); + } + + #[test] + fn test_trace_error_to_geth_error_string() { + assert_eq!( + TraceError::Reverted.to_geth_error_string(), + "execution reverted" + ); + assert_eq!(TraceError::OutOfGas.to_geth_error_string(), "out of gas"); + } }