feat: improve error decoding and value formatting (#6286)

* feat: better formatting

* fix: use empty string to denote None in fuzz errors

* fix tests

crates/common/src/calc.rs

@@ -1,6 +1,6 @@
 //! Commonly used calculations.
 
-use alloy_primitives::U256;
+use alloy_primitives::{Sign, U256};
 use std::ops::Div;
 
 /// Returns the mean of the slice
@@ -44,7 +44,7 @@ pub fn median_sorted(values: &[U256]) -> U256 {
 ///     10000000 -> 1e7
 /// ```
 #[inline]
-pub fn to_exp_notation(value: U256, precision: usize, trim_end_zeros: bool) -> String {
+pub fn to_exp_notation(value: U256, precision: usize, trim_end_zeros: bool, sign: Sign) -> String {
     let stringified = value.to_string();
     let exponent = stringified.len() - 1;
     let mut mantissa = stringified.chars().take(precision).collect::<String>();
@@ -61,7 +61,7 @@ pub fn to_exp_notation(value: U256, precision: usize, trim_end_zeros: bool) -> S
         mantissa.insert(1, '.');
     }
 
-    format!("{mantissa}e{exponent}")
+    format!("{sign}{mantissa}e{exponent}")
 }
 
 #[cfg(test)]
@@ -119,18 +119,18 @@ mod tests {
     fn test_format_to_exponential_notation() {
         let value = 1234124124u64;
 
-        let formatted = to_exp_notation(U256::from(value), 4, false);
+        let formatted = to_exp_notation(U256::from(value), 4, false, Sign::Positive);
         assert_eq!(formatted, "1.234e9");
 
-        let formatted = to_exp_notation(U256::from(value), 3, true);
+        let formatted = to_exp_notation(U256::from(value), 3, true, Sign::Positive);
         assert_eq!(formatted, "1.23e9");
 
         let value = 10000000u64;
 
-        let formatted = to_exp_notation(U256::from(value), 4, false);
+        let formatted = to_exp_notation(U256::from(value), 4, false, Sign::Positive);
         assert_eq!(formatted, "1.000e7");
 
-        let formatted = to_exp_notation(U256::from(value), 3, true);
+        let formatted = to_exp_notation(U256::from(value), 3, true, Sign::Positive);
         assert_eq!(formatted, "1e7");
     }
 }

crates/common/src/fmt.rs

@@ -2,9 +2,9 @@
 
 use crate::{calc::to_exp_notation, TransactionReceiptWithRevertReason};
 use alloy_dyn_abi::{DynSolType, DynSolValue};
-use alloy_primitives::{hex, U256};
+use alloy_primitives::{hex, Sign, I256, U256};
 use eyre::Result;
-use std::fmt::{self, Display};
+use std::fmt;
 use yansi::Paint;
 
 pub use foundry_macros::fmt::*;
@@ -18,44 +18,55 @@ impl DynValueFormatter {
     /// Recursively formats a [`DynSolValue`].
     fn value(&self, value: &DynSolValue, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match value {
-            DynSolValue::Address(inner) => Display::fmt(inner, f),
-            DynSolValue::Function(inner) => Display::fmt(inner, f),
+            DynSolValue::Address(inner) => write!(f, "{inner}"),
+            DynSolValue::Function(inner) => write!(f, "{inner}"),
             DynSolValue::Bytes(inner) => f.write_str(&hex::encode_prefixed(inner)),
-            DynSolValue::FixedBytes(inner, _) => f.write_str(&hex::encode_prefixed(inner)),
+            DynSolValue::FixedBytes(inner, _) => write!(f, "{inner}"),
             DynSolValue::Uint(inner, _) => {
                 if self.raw {
                     write!(f, "{inner}")
                 } else {
                     f.write_str(&format_uint_exp(*inner))
                 }
             }
-            DynSolValue::Int(inner, _) => write!(f, "{inner}"),
+            DynSolValue::Int(inner, _) => {
+                if self.raw {
+                    write!(f, "{inner}")
+                } else {
+                    f.write_str(&format_int_exp(*inner))
+                }
+            }
             DynSolValue::Array(values) | DynSolValue::FixedArray(values) => {
                 f.write_str("[")?;
                 self.list(values, f)?;
                 f.write_str("]")
             }
             DynSolValue::Tuple(values) => self.tuple(values, f),
-            DynSolValue::String(inner) => f.write_str(inner),
-            DynSolValue::Bool(inner) => Display::fmt(inner, f),
+            DynSolValue::String(inner) => write!(f, "{inner:?}"), // escape strings
+            DynSolValue::Bool(inner) => write!(f, "{inner}"),
             DynSolValue::CustomStruct { name, prop_names, tuple } => {
                 if self.raw {
                     return self.tuple(tuple, f);
                 }
 
                 f.write_str(name)?;
-                f.write_str(" { ")?;
 
-                for (i, (prop_name, value)) in std::iter::zip(prop_names, tuple).enumerate() {
-                    if i > 0 {
-                        f.write_str(", ")?;
+                if prop_names.len() == tuple.len() {
+                    f.write_str("({ ")?;
+
+                    for (i, (prop_name, value)) in std::iter::zip(prop_names, tuple).enumerate() {
+                        if i > 0 {
+                            f.write_str(", ")?;
+                        }
+                        f.write_str(prop_name)?;
+                        f.write_str(": ")?;
+                        self.value(value, f)?;
                     }
-                    f.write_str(prop_name)?;
-                    f.write_str(": ")?;
-                    self.value(value, f)?;
-                }
 
-                f.write_str(" }")
+                    f.write_str(" })")
+                } else {
+                    self.tuple(tuple, f)
+                }
             }
         }
     }
@@ -143,7 +154,7 @@ pub fn format_token_raw(value: &DynSolValue) -> String {
 /// use alloy_primitives::U256;
 /// use foundry_common::fmt::format_uint_exp as f;
 ///
-/// yansi::Paint::disable();
+/// # yansi::Paint::disable();
 /// assert_eq!(f(U256::from(0)), "0");
 /// assert_eq!(f(U256::from(1234)), "1234");
 /// assert_eq!(f(U256::from(1234567890)), "1234567890 [1.234e9]");
@@ -155,8 +166,44 @@ pub fn format_uint_exp(num: U256) -> String {
         return num.to_string()
     }
 
-    let exp = to_exp_notation(num, 4, true);
-    format!("{} {}", num, Paint::default(format!("[{exp}]")).dimmed())
+    let exp = to_exp_notation(num, 4, true, Sign::Positive);
+    format!("{num} {}", Paint::default(format!("[{exp}]")).dimmed())
+}
+
+/// Formats a U256 number to string, adding an exponential notation _hint_.
+///
+/// Same as [`format_uint_exp`].
+///
+/// # Examples
+///
+/// ```
+/// use alloy_primitives::I256;
+/// use foundry_common::fmt::format_int_exp as f;
+///
+/// # yansi::Paint::disable();
+/// assert_eq!(f(I256::try_from(0).unwrap()), "0");
+/// assert_eq!(f(I256::try_from(-1).unwrap()), "-1");
+/// assert_eq!(f(I256::try_from(1234).unwrap()), "1234");
+/// assert_eq!(f(I256::try_from(1234567890).unwrap()), "1234567890 [1.234e9]");
+/// assert_eq!(f(I256::try_from(-1234567890).unwrap()), "-1234567890 [-1.234e9]");
+/// assert_eq!(f(I256::try_from(1000000000000000000_u128).unwrap()), "1000000000000000000 [1e18]");
+/// assert_eq!(
+///     f(I256::try_from(10000000000000000000000_u128).unwrap()),
+///     "10000000000000000000000 [1e22]"
+/// );
+/// assert_eq!(
+///     f(I256::try_from(-10000000000000000000000_i128).unwrap()),
+///     "-10000000000000000000000 [-1e22]"
+/// );
+/// ```
+pub fn format_int_exp(num: I256) -> String {
+    let (sign, abs) = num.into_sign_and_abs();
+    if abs < U256::from(10_000) {
+        return format!("{sign}{abs}");
+    }
+
+    let exp = to_exp_notation(abs, 4, true, sign);
+    format!("{sign}{abs} {}", Paint::default(format!("[{exp}]")).dimmed())
 }
 
 impl UIfmt for TransactionReceiptWithRevertReason {

crates/evm/core/src/decode.rs

@@ -70,27 +70,41 @@ pub fn decode_revert(
     maybe_abi: Option<&JsonAbi>,
     status: Option<InstructionResult>,
 ) -> String {
+    maybe_decode_revert(err, maybe_abi, status).unwrap_or_else(|| {
+        if err.is_empty() {
+            "<no data>".to_string()
+        } else {
+            trimmed_hex(err)
+        }
+    })
+}
+
+pub fn maybe_decode_revert(
+    err: &[u8],
+    maybe_abi: Option<&JsonAbi>,
+    status: Option<InstructionResult>,
+) -> Option<String> {
     if err.len() < SELECTOR_LEN {
         if let Some(status) = status {
             if !status.is_ok() {
-                return format!("EvmError: {status:?}")
+                return Some(format!("EvmError: {status:?}"));
             }
         }
         return if err.is_empty() {
-            "<no data>".to_string()
+            None
         } else {
-            format!("custom error bytes {}", hex::encode_prefixed(err))
+            Some(format!("custom error bytes {}", hex::encode_prefixed(err)))
         }
     }
 
     if err == crate::constants::MAGIC_SKIP {
         // Also used in forge fuzz runner
-        return "SKIPPED".to_string()
+        return Some("SKIPPED".to_string());
     }
 
     // Solidity's `Error(string)` or `Panic(uint256)`
     if let Ok(e) = alloy_sol_types::GenericContractError::abi_decode(err, false) {
-        return e.to_string()
+        return Some(e.to_string());
     }
 
     let (selector, data) = err.split_at(SELECTOR_LEN);
@@ -99,21 +113,18 @@ pub fn decode_revert(
     match *selector {
         // `CheatcodeError(string)`
         Vm::CheatcodeError::SELECTOR => {
-            if let Ok(e) = Vm::CheatcodeError::abi_decode_raw(data, false) {
-                return e.message
-            }
+            let e = Vm::CheatcodeError::abi_decode_raw(data, false).ok()?;
+            return Some(e.message);
         }
         // `expectRevert(bytes)`
         Vm::expectRevert_2Call::SELECTOR => {
-            if let Ok(e) = Vm::expectRevert_2Call::abi_decode_raw(data, false) {
-                return decode_revert(&e.revertData[..], maybe_abi, status)
-            }
+            let e = Vm::expectRevert_2Call::abi_decode_raw(data, false).ok()?;
+            return maybe_decode_revert(&e.revertData[..], maybe_abi, status);
         }
         // `expectRevert(bytes4)`
         Vm::expectRevert_1Call::SELECTOR => {
-            if let Ok(e) = Vm::expectRevert_1Call::abi_decode_raw(data, false) {
-                return decode_revert(&e.revertData[..], maybe_abi, status)
-            }
+            let e = Vm::expectRevert_1Call::abi_decode_raw(data, false).ok()?;
+            return maybe_decode_revert(&e.revertData[..], maybe_abi, status);
         }
         _ => {}
     }
@@ -123,31 +134,31 @@ pub fn decode_revert(
         if let Some(abi_error) = abi.errors().find(|e| selector == e.selector()) {
             // if we don't decode, don't return an error, try to decode as a string later
             if let Ok(decoded) = abi_error.abi_decode_input(data, false) {
-                return format!(
+                return Some(format!(
                     "{}({})",
                     abi_error.name,
                     decoded.iter().map(foundry_common::fmt::format_token).format(", ")
-                )
+                ));
             }
         }
     }
 
     // ABI-encoded `string`
     if let Ok(s) = String::abi_decode(err, false) {
-        return s
+        return Some(s);
     }
 
     // UTF-8-encoded string
     if let Ok(s) = std::str::from_utf8(err) {
-        return s.to_string()
+        return Some(s.to_string());
     }
 
     // Generic custom error
-    format!(
+    Some(format!(
         "custom error {}:{}",
         hex::encode(selector),
         std::str::from_utf8(data).map_or_else(|_| trimmed_hex(data), String::from)
-    )
+    ))
 }
 
 fn trimmed_hex(s: &[u8]) -> String {

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

@@ -129,7 +129,9 @@ impl<'a> FuzzedExecutor<'a> {
                     // case.
                     let call_res = _counterexample.1.result.clone();
                     *counterexample.borrow_mut() = _counterexample;
-                    Err(TestCaseError::fail(decode::decode_revert(&call_res, errors, Some(status))))
+                    // HACK: we have to use an empty string here to denote `None`
+                    let reason = decode::maybe_decode_revert(&call_res, errors, Some(status));
+                    Err(TestCaseError::fail(reason.unwrap_or_default()))
                 }
             }
         });

crates/evm/fuzz/Cargo.toml

@@ -37,6 +37,7 @@ ethers-core.workspace = true
 arbitrary = "1.3.1"
 eyre = "0.6"
 hashbrown = { version = "0.14", features = ["serde"] }
+itertools.workspace = true
 parking_lot = "0.12"
 proptest = "1"
 rand.workspace = true

crates/evm/fuzz/src/lib.rs

@@ -13,6 +13,7 @@ use ethers_core::types::Log;
 use foundry_common::{calc, contracts::ContractsByAddress};
 use foundry_evm_coverage::HitMaps;
 use foundry_evm_traces::CallTraceArena;
+use itertools::Itertools;
 use serde::{Deserialize, Serialize};
 use std::{collections::BTreeMap, fmt};
 
@@ -92,8 +93,6 @@ impl BaseCounterExample {
 
 impl fmt::Display for BaseCounterExample {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let args = foundry_common::fmt::format_tokens(&self.args).collect::<Vec<_>>().join(", ");
-
         if let Some(sender) = self.sender {
             write!(f, "sender={sender} addr=")?
         }
@@ -112,7 +111,7 @@ impl fmt::Display for BaseCounterExample {
             write!(f, "calldata={}", self.calldata)?
         }
 
-        write!(f, ", args=[{args}]")
+        write!(f, " args=[{}]", foundry_common::fmt::format_tokens(&self.args).format(", "))
     }
 }
 

crates/forge/bin/cmd/create.rs

@@ -205,9 +205,9 @@ impl CreateArgs {
 
         let is_args_empty = args.is_empty();
         let deployer =
-            factory.deploy_tokens(args.clone()).context("Failed to deploy contract").map_err(|e| {
+            factory.deploy_tokens(args.clone()).context("failed to deploy contract").map_err(|e| {
                 if is_args_empty {
-                    e.wrap_err("No arguments provided for contract constructor. Consider --constructor-args or --constructor-args-path")
+                    e.wrap_err("no arguments provided for contract constructor; consider --constructor-args or --constructor-args-path")
                 } else {
                     e
                 }