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

fix: compute stored test address after setting the tx nonce on scripts #727

Merged
merged 3 commits into from
Nov 19, 2024

Conversation

elfedy
Copy link
Contributor

@elfedy elfedy commented Nov 14, 2024

What 💻

Compute and store test address after (optionally) updating the deployer nonce on scripts.

Why ✋

Closes #720
On instances where evm_opts.sender is the same as the CALLER constant (which is the default value, happening for example when no raw private key is sent when invoking script), the nonce of the deployer of the script is modified to prevent nonce mismatches between simulation and execution. In that case we need to compute the test address with that modified nonce, so that we correctly forward calls/creates involving it to the EVM.

Evidence 📷

Added test fails before the change and passes after. Also running scripts with --account --password now work as intended (previously, setUp and run were invoked on the zkEVM causing undefined behavior).

@elfedy elfedy requested a review from a team as a code owner November 14, 2024 18:13
crates/forge/tests/it/zk/script.rs Outdated Show resolved Hide resolved
crates/script/src/runner.rs Show resolved Hide resolved
Copy link
Contributor

@Karrq Karrq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbh I think #731 might make this unnecessary since we would never end up attempting to create the test/script contract in EraVM anymore, but nonetheless it's a fix for our existing issue

@elfedy elfedy merged commit dc26b62 into main Nov 19, 2024
12 checks passed
@elfedy elfedy deleted the elfedy-fix-wallet-script branch November 19, 2024 12:20
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

Successfully merging this pull request may close these issues.

call may fail or behave unexpectedly due to empty code target
4 participants