Skip to content

Commit

Permalink
fix(fuzz): exclude exernal libraries addresses from fuzz inputs (#9527)
Browse files Browse the repository at this point in the history
  • Loading branch information
grandizzy authored Dec 10, 2024
1 parent 09894ef commit 59f354c
Show file tree
Hide file tree
Showing 9 changed files with 98 additions and 13 deletions.
14 changes: 10 additions & 4 deletions crates/evm/evm/src/executors/fuzz/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,20 @@ 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,
progress: Option<&ProgressBar>,
) -> 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),
Expand Down Expand Up @@ -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,
)
}
}
}
11 changes: 8 additions & 3 deletions crates/evm/evm/src/executors/invariant/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ impl<'a> InvariantExecutor<'a> {
&mut self,
invariant_contract: InvariantContract<'_>,
fuzz_fixtures: &FuzzFixtures,
deployed_libs: &[Address],
progress: Option<&ProgressBar>,
) -> Result<InvariantFuzzTestResult> {
// Throw an error to abort test run if the invariant function accepts input params
Expand All @@ -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);
Expand Down Expand Up @@ -506,15 +507,19 @@ impl<'a> InvariantExecutor<'a> {
&mut self,
invariant_contract: &InvariantContract<'_>,
fuzz_fixtures: &FuzzFixtures,
deployed_libs: &[Address],
) -> Result<(InvariantTest, impl Strategy<Value = BasicTxDetails>)> {
// Finds out the chosen deployed contracts and/or senders.
self.select_contract_artifacts(invariant_contract.address)?;
let (targeted_senders, targeted_contracts) =
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(
Expand Down
16 changes: 14 additions & 2 deletions crates/evm/fuzz/src/strategies/param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://github.com/foundry-rs/foundry/issues/8639>.
if !deployed_libs.contains(&fuzzed_addr) {
Some(DynSolValue::Address(fuzzed_addr))
} else {
None
}
})
.boxed()
}
DynSolType::Function => value()
.prop_map(move |value| {
Expand Down Expand Up @@ -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),
Expand Down
10 changes: 8 additions & 2 deletions crates/evm/fuzz/src/strategies/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,24 @@ const PUSH_BYTE_ANALYSIS_LIMIT: usize = 24 * 1024;
#[derive(Clone, Debug)]
pub struct EvmFuzzState {
inner: Arc<RwLock<FuzzDictionary>>,
/// Addresses of external libraries deployed in test setup, excluded from fuzz test inputs.
pub deployed_libs: Vec<Address>,
}

impl EvmFuzzState {
pub fn new<DB: DatabaseRef>(db: &CacheDB<DB>, config: FuzzDictionaryConfig) -> Self {
pub fn new<DB: DatabaseRef>(
db: &CacheDB<DB>,
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::<Vec<_>>();
accs.sort_by_key(|(address, _)| *address);

// 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<Item = B256>) {
Expand Down
2 changes: 2 additions & 0 deletions crates/forge/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,8 @@ pub struct TestSetup {
pub traces: Traces,
/// Coverage info during setup.
pub coverage: Option<HitMaps>,
/// Addresses of external libraries deployed during setup.
pub deployed_libs: Vec<Address>,

/// The reason the setup failed, if it did.
pub reason: Option<String>,
Expand Down
8 changes: 8 additions & 0 deletions crates/forge/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions crates/forge/tests/it/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
3 changes: 3 additions & 0 deletions crates/forge/tests/it/repros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
43 changes: 43 additions & 0 deletions testdata/default/repros/Issue8639.t.sol
Original file line number Diff line number Diff line change
@@ -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));
}
}

0 comments on commit 59f354c

Please sign in to comment.