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(forge): selectively enable Etherscan trace resolving when a test in ran in a forked environment and return block number in trace on a failed test #7606

Open
wants to merge 52 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
68bfdb8
add has_forks to backend
zerosnacks Apr 8, 2024
7251ef4
Merge branch 'master' into zerosnacks/fix-tracing-regression
zerosnacks Apr 8, 2024
5eca07c
pass down environment, add dynamic enabled / disabled flag on a per t…
zerosnacks Apr 9, 2024
004dc11
add clarifying comment
zerosnacks Apr 9, 2024
46ae039
check has_forks at test runtime
zerosnacks Apr 9, 2024
acba4c4
pass down block number
zerosnacks Apr 9, 2024
f3fc621
fix build issues, cast block number as u64
zerosnacks Apr 9, 2024
d63e4fc
flip order
zerosnacks Apr 9, 2024
0ff4a3f
clarify
zerosnacks Apr 9, 2024
c42c3a6
remove unused code
zerosnacks Apr 9, 2024
f9ad7c0
Merge branch 'master' into zerosnacks/fix-tracing-regression
zerosnacks Apr 10, 2024
c5a4690
simplify by making enable_etherscan flip based on parameter `yes`
zerosnacks Apr 10, 2024
15a9bfd
add environment report formatter, add block on failure
zerosnacks Apr 10, 2024
7e4bb99
add block format to test, looks incorrect, especially in the USDTCall…
zerosnacks Apr 10, 2024
c473f01
fix, has_forks was inverted
zerosnacks Apr 10, 2024
7f1f3c2
Merge branch 'master' into zerosnacks/fix-tracing-regression
zerosnacks Apr 15, 2024
efc05d3
to be able to capture forking state inside of calls (as in vm.createS…
zerosnacks Apr 15, 2024
d2ba46b
filter out block number at genesis, assume non-forked environment in …
zerosnacks Apr 15, 2024
5ae23e5
add basic tests cases
zerosnacks Apr 15, 2024
4edb749
simplify has_forks check, we can simply check `issued_local_forks_ids…
zerosnacks Apr 15, 2024
8fb7445
add specific test for verbose tracing and non verbose tracing for bot…
zerosnacks Apr 15, 2024
676b249
revert simplification
zerosnacks Apr 15, 2024
d48c3b3
add note
zerosnacks Apr 15, 2024
b48a587
pull in latest master
zerosnacks Apr 16, 2024
5404c5c
revert repro_6531, clean up comments
zerosnacks Apr 16, 2024
059decc
make sure to output block number in both test results as well as the …
zerosnacks Apr 16, 2024
5799cc2
Merge branch 'master' into zerosnacks/fix-tracing-regression
zerosnacks Apr 22, 2024
07dde75
merge in master
zerosnacks Apr 23, 2024
07fceb5
remove env passing, to implement an alternative, move etherscan toggl…
zerosnacks Apr 23, 2024
bfca63c
add context capturing, collecting all block changes during execution
zerosnacks Apr 23, 2024
85bb139
fix overflow
zerosnacks Apr 23, 2024
39807c9
fix clippy
zerosnacks Apr 23, 2024
56e1006
add test trace for multiblock fork
zerosnacks Apr 23, 2024
52e2ca5
fix title
zerosnacks Apr 23, 2024
28986ac
add context tracking to setup
zerosnacks Apr 23, 2024
ad74f98
add context tracking support for fuzz and invariant counter example c…
zerosnacks Apr 23, 2024
4fcbdce
enhance with context, move definition to core
zerosnacks Apr 23, 2024
a2ba880
re-add environment with context
zerosnacks Apr 23, 2024
dd2592b
fix clippy
zerosnacks Apr 23, 2024
715d8b2
Merge branch 'master' into zerosnacks/fix-tracing-regression
zerosnacks Apr 25, 2024
f6310ff
appears formatting issue could relate to diff error throw
zerosnacks Apr 25, 2024
00897fa
update trace
zerosnacks Apr 25, 2024
163e337
add fuzz test case, also logs block correctly - avoid pushing genesis…
zerosnacks Apr 25, 2024
e5037d1
remove debug trace
zerosnacks Apr 25, 2024
2b6865f
improve documentation
zerosnacks Apr 25, 2024
b311550
pull in master
zerosnacks Apr 30, 2024
1536799
only rely on context tracing to determine fork status
zerosnacks Apr 30, 2024
af34ff8
remove context as inspector, simply inline
zerosnacks Apr 30, 2024
d42eb2e
remove inspector, only rely on executor env block number at the end o…
zerosnacks Apr 30, 2024
2e5918b
re-add inspector - cleaner than inlining and allows capture of transf…
zerosnacks Apr 30, 2024
c0775aa
add comment on block number modification due to cheat codes
zerosnacks Apr 30, 2024
747e17c
remove wrongly merged funcs file, avoid formatting diff
zerosnacks Apr 30, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 19 additions & 1 deletion crates/evm/core/src/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,11 @@ impl Backend {
}
}

/// Returns whether the backend has any forks
pub fn has_forks(&self) -> bool {
self.inner.has_forks()
}

/// Returns all snapshots created in this backend
pub fn snapshots(&self) -> &Snapshots<BackendSnapshot<BackendDatabaseSnapshot>> {
&self.inner.snapshots
Expand Down Expand Up @@ -1540,7 +1545,7 @@ pub struct BackendInner {
/// issued _local_ numeric identifier, that remains constant, even if the underlying fork
/// backend changes.
pub issued_local_fork_ids: HashMap<LocalForkId, ForkId>,
/// tracks all the created forks
/// Tracks all the created forks
/// Contains the index of the corresponding `ForkDB` in the `forks` vec
pub created_forks: HashMap<ForkId, ForkLookupIndex>,
/// Holds all created fork databases
Expand Down Expand Up @@ -1599,6 +1604,19 @@ impl BackendInner {
self.ensure_fork_index(self.ensure_fork_id(id)?)
}

/// Returns whether there are any forks
pub fn has_forks(&self) -> bool {
if self.launched_with_fork.is_some() {
return true;
}

if !self.created_forks.is_empty() {
return true;
}

self.forks.iter().any(|fork| fork.is_some())
}

/// Returns the underlying fork mapped to the index
#[track_caller]
fn get_fork(&self, idx: ForkLookupIndex) -> &Fork {
Expand Down
8 changes: 7 additions & 1 deletion crates/evm/evm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ foundry-evm-traces.workspace = true

alloy-dyn-abi = { workspace = true, features = ["arbitrary", "eip712"] }
alloy-json-abi.workspace = true
alloy-primitives = { workspace = true, features = ["serde", "getrandom", "arbitrary", "rlp"] }
alloy-primitives = { workspace = true, features = [
"serde",
"getrandom",
"arbitrary",
"rlp",
] }
alloy-sol-types.workspace = true
revm = { workspace = true, default-features = false, features = [
"std",
Expand All @@ -43,5 +48,6 @@ parking_lot = "0.12"
proptest = "1"
rand.workspace = true
rayon = "1"
serde.workspace = true
thiserror = "1"
tracing = "0.1"
8 changes: 6 additions & 2 deletions crates/evm/evm/src/executors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
// the concrete `Executor` type.

use crate::inspectors::{
cheatcodes::BroadcastableTransactions, Cheatcodes, InspectorData, InspectorStack,
cheatcodes::BroadcastableTransactions, Cheatcodes, Context, InspectorData, InspectorStack,
};
use alloy_dyn_abi::{DynSolValue, FunctionExt, JsonAbiExt};
use alloy_json_abi::Function;
Expand Down Expand Up @@ -662,6 +662,8 @@ pub struct RawCallResult {
pub labels: HashMap<Address, String>,
/// The traces of the call
pub traces: Option<CallTraceArena>,
/// The contexts created during the call
pub contexts: Vec<Context>,
/// The coverage info collected during the call
pub coverage: Option<HitMaps>,
/// The debug nodes of the call
Expand Down Expand Up @@ -696,6 +698,7 @@ impl Default for RawCallResult {
logs: Vec::new(),
labels: HashMap::new(),
traces: None,
contexts: Vec::new(),
coverage: None,
debug: None,
transactions: None,
Expand Down Expand Up @@ -810,7 +813,7 @@ fn convert_executed_result(
_ => Bytes::new(),
};

let InspectorData { logs, labels, traces, coverage, debug, cheatcodes, chisel_state } =
let InspectorData { logs, labels, traces, contexts, coverage, debug, cheatcodes, chisel_state } =
inspector.collect();

let transactions = match cheatcodes.as_ref() {
Expand All @@ -831,6 +834,7 @@ fn convert_executed_result(
logs,
labels,
traces,
contexts,
coverage,
debug,
transactions,
Expand Down
38 changes: 38 additions & 0 deletions crates/evm/evm/src/inspectors/context.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
use alloy_primitives::U256;
use revm::{
interpreter::{CallInputs, CallOutcome},
Database, EvmContext, Inspector,
};
use serde::{Deserialize, Serialize};

#[derive(Clone, Default, Debug, Serialize, Deserialize, PartialEq, Eq, Hash)]
/// A context
pub struct Context {
/// The block number of the context.
pub block_number: U256,
}

/// An inspector that collects EVM context during execution.
#[derive(Clone, Debug, Default)]
pub struct ContextCollector {
/// The collected contexts.
pub contexts: Vec<Context>,
}

impl<DB: Database> Inspector<DB> for ContextCollector {
fn call(&mut self, ecx: &mut EvmContext<DB>, _call: &mut CallInputs) -> Option<CallOutcome> {
let block_number = ecx.inner.env.block.number;

// Skip if the previous context is the same
if let Some(Context { block_number: prev_block_number }) = self.contexts.last() {
if *prev_block_number == block_number {
return None;
}
}

// Push the new context
self.contexts.push(Context { block_number });

None
}
}
3 changes: 3 additions & 0 deletions crates/evm/evm/src/inspectors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ pub use revm_inspectors::access_list::AccessListInspector;
mod chisel_state;
pub use chisel_state::ChiselState;

mod context;
pub use context::{Context, ContextCollector};

mod debugger;
pub use debugger::Debugger;

Expand Down
18 changes: 16 additions & 2 deletions crates/evm/evm/src/inspectors/stack.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{
Cheatcodes, CheatsConfig, ChiselState, CoverageCollector, Debugger, Fuzzer, LogCollector,
StackSnapshotType, TracingInspector, TracingInspectorConfig,
context::Context, Cheatcodes, CheatsConfig, ChiselState, ContextCollector, CoverageCollector,
Debugger, Fuzzer, LogCollector, StackSnapshotType, TracingInspector, TracingInspectorConfig,
};
use alloy_primitives::{Address, Bytes, Log, U256};
use foundry_evm_core::{
Expand Down Expand Up @@ -40,6 +40,8 @@ pub struct InspectorStackBuilder {
pub fuzzer: Option<Fuzzer>,
/// Whether to enable tracing.
pub trace: Option<bool>,
/// Whether to enable context capture.
pub context: Option<bool>,
/// Whether to enable the debugger.
pub debug: Option<bool>,
/// Whether logs should be collected.
Expand Down Expand Up @@ -151,6 +153,7 @@ impl InspectorStackBuilder {
cheatcodes,
fuzzer,
trace,
context,
debug,
logs,
coverage,
Expand All @@ -172,6 +175,7 @@ impl InspectorStackBuilder {
}
stack.collect_coverage(coverage.unwrap_or(false));
stack.collect_logs(logs.unwrap_or(true));
stack.collect_context(context.unwrap_or(true));
stack.enable_debugger(debug.unwrap_or(false));
stack.print(print.unwrap_or(false));
stack.tracing(trace.unwrap_or(false));
Expand Down Expand Up @@ -249,6 +253,7 @@ pub struct InspectorData {
pub logs: Vec<Log>,
pub labels: HashMap<Address, String>,
pub traces: Option<CallTraceArena>,
pub contexts: Vec<Context>,
pub debug: Option<DebugArena>,
pub coverage: Option<HitMaps>,
pub cheatcodes: Option<Cheatcodes>,
Expand Down Expand Up @@ -285,6 +290,7 @@ pub struct InspectorStack {
pub debugger: Option<Debugger>,
pub fuzzer: Option<Fuzzer>,
pub log_collector: Option<LogCollector>,
pub context_collector: Option<ContextCollector>,
pub printer: Option<CustomPrintTracer>,
pub tracer: Option<TracingInspector>,
pub enable_isolation: bool,
Expand Down Expand Up @@ -370,6 +376,12 @@ impl InspectorStack {
self.log_collector = yes.then(Default::default);
}

/// Set whether to enable the context collector.
#[inline]
pub fn collect_context(&mut self, yes: bool) {
self.context_collector = yes.then(Default::default);
}

/// Set whether to enable the trace printer.
#[inline]
pub fn print(&mut self, yes: bool) {
Expand Down Expand Up @@ -404,6 +416,7 @@ impl InspectorStack {
})
.unwrap_or_default(),
traces: self.tracer.map(|tracer| tracer.get_traces().clone()),
contexts: self.context_collector.map(|context| context.contexts).unwrap_or_default(),
debug: self.debugger.map(|debugger| debugger.arena),
coverage: self.coverage.map(|coverage| coverage.maps),
cheatcodes: self.cheatcodes,
Expand Down Expand Up @@ -654,6 +667,7 @@ impl<DB: DatabaseExt + DatabaseCommit> Inspector<&mut DB> for InspectorStack {
&mut self.debugger,
&mut self.tracer,
&mut self.log_collector,
&mut self.context_collector,
&mut self.cheatcodes,
],
|inspector| {
Expand Down
12 changes: 12 additions & 0 deletions crates/evm/traces/src/identifier/etherscan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ pub struct EtherscanIdentifier {
invalid_api_key: Arc<AtomicBool>,
pub contracts: BTreeMap<Address, Metadata>,
pub sources: BTreeMap<u32, String>,
// Tracks whether the Etherscan identifier is enabled
// Enabled for forking tests
// Disabled for local tests
pub enabled: bool,
}

impl EtherscanIdentifier {
Expand All @@ -53,9 +57,17 @@ impl EtherscanIdentifier {
invalid_api_key: Arc::new(AtomicBool::new(false)),
contracts: BTreeMap::new(),
sources: BTreeMap::new(),
// By default, the Etherscan identifier is enabled to cover edge cases.
// It is disabled for local tests and enabled for forked tests on a per test basis.
enabled: true,
}))
}

/// Enables or disables the Etherscan identifier.
pub fn enable(&mut self, yes: bool) {
self.enabled = yes;
}

/// Goes over the list of contracts we have pulled from the traces, clones their source from
/// Etherscan and compiles them locally, for usage in the debugger.
pub async fn get_compiled_contracts(&self) -> eyre::Result<ContractSources> {
Expand Down
11 changes: 10 additions & 1 deletion crates/evm/traces/src/identifier/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ impl TraceIdentifier for TraceIdentifiers<'_> {
identities.extend(local.identify_addresses(addresses.clone()));
}
if let Some(etherscan) = &mut self.etherscan {
identities.extend(etherscan.identify_addresses(addresses));
if etherscan.enabled {
identities.extend(etherscan.identify_addresses(addresses));
}
}
identities
}
Expand All @@ -86,6 +88,13 @@ impl<'a> TraceIdentifiers<'a> {
Ok(self)
}

/// Enables or disables the Etherscan identifier.
pub fn enable_etherscan(&mut self, yes: bool) {
if let Some(etherscan) = &mut self.etherscan {
etherscan.enable(yes);
}
}

/// Returns `true` if there are no set identifiers.
pub fn is_empty(&self) -> bool {
self.local.is_none() && self.etherscan.is_none()
Expand Down
7 changes: 7 additions & 0 deletions crates/forge/bin/cmd/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,13 @@ impl TestArgs {
.labels
.extend(result.labeled_addresses.iter().map(|(k, v)| (*k, v.clone())));

// Enable Etherscan decoding for forking tests.
// Disable Etherscan decoding for local tests to avoid unnecessary API calls
// that can slow down execution significantly if rate-limited.
if identify_addresses {
identifier.enable_etherscan(result.is_fork());
}

// Identify addresses and decode traces.
let mut decoded_traces = Vec::with_capacity(result.traces.len());
for (kind, arena) in &result.traces {
Expand Down