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: do not flood dictionary with data dependent on fuzz inputs #7552

Merged
merged 3 commits into from
Apr 4, 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
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
Loading