hardhat-gas-reporter icon indicating copy to clipboard operation
hardhat-gas-reporter copied to clipboard

Price data should be fetched async and configured for reporter

Open SimonVillage opened this issue 3 years ago • 7 comments

I added my own CMC API key. Sometimes the column usd (avg) does not contain any value and sometimes it does.

Does anyone have the same issue? It is enough to just run the test like 5 times in a row to get the price at some point.

SimonVillage avatar Mar 30 '21 13:03 SimonVillage

@SimonHausdorf Could you share your config (without the key value) just to double-check that it's ok?

cgewecke avatar Mar 30 '21 15:03 cgewecke

I created a small example: https://github.com/SimonHausdorf/GasFeeTest

I changed the cmc key in the config. My current output looks like this

·-----------------------|----------------------------|-------------|----------------------------·
|  Solc version: 0.8.3  ·  Optimizer enabled: false  ·  Runs: 200  ·  Block limit: 9500000 gas  │
························|····························|·············|·····························
|  Methods                                                                                      │
·············|··········|··············|·············|·············|··············|··············
|  Contract  ·  Method  ·  Min         ·  Max        ·  Avg        ·  # calls     ·  usd (avg)  │
·············|··········|··············|·············|·············|··············|··············
|  Box       ·  burn    ·           -  ·          -  ·      35973  ·           1  ·          -  │
·············|··········|··············|·············|·············|··············|··············
|  Deployments          ·                                          ·  % of limit  ·             │
························|··············|·············|·············|··············|··············
|  Box                  ·           -  ·          -  ·    1502095  ·      15.8 %  ·          -  │
·-----------------------|--------------|-------------|-------------|--------------|-------------·

Edit: I checked out the tests in the original repository and have the same issue.

Edit2: CMC is only used to get the current ETH price? If yes, maybe we can switch to something free: https://api.coinpaprika.com/ so users don't have to maintain / configure a cmc key

Edit3: https://github.com/cgewecke/eth-gas-reporter/blob/bbf521f137407f106aeec44c730541dda6734ce3/index.js#L53 should be await?

 * An exception is made for fetching gas & currency price data from coinmarketcap and
 * ethgasstation (we hope that single call will complete by the time the tests finish running)

I think there is too much hope here :P

SimonVillage avatar Mar 30 '21 17:03 SimonVillage

Ah! Thanks for the reproduction and analysis.

Yes, you're hitting a latency problem because your tests execute so quickly. My computer's a little slow (or my internet connection is faster?) and was able to run your example repeatedly without missing the data.

In hardhat-gas-reporter we can fetch that data async so we should. In Truffle / eth-gas-reporter it's not really possible because the mocha 3rd party reporter api only supports synchronous calls.

Screen Shot 2021-03-30 at 1 24 39 PM

cgewecke avatar Mar 30 '21 20:03 cgewecke

CMC is only used to get the current ETH price? If yes, maybe we can switch to something free: https://api.coinpaprika.com/ so users don't have to maintain / configure a cmc key

Yes, coinmarketcap used to have open endpoints as well ... would definitely prefer if this feature was zero config. But I'm hesitant to change data providers because it's almost inevitable that they'll add an api key eventually to prevent DDOS.

cgewecke avatar Mar 30 '21 20:03 cgewecke

I guess my current location slows it down. I added this to my tests to temporarily solve the problem:

await new Promise((r) => setTimeout(r, 500));

Yes, coinmarketcap used to have open endpoints as well ... would definitely prefer if this feature was zero-config. But I'm hesitant to change data providers because it's almost inevitable that they'll add an API key eventually to prevent DDOS.

coinpaprika exists since a long time too, and their promise is: Delivered at No Cost - because “Information wants to be free” Guess there are a few people which might oversee the point with CMC's API key configuration. Maybe a warning when starting the test would be nice? Warning: You did not configure your own CMC key, something like that.

Anyways, thanks for the work :) I will not maintain the issue any longer, so feel free to close it or to keep it for a backlog.

SimonVillage avatar Mar 31 '21 03:03 SimonVillage

Will keep open and consider coinpaprika, thank you. Renaming.

cgewecke avatar Mar 31 '21 12:03 cgewecke

I think this issue needs to be renamed? IIUC, the race condition was already fixed and released, and so this is open for the purposes of considering switching to another price feed service like coinpaprika, which sounds like a fantastic idea to me.

BTW, https://www.coingecko.com/en/api says:

image

so that's another possibility.

aspiers avatar Jan 07 '22 17:01 aspiers

This is fixed in the latest version. All price data is fetched async at the end of the test run.

https://github.com/cgewecke/hardhat-gas-reporter/releases/tag/v2.0.0

cgewecke avatar Mar 14 '24 05:03 cgewecke