From e028b92698eae7e5019025e1784e7c06c3cae534 Mon Sep 17 00:00:00 2001 From: grandizzy <38490174+grandizzy@users.noreply.github.com> Date: Sun, 10 Nov 2024 13:19:11 +0200 Subject: [PATCH] fix(trace): check fn sigs for contract with fallbacks (#9287) * fix(trace): check fn sigs for contract with fallbacks * Add Json test * Execute test with traces * Simplify, check only for decoded function --- crates/evm/traces/src/decoder/mod.rs | 23 ++++- crates/forge/tests/cli/cmd.rs | 144 +++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 1 deletion(-) diff --git a/crates/evm/traces/src/decoder/mod.rs b/crates/evm/traces/src/decoder/mod.rs index 43a53286c17b..b63c7f1d7209 100644 --- a/crates/evm/traces/src/decoder/mod.rs +++ b/crates/evm/traces/src/decoder/mod.rs @@ -121,6 +121,8 @@ pub struct CallTraceDecoder { pub labels: HashMap, /// Contract addresses that have a receive function. pub receive_contracts: Vec
, + /// Contract addresses that have fallback functions, mapped to function sigs. + pub fallback_contracts: HashMap>, /// All known functions. pub functions: HashMap>, @@ -188,6 +190,7 @@ impl CallTraceDecoder { (POINT_EVALUATION, "PointEvaluation".to_string()), ]), receive_contracts: Default::default(), + fallback_contracts: Default::default(), functions: hh_funcs() .chain( @@ -222,6 +225,7 @@ impl CallTraceDecoder { } self.receive_contracts.clear(); + self.fallback_contracts.clear(); } /// Identify unknown addresses in the specified call trace using the specified identifier. @@ -317,6 +321,14 @@ impl CallTraceDecoder { if abi.receive.is_some() { self.receive_contracts.push(*address); } + + if abi.fallback.is_some() { + let mut functions_sig = vec![]; + for function in abi.functions() { + functions_sig.push(function.signature()); + } + self.fallback_contracts.insert(*address, functions_sig); + } } } @@ -379,9 +391,18 @@ impl CallTraceDecoder { }; }; + // If traced contract is a fallback contract, check if it has the decoded function. + // If not, then replace call data signature with `fallback`. + let mut call_data = self.decode_function_input(trace, func); + if let Some(fallback_functions) = self.fallback_contracts.get(&trace.address) { + if !fallback_functions.contains(&func.signature()) { + call_data.signature = "fallback()".into(); + } + } + DecodedCallTrace { label, - call_data: Some(self.decode_function_input(trace, func)), + call_data: Some(call_data), return_data: self.decode_function_output(trace, functions), } } else { diff --git a/crates/forge/tests/cli/cmd.rs b/crates/forge/tests/cli/cmd.rs index feb608a1e12a..89335a492d01 100644 --- a/crates/forge/tests/cli/cmd.rs +++ b/crates/forge/tests/cli/cmd.rs @@ -2390,6 +2390,150 @@ contract CounterTest is DSTest { ); }); +// +forgetest_init!(gas_report_with_fallback, |prj, cmd| { + prj.add_test( + "DelegateProxyTest.sol", + r#" +import {Test} from "forge-std/Test.sol"; + +contract ProxiedContract { + uint256 public amount; + + function deposit(uint256 aba) external { + amount = amount * 2; + } + + function deposit() external { + } +} + +contract DelegateProxy { + address internal implementation; + + constructor(address counter) { + implementation = counter; + } + + function deposit() external { + } + + fallback() external payable { + address addr = implementation; + + assembly { + calldatacopy(0, 0, calldatasize()) + let result := delegatecall(gas(), addr, 0, calldatasize(), 0, 0) + returndatacopy(0, 0, returndatasize()) + switch result + case 0 { revert(0, returndatasize()) } + default { return(0, returndatasize()) } + } + } +} + +contract GasReportFallbackTest is Test { + function test_fallback_gas_report() public { + ProxiedContract proxied = ProxiedContract(address(new DelegateProxy(address(new ProxiedContract())))); + proxied.deposit(100); + proxied.deposit(); + } +} +"#, + ) + .unwrap(); + + cmd.args(["test", "--mt", "test_fallback_gas_report", "-vvvv", "--gas-report"]) + .assert_success() + .stdout_eq(str![[r#" +... +Ran 1 test for test/DelegateProxyTest.sol:GasReportFallbackTest +[PASS] test_fallback_gas_report() ([GAS]) +Traces: + [331067] GasReportFallbackTest::test_fallback_gas_report() + ├─ [106511] → new ProxiedContract@[..] + │ └─ ← [Return] 246 bytes of code + ├─ [108698] → new DelegateProxy@[..] + │ └─ ← [Return] 143 bytes of code + ├─ [29396] DelegateProxy::fallback(100) + │ ├─ [3320] ProxiedContract::deposit(100) [delegatecall] + │ │ └─ ← [Stop] + │ └─ ← [Return] + ├─ [21160] DelegateProxy::deposit() + │ └─ ← [Stop] + └─ ← [Stop] + +Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] +| test/DelegateProxyTest.sol:DelegateProxy contract | | | | | | +|---------------------------------------------------|-----------------|-------|--------|-------|---------| +| Deployment Cost | Deployment Size | | | | | +| 108698 | 315 | | | | | +| Function Name | min | avg | median | max | # calls | +| deposit | 21160 | 21160 | 21160 | 21160 | 1 | +| fallback | 29396 | 29396 | 29396 | 29396 | 1 | + + +| test/DelegateProxyTest.sol:ProxiedContract contract | | | | | | +|-----------------------------------------------------|-----------------|------|--------|------|---------| +| Deployment Cost | Deployment Size | | | | | +| 106511 | 276 | | | | | +| Function Name | min | avg | median | max | # calls | +| deposit | 3320 | 3320 | 3320 | 3320 | 1 | +... + +"#]]); + + cmd.forge_fuse() + .args(["test", "--mt", "test_fallback_gas_report", "--gas-report", "--json"]) + .assert_success() + .stdout_eq( + str![[r#" +[ + { + "contract": "test/DelegateProxyTest.sol:DelegateProxy", + "deployment": { + "gas": 108698, + "size": 315 + }, + "functions": { + "deposit()": { + "calls": 1, + "min": 21160, + "mean": 21160, + "median": 21160, + "max": 21160 + }, + "fallback()": { + "calls": 1, + "min": 29396, + "mean": 29396, + "median": 29396, + "max": 29396 + } + } + }, + { + "contract": "test/DelegateProxyTest.sol:ProxiedContract", + "deployment": { + "gas": 106511, + "size": 276 + }, + "functions": { + "deposit(uint256)": { + "calls": 1, + "min": 3320, + "mean": 3320, + "median": 3320, + "max": 3320 + } + } + } +] +"#]] + .is_json(), + ); +}); + forgetest_init!(can_use_absolute_imports, |prj, cmd| { let remapping = prj.paths().libraries[0].join("myDependency"); let config = Config {