Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contracts can be unintenionally mocked when snapshots are used #178

Open
fvictorio opened this issue Jan 2, 2023 · 4 comments
Open

Contracts can be unintenionally mocked when snapshots are used #178

fvictorio opened this issue Jan 2, 2023 · 4 comments

Comments

@fvictorio
Copy link
Collaborator

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:

  • Just drop all the fakes/mocks when hardhat_revert is called. We emit an event in the provider when this happens, so you should be able to listen for it.
  • If you think the user should handle this, then add something like Allow mock/fake.resetAll() #129 would help, but it should be a global function instead that just clears everything.
@wei3erHase
Copy link
Member

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
Copy link
Collaborator Author

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
Copy link
Member

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
Copy link
Collaborator Author

fvictorio commented Jan 2, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants