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: add ability to set evm version #41

Closed
wants to merge 2 commits into from
Closed

Conversation

Maddiaa0
Copy link
Member

Allow developers to pick which evm version they deploy to.

depends on: huff-language/huff-rs#281

HuffConfig config = HuffDeployer.config().with_evm_version("paris");
address withParis = config.deploy("test/contracts/EVMVersionCheck");

bytes memory parisBytecode = getBytecode(withParis);
Copy link
Contributor

Choose a reason for hiding this comment

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

You could use directly withParis.code and delete getBytecode function if I am not mistaken.

/// @dev test that compilation is different with new evm versions
function testSettingEVMVersion() public {
/// expected bytecode for EVM version "paris"
bytes memory expectedParis = hex"60028060093d393df36000";
Copy link
Contributor

@obatirou obatirou Jul 10, 2023

Choose a reason for hiding this comment

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

Wouldn't this should be the following for EVMVersionCheck ?
The test check the bytecode at the deployed address

bytes memory expectedParis = hex"6000";
...
bytes memory expectedShanghai = hex"5f";
...
bytes memory defaultBytecode = hex"5f";

@obatirou
Copy link
Contributor

obatirou commented Jul 10, 2023

It seems there are 2 distincts problems in tests:

  • old tests that are not passing (also failing on main): here is a commit with message with what I understood from the problem and a proposed solution that passes in local.
  • huff contracts that were deployed with Paris that do not get deployed with Shanghai.

For the second, point I did not found yet how to resolve this

Update: here is a branch with all tests fixed (in local)

@Maddiaa0
Copy link
Member Author

dropped in favour of obas pr

@Maddiaa0 Maddiaa0 closed this Jul 10, 2023
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.

2 participants