Skip to content

Conversation

@Maddiaa0
Copy link
Member

@Maddiaa0 Maddiaa0 commented Jul 8, 2023

Updates huffmate to support shanghai (mainly push0)

This will fix master after the 0.3.2 release

@Maddiaa0 Maddiaa0 force-pushed the md/target-shanghai branch 2 times, most recently from 9d32622 to d02a50b Compare July 8, 2023 20:47
@Maddiaa0 Maddiaa0 marked this pull request as draft July 8, 2023 22:01
@Maddiaa0 Maddiaa0 force-pushed the md/target-shanghai branch from a95cabd to 38c7499 Compare July 10, 2023 20:56
@Maddiaa0
Copy link
Member Author

Note: only asserting these two tests use shanghai as they are the only ones which have distinct bytecode offset markers in the tests.

@Maddiaa0 Maddiaa0 marked this pull request as ready for review July 10, 2023 20:59
@Maddiaa0 Maddiaa0 requested a review from MathisGD July 10, 2023 21:00
out = 'out'
libs = ['lib']
ffi = true
evm_version = "shanghai"
Copy link
Contributor

@obatirou obatirou Jul 12, 2023

Choose a reason for hiding this comment

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

Tests are still running in 0.8.15, wouldn't this need to be 0.8.19 as it is minimum supported solidity version for shanghai ?

Link to CI
Capture d’écran 2023-07-12 à 11 05 21

Some tests files have fixed pragma set to 0.8.15 and should be easily fixable.
One problem is that huffmate depends on solmate and more precisely do an import:
import { ERC1155Recipient } from "solmate/test/ERC1155.t.sol";
which also have a fixed pragma.

Copy link
Collaborator

Choose a reason for hiding this comment

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

so we need a quick PR in Solmate (I don't think that this is so much work)

Copy link
Contributor

Choose a reason for hiding this comment

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

Here is the PR for solmate transmissions11/solmate#382


// Deploy the Mintable ERC20
address mintableTokenAddress = HuffDeployer.config()
.with_evm_version("shanghai")
Copy link
Collaborator

Choose a reason for hiding this comment

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

it seems to be the default (src). You want to do this to avoid future issues ?


// Copy the runtime bytecode with constructor argument concatenated.
0x67 // [offset] - constructor code size
0x66 // [offset] - constructor code size
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
0x66 // [offset] - constructor code size
__codesize(CONSTRUCTOR) // [offset] - constructor code size

still not perfect but better than hardcoded

out = 'out'
libs = ['lib']
ffi = true
evm_version = "shanghai"
Copy link
Collaborator

Choose a reason for hiding this comment

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

so we need a quick PR in Solmate (I don't think that this is so much work)

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.

4 participants