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: support eip2935 blockhash within 256 blocks of genesis #360

Conversation

gballet
Copy link
Owner

@gballet gballet commented Feb 5, 2024

There was the issue that the old behavior described in the EIP would not work for testnets that activate eip 2935 at genesis, which meant that stateless clients would not be able to execute a BLOCKHASH instruction within 256 blocks of genesis.

I have pushed an update to this EIP and now this PR updates the code to this spec. This should actually not change the old behavior, only now it's semi-official.

// For testnets that start with eip2935 activated at genesis time,
// activate this code at block 1.
if evm.chainRules.IsPrague {
if getBlockHashFromContract(max(bnum, 256)-256, evm.StateDB, evm.Accesses) != (common.Hash{}) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

instead of reading the block hash at a "wrap around" negative number, it is now assumed that if eip2935 was activated at genesis time, then we can read from the contract no matter how many blocks away from the genesis we are.

Copy link
Owner Author

Choose a reason for hiding this comment

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

this is a bit weird, but it's just meant to be the same thing as max(block.number-256, 0)

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMHO, I'd prefer to change this to max(block.number-256,0) in the code, to not have to mentally deal with this difference comparing with the EIP.

@gballet gballet requested a review from jsign February 5, 2024 12:43
Copy link
Collaborator

@jsign jsign left a comment

Choose a reason for hiding this comment

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

LGTM

@gballet
Copy link
Owner Author

gballet commented Feb 5, 2024

We're changing the eip again. Closing.

@gballet gballet closed this Feb 5, 2024
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