Skip to content

Fix test_builder_payment_after_missed_epochs - #5120

Merged
jtraglia merged 3 commits into
ethereum:masterfrom
jtraglia:fix-missed-epochs-tests
Apr 17, 2026
Merged

Fix test_builder_payment_after_missed_epochs#5120
jtraglia merged 3 commits into
ethereum:masterfrom
jtraglia:fix-missed-epochs-tests

Conversation

@jtraglia

Copy link
Copy Markdown
Member

In #5094, I made a mistake with this test. I mutated the state after yielding it. Not supposed to do that 😅

Thank you @michaelsproul for reporting!

For context:

image

@github-actions github-actions Bot added the testing CI, actions, tests, testing infra label Apr 17, 2026
# the same state transition. This makes block_1 build on a full parent; by
# also aligning bid_1.block_hash with latest_execution_payload_bid.block_hash
# below, block_2 will also see its parent (block_1) as full.
set_parent_block_full(spec, state)

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.

I'm not super familiar with the Python test generators, but isn't this function kind of a hack? Shouldn't it be possible to construct the test state without manually editing the state.latest_block_hash?

E.g. could we construct block_1 with bid.parent_block_hash == state.latest_execution_payload_bid.block_hash, which would trigger apply_parent_execution_payload when block_1 is applied, thus making the block prior to block_1 full?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes that seems possible & less hacky. I'll work on this tomorrow.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just pushed 2a6425e. It's slightly less hacky, but this is still pretty hacky:

bid.parent_block_hash = state.latest_execution_payload_bid.block_hash
bid.block_hash = state.latest_execution_payload_bid.block_hash

Without more complex changes to the testing framework, this is necessary right now 🫤

@jtraglia

Copy link
Copy Markdown
Member Author

Okay I've confirmed that without the last else section in the spec, this test will fail. Tested in Sproul's branch with the latest version of this reference test. As-is the test will pass, when I comment out the else block, it fails.

image

@jtraglia

Copy link
Copy Markdown
Member Author

Going to merge now so we can generate a full suite of reference tests. We can make this less hacky in the future.

@jtraglia
jtraglia merged commit 6df1220 into ethereum:master Apr 17, 2026
15 checks passed
@jtraglia
jtraglia deleted the fix-missed-epochs-tests branch April 18, 2026 14:12
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.

2 participants