Skip to content

feat(aggregation): score interval-2 aggregation jobs like the block builder#509

Draft
MegaRedHand wants to merge 2 commits into
mainfrom
feat/scored-aggregation
Draft

feat(aggregation): score interval-2 aggregation jobs like the block builder#509
MegaRedHand wants to merge 2 commits into
mainfrom
feat/scored-aggregation

Conversation

@MegaRedHand

Copy link
Copy Markdown
Collaborator

🗒️ Description / Motivation

The interval-2 committee-signature aggregation session built its job list with snapshot_current_slot_aggregation_inputs: it aggregated the current slot's raw gossip groups in arbitrary iteration order, ignored existing proofs, and never touched stale-slot gossip. That is safe but blunt — it cannot prioritize the groups whose aggregation most advances consensus, and it spends prover time (bounded by the 750 ms AGGREGATION_DEADLINE) without regard to whether a block could ever use the result.

This PR reworks snapshot_aggregation_inputs into a tiered greedy selector modeled on the block builder's select_attestations, so the aggregator prepares proofs in the same consensus-value order the proposer packs them.

What Changed

  • crates/blockchain/src/aggregation.rs
    • snapshot_aggregation_inputs(store, current_slot) is now a greedy selector: an up-front store pass resolves each candidate AttestationData's material once, then a loop scores candidates and emits at most MAX_AGGREGATION_JOBS (3) jobs against an optimistically-projected state.
    • Ordering key (lower wins): (slot_bucket, tier, dims, data_root) — current-slot groups before stale, then Finalize > Justify > Build, then the block builder's tier-dependent dims, then data_root.
    • resolve_job (reworked from build_job): raw-first + trim. Takes all raw sigs, fills remaining coverage with up to MAX_AGGREGATION_CHILDREN (2) payloads, then trims raw sigs the chosen payloads already cover.
    • Deleted snapshot_current_slot_aggregation_inputs and build_raw_signature_job (subsumed).
  • crates/blockchain/src/block_builder.rs: extracted a coverage-based score_from_coverage core (so score_entry is a thin wrapper, behavior identical) and widened the minimal set of scoring/filter items to pub(crate) so aggregation reuses them.
  • crates/blockchain/src/lib.rs: interval-2 session now calls snapshot_aggregation_inputs(&self.store, slot).
  • crates/blockchain/Cargo.toml: leansig + rand added as dev-deps (real XMSS signatures in unit tests).

Correctness / Behavior Guarantees

  • No-overlap invariant (correctness-critical): the trim guarantees aggregate_mixed never receives the same validator both as a raw participant and inside a child proof (double inclusion corrupts the aggregate). Going raw-first re-introduces that possibility, so the trim restores it; the old children-first ordering got it for free.
  • Realized-coverage scoring: scoring uses the exact validator set the produced proof will attest to (raw_kept ∪ chosen children), not the full proof union, so scores and the projection stay consistent with the ≤2-children cap.
  • Chain-view filtering: candidates are filtered by the block builder's entry_passes_filters against a chain view covering [0, head_slot] — the head root is pushed onto head_state.historical_block_hashes, which by construction omits the head block's own root. Prover time is spent only on aggregations a block could actually pack.
  • Shared scoring: the consensus-value scoring/filter logic is shared with the block builder, so it cannot drift between what the proposer packs and what the aggregator prepares.
  • Behavior change reviewers should know: aggregation is now filtered by justifiability/chain checks (previously the current-slot path was unfiltered). Because fork-choice weight derives from the aggregated-payloads pool, during a finality stall this node will not locally aggregate votes with unjustifiable targets from other clients (ethlambda already clamps its own targets to justifiable slots). The block builder applies the identical filters.
  • Optimistic projection: advancing justified_slots/finalized_slot while selecting jobs is speculative (aggregating ≠ processing a block). It only affects the ordering of prover work within the deadline, never the correctness of a produced proof; it correctly re-tiers same-target candidates across rounds.
  • Unchanged: AggregationJob / AggregationSnapshot / AggregatedGroupOutput shapes, aggregate_job, the off-thread worker, and the deadline/cancellation machinery.

Tests Added / Run

New unit tests in aggregation.rs (11 total in the module):

  • resolve_job raw-first, trim, lone-raw rejection, payload-only merge.
  • Ordering: current-slot before stale; Finalize > Justify > Build within a slot; data_root tiebreak.
  • pick_best_candidate re-tiers a same-target candidate after the first is selected.
  • Job cap = 3; None on empty / all-filtered / lone-raw.
  • snapshot_aggregates_vote_for_current_head_on_non_genesis_chain — regression guard: a candidate voting for the current head on a non-genesis chain (head_slot = 4) is aggregated. This fails against an unextended chain view and passes with the head-root push, covering a bug the original genesis-only fixtures missed.

Verification run:

  • cargo build -p ethlambda-blockchain — clean
  • cargo clippy -p ethlambda-blockchain --all-targets -- -D warnings — clean
  • cargo test -p ethlambda-blockchain --lib — 53 passed, 0 failed
  • cargo fmt -p ethlambda-blockchain --check — clean

Related Issues / PRs

  • Follows the block builder's tiered greedy select_attestations (leanSpec #1149) and reuses its filtering.

✅ Verification Checklist

  • Ran make fmt — clean
  • Ran cargo clippy -p ethlambda-blockchain --all-targets -- -D warnings — clean (crate-scoped; full make lint not run)
  • Ran cargo test --workspace --release — pending; ethlambda-blockchain --lib (53 tests) passing

The interval-2 session aggregated current-slot gossip groups in arbitrary
order and ignored existing proofs, so it could not prioritize the groups
whose aggregation most advances consensus. Rework snapshot_aggregation_inputs
into a tiered greedy selector modeled on the block builder's
select_attestations: an up-front store pass resolves each candidate's
material once, then a loop scores candidates by (current-slot-first,
Finalize > Justify > Build) and emits at most MAX_AGGREGATION_JOBS jobs
against an optimistically-projected state.

Raw-vs-payloads is a within-job decision: resolve_job takes all raw sigs,
fills remaining coverage with up to MAX_AGGREGATION_CHILDREN payloads, then
trims raw sigs the chosen payloads already cover, preserving the no-overlap
invariant aggregate_mixed requires.

Candidates are filtered by the block builder's entry_passes_filters against
a chain view covering [0, head_slot] (head root pushed onto the state's
historical_block_hashes, which omits the head's own root), so prover time is
spent only on aggregations a block could actually pack.

Reuse the block builder's Tier/EntryScore/entry_passes_filters via a
coverage-based score_from_coverage core so the consensus scoring cannot drift
between proposer and aggregator. Deletes snapshot_current_slot_aggregation_inputs
and build_raw_signature_job (subsumed).
Integrates #487 (start aggregation early when 2/3 of subnet signatures
arrive) with the scored selector on this branch. Both the early-start
trigger and the interval-2 tick now route through start_aggregation_session,
which calls the scored snapshot_aggregation_inputs; #487's deleted-on-branch
predecessor snapshot_current_slot_aggregation_inputs stays gone. Kept #487's
800ms deadline (measured from session start), early-aggregation window, and
publish-till-T2 alignment.

Verified on the merged tree: cargo build (blockchain + bin), clippy -D
warnings, and cargo test -p ethlambda-blockchain --lib (56 passed) all clean.
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