Skip to content

Commit

Permalink
fix: do not flood dictionary with data dependent on fuzz inputs (#7552)
Browse files Browse the repository at this point in the history
* fix dictionary

* clippy + fmt

* fix
  • Loading branch information
klkvr committed Apr 6, 2024
1 parent 5efa20e commit 398ff1f
Show file tree
Hide file tree
Showing 8 changed files with 170 additions and 129 deletions.
15 changes: 4 additions & 11 deletions crates/evm/evm/src/executors/fuzz/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ use foundry_evm_core::{
};
use foundry_evm_coverage::HitMaps;
use foundry_evm_fuzz::{
strategies::{
build_initial_state, collect_state_from_call, fuzz_calldata, fuzz_calldata_from_state,
EvmFuzzState,
},
strategies::{build_initial_state, fuzz_calldata, fuzz_calldata_from_state, EvmFuzzState},
BaseCounterExample, CounterExample, FuzzCase, FuzzError, FuzzTestResult,
};
use foundry_evm_traces::CallTraceArena;
Expand Down Expand Up @@ -89,7 +86,7 @@ impl FuzzedExecutor {
];
debug!(func=?func.name, should_fail, "fuzzing");
let run_result = self.runner.clone().run(&strat, |calldata| {
let fuzz_res = self.single_fuzz(&state, address, should_fail, calldata)?;
let fuzz_res = self.single_fuzz(address, should_fail, calldata)?;

match fuzz_res {
FuzzOutcome::Case(case) => {
Expand Down Expand Up @@ -195,7 +192,6 @@ impl FuzzedExecutor {
/// or a `CounterExampleOutcome`
pub fn single_fuzz(
&self,
state: &EvmFuzzState,
address: Address,
should_fail: bool,
calldata: alloy_primitives::Bytes,
Expand All @@ -206,9 +202,6 @@ impl FuzzedExecutor {
.map_err(|_| TestCaseError::fail(FuzzError::FailedContractCall))?;
let state_changeset = call.state_changeset.take().unwrap();

// Build fuzzer state
collect_state_from_call(&call.logs, &state_changeset, state, &self.config.dictionary);

// When the `assume` cheatcode is called it returns a special string
if call.result.as_ref() == MAGIC_ASSUME {
return Err(TestCaseError::reject(FuzzError::AssumeReject))
Expand Down Expand Up @@ -247,9 +240,9 @@ impl FuzzedExecutor {
/// Stores fuzz state for use with [fuzz_calldata_from_state]
pub fn build_fuzz_state(&self) -> EvmFuzzState {
if let Some(fork_db) = self.executor.backend.active_fork_db() {
build_initial_state(fork_db, &self.config.dictionary)
build_initial_state(fork_db, self.config.dictionary)
} else {
build_initial_state(self.executor.backend.mem_db(), &self.config.dictionary)
build_initial_state(self.executor.backend.mem_db(), self.config.dictionary)
}
}
}
24 changes: 10 additions & 14 deletions crates/evm/evm/src/executors/invariant/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use alloy_primitives::{Address, FixedBytes, U256};
use alloy_sol_types::{sol, SolCall};
use eyre::{eyre, ContextCompat, Result};
use foundry_common::contracts::{ContractsByAddress, ContractsByArtifact};
use foundry_config::{FuzzDictionaryConfig, InvariantConfig};
use foundry_config::InvariantConfig;
use foundry_evm_core::{
constants::{CALLER, CHEATCODE_ADDRESS, HARDHAT_CONSOLE_ADDRESS, MAGIC_ASSUME},
utils::{get_function, StateChangeset},
Expand All @@ -17,8 +17,8 @@ use foundry_evm_fuzz::{
RandomCallGenerator, SenderFilters, TargetedContracts,
},
strategies::{
build_initial_state, collect_created_contracts, collect_state_from_call, invariant_strat,
override_call_strat, CalldataFuzzDictionary, EvmFuzzState,
build_initial_state, collect_created_contracts, invariant_strat, override_call_strat,
CalldataFuzzDictionary, EvmFuzzState,
},
FuzzCase, FuzzedCases,
};
Expand Down Expand Up @@ -246,13 +246,7 @@ impl<'a> InvariantExecutor<'a> {
let mut state_changeset =
call_result.state_changeset.to_owned().expect("no changesets");

collect_data(
&mut state_changeset,
sender,
&call_result,
&fuzz_state,
&self.config.dictionary,
);
collect_data(&mut state_changeset, sender, &call_result, &fuzz_state);

if let Err(error) = collect_created_contracts(
&state_changeset,
Expand Down Expand Up @@ -325,11 +319,14 @@ impl<'a> InvariantExecutor<'a> {
}
fuzz_cases.borrow_mut().push(FuzzedCases::new(fuzz_runs));

// Revert state to not persist values between runs.
fuzz_state.revert();

Ok(())
});

trace!(target: "forge::test::invariant::calldata_address_fuzz_dictionary", "{:?}", calldata_fuzz_dictionary.inner.addresses);
trace!(target: "forge::test::invariant::dictionary", "{:?}", fuzz_state.read().values().iter().map(hex::encode).collect::<Vec<_>>());
trace!(target: "forge::test::invariant::dictionary", "{:?}", fuzz_state.dictionary_read().values().iter().map(hex::encode).collect::<Vec<_>>());

let (reverts, error) = failures.into_inner().into_inner();

Expand Down Expand Up @@ -361,7 +358,7 @@ impl<'a> InvariantExecutor<'a> {

// 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);
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.
Expand Down Expand Up @@ -668,7 +665,6 @@ fn collect_data(
sender: &Address,
call_result: &RawCallResult,
fuzz_state: &EvmFuzzState,
config: &FuzzDictionaryConfig,
) {
// Verify it has no code.
let mut has_code = false;
Expand All @@ -683,7 +679,7 @@ fn collect_data(
sender_changeset = state_changeset.remove(sender);
}

collect_state_from_call(&call_result.logs, &*state_changeset, fuzz_state, config);
fuzz_state.collect_state_from_call(&call_result.logs, &*state_changeset);

// Re-add changes
if let Some(changed) = sender_changeset {
Expand Down
7 changes: 2 additions & 5 deletions crates/evm/fuzz/src/inspector.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{invariant::RandomCallGenerator, strategies::EvmFuzzState};
use alloy_primitives::U256;
use revm::{
interpreter::{CallInputs, CallOutcome, CallScheme, Interpreter},
Database, EvmContext, Inspector,
Expand Down Expand Up @@ -61,11 +62,7 @@ impl<DB: Database> Inspector<DB> for Fuzzer {
impl Fuzzer {
/// Collects `stack` and `memory` values into the fuzz dictionary.
fn collect_data(&mut self, interpreter: &Interpreter) {
let mut state = self.fuzz_state.write();

for slot in interpreter.stack().data() {
state.values_mut().insert(slot.to_be_bytes());
}
self.fuzz_state.collect_values(interpreter.stack().data().iter().map(U256::to_be_bytes));

// TODO: disabled for now since it's flooding the dictionary
// for index in 0..interpreter.shared_memory.len() / 32 {
Expand Down
2 changes: 1 addition & 1 deletion crates/evm/fuzz/src/strategies/calldata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl CalldataFuzzDictionaryConfig {
if dict_size > 0 {
addresses.extend(std::iter::repeat_with(Address::random).take(dict_size));
// Add all addresses that already had their PUSH bytes collected.
addresses.extend(state.read().addresses());
addresses.extend(state.dictionary_read().addresses());
}

Self { addresses: addresses.into_iter().collect() }
Expand Down
3 changes: 1 addition & 2 deletions crates/evm/fuzz/src/strategies/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ pub use calldata::{

mod state;
pub use state::{
build_initial_state, collect_created_contracts, collect_state_from_call,
fuzz_calldata_from_state, EvmFuzzState,
build_initial_state, collect_created_contracts, fuzz_calldata_from_state, EvmFuzzState,
};

mod invariants;
Expand Down
4 changes: 2 additions & 2 deletions crates/evm/fuzz/src/strategies/param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub fn fuzz_param_from_state(
let state = state.clone();
// Use `Index` instead of `Selector` to not iterate over the entire dictionary.
any::<prop::sample::Index>().prop_map(move |index| {
let state = state.read();
let state = state.dictionary_read();
let values = state.values();
let index = index.index(values.len());
*values.iter().nth(index).unwrap()
Expand Down Expand Up @@ -184,7 +184,7 @@ mod tests {
let f = "testArray(uint64[2] calldata values)";
let func = get_func(f).unwrap();
let db = CacheDB::new(EmptyDB::default());
let state = build_initial_state(&db, &FuzzDictionaryConfig::default());
let state = build_initial_state(&db, FuzzDictionaryConfig::default());
let strat = proptest::prop_oneof![
60 => fuzz_calldata(func.clone()),
40 => fuzz_calldata_from_state(func, &state),
Expand Down
Loading

0 comments on commit 398ff1f

Please sign in to comment.