Skip to content

Commit

Permalink
fix(--gas-report): add back signatures, even if empty, avoid nestin…
Browse files Browse the repository at this point in the history 
…g multiple selectors (#9229)

* add back signatures, even if empty, flatten multiple selectors per feedback #9216 (comment)

* avoid manually serializing `gas_info`, already implements serialize
  • Loading branch information
@zerosnacks
zerosnacks authored Oct 30, 2024
1 parent ec2fd7d commit 748af79
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 72 deletions.
37 changes: 5 additions & 32 deletions crates/forge/src/gas_report.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,17 +181,6 @@ impl Display for GasReport {

impl GasReport {
fn format_json_output(&self) -> String {
#[inline]
fn format_gas_info(gas_info: &GasInfo) -> serde_json::Value {
json!({
"calls": gas_info.calls,
"min": gas_info.min,
"mean": gas_info.mean,
"median": gas_info.median,
"max": gas_info.max,
})
}

serde_json::to_string(
&self
.contracts
Expand All @@ -205,27 +194,11 @@ impl GasReport {
let functions = contract
.functions
.iter()
.map(|(fname, sigs)| {
// If there is only one signature, display the gas info directly.
let function_value = if sigs.len() == 1 {
format_gas_info(sigs.values().next().unwrap())
} else {
// If there are multiple signatures, e.g. overloads like:
// - `foo(uint256)`
// - `foo(int256)`
// display the gas info as a map with the signature as the key.
let signatures = sigs
.iter()
.map(|(sig, gas_info)| {
let display_name = sig.replace(':', "");
(display_name, format_gas_info(gas_info))
})
.collect::<BTreeMap<_, _>>();

json!(signatures)
};

(fname.to_string(), function_value)
.flat_map(|(_, sigs)| {
sigs.iter().map(|(sig, gas_info)| {
let display_name = sig.replace(':', "");
(display_name, gas_info)
})
})
.collect::<BTreeMap<_, _>>();

Expand Down
78 changes: 38 additions & 40 deletions crates/forge/tests/cli/cmd.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1601,7 +1601,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
"size": 255
},
"functions": {
"foo": {
"foo()": {
"calls": 1,
"min": 45387,
"mean": 45387,
Expand All @@ -1617,7 +1617,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
"size": 256
},
"functions": {
"baz": {
"baz()": {
"calls": 1,
"min": 260712,
"mean": 260712,
Expand All @@ -1633,7 +1633,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
"size": 255
},
"functions": {
"bar": {
"bar()": {
"calls": 1,
"min": 64984,
"mean": 64984,
Expand Down Expand Up @@ -1685,7 +1685,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
"size": 255
},
"functions": {
"foo": {
"foo()": {
"calls": 1,
"min": 45387,
"mean": 45387,
Expand All @@ -1701,7 +1701,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
"size": 256
},
"functions": {
"baz": {
"baz()": {
"calls": 1,
"min": 260712,
"mean": 260712,
Expand All @@ -1717,7 +1717,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
"size": 255
},
"functions": {
"bar": {
"bar()": {
"calls": 1,
"min": 64984,
"mean": 64984,
Expand Down Expand Up @@ -1769,7 +1769,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
"size": 255
},
"functions": {
"foo": {
"foo()": {
"calls": 1,
"min": 45387,
"mean": 45387,
Expand All @@ -1785,7 +1785,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
"size": 256
},
"functions": {
"baz": {
"baz()": {
"calls": 1,
"min": 260712,
"mean": 260712,
Expand All @@ -1801,7 +1801,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
"size": 255
},
"functions": {
"bar": {
"bar()": {
"calls": 1,
"min": 64984,
"mean": 64984,
Expand Down Expand Up @@ -1860,7 +1860,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
"size": 255
},
"functions": {
"foo": {
"foo()": {
"calls": 1,
"min": 45387,
"mean": 45387,
Expand All @@ -1876,7 +1876,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
"size": 256
},
"functions": {
"baz": {
"baz()": {
"calls": 1,
"min": 260712,
"mean": 260712,
Expand All @@ -1892,7 +1892,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
"size": 255
},
"functions": {
"bar": {
"bar()": {
"calls": 1,
"min": 64984,
"mean": 64984,
Expand Down Expand Up @@ -1935,7 +1935,7 @@ forgetest!(gas_report_some_contracts, |prj, cmd| {
"size": 255
},
"functions": {
"foo": {
"foo()": {
"calls": 1,
"min": 45387,
"mean": 45387,
Expand Down Expand Up @@ -1973,7 +1973,7 @@ forgetest!(gas_report_some_contracts, |prj, cmd| {
"size": 255
},
"functions": {
"bar": {
"bar()": {
"calls": 1,
"min": 64984,
"mean": 64984,
Expand Down Expand Up @@ -2014,7 +2014,7 @@ forgetest!(gas_report_some_contracts, |prj, cmd| {
"size": 256
},
"functions": {
"baz": {
"baz()": {
"calls": 1,
"min": 260712,
"mean": 260712,
Expand Down Expand Up @@ -2069,7 +2069,7 @@ forgetest!(gas_report_ignore_some_contracts, |prj, cmd| {
"size": 256
},
"functions": {
"baz": {
"baz()": {
"calls": 1,
"min": 260712,
"mean": 260712,
Expand All @@ -2085,7 +2085,7 @@ forgetest!(gas_report_ignore_some_contracts, |prj, cmd| {
"size": 255
},
"functions": {
"bar": {
"bar()": {
"calls": 1,
"min": 64984,
"mean": 64984,
Expand Down Expand Up @@ -2136,7 +2136,7 @@ forgetest!(gas_report_ignore_some_contracts, |prj, cmd| {
"size": 255
},
"functions": {
"foo": {
"foo()": {
"calls": 1,
"min": 45387,
"mean": 45387,
Expand All @@ -2152,7 +2152,7 @@ forgetest!(gas_report_ignore_some_contracts, |prj, cmd| {
"size": 256
},
"functions": {
"baz": {
"baz()": {
"calls": 1,
"min": 260712,
"mean": 260712,
Expand Down Expand Up @@ -2231,7 +2231,7 @@ warning: ContractThree is listed in both 'gas_reports' and 'gas_reports_ignore'.
"size": 255
},
"functions": {
"foo": {
"foo()": {
"calls": 1,
"min": 45387,
"mean": 45387,
Expand All @@ -2247,7 +2247,7 @@ warning: ContractThree is listed in both 'gas_reports' and 'gas_reports_ignore'.
"size": 256
},
"functions": {
"baz": {
"baz()": {
"calls": 1,
"min": 260712,
"mean": 260712,
Expand All @@ -2263,7 +2263,7 @@ warning: ContractThree is listed in both 'gas_reports' and 'gas_reports_ignore'.
"size": 255
},
"functions": {
"bar": {
"bar()": {
"calls": 1,
"min": 64984,
"mean": 64984,
Expand All @@ -2283,7 +2283,7 @@ warning: ContractThree is listed in both 'gas_reports' and 'gas_reports_ignore'.
"#]]);
});

forgetest!(gas_report_multiple_selectors, |prj, cmd| {
forgetest!(gas_report_flatten_multiple_selectors, |prj, cmd| {
prj.insert_ds_test();
prj.add_source(
"Counter.sol",
Expand Down Expand Up @@ -2351,35 +2351,33 @@ contract CounterTest is DSTest {
"size": 250
},
"functions": {
"a": {
"a()": {
"calls": 1,
"min": 2261,
"mean": 2261,
"median": 2261,
"max": 2261
},
"b": {
"b()": {
"calls": 1,
"min": 2305,
"mean": 2305,
"median": 2305,
"max": 2305
},
"setNumber": {
"setNumber(int256)": {
"calls": 2,
"min": 23648,
"mean": 33604,
"median": 33604,
"max": 43560
},
"setNumber(uint256)": {
"calls": 2,
"min": 23604,
"mean": 33560,
"median": 33560,
"max": 43516
}
"setNumber(int256)": {
"calls": 2,
"min": 23648,
"mean": 33604,
"median": 33604,
"max": 43560
},
"setNumber(uint256)": {
"calls": 2,
"min": 23604,
"mean": 33560,
"median": 33560,
"max": 43516
}
}
}
Expand Down

0 comments on commit 748af79

Please sign in to comment.