Skip to content
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

User-defined gas reporting #103

Closed
holic opened this issue Apr 10, 2023 · 4 comments
Closed

User-defined gas reporting #103

holic opened this issue Apr 10, 2023 · 4 comments
Assignees

Comments

@holic
Copy link

holic commented Apr 10, 2023

Foundry's built in gas reports leave a lot to be desired. Often times what I want to test in terms of gas usage is not 1:1 with tests, so creating individual tests just for the purposes of making a call to determine gas usage feels a little silly.

In our recently overhaul of MUD internals, we've added our own gas report tooling using a special // !gasreport line in our tests that wraps the next line in a gasleft() calculation, executes the test, and then records the gas usage to a file that we check in and compare in each PR to see how gas is changing. This works but is very slow and clunky.

I'd love for some official Solidity gas reporting support, something along the lines of:

function testBehavior() public {
  // calculates gas used by next line 
  reportGas('abi encode');
  abi.encode('hello world');

  // calculates gas within the start/end calls
  // block added for readability
  startReportGas('mint and burn');
  {
    mint();
    burn();
  }
  endReportGas();
}

Related:

@holic
Copy link
Author

holic commented Apr 10, 2023

cc @alvrs @dk1a

@vdrg
Copy link
Contributor

vdrg commented Apr 11, 2023

Thanks @holic.

I'm not sure how we could implement the first way you propose (report for next line) at the Vulcan layer. There might be ways we can come up with but for external calls only, which doesn't seem to be what you are looking for.

For the second method I'm sure we can figure something out. Are you proposing that we should write the report to a file or do you envision something more generic which users can later extend? For example, we could store a mapping of name -> gasUsed and then users can decide how to store the results (or log them), and we could also provide some optional and opinionated ways of doing this.

Another alternative I'm thinking of, but only works for external calls, would be to integrate something like this into our "watchers" module, so you could do something like:

address(myContract).watch();

myContract.doSomething();

// Use any of the matchers, like toEqual, toBeGreaterThan, etc
expect(address(myContract).lastCall().gasUsed).toEqual(123);

// Or print it
println("{uint}", address(myContract).lastCall().gasUsed);

Thoughts?

@holic
Copy link
Author

holic commented Apr 11, 2023

Are you proposing that we should write the report to a file or do you envision something more generic which users can later extend?

It'd be great if we could somehow hook into + append to the existing gas report, since it already has the capability to output in different formats, do comparisons, etc. But I'm not sure how feasible that is.

At minimum, a gas-report-specific log line or trace line would do well enough for us. We can output with verbosity and parse out those messages for creating the reports we'll use in comparisons/CI, etc. Anywhere in between is great too.

I think the main thing I'd like to see (that seems to be missing from foundry/forge-std) is gas reporting for internal calls. We make use of a lot of libraries with internal methods and want to be able to test gas use of those. We arrived at our current method of comment -> codemod -> forge test -> parse output because we weren't seeing gas reports that we needed on those internal calls.

@holic holic mentioned this issue Apr 16, 2023
@gnkz gnkz moved this to Todo in Nomoi Global Board Aug 22, 2023
@gnkz gnkz moved this from Todo to Done in Nomoi Global Board Aug 22, 2023
@gnkz
Copy link
Contributor

gnkz commented Aug 22, 2023

Closing this as this feature was added in #104

@gnkz gnkz closed this as completed Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

3 participants