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

Change header encoding mechanism #300

Merged
merged 4 commits into from
Dec 20, 2024

Conversation

piersy
Copy link

@piersy piersy commented Dec 17, 2024

This changes the way that we detect pre-gingerbread headers when encoding.

Closes https://github.com/celo-org/celo-blockchain-planning/issues/588

Previously we would interpret a gasLimit of zero to indicate pre-gingerbread headers, since prior to the gingerbread hardfork celo blocks lacked the gasLimit field.

This however caused lots of problems with the go tests (existing and ones that continue to be added) that routinely use blocks where gasLimit has not been set.

So now we detect pre-gingerbread blocks when decoding and set a flag on the struct that is used to select the encoding.
New blocks are assumed to be post gingerbread, which is ok because it's not possible to run op-geth without gingerbread set, because we haven't ported across the pre-gingerbread state transition logic.

I was able to revert some test modifications because of this but this didn't fix the tests that broke during the celo11 rebase (skipped in this commit), it seems like those problems were caused by something else.

Remove changes to do with determining whether a decoded block was
pre-gingerbread based on it's gas limit.
Previously we looked for a gas limit of zero, now we use an explicit
unexported field set at creation time.
@piersy piersy requested a review from alecps December 17, 2024 20:56
@piersy piersy requested a review from karlb December 18, 2024 08:55
Copy link

@karlb karlb left a comment

Choose a reason for hiding this comment

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

I clearly prefer this to the previous approach!

@piersy piersy merged commit 57eef86 into celo11 Dec 20, 2024
7 checks passed
@piersy piersy deleted the piersy/change-header-encoding-mechanism branch December 20, 2024 10:09
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.

3 participants