From a40b59c4886ab5da9995b96ccd97bf821f12e7b6 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Fri, 8 Nov 2024 14:11:31 +0200 Subject: [PATCH 1/4] fix(trace): check fn sigs for contract with fallbacks --- crates/evm/traces/src/decoder/mod.rs | 25 ++++++++- crates/forge/tests/cli/cmd.rs | 80 ++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/crates/evm/traces/src/decoder/mod.rs b/crates/evm/traces/src/decoder/mod.rs index 43a53286c17b..df521376b85c 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,20 @@ impl CallTraceDecoder { }; }; + // If traced contract is a fallback contract, then check if it has the decoded + // functions. 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 functions.iter().any(|decoded_function| { + !fallback_functions.contains(&decoded_function.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..22b00f420d1b 100644 --- a/crates/forge/tests/cli/cmd.rs +++ b/crates/forge/tests/cli/cmd.rs @@ -2818,3 +2818,83 @@ forgetest_init!(gas_report_include_tests, |prj, cmd| { .is_json(), ); }); + +// +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", "--gas-report"]) + .assert_success() + .stdout_eq(str![[r#" +... +Ran 1 test for test/DelegateProxyTest.sol:GasReportFallbackTest +[PASS] test_fallback_gas_report() ([GAS]) +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 | +... + +"#]]); +}); From 7696273cef95ca9efdede0a015d3731728c78a02 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Fri, 8 Nov 2024 15:58:19 +0200 Subject: [PATCH 2/4] Add Json test --- crates/forge/tests/cli/cmd.rs | 210 +++++++++++++++++++++------------- 1 file changed, 130 insertions(+), 80 deletions(-) diff --git a/crates/forge/tests/cli/cmd.rs b/crates/forge/tests/cli/cmd.rs index 22b00f420d1b..ad4dcb0d3d35 100644 --- a/crates/forge/tests/cli/cmd.rs +++ b/crates/forge/tests/cli/cmd.rs @@ -2390,6 +2390,136 @@ 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", "--gas-report"]) + .assert_success() + .stdout_eq(str![[r#" +... +Ran 1 test for test/DelegateProxyTest.sol:GasReportFallbackTest +[PASS] test_fallback_gas_report() ([GAS]) +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 { @@ -2818,83 +2948,3 @@ forgetest_init!(gas_report_include_tests, |prj, cmd| { .is_json(), ); }); - -// -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", "--gas-report"]) - .assert_success() - .stdout_eq(str![[r#" -... -Ran 1 test for test/DelegateProxyTest.sol:GasReportFallbackTest -[PASS] test_fallback_gas_report() ([GAS]) -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 | -... - -"#]]); -}); From 3bee48f48559a89313346acd195bde7f65c93734 Mon Sep 17 00:00:00 2001 From: grandizzy Date: Sun, 10 Nov 2024 12:29:24 +0200 Subject: [PATCH 3/4] Execute test with traces --- crates/forge/tests/cli/cmd.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/forge/tests/cli/cmd.rs b/crates/forge/tests/cli/cmd.rs index ad4dcb0d3d35..89335a492d01 100644 --- a/crates/forge/tests/cli/cmd.rs +++ b/crates/forge/tests/cli/cmd.rs @@ -2443,12 +2443,26 @@ contract GasReportFallbackTest is Test { ) .unwrap(); - cmd.args(["test", "--mt", "test_fallback_gas_report", "--gas-report"]) + 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 | | | | | | |---------------------------------------------------|-----------------|-------|--------|-------|---------| From c6001593da3507e6069d59ea81de030d950ea1fc Mon Sep 17 00:00:00 2001 From: grandizzy Date: Sun, 10 Nov 2024 12:52:41 +0200 Subject: [PATCH 4/4] Simplify, check only for decoded function --- crates/evm/traces/src/decoder/mod.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/evm/traces/src/decoder/mod.rs b/crates/evm/traces/src/decoder/mod.rs index df521376b85c..b63c7f1d7209 100644 --- a/crates/evm/traces/src/decoder/mod.rs +++ b/crates/evm/traces/src/decoder/mod.rs @@ -391,13 +391,11 @@ impl CallTraceDecoder { }; }; - // If traced contract is a fallback contract, then check if it has the decoded - // functions. If not, then replace call data signature with `fallback`. + // 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 functions.iter().any(|decoded_function| { - !fallback_functions.contains(&decoded_function.signature()) - }) { + if !fallback_functions.contains(&func.signature()) { call_data.signature = "fallback()".into(); } }