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

feat: report benchmarks of each tool #3

Open
mds1 opened this issue Dec 8, 2021 · 3 comments
Open

feat: report benchmarks of each tool #3

mds1 opened this issue Dec 8, 2021 · 3 comments

Comments

@mds1
Copy link
Owner

mds1 commented Dec 8, 2021

For each tool, benchmark scripts should report

  • full execution time, i.e. including tool startup time
  • execution time of just the transaction simulation (not sure if this is possible for each tool)
  • gas used
  • some simple state verification, e.g. check logs or traces, or perhaps we just check this one manually for now?
@davidmurdoch
Copy link

I think we need to determine which gasUsed is the correct gasUsed, as there really is only one correct answer, so benchmarks that get it wrong should fail. Anything else just means that the tool is doing it wrong and the results can't be trusted anyway.

I'd love to get all forking maintainers on a call together to share our knowledge with eachother (cc @MicaiahReid)

@brockelmore
Copy link

eh - i think there is a specific configuration that should be standardized but what is actually useful as test output to the user maybe be a difference of opinion (and thus may differ between tools). Some tools like dapptools strip out things like creation of the test contract and base fee and calldata cost to give the user a better picture of execution costs.

@davidmurdoch
Copy link

@brockelmore yeah, it may make sense to ensure state changes are as expected; not sure what would look like for each tool though.

mds1 pushed a commit that referenced this issue Mar 7, 2022
* Add Blocknative (#1)

* Add blocknative tx-preview

* Linitng

* Merge commits from public repo (#2)

* Feature | Misc updates (#3)

* Update per mds1 comments - add gasUsed field from simulation and console.log it

* Makefile save and Readme update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants