feat(blockchain): add chain-event bus (head/block/justified/finalized)#516
feat(blockchain): add chain-event bus (head/block/justified/finalized)#516MegaRedHand wants to merge 1 commit into
Conversation
🤖 Codex Code ReviewFindings
No other material correctness, security, or Rust-pattern issues stood out in the diff itself. I could not run Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
🤖 Kimi Code ReviewOverall this is a clean, well-architected PR. The event bus design correctly prioritizes consensus liveness over delivery guarantees, and the integration points are surg Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
Greptile SummaryThis PR adds chain-event publishing to the blockchain actor. The main changes are:
Confidence Score: 5/5No additional blocking issue qualifies for this follow-up review.
|
| Filename | Overview |
|---|---|
| crates/blockchain/src/events.rs | Adds chain-event types, topic mapping, serialization, and the broadcast bus. |
| crates/blockchain/src/lib.rs | Threads the event bus into the actor and emits state changes around ticks and block imports. |
| bin/ethlambda/src/main.rs | Creates the production event bus and adds it to the blockchain configuration. |
| crates/blockchain/Cargo.toml | Moves serde into production dependencies for event serialization. |
Reviews (3): Last reviewed commit: "feat(blockchain): add chain-event bus (h..." | Re-trigger Greptile
🤖 Claude Code ReviewReview: PR 516 — chain-event busOverall this is clean, well-scoped work. Threading Three points worth a look, none blocking:
No fork-choice/justification/finalization logic changes here — the PR is additive observability around existing store semantics, and the diffs read faithfully against what's actually mutated (e.g., the Automated review by Claude (Anthropic) · sonnet · custom prompt |
🤖 Kimi Code ReviewOverall this is a clean, well-documented addition that correctly implements a best-effort event bus without affecting consensus safety. A few specific notes: Code correctness
Safety & performance
Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Codex Code ReviewFindings
Assumptions / gaps
Summary Aside from Point 1, the event plumbing looks disciplined: single-writer bus, non-blocking broadcast, and the Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
🤖 Claude Code ReviewReview:
|
23825f1 to
3a3c804
Compare
🤖 Kimi Code ReviewOverall this is a well-architected PR. The snapshot-diff pattern cleanly separates observability from consensus logic, and the best-effort broadcast design correctly prioritizes liveness over event delivery guarantees. Correctness & Safety
Code Quality
Testing
Nits
The PR is safe to merge. The snapshot/diff approach is robust and keeps the store code path clean of event plumbing. Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Codex Code ReviewFindings:
No consensus-state-transition, attestation-validation, SSZ, or XMSS handling regressions stood out in the touched code beyond the event-stream consistency issues above. I couldn’t run Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
🤖 Claude Code ReviewReview:
|
3a3c804 to
a966632
Compare
First PR of the chain-events series (re-cut of #460), shipping only the pub-sub mechanism so the real design decisions (event shape, publisher model, back-pressure policy) are reviewable on their own. The SSE transport and topic filtering follow in separate PRs; the bus stays unconsumed until the endpoint lands. Emission is actor-layer only, not threaded through the store. Before each state-changing store call (store::on_tick, store::on_block) the actor snapshots (head, latest_justified, latest_finalized), runs the store function completely unchanged, then diffs the snapshot against the post-call state and emits in order block -> justified_checkpoint -> head -> finalized_checkpoint. This keeps store.rs, both spec-test files, and test_driver.rs byte-identical to main: no Option<&EventBus> parameter spreads across on_tick/on_block/update_head or any of their test call sites, and the sole-publisher property is structural (only BlockChainServer ever holds a live EventBus) rather than something every call site has to preserve by convention. Two consequences of diffing at this level, both accepted: - `block` needs a was-it-new guard: on_block returns Ok early for an already-imported root, so the actor checks block-known-ness (the same has_state check the store itself uses) before importing, and only emits `block` for a genuinely new root. - Several head moves inside a single tick or block import coalesce into one `head` event carrying the net move; fine for subscribers, since the beacon-API analog is also last-value-wins per slot. The bus itself is a facade over a single bounded tokio broadcast channel: emit() never blocks or fails, since a slow subscriber lags and drops instead of back-pressuring consensus, and a receiver-count guard makes emission free when nobody is listening. ChainEvent serializes untagged so payloads stay flat: the topic name travels out-of-band via ChainEvent::topic(), fixing #460's double-tagged SSE payload before any transport exists. The `head` event is additionally gated by HEAD_EVENT_RECENCY_SLOTS, adopted from a review of Lighthouse's canonical-head recompute path: a new head is only emitted when its slot is within that many slots of the wall clock, so startup catch-up and backfill don't spam subscribers with historical head moves on the way to the tip. `block`, justified_checkpoint, and finalized_checkpoint stay ungated; consumers tracking sync progress should watch `block` instead.
a966632 to
60d9c2e
Compare
Adds a chain-event pub-sub mechanism to the blockchain actor: a
ChainEventenum (head,block,justified_checkpoint,finalized_checkpoint) published through anEventBusfacade over a bounded tokio broadcast channel. The actor is the sole publisher; subscribers attach their own receivers, and a slow consumer skips missed events instead of back-pressuring the actor. Near-zero cost with no subscribers (receiver_countguard). No HTTP surface in this PR: the RPC consumer arrives in the next PR of the series.Emission is actor-layer only, not threaded through the store. Before each state-changing store call (
store::on_tick,store::on_block)BlockChainServersnapshots(head, latest_justified, latest_finalized), runs the store function completely unchanged, then diffs the snapshot against the post-call state and emits in orderblock→justified_checkpoint→head→finalized_checkpoint.store.rs, both spec-test files, andtest_driver.rsstay byte-identical tomain: noOption<&EventBus>parameter spreads acrosson_tick/on_block/update_heador any of their test call sites, and the sole-publisher property is structural (onlyBlockChainServerever holds a liveEventBus) rather than something every call site has to preserve by convention.Two consequences of diffing at this level, both accepted:
blockneeds a was-it-new guard:on_blockreturnsOkearly for an already-imported root, so the actor checks block-known-ness (the samehas_statecheck the store itself uses) before importing, and only emitsblockfor a genuinely new root.headevent carrying the net move; fine for subscribers, since the beacon-API analog is also last-value-wins per slot.The
headevent is additionally recency-gated: it only fires when the new head is withinHEAD_EVENT_RECENCY_SLOTS(32) of the wall-clock slot, so catch-up/backfill never spams head events (adopted from Lighthouse's head-SSE recency filter); the other events stay ungated.Payload serialization is untagged, so the JSON body stays flat and topic names live only on the transport's
event:line.Supersedes #460, re-cut as a stacked series for easier review:
GET /lean/v0/eventsSSE endpoint (all events)?topics=server-side filteringchain_reorg+safe_targeteventsattestation/aggregateeventsHas unit tests and passes clippy with
-D warnings.