Skip to content

feat(events): add block_gossip chain event#535

Open
MegaRedHand wants to merge 3 commits into
mainfrom
feat/events-block-gossip
Open

feat(events): add block_gossip chain event#535
MegaRedHand wants to merge 3 commits into
mainfrom
feat/events-block-gossip

Conversation

@MegaRedHand

@MegaRedHand MegaRedHand commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Continues the chain-event pub-sub series (#516 / #517 / #518, all merged). Adds the beacon block_gossip analog.

Event added

Topic Payload Emitted when
block_gossip { slot, block } A block is seen on the network, before import

Notes

  • Emitted from the blockchain actor's NewBlock handler, not a second P2P-side publisher, so the actor stays the sole event publisher and the write flow stays one-directional (a core requirement of the series' design).
  • Fires for both gossip'd and by-root-fetched blocks, since both re-enter through NewBlock. Import may pend the block while its parent chain is fetched, so this precedes import.
  • Ungated like block (not recency-gated), so subscribers can watch sync progress. Low-rate (one per block), so the payload is built unconditionally behind emit's no-subscriber guard.

Testing

cargo fmt, cargo clippy -p ethlambda-blockchain -p ethlambda-rpc (clean), blockchain lib + rpc events tests pass.


Independent PR, based off main. One of three sibling PRs continuing the series — alongside #533 (chain_reorg/safe_target) and #534 (attestation/aggregate), each independently based off main. They touch overlapping regions of events.rs / docs/rpc.md, so whichever two merge later will each need a small conflict rebase. Opened as draft.

@MegaRedHand
MegaRedHand force-pushed the feat/events-attestation-aggregate branch from 51184e6 to 5be589f Compare July 21, 2026 22:44
Add the block_gossip topic to the /lean/v0/events stream, the analog of
the beacon block_gossip event: a block seen on the network before import
(import may pend it while its parent chain is fetched).

Emitted from the blockchain actor's NewBlock handler rather than a
second P2P-side publisher, so the actor stays the sole event publisher
and the write flow stays one-directional. Fires for both gossip'd and
by-root-fetched blocks, since both re-enter through NewBlock. Ungated
like block (not recency-gated), so subscribers can watch sync progress.
Low-rate (one per block), so the payload is built unconditionally behind
emit's no-subscriber guard.
@MegaRedHand
MegaRedHand force-pushed the feat/events-block-gossip branch from e17223c to a492c00 Compare July 22, 2026 13:35
@MegaRedHand
MegaRedHand changed the base branch from feat/events-attestation-aggregate to main July 22, 2026 13:35
@MegaRedHand
MegaRedHand marked this pull request as ready for review July 23, 2026 15:59
@github-actions

Copy link
Copy Markdown

🤖 Kimi Code Review

This is a clean, straightforward addition of a block_gossip event topic. The implementation correctly follows the existing patterns for event emission and matches the Beacon API semantics where block_gossip fires when a block is first seen on the network (pre-validation/import).

No critical issues found. A few minor observations:

  1. Event ordering semantics (crates/blockchain/src/lib.rs:1325-1328):
    The gossip event is intentionally emitted before on_block() validates and imports the block. This is correct per the Beacon API specification for block_gossip (fire on receipt, not on acceptance), but be aware that consumers may see events for blocks that later fail validation. Document this "at-least-once, pre-validation" semantics if not already clear to API consumers.

  2. Hash computation cost (crates/blockchain/src/lib.rs:1327):
    hash_tree_root() is a cryptographic hash over the entire block. For large blocks this adds latency to the hot P2P receive path. Consider if this is acceptable or if the hash could be cached/precomputed elsewhere in the P2P pipeline, though this is likely negligible compared to SSZ deserialization costs.

  3. Documentation consistency:
    The docs/rpc.md update correctly notes that block_gossip has no state field (unlike checkpoint events), which is appropriate since the post-state hasn't been computed yet at gossip time.

Code quality:

  • Proper exhaustive match arm additions in Topic::as_str(), FromStr, and ChainEvent::topic().
  • Tests updated to include the new variant in ALL_TOPICS.
  • Consistent use of H256 for the block root.

LGTM.


Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

@github-actions

Copy link
Copy Markdown

🤖 Codex Code Review

  1. block_gossip is emitted for non-gossip blocks, so the new event is semantically incorrect as implemented. In crates/blockchain/src/lib.rs:1325, every NewBlock triggers ChainEvent::BlockGossip, but NewBlock is also used for blocks arriving from req/resp sync, not just gossipsub, as shown in crates/net/p2p/src/req_resp/handlers.rs:336 and crates/net/p2p/src/req_resp/handlers.rs:374. That means range-sync and missing-parent fetches will be surfaced to RPC clients as block_gossip, which contradicts the event name, the new docs, and beacon-API expectations. This should either carry source metadata (gossip vs req_resp) or emit the event only on the gossipsub path.

I did not find additional consensus, security, or memory-safety issues in this diff itself.

Verification note: I could not run the targeted Rust tests in this environment because Cargo/Rustup needs to write under read-only home directories (~/.cargo / ~/.rustup) to resolve dependencies.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds the beacon-style block_gossip chain event.

  • Extends chain-event topic parsing, naming, payload mapping, and tests.
  • Emits each incoming block's slot and root before import processing.
  • Documents the fifth SSE event topic and payload.

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking opportunity to avoid redundant block-root hashing when no event subscribers exist.

The new event is wired through topic parsing, filtering, serialization, emission, tests, and documentation; the only accepted concern is avoidable payload computation before the event bus can take its no-subscriber fast path.

crates/blockchain/src/lib.rs

Important Files Changed

Filename Overview
crates/blockchain/src/events.rs Adds complete topic parsing, naming, event mapping, and unit coverage for BlockGossip.
crates/blockchain/src/lib.rs Emits BlockGossip before import, but computes the block root even when the event bus has no subscribers.
docs/rpc.md Accurately documents the new SSE topic, payload, and emission timing.

Sequence Diagram

sequenceDiagram
  participant P2P
  participant BC as Blockchain actor
  participant Bus as Event bus
  participant Import as Block import
  participant SSE as SSE subscriber
  P2P->>BC: NewBlock
  BC->>BC: Compute block root
  BC->>Bus: emit(block_gossip)
  Bus-->>SSE: event: block_gossip
  BC->>Import: on_block
  Import->>Import: Compute block root and import or pend
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
crates/blockchain/src/lib.rs:1327
**Avoid redundant block-root hashing**

The block root is computed before `EventBus::emit` can apply its no-subscriber guard, so handling `NewBlock` without SSE subscribers computes and discards a full SSZ root before normal block processing computes the same root again. Exposing a lazy emission path or reusing the root would avoid this actor-thread CPU cost.

Reviews (1): Last reviewed commit: "Merge branch 'main' into feat/events-blo..." | Re-trigger Greptile

Comment thread crates/blockchain/src/lib.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant