Price data should be fetched async and configured for reporter

[SimonVillage]

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.

[cgewecke]

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

[SimonVillage]

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

[cgewecke]

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.

[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.

[SimonVillage]

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.

[cgewecke]

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

[aspiers]

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:

so that's another possibility.

[cgewecke]

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