Repository navigation
feat: listen for gossipsub blocks - #9
Merged
Merged
Conversation
MegaRedHand
marked this pull request as ready for review
January 8, 2026 17:42
This was referenced Feb 2, 2026
tcoratger
pushed a commit
to tcoratger/ethlambda
that referenced
this pull request
Oct 30, 2026
…ambdaclass#9) * feat(blockchain): drive the beacon chain from BlockChainServer The actor was the last lean-only layer in a stack that has already unified types, storage, the state transition, fork choice, p2p dispatch and startup: ChainSetup.chain was None on mainnet, so run_node spawned nothing and gossip blocks were decoded only to be dropped. Nothing is wired up here. Feeding the actor beacon gossip and spawning it from run_node are later slices, so this changes no binary behaviour and the beacon arm is exercised by tests rather than by a running node. Blocks now cross the actor boundary as SignedBeaconBlock. Its Lean variant already exists and its message accessors already answer for it, so widening P2PToBlockChain::new_block and Store::insert_pending_block makes the whole pending-parent cascade chain-generic instead of duplicated, and removes two expect_lean assertions that held only because of a chain tag elsewhere. ChainDuties moves the seven lean-only fields behind a Lean variant, so they are unreachable on the beacon arm by construction rather than sitting inert there. The store, the pending maps, the sync-status tracker and the event bus stay shared. An early beacon block is deferred rather than dropped. Beacon's on_block requires a block's slot to be in the past, and the specification's own instruction above that assertion is that such a block's consideration "must be delayed until they are in the past". One dropped early block froze the live mainnet follower permanently on 2026-09-03: it arrived 273ms before its slot, and every later block was then an orphan of a root the node would never obtain. The window is wider than any clock-disparity tolerance, since the beacon store clock advances only in the tick handler and a multi-second import blocks that same thread, so the rule is really "before the first tick at or after the slot boundary". Deferral is bounded by BEACON_DEFERRAL_HORIZON_SLOTS: it persists the block, so deferring on "is in the future" alone would let one peer spend this node's disk on slots years away. The lean arm keeps its existing whole-slot margin and is not touched. It cannot be shared either way: beacon carries a specification assertion that the fork-choice fixture suite tests. Beacon ticks once per slot boundary. on_tick_per_slot resets proposer boost at a slot boundary and pulls up unrealized checkpoints at an epoch boundary, and does nothing between them; Config's attestation_due_bps and its siblings are validator duty deadlines that a follower never reads. Chain events needed fixing to get here. diff_and_emit and checkpoint_state_root both read through Store::get_block_header, which decodes a lean BlockHeader and so panics on a beacon directory, where that table holds the whole signed block. Both wanted the same two fields, so Store::block_slot_and_state_root answers for either chain in one read, keeping a slot and a state root from ever being paired across two blocks. Known gaps, all p2p-side and all deliberate: there is no beacon by-root fetch, so request_missing_block is a logged no-op on beacon and a block with a missing parent pends indefinitely. This arm can follow a contiguous run and nothing else until that lands. * feat(beacon): follow mainnet, from the anchor to the tip The wire and the chain actor existed and did not touch each other. The mainnet arm spawned no actor, gossip blocks were decoded and dropped, fetched blocks were validated and dropped, both request senders were exported and called from nowhere, and Status advertised zeroes so peers had no reason to ask us for anything. Connect them: - `run_node` spawns a chain actor on both arms. `ChainSetup.chain` becomes a `ChainActor` enum rather than a lean-shaped Option, so the InitP2P/InitBlockChain wiring and `wait_for_shutdown` now cover the beacon follower too. - Beacon gossip blocks go to `new_block` with `BlockSource::Gossip`. Aggregates still do not: that topic carries ~1000 per slot at ~30ms each in `on_attestation`, more work per slot than a slot lasts. - Fetched blocks go to `new_block` with `BlockSource::Sync`, keeping every per-block range and root check. - A peer's Status starts range sync, keyed off a new `beacon_fetched_through` watermark rather than the store's head. The head lags a delivered batch by the whole actor mailbox, which is what once cost 11,213 blocks off the wire to import 100. - `fetch_block_from_peer` picks its protocol from the wire, so `BlockChainToP2P::fetch_block` stays one chain-agnostic method and the actor's `request_missing_block` no longer no-ops on beacon. - `build_status` reads the store: head, finalized checkpoint, and an `earliest_available_slot` of the anchor rather than 0, since this node cannot serve genesis. Two of the three defects that froze the 2026-09-03 follower are fixed here. A by-root fetch now arms its own deadline, `RootFetchTimeout`, because `pending_root_requests` otherwise clears only on a libp2p outcome and a request drawing none locks that root for the life of the process. A finished range session now clears back to None instead of disabling the resync path forever. Verified against mainnet, and the wire half works end to end: anchored at slot 15171200 (fulu, 2,361,891 validators), Status drove range sync, 70 blocks came back on beacon_blocks_by_range_v2 and 2 on beacon_blocks_by_root_v2, gossip blocks reached the actor and pended on their missing parent, which then asked for it. It does not catch up yet, for a reason outside this change: one import at that validator count ran over ten minutes at 100% CPU, and a stack sample puts it in `process_attestation` calling `get_total_active_balance` and `get_active_validator_indices`, both of which walk the whole registry per attestation. Because the actor is single-threaded, that also stops its ticks, so `store.time()` freezes and head recomputation never runs. Making those accessors cheap is its own change. * fix(beacon): feed a block's own attestations to fork choice The follower imported blocks and computed nothing from them. Measured on ethlambda-4 against mainnet: a contiguous chain from slot 15171489 to 15171523 landed off the anchor at 15171488, while head, justified and finalized all stayed pinned at the anchor and only the wall-clock slot advanced. `fork_choice::on_block` does not iterate a block's attestations, since the specification leaves that to the caller, and the actor did not either. So the latest-messages map was permanently empty, every block weighed nothing, and `get_head` never descended past the justified root. Gossip aggregates are deliberately not forwarded, that topic being ~1000 aggregates per slot at ~30ms each, so block bodies are the only vote source there is meant to be. This makes that true. `on_block_attestation` takes the carrying block's post-state as the committee source rather than each attestation's target checkpoint state. The two name the same committees, and asking the checkpoint instead is the expensive way to reach a verdict this node already has: `process_attestation` ran the same `is_valid_indexed_attestation` on the way to producing the block, and once a target's post-state falls out of the recency cache, rebuilding it replays every block since the last pinned boundary. On the earlier follower that was one import at 78.9s against a 3.6s steady state. `on_attestation` is untouched, so the spec-conformance fixtures keep exercising the verifying path. A body item that cannot be evaluated is not an import failure. The block is in the store whatever the loop returns, and returning `Err` would make the caller stop the pending-block cascade, leaving held descendants stuck behind a block that did import. A checkpoint-synced follower reaches this legitimately: an attestation may name a target up to `SLOTS_PER_EPOCH` slots back, and for the first epochs after the anchor that target is below it and was never fetched, so `validate_on_attestation` rejects it. Each call therefore logs at trace and continues. Ported from `feat/mainnet-network`, without that branch's `fork_choice::block_state` helper or its block-root-returning `on_block`: the actor already holds the root, and the post-state was written under it by the import a moment earlier, so `Store::get_state` is a direct hit. * refactor(beacon): hold an early block in a timer, not in a map A beacon block that arrives before its own slot starts is held rather than dropped, but the hold cost the follower a map and the DB a write: the root went into `BeaconFollower::deferred_blocks`, the block into the pending-block table, and every tick ran a range query to find what had become replayable. The actor can wait on its own. `process_or_pend_block` now hands an early block back to `run_import_cascade`, which re-delivers it to this same actor with `send_after`, timed to the start of the block's own slot. The block rides in the message, so nothing is written for it and nothing is left to reconcile if the process stops before its slot arrives. `BEACON_DEFERRAL_HORIZON_SLOTS` still bounds what one peer can make this node hold, in memory now rather than on disk. The re-delivery handler runs the slot's tick before importing. Its timer is set off the wall clock, but every slot comparison the import makes reads the store clock, which only the tick advances, and that tick is armed for the same instant. `on_beacon_tick` is idempotent through its own store-clock guard, so running it here settles the ordering rather than betting on which message the mailbox delivers first. `BeaconFollower` held nothing else, so `ChainDuties::Beacon` becomes a unit variant and the beacon tick loses its replay pass. The unit tests that asserted on the map's contents go with it. * refactor(p2p): drop the by-root fetch deadline Every by-root fetch armed a `RootFetchTimeout` self-message as a backstop for a request that drew neither a response nor an `OutboundFailure`, and so left its `pending_root_requests` entry stranded for the life of the process. Nothing is left for it to catch. The response paths that ended an attempt without retiring the root were fixed in lambdaclass#608, and a request that draws no reply at all is already bounded by the request-response behaviour's own 10s `request_timeout`, which arrives as `OutboundFailure::Timeout` and goes through `handle_fetch_failure` like any other failure. What remains is a second mechanism over the same entry, and a `ctx` parameter that `fetch_block_from_peer` carried for nothing else. * refactor(beacon): re-deliver a held block as a NewBlock, not a message of its own Holding an early block until its slot needs no protocol message of its own. The actor already has one for "here is a block": `NewBlock` carries a `BlockSource`, and what a deferral changes is the source, not the question being asked. So `BlockSource` grows a `Deferred` variant, the one the p2p layer never sends, and `defer_early_block` arms its timer with `NewBlock` instead. `Handler<NewBlock>` turns its `if source == Gossip` into a match, so every source now has to be answered for: gossip keeps the `ChainEvent::BlockGossip` emission and the arrival histogram, sync keeps skipping both, and deferred skips them because the block's first arrival is what they already described. The tick that has to run before a deferred import, and the reason it does, move into that arm with it. * refactor(beacon): decide an early block at arrival, against the wall clock The hold for a block whose slot has not started was decided a whole slot wide and against the store clock, inside the import cascade. Both halves were larger than they needed to be. The window is now `MAXIMUM_GOSSIP_CLOCK_DISPARITY`, 500ms, the phase0 p2p-interface constant that already says how far apart two honest clocks may read. A block early by more than that is not early, it is wrong, and a whole slot of headroom was only ever a bound on how much memory one peer could make this node hold. The decision moves to `Handler<NewBlock>`, where it is one comparison of the block's own slot-start timestamp against the wall clock. Arrival is the only place a hold is still possible anyway: by the time the cascade has the block, its caller is a loop with nowhere to put it back, which is what the `Option<SignedBeaconBlock>` return and the `ctx` threaded through `on_block` and `run_import_cascade` were paying for. All three go back to their previous shapes. Reading the wall clock costs one thing the store clock gave for free. The old comparison also covered a tick that had not run yet, since an unticked slot simply deferred the block again; a wall-clock comparison would instead hand it to `on_block` and have the spec assertion reject it. So the admit path catches the store clock up first, and only when it still reads a slot below the block's, which keeps sync backfill out of it entirely. * refactor(blockchain): one tick pipeline for both chains `on_lean_tick` and `on_beacon_tick` were two functions because their duties differ, but a tick is the same four steps on either chain: decode the clock, advance the chain, run this chain's duties, refresh the gauges. Only the middle two actually differ, and both were carrying their own copy of the other two. `on_tick` is now that sequence, and each step answers for the chain it runs on. `begin_tick` decodes the wall clock onto the chain's grid and drops a tick the store clock has already covered, returning a `Tick` that every later step reads instead of deriving a slot of its own. `advance_chain` moves the store clock: `store::on_tick` on lean, which carries its own fork-choice work, and `fork_choice::on_tick` plus `get_head` on beacon, where nothing else records a head. `run_duties` is lean's interval grid, and returns immediately on a chain whose tick carries no interval. `observe_tick` refreshes the head, justified and finalized gauges, and lean's safe target. Only the chain advance stays inside the event-diff window. A lean duty can import a block, and `process_block` diffs and emits around its own import, so a window stretched over the duty phase would emit that block's head and finality moves a second time. That constraint is why beacon's `get_head` belongs to the advance rather than to the duties. Three orderings change, all of them for the better: lean's sync status and current-slot metric now read post-promote state rather than the state before its own store tick, beacon's sync status reads the head `get_head` just computed, and lean refreshes the justified and finalized gauges every tick as beacon already did, rather than only on import. The interval match, the aggregator snapshot and the XMSS key advance move verbatim. * refactor: simplify new block handler guard * refactor(blockchain): keep the tick the function it already was The previous commit unified the two ticks by inventing a shape for them: a `Tick` value and four phases to pass it through. One tick pipeline was the right call, but the shape was not the one this actor already had, and the interval grid is easier to follow written out in the order it happens than split across `begin_tick`, `advance_chain`, `run_duties` and `observe_tick`. So `on_tick` goes back to the function main has, and beacon is woven into it in place. Six edits carry it: the idempotency guard expresses this tick's position in whichever unit the store clock keeps (intervals on lean, Unix seconds on beacon); the empty-validator fail-fast asks first whether this is a lean store, since `head_state` panics on the other one; the sync-status and head-metric reads go through a `head_slot` accessor that decodes the right table; the aggregator flag goes through an `is_aggregator` accessor that answers false for a follower; the one `store::on_tick` line becomes a match, lean's call on one arm and beacon's `fork_choice::on_tick` plus `get_head` on the other; and the duty match is wrapped in a lean guard, arms untouched. The justified and finalized gauges stay in the tail for both chains. Beacon needs them there: its clock advance can pull up unrealized checkpoints at an epoch boundary, moving both without importing anything. Two properties of the code already here are why the rest needed no edit at all. `get_our_proposer` answers `None` on a follower, so the `is_proposer` line is correct unchanged, and a beacon tick fires on slot boundaries, so `interval` resolves to `BlockPublication`, whose arm is empty. The guard on the match is for the tick that lands late. * refactor(blockchain): give the interval duties their own function Weaving beacon into `on_tick` left the interval match nested inside a lean guard, indented a level away from where it reads best and sitting in the middle of a function that is otherwise a sequence of one-line steps. It is the one part of the tick that is a chain's duties rather than every chain's bookkeeping, so it moves out. `run_interval_duties` takes the interval, the slot and the aggregator flag the tick already resolved, and asks the lean question itself, so the call site stays the single line the match displaced. Its arms are unchanged and back at their original indentation. The safe-target metric stays in `on_tick`. It reports what `store::on_tick` did at interval 3 rather than anything a duty did, so it keeps its place after the call, with the chain guard the surrounding `if` used to give it. One thing improves in passing: the `let ChainDuties::Lean(lean) = .. else { return }` guards inside the arms now return from the duties function rather than from the whole tick, so a miss (unreachable, since the function checked already) can no longer skip the head, justified and finalized gauges or the key advance behind them. * docs(beacon): say that the follower now imports what it fetches The base branch's notes describe a wire that asks and advertises honestly but drops what it fetches, since it has no actor. Both are true there and neither is true here: fetched blocks reach the actor as `BlockSource::Sync`, and the by-root path is what resolves a gossiped block's missing parent rather than an unexercised guard. * refactor: simplify * perf(beacon): skip a block whose post-state the store already holds `fork_choice::on_block` does not short-circuit on a known root. It goes from cloning the parent state straight into `state_transition`, so every re-delivery of a block the store already has pays the whole import a second time. Measured on a mainnet follower 2026-09-08: 116 imports produced 79 distinct blocks, so 37 of them were duplicates, a third of the actor's import budget spent recomputing post-states it already held. The duplicate gaps were indistinguishable from first-pass gaps (6.1 to 7.7 seconds), which is how you tell a real re-import from a cheap no-op. Re-deliveries are ordinary, not pathological: a block reaches the actor from gossip, from a `BlocksByRange` response, and from the backward `BlocksByRoot` walk an orphan starts, and nothing upstream dedupes. Check for the post-state before anything else in `process_or_pend_block` and return, so a re-delivery costs a lookup. Its pending children are still collected: the root did import, whether or not this delivery is what imported it. Beacon-only, because lean's `store::on_block` has its own already-imported early return; the comment in `process_block` that claimed both did is corrected here too, since it is what made this look like a solved problem. * fix(beacon): recompute the head when a block arrives, not only on the tick `lean_head_slot` sat flat at the checkpoint-sync anchor for a follower's entire catch-up, while imports landed every few seconds. That is what "the head is not advancing" looked like on the dashboard, and it was not a reporting artifact: the head really was not moving. `fork_choice::on_block` computes no head, and `Store::beacon_head` only reads back whatever the last `fork_choice::get_head` recorded. So an import adds a block and a post-state and moves no head. The sole caller of `get_head` was the tick, which is one message per slot in the same mailbox as every arriving block; a follower catching up imports back-to-back and never drains that mailbox, so the tick did not run and the recorded head stayed at the anchor. `process_block` refreshed the gauge after every import, but from that same stale recorded value. Re-run fork choice at the end of `on_block`, the arrival boundary the store clock is already brought up to real time at, and republish the head there. Measured at ~96ms per head computation against imports costing seconds, and it is once per arrival rather than once per block in the cascade, matching how that boundary already treats the clock. The tick keeps a call of its own, which is still needed for a slot in which nothing arrived; both now go through one function. Publish `lean_current_slot` from the same place, since it had the same single writer: with both gauges only written by a starved tick, `lean_current_slot - lean_head_slot` was a difference between two stale numbers rather than the head lag every panel and alert reads it as. It comes from the wall clock, since the store clock only moves when the tick or an early arrival moves it; `process_block` already needed that reading for its head-recency gate, so the two now share `wall_clock_slot`. Observed on a mainnet follower on the previous two commits: 29 blocks imported to slot 15178365, head gauge still reading the anchor 15178336, current-slot gauge 6 slots behind wall clock, and zero blocks rejected. * Update crates/net/p2p/src/lib.rs * style: rustfmt
tcoratger
pushed a commit
to tcoratger/ethlambda
that referenced
this pull request
Oct 30, 2026
The beacon follower ticks once per slot, so the store clock sat at the slot's start for the whole slot. on_block read it for block timeliness (attestation deadline and, from gloas, the payload timeliness committee deadline), found zero milliseconds into the slot, and called every current-slot block timely, late ones included. A late block therefore got proposer boost and was exempt from the late-head reorg rules. Advance the store clock to the block's wire receipt (decode_start, else hand-off) before on_block. The spec's clock is whole seconds and its on_tick runs whenever time > store.time, so the deadlines are judged on the second the block fell in. The advance never goes backwards and never crosses a slot boundary, which stays the tick's job since it also drains deferred aggregates and recomputes the head. Why the tick was per slot: the follower has no sub-slot duties (see ChainDuties::Beacon, introduced with the chain actor in lambdaclass#9). That is a statement about duties, not about fork choice needing a coarse clock, so arrival-time advances are safe. The envelope and payload-attestation handlers are not called live yet; they get the same hook when wired. Pre-gloas blocks now also get spec-correct proposer boost: a late block is no longer boosted.
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
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.
No description provided.