fix(aggregation): count and report the groups a session drops - #419
Open
mananuf wants to merge 36 commits into
Open
fix(aggregation): count and report the groups a session drops#419mananuf wants to merge 36 commits into
mananuf wants to merge 36 commits into
Conversation
An aggregation session drops a group in four places — target already
justified, no stored state for the target checkpoint, fewer than two
signers, and a prepare error — and every one of them returned silently.
The session then logged produced=0, which reads exactly like an
aggregator with nothing to do.
On devnet-5 that ambiguity hid a healthy aggregator producing nothing
for 355 of 356 consecutive slots while holding 753 signatures: every
group was landing on the missing-target-state path, in microseconds,
with no log line and no metric to say so.
Each drop now increments lean_aggregation_groups_skipped_total by
bounded reason, and the worker line carries the same counts, so a
session that produces nothing says why:
aggregation worker: slot=N produced=0 duration=61µs
skipped=missing_target_state=12
This is diagnosis only — which groups get dropped is unchanged.
Refs #418
…istry A session looked up a stored state per attestation target and dropped the group when that state was absent. On devnet-5 that was every group, every slot: an aggregator holding 753 signatures produced nothing for 355 consecutive slots while its head tracked the chain. The lookup was never necessary. The registry is written once in generate_genesis and never by the state transition, so every state on the chain carries the same validators and the head state — which SnapshotInputs already requires — resolves the same signers. Using it removes the failure mode rather than reporting it. This also drops a store read per data root from the snapshot and the per-target state map it filled. The missing_target_state skip reason goes with it; the condition it named can no longer occur. Refs #418
The lean_attestation_aggregate_coverage_* gauges were registered but never set, so gean published no series for them and could not appear on the shared devnet Aggregation Coverage panels at all. That is half the reason an aggregator producing nothing went unnoticed: the panel an operator checks first is blank for gean whether or not it is working. Emits the same sections and labels the other clients use, so the numbers line up on one panel: timely new payloads captured before the tick promotes them late new payloads for the same round arriving after block votes for the round carried by the canonical head block combined union of the three agg_start_new what the session is about to work from, at interval 2 proposal_combined what our own proposal covers plus the block-vs-timely symmetric difference. Subnet split is validator_id % committee_count, matching p2p.SubnetID. The four post-block sections record a genuine all-zero reading: an empty slot is real information and should read as a dip, not as a gauge holding its last value. The diff gauges instead keep their previous value until a block has reported the round, since before that the comparison is undefined rather than empty. Pure observability; nothing here feeds fork choice or the transition. Refs #418
A single-aggregator devnet leaves the payload buffers empty whenever the emitters read them, so the sections there are legitimately zero and prove nothing. These feed known votes for a round straight into the reporter and assert the block/timely split and the symmetric difference, plus the empty-round and no-head-state paths. Refs #418
feat(metrics): report attestation-aggregate coverage
…stry fix(aggregation): resolve signers from the head state's validator registry
10 tasks
Three comments described behaviour the code does not have. worker.go said groups are proven newest-first; orderedGroups sorts by ascending target slot, deliberately, so a short budget is spent on the lowest unjustified targets and finalization keeps moving. tick.go said the proving gate's proposal priority handles contention with an upcoming proposal duty. The priority flag only blocks the next background Acquire; a session already holding the token runs to completion, so a proposal landing mid-session waits for the whole session. Naming the per-session group cap as the real bound keeps the next reader from trusting a guarantee that is not there. validate.go said admission mirrors the prune predicate. Admission tests head slot plus ancestry from finalized, which is leanSpec's rule; PruneBelow tests Data.Slot for signatures and Target.Slot for payloads. They are three different predicates. Comments only; no behaviour change.
Two counters described a session more kindly than it deserved.
IncProofOperation("aggregation", "success") fired on every session that
reached the end of the worker, including one that dropped every group and
published nothing. On devnet-5 an aggregator produced nothing for 355
consecutive slots while the success rate read 100%, which is precisely
the case the counter exists to surface. A session with no output is now
counted as "empty".
A budget stop defers every group still queued, but skips.add recorded a
single "budget" skip because the break follows it immediately. The count
now covers the whole remaining queue, so the skip summary distinguishes a
session that gave up on one group from one that gave up on thirty.
Adds addN to groupSkips and a test asserting a three-group queue stopped
at the deadline reports three deferrals.
Recursive proofs cost 1.68-2.96s against raw-only proofs at 0.31-0.91s,
measured on a 16-core host with no overlap between the two ranges. With
four aggregators on four subnets, 77% of gean's groups were recursive,
sessions overran the budget on essentially every slot, and the node fell
129 slots behind while the chain forked. ethlambda ran the identical
topology on the same host at 23% recursion and stayed 9 slots behind,
finalizing.
The share is not a property of the network. selectChildProofs ran before
any raw signature was considered, so every group holding a usable child
proof became recursive. Seeding coverage from the raw signatures first
leaves most children with nothing to add, and a child that adds nothing
is never selected.
Three changes, one selection pass:
- Raw signatures claim coverage first; children fill only the gaps.
- Children are chosen greedily on coverage rather than in pool order.
Stored order can take several narrow proofs where one wide proof
covers the same validators, and each extra child is a recursive input
the prover pays for. Ties break on the participant bitfield so the
choice is stable run to run.
- Raw signatures a chosen child already covers are trimmed. This is
required, not an optimisation: child-first ordering was what
guaranteed a validator never appeared both as a raw participant and
inside a child, and reversing the order removes that guarantee.
This diverges from leanSpec's aggregate(), which selects children first
and fills with raw. The output is a single valid aggregate over the same
participants either way, so ordering is an implementation freedom; the
spec models no cost. ethlambda's resolve_job makes the same choice.
Group ordering is untouched: orderedGroups still runs frontier-first by
ascending target slot to keep finalization moving under a short budget.
Raw-first selection removes most recursion, but it does not bound what a single group can reach for. A group with little local raw coverage still folds in every child that adds validators, and each one is a recursive input: 1.68-2.96s measured against 0.31-0.91s for raw-only groups, with no overlap between the ranges. Two children is the same value ethlambda and lantern settled on, both noting recursion as the cost to bound. The cap counts children already in the slice rather than per call, so it holds across the separate new-payload and known-payload passes that build one group's inputs. Selecting from the wider pool first still applies: greedy coverage means the two children kept are the two that cover most.
A session's cost was whatever the backlog cost. With four aggregators covering four subnets that saturated a 16-core host: the node fell 129 slots behind, the chain forked one slot after justification stopped, and essentially every session overran its budget. The wall-clock budget cannot prevent this on its own. It is checked between groups, so it bounds when a session stops starting work, not what a session may cost in total, and the first group always runs. A count is the cruder bound but the one that holds before any proof begins. Two groups per session, dropping to one in the slot before this node proposes. The proving gate gives proposals priority, but priority only defers the next background acquire: a session already holding the token runs to completion and the proposal waits for it, so the cap is what bounds that wait. The cap counts proof attempts rather than loop iterations, since groups dropped for a justified target or too few signers never reach the prover and cost nothing. Groups it defers are reported as session_cap, kept separate from budget so the skip summary distinguishes "ran out of time" from "reached the limit". The proposer lookahead reuses the head state dispatchAggregationCycle had already decoded, via a new proposingAt helper, rather than calling getOurProposer and paying a second SSZ decode on the tick loop.
SessionBudget is two intervals measured from the moment the worker acquires the prover. Its own comment gives the reason as leaving interval 4 free for results to be promoted and gossiped, so the real constraint is a boundary, not a span. Expressing it as a span is only correct when the session starts exactly at interval 2. It does not. Two cases came out wrong: The early path dispatches in late interval 1 to give the slow proof a head start. Starting at 0.8s and adding 1.6s put its deadline at 2.4s, mid-interval 3, when it could safely have run to 3.2s. The head start the path exists to create was handed straight back. workerStart is taken after the gate is acquired, so a session that waited on the prover got a full budget from whenever it won the token. Waiting 700ms then running 1.6s ends at 3.9s, past the promotion the aggregate was produced for. The dispatcher now computes the deadline from the slot clock and passes it on Dispatch; the worker falls back to SessionBudget when it is unset. Both dispatch paths land on the interval-4 boundary, and gate waiting comes out of the window instead of extending it. The overrun log reports the window actually allowed rather than the nominal constant. The deadline is derived from the tick's own timestamp rather than a fresh clock read, so it is exactly the boundary and does not drift by however long the tick took to reach the dispatch. recovery.go already treats aggregationDispatchOffset + SessionBudget as the end of the window; that stays equal to the interval-4 boundary.
observe divided a group's wall time by rawCount+childCount, so a child
proof and a raw signature cost the same unit. They do not: a group of ten
signatures ran 0.6s while the same ten plus one child ran 2.3s. Folding
both into one average left the estimate roughly twenty times wrong for
each population, and maxUnitsWithin then sized every group by a figure
that described neither.
The estimator now tracks perRawSeconds and perChildSeconds. A raw-only
group prices the signature directly; a group carrying children attributes
the raw share at the current raw estimate and charges the residual to the
children. That is well conditioned because raw-only groups are the common
case once selection is raw-first. A group cheaper than its raw share
alone teaches nothing about its children and is ignored rather than
driving the estimate negative.
Selection charges a child childUnitCost in raw-signature units instead of
one. Two exemptions keep the price from starving the thing it is meant to
protect:
- Children admitted while rawCount+children is below two, since the
group is not yet spec-viable and charging for them could leave it
unable to produce anything at all.
- The first child of any group. Raw-first selection means a chosen
child only ever covers validators no raw signature reaches, so those
votes have no fallback; pricing that child out under a tight budget
would defer them every session for as long as the pressure lasts.
Everything after that is charged. Worst case per session is still one
in-flight proof, since the deadline check between groups stops the next
one.
aggregatedPayloadCap and newPayloadCap were both 0, so the FIFO eviction in PayloadBuffer.Push has never run: it is gated on capacity > 0. AttestationSignatureMap had no capacity field at all. That is survivable only while finalization advances, because PruneOnFinalization is the sole path that clears these three pools and it runs on finalization alone. PeriodicPrune, the fallback for a stall, prunes non-canonical states and blocks and touches none of them. So the one situation that makes the pools grow without limit is also the one that switches off the only thing that empties them, which is the shape seen on devnet-5. Caps are stall insurance rather than an operating limit; a healthy node never approaches them between prunes. Signatures evict whole data roots oldest-first rather than individual signatures, so a surviving root still carries every vote it collected, which is what an aggregate needs. This does not reduce resident memory. A node at 3.15 GB RSS reported 102 MB held by the Go runtime, so these pools are not where the memory is; the prover's Rust allocations are. The caps remove a worst case, they do not move today's number.
Insert appended without checking whether the validator had already voted for that attestation data, and nothing upstream deduplicated. Gossip meshes deliver the same attestation more than once, and replayPendingAttestations re-enters onGossipAttestation for every buffered vote once its head block arrives, so the same signature was stored repeatedly. Two costs. Each duplicate is a SignatureSize array kept for nothing. And SignatureCountForSlot sums len(entry.Signatures), so a duplicate inflates it — that count is compared against ceil(2n/3), a threshold over distinct validators, to decide whether to pull the aggregation session forward. Verification is the expensive half: an XMSS check costs hundreds of milliseconds, and a duplicate cannot change its outcome. The store's own record of what it holds serves as the seen set, so it needs no separate cache and is pruned along with the signatures.
GetState deserializes the whole state from SSZ on every call, and dispatchAggregationCycle paid for it twice: once in the guard that only checked the state was present, then again inside SnapshotInputs. Both on the tick goroutine, which also imports blocks and updates the head. SnapshotInputs now takes the state the dispatcher already resolved. With the proposer lookahead in the session-cap commit reusing the same state, a dispatch decodes it once. Deliberately not adding a state cache. States are content-addressed and immutable in principle, so an LRU keyed by root looks free, but StateTransition mutates its argument in place and both blockprocessor.Process and the proposal path run it directly on what GetState returned. Handing those callers a cached pointer would let them corrupt the entry for every later reader. A cache needs either copy-on-read or a separate read-only accessor, which is a change of its own rather than a line in this one.
selectProofs ran its greedy loop until no proof added coverage, and handed the whole slice to Merge. Merge builds a proof from children alone, with no raw signatures to anchor it, so every proof it receives is a recursive input and the result is the most expensive shape the prover produces. It runs on the proposal path, holding the proving gate at priority, where an overrun delays the block itself. Capping the aggregation session while leaving this unbounded would move the cost rather than remove it. Two, matching the per-group cap aggregation now applies. Not attempting the further heuristic of preferring a single proof when merging adds little coverage: "little" needs a threshold, and there is no measurement behind one yet.
maybeEarlyAggregate compares the votes collected for a slot against ceil(2n/3) of the entire validator registry. A validator votes on the subnet given by its index modulo the committee count, and a node only receives the subnets it joined, so an aggregator covering a subset can never reach that threshold however complete its view of its own subnets is. The early path then never fires for it, and the proving head start that path exists to give is never taken. The threshold is now measured against the validators assigned to the subnets this node actually subscribes to. With one committee, or an aggregator subscribed to every subnet, that is the whole registry and nothing changes; the devnet runs so far have all been in that shape, which is why this stayed hidden. AggregateSubnetIDs reached p2p but never the engine, so it is now a field main sets after New.
Two problems, one cause. The signature map pruned on Data.Slot while both payload buffers pruned on Data.Target.Slot, so each kept data roots the other had dropped. The target is the correct key: it decides whether a vote can still advance finality, it is what lantern and ethlambda's payload buffer use, and it is what orderedGroups already reads. A malformed entry without a target falls back to the attestation slot, matching orderedGroups. More importantly, all three pools are pruned only by PruneOnFinalization, which runs when the finalized slot advances. PeriodicPrune, the existing stall fallback, prunes non-canonical states and blocks and touches none of them, requires finalization to be more than two pruning intervals behind, and fires only on exact multiples of that interval, so one skipped slot costs another interval. The single situation that makes the pools grow without limit is therefore the situation that switches off everything that empties them, which is the shape observed on devnet-5: target_justified skips climbing past 1,600 while nothing pruned. PruneStaleAttestationPools sweeps against a head-relative cutoff instead, run each slot at interval 3 alongside PeriodicPrune. It is a no-op below the finalized slot, so a healthy node never pays for it and PruneOnFinalization keeps owning that range. Data roots holding an aggregated payload keep their raw signatures, since those are the coverage a live aggregate was built from; ream and grandine both protect their head-relative sweeps the same way.
orderedGroups sorted purely by ascending target slot. The reasoning behind that is sound and stands: finalization advances only when the checkpoint after the current source is justified, so a short budget is best spent on the lowest unjustified targets. It just is not the whole ordering. This slot's votes are the only ones with a deadline — they have to be aggregated and gossiped in time to reach the next block, while a backlog entry loses nothing by waiting a slot. With a session now capped at two groups, sorting by target alone would spend both on the oldest backlog and leave the current slot's own votes unaggregated, every slot, for as long as a backlog exists. Current-slot groups first, then the existing frontier rule within each tier. ethlambda's scorer makes the same first cut for the same reason: the slot's committee aggregate is the one piece of work with a deadline. This is the smaller half of the change discussed. The full tiering — Finalize before Justify before Build, scored against a projection of the head state — can follow; it needs a projection this pass does not build.
Pruning the signature map removed each stale root from the insertion order individually, rescanning that slice per root. Quadratic exactly when a sweep has the most to drop, which is the stall case the sweep was added for. The order is now rebuilt in one pass after the map is filtered. Anchoring the deadline to the slot means waiting on the prover, or behind a session that overran, can consume the whole window before this one starts. The session then ran with an expired deadline, proved nothing, and reported "hit budget without output" — the starvation warning that surfaced the estimator latch, raised for a session that never had time to begin with. Such a dispatch is now skipped with its own log and result label, so the starvation alarm keeps meaning what the team reads it to mean.
Two more found reviewing the branch. expectedVotersPerSlot counted validators on every call, and it is called once per verified attestation via the early-aggregation wake-up. That is a loop over the registry per arrival, sitting directly beside the comment explaining that numValidators is cached to avoid exactly that. Its inputs are fixed once the registry is known, so it is now cached the same way. The stale sweep exempted data roots holding an aggregated payload, following ream and grandine, on the reasoning that those signatures are the coverage a live aggregate was built from. That exemption cannot do anything here: a root's signature entry and its payload entry hold the same AttestationData, so they carry the same target slot and go stale in the same sweep. Nothing is ever exempt. Removed the protected set, the Roots accessor added to serve it, and the ordering constraint it imposed; a test now asserts the two leave together.
aggregationDeadline subtracted the slot position from the interval-4 offset and only then checked whether the position was past it. These are unsigned milliseconds, so for a position at or beyond the boundary the difference wraps to an enormous value and the conversion to a Duration overflows. The guard overwrote the result, so nothing observable went wrong, but the ordering is a trap for the next edit. Handle the out-of-range case first and subtract only where the result is known to be positive.
The cap counts proofs, and PushData adds entries carrying none, so the data-only entries block import creates are invisible to it. On devnet-5 most of roughly 2,900 known entries were of that kind. The previous comment implied the cap bounded the buffer as a whole. The weighting is right: an entry is an AttestationData of a few hundred bytes while a proof may reach 512 KiB, so proofs are what bound memory, and entry growth is bounded by pruning instead. Only the description was wrong. Both constants now cite the numbers they were chosen against — roughly 2,300 signatures held between prunes on a devnet-5 aggregator, and a live proof count in the tens at eight committees — rather than reading as round numbers with no provenance.
Measured on the 16-core host with four aggregators running, a group of two raw signatures took 2.0-5.2s and produced ~146 KB of proof. Carrying more signatures barely moved either figure: the cost is the proof, not what it covers. The estimator assumed otherwise. observe divided a group's duration by its signature count and read the result as a per-signature price, so a 4.2s two-signature group recorded 2.1s per signature. maxUnitsWithin then computed 1.6s / 2.1s = 0 and returned its floor of two, and every later group carried exactly two signatures. The estimate confirmed itself: small groups look expensive per signature, which keeps the next group small. The cost of that is paying 2-5s for a proof covering two validators when the same proof could have covered every signature the group held. Twice the groups for the same coverage, and the surplus lands in the backlog — 333 groups deferred in a single observed session, with the frontier groups finalization needs among them. Raw signatures are no longer rationed. What is rationed is proofs, which is what actually costs: the per-session group cap and the deadline. Cost is now modelled as perGroupSeconds + children x perChildSeconds, where a raw-only group prices the fixed term directly and a group carrying children charges them whatever that term does not explain. This removes maxUnitsWithin, childUnitCost, perRawSeconds and seedPerRawSeconds; observe folds into observeGroup, which already measured whole-group wall time and was already called. Children keep their price, charged in wall time against the window rather than in signature-equivalent units, and keep both exemptions. Three tests asserted the two-signature floor as expected behaviour and now assert the full signature set.
… raw Removing the per-signature budget rests on proof cost being independent of how much a proof covers. Measured across four aggregators: ~146 KB from two signatures through four, ~170 KB from five through eleven. A step function, nearly flat within each step, so eleven signatures cost 16% more proof than two while carrying five and a half times the coverage. The distribution also shows what the floor was doing: 2,159 groups at exactly two signatures against single digits at every other size. The steps are logarithmic, so a group covering a 512-node network stays inside the 512 KiB proof ceiling, and past it the prover returns ErrProofTooBig and the group is skipped rather than failing unsafely.
fix(aggregation): recover from over-budget cost estimates
lean_tick_interval_duration_seconds is observed inside onTick, so it
records nothing at all while the dispatch loop is blocked: the failure it
should report makes it go quiet rather than spike. Its top bucket is 1.6s
besides, so it could not size a stall even when one ends.
Add lean_tick_last_age_seconds, written from outside the loop, and
lean_dispatch_event_duration_seconds{event} with buckets to 600s so a slow
handler is attributable rather than only visible as a late tick.
updateFinalizedFromHead called FC.Prune before PruneOnFinalization. FC.Prune removes the finalized-below ancestors and every losing branch from the ProtoArray; PruneOnFinalization then asks that same array which roots to delete, via GetCanonicalAnalysis. After the prune the parent walk terminates at the new root, so canonical has length 1 and canonical[1:] is empty, and the siblings are already gone so nonCanonical is empty. Both delete lists came back empty every time. The effect is that TableStates and TableBlockHeaders were never pruned at all. Only pruneLiveChain worked, because it scans by slot rather than going through fork choice. On devnet-5 that left 13,184 states and ~22,600 headers after thousands of finalizations, which is what made the full-table scans on the dispatch loop expensive enough to stall the slot clock. Take the delete lists before pruning fork choice. The regression test fails on the old ordering with all three symptoms.
The dispatch loop is one goroutine and it keeps the slot clock. Four full-table Pebble scans ran on it, all O(chain length). At ~22,600 blocks the mean tick interval reached 38-288s against an 800ms target, with 49% of CPU in pebbleIterator.Next and 24% in snappy.decode. Every duty the clock drives - propose, attest, aggregate, head update, prune - stopped. EstimateTableBytes iterated every entry of every table and copied keys and values, and ran for all six tables on each imported block. Ask Pebble's version metadata for the range instead, and sample it off the loop. The number changes: SST bytes for the range, not logical live bytes, and it excludes the memtable. StatesCount scanned 13,184 states once per slot to print one log line. Removed. MaxStoredBlockSlot scanned and SSZ-decoded every header up to three times a slot for the duty gate. It is a high-water mark now, raised on both write paths and seeded by one scan per process. Seeding matters: a fresh store reporting 0 would read the whole chain as a network stall. It deliberately still covers pending headers, not just imported ones - a live-chain index would miss a persisted pending block and let a restarted node resume duties on a stale head. BlockRoots materialised every root into a map per proposal to answer a handful of membership questions. KnownRoots is a predicate now. Also time every dispatch event and publish tick age, both off the loop.
gean applied the sync-lag duty gate to aggregation as well as to block production and attestation, reasoning that aggregates built on a stale view get dropped anyway. That holds with several aggregators. It inverts with one: the sole aggregator withholds the aggregates the network is waiting on at exactly the moment it is furthest behind, so nothing justifies, nothing finalizes, finalization pruning never runs, and the lag that closed the gate gets worse. Observed on devnet-5 as not_synced climbing to ~4,100 per node with justification frozen; stopping two lagging nodes advanced justification 1,520 slots in five minutes. leanSpec gates interval 2 on is_aggregator alone. ethlambda has a duty gate and applies it to attestation and proposal but not to aggregation. lantern has no gate. Only ream gates aggregation. The work stays bounded without the gate: a session has a slot-anchored deadline and MaxGroupsPerSession, so an aggregate built on a stale view costs one bounded proving budget and is dropped by peers, which is strictly better than producing none. Drops the not_synced skip reason with it - nothing can emit it now.
Both found in review of #423. A failed seeding scan latched permanently. scanMaxStoredBlockSlot returned 0 for an unopenable read view, and a mid-iteration failure was indistinguishable from reaching the end of the table, because Next reports false either way. sync.Once then cached that understated mark forever. It is not just a bad gauge: an understated watermark inflates the duty gate's computed network lag, which can trip the network-stall carve-out and let the node resume duties on a stale head - the behaviour the carve-out exists to prevent. Iterator grows an Err method so a truncated scan is distinguishable from a complete one. The seed reports errors, latches only on success, and is now run at startup before the dispatch loop, which also removes the last full-table scan from the duty path. The storage sampler could outlive shutdown. waitForShutdown cancels, sleeps 500ms, and returns; backend.Close then runs from a defer with nothing joining the workers. A sampler still mid-round calls into a closed Pebble instance, which panics. Track the storage-reading goroutines on a WaitGroup and join them in main before Close. Cancellation checks between tables help responsiveness but are not what makes it safe. Tests cover both: a truncated scan that must not latch, and a wait that must block until the sampler returns.
The previous test asserted recovery by constructing a second ConsensusStore, whose seeded flag is unset regardless of whether the first store latched its failure. It would have passed against the bug it claims to cover. Seed twice on the same store, against a backend that truncates the first block-header scan and then behaves normally. The truncation yields one real entry before failing, so the partial answer is a plausible lower slot rather than an obvious zero - the shape that would otherwise be cached as a reasonable-looking watermark. Verified both tests fail when the seed latches on failure and pass when it does not. Also narrows the WaitForStorageWorkers documentation to what it actually covers. The aggregation, proposal, recovery and attestation workers and the fetch batcher all read storage and none is joined; that is a pre-existing shutdown gap, and closing it means deciding how long shutdown may block on in-flight proving.
fix(node,store): take the full-table scans off the dispatch loop and repair finalization pruning
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
3 participants
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.
Step 1 of #418: make the aggregator say why it produced nothing.
The problem
An aggregation session drops a group in four places, and every one returned silently:
orderedGroupsaggregateFromSnapshotaggregateFromSnapshotaggregateFromSnapshotThe session then logged
produced=0— indistinguishable from an aggregator with nothing to do.On devnet-5 (2026-08-31, 14:35–15:04 UTC) that ambiguity hid
gean_7producing nothing in 355 of 356 slots while healthy (Behind: 0–3) and holding 753 signatures. Every zero run finished in 14–90 µs, against 130–830 ms for a working session — so no group was ever attempted. Nothing in the logs or metrics said why, which is why the operator could only report "it wasn't aggregating any signatures".What this changes
Each drop increments
lean_aggregation_groups_skipped_total{reason}with bounded reasons —missing_target_state,target_justified,too_few_signers,budget,error— and the worker line carries the same counts:Diagnosis only. Which groups get dropped is unchanged; this makes the existing behaviour observable. Removing the dependency on per-target stored state is step 2 of #418 and deliberately not in this PR — it changes what gets aggregated, and it should land on top of instrumentation that can prove it worked.
Testing
make test— all 25 packages green;make lintandgo vetclean;-raceclean onaggregationandnode.