Repository navigation
docs: update roadmap - #11
Merged
Merged
Conversation
tcoratger
pushed a commit
to tcoratger/ethlambda
that referenced
this pull request
Oct 30, 2026
…ailability gate (lambdaclass#11) * feat(beacon): define the das-core custody settings once The five custody values are identical across configs/mainnet.yaml and configs/minimal.yaml, so they belong as constants in the types crate rather than Config fields: a Config field changes the SSZ encoding persisted under Metadata[KEY_CONFIG], forcing DB_VERSION up and refusing every existing lean and beacon data directory. The p2p crate's own CUSTODY_REQUIREMENT now re-exports the shared constant so the ENR cgc entry, MetaDataV3, subnet subscription, and the future availability check cannot drift apart. * feat(beacon): compute the custody groups a node id is assigned * test(beacon): run the das-core custody fixtures * docs(beacon): correct the sidecar serve window's direction The window is the range a node must serve, so a node may refuse below it, not within it. Also drops the forward-looking hedge from the multiple-of test now that compute_columns_for_custody_group exists. * feat(beacon): verify a sidecar's commitments against its block Adds verify_data_column_sidecar_inclusion_proof, the third and last of the fulu column-sidecar checks: it proves the commitments a sidecar carries are the ones its named block actually committed to, closing the gap where a peer could otherwise pair a valid column with any block header it liked and still pass the existing structural and KZG checks. * refactor(beacon): express the custody asserts the way the crate does * feat(storage): keep custodied data column sidecars The beacon follower needs its data column sidecars before a block that depends on them imports (for the availability check), after a restart (a resync shouldn't re-fetch what was already verified), and when peers ask for them by root or by range. Add a DataColumns table keyed slot-first, like BlockProof, so a by-range scan and a future pruner can both work in slot order and stop early. Nothing writes to it yet; the gossip path does in a later task. * test(beacon): report which custody group differs * feat(p2p): subscribe to the column subnets this node custodies ethlambda beacon advertised a custody group count in its ENR while subscribing to no data column subnet at all. Subscribe to exactly the subnets this node's own discv5 node id selects for custody, computed from the *sampling* size rather than the custody requirement, since a node samples more groups than it strictly custodies and the sampled columns need a source too. * docs(beacon): credit the check that actually catches a shifted field The subtree-index unit test only pins BLOB_KZG_COMMITMENTS_SUBTREE_INDEX against the fixture's stated generalized index; it never touches BeaconBlockBody, so a field shifting blob_kzg_commitments's real position would pass it unchanged. The merkle_proof fixtures' end-to-end check, which decodes a real body and drives it through verify_data_column_sidecar_inclusion_proof, is what actually catches that. Reword both comments to credit it correctly. Also: name KZG_COMMITMENTS_INCLUSION_PROOF_DEPTH instead of spelling out its leaf count in prose, and select merkle_proof's fulu cases by the case's own name field instead of an id() meant for failure messages plus a redundant fork check that would silently drop a future fork's cases from this suite. * docs(beacon): state the custody index precedent at its real granularity CustodyIndex's justification for staying local cited RowIndex's precedent but phrased it as staying beside one container, when RowIndex's own comment is single-struct scoped ("only MatrixEntry needs it"). CustodyIndex is used across several functions of a whole module, so reword the analogy to match: staying beside its only consumers, not beside a single container. * fix(storage): make the earliest custodied slot honest under concurrency put_data_column_sidecar's earliest-slot read and its conditional write are two separate backend calls with no isolation between them. Investigated whether a second writer can exist: the chain actor owns its Store outright and processes one message at a time (GenServer pattern), and every other Store clone (ethlambda-rpc, ethlambda-p2p) only reads it, confirmed by grep for mutating calls in both crates. A single writer is genuinely guaranteed today, so document that assumption precisely at the call site instead of adding a mutex the evidence doesn't call for, and correct the doc comment that used to cite "concurrent readers/writers" as the reason for `&self`, which implied a guarantee the code was never given. Also: name the actual reason data_column_sidecars_in_range scans per slot (StorageReadView has no range-iterator counterpart to delete_range) rather than only the multi-block-per-slot reason, which is separate and stays; add a test that stores several column indices across two sibling blocks at one slot and asserts a range query returns exactly the requested subset, since every existing test wrote and queried only column index 0; and drop the test_beacon_store wrapper in favor of the existing beacon_test_store(Arc::new(InMemoryBackend::new())) call pattern already used twelve other times in this module, rather than leaving two spellings of the same setup. * feat(beacon): verify and keep the column sidecars we custody Wires the fulu data-availability sampling gossip path end to end: the p2p actor decodes a data_column_sidecar_N message, runs the checks that need nothing but the sidecar and the store's own checkpoints (subnet match, structural validity, finality, seen-dedup), and hands what survives to the chain actor. The chain actor runs the checks that need state and fork choice (bounded slot, known and finalized-descendant parent, inclusion proof, KZG batch, proposer signature) and writes accepted sidecars to the DataColumns table. Adds a future-slot bound before the proposer check's process_slots call and a finalized-ancestor check alongside the existing finalized-slot check, both present in fork_choice::on_block's own import path but missing from the task's reference implementation; without the first, a header naming an arbitrarily large slot would drive process_slots one slot at a time on the actor's own thread. release_block_if_columns_complete is a no-op notification point for the data-availability gate a later task adds. * fix(p2p): dedupe column topics through the map and reuse the typed key error Two code-review fixes for 31ff52e2. BeaconTopics::new built `topics` in parallel with `column_topics` instead of deriving it, so a network where NUMBER_OF_CUSTODY_GROUPS and DATA_COLUMN_SIDECAR_SUBNET_COUNT diverge would double-subscribe a shared subnet; column_topics is now built first (as a BTreeMap, for a stable ascending order) and topics derives its column tail from it. node_id_from_secret_key returned a bare secp256k1::Error instead of the crate's existing DiscoveryError::NodeKey, so an invalid --node-key reported a different message depending on which of two code paths parsed it first. Also: routed main.rs's key-to-id derivation through a new beacon::beacon_node_id wrapper so that composition is unit-testable, an intra-doc link to a pub(crate) item, and two stale doc/log wordings. * feat(p2p): serve and request data column sidecars The node now custodies the columns its node id selects, so the two DataColumnSidecars protocols no longer need to lie at stream negotiation: by-root answers whichever named columns this node holds, skipping the rest, and by-range serves the custodied slot window, refusing RESOURCE_UNAVAILABLE below the earliest slot ever custodied. * fix(beacon): close the unbounded seen_data_columns growth and untested check Spec review on the sidecar gossip path (eedae86f) found the p2p future-slot filter was more than a nice-to-have: nothing checked a sidecar's slot magnitude before p2p inserted its (slot, proposer, index) tuple into seen_data_columns, so a single gossip message per fabricated far-future slot grew the set forever, since pruning only drops entries finality has actually passed and finality never reaches a fake slot. The chain actor's own future-slot guard cannot help: the entry is already in the set before that actor ever runs. Adds the p2p-side check, ahead of the dedup insert, using MAXIMUM_GOSSIP_CLOCK_DISPARITY moved to ethlambda_types::beacon::constants (milliseconds, matching every other value in that module, rather than the Duration it was as a blockchain-crate-private constant) since it is now a spec value two crates need. The blockchain crate re-derives its own Duration-typed constant from the shared number instead of defining it. Moves seen_data_columns pruning onto the discovery tick (every 1-5s, independent of peer churn) rather than relying solely on new beacon connections, which a stable, well-scored node might not open for a long time. Extracts the finalized-ancestor check into its own method so it can be unit-tested directly: the existing end-to-end sidecar test could not actually distinguish a correct implementation from one that always accepted, since the checks after it in the pipeline reject the same default-shaped test sidecar on their own. * feat(p2p): fetch the columns a held block is missing Adds the asking side of DataColumnsByRoot: fetch_data_columns on BlockChainToP2P, a Handler<FetchDataColumns> mirroring FetchBlock's peer selection and retry ladder, and a PendingColumnRequest/Columns pending-kind pair for the response routing and COLUMN_LOOKUP_MAX_DURATION bookkeeping. Every fetched sidecar is forwarded through new_data_column_sidecar, the same entry point a gossiped one takes, so the spec's "treat as gossip" requirement holds regardless of how a sidecar arrived. The consumer that decides a block is missing columns and calls this is the next task. * feat(beacon): hold a block until its custody columns are available Fulu blocks imported with fork_choice::DataAvailability::NotRequired unconditionally, so a node accepted a block whether or not the columns it sampled for it were actually present. Gate process_block on this node's custody columns: hold the block (persisted, invisible to fork choice) until every column arrives, ask peers for what's missing, and release it through the same import path a new block takes once the set is complete. Gated by --beacon-da-enforce, on by default. * test(p2p): cover the data-column future-slot check and stop duplicating its clock handle_beacon_data_column had no unit tests, so nothing would catch a regression of the future-slot check back to running after the dedup insert (the exact shape of the bug it fixed). Adds a test pinning the property that actually matters — a rejected far-future sidecar must never enter seen_data_columns — plus one confirming the disparity tolerance is neither zero nor inverted. Verified both by temporarily breaking the check (deleting it, then zeroing the tolerance) and watching the matching test fail before restoring it. Also lifts unix_now_ms into ethlambda_types::time, the same precedent this round already applied to MAXIMUM_GOSSIP_CLOCK_DISPARITY: the p2p and blockchain crates have no dependency on each other but both depend on types, and the function was a verbatim private copy in both. Named it out of constants.rs rather than into it, since that module is documented as holding spec-fixed values, not helpers. Minor: names the discovery-tick constants instead of spelling out their interval in prose, and drops the connection-time seen_data_columns prune, which the tick already makes redundant. * fix(storage): scope a data-column range answer to each slot's canonical root data_column_sidecars_in_range prefix-scanned by slot and filtered only by column index, with no root filter. Gossip import requires a known, finalized-descendant parent rather than a canonical one, so a live fork can leave both siblings' columns stored at one slot; Table::DataColumns is never pruned, so an orphaned sidecar sat there forever and leaked into every future by-range answer covering that slot, violating the spec's requirement that a response be consistent from a single chain. Fixes it by resolving each slot's canonical root through Table::BlockRoots before scanning, the same index get_signed_blocks_by_slot_range and the block-range handler already key off. Confirmed BlockRoots is populated on the beacon path too: BlockChainServer's tick calls fork_choice::get_head (crates/blockchain/src/lib.rs:1737), which writes through Store::update_checkpoints unconditionally on every call, on both chains. By-root needed no change, since the peer names the root directly. Two existing tests assumed the unfiltered behavior (one wrote sibling roots at a slot and asserted both leaked through); updated them and added a test pinning that only the canonical root's columns come back. Verified the new test by reverting to the unfiltered scan and watching it fail before restoring the fix. Also adds a round-trip test for write_data_column_sidecars_response into decode_data_column_sidecars_response (per-item context-digest derivation across a fork boundary, and abort on a genesis_validators_root mismatch), verified failing the same way with each check disabled in turn. Minor: fixes a stale function name in protocols.rs's own doc, and updates has_context's doc for the two column protocols that now set it. * fix(beacon): close the fetch-path verification gap and a dead retry bound A fetched data column sidecar skipped verify_data_column_sidecar: gossip runs it in the p2p actor before forwarding, but a DataColumnsByRoot answer went straight to new_data_column_sidecar, and the chain actor never ran it either. Not an acceptance bypass (c-kzg fails rather than returns false on length-mismatched input, and the inclusion proof pins the commitments), but a cost asymmetry: a peer answering a fetch could force a full KZG batch on garbage gossip rejects for free. Fixes it by running the check in on_gossip_data_column itself, the one entry point both paths already share, so it is a single choke point rather than a second copy on the fetch side. Gossip keeps its own copy where it is, since it still saves a mailbox hop. COLUMN_LOOKUP_MAX_DURATION could never fire: MAX_FETCH_RETRIES exhausts in roughly a hundred seconds (ten attempts, each bounded by the request-response layer's ten-second timeout, spaced by the doubling backoff), and the attempts check ran before the duration check, so the ladder always gave up first. The value came from Lighthouse without checking it against this codebase's timings. No path exists where wall time advances without either an attempt completing or the lookup resolving, so there is no way to make it reachable; deleted it and documented the retry ladder as the actual bound instead. The fetch handler's dedup dropped a second FetchDataColumns for a root already pending, which is only safe if a re-ask's columns are always a subset of what is in flight — an invariant nothing enforces, and one the availability gate (committed after this fetch path) may not honor as columns trickle in. Merges the requested columns into the pending entry instead: a column added this way is not in the request already on the wire, but rides the next retry of this lookup or a fresh fetch once this one resolves. Adds a test for the structural check now running as a shared choke point, verified failing with the check disabled. The dedup-merge fix has no dedicated test, matching how FetchBlock's parallel dedup logic is untested today: both live in an actor Handler that takes a live Context, which this crate has no harness for (see 64a6bce9). * fix(beacon): stop a held block's fan-out from livelocking the import cascade process_block returned a bare Ok(()) for a held block, so process_or_pend_block treated the hold exactly like a genuine import and called collect_pending_children on it. A child block naming the still-held block as parent re-enqueued it via the missing-parent ancestor walk, which re-held it and re-collected the same child, forever, on run_import_cascade's synchronous loop with no yield point. Give process_block an ImportOutcome so only a real import unblocks pending children; a hold now waits for release_block_if_columns_complete instead. Also: sweep blocks_awaiting_columns when finality advances, since nothing else evicted a withheld hold and a gossiped block's claims are not signature-checked before the gate can hold it; make the deneb/electra no-blob-pipeline warning observable per occurrence (fork, root, counter) instead of a single shared one-shot log; guard the vacuous-accept case of an empty custody set; and fix stale ChainActor docs left over from the previous change. * docs(beacon): describe the DAS wire honestly and watch the disk it grows Ten tasks landed custody-column subscription, storage and serving without updating beacon_wire.md or data_storage.md, which still described a node that subscribed to no column subnet and served nothing back. Bring both in line with the code, add the ninth table's metrics entries, and close the two operational gaps the docs surfaced along the way: the never-pruned DataColumns table had no dedicated growth signal an operator could watch before disk fills it, and a beacon follower without --node-key silently custodies a different column set on every restart with no warning next to the custody log that would make it obvious. The growth signal turns out to already exist: lean_table_bytes{table= "data_columns"} covers it, since DataColumns has been in ALL_TABLES since 865ec49a. No new metric needed, just the documentation saying so. * docs(beacon): name constants instead of quoting their values in the DAS wire section Follow-up to 364174b: name NUMBER_OF_COLUMNS, MAX_FETCH_RETRIES and INITIAL_BACKOFF_MS instead of restating their current values, matching house style for prose, and drop a confusing, redundant claim about sidecar re-announcement now that the new subsection already covers cross-seeding directly. * fix(beacon): stop the DAS startup warning and docs from denying what is now served Three strings still described this node as storing/serving no custody columns and as advertising a count wider than what it serves. Both are now false: columns are stored and served, and the advertised cgc is a floor this node's actual custody always meets or exceeds. Rewrite the wire_params startup warning, the ENR cgc doc, and build_metadata's doc to say what remains true (no attestation/sync-committee subnet subscription, no publishing) instead of contradicting the neighboring custody log line. * test(blockchain): cover the release path draining a parked child, and fix the doc it falsified The livelock fix's regression test proves the cascade terminates but never reaches collect_pending_children on release: its structurally-fake block fails real RANDAO verification, so the release attempt takes the Err arm. Add a test that seeds a released root's post-state directly (the same shape an independent import racing the release would leave) and asserts the parked child drains from both pending maps through process_or_pend_block's real "already in the store" branch. Reaching process_block's own Ok(ImportOutcome::Imported) arm from this path needs either a real BLS-signed fulu block (this crate has no signing helper, only blst-backed verification) or process_block's `_ if !is_new` shortcut, which is unreachable here since process_or_pend_block's guard already intercepts a known root before process_block ever runs. Both are documented in the new test rather than faked. Also update docs/beacon_wire.md's live-network transcript: it quoted the startup warning this branch's earlier commit rewrote, plus a now-stale caveat apologizing for wording that commit already fixed. * fix(p2p): answer DAS by-root through the chain-aware block accessor `handle_data_column_sidecars_by_root_request` recovered a block's slot from its root through `Store::get_block_header`, which decodes `Table::BlockHeaders` as lean's fixed-size `BlockHeader`. A beacon directory keeps the whole signed block in that row, so the decode failed inside SSZ and panicked the actor holding it. The eth-4 mainnet follower died that way: a peer asked for columns by root, the swarm adapter exited, and what was left was a process that still served metrics and imported nothing for fourteen hours. A peer's request must not be able to do that, and this handler only ever runs on a beacon store, so it was never going to be right. `block_entry` reads the same fields and decodes per chain, which is what every other caller spanning both chains already uses. The four lean-only accessors now say so themselves. Reaching one from a beacon path used to surface as an SSZ length mismatch that named neither the accessor nor the caller, which is most of why this took a log dive to find; it now names the accessor and, through `#[track_caller]`, the line that called it. * fix(beacon): stop the availability gate deadlocking on its own held blocks A follower running --beacon-da-enforce could not follow mainnet. One slot whose custody columns did not arrive in time stopped the chain permanently, at the tip as much as during backfill: head frozen, and 161 of 161 subsequent gossip sidecars rejected. The cycle is self-sustaining. A held block never reaches on_block, so it never writes a post-state; on_gossip_data_column resolves a sidecar's parent with get_state and dropped the sidecar outright when that returned None; so from the moment block S is held, every sidecar of every child of S fails that lookup and is discarded. The node cannot collect the next block's columns while it waits for this one's, and a column is gossiped once, in its own slot. The specification does not ask for that drop. Its rule is `[IGNORE] The sidecar's block's parent has been seen (MAY be queued for processing once the parent block is retrieved)`, distinct from `[REJECT] The sidecar's block's parent passes validation`. A held parent is neither: it was seen, and it did not fail. So park those sidecars against the parent root instead and replay them when it gains a post-state, bounded by MAX_SIDECARS_AWAITING_PARENT and swept by finality on the same schedule held blocks are. Parking happens ahead of the inclusion-proof and KZG checks, so a replay pays for them once. The reject reason `unknown_parent` goes with it: no path reaches it now, and while it existed it made the deadlock invisible, reading as a verdict on the sidecar when the fault was upstream of it. `awaiting_parent_full` takes its place in the label set, and lean_sidecars_awaiting_parent reports the depth. Second fix, same area: the gate applied to every fulu block at any age. Below the availability boundary no peer is obliged to answer for a column at all (`MAY respond with error code 3: ResourceUnavailable`), so gating there holds a block against data the network is entitled to have dropped. da_check_required_for_slot bounds it to the spec's own max(current_epoch - MIN_EPOCHS_FOR_DATA_COLUMN_SIDECARS_REQUESTS, FULU_FORK_EPOCH), mirroring lighthouse's da_check_required_for_epoch. That boundary is why a_childs_fan_out_does_not_livelock_a_held_parent now builds its store with fulu at genesis: its blocks sit at single-digit slots, which under mainnet's real schedule are thousands of epochs before fulu and so are not blocks the gate has any business holding. * fix(p2p): ask the peers that custody a column, and fetch columns by range Two halves of the same problem: a follower could not obtain a data column it had missed on gossip, so the availability gate had nothing to pass on. Peer selection ignored custody entirely. `fetch_data_columns_from_peer` chose with `pool.choose(&mut rand::thread_rng())`, and at mainnet's CUSTODY_REQUIREMENT a peer holds 8 of 128 columns, so a request for specific columns nearly always came back empty and was charged as a failed attempt. The information to do better is public and needs no handshake: the spec notes that "due to the deterministic custody functions, a node knows exactly what a peer should be able to respond to". So compute each peer's set from its node id and its advertised custody group count, and send each column to someone who holds it, splitting one lookup across several peers and preferring the one already carrying fewest of its columns. The count comes from `metadata/3`, now requested once per connection behind Status, and is seeded from the ENR `cgc` for a dialed peer. Both sources, with metadata authoritative, the way lighthouse splits them; `cgc` outside CUSTODY_REQUIREMENT..=NUMBER_OF_CUSTODY_GROUPS is discarded rather than clamped, since it describes no custody set the spec defines. A peer that has supplied neither is not assumed to custody anything and not assumed to custody everything: unknown means the by-root path still asks it at random, which is all it could ever do. The node id comes from the PeerId itself. libp2p stores a key of 42 bytes or fewer directly in the multihash, so a secp256k1 key reads back out and goes through the same keccak256(uncompressed) our own id does; the round trip is asserted against node_id_from_secret_key, because a divergence would aim every request at the wrong peers silently. Second half: backfill had no bulk path at all. `DataColumnsByRange` was registered inbound-only ("has no sender yet"), so the only source of a historical column was the by-root lookup a block triggers after it has already arrived and been held, one block at a time. The spec names the protocol for exactly this case: "DataColumnSidecarsByRange is primarily used to sync data columns that may have been missed on gossip and to sync within the MIN_EPOCHS_FOR_DATA_COLUMN_SIDECARS_REQUESTS window". Every beacon range batch now sends one alongside its BeaconBlocksByRange, over the same capped span, so the columns are normally stored before their blocks reach the gate. Nothing waits on it: a short, empty or refused answer costs nothing and is not retried, and the by-root path stays as the backstop. Splitting a lookup across peers needed the retry ladder taught what a round is. Each failure used to be its own attempt and schedule its own fan-out, so one unanswered lookup against eight custodians would have become eight retries, then sixty-four. `PendingColumnRequest::in_flight` counts the requests of a round and only the last to report back decides the attempt. That arithmetic is `retire_column_attempt`, split out from the handler so it can be tested without an actor context. * fix(beacon): keep asking for a held block's columns, and dial the peers that have them With the gate on, the follower reached the tip and then stopped on the first block whose sampling columns no connected peer custodied. Every peer answered `DataColumnsByRoot` with an empty list, the lookup spent its retry ladder in seconds, and the hold was left with nothing that would ever disturb it again: `hold_block_for_columns` asks once, and the only other thing that revisits a hold is a sidecar for that exact block arriving on gossip, which by definition was not coming. Head froze while the chain kept moving and the parked-sidecar queue filled behind it. Every peer connected at the time advertised the minimum custody_group_count, so a dozen of them held a small fraction of the columns between them, and two of the eight this node samples were not among them. Three changes, at the three points where that state can end: The chain actor re-drives every held block once a slot, releasing the ones whose columns have quietly completed and re-asking for what the rest still lack. Peers churn constantly, so the ask that fails now is worth repeating. The p2p dedup entry expires. It exists so a second ask for a root in flight merges rather than opening a parallel ladder, which is only correct while the lookup it defers to is alive; past STALE_COLUMN_LOOKUP the entry is replaced, so a round that never reported back cannot swallow the re-drive above. Discovery ranks candidates by the custody columns they would cover before the attestation subnets they would cover. A column nobody holds cannot be fetched at all and stops the chain; an uncovered subnet only narrows the view. The ordering also puts a supernode ahead of everything else while any column is uncovered, which is the fastest way out of that state. * fix(beacon): fetch a backlog's columns by range, not one block at a time A held block at the tip is answered by gossip within a second: this node subscribes to every subnet it samples, and the columns are usually on their way before the block is. A held block well behind the tip has no such path, and the blocks queued behind it are each reduced to their own by-root lookup. One round trip per block is slower than the chain produces blocks, so a follower that falls behind stays behind: measured on mainnet, 1 slot imported per 15s against a 12s slot, with the gap growing. A range ask is also far less sensitive to how the peer set happens to be made up. A peer answers only for the columns it custodies, and most of mainnet custodies the minimum four of a hundred and twenty-eight, so a by-root ask for one column is a lottery over the connected peers; one range answered by one supernode covers every column of every slot in the span. So the per-slot re-drive now also asks for the whole span the node is behind by, whenever a held block is more than BACKLOG_PREFETCH_SLOTS behind the current slot. Nothing waits on that answer and nothing retries it: it either shortens the work the by-root lookups have left or it does not, and the next tick asks again while the backlog lasts. * fix(beacon): stop paying full verification for a sidecar this node already has The backlog prefetch re-asks an overlapping span of slots every tick while the node is behind, and a peer answering the same span twice re-delivers every sidecar in it. During a drain most arrivals are therefore ones already in the store, and each was paying the whole of on_gossip_data_column: a full post-state read for the proposer check, the inclusion proof, the KZG batch and a signature verification. All of that runs on the single-threaded chain actor, which is the same thread the imports the drain is waiting on run on. Measured on eth-4 while 170 slots behind: 684 sidecars a minute arriving against 5 blocks a minute importing, the actor pinned at 91% of a core, and the head losing a slot every 24 seconds against a 12-second slot. A presence check on (slot, block root, index) is one store read and sits ahead of everything expensive. It also covers the gossip case it was always true of: a duplicate on a subnet this node subscribes to. * fix(p2p): let a range prefetch guess at peers that have said nothing The follower stalls for minutes at a time on a fresh checkpoint backfill: a block is held, its columns have no known custodian among the connected peers, and neither fetch path can reach them. The by-root path at least asks someone; the range path asked no one, so the columns only arrived when the peer set happened to churn into a custodian. Observed on eth-4: head stuck at 15194370 and then 15194372 for minutes each, with the by-root ladder giving up on a fresh set of peers every slot. The gap is what "uncovered" means. A column is uncovered when no peer is *known* to custody it, and custody is known only once a peer's metadata/3 answer or its ENR cgc has been read. A peer that has supplied neither is not a peer that lacks the column, and there are always a few: freshly connected, or answering metadata/3 on a version this node's codec rejects. So the uncovered columns now go to a couple of those, and only those. A peer that has said what it keeps has already answered the question, and asking it for what it does not keep is the waste the original comment was right about. Two peers rather than all of them, because a range answer is megabytes when it does land. * fix(beacon): park the sidecars nearest the head, not the ones that arrived first A follower behind the tip receives gossip for the tip continuously, and every one of those sidecars names a parent it does not have yet, so the parked queue fills with blocks it will not reach for minutes. Arrival order then decides what is lost, which means it refuses the sidecars for the block it is about to import while holding the ones it cannot use. Measured on the eth-4 follower about a hundred slots behind a fresh checkpoint anchor: the queue sat pinned at its cap and dropped 2,561 sidecars in ten minutes, while the chain ground through by-root lookups for exactly the slots whose columns gossip had already handed over and this function had thrown away. That is the difference between bursts at twice chain rate and stalls of a minute or two, which is what the head was doing. A full queue now gives up its furthest-ahead entry to admit a nearer one, and refuses an arrival further ahead than everything it holds. One sidecar is lost either way, so the counter moves either way; what changes is which one. * fix(p2p): reserve a third of the connection ceiling for peers we dial The mainnet follower sat at 178 inbound peers and zero outbound ones for two days. Every block waited a median 217s on custody columns no connected peer held, against 6.6s to actually import it, so its head advanced at roughly 13 slots an hour while the chain produced 300. Two things combined to produce that. The inbound allowance was 90% of the ceiling, and `dial_tick` gated on the *total* peer count, so once inbound demand filled the table the dial loop stopped entirely and the 20 reserved outbound slots were never used. A reservation the dial loop stops trying to fill reserves nothing. Outbound peers are the only ones this node chooses, so they are also the only lever it has on its own custody-column coverage: inbound peers arrive at random with respect to what they custody. Inbound is now capped at 70% of the ceiling, and the dial budget takes the larger of the total and outbound shortfalls so inbound saturation cannot suppress dialing. Lean is unaffected: it runs unlimited connections and has nothing to reserve against. Expressing any of that needs connections tagged, so `connected_peers` carries a `ConnectionDirection` per peer rather than being a bare set. That also fixes an accounting drift: a peer holding both an inbound and an outbound connection was attributed on disconnect to whichever socket closed last, so the per-direction connect and disconnect counters could diverge from each other. Three gauges, all set from a full re-count rather than incremented and decremented per event, because a gauge meant to reveal leaks must not be able to leak itself: lean_peers_by_direction peers by which side opened the connection lean_swarm_established_connections libp2p's own counters, which the limits are enforced against, so a connection charged to the cap that no live peer is using becomes visible rather than assumed absent lean_custody_column_peers peers known to custody each column this node samples, where a zero is a stall waiting to happen and is invisible in a total peer count The allowances are derived from the percentage and guarded by three compile-time asserts, including one pinning the direction integer division rounds, so a ceiling that is not a multiple of 100 can never round part of the reservation back to inbound. * perf(p2p): pace dialing on ethrex's curve instead of a flat tick The dial loop ran on a flat 5s tick and dialed a batch of 8, which is 1.6 dials a second. Measured on the eth-4 follower: ~3,360 dials were possible in 35 minutes and 21 happened, while five of its eight sampled columns sat at zero known custodians and blocks waited minutes on columns nobody connected held. The rate is now the tick, not the batch. `dial_interval` is ethrex's `lookup_interval_function` — the easeInOutCubic curve it paces both discv5 lookups and RLPx dialing with — so all three layers ramp the same way: progress 0.00 0.25 0.50 0.75 1.00 interval 20ms 56ms 310ms 564ms 600ms rate 50/s 17.8/s 3.2/s 1.8/s 1.7/s The ceiling is ethrex's `LOOKUP_INTERVAL_MS` exactly, because that end is reached while the table is nearly full and peers still churn: a slower ceiling replaces losses at the rate they happen rather than ahead of it. The floor is 50/s against ethrex's 100ms, since 96% of outbound dials to mainnet never establish and landing the other 4% is a numbers game. Dialing stops outright at target, where `dial_budget` returns 0, so the rate reaches zero rather than merely flattening. `progress` is the *minimum* of the total and outbound-reservation ratios, not `connected / target`. With 140 inbound peers and no outbound ones the total ratio alone reads 0.7 and would pace the loop down to half a second between dials, which is the state that stalled this follower for two days. Taking the minimum holds it at the floor until the reservation fills, mirroring `dial_budget` taking the maximum of the same two shortfalls. Two things follow from a tick that can fire 50 times a second. `MAX_PENDING_OUTBOUND_CONNECTIONS` bounds dials in flight. The connection ceiling and its two halves count only *established* connections, and a dial that never establishes is invisible to them. At 1.6 dials a second that gap was harmless; at 50 it is the dial rate times however long the slowest peer takes not to answer. Four seconds of dialing at full rate, far above what a healthy node has outstanding and still a hard ceiling on file descriptors. `prune_seen_data_columns` gets its own clock. It rode the discovery tick back when that was a steady 1-5s heartbeat, and it costs a store read plus a scan of the whole seen-column set. Inheriting the new cadence would have run it hundreds of times a second on the p2p actor's single thread, to re-derive a boundary that moves once an epoch. * feat(beacon): make the data-availability gate unconditional `--beacon-da-enforce` existed as an off switch because the gate could not follow mainnet: a block whose custody columns never arrived stalled the head behind it until finality cleared it, and that had to be escapable without a rebuild. The column-fetch work on this branch removed that failure mode, so the switch now guards nothing and the specification's own behaviour is the only one worth having. Dropping `da_enforced` from `BlockChainServer` leaves lean's import path untouched. The field is read in one place, the `beacon_block` arm of `process_block`, and lean passed `false` to say "no column evidence to gate on" rather than to disable anything, so the gate is now governed by `da_check_required_for_slot` alone. * refactor(storage): record the anchor slot instead of the earliest custodied one `earliest_column_slot` answered "what is the lowest slot we can serve columns from" by tracking, on the gossip write path, the lowest slot a sidecar had ever been written at. That put a read-decide-write span in `put_data_column_sidecar` whose correctness rested entirely on there being exactly one writer, and it cost a backend round trip on every `data_column_sidecars_by_range/1` request to read a value that cannot change while the process runs. The question it was answering has a fixed answer: where this directory's chain begins. That is the anchor block's slot, so record that instead, once, at bootstrap. `Store` caches it in a field beside `config` and `chain`, for the same reason those are cached. It has to be persisted rather than derived, because `from_db_state` is a constructor too and has nothing to recover it from: `latest_finalized` is seeded to the anchor but climbs away from it at the first finalization. Hence `KEY_ANCHOR_SLOT`, written by both bootstrap paths and read back on resume, and hence the `DB_VERSION` bump: a version 1 directory does not carry the key, and is refused at the version check rather than opened with a defaulted 0 that would have it advertise data it does not hold. Both bootstrap paths take the value from the anchor block's own slot rather than from the anchor checkpoint. On beacon that distinction is load-bearing: `beacon_checkpoint_as_stored` converts an epoch to that epoch's start slot, so a mid-epoch anchor would otherwise record a floor below anything the directory holds. This also fixes `build_status`, which reported `latest_finalized().slot` as `earliest_available_slot` and so understated the servable range by a margin that grew with every finalization. The by-range floor and the `Status` advertisement are now the same value, which is what keeps a refusal from contradicting an advertisement. Callers keep the full range semantics with one behavior change: a request in the window between the anchor and the first column actually custodied is answered with an empty list rather than `RESOURCE_UNAVAILABLE`. A node that range-synced has already backfilled columns alongside blocks across that window, so the two floors converge in practice. * refactor(p2p): ask for a block and its columns in one request The chain actor had three ways to ask the p2p layer for something: `fetch_block` for a root, `fetch_data_columns` for a root's columns, and `fetch_data_columns_by_range` for a span of them. That left the caller choosing which protocol to reach for, which is the p2p layer's decision, and the range one duplicated work p2p already does: every `BeaconBlocksByRange` batch has pulled its own span's columns alongside the blocks since 4854755f, so the chain's per-tick backlog prefetch was a second, worse-paced copy of a request already on the wire. One `fetch_block(FetchRequest)` replaces all three. `FetchRequest` names the root, whether the block itself is missing, and which columns are, so a held block's per-slot re-drive asks for columns without also putting a redundant `BlocksByRoot` on the wire. `needs_block` is explicit rather than inferred from an empty column list: the two cases are disjoint only by coincidence of today's callers, and a p2p layer reading its own store to find out would pay a DB read to re-derive what the caller knew. `new_data_column_sidecar` becomes `new_data_column_sidecars` and takes a batch. Every producer but gossip has one to hand -- a by-root answer carries every column of a block, a by-range answer a whole span -- and sending them one at a time put a mailbox hop per sidecar between the answer and the actor draining the backlog that was waiting on it. Parked sidecars move out of memory. `sidecars_awaiting_parent` held whole `DataColumnSidecar`s, cells and all, for a queue whose size a peer gets to choose; it now holds only the key each one was written under, and the bytes go to a new `Table::PendingDataColumns`. A table of its own, not `DataColumns`: a parked sidecar has passed none of the checks that matter, and `data_column_indices_for` is what the availability gate believes, so an unverified row there would let a peer release a held block with a column it invented. A row moves between the two only by passing every check on replay. `MAX_SIDECARS_AWAITING_PARENT` goes with the move. It existed to bound this actor's memory, which the move already does, and a follower behind the tip hit it constantly: gossip for the tip arrives continuously, so the queue filled with sidecars for blocks minutes away and then shed the ones for the block about to import. What is left bounding the queue is the finality sweep, which bounds how long a row lives but not how fast rows arrive -- `on_gossip_data_column` does not require `parent_root` to name a known block, and the p2p seen-set dedups on header fields a fabricated header chooses freely -- so `lean_sidecars_awaiting_parent` is now the only warning that a peer is parking rows it never means to resolve. Documented on the field, the metric, and in data_storage.md. Two things the move to disk needs. Parking dedups, because the fetch paths have no seen-set and a re-delivery would otherwise strand a key whose row the first replay took. And the actor clears the whole table at startup, since the map it indexes from does not survive a restart and every row written before one is unreachable by construction. * refactor(p2p): derive the connection limits from the peer target A review of the five commits since ee123ae7 turned up one mechanism whose depth was wrong and a handful of duplications. The reservation the dial loop chases was a fixed 60 slots, a share of a connection ceiling that had nothing to do with what the operator asked for. It read as "30% of target" only because the default target and the ceiling were both 200. Set `--discovery.target-peers 50` and the loop kept dialing to 60 outbound peers, past the 50 asked for; set it to 0, documented as "never dial", and the outbound shortfall was still 60 and the loop dialed against an explicit opt-out. `max_connections`, `max_inbound_connections` and `max_outbound_connections` are now functions of `target_peers`, and `SwarmConfig` carries it so `build_swarm` derives the limits libp2p enforces from the same number the dial loop reads. What this node refuses and what it goes looking for are two readings of one number, so a reservation the swarm does not keep can no longer be one the loop chases. A target of 0 now holds no peers at all, which `cli.rs` and `docs/discovery.md` both say instead of "discover and serve, never dial". The share is taken in `u64`: the product overflows `u32` above a ceiling of about 61 million, and a saturated one would silently stop being a percentage. Two of the three compile-time asserts pinned facts that were true by construction of the consts they guarded; with no const ceiling left they are a test over targets that actually exercise the rounding. The dial loop was also paced on a shortfall a network may be unable to close: lean's default target is 200 against a devnet of 32, which held the tick at its 20ms floor for the life of the process, re-drawing a candidate pool of peers it was already connected to, hundreds of times a second on the p2p actor's thread. `dial_tick` now reports whether it opened a dial, and one that did not waits the full ceiling. `lean_custody_column_peers` was refreshed from the two `connected_peers` mutation sites, but its other input `peer_custody` is written by `record_peer_custody`, which for the `metadata/3` path runs after the peer connected. On a stable peer set every column therefore read the zero it had before the peer said anything, which is the exact reading the gauge was added to mean "a stall waiting to happen". It republishes from that writer now, and counts through `columns_custodied_by` so the gauge cannot claim custodians a lookup would not find. The rest is duplication the same five commits introduced. `dial_interval` transcribed ethrex's easeInOutCubic curve, which is `pub`, so it calls it and `DIAL_INTERVAL_AT_TARGET` reads `LOOKUP_INTERVAL_MS` rather than repeating 600. `dial_progress` and `dial_budget` each derived the outbound count and the reservation; they share an accessor. `dial_budget` was called twice per tick. `put_data_column_sidecar` and `put_pending_data_column_sidecar` became byte-identical when the anchor slot moved out of the write path. `ParkedColumn::key` and `forget_parked_columns` were two shims between one caller and one store method. The parked queue deduped by scanning a `Vec` that the same commit uncapped, and is a `HashSet` now. A replayed sidecar was decoded and then re-encoded to bytes it already had. Three doc links pointed at symbols those commits deleted, and the three peer gauges they added were undocumented. * fix(p2p): dial a peer's quic address before its tcp one Both addresses went into one dial and libp2p's default concurrency factor started both handshakes, so the transport a peer ended up on was decided by whichever won the race. That read as a neutral choice between two equal wires and is not one. Mainnet beacon peers answer `na` to a yamux-only proposal, so every TCP win negotiates mplex, and a 20s profile of the eth-4 follower put `libp2p_mplex::io::NotifierWrite::register` at 24.4% of process cycles, the single largest symbol, ahead of the entire beacon state transition. QUIC multiplexes natively and reaches none of that code. TCP was winning 62% of outbound establishments over six hours (3,817 against 2,319) while inbound peers, who choose for themselves, picked QUIC 71% of the time. So the race was steering us onto the expensive transport precisely where we had a say. `dial_concurrency_factor` is pinned to one, which makes libp2p walk the address list in order instead of racing it, and `dial_addrs` already puts `quic` first. The cost is the one the race avoided, now bounded rather than removed: a peer advertising a `quic` port nothing answers waits out libp2p-quic's handshake timeout before its `tcp` address is tried. A quarter of dial targets advertise no `quic` at all and are unaffected, as is lean, whose records carry `quic` alone. That ordering is now load-bearing where it used to be incidental, so it is asserted in a test and both doc comments that told the reader to read nothing into it are rewritten. * fix(blockchain): run the chain actor off the shared tokio runtime This actor's handlers are long stretches of synchronous CPU with no await in them: a state transition, a merkleization, a fork-choice pass. As a task on the shared runtime that starves every other task the runtime is serving, and the P2P actor is the one that suffers, because it owns both this node's sockets and its dial loop. `spawned_concurrency` documents `Backend::Thread` as the backend for CPU-bound work and even ships a `WarnOnBlocking` detector for this exact shape, but that detector is `#[cfg(debug_assertions)]`, so a release build says nothing and every actor here took the default. Measured on the mainnet follower, one node, one build, one fresh-anchor backfill, with this actor pinned at ~100% of a core either way: Backend::Async Backend::Thread dial tick samples 12,494 19,251 ticks over 10s late 5 0 ticks over 0.5s late 16 0 worst sample over 10s 25-100ms mean lateness 23-37ms 1.06ms longest gap in gossip 214.92s 5.28s The whole multi-second tail is gone while the actor works exactly as hard, so this is the cause rather than a correlation: the only thing that changed is which runtime it runs on. The symptom points away from the cause, which is what made it expensive to find. Everything on the P2P actor measures fast, 64,000 handler samples with none over 10ms, which reads as a healthy actor. It was not blocked. It was not being polled. Both chains, from the one `start_actor` that `spawn` and `spawn_beacon` share. Lean's state transition is far cheaper than the beacon one, but leanVM aggregation on an aggregator node is not, and it runs on this same actor with the same P2P actor beside it. The cost either way is a second tokio runtime, which `spawned_rt::threads::block_on` builds on the new thread; on the follower that took the process from 2 non-tokio threads to 19. Verified on the beacon path against a live mainnet node. The lean path is compiled and unit-tested but has not been run on a devnet. * feat(beacon): make the custody group count a flag The follower advertised `CUSTODY_REQUIREMENT` and custodied the columns that implies, with no way to say otherwise. That is the least useful a node is allowed to be, and on a network where a `cgc=4` peer holds 4 of 128 columns it is also what decides whether anyone's lookup finds a custodian: the chance no connected peer holds a given column is `(1 - 4/128)^N`, still better than even at twenty such peers. Running a supernode, or anything above the floor, was a recompile. `--custody-group-count` now sets it, and one value feeds all three places the old constant was read: the custody set via `sampling_size`, the ENR's `cgc`, and the startup line that says what is being advertised. It lives in a new `MainnetOptions` rather than in `CommonOptions`, mirroring `LeanOptions`. Lean has no data-availability domain and omits `cgc` from its ENR entirely, so a flag in the common struct would be one the lean path must carry and ignore. That makes `Network::Mainnet` a tuple variant, which is what its doc comment had been promising would happen once this chain had a flag of its own. Range is `CUSTODY_REQUIREMENT..=NUMBER_OF_CUSTODY_GROUPS`, enforced by clap at parse time rather than clamped: a node that quietly serves a different set than the operator asked for is the failure hardest to notice from outside. Two things worth knowing, both now in `docs/cli.md`. The flag is not the number of columns custodied: `sampling_size` is the larger of it and `SAMPLES_PER_SLOT`, so the default still custodies 8 and the two only converge once it is raised past 8. And changing it changes which columns this node custodies, because the custody set is a function of the node id and the count together, so sidecars already on disk belong to the old set. Nothing is corrupted by that; the node advertises a set it has not finished filling until it backfills the difference. * docs: the mainnet network variant is no longer a unit `--custody-group-count` gave `beacon` its first flag that is not a common one, so `Network::Mainnet` carries a `MainnetOptions` now. The startup section still described it as a unit variant and gave "every flag `beacon` takes is a common one" as the reason. * refactor(metrics): drop the blocks-admitted-without-blob-check counter `lean_blocks_admitted_without_blob_check_total` counted, by fork, what `warn_no_blob_pipeline` already logs on every occurrence with both the fork and the block root: a deneb or electra block carrying commitments that this node admits unchecked, having no blob-and-proof pipeline to source evidence from. Two reports of one event, the narrower of which could not say which block. The warning stays and is unchanged. Going with the counter are its zero-seeding roster `BLOB_PIPELINE_MISSING_FORKS`, the `init` loop that made both forks reachable series, the `inc_` wrapper, and the metric's row in metrics.md and its mention in beacon_wire.md's DAS list. * perf(p2p): poll the peer table off the actor's thread `get_contact_to_initiate` is an actor round trip, and `draw_candidates` awaited up to `DISCOVERY_CANDIDATE_BATCH` of them inline in `dial_tick`, which holds `&mut P2PServer` for the length of a tick. Every refill therefore parked the whole p2p actor -- gossip forwarding, req/resp, swarm events, every other tick -- behind the peer table. Pacing the dial loop on a curve made that worse rather than better: the refill is due whenever the candidate queue runs out, and the loop now reaches its 20ms floor whenever this node is short of peers. The round trips move to a tokio task. It draws contacts, runs the same `LeanFilter::dial_target` admission, and sends already-judged `DiscoveredPeer`s down an mpsc channel; `DiscoveryState` holds the receiver in place of the `PeerTable` and the filter, and the refill is a `try_recv` drain that awaits nothing. Ranking, the dial path and the pacing are untouched, and an error from the table is still read as "nothing right now" and retried, as `draw_candidates` did. The channel is bounded and the task reserves its slot *before* asking, which is the part that matters. ethrex marks every contact tried on the way out, so a contact drawn with nowhere to put it is not offered again and would be lost rather than queued, and a task free to run ahead of the dial loop would draw the table's eligible set down for candidates nobody dials. A full buffer stops it asking instead. The loop keeps refilling only when its queue is empty for the same reason: draining every tick would pour the bounded channel into an unbounded `VecDeque` and there would be no backpressure left. `CONTACT_POLL_IDLE` is how often the task re-asks an empty table. At 250ms it stays under ethrex's own lookup interval, so the crawl remains what decides when a peer becomes available. A slower idle would be visible at startup, where the table is empty, the dial loop used to re-ask on every 20ms tick, and this task is now the only thing asking.
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.