docs: record API-02 block observation audit - #13
Merged
Merged
Conversation
This was referenced Aug 10, 2026
📝 WalkthroughWalkthroughThe documentation updates establish an August 11, 2026 baseline, record API-02 observation behavior and validation, update Builder review tracking, and define shutdown, source-affinity, reconciliation, replay, and failover requirements. ChangesAPI-02 Builder observation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/living-technical-note.md`:
- Line 882: Update the observer’s root-consumption policy so transient retrieval
retry exhaustion does not leave the root retained indefinitely: separate
in-flight deduplication from completed outcomes, and remove the root or schedule
a bounded retry when transient retries are exhausted. Continue retaining roots
for deterministic fork skips and malformed responses.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e0630f60-6119-45f2-9264-5fdd5ac42d8d
📒 Files selected for processing (4)
docs/beacon-apis-599-evidence-comment.mddocs/implementation-plan.mddocs/living-technical-note.mddocs/work-log.md
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/beacon-apis-599-evidence-comment.md`:
- Line 13: Update the evidence comment to state the authoritative retry limit
and backoff policy, plus that block-root deduplication preserves first-seen FIFO
order. If these rules are defined elsewhere, link to the authoritative API-02
policy instead of leaving the bounded-retry and FIFO guarantees implicit.
- Around line 11-15: Update the evidence conclusion around the replay discussion
to state that reconnect, replay, and restart recovery are deferred to REL-01. Do
not imply that adding a stable replay identifier alone provides recovery;
describe replay as requiring defined SSE retention/replay semantics or an
explicit reconciliation mechanism, while preserving the current bounded
block-plus-getBlockV2 fallback.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: babad509-2c97-446c-8307-c16305bbb88a
📒 Files selected for processing (4)
docs/beacon-apis-599-evidence-comment.mddocs/implementation-plan.mddocs/living-technical-note.mddocs/work-log.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/work-log.md
- docs/implementation-plan.md
Comment on lines
+11
to
+15
| 3. Lodestar event frames contain `event` and `data` but no SSE `id`. EventSource can reconnect, but standard `Last-Event-ID` resumption cannot identify or replay a selection notification missed during the disconnect. A Builder therefore needs a separate bounded reconciliation path, and longer gaps or competing roots require a stronger contract than transport reconnection alone. | ||
|
|
||
| The implementation retries 404, server, timeout, and transport failures; does not retry other 4xx responses, cancellation, or response-decoding failures; and deduplicates block roots before retrieval. This works without a Lodestar-specific endpoint or a required Beacon API change. | ||
|
|
||
| The implementation evidence therefore supports treating either an enriched `block` event carrying selection identity or a dedicated `bid_included` event as an efficiency and interoperability improvement. It would remove the per-block fetch and give external Builders a more explicit cross-client contract. A stable replay identifier or an explicitly specified reconciliation mechanism would also make reconnect behavior interoperable, while the existing `block` plus `getBlockV2` path remains a functional bounded fallback. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Define replay semantics, not only a replay identifier.
Line 11 correctly states that the current frames cannot support Last-Event-ID resumption. A stable identifier alone does not provide replay. The server must emit an SSE id and define event retention and replay, or provide reconciliation. State that this recovery is deferred to REL-01 so the evidence comment does not imply that the current implementation provides it.
As per PR objectives, reconnect, replay, and restart recovery remain deferred to REL-01.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/beacon-apis-599-evidence-comment.md` around lines 11 - 15, Update the
evidence conclusion around the replay discussion to state that reconnect,
replay, and restart recovery are deferred to REL-01. Do not imply that adding a
stable replay identifier alone provides recovery; describe replay as requiring
defined SSE retention/replay semantics or an explicit reconciliation mechanism,
while preserving the current bounded block-plus-getBlockV2 fallback.
| 2. Lodestar emits the event after state transition and fork-choice import. Its retrieval path can serve the block from fork choice, the seen-block input cache, or the database, and the cache is pruned only after the database write. We therefore do not expect a 404 window on Lodestar. The Beacon API does not specify that event delivery must follow retrievability, and another client may choose a different emission point. A BN-agnostic Builder still needs bounded retry for an event-before-block window. | ||
| 3. Lodestar event frames contain `event` and `data` but no SSE `id`. EventSource can reconnect, but standard `Last-Event-ID` resumption cannot identify or replay a selection notification missed during the disconnect. A Builder therefore needs a separate bounded reconciliation path, and longer gaps or competing roots require a stronger contract than transport reconnection alone. | ||
|
|
||
| The implementation retries 404, server, timeout, and transport failures; does not retry other 4xx responses, cancellation, or response-decoding failures; and deduplicates block roots before retrieval. This works without a Lodestar-specific endpoint or a required Beacon API change. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
State the retry and deduplication policy precisely.
Line 13 lists retry categories and says roots are deduplicated. It does not state the retry limit, backoff, or FIFO deduplication behavior. Add those rules or link to the authoritative API-02 policy. Without them, this evidence comment does not substantiate the bounded-retry and FIFO guarantees.
As per PR objectives, this PR defines bounded retry and FIFO deduplication.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/beacon-apis-599-evidence-comment.md` at line 13, Update the evidence
comment to state the authoritative retry limit and backoff policy, plus that
block-root deduplication preserves first-seen FIFO order. If these rules are
defined elsewhere, link to the authoritative API-02 policy instead of leaving
the bounded-retry and FIFO guarantees implicit.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/work-log.md`:
- Line 119: Update the work-log entry describing beacon-APIs `#599` to say the
implementation evidence was staged for manual posting, not posted. Remove the
issue-comment URL while it remains unpublished, and preserve the surrounding
correctness and efficiency context.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c5fbd9ab-ed22-4147-b8bd-191785489e3f
📒 Files selected for processing (3)
docs/implementation-plan.mddocs/living-technical-note.mddocs/work-log.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/implementation-plan.md
- docs/living-technical-note.md
| - Began `API-02` from Lodestar `unstable` at `1dde9abaa66ca4d4f3500e8fb06106fa635a066d`, then merged the current `unstable` baseline at `713b21812905bb34e547ca85d5f630060179da67`. Audited the standard `block` SSE payload, Lodestar's post-import emission order, `getBlockV2` fork metadata and retrieval path, and the accepted-bid notification gap in beacon-APIs #599. | ||
| - Implemented a Builder `BlockObserver` that subscribes through the source BN REST client, retrieves post-Gloas signed blocks by root with bounded retry, deduplicates concurrent and sequential observations, preserves exact bid values, and hands observations to isolated registered callbacks without adding p2p, reveal, selection, metrics, or recovery behavior. | ||
| - Added 25 focused tests covering stream wiring, Gloas and Heze fork-correct output, sequential and concurrent duplicates, event-before-block 404 retry, retry exhaustion, error classification, cancellation, unsupported forks, metadata/body disagreement, bounded eviction, self-build sentinel handling, unexpected stream closure, callback isolation, and shared abort-signal use. The complete Builder package suite now contains 31 passing tests. | ||
| - Confirmed that standard `block` plus `getBlockV2` is sufficient for correctness. Posted the implementation evidence to [beacon-APIs #599](https://github.com/ethereum/beacon-APIs/issues/599#issuecomment-5257353985); an enriched event remains an efficiency and interoperability improvement rather than a prerequisite. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Record the #599 comment as staged, not posted.
Line 119 says that the implementation evidence was posted. The PR objective says that the comment is staged for manual posting after the related Lodestar PR and evidence are stable. Change “Posted” to “Staged” and add the issue-comment URL only after publication.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/work-log.md` at line 119, Update the work-log entry describing
beacon-APIs `#599` to say the implementation evidence was staged for manual
posting, not posted. Remove the issue-comment URL while it remains unpublished,
and preserve the surrounding correctness and efficiency context.
1 task
nflaig
pushed a commit
to ChainSafe/lodestar
that referenced
this pull request
Sep 10, 2026
## Motivation The Lodestar Builder needs a source beacon node API path for learning whether its bid was selected without joining libp2p. This PR implements the standard `block` SSE topic plus `getBlockV2` as a bounded compatibility path. Beacon APIs #630 and merged Lodestar #9832 also forward the signed winning block directly to the selected external Builder. The main purpose of that flow is to let the Builder help disseminate the block, with timely win notification as an additional benefit. Maintainer input is requested on whether this observer should remain as a compatibility fallback alongside direct delivery. ## Changes - Subscribe only to the standard `block` SSE topic. - Deduplicate roots before asynchronous work and retain a FIFO window of 256 roots. - Skip locally pre-Gloas slots and retrieve each new post-Gloas block by root with `getBlockV2`. - Use the Builder abort signal for both the SSE stream and block requests. - Retry 404, server, timeout, and non-input transport failures for up to six attempts with five 200 ms delays. Other 4xx responses, cancellation, decoding failures, input fetch errors, and structural failures are terminal. - Treat response version metadata as fork authority, require a Gloas-compatible body, and verify the returned slot against the event. - Preserve the exact signed bid, including later-fork fields, exact-width values, and `BUILDER_INDEX_SELF_BUILD`. - Dispatch observations concurrently through isolated `runOnBlock` callbacks. The existing API-client request timeout is unchanged. This PR bounds attempts and explicit retry delays, not total wall-clock or slot-relative time. Selection and reveal code will own deadline policy. The observer starts after the existing genesis, configuration, readiness, Gloas, and Builder identity gates. It joins the clock and `BuilderStatusTracker` as a long-lived Builder background service and shares their abort controller. This PR does not add p2p, `block_gossip`, canonical-chain filtering, local-bid matching, reveal behavior, metrics, reconnect, replay, restart recovery, multi-BN failover, or a new API endpoint. The observer intentionally evaluates blocks before SELECT-01 registers its first production consumer so this compatibility path remains active and evidenced. Event-time `executionOptimistic` comes from the triggering event, while the response metadata is authoritative for the fork. Terminally failed roots remain consumed until FIFO eviction; REL-01 owns controlled reconciliation. Aggregate retrieval concurrency, observer-specific metrics, and block-root recomputation before financial decisions are tracked in SEC-01, QA-01, and SELECT-01 respectively. ## API behavior and compatibility The standard `block` event contains the slot, beacon block root, and execution optimism. `getBlockV2` supplies the signed fork-correct block and `Eth-Consensus-Version` metadata. Imported non-head blocks remain valid observations, so `head` and `head_v2` are not substitutes. Lodestar emits `block` after state transition and fork-choice import. Root lookup checks fork choice for presence, then serves the block from the seen-block input cache or database. This ordering provides no expected Lodestar event-before-block window, but the Beacon API does not require equivalent ordering across clients, so bounded 404 retry remains a cross-client precaution. The implementation audit is recorded in merged [Builder docs PR #13](krisoshea-eth/lodestar-eip-7732-builder-docs#13). The provisional direct-Engine planning reconciliation is recorded in merged [Builder docs PR #18](krisoshea-eth/lodestar-eip-7732-builder-docs#18), the reproducible real-BN and shutdown evidence in merged [Builder docs PR #19](krisoshea-eth/lodestar-eip-7732-builder-docs#19), and the recent upstream PR audit in merged [Builder docs PR #20](krisoshea-eth/lodestar-eip-7732-builder-docs#20). Implementation evidence was posted to [beacon-APIs #599](ethereum/beacon-APIs#599 (comment)). Marco's open Lodestar PoCs [#9854](#9854), [#9875](#9875), [#9876](#9876), and [#9896](#9896) explore optional event improvements separately. The API-02 diff is limited to five Builder files. The latest review fixes build on Nico's updated branch at `09ea035863a5712eb417949f71e31c9d0f97f0fb`; no additional `unstable` merge was made for these fixes. Historical validation base: `f22c5ce63e`. Historical specification baseline: consensus-specs `v1.7.0-alpha.14`. Project issue: [krisoshea-eth#12](krisoshea-eth#12). ## Testing Validated locally on 2026-09-10 at `d74cf21de82d8ef4a8ad4b65627d2df131b42a16`, with Node 24.13.0 and pnpm 11.0.0: - 58 targeted tests across the observer, Builder lifecycle, identity, readiness and payload store. - Ordinary Builder package type-check. - Changed-file Biome. - Builder package build and module import check. - `git diff --check`. Dependencies were installed from this branch's lockfile and the Builder dependency packages were rebuilt in an isolated checkout. The new startup-log and warning regressions failed before the source changes and passed afterward. Coverage includes SSE wiring, cancellation, Gloas and Heze output, signed-bid preservation, slot consistency, duplicate suppression, bounded retry, decoding failures, FIFO eviction, self-builds, stream failures and callback isolation. These are local results, not a fresh CI or real-BN/EL run. The earlier real-BN and shutdown evidence remains in Builder docs PR #19. ## AI assistance disclosure - [x] External Contributors: I have read the contributor guidelines and disclosed my usage of AI below. > AI assistance was used for codebase research, implementation, testing, and review. I reviewed and revised the submitted code and PR text, made manual edits and technical decisions, and take responsibility for the final result.
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
1 participant
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
Records the API-02 block-observation audit and implementation decision in the Builder documentation.
blockplusgetBlockV2path and whyhead/head_v2are insufficient.execution_payloadevent already uses the selection-identity field names, but emits them too late to trigger reveal.SIGTERMevidence to ENV-01.The standard flow is sufficient for correctness without a Lodestar-specific endpoint. An enriched
blockevent or dedicatedbid_includedevent remains an evidence-backed efficiency and interoperability improvement, and a standard replay or reconciliation contract remains useful for recovery.Implementation evidence
BlockObservertestsunstablebaseline713b21812905bb34e547ca85d5f630060179da67Validation
git diff --check.Related project issue: krisoshea-eth/lodestar#12
Summary by CodeRabbit