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(fuzz): exclude exernal libraries addresses from fuzz inputs #9527

Merged
merged 1 commit into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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 @@ -127,6 +127,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 @@ -611,6 +617,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 @@ -725,6 +732,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,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new deployment trace added by the external library test

"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));
}
}
Loading