fix(executor): wait for chain head advance between setup and test steps - #154
Closed
qu0b wants to merge 1 commit into
Closed
fix(executor): wait for chain head advance between setup and test steps#154qu0b wants to merge 1 commit into
qu0b wants to merge 1 commit into
Conversation
When running multi-block EEST fixtures, the setup step deploys contracts via engine_newPayload + forkchoiceUpdated, and the test step immediately sends the benchmark block. Without waiting for the node to commit the setup block as canonical head, the test block's engine_newPayload returns SYNCING because the parent block isn't recognized yet. This adds a waitForChainHeadAdvance poll between setup and test steps that checks eth_blockNumber until it advances beyond the pre-setup block. Polls every 200ms with a 30s timeout. Falls back to a 500ms sleep if no baseline block info is available. This is needed for any EEST fixture with setup+execution blocks, such as the BAL scenario benchmarks (parallel execution, state root computation, deploy-then-interact, mixed dependency graphs). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
Author
|
Closing — the multi-block SYNCING issue was caused by genesis hash mismatch between EELS and geth, not by missing chain head synchronization. With the genesis fixes in jochem-brouwer/execution-specs#3 and fselmo/go-ethereum#1, all 44 BAL scenario tests (including multi-block deploy_then_interact) pass on upstream benchmarkoor without this change. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When running multi-block EEST fixtures (e.g., BAL scenario benchmarks), the executor runs setup and test steps sequentially. The setup step deploys contracts via
engine_newPayload+engine_forkchoiceUpdated, and the test step immediately sends the benchmark block.Problem: The test block's
engine_newPayloadreturnsSYNCINGbecause the node hasn't committed the setup block as the canonical head yet. Theengine_forkchoiceUpdatedfrom the setup step returnsVALID, but the internal state update is asynchronous — the chain head hasn't actually advanced when the nextengine_newPayloadarrives milliseconds later.Fix: After the setup step completes successfully and before the test step begins, poll
eth_blockNumberuntil the chain head advances beyond the pre-setup block number. This ensures the node has fully committed the setup block before receiving the test block.retry_new_payloads_syncing_statemechanism can still recover)Context
This is needed for EEST fixtures with setup+execution blocks, such as the BAL scenario benchmarks which test:
These tests deploy contracts in a setup block and benchmark calls to them in the execution block. Without this fix, all multi-block tests fail with
SYNCING.Test plan
🤖 Generated with Claude Code