feat: gas snapshots over arbitrary sections #8952
Conversation
…as-section-snapshot-cheatcodes
…ub.com:foundry-rs/foundry into zerosnacks/add-gas-section-snapshot-cheatcodes
grandizzy
left a comment
There was a problem hiding this comment.
lgtm, have couple of nits and question re calling meter_gas_rrecord in step_end
| // Check for differences in gas snapshots if `FORGE_SNAPSHOT_CHECK` is set. | ||
| // Exiting early with code 1 if differences are found. | ||
| if std::env::var("FORGE_SNAPSHOT_CHECK").is_ok() { | ||
| let differences_found = gas_snapshots.clone().into_iter().fold( |
There was a problem hiding this comment.
for fmt we use TextDiff::from_lines and then compute ratio to figure out if files differs / should be written to disk, wonder if it would be suitable to use same here
foundry/crates/forge/bin/cmd/fmt.rs
Lines 128 to 129 in 3ff3d05
There was a problem hiding this comment.
Good point, I think this could be added as a future optimization, to only write snapshots that differ. Combine this with a glob over the files in the snapshots folder to filter out unmatched snapshots that only exist in the old and not in the new (to avoid staining).
mattsse
left a comment
There was a problem hiding this comment.
some stlye nits, otherwise lgtm
pending @klkvr and @grandizzy
Motivation
Supersedes #8755
Renaming of
state_snapshothandled in #8945This has been done so that the gas snapshot implementation can be reviewed more easily without being cluttered by renamings.
Closes: #2065 + Closes: #2056 + Closes: #8713 + Closes: #7900
Supports:
Uses a
snapshotsdirectory without.prefix to indicate it is meant to be checked in.Cheatcodes:
snapshotValuesnapshotGasLastCall(wrapslastCallGas)startSnapshotGasstopSnapshotGasAll cheatcodes support
grouping, allowing you to group arbitrary snapshots together by group name. If the group name is not supplied the name is used as filename.We support multiple outputs:
If no
groupis supplied, uses thecontract nameas file name e.g.snapshots/GasSnapshotTest.json:Where the order is always sorted alphabetically and deterministic.
If a
groupname is supplied, uses the custom name e.g.CustomGroup(stored insnapshots/CustomGroup.json)In terms of accuracy this is a slight difference between the cheatcode implementation (
A) and the nativegasleft()implementation (B). I've added an assertion check in the test suite to make sure this does not drift further unexpectedly.Supports
FORGE_SNAPSHOT_CHECK=trueflag to assert gas snapshots haven't changed across runs. By default on every invocation offorge testthe snapshot directory is cleared out to prevent stale snapshots. To be able to perform the diff check this is disabled when theFORGE_SNAPSHOT_CHECKis set.Tasks
FORGE_SNAPSHOT_CHECK=trueenvironment variable to revert on a mismatch in the snapshot with gas usedgroupis passed automatically generate the snapshot group: for exampletestContractNameas a default if not overridden with a specific string.vm.snapshotGasLastCallto wraplastCallGas? Single call is a very common use case and would be nice to skip the wrapping stepsforge cleanFollow up
Deriving function names automatically is quite complex as the test being ran isn't aware of the function specific context, only the contract.
I've set it up in a way (w/ name:
Option<String>) to easily add it in a follow-up PR in a non-breaking way.If no
nameis passed automatically generate the snapshot name: for exampletestFunctionas a default if not overridden with a specific string.Currently it is not possible to start / nest multiple snapshots without first closing the started snapshot. This could be added in a follow-up, see: feat: gas snapshots over arbitrary sections #8952 (comment)
Currently gas snapshots are only supported in unit tests, not fuzz tests. This is largely due to it conflicting with the
FORGE_SNAPSHOT_CHECKwhere it is highly likely that with different fuzzed input the gas measurement differs as well. In the future it would be an idea to capture the average gas.Possible add
snapshotGasNextCallto mirrorvm.snapshotGasLastCallfor improved user experience