Repository: foundry-rs/foundry (language: rust)

Issue:
bug(`forge`): invariant failures that persist on the cache are retried even if they should be excluded according to the configuration.
### Component

Forge

### Have you ensured that all of these are up to date?

- [x] Foundry
- [x] Foundryup

### What version of Foundry are you on?

forge Version: 1.0.0-nightly Commit SHA: 9bcfbaa6e6066fb9f87ce07163f5479d4b124563 Build Timestamp: 2025-02-26T00:21:24.429838503Z (1740529284) Build Profile: maxperf

### What version of Foundryup are you on?

_No response_

### What command(s) is the bug in?

forge test

### Operating System

Linux

### Describe the bug

If a failed invariant test is cached, and then you change the target configuration (e.g., targetSenders, targetContracts, etc.), when you execute the tests again, this failing test will be reported as failed even if it should not be according to the new targets configuration.

### Code snippet
```solidity
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";

contract Ownable {
    address public owner;
    event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);

    constructor(address _owner) {
        owner = _owner;
    }

    modifier onlyOwner() {
        require(msg.sender == owner, "Ownable: caller is not the owner");
        _;
    }

    function changeOwner(address _owner) external onlyOwner {
        owner = _owner;
        emit OwnershipTransferred(msg.sender, _owner);
    }
}

contract OwnableTest is Test {
    Ownable public ownable;
    address initial_owner = makeAddr("initial_owner");
    address never_owner = makeAddr("never_owner");

    function setUp() public {
        ownable = new Ownable(initial_owner);
        // excludeSender(initial_owner);
    }

    function invariant_never_owner() public {
        assertNotEq(ownable.owner(), never_owner);
    }
}

```

### Reproduction
* init a new project and copy the given snippet.
* run `forge test --mc OwnableTest`, the test should fail and create a new cached failing invariant sequence in the cache folder.
![Image](https://github.com/user-attachments/assets/8cbde9a7-948e-4990-988b-877f36825e35)
* uncomment the `excludeSender(initial_owner);` and execute the test again.
* Now, the test should not fail, as it is not possible with the current configuration, however, the failing sequence is still reported as a test failure. You need to clear the cache and run the test again to pass.



Discussion / hints:
thank you, makes sense. Thats not straightforward to implement since we only persist the failed call sequence in the file but we can remove the file if test recompiles or keep a hash of the test file and compare when rerunning

Fix patch:
diff --git a/crates/config/src/invariant.rs b/crates/config/src/invariant.rs
index 3fb3b8f7e1d4..0c5dbc567d36 100644
--- a/crates/config/src/invariant.rs
+++ b/crates/config/src/invariant.rs
@@ -73,12 +73,4 @@ impl InvariantConfig {
             show_solidity: false,
         }
     }
-
-    /// Returns path to failure dir of given invariant test contract.
-    pub fn failure_dir(self, contract_name: &str) -> PathBuf {
-        self.failure_persist_dir
-            .unwrap()
-            .join("failures")
-            .join(contract_name.split(':').next_back().unwrap())
-    }
 }
diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs
index 893c74baf449..70466c76be7b 100644
--- a/crates/forge/src/runner.rs
+++ b/crates/forge/src/runner.rs
@@ -9,10 +9,11 @@ use crate::{
 };
 use alloy_dyn_abi::DynSolValue;
 use alloy_json_abi::Function;
-use alloy_primitives::{address, map::HashMap, Address, U256};
+use alloy_primitives::{address, map::HashMap, Address, Bytes, U256};
 use eyre::Result;
 use foundry_common::{contracts::ContractsByAddress, TestFunctionExt, TestFunctionKind};
-use foundry_config::Config;
+use foundry_compilers::utils::canonicalized;
+use foundry_config::{Config, InvariantConfig};
 use foundry_evm::{
     constants::CALLER,
     decode::RevertDecoder,
@@ -34,7 +35,15 @@ use proptest::test_runner::{
     FailurePersistence, FileFailurePersistence, RngAlgorithm, TestError, TestRng, TestRunner,
 };
 use rayon::prelude::*;
-use std::{borrow::Cow, cmp::min, collections::BTreeMap, sync::Arc, time::Instant};
+use serde::{Deserialize, Serialize};
+use std::{
+    borrow::Cow,
+    cmp::min,
+    collections::BTreeMap,
+    path::{Path, PathBuf},
+    sync::Arc,
+    time::Instant,
+};
 use tracing::Span;
 
 /// When running tests, we deploy all external libraries present in the project. To avoid additional
@@ -504,7 +513,13 @@ impl<'a> FunctionRunner<'a> {
             TestFunctionKind::UnitTest { .. } => self.run_unit_test(func),
             TestFunctionKind::FuzzTest { .. } => self.run_fuzz_test(func),
             TestFunctionKind::InvariantTest => {
-                self.run_invariant_test(func, call_after_invariant, identified_contracts.unwrap())
+                let test_bytecode = &self.cr.contract.bytecode;
+                self.run_invariant_test(
+                    func,
+                    call_after_invariant,
+                    identified_contracts.unwrap(),
+                    test_bytecode,
+                )
             }
             _ => unreachable!(),
         }
@@ -556,6 +571,7 @@ impl<'a> FunctionRunner<'a> {
         func: &Function,
         call_after_invariant: bool,
         identified_contracts: &ContractsByAddress,
+        test_bytecode: &Bytes,
     ) -> TestResult {
         // First, run the test normally to see if it needs to be skipped.
         if let Err(EvmError::Skip(reason)) = self.executor.call(
@@ -587,13 +603,16 @@ impl<'a> FunctionRunner<'a> {
             abi: &self.cr.contract.abi,
         };
 
-        let failure_dir = invariant_config.clone().failure_dir(self.cr.name);
-        let failure_file = failure_dir.join(&invariant_contract.invariant_function.name);
-        let show_solidity = invariant_config.clone().show_solidity;
+        let (failure_dir, failure_file) = invariant_failure_paths(
+            invariant_config,
+            self.cr.name,
+            &invariant_contract.invariant_function.name,
+        );
+        let show_solidity = invariant_config.show_solidity;
 
         // Try to replay recorded failure if any.
-        if let Ok(mut call_sequence) =
-            foundry_common::fs::read_json_file::<Vec<BaseCounterExample>>(failure_file.as_path())
+        if let Some(mut call_sequence) =
+            persisted_call_sequence(failure_file.as_path(), test_bytecode)
         {
             // Create calls from failed sequence and check if invariant still broken.
             let txes = call_sequence
@@ -696,7 +715,10 @@ impl<'a> FunctionRunner<'a> {
                                     error!(%err, "Failed to create invariant failure dir");
                                 } else if let Err(err) = foundry_common::fs::write_json_file(
                                     failure_file.as_path(),
-                                    &call_sequence,
+                                    &InvariantPersistedFailure {
+                                        call_sequence: call_sequence.clone(),
+                                        driver_bytecode: Some(test_bytecode.clone()),
+                                    },
                                 ) {
                                     error!(%err, "Failed to record call sequence");
                                 }
@@ -894,3 +916,50 @@ fn fuzzer_with_cases(
         TestRunner::new(config)
     }
 }
+
+/// Holds data about a persisted invariant failure.
+#[derive(Serialize, Deserialize)]
+struct InvariantPersistedFailure {
+    /// Recorded counterexample.
+    call_sequence: Vec<BaseCounterExample>,
+    /// Bytecode of the test contract that generated the counterexample.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    driver_bytecode: Option<Bytes>,
+}
+
+/// Helper function to load failed call sequence from file.
+/// Ignores failure if generated with different test contract than the current one.
+fn persisted_call_sequence(path: &Path, bytecode: &Bytes) -> Option<Vec<BaseCounterExample>> {
+    foundry_common::fs::read_json_file::<InvariantPersistedFailure>(path).ok().and_then(
+        |persisted_failure| {
+            if let Some(persisted_bytecode) = &persisted_failure.driver_bytecode {
+                // Ignore persisted sequence if test bytecode doesn't match.
+                if !bytecode.eq(persisted_bytecode) {
+                    let _= sh_warn!("\
+                            Failure from {:?} file was ignored because test contract bytecode has changed.",
+                        path
+                    );
+                    return None;
+                }
+            };
+            Some(persisted_failure.call_sequence)
+        },
+    )
+}
+
+/// Helper functions to return canonicalized invariant failure paths.
+fn invariant_failure_paths(
+    config: &InvariantConfig,
+    contract_name: &str,
+    invariant_name: &str,
+) -> (PathBuf, PathBuf) {
+    let dir = config
+        .failure_persist_dir
+        .clone()
+        .unwrap()
+        .join("failures")
+        .join(contract_name.split(':').next_back().unwrap());
+    let dir = canonicalized(dir);
+    let file = canonicalized(dir.join(invariant_name));
+    (dir, file)
+}


Test patch:
diff --git a/crates/forge/tests/it/invariant.rs b/crates/forge/tests/it/invariant.rs
index a88ed3db21c4..c5705eb3b3d6 100644
--- a/crates/forge/tests/it/invariant.rs
+++ b/crates/forge/tests/it/invariant.rs
@@ -1173,3 +1173,97 @@ Encountered a total of 1 failing tests, 0 tests succeeded
 "#]],
     );
 });
+
+// Tests that persisted failure is discarded if test contract was modified.
+// <https://github.com/foundry-rs/foundry/issues/9965>
+forgetest_init!(invariant_replay_with_different_bytecode, |prj, cmd| {
+    prj.update_config(|config| {
+        config.invariant.runs = 5;
+        config.invariant.depth = 5;
+    });
+    prj.add_source(
+        "Ownable.sol",
+        r#"
+contract Ownable {
+    address public owner = address(777);
+
+    function backdoor(address _owner) external {
+        owner = address(888);
+    }
+
+    function changeOwner(address _owner) external {
+    }
+}
+   "#,
+    )
+    .unwrap();
+    prj.add_test(
+        "OwnableTest.t.sol",
+        r#"
+import {Test} from "forge-std/Test.sol";
+import "src/Ownable.sol";
+
+contract OwnableTest is Test {
+    Ownable ownable;
+
+    function setUp() public {
+        ownable = new Ownable();
+    }
+
+    function invariant_never_owner() public {
+        require(ownable.owner() != address(888), "never owner");
+    }
+}
+   "#,
+    )
+    .unwrap();
+
+    cmd.args(["test", "--mt", "invariant_never_owner"]).assert_failure().stdout_eq(str![[r#"
+...
+[FAIL: revert: never owner]
+...
+"#]]);
+
+    // Should replay failure if same test.
+    cmd.assert_failure().stdout_eq(str![[r#"
+...
+[FAIL: invariant_never_owner replay failure]
+...
+"#]]);
+
+    // Different test driver that should not fail the invariant.
+    prj.add_test(
+        "OwnableTest.t.sol",
+        r#"
+import {Test} from "forge-std/Test.sol";
+import "src/Ownable.sol";
+
+contract OwnableTest is Test {
+    Ownable ownable;
+
+    function setUp() public {
+        ownable = new Ownable();
+        // Ignore selector that fails invariant.
+        bytes4[] memory selectors = new bytes4[](1);
+        selectors[0] = Ownable.changeOwner.selector;
+        targetSelector(FuzzSelector({addr: address(ownable), selectors: selectors}));
+    }
+
+    function invariant_never_owner() public {
+        require(ownable.owner() != address(888), "never owner");
+    }
+}
+   "#,
+    )
+    .unwrap();
+    cmd.assert_success().stderr_eq(str![[r#"
+...
+Warning: Failure from "[..]/invariant/failures/OwnableTest/invariant_never_owner" file was ignored because test contract bytecode has changed.
+...
+"#]])
+    .stdout_eq(str![[r#"
+...
+[PASS] invariant_never_owner() (runs: 5, calls: 25, reverts: 0)
+...
+"#]]);
+});