Skip to content

Commit

Permalink
fix: address review from @klkvr
Browse files Browse the repository at this point in the history
  • Loading branch information
topocount committed Sep 17, 2024
1 parent 50ddba2 commit 2d9590e
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 51 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ chrono = { version = "0.4", default-features = false, features = [
"std",
] }
color-eyre = "0.6"
owo-colors = "3"
derive_more = { version = "1.0", features = ["full"] }
dunce = "1"
evm-disassembler = "0.5"
Expand Down
117 changes: 102 additions & 15 deletions crates/cheatcodes/src/test/expect.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use crate::{Cheatcode, Cheatcodes, CheatsCtxt, DatabaseExt, Error, Result, Vm::*};
use crate::{
Cheatcode, Cheatcodes, CheatcodesExecutor, CheatsCtxt, DatabaseExt, Error, Result, Vm::*,
};
use alloy_primitives::{address, hex, Address, Bytes, LogData as RawLog, U256};
use alloy_sol_types::{SolError, SolValue};
use foundry_common::ContractsByArtifact;
use foundry_evm_core::decode::RevertDecoder;
use foundry_evm_traces::TracingInspector;
use revm::interpreter::{
return_ok, InstructionResult, Interpreter, InterpreterAction, InterpreterResult,
};
Expand Down Expand Up @@ -208,10 +211,20 @@ impl Cheatcode for expectCallMinGas_1Call {
}

impl Cheatcode for expectEmit_0Call {
fn apply_stateful<DB: DatabaseExt>(&self, ccx: &mut CheatsCtxt<DB>) -> Result {
fn apply_full<DB: DatabaseExt, E: CheatcodesExecutor>(
&self,
ccx: &mut CheatsCtxt<DB>,
executor: &mut E,
) -> Result {
let Self { checkTopic1, checkTopic2, checkTopic3, checkData } = *self;

let Some(tracer) = executor.tracing_inspector().and_then(|t| t.as_ref()) else {
return Ok(Default::default());
};

expect_emit(
ccx.state,
tracer,
ccx.ecx.journaled_state.depth(),
[true, checkTopic1, checkTopic2, checkTopic3, checkData],
None,
Expand All @@ -221,10 +234,19 @@ impl Cheatcode for expectEmit_0Call {
}

impl Cheatcode for expectEmit_1Call {
fn apply_stateful<DB: DatabaseExt>(&self, ccx: &mut CheatsCtxt<DB>) -> Result {
fn apply_full<DB: DatabaseExt, E: CheatcodesExecutor>(
&self,
ccx: &mut CheatsCtxt<DB>,
executor: &mut E,
) -> Result {
let Self { checkTopic1, checkTopic2, checkTopic3, checkData, emitter } = *self;
let Some(tracer) = executor.tracing_inspector().and_then(|t| t.as_ref()) else {
return Ok(Default::default());
};

expect_emit(
ccx.state,
tracer,
ccx.ecx.journaled_state.depth(),
[true, checkTopic1, checkTopic2, checkTopic3, checkData],
Some(emitter),
Expand All @@ -234,24 +256,53 @@ impl Cheatcode for expectEmit_1Call {
}

impl Cheatcode for expectEmit_2Call {
fn apply_stateful<DB: DatabaseExt>(&self, ccx: &mut CheatsCtxt<DB>) -> Result {
fn apply_full<DB: DatabaseExt, E: CheatcodesExecutor>(
&self,
ccx: &mut CheatsCtxt<DB>,
executor: &mut E,
) -> Result {
let Self {} = self;
expect_emit(ccx.state, ccx.ecx.journaled_state.depth(), [true; 5], None, false)
let Some(tracer) = executor.tracing_inspector().and_then(|t| t.as_ref()) else {
return Ok(Default::default());
};
expect_emit(ccx.state, tracer, ccx.ecx.journaled_state.depth(), [true; 5], None, false)
}
}

impl Cheatcode for expectEmit_3Call {
fn apply_stateful<DB: DatabaseExt>(&self, ccx: &mut CheatsCtxt<DB>) -> Result {
fn apply_full<DB: DatabaseExt, E: CheatcodesExecutor>(
&self,
ccx: &mut CheatsCtxt<DB>,
executor: &mut E,
) -> Result {
let Self { emitter } = *self;
expect_emit(ccx.state, ccx.ecx.journaled_state.depth(), [true; 5], Some(emitter), false)
let Some(tracer) = executor.tracing_inspector().and_then(|t| t.as_ref()) else {
return Ok(Default::default());
};
expect_emit(
ccx.state,
tracer,
ccx.ecx.journaled_state.depth(),
[true; 5],
Some(emitter),
false,
)
}
}

impl Cheatcode for expectEmitAnonymous_0Call {
fn apply_stateful<DB: DatabaseExt>(&self, ccx: &mut CheatsCtxt<DB>) -> Result {
fn apply_full<DB: DatabaseExt, E: CheatcodesExecutor>(
&self,
ccx: &mut CheatsCtxt<DB>,
executor: &mut E,
) -> Result {
let Self { checkTopic0, checkTopic1, checkTopic2, checkTopic3, checkData } = *self;
let Some(tracer) = executor.tracing_inspector().and_then(|t| t.as_ref()) else {
return Ok(Default::default());
};
expect_emit(
ccx.state,
tracer,
ccx.ecx.journaled_state.depth(),
[checkTopic0, checkTopic1, checkTopic2, checkTopic3, checkData],
None,
Expand All @@ -261,10 +312,18 @@ impl Cheatcode for expectEmitAnonymous_0Call {
}

impl Cheatcode for expectEmitAnonymous_1Call {
fn apply_stateful<DB: DatabaseExt>(&self, ccx: &mut CheatsCtxt<DB>) -> Result {
fn apply_full<DB: DatabaseExt, E: CheatcodesExecutor>(
&self,
ccx: &mut CheatsCtxt<DB>,
executor: &mut E,
) -> Result {
let Self { checkTopic0, checkTopic1, checkTopic2, checkTopic3, checkData, emitter } = *self;
let Some(tracer) = executor.tracing_inspector().and_then(|t| t.as_ref()) else {
return Ok(Default::default());
};
expect_emit(
ccx.state,
tracer,
ccx.ecx.journaled_state.depth(),
[checkTopic0, checkTopic1, checkTopic2, checkTopic3, checkData],
Some(emitter),
Expand All @@ -274,16 +333,37 @@ impl Cheatcode for expectEmitAnonymous_1Call {
}

impl Cheatcode for expectEmitAnonymous_2Call {
fn apply_stateful<DB: DatabaseExt>(&self, ccx: &mut CheatsCtxt<DB>) -> Result {
fn apply_full<DB: DatabaseExt, E: CheatcodesExecutor>(
&self,
ccx: &mut CheatsCtxt<DB>,
executor: &mut E,
) -> Result {
let Self {} = self;
expect_emit(ccx.state, ccx.ecx.journaled_state.depth(), [true; 5], None, true)
let Some(tracer) = executor.tracing_inspector().and_then(|t| t.as_ref()) else {
return Ok(Default::default());
};
expect_emit(ccx.state, tracer, ccx.ecx.journaled_state.depth(), [true; 5], None, true)
}
}

impl Cheatcode for expectEmitAnonymous_3Call {
fn apply_stateful<DB: DatabaseExt>(&self, ccx: &mut CheatsCtxt<DB>) -> Result {
fn apply_full<DB: DatabaseExt, E: CheatcodesExecutor>(
&self,
ccx: &mut CheatsCtxt<DB>,
executor: &mut E,
) -> Result {
let Self { emitter } = *self;
expect_emit(ccx.state, ccx.ecx.journaled_state.depth(), [true; 5], Some(emitter), true)
let Some(tracer) = executor.tracing_inspector().and_then(|t| t.as_ref()) else {
return Ok(Default::default());
};
expect_emit(
ccx.state,
tracer,
ccx.ecx.journaled_state.depth(),
[true; 5],
Some(emitter),
true,
)
}
}

Expand Down Expand Up @@ -461,13 +541,20 @@ fn expect_call(

fn expect_emit(
state: &mut Cheatcodes,
tracer: &TracingInspector,
depth: u64,
checks: [bool; 5],
address: Option<Address>,
anonymous: bool,
) -> Result {
let expected_emit = ExpectedEmit { depth, checks, address, found: false, log: None, anonymous,
sequence: tracer.traces().nodes().last().expect("no traces").logs.len() as u16
let expected_emit = ExpectedEmit {
depth,
checks,
address,
found: false,
log: None,
anonymous,
sequence: tracer.traces().nodes().last().expect("no traces").logs.len() as u16,
};
if let Some(found_emit_pos) = state.expected_emits.iter().position(|emit| emit.found) {
// The order of emits already found (back of queue) should not be modified, hence push any
Expand Down
27 changes: 0 additions & 27 deletions crates/evm/core/src/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ impl fmt::Display for SkipReason {
}
}

pub enum VmErr {
UnemittedEventError(u16),
None,
}

/// Decode a set of logs, only returning logs from DSTest logging events and Hardhat's `console.log`
pub fn decode_console_logs(logs: &[Log]) -> Vec<String> {
logs.iter().filter_map(decode_console_log).collect()
Expand Down Expand Up @@ -140,32 +135,10 @@ impl RevertDecoder {
})
}

pub fn decode_structured(&self, err: &[u8], status: Option<InstructionResult>) -> VmErr {
if err.len() < SELECTOR_LEN || status != Some(InstructionResult::Revert) {
return VmErr::None;
}

let (selector, data) = err.split_at(SELECTOR_LEN);
let selector: &[u8; 4] = selector.try_into().unwrap();

match *selector {
Vm::UnemittedEventError::SELECTOR => {
let Some(e) = Vm::UnemittedEventError::abi_decode_raw(data, false).ok() else {
return VmErr::None;
};
return VmErr::UnemittedEventError(e.positionExpected);
}
_ => {
return VmErr::None;
}
}
}

/// Tries to decode an error message from the given revert bytes.
///
/// See [`decode`](Self::decode) for more information.
pub fn maybe_decode(&self, err: &[u8], status: Option<InstructionResult>) -> Option<String> {
// TODO: Convert to return an optional structured error as a tuple
if err.len() < SELECTOR_LEN {
if let Some(status) = status {
if !status.is_ok() {
Expand Down
1 change: 1 addition & 0 deletions crates/evm/traces/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ eyre.workspace = true
futures.workspace = true
itertools.workspace = true
serde.workspace = true
owo-colors.workspace = true
tokio = { workspace = true, features = ["time", "macros"] }
tracing.workspace = true
rustc-hash.workspace = true
Expand Down
26 changes: 17 additions & 9 deletions crates/evm/traces/src/decoder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::{
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 alloy_sol_types::SolError;
use foundry_common::{
abi::get_indexed_event, fmt::format_token, get_contract_name, ContractsByArtifact, SELECTOR_LEN,
};
Expand All @@ -17,13 +18,14 @@ use foundry_evm_core::{
CALLER, CHEATCODE_ADDRESS, DEFAULT_CREATE2_DEPLOYER, HARDHAT_CONSOLE_ADDRESS,
TEST_CONTRACT_ADDRESS,
},
decode::{RevertDecoder, VmErr},
decode::RevertDecoder,
precompiles::{
BLAKE_2F, EC_ADD, EC_MUL, EC_PAIRING, EC_RECOVER, IDENTITY, MOD_EXP, POINT_EVALUATION,
RIPEMD_160, SHA_256,
},
};
use itertools::Itertools;
use owo_colors::OwoColorize;
use revm_inspectors::tracing::types::{DecodedCallLog, DecodedCallTrace};
use rustc_hash::FxHashMap;
use std::{
Expand Down Expand Up @@ -328,16 +330,22 @@ impl CallTraceDecoder {
for node in traces {
node.trace.decoded = self.decode_function(&node.trace).await;

if let VmErr::UnemittedEventError(error_index) =
self.revert_decoder.decode_structured(&node.trace.output, Some(node.trace.status))
{
node.logs[error_index as usize].unmatched = true;
}

for log in node.logs.iter_mut() {
log.decoded = self.decode_event(&log.raw_log).await;
}

if let Ok(e) = Vm::UnemittedEventError::abi_decode(&node.trace.output, false) {
let log_name = node.logs[e.positionExpected as usize]
.decoded
.name
.as_mut()
.expect("already decoded");

// set color to red fo the given event Id
node.logs[e.positionExpected as usize].decoded.name =
Some(log_name.red().to_string());
}

if let Some(debug) = self.debug_identifier.as_ref() {
if let Some(identified) = self.contracts.get(&node.trace.address) {
debug.identify_node_steps(node, get_contract_name(identified))
Expand Down Expand Up @@ -482,7 +490,7 @@ impl CallTraceDecoder {
"parseJsonBytes32Array" |
"writeJson" |
// `keyExists` is being deprecated in favor of `keyExistsJson`. It will be removed in future versions.
"keyExists" |
"keyExists" |
"keyExistsJson" |
"serializeBool" |
"serializeUint" |
Expand All @@ -498,7 +506,7 @@ impl CallTraceDecoder {
let mut decoded = func.abi_decode_input(&data[SELECTOR_LEN..], false).ok()?;
let token = if func.name.as_str() == "parseJson" ||
// `keyExists` is being deprecated in favor of `keyExistsJson`. It will be removed in future versions.
func.name.as_str() == "keyExists" ||
func.name.as_str() == "keyExists" ||
func.name.as_str() == "keyExistsJson"
{
"<JSON file>"
Expand Down

0 comments on commit 2d9590e

Please sign in to comment.