feat(forge): revert diagnostic inspector

crates/anvil/src/eth/api.rs

@@ -2301,7 +2301,7 @@ impl EthApi {
                                 .coerce_status()
                             {
                                 if let Some(reason) =
-                                    RevertDecoder::new().maybe_decode(&output, None)
+                                    RevertDecoder::new().maybe_decode(&output, None, None)
                                 {
                                     tx.other.insert(
                                         "revertReason".to_string(),

crates/anvil/src/eth/backend/mem/mod.rs

@@ -1304,6 +1304,7 @@ impl Backend {
                     let r = RevertDecoder::new().decode(
                         info.out.as_ref().map(|b| &b[..]).unwrap_or_default(),
                         Some(info.exit),
+                        None,
                     );
                     node_info!("    Error: reverted with: {r}");
                 }

crates/anvil/src/eth/error.rs

@@ -359,7 +359,7 @@ impl<T: Serialize> ToRpcResponseResult for Result<T> {
                         let mut msg = "execution reverted".to_string();
                         if let Some(reason) = data
                             .as_ref()
-                            .and_then(|data| RevertDecoder::new().maybe_decode(data, None))
+                            .and_then(|data| RevertDecoder::new().maybe_decode(data, None, None))
                         {
                             msg = format!("{msg}: {reason}");
                         }

crates/cheatcodes/src/test/revert_handlers.rs

@@ -102,8 +102,8 @@ fn handle_revert(
         let (actual, expected) = if let Some(contracts) = known_contracts {
             let decoder = RevertDecoder::new().with_abis(contracts.values().map(|c| &c.abi));
             (
-                &decoder.decode(actual_revert.as_slice(), Some(status)),
-                &decoder.decode(expected_reason, Some(status)),
+                &decoder.decode(actual_revert.as_slice(), Some(status), None),
+                &decoder.decode(expected_reason, Some(status), None),
             )
         } else {
             (&stringify(&actual_revert), &stringify(expected_reason))

crates/evm/core/src/decode.rs

@@ -3,7 +3,7 @@
 use crate::abi::{console, Vm};
 use alloy_dyn_abi::JsonAbiExt;
 use alloy_json_abi::{Error, JsonAbi};
-use alloy_primitives::{hex, map::HashMap, Log, Selector};
+use alloy_primitives::{hex, map::HashMap, Address, Log, Selector};
 use alloy_sol_types::{
     ContractError::Revert, RevertReason, RevertReason::ContractError, SolEventInterface,
     SolInterface, SolValue,
@@ -13,6 +13,26 @@ use itertools::Itertools;
 use revm::interpreter::InstructionResult;
 use std::{fmt, sync::OnceLock};
 
+#[derive(Debug, Clone, Copy)]
+pub enum DetailedRevertReason {
+    CallToNonContract(Address),
+    DelegateCallToNonContract(Address),
+}
+
+impl fmt::Display for DetailedRevertReason {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::CallToNonContract(addr) => {
+                write!(f, "call to non-contract address `{addr}`")
+            }
+            Self::DelegateCallToNonContract(addr) => write!(
+                f,
+                "delegatecall to non-contract address `{addr}` (usually an unliked library)"
+            ),
+        }
+    }
+}
+
 /// A skip reason.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct SkipReason(pub Option<String>);
@@ -127,8 +147,13 @@ impl RevertDecoder {
     ///
     /// Note that this is just a best-effort guess, and should not be relied upon for anything other
     /// than user output.
-    pub fn decode(&self, err: &[u8], status: Option<InstructionResult>) -> String {
-        self.maybe_decode(err, status).unwrap_or_else(|| {
+    pub fn decode(
+        &self,
+        err: &[u8],
+        status: Option<InstructionResult>,
+        revert_reason: Option<DetailedRevertReason>,
+    ) -> String {
+        self.maybe_decode(err, status, revert_reason).unwrap_or_else(|| {
             if err.is_empty() {
                 "<empty revert data>".to_string()
             } else {
@@ -140,7 +165,12 @@ impl RevertDecoder {
     /// Tries to decode an error message from the given revert bytes.
     ///
     /// See [`decode`](Self::decode) for more information.
-    pub fn maybe_decode(&self, err: &[u8], status: Option<InstructionResult>) -> Option<String> {
+    pub fn maybe_decode(
+        &self,
+        err: &[u8],
+        status: Option<InstructionResult>,
+        revert_reason: Option<DetailedRevertReason>,
+    ) -> Option<String> {
         if let Some(reason) = SkipReason::decode(err) {
             return Some(reason.to_string());
         }
@@ -196,6 +226,10 @@ impl RevertDecoder {
         }
 
         if let Some(status) = status {
+            if let Some(reason) = revert_reason {
+                return Some(format!("EvmError: {reason}"));
+            }
+
             if !status.is_ok() {
                 return Some(format!("EvmError: {status:?}"));
             }
@@ -272,7 +306,7 @@ mod tests {
             "0xe17594de"
             "756688fe00000000000000000000000000000000000000000000000000000000"
         );
-        assert_eq!(decoder.decode(data, None), "ValidationFailed(0x)");
+        assert_eq!(decoder.decode(data, None, None), "ValidationFailed(0x)");
 
         /*
         abi.encodeWithSelector(ValidationFailed.selector, abi.encodeWithSelector(InvalidNonce.selector))
@@ -283,6 +317,6 @@ mod tests {
             "0000000000000000000000000000000000000000000000000000000000000004"
             "756688fe00000000000000000000000000000000000000000000000000000000"
         );
-        assert_eq!(decoder.decode(data, None), "ValidationFailed(0x756688fe)");
+        assert_eq!(decoder.decode(data, None, None), "ValidationFailed(0x756688fe)");
     }
 }

crates/evm/evm/src/executors/fuzz/mod.rs

@@ -150,7 +150,7 @@ impl FuzzedExecutor {
                     // since that input represents the last run case, which may not correspond with
                     // our failure - when a fuzz case fails, proptest will try to run at least one
                     // more case to find a minimal failure case.
-                    let reason = rd.maybe_decode(&outcome.1.result, Some(status));
+                    let reason = rd.maybe_decode(&outcome.1.result, Some(status), None);
                     execution_data.borrow_mut().logs.extend(outcome.1.logs.clone());
                     execution_data.borrow_mut().counterexample = outcome;
                     // HACK: we have to use an empty string here to denote `None`.

crates/evm/evm/src/executors/invariant/error.rs

@@ -80,7 +80,7 @@ impl FailedInvariantCaseData {
         let revert_reason = RevertDecoder::new()
             .with_abis(targeted_contracts.targets.lock().values().map(|c| &c.abi))
             .with_abi(invariant_contract.abi)
-            .decode(call_result.result.as_ref(), Some(call_result.exit_reason));
+            .decode(call_result.result.as_ref(), Some(call_result.exit_reason), None);
 
         let func = invariant_contract.invariant_function;
         debug_assert!(func.inputs.is_empty());

crates/evm/evm/src/executors/mod.rs

@@ -22,7 +22,7 @@ use foundry_evm_core::{
         CALLER, CHEATCODE_ADDRESS, CHEATCODE_CONTRACT_HASH, DEFAULT_CREATE2_DEPLOYER,
         DEFAULT_CREATE2_DEPLOYER_CODE, DEFAULT_CREATE2_DEPLOYER_DEPLOYER,
     },
-    decode::{RevertDecoder, SkipReason},
+    decode::{DetailedRevertReason, RevertDecoder, SkipReason},
     utils::StateChangeset,
     InspectorExt,
 };
@@ -769,6 +769,9 @@ impl From<DeployResult> for RawCallResult {
 pub struct RawCallResult {
     /// The status of the call
     pub exit_reason: InstructionResult,
+    /// Fallback reason for reverts.
+    /// Should only be used if no custom errors, or string errors can be decoded.
+    pub maybe_reason: Option<DetailedRevertReason>,
     /// Whether the call reverted or not
     pub reverted: bool,
     /// Whether the call includes a snapshot failure
@@ -810,6 +813,7 @@ impl Default for RawCallResult {
     fn default() -> Self {
         Self {
             exit_reason: InstructionResult::Continue,
+            maybe_reason: None,
             reverted: false,
             has_state_snapshot_failure: false,
             result: Bytes::new(),
@@ -853,7 +857,8 @@ impl RawCallResult {
         if let Some(reason) = SkipReason::decode(&self.result) {
             return EvmError::Skip(reason);
         }
-        let reason = rd.unwrap_or_default().decode(&self.result, Some(self.exit_reason));
+        let reason =
+            rd.unwrap_or_default().decode(&self.result, Some(self.exit_reason), self.maybe_reason);
         EvmError::Execution(Box::new(self.into_execution_error(reason)))
     }
 
@@ -950,8 +955,15 @@ fn convert_executed_result(
         _ => Bytes::new(),
     };
 
-    let InspectorData { mut logs, labels, traces, coverage, cheatcodes, chisel_state } =
-        inspector.collect();
+    let InspectorData {
+        mut logs,
+        labels,
+        traces,
+        coverage,
+        cheatcodes,
+        chisel_state,
+        maybe_reason,
+    } = inspector.collect();
 
     if logs.is_empty() {
         logs = exec_logs;
@@ -964,6 +976,7 @@ fn convert_executed_result(
 
     Ok(RawCallResult {
         exit_reason,
+        maybe_reason,
         reverted: !matches!(exit_reason, return_ok!()),
         has_state_snapshot_failure,
         result,

crates/evm/evm/src/inspectors/mod.rs

@@ -18,3 +18,5 @@ pub use script::ScriptExecutionInspector;
 
 mod stack;
 pub use stack::{InspectorData, InspectorStack, InspectorStackBuilder};
+
+mod revert_diagnostic;

crates/evm/evm/src/inspectors/revert_diagnostic.rs

@@ -0,0 +1,147 @@
+use alloy_primitives::{Address, U256};
+use foundry_evm_core::{
+    backend::DatabaseExt,
+    constants::{CHEATCODE_ADDRESS, HARDHAT_CONSOLE_ADDRESS},
+    decode::DetailedRevertReason,
+};
+use revm::{
+    interpreter::{
+        opcode::EXTCODESIZE, CallInputs, CallOutcome, CallScheme, InstructionResult, Interpreter,
+    },
+    precompile::{PrecompileSpecId, Precompiles},
+    primitives::SpecId,
+    Database, EvmContext, Inspector,
+};
+
+const IGNORE: [Address; 2] = [HARDHAT_CONSOLE_ADDRESS, CHEATCODE_ADDRESS];
+
+/// An inspector that tracks call context to enhances revert diagnostics.
+/// Useful for understanding reverts that are not linked to custom errors or revert strings.
+#[derive(Clone, Debug, Default)]
+pub struct RevertDiagnostic {
+    /// Tracks calls with calldata that target an address without executable code.
+    pub non_contract_call: Option<(Address, CallScheme, u64)>,
+    /// Tracks EXTCODESIZE checks that target an address without executable code.
+    pub non_contract_size_check: Option<(Address, u64)>,
+    /// Whether the step opcode is EXTCODESIZE or not.
+    pub is_extcodesize_step: bool,
+    /// Tracks whether a failed call has been spotted or not.
+    pub reverted: bool,
+}
+
+impl RevertDiagnostic {
+    fn is_delegatecall(&self, scheme: CallScheme) -> bool {
+        matches!(
+            scheme,
+            CallScheme::DelegateCall | CallScheme::ExtDelegateCall | CallScheme::CallCode
+        )
+    }
+
+    fn is_precompile(&self, spec_id: SpecId, target: Address) -> bool {
+        let precompiles = Precompiles::new(PrecompileSpecId::from_spec_id(spec_id));
+        precompiles.contains(&target)
+    }
+
+    pub fn no_code_target_address(&self, inputs: &mut CallInputs) -> Address {
+        if self.is_delegatecall(inputs.scheme) {
+            inputs.bytecode_address
+        } else {
+            inputs.target_address
+        }
+    }
+
+    pub fn reason(&self) -> Option<DetailedRevertReason> {
+        if self.reverted {
+            if let Some((addr, scheme, _)) = self.non_contract_call {
+                let reason = if self.is_delegatecall(scheme) {
+                    DetailedRevertReason::DelegateCallToNonContract(addr)
+                } else {
+                    DetailedRevertReason::CallToNonContract(addr)
+                };
+
+                return Some(reason);
+            }
+        }
+
+        if let Some((addr, _)) = self.non_contract_size_check {
+            // call never took place, so schema is unknown --> output most generic msg
+            return Some(DetailedRevertReason::CallToNonContract(addr));
+        }
+
+        None
+    }
+}
+
+impl<DB: Database + DatabaseExt> Inspector<DB> for RevertDiagnostic {
+    /// Tracks the first call, with non-zero calldata, that targeted a non-contract address.
+    /// Excludes precompiles and test addresses.
+    fn call(&mut self, ctx: &mut EvmContext<DB>, inputs: &mut CallInputs) -> Option<CallOutcome> {
+        let target = self.no_code_target_address(inputs);
+
+        if IGNORE.contains(&target) || self.is_precompile(ctx.spec_id(), target) {
+            return None;
+        }
+
+        if let Ok(state) = ctx.code(target) {
+            if state.is_empty() && !inputs.input.is_empty() && !self.reverted {
+                self.non_contract_call = Some((target, inputs.scheme, ctx.journaled_state.depth()));
+            }
+        }
+        None
+    }
+
+    /// Tracks `EXTCODESIZE` opcodes. Clears the cache when the call stack depth changes.
+    fn step(&mut self, interp: &mut Interpreter, ctx: &mut EvmContext<DB>) {
+        if let Some((_, depth)) = self.non_contract_size_check {
+            if depth != ctx.journaled_state.depth() {
+                self.non_contract_size_check = None;
+            }
+
+            return;
+        }
+
+        if EXTCODESIZE == interp.current_opcode() {
+            if let Ok(word) = interp.stack().peek(0) {
+                let addr = Address::from_word(word.into());
+                if IGNORE.contains(&addr) || self.is_precompile(ctx.spec_id(), addr) {
+                    return;
+                }
+
+                self.non_contract_size_check = Some((addr, ctx.journaled_state.depth()));
+                self.is_extcodesize_step = true;
+            }
+        }
+    }
+
+    /// Tracks `EXTCODESIZE` output. If the bytecode size is 0, clears the cache.
+    fn step_end(&mut self, interp: &mut Interpreter, _ctx: &mut EvmContext<DB>) {
+        if self.is_extcodesize_step {
+            if let Ok(size) = interp.stack().peek(0) {
+                if size != U256::ZERO {
+                    self.non_contract_size_check = None;
+                }
+            }
+
+            self.is_extcodesize_step = false;
+        }
+    }
+
+    /// Records whether the call reverted or not. Only if the revert call depth matches the
+    /// inspector cache.
+    fn call_end(
+        &mut self,
+        ctx: &mut EvmContext<DB>,
+        _inputs: &CallInputs,
+        outcome: CallOutcome,
+    ) -> CallOutcome {
+        if let Some((_, _, depth)) = self.non_contract_call {
+            if outcome.result.result == InstructionResult::Revert &&
+                ctx.journaled_state.depth() == depth - 1
+            {
+                self.reverted = true
+            };
+        }
+
+        outcome
+    }
+}