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

Isthmus: re-introduce withdrawals root in block header #451

Merged
merged 2 commits into from
Dec 16, 2024

Conversation

vdamle
Copy link
Contributor

@vdamle vdamle commented Dec 13, 2024

Description

Restores the changes originally in #383. We had to revert this to de-risk Holocene RC. This PR re-introduces the changes for L2 withdrawals root. An additional change was added to do away passing the ChainConfig directly to NewBlock() and instead pass a BlockType interface that is implement d by ChainConfig. An additional BlockConfig struct implements the interface as well, for usage in tests here and in the monorepo.

Tests

Additional context

Original PR for this: #383
Reverted changes: #449

Metadata

Ref: ethereum-optimism/optimism#12044

@vdamle vdamle requested a review from protolambda December 13, 2024 21:41
@vdamle vdamle requested a review from a team as a code owner December 13, 2024 21:41
Copy link
Collaborator

@protolambda protolambda left a comment

Choose a reason for hiding this comment

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

Looks good to me. Checked the diff against the original PR, and the changes are good.

One issue though: this PR is going to conflict with the v1.14.2 changes. I think we can merge this first, and then fix the conflicts in the v1.14.2 PR.

@protolambda
Copy link
Collaborator

Will confirm with @sebastianst if we can merge this now.

* Adds a `BlockType` interface with a method to check if the header
  has a CustomWithdrawalsRoot, to be used when Isthmus fork is active.
* `ChainConfig` implements the interface and is passed to NewBlock() api.
* `BlockConfig` implements the interface as well and is used for invocation from
   tests.
@vdamle vdamle force-pushed the l2-withdrawals-root-new branch from cf1bdd0 to a55434e Compare December 16, 2024 15:05
@protolambda
Copy link
Collaborator

Merging with a merge-commit, to keep the re-apply commit in there, since it undoes a revert.

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