feat(aggregation): score interval-2 aggregation jobs like the block builder#509
Draft
MegaRedHand wants to merge 2 commits into
Draft
feat(aggregation): score interval-2 aggregation jobs like the block builder#509MegaRedHand wants to merge 2 commits into
MegaRedHand wants to merge 2 commits into
Conversation
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.
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.
🗒️ 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 msAGGREGATION_DEADLINE) without regard to whether a block could ever use the result.This PR reworks
snapshot_aggregation_inputsinto a tiered greedy selector modeled on the block builder'sselect_attestations, so the aggregator prepares proofs in the same consensus-value order the proposer packs them.What Changed
crates/blockchain/src/aggregation.rssnapshot_aggregation_inputs(store, current_slot)is now a greedy selector: an up-front store pass resolves each candidateAttestationData's material once, then a loop scores candidates and emits at mostMAX_AGGREGATION_JOBS(3) jobs against an optimistically-projected state.(slot_bucket, tier, dims, data_root)— current-slot groups before stale, thenFinalize > Justify > Build, then the block builder's tier-dependent dims, thendata_root.resolve_job(reworked frombuild_job): raw-first + trim. Takes all raw sigs, fills remaining coverage with up toMAX_AGGREGATION_CHILDREN(2) payloads, then trims raw sigs the chosen payloads already cover.snapshot_current_slot_aggregation_inputsandbuild_raw_signature_job(subsumed).crates/blockchain/src/block_builder.rs: extracted a coverage-basedscore_from_coveragecore (soscore_entryis a thin wrapper, behavior identical) and widened the minimal set of scoring/filter items topub(crate)so aggregation reuses them.crates/blockchain/src/lib.rs: interval-2 session now callssnapshot_aggregation_inputs(&self.store, slot).crates/blockchain/Cargo.toml:leansig+randadded as dev-deps (real XMSS signatures in unit tests).Correctness / Behavior Guarantees
aggregate_mixednever 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.raw_kept ∪ chosen children), not the full proof union, so scores and the projection stay consistent with the ≤2-children cap.entry_passes_filtersagainst a chain view covering[0, head_slot]— the head root is pushed ontohead_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.justified_slots/finalized_slotwhile 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.AggregationJob/AggregationSnapshot/AggregatedGroupOutputshapes,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_jobraw-first, trim, lone-raw rejection, payload-only merge.Finalize > Justify > Buildwithin a slot;data_roottiebreak.pick_best_candidatere-tiers a same-target candidate after the first is selected.Noneon 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— cleancargo clippy -p ethlambda-blockchain --all-targets -- -D warnings— cleancargo test -p ethlambda-blockchain --lib— 53 passed, 0 failedcargo fmt -p ethlambda-blockchain --check— cleanRelated Issues / PRs
select_attestations(leanSpec #1149) and reuses its filtering.✅ Verification Checklist
make fmt— cleancargo clippy -p ethlambda-blockchain --all-targets -- -D warnings— clean (crate-scoped; fullmake lintnot run)cargo test --workspace --release— pending;ethlambda-blockchain --lib(53 tests) passing