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

feat(config): set default evm version to cancun #9131

Merged
merged 7 commits into from
Nov 18, 2024

Conversation

grandizzy
Copy link
Collaborator

@grandizzy grandizzy commented Oct 16, 2024

Motivation

Closes #8614
Closes #7014 : with updated version we can proper normalize versions post paris
Related tests

// See <https://github.com/foundry-rs/foundry/issues/7014>
let output = cmd
.forge_fuse()
.args(["config", "--use", "0.8.17", "--json"])
.assert_success()
.get_output()
.stdout_lossy();
let config: Config = serde_json::from_str(&output).unwrap();
assert_eq!(config.evm_version, EvmVersion::London);

Closes #5513 : with updated cancun version, warm coinbase(EIP-3651) is set by default

Solution

  • moved cancun test profile as default. created a paris profile for specific tests
  • updated gas and bytecode related test. executed existing gas snapshot related tests with paris to preserve values
  • updated forked tests that are not specific to hardforks to use newer blocks/txes
  • moved beforeTest with selfdestruct from repros to core and executed with Paris
  • changed testEtch to use different address than precompile

@zerosnacks
Copy link
Member

zerosnacks commented Oct 17, 2024

Related: #8614

@grandizzy grandizzy force-pushed the issue-7014 branch 8 times, most recently from ab270ce to cdda83c Compare November 7, 2024 09:29
@grandizzy grandizzy changed the title fix(forge config): set default evm version feat(forge): set default evm version to cancun Nov 7, 2024
@grandizzy grandizzy changed the title feat(forge): set default evm version to cancun feat(config): set default evm version to cancun Nov 7, 2024
@grandizzy grandizzy marked this pull request as ready for review November 7, 2024 10:14
Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

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

Supportive of moving to Cancun as default now, some minor notes / questions in regards to changes

crates/forge/tests/cli/script.rs Outdated Show resolved Hide resolved
crates/forge/tests/cli/script.rs Outdated Show resolved Hide resolved
crates/forge/tests/it/fuzz.rs Show resolved Hide resolved
crates/forge/tests/it/spec.rs Outdated Show resolved Hide resolved
testdata/default/cheats/Etch.t.sol Show resolved Hide resolved
Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

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

Lgtm 👍

@grandizzy
Copy link
Collaborator Author

I think going forward the test structure and such updates could be simplified a lot with #8564 that is allowing inline evm config version.

@zerosnacks
Copy link
Member

pending review cc @yash-atreya / @klkvr

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
3 participants