feat: epbs-devnet-0 merged with unstable#9100
feat: epbs-devnet-0 merged with unstable#9100lodekeeper wants to merge 24 commits intoChainSafe:unstablefrom
Conversation
Squash merge of ChainSafe#8739
Squash merge of ChainSafe#8868
We need the PTC cached for previous epoch in case of epoch boundary lookups, eg. simple case is slot N block validating slot N-1 attestation ~~but also if there are deeper reorgs / missed slots.~~ (this isn't required as ptc messages are only valid for one slot). See ChainSafe#8983 for more details.
## Summary Adds serving-side req/resp support for Gloas `execution_payload_envelopes_by_range/1`, using the existing by-root implementation as reference. Scope is intentionally **serve-only** (no consumer/download path changes). ## What changed - Add new req/resp method: `execution_payload_envelopes_by_range` - Register protocol in req/resp protocol set (Gloas boundary) - Add handler wiring in `handlers/index.ts` - Implement range handler: - finalized range -> `executionPayloadEnvelopeArchive` by slot - non-finalized range -> canonical head-chain roots -> `executionPayloadEnvelope` - enforces `earliestAvailableSlot` gate (same pattern as other by-range handlers) - emits fork boundary by slot epoch - Add request/response SSZ mapping + method enum typing - Add inbound rate-limit entry for by-range - Add req/resp score handling for by-range timeout classification - Add unit tests for serving behavior and earliestAvailableSlot gating ## Validation - `pnpm lint` (repo) - `pnpm vitest run --project unit test/unit/network/reqresp/executionPayloadEnvelopesByRange.test.ts` - `pnpm check-types` (packages/beacon-node) ## Notes - This PR does **not** implement consuming these by-range envelopes yet, per task scope. --------- Co-authored-by: lodekeeper <[email protected]>
This reverts commit ffbb1d1.
# Conflicts: # packages/api/src/beacon/routes/events.ts # packages/api/test/unit/beacon/testData/events.ts # packages/beacon-node/src/api/impl/beacon/blocks/index.ts # packages/beacon-node/src/api/impl/beacon/state/utils.ts # packages/beacon-node/src/api/impl/validator/index.ts # packages/beacon-node/src/chain/archiveStore/archiveStore.ts # packages/beacon-node/src/chain/archiveStore/interface.ts # packages/beacon-node/src/chain/archiveStore/strategies/frequencyStateArchiveStrategy.ts # packages/beacon-node/src/chain/archiveStore/utils/archiveBlocks.ts # packages/beacon-node/src/chain/blocks/blockInput/blockInput.ts # packages/beacon-node/src/chain/blocks/blockInput/types.ts # packages/beacon-node/src/chain/blocks/importBlock.ts # packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts # packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts # packages/beacon-node/src/chain/blocks/verifyBlocksSignatures.ts # packages/beacon-node/src/chain/chain.ts # packages/beacon-node/src/chain/emitter.ts # packages/beacon-node/src/chain/forkChoice/index.ts # packages/beacon-node/src/chain/interface.ts # packages/beacon-node/src/chain/prepareNextSlot.ts # packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts # packages/beacon-node/src/chain/regen/interface.ts # packages/beacon-node/src/chain/regen/queued.ts # packages/beacon-node/src/chain/regen/regen.ts # packages/beacon-node/src/chain/seenCache/index.ts # packages/beacon-node/src/chain/seenCache/seenGossipBlockInput.ts # packages/beacon-node/src/chain/stateCache/fifoBlockStateCache.ts # packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts # packages/beacon-node/src/chain/stateCache/types.ts # packages/beacon-node/src/chain/validation/block.ts # packages/beacon-node/src/chain/validation/dataColumnSidecar.ts # packages/beacon-node/src/chain/validation/executionPayloadEnvelope.ts # packages/beacon-node/src/chain/validation/voluntaryExit.ts # packages/beacon-node/src/network/peers/peerManager.ts # packages/beacon-node/src/network/processor/gossipHandlers.ts # packages/beacon-node/src/network/reqresp/handlers/beaconBlocksByRange.ts # packages/beacon-node/src/network/reqresp/handlers/blobSidecarsByRange.ts # packages/beacon-node/src/network/reqresp/handlers/dataColumnSidecarsByRange.ts # packages/beacon-node/src/sync/utils/downloadByRange.ts # packages/beacon-node/src/sync/utils/downloadByRoot.ts # packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts # packages/beacon-node/test/unit-minimal/chain/stateCache/persistentCheckpointsCache.test.ts # packages/beacon-node/test/unit/chain/archiveStore/blockArchiver.test.ts # packages/beacon-node/test/unit/chain/forkChoice/forkChoice.test.ts # packages/beacon-node/test/unit/chain/seenCache/seenBlockInput.test.ts # packages/beacon-node/test/utils/state.ts # packages/beacon-node/test/utils/validationData/attestation.ts # packages/fork-choice/src/forkChoice/forkChoice.ts # packages/fork-choice/src/forkChoice/interface.ts # packages/fork-choice/src/forkChoice/store.ts # packages/fork-choice/src/index.ts # packages/fork-choice/src/protoArray/interface.ts # packages/fork-choice/src/protoArray/protoArray.ts # packages/fork-choice/test/perf/forkChoice/util.ts # packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts # packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts # packages/fork-choice/test/unit/forkChoice/shouldOverrideForkChoiceUpdate.test.ts # packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts # packages/fork-choice/test/unit/protoArray/getCommonAncestor.test.ts # packages/fork-choice/test/unit/protoArray/gloas.test.ts # packages/fork-choice/test/unit/protoArray/protoArray.test.ts # packages/state-transition/src/block/processVoluntaryExit.ts # packages/state-transition/src/signatureSets/index.ts # packages/state-transition/src/signatureSets/voluntaryExits.ts
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates the foundational changes required for the enshrined Proposer-Builder Separation (ePBS) model, specifically targeting the Gloas fork. It introduces new capabilities for interacting with execution payload envelopes via a dedicated API and event, while fundamentally refactoring core block processing, fork choice, and peer-to-peer synchronization to accommodate the separated execution layer data. The changes ensure the network can correctly validate, propagate, and build upon Gloas blocks where execution payloads are delivered independently. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant changes to integrate ePBS functionality into the Lodestar beacon node. It includes a new API endpoint, updates to event handling, and modifications to core block processing and validation logic to support the new ePBS block structure. The changes also address data availability and signature verification in the context of ePBS. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Resolve 1 conflict in network/processor/index.ts: - PR ChainSafe#9059 refactored awaiting-block mechanism for all gossip types - Keep unstable's refactored structure, add back ePBS execution_payload bypass
There was a problem hiding this comment.
Code Review
This pull request introduces a significant set of changes to implement enshrined Proposer-Builder Separation (ePBS). The modifications span across the API, chain logic, block processing, and networking layers, reflecting a deep and thorough effort to align with the ePBS specifications. The code is generally of high quality, with complex logic often accompanied by explanatory comments. However, I've identified a couple of critical issues in the core ePBS implementation that need to be addressed to ensure correct chain operation.
| const parentPayloadInput = this.seenPayloadEnvelopeInputCache.get(parentRootHex); | ||
| const childWantsFullParent = | ||
| parentPayloadInput !== undefined && childParentBlockHash === parentPayloadInput.getBlockHashHex(); |
There was a problem hiding this comment.
There appears to be a mix-up between the old and new payload input caches. The logic to decide if a child block wants a FULL parent should use the new seenPayloadEnvelopeCache, which is populated with ePBS bid information during gossip validation. The current implementation uses seenPayloadEnvelopeInputCache, which may not contain the necessary ePBS-specific data.
Additionally, the method getBlockHashHex() does not seem to exist on the PayloadEnvelopeInput type stored in the new cache. The correct property to use is likely blockHashFromBid.
| const parentPayloadInput = this.seenPayloadEnvelopeInputCache.get(parentRootHex); | |
| const childWantsFullParent = | |
| parentPayloadInput !== undefined && childParentBlockHash === parentPayloadInput.getBlockHashHex(); | |
| const parentPayloadInput = this.seenPayloadEnvelopeCache.get(parentRootHex); | |
| const childWantsFullParent = | |
| parentPayloadInput !== undefined && childParentBlockHash === parentPayloadInput.blockHashFromBid; |
There was a problem hiding this comment.
getBlockHashHex() does exist on PayloadEnvelopeInput — it returns toRootHex(this.bid.blockHash) (see payloadEnvelopeInput.ts:131). The seenPayloadEnvelopeInputCache stores PayloadEnvelopeInput objects which have all the bid info. No issue here.
| this.forkChoice.onExecutionPayload( | ||
| blockRootHex, | ||
| toRootHex(postEnvelopeState.latestBlockHash), | ||
| // TODO GLOAS: this is not right but we don't need to track it as part of consensus spec, lighthouse also does not track it |
There was a problem hiding this comment.
The executionPayloadNumber is hardcoded to 0 when calling this.forkChoice.onExecutionPayload. This is a critical issue because the executionPayloadNumber is stored in the ProtoNode and is used to determine the block number of subsequent child blocks. An incorrect value here will lead to a cascade of incorrect block numbers for all descendant blocks, potentially causing issues with block production and validation.
The correct executionPayloadNumber is available within the signedExecutionPayloadEnvelope that is processed in verifyBlocksStateTransitionOnly. This value should be plumbed through to this point, for example, by adding it to the FullyVerifiedBlock type.
There was a problem hiding this comment.
This is pre-existing on epbs-devnet-0 — the TODO comment and hardcoded 0 were already there before this merge PR. The comment explains: "we don't need to track it as part of consensus spec, lighthouse also does not track it." Not a regression from this PR.
There was a problem hiding this comment.
Code Review
This pull request introduces significant changes to support the ePBS (enshrined Proposer-Builder Separation) feature, which is a major update for the Ethereum consensus layer. The changes are extensive, touching upon API routes, event handling, block processing, fork choice logic, state transitions, networking protocols, and synchronization mechanisms.
My review focused on the correctness and robustness of the new ePBS implementation. Key areas of change include:
- New API Endpoints and Events: Addition of
getExecutionPayloadEnvelopeendpoint andexecutionPayloadBidevent type. - Block Processing Pipeline: The entire block processing pipeline has been updated to handle
ExecutionPayloadEnvelopesseparately from beacon blocks. This includes changes toBlockInputtypes,verifyBlock,importBlock, and state transition functions. - Fork Choice Logic: The fork choice algorithm has been enhanced to manage different payload statuses (
PENDING,EMPTY,FULL) for blocks, which is a core concept in ePBS. This includes new logic for head selection and attestation validation. - Networking: New req/resp protocols have been added for fetching execution payload envelopes.
- Synchronization: Both range sync and unknown block sync have been updated to download and process envelopes alongside blocks, with improved error handling and retry mechanisms to deal with the new race conditions introduced by ePBS.
Overall, the changes are well-implemented and demonstrate a deep understanding of the complexities of ePBS. The code includes thoughtful comments explaining the rationale behind complex logic, especially concerning race conditions and state management. The addition of new tests for the new functionality is also commendable. I did not find any issues of medium severity or higher.
… non-FULL, add opposite-variant fallback
The merge into epbs-devnet-0-merged-unstable took the unstable version of
getClosestHeadState (2 fallbacks, throws on PENDING) while the test file
came from epbs-devnet-0 which expects a 3rd fallback (opposite payload
variant). This caused a unit test failure:
expected null to be { tag: 'opposite' }
Fix: restore the 3-fallback implementation from epbs-devnet-0:
- Treat PENDING blocks as non-FULL (no throw) so restart edge cases work
- Add 3rd fallback to opposite payload variant to avoid 'headState does
not exist' startup failures when fork-choice payload status mismatches
the persisted checkpoint cache variant
Also remove now-unused fromHex import.
| this.blockStateCache.get(head.stateRoot) | ||
| this.checkpointStateCache.getLatest(head.blockRoot, Infinity, preferredPayloadPresent) || | ||
| this.blockStateCache.get(head.stateRoot) || | ||
| this.checkpointStateCache.getLatest(head.blockRoot, Infinity, !preferredPayloadPresent) |
There was a problem hiding this comment.
@lodekeeper are the changes from this commit part of epbs-devnet-0 branch, if not the last commit you did on this branch should be reverted
There was a problem hiding this comment.
Good catch — the merge with unstable picked unstable's version (which throws RegenError on PENDING) instead of keeping the epbs-devnet-0 version (graceful fallback).
The 3-fallback getClosestHeadState and the graceful null return in getPreStateSync ARE part of epbs-devnet-0 — they were the restart fix. The merge conflict resolution picked the wrong side.
Fixed in 70dbd2e — restored both functions to match epbs-devnet-0:
getPreStateSync: returnsnullfor Gloas blocks needing FULL parent (triggers queued regen retry) instead of hard throwgetClosestHeadState: 3-tier fallback (preferred variant → block state → opposite variant)
…ClosestHeadState The merge with unstable incorrectly picked unstable's version which throws on PENDING payloadStatus. Restore the epbs-devnet-0 version: - getPreStateSync: graceful null return for Gloas blocks needing FULL parent (triggers queued regen retry) instead of hard throw - getClosestHeadState: 3-tier fallback (preferred variant → block state → opposite variant) to handle restart edge cases where fork-choice head variant differs from persisted checkpoint state
…et-0-merged-unstable
Clean rebase of
epbs-devnet-0withunstablemerged in — all 64 conflict files resolved, build and lint pass.This shows the ePBS-specific diff only (code not yet on unstable).
Replaces PR #9091 (which had unresolved conflicts blocking GitHub's diff view).
Stats: 93 files changed, +3172/-421