Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 26 additions & 24 deletions crates/evm/evm/src/executors/invariant/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -248,17 +248,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());

Expand Down Expand Up @@ -308,7 +311,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);
}
Expand Down Expand Up @@ -352,19 +355,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);

Expand Down Expand Up @@ -499,7 +493,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 =
Expand Down Expand Up @@ -531,7 +525,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
Expand Down Expand Up @@ -707,7 +709,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)
});

Expand Down
18 changes: 17 additions & 1 deletion crates/evm/fuzz/src/invariant/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
6 changes: 3 additions & 3 deletions crates/evm/fuzz/src/strategies/invariants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>()
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions crates/evm/fuzz/src/strategies/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
35 changes: 35 additions & 0 deletions crates/forge/tests/it/invariant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,14 @@ async fn test_invariant() {
"default/fuzz/invariant/common/InvariantAssume.t.sol:InvariantAssume",
vec![("invariant_dummy()", 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)],
),
]),
);
}
Expand Down Expand Up @@ -411,3 +419,30 @@ async fn test_invariant_assume_respects_restrictions() {
)]),
);
}

#[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,
)],
),
]),
);
}
Original file line number Diff line number Diff line change
@@ -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 {}
}
Loading