1) enable reporter for imported contracts 2) gas-reporter discrepancies with mainnet

[gitpusha]

Hi @cgewecke ,

I realized that hardhat-gas-reporter only seems to report for contracts who were compiled with hardhat and thus have certain files associated with them in artifacts/ .

Is this correct?

In our integration testing we make use of forking and thus instantiate many contracts with await ethers.getContractAt(abi, addresss) , where the abi is in a pre-compiled/ folder.

It would be amazing if hardhat-gas-reporter would also report gas usage of methods called on those instances.

What would need to change for that to happen?

[cgewecke]

Hi @gitpusha,

Yes, agree this would be really useful and should be straightforward to implement.

If there was a gasReporter option like remoteContracts which took an array of

[
  {
    contractName: <string>
    address: <string>
    abi:  any
  },
]

would you be able to supply that info for the on-chain contracts before the tests run?
(Very sorry, you say you do in your comment.)

Do you have a repo I could test an option like this with?

[gitpusha]

Yes, such an option would be great !

Here is a repo you can test this with.

We have some massive integration tests in there.

Maybe just test with this one.

Here you can find all the pre-compiles this test uses.

And here are the addresses of the pre-compiles.

If you wanna, you can release and rc and I can test this stuff too.

[cgewecke]

Have published an experimental rc version of this:

yarn install hardhat-gas-reporter@rc --dev

To test, I added the following to your hardhat.config.js

const fs = require('fs');
const path = require('path');

// Load remote contract address and ABI data  
const remoteContracts = [];
const preCompilesPath = 'pre-compiles';
for (const fileName of fs.readdirSync(preCompilesPath)){
  const artifactPath = path.join(process.cwd(), preCompilesPath, fileName)
  const preDeployed = require(artifactPath);
  const address = mainnetDeployments[preDeployed.contractName]
  if (address){
    remoteContracts.push({
      name: preDeployed.contractName,
      abi: preDeployed.abi,
      address: address
    })
  }
}

and added remoteContracts to the gasReporter options

 gasReporter: {
     enabled: process.env.REPORT_GAS ? true : false,
     maxMethodDiff: 25,
     coinmarketcap: process.env.COINMARKETCAP_API_KEY,
+    remoteContracts
  },

However I'm not sure the results are what is expected. Only caught a couple of additional method calls, to ConnectAuth and InstaConnectors.

Are you expecting more data here? Do you have lots of calls routed through a proxy or call-forwarding contract?

[gitpusha]

Amazing! Thanks so much for the hands-on support.

The table looks pretty good to me. I was opening this issue with generality in mind but I only had a couple of methods missing. So yes, looks good to me.

Do you have lots of calls routed through a proxy or call-forwarding contract?

Yes, that is why there aren't too many new methods shown, but that's expected.

I will use your rc and test it myself. When do you think will you make it a stable release?

I see that your method only requires:

    remoteContracts.push({
      name: preDeployed.contractName,
      abi: preDeployed.abi,
      address: address

Hence a contractName and abi first-level property on whatever artifact is in my pre-compiles folder. This basically means that as long as my pre-compiles have those 2 properties it should work right? They don't need to be a hardhat style artifact, correct?

Also I wanted to ask about how the gas-reporter measures gas. I had a quick look at the code and I saw it makes use of a txReceipt.gasUsed property, which is kinda what I intuitively expected. However, when we measured our gas used manually with ethers.js and the gasUsed property on a TransactionReceipt we found that our manual measurements diverged quite a bit from the gas-reporter tool. Notably, the manual measurements were quite a bit higher. Do you know why this could be the case ?

[cgewecke]

Yes, that is why there aren't too many new methods shown, but that's expected.

There's an option available which lets you to pass the reporter a method to resolve the real identity of proxy calls. It's a little complicated (but it's theoretically possible...).

This basically means that as long as my pre-compiles have those 2 properties it should work right?

Yes, exactly.

when we measured our gas used manually with ethers.js and the gasUsed property on a TransactionReceipt ...

Oh! Super interesting. How did you measure manually?

The hardhat reporter really just wraps the Ethers provider - it would be great to track down exactly where the gap you're seeing comes from. We run the eth-gas-reporter test suite against ganache and geth and their results differ by a few hundred gas for very simple methods.

• geth
• ganache

Also, recently a bug was reported at ganache that gas readings are substantially off when running in forked mode.

(Don't know how closely HH tracks geth - should add the same test to this repo.)

[cgewecke]

Missed a question...

When do you think will you make it a stable release?

I'm not sure...it might be a couple weeks. There are a couple other fixes I'd like to get in here

[gitpusha]

There's an option available which lets you to pass the reporter a method to resolve the real identity of proxy calls. It's a little complicated (but it's theoretically possible...).

I had a look at it. Pretty cool. But actually we are not making typical Proxy calls via a fallback method. The problem for gas reporting is rather that we use a single entry point function for different payloads. This function wraps around many different paths of execution. And we want to measure gas for those different execution paths.

So even though the important stuff for gas-reporting is hidden in the payload, the gas-reporter tool will average out gasUsed across all our tested execution paths, which renders it kind of useless in that context because we want to differentiate between them distinctly.

The difference in execution paths are nested deep down, so I cannot think of a simple way to write a general Resolver for this. I think it's easier for now to just Mock the different paths by giving them all a distinct method name and then submit the respective payloads via this distinct method name, thereby tricking the tool into reporting gas for these different paths. The real system will only have one entry method though, not several.

Oh! Super interesting. How did you measure manually?

Just using ethers TransactionReceipt.gasUsed property. Probably similar to what you do.

Also, recently a bug was reported at ganache that gas readings are substantially off when running in forked mode.

We also run our tests in hardhat forkmode. So maybe something similar is going on here. The measurements were off by a significant margin (something like 600k gas) .

I will probably have some files where you can review this soon and share them with you.

[cgewecke]

I think it's easier for now to just Mock the different paths by giving them all a distinct method name and then submit the respective payloads via this distinct method name, thereby tricking the tool into reporting gas for these different paths.

Yes this makes a lot of sense.

I will probably have some files where you can review this soon and share them with you.

Ok cool, yes I'd really like to see this and get reproduction steps for the difference.

[gitpusha]

Ok cool, yes I'd really like to see this and get reproduction steps for the difference.

Ok, so I just tried to reproduce myself and for me the manual ethers method

const txResponse = await contract.method();
const { gasUsed } = await txResponse.wait();
console.log(gasUsed);

yields exactly the same results as hardhat-gas-reporter which should be expected and which is a relief of sorts. My colleague reported the disrepancy to me but I wasnt able to reproduce myself.

However, what is clear is that both ethers and hardhat-gas-reporter report wrong values apparently because on mainnet the same TX consumes much more gas.

And I believe that my colleague really did find these disrepancies in testing because we went with the higher gas values he found during testing using the manual gasUsed method and they turn out to be close to mainnet gasUsed. Luckily we hardcoded those ones into our contract before we deployed.

However, I am still confused why now both the tool and the manual method report the same apparently too-low gasUsed value.

We run this stuff on a hardhat-network fork of mainnet.

[gitpusha]

Actually new finding:

I added some hardhat console.log to the solidity file to see what that has to say about gasUsed

Clearly hardhat network console.logs also think there is more gas consumed than both ethers.js and hardhat-gas-reporter.

Is there ever a scenario where hardhat-gas-reporter's values could diverge from vanilla ethers.js values? No right? They are the same values read from the same thing basically right?

If you want to you can run the test yourself:

Here is the repo.

Here is the test to run with:

yarn test:gas test/gas/debt_bridge/full/from_maker/1_ETHA-ETHB-WITH-Vault-Creation.mock.test.js

If you wanna look at where the TX is sent, you can find that here.

And you can look at the contract we call here.

[cgewecke]

Thanks for this!

Clearly hardhat network console.logs also think there is more gas consumed than both ethers.js and hardhat-gas-reporter.

Could you give a quick example of where/what/how you're console.logging in Solidity? (It's possible the logging itself distorts gas values a little at runtime.)

Also, is there a difference in the optimization levels you use to deploy to mainnet vs. local testing?

Is there ever a scenario where hardhat-gas-reporter's values could diverge from vanilla ethers.js values? No right? They are the same values read from the same thing basically right?

Yes they are supposed to be, there could be a bug somewhere though.

[gitpusha]

Here in the contract I shared you can see the console.logs.

    function execViaRoute0AndOpenVault(TaskReceipt memory _taskReceipt) public {
        uint256 gasLeft = gasleft();
        IGelatoCore(gelatoCore).exec(_taskReceipt);
        console.log(
            "Gas Cost execViaRoute0AndOpenVault: %s",
            gasLeft - gasleft()
        );
    }

And here is the TX from mainnet which is almost identical to the execViaRoute0AndOpenVault test transaction:
https://etherscan.io/tx/0xdc810ac1cdd34a613ac739eea49807f62c17ba31ec9b1c5516837c07c35c1ab6

As you can see gasUsed is 2,215,931 here which is closer to the console.log value.

[gitpusha]

This got solved with the new hardhat release! https://github.com/nomiclabs/hardhat/releases/tag/hardhat-core-v2.0.4
NomicFoundation/hardhat#990
NomicFoundation/hardhat#1043~~

Yeah!!

Closing!

[gitpusha]

Reopening because I still observe about 10% higher gasUsed values on mainnet when comparing with hardhat network in forking mode.

[cgewecke]

@gitpusha The remoteContracts option is now published with 1.0.2. And I suspect the discrepancy in gas readings is only fixable at hardhat.

Going to close this for now. Thanks for reporting all this stuff though.

[gitpusha]

Ok !