From 838674e14d0e268376c6cfe1a646a00b8d579856 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Sun, 25 Sep 2022 16:00:18 +0200 Subject: [PATCH] fix: patch empty event args --- evm/src/trace/decoder.rs | 30 ++++++++++++++++++-- forge/tests/it/config.rs | 5 ++++ forge/tests/it/repros.rs | 52 ++++++++++++++++++++++++++++++++++- testdata/repros/Issue3347.sol | 14 ++++++++++ 4 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 testdata/repros/Issue3347.sol diff --git a/evm/src/trace/decoder.rs b/evm/src/trace/decoder.rs index c46957205eca..582c85bc071f 100644 --- a/evm/src/trace/decoder.rs +++ b/evm/src/trace/decoder.rs @@ -14,6 +14,7 @@ use ethers::{ }; use foundry_common::SELECTOR_LEN; use foundry_utils::get_indexed_event; +use hashbrown::HashSet; use std::collections::{BTreeMap, HashMap}; /// Build a new [CallTraceDecoder]. @@ -316,14 +317,24 @@ impl CallTraceDecoder { } } - for event in events { + for mut event in events { + // ensure all params are named, otherwise this will cause issues with decoding: See also + let empty_params = patch_nameless_params(&mut event); if let Ok(decoded) = event.parse_log(raw_log.clone()) { *log = RawOrDecodedLog::Decoded( event.name, decoded .params .into_iter() - .map(|param| (param.name, self.apply_label(¶m.value))) + .map(|param| { + // undo patched names + let name = if empty_params.contains(¶m.name) { + "".to_string() + } else { + param.name + }; + (name, self.apply_label(¶m.value)) + }) .collect(), ); break @@ -337,6 +348,21 @@ impl CallTraceDecoder { } } +/// This is a bit horrible but due to we need to patch nameless (valid) params before decoding a logs, otherwise [`Event::parse_log()`] will result in wrong results since they're identified by name. +/// +/// Returns a set of patched param names, that originally were empty. +fn patch_nameless_params(event: &mut Event) -> HashSet { + let mut patches = HashSet::new(); + if event.inputs.iter().filter(|input| input.name.is_empty()).count() > 1 { + for (idx, param) in event.inputs.iter_mut().enumerate() { + // this is an illegal arg name, which ensures patched identifiers are unique + param.name = format!("", idx); + patches.insert(param.name.clone()); + } + } + patches +} + fn precompile(number: u8, name: impl ToString, inputs: I, outputs: O) -> (Address, Function) where I: IntoIterator, diff --git a/forge/tests/it/config.rs b/forge/tests/it/config.rs index 9e479ac8509f..15f921f914ba 100644 --- a/forge/tests/it/config.rs +++ b/forge/tests/it/config.rs @@ -46,6 +46,11 @@ impl TestConfig { self } + /// Executes the test runner + pub fn test(&mut self) -> BTreeMap { + self.runner.test(&self.filter, None, self.opts).unwrap() + } + #[track_caller] pub fn run(&mut self) { self.try_run().unwrap() diff --git a/forge/tests/it/repros.rs b/forge/tests/it/repros.rs index db0a39158a2e..4fa1d96057d5 100644 --- a/forge/tests/it/repros.rs +++ b/forge/tests/it/repros.rs @@ -1,7 +1,7 @@ //! Tests for reproducing issues use crate::{config::*, test_helpers::filter::Filter}; -use ethers::abi::Address; +use ethers::abi::{Address, Event, EventParam, Log, LogParam, ParamType, RawLog, Token}; use foundry_config::Config; use std::str::FromStr; @@ -37,6 +37,25 @@ macro_rules! test_repro_with_sender { }; } +macro_rules! run_test_repro { + ($issue:expr) => { + run_test_repro!($issue, false, None) + }; + ($issue:expr, $should_fail:expr, $sender:expr) => {{ + let pattern = concat!(".*repros/", $issue); + let filter = Filter::path(pattern); + + let mut config = Config::default(); + if let Some(sender) = $sender { + config.sender = sender; + } + + let mut config = TestConfig::with_filter(runner_with_config(config), filter) + .set_should_fail($should_fail); + config.test() + }}; +} + // #[test] fn test_issue_2623() { @@ -134,3 +153,34 @@ fn test_issue_3223() { fn test_issue_3220() { test_repro!("Issue3220"); } + +// +#[test] +fn test_issue_3347() { + let mut res = run_test_repro!("Issue3347"); + let mut res = res.remove("repros/Issue3347.sol:Issue3347Test").unwrap(); + let test = res.test_results.remove("test()").unwrap(); + assert_eq!(test.logs.len(), 1); + let event = Event { + name: "log2".to_string(), + inputs: vec![ + EventParam { name: "x".to_string(), kind: ParamType::Uint(256), indexed: false }, + EventParam { name: "y".to_string(), kind: ParamType::Uint(256), indexed: false }, + ], + anonymous: false, + }; + let raw_log = RawLog { + topics: test.logs[0].topics.clone(), + data: test.logs[0].data.clone().to_vec().into(), + }; + let log = event.parse_log(raw_log).unwrap(); + assert_eq!( + log, + Log { + params: vec![ + LogParam { name: "x".to_string(), value: Token::Uint(1u64.into()) }, + LogParam { name: "y".to_string(), value: Token::Uint(2u64.into()) } + ] + } + ); +} diff --git a/testdata/repros/Issue3347.sol b/testdata/repros/Issue3347.sol new file mode 100644 index 000000000000..7dd2b5881831 --- /dev/null +++ b/testdata/repros/Issue3347.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity >=0.8.0; + +import "ds-test/test.sol"; +import "../cheats/Cheats.sol"; + +// https://github.com/foundry-rs/foundry/issues/3347 +contract Issue3347Test is DSTest { + event log2(uint256, uint256); + + function test() public { + emit log2(1, 2); + } +}