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

bug: tests do not deploy via create2 the same as scripts #5529

Closed
2 tasks done
mds1 opened this issue Aug 2, 2023 · 3 comments · Fixed by #6656
Closed
2 tasks done

bug: tests do not deploy via create2 the same as scripts #5529

mds1 opened this issue Aug 2, 2023 · 3 comments · Fixed by #6656
Assignees
Labels
T-bug Type: bug
Milestone

Comments

@mds1
Copy link
Collaborator

mds1 commented Aug 2, 2023

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (ca67d15 2023-08-02T00:22:03.700607000Z)

What command(s) is the bug in?

forge test

Operating System

None

Describe the bug

  1. forge init a new repo
  2. Add the below test
function test1() public {
    address a = computeCreate2Address(0, hashInitCode(type(Counter).creationCode), address(this));
    address b = address(new Counter{salt: 0}());
    require(a == b, "create2 address mismatch");
}
  1. Run forge test --mt test1 -vvvv, the test passes and deploys to new Counter@0xBE03C2aFB559A5Ce6926b01A813F37584476c95f
  2. Update the default script to run:
function run() public {
    vm.broadcast();
    new Counter{salt: 0}();
}
  1. Run forge script script/Counter.s.sol -vvvv and you'll see it deploys to new Counter@0xE414be17fD7aa8Ee7194000dc622AdEF3c81aFCd

This shows that scripts (as expected) route create2 deploys through the default create2 deployer. But tests (unexpectedly) deploy directly from the test contract.

I think the two behaviors should be consistent, especially since it's ideal to test scripts, and this behavior means we get different addresses depending on which command you invoke the script with (directly, or indirectly trough a test) which is confusing behavior

cc @zobront

@mds1 mds1 added the T-bug Type: bug label Aug 2, 2023
@zobront
Copy link

zobront commented Aug 2, 2023

For a little context, the reason this came up is because I use deployment scripts as a part of test setup.

But the deployments via CREATE2 were ending up at different addresses between the two, which was causing certain tests to pass that shouldn't have.

@Evalir Evalir self-assigned this Aug 7, 2023
@Evalir Evalir added this to the v1.0.0 milestone Aug 7, 2023
@reednaa
Copy link

reednaa commented Oct 18, 2023

For anyone looking for a workaround, by adding vm.broadcast (or vm.startBroadcast) before deploying the contract, it will be deployed with the CREATE2 deployer.

@mds1
Copy link
Collaborator Author

mds1 commented Oct 18, 2023

@Evalir Is this one an easy fix? Using deploy scripts for test setups is recommended in the foundry book best practices page, but this issue adds a hurdle/source of confusion to doing that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
No open projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

4 participants