defi-wonderland / smock

The Solidity mocking library
MIT License
321 stars 39 forks source link

Contracts can be unintenionally mocked when snapshots are used #178

Open fvictorio opened 1 year ago

fvictorio commented 1 year ago

Describe the bug If you use snapshots and smock, a non-mocked contract in one test can accidentally get the mocked behavior from a previous test.

Reproduction steps

Consider this contract:

contract Foo {
  function f() public view returns (uint) {
    return 42;
  }
}

and these tests:

it("test 1", async function () {
  const snapshotId = await network.provider.send("evm_snapshot")
  const Foo = await smock.mock("Foo");
  const foo = await Foo.deploy();

  console.log("Address:", foo.address);

  foo.f.returns(1);

  console.log(await foo.f());

  await network.provider.send("evm_revert", [snapshotId])
});

it("test 2", async function () {
  const Foo = await smock.mock("Foo");
  const foo = await Foo.deploy();

  console.log("Address:", foo.address);

  console.log(await foo.f());
});

If you run it, the second test will log 1 instead of the expected 42.

Expected behavior Newly deployed contracts don't have their functions mocked, even if their address match the address of a previously deployed and mocked contract.

Additional context See this issue in Hardhat's repo.

Possible solutions I'm not sure what's the right solution here, but some ideas:

wei3erHase commented 1 year ago

Thanks @fvictorio for sharing the bug details! This is definitely not a fix, but maybe a workaround, can u confirm if the error persists if some hardhat contract is deployed before the snapshot?

fvictorio commented 1 year ago

can u confirm if the error persists if some hardhat contract is deployed before the snapshot

What do you mean? Before the first snapshot? Or before the first revert?

wei3erHase commented 1 year ago

before the first snapshot, i suffered this issue and got it solved by deploying literally contract DumbContract{} before making any snapshot and it seemed to work ok, i'm just curious why, maybe you'll be more able to detect it.

fvictorio commented 1 year ago

Same behavior if I do this:

  await ethers.deployContract("Dummy");
  const snapshotId = await network.provider.send("evm_snapshot")

If I do this instead, it works:

  const snapshotId = await network.provider.send("evm_snapshot")
  await ethers.deployContract("Dummy");

But that's because this changes the nonce of the deployer, and so both Foo contracts have different addresses.