Skip to content

Commit

Permalink
fix(trace): check fn sigs for contract with fallbacks (#9287)
Browse files Browse the repository at this point in the history
* fix(trace): check fn sigs for contract with fallbacks

* Add Json test

* Execute test with traces

* Simplify, check only for decoded function
  • Loading branch information
grandizzy authored Nov 10, 2024
1 parent 765969d commit e028b92
Show file tree
Hide file tree
Showing 2 changed files with 166 additions and 1 deletion.
23 changes: 22 additions & 1 deletion crates/evm/traces/src/decoder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ pub struct CallTraceDecoder {
pub labels: HashMap<Address, String>,
/// Contract addresses that have a receive function.
pub receive_contracts: Vec<Address>,
/// Contract addresses that have fallback functions, mapped to function sigs.
pub fallback_contracts: HashMap<Address, Vec<String>>,

/// All known functions.
pub functions: HashMap<Selector, Vec<Function>>,
Expand Down Expand Up @@ -188,6 +190,7 @@ impl CallTraceDecoder {
(POINT_EVALUATION, "PointEvaluation".to_string()),
]),
receive_contracts: Default::default(),
fallback_contracts: Default::default(),

functions: hh_funcs()
.chain(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -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 {
Expand Down
144 changes: 144 additions & 0 deletions crates/forge/tests/cli/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2390,6 +2390,150 @@ contract CounterTest is DSTest {
);
});

// <https://github.com/foundry-rs/foundry/issues/9115>
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 {
Expand Down

0 comments on commit e028b92

Please sign in to comment.