fix(invariants): respect fail_on_revert properly and do not populate logs/traces twice

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

@@ -11,7 +11,7 @@ use revm::primitives::U256;
 
 /// Given the executor state, asserts that no invariant has been broken. Otherwise, it fills the
 /// external `invariant_failures.failed_invariant` map and returns a generic error.
-/// Returns the mapping of (Invariant Function Name -> Call Result).
+/// Either returns the call result if successful, or nothing if there was an error.
 pub fn assert_invariants(
     invariant_contract: &InvariantContract<'_>,
     executor: &Executor,

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

@@ -115,15 +115,21 @@ impl<'a> InvariantExecutor<'a> {
         // Stores data related to reverts or failed assertions of the test.
         let failures = RefCell::new(InvariantFailures::new());
 
+        // Stores the calldata in the last run.
+        let last_run_calldata: RefCell<Vec<BasicTxDetails>> = RefCell::new(vec![]);
+
+        // Let's make sure the invariant is sound before actually starting the run:
+        // We'll assert the invariant in its initial state, and if it fails, we'll
+        // already know if we can early exit the invariant run.
+        // This does not count as a fuzz run. It will just register the revert.
         let last_call_results = RefCell::new(assert_invariants(
             &invariant_contract,
             &self.executor,
             &[],
             &mut failures.borrow_mut(),
             self.config.shrink_sequence,
         ));
-        let last_run_calldata: RefCell<Vec<BasicTxDetails>> = RefCell::new(vec![]);
-        // Make sure invariants are sound even before starting to fuzz
+
         if last_call_results.borrow().is_none() {
             fuzz_cases.borrow_mut().push(FuzzedCases::new(vec![]));
         }
@@ -134,8 +140,8 @@ impl<'a> InvariantExecutor<'a> {
         // values.
         let branch_runner = RefCell::new(self.runner.clone());
         let _ = self.runner.run(&strat, |mut inputs| {
-            // Scenarios where we want to fail as soon as possible.
-            if self.config.fail_on_revert && failures.borrow().reverts == 1 {
+            // We stop the run immediately if we have reverted, and `fail_on_revert` is set.
+            if self.config.fail_on_revert && failures.borrow().reverts > 0 {
                 return Err(TestCaseError::fail("Revert occurred."))
             }
 
@@ -745,10 +751,9 @@ fn can_continue(
             return RichInvariantResults::new(false, None)
         }
     } else {
+        // Increase the amount of reverts.
         failures.reverts += 1;
-
-        // The user might want to stop all execution if a revert happens to
-        // better bound their testing space.
+        // If fail on revert is set, we must return inmediately.
         if fail_on_revert {
             let error = InvariantFuzzError::new(
                 invariant_contract,
@@ -760,6 +765,7 @@ fn can_continue(
             );
 
             failures.revert_reason = Some(error.revert_reason.clone());
+            failures.error = Some(error);
 
             return RichInvariantResults::new(false, None)
         }

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

@@ -20,7 +20,7 @@ pub type BasicTxDetails = (Address, (Address, Bytes));
 pub struct InvariantContract<'a> {
     /// Address of the test contract.
     pub address: Address,
-    /// Invariant functions present in the test contract.
+    /// Invariant function present in the test contract.
     pub invariant_function: &'a Function,
     /// Abi of the test contract.
     pub abi: &'a Abi,

crates/forge/src/runner.rs

@@ -487,12 +487,6 @@ impl<'a> ContractRunner<'a> {
                         error!(?err, "Failed to replay invariant error")
                     }
                 };
-
-                logs.extend(error.logs);
-
-                if let Some(error_traces) = error.traces {
-                    traces.push((TraceKind::Execution, error_traces));
-                }
             }
 
             // If invariants ran successfully, replay the last run to collect logs and

crates/forge/tests/it/invariant.rs

@@ -20,6 +20,10 @@ async fn test_invariant() {
     assert_multiple(
         &results,
         BTreeMap::from([
+            (
+                "fuzz/invariant/common/InvariantHandlerFailure.t.sol:InvariantHandlerFailure",
+                vec![("statefulFuzz_BrokenInvariant()", true, None, None, None)],
+            ),
             (
                 "fuzz/invariant/common/InvariantInnerContract.t.sol:InvariantInnerContract",
                 vec![("invariantHideJesus()", false, Some("jesus betrayed.".into()), None, None)],
@@ -113,6 +117,39 @@ async fn test_invariant_override() {
     );
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn test_invariant_fail_on_revert() {
+    let mut runner = runner().await;
+
+    let mut opts = test_opts();
+    opts.invariant.fail_on_revert = true;
+    opts.invariant.runs = 1;
+    opts.invariant.depth = 10;
+    runner.test_options = opts.clone();
+
+    let results = runner
+        .test(
+            &Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantHandlerFailure.t.sol"),
+            None,
+            opts,
+        )
+        .await;
+
+    assert_multiple(
+        &results,
+        BTreeMap::from([(
+            "fuzz/invariant/common/InvariantHandlerFailure.t.sol:InvariantHandlerFailure",
+            vec![(
+                "statefulFuzz_BrokenInvariant()",
+                false,
+                Some("failed on revert".into()),
+                None,
+                None,
+            )],
+        )]),
+    );
+}
+
 #[tokio::test(flavor = "multi_thread")]
 #[ignore]
 async fn test_invariant_storage() {

testdata/fuzz/invariant/common/InvariantHandlerFailure.t.sol

@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.0;
+
+import "ds-test/test.sol";
+
+struct FuzzSelector {
+    address addr;
+    bytes4[] selectors;
+}
+
+contract Handler is DSTest {
+    function doSomething() public {
+        require(false, "failed on revert");
+    }
+}
+
+contract InvariantHandlerFailure is DSTest {
+    bytes4[] internal selectors;
+
+    Handler handler;
+
+    function targetSelectors() public returns (FuzzSelector[] memory) {
+        FuzzSelector[] memory targets = new FuzzSelector[](1);
+        bytes4[] memory selectors = new bytes4[](1);
+        selectors[0] = handler.doSomething.selector;
+        targets[0] = FuzzSelector(address(handler), selectors);
+        return targets;
+    }
+
+    function setUp() public {
+        handler = new Handler();
+    }
+
+    function statefulFuzz_BrokenInvariant() public {}
+}