Skip to content

node, cli: opt-in aggregator tunables for stuck-chain operation - #904

Closed
ch4r10t33r wants to merge 1 commit into
mainfrom
node/aggregator-tunables
Closed

node, cli: opt-in aggregator tunables for stuck-chain operation#904
ch4r10t33r wants to merge 1 commit into
mainfrom
node/aggregator-tunables

Conversation

@ch4r10t33r

Copy link
Copy Markdown
Contributor

Summary

Three opt-in operator flags for stalled-chain aggregator behaviour, surfaced by #899. Every flag defaults to the pre-existing behaviour — non-aggregator nodes and healthy-chain aggregators see no change unless the flag is set.

1. `--gossip-attestation-max-age-slots N` (default 0 = off)

Drops gossip attestations whose `data.slot + N < current_slot`. Symmetric to the implicit future-side bound `GOSSIP_DISPARITY_INTERVALS` (`BeamChain.attestationIsTooFarInFuture`).

Implementation:

  • New `AttestationValidationError.AttestationTooOld` variant (deliberately not re-using `AttestationTooFarInFuture` so the pending-attestation buffer's replay logic does not retry too-old entries — there is no expected near-term event that would make a too-old attestation valid again).
  • Block-included attestations skip the check, consistent with the future-side bound.

Useful on devnets where stale peers re-broadcast attestations from finalisation-orphaned slots and pile them into the aggregator input set faster than the aggregator can drain them.

2. `--max-unfinalized-attestation-age-slots N` (default 0 = off)

Periodic non-finalisation prune. Today's `pruneStaleAttestationData(finalized_slot)` only runs from `processFinalizationAdvancement`, so on a chain that never finalises (devnet partition, gossip drop, validator-set issue) the retained maps grow unboundedly and slow every aggregate pass.

The new `ForkChoice.pruneStaleAttestationDataByHeadAge(head_slot, max_age_slots)` evicts `attestation_signatures` and aggregated_payloads entries with `data.slot + N < head_slot` regardless of finalisation state. Called from `submitAggregateOnInterval` once per slot. Finalisation-based pruning is unchanged. No-op when N=0.

New helper `prunePayloadMapByDataSlot` is a sibling of the existing `prunePayloadMapBySlot` that filters by `AttestationData.slot` instead of `target.slot` (the head-age window is independent of where the chain's target checkpoint sits).

3. `--aggregate-concurrent-limit N` (default 1)

Surfaces the `aggregate_group.concurrent_limit` knob currently hardcoded to `.limited(1)` (the #873 invariant). Default `1` preserves the historical behaviour exactly. Larger values let a slow pass not skip the next interval on hosts where each pass is well under one slot (after #900's slot window + #903's ThinLTO + `--rayon-threads`).

Plumbing

CLI args → `pkgs/cli/src/node.zig:NodeOptions` → `pkgs/node/src/node.zig:NodeOpts` → `pkgs/node/src/chain.zig:ChainOpts` → `BeamChain`. The `std.Io.Threaded.init` call in `BeamChain.init` now reads `opts.aggregate_concurrent_limit` instead of the literal `.limited(1)`.

Test plan

  • `zig fmt --check .`
  • `cargo fmt --manifest-path rust/Cargo.toml --all -- --check`
  • `cargo clippy --manifest-path rust/Cargo.toml --workspace -- -D warnings`
  • `zig build test --summary all` — all targets pass
  • `zig build simtest --summary all` — all 15 integration tests pass
  • On devnet: verify a zeam aggregator started with `--gossip-attestation-max-age-slots 32 --max-unfinalized-attestation-age-slots 64 --aggregate-concurrent-limit 1` drops fewer aggregate intervals (`zeam_aggregate_skip_total{reason="in_flight"}`) without regressing block-proposal coverage

Related

  • #899 — parent investigation
  • #900 — slot-window the att_data set (already merged); this PR is its operator-knob sibling
  • #902 — aggregator observability counters (companion in this series)
  • #903 — ThinLTO multisig-release profile + `--rayon-threads` (companion in this series)
  • #873 — original concurrent_limit=1 invariant on aggregate_group

Three new operator flags surfaced by #899 for handling stalled-chain
scenarios. Each defaults to the pre-existing behaviour so non-aggregator
nodes and standard devnet aggregators see no change unless the flag is
set explicitly.

1. `--gossip-attestation-max-age-slots N` (default 0 = off).
   Drops gossip attestations whose `data.slot + N < current_slot`.
   Symmetric to the implicit future-side bound
   `GOSSIP_DISPARITY_INTERVALS` (`attestationIsTooFarInFuture`); we add
   an `AttestationTooOld` error variant rather than re-using the
   future-side variant because the pending-attestation buffer's replay
   logic should not retry too-old entries. Block-included attestations
   skip the check (consistent with the future-side bound). Useful on
   devnets where stale peers re-broadcast attestations from
   finalisation-orphaned slots and pile them into the aggregator input
   set faster than the aggregator can drain them.

2. `--max-unfinalized-attestation-age-slots N` (default 0 = off).
   Adds a periodic non-finalisation prune that runs once per aggregator
   interval. The existing `pruneStaleAttestationData(finalized_slot)`
   only runs when finalisation advances; on a chain that never
   finalises the retained maps grow unboundedly, slowing every
   aggregate pass. The new
   `pruneStaleAttestationDataByHeadAge(head_slot, max_age_slots)`
   evicts `attestation_signatures` and aggregated_payloads entries with
   `data.slot + N < head_slot` regardless of finalisation state. New
   helper `prunePayloadMapByDataSlot` is a sibling of the existing
   target-slot-based prune. No-op when N=0.

3. `--aggregate-concurrent-limit N` (default 1).
   Surfaces the `aggregate_group.concurrent_limit` knob currently
   hardcoded to `.limited(1)` (the #873 invariant). Default `1`
   preserves the historical behaviour exactly; larger values let a slow
   pass not skip the next interval on hosts where each pass is well
   under one slot (after #900's slot window + #903's ThinLTO +
   `--rayon-threads`).

Plumbed end-to-end: CLI args → `NodeOptions` → `NodeOpts` → `ChainOpts`
→ `BeamChain`. The `Io.Threaded.init` call now reads
`opts.aggregate_concurrent_limit` instead of the literal `.limited(1)`.

Related: #899
@ch4r10t33r
ch4r10t33r marked this pull request as draft May 20, 2026 19:31
@ch4r10t33r

Copy link
Copy Markdown
Contributor Author

Closing this PR — the analysis I built it on is stale once #900 lands the slot window.

Why I'm closing

The thesis was "aggregators suffocate on accumulated attestation_signatures / latest_*_aggregated_payloads entries on stuck chains." That was correct when I opened #899, but #900's {current-1, current} slot window changed the cost model:

  • STARK count per pass is bounded to 2 slots, not unbounded.
  • The aggregator no longer re-walks ancient entries even when they sit in the maps.
  • The dominant in_flight skip-rate cause is gone.

That collapses the case for each of the three flags here:

  • --gossip-attestation-max-age-slots: spec deviation (drops attestations the spec accepts) for ~the cost of one map insert, because sig verification still has to run before the age check.
  • --max-unfinalized-attestation-age-slots: spec deviation (drops fork-choice-relevant votes before they reach aggregation) to bound memory on chains that are stuck for hours+. The right fix for a sustained stall is to fix the stall, not to silently delete unfinalized votes; OOM-then-restart is a clearer operator signal than silent eviction.
  • --aggregate-concurrent-limit: spec-clean, but post-Limit aggregate signature builds to active slot #900 the benefit is speculative and the most likely measured outcome (two FFI calls contending for the same rayon pool) is a net regression without --rayon-threads tuning — and even then the gain is unproven.

What remains useful from this series

If a memory cap is wanted later

A bounded MAX_UNFINALIZED_ATTESTATION_DATA_KEEP constant (FIFO-evicting on overflow, in the same shape as MAX_PENDING_BLOCKS / MAX_PENDING_ATTESTATIONS) would be the right primitive — a non-operator-tunable OOM guard rather than a policy knob. Worth a separate, smaller PR if/when this becomes pressing.

@ch4r10t33r ch4r10t33r closed this May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant