Repository navigation
feat: add some more metrics from leanMetrics - #38
Merged
Merged
Conversation
MegaRedHand
marked this pull request as ready for review
January 19, 2026 18:32
tcoratger
pushed a commit
to tcoratger/ethlambda
that referenced
this pull request
Oct 30, 2026
* feat(beacon): judge a block by the rules that need no post-state Beacon gossip validation needs the `beacon_block` rules that can be applied before a block's own state transition runs, and the chain actor will want the same rules before it parks a block. They live in one place, `precheck::precheck_block`: a slot after its parent's, the proposer fulu's lookahead already fixed for that slot, and a valid proposer signature. A block whose parent has no post-state yet is judged against a recent state, for the signature alone. `bls::DST` is public and `test_state` gains `secret_key_for`, so a test can sign a block these rules accept. * feat(types): expose a beacon block's blob count and payload timestamp The beacon_block gossip rules bound the commitment count and check the payload timestamp before any state is read; both fields sit behind the fork enum. * feat(beacon): gossip verdict types and seen caches Gossip validation needs a verdict that says both what gossipsub does and whether the chain still wants the object (the spec's MAY-queue cases); fixed reason enums keep metric labels bounded. * build: fetch the spec's gossip validation vectors They first ship in v1.7.0-beta.1, so they come from a tree of their own instead of moving the main fixture pin. Only the fulu block and column handlers are extracted, since those are the topics this node validates. * feat(beacon): beacon_block gossip rules Split by cost so the p2p actor can run the state-free half inline and the rest off its loop; reuses precheck for the proposer and signature rules and reads states only from the cache. * test(beacon): run the spec's beacon_block gossip vectors One vector is skipped by design (a parent without a post-state is queued until a bad-block cache exists); everything else must match. * feat(beacon): data_column_sidecar gossip rules The proposer check reads the parent's lookahead instead of advancing a cloned state, so it is cheap enough for the gossip path; slots outside the window are queued as the spec allows. * test(beacon): run the spec's data_column_sidecar gossip vectors reject_parent_failed_validation is skipped for the same reason as the block runner's parent-not-verified vector: without a bad-block cache we cannot tell a failed parent from one still importing, so it is queued rather than rejected. * fix(beacon): judge the lookahead last and never reject on a failed ancestor walk Code-review fixes to the beacon gossip validation rules: - Block `stateful_checks` ran `precheck::fixed_proposer(..).is_none()` before `precheck_block`, the finalized-ancestry check and the payload-timestamp check. A block whose slot the lookahead cannot place at all (before the parent's own epoch, or a pre-fulu parent) short-circuited into `Queue(ShufflingUnavailable)` after only a head-state signature check, turning spec REJECTs (slot not after parent, finalized not an ancestor, bad timestamp) into IGNOREs. The lookahead gate now runs last, after `precheck_block` already checked slot order, known proposer and signature against the parent's own state. - `descends_from_finalized` collapsed a failed ancestor walk into "not an ancestor", but `fork_choice::get_checkpoint_block` erroring means a row is missing from `LiveChain` (for example after `invalidate_subtree` deletes a late-invalidated parent's rows while its cached state stays), not that the chain has forked away from the finalized checkpoint. Rejecting such a block penalizes peers who haven't yet learned of the invalidation. Replaced the bool with a three-way `FinalizedAncestry` (`Descends` / `Conflicts` / `Unknown`), used by both `gossip::block` and `gossip::column`; `Unknown` queues (`ParentNotReady`) instead of rejecting. - `column::verify_header_proposer` checked the lookahead before the signature, so a slot the lookahead cannot place returned `Queue(ShufflingUnavailable)` before ever checking the signature, even though the signature does not depend on the shuffling. The signature (and the known-proposer check) now run unconditionally; only the proposer-identity question is deferred to a `Queue` when the lookahead has no answer. Tests now cover the head-state signature path for an unknown parent, the lookahead queue path once the parent's own checks have run, and the regression case (a slot before the parent's own lookahead window used to mask a not-after-parent violation as a queue). The spec runner's config fallback now covers every fork up to the case's own rather than hardcoding fulu, and its comment cites the vectors' own README convention instead of a claim the design spec doesn't make. Clock-disparity magic numbers in the gossip unit tests are now derived from `MAXIMUM_GOSSIP_CLOCK_DISPARITY`. * feat(metrics): count beacon gossip verdicts and late verdicts Validation now gates propagation, so how often each rule fires, how long a verdict takes, and whether it beat gossipsub's cache eviction are the three things to watch. * feat(p2p): hold beacon gossip until it has a verdict Gossipsub forwarded every beacon message before anything checked it; lean keeps auto-forwarding because its handlers produce no verdicts. Beacon gossip stops propagating until the verdict plumbing lands; do not deploy this commit alone. * fix(beacon): check a queued column's header signature, and test every rule A queued column is written to the chain actor's PendingDataColumns the same way a queued block is, but only blocks got a signature check before being parked: a forged block is rejected up front, a forged column just sat there. Give columns the same treatment on both queue paths that precede the parent's own state (parent unknown, parent known but uncached), reusing the head-state check block.rs already has. Where the parent's state *is* already in hand but the finalized-ancestry walk itself fails, check the signature against that state instead of the head's, since it is the authoritative one. Also add the tests this rule and the surrounding ones were missing (one per outcome in the design's rule tables for both beacon_block and data_column_sidecar), fold the duplicated `store`/`seen`/`fulu_parent`/clock test helpers shared by block.rs and column.rs into a `test_support` module, and reword two doc comments whose sources had drifted from what they now claim. * feat(p2p): run stateful gossip checks off the actor loop The chain actor's mailbox waits reach seconds at p90 on the followers, past gossipsub's message-cache window, so verdicts come from bounded one-off blocking tasks that report back to the p2p actor instead. * feat(p2p): validate beacon gossip before propagating it Blocks and data column sidecars are judged by the fulu gossip rules and forwarded only on Accept; the other subscribed topics are ignored until they get validators. Undecodable payloads are now rejected instead of relayed. * refactor(p2p): decide a beacon gossip verdict before acting on it Each beacon gossip handler both decided a verdict and acted on it (calling verdict::report or verdict::spawn_stateful_checks inline), which meant the decision could only be exercised through a running actor's Context and never in a unit test. Split the two: the handlers now return a Dispatch describing what to do, and handle_beacon_gossip is the single place that reports or spawns. That single call site is also where a future cheap check that answered Queue would need to forward its object; today no cheap check does, so Report carries no object, and a debug_assert! catches the invariant instead of widening the type for a case that cannot happen yet. The verdict-time seen-cache re-check (first Accept wins) moves out of the actor's Handler impl into its own settle function for the same reason: it needs a &mut P2PServer, not a Context. Both the triage decisions and the settle re-check now have unit tests. A caught validation panic is also logged, since Ignore(Internal) in the metrics previously left no trace of what panicked. * fix(beacon): keep overloaded gossip, and check a column's signature first With every validation permit taken, a message was ignored and dropped. Gossipsub's duplicate cache then hides every other peer's copy, so the chain actor learned of the block only through a child's by-root fetch or range sync. It is still ignored, but now also handed to the chain actor, which judges it itself as it did before gossip validation existed. A column's header signature is now checked right after its parent's state is found, before the finalized-ancestry walk, the inclusion proof and KZG, so a forged header costs one BLS check rather than all three while holding a permit. The lookahead is judged last, as for blocks. The already-stored check read every stored sibling sidecar inline in the p2p actor; it is now a point lookup (`Store::has_data_column`). The p2p test helper ignored its config, so a triage test passed for a different reason than its comment gave; it now honours it. * docs: describe beacon gossip validation Every beacon message now waits for a verdict, so the wire guide, the metrics reference and the chain actor's notes on parked sidecars said things that are no longer true: that columns are deduplicated by a finality-pruned set, that fabricated headers park freely from gossip, and that the import's decode phase ends at the decode. * test(beacon): pass the gossip runner's on_block a committee cache lambdaclass#22 added the parameter, and the merge left the gossip conformance runner calling the old signature, so the spec-test binary no longer compiled. A fresh cache per call, as the fork-choice runner does. * build: derive the gossip fixture configs from the main tree's Each run reads its own preset from both fixture trees, so the two config lists always named the same presets and CI had to set both to agree. The gossip list now follows CONSENSUS_SPEC_TESTS_CONFIGS minus `general`, which has no `networking` runner to extract.
tcoratger
pushed a commit
to tcoratger/ethlambda
that referenced
this pull request
Oct 30, 2026
…relaying them (lambdaclass#47) * refactor(beacon): keep the committee cache in the Store, shared by both actors Validating aggregates and subnet attestations in p2p needs the same shufflings the chain actor derives, and a shuffle of the mainnet active set is too expensive to run twice per epoch. The actor owned its CommitteeCache as a &mut value, so nothing else could read it. The cache now lives in the Store, which both actors already share, and locks internally: the entry list is locked only to find or insert an entry, never while a shuffle runs, and each entry is a OnceLock so concurrent misses on one shuffling run one derivation and the rest wait for it. Every method takes &self, so the STF and fork choice take &CommitteeCache where they took &mut. storage cannot name state_transition's types, so the pieces split by what they need: EpochCommittees, a plain data type, moves to ethlambda-types; the container, keyed by an opaque ShufflingKey, moves to ethlambda-storage; deriving a shuffling and its key stays in state_transition, behind CommitteeCacheExt so call sites keep calling .committees(state, epoch). No behavior changes. * fix(p2p): refuse a 64-bit attestation subnet prefix The guard accepted a prefix of exactly u64::BITS, and the shift that follows it overflows there: a panic in debug, a wrap to 1 in release. No shipped config reaches it, but a custom --network config can. * feat(beacon): gossip rules for aggregates and subnet attestations beacon_aggregate_and_proof and beacon_attestation_{subnet_id} get the same treatment lambdaclass#38 gave blocks and columns: gossip::aggregate and gossip::attestation hold every condition of the spec's validators, split into cheap checks (message, clock, seen caches) and stateful checks. Three deliberate departures from the spec's text, each documented at the module: - The state is the voted block's cached post-state, not the head's. It is the attested chain's own state, so the shuffling is the one the attesters were assigned even on a fork whose deciding block differs from the head's, and its block_roots answer both ancestry checks without a LiveChain scan. - Pubkey-only signatures run before any committee is derived. A state that cannot key its shuffling derives one uncached, which is a full shuffle of the active set; a forged message must not reach that step. - A voted block with no cached post-state is IGNORE, not the spec's REJECT, for the reason blocks and columns already give: without a bad-block cache, a block that failed is indistinguishable from one still importing. The seen caches follow the spec's semantics rather than a running union: an aggregate is ignored only when one already-accepted aggregate covers its bits, so aggregates the spec says to propagate are not dropped. Both are LRU-bounded, never pruned by finality. The v1.7.0-beta.1 vectors for both topics now run on both presets. The two reject_block_failed_validation cases are skipped for the reason above. The fixture stamp now names its handler list, so a tree extracted before a handler was added re-extracts. * feat(p2p): validate aggregates and subnet attestations before relaying them Since lambdaclass#38 a beacon message is relayed only once p2p reports a verdict. Aggregates were validated in the chain actor instead, after a one-slot deferral, so they were never relayed, and subnet attestations got no verdict at all. Both now run through the verdict path with the rules from the previous commit. Only an aggregate gossip accepted reaches the actor, carrying the attesting indices whose signature p2p verified. The actor no longer derives committees or checks signatures: apply_verified_aggregate runs fork choice's own conditions and records the votes. This closes both blocking review findings on lambdaclass#19. The deferral queue can no longer be filled with unverified aggregates, and no aggregate rebuilds committees on the actor. The actor's applied-bits gate is pruned by the store clock rather than by finality, and a drain scans the block index once, only when something is ready. Aggregates and subnet attestations get a permit pool of their own, so a per-slot burst of them can never make a block or column answer Overloaded. An aggregate is forwarded only on Accept, never on Overloaded, since the actor no longer judges it. Subnet attestations are relayed but never forwarded, as before. The seen-attestation capacity is sized from the per-subnet bound, times this node's backbone subnets.
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.
This PR adds the following metrics from the leanMetrics specification:
lean_current_slotlean_safe_target_slotlean_latest_justified_slotlean_latest_finalized_slotlean_validators_countIt also adds a copy of the spec, with a column specifying which metrics are supported.