close
Skip to content

Fix invalid block-root membership check in fork-choice helper#5259

Merged
brech1 merged 3 commits into
ethereum:masterfrom
lmorett1:fix-fork-choice-invalid-root-membership-check
May 27, 2026
Merged

Fix invalid block-root membership check in fork-choice helper#5259
brech1 merged 3 commits into
ethereum:masterfrom
lmorett1:fix-fork-choice-invalid-root-membership-check

Conversation

@lmorett1

Copy link
Copy Markdown
Contributor

Description

This PR fixes a correctness gap in fork-choice compliance test validation by ensuring invalid block events are checked against the fork-choice store’s block-root index. This is necessary because the previous check compared a block root to stored block objects, which could silently pass even when the invalid-block path was not being validated as intended; as a result, store-pollution regressions in invalid scenarios could be missed and test-vector reliability reduced.

Checklist

  • Update documentation (if applicable)
  • Add tests for new functionality (if applicable)
  • Run make lint to check formatting
  • Run make test to check tests

Relations

Related to #3831

@github-actions github-actions Bot added the testing CI, actions, tests, testing infra label May 17, 2026

@jtraglia jtraglia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! This appears to be a valid fix. @ericsson49 can you confirm?

@ericsson49 ericsson49 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch!

@ericsson49

Copy link
Copy Markdown
Contributor

Thanks! This appears to be a valid fix. @ericsson49 can you confirm?

yes, nice catch!

@brech1 brech1 merged commit 9ea1722 into ethereum:master May 27, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing CI, actions, tests, testing infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants