Skip to content

docs: record API-02 block observation audit #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
krisoshea-eth merged 6 commits into from
Aug 12, 2026
Merged

docs: record API-02 block observation audit #13

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8010397
docs: record API-02 block observation audit
krisoshea-eth Aug 10, 2026
4bb0044
docs: refresh API-02 pre-review evidence
krisoshea-eth Aug 11, 2026
b95e0ea
docs: record SSE recovery boundaries
krisoshea-eth Aug 11, 2026
3af0923
docs: correct API-02 delivery week
krisoshea-eth Aug 11, 2026
abf604c
docs: refine API event evidence
krisoshea-eth Aug 11, 2026
bed12b7
docs: remove posted beacon API evidence draft
krisoshea-eth Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions docs/beacon-apis-599-evidence-comment.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Draft evidence comment for beacon-APIs #599

> Draft for Kris to review and post after the API-02 Lodestar PR is available. Do not post automatically.

We implemented the interim Builder observation path in Lodestar using the standard `block` event followed by `getBlockV2` for the event's block root.

The flow is sufficient for correctness, but it exposes two API costs that may help resolve this issue:

1. The `block` event contains the beacon block root, slot, and `execution_optimistic`. It does not contain the fork version, selected Builder index, or execution block hash. An external Builder therefore performs one `getBlockV2` request for every newly observed imported post-Gloas block merely to learn whether its bid was selected.
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.

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.

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ 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.


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, while the existing `block` plus `getBlockV2` path remains a functional fallback.
20 changes: 10 additions & 10 deletions docs/implementation-plan.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
**Working model:** one shared feature branch; merge `unstable` regularly; split only when review benefits from it
**Formal product name:** `lodestar builder` / `packages/builder`
**Optional EPF mission codename:** Forgestar
**Status:** v1.0 merged on 5 August 2026. All plan-review comments are resolved and no plan-level architecture question remains open. This execution copy was reconciled on 10 August against Lodestar v1.46.0-rc.1, the merged equivocation work, Builder PR #9781, the TEST-01/MET-01/REVIEW-01 issue split, the latest Lodestar-team discussion, and the Linear/GitHub backlog. The full plan remains canonical on GitHub because it exceeds HackMD's per-note character limit. Moving implementation details continue to be refined through their owning issues without reopening the accepted architecture.
**Status:** v1.0 merged on 5 August 2026. All plan-review comments are resolved and no plan-level architecture question remains open. This execution copy was reconciled on 11 August against Lodestar v1.46.0-rc.1, the merged equivocation work, Builder PR #9781, the TEST-01/MET-01/REVIEW-01 issue split, the latest Lodestar-team discussion, and the Linear/GitHub backlog. The full plan remains canonical on GitHub because it exceeds HackMD's per-note character limit. Moving implementation details continue to be refined through their owning issues without reopening the accepted architecture.

> **Project documents:** [Merged proposal](https://github.com/eth-protocol-fellows/cohort-seven/blob/master/projects/lodestar-eip-7732-builder.md) · [Canonical Living Technical Note](https://github.com/krisoshea-eth/lodestar-eip-7732-builder-docs/blob/main/docs/living-technical-note.md) · [HackMD mirror](https://hackmd.io/@krisos/S1a9mdB7fl) · [Presentation slides](https://docs.google.com/presentation/d/1cmC3fpu652gZFTIm2_P1lIYOfC2M_w3c5qXSUZ4B6lc) · [Lodestar repository](https://github.com/ChainSafe/lodestar) · [Maintained Beacon API Builder flow](https://github.com/ethereum/beacon-APIs/blob/master/validator-flow.md#builder-optional)

Expand Down Expand Up @@ -454,7 +454,7 @@ The exact preparation route, initial publication offset, and cache hook remain i

### Verified implementation baseline at final review

This snapshot was refreshed on 10 August 2026. It records work that can narrow the board issues without treating a draft, an open follow-up, or partially verified work as Done.
This snapshot was refreshed on 11 August 2026. It records work that can narrow the board issues without treating a draft, an open follow-up, or partially verified work as Done.

| Source | Verified state | Effect on this plan |
|---|---|---|
Expand All @@ -471,10 +471,10 @@ This snapshot was refreshed on 10 August 2026. It records work that can narrow t
| [Lodestar #9758](https://github.com/ChainSafe/lodestar/pull/9758) | Merged at `74a3175`: initial `@lodestar/builder` package and CLI scaffolding, local keystore, bid/envelope signing and tests, source-BN wiring, `waitForGenesis`, shutdown, shared `assertEqualParams`, and active-Builder resolution. The first package was also published to npm | `SIGN-01` is Done with merged review and CI evidence. Remaining CLI/source-BN implementation is now reviewed through #9781, with tests and metrics split into `TEST-01` and `MET-01` |
| [Lodestar #9766](https://github.com/ChainSafe/lodestar/pull/9766) | Merged: aligns the Builder package build and type-check scripts with the workspace TypeScript 7 migration by using `tsc` | Record the CI follow-up under completed `CLI-01`; any remaining #9781 implementation review belongs to `REVIEW-01` |
| [Lodestar #9770](https://github.com/ChainSafe/lodestar/pull/9770) | Merged: hides the generated Builder CLI page from the public docs sidebar while the command is not yet functional | Keep the page hidden or clearly marked work in progress until the command is functionally ready and the open `REVIEW-01` work is closed, then restore it as part of `HANDOFF-01`; the administrative closure of `CLI-01` alone is not the publication signal |
| [Lodestar #9781](https://github.com/ChainSafe/lodestar/pull/9781) | Open, review-required, and not draft at head `7386db08081ce635662d6898c925036cd2015b71` with 30 commits on 10 August. It contains Builder identity and status tracking, BN/EL readiness, execution fee-recipient and timeout CLI wiring, plus identity/tracker tests. The two latest commits replace the manual Builder-response check with the API client's standard `.value()` path and fix the asynchronous API-error response mocks. Four review threads remain unresolved. The reviewed later-Builder direction is to keep an unknown configured key inert and retry until it is deposited or activated, while preserving a returned non-active lifecycle status as a distinct operator-visible result | Preserve Marko's completed `CLI-01` and `API-01` statuses. Track the remaining review, later-Builder lifecycle implementation, responsibility documentation, and merge in `REVIEW-01`; keep the regression matrix and metrics in `TEST-01` and `MET-01` |
| [Lodestar #9781](https://github.com/ChainSafe/lodestar/pull/9781) | Open draft and review-required at head `251347b7b5c5723a84ca6851e603604cdc75cd80` with 33 commits and eight unresolved Nico review threads on 11 August. It contains Builder identity and status tracking, BN/EL readiness, execution fee-recipient and timeout CLI wiring, plus identity/tracker tests. The branch uses the API client's standard `.value()` response path and asynchronous API-error response mocks. The reviewed later-Builder direction is to keep an unknown configured key inert and retry until it is deposited or activated, while preserving a returned non-active lifecycle status as a distinct operator-visible result | Preserve Marko's completed `CLI-01` and `API-01` statuses. Track the remaining review, later-Builder lifecycle implementation, responsibility documentation, and merge in `REVIEW-01`; keep the regression matrix and metrics in `TEST-01` and `MET-01` |
| [Lodestar v1.46.0-rc.1](https://github.com/ChainSafe/lodestar/releases/tag/v1.46.0-rc.1), [#9790](https://github.com/ChainSafe/lodestar/pull/9790), [#9792](https://github.com/ChainSafe/lodestar/pull/9792), and [#9793](https://github.com/ChainSafe/lodestar/pull/9793) | rc.1 at `e2b315e` supersedes rc.0 as the newest immutable audit target. #9790 protects state persistence/database close during a stuck network-worker shutdown and #9792 fixes a QUIC resource leak. #9793 closed without merge because its self-signal/force-exit approach did not generalize, especially for default container PID 1. The underlying stuck handle remains unidentified | Advance `BASELINE-01` to rc.1 and test state persistence, process-manager timeout, and Builder restart/cache/reveal recovery. Retain #9793 as diagnostic history, not an active implementation requirement or root-cause fix |

The board preserves Marko's `CLI-01`, `API-01`, and `SIGN-01` **Done** statuses. `REVIEW-01` is **In progress** for the open #9781 review, lifecycle implementation, documentation, and merge work; `TEST-01` and `API-02` are **In progress**, while `MET-01` remains **Todo**. `API-02` may begin its event/API audit in Cycle 2, but `ENV-01` remains a dependency for its end-to-end evidence. Landed upstream prerequisites may narrow a consuming issue, but do not automatically complete it.
The board preserves Marko's `CLI-01`, `API-01`, and `SIGN-01` **Done** statuses. `REVIEW-01` and `TEST-01` are **In progress** for the open #9781 review and separated regression work; `API-02` is **In review**, while `MET-01` remains **Todo**. `ENV-01` remains a dependency for API-02's end-to-end evidence. Landed upstream prerequisites may narrow a consuming issue, but do not automatically complete it.

### Board hierarchy and pickup model

Expand Down Expand Up @@ -864,12 +864,12 @@ flowchart TD

**Tasks**

- [ ] Audit existing block/SSE events on the pinned SHA and the known upstream gap in [beacon-APIs #599](https://github.com/ethereum/beacon-APIs/issues/599).
- [ ] Prefer the standard block event plus `getBlockV2` when sufficient; otherwise add the smallest `/beacon` event-field or event-type change suitable for an upstream specification proposal.
- [ ] Retrieve the signed fork-correct block by root and inspect the selected bid.
- [ ] Handle an event arriving before the block is immediately queryable with bounded retry.
- [ ] Deduplicate repeated observations of the same block root.
- [ ] Add duplicate, event-before-block, and unsupported-fork tests; deeper reconnect/replay hardening follows only after the happy path works.
- [x] Audit existing block/SSE events on the pinned SHA and the known upstream gap in [beacon-APIs #599](https://github.com/ethereum/beacon-APIs/issues/599).
- [x] Prefer the standard block event plus `getBlockV2` when sufficient; otherwise add the smallest `/beacon` event-field or event-type change suitable for an upstream specification proposal.
- [x] Retrieve the signed fork-correct block by root and inspect the selected bid.
- [x] Handle an event arriving before the block is immediately queryable with bounded retry.
- [x] Deduplicate repeated observations of the same block root.
- [x] Add duplicate, event-before-block, and unsupported-fork tests; deeper reconnect/replay hardening follows only after the happy path works.

**Done when:** One BN event leads to one bounded evaluation of the corresponding signed block, with no direct p2p subscription.

Expand Down
Loading