-
Notifications
You must be signed in to change notification settings - Fork 2k
Enable writing gas reports for specific contracts to .snapshot
#2065
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Another idea would be to just have a new |
As mentioned in another snapshot/gas report issue/request recently we used to talk about just merging |
Ok got it, thanks. Feel free to close if this is a duplicate. |
@gakonst I'm happy to work on this if we can get consensus on what to implement :) |
Maybe some optimizoors disagree (and I'd be curious to hear their thoughts first), but my suggestion would be:
If we go this route I'd split things into a few PRs:
|
Sorry @mds1 just saw your reply. That sounds reasonable to me! I'd propose default new snapshot is just to write gas-report. |
Relevant: https://github.com/marktoda/forge-gas-snapshot/ Would be great to upstream this into Foundry |
Component
Forge
Describe the feature you would like
Problem:
Currently, changes in
.snapshot
do not necessarily reflect gas changes a developer cares about. Snapshots directly reflect the gas use of the test functions, and only indirectly reflect the gas used by the contract being tested.If a developer changes a test and the contract being tested in the same diff, they lose signal in the
.snapshot
as to the gas effect of their changes.We see many developers solve this by creating super minimal tests with names like
testXGas
so that they always have some indication of exclusively the changing gas use in the contract being tested.In fact, for me personally, I generally only care about the snapshot of these functions. The gas snapshot of all the other functions is mostly just noise.
Proposed solution
A very useful tool is
forge test --gas-report
it shows the gas use of each function in each contract! This is great, and could be super powered by (1) enabling writing this gas report to the snapshot (2) enabling passing arguments as to which contracts we exclusively want gas reports for.The command might look something like
This command could run existing snapshot functionality and additionally write the gas report of the specified contracts to the end of the snapshot. Though, for me personally, if writing the gas-report to snapshot existed, I could do without the existing snapshot functionality entirely, I think. Possibly still nice for quickly checking gas of something, though.
Additional context
No response
The text was updated successfully, but these errors were encountered: