diff --git a/src/tracing/utils.rs b/src/tracing/utils.rs index dd61c76..41f679b 100644 --- a/src/tracing/utils.rs +++ b/src/tracing/utils.rs @@ -114,22 +114,27 @@ pub(crate) fn load_account_code( } /// Returns a non-empty revert reason if the output is a revert/error. +/// Follows Geth's UnpackRevert logic +/// . #[inline] pub(crate) fn maybe_revert_reason(output: &[u8]) -> Option { - let reason = match GenericRevertReason::decode(output)? { + match GenericRevertReason::decode(output)? { GenericRevertReason::ContractError(err) => { - match err { + let reason = match err { // return the raw revert reason and don't use the revert's display message ContractError::Revert(revert) => revert.reason, + // return as geth panic error string + ContractError::Panic(panic) => panic.as_geth_str().to_string(), err => err.to_string(), + }; + + if reason.is_empty() || reason.trim_matches('\0').is_empty() { + None + } else { + Some(reason) } } - GenericRevertReason::RawString(err) => err, - }; - if reason.is_empty() { - None - } else { - Some(reason) + _ => None, } } @@ -153,4 +158,52 @@ mod tests { let reason = maybe_revert_reason(&err[..]).unwrap(); assert_eq!(reason, "UniswapV2: INSUFFICIENT_INPUT_AMOUNT"); } + + #[test] + fn decode_revert_reason_with_null_bytes() { + let empty_err = GenericContractError::Revert("".into()); + let encoded = empty_err.abi_encode(); + assert!(maybe_revert_reason(&encoded).is_none()); + + let null_bytes_err = + GenericContractError::Revert(String::from_utf8(vec![0u8; 32]).unwrap().into()); + let encoded = null_bytes_err.abi_encode(); + assert!(maybe_revert_reason(&encoded).is_none()); + } + + #[test] + fn decode_revert_reason_with_invalid_selector() { + // Test data that doesn't start with revert or panic selector + let invalid_data = vec![0x12, 0x34, 0x56, 0x78]; // Invalid selector + assert_eq!( + maybe_revert_reason(&invalid_data), + None, + "Should return None for invalid selector" + ); + + // Test data too short + let short_data = vec![0x08, 0xc3]; // Only 2 bytes + assert_eq!(maybe_revert_reason(&short_data), None, "Should return None for data too short"); + + // Test valid string but with proper selector validation + let control_char_err = GenericContractError::Revert("\u{000f}.[l".into()); + let encoded = control_char_err.abi_encode(); + // This should now return the string since we're using Geth's validation logic + assert_eq!(maybe_revert_reason(&encoded), Some("\u{000f}.[l".to_string())); + + // Test normal readable string + let readable_err = GenericContractError::Revert("Normal error message".into()); + let encoded = readable_err.abi_encode(); + assert_eq!(maybe_revert_reason(&encoded), Some("Normal error message".to_string())); + } + + #[test] + fn decode_revert_reason_with_raw_string() { + let non_readable_data = "\u{9a62}\u{0002}".as_bytes(); + assert_eq!( + maybe_revert_reason(non_readable_data), + None, + "Should return None for raw strings" + ); + } } diff --git a/tests/it/geth.rs b/tests/it/geth.rs index cfe811c..50ff6f3 100644 --- a/tests/it/geth.rs +++ b/tests/it/geth.rs @@ -647,3 +647,57 @@ fn test_geth_prestate_disable_code_in_diff_mode() { _ => panic!("Expected Diff mode PreStateFrame"), } } + +#[test] +fn test_geth_calltracer_null_bytes_revert_reason_omitted() { + /* + Test that verifies revertReason field is omitted when revert data contains only null bytes. + This simulates scenarios where contract reverts with empty or null-byte-only data, + ensuring compatibility with Geth behavior of omitting such meaningless revert reasons. + */ + let mut evm = Context::mainnet().with_db(CacheDB::new(EmptyDB::default())).build_mainnet(); + + // Deploy a simple contract that reverts with empty or null bytes + // This bytecode reverts with 32 null bytes: revert(0, 32) + // 60206000fd - PUSH1 32, PUSH1 0, REVERT + let code = hex!("608060405234801561001057600080fd5b5060988061001f6000396000f3fe6080604052348015600f57600080fd5b506004361060285760003560e01c80633bc5de3014602d575b600080fd5b60336035565b005b60206000fd5b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"); + + let deployer = Address::ZERO; + let addr = + deploy_contract(&mut evm, code.into(), deployer, SpecId::LONDON).created_address().unwrap(); + + let mut insp = TracingInspector::new(TracingInspectorConfig::default_geth()); + let mut evm = evm.with_inspector(&mut insp); + + // Call function that reverts with null bytes + let res = evm + .inspect_tx(TxEnv { + caller: deployer, + kind: TxKind::Call(addr), + data: hex!("3bc5de30").into(), // call function selector + gas_limit: 1_000_000, + nonce: 1, // Set correct nonce after contract deployment + ..Default::default() + }) + .unwrap(); + + let call_config = CallConfig::default(); + let call_frame = insp.geth_builder().geth_call_traces(call_config, res.result.gas_used()); + + assert!(call_frame.error.is_some(), "Call should have an error"); + + assert!( + call_frame.revert_reason.is_none(), + "revertReason should be omitted for null-byte-only revert data" + ); + + // Test JSON serialization to ensure field is omitted + assert!( + !serde_json::to_value(&call_frame) + .unwrap() + .as_object() + .unwrap() + .contains_key("revertReason"), + "revertReason field should not be present in JSON when containing only null bytes" + ); +}