Skip to content

Fix: Ensure Isthmus withdrawals root validation for non-canonical parent state in Optimism engine#20016

Closed
petrokamin10 wants to merge 6 commits intoparadigmxyz:mainfrom
petrokamin10:patch-1
Closed

Fix: Ensure Isthmus withdrawals root validation for non-canonical parent state in Optimism engine#20016
petrokamin10 wants to merge 6 commits intoparadigmxyz:mainfrom
petrokamin10:patch-1

Conversation

@petrokamin10
Copy link

Previously, when state_by_block_hash failed, the validator returned Ok(()), effectively skipping withdrawals root verification for blocks whose parent was not yet in the canonical chain.
This created a window where blocks could bypass a critical consensus check during reorgs or while processing non-canonical branches.

  • Fix OpEngineValidator::validate_block_post_execution_with_hashed_state to always validate the Isthmus withdrawals root, even when the parent block state is not yet canonical.
  • Resolve state lookups by first trying historical state and then pending state; if neither is available, return a consensus error instead of silently accepting the block.

@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Nov 27, 2025
@petrokamin10 petrokamin10 marked this pull request as ready for review November 27, 2025 16:47
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this is a bit more tricky, and the current design for this is a bit of a footgun

I guess we'd need to extend this with like parent_state: BlockState to alway provide access to any parent state

Comment on lines -131 to -133
// FIXME: we don't necessarily have access to the parent block here because the
// parent block isn't necessarily part of the canonical chain yet. Instead this
// function should receive the list of in memory blocks as input
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is a bit more tricky than that because technically, the block's parent isn't necessarily part of the canonical chain yet
in which case we'd fail to resolve it via the provider which only gives access to the canonical chain + pending block

so this change doesnt fully cover all scenarios, but in almost all cases we ca n assume that this is the case

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Dec 25, 2025
@github-actions github-actions bot removed the S-stale This issue/PR is stale and will close with no further activity label Jan 17, 2026
@petrokamin10 petrokamin10 requested a review from mattsse January 18, 2026 13:16
@mattsse
Copy link
Collaborator

mattsse commented Feb 6, 2026

Closing this PR as op-reth has been moved to its own repository. See #21532 and the updated README for the new location. If this is still relevant, please re-open in the new op-reth repository. 🙏

@mattsse mattsse closed this Feb 6, 2026
@github-project-automation github-project-automation bot moved this from Backlog to Done in Reth Tracker Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants