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

Update dependency hardhat-gas-reporter to v2 #4979

Merged
merged 2 commits into from
Apr 3, 2024

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Mar 27, 2024

Mend Renovate

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
hardhat-gas-reporter ^1.0.9 -> ^2.0.0 age adoption passing confidence

Release Notes

cgewecke/hardhat-gas-reporter (hardhat-gas-reporter)

v2.0.2: Fix crash with --parallel flag

Compare Source

What's Changed

Full Changelog: cgewecke/hardhat-gas-reporter@v2.0.1...v2.0.2

v2.0.1: Optimism Ecotone (EIP-4844)

Compare Source

What's Changed

Full Changelog: cgewecke/hardhat-gas-reporter@v2.0.0...v2.0.1

v2.0.0: : Optimism L2 & View/Pure Method Gas

Compare Source

Screen Shot 2024-03-14 at 12 48 35 PM

What's New

  • Configuration for chains with Etherscan support has been simplified to a single key. Just set the L1 option to one of the supported networks and the reporter will take care of the rest. (You'll still need a coinmarketcap api key)

    gasReporter: {
      L1: "gnosis",
      coinmarketcap: "abc...",
    }
    
  • Gas reporting for L2 networks is coming online, starting with Optimism.

    gasReporter: {
      L2: "optimism",
      coinmarketcap: "abc...",
    }
    
  • Gas metrics for view and pure methods are now available as an option. You can also exclude intrinsic gas costs for state-changing methods. (⚠️ There are performance hits when the view and pure options are enabled)

    gasReporter: {
      // Debits intrinsic gas for state-changing method calls in order to model contracts
      // that will never be called by an EOA
      includeIntrinsicGas: false,
    
      // This option executes an additional `eth_estimateGas` for every `eth_call`
      // detected by the reporter. If you have 1000's of tests setting it to true has a
      // noticeable performance impact
      reportPureAndViewMethods: true,
    
      // This option can add SIGNIFICANT LAG to test startup time if you have
      // 100's of contracts in your project. (It parses all the sources in your dependency tree
      // to identify state variable declarations)
      excludeAutoGeneratedGetters: true,
    }
    
  • There are multiple report formats, including markdown.

  • The reporter now supports sub-gwei gas prices. Sub-penny cost display is possible by configuring the currencyDisplayPrecision option

  • Dedicated support for the OpenZeppelin Upgrades plugin has been added. (Their proxy pattern often resulted in missing gas data because the reporter didn't know what contract was being called - that's all handled under the hood now.)

  • Dedicated support for the hardhat-viem plugin has been added (this was broken and should be fixed by this release)

  • There are many new output, display and low-level options - check out the Config Examples section of the docs

  • There's additional support for custom proxy contract resolution. If you're routing your calls through contract middleware you can configure the reporter to understand how that works and get the data you expect.

  • Additionally:

    • eth-gas-reporter's logic has been ported here and translated to Typescript
    • The plugin has been decoupled from Mocha so it can be seamlessly integrated with lots of other tasks or test frameworks
    • There have been big architectural changes and testing improvements and additional features are in the pipeline

Breaking

  • Codechecks support was removed because it hasn't been accepting users for a while. I loved codechecks. (Building a github action for the reporter is on the V2 roadmap though).
  • The JSON object emitted by the reporter has changed to reflect the plugin's internal types. If you've been post-processing that data you'll need to look at the JSON Output docs and update your logic.
  • The gas-reporter:merge task has been renamed hhgas:merge
  • The onlyCalledMethods option has been renamed showUncalledMethods and must be set to true (if you want that).

Funding

Work on V2 was funded in part by OpenZeppelin via DRIPS, a public goods protocol that helps direct funding to packages in your dependency tree. If you're using DRIPS and want to add hardhat-gas-reporter to the packages you support its page is here.

Full Changelog: cgewecke/hardhat-gas-reporter@v1.10.0...v2.0.0

v1.0.10

Compare Source


Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.


  • If you want to rebase/retry this PR, check this box

This PR has been generated by Mend Renovate. View repository job log here.

Copy link

changeset-bot bot commented Mar 27, 2024

⚠️ No Changeset found

Latest commit: a2c6a4e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

socket-security bot commented Mar 27, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/[email protected] Transitive: environment, filesystem, shell +23 2.69 MB isaacs
npm/[email protected] environment, filesystem Transitive: network, unsafe +70 69 MB cgewecke

🚮 Removed packages: npm/[email protected], npm/[email protected]

View full report↗︎

@Amxx
Copy link
Collaborator

Amxx commented Mar 27, 2024

The breaks gas comparaison because report either changed format or location. Need to fix the comparaison script.

@Amxx
Copy link
Collaborator

Amxx commented Mar 27, 2024

@ernestognw I updated the script for gas comparaison. Also needed a hardhat config to replicate old behavior.

@Amxx
Copy link
Collaborator

Amxx commented Mar 27, 2024

Comparaison gets generated, but results are widelly different from what we have before. That is strange. Will investigate.

Edit running only test/token/ERC20/ERC20.test.js with both version of the plugin shows no differences at all.

@Amxx Amxx self-assigned this Mar 27, 2024
@Amxx Amxx added this to the 5.1 milestone Mar 27, 2024
@Amxx Amxx added the CI label Mar 27, 2024
@cgewecke
Copy link
Contributor

cgewecke commented Mar 27, 2024

Apologies for looking over your shoulder @Amxx...

Are the methods with a lot of variance using OZ's upgrades plugin? That's a possible source of change in the gas readings because v1.0 was unable to resolve shadowed method calls routed through proxies correctly. v2.0 now does this automatically if it detects the upgrades plugin in the environment.

UPDATE

Looking through the markdown report it seems this is likely - V2 has discovered some new contract/method entries which did not appear in the V1 report and V1 was incorrectly attributing gas readings for these methods to other contracts whose methods have the same function signature. (The gas-reporter's policy has been to fall back on method signatures alone to associate gas data with a contract if all else fails.)

For example

  • ERC4626._approve(address,address,uint256) is a brand new entry with a min of 24823
  • ERC20._approve(address,address,uint256) has seen its min increase by 4824 to 29647

This means the V1 gas-reporter was misattributing the ERC4626 method call to ERC20 whereas V2 correctly resolves it.

Copy link
Contributor Author

renovate bot commented Mar 28, 2024

Edited/Blocked Notification

Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR.

You can manually request rebase by checking the rebase/retry box above.

Warning: custom changes will be lost.

@Amxx Amxx requested a review from ernestognw March 28, 2024 09:22
@ernestognw
Copy link
Member

Thanks for the clarification @cgewecke, it would've taken a lot for us to figure it out.

The gas differences are mainly shown in ERC4626, which make sense with what you shared.

@ernestognw ernestognw merged commit 6f4ebf1 into master Apr 3, 2024
21 checks passed
@ernestognw ernestognw deleted the renovate/hardhat-gas-reporter-2.x branch April 3, 2024 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants