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

Incosistent Gas Report Per Test. Caching?

Open simondlr opened this issue 2 years ago • 4 comments

Hey. Trying to figure out why gas-report is showing inconsistent gas reporting per unit test. The averages box seems correct, but the gas reported per test seems off/wrong?

I'm using a hardhat node. 2.6.1 And secondly, using gas-reporter 1.0.4.

Example of quirk. This test uses 17m gas because it's deploying two contracts + doing some individual transactions. ✓ S: test claim by transferring OG id to another account after claimed. (17735020 gas)

If you copy the test, the second test shows a much smaller gas usage number.

✓ S: test claim by transferring OG id to another account after claimed. (17735008 gas) ✓ S: test claim by transferring OG id to another account after claimed. (24608 gas)

Here's the test. It mints an NFT and then allows an original holder to claim an NFT of a new project.

  it("S: test claim by transferring OG id to another account after claimed.", async () => {
    const s2 = await S.deploy("Souls", "SOULS3", accounts[2], '100', '3541431094', '0xaF69610ea9ddc95883f97a6a3171d52165b69B03'); // wide campaign window for tests. dates tested separately
    await s2.deployed();
    const tx = await s2.connect(signers[1]).mintSoul({value: ethers.utils.parseEther(dxPrice), gasLimit});
    const receipt = await tx.wait();
    const tokenId = receipt.events[0].args.tokenId.toString();

    const s3 = await S.deploy("Souls", "SOULS2", accounts[2], '100', '3541431094', s2.address); // wide campaign window for tests. dates tested separately
    await s3.deployed();

    await s3.connect(signers[1]).claimSoul(tokenId, {gasLimit});
    await s2.connect(signers[1]).transferFrom(accounts[1], accounts[2], tokenId, {gasLimit});

    await expect(s3.connect(signers[2]).claimSoul(tokenId, {gasLimit})).to.be.revertedWith("AC_ID ALREADY CLAIMED");
  });

The only indication I can gather here is that because I'm reverting to a clean snapshot in between each test, that the contracts that end up being deployed have the same addresses.

 this.beforeEach(async function() {
    await provider.send('evm_revert', [snapshot]);
    snapshot = await provider.send('evm_snapshot', []);
  });

So, maybe this revert process is tripping up gas reporting? Is it the hardhat node doing optimizations automatically (no idea what it could be), or is the gas reporter? It's not urgent since I'm more interested in the average numbers of function calls vs gas per test, which seems to be reporting appropriately.

Any thoughts?

simondlr avatar Sep 11 '21 17:09 simondlr

@simondlr I think something is cacheing but if you're deploying within the it block, snapshot/revert shouldn't interfere with the gas spent.

Are you using hardhat_reset to do that? Or evm_snapshot, etc?

Are you using any other plugins like hardhat-deploy? If so I think hh-deploy is smart about using deployed contracts when possible ... could be happening there.

cgewecke avatar Sep 13 '21 12:09 cgewecke

Hey @cgewecke!

using evm_snapshot & custom deploy.

snapshot/revert shouldn't interfere with the gas spent.

I believe that the revert is causing all kinds of problems. I did some more tests.

beforeAll is only used to create a snapshot. In the following report, the snapshot is not saved. And then, if you are not doing revert in beforeEach, then all tests produce the same gas report.

✓ S: test claim by transferring OG id to another account after claimed. (18016259 gas) ✓ S: test claim by transferring OG id to another account after claimed. (18016259 gas) ✓ S: test claim by transferring OG id to another account after claimed. (18016259 gas)

But. If you add in a snapshot in beforeAll.

  this.beforeAll(async function() {
    provider = new ethers.providers.Web3Provider(web3.currentProvider);
    signers = await ethers.getSigners();
    accounts = await Promise.all(signers.map(async function(signer) {return await signer.getAddress(); }));
    S = await ethers.getContractFactory("Souls");
    snapshot = await provider.send('evm_snapshot', []);
  });

and then add the revert in beforeEach back in again, it throws the wonky gas reports.

✓ S: test claim by transferring OG id to another account after claimed. (17886795 gas) ✓ S: test claim by transferring OG id to another account after claimed. (129464 gas) ✓ S: test claim by transferring OG id to another account after claimed. (129464 gas)

Not sure where is the best next step to do more debugging. It doesn't make sense why the first test would be different here either.

simondlr avatar Sep 13 '21 14:09 simondlr

Ah ok, thanks. Will take a look and see if I can reproduce.

cgewecke avatar Sep 13 '21 16:09 cgewecke

There might be a simpler test here that replicates the issues, but it's not a big problem atm for me, so not prioritisting it. Let me know however if you need more samples or configurations. :)

simondlr avatar Sep 13 '21 17:09 simondlr