refactor: implement BeaconEngine#9550
Draft
twoeths wants to merge 24 commits into
Draft
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces the BeaconEngine class to centralize consensus validation and block production logic, separating it from the facade BeaconChain. It also refactors the fork choice into read-only (IForkChoiceRead) and full (IForkChoice) interfaces. The review feedback highlights a critical issue where simplifying the executionPayloadEnabled check bypasses the Terminal Total Difficulty (TTD) transition check, which could cause failures during the Merge transition. Additionally, the reviewer suggests optimizing performance on the API hot path by passing raw request body bytes directly to handlers to avoid expensive serialization overhead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+151
to
+162
| // TODO - beacon engine: this caused failed spec tests? | ||
| // the BeaconEngine cannot provide a BeaconState | ||
| // const executionPayloadEnabled = | ||
| // isStatePostBellatrix(preState0) && | ||
| // preState0.isExecutionStateType && | ||
| // isExecutionBlockBodyType(block.message.body) && | ||
| // preState0.isExecutionEnabled(block.message) | ||
| // ? block.message.body.executionPayload | ||
| // : null; | ||
| const executionPayloadEnabled = isExecutionBlockBodyType(block.message.body) | ||
| ? block.message.body.executionPayload | ||
| : null; |
Contributor
There was a problem hiding this comment.
Commenting out the isExecutionEnabled check and simplifying executionPayloadEnabled bypasses the Terminal Total Difficulty (TTD) transition check. This will cause failures during the Merge transition period (where Bellatrix is active but TTD is not yet reached) and in historical sync/spec tests (as noted in the TODO).
To resolve this without violating the BeaconEngine boundaries, you can retrieve preState0 in verifyBlocksInEpoch (which is a cheap cache hit) and pass it (or a boolean flag indicating if execution is enabled) to verifyBlocksExecutionPayload.
| return; | ||
| } | ||
| // TODO - engine: get block bytes from upstream | ||
| const blockBytes = config.getForkTypes(slot).SignedBeaconBlock.serialize(signedBlock); |
Contributor
There was a problem hiding this comment.
To avoid the expensive serialization overhead on the API hot path (e.g., SignedBeaconBlock.serialize), consider updating the API router/framework to pass the raw request body bytes directly to these handlers instead of re-serializing the parsed JSON objects.
| try { | ||
| await validateGossipProposerPreferences(chain, signed); | ||
| // TODO - beacon engine: get bytes from the api | ||
| const preferencesBytes = ssz.gloas.SignedProposerPreferences.serialize(signed); |
Contributor
There was a problem hiding this comment.
To avoid the expensive serialization overhead on the API hot path (e.g., SignedProposerPreferences.serialize), consider updating the API router/framework to pass the raw request body bytes directly to these handlers instead of re-serializing the parsed JSON objects.
Contributor
Performance Report✔️ no performance regression detected Full benchmark results
|
twoeths
force-pushed
the
te/beacon_engine
branch
from
6ed95ab to
452cc42
Compare
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
3 participants
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.
Motivation
Description
AI Assistance Disclosure