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

Initial address incompatibility #249

Closed
exp-table opened this issue Dec 17, 2021 · 14 comments · Fixed by #446
Closed

Initial address incompatibility #249

exp-table opened this issue Dec 17, 2021 · 14 comments · Fixed by #446
Labels
T-bug Type: bug

Comments

@exp-table
Copy link

I noticed that the sequence of addresses assigned to contracts deployed in any test is not identical to the one produced by dapptools.

In my case, it breaks a particular case where I generate a merkle tree with the addresses of custom User contracts. Their respectives proofs are hardcoded. Thus, the proofs ingested by some functions are therefore invalid.

@brockelmore and @gakonst suggested that the reasons are probably the initial address and the nonce(s).

@mds1
Copy link
Collaborator

mds1 commented Dec 17, 2021

If I'm understanding this issue correctly, then @gakonst we saw this a while back when you tried to verify foundry against the Drai repo, since there's a hardcoded address validation test here. That test passes on dapptools but failed against foundry, so it might be another useful reference to test against

@gakonst
Copy link
Member

gakonst commented Dec 17, 2021

Yep, same issue @mds1

@gakonst gakonst added the T-bug Type: bug label Dec 17, 2021
@brockelmore
Copy link
Member

does anyone know the dapptools initial address? easiest to just match that as default to maintain max compatibility

@transmissions11
Copy link
Contributor

It's in the hevm README I'll dig it up in a sec

@brockelmore
Copy link
Member

looks like its 0x00a329c0648769a73afac7f9381e08fb43dbea72

@ghost
Copy link

ghost commented Dec 17, 2021

@exp-table Interested to see how you use dapp.tools to test a merkle tree. Could you share the repo?

@transmissions11
Copy link
Contributor

0x00a329c0648769a73afac7f9381e08fb43dbea72

yup https://github.com/dapphub/dapptools/tree/master/src/hevm#environment-variables

@brockelmore
Copy link
Member

brockelmore commented Dec 17, 2021

ah interesting issue here: i think the real issue is the address of the test contract, not the sender. See here. I tried on @exp-table's repo setting the sender variable to the above, and it still failed.

The hard part is that we support multiple test contracts. Which is the right thing to do for parallelizing tests, but means that we can't easily support without some compromises. One option is to not pre-deploy, and just deploy per test, and in run_test do a set_code into the specified address, which is what i imagine hevm does under the hood

@brockelmore
Copy link
Member

Work around for anyone stumbling upon this and needs a short term fix:

contract Test {
    function setUp() public {
        vm.startPrank(<expectedAddress>);
        DeterministicContractAddress contract = new DeterministicContractAddress();
    }
}

use the startPrank cheatcode in your setup function prior to deploying any subsequent contracts. This may be the preferred long term solution

@fmhall
Copy link

fmhall commented Jan 10, 2022

Interestingly, setting DAPP_TEST_CALLER and DAPP_TEST_ADDRESS sets the addresses, but to different ones than I specified

@onbjerg
Copy link
Member

onbjerg commented Jan 12, 2022

Interestingly, setting DAPP_TEST_CALLER and DAPP_TEST_ADDRESS sets the addresses, but to different ones than I specified

DAPP_TEST_CALLER is (mistakenly) not used by Forge, it uses DAPP_TEST_ADDRESS to set the sender (see #253). If you set DAPP_TEST_ADDRESS to 0x00a329c0648769a73afac7f9381e08fb43dbea72 then the first test contract address will match dapptools

@onbjerg
Copy link
Member

onbjerg commented Jan 13, 2022

Hmm, this seems to not have been fixed anyway, which is strange. The sender is correct, but now the test contract address is always incorrect (not even the first one matches)

@gakonst gakonst reopened this Jan 13, 2022
@gakonst
Copy link
Member

gakonst commented Jan 13, 2022

You are right, it is still happening. Unsure / haven't debugged.

@onbjerg
Copy link
Member

onbjerg commented Jan 13, 2022

Figured out the issue, it seems that the dapptools address is from nonce 1 of the 0x00a329c0648769A73afAc7F9381E08FB43dBEA72 account whereas we deploy the contract at nonce 0.

PR #446 fixes it

charisma98 added a commit to charisma98/foundry that referenced this issue Mar 4, 2023
* feat(fuzz): expose function to get internal evm

* refactor(evm): move EvmOpts from cli to evm-adapters

* feat(evm): add helper for creating sputnik backend

* feat(forge): add base evm opts for test usage

* feat(evm): derive default for EvmOpts

* test(forge): add utils for instantiating backend

* feat(forge): instantiate runner with EvmOpts instead of an EVM

This allows us to instantiate as many EVMs as we want inside of the runner,
which in turn will enable running tests in parallel

* feat(forge): pass evm by reference instead of using self.evm

* feat(forge): run unit tests with unique evm instantiation

previously we'd reuse the same EVM, now, we use a different EVM
per test, allowing us to get rid of the mutable reference on self

* feat(forge): run fuzz tests with unique evm instantiations

* test(forge): adjust tests to new instantiation style

* feat(forge): run tests in parallel with rayon

* feat(evm-adapters): put backend behind enum to avoid trait object

* chore(forge): move fuzzer instead of ref

* feat(forge): make multi contract runner compatible with new runner

* feat(forge): parallelize multi contract runner by file

* chore(cli): remove unused helper functions

* fix(cli/run): use new contract runner initialization

There's a TODO here around how we should do the evm.debug_calls check which we should figure out

* fix(cli/test): use evm_opts instead of directly passing evm

* chore: formatting fixes

* chore: update lockfile

* fix(evm-adapters): correctly init test caller and origin

fixes foundry-rs/foundry#249
fixes foundry-rs/foundry#253

* chore: clippy lint on unreachable code w disabled features

* fix: instantiate evm cfg without contract size limit

* fix debugging (#445)

* merge cleanup

Co-authored-by: brockelmore <31553173+brockelmore@users.noreply.github.com>
Co-authored-by: Brock <brock.elmore@gmail.com>
0129general added a commit to 0129general/FoundryProject that referenced this issue May 8, 2024
* feat(fuzz): expose function to get internal evm

* refactor(evm): move EvmOpts from cli to evm-adapters

* feat(evm): add helper for creating sputnik backend

* feat(forge): add base evm opts for test usage

* feat(evm): derive default for EvmOpts

* test(forge): add utils for instantiating backend

* feat(forge): instantiate runner with EvmOpts instead of an EVM

This allows us to instantiate as many EVMs as we want inside of the runner,
which in turn will enable running tests in parallel

* feat(forge): pass evm by reference instead of using self.evm

* feat(forge): run unit tests with unique evm instantiation

previously we'd reuse the same EVM, now, we use a different EVM
per test, allowing us to get rid of the mutable reference on self

* feat(forge): run fuzz tests with unique evm instantiations

* test(forge): adjust tests to new instantiation style

* feat(forge): run tests in parallel with rayon

* feat(evm-adapters): put backend behind enum to avoid trait object

* chore(forge): move fuzzer instead of ref

* feat(forge): make multi contract runner compatible with new runner

* feat(forge): parallelize multi contract runner by file

* chore(cli): remove unused helper functions

* fix(cli/run): use new contract runner initialization

There's a TODO here around how we should do the evm.debug_calls check which we should figure out

* fix(cli/test): use evm_opts instead of directly passing evm

* chore: formatting fixes

* chore: update lockfile

* fix(evm-adapters): correctly init test caller and origin

fixes foundry-rs/foundry#249
fixes foundry-rs/foundry#253

* chore: clippy lint on unreachable code w disabled features

* fix: instantiate evm cfg without contract size limit

* fix debugging (#445)

* merge cleanup

Co-authored-by: brockelmore <31553173+brockelmore@users.noreply.github.com>
Co-authored-by: Brock <brock.elmore@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants