Inconsistent reverts

[kyriediculous]

I have recently upgraded our repository to Hardhat v2.12.4, since upgrading I'm noticing some weird inconsistencies in even simple unit tests.

My current last struggle is a simple approve/transferFrom unit test.

1.A approves 50 tokens to B

2. B tries to transferFrom 100 tokens from A to B
3. Expect revert with message

The test works fine when I run the file separately. However when I run the test with other (entirely isolated) unit tests the transaction will revert state, but not fail.

Excerpt from logging some info before and after the call:

Allowance A -> B 50.0
UserA balance before 100.0
UserB balance before 0.0
Tx status 1
UserA balance after 100.0
UserB balance after 0.0

When I add a hardhat/console statement before the require statement that's supposed to fail it also works.

Reproducable at this commit: https://github.com/Tenderize/tender-core/blob/ec05e2734751288035fc76d763f4af15c1dce9dd/test/unit/tenderToken.test.ts#L303

Reproduces at this commit on two machines.

[fvictorio]

Hi @kyriediculous, I think the underlying problem here is that smock's mocked functions are not reset when a snapshot is reverted.

I reduced the problem to a single pair of tests (code included at the end of this comment). Notice that I added two console.logs, showing that the address of those two tokens is the same.

I think the reason why adding a console.log to the contract fixed the test was actually because it was triggering this bug, which was "cancelling" the mocked function.

I'm not sure what you should do here. Resetting the network seems to clear the mocks, so maybe you can add a hardhat_reset after all the tests in that file are run. It's a shitty solution, but might be enough as a temporary workaround. In the meantime, I will open a follow-up issue in the smock repo about this.

I will close this issue now because I don't think there's much we can do on our side.

it("reverts if transfer fails", async () => {
  let snapshotId = await rpc.snapshot();

  const signers = await ethers.getSigners();

  const TenderizerFactory = await smock.mock<Livepeer__factory>("Livepeer");

  const tenderizerMock = await TenderizerFactory.deploy();

  const TenderTokenFactory = await smock.mock<TenderToken__factory>("TenderToken");

  const LPTokenFactory = await smock.mock<LiquidityPoolToken__factory>("LiquidityPoolToken");

  const TenderFarmFactory = await ethers.getContractFactory("TenderFarm", signers[0]);

  const account0 = await signers[0].getAddress();
  const account1 = await signers[1].getAddress();
  const account2 = await signers[2].getAddress();

  const lpToken = await LPTokenFactory.deploy();
  console.log("lpToken:", lpToken.address);
  const tenderToken = await TenderTokenFactory.deploy();
  await lpToken.deployed();
  await tenderToken.deployed();
  await tenderToken.initialize("Tender Token", "Tender", tenderizerMock.address);
  await lpToken.initialize("LP token", "LP");

  const tenderFarm = await upgrades.deployProxy(TenderFarmFactory, [lpToken.address, tenderToken.address, account0]);
  await tenderFarm.deployed();

  const supply = ethers.utils.parseEther("1000000000000000");
  await lpToken.mint(account0, supply);
  await tenderToken.mint(account0, supply);
  tenderizerMock.totalStakedTokens.returns(supply);
  const TenderFarmFactory = await ethers.getContractFactory("TenderFarm", signers[0]);
  tenderFarm = (await TenderFarmFactory.deploy()) as TenderFarm;
  await tenderFarm.initialize(lpToken.address, tenderToken.address, account0);

  // stake for an account
  lpToken.transferFrom.returns(true);
  const amount = ethers.utils.parseEther("150");
  await lpToken.approve(tenderFarm.address, amount);
  await tenderFarm.farm(amount);

  lpToken.transfer.returns(false);

  await expect(tenderFarm.unfarm(amount)).to.be.revertedWith("TRANSFER_FAIL");

  lpToken.transfer.returns(true);

  await rpc.revert(snapshotId);
});

it("reverts when amount exceeds allowance", async () => {
  const initialAmount = ethers.utils.parseEther("100");
  const transferAmount = ethers.utils.parseEther("50");
  const signers = await ethers.getSigners();
  const TenderizerFactory = await smock.mock<Livepeer__factory>("Livepeer");

  const tenderizerMock = await TenderizerFactory.deploy();
  const TokenFactory = await ethers.getContractFactory("TenderToken", signers[0]);

  const account0 = await signers[0].getAddress();
  const account1 = await signers[1].getAddress();
  const account2 = await signers[2].getAddress();

  const tenderToken = (await TokenFactory.deploy()) as TenderToken;
  await tenderToken.deployed();
  console.log("tenderToken:", tenderToken.address);
  await tenderToken.initialize("Mock", "MCK", tenderizerMock.address);

  await tenderToken.mint(account0, initialAmount);
  tenderizerMock.totalStakedTokens.returns(initialAmount);
  await tenderToken.approve(account1, transferAmount);
  await tenderToken.connect(signers[1]).approve(account2, transferAmount); // account1 has no tokens

  await expect(
    tenderToken.connect(signers[1]).transferFrom(account0, account1, transferAmount.mul(ethers.constants.Two)),
  ).to.be.revertedWith("TRANSFER_AMOUNT_EXCEEDS_ALLOWANCE");
});

[kyriediculous]

THanks @fvictorio !