fix(invariant): honor targetContract setting, don't update targets if any (#7595)

* fix(invariant): respect targetContract setup

* Fix test fmt

* Check identified contracts after collecting `targetInterfaces`

Author: grandizzy

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

@@ -118,7 +118,7 @@ impl FailedInvariantCaseData {
         };
 
         // Collect abis of fuzzed and invariant contracts to decode custom error.
-        let targets = targeted_contracts.lock();
+        let targets = targeted_contracts.targets.lock();
         let abis = targets
             .iter()
             .map(|contract| &contract.1 .1)

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

@@ -23,7 +23,7 @@ use foundry_evm_fuzz::{
     FuzzCase, FuzzedCases,
 };
 use foundry_evm_traces::CallTraceArena;
-use parking_lot::{Mutex, RwLock};
+use parking_lot::RwLock;
 use proptest::{
     strategy::{BoxedStrategy, Strategy, ValueTree},
     test_runner::{TestCaseError, TestRunner},
@@ -249,17 +249,20 @@ impl<'a> InvariantExecutor<'a> {
 
                     collect_data(&mut state_changeset, sender, &call_result, &fuzz_state);
 
-                    if let Err(error) = collect_created_contracts(
-                        &state_changeset,
-                        self.project_contracts,
-                        self.setup_contracts,
-                        &self.artifact_filters,
-                        targeted_contracts.clone(),
-                        &mut created_contracts,
-                    ) {
-                        warn!(target: "forge::test", "{error}");
+                    // Collect created contracts and add to fuzz targets only if targeted contracts
+                    // are updatable.
+                    if targeted_contracts.is_updatable {
+                        if let Err(error) = collect_created_contracts(
+                            &state_changeset,
+                            self.project_contracts,
+                            self.setup_contracts,
+                            &self.artifact_filters,
+                            &targeted_contracts,
+                            &mut created_contracts,
+                        ) {
+                            warn!(target: "forge::test", "{error}");
+                        }
                     }
-
                     // Commit changes to the database.
                     executor.backend.commit(state_changeset.clone());
 
@@ -309,7 +312,7 @@ impl<'a> InvariantExecutor<'a> {
 
             // We clear all the targeted contracts created during this run.
             if !created_contracts.is_empty() {
-                let mut writable_targeted = targeted_contracts.lock();
+                let mut writable_targeted = targeted_contracts.targets.lock();
                 for addr in created_contracts.iter() {
                     writable_targeted.remove(addr);
                 }
@@ -353,19 +356,10 @@ impl<'a> InvariantExecutor<'a> {
         let (targeted_senders, targeted_contracts) =
             self.select_contracts_and_senders(invariant_contract.address)?;
 
-        if targeted_contracts.is_empty() {
-            eyre::bail!("No contracts to fuzz.");
-        }
-
         // Stores fuzz state for use with [fuzz_calldata_from_state].
         let fuzz_state: EvmFuzzState =
             build_initial_state(self.executor.backend.mem_db(), self.config.dictionary);
 
-        // During execution, any newly created contract is added here and used through the rest of
-        // the fuzz run.
-        let targeted_contracts: FuzzRunIdentifiedContracts =
-            Arc::new(Mutex::new(targeted_contracts));
-
         let calldata_fuzz_config =
             CalldataFuzzDictionary::new(&self.config.dictionary, &fuzz_state);
 
@@ -500,7 +494,7 @@ impl<'a> InvariantExecutor<'a> {
     pub fn select_contracts_and_senders(
         &self,
         to: Address,
-    ) -> eyre::Result<(SenderFilters, TargetedContracts)> {
+    ) -> eyre::Result<(SenderFilters, FuzzRunIdentifiedContracts)> {
         let targeted_senders =
             self.call_sol_default(to, &IInvariantTest::targetSendersCall {}).targetedSenders;
         let excluded_senders =
@@ -532,7 +526,15 @@ impl<'a> InvariantExecutor<'a> {
 
         self.select_selectors(to, &mut contracts)?;
 
-        Ok((SenderFilters::new(targeted_senders, excluded_senders), contracts))
+        // There should be at least one contract identified as target for fuzz runs.
+        if contracts.is_empty() {
+            eyre::bail!("No contracts to fuzz.");
+        }
+
+        Ok((
+            SenderFilters::new(targeted_senders, excluded_senders),
+            FuzzRunIdentifiedContracts::new(contracts, selected.is_empty()),
+        ))
     }
 
     /// Extends the contracts and selectors to fuzz with the addresses and ABIs specified in
@@ -708,7 +710,7 @@ fn can_continue(
     let mut call_results = None;
 
     // Detect handler assertion failures first.
-    let handlers_failed = targeted_contracts.lock().iter().any(|contract| {
+    let handlers_failed = targeted_contracts.targets.lock().iter().any(|contract| {
         !executor.is_success(*contract.0, false, Cow::Borrowed(state_changeset), false)
     });
 

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

@@ -10,7 +10,23 @@ mod filters;
 pub use filters::{ArtifactFilters, SenderFilters};
 
 pub type TargetedContracts = BTreeMap<Address, (String, JsonAbi, Vec<Function>)>;
-pub type FuzzRunIdentifiedContracts = Arc<Mutex<TargetedContracts>>;
+
+/// Contracts identified as targets during a fuzz run.
+/// During execution, any newly created contract is added as target and used through the rest of
+/// the fuzz run if the collection is updatable (no `targetContract` specified in `setUp`).
+#[derive(Clone, Debug)]
+pub struct FuzzRunIdentifiedContracts {
+    /// Contracts identified as targets during a fuzz run.
+    pub targets: Arc<Mutex<TargetedContracts>>,
+    /// Whether target contracts are updatable or not.
+    pub is_updatable: bool,
+}
+
+impl FuzzRunIdentifiedContracts {
+    pub fn new(targets: TargetedContracts, is_updatable: bool) -> Self {
+        Self { targets: Arc::new(Mutex::new(targets)), is_updatable }
+    }
+}
 
 /// (Sender, (TargetContract, Calldata))
 pub type BasicTxDetails = (Address, (Address, Bytes));

crates/evm/fuzz/src/strategies/invariants.rs

@@ -16,7 +16,7 @@ pub fn override_call_strat(
     target: Arc<RwLock<Address>>,
     calldata_fuzz_config: CalldataFuzzDictionary,
 ) -> SBoxedStrategy<(Address, Bytes)> {
-    let contracts_ref = contracts.clone();
+    let contracts_ref = contracts.targets.clone();
     proptest::prop_oneof![
         80 => proptest::strategy::LazyJust::new(move || *target.read()),
         20 => any::<prop::sample::Selector>()
@@ -27,7 +27,7 @@ pub fn override_call_strat(
         let calldata_fuzz_config = calldata_fuzz_config.clone();
 
         let func = {
-            let contracts = contracts.lock();
+            let contracts = contracts.targets.lock();
             let (_, abi, functions) = contracts.get(&target_address).unwrap_or_else(|| {
                 // Choose a random contract if target selected by lazy strategy is not in fuzz run
                 // identified contracts. This can happen when contract is created in `setUp` call
@@ -81,7 +81,7 @@ fn generate_call(
     any::<prop::sample::Selector>()
         .prop_flat_map(move |selector| {
             let (contract, func) = {
-                let contracts = contracts.lock();
+                let contracts = contracts.targets.lock();
                 let contracts =
                     contracts.iter().filter(|(_, (_, abi, _))| !abi.functions.is_empty());
                 let (&contract, (_, abi, functions)) = selector.select(contracts);

crates/evm/fuzz/src/strategies/state.rs

@@ -304,10 +304,10 @@ pub fn collect_created_contracts(
     project_contracts: &ContractsByArtifact,
     setup_contracts: &ContractsByAddress,
     artifact_filters: &ArtifactFilters,
-    targeted_contracts: FuzzRunIdentifiedContracts,
+    targeted_contracts: &FuzzRunIdentifiedContracts,
     created_contracts: &mut Vec<Address>,
 ) -> eyre::Result<()> {
-    let mut writable_targeted = targeted_contracts.lock();
+    let mut writable_targeted = targeted_contracts.targets.lock();
     for (address, account) in state_changeset {
         if !setup_contracts.contains_key(address) {
             if let (true, Some(code)) = (&account.is_touched(), &account.info.code) {

crates/forge/tests/it/invariant.rs

@@ -149,6 +149,14 @@ async fn test_invariant() {
                 "default/fuzz/invariant/common/InvariantCustomError.t.sol:InvariantCustomError",
                 vec![("invariant_decode_error()", true, None, None, None)],
             ),
+            (
+                "default/fuzz/invariant/target/FuzzedTargetContracts.t.sol:ExplicitTargetContract",
+                vec![("invariant_explicit_target()", true, None, None, None)],
+            ),
+            (
+                "default/fuzz/invariant/target/FuzzedTargetContracts.t.sol:DynamicTargetContract",
+                vec![("invariant_dynamic_targets()", true, None, None, None)],
+            ),
         ]),
     );
 }
@@ -436,3 +444,30 @@ async fn test_invariant_decode_custom_error() {
         )]),
     );
 }
+
+#[tokio::test(flavor = "multi_thread")]
+async fn test_invariant_fuzzed_selected_targets() {
+    let filter = Filter::new(".*", ".*", ".*fuzz/invariant/target/FuzzedTargetContracts.t.sol");
+    let mut runner = TEST_DATA_DEFAULT.runner();
+    runner.test_options.invariant.fail_on_revert = true;
+    let results = runner.test_collect(&filter);
+    assert_multiple(
+        &results,
+        BTreeMap::from([
+            (
+                "default/fuzz/invariant/target/FuzzedTargetContracts.t.sol:ExplicitTargetContract",
+                vec![("invariant_explicit_target()", true, None, None, None)],
+            ),
+            (
+                "default/fuzz/invariant/target/FuzzedTargetContracts.t.sol:DynamicTargetContract",
+                vec![(
+                    "invariant_dynamic_targets()",
+                    false,
+                    Some("revert: wrong target selector called".into()),
+                    None,
+                    None,
+                )],
+            ),
+        ]),
+    );
+}

testdata/default/fuzz/invariant/target/FuzzedTargetContracts.t.sol

@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: MIT OR Apache-2.0
+pragma solidity 0.8.18;
+
+import "ds-test/test.sol";
+
+interface Vm {
+    function etch(address target, bytes calldata newRuntimeBytecode) external;
+}
+
+// https://github.com/foundry-rs/foundry/issues/5625
+// https://github.com/foundry-rs/foundry/issues/6166
+// `Target.wrongSelector` is not called when handler added as `targetContract`
+// `Target.wrongSelector` is called (and test fails) when no `targetContract` set
+contract Target {
+    uint256 count;
+
+    function wrongSelector() external {
+        revert("wrong target selector called");
+    }
+
+    function goodSelector() external {
+        count++;
+    }
+}
+
+contract Handler is DSTest {
+    function increment() public {
+        Target(0x6B175474E89094C44Da98b954EedeAC495271d0F).goodSelector();
+    }
+}
+
+contract ExplicitTargetContract is DSTest {
+    Vm vm = Vm(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D);
+    Handler handler;
+
+    function setUp() public {
+        Target target = new Target();
+        bytes memory targetCode = address(target).code;
+        vm.etch(address(0x6B175474E89094C44Da98b954EedeAC495271d0F), targetCode);
+
+        handler = new Handler();
+    }
+
+    function targetContracts() public returns (address[] memory) {
+        address[] memory addrs = new address[](1);
+        addrs[0] = address(handler);
+        return addrs;
+    }
+
+    function invariant_explicit_target() public {}
+}
+
+contract DynamicTargetContract is DSTest {
+    Vm vm = Vm(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D);
+    Handler handler;
+
+    function setUp() public {
+        Target target = new Target();
+        bytes memory targetCode = address(target).code;
+        vm.etch(address(0x6B175474E89094C44Da98b954EedeAC495271d0F), targetCode);
+
+        handler = new Handler();
+    }
+
+    function invariant_dynamic_targets() public {}
+}