From 04606334bdf4812b3628d0e204f84e799c01505e Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Thu, 20 Jun 2024 04:04:19 +0300 Subject: [PATCH 01/30] fix: small debugger updates --- crates/debugger/src/tui/draw.rs | 14 +++++++++----- crates/evm/traces/src/identifier/etherscan.rs | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/debugger/src/tui/draw.rs b/crates/debugger/src/tui/draw.rs index 7ac0c64e5c6e..79533ad4caae 100644 --- a/crates/debugger/src/tui/draw.rs +++ b/crates/debugger/src/tui/draw.rs @@ -279,7 +279,7 @@ impl DebuggerContext<'_> { // Highlighted text: cyan, bold. let h_text = Style::new().fg(Color::Cyan).add_modifier(Modifier::BOLD); - let mut lines = SourceLines::new(decimal_digits(num_lines)); + let mut lines = SourceLines::new(start_line, end_line); // We check if there is other text on the same line before the highlight starts. if let Some(last) = before.pop() { @@ -625,12 +625,13 @@ impl DebuggerContext<'_> { /// Wrapper around a list of [`Line`]s that prepends the line number on each new line. struct SourceLines<'a> { lines: Vec>, + start_line: usize, max_line_num: usize, } impl<'a> SourceLines<'a> { - fn new(max_line_num: usize) -> Self { - Self { lines: Vec::new(), max_line_num } + fn new(start_line: usize, end_line: usize) -> Self { + Self { lines: Vec::new(), start_line, max_line_num: decimal_digits(end_line) } } fn push(&mut self, line_number_style: Style, line: &'a str, line_style: Style) { @@ -640,8 +641,11 @@ impl<'a> SourceLines<'a> { fn push_raw(&mut self, line_number_style: Style, spans: &[Span<'a>]) { let mut line_spans = Vec::with_capacity(4); - let line_number = - format!("{number: >width$} ", number = self.lines.len() + 1, width = self.max_line_num); + let line_number = format!( + "{number: >width$} ", + number = self.start_line + self.lines.len() + 1, + width = self.max_line_num + ); line_spans.push(Span::styled(line_number, line_number_style)); // Space between line number and line text. diff --git a/crates/evm/traces/src/identifier/etherscan.rs b/crates/evm/traces/src/identifier/etherscan.rs index 87d7e9c92d83..54921beffcd8 100644 --- a/crates/evm/traces/src/identifier/etherscan.rs +++ b/crates/evm/traces/src/identifier/etherscan.rs @@ -85,7 +85,7 @@ impl EtherscanIdentifier { // construct the map for res in outputs { - let (project, output, _) = res?; + let (project, output, _root) = res?; sources.insert(&output, project.root(), None)?; } From 9fd779a59e80886e635337f9c1acddbbe57ecfe5 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 21 Jun 2024 03:37:32 +0300 Subject: [PATCH 02/30] [wip] feat: identify internal function invocations in traces --- Cargo.lock | 126 +++++---- Cargo.toml | 3 + crates/cast/bin/cmd/call.rs | 8 +- crates/cast/bin/cmd/run.rs | 8 +- crates/cli/src/utils/cmd.rs | 30 ++- crates/debugger/src/identifier.rs | 249 ++++++++++++++++++ crates/debugger/src/lib.rs | 2 + crates/debugger/src/tui/builder.rs | 60 ++--- crates/debugger/src/tui/draw.rs | 72 +---- crates/debugger/src/tui/mod.rs | 30 +-- .../evm/evm/src/executors/invariant/replay.rs | 2 +- crates/evm/evm/src/executors/mod.rs | 4 +- crates/evm/evm/src/executors/tracing.rs | 3 +- crates/evm/evm/src/inspectors/stack.rs | 38 ++- crates/evm/traces/src/lib.rs | 137 ++++++++-- crates/forge/bin/cmd/test/mod.rs | 58 +++- crates/forge/src/runner.rs | 6 +- crates/script/src/execute.rs | 6 +- crates/verify/src/bytecode.rs | 2 +- 19 files changed, 571 insertions(+), 273 deletions(-) create mode 100644 crates/debugger/src/identifier.rs diff --git a/Cargo.lock b/Cargo.lock index 8f49b41f137a..6d59c0b84905 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -306,7 +306,7 @@ checksum = "8037e03c7f462a063f28daec9fda285a9a89da003c552f8637a80b9c8fd96241" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -552,7 +552,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -569,7 +569,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", "syn-solidity", "tiny-keccak", ] @@ -587,7 +587,7 @@ dependencies = [ "proc-macro2", "quote", "serde_json", - "syn 2.0.66", + "syn 2.0.67", "syn-solidity", ] @@ -1097,7 +1097,7 @@ checksum = "3b43422f69d8ff38f95f1b2bb76517c91589a924d1559a0e935d7c8ce0274c11" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -1119,7 +1119,7 @@ checksum = "16e62a023e7c117e27523144c5d2459f4397fcc3cab0085af8e2224f643a0193" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -1130,7 +1130,7 @@ checksum = "c6fa2087f2753a7da8cc1c0dbfcf89579dd57458e36769de5ac750b4671737ca" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -1177,7 +1177,7 @@ checksum = "3c87f3f15e7794432337fc718554eaa4dc8f04c9677a950ffe366f20a162ae42" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -1491,15 +1491,14 @@ dependencies = [ [[package]] name = "aws-types" -version = "1.3.1" +version = "1.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f734808d43702a67e57d478a12e227d4d038d0b90c9005a78c87890d3805922" +checksum = "2009a9733865d0ebf428a314440bbe357cc10d0c16d86a8e15d32e9b47c1e80e" dependencies = [ "aws-credential-types", "aws-smithy-async", "aws-smithy-runtime-api", "aws-smithy-types", - "http 0.2.12", "rustc_version 0.4.0", "tracing", ] @@ -2109,9 +2108,9 @@ dependencies = [ [[package]] name = "clap_complete" -version = "4.5.5" +version = "4.5.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d2020fa13af48afc65a9a87335bda648309ab3d154cd03c7ff95b378c7ed39c4" +checksum = "fbca90c87c2a04da41e95d1856e8bcd22f159bdbfa147314d2ce5218057b0e58" dependencies = [ "clap", ] @@ -2135,7 +2134,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -2565,7 +2564,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.11.1", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -2576,7 +2575,7 @@ checksum = "733cabb43482b1a1b53eee8583c2b9e8684d592215ea83efd305dd31bc2f0178" dependencies = [ "darling_core", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -2648,7 +2647,7 @@ checksum = "67e77553c4162a157adbf834ebae5b415acbecbeafc7a74b0e886657506a7611" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -2669,7 +2668,7 @@ dependencies = [ "darling", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -2679,7 +2678,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "206868b8242f27cecce124c19fd88157fbd0dd334df2587f36417bafbc85097b" dependencies = [ "derive_builder_core", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -2692,7 +2691,7 @@ dependencies = [ "proc-macro2", "quote", "rustc_version 0.4.0", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -2793,13 +2792,13 @@ dependencies = [ [[package]] name = "displaydoc" -version = "0.2.4" +version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "487585f4d0c6655fe74905e2504d8ad6908e4db67f744eb140876906c2f3175d" +checksum = "97369cbbc041bc366949bc74d34658d6cda5621039731c6310521892a3a20ae0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -2935,7 +2934,7 @@ checksum = "6fd000fd6988e73bbe993ea3db9b1aa64906ab88766d654973924340c8cddb42" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -3085,7 +3084,7 @@ dependencies = [ "regex", "serde", "serde_json", - "syn 2.0.66", + "syn 2.0.67", "toml 0.8.14", "walkdir", ] @@ -3113,7 +3112,7 @@ dependencies = [ "serde", "serde_json", "strum", - "syn 2.0.66", + "syn 2.0.67", "tempfile", "thiserror", "tiny-keccak", @@ -3477,7 +3476,7 @@ dependencies = [ "proc-macro2", "quote", "serde_json", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -3987,7 +3986,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -4146,7 +4145,7 @@ checksum = "87750cf4b7a4c0625b1529e4c543c2182106e4dedc60a2a6455e00d212c489ac" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -4634,7 +4633,7 @@ dependencies = [ "markup5ever", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -5477,7 +5476,7 @@ checksum = "49e7bc1560b95a3c4a25d03de42fe76ca718ab92d1a22a55b9b4cf67b3ae635c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -5547,7 +5546,7 @@ dependencies = [ "cfg-if", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -5778,7 +5777,7 @@ dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -5901,7 +5900,7 @@ checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -6072,7 +6071,7 @@ dependencies = [ "proc-macro2", "proc-macro2-diagnostics", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -6131,7 +6130,7 @@ dependencies = [ "pest_meta", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -6215,7 +6214,7 @@ dependencies = [ "phf_shared 0.11.2", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -6273,7 +6272,7 @@ checksum = "2f38a4412a78282e09a2cf38d195ea5420d15ba0602cb375210efbc877243965" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -6389,7 +6388,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5f12335488a2f3b0a83b14edad48dca9879ce89b2edd10e80237e4e852dd645e" dependencies = [ "proc-macro2", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -6450,9 +6449,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.85" +version = "1.0.86" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22244ce15aa966053a896d1accb3a6e68469b97c7f33f284b99f0d576879fc23" +checksum = "5e719e8df665df0d1c8fbfd238015744736151d4445ec0836b8e628aae103b77" dependencies = [ "unicode-ident", ] @@ -6465,7 +6464,7 @@ checksum = "af066a9c399a26e020ada66a034357a868728e72cd426f3adcd35f80d88d88c8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", "version_check", "yansi", ] @@ -6532,7 +6531,7 @@ dependencies = [ "itertools 0.12.1", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -6946,8 +6945,7 @@ dependencies = [ [[package]] name = "revm-inspectors" version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eba2e187811b160463663fd71881b4e5d653720ba00be0f1e85962d4db60341c" +source = "git+https://github.com/klkvr/evm-inspectors?rev=7b649e5#7b649e5ca690f94bd6bbd9f8bcc655bc282a16b6" dependencies = [ "alloy-primitives", "alloy-rpc-types", @@ -7415,7 +7413,7 @@ dependencies = [ "proc-macro2", "quote", "serde_derive_internals", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -7571,7 +7569,7 @@ checksum = "500cbc0ebeb6f46627f50f3f5811ccf6bf00643be300b4c3eabc0ef55dc5b5ba" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -7582,7 +7580,7 @@ checksum = "18d26a20a969b9e3fdf2fc2d9f21eda6c40e2de84c9408bb5d3b05d499aae711" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -7625,7 +7623,7 @@ checksum = "6c64451ba24fc7a6a2d60fc75dd9c83c90903b19028d4eff35e88fc1e86564e9" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -7671,7 +7669,7 @@ checksum = "82fe9db325bcef1fbcde82e078a5cc4efdf787e96b3b9cf45b50b529f2083d67" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -7946,7 +7944,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2ff9eaf853dec4c8802325d8b6d3dffa86cc707fd7a1a4cdbf416e13b061787a" dependencies = [ "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -8018,7 +8016,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -8036,9 +8034,9 @@ dependencies = [ [[package]] name = "subtle" -version = "2.5.0" +version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81cdd64d312baedb58e21336b31bc043b77e01cc99033ce76ef539f78e965ebc" +checksum = "0d0208408ba0c3df17ed26eb06992cb1a1268d41b2c0e12e65203fbe3972cee5" [[package]] name = "svm-rs" @@ -8086,9 +8084,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.66" +version = "2.0.67" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c42f3f41a2de00b01c0aaad383c5a45241efc8b2d1eda5661812fda5f3cdcff5" +checksum = "ff8655ed1d86f3af4ee3fd3263786bc14245ad17c4c7e85ba7187fb3ae028c90" dependencies = [ "proc-macro2", "quote", @@ -8104,7 +8102,7 @@ dependencies = [ "paste", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -8226,7 +8224,7 @@ checksum = "46c3384250002a6d5af4d114f2845d37b57521033f30d5c3f46c4d70e1197533" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -8372,7 +8370,7 @@ checksum = "5f5ae998a069d4b5aba8ee9dad856af7d520c3699e6159b185c2acd48155d39a" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -8651,7 +8649,7 @@ checksum = "34704c8d6ebcbc939824180af020566b01a7c01f80641264eba0999f6c2b6be7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -9012,7 +9010,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", "wasm-bindgen-shared", ] @@ -9046,7 +9044,7 @@ checksum = "e94f17b526d0a461a191c78ea52bbce64071ed5c04c9ffe424dcb38f74171bb7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -9507,7 +9505,7 @@ checksum = "15e934569e47891f7d9411f1a451d947a60e000ab3bd24fbb970f000387d1b3b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] @@ -9527,7 +9525,7 @@ checksum = "ce36e65b0d2999d2aafac989fb249189a141aee1f53c612c1f37d72631959f69" dependencies = [ "proc-macro2", "quote", - "syn 2.0.66", + "syn 2.0.67", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 2d44d6d671d1..fa1ccc3fc9f5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -259,3 +259,6 @@ tower = "0.4" tower-http = "0.5" # soldeer soldeer = "0.2.15" + +[patch.crates-io] +revm-inspectors = { git = "https://github.com/klkvr/evm-inspectors", rev = "7b649e5" } diff --git a/crates/cast/bin/cmd/call.rs b/crates/cast/bin/cmd/call.rs index 0499f5e7b275..25bb08bb1610 100644 --- a/crates/cast/bin/cmd/call.rs +++ b/crates/cast/bin/cmd/call.rs @@ -43,6 +43,9 @@ pub struct CallArgs { #[arg(long, requires = "trace")] debug: bool, + #[arg(long, requires = "trace")] + decode_internal: bool, + /// Labels to apply to the traces; format: `address:label`. /// Can only be used with `--trace`. #[arg(long, requires = "trace")] @@ -106,6 +109,7 @@ impl CallArgs { trace, evm_version, debug, + decode_internal, labels, data, } = self; @@ -159,7 +163,7 @@ impl CallArgs { } let (env, fork, chain) = TracingExecutor::get_fork_material(&config, evm_opts).await?; - let mut executor = TracingExecutor::new(env, fork, evm_version, debug); + let mut executor = TracingExecutor::new(env, fork, evm_version, debug, decode_internal); let value = tx.value.unwrap_or_default(); let input = tx.inner.input.into_input().unwrap_or_default(); @@ -175,7 +179,7 @@ impl CallArgs { ), }; - handle_traces(trace, &config, chain, labels, debug).await?; + handle_traces(trace, &config, chain, labels, debug, decode_internal).await?; return Ok(()); } diff --git a/crates/cast/bin/cmd/run.rs b/crates/cast/bin/cmd/run.rs index c830f5ab1aa4..70b27313d6e3 100644 --- a/crates/cast/bin/cmd/run.rs +++ b/crates/cast/bin/cmd/run.rs @@ -27,6 +27,10 @@ pub struct RunArgs { #[arg(long, short)] debug: bool, + /// Whether to identify internal functions in traces. + #[arg(long)] + decode_internal: bool, + /// Print out opcode traces. #[arg(long, short)] trace_printer: bool, @@ -142,7 +146,7 @@ impl RunArgs { } } - let mut executor = TracingExecutor::new(env.clone(), fork, evm_version, self.debug); + let mut executor = TracingExecutor::new(env.clone(), fork, evm_version, self.debug, self.decode_internal); let mut env = EnvWithHandlerCfg::new_with_spec_id(Box::new(env.clone()), executor.spec_id()); @@ -220,7 +224,7 @@ impl RunArgs { } }; - handle_traces(result, &config, chain, self.label, self.debug).await?; + handle_traces(result, &config, chain, self.label, self.debug, self.decode_internal).await?; Ok(()) } diff --git a/crates/cli/src/utils/cmd.rs b/crates/cli/src/utils/cmd.rs index 13da8d01df8d..860f1879d0d2 100644 --- a/crates/cli/src/utils/cmd.rs +++ b/crates/cli/src/utils/cmd.rs @@ -9,14 +9,13 @@ use foundry_compilers::{ Artifact, ProjectCompileOutput, }; use foundry_config::{error::ExtractConfigError, figment::Figment, Chain, Config, NamedChain}; -use foundry_debugger::Debugger; +use foundry_debugger::{DebugTraceIdentifier, Debugger}; use foundry_evm::{ debug::DebugArena, executors::{DeployResult, EvmError, RawCallResult}, opts::EvmOpts, traces::{ - identifier::{EtherscanIdentifier, SignaturesIdentifier}, - render_trace_arena, CallTraceDecoder, CallTraceDecoderBuilder, TraceKind, Traces, + identifier::{EtherscanIdentifier, SignaturesIdentifier}, render_trace_arena, render_trace_arena_with_internals, CallTraceDecoder, CallTraceDecoderBuilder, TraceKind, Traces }, }; use std::{ @@ -357,6 +356,7 @@ pub async fn handle_traces( chain: Option, labels: Vec, debug: bool, + decode_internal: bool, ) -> Result<()> { let labels = labels.iter().filter_map(|label_str| { let mut iter = label_str.split(':'); @@ -392,23 +392,37 @@ pub async fn handle_traces( }; let mut debugger = Debugger::builder() .debug_arena(result.debug.as_ref().expect("missing debug arena")) - .decoder(&decoder) - .sources(sources) + .identifier(|b| b.decoder(&decoder).sources(sources)) .build(); debugger.try_run()?; } else { - print_traces(&mut result, &decoder).await?; + let identifier = if decode_internal { + let sources = if let Some(etherscan_identifier) = etherscan_identifier { + etherscan_identifier.get_compiled_contracts().await? + } else { + Default::default() + }; + Some(DebugTraceIdentifier::builder().sources(sources).decoder(&decoder).build()) + } else { + None + }; + print_traces(&mut result, &decoder, identifier.as_ref()).await?; } Ok(()) } -pub async fn print_traces(result: &mut TraceResult, decoder: &CallTraceDecoder) -> Result<()> { +pub async fn print_traces(result: &mut TraceResult, decoder: &CallTraceDecoder, identifier: Option<&DebugTraceIdentifier>) -> Result<()> { let traces = result.traces.as_ref().expect("No traces found"); println!("Traces:"); for (_, arena) in traces { - println!("{}", render_trace_arena(arena, decoder).await?); + let arena = if let Some(identifier) = identifier { + render_trace_arena_with_internals(arena, decoder, &identifier.identify_arena(arena)).await? + } else { + render_trace_arena(arena, decoder).await? + }; + println!("{}", arena); } println!(); diff --git a/crates/debugger/src/identifier.rs b/crates/debugger/src/identifier.rs new file mode 100644 index 000000000000..43d041961051 --- /dev/null +++ b/crates/debugger/src/identifier.rs @@ -0,0 +1,249 @@ +use alloy_primitives::Address; +use foundry_common::{compile::ContractSources, get_contract_name}; +use foundry_compilers::{ + artifacts::sourcemap::{Jump, SourceElement}, + multi::MultiCompilerLanguage, +}; +use foundry_evm_core::utils::PcIcMap; +use foundry_evm_traces::{CallTraceArena, CallTraceDecoder, CallTraceNode, DecodedTraceStep}; +use revm::interpreter::OpCode; +use std::collections::HashMap; + +pub struct DebugTraceIdentifier { + /// Mapping of contract address to identified contract name. + identified_contracts: HashMap, + /// Source map of contract sources + contracts_sources: ContractSources, + /// A mapping of source -> (PC -> IC map for deploy code, PC -> IC map for runtime code) + pc_ic_maps: HashMap, +} + +impl DebugTraceIdentifier { + pub fn builder() -> DebugTraceIdentifierBuilder { + DebugTraceIdentifierBuilder::default() + } + + pub fn new( + identified_contracts: HashMap, + contracts_sources: ContractSources, + ) -> Self { + let pc_ic_maps = contracts_sources + .entries() + .filter_map(|(name, artifact, _)| { + Some(( + name.to_owned(), + ( + PcIcMap::new(artifact.bytecode.bytecode.bytes()?), + PcIcMap::new(artifact.bytecode.deployed_bytecode.bytes()?), + ), + )) + }) + .collect(); + Self { identified_contracts, contracts_sources, pc_ic_maps } + } + + pub fn identify( + &self, + address: &Address, + pc: usize, + init_code: bool, + ) -> core::result::Result<(SourceElement, &str, &str), String> { + let Some(contract_name) = self.identified_contracts.get(address) else { + return Err(format!("Unknown contract at address {address}")); + }; + + let Some(mut files_source_code) = self.contracts_sources.get_sources(contract_name) else { + return Err(format!("No source map index for contract {contract_name}")); + }; + + let Some((create_map, rt_map)) = self.pc_ic_maps.get(contract_name) else { + return Err(format!("No PC-IC maps for contract {contract_name}")); + }; + + let Some((source_element, source_code, source_file)) = + files_source_code.find_map(|(artifact, source)| { + let bytecode = if init_code { + &artifact.bytecode.bytecode + } else { + artifact.bytecode.deployed_bytecode.bytecode.as_ref()? + }; + let source_map = bytecode.source_map()?.expect("failed to parse"); + + let pc_ic_map = if init_code { create_map } else { rt_map }; + let ic = pc_ic_map.get(pc)?; + + // Solc indexes source maps by instruction counter, but Vyper indexes by program + // counter. + let source_element = if matches!(source.language, MultiCompilerLanguage::Solc(_)) { + source_map.get(ic)? + } else { + source_map.get(pc)? + }; + // if the source element has an index, find the sourcemap for that index + let res = source_element + .index() + // if index matches current file_id, return current source code + .and_then(|index| { + (index == artifact.file_id) + .then(|| (source_element.clone(), source.source.as_str(), &source.name)) + }) + .or_else(|| { + // otherwise find the source code for the element's index + self.contracts_sources + .sources_by_id + .get(&artifact.build_id)? + .get(&source_element.index()?) + .map(|source| { + (source_element.clone(), source.source.as_str(), &source.name) + }) + }); + + res + }) + else { + return Err(format!("No source map for contract {contract_name}")); + }; + + Ok((source_element, source_code, source_file)) + } + + pub fn identify_arena(&self, arena: &CallTraceArena) -> Vec>> { + arena.nodes().iter().map(move |node| self.identify_node_steps(node)).collect() + } + + pub fn identify_node_steps(&self, node: &CallTraceNode) -> Vec> { + let mut stack = Vec::new(); + let mut identified = Vec::new(); + + // Flag marking whether previous instruction was a jump into function. + // If it was, we expect next instruction to be a JUMPDEST with source location pointing to + // the function. + let mut prev_step_jump_in = false; + for (step_idx, step) in node.trace.steps.iter().enumerate() { + // We are only interested in JUMPs. + if step.op != OpCode::JUMP && step.op != OpCode::JUMPI && step.op != OpCode::JUMPDEST { + continue; + } + + // Resolve source map if possible. + let Ok((source_element, source_code, _)) = + self.identify(&node.trace.address, step.pc, node.trace.kind.is_any_create()) + else { + prev_step_jump_in = false; + continue; + }; + + // Get slice of the source code that corresponds to the current step. + let source_part = { + let start = source_element.offset() as usize; + let end = start + source_element.length() as usize; + &source_code[start..end] + }; + + // If previous step was a jump record source location at JUMPDEST. + if prev_step_jump_in { + if step.op == OpCode::JUMPDEST { + if let Some(name) = parse_function_name(source_part) { + stack.push((name, step_idx)); + } + }; + prev_step_jump_in = false; + } + + match source_element.jump() { + // Source location is collected on the next step. + Jump::In => prev_step_jump_in = true, + Jump::Out => { + // Find index matching the beginning of this function + if let Some(name) = parse_function_name(source_part) { + if let Some((i, _)) = + stack.iter().enumerate().rfind(|(_, (n, _))| n == &name) + { + // We've found a match, remove all records between start and end, those + // are considered invalid. + let (_, start_idx) = stack.split_off(i)[0]; + + let gas_used = node.trace.steps[start_idx].gas_remaining as i64 - + node.trace.steps[step_idx].gas_remaining as i64; + + identified.push(DecodedTraceStep { + start_step_idx: start_idx, + end_step_idx: step_idx, + function_name: name, + gas_used, + }); + } + } + } + _ => {} + }; + } + + // Sort by start step index. + identified.sort_by_key(|i| i.start_step_idx); + + identified + } +} + +/// [DebugTraceIdentifier] builder +#[derive(Debug, Default)] +#[must_use = "builders do nothing unless you call `build` on them"] +pub struct DebugTraceIdentifierBuilder { + /// Identified contracts. + identified_contracts: HashMap, + /// Map of source files. + sources: ContractSources, +} + +impl DebugTraceIdentifierBuilder { + /// Extends the identified contracts from multiple decoders. + #[inline] + pub fn decoders(mut self, decoders: &[CallTraceDecoder]) -> Self { + for decoder in decoders { + self = self.decoder(decoder); + } + self + } + + /// Extends the identified contracts from a decoder. + #[inline] + pub fn decoder(self, decoder: &CallTraceDecoder) -> Self { + let c = decoder.contracts.iter().map(|(k, v)| (*k, get_contract_name(v).to_string())); + self.identified_contracts(c) + } + + /// Extends the identified contracts. + #[inline] + pub fn identified_contracts( + mut self, + identified_contracts: impl IntoIterator, + ) -> Self { + self.identified_contracts.extend(identified_contracts); + self + } + + /// Sets the sources for the debugger. + #[inline] + pub fn sources(mut self, sources: ContractSources) -> Self { + self.sources = sources; + self + } + + /// Builds the [DebugTraceIdentifier]. + #[inline] + pub fn build(self) -> DebugTraceIdentifier { + let Self { identified_contracts, sources } = self; + DebugTraceIdentifier::new(identified_contracts, sources) + } +} + +fn parse_function_name(source: &str) -> Option<&str> { + if !source.starts_with("function") { + return None; + } + if !source.contains("internal") && !source.contains("private") { + return None; + } + Some(source.split_once("function")?.1.split('(').next()?.trim()) +} diff --git a/crates/debugger/src/lib.rs b/crates/debugger/src/lib.rs index ed5da934271a..052dbb1aab47 100644 --- a/crates/debugger/src/lib.rs +++ b/crates/debugger/src/lib.rs @@ -10,5 +10,7 @@ extern crate tracing; mod op; +mod identifier; mod tui; +pub use identifier::DebugTraceIdentifier; pub use tui::{Debugger, DebuggerBuilder, ExitReason}; diff --git a/crates/debugger/src/tui/builder.rs b/crates/debugger/src/tui/builder.rs index 6289b0b8814f..ea80335a39b9 100644 --- a/crates/debugger/src/tui/builder.rs +++ b/crates/debugger/src/tui/builder.rs @@ -1,11 +1,8 @@ //! TUI debugger builder. -use crate::Debugger; -use alloy_primitives::Address; -use foundry_common::{compile::ContractSources, evm::Breakpoints, get_contract_name}; +use crate::{identifier::DebugTraceIdentifierBuilder, Debugger}; +use foundry_common::evm::Breakpoints; use foundry_evm_core::debug::{DebugArena, DebugNodeFlat}; -use foundry_evm_traces::CallTraceDecoder; -use std::collections::HashMap; /// Debugger builder. #[derive(Debug, Default)] @@ -13,10 +10,8 @@ use std::collections::HashMap; pub struct DebuggerBuilder { /// Debug traces returned from the EVM execution. debug_arena: Vec, - /// Identified contracts. - identified_contracts: HashMap, - /// Map of source files. - sources: ContractSources, + /// Builder for [DebugTraceIdentifier]. + identifier: DebugTraceIdentifierBuilder, /// Map of the debugger breakpoints. breakpoints: Breakpoints, } @@ -28,6 +23,16 @@ impl DebuggerBuilder { Self::default() } + /// Configures the [DebugTraceIdentifier]. + #[inline] + pub fn identifier( + mut self, + f: impl FnOnce(DebugTraceIdentifierBuilder) -> DebugTraceIdentifierBuilder, + ) -> Self { + self.identifier = f(self.identifier); + self + } + /// Extends the debug arena. #[inline] pub fn debug_arenas(mut self, arena: &[DebugArena]) -> Self { @@ -44,39 +49,6 @@ impl DebuggerBuilder { self } - /// Extends the identified contracts from multiple decoders. - #[inline] - pub fn decoders(mut self, decoders: &[CallTraceDecoder]) -> Self { - for decoder in decoders { - self = self.decoder(decoder); - } - self - } - - /// Extends the identified contracts from a decoder. - #[inline] - pub fn decoder(self, decoder: &CallTraceDecoder) -> Self { - let c = decoder.contracts.iter().map(|(k, v)| (*k, get_contract_name(v).to_string())); - self.identified_contracts(c) - } - - /// Extends the identified contracts. - #[inline] - pub fn identified_contracts( - mut self, - identified_contracts: impl IntoIterator, - ) -> Self { - self.identified_contracts.extend(identified_contracts); - self - } - - /// Sets the sources for the debugger. - #[inline] - pub fn sources(mut self, sources: ContractSources) -> Self { - self.sources = sources; - self - } - /// Sets the breakpoints for the debugger. #[inline] pub fn breakpoints(mut self, breakpoints: Breakpoints) -> Self { @@ -87,7 +59,7 @@ impl DebuggerBuilder { /// Builds the debugger. #[inline] pub fn build(self) -> Debugger { - let Self { debug_arena, identified_contracts, sources, breakpoints } = self; - Debugger::new(debug_arena, identified_contracts, sources, breakpoints) + let Self { debug_arena, identifier, breakpoints } = self; + Debugger::new(debug_arena, identifier.build(), breakpoints) } } diff --git a/crates/debugger/src/tui/draw.rs b/crates/debugger/src/tui/draw.rs index 79533ad4caae..72bb7b9a41ff 100644 --- a/crates/debugger/src/tui/draw.rs +++ b/crates/debugger/src/tui/draw.rs @@ -3,9 +3,7 @@ use super::context::{BufferKind, DebuggerContext}; use crate::op::OpcodeParam; use alloy_primitives::U256; -use foundry_compilers::{ - artifacts::sourcemap::SourceElement, compilers::multi::MultiCompilerLanguage, -}; +use foundry_compilers::artifacts::sourcemap::SourceElement; use ratatui::{ layout::{Alignment, Constraint, Direction, Layout, Rect}, style::{Color, Modifier, Style}, @@ -342,69 +340,11 @@ impl DebuggerContext<'_> { /// Returns source map, source code and source name of the current line. fn src_map(&self) -> Result<(SourceElement, &str, &str), String> { - let address = self.address(); - let Some(contract_name) = self.debugger.identified_contracts.get(address) else { - return Err(format!("Unknown contract at address {address}")); - }; - - let Some(mut files_source_code) = - self.debugger.contracts_sources.get_sources(contract_name) - else { - return Err(format!("No source map index for contract {contract_name}")); - }; - - let Some((create_map, rt_map)) = self.debugger.pc_ic_maps.get(contract_name) else { - return Err(format!("No PC-IC maps for contract {contract_name}")); - }; - - let is_create = matches!(self.call_kind(), CallKind::Create | CallKind::Create2); - let pc = self.current_step().pc; - let Some((source_element, source_code, source_file)) = - files_source_code.find_map(|(artifact, source)| { - let bytecode = if is_create { - &artifact.bytecode.bytecode - } else { - artifact.bytecode.deployed_bytecode.bytecode.as_ref()? - }; - let source_map = bytecode.source_map()?.expect("failed to parse"); - - let pc_ic_map = if is_create { create_map } else { rt_map }; - let ic = pc_ic_map.get(pc)?; - - // Solc indexes source maps by instruction counter, but Vyper indexes by program - // counter. - let source_element = if matches!(source.language, MultiCompilerLanguage::Solc(_)) { - source_map.get(ic)? - } else { - source_map.get(pc)? - }; - // if the source element has an index, find the sourcemap for that index - let res = source_element - .index() - // if index matches current file_id, return current source code - .and_then(|index| { - (index == artifact.file_id) - .then(|| (source_element.clone(), source.source.as_str(), &source.name)) - }) - .or_else(|| { - // otherwise find the source code for the element's index - self.debugger - .contracts_sources - .sources_by_id - .get(&artifact.build_id)? - .get(&source_element.index()?) - .map(|source| { - (source_element.clone(), source.source.as_str(), &source.name) - }) - }); - - res - }) - else { - return Err(format!("No source map for contract {contract_name}")); - }; - - Ok((source_element, source_code, source_file)) + self.debugger.identifier.identify( + self.address(), + self.current_step().pc, + self.call_kind().is_any_create(), + ) } fn draw_op_list(&self, f: &mut Frame<'_>, area: Rect) { diff --git a/crates/debugger/src/tui/mod.rs b/crates/debugger/src/tui/mod.rs index c810440e5b13..e0b01237c666 100644 --- a/crates/debugger/src/tui/mod.rs +++ b/crates/debugger/src/tui/mod.rs @@ -1,20 +1,19 @@ //! The TUI implementation. -use alloy_primitives::Address; +use crate::DebugTraceIdentifier; use crossterm::{ event::{self, DisableMouseCapture, EnableMouseCapture, Event}, execute, terminal::{disable_raw_mode, enable_raw_mode, EnterAlternateScreen, LeaveAlternateScreen}, }; use eyre::Result; -use foundry_common::{compile::ContractSources, evm::Breakpoints}; -use foundry_evm_core::{debug::DebugNodeFlat, utils::PcIcMap}; +use foundry_common::evm::Breakpoints; +use foundry_evm_core::debug::DebugNodeFlat; use ratatui::{ backend::{Backend, CrosstermBackend}, Terminal, }; use std::{ - collections::{BTreeMap, HashMap}, io, ops::ControlFlow, sync::{mpsc, Arc}, @@ -42,11 +41,7 @@ pub enum ExitReason { /// The TUI debugger. pub struct Debugger { debug_arena: Vec, - identified_contracts: HashMap, - /// Source map of contract sources - contracts_sources: ContractSources, - /// A mapping of source -> (PC -> IC map for deploy code, PC -> IC map for runtime code) - pc_ic_maps: BTreeMap, + identifier: DebugTraceIdentifier, breakpoints: Breakpoints, } @@ -60,23 +55,10 @@ impl Debugger { /// Creates a new debugger. pub fn new( debug_arena: Vec, - identified_contracts: HashMap, - contracts_sources: ContractSources, + identifier: DebugTraceIdentifier, breakpoints: Breakpoints, ) -> Self { - let pc_ic_maps = contracts_sources - .entries() - .filter_map(|(name, artifact, _)| { - Some(( - name.to_owned(), - ( - PcIcMap::new(artifact.bytecode.bytecode.bytes()?), - PcIcMap::new(artifact.bytecode.deployed_bytecode.bytes()?), - ), - )) - }) - .collect(); - Self { debug_arena, identified_contracts, contracts_sources, pc_ic_maps, breakpoints } + Self { debug_arena, identifier, breakpoints } } /// Starts the debugger TUI. Terminates the current process on failure or user exit. diff --git a/crates/evm/evm/src/executors/invariant/replay.rs b/crates/evm/evm/src/executors/invariant/replay.rs index cf9fa12e81fc..18f0b5222a3d 100644 --- a/crates/evm/evm/src/executors/invariant/replay.rs +++ b/crates/evm/evm/src/executors/invariant/replay.rs @@ -33,7 +33,7 @@ pub fn replay_run( inputs: &[BasicTxDetails], ) -> Result> { // We want traces for a failed case. - executor.set_tracing(true); + executor.set_tracing(true, false); let mut counterexample_sequence = vec![]; diff --git a/crates/evm/evm/src/executors/mod.rs b/crates/evm/evm/src/executors/mod.rs index 0959f2e21584..fd354d9617b7 100644 --- a/crates/evm/evm/src/executors/mod.rs +++ b/crates/evm/evm/src/executors/mod.rs @@ -172,8 +172,8 @@ impl Executor { } #[inline] - pub fn set_tracing(&mut self, tracing: bool) -> &mut Self { - self.inspector.tracing(tracing); + pub fn set_tracing(&mut self, tracing: bool, debug: bool) -> &mut Self { + self.inspector.tracing(tracing, debug); self } diff --git a/crates/evm/evm/src/executors/tracing.rs b/crates/evm/evm/src/executors/tracing.rs index 08c5d92ef5b2..29373ad66865 100644 --- a/crates/evm/evm/src/executors/tracing.rs +++ b/crates/evm/evm/src/executors/tracing.rs @@ -16,13 +16,14 @@ impl TracingExecutor { fork: Option, version: Option, debug: bool, + trace_steps: bool, ) -> Self { let db = Backend::spawn(fork); Self { // configures a bare version of the evm executor: no cheatcode inspector is enabled, // tracing will be enabled only for the targeted transaction executor: ExecutorBuilder::new() - .inspectors(|stack| stack.trace(true).debug(debug)) + .inspectors(|stack| stack.trace(true).debug(debug).debug_trace(trace_steps)) .spec(evm_spec_id(&version.unwrap_or_default())) .build(env, db), } diff --git a/crates/evm/evm/src/inspectors/stack.rs b/crates/evm/evm/src/inspectors/stack.rs index 8e235efe83de..feec3ce5b52c 100644 --- a/crates/evm/evm/src/inspectors/stack.rs +++ b/crates/evm/evm/src/inspectors/stack.rs @@ -40,6 +40,8 @@ pub struct InspectorStackBuilder { pub fuzzer: Option, /// Whether to enable tracing. pub trace: Option, + /// Whether to enable steps tracking in the tracer. + pub debug_trace: Option, /// Whether to enable the debugger. pub debug: Option, /// Whether logs should be collected. @@ -133,6 +135,13 @@ impl InspectorStackBuilder { self } + /// Set whether to enable steps tracking in the tracer. + #[inline] + pub fn debug_trace(mut self, yes: bool) -> Self { + self.debug_trace = Some(yes); + self + } + /// Set whether to enable the call isolation. /// For description of call isolation, see [`InspectorStack::enable_isolation`]. #[inline] @@ -155,6 +164,7 @@ impl InspectorStackBuilder { print, chisel_state, enable_isolation, + debug_trace, } = self; let mut stack = InspectorStack::new(); @@ -172,7 +182,7 @@ impl InspectorStackBuilder { stack.collect_logs(logs.unwrap_or(true)); stack.enable_debugger(debug.unwrap_or(false)); stack.print(print.unwrap_or(false)); - stack.tracing(trace.unwrap_or(false)); + stack.tracing(trace.unwrap_or(false), debug_trace.unwrap_or(false)); stack.enable_isolation(enable_isolation); @@ -386,17 +396,21 @@ impl InspectorStack { /// Set whether to enable the tracer. #[inline] - pub fn tracing(&mut self, yes: bool) { - self.tracer = yes.then(|| { - TracingInspector::new(TracingInspectorConfig { - record_steps: false, - record_memory_snapshots: false, - record_stack_snapshots: StackSnapshotType::None, - record_state_diff: false, - exclude_precompile_calls: false, - record_logs: true, - }) - }); + pub fn tracing(&mut self, yes: bool, debug: bool) { + if self.tracer.is_none() && yes || + !self.tracer.as_ref().map_or(false, |t| t.config().record_steps) && debug + { + self.tracer = Some({ + TracingInspector::new(TracingInspectorConfig { + record_steps: debug, + record_memory_snapshots: false, + record_stack_snapshots: StackSnapshotType::None, + record_state_diff: false, + exclude_precompile_calls: false, + record_logs: true, + }) + }); + } } /// Collects all the data gathered during inspection into a single struct. diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index 729a1b638580..fbb795d9deb5 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -25,7 +25,7 @@ use identifier::{LocalTraceIdentifier, TraceIdentifier}; mod decoder; pub use decoder::{CallTraceDecoder, CallTraceDecoderBuilder}; -use revm_inspectors::tracing::types::LogCallOrder; +use revm_inspectors::tracing::types::TraceMemberOrder; pub use revm_inspectors::tracing::{ types::{CallKind, CallTrace, CallTraceNode}, CallTraceArena, GethTraceBuilder, ParityTraceBuilder, StackSnapshotType, TracingInspector, @@ -59,68 +59,132 @@ pub enum DecodedCallLog<'a> { Decoded(String, Vec<(String, String)>), } +#[derive(Debug, Clone)] +pub struct DecodedTraceStep<'a> { + pub start_step_idx: usize, + pub end_step_idx: usize, + pub function_name: &'a str, + pub gas_used: i64, +} + const PIPE: &str = " │ "; const EDGE: &str = " └─ "; const BRANCH: &str = " ├─ "; const CALL: &str = "→ "; const RETURN: &str = "← "; -/// Render a collection of call traces. -/// -/// The traces will be decoded using the given decoder, if possible. -pub async fn render_trace_arena( +pub async fn render_trace_arena_with_internals<'a>( arena: &CallTraceArena, decoder: &CallTraceDecoder, + identified_internals: &'a [Vec>], ) -> Result { decoder.prefetch_signatures(arena.nodes()).await; - fn inner<'a>( + fn render_items<'a>( arena: &'a [CallTraceNode], decoder: &'a CallTraceDecoder, + identified_internals: &'a [Vec>], s: &'a mut String, - idx: usize, + node_idx: usize, + mut ordering_idx: usize, + internal_end_step_idx: Option, left: &'a str, - child: &'a str, - ) -> BoxFuture<'a, Result<(), std::fmt::Error>> { + right: &'a str, + ) -> BoxFuture<'a, Result> { async move { - let node = &arena[idx]; + let node = &arena[node_idx]; - // Display trace header - let (trace, return_data) = render_trace(&node.trace, decoder).await?; - writeln!(s, "{left}{trace}")?; - - // Display logs and subcalls - let left_prefix = format!("{child}{BRANCH}"); - let right_prefix = format!("{child}{PIPE}"); - for child in &node.ordering { + while ordering_idx < node.ordering.len() { + let child = &node.ordering[ordering_idx]; match child { - LogCallOrder::Log(index) => { + TraceMemberOrder::Log(index) => { let log = render_trace_log(&node.logs[*index], decoder).await?; // Prepend our tree structure symbols to each line of the displayed log log.lines().enumerate().try_for_each(|(i, line)| { - writeln!( - s, - "{}{}", - if i == 0 { &left_prefix } else { &right_prefix }, - line - ) + writeln!(s, "{}{}", if i == 0 { left } else { right }, line) })?; } - LogCallOrder::Call(index) => { + TraceMemberOrder::Call(index) => { inner( arena, decoder, + identified_internals, s, node.children[*index], - &left_prefix, - &right_prefix, + left, + right, ) .await?; } + TraceMemberOrder::Step(step_idx) => { + if let Some(internal_step_end_idx) = internal_end_step_idx { + if *step_idx >= internal_step_end_idx { + return Ok(ordering_idx); + } + } + if let Some(decoded) = identified_internals[node_idx] + .iter() + .find(|d| *step_idx == d.start_step_idx) + { + writeln!(s, "{left}[{}] {}", decoded.gas_used, decoded.function_name)?; + let left_prefix = format!("{right}{BRANCH}"); + let right_prefix = format!("{right}{PIPE}"); + ordering_idx = render_items( + arena, + decoder, + identified_internals, + s, + node_idx, + ordering_idx + 1, + Some(decoded.end_step_idx), + &left_prefix, + &right_prefix, + ) + .await?; + + writeln!(s, "{right}{EDGE}{}", RETURN,)?; + } + } } + ordering_idx += 1; } + Ok(ordering_idx) + } + .boxed() + } + + fn inner<'a>( + arena: &'a [CallTraceNode], + decoder: &'a CallTraceDecoder, + identified_internals: &'a [Vec>], + s: &'a mut String, + idx: usize, + left: &'a str, + child: &'a str, + ) -> BoxFuture<'a, Result<(), std::fmt::Error>> { + async move { + let node = &arena[idx]; + + // Display trace header + let (trace, return_data) = render_trace(&node.trace, decoder).await?; + writeln!(s, "{left}{trace}")?; + + // Display logs and subcalls + render_items( + arena, + decoder, + identified_internals, + s, + idx, + 0, + None, + &format!("{child}{BRANCH}"), + &format!("{child}{PIPE}"), + ) + .await?; + // Display trace return data let color = trace_color(&node.trace); write!( @@ -145,10 +209,25 @@ pub async fn render_trace_arena( } let mut s = String::new(); - inner(arena.nodes(), decoder, &mut s, 0, " ", " ").await?; + inner(arena.nodes(), decoder, identified_internals, &mut s, 0, " ", " ").await?; Ok(s) } +/// Render a collection of call traces. +/// +/// The traces will be decoded using the given decoder, if possible. +pub async fn render_trace_arena( + arena: &CallTraceArena, + decoder: &CallTraceDecoder, +) -> Result { + render_trace_arena_with_internals( + arena, + decoder, + &std::iter::repeat(Vec::new()).take(arena.nodes().len()).collect::>(), + ) + .await +} + /// Render a call trace. /// /// The trace will be decoded using the given decoder, if possible. diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 6f65dcfec5b4..32316b0eb3fa 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -1,13 +1,16 @@ use super::{install, test::filter::ProjectPathsAwareFilter, watch::WatchArgs}; use alloy_primitives::U256; use clap::Parser; -use eyre::Result; +use eyre::{OptionExt, Result}; use forge::{ decode::decode_console_logs, gas_report::GasReport, multi_runner::matches_contract, result::{SuiteResult, TestOutcome, TestStatus}, - traces::{identifier::SignaturesIdentifier, CallTraceDecoderBuilder, TraceKind}, + traces::{ + identifier::SignaturesIdentifier, render_trace_arena_with_internals, + CallTraceDecoderBuilder, TraceKind, + }, MultiContractRunner, MultiContractRunnerBuilder, TestFilter, TestOptions, TestOptionsBuilder, }; use foundry_cli::{ @@ -32,7 +35,7 @@ use foundry_config::{ }, get_available_profiles, Config, }; -use foundry_debugger::Debugger; +use foundry_debugger::{DebugTraceIdentifier, Debugger}; use foundry_evm::traces::identifier::TraceIdentifiers; use regex::Regex; use std::{ @@ -76,6 +79,10 @@ pub struct TestArgs { #[arg(long, value_name = "TEST_FUNCTION")] debug: Option, + /// Whether to identify internal functions in traces. + #[arg(long)] + decode_internal: bool, + /// Print a gas report. #[arg(long, env = "FORGE_GAS_REPORT")] gas_report: bool, @@ -293,7 +300,7 @@ impl TestArgs { let env = evm_opts.evm_env().await?; // Prepare the test builder - let should_debug = self.debug.is_some(); + let should_debug = self.debug.is_some() || self.decode_internal; // Clone the output only if we actually need it later for the debugger. let output_clone = should_debug.then(|| output.clone()); @@ -341,16 +348,43 @@ impl TestArgs { Some(&libraries), )?; - // Run the debugger. - let mut builder = Debugger::builder() - .debug_arenas(test_result.debug.as_slice()) - .sources(sources) - .breakpoints(test_result.breakpoints.clone()); - if let Some(decoder) = &outcome.last_run_decoder { + if self.decode_internal { + let mut builder = DebugTraceIdentifier::builder().sources(sources); + let Some(decoder) = &outcome.last_run_decoder else { + eyre::bail!("Missing decoder for debugging"); + }; builder = builder.decoder(decoder); + + let identifier = builder.build(); + let arena = &test_result + .traces + .iter() + .find(|(t, _)| t.is_execution()) + .ok_or_eyre("didn't find execution trace for debugging")? + .1; + + let identified = identifier.identify_arena(arena); + + println!( + "{}", + render_trace_arena_with_internals(arena, &decoder, &identified).await? + ); + } else if self.debug.is_some() { + // Run the debugger. + let builder = Debugger::builder() + .debug_arenas(test_result.debug.as_slice()) + .identifier(|mut builder| { + if let Some(decoder) = &outcome.last_run_decoder { + builder = builder.decoder(decoder); + } + + builder.sources(sources) + }) + .breakpoints(test_result.breakpoints.clone()); + + let mut debugger = builder.build(); + debugger.try_run()?; } - let mut debugger = builder.build(); - debugger.try_run()?; } Ok(outcome) diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index c0bb78fa26b7..4b5bec604c0a 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -335,13 +335,13 @@ impl<'a> ContractRunner<'a> { let has_invariants = self.contract.abi.functions().any(|func| func.is_invariant_test()); let tmp_tracing = self.executor.inspector.tracer.is_none() && has_invariants && call_setup; if tmp_tracing { - self.executor.set_tracing(true); + self.executor.set_tracing(true, false); } let setup_time = Instant::now(); let setup = self.setup(call_setup); debug!("finished setting up in {:?}", setup_time.elapsed()); if tmp_tracing { - self.executor.set_tracing(false); + self.executor.set_tracing(false, false); } if setup.reason.is_some() { @@ -676,7 +676,7 @@ impl<'a> ContractRunner<'a> { let mut debug_executor = self.executor.clone(); // turn the debug traces on debug_executor.inspector.enable_debugger(true); - debug_executor.inspector.tracing(true); + debug_executor.inspector.tracing(true, true); let calldata = if let Some(counterexample) = result.counterexample.as_ref() { match counterexample { CounterExample::Single(ce) => ce.calldata.clone(), diff --git a/crates/script/src/execute.rs b/crates/script/src/execute.rs index 2b57468e276b..162b1bc9ba20 100644 --- a/crates/script/src/execute.rs +++ b/crates/script/src/execute.rs @@ -493,8 +493,10 @@ impl PreSimulationState { pub fn run_debugger(&self) -> Result<()> { let mut debugger = Debugger::builder() .debug_arenas(self.execution_result.debug.as_deref().unwrap_or_default()) - .decoder(&self.execution_artifacts.decoder) - .sources(self.build_data.sources.clone()) + .identifier(|b| { + b.decoder(&self.execution_artifacts.decoder) + .sources(self.build_data.sources.clone()) + }) .breakpoints(self.execution_result.breakpoints.clone()) .build(); debugger.try_run()?; diff --git a/crates/verify/src/bytecode.rs b/crates/verify/src/bytecode.rs index cc7a733995e7..80be4f82493a 100644 --- a/crates/verify/src/bytecode.rs +++ b/crates/verify/src/bytecode.rs @@ -284,7 +284,7 @@ impl VerifyBytecodeArgs { TracingExecutor::get_fork_material(&fork_config, evm_opts).await?; let mut executor = - TracingExecutor::new(env.clone(), fork, Some(fork_config.evm_version), false); + TracingExecutor::new(env.clone(), fork, Some(fork_config.evm_version), false, false); env.block.number = U256::from(simulation_block); let block = provider.get_block(simulation_block.into(), true.into()).await?; From 83c7a23f167becdd36695a53962daaecc63ae6a6 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 21 Jun 2024 04:36:53 +0300 Subject: [PATCH 03/30] fmt --- crates/cast/bin/cmd/run.rs | 3 ++- crates/cli/src/utils/cmd.rs | 15 +++++++++++---- crates/evm/traces/src/lib.rs | 2 +- crates/forge/bin/cmd/test/mod.rs | 2 +- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/cast/bin/cmd/run.rs b/crates/cast/bin/cmd/run.rs index 70b27313d6e3..b7f31cfc7514 100644 --- a/crates/cast/bin/cmd/run.rs +++ b/crates/cast/bin/cmd/run.rs @@ -146,7 +146,8 @@ impl RunArgs { } } - let mut executor = TracingExecutor::new(env.clone(), fork, evm_version, self.debug, self.decode_internal); + let mut executor = + TracingExecutor::new(env.clone(), fork, evm_version, self.debug, self.decode_internal); let mut env = EnvWithHandlerCfg::new_with_spec_id(Box::new(env.clone()), executor.spec_id()); diff --git a/crates/cli/src/utils/cmd.rs b/crates/cli/src/utils/cmd.rs index 860f1879d0d2..341494b46c03 100644 --- a/crates/cli/src/utils/cmd.rs +++ b/crates/cli/src/utils/cmd.rs @@ -15,7 +15,9 @@ use foundry_evm::{ executors::{DeployResult, EvmError, RawCallResult}, opts::EvmOpts, traces::{ - identifier::{EtherscanIdentifier, SignaturesIdentifier}, render_trace_arena, render_trace_arena_with_internals, CallTraceDecoder, CallTraceDecoderBuilder, TraceKind, Traces + identifier::{EtherscanIdentifier, SignaturesIdentifier}, + render_trace_arena, render_trace_arena_with_internals, CallTraceDecoder, + CallTraceDecoderBuilder, TraceKind, Traces, }, }; use std::{ @@ -412,17 +414,22 @@ pub async fn handle_traces( Ok(()) } -pub async fn print_traces(result: &mut TraceResult, decoder: &CallTraceDecoder, identifier: Option<&DebugTraceIdentifier>) -> Result<()> { +pub async fn print_traces( + result: &mut TraceResult, + decoder: &CallTraceDecoder, + identifier: Option<&DebugTraceIdentifier>, +) -> Result<()> { let traces = result.traces.as_ref().expect("No traces found"); println!("Traces:"); for (_, arena) in traces { let arena = if let Some(identifier) = identifier { - render_trace_arena_with_internals(arena, decoder, &identifier.identify_arena(arena)).await? + render_trace_arena_with_internals(arena, decoder, &identifier.identify_arena(arena)) + .await? } else { render_trace_arena(arena, decoder).await? }; - println!("{}", arena); + println!("{arena}"); } println!(); diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index fbb795d9deb5..27741d157b61 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -143,7 +143,7 @@ pub async fn render_trace_arena_with_internals<'a>( ) .await?; - writeln!(s, "{right}{EDGE}{}", RETURN,)?; + writeln!(s, "{right}{EDGE}{RETURN}")?; } } } diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 32316b0eb3fa..15f8bdd64cfb 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -367,7 +367,7 @@ impl TestArgs { println!( "{}", - render_trace_arena_with_internals(arena, &decoder, &identified).await? + render_trace_arena_with_internals(arena, decoder, &identified).await? ); } else if self.debug.is_some() { // Run the debugger. From b1a365f07350c2c2a30db56566d91c1ff0dc328f Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 21 Jun 2024 04:38:21 +0300 Subject: [PATCH 04/30] doc --- crates/debugger/src/tui/builder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/debugger/src/tui/builder.rs b/crates/debugger/src/tui/builder.rs index ea80335a39b9..0b2bb022990f 100644 --- a/crates/debugger/src/tui/builder.rs +++ b/crates/debugger/src/tui/builder.rs @@ -10,7 +10,7 @@ use foundry_evm_core::debug::{DebugArena, DebugNodeFlat}; pub struct DebuggerBuilder { /// Debug traces returned from the EVM execution. debug_arena: Vec, - /// Builder for [DebugTraceIdentifier]. + /// Builder for [crate::DebugTraceIdentifier]. identifier: DebugTraceIdentifierBuilder, /// Map of the debugger breakpoints. breakpoints: Breakpoints, @@ -23,7 +23,7 @@ impl DebuggerBuilder { Self::default() } - /// Configures the [DebugTraceIdentifier]. + /// Configures the [crate::DebugTraceIdentifier]. #[inline] pub fn identifier( mut self, From 2d17d370f7d35edac4c862c6c1e07af3d40eab25 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 21 Jun 2024 06:25:37 +0300 Subject: [PATCH 05/30] correctly enable tracing --- crates/forge/bin/cmd/test/mod.rs | 1 + crates/forge/src/multi_runner.rs | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 15f8bdd64cfb..3bf3d381d8f3 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -309,6 +309,7 @@ impl TestArgs { let runner = MultiContractRunnerBuilder::new(config.clone()) .set_debug(should_debug) + .set_trace_steps(self.decode_internal) .initial_balance(evm_opts.initial_balance) .evm_spec(config.evm_spec_id()) .sender(evm_opts.sender) diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 270a25202a8e..8b24426de76f 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -60,6 +60,8 @@ pub struct MultiContractRunner { pub coverage: bool, /// Whether to collect debug info pub debug: bool, + /// Whether to enable steps tracking in the tracer. + pub trace_steps: bool, /// Settings related to fuzz and/or invariant tests pub test_options: TestOptions, /// Whether to enable call isolation @@ -240,8 +242,9 @@ impl MultiContractRunner { .inspectors(|stack| { stack .cheatcodes(Arc::new(cheats_config)) - .trace(self.evm_opts.verbosity >= 3 || self.debug) + .trace(self.evm_opts.verbosity >= 3 || self.debug || self.trace_steps) .debug(self.debug) + .debug_trace(self.trace_steps) .coverage(self.coverage) .enable_isolation(self.isolation) }) @@ -295,6 +298,8 @@ pub struct MultiContractRunnerBuilder { pub coverage: bool, /// Whether or not to collect debug info pub debug: bool, + /// Whether to enable steps tracking in the tracer. + pub trace_steps: bool, /// Whether to enable call isolation pub isolation: bool, /// Settings related to fuzz and/or invariant tests @@ -313,6 +318,7 @@ impl MultiContractRunnerBuilder { debug: Default::default(), isolation: Default::default(), test_options: Default::default(), + trace_steps: Default::default(), } } @@ -351,6 +357,11 @@ impl MultiContractRunnerBuilder { self } + pub fn set_trace_steps(mut self, enable: bool) -> Self { + self.trace_steps = enable; + self + } + pub fn enable_isolation(mut self, enable: bool) -> Self { self.isolation = enable; self @@ -418,6 +429,7 @@ impl MultiContractRunnerBuilder { config: self.config, coverage: self.coverage, debug: self.debug, + trace_steps: self.trace_steps, test_options: self.test_options.unwrap_or_default(), isolation: self.isolation, known_contracts, From 4728d2ed554d6fdb88fb9beb6238b802a464edf2 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 21 Jun 2024 06:25:37 +0300 Subject: [PATCH 06/30] correctly enable tracing --- Cargo.lock | 7 +- Cargo.toml | 3 - crates/common/Cargo.toml | 1 - crates/common/src/compile.rs | 138 +------------- crates/debugger/src/identifier.rs | 42 ++--- crates/evm/core/src/ic.rs | 1 + crates/evm/traces/Cargo.toml | 1 + crates/evm/traces/src/identifier/etherscan.rs | 4 +- crates/evm/traces/src/identifier/mod.rs | 3 + crates/evm/traces/src/identifier/sources.rs | 175 ++++++++++++++++++ crates/forge/bin/cmd/test/mod.rs | 10 +- crates/script/src/build.rs | 6 +- 12 files changed, 209 insertions(+), 182 deletions(-) create mode 100644 crates/evm/traces/src/identifier/sources.rs diff --git a/Cargo.lock b/Cargo.lock index 6d59c0b84905..8a8542f7d00e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3656,7 +3656,6 @@ dependencies = [ "foundry-block-explorers", "foundry-compilers", "foundry-config", - "foundry-linking", "foundry-macros", "num-format", "once_cell", @@ -3957,6 +3956,7 @@ dependencies = [ "foundry-compilers", "foundry-config", "foundry-evm-core", + "foundry-linking", "futures", "itertools 0.13.0", "once_cell", @@ -6944,8 +6944,9 @@ dependencies = [ [[package]] name = "revm-inspectors" -version = "0.1.0" -source = "git+https://github.com/klkvr/evm-inspectors?rev=7b649e5#7b649e5ca690f94bd6bbd9f8bcc655bc282a16b6" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b0971cad2f8f1ecb10e270d80646e63bf19daef0dc0a17a45680d24bb346b7c" dependencies = [ "alloy-primitives", "alloy-rpc-types", diff --git a/Cargo.toml b/Cargo.toml index fa1ccc3fc9f5..2d44d6d671d1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -259,6 +259,3 @@ tower = "0.4" tower-http = "0.5" # soldeer soldeer = "0.2.15" - -[patch.crates-io] -revm-inspectors = { git = "https://github.com/klkvr/evm-inspectors", rev = "7b649e5" } diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index f50435610ae6..df8741cd112c 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -16,7 +16,6 @@ workspace = true foundry-block-explorers = { workspace = true, features = ["foundry-compilers"] } foundry-compilers.workspace = true foundry-config.workspace = true -foundry-linking.workspace = true alloy-consensus.workspace = true alloy-contract.workspace = true diff --git a/crates/common/src/compile.rs b/crates/common/src/compile.rs index 3be317e51cca..468bc8ecb68a 100644 --- a/crates/common/src/compile.rs +++ b/crates/common/src/compile.rs @@ -1,28 +1,24 @@ //! Support for compiling [foundry_compilers::Project] -use crate::{compact_to_contract, term::SpinnerReporter, TestFunctionExt}; +use crate::{term::SpinnerReporter, TestFunctionExt}; use comfy_table::{presets::ASCII_MARKDOWN, Attribute, Cell, CellAlignment, Color, Table}; -use eyre::{Context, Result}; +use eyre::Result; use foundry_block_explorers::contract::Metadata; use foundry_compilers::{ - artifacts::{remappings::Remapping, BytecodeObject, ContractBytecodeSome, Libraries, Source}, + artifacts::{remappings::Remapping, BytecodeObject, Source}, compilers::{ - multi::MultiCompilerLanguage, solc::{Solc, SolcCompiler}, Compiler, }, report::{BasicStdoutReporter, NoReporter, Report}, Artifact, Project, ProjectBuilder, ProjectCompileOutput, ProjectPathsConfig, SolcConfig, }; -use foundry_linking::Linker; use num_format::{Locale, ToFormattedString}; -use rustc_hash::FxHashMap; use std::{ - collections::{BTreeMap, HashMap}, + collections::BTreeMap, fmt::Display, io::IsTerminal, path::{Path, PathBuf}, - sync::Arc, time::Instant, }; @@ -261,132 +257,6 @@ impl ProjectCompiler { } } -#[derive(Clone, Debug)] -pub struct SourceData { - pub source: Arc, - pub language: MultiCompilerLanguage, - pub name: String, -} - -#[derive(Clone, Debug)] -pub struct ArtifactData { - pub bytecode: ContractBytecodeSome, - pub build_id: String, - pub file_id: u32, -} - -/// Contract source code and bytecode data used for debugger. -#[derive(Clone, Debug, Default)] -pub struct ContractSources { - /// Map over build_id -> file_id -> (source code, language) - pub sources_by_id: HashMap>, - /// Map over contract name -> Vec<(bytecode, build_id, file_id)> - pub artifacts_by_name: HashMap>, -} - -impl ContractSources { - /// Collects the contract sources and artifacts from the project compile output. - pub fn from_project_output( - output: &ProjectCompileOutput, - root: impl AsRef, - libraries: Option<&Libraries>, - ) -> Result { - let mut sources = Self::default(); - - sources.insert(output, root, libraries)?; - - Ok(sources) - } - - pub fn insert( - &mut self, - output: &ProjectCompileOutput, - root: impl AsRef, - libraries: Option<&Libraries>, - ) -> Result<()> - where - C::Language: Into, - { - let root = root.as_ref(); - let link_data = libraries.map(|libraries| { - let linker = Linker::new(root, output.artifact_ids().collect()); - (linker, libraries) - }); - - for (id, artifact) in output.artifact_ids() { - if let Some(file_id) = artifact.id { - let artifact = if let Some((linker, libraries)) = link_data.as_ref() { - linker.link(&id, libraries)?.into_contract_bytecode() - } else { - artifact.clone().into_contract_bytecode() - }; - let bytecode = compact_to_contract(artifact.clone().into_contract_bytecode())?; - - self.artifacts_by_name.entry(id.name.clone()).or_default().push(ArtifactData { - bytecode, - build_id: id.build_id.clone(), - file_id, - }); - } else { - warn!(id = id.identifier(), "source not found"); - } - } - - // Not all source files produce artifacts, so we are populating sources by using build - // infos. - let mut files: BTreeMap> = BTreeMap::new(); - for (build_id, build) in output.builds() { - for (source_id, path) in &build.source_id_to_path { - let source_code = if let Some(source) = files.get(path) { - source.clone() - } else { - let source = Source::read(path).wrap_err_with(|| { - format!("failed to read artifact source file for `{}`", path.display()) - })?; - files.insert(path.clone(), source.content.clone()); - source.content - }; - - self.sources_by_id.entry(build_id.clone()).or_default().insert( - *source_id, - SourceData { - source: source_code, - language: build.language.into(), - name: path.strip_prefix(root).unwrap_or(path).to_string_lossy().to_string(), - }, - ); - } - } - - Ok(()) - } - - /// Returns all sources for a contract by name. - pub fn get_sources( - &self, - name: &str, - ) -> Option> { - self.artifacts_by_name.get(name).map(|artifacts| { - artifacts.iter().filter_map(|artifact| { - let source = - self.sources_by_id.get(artifact.build_id.as_str())?.get(&artifact.file_id)?; - Some((artifact, source)) - }) - }) - } - - /// Returns all (name, bytecode, source) sets. - pub fn entries(&self) -> impl Iterator { - self.artifacts_by_name.iter().flat_map(|(name, artifacts)| { - artifacts.iter().filter_map(|artifact| { - let source = - self.sources_by_id.get(artifact.build_id.as_str())?.get(&artifact.file_id)?; - Some((name.as_str(), artifact, source)) - }) - }) - } -} - // https://eips.ethereum.org/EIPS/eip-170 const CONTRACT_SIZE_LIMIT: usize = 24576; diff --git a/crates/debugger/src/identifier.rs b/crates/debugger/src/identifier.rs index 43d041961051..4b1278c66864 100644 --- a/crates/debugger/src/identifier.rs +++ b/crates/debugger/src/identifier.rs @@ -1,11 +1,12 @@ use alloy_primitives::Address; -use foundry_common::{compile::ContractSources, get_contract_name}; +use foundry_common::get_contract_name; use foundry_compilers::{ artifacts::sourcemap::{Jump, SourceElement}, multi::MultiCompilerLanguage, }; -use foundry_evm_core::utils::PcIcMap; -use foundry_evm_traces::{CallTraceArena, CallTraceDecoder, CallTraceNode, DecodedTraceStep}; +use foundry_evm_traces::{ + identifier::ContractSources, CallTraceArena, CallTraceDecoder, CallTraceNode, DecodedTraceStep, +}; use revm::interpreter::OpCode; use std::collections::HashMap; @@ -14,8 +15,6 @@ pub struct DebugTraceIdentifier { identified_contracts: HashMap, /// Source map of contract sources contracts_sources: ContractSources, - /// A mapping of source -> (PC -> IC map for deploy code, PC -> IC map for runtime code) - pc_ic_maps: HashMap, } impl DebugTraceIdentifier { @@ -27,19 +26,7 @@ impl DebugTraceIdentifier { identified_contracts: HashMap, contracts_sources: ContractSources, ) -> Self { - let pc_ic_maps = contracts_sources - .entries() - .filter_map(|(name, artifact, _)| { - Some(( - name.to_owned(), - ( - PcIcMap::new(artifact.bytecode.bytecode.bytes()?), - PcIcMap::new(artifact.bytecode.deployed_bytecode.bytes()?), - ), - )) - }) - .collect(); - Self { identified_contracts, contracts_sources, pc_ic_maps } + Self { identified_contracts, contracts_sources } } pub fn identify( @@ -56,20 +43,19 @@ impl DebugTraceIdentifier { return Err(format!("No source map index for contract {contract_name}")); }; - let Some((create_map, rt_map)) = self.pc_ic_maps.get(contract_name) else { - return Err(format!("No PC-IC maps for contract {contract_name}")); - }; - let Some((source_element, source_code, source_file)) = files_source_code.find_map(|(artifact, source)| { - let bytecode = if init_code { - &artifact.bytecode.bytecode + let source_map = if init_code { + artifact.source_map.as_ref() } else { - artifact.bytecode.deployed_bytecode.bytecode.as_ref()? - }; - let source_map = bytecode.source_map()?.expect("failed to parse"); + artifact.source_map_runtime.as_ref() + }?; - let pc_ic_map = if init_code { create_map } else { rt_map }; + let pc_ic_map = if init_code { + artifact.pc_ic_map.as_ref() + } else { + artifact.pc_ic_map_runtime.as_ref() + }?; let ic = pc_ic_map.get(pc)?; // Solc indexes source maps by instruction counter, but Vyper indexes by program diff --git a/crates/evm/core/src/ic.rs b/crates/evm/core/src/ic.rs index c2792ab87ba4..acb9cc50e6b3 100644 --- a/crates/evm/core/src/ic.rs +++ b/crates/evm/core/src/ic.rs @@ -4,6 +4,7 @@ use rustc_hash::FxHashMap; /// Maps from program counter to instruction counter. /// /// Inverse of [`IcPcMap`]. +#[derive(Debug, Clone)] pub struct PcIcMap { pub inner: FxHashMap, } diff --git a/crates/evm/traces/Cargo.toml b/crates/evm/traces/Cargo.toml index b076db127821..a6c4f6688723 100644 --- a/crates/evm/traces/Cargo.toml +++ b/crates/evm/traces/Cargo.toml @@ -17,6 +17,7 @@ workspace = true foundry-block-explorers.workspace = true foundry-common.workspace = true foundry-compilers.workspace = true +foundry-linking.workspace = true foundry-config.workspace = true foundry-evm-core.workspace = true diff --git a/crates/evm/traces/src/identifier/etherscan.rs b/crates/evm/traces/src/identifier/etherscan.rs index 54921beffcd8..91e0a48c692f 100644 --- a/crates/evm/traces/src/identifier/etherscan.rs +++ b/crates/evm/traces/src/identifier/etherscan.rs @@ -1,10 +1,10 @@ -use super::{AddressIdentity, TraceIdentifier}; +use super::{AddressIdentity, ContractSources, TraceIdentifier}; use alloy_primitives::Address; use foundry_block_explorers::{ contract::{ContractMetadata, Metadata}, errors::EtherscanError, }; -use foundry_common::compile::{etherscan_project, ContractSources}; +use foundry_common::compile::etherscan_project; use foundry_config::{Chain, Config}; use futures::{ future::{join_all, Future}, diff --git a/crates/evm/traces/src/identifier/mod.rs b/crates/evm/traces/src/identifier/mod.rs index a16b108d8537..5a4a879c8684 100644 --- a/crates/evm/traces/src/identifier/mod.rs +++ b/crates/evm/traces/src/identifier/mod.rs @@ -14,6 +14,9 @@ pub use etherscan::EtherscanIdentifier; mod signatures; pub use signatures::{SignaturesIdentifier, SingleSignaturesIdentifier}; +mod sources; +pub use sources::ContractSources; + /// An address identity pub struct AddressIdentity<'a> { /// The address this identity belongs to diff --git a/crates/evm/traces/src/identifier/sources.rs b/crates/evm/traces/src/identifier/sources.rs new file mode 100644 index 000000000000..13920f325682 --- /dev/null +++ b/crates/evm/traces/src/identifier/sources.rs @@ -0,0 +1,175 @@ +use std::{ + collections::{BTreeMap, HashMap}, + path::{Path, PathBuf}, + sync::Arc, +}; + +use eyre::{Context, Result}; +use foundry_common::compact_to_contract; +use foundry_compilers::{ + artifacts::{sourcemap::SourceMap, Bytecode, ContractBytecodeSome, Libraries, Source}, + multi::MultiCompilerLanguage, + Artifact, Compiler, ProjectCompileOutput, +}; +use foundry_evm_core::utils::PcIcMap; +use foundry_linking::Linker; +use rustc_hash::FxHashMap; + +#[derive(Clone, Debug)] +pub struct SourceData { + pub source: Arc, + pub language: MultiCompilerLanguage, + pub name: String, +} + +#[derive(Clone, Debug)] +pub struct ArtifactData { + pub source_map: Option, + pub source_map_runtime: Option, + pub pc_ic_map: Option, + pub pc_ic_map_runtime: Option, + pub build_id: String, + pub file_id: u32, +} + +impl ArtifactData { + fn new(bytecode: ContractBytecodeSome, build_id: String, file_id: u32) -> Result { + let parse = |b: &Bytecode| { + // Only parse source map if it's not empty. + let source_map = if b.source_map.as_ref().map_or(true, |s| s.is_empty()) { + Ok(None) + } else { + b.source_map().transpose() + }; + + // Only parse bytecode if it's not empty. + let pc_ic_map = if let Some(bytes) = b.bytes() { + (!bytes.is_empty()).then(|| PcIcMap::new(bytes)) + } else { + None + }; + + source_map.map(|source_map| (source_map, pc_ic_map)) + }; + let (source_map, pc_ic_map) = parse(&bytecode.bytecode)?; + let (source_map_runtime, pc_ic_map_runtime) = bytecode + .deployed_bytecode + .bytecode + .map(|b| parse(&b)) + .unwrap_or_else(|| Ok((None, None)))?; + + Ok(Self { source_map, source_map_runtime, pc_ic_map, pc_ic_map_runtime, build_id, file_id }) + } +} + +/// Container with artifacts data useful for identifying individual execution steps. +#[derive(Clone, Debug, Default)] +pub struct ContractSources { + /// Map over build_id -> file_id -> (source code, language) + pub sources_by_id: HashMap>, + /// Map over contract name -> Vec<(bytecode, build_id, file_id)> + pub artifacts_by_name: HashMap>, +} + +impl ContractSources { + /// Collects the contract sources and artifacts from the project compile output. + pub fn from_project_output( + output: &ProjectCompileOutput, + root: impl AsRef, + libraries: Option<&Libraries>, + ) -> Result { + let mut sources = Self::default(); + + sources.insert(output, root, libraries)?; + + Ok(sources) + } + + pub fn insert( + &mut self, + output: &ProjectCompileOutput, + root: impl AsRef, + libraries: Option<&Libraries>, + ) -> Result<()> + where + C::Language: Into, + { + let root = root.as_ref(); + let link_data = libraries.map(|libraries| { + let linker = Linker::new(root, output.artifact_ids().collect()); + (linker, libraries) + }); + + for (id, artifact) in output.artifact_ids() { + if let Some(file_id) = artifact.id { + let artifact = if let Some((linker, libraries)) = link_data.as_ref() { + linker.link(&id, libraries)?.into_contract_bytecode() + } else { + artifact.clone().into_contract_bytecode() + }; + let bytecode = compact_to_contract(artifact.clone().into_contract_bytecode())?; + + self.artifacts_by_name.entry(id.name.clone()).or_default().push(ArtifactData::new( + bytecode, + id.build_id.clone(), + file_id, + )?); + } else { + warn!(id = id.identifier(), "source not found"); + } + } + + // Not all source files produce artifacts, so we are populating sources by using build + // infos. + let mut files: BTreeMap> = BTreeMap::new(); + for (build_id, build) in output.builds() { + for (source_id, path) in &build.source_id_to_path { + let source_code = if let Some(source) = files.get(path) { + source.clone() + } else { + let source = Source::read(path).wrap_err_with(|| { + format!("failed to read artifact source file for `{}`", path.display()) + })?; + files.insert(path.clone(), source.content.clone()); + source.content + }; + + self.sources_by_id.entry(build_id.clone()).or_default().insert( + *source_id, + SourceData { + source: source_code, + language: build.language.into(), + name: path.strip_prefix(root).unwrap_or(path).to_string_lossy().to_string(), + }, + ); + } + } + + Ok(()) + } + + /// Returns all sources for a contract by name. + pub fn get_sources( + &self, + name: &str, + ) -> Option> { + self.artifacts_by_name.get(name).map(|artifacts| { + artifacts.iter().filter_map(|artifact| { + let source = + self.sources_by_id.get(artifact.build_id.as_str())?.get(&artifact.file_id)?; + Some((artifact, source)) + }) + }) + } + + /// Returns all (name, bytecode, source) sets. + pub fn entries(&self) -> impl Iterator { + self.artifacts_by_name.iter().flat_map(|(name, artifacts)| { + artifacts.iter().filter_map(|artifact| { + let source = + self.sources_by_id.get(artifact.build_id.as_str())?.get(&artifact.file_id)?; + Some((name.as_str(), artifact, source)) + }) + }) + } +} diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 3bf3d381d8f3..c5bdf55a9e21 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -8,8 +8,8 @@ use forge::{ multi_runner::matches_contract, result::{SuiteResult, TestOutcome, TestStatus}, traces::{ - identifier::SignaturesIdentifier, render_trace_arena_with_internals, - CallTraceDecoderBuilder, TraceKind, + identifier::{ContractSources, SignaturesIdentifier}, + render_trace_arena_with_internals, CallTraceDecoderBuilder, TraceKind, }, MultiContractRunner, MultiContractRunnerBuilder, TestFilter, TestOptions, TestOptionsBuilder, }; @@ -17,11 +17,7 @@ use foundry_cli::{ opts::CoreBuildArgs, utils::{self, LoadConfig}, }; -use foundry_common::{ - compile::{ContractSources, ProjectCompiler}, - evm::EvmArgs, - shell, -}; +use foundry_common::{compile::ProjectCompiler, evm::EvmArgs, shell}; use foundry_compilers::{ artifacts::output_selection::OutputSelection, compilers::{multi::MultiCompilerLanguage, CompilerSettings, Language}, diff --git a/crates/script/src/build.rs b/crates/script/src/build.rs index 48242ca19b02..81d22b020494 100644 --- a/crates/script/src/build.rs +++ b/crates/script/src/build.rs @@ -10,9 +10,7 @@ use alloy_provider::Provider; use eyre::{OptionExt, Result}; use foundry_cheatcodes::ScriptWallets; use foundry_common::{ - compile::{ContractSources, ProjectCompiler}, - provider::try_get_http_provider, - ContractData, ContractsByArtifact, + compile::ProjectCompiler, provider::try_get_http_provider, ContractData, ContractsByArtifact, }; use foundry_compilers::{ artifacts::{BytecodeObject, Libraries}, @@ -21,7 +19,7 @@ use foundry_compilers::{ utils::source_files_iter, ArtifactId, ProjectCompileOutput, }; -use foundry_evm::constants::DEFAULT_CREATE2_DEPLOYER; +use foundry_evm::{constants::DEFAULT_CREATE2_DEPLOYER, traces::identifier::ContractSources}; use foundry_linking::Linker; use std::{path::PathBuf, str::FromStr, sync::Arc}; From 5ed1abf0b5d86e2a3b7d32c0b5973532b6345589 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Sat, 22 Jun 2024 23:08:30 +0300 Subject: [PATCH 07/30] collect contract definition locs --- Cargo.lock | 2 + crates/debugger/src/identifier.rs | 8 +- crates/debugger/src/tui/draw.rs | 6 +- crates/evm/traces/Cargo.toml | 2 + crates/evm/traces/src/identifier/sources.rs | 137 ++++++++++++++------ 5 files changed, 111 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8a8542f7d00e..43df21005414 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3960,9 +3960,11 @@ dependencies = [ "futures", "itertools 0.13.0", "once_cell", + "rayon", "revm-inspectors", "rustc-hash", "serde", + "solang-parser", "tempfile", "tokio", "tracing", diff --git a/crates/debugger/src/identifier.rs b/crates/debugger/src/identifier.rs index 4b1278c66864..8b261f38406d 100644 --- a/crates/debugger/src/identifier.rs +++ b/crates/debugger/src/identifier.rs @@ -8,7 +8,7 @@ use foundry_evm_traces::{ identifier::ContractSources, CallTraceArena, CallTraceDecoder, CallTraceNode, DecodedTraceStep, }; use revm::interpreter::OpCode; -use std::collections::HashMap; +use std::{collections::HashMap, path::Path}; pub struct DebugTraceIdentifier { /// Mapping of contract address to identified contract name. @@ -34,7 +34,7 @@ impl DebugTraceIdentifier { address: &Address, pc: usize, init_code: bool, - ) -> core::result::Result<(SourceElement, &str, &str), String> { + ) -> core::result::Result<(SourceElement, &str, &Path), String> { let Some(contract_name) = self.identified_contracts.get(address) else { return Err(format!("Unknown contract at address {address}")); }; @@ -71,7 +71,7 @@ impl DebugTraceIdentifier { // if index matches current file_id, return current source code .and_then(|index| { (index == artifact.file_id) - .then(|| (source_element.clone(), source.source.as_str(), &source.name)) + .then(|| (source_element.clone(), source.source.as_str(), &source.path)) }) .or_else(|| { // otherwise find the source code for the element's index @@ -80,7 +80,7 @@ impl DebugTraceIdentifier { .get(&artifact.build_id)? .get(&source_element.index()?) .map(|source| { - (source_element.clone(), source.source.as_str(), &source.name) + (source_element.clone(), source.source.as_str(), &source.path) }) }); diff --git a/crates/debugger/src/tui/draw.rs b/crates/debugger/src/tui/draw.rs index 72bb7b9a41ff..d2182720e779 100644 --- a/crates/debugger/src/tui/draw.rs +++ b/crates/debugger/src/tui/draw.rs @@ -13,7 +13,7 @@ use ratatui::{ }; use revm::interpreter::opcode; use revm_inspectors::tracing::types::CallKind; -use std::{collections::VecDeque, fmt::Write, io}; +use std::{collections::VecDeque, fmt::Write, io, path::Path}; impl DebuggerContext<'_> { /// Draws the TUI layout and subcomponents to the given terminal. @@ -335,11 +335,11 @@ impl DebuggerContext<'_> { } } - (Text::from(lines.lines), Some(source_file)) + (Text::from(lines.lines), source_file.to_str()) } /// Returns source map, source code and source name of the current line. - fn src_map(&self) -> Result<(SourceElement, &str, &str), String> { + fn src_map(&self) -> Result<(SourceElement, &str, &Path), String> { self.debugger.identifier.identify( self.address(), self.current_step().pc, diff --git a/crates/evm/traces/Cargo.toml b/crates/evm/traces/Cargo.toml index a6c4f6688723..a9c7e395812d 100644 --- a/crates/evm/traces/Cargo.toml +++ b/crates/evm/traces/Cargo.toml @@ -43,6 +43,8 @@ tracing.workspace = true yansi.workspace = true rustc-hash.workspace = true tempfile.workspace = true +rayon.workspace = true +solang-parser.workspace = true [dev-dependencies] tempfile.workspace = true diff --git a/crates/evm/traces/src/identifier/sources.rs b/crates/evm/traces/src/identifier/sources.rs index 13920f325682..e4c41a81ed31 100644 --- a/crates/evm/traces/src/identifier/sources.rs +++ b/crates/evm/traces/src/identifier/sources.rs @@ -1,9 +1,3 @@ -use std::{ - collections::{BTreeMap, HashMap}, - path::{Path, PathBuf}, - sync::Arc, -}; - use eyre::{Context, Result}; use foundry_common::compact_to_contract; use foundry_compilers::{ @@ -13,13 +7,62 @@ use foundry_compilers::{ }; use foundry_evm_core::utils::PcIcMap; use foundry_linking::Linker; +use rayon::prelude::*; use rustc_hash::FxHashMap; +use solang_parser::pt::SourceUnitPart; +use std::{ + collections::{BTreeMap, HashMap}, + path::{Path, PathBuf}, + sync::Arc, +}; #[derive(Clone, Debug)] pub struct SourceData { pub source: Arc, pub language: MultiCompilerLanguage, - pub name: String, + pub path: PathBuf, + /// Maps contract name to (start, end) of the contract definition in the source code. + /// This is useful for determining which contract contains given function definition. + pub contract_definitions: HashMap, +} + +impl SourceData { + pub fn new(source: Arc, language: MultiCompilerLanguage, path: PathBuf) -> Self { + let mut contract_definitions = HashMap::new(); + + match language { + MultiCompilerLanguage::Vyper(_) => { + // Vyper contracts have the same name as the file name. + if let Some(name) = path.file_name().map(|s| s.to_string_lossy().to_string()) { + contract_definitions.insert(name, (0, source.len())); + } + } + MultiCompilerLanguage::Solc(_) => { + if let Ok((parsed, _)) = solang_parser::parse(&source, 0) { + for item in parsed.0 { + let SourceUnitPart::ContractDefinition(contract) = item else { + continue; + }; + let Some(name) = contract.name else { + continue; + }; + contract_definitions + .insert(name.name, (name.loc.start(), contract.loc.end())); + } + } + } + } + + Self { source, language, path, contract_definitions } + } + + /// Finds name of contract that contains given loc. + pub fn find_contract_name(&self, start: usize, end: usize) -> Option<&str> { + self.contract_definitions + .iter() + .find(|(_, (s, e))| start >= *s && end <= *e) + .map(|(name, _)| name.as_str()) + } } #[derive(Clone, Debug)] @@ -66,7 +109,7 @@ impl ArtifactData { #[derive(Clone, Debug, Default)] pub struct ContractSources { /// Map over build_id -> file_id -> (source code, language) - pub sources_by_id: HashMap>, + pub sources_by_id: HashMap>>, /// Map over contract name -> Vec<(bytecode, build_id, file_id)> pub artifacts_by_name: HashMap>, } @@ -100,48 +143,68 @@ impl ContractSources { (linker, libraries) }); - for (id, artifact) in output.artifact_ids() { - if let Some(file_id) = artifact.id { - let artifact = if let Some((linker, libraries)) = link_data.as_ref() { - linker.link(&id, libraries)?.into_contract_bytecode() + let artifacts: Vec<_> = output + .artifact_ids() + .collect::>() + .par_iter() + .map(|(id, artifact)| { + let mut artifacts = Vec::new(); + if let Some(file_id) = artifact.id { + let artifact = if let Some((linker, libraries)) = link_data.as_ref() { + linker.link(&id, libraries)?.into_contract_bytecode() + } else { + (*artifact).clone().into_contract_bytecode() + }; + let bytecode = compact_to_contract(artifact.clone().into_contract_bytecode())?; + + artifacts.push(( + id.name.clone(), + ArtifactData::new(bytecode, id.build_id.clone(), file_id)?, + )); } else { - artifact.clone().into_contract_bytecode() + warn!(id = id.identifier(), "source not found"); }; - let bytecode = compact_to_contract(artifact.clone().into_contract_bytecode())?; - self.artifacts_by_name.entry(id.name.clone()).or_default().push(ArtifactData::new( - bytecode, - id.build_id.clone(), - file_id, - )?); - } else { - warn!(id = id.identifier(), "source not found"); - } + Ok(artifacts) + }) + .collect::>>()? + .into_iter() + .flatten() + .collect(); + + for (name, artifact) in artifacts { + self.artifacts_by_name.entry(name).or_default().push(artifact); } // Not all source files produce artifacts, so we are populating sources by using build // infos. - let mut files: BTreeMap> = BTreeMap::new(); + let mut files: BTreeMap> = BTreeMap::new(); for (build_id, build) in output.builds() { for (source_id, path) in &build.source_id_to_path { - let source_code = if let Some(source) = files.get(path) { - source.clone() + let source_data = if let Some(source_data) = files.get(path) { + source_data.clone() } else { let source = Source::read(path).wrap_err_with(|| { format!("failed to read artifact source file for `{}`", path.display()) })?; - files.insert(path.clone(), source.content.clone()); - source.content + + let stripped = path.strip_prefix(root).unwrap_or(path).to_path_buf(); + + let source_data = Arc::new(SourceData::new( + source.content.clone(), + build.language.into(), + stripped, + )); + + files.insert(path.clone(), source_data.clone()); + + source_data }; - self.sources_by_id.entry(build_id.clone()).or_default().insert( - *source_id, - SourceData { - source: source_code, - language: build.language.into(), - name: path.strip_prefix(root).unwrap_or(path).to_string_lossy().to_string(), - }, - ); + self.sources_by_id + .entry(build_id.clone()) + .or_default() + .insert(*source_id, source_data); } } @@ -157,7 +220,7 @@ impl ContractSources { artifacts.iter().filter_map(|artifact| { let source = self.sources_by_id.get(artifact.build_id.as_str())?.get(&artifact.file_id)?; - Some((artifact, source)) + Some((artifact, source.as_ref())) }) }) } @@ -168,7 +231,7 @@ impl ContractSources { artifacts.iter().filter_map(|artifact| { let source = self.sources_by_id.get(artifact.build_id.as_str())?.get(&artifact.file_id)?; - Some((name.as_str(), artifact, source)) + Some((name.as_str(), artifact, source.as_ref())) }) }) } From 6518cb941b3fce98075ab644ee47a68b407c1d49 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Sat, 22 Jun 2024 23:29:03 +0300 Subject: [PATCH 08/30] feat: print traces in format of Contract::function --- crates/debugger/src/identifier.rs | 127 ++++++++++++------------ crates/debugger/src/tui/draw.rs | 18 ++-- crates/evm/traces/src/identifier/mod.rs | 2 +- crates/evm/traces/src/lib.rs | 10 +- 4 files changed, 79 insertions(+), 78 deletions(-) diff --git a/crates/debugger/src/identifier.rs b/crates/debugger/src/identifier.rs index 8b261f38406d..efcae1c5d823 100644 --- a/crates/debugger/src/identifier.rs +++ b/crates/debugger/src/identifier.rs @@ -5,10 +5,11 @@ use foundry_compilers::{ multi::MultiCompilerLanguage, }; use foundry_evm_traces::{ - identifier::ContractSources, CallTraceArena, CallTraceDecoder, CallTraceNode, DecodedTraceStep, + identifier::{ContractSources, SourceData}, + CallTraceArena, CallTraceDecoder, CallTraceNode, DecodedTraceStep, }; use revm::interpreter::OpCode; -use std::{collections::HashMap, path::Path}; +use std::collections::HashMap; pub struct DebugTraceIdentifier { /// Mapping of contract address to identified contract name. @@ -34,7 +35,7 @@ impl DebugTraceIdentifier { address: &Address, pc: usize, init_code: bool, - ) -> core::result::Result<(SourceElement, &str, &Path), String> { + ) -> core::result::Result<(SourceElement, &SourceData), String> { let Some(contract_name) = self.identified_contracts.get(address) else { return Err(format!("Unknown contract at address {address}")); }; @@ -43,61 +44,56 @@ impl DebugTraceIdentifier { return Err(format!("No source map index for contract {contract_name}")); }; - let Some((source_element, source_code, source_file)) = - files_source_code.find_map(|(artifact, source)| { - let source_map = if init_code { - artifact.source_map.as_ref() - } else { - artifact.source_map_runtime.as_ref() - }?; - - let pc_ic_map = if init_code { - artifact.pc_ic_map.as_ref() - } else { - artifact.pc_ic_map_runtime.as_ref() - }?; - let ic = pc_ic_map.get(pc)?; - - // Solc indexes source maps by instruction counter, but Vyper indexes by program - // counter. - let source_element = if matches!(source.language, MultiCompilerLanguage::Solc(_)) { - source_map.get(ic)? - } else { - source_map.get(pc)? - }; - // if the source element has an index, find the sourcemap for that index - let res = source_element - .index() - // if index matches current file_id, return current source code - .and_then(|index| { - (index == artifact.file_id) - .then(|| (source_element.clone(), source.source.as_str(), &source.path)) - }) - .or_else(|| { - // otherwise find the source code for the element's index - self.contracts_sources - .sources_by_id - .get(&artifact.build_id)? - .get(&source_element.index()?) - .map(|source| { - (source_element.clone(), source.source.as_str(), &source.path) - }) - }); - - res - }) - else { + let Some((source_element, source)) = files_source_code.find_map(|(artifact, source)| { + let source_map = if init_code { + artifact.source_map.as_ref() + } else { + artifact.source_map_runtime.as_ref() + }?; + + let pc_ic_map = if init_code { + artifact.pc_ic_map.as_ref() + } else { + artifact.pc_ic_map_runtime.as_ref() + }?; + let ic = pc_ic_map.get(pc)?; + + // Solc indexes source maps by instruction counter, but Vyper indexes by program + // counter. + let source_element = if matches!(source.language, MultiCompilerLanguage::Solc(_)) { + source_map.get(ic)? + } else { + source_map.get(pc)? + }; + // if the source element has an index, find the sourcemap for that index + let res = source_element + .index() + // if index matches current file_id, return current source code + .and_then(|index| { + (index == artifact.file_id).then(|| (source_element.clone(), source)) + }) + .or_else(|| { + // otherwise find the source code for the element's index + self.contracts_sources + .sources_by_id + .get(&artifact.build_id)? + .get(&source_element.index()?) + .map(|source| (source_element.clone(), source.as_ref())) + }); + + res + }) else { return Err(format!("No source map for contract {contract_name}")); }; - Ok((source_element, source_code, source_file)) + Ok((source_element, source)) } - pub fn identify_arena(&self, arena: &CallTraceArena) -> Vec>> { + pub fn identify_arena(&self, arena: &CallTraceArena) -> Vec> { arena.nodes().iter().map(move |node| self.identify_node_steps(node)).collect() } - pub fn identify_node_steps(&self, node: &CallTraceNode) -> Vec> { + pub fn identify_node_steps(&self, node: &CallTraceNode) -> Vec { let mut stack = Vec::new(); let mut identified = Vec::new(); @@ -112,24 +108,17 @@ impl DebugTraceIdentifier { } // Resolve source map if possible. - let Ok((source_element, source_code, _)) = + let Ok((source_element, source)) = self.identify(&node.trace.address, step.pc, node.trace.kind.is_any_create()) else { prev_step_jump_in = false; continue; }; - // Get slice of the source code that corresponds to the current step. - let source_part = { - let start = source_element.offset() as usize; - let end = start + source_element.length() as usize; - &source_code[start..end] - }; - // If previous step was a jump record source location at JUMPDEST. if prev_step_jump_in { if step.op == OpCode::JUMPDEST { - if let Some(name) = parse_function_name(source_part) { + if let Some(name) = parse_function_name(source, &source_element) { stack.push((name, step_idx)); } }; @@ -141,7 +130,7 @@ impl DebugTraceIdentifier { Jump::In => prev_step_jump_in = true, Jump::Out => { // Find index matching the beginning of this function - if let Some(name) = parse_function_name(source_part) { + if let Some(name) = parse_function_name(source, &source_element) { if let Some((i, _)) = stack.iter().enumerate().rfind(|(_, (n, _))| n == &name) { @@ -224,12 +213,22 @@ impl DebugTraceIdentifierBuilder { } } -fn parse_function_name(source: &str) -> Option<&str> { - if !source.starts_with("function") { +/// Tries to parse the function name from the source code and detect the contract name which +/// contains the given function. +/// +/// Returns string in the format `Contract::function`. +fn parse_function_name(source: &SourceData, loc: &SourceElement) -> Option { + let start = loc.offset() as usize; + let end = start + loc.length() as usize; + let source_part = &source.source[start..end]; + if !source_part.starts_with("function") { return None; } - if !source.contains("internal") && !source.contains("private") { + if !source_part.contains("internal") && !source_part.contains("private") { return None; } - Some(source.split_once("function")?.1.split('(').next()?.trim()) + let function_name = source_part.split_once("function")?.1.split('(').next()?.trim(); + let contract_name = source.find_contract_name(start, end)?; + + Some(format!("{contract_name}::{function_name}")) } diff --git a/crates/debugger/src/tui/draw.rs b/crates/debugger/src/tui/draw.rs index d2182720e779..ef7f9c05c458 100644 --- a/crates/debugger/src/tui/draw.rs +++ b/crates/debugger/src/tui/draw.rs @@ -4,6 +4,7 @@ use super::context::{BufferKind, DebuggerContext}; use crate::op::OpcodeParam; use alloy_primitives::U256; use foundry_compilers::artifacts::sourcemap::SourceElement; +use foundry_evm_traces::identifier::SourceData; use ratatui::{ layout::{Alignment, Constraint, Direction, Layout, Rect}, style::{Color, Modifier, Style}, @@ -13,7 +14,7 @@ use ratatui::{ }; use revm::interpreter::opcode; use revm_inspectors::tracing::types::CallKind; -use std::{collections::VecDeque, fmt::Write, io, path::Path}; +use std::{collections::VecDeque, fmt::Write, io}; impl DebuggerContext<'_> { /// Draws the TUI layout and subcomponents to the given terminal. @@ -211,7 +212,7 @@ impl DebuggerContext<'_> { } fn src_text(&self, area: Rect) -> (Text<'_>, Option<&str>) { - let (source_element, source_code, source_file) = match self.src_map() { + let (source_element, source) = match self.src_map() { Ok(r) => r, Err(e) => return (Text::from(e), None), }; @@ -222,15 +223,16 @@ impl DebuggerContext<'_> { // minus `sum(push_bytes[..pc])`. let offset = source_element.offset() as usize; let len = source_element.length() as usize; - let max = source_code.len(); + let max = source.source.len(); // Split source into before, relevant, and after chunks, split by line, for formatting. let actual_start = offset.min(max); let actual_end = (offset + len).min(max); - let mut before: Vec<_> = source_code[..actual_start].split_inclusive('\n').collect(); - let actual: Vec<_> = source_code[actual_start..actual_end].split_inclusive('\n').collect(); - let mut after: VecDeque<_> = source_code[actual_end..].split_inclusive('\n').collect(); + let mut before: Vec<_> = source.source[..actual_start].split_inclusive('\n').collect(); + let actual: Vec<_> = + source.source[actual_start..actual_end].split_inclusive('\n').collect(); + let mut after: VecDeque<_> = source.source[actual_end..].split_inclusive('\n').collect(); let num_lines = before.len() + actual.len() + after.len(); let height = area.height as usize; @@ -335,11 +337,11 @@ impl DebuggerContext<'_> { } } - (Text::from(lines.lines), source_file.to_str()) + (Text::from(lines.lines), source.path.to_str()) } /// Returns source map, source code and source name of the current line. - fn src_map(&self) -> Result<(SourceElement, &str, &Path), String> { + fn src_map(&self) -> Result<(SourceElement, &SourceData), String> { self.debugger.identifier.identify( self.address(), self.current_step().pc, diff --git a/crates/evm/traces/src/identifier/mod.rs b/crates/evm/traces/src/identifier/mod.rs index 5a4a879c8684..ea1c010f60e6 100644 --- a/crates/evm/traces/src/identifier/mod.rs +++ b/crates/evm/traces/src/identifier/mod.rs @@ -15,7 +15,7 @@ mod signatures; pub use signatures::{SignaturesIdentifier, SingleSignaturesIdentifier}; mod sources; -pub use sources::ContractSources; +pub use sources::{ContractSources, SourceData}; /// An address identity pub struct AddressIdentity<'a> { diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index 27741d157b61..977fc6354ecd 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -60,10 +60,10 @@ pub enum DecodedCallLog<'a> { } #[derive(Debug, Clone)] -pub struct DecodedTraceStep<'a> { +pub struct DecodedTraceStep { pub start_step_idx: usize, pub end_step_idx: usize, - pub function_name: &'a str, + pub function_name: String, pub gas_used: i64, } @@ -76,14 +76,14 @@ const RETURN: &str = "← "; pub async fn render_trace_arena_with_internals<'a>( arena: &CallTraceArena, decoder: &CallTraceDecoder, - identified_internals: &'a [Vec>], + identified_internals: &'a [Vec], ) -> Result { decoder.prefetch_signatures(arena.nodes()).await; fn render_items<'a>( arena: &'a [CallTraceNode], decoder: &'a CallTraceDecoder, - identified_internals: &'a [Vec>], + identified_internals: &'a [Vec], s: &'a mut String, node_idx: usize, mut ordering_idx: usize, @@ -158,7 +158,7 @@ pub async fn render_trace_arena_with_internals<'a>( fn inner<'a>( arena: &'a [CallTraceNode], decoder: &'a CallTraceDecoder, - identified_internals: &'a [Vec>], + identified_internals: &'a [Vec], s: &'a mut String, idx: usize, left: &'a str, From 06dc30af32c737cfa054e103c14fdd2cb396a001 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Sun, 23 Jun 2024 07:24:33 +0300 Subject: [PATCH 09/30] wip --- crates/common/src/compile.rs | 2 +- crates/debugger/src/identifier.rs | 147 ++++++++------------ crates/debugger/src/tui/draw.rs | 10 +- crates/evm/traces/src/identifier/sources.rs | 51 ++++++- crates/evm/traces/src/lib.rs | 4 +- crates/forge/bin/cmd/coverage.rs | 2 +- crates/forge/bin/cmd/test/mod.rs | 96 +++++++------ 7 files changed, 169 insertions(+), 143 deletions(-) diff --git a/crates/common/src/compile.rs b/crates/common/src/compile.rs index 468bc8ecb68a..d7146cefcf50 100644 --- a/crates/common/src/compile.rs +++ b/crates/common/src/compile.rs @@ -373,7 +373,7 @@ pub fn etherscan_project( let sources_path = target_path.join(&metadata.contract_name); metadata.source_tree().write_to(&target_path)?; - let mut settings = metadata.source_code.settings()?.unwrap_or_default(); + let mut settings = metadata.settings()?; // make remappings absolute with our root for remapping in settings.remappings.iter_mut() { diff --git a/crates/debugger/src/identifier.rs b/crates/debugger/src/identifier.rs index efcae1c5d823..b2dc51d4c3d9 100644 --- a/crates/debugger/src/identifier.rs +++ b/crates/debugger/src/identifier.rs @@ -36,57 +36,7 @@ impl DebugTraceIdentifier { pc: usize, init_code: bool, ) -> core::result::Result<(SourceElement, &SourceData), String> { - let Some(contract_name) = self.identified_contracts.get(address) else { - return Err(format!("Unknown contract at address {address}")); - }; - - let Some(mut files_source_code) = self.contracts_sources.get_sources(contract_name) else { - return Err(format!("No source map index for contract {contract_name}")); - }; - - let Some((source_element, source)) = files_source_code.find_map(|(artifact, source)| { - let source_map = if init_code { - artifact.source_map.as_ref() - } else { - artifact.source_map_runtime.as_ref() - }?; - - let pc_ic_map = if init_code { - artifact.pc_ic_map.as_ref() - } else { - artifact.pc_ic_map_runtime.as_ref() - }?; - let ic = pc_ic_map.get(pc)?; - - // Solc indexes source maps by instruction counter, but Vyper indexes by program - // counter. - let source_element = if matches!(source.language, MultiCompilerLanguage::Solc(_)) { - source_map.get(ic)? - } else { - source_map.get(pc)? - }; - // if the source element has an index, find the sourcemap for that index - let res = source_element - .index() - // if index matches current file_id, return current source code - .and_then(|index| { - (index == artifact.file_id).then(|| (source_element.clone(), source)) - }) - .or_else(|| { - // otherwise find the source code for the element's index - self.contracts_sources - .sources_by_id - .get(&artifact.build_id)? - .get(&source_element.index()?) - .map(|source| (source_element.clone(), source.as_ref())) - }); - - res - }) else { - return Err(format!("No source map for contract {contract_name}")); - }; - - Ok((source_element, source)) + } pub fn identify_arena(&self, arena: &CallTraceArena) -> Vec> { @@ -97,13 +47,17 @@ impl DebugTraceIdentifier { let mut stack = Vec::new(); let mut identified = Vec::new(); + // Helper to get a unique identifier for a source location. + let get_loc_id = |loc: &SourceElement| (loc.index(), loc.offset(), loc.length()); + // Flag marking whether previous instruction was a jump into function. // If it was, we expect next instruction to be a JUMPDEST with source location pointing to // the function. - let mut prev_step_jump_in = false; + let mut prev_step = None; for (step_idx, step) in node.trace.steps.iter().enumerate() { // We are only interested in JUMPs. if step.op != OpCode::JUMP && step.op != OpCode::JUMPI && step.op != OpCode::JUMPDEST { + prev_step = None; continue; } @@ -111,47 +65,69 @@ impl DebugTraceIdentifier { let Ok((source_element, source)) = self.identify(&node.trace.address, step.pc, node.trace.kind.is_any_create()) else { - prev_step_jump_in = false; + prev_step = None; continue; }; - // If previous step was a jump record source location at JUMPDEST. - if prev_step_jump_in { - if step.op == OpCode::JUMPDEST { - if let Some(name) = parse_function_name(source, &source_element) { - stack.push((name, step_idx)); - } - }; - prev_step_jump_in = false; - } + let Some((prev_source_element, prev_source)) = prev_step else { + prev_step = Some((source_element, source)); + continue; + }; - match source_element.jump() { - // Source location is collected on the next step. - Jump::In => prev_step_jump_in = true, - Jump::Out => { - // Find index matching the beginning of this function - if let Some(name) = parse_function_name(source, &source_element) { - if let Some((i, _)) = - stack.iter().enumerate().rfind(|(_, (n, _))| n == &name) - { - // We've found a match, remove all records between start and end, those - // are considered invalid. - let (_, start_idx) = stack.split_off(i)[0]; - - let gas_used = node.trace.steps[start_idx].gas_remaining as i64 - - node.trace.steps[step_idx].gas_remaining as i64; - - identified.push(DecodedTraceStep { - start_step_idx: start_idx, - end_step_idx: step_idx, - function_name: name, - gas_used, - }); + match prev_source_element.jump() { + Jump::In => { + let invocation_loc_id = get_loc_id(&prev_source_element); + let fn_loc_id = get_loc_id(&source_element); + + // This usually means that this is a jump into the external function which is an + // entrypoint for the current frame. We don't want to include this to avoid + // duplicating traces. + if invocation_loc_id != fn_loc_id { + if let Some(name) = parse_function_name(source, &source_element) { + stack.push((name, step_idx, invocation_loc_id, fn_loc_id)); } } } + Jump::Out => { + let invocation_loc_id = get_loc_id(&source_element); + let fn_loc_id = get_loc_id(&prev_source_element); + + if let Some((i, _)) = + stack.iter().enumerate().rfind(|(_, (_, _, i_loc, f_loc))| { + *i_loc == invocation_loc_id || *f_loc == fn_loc_id + }) + { + // We've found a match, remove all records between start and end, those + // are considered invalid. + let (function_name, start_idx, ..) = stack.split_off(i).swap_remove(0); + + let gas_used = node.trace.steps[start_idx].gas_remaining as i64 - + node.trace.steps[step_idx].gas_remaining as i64; + + identified.push(DecodedTraceStep { + start_step_idx: start_idx, + end_step_idx: Some(step_idx), + function_name, + gas_used, + }); + } + } _ => {} }; + + prev_step = Some((source_element, source)); + } + + for (name, step_idx, ..) in stack { + let gas_used = node.trace.steps[step_idx].gas_remaining as i64 - + node.trace.steps.last().map(|s| s.gas_remaining).unwrap_or_default() as i64; + + identified.push(DecodedTraceStep { + start_step_idx: step_idx, + end_step_idx: None, + function_name: name.clone(), + gas_used, + }); } // Sort by start step index. @@ -224,9 +200,6 @@ fn parse_function_name(source: &SourceData, loc: &SourceElement) -> Option { /// Returns source map, source code and source name of the current line. fn src_map(&self) -> Result<(SourceElement, &SourceData), String> { - self.debugger.identifier.identify( - self.address(), - self.current_step().pc, - self.call_kind().is_any_create(), - ) + let Some(contract_name) = self.identified_contracts.get(address) else { + return Err(format!("Unknown contract at address {address}")); + }; + + } fn draw_op_list(&self, f: &mut Frame<'_>, area: Rect) { diff --git a/crates/evm/traces/src/identifier/sources.rs b/crates/evm/traces/src/identifier/sources.rs index e4c41a81ed31..096a67da2956 100644 --- a/crates/evm/traces/src/identifier/sources.rs +++ b/crates/evm/traces/src/identifier/sources.rs @@ -1,7 +1,10 @@ use eyre::{Context, Result}; use foundry_common::compact_to_contract; use foundry_compilers::{ - artifacts::{sourcemap::SourceMap, Bytecode, ContractBytecodeSome, Libraries, Source}, + artifacts::{ + sourcemap::{SourceElement, SourceMap}, + Bytecode, ContractBytecodeSome, Libraries, Source, + }, multi::MultiCompilerLanguage, Artifact, Compiler, ProjectCompileOutput, }; @@ -235,4 +238,50 @@ impl ContractSources { }) }) } + + pub fn find_source_mapping( + &self, + contract_name: &str, + pc: usize, + init_code: bool, + ) -> Option<(SourceElement, &SourceData)> { + self.get_sources(contract_name)?.find_map(|(artifact, source)| { + let source_map = if init_code { + artifact.source_map.as_ref() + } else { + artifact.source_map_runtime.as_ref() + }?; + + let pc_ic_map = if init_code { + artifact.pc_ic_map.as_ref() + } else { + artifact.pc_ic_map_runtime.as_ref() + }?; + let ic = pc_ic_map.get(pc)?; + + // Solc indexes source maps by instruction counter, but Vyper indexes by program + // counter. + let source_element = if matches!(source.language, MultiCompilerLanguage::Solc(_)) { + source_map.get(ic)? + } else { + source_map.get(pc)? + }; + // if the source element has an index, find the sourcemap for that index + let res = source_element + .index() + // if index matches current file_id, return current source code + .and_then(|index| { + (index == artifact.file_id).then(|| (source_element.clone(), source)) + }) + .or_else(|| { + // otherwise find the source code for the element's index + self.sources_by_id + .get(&artifact.build_id)? + .get(&source_element.index()?) + .map(|source| (source_element.clone(), source.as_ref())) + }); + + res + }) + } } diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index b677ab9d8661..0ce8a8daf256 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -70,7 +70,7 @@ pub enum DecodedCallLog<'a> { #[derive(Debug, Clone)] pub struct DecodedTraceStep { pub start_step_idx: usize, - pub end_step_idx: usize, + pub end_step_idx: Option, pub function_name: String, pub gas_used: i64, } @@ -145,7 +145,7 @@ pub async fn render_trace_arena_with_internals<'a>( s, node_idx, ordering_idx + 1, - Some(decoded.end_step_idx), + decoded.end_step_idx, &left_prefix, &right_prefix, ) diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index dc9b25c148ec..aab591f1ec4a 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -249,7 +249,7 @@ impl CoverageArgs { let outcome = self .test - .run_tests(runner, config.clone(), verbosity, &self.test.filter(&config)) + .run_tests(runner, config.clone(), verbosity, &self.test.filter(&config), None) .await?; // Add hit data to the coverage report diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index c5bdf55a9e21..4a0f9c18aa59 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -1,7 +1,7 @@ use super::{install, test::filter::ProjectPathsAwareFilter, watch::WatchArgs}; use alloy_primitives::U256; use clap::Parser; -use eyre::{OptionExt, Result}; +use eyre::Result; use forge::{ decode::decode_console_logs, gas_report::GasReport, @@ -296,10 +296,10 @@ impl TestArgs { let env = evm_opts.evm_env().await?; // Prepare the test builder - let should_debug = self.debug.is_some() || self.decode_internal; + let should_debug = self.debug.is_some(); // Clone the output only if we actually need it later for the debugger. - let output_clone = should_debug.then(|| output.clone()); + let output_clone = (should_debug || self.decode_internal).then(|| output.clone()); let config = Arc::new(config); @@ -314,6 +314,12 @@ impl TestArgs { .enable_isolation(evm_opts.isolate) .build(project_root, output, env, evm_opts)?; + let debug_sources = output_clone + .map(|output| { + ContractSources::from_project_output(&output, project_root, Some(&runner.libraries)) + }) + .transpose()?; + if let Some(debug_test_pattern) = &self.debug { let test_pattern = &mut filter.args_mut().test_pattern; if test_pattern.is_some() { @@ -325,8 +331,8 @@ impl TestArgs { *test_pattern = Some(debug_test_pattern.clone()); } - let libraries = runner.libraries.clone(); - let outcome = self.run_tests(runner, config, verbosity, &filter).await?; + let outcome = + self.run_tests(runner, config, verbosity, &filter, debug_sources.as_ref()).await?; if should_debug { // Get first non-empty suite result. We will have only one such entry @@ -339,49 +345,22 @@ impl TestArgs { return Err(eyre::eyre!("no tests were executed")); }; - let sources = ContractSources::from_project_output( - output_clone.as_ref().unwrap(), - project.root(), - Some(&libraries), - )?; - - if self.decode_internal { - let mut builder = DebugTraceIdentifier::builder().sources(sources); - let Some(decoder) = &outcome.last_run_decoder else { - eyre::bail!("Missing decoder for debugging"); - }; - builder = builder.decoder(decoder); - - let identifier = builder.build(); - let arena = &test_result - .traces - .iter() - .find(|(t, _)| t.is_execution()) - .ok_or_eyre("didn't find execution trace for debugging")? - .1; - - let identified = identifier.identify_arena(arena); + let sources = debug_sources.unwrap(); - println!( - "{}", - render_trace_arena_with_internals(arena, decoder, &identified).await? - ); - } else if self.debug.is_some() { - // Run the debugger. - let builder = Debugger::builder() - .debug_arenas(test_result.debug.as_slice()) - .identifier(|mut builder| { - if let Some(decoder) = &outcome.last_run_decoder { - builder = builder.decoder(decoder); - } + // Run the debugger. + let builder = Debugger::builder() + .debug_arenas(test_result.debug.as_slice()) + .identifier(|mut builder| { + if let Some(decoder) = &outcome.last_run_decoder { + builder = builder.decoder(decoder); + } - builder.sources(sources) - }) - .breakpoints(test_result.breakpoints.clone()); + builder.sources(sources) + }) + .breakpoints(test_result.breakpoints.clone()); - let mut debugger = builder.build(); - debugger.try_run()?; - } + let mut debugger = builder.build(); + debugger.try_run()?; } Ok(outcome) @@ -394,6 +373,7 @@ impl TestArgs { config: Arc, verbosity: u8, filter: &ProjectPathsAwareFilter, + sources: Option<&ContractSources>, ) -> eyre::Result { if self.list { return list(runner, filter, self.json); @@ -527,7 +507,31 @@ impl TestArgs { }; if should_include { - decoded_traces.push(render_trace_arena(arena, &decoder).await?); + let decoded_internal = if self.decode_internal { + if let Some(sources) = sources { + let decoder = DebugTraceIdentifier::builder() + .sources(sources.clone()) + .decoder(&decoder) + .build(); + Some(decoder.identify_arena(arena)) + } else { + None + } + } else { + None + }; + if let Some(decoded_internal) = decoded_internal { + decoded_traces.push( + render_trace_arena_with_internals( + arena, + &decoder, + &decoded_internal, + ) + .await?, + ); + } else { + decoded_traces.push(render_trace_arena(arena, &decoder).await?); + } } } From 216e9da8a28fcc57bcba1c6c4986aa5353472cc5 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Sun, 23 Jun 2024 08:10:31 +0300 Subject: [PATCH 10/30] refactor --- Cargo.lock | 1 + crates/cli/src/utils/cmd.rs | 47 +++----- crates/debugger/src/lib.rs | 2 - crates/debugger/src/tui/builder.rs | 60 ++++++--- crates/debugger/src/tui/draw.rs | 14 ++- crates/debugger/src/tui/mod.rs | 13 +- crates/evm/traces/Cargo.toml | 3 +- .../traces/src/debug/mod.rs} | 114 ++++-------------- .../src/{identifier => debug}/sources.rs | 0 crates/evm/traces/src/decoder/mod.rs | 34 +++++- crates/evm/traces/src/identifier/etherscan.rs | 4 +- crates/evm/traces/src/identifier/mod.rs | 3 - crates/evm/traces/src/lib.rs | 24 ++-- crates/forge/bin/cmd/test/mod.rs | 53 +++----- crates/script/src/build.rs | 2 +- crates/script/src/execute.rs | 6 +- 16 files changed, 172 insertions(+), 208 deletions(-) rename crates/{debugger/src/identifier.rs => evm/traces/src/debug/mod.rs} (59%) rename crates/evm/traces/src/{identifier => debug}/sources.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index 43df21005414..672faaa9605a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3961,6 +3961,7 @@ dependencies = [ "itertools 0.13.0", "once_cell", "rayon", + "revm", "revm-inspectors", "rustc-hash", "serde", diff --git a/crates/cli/src/utils/cmd.rs b/crates/cli/src/utils/cmd.rs index 341494b46c03..521e0304b9e3 100644 --- a/crates/cli/src/utils/cmd.rs +++ b/crates/cli/src/utils/cmd.rs @@ -9,15 +9,15 @@ use foundry_compilers::{ Artifact, ProjectCompileOutput, }; use foundry_config::{error::ExtractConfigError, figment::Figment, Chain, Config, NamedChain}; -use foundry_debugger::{DebugTraceIdentifier, Debugger}; +use foundry_debugger::Debugger; use foundry_evm::{ debug::DebugArena, executors::{DeployResult, EvmError, RawCallResult}, opts::EvmOpts, traces::{ + debug::DebugTraceIdentifier, identifier::{EtherscanIdentifier, SignaturesIdentifier}, - render_trace_arena, render_trace_arena_with_internals, CallTraceDecoder, - CallTraceDecoderBuilder, TraceKind, Traces, + render_trace_arena, CallTraceDecoder, CallTraceDecoderBuilder, TraceKind, Traces, }, }; use std::{ @@ -386,6 +386,18 @@ pub async fn handle_traces( } } + if decode_internal { + let sources = if let Some(etherscan_identifier) = ðerscan_identifier { + etherscan_identifier.get_compiled_contracts().await? + } else { + Default::default() + }; + + let identifier = DebugTraceIdentifier::new(sources); + + decoder.debug_identifier = Some(identifier); + } + if debug { let sources = if let Some(etherscan_identifier) = etherscan_identifier { etherscan_identifier.get_compiled_contracts().await? @@ -394,42 +406,23 @@ pub async fn handle_traces( }; let mut debugger = Debugger::builder() .debug_arena(result.debug.as_ref().expect("missing debug arena")) - .identifier(|b| b.decoder(&decoder).sources(sources)) + .decoder(&decoder) + .sources(sources) .build(); debugger.try_run()?; } else { - let identifier = if decode_internal { - let sources = if let Some(etherscan_identifier) = etherscan_identifier { - etherscan_identifier.get_compiled_contracts().await? - } else { - Default::default() - }; - Some(DebugTraceIdentifier::builder().sources(sources).decoder(&decoder).build()) - } else { - None - }; - print_traces(&mut result, &decoder, identifier.as_ref()).await?; + print_traces(&mut result, &decoder).await?; } Ok(()) } -pub async fn print_traces( - result: &mut TraceResult, - decoder: &CallTraceDecoder, - identifier: Option<&DebugTraceIdentifier>, -) -> Result<()> { +pub async fn print_traces(result: &mut TraceResult, decoder: &CallTraceDecoder) -> Result<()> { let traces = result.traces.as_ref().expect("No traces found"); println!("Traces:"); for (_, arena) in traces { - let arena = if let Some(identifier) = identifier { - render_trace_arena_with_internals(arena, decoder, &identifier.identify_arena(arena)) - .await? - } else { - render_trace_arena(arena, decoder).await? - }; - println!("{arena}"); + println!("{}", render_trace_arena(arena, decoder).await?); } println!(); diff --git a/crates/debugger/src/lib.rs b/crates/debugger/src/lib.rs index 052dbb1aab47..ed5da934271a 100644 --- a/crates/debugger/src/lib.rs +++ b/crates/debugger/src/lib.rs @@ -10,7 +10,5 @@ extern crate tracing; mod op; -mod identifier; mod tui; -pub use identifier::DebugTraceIdentifier; pub use tui::{Debugger, DebuggerBuilder, ExitReason}; diff --git a/crates/debugger/src/tui/builder.rs b/crates/debugger/src/tui/builder.rs index 0b2bb022990f..f5771d46ae2c 100644 --- a/crates/debugger/src/tui/builder.rs +++ b/crates/debugger/src/tui/builder.rs @@ -1,8 +1,11 @@ //! TUI debugger builder. -use crate::{identifier::DebugTraceIdentifierBuilder, Debugger}; -use foundry_common::evm::Breakpoints; +use crate::Debugger; +use alloy_primitives::Address; +use foundry_common::{evm::Breakpoints, get_contract_name}; use foundry_evm_core::debug::{DebugArena, DebugNodeFlat}; +use foundry_evm_traces::{debug::ContractSources, CallTraceDecoder}; +use std::collections::HashMap; /// Debugger builder. #[derive(Debug, Default)] @@ -10,8 +13,10 @@ use foundry_evm_core::debug::{DebugArena, DebugNodeFlat}; pub struct DebuggerBuilder { /// Debug traces returned from the EVM execution. debug_arena: Vec, - /// Builder for [crate::DebugTraceIdentifier]. - identifier: DebugTraceIdentifierBuilder, + /// Identified contracts. + identified_contracts: HashMap, + /// Map of source files. + sources: ContractSources, /// Map of the debugger breakpoints. breakpoints: Breakpoints, } @@ -23,16 +28,6 @@ impl DebuggerBuilder { Self::default() } - /// Configures the [crate::DebugTraceIdentifier]. - #[inline] - pub fn identifier( - mut self, - f: impl FnOnce(DebugTraceIdentifierBuilder) -> DebugTraceIdentifierBuilder, - ) -> Self { - self.identifier = f(self.identifier); - self - } - /// Extends the debug arena. #[inline] pub fn debug_arenas(mut self, arena: &[DebugArena]) -> Self { @@ -49,6 +44,39 @@ impl DebuggerBuilder { self } + /// Extends the identified contracts from multiple decoders. + #[inline] + pub fn decoders(mut self, decoders: &[CallTraceDecoder]) -> Self { + for decoder in decoders { + self = self.decoder(decoder); + } + self + } + + /// Extends the identified contracts from a decoder. + #[inline] + pub fn decoder(self, decoder: &CallTraceDecoder) -> Self { + let c = decoder.contracts.iter().map(|(k, v)| (*k, get_contract_name(v).to_string())); + self.identified_contracts(c) + } + + /// Extends the identified contracts. + #[inline] + pub fn identified_contracts( + mut self, + identified_contracts: impl IntoIterator, + ) -> Self { + self.identified_contracts.extend(identified_contracts); + self + } + + /// Sets the sources for the debugger. + #[inline] + pub fn sources(mut self, sources: ContractSources) -> Self { + self.sources = sources; + self + } + /// Sets the breakpoints for the debugger. #[inline] pub fn breakpoints(mut self, breakpoints: Breakpoints) -> Self { @@ -59,7 +87,7 @@ impl DebuggerBuilder { /// Builds the debugger. #[inline] pub fn build(self) -> Debugger { - let Self { debug_arena, identifier, breakpoints } = self; - Debugger::new(debug_arena, identifier.build(), breakpoints) + let Self { debug_arena, identified_contracts, sources, breakpoints } = self; + Debugger::new(debug_arena, identified_contracts, sources, breakpoints) } } diff --git a/crates/debugger/src/tui/draw.rs b/crates/debugger/src/tui/draw.rs index f17049c9f6e8..570abe8cdb56 100644 --- a/crates/debugger/src/tui/draw.rs +++ b/crates/debugger/src/tui/draw.rs @@ -4,7 +4,7 @@ use super::context::{BufferKind, DebuggerContext}; use crate::op::OpcodeParam; use alloy_primitives::U256; use foundry_compilers::artifacts::sourcemap::SourceElement; -use foundry_evm_traces::identifier::SourceData; +use foundry_evm_traces::debug::SourceData; use ratatui::{ layout::{Alignment, Constraint, Direction, Layout, Rect}, style::{Color, Modifier, Style}, @@ -342,11 +342,19 @@ impl DebuggerContext<'_> { /// Returns source map, source code and source name of the current line. fn src_map(&self) -> Result<(SourceElement, &SourceData), String> { - let Some(contract_name) = self.identified_contracts.get(address) else { + let address = self.address(); + let Some(contract_name) = self.debugger.identified_contracts.get(address) else { return Err(format!("Unknown contract at address {address}")); }; - + self.debugger + .contracts_sources + .find_source_mapping( + contract_name, + self.current_step().pc, + self.debug_call().kind.is_any_create(), + ) + .ok_or_else(|| format!("No source map for contract {contract_name}")) } fn draw_op_list(&self, f: &mut Frame<'_>, area: Rect) { diff --git a/crates/debugger/src/tui/mod.rs b/crates/debugger/src/tui/mod.rs index e0b01237c666..ee44bc301412 100644 --- a/crates/debugger/src/tui/mod.rs +++ b/crates/debugger/src/tui/mod.rs @@ -1,6 +1,6 @@ //! The TUI implementation. -use crate::DebugTraceIdentifier; +use alloy_primitives::Address; use crossterm::{ event::{self, DisableMouseCapture, EnableMouseCapture, Event}, execute, @@ -9,11 +9,13 @@ use crossterm::{ use eyre::Result; use foundry_common::evm::Breakpoints; use foundry_evm_core::debug::DebugNodeFlat; +use foundry_evm_traces::debug::ContractSources; use ratatui::{ backend::{Backend, CrosstermBackend}, Terminal, }; use std::{ + collections::HashMap, io, ops::ControlFlow, sync::{mpsc, Arc}, @@ -41,7 +43,9 @@ pub enum ExitReason { /// The TUI debugger. pub struct Debugger { debug_arena: Vec, - identifier: DebugTraceIdentifier, + identified_contracts: HashMap, + /// Source map of contract sources + contracts_sources: ContractSources, breakpoints: Breakpoints, } @@ -55,10 +59,11 @@ impl Debugger { /// Creates a new debugger. pub fn new( debug_arena: Vec, - identifier: DebugTraceIdentifier, + identified_contracts: HashMap, + contracts_sources: ContractSources, breakpoints: Breakpoints, ) -> Self { - Self { debug_arena, identifier, breakpoints } + Self { debug_arena, identified_contracts, contracts_sources, breakpoints } } /// Starts the debugger TUI. Terminates the current process on failure or user exit. diff --git a/crates/evm/traces/Cargo.toml b/crates/evm/traces/Cargo.toml index a9c7e395812d..3714807f1c5f 100644 --- a/crates/evm/traces/Cargo.toml +++ b/crates/evm/traces/Cargo.toml @@ -32,7 +32,7 @@ alloy-primitives = { workspace = true, features = [ alloy-sol-types.workspace = true revm-inspectors.workspace = true -eyre .workspace = true +eyre.workspace = true futures.workspace = true hex.workspace = true itertools.workspace = true @@ -45,6 +45,7 @@ rustc-hash.workspace = true tempfile.workspace = true rayon.workspace = true solang-parser.workspace = true +revm.workspace = true [dev-dependencies] tempfile.workspace = true diff --git a/crates/debugger/src/identifier.rs b/crates/evm/traces/src/debug/mod.rs similarity index 59% rename from crates/debugger/src/identifier.rs rename to crates/evm/traces/src/debug/mod.rs index b2dc51d4c3d9..e79f2890f559 100644 --- a/crates/debugger/src/identifier.rs +++ b/crates/evm/traces/src/debug/mod.rs @@ -1,49 +1,29 @@ -use alloy_primitives::Address; -use foundry_common::get_contract_name; -use foundry_compilers::{ - artifacts::sourcemap::{Jump, SourceElement}, - multi::MultiCompilerLanguage, -}; -use foundry_evm_traces::{ - identifier::{ContractSources, SourceData}, - CallTraceArena, CallTraceDecoder, CallTraceNode, DecodedTraceStep, -}; +mod sources; +pub use sources::{ArtifactData, ContractSources, SourceData}; + +use crate::{CallTraceNode, DecodedTraceStep}; +use foundry_compilers::artifacts::sourcemap::{Jump, SourceElement}; use revm::interpreter::OpCode; -use std::collections::HashMap; +#[derive(Clone, Debug)] pub struct DebugTraceIdentifier { - /// Mapping of contract address to identified contract name. - identified_contracts: HashMap, /// Source map of contract sources contracts_sources: ContractSources, } impl DebugTraceIdentifier { - pub fn builder() -> DebugTraceIdentifierBuilder { - DebugTraceIdentifierBuilder::default() - } - - pub fn new( - identified_contracts: HashMap, - contracts_sources: ContractSources, - ) -> Self { - Self { identified_contracts, contracts_sources } + pub fn new(contracts_sources: ContractSources) -> Self { + Self { contracts_sources } } - pub fn identify( + /// Identifies internal function invocations in a given [CallTraceNode]. + /// + /// Accepts the node itself and identified name of the contract which node corresponds to. + pub fn identify_node_steps( &self, - address: &Address, - pc: usize, - init_code: bool, - ) -> core::result::Result<(SourceElement, &SourceData), String> { - - } - - pub fn identify_arena(&self, arena: &CallTraceArena) -> Vec> { - arena.nodes().iter().map(move |node| self.identify_node_steps(node)).collect() - } - - pub fn identify_node_steps(&self, node: &CallTraceNode) -> Vec { + node: &CallTraceNode, + contract_name: &str, + ) -> Vec { let mut stack = Vec::new(); let mut identified = Vec::new(); @@ -56,20 +36,22 @@ impl DebugTraceIdentifier { let mut prev_step = None; for (step_idx, step) in node.trace.steps.iter().enumerate() { // We are only interested in JUMPs. - if step.op != OpCode::JUMP && step.op != OpCode::JUMPI && step.op != OpCode::JUMPDEST { + if step.op != OpCode::JUMP && step.op != OpCode::JUMPDEST { prev_step = None; continue; } // Resolve source map if possible. - let Ok((source_element, source)) = - self.identify(&node.trace.address, step.pc, node.trace.kind.is_any_create()) - else { + let Some((source_element, source)) = self.contracts_sources.find_source_mapping( + contract_name, + step.pc, + node.trace.kind.is_any_create(), + ) else { prev_step = None; continue; }; - let Some((prev_source_element, prev_source)) = prev_step else { + let Some((prev_source_element, _)) = prev_step else { prev_step = Some((source_element, source)); continue; }; @@ -137,58 +119,6 @@ impl DebugTraceIdentifier { } } -/// [DebugTraceIdentifier] builder -#[derive(Debug, Default)] -#[must_use = "builders do nothing unless you call `build` on them"] -pub struct DebugTraceIdentifierBuilder { - /// Identified contracts. - identified_contracts: HashMap, - /// Map of source files. - sources: ContractSources, -} - -impl DebugTraceIdentifierBuilder { - /// Extends the identified contracts from multiple decoders. - #[inline] - pub fn decoders(mut self, decoders: &[CallTraceDecoder]) -> Self { - for decoder in decoders { - self = self.decoder(decoder); - } - self - } - - /// Extends the identified contracts from a decoder. - #[inline] - pub fn decoder(self, decoder: &CallTraceDecoder) -> Self { - let c = decoder.contracts.iter().map(|(k, v)| (*k, get_contract_name(v).to_string())); - self.identified_contracts(c) - } - - /// Extends the identified contracts. - #[inline] - pub fn identified_contracts( - mut self, - identified_contracts: impl IntoIterator, - ) -> Self { - self.identified_contracts.extend(identified_contracts); - self - } - - /// Sets the sources for the debugger. - #[inline] - pub fn sources(mut self, sources: ContractSources) -> Self { - self.sources = sources; - self - } - - /// Builds the [DebugTraceIdentifier]. - #[inline] - pub fn build(self) -> DebugTraceIdentifier { - let Self { identified_contracts, sources } = self; - DebugTraceIdentifier::new(identified_contracts, sources) - } -} - /// Tries to parse the function name from the source code and detect the contract name which /// contains the given function. /// diff --git a/crates/evm/traces/src/identifier/sources.rs b/crates/evm/traces/src/debug/sources.rs similarity index 100% rename from crates/evm/traces/src/identifier/sources.rs rename to crates/evm/traces/src/debug/sources.rs diff --git a/crates/evm/traces/src/decoder/mod.rs b/crates/evm/traces/src/decoder/mod.rs index ec1ea199ffdd..823ba6f82f6f 100644 --- a/crates/evm/traces/src/decoder/mod.rs +++ b/crates/evm/traces/src/decoder/mod.rs @@ -1,14 +1,16 @@ use crate::{ + debug::DebugTraceIdentifier, identifier::{ AddressIdentity, LocalTraceIdentifier, SingleSignaturesIdentifier, TraceIdentifier, }, CallTrace, CallTraceArena, CallTraceNode, DecodedCallData, DecodedCallLog, DecodedCallTrace, + DecodedTraceStep, }; use alloy_dyn_abi::{DecodedEvent, DynSolValue, EventExt, FunctionExt, JsonAbiExt}; use alloy_json_abi::{Error, Event, Function, JsonAbi}; use alloy_primitives::{Address, LogData, Selector, B256}; use foundry_common::{ - abi::get_indexed_event, fmt::format_token, ContractsByArtifact, SELECTOR_LEN, + abi::get_indexed_event, fmt::format_token, get_contract_name, ContractsByArtifact, SELECTOR_LEN, }; use foundry_evm_core::{ abi::{Console, HardhatConsole, Vm, HARDHAT_CONSOLE_SELECTOR_PATCHES}, @@ -83,6 +85,13 @@ impl CallTraceDecoderBuilder { self } + /// Sets the debug identifier for the decoder. + #[inline] + pub fn with_debug_identifier(mut self, identifier: DebugTraceIdentifier) -> Self { + self.decoder.debug_identifier = Some(identifier); + self + } + /// Build the decoder. #[inline] pub fn build(self) -> CallTraceDecoder { @@ -119,6 +128,9 @@ pub struct CallTraceDecoder { pub signature_identifier: Option, /// Verbosity level pub verbosity: u8, + + /// Optional identifier of individual trace steps. + pub debug_identifier: Option, } impl CallTraceDecoder { @@ -181,6 +193,8 @@ impl CallTraceDecoder { signature_identifier: None, verbosity: 0, + + debug_identifier: None, } } @@ -600,6 +614,24 @@ impl CallTraceDecoder { } format_token(value) } + + pub fn identify_arena_steps(&self, arena: &CallTraceArena) -> Vec> { + arena + .nodes() + .iter() + .map(|node| { + if let Some(identifier) = &self.debug_identifier { + if let Some(identified) = self.contracts.get(&node.trace.address) { + identifier.identify_node_steps(node, get_contract_name(identified)) + } else { + vec![] + } + } else { + vec![] + } + }) + .collect() + } } /// Restore the order of the params of a decoded event, diff --git a/crates/evm/traces/src/identifier/etherscan.rs b/crates/evm/traces/src/identifier/etherscan.rs index 91e0a48c692f..9496e7ae83d0 100644 --- a/crates/evm/traces/src/identifier/etherscan.rs +++ b/crates/evm/traces/src/identifier/etherscan.rs @@ -1,4 +1,6 @@ -use super::{AddressIdentity, ContractSources, TraceIdentifier}; +use crate::debug::ContractSources; + +use super::{AddressIdentity, TraceIdentifier}; use alloy_primitives::Address; use foundry_block_explorers::{ contract::{ContractMetadata, Metadata}, diff --git a/crates/evm/traces/src/identifier/mod.rs b/crates/evm/traces/src/identifier/mod.rs index ea1c010f60e6..a16b108d8537 100644 --- a/crates/evm/traces/src/identifier/mod.rs +++ b/crates/evm/traces/src/identifier/mod.rs @@ -14,9 +14,6 @@ pub use etherscan::EtherscanIdentifier; mod signatures; pub use signatures::{SignaturesIdentifier, SingleSignaturesIdentifier}; -mod sources; -pub use sources::{ContractSources, SourceData}; - /// An address identity pub struct AddressIdentity<'a> { /// The address this identity belongs to diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index 0ce8a8daf256..ed5d2c8cc4ad 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -32,6 +32,7 @@ use identifier::{LocalTraceIdentifier, TraceIdentifier}; mod decoder; pub use decoder::{CallTraceDecoder, CallTraceDecoderBuilder}; +pub mod debug; use revm_inspectors::tracing::types::TraceMemberOrder; pub use revm_inspectors::tracing::{ @@ -81,13 +82,17 @@ const BRANCH: &str = " ├─ "; const CALL: &str = "→ "; const RETURN: &str = "← "; -pub async fn render_trace_arena_with_internals<'a>( +/// Render a collection of call traces. +/// +/// The traces will be decoded using the given decoder, if possible. +pub async fn render_trace_arena( arena: &CallTraceArena, decoder: &CallTraceDecoder, - identified_internals: &'a [Vec], ) -> Result { decoder.prefetch_signatures(arena.nodes()).await; + let identified_internals = &decoder.identify_arena_steps(arena); + fn render_items<'a>( arena: &'a [CallTraceNode], decoder: &'a CallTraceDecoder, @@ -221,21 +226,6 @@ pub async fn render_trace_arena_with_internals<'a>( Ok(s) } -/// Render a collection of call traces. -/// -/// The traces will be decoded using the given decoder, if possible. -pub async fn render_trace_arena( - arena: &CallTraceArena, - decoder: &CallTraceDecoder, -) -> Result { - render_trace_arena_with_internals( - arena, - decoder, - &std::iter::repeat(Vec::new()).take(arena.nodes().len()).collect::>(), - ) - .await -} - /// Render a call trace. /// /// The trace will be decoded using the given decoder, if possible. diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 4a0f9c18aa59..e24bcd781ca7 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -8,8 +8,9 @@ use forge::{ multi_runner::matches_contract, result::{SuiteResult, TestOutcome, TestStatus}, traces::{ - identifier::{ContractSources, SignaturesIdentifier}, - render_trace_arena_with_internals, CallTraceDecoderBuilder, TraceKind, + debug::{ContractSources, DebugTraceIdentifier}, + identifier::SignaturesIdentifier, + CallTraceDecoderBuilder, TraceKind, }, MultiContractRunner, MultiContractRunnerBuilder, TestFilter, TestOptions, TestOptionsBuilder, }; @@ -31,7 +32,7 @@ use foundry_config::{ }, get_available_profiles, Config, }; -use foundry_debugger::{DebugTraceIdentifier, Debugger}; +use foundry_debugger::Debugger; use foundry_evm::traces::identifier::TraceIdentifiers; use regex::Regex; use std::{ @@ -348,17 +349,15 @@ impl TestArgs { let sources = debug_sources.unwrap(); // Run the debugger. - let builder = Debugger::builder() + let mut builder = Debugger::builder() .debug_arenas(test_result.debug.as_slice()) - .identifier(|mut builder| { - if let Some(decoder) = &outcome.last_run_decoder { - builder = builder.decoder(decoder); - } - - builder.sources(sources) - }) + .sources(sources) .breakpoints(test_result.breakpoints.clone()); + if let Some(decoder) = &outcome.last_run_decoder { + builder = builder.decoder(decoder); + } + let mut debugger = builder.build(); debugger.try_run()?; } @@ -428,6 +427,12 @@ impl TestArgs { config.offline, )?); } + + if self.decode_internal { + if let Some(sources) = sources { + builder = builder.with_debug_identifier(DebugTraceIdentifier::new(sources.clone())); + } + } let mut decoder = builder.build(); let mut gas_report = self @@ -507,31 +512,7 @@ impl TestArgs { }; if should_include { - let decoded_internal = if self.decode_internal { - if let Some(sources) = sources { - let decoder = DebugTraceIdentifier::builder() - .sources(sources.clone()) - .decoder(&decoder) - .build(); - Some(decoder.identify_arena(arena)) - } else { - None - } - } else { - None - }; - if let Some(decoded_internal) = decoded_internal { - decoded_traces.push( - render_trace_arena_with_internals( - arena, - &decoder, - &decoded_internal, - ) - .await?, - ); - } else { - decoded_traces.push(render_trace_arena(arena, &decoder).await?); - } + decoded_traces.push(render_trace_arena(arena, &decoder).await?); } } diff --git a/crates/script/src/build.rs b/crates/script/src/build.rs index 81d22b020494..14feb42ff00e 100644 --- a/crates/script/src/build.rs +++ b/crates/script/src/build.rs @@ -19,7 +19,7 @@ use foundry_compilers::{ utils::source_files_iter, ArtifactId, ProjectCompileOutput, }; -use foundry_evm::{constants::DEFAULT_CREATE2_DEPLOYER, traces::identifier::ContractSources}; +use foundry_evm::{constants::DEFAULT_CREATE2_DEPLOYER, traces::debug::ContractSources}; use foundry_linking::Linker; use std::{path::PathBuf, str::FromStr, sync::Arc}; diff --git a/crates/script/src/execute.rs b/crates/script/src/execute.rs index 162b1bc9ba20..2b57468e276b 100644 --- a/crates/script/src/execute.rs +++ b/crates/script/src/execute.rs @@ -493,10 +493,8 @@ impl PreSimulationState { pub fn run_debugger(&self) -> Result<()> { let mut debugger = Debugger::builder() .debug_arenas(self.execution_result.debug.as_deref().unwrap_or_default()) - .identifier(|b| { - b.decoder(&self.execution_artifacts.decoder) - .sources(self.build_data.sources.clone()) - }) + .decoder(&self.execution_artifacts.decoder) + .sources(self.build_data.sources.clone()) .breakpoints(self.execution_result.breakpoints.clone()) .build(); debugger.try_run()?; From d92f4367ddf667655b20978f219220bdb43ea80f Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Sun, 23 Jun 2024 08:24:52 +0300 Subject: [PATCH 11/30] clippy --- crates/evm/traces/src/debug/sources.rs | 4 ++-- crates/evm/traces/src/lib.rs | 8 +------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/crates/evm/traces/src/debug/sources.rs b/crates/evm/traces/src/debug/sources.rs index 096a67da2956..1782265e86a2 100644 --- a/crates/evm/traces/src/debug/sources.rs +++ b/crates/evm/traces/src/debug/sources.rs @@ -154,11 +154,11 @@ impl ContractSources { let mut artifacts = Vec::new(); if let Some(file_id) = artifact.id { let artifact = if let Some((linker, libraries)) = link_data.as_ref() { - linker.link(&id, libraries)?.into_contract_bytecode() + linker.link(id, libraries)?.into_contract_bytecode() } else { (*artifact).clone().into_contract_bytecode() }; - let bytecode = compact_to_contract(artifact.clone().into_contract_bytecode())?; + let bytecode = compact_to_contract(artifact.into_contract_bytecode())?; artifacts.push(( id.name.clone(), diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index ed5d2c8cc4ad..f6ac84d5a27c 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -34,13 +34,6 @@ pub use decoder::{CallTraceDecoder, CallTraceDecoderBuilder}; pub mod debug; -use revm_inspectors::tracing::types::TraceMemberOrder; -pub use revm_inspectors::tracing::{ - types::{CallKind, CallTrace, CallTraceNode}, - CallTraceArena, GethTraceBuilder, ParityTraceBuilder, StackSnapshotType, TracingInspector, - TracingInspectorConfig, -}; - pub type Traces = Vec<(TraceKind, CallTraceArena)>; #[derive(Default, Debug, Eq, PartialEq)] @@ -93,6 +86,7 @@ pub async fn render_trace_arena( let identified_internals = &decoder.identify_arena_steps(arena); + #[allow(clippy::too_many_arguments)] fn render_items<'a>( arena: &'a [CallTraceNode], decoder: &'a CallTraceDecoder, From 7fa698b946008ef585033725e59732b4d83e4c89 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Sun, 23 Jun 2024 08:28:53 +0300 Subject: [PATCH 12/30] fix doc --- crates/common/src/contracts.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/common/src/contracts.rs b/crates/common/src/contracts.rs index aeaec4652b38..97fa759ea03f 100644 --- a/crates/common/src/contracts.rs +++ b/crates/common/src/contracts.rs @@ -88,9 +88,6 @@ pub struct ContractsByArtifact(Arc>); impl ContractsByArtifact { /// Creates a new instance by collecting all artifacts with present bytecode from an iterator. - /// - /// It is recommended to use this method with an output of - /// [foundry_linking::Linker::get_linked_artifacts]. pub fn new(artifacts: impl IntoIterator) -> Self { let map = artifacts .into_iter() From b3ef110b6f4fdabe761a6ae749d009853daba5b1 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Mon, 24 Jun 2024 18:42:33 +0400 Subject: [PATCH 13/30] track input/output values --- crates/evm/evm/src/inspectors/stack.rs | 8 +- crates/evm/traces/src/debug/mod.rs | 151 ++++++++++++++++++++++--- crates/evm/traces/src/lib.rs | 22 +++- 3 files changed, 163 insertions(+), 18 deletions(-) diff --git a/crates/evm/evm/src/inspectors/stack.rs b/crates/evm/evm/src/inspectors/stack.rs index feec3ce5b52c..ec55e7ec73c5 100644 --- a/crates/evm/evm/src/inspectors/stack.rs +++ b/crates/evm/evm/src/inspectors/stack.rs @@ -403,8 +403,12 @@ impl InspectorStack { self.tracer = Some({ TracingInspector::new(TracingInspectorConfig { record_steps: debug, - record_memory_snapshots: false, - record_stack_snapshots: StackSnapshotType::None, + record_memory_snapshots: debug, + record_stack_snapshots: if debug { + StackSnapshotType::Full + } else { + StackSnapshotType::None + }, record_state_diff: false, exclude_precompile_calls: false, record_logs: true, diff --git a/crates/evm/traces/src/debug/mod.rs b/crates/evm/traces/src/debug/mod.rs index e79f2890f559..d1713603a2cc 100644 --- a/crates/evm/traces/src/debug/mod.rs +++ b/crates/evm/traces/src/debug/mod.rs @@ -1,9 +1,13 @@ mod sources; -pub use sources::{ArtifactData, ContractSources, SourceData}; - use crate::{CallTraceNode, DecodedTraceStep}; +use alloy_dyn_abi::{DynSolType, DynSolValue}; +use alloy_json_abi::Function; +use alloy_primitives::U256; +use foundry_common::fmt::format_token; use foundry_compilers::artifacts::sourcemap::{Jump, SourceElement}; use revm::interpreter::OpCode; +use revm_inspectors::tracing::types::CallTraceStep; +pub use sources::{ArtifactData, ContractSources, SourceData}; #[derive(Clone, Debug)] pub struct DebugTraceIdentifier { @@ -30,9 +34,6 @@ impl DebugTraceIdentifier { // Helper to get a unique identifier for a source location. let get_loc_id = |loc: &SourceElement| (loc.index(), loc.offset(), loc.length()); - // Flag marking whether previous instruction was a jump into function. - // If it was, we expect next instruction to be a JUMPDEST with source location pointing to - // the function. let mut prev_step = None; for (step_idx, step) in node.trace.steps.iter().enumerate() { // We are only interested in JUMPs. @@ -65,8 +66,16 @@ impl DebugTraceIdentifier { // entrypoint for the current frame. We don't want to include this to avoid // duplicating traces. if invocation_loc_id != fn_loc_id { - if let Some(name) = parse_function_name(source, &source_element) { - stack.push((name, step_idx, invocation_loc_id, fn_loc_id)); + if let Some((name, maybe_function)) = + parse_function_from_loc(source, &source_element) + { + stack.push(( + name, + maybe_function, + step_idx - 1, + invocation_loc_id, + fn_loc_id, + )); } } } @@ -75,20 +84,31 @@ impl DebugTraceIdentifier { let fn_loc_id = get_loc_id(&prev_source_element); if let Some((i, _)) = - stack.iter().enumerate().rfind(|(_, (_, _, i_loc, f_loc))| { + stack.iter().enumerate().rfind(|(_, (_, _, _, i_loc, f_loc))| { *i_loc == invocation_loc_id || *f_loc == fn_loc_id }) { // We've found a match, remove all records between start and end, those // are considered invalid. - let (function_name, start_idx, ..) = stack.split_off(i).swap_remove(0); + let (function_name, maybe_function, start_idx, ..) = + stack.split_off(i).swap_remove(0); let gas_used = node.trace.steps[start_idx].gas_remaining as i64 - node.trace.steps[step_idx].gas_remaining as i64; + let inputs = maybe_function.as_ref().and_then(|f| { + try_decode_args_from_step(&f, true, &node.trace.steps[start_idx + 1]) + }); + + let outputs = maybe_function + .as_ref() + .and_then(|f| try_decode_args_from_step(&f, false, &step)); + identified.push(DecodedTraceStep { start_step_idx: start_idx, end_step_idx: Some(step_idx), + inputs, + outputs, function_name, gas_used, }); @@ -100,17 +120,26 @@ impl DebugTraceIdentifier { prev_step = Some((source_element, source)); } - for (name, step_idx, ..) in stack { - let gas_used = node.trace.steps[step_idx].gas_remaining as i64 - + /* + // Handle stack entires which didn't match any jumps out. + for (name, maybe_function, start_idx, ..) in stack { + let gas_used = node.trace.steps[start_idx].gas_remaining as i64 - node.trace.steps.last().map(|s| s.gas_remaining).unwrap_or_default() as i64; + let inputs = maybe_function + .as_ref() + .and_then(|f| try_decode_args_from_step(&f, true, &node.trace.steps[start_idx])); + identified.push(DecodedTraceStep { - start_step_idx: step_idx, + start_step_idx: start_idx, + inputs, + outputs: None, end_step_idx: None, function_name: name.clone(), gas_used, }); } + */ // Sort by start step index. identified.sort_by_key(|i| i.start_step_idx); @@ -123,7 +152,10 @@ impl DebugTraceIdentifier { /// contains the given function. /// /// Returns string in the format `Contract::function`. -fn parse_function_name(source: &SourceData, loc: &SourceElement) -> Option { +fn parse_function_from_loc( + source: &SourceData, + loc: &SourceElement, +) -> Option<(String, Option)> { let start = loc.offset() as usize; let end = start + loc.length() as usize; let source_part = &source.source[start..end]; @@ -133,5 +165,96 @@ fn parse_function_name(source: &SourceData, loc: &SourceElement) -> Option Option { + let start = loc.offset() as usize; + let end = start + loc.length() as usize; + let source_part = &source.source[start..end]; + + let source_part = source_part.split_once("{")?.0.trim(); + + let source_part = source_part + .replace('\n', "") + .replace("public", "") + .replace("private", "") + .replace("internal", "") + .replace("external", "") + .replace("payable", "") + .replace("view", "") + .replace("pure", "") + .replace("virtual", "") + .replace("override", ""); + + Function::parse(&source_part).ok() +} + +/// GIven [Function] and [CallTraceStep], tries to decode function inputs or outputs from stack and +/// memory contents. +fn try_decode_args_from_step( + func: &Function, + input: bool, + step: &CallTraceStep, +) -> Option> { + let params = if input { &func.inputs } else { &func.outputs }; + + if params.len() == 0 { + return Some(vec![]); + } + + // We can only decode primitive types at the moment. This will filter out any user defined types + // (e.g. structs, enums, etc). + let Ok(types) = + params.iter().map(|p| DynSolType::parse(&p.selector_type())).collect::, _>>() + else { + return None; + }; + + let stack = step.stack.as_ref()?; + + if stack.len() < types.len() { + return None; + } + + let inputs = &stack[stack.len() - types.len()..]; + + let decoded = inputs + .iter() + .zip(types.iter()) + .map(|(input, type_)| { + let maybe_decoded = match type_ { + // read `bytes` and `string` from memory + DynSolType::Bytes | DynSolType::String => { + let memory_offset = input.to::(); + if step.memory.len() < memory_offset as usize { + None + } else { + let length = &step.memory.as_bytes()[memory_offset..memory_offset + 32]; + let length = + U256::from_be_bytes::<32>(length.try_into().unwrap()).to::(); + let data = &step.memory.as_bytes() + [memory_offset + 32..memory_offset + 32 + length as usize]; + + match type_ { + DynSolType::Bytes => Some(DynSolValue::Bytes(data.to_vec())), + DynSolType::String => { + Some(DynSolValue::String(String::from_utf8_lossy(data).to_string())) + } + _ => unreachable!(), + } + } + } + // read other types from stack + _ => type_.abi_decode(&input.to_be_bytes::<32>()).ok(), + }; + if let Some(value) = maybe_decoded { + format_token(&value) + } else { + "".to_string() + } + }) + .collect(); + + Some(decoded) } diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index f6ac84d5a27c..1dde3570412b 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -66,6 +66,8 @@ pub struct DecodedTraceStep { pub start_step_idx: usize, pub end_step_idx: Option, pub function_name: String, + pub inputs: Option>, + pub outputs: Option>, pub gas_used: i64, } @@ -134,7 +136,17 @@ pub async fn render_trace_arena( .iter() .find(|d| *step_idx == d.start_step_idx) { - writeln!(s, "{left}[{}] {}", decoded.gas_used, decoded.function_name)?; + writeln!( + s, + "{left}[{}] {}{}", + decoded.gas_used, + decoded.function_name, + decoded + .inputs + .as_ref() + .map(|v| format!("({})", v.join(", "))) + .unwrap_or_default() + )?; let left_prefix = format!("{right}{BRANCH}"); let right_prefix = format!("{right}{PIPE}"); ordering_idx = render_items( @@ -150,7 +162,13 @@ pub async fn render_trace_arena( ) .await?; - writeln!(s, "{right}{EDGE}{RETURN}")?; + write!(s, "{right}{EDGE}{RETURN}")?; + + if let Some(outputs) = &decoded.outputs { + write!(s, " {}", outputs.join(", "))?; + } + + writeln!(s)?; } } } From 5972083b1894f5053eb9e2cfb6937431c7a87732 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Mon, 24 Jun 2024 19:19:41 +0400 Subject: [PATCH 14/30] clippy --- crates/evm/traces/src/debug/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/evm/traces/src/debug/mod.rs b/crates/evm/traces/src/debug/mod.rs index d1713603a2cc..3ae179710736 100644 --- a/crates/evm/traces/src/debug/mod.rs +++ b/crates/evm/traces/src/debug/mod.rs @@ -97,12 +97,12 @@ impl DebugTraceIdentifier { node.trace.steps[step_idx].gas_remaining as i64; let inputs = maybe_function.as_ref().and_then(|f| { - try_decode_args_from_step(&f, true, &node.trace.steps[start_idx + 1]) + try_decode_args_from_step(f, true, &node.trace.steps[start_idx + 1]) }); let outputs = maybe_function .as_ref() - .and_then(|f| try_decode_args_from_step(&f, false, &step)); + .and_then(|f| try_decode_args_from_step(f, false, step)); identified.push(DecodedTraceStep { start_step_idx: start_idx, @@ -199,7 +199,7 @@ fn try_decode_args_from_step( ) -> Option> { let params = if input { &func.inputs } else { &func.outputs }; - if params.len() == 0 { + if params.is_empty() { return Some(vec![]); } @@ -227,14 +227,14 @@ fn try_decode_args_from_step( // read `bytes` and `string` from memory DynSolType::Bytes | DynSolType::String => { let memory_offset = input.to::(); - if step.memory.len() < memory_offset as usize { + if step.memory.len() < memory_offset { None } else { let length = &step.memory.as_bytes()[memory_offset..memory_offset + 32]; let length = U256::from_be_bytes::<32>(length.try_into().unwrap()).to::(); let data = &step.memory.as_bytes() - [memory_offset + 32..memory_offset + 32 + length as usize]; + [memory_offset + 32..memory_offset + 32 + length]; match type_ { DynSolType::Bytes => Some(DynSolValue::Bytes(data.to_vec())), From e9e97a0d51706c2e2cf8cdc0c7ce93db3089d8e4 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Wed, 26 Jun 2024 12:34:37 +0400 Subject: [PATCH 15/30] clean up --- crates/evm/traces/src/debug/mod.rs | 417 +++++++++++++++++------------ 1 file changed, 239 insertions(+), 178 deletions(-) diff --git a/crates/evm/traces/src/debug/mod.rs b/crates/evm/traces/src/debug/mod.rs index 3ae179710736..8ea8db052f09 100644 --- a/crates/evm/traces/src/debug/mod.rs +++ b/crates/evm/traces/src/debug/mod.rs @@ -1,7 +1,9 @@ mod sources; use crate::{CallTraceNode, DecodedTraceStep}; -use alloy_dyn_abi::{DynSolType, DynSolValue}; -use alloy_json_abi::Function; +use alloy_dyn_abi::{ + parser::{Parameters, Storage}, + DynSolType, DynSolValue, Specifier, +}; use alloy_primitives::U256; use foundry_common::fmt::format_token; use foundry_compilers::artifacts::sourcemap::{Jump, SourceElement}; @@ -28,123 +30,163 @@ impl DebugTraceIdentifier { node: &CallTraceNode, contract_name: &str, ) -> Vec { - let mut stack = Vec::new(); - let mut identified = Vec::new(); - - // Helper to get a unique identifier for a source location. - let get_loc_id = |loc: &SourceElement| (loc.index(), loc.offset(), loc.length()); - - let mut prev_step = None; - for (step_idx, step) in node.trace.steps.iter().enumerate() { - // We are only interested in JUMPs. - if step.op != OpCode::JUMP && step.op != OpCode::JUMPDEST { - prev_step = None; - continue; - } + DebugStepsWalker::new(node, &self.contracts_sources, contract_name).walk() + } +} - // Resolve source map if possible. - let Some((source_element, source)) = self.contracts_sources.find_source_mapping( - contract_name, - step.pc, - node.trace.kind.is_any_create(), - ) else { - prev_step = None; - continue; - }; +struct DebugStepsWalker<'a> { + node: &'a CallTraceNode, + current_step: usize, + stack: Vec<(String, usize)>, + identified: Vec, + sources: &'a ContractSources, + contract_name: &'a str, +} - let Some((prev_source_element, _)) = prev_step else { - prev_step = Some((source_element, source)); - continue; - }; +impl<'a> DebugStepsWalker<'a> { + pub fn new( + node: &'a CallTraceNode, + sources: &'a ContractSources, + contract_name: &'a str, + ) -> Self { + Self { + node, + current_step: 0, + stack: Vec::new(), + identified: Vec::new(), + sources, + contract_name, + } + } - match prev_source_element.jump() { - Jump::In => { - let invocation_loc_id = get_loc_id(&prev_source_element); - let fn_loc_id = get_loc_id(&source_element); - - // This usually means that this is a jump into the external function which is an - // entrypoint for the current frame. We don't want to include this to avoid - // duplicating traces. - if invocation_loc_id != fn_loc_id { - if let Some((name, maybe_function)) = - parse_function_from_loc(source, &source_element) - { - stack.push(( - name, - maybe_function, - step_idx - 1, - invocation_loc_id, - fn_loc_id, - )); - } - } - } - Jump::Out => { - let invocation_loc_id = get_loc_id(&source_element); - let fn_loc_id = get_loc_id(&prev_source_element); - - if let Some((i, _)) = - stack.iter().enumerate().rfind(|(_, (_, _, _, i_loc, f_loc))| { - *i_loc == invocation_loc_id || *f_loc == fn_loc_id - }) - { - // We've found a match, remove all records between start and end, those - // are considered invalid. - let (function_name, maybe_function, start_idx, ..) = - stack.split_off(i).swap_remove(0); - - let gas_used = node.trace.steps[start_idx].gas_remaining as i64 - - node.trace.steps[step_idx].gas_remaining as i64; - - let inputs = maybe_function.as_ref().and_then(|f| { - try_decode_args_from_step(f, true, &node.trace.steps[start_idx + 1]) - }); - - let outputs = maybe_function - .as_ref() - .and_then(|f| try_decode_args_from_step(f, false, step)); - - identified.push(DecodedTraceStep { - start_step_idx: start_idx, - end_step_idx: Some(step_idx), - inputs, - outputs, - function_name, - gas_used, - }); - } - } - _ => {} - }; + fn current_step(&self) -> &CallTraceStep { + &self.node.trace.steps[self.current_step] + } - prev_step = Some((source_element, source)); + fn src_map(&self, step: usize) -> Option<(SourceElement, &SourceData)> { + self.sources.find_source_mapping( + self.contract_name, + self.node.trace.steps[step].pc, + self.node.trace.kind.is_any_create(), + ) + } + + fn prev_src_map(&self) -> Option<(SourceElement, &SourceData)> { + if self.current_step == 0 { + return None; } - /* - // Handle stack entires which didn't match any jumps out. - for (name, maybe_function, start_idx, ..) in stack { - let gas_used = node.trace.steps[start_idx].gas_remaining as i64 - - node.trace.steps.last().map(|s| s.gas_remaining).unwrap_or_default() as i64; + self.src_map(self.current_step - 1) + } - let inputs = maybe_function - .as_ref() - .and_then(|f| try_decode_args_from_step(&f, true, &node.trace.steps[start_idx])); - - identified.push(DecodedTraceStep { - start_step_idx: start_idx, - inputs, - outputs: None, - end_step_idx: None, - function_name: name.clone(), - gas_used, - }); + fn current_src_map(&self) -> Option<(SourceElement, &SourceData)> { + self.src_map(self.current_step) + } + + fn is_same_loc(&self, step: usize, other: usize) -> bool { + let Some((loc, _)) = self.src_map(step) else { + return false; + }; + let Some((other_loc, _)) = self.src_map(other) else { + return false; + }; + + loc.offset() == other_loc.offset() && + loc.length() == other_loc.length() && + loc.index() == other_loc.index() + } + + /// Invoked when current step is a JUMPDEST preceeded by a JUMP marked as [Jump::In]. + fn jump_in(&mut self) { + // This usually means that this is a jump into the external function which is an + // entrypoint for the current frame. We don't want to include this to avoid + // duplicating traces. + if self.is_same_loc(self.current_step, self.current_step - 1) { + return; + } + + let Some((source_element, source)) = self.current_src_map() else { + return; + }; + + if let Some(name) = parse_function_from_loc(source, &source_element) { + self.stack.push((name, self.current_step - 1)); + } + } + + /// Invoked when current step is a JUMPDEST preceeded by a JUMP marked as [Jump::Out]. + fn jump_out(&mut self) { + let Some((i, _)) = self.stack.iter().enumerate().rfind(|(_, (_, step_idx))| { + self.is_same_loc(*step_idx, self.current_step) || + self.is_same_loc(step_idx + 1, self.current_step - 1) + }) else { + return + }; + // We've found a match, remove all records between start and end, those + // are considered invalid. + let (function_name, start_idx) = self.stack.split_off(i).swap_remove(0); + + let gas_used = self.node.trace.steps[start_idx].gas_remaining as i64 - + self.node.trace.steps[self.current_step].gas_remaining as i64; + + // Try to decode function inputs and outputs from the stack and memory. + let (inputs, outputs) = self + .src_map(start_idx + 1) + .map(|(source_element, source)| { + let start = source_element.offset() as usize; + let end = start + source_element.length() as usize; + let fn_definition = source.source[start..end].replace('\n', ""); + let (inputs, outputs) = parse_types(&fn_definition); + + ( + inputs.and_then(|t| { + try_decode_args_from_step(&t, &self.node.trace.steps[start_idx + 1]) + }), + outputs.and_then(|t| try_decode_args_from_step(&t, self.current_step())), + ) + }) + .unwrap_or_default(); + + self.identified.push(DecodedTraceStep { + start_step_idx: start_idx, + end_step_idx: Some(self.current_step), + inputs, + outputs, + function_name, + gas_used, + }); + } + + fn process(&mut self) { + // We are only interested in JUMPs. + if self.current_step().op != OpCode::JUMP && self.current_step().op != OpCode::JUMPDEST { + return; } - */ - // Sort by start step index. - identified.sort_by_key(|i| i.start_step_idx); + let Some((prev_source_element, _)) = self.prev_src_map() else { + return; + }; - identified + match prev_source_element.jump() { + Jump::In => self.jump_in(), + Jump::Out => self.jump_out(), + _ => {} + }; + } + + fn step(&mut self) { + self.process(); + self.current_step += 1; + } + + pub fn walk(mut self) -> Vec { + while self.current_step < self.node.trace.steps.len() { + self.step(); + } + + self.identified.sort_by_key(|i| i.start_step_idx); + + self.identified } } @@ -152,10 +194,7 @@ impl DebugTraceIdentifier { /// contains the given function. /// /// Returns string in the format `Contract::function`. -fn parse_function_from_loc( - source: &SourceData, - loc: &SourceElement, -) -> Option<(String, Option)> { +fn parse_function_from_loc(source: &SourceData, loc: &SourceElement) -> Option { let start = loc.offset() as usize; let end = start + loc.length() as usize; let source_part = &source.source[start..end]; @@ -165,51 +204,33 @@ fn parse_function_from_loc( let function_name = source_part.split_once("function")?.1.split('(').next()?.trim(); let contract_name = source.find_contract_name(start, end)?; - Some((format!("{contract_name}::{function_name}"), parse_function(source, loc))) + Some(format!("{contract_name}::{function_name}")) } -fn parse_function(source: &SourceData, loc: &SourceElement) -> Option { - let start = loc.offset() as usize; - let end = start + loc.length() as usize; - let source_part = &source.source[start..end]; - - let source_part = source_part.split_once("{")?.0.trim(); - - let source_part = source_part - .replace('\n', "") - .replace("public", "") - .replace("private", "") - .replace("internal", "") - .replace("external", "") - .replace("payable", "") - .replace("view", "") - .replace("pure", "") - .replace("virtual", "") - .replace("override", ""); - - Function::parse(&source_part).ok() +/// Parses function input and output types into [Parameters]. +fn parse_types(source: &str) -> (Option>, Option>) { + let inputs = source.find('(').and_then(|params_start| { + let params_end = params_start + source[params_start..].find(')')?; + Parameters::parse(&source[params_start..params_end + 1]).ok() + }); + let outputs = source.find("returns").and_then(|returns_start| { + let return_params_start = returns_start + source[returns_start..].find('(')?; + let return_params_end = return_params_start + source[return_params_start..].find(')')?; + Parameters::parse(&source[return_params_start..return_params_end + 1]).ok() + }); + + (inputs, outputs) } -/// GIven [Function] and [CallTraceStep], tries to decode function inputs or outputs from stack and -/// memory contents. -fn try_decode_args_from_step( - func: &Function, - input: bool, - step: &CallTraceStep, -) -> Option> { - let params = if input { &func.inputs } else { &func.outputs }; +/// Given [Parameters] and [CallTraceStep], tries to decode parameters by using stack and memory. +fn try_decode_args_from_step(args: &Parameters<'_>, step: &CallTraceStep) -> Option> { + let params = &args.params; if params.is_empty() { return Some(vec![]); } - // We can only decode primitive types at the moment. This will filter out any user defined types - // (e.g. structs, enums, etc). - let Ok(types) = - params.iter().map(|p| DynSolType::parse(&p.selector_type())).collect::, _>>() - else { - return None; - }; + let types = params.iter().map(|p| p.resolve().ok().map(|t| (t, p.storage))).collect::>(); let stack = step.stack.as_ref()?; @@ -222,39 +243,79 @@ fn try_decode_args_from_step( let decoded = inputs .iter() .zip(types.iter()) - .map(|(input, type_)| { - let maybe_decoded = match type_ { - // read `bytes` and `string` from memory - DynSolType::Bytes | DynSolType::String => { - let memory_offset = input.to::(); - if step.memory.len() < memory_offset { - None - } else { - let length = &step.memory.as_bytes()[memory_offset..memory_offset + 32]; - let length = - U256::from_be_bytes::<32>(length.try_into().unwrap()).to::(); - let data = &step.memory.as_bytes() - [memory_offset + 32..memory_offset + 32 + length]; - - match type_ { - DynSolType::Bytes => Some(DynSolValue::Bytes(data.to_vec())), - DynSolType::String => { - Some(DynSolValue::String(String::from_utf8_lossy(data).to_string())) - } - _ => unreachable!(), + .map(|(input, type_and_storage)| { + type_and_storage + .as_ref() + .and_then(|(type_, storage)| { + match (type_, storage) { + // HACK: alloy parser treates user-defined types as uint8: https://github.com/alloy-rs/core/pull/386 + // + // filter out `uint8` params which are marked as storage or memory as this + // is not possible in Solidity and means that type is user-defined + (DynSolType::Uint(8), Some(Storage::Memory | Storage::Storage)) => None, + (_, Some(Storage::Memory)) => { + decode_from_memory(type_, step.memory.as_bytes(), input.to::()) } + // Read other types from stack + _ => type_.abi_decode(&input.to_be_bytes::<32>()).ok(), } - } - // read other types from stack - _ => type_.abi_decode(&input.to_be_bytes::<32>()).ok(), - }; - if let Some(value) = maybe_decoded { - format_token(&value) - } else { - "".to_string() - } + }) + .as_ref() + .map(format_token) + .unwrap_or_else(|| "".to_string()) }) .collect(); Some(decoded) } + +/// Decodes given [DynSolType] from memory. +fn decode_from_memory(ty: &DynSolType, memory: &[u8], location: usize) -> Option { + let first_word = &memory[location..location + 32]; + + match ty { + // For `string` and `bytes` layout is a word with length followed by the data + DynSolType::String | DynSolType::Bytes => { + let length = U256::from_be_bytes::<32>(first_word.try_into().unwrap()).to::(); + let data = &memory[location + 32..location + 32 + length]; + + match ty { + DynSolType::Bytes => Some(DynSolValue::Bytes(data.to_vec())), + DynSolType::String => { + Some(DynSolValue::String(String::from_utf8_lossy(data).to_string())) + } + _ => unreachable!(), + } + } + // Dynamic arrays are encoded as a word with length followed by words with elements + // Fixed arrays are encoded as words with elements + DynSolType::Array(inner) | DynSolType::FixedArray(inner, _) => { + let (length, start) = match ty { + DynSolType::FixedArray(_, length) => (*length, location), + DynSolType::Array(_) => ( + U256::from_be_bytes::<32>(first_word.try_into().unwrap()).to::(), + location + 32, + ), + _ => unreachable!(), + }; + let mut decoded = Vec::with_capacity(length); + + for i in 0..length { + let offset = start + i * 32; + let location = match inner.as_ref() { + // Arrays of variable length types are arrays of pointers to the values + DynSolType::String | DynSolType::Bytes | DynSolType::Array(_) => { + U256::from_be_bytes::<32>(memory[offset..offset + 32].try_into().unwrap()) + .to() + } + _ => offset, + }; + + decoded.push(decode_from_memory(inner, memory, location)?); + } + + Some(DynSolValue::Array(decoded)) + } + _ => ty.abi_decode(first_word).ok(), + } +} From 08fd6c5bd12586aa72a5fa4e2cc980c50d612025 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Thu, 27 Jun 2024 10:50:21 +0400 Subject: [PATCH 16/30] TraceMode --- Cargo.lock | 1 - crates/chisel/src/executor.rs | 4 +- crates/debugger/Cargo.toml | 1 - crates/debugger/src/tui/builder.rs | 4 +- crates/debugger/src/tui/mod.rs | 4 +- .../evm/evm/src/executors/invariant/replay.rs | 4 +- crates/evm/evm/src/executors/mod.rs | 6 +- crates/evm/evm/src/executors/trace.rs | 12 +++- crates/evm/evm/src/inspectors/stack.rs | 69 +++++-------------- crates/evm/traces/src/lib.rs | 54 ++++++++++++++- crates/forge/bin/cmd/test/mod.rs | 2 +- crates/forge/src/multi_runner.rs | 28 +++++--- crates/forge/src/runner.rs | 6 +- crates/script/src/lib.rs | 5 +- 14 files changed, 115 insertions(+), 85 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2998ee760197..451d14ea99ef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3836,7 +3836,6 @@ dependencies = [ "eyre", "foundry-common", "foundry-compilers", - "foundry-evm-core", "foundry-evm-traces", "ratatui", "revm", diff --git a/crates/chisel/src/executor.rs b/crates/chisel/src/executor.rs index 5aa2190df589..c6282b3da409 100644 --- a/crates/chisel/src/executor.rs +++ b/crates/chisel/src/executor.rs @@ -13,7 +13,7 @@ use eyre::{Result, WrapErr}; use foundry_compilers::Artifact; use foundry_evm::{ backend::Backend, decode::decode_console_logs, executors::ExecutorBuilder, - inspectors::CheatsConfig, + inspectors::CheatsConfig, traces::TraceMode, }; use solang_parser::pt::{self, CodeLocation}; use std::str::FromStr; @@ -302,7 +302,7 @@ impl SessionSource { // Build a new executor let executor = ExecutorBuilder::new() .inspectors(|stack| { - stack.chisel_state(final_pc).trace(true).cheatcodes( + stack.chisel_state(final_pc).trace_mode(TraceMode::Call).cheatcodes( CheatsConfig::new( &self.config.foundry_config, self.config.evm_opts.clone(), diff --git a/crates/debugger/Cargo.toml b/crates/debugger/Cargo.toml index 094f2aa1fb6b..9f96eb7f06da 100644 --- a/crates/debugger/Cargo.toml +++ b/crates/debugger/Cargo.toml @@ -15,7 +15,6 @@ workspace = true [dependencies] foundry-common.workspace = true foundry-compilers.workspace = true -foundry-evm-core.workspace = true foundry-evm-traces.workspace = true revm-inspectors.workspace = true diff --git a/crates/debugger/src/tui/builder.rs b/crates/debugger/src/tui/builder.rs index 70055243ea38..81632dcca648 100644 --- a/crates/debugger/src/tui/builder.rs +++ b/crates/debugger/src/tui/builder.rs @@ -2,8 +2,8 @@ use crate::{node::flatten_call_trace, DebugNode, Debugger}; use alloy_primitives::Address; -use foundry_common::{compile::ContractSources, evm::Breakpoints, get_contract_name}; -use foundry_evm_traces::{CallTraceArena, CallTraceDecoder, Traces}; +use foundry_common::{evm::Breakpoints, get_contract_name}; +use foundry_evm_traces::{debug::ContractSources, CallTraceArena, CallTraceDecoder, Traces}; use std::collections::HashMap; /// Debugger builder. diff --git a/crates/debugger/src/tui/mod.rs b/crates/debugger/src/tui/mod.rs index 43dacd652538..6b522250b8aa 100644 --- a/crates/debugger/src/tui/mod.rs +++ b/crates/debugger/src/tui/mod.rs @@ -7,8 +7,8 @@ use crossterm::{ terminal::{disable_raw_mode, enable_raw_mode, EnterAlternateScreen, LeaveAlternateScreen}, }; use eyre::Result; -use foundry_common::{compile::ContractSources, evm::Breakpoints}; -use foundry_evm_core::utils::PcIcMap; +use foundry_common::evm::Breakpoints; +use foundry_evm_traces::debug::ContractSources; use ratatui::{ backend::{Backend, CrosstermBackend}, Terminal, diff --git a/crates/evm/evm/src/executors/invariant/replay.rs b/crates/evm/evm/src/executors/invariant/replay.rs index 391bf998804b..6f04e8b6daa5 100644 --- a/crates/evm/evm/src/executors/invariant/replay.rs +++ b/crates/evm/evm/src/executors/invariant/replay.rs @@ -12,7 +12,7 @@ use foundry_evm_fuzz::{ invariant::{BasicTxDetails, InvariantContract}, BaseCounterExample, }; -use foundry_evm_traces::{load_contracts, TraceKind, Traces}; +use foundry_evm_traces::{load_contracts, TraceKind, TraceMode, Traces}; use indicatif::ProgressBar; use parking_lot::RwLock; use proptest::test_runner::TestError; @@ -34,7 +34,7 @@ pub fn replay_run( ) -> Result> { // We want traces for a failed case. if executor.inspector().tracer.is_none() { - executor.set_tracing(true, false); + executor.set_tracing(Some(TraceMode::Call)); } let mut counterexample_sequence = vec![]; diff --git a/crates/evm/evm/src/executors/mod.rs b/crates/evm/evm/src/executors/mod.rs index 09fd620f5f23..efcadc0a2e84 100644 --- a/crates/evm/evm/src/executors/mod.rs +++ b/crates/evm/evm/src/executors/mod.rs @@ -23,7 +23,7 @@ use foundry_evm_core::{ utils::StateChangeset, }; use foundry_evm_coverage::HitMaps; -use foundry_evm_traces::CallTraceArena; +use foundry_evm_traces::{CallTraceArena, TraceMode}; use revm::{ db::{DatabaseCommit, DatabaseRef}, interpreter::{return_ok, InstructionResult}, @@ -211,8 +211,8 @@ impl Executor { } #[inline] - pub fn set_tracing(&mut self, tracing: bool, debug: bool) -> &mut Self { - self.inspector_mut().tracing(tracing, debug); + pub fn set_tracing(&mut self, mode: Option) -> &mut Self { + self.inspector_mut().tracing(mode); self } diff --git a/crates/evm/evm/src/executors/trace.rs b/crates/evm/evm/src/executors/trace.rs index 29373ad66865..56718cf370fd 100644 --- a/crates/evm/evm/src/executors/trace.rs +++ b/crates/evm/evm/src/executors/trace.rs @@ -2,6 +2,7 @@ use crate::executors::{Executor, ExecutorBuilder}; use foundry_compilers::artifacts::EvmVersion; use foundry_config::{utils::evm_spec_id, Chain, Config}; use foundry_evm_core::{backend::Backend, fork::CreateFork, opts::EvmOpts}; +use foundry_evm_traces::TraceMode; use revm::primitives::{Env, SpecId}; use std::ops::{Deref, DerefMut}; @@ -16,14 +17,21 @@ impl TracingExecutor { fork: Option, version: Option, debug: bool, - trace_steps: bool, + decode_internal: bool, ) -> Self { let db = Backend::spawn(fork); + let trace_mode = if debug { + TraceMode::Debug + } else if decode_internal { + TraceMode::Jump + } else { + TraceMode::Call + }; Self { // configures a bare version of the evm executor: no cheatcode inspector is enabled, // tracing will be enabled only for the targeted transaction executor: ExecutorBuilder::new() - .inspectors(|stack| stack.trace(true).debug(debug).debug_trace(trace_steps)) + .inspectors(|stack| stack.trace_mode(trace_mode)) .spec(evm_spec_id(&version.unwrap_or_default())) .build(env, db), } diff --git a/crates/evm/evm/src/inspectors/stack.rs b/crates/evm/evm/src/inspectors/stack.rs index 9c78d7f8a1ff..b5d5a734d585 100644 --- a/crates/evm/evm/src/inspectors/stack.rs +++ b/crates/evm/evm/src/inspectors/stack.rs @@ -1,6 +1,6 @@ use super::{ Cheatcodes, CheatsConfig, ChiselState, CoverageCollector, Fuzzer, LogCollector, - StackSnapshotType, TracingInspector, TracingInspectorConfig, + TracingInspector, }; use alloy_primitives::{Address, Bytes, Log, TxKind, U256}; use foundry_cheatcodes::CheatcodesExecutor; @@ -9,7 +9,7 @@ use foundry_evm_core::{ InspectorExt, }; use foundry_evm_coverage::HitMaps; -use foundry_evm_traces::CallTraceArena; +use foundry_evm_traces::{CallTraceArena, TraceMode}; use revm::{ inspectors::CustomPrintTracer, interpreter::{ @@ -45,15 +45,7 @@ pub struct InspectorStackBuilder { /// The fuzzer inspector and its state, if it exists. pub fuzzer: Option, /// Whether to enable tracing. - pub trace: Option, -<<<<<<< HEAD - /// Whether to enable steps tracking in the tracer. - pub debug_trace: Option, - /// Whether to enable the debugger. -======= - /// Whether to enable debug traces. ->>>>>>> master - pub debug: Option, + pub trace_mode: Option, /// Whether logs should be collected. pub logs: Option, /// Whether coverage info should be collected. @@ -124,13 +116,6 @@ impl InspectorStackBuilder { self } - /// Set whether to enable the debugger. - #[inline] - pub fn debug(mut self, yes: bool) -> Self { - self.debug = Some(yes); - self - } - /// Set whether to enable the trace printer. #[inline] pub fn print(mut self, yes: bool) -> Self { @@ -140,15 +125,16 @@ impl InspectorStackBuilder { /// Set whether to enable the tracer. #[inline] - pub fn trace(mut self, yes: bool) -> Self { - self.trace = Some(yes); - self - } - - /// Set whether to enable steps tracking in the tracer. - #[inline] - pub fn debug_trace(mut self, yes: bool) -> Self { - self.debug_trace = Some(yes); + pub fn trace_mode(mut self, mode: impl Into>) -> Self { + if let Some(mode) = mode.into() { + if let Some(current) = self.trace_mode.as_mut() { + if *current <= mode { + *current = mode; + } + } else { + self.trace_mode = Some(mode); + } + } self } @@ -167,14 +153,12 @@ impl InspectorStackBuilder { gas_price, cheatcodes, fuzzer, - trace, - debug, + trace_mode, logs, coverage, print, chisel_state, enable_isolation, - debug_trace, } = self; let mut stack = InspectorStack::new(); @@ -191,11 +175,7 @@ impl InspectorStackBuilder { stack.collect_coverage(coverage.unwrap_or(false)); stack.collect_logs(logs.unwrap_or(true)); stack.print(print.unwrap_or(false)); -<<<<<<< HEAD - stack.tracing(trace.unwrap_or(false), debug_trace.unwrap_or(false)); -======= - stack.tracing(trace.unwrap_or(false), debug.unwrap_or(false)); ->>>>>>> master + stack.tracing(trace_mode); stack.enable_isolation(enable_isolation); @@ -432,23 +412,8 @@ impl InspectorStack { /// Set whether to enable the tracer. #[inline] - pub fn tracing(&mut self, yes: bool, debug: bool) { - self.tracer = yes.then(|| { - TracingInspector::new(TracingInspectorConfig { - record_steps: debug, - record_memory_snapshots: debug, - record_stack_snapshots: if debug { - StackSnapshotType::Full - } else { - StackSnapshotType::None - }, - record_state_diff: false, - exclude_precompile_calls: false, - record_logs: true, - record_opcodes_filter: None, - record_returndata_snapshots: debug, - }) - }); + pub fn tracing(&mut self, mode: Option) { + self.tracer = mode.map(|mode| TracingInspector::new(mode.into())); } /// Collects all the data gathered during inspection into a single struct. diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index a44367e48366..6f6d46586ee0 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -12,7 +12,8 @@ use alloy_primitives::{hex, LogData}; use foundry_common::contracts::{ContractsByAddress, ContractsByArtifact}; use foundry_evm_core::constants::CHEATCODE_ADDRESS; use futures::{future::BoxFuture, FutureExt}; -use revm_inspectors::tracing::types::TraceMemberOrder; +use revm::interpreter::OpCode; +use revm_inspectors::tracing::{types::TraceMemberOrder, OpcodeFilter}; use serde::{Deserialize, Serialize}; use std::fmt::Write; use yansi::{Color, Paint}; @@ -400,3 +401,54 @@ pub fn load_contracts<'a>( } contracts } + +/// Different kinds of traces used by different foundry components. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default)] +pub enum TraceMode { + /// Simple call trace, no steps tracing required. + #[default] + Call, + /// Call trace with tracing for JUMP and JUMPDEST opcode steps. + /// + /// Used for internal functions identification. + Jump, + /// Call trace with complete steps tracing. + /// + /// Used by debugger. + Debug, +} + +impl TraceMode { + pub const fn is_call(self) -> bool { + matches!(self, Self::Call) + } + + pub const fn is_jump(self) -> bool { + matches!(self, Self::Jump) + } + + pub const fn is_debug(self) -> bool { + matches!(self, Self::Debug) + } +} + +impl From for TracingInspectorConfig { + fn from(mode: TraceMode) -> Self { + TracingInspectorConfig { + record_steps: mode.is_debug() || mode.is_jump(), + record_memory_snapshots: mode.is_debug() || mode.is_jump(), + record_stack_snapshots: if mode.is_debug() || mode.is_jump() { + StackSnapshotType::Full + } else { + StackSnapshotType::None + }, + record_logs: true, + record_state_diff: false, + record_returndata_snapshots: mode.is_debug(), + record_opcodes_filter: mode + .is_jump() + .then(|| OpcodeFilter::new().enable(OpCode::JUMP).enable(OpCode::JUMPDEST)), + exclude_precompile_calls: false, + } + } +} diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 81efe49737d2..f593839e9682 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -306,7 +306,7 @@ impl TestArgs { let runner = MultiContractRunnerBuilder::new(config.clone()) .set_debug(should_debug) - .set_trace_steps(self.decode_internal) + .set_decode_internal(self.decode_internal) .initial_balance(evm_opts.initial_balance) .evm_spec(config.evm_spec_id()) .sender(evm_opts.sender) diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index ead8940c6d53..0eea4e57a658 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -14,7 +14,7 @@ use foundry_compilers::{ use foundry_config::Config; use foundry_evm::{ backend::Backend, decode::RevertDecoder, executors::ExecutorBuilder, fork::CreateFork, - inspectors::CheatsConfig, opts::EvmOpts, revm, + inspectors::CheatsConfig, opts::EvmOpts, revm, traces::TraceMode, }; use foundry_linking::{LinkOutput, Linker}; use rayon::prelude::*; @@ -61,7 +61,7 @@ pub struct MultiContractRunner { /// Whether to collect debug info pub debug: bool, /// Whether to enable steps tracking in the tracer. - pub trace_steps: bool, + pub decode_internal: bool, /// Settings related to fuzz and/or invariant tests pub test_options: TestOptions, /// Whether to enable call isolation @@ -237,14 +237,22 @@ impl MultiContractRunner { None, Some(artifact_id.version.clone()), ); + + let trace_mode = if self.debug { + Some(TraceMode::Debug) + } else if self.decode_internal { + Some(TraceMode::Jump) + } else if self.evm_opts.verbosity >= 3 { + Some(TraceMode::Call) + } else { + None + }; let executor = ExecutorBuilder::new() .inspectors(|stack| { stack .cheatcodes(Arc::new(cheats_config)) - .trace(self.evm_opts.verbosity >= 3 || self.debug || self.trace_steps) - .debug(self.debug) - .debug_trace(self.trace_steps) + .trace_mode(trace_mode) .coverage(self.coverage) .enable_isolation(self.isolation) }) @@ -302,7 +310,7 @@ pub struct MultiContractRunnerBuilder { /// Whether or not to collect debug info pub debug: bool, /// Whether to enable steps tracking in the tracer. - pub trace_steps: bool, + pub decode_internal: bool, /// Whether to enable call isolation pub isolation: bool, /// Settings related to fuzz and/or invariant tests @@ -321,7 +329,7 @@ impl MultiContractRunnerBuilder { debug: Default::default(), isolation: Default::default(), test_options: Default::default(), - trace_steps: Default::default(), + decode_internal: Default::default(), } } @@ -360,8 +368,8 @@ impl MultiContractRunnerBuilder { self } - pub fn set_trace_steps(mut self, enable: bool) -> Self { - self.trace_steps = enable; + pub fn set_decode_internal(mut self, enable: bool) -> Self { + self.decode_internal = enable; self } @@ -432,7 +440,7 @@ impl MultiContractRunnerBuilder { config: self.config, coverage: self.coverage, debug: self.debug, - trace_steps: self.trace_steps, + decode_internal: self.decode_internal, test_options: self.test_options.unwrap_or_default(), isolation: self.isolation, known_contracts, diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index ac186a3e1fbc..dd9736533378 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -32,7 +32,7 @@ use foundry_evm::{ invariant::{CallDetails, InvariantContract}, CounterExample, FuzzFixtures, }, - traces::{load_contracts, TraceKind}, + traces::{load_contracts, TraceKind, TraceMode}, }; use proptest::test_runner::TestRunner; use rayon::prelude::*; @@ -312,13 +312,13 @@ impl<'a> ContractRunner<'a> { let tmp_tracing = self.executor.inspector().tracer.is_none() && has_invariants && call_setup; if tmp_tracing { - self.executor.set_tracing(true, false); + self.executor.set_tracing(Some(TraceMode::Call)); } let setup_time = Instant::now(); let setup = self.setup(call_setup); debug!("finished setting up in {:?}", setup_time.elapsed()); if tmp_tracing { - self.executor.set_tracing(false, false); + self.executor.set_tracing(None); } if setup.reason.is_some() { diff --git a/crates/script/src/lib.rs b/crates/script/src/lib.rs index c1ae8882e35d..92ce12f33a38 100644 --- a/crates/script/src/lib.rs +++ b/crates/script/src/lib.rs @@ -43,7 +43,7 @@ use foundry_evm::{ CheatsConfig, }, opts::EvmOpts, - traces::Traces, + traces::{TraceMode, Traces}, }; use foundry_wallets::MultiWalletOpts; use serde::{Deserialize, Serialize}; @@ -574,14 +574,13 @@ impl ScriptConfig { // We need to enable tracing to decode contract names: local or external. let mut builder = ExecutorBuilder::new() - .inspectors(|stack| stack.trace(true)) + .inspectors(|stack| stack.trace_mode(if debug { TraceMode::Debug } else { TraceMode::Call })) .spec(self.config.evm_spec_id()) .gas_limit(self.evm_opts.gas_limit()); if let Some((known_contracts, script_wallets, target)) = cheats_data { builder = builder.inspectors(|stack| { stack - .debug(debug) .cheatcodes( CheatsConfig::new( &self.config, From 34644d3ad9b760b7a1301bb6d63909062f70b648 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Thu, 27 Jun 2024 11:05:40 +0400 Subject: [PATCH 17/30] small fixes --- crates/evm/traces/src/debug/mod.rs | 12 +++++++----- crates/forge/src/multi_runner.rs | 2 +- crates/script/src/lib.rs | 4 +++- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/evm/traces/src/debug/mod.rs b/crates/evm/traces/src/debug/mod.rs index 8ea8db052f09..7dd8eadac730 100644 --- a/crates/evm/traces/src/debug/mod.rs +++ b/crates/evm/traces/src/debug/mod.rs @@ -248,7 +248,7 @@ fn try_decode_args_from_step(args: &Parameters<'_>, step: &CallTraceStep) -> Opt .as_ref() .and_then(|(type_, storage)| { match (type_, storage) { - // HACK: alloy parser treates user-defined types as uint8: https://github.com/alloy-rs/core/pull/386 + // HACK: alloy parser treats user-defined types as uint8: https://github.com/alloy-rs/core/pull/386 // // filter out `uint8` params which are marked as storage or memory as this // is not possible in Solidity and means that type is user-defined @@ -271,13 +271,13 @@ fn try_decode_args_from_step(args: &Parameters<'_>, step: &CallTraceStep) -> Opt /// Decodes given [DynSolType] from memory. fn decode_from_memory(ty: &DynSolType, memory: &[u8], location: usize) -> Option { - let first_word = &memory[location..location + 32]; + let first_word = memory.get(location..location + 32)?; match ty { // For `string` and `bytes` layout is a word with length followed by the data DynSolType::String | DynSolType::Bytes => { let length = U256::from_be_bytes::<32>(first_word.try_into().unwrap()).to::(); - let data = &memory[location + 32..location + 32 + length]; + let data = memory.get(location + 32..location + 32 + length)?; match ty { DynSolType::Bytes => Some(DynSolValue::Bytes(data.to_vec())), @@ -305,8 +305,10 @@ fn decode_from_memory(ty: &DynSolType, memory: &[u8], location: usize) -> Option let location = match inner.as_ref() { // Arrays of variable length types are arrays of pointers to the values DynSolType::String | DynSolType::Bytes | DynSolType::Array(_) => { - U256::from_be_bytes::<32>(memory[offset..offset + 32].try_into().unwrap()) - .to() + U256::from_be_bytes::<32>( + memory.get(offset..offset + 32)?.try_into().unwrap(), + ) + .to() } _ => offset, }; diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 0eea4e57a658..06dcb31c6536 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -237,7 +237,7 @@ impl MultiContractRunner { None, Some(artifact_id.version.clone()), ); - + let trace_mode = if self.debug { Some(TraceMode::Debug) } else if self.decode_internal { diff --git a/crates/script/src/lib.rs b/crates/script/src/lib.rs index 92ce12f33a38..b088713b5565 100644 --- a/crates/script/src/lib.rs +++ b/crates/script/src/lib.rs @@ -574,7 +574,9 @@ impl ScriptConfig { // We need to enable tracing to decode contract names: local or external. let mut builder = ExecutorBuilder::new() - .inspectors(|stack| stack.trace_mode(if debug { TraceMode::Debug } else { TraceMode::Call })) + .inspectors(|stack| { + stack.trace_mode(if debug { TraceMode::Debug } else { TraceMode::Call }) + }) .spec(self.config.evm_spec_id()) .gas_limit(self.evm_opts.gas_limit()); From a725b2873752cdea3faea759d8b59e23ec367a23 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Thu, 27 Jun 2024 11:42:52 +0400 Subject: [PATCH 18/30] add doc --- crates/cli/src/utils/cmd.rs | 5 +---- crates/evm/traces/src/debug/mod.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/crates/cli/src/utils/cmd.rs b/crates/cli/src/utils/cmd.rs index fdc044555b08..1242e77bde37 100644 --- a/crates/cli/src/utils/cmd.rs +++ b/crates/cli/src/utils/cmd.rs @@ -385,10 +385,7 @@ pub async fn handle_traces( } else { Default::default() }; - - let identifier = DebugTraceIdentifier::new(sources); - - decoder.debug_identifier = Some(identifier); + decoder.debug_identifier = Some(DebugTraceIdentifier::new(sources)); } if debug { diff --git a/crates/evm/traces/src/debug/mod.rs b/crates/evm/traces/src/debug/mod.rs index 7dd8eadac730..8fecf9c9bd2d 100644 --- a/crates/evm/traces/src/debug/mod.rs +++ b/crates/evm/traces/src/debug/mod.rs @@ -34,6 +34,32 @@ impl DebugTraceIdentifier { } } +/// Walks through the [CallTraceStep]s attempting to match JUMPs to internal functions. +/// +/// This is done by looking up jump kinds in the source maps. The structure of internal function +/// call always looks like this: +/// - JUMP +/// - JUMPDEST +/// ... function steps ... +/// - JUMP +/// - JUMPDEST +/// +/// The assumption we rely on is that first JUMP into function will be marked as [Jump::In] in +/// source map, and second JUMP out of the function will be marked as [Jump::Out]. +/// +/// Also, we rely on JUMPDEST after first JUMP pointing to the source location of the body of +/// function which was entered. We pass this source part to [parse_function_from_loc] to extract the +/// function name. +/// +/// When we find a [Jump::In] and identify the function name, we push it to the stack. +/// +/// When we find a [Jump::Out] we try to find a matching [Jump::In] in the stack. A match is found +/// when source location of the JUMP-in matches the source location of final JUMPDEST (this would be +/// the location of the function invocation), or when source location of first JUMODEST matches the +/// source location of the JUMP-out (this would be the location of function body). +/// +/// When a match is found, all items which were pushed after the matched function are removed. There +/// is a lot of such items due to source maps getting malformed during optimization. struct DebugStepsWalker<'a> { node: &'a CallTraceNode, current_step: usize, From c47abbbbf717dbce4618b917da340bcd5f68377a Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Thu, 27 Jun 2024 11:56:45 +0400 Subject: [PATCH 19/30] clippy --- crates/evm/traces/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index 6f6d46586ee0..bc28f4b53f4c 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -434,7 +434,7 @@ impl TraceMode { impl From for TracingInspectorConfig { fn from(mode: TraceMode) -> Self { - TracingInspectorConfig { + Self { record_steps: mode.is_debug() || mode.is_jump(), record_memory_snapshots: mode.is_debug() || mode.is_jump(), record_stack_snapshots: if mode.is_debug() || mode.is_jump() { From 7d362f7522c74d10a5f742faec067027e19e500f Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 28 Jun 2024 15:47:21 +0400 Subject: [PATCH 20/30] safer decofing from stack and memory --- crates/evm/traces/src/debug/mod.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/crates/evm/traces/src/debug/mod.rs b/crates/evm/traces/src/debug/mod.rs index 8fecf9c9bd2d..d087559fd1c3 100644 --- a/crates/evm/traces/src/debug/mod.rs +++ b/crates/evm/traces/src/debug/mod.rs @@ -279,9 +279,11 @@ fn try_decode_args_from_step(args: &Parameters<'_>, step: &CallTraceStep) -> Opt // filter out `uint8` params which are marked as storage or memory as this // is not possible in Solidity and means that type is user-defined (DynSolType::Uint(8), Some(Storage::Memory | Storage::Storage)) => None, - (_, Some(Storage::Memory)) => { - decode_from_memory(type_, step.memory.as_bytes(), input.to::()) - } + (_, Some(Storage::Memory)) => decode_from_memory( + type_, + step.memory.as_bytes(), + input.try_into().ok()?, + ), // Read other types from stack _ => type_.abi_decode(&input.to_be_bytes::<32>()).ok(), } @@ -302,7 +304,7 @@ fn decode_from_memory(ty: &DynSolType, memory: &[u8], location: usize) -> Option match ty { // For `string` and `bytes` layout is a word with length followed by the data DynSolType::String | DynSolType::Bytes => { - let length = U256::from_be_bytes::<32>(first_word.try_into().unwrap()).to::(); + let length: usize = U256::from_be_slice(first_word).try_into().ok()?; let data = memory.get(location + 32..location + 32 + length)?; match ty { @@ -318,10 +320,9 @@ fn decode_from_memory(ty: &DynSolType, memory: &[u8], location: usize) -> Option DynSolType::Array(inner) | DynSolType::FixedArray(inner, _) => { let (length, start) = match ty { DynSolType::FixedArray(_, length) => (*length, location), - DynSolType::Array(_) => ( - U256::from_be_bytes::<32>(first_word.try_into().unwrap()).to::(), - location + 32, - ), + DynSolType::Array(_) => { + (U256::from_be_slice(first_word).try_into().ok()?, location + 32) + } _ => unreachable!(), }; let mut decoded = Vec::with_capacity(length); @@ -331,10 +332,7 @@ fn decode_from_memory(ty: &DynSolType, memory: &[u8], location: usize) -> Option let location = match inner.as_ref() { // Arrays of variable length types are arrays of pointers to the values DynSolType::String | DynSolType::Bytes | DynSolType::Array(_) => { - U256::from_be_bytes::<32>( - memory.get(offset..offset + 32)?.try_into().unwrap(), - ) - .to() + U256::from_be_slice(memory.get(offset..offset + 32)?).try_into().ok()? } _ => offset, }; From 4d44529a910ad7ec604a971ab124a56e7bd3742b Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 28 Jun 2024 19:33:51 +0400 Subject: [PATCH 21/30] use Into> --- crates/evm/evm/src/executors/invariant/replay.rs | 2 +- crates/evm/evm/src/executors/mod.rs | 4 ++-- crates/forge/src/runner.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/evm/evm/src/executors/invariant/replay.rs b/crates/evm/evm/src/executors/invariant/replay.rs index 6f04e8b6daa5..9de5bf531714 100644 --- a/crates/evm/evm/src/executors/invariant/replay.rs +++ b/crates/evm/evm/src/executors/invariant/replay.rs @@ -34,7 +34,7 @@ pub fn replay_run( ) -> Result> { // We want traces for a failed case. if executor.inspector().tracer.is_none() { - executor.set_tracing(Some(TraceMode::Call)); + executor.set_tracing(TraceMode::Call); } let mut counterexample_sequence = vec![]; diff --git a/crates/evm/evm/src/executors/mod.rs b/crates/evm/evm/src/executors/mod.rs index efcadc0a2e84..1071ae7ddf6b 100644 --- a/crates/evm/evm/src/executors/mod.rs +++ b/crates/evm/evm/src/executors/mod.rs @@ -211,8 +211,8 @@ impl Executor { } #[inline] - pub fn set_tracing(&mut self, mode: Option) -> &mut Self { - self.inspector_mut().tracing(mode); + pub fn set_tracing(&mut self, mode: impl Into>) -> &mut Self { + self.inspector_mut().tracing(mode.into()); self } diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index dd9736533378..16e22350f854 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -312,7 +312,7 @@ impl<'a> ContractRunner<'a> { let tmp_tracing = self.executor.inspector().tracer.is_none() && has_invariants && call_setup; if tmp_tracing { - self.executor.set_tracing(Some(TraceMode::Call)); + self.executor.set_tracing(TraceMode::Call); } let setup_time = Instant::now(); let setup = self.setup(call_setup); From ecb5f139b1c2663665b280a440e43958bd5f284a Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 28 Jun 2024 19:44:38 +0400 Subject: [PATCH 22/30] TraceMode::None --- crates/evm/evm/src/executors/mod.rs | 4 +-- crates/evm/evm/src/inspectors/stack.rs | 18 ++++------ crates/evm/traces/src/lib.rs | 46 +++++++++++++++----------- crates/forge/src/multi_runner.rs | 8 ++--- crates/forge/src/runner.rs | 2 +- 5 files changed, 40 insertions(+), 38 deletions(-) diff --git a/crates/evm/evm/src/executors/mod.rs b/crates/evm/evm/src/executors/mod.rs index 1071ae7ddf6b..41e7f2931d1e 100644 --- a/crates/evm/evm/src/executors/mod.rs +++ b/crates/evm/evm/src/executors/mod.rs @@ -211,8 +211,8 @@ impl Executor { } #[inline] - pub fn set_tracing(&mut self, mode: impl Into>) -> &mut Self { - self.inspector_mut().tracing(mode.into()); + pub fn set_tracing(&mut self, mode: TraceMode) -> &mut Self { + self.inspector_mut().tracing(mode); self } diff --git a/crates/evm/evm/src/inspectors/stack.rs b/crates/evm/evm/src/inspectors/stack.rs index b5d5a734d585..2f40903da425 100644 --- a/crates/evm/evm/src/inspectors/stack.rs +++ b/crates/evm/evm/src/inspectors/stack.rs @@ -45,7 +45,7 @@ pub struct InspectorStackBuilder { /// The fuzzer inspector and its state, if it exists. pub fuzzer: Option, /// Whether to enable tracing. - pub trace_mode: Option, + pub trace_mode: TraceMode, /// Whether logs should be collected. pub logs: Option, /// Whether coverage info should be collected. @@ -125,15 +125,9 @@ impl InspectorStackBuilder { /// Set whether to enable the tracer. #[inline] - pub fn trace_mode(mut self, mode: impl Into>) -> Self { - if let Some(mode) = mode.into() { - if let Some(current) = self.trace_mode.as_mut() { - if *current <= mode { - *current = mode; - } - } else { - self.trace_mode = Some(mode); - } + pub fn trace_mode(mut self, mode: TraceMode) -> Self { + if self.trace_mode < mode { + self.trace_mode = mode } self } @@ -412,8 +406,8 @@ impl InspectorStack { /// Set whether to enable the tracer. #[inline] - pub fn tracing(&mut self, mode: Option) { - self.tracer = mode.map(|mode| TracingInspector::new(mode.into())); + pub fn tracing(&mut self, mode: TraceMode) { + self.tracer = mode.into_config().map(TracingInspector::new); } /// Collects all the data gathered during inspection into a single struct. diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index bc28f4b53f4c..0943bb9b27aa 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -405,8 +405,10 @@ pub fn load_contracts<'a>( /// Different kinds of traces used by different foundry components. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default)] pub enum TraceMode { - /// Simple call trace, no steps tracing required. + /// Disabled tracing. #[default] + None, + /// Simple call trace, no steps tracing required. Call, /// Call trace with tracing for JUMP and JUMPDEST opcode steps. /// @@ -419,6 +421,10 @@ pub enum TraceMode { } impl TraceMode { + pub const fn is_none(self) -> bool { + matches!(self, Self::None) + } + pub const fn is_call(self) -> bool { matches!(self, Self::Call) } @@ -430,25 +436,27 @@ impl TraceMode { pub const fn is_debug(self) -> bool { matches!(self, Self::Debug) } -} -impl From for TracingInspectorConfig { - fn from(mode: TraceMode) -> Self { - Self { - record_steps: mode.is_debug() || mode.is_jump(), - record_memory_snapshots: mode.is_debug() || mode.is_jump(), - record_stack_snapshots: if mode.is_debug() || mode.is_jump() { - StackSnapshotType::Full - } else { - StackSnapshotType::None - }, - record_logs: true, - record_state_diff: false, - record_returndata_snapshots: mode.is_debug(), - record_opcodes_filter: mode - .is_jump() - .then(|| OpcodeFilter::new().enable(OpCode::JUMP).enable(OpCode::JUMPDEST)), - exclude_precompile_calls: false, + pub fn into_config(self) -> Option { + if self.is_none() { + None + } else { + TracingInspectorConfig { + record_steps: self.is_debug() || self.is_jump(), + record_memory_snapshots: self.is_debug() || self.is_jump(), + record_stack_snapshots: if self.is_debug() || self.is_jump() { + StackSnapshotType::Full + } else { + StackSnapshotType::None + }, + record_logs: true, + record_state_diff: false, + record_returndata_snapshots: self.is_debug(), + record_opcodes_filter: self + .is_jump() + .then(|| OpcodeFilter::new().enable(OpCode::JUMP).enable(OpCode::JUMPDEST)), + exclude_precompile_calls: false, + }.into() } } } diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 06dcb31c6536..b924de1cce82 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -239,13 +239,13 @@ impl MultiContractRunner { ); let trace_mode = if self.debug { - Some(TraceMode::Debug) + TraceMode::Debug } else if self.decode_internal { - Some(TraceMode::Jump) + TraceMode::Jump } else if self.evm_opts.verbosity >= 3 { - Some(TraceMode::Call) + TraceMode::Call } else { - None + TraceMode::None }; let executor = ExecutorBuilder::new() diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index 16e22350f854..2941f4aea7fa 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -318,7 +318,7 @@ impl<'a> ContractRunner<'a> { let setup = self.setup(call_setup); debug!("finished setting up in {:?}", setup_time.elapsed()); if tmp_tracing { - self.executor.set_tracing(None); + self.executor.set_tracing(TraceMode::None); } if setup.reason.is_some() { From 6503571e09712c64e91af44192b3eed112b02580 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 28 Jun 2024 19:48:38 +0400 Subject: [PATCH 23/30] fmt --- crates/evm/traces/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index 0943bb9b27aa..51e808093c7d 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -456,7 +456,8 @@ impl TraceMode { .is_jump() .then(|| OpcodeFilter::new().enable(OpCode::JUMP).enable(OpCode::JUMPDEST)), exclude_precompile_calls: false, - }.into() + } + .into() } } } From ec783d60fb136795d2c27dab041da77644f8dff9 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Tue, 9 Jul 2024 17:00:37 +0300 Subject: [PATCH 24/30] review fixes --- crates/evm/traces/src/debug/sources.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/evm/traces/src/debug/sources.rs b/crates/evm/traces/src/debug/sources.rs index 209fb8ea0832..66c3227ea0fa 100644 --- a/crates/evm/traces/src/debug/sources.rs +++ b/crates/evm/traces/src/debug/sources.rs @@ -26,7 +26,7 @@ pub struct SourceData { pub path: PathBuf, /// Maps contract name to (start, end) of the contract definition in the source code. /// This is useful for determining which contract contains given function definition. - pub contract_definitions: HashMap, + contract_definitions: HashMap, } impl SourceData { @@ -250,16 +250,16 @@ impl ContractSources { artifact.source_map_runtime.as_ref() }?; - let pc_ic_map = if init_code { - artifact.pc_ic_map.as_ref() - } else { - artifact.pc_ic_map_runtime.as_ref() - }?; - let ic = pc_ic_map.get(pc)?; - // Solc indexes source maps by instruction counter, but Vyper indexes by program // counter. let source_element = if matches!(source.language, MultiCompilerLanguage::Solc(_)) { + let pc_ic_map = if init_code { + artifact.pc_ic_map.as_ref() + } else { + artifact.pc_ic_map_runtime.as_ref() + }?; + let ic = pc_ic_map.get(pc)?; + source_map.get(ic)? } else { source_map.get(pc)? From ff2a11bbaa46ee0d165268bd6558ed5a03e47cf0 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Tue, 9 Jul 2024 17:28:28 +0300 Subject: [PATCH 25/30] --decode-internal for single fn --- crates/evm/traces/src/debug/sources.rs | 2 +- crates/forge/bin/cmd/test/mod.rs | 41 +++++++++++++++++--------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/crates/evm/traces/src/debug/sources.rs b/crates/evm/traces/src/debug/sources.rs index 66c3227ea0fa..ff06cea1b4d0 100644 --- a/crates/evm/traces/src/debug/sources.rs +++ b/crates/evm/traces/src/debug/sources.rs @@ -259,7 +259,7 @@ impl ContractSources { artifact.pc_ic_map_runtime.as_ref() }?; let ic = pc_ic_map.get(pc)?; - + source_map.get(ic)? } else { source_map.get(pc)? diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 486fdeabcc16..112c614d43b7 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -78,8 +78,14 @@ pub struct TestArgs { debug: Option, /// Whether to identify internal functions in traces. - #[arg(long)] - decode_internal: bool, + /// + /// The argument passed to this flag is the name of the test function you want to run, and it + /// works the same as --match-test. + /// + /// If more than one test matches your specified criteria, you must add additional filters + /// until only one test is found (see --match-contract and --match-path). + #[arg(long, value_name = "TEST_FUNCTION")] + decode_internal: Option, /// Print a gas report. #[arg(long, env = "FORGE_GAS_REPORT")] @@ -307,7 +313,7 @@ impl TestArgs { let config = Arc::new(config); let runner = MultiContractRunnerBuilder::new(config.clone()) .set_debug(should_debug) - .set_decode_internal(self.decode_internal) + .set_decode_internal(self.decode_internal.is_some()) .initial_balance(evm_opts.initial_balance) .evm_spec(config.evm_spec_id()) .sender(evm_opts.sender) @@ -316,16 +322,23 @@ impl TestArgs { .enable_isolation(evm_opts.isolate) .build(project_root, &output, env, evm_opts)?; - if let Some(debug_test_pattern) = &self.debug { - let test_pattern = &mut filter.args_mut().test_pattern; - if test_pattern.is_some() { - eyre::bail!( - "Cannot specify both --debug and --match-test. \ - Use --match-contract and --match-path to further limit the search instead." - ); + let mut maybe_override_mt = |flag, maybe_regex: Option<&Regex>| { + if let Some(regex) = maybe_regex { + let test_pattern = &mut filter.args_mut().test_pattern; + if test_pattern.is_some() { + eyre::bail!( + "Cannot specify both --{flag} and --match-test. \ + Use --match-contract and --match-path to further limit the search instead." + ); + } + *test_pattern = Some(regex.clone()); } - *test_pattern = Some(debug_test_pattern.clone()); - } + + Ok(()) + }; + + maybe_override_mt("debug", self.debug.as_ref())?; + maybe_override_mt("decode_internal", self.decode_internal.as_ref())?; let libraries = runner.libraries.clone(); let outcome = self.run_tests(runner, config, verbosity, &filter, &output).await?; @@ -379,7 +392,7 @@ impl TestArgs { trace!(target: "forge::test", "running all tests"); let num_filtered = runner.matching_test_functions(filter).count(); - if self.debug.is_some() && num_filtered != 1 { + if (self.debug.is_some() || self.decode_internal.is_some()) && num_filtered != 1 { eyre::bail!( "{num_filtered} tests matched your criteria, but exactly 1 test must match in order to run the debugger.\n\n\ Use --match-contract and --match-path to further limit the search.\n\ @@ -428,7 +441,7 @@ impl TestArgs { )?); } - if self.decode_internal { + if self.decode_internal.is_some() { let sources = ContractSources::from_project_output(output, &config.root, Some(&libraries))?; builder = builder.with_debug_identifier(DebugTraceIdentifier::new(sources)); From e46c0174f2032c2c1cc178ed641cd1c35769a77d Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Wed, 10 Jul 2024 00:17:21 +0300 Subject: [PATCH 26/30] use Vec --- crates/evm/traces/src/debug/sources.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/evm/traces/src/debug/sources.rs b/crates/evm/traces/src/debug/sources.rs index ff06cea1b4d0..feb87038190b 100644 --- a/crates/evm/traces/src/debug/sources.rs +++ b/crates/evm/traces/src/debug/sources.rs @@ -26,18 +26,18 @@ pub struct SourceData { pub path: PathBuf, /// Maps contract name to (start, end) of the contract definition in the source code. /// This is useful for determining which contract contains given function definition. - contract_definitions: HashMap, + contract_definitions: Vec<(String, usize, usize)>, } impl SourceData { pub fn new(source: Arc, language: MultiCompilerLanguage, path: PathBuf) -> Self { - let mut contract_definitions = HashMap::new(); + let mut contract_definitions = Vec::new(); match language { MultiCompilerLanguage::Vyper(_) => { // Vyper contracts have the same name as the file name. if let Some(name) = path.file_name().map(|s| s.to_string_lossy().to_string()) { - contract_definitions.insert(name, (0, source.len())); + contract_definitions.push((name, 0, source.len())); } } MultiCompilerLanguage::Solc(_) => { @@ -49,8 +49,11 @@ impl SourceData { let Some(name) = contract.name else { continue; }; - contract_definitions - .insert(name.name, (name.loc.start(), contract.loc.end())); + contract_definitions.push(( + name.name, + name.loc.start(), + contract.loc.end(), + )); } } } @@ -63,8 +66,8 @@ impl SourceData { pub fn find_contract_name(&self, start: usize, end: usize) -> Option<&str> { self.contract_definitions .iter() - .find(|(_, (s, e))| start >= *s && end <= *e) - .map(|(name, _)| name.as_str()) + .find(|(_, s, e)| start >= *s && end <= *e) + .map(|(name, _, _)| name.as_str()) } } From 062d55044e7be47038bf73907cb91665231571ad Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Wed, 10 Jul 2024 00:26:20 +0300 Subject: [PATCH 27/30] TraceMode builder --- crates/evm/evm/src/executors/trace.rs | 8 +------- crates/evm/traces/src/lib.rs | 24 ++++++++++++++++++++++++ crates/forge/src/multi_runner.rs | 13 ++++--------- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/crates/evm/evm/src/executors/trace.rs b/crates/evm/evm/src/executors/trace.rs index 56718cf370fd..917c86eab005 100644 --- a/crates/evm/evm/src/executors/trace.rs +++ b/crates/evm/evm/src/executors/trace.rs @@ -20,13 +20,7 @@ impl TracingExecutor { decode_internal: bool, ) -> Self { let db = Backend::spawn(fork); - let trace_mode = if debug { - TraceMode::Debug - } else if decode_internal { - TraceMode::Jump - } else { - TraceMode::Call - }; + let trace_mode = TraceMode::Call.with_debug(debug).with_decode_internal(decode_internal); Self { // configures a bare version of the evm executor: no cheatcode inspector is enabled, // tracing will be enabled only for the targeted transaction diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index afe30a0893d2..c4118344acfb 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -143,6 +143,30 @@ impl TraceMode { matches!(self, Self::Debug) } + pub fn with_debug(self, yes: bool) -> Self { + if yes { + std::cmp::max(self, Self::Debug) + } else { + self + } + } + + pub fn with_decode_internal(self, yes: bool) -> Self { + if yes { + std::cmp::max(self, Self::Jump) + } else { + self + } + } + + pub fn with_verbosity(self, verbosiy: u8) -> Self { + if verbosiy >= 3 { + std::cmp::max(self, Self::Call) + } else { + self + } + } + pub fn into_config(self) -> Option { if self.is_none() { None diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 93709d77fd3e..467b0de80310 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -238,15 +238,10 @@ impl MultiContractRunner { Some(artifact_id.version.clone()), ); - let trace_mode = if self.debug { - TraceMode::Debug - } else if self.decode_internal { - TraceMode::Jump - } else if self.evm_opts.verbosity >= 3 { - TraceMode::Call - } else { - TraceMode::None - }; + let trace_mode = TraceMode::default() + .with_debug(self.debug) + .with_decode_internal(self.decode_internal) + .with_verbosity(self.evm_opts.verbosity); let executor = ExecutorBuilder::new() .inspectors(|stack| { From 9db774fcaaae4d5e2656e9f06e32232cf52f923e Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Thu, 11 Jul 2024 17:23:12 +0400 Subject: [PATCH 28/30] optional --decode-internal and tests --- crates/evm/traces/src/debug/mod.rs | 2 +- crates/evm/traces/src/lib.rs | 27 +++++-- crates/forge/bin/cmd/test/mod.rs | 21 ++++- crates/forge/src/multi_runner.rs | 12 +++ crates/forge/tests/cli/test_cmd.rs | 119 ++++++++++++++++++++++++++++- 5 files changed, 169 insertions(+), 12 deletions(-) diff --git a/crates/evm/traces/src/debug/mod.rs b/crates/evm/traces/src/debug/mod.rs index e972612b8528..a651a3acc5cb 100644 --- a/crates/evm/traces/src/debug/mod.rs +++ b/crates/evm/traces/src/debug/mod.rs @@ -258,7 +258,7 @@ fn try_decode_args_from_step(args: &Parameters<'_>, step: &CallTraceStep) -> Opt (DynSolType::Uint(8), Some(Storage::Memory | Storage::Storage)) => None, (_, Some(Storage::Memory)) => decode_from_memory( type_, - step.memory.as_ref().unwrap().as_bytes(), + step.memory.as_ref()?.as_bytes(), input.try_into().ok()?, ), // Read other types from stack diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index c4118344acfb..77a9ceece341 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -118,7 +118,11 @@ pub enum TraceMode { Call, /// Call trace with tracing for JUMP and JUMPDEST opcode steps. /// - /// Used for internal functions identification. + /// Used for internal functions identification. Does not track memory snapshots. + JumpSimple, + /// Call trace with tracing for JUMP and JUMPDEST opcode steps. + /// + /// Same as `JumpSimple`, but tracks memory snapshots as well. Jump, /// Call trace with complete steps tracing. /// @@ -135,6 +139,10 @@ impl TraceMode { matches!(self, Self::Call) } + pub const fn is_jump_simple(self) -> bool { + matches!(self, Self::JumpSimple) + } + pub const fn is_jump(self) -> bool { matches!(self, Self::Jump) } @@ -159,6 +167,14 @@ impl TraceMode { } } + pub fn with_decode_internal_simple(self, yes: bool) -> Self { + if yes { + std::cmp::max(self, Self::JumpSimple) + } else { + self + } + } + pub fn with_verbosity(self, verbosiy: u8) -> Self { if verbosiy >= 3 { std::cmp::max(self, Self::Call) @@ -172,9 +188,9 @@ impl TraceMode { None } else { TracingInspectorConfig { - record_steps: self.is_debug() || self.is_jump(), - record_memory_snapshots: self.is_debug() || self.is_jump(), - record_stack_snapshots: if self.is_debug() || self.is_jump() { + record_steps: self >= Self::JumpSimple, + record_memory_snapshots: self >= Self::Jump, + record_stack_snapshots: if self >= Self::JumpSimple { StackSnapshotType::Full } else { StackSnapshotType::None @@ -182,8 +198,7 @@ impl TraceMode { record_logs: true, record_state_diff: false, record_returndata_snapshots: self.is_debug(), - record_opcodes_filter: self - .is_jump() + record_opcodes_filter: (self.is_jump() || self.is_jump_simple()) .then(|| OpcodeFilter::new().enabled(OpCode::JUMP).enabled(OpCode::JUMPDEST)), exclude_precompile_calls: false, } diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 83b6b7ac9e3b..4c822d3978c7 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -84,7 +84,7 @@ pub struct TestArgs { /// If more than one test matches your specified criteria, you must add additional filters /// until only one test is found (see --match-contract and --match-path). #[arg(long, value_name = "TEST_FUNCTION")] - decode_internal: Option, + decode_internal: Option>, /// Print a gas report. #[arg(long, env = "FORGE_GAS_REPORT")] @@ -307,12 +307,20 @@ impl TestArgs { let env = evm_opts.evm_env().await?; + // If we are provided with test function regex, we are enabling complete internal fns + // tracing. + let decode_internal = self.decode_internal.as_ref().map_or(false, |v| v.is_some()); + // If we are provided with just --decode-internal flag, we enable simple tracing (without + // memory decoding). + let decode_internal_simple = self.decode_internal.is_some(); + // Prepare the test builder. let should_debug = self.debug.is_some(); let config = Arc::new(config); let runner = MultiContractRunnerBuilder::new(config.clone()) .set_debug(should_debug) - .set_decode_internal(self.decode_internal.is_some()) + .set_decode_internal(decode_internal) + .set_decode_internal_simple(decode_internal_simple) .initial_balance(evm_opts.initial_balance) .evm_spec(config.evm_spec_id()) .sender(evm_opts.sender) @@ -337,7 +345,10 @@ impl TestArgs { }; maybe_override_mt("debug", self.debug.as_ref())?; - maybe_override_mt("decode_internal", self.decode_internal.as_ref())?; + maybe_override_mt( + "decode-internal", + self.decode_internal.as_ref().and_then(|v| v.as_ref()), + )?; let libraries = runner.libraries.clone(); let outcome = self.run_tests(runner, config, verbosity, &filter, &output).await?; @@ -391,7 +402,9 @@ impl TestArgs { trace!(target: "forge::test", "running all tests"); let num_filtered = runner.matching_test_functions(filter).count(); - if (self.debug.is_some() || self.decode_internal.is_some()) && num_filtered != 1 { + if (self.debug.is_some() || self.decode_internal.as_ref().map_or(false, |v| v.is_some())) && + num_filtered != 1 + { eyre::bail!( "{num_filtered} tests matched your criteria, but exactly 1 test must match in order to run the debugger.\n\n\ Use --match-contract and --match-path to further limit the search.\n\ diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 467b0de80310..95dea397c07a 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -62,6 +62,8 @@ pub struct MultiContractRunner { pub debug: bool, /// Whether to enable steps tracking in the tracer. pub decode_internal: bool, + /// Whether to enable simple steps tracing (without memory recording). + pub decode_internal_simple: bool, /// Settings related to fuzz and/or invariant tests pub test_options: TestOptions, /// Whether to enable call isolation @@ -240,6 +242,7 @@ impl MultiContractRunner { let trace_mode = TraceMode::default() .with_debug(self.debug) + .with_decode_internal_simple(self.decode_internal_simple) .with_decode_internal(self.decode_internal) .with_verbosity(self.evm_opts.verbosity); @@ -307,6 +310,8 @@ pub struct MultiContractRunnerBuilder { pub debug: bool, /// Whether to enable steps tracking in the tracer. pub decode_internal: bool, + /// Whether to enable simple steps tracing (without memory recording). + pub decode_internal_simple: bool, /// Whether to enable call isolation pub isolation: bool, /// Settings related to fuzz and/or invariant tests @@ -326,6 +331,7 @@ impl MultiContractRunnerBuilder { isolation: Default::default(), test_options: Default::default(), decode_internal: Default::default(), + decode_internal_simple: Default::default(), } } @@ -369,6 +375,11 @@ impl MultiContractRunnerBuilder { self } + pub fn set_decode_internal_simple(mut self, enable: bool) -> Self { + self.decode_internal_simple = enable; + self + } + pub fn enable_isolation(mut self, enable: bool) -> Self { self.isolation = enable; self @@ -440,6 +451,7 @@ impl MultiContractRunnerBuilder { coverage: self.coverage, debug: self.debug, decode_internal: self.decode_internal, + decode_internal_simple: self.decode_internal_simple, test_options: self.test_options.unwrap_or_default(), isolation: self.isolation, known_contracts, diff --git a/crates/forge/tests/cli/test_cmd.rs b/crates/forge/tests/cli/test_cmd.rs index 01db02fddc30..347b8a290d60 100644 --- a/crates/forge/tests/cli/test_cmd.rs +++ b/crates/forge/tests/cli/test_cmd.rs @@ -3,7 +3,7 @@ use alloy_primitives::U256; use foundry_config::{Config, FuzzConfig}; use foundry_test_utils::{ - rpc, + rpc, str, util::{OutputExt, OTHER_SOLC_VERSION, SOLC_VERSION}, }; use std::{path::PathBuf, str::FromStr}; @@ -853,3 +853,120 @@ forgetest_init!(should_not_show_logs_when_fuzz_test_inline_config, |prj, cmd| { let stdout = cmd.stdout_lossy(); assert!(!stdout.contains("inside fuzz test, x is:"), "\n{stdout}"); }); + +// tests internal functions trace +forgetest_init!(internal_functions_trace, |prj, cmd| { + prj.wipe_contracts(); + prj.clear(); + + // Disable optimizer because for simple contract most functions will get inlined. + prj.write_config(Config { optimizer: false, ..Default::default() }); + + prj.add_test( + "Simple", + r#"pragma solidity 0.8.24; + import {Test, console2} from "forge-std/Test.sol"; +contract SimpleContract { + uint256 public num; + address public addr; + + function _setNum(uint256 _num) internal returns(uint256 prev) { + prev = num; + num = _num; + } + + function _setAddr(address _addr) internal returns(address prev) { + prev = addr; + addr = _addr; + } + + function increment() public { + _setNum(num + 1); + } + + function setValues(uint256 _num, address _addr) public { + _setNum(_num); + _setAddr(_addr); + } +} + +contract SimpleContractTest is Test { + function test() public { + SimpleContract c = new SimpleContract(); + c.increment(); + c.setValues(100, address(0x123)); + } +} + "#, + ) + .unwrap(); + cmd.args(["test", "-vvvv", "--decode-internal"]).assert_success().stdout_eq(str![[r#" +... +Traces: + [250463] SimpleContractTest::test() + ├─ [171014] → new SimpleContract@0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f + │ └─ ← [Return] 854 bytes of code + ├─ [22638] SimpleContract::increment() + │ ├─ [20150] SimpleContract::_setNum(1) + │ │ └─ ← 0 + │ └─ ← [Stop] + ├─ [23219] SimpleContract::setValues(100, 0x0000000000000000000000000000000000000123) + │ ├─ [250] SimpleContract::_setNum(100) + │ │ └─ ← 1 + │ ├─ [22339] SimpleContract::_setAddr(0x0000000000000000000000000000000000000123) + │ │ └─ ← 0x0000000000000000000000000000000000000000 + │ └─ ← [Stop] + └─ ← [Stop] +... +"#]]); +}); + +// tests internal functions trace with memory decoding +forgetest_init!(internal_functions_trace_memory, |prj, cmd| { + prj.wipe_contracts(); + prj.clear(); + + // Disable optimizer because for simple contract most functions will get inlined. + prj.write_config(Config { optimizer: false, ..Default::default() }); + + prj.add_test( + "Simple", + r#"pragma solidity 0.8.24; +import {Test, console2} from "forge-std/Test.sol"; + +contract SimpleContract { + string public str = "initial value"; + + function _setStr(string memory _str) internal returns(string memory prev) { + prev = str; + str = _str; + } + + function setStr(string memory _str) public { + _setStr(_str); + } +} + +contract SimpleContractTest is Test { + function test() public { + SimpleContract c = new SimpleContract(); + c.setStr("new value"); + } +} + "#, + ) + .unwrap(); + cmd.args(["test", "-vvvv", "--decode-internal", "test"]).assert_success().stdout_eq(str![[r#" +... +Traces: + [421960] SimpleContractTest::test() + ├─ [385978] → new SimpleContract@0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f + │ └─ ← [Return] 1814 bytes of code + ├─ [2534] SimpleContract::setStr("new value") + │ ├─ [1600] SimpleContract::_setStr("new value") + │ │ └─ ← "initial value" + │ └─ ← [Stop] + └─ ← [Stop] +... +"#]]); +}); From 97c2b4b35bfec7068f2dedc3e997d3b094ddb18c Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Thu, 11 Jul 2024 17:26:38 +0400 Subject: [PATCH 29/30] update doc --- crates/forge/bin/cmd/test/mod.rs | 8 ++++++-- crates/forge/tests/cli/test_cmd.rs | 6 ++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 4c822d3978c7..a877dcfe9887 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -78,8 +78,12 @@ pub struct TestArgs { /// Whether to identify internal functions in traces. /// - /// The argument passed to this flag is the name of the test function you want to run, and it - /// works the same as --match-test. + /// If no argument is passed to this flag, it will trace internal functions scope and decode + /// stack parameters, but parameters stored in memory (such as bytes or arrays) will not be + /// decoded. + /// + /// To decode memory parameters, you should pass an argument with a test function name, + /// similarly to --debug and --match-test. /// /// If more than one test matches your specified criteria, you must add additional filters /// until only one test is found (see --match-contract and --match-path). diff --git a/crates/forge/tests/cli/test_cmd.rs b/crates/forge/tests/cli/test_cmd.rs index 347b8a290d60..5608cc1c5e12 100644 --- a/crates/forge/tests/cli/test_cmd.rs +++ b/crates/forge/tests/cli/test_cmd.rs @@ -956,7 +956,8 @@ contract SimpleContractTest is Test { "#, ) .unwrap(); - cmd.args(["test", "-vvvv", "--decode-internal", "test"]).assert_success().stdout_eq(str![[r#" + cmd.args(["test", "-vvvv", "--decode-internal", "test"]).assert_success().stdout_eq(str![[ + r#" ... Traces: [421960] SimpleContractTest::test() @@ -968,5 +969,6 @@ Traces: │ └─ ← [Stop] └─ ← [Stop] ... -"#]]); +"# + ]]); }); From 92ece42ee730cf67a98ccebdf26c2cb02ac3454e Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Thu, 11 Jul 2024 17:54:37 +0400 Subject: [PATCH 30/30] InternalTraceMode --- crates/evm/evm/src/executors/trace.rs | 9 +++++-- crates/evm/traces/src/lib.rs | 37 +++++++++++++++++---------- crates/forge/bin/cmd/test/mod.rs | 21 +++++++++------ crates/forge/src/multi_runner.rs | 30 +++++++++------------- 4 files changed, 55 insertions(+), 42 deletions(-) diff --git a/crates/evm/evm/src/executors/trace.rs b/crates/evm/evm/src/executors/trace.rs index 917c86eab005..cc3e687766f8 100644 --- a/crates/evm/evm/src/executors/trace.rs +++ b/crates/evm/evm/src/executors/trace.rs @@ -2,7 +2,7 @@ use crate::executors::{Executor, ExecutorBuilder}; use foundry_compilers::artifacts::EvmVersion; use foundry_config::{utils::evm_spec_id, Chain, Config}; use foundry_evm_core::{backend::Backend, fork::CreateFork, opts::EvmOpts}; -use foundry_evm_traces::TraceMode; +use foundry_evm_traces::{InternalTraceMode, TraceMode}; use revm::primitives::{Env, SpecId}; use std::ops::{Deref, DerefMut}; @@ -20,7 +20,12 @@ impl TracingExecutor { decode_internal: bool, ) -> Self { let db = Backend::spawn(fork); - let trace_mode = TraceMode::Call.with_debug(debug).with_decode_internal(decode_internal); + let trace_mode = + TraceMode::Call.with_debug(debug).with_decode_internal(if decode_internal { + InternalTraceMode::Full + } else { + InternalTraceMode::None + }); Self { // configures a bare version of the evm executor: no cheatcode inspector is enabled, // tracing will be enabled only for the targeted transaction diff --git a/crates/evm/traces/src/lib.rs b/crates/evm/traces/src/lib.rs index 77a9ceece341..c098fe4be160 100644 --- a/crates/evm/traces/src/lib.rs +++ b/crates/evm/traces/src/lib.rs @@ -108,6 +108,27 @@ pub fn load_contracts<'a>( contracts } +/// Different kinds of internal functions tracing. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default)] +pub enum InternalTraceMode { + #[default] + None, + /// Traces internal functions without decoding inputs/outputs from memory. + Simple, + /// Same as `Simple`, but also tracks memory snapshots. + Full, +} + +impl From for TraceMode { + fn from(mode: InternalTraceMode) -> Self { + match mode { + InternalTraceMode::None => Self::None, + InternalTraceMode::Simple => Self::JumpSimple, + InternalTraceMode::Full => Self::Jump, + } + } +} + // Different kinds of traces used by different foundry components. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default)] pub enum TraceMode { @@ -159,20 +180,8 @@ impl TraceMode { } } - pub fn with_decode_internal(self, yes: bool) -> Self { - if yes { - std::cmp::max(self, Self::Jump) - } else { - self - } - } - - pub fn with_decode_internal_simple(self, yes: bool) -> Self { - if yes { - std::cmp::max(self, Self::JumpSimple) - } else { - self - } + pub fn with_decode_internal(self, mode: InternalTraceMode) -> Self { + std::cmp::max(self, mode.into()) } pub fn with_verbosity(self, verbosiy: u8) -> Self { diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index a877dcfe9887..248d676db8de 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -11,7 +11,7 @@ use forge::{ debug::{ContractSources, DebugTraceIdentifier}, decode_trace_arena, identifier::SignaturesIdentifier, - render_trace_arena, CallTraceDecoderBuilder, TraceKind, + render_trace_arena, CallTraceDecoderBuilder, InternalTraceMode, TraceKind, }, MultiContractRunner, MultiContractRunnerBuilder, TestFilter, TestOptions, TestOptionsBuilder, }; @@ -311,12 +311,18 @@ impl TestArgs { let env = evm_opts.evm_env().await?; - // If we are provided with test function regex, we are enabling complete internal fns - // tracing. - let decode_internal = self.decode_internal.as_ref().map_or(false, |v| v.is_some()); - // If we are provided with just --decode-internal flag, we enable simple tracing (without - // memory decoding). - let decode_internal_simple = self.decode_internal.is_some(); + // Choose the internal function tracing mode, if --decode-internal is provided. + let decode_internal = if let Some(maybe_fn) = self.decode_internal.as_ref() { + if maybe_fn.is_some() { + // If function filter is provided, we enable full tracing. + InternalTraceMode::Full + } else { + // If no function filter is provided, we enable simple tracing. + InternalTraceMode::Simple + } + } else { + InternalTraceMode::None + }; // Prepare the test builder. let should_debug = self.debug.is_some(); @@ -324,7 +330,6 @@ impl TestArgs { let runner = MultiContractRunnerBuilder::new(config.clone()) .set_debug(should_debug) .set_decode_internal(decode_internal) - .set_decode_internal_simple(decode_internal_simple) .initial_balance(evm_opts.initial_balance) .evm_spec(config.evm_spec_id()) .sender(evm_opts.sender) diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 95dea397c07a..9ac3069a61d5 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -13,8 +13,14 @@ use foundry_compilers::{ }; use foundry_config::Config; use foundry_evm::{ - backend::Backend, decode::RevertDecoder, executors::ExecutorBuilder, fork::CreateFork, - inspectors::CheatsConfig, opts::EvmOpts, revm, traces::TraceMode, + backend::Backend, + decode::RevertDecoder, + executors::ExecutorBuilder, + fork::CreateFork, + inspectors::CheatsConfig, + opts::EvmOpts, + revm, + traces::{InternalTraceMode, TraceMode}, }; use foundry_linking::{LinkOutput, Linker}; use rayon::prelude::*; @@ -61,9 +67,7 @@ pub struct MultiContractRunner { /// Whether to collect debug info pub debug: bool, /// Whether to enable steps tracking in the tracer. - pub decode_internal: bool, - /// Whether to enable simple steps tracing (without memory recording). - pub decode_internal_simple: bool, + pub decode_internal: InternalTraceMode, /// Settings related to fuzz and/or invariant tests pub test_options: TestOptions, /// Whether to enable call isolation @@ -242,7 +246,6 @@ impl MultiContractRunner { let trace_mode = TraceMode::default() .with_debug(self.debug) - .with_decode_internal_simple(self.decode_internal_simple) .with_decode_internal(self.decode_internal) .with_verbosity(self.evm_opts.verbosity); @@ -309,9 +312,7 @@ pub struct MultiContractRunnerBuilder { /// Whether or not to collect debug info pub debug: bool, /// Whether to enable steps tracking in the tracer. - pub decode_internal: bool, - /// Whether to enable simple steps tracing (without memory recording). - pub decode_internal_simple: bool, + pub decode_internal: InternalTraceMode, /// Whether to enable call isolation pub isolation: bool, /// Settings related to fuzz and/or invariant tests @@ -331,7 +332,6 @@ impl MultiContractRunnerBuilder { isolation: Default::default(), test_options: Default::default(), decode_internal: Default::default(), - decode_internal_simple: Default::default(), } } @@ -370,13 +370,8 @@ impl MultiContractRunnerBuilder { self } - pub fn set_decode_internal(mut self, enable: bool) -> Self { - self.decode_internal = enable; - self - } - - pub fn set_decode_internal_simple(mut self, enable: bool) -> Self { - self.decode_internal_simple = enable; + pub fn set_decode_internal(mut self, mode: InternalTraceMode) -> Self { + self.decode_internal = mode; self } @@ -451,7 +446,6 @@ impl MultiContractRunnerBuilder { coverage: self.coverage, debug: self.debug, decode_internal: self.decode_internal, - decode_internal_simple: self.decode_internal_simple, test_options: self.test_options.unwrap_or_default(), isolation: self.isolation, known_contracts,