From 59f354c179f4e7f6d7292acb3d068815c79286d1 Mon Sep 17 00:00:00 2001 From: grandizzy <38490174+grandizzy@users.noreply.github.com> Date: Tue, 10 Dec 2024 18:08:40 +0200 Subject: [PATCH] fix(fuzz): exclude exernal libraries addresses from fuzz inputs (#9527) --- crates/evm/evm/src/executors/fuzz/mod.rs | 14 ++++-- crates/evm/evm/src/executors/invariant/mod.rs | 11 +++-- crates/evm/fuzz/src/strategies/param.rs | 16 ++++++- crates/evm/fuzz/src/strategies/state.rs | 10 ++++- crates/forge/src/result.rs | 2 + crates/forge/src/runner.rs | 8 ++++ crates/forge/tests/it/core.rs | 4 +- crates/forge/tests/it/repros.rs | 3 ++ testdata/default/repros/Issue8639.t.sol | 43 +++++++++++++++++++ 9 files changed, 98 insertions(+), 13 deletions(-) create mode 100644 testdata/default/repros/Issue8639.t.sol diff --git a/crates/evm/evm/src/executors/fuzz/mod.rs b/crates/evm/evm/src/executors/fuzz/mod.rs index 0d79f8fa4961..b99499cf3c5b 100644 --- a/crates/evm/evm/src/executors/fuzz/mod.rs +++ b/crates/evm/evm/src/executors/fuzz/mod.rs @@ -77,10 +77,12 @@ impl FuzzedExecutor { /// test case. /// /// Returns a list of all the consumed gas and calldata of every fuzz case + #[allow(clippy::too_many_arguments)] pub fn fuzz( &self, func: &Function, fuzz_fixtures: &FuzzFixtures, + deployed_libs: &[Address], address: Address, should_fail: bool, rd: &RevertDecoder, @@ -88,7 +90,7 @@ impl FuzzedExecutor { ) -> FuzzTestResult { // Stores the fuzz test execution data. let execution_data = RefCell::new(FuzzTestData::default()); - let state = self.build_fuzz_state(); + let state = self.build_fuzz_state(deployed_libs); let dictionary_weight = self.config.dictionary.dictionary_weight.min(100); let strategy = proptest::prop_oneof![ 100 - dictionary_weight => fuzz_calldata(func.clone(), fuzz_fixtures), @@ -274,11 +276,15 @@ impl FuzzedExecutor { } /// Stores fuzz state for use with [fuzz_calldata_from_state] - pub fn build_fuzz_state(&self) -> EvmFuzzState { + pub fn build_fuzz_state(&self, deployed_libs: &[Address]) -> EvmFuzzState { if let Some(fork_db) = self.executor.backend().active_fork_db() { - EvmFuzzState::new(fork_db, self.config.dictionary) + EvmFuzzState::new(fork_db, self.config.dictionary, deployed_libs) } else { - EvmFuzzState::new(self.executor.backend().mem_db(), self.config.dictionary) + EvmFuzzState::new( + self.executor.backend().mem_db(), + self.config.dictionary, + deployed_libs, + ) } } } diff --git a/crates/evm/evm/src/executors/invariant/mod.rs b/crates/evm/evm/src/executors/invariant/mod.rs index d5fdb5668f5e..4582e46822a8 100644 --- a/crates/evm/evm/src/executors/invariant/mod.rs +++ b/crates/evm/evm/src/executors/invariant/mod.rs @@ -323,6 +323,7 @@ impl<'a> InvariantExecutor<'a> { &mut self, invariant_contract: InvariantContract<'_>, fuzz_fixtures: &FuzzFixtures, + deployed_libs: &[Address], progress: Option<&ProgressBar>, ) -> Result { // Throw an error to abort test run if the invariant function accepts input params @@ -331,7 +332,7 @@ impl<'a> InvariantExecutor<'a> { } let (invariant_test, invariant_strategy) = - self.prepare_test(&invariant_contract, fuzz_fixtures)?; + self.prepare_test(&invariant_contract, fuzz_fixtures, deployed_libs)?; // Start timer for this invariant test. let timer = FuzzTestTimer::new(self.config.timeout); @@ -506,6 +507,7 @@ impl<'a> InvariantExecutor<'a> { &mut self, invariant_contract: &InvariantContract<'_>, fuzz_fixtures: &FuzzFixtures, + deployed_libs: &[Address], ) -> Result<(InvariantTest, impl Strategy)> { // Finds out the chosen deployed contracts and/or senders. self.select_contract_artifacts(invariant_contract.address)?; @@ -513,8 +515,11 @@ impl<'a> InvariantExecutor<'a> { self.select_contracts_and_senders(invariant_contract.address)?; // Stores fuzz state for use with [fuzz_calldata_from_state]. - let fuzz_state = - EvmFuzzState::new(self.executor.backend().mem_db(), self.config.dictionary); + let fuzz_state = EvmFuzzState::new( + self.executor.backend().mem_db(), + self.config.dictionary, + deployed_libs, + ); // Creates the invariant strategy. let strategy = invariant_strat( diff --git a/crates/evm/fuzz/src/strategies/param.rs b/crates/evm/fuzz/src/strategies/param.rs index 7e5218fd872b..643c70bde3d4 100644 --- a/crates/evm/fuzz/src/strategies/param.rs +++ b/crates/evm/fuzz/src/strategies/param.rs @@ -130,7 +130,19 @@ pub fn fuzz_param_from_state( // Convert the value based on the parameter type match *param { DynSolType::Address => { - value().prop_map(move |value| DynSolValue::Address(Address::from_word(value))).boxed() + let deployed_libs = state.deployed_libs.clone(); + value() + .prop_filter_map("filter address fuzzed from state", move |value| { + let fuzzed_addr = Address::from_word(value); + // Do not use addresses of deployed libraries as fuzz input. + // See . + if !deployed_libs.contains(&fuzzed_addr) { + Some(DynSolValue::Address(fuzzed_addr)) + } else { + None + } + }) + .boxed() } DynSolType::Function => value() .prop_map(move |value| { @@ -217,7 +229,7 @@ mod tests { let f = "testArray(uint64[2] calldata values)"; let func = get_func(f).unwrap(); let db = CacheDB::new(EmptyDB::default()); - let state = EvmFuzzState::new(&db, FuzzDictionaryConfig::default()); + let state = EvmFuzzState::new(&db, FuzzDictionaryConfig::default(), &[]); let strategy = proptest::prop_oneof![ 60 => fuzz_calldata(func.clone(), &FuzzFixtures::default()), 40 => fuzz_calldata_from_state(func, &state), diff --git a/crates/evm/fuzz/src/strategies/state.rs b/crates/evm/fuzz/src/strategies/state.rs index 4df55772c436..b1c5d8e00424 100644 --- a/crates/evm/fuzz/src/strategies/state.rs +++ b/crates/evm/fuzz/src/strategies/state.rs @@ -27,10 +27,16 @@ const PUSH_BYTE_ANALYSIS_LIMIT: usize = 24 * 1024; #[derive(Clone, Debug)] pub struct EvmFuzzState { inner: Arc>, + /// Addresses of external libraries deployed in test setup, excluded from fuzz test inputs. + pub deployed_libs: Vec
, } impl EvmFuzzState { - pub fn new(db: &CacheDB, config: FuzzDictionaryConfig) -> Self { + pub fn new( + db: &CacheDB, + config: FuzzDictionaryConfig, + deployed_libs: &[Address], + ) -> Self { // Sort accounts to ensure deterministic dictionary generation from the same setUp state. let mut accs = db.accounts.iter().collect::>(); accs.sort_by_key(|(address, _)| *address); @@ -38,7 +44,7 @@ impl EvmFuzzState { // Create fuzz dictionary and insert values from db state. let mut dictionary = FuzzDictionary::new(config); dictionary.insert_db_values(accs); - Self { inner: Arc::new(RwLock::new(dictionary)) } + Self { inner: Arc::new(RwLock::new(dictionary)), deployed_libs: deployed_libs.to_vec() } } pub fn collect_values(&self, values: impl IntoIterator) { diff --git a/crates/forge/src/result.rs b/crates/forge/src/result.rs index eed3f2977e90..5b134194fe97 100644 --- a/crates/forge/src/result.rs +++ b/crates/forge/src/result.rs @@ -752,6 +752,8 @@ pub struct TestSetup { pub traces: Traces, /// Coverage info during setup. pub coverage: Option, + /// Addresses of external libraries deployed during setup. + pub deployed_libs: Vec
, /// The reason the setup failed, if it did. pub reason: Option, diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index ef769c6625e3..7b1293a72824 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -130,6 +130,12 @@ impl<'a> ContractRunner<'a> { U256::ZERO, Some(&self.mcr.revert_decoder), ); + + // Record deployed library address. + if let Ok(deployed) = &deploy_result { + result.deployed_libs.push(deployed.address); + } + let (raw, reason) = RawCallResult::from_evm_result(deploy_result.map(Into::into))?; result.extend(raw, TraceKind::Deployment); if reason.is_some() { @@ -614,6 +620,7 @@ impl<'a> FunctionRunner<'a> { let invariant_result = match evm.invariant_fuzz( invariant_contract.clone(), &self.setup.fuzz_fixtures, + &self.setup.deployed_libs, progress.as_ref(), ) { Ok(x) => x, @@ -728,6 +735,7 @@ impl<'a> FunctionRunner<'a> { let result = fuzzed_executor.fuzz( func, &self.setup.fuzz_fixtures, + &self.setup.deployed_libs, self.address, should_fail, &self.cr.mcr.revert_decoder, diff --git a/crates/forge/tests/it/core.rs b/crates/forge/tests/it/core.rs index a2b4916d3e7c..94dc945e5938 100644 --- a/crates/forge/tests/it/core.rs +++ b/crates/forge/tests/it/core.rs @@ -742,8 +742,8 @@ async fn test_trace() { assert_eq!( deployment_traces.count(), - 12, - "Test {test_name} did not have exactly 12 deployment trace." + 13, + "Test {test_name} did not have exactly 13 deployment trace." ); assert!(setup_traces.count() <= 1, "Test {test_name} had more than 1 setup trace."); assert_eq!( diff --git a/crates/forge/tests/it/repros.rs b/crates/forge/tests/it/repros.rs index 96708bdf5548..2a47d3d3efd1 100644 --- a/crates/forge/tests/it/repros.rs +++ b/crates/forge/tests/it/repros.rs @@ -386,3 +386,6 @@ test_repro!(8971; |config| { prj_config.isolate = true; config.runner.config = Arc::new(prj_config); }); + +// https://github.com/foundry-rs/foundry/issues/8639 +test_repro!(8639); diff --git a/testdata/default/repros/Issue8639.t.sol b/testdata/default/repros/Issue8639.t.sol new file mode 100644 index 000000000000..6f0a7b526336 --- /dev/null +++ b/testdata/default/repros/Issue8639.t.sol @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 +pragma solidity ^0.8.18; + +import "ds-test/test.sol"; + +library ExternalLibrary { + function doWork(uint256 a) public returns (uint256) { + return a++; + } +} + +contract Counter { + uint256 public number; + + function setNumber(uint256 newNumber) public { + ExternalLibrary.doWork(1); + } + + function increment() public {} +} + +// https://github.com/foundry-rs/foundry/issues/8639 +contract Issue8639Test is DSTest { + Counter counter; + + function setUp() public { + counter = new Counter(); + } + + /// forge-config: default.fuzz.runs = 1000 + /// forge-config: default.fuzz.seed = '100' + function test_external_library_address(address test) public { + require(test != address(ExternalLibrary)); + } +} + +contract Issue8639AnotherTest is DSTest { + /// forge-config: default.fuzz.runs = 1000 + /// forge-config: default.fuzz.seed = '100' + function test_another_external_library_address(address test) public { + require(test != address(ExternalLibrary)); + } +}