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

[MegaRedHand]

What

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 aggregation material once (raw-first + trim), then a loop scores
candidates by (current-slot-first, Finalize > Justify > Build) against an
optimistically-projected state, emitting at most MAX_AGGREGATION_JOBS jobs.

Why

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.

How

• 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 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.
• Reuses the shared ProjectedState (from_head_state + advance) and
  Tier/EntryScore/entry_passes_filters via a coverage-based
  score_from_coverage core, so proposer and aggregator can never drift on
  either the justify/finalize projection or the tiering.
• Deletes snapshot_current_slot_aggregation_inputs and
  build_raw_signature_job (subsumed).

Stacking

Stacked on #525 (the block builder refactor). Review/merge that first; GitHub
retargets this PR to main once it lands.

Checks

• cargo fmt clean
• cargo clippy --all-targets -- -D warnings clean
• cargo test -p ethlambda-blockchain --lib green (incl. resolve_job / pick_best_candidate / snapshot tests)

[github-actions]

🤖 Kimi Code Review

Overall Assessment: High-quality PR. The refactoring correctly unifies scoring logic between block building and aggregation, fixes a critical chain-view bug for current-head attestations, and properly prevents XMSS double-inclusion. Good test coverage.

Critical Consensus Correctness

• Item 1: Chain view extension for current-head votes (aggregation.rs:254-256)
  Correctly extends historical_block_hashes with store.head() to cover the tip at index head_slot. Without this, attestation_data_matches_chain would reject valid attestations targeting the current head (since process_block_header only pushes the parent root). The regression test snapshot_aggregates_vote_for_current_head_on_non_genesis_chain validates this fix.

• Item 2: Double-inclusion prevention in XMSS aggregation (aggregation.rs:481-486)
  The resolve_job function correctly trims raw signatures whose validators are already covered by selected child proofs. The comment explicitly notes that aggregate_mixed must never receive a validator both as a raw participant and inside a child. This is security-critical for XMSS signature validity.

• Item 3: Justified target filtering (aggregation.rs:400-403 via entry_passes_filters)
  Correctly filters out attestations targeting already-justified/finalized slots. This prevents wasting prover cycles on attestations that cannot contribute to fork choice.

Code Correctness & Edge Cases

• Item 4: Duplicate validator ID handling in resolve_job (aggregation.rs:459-467)
  If validator_sigs contains duplicate validator IDs (same vid appearing twice), raw_by_id.insert(*vid, ...) will silently overwrite the first signature with the second. While keys_to_delete correctly captures both for cleanup, the aggregation loses the first signature. Consider using HashMap::entry or asserting uniqueness if the protocol guarantees no duplicates.

• Item 5: Viability threshold consistency (aggregation.rs:496-501)
  The logic rejecting lone raw signatures (raw_ids.len() <= 1 && children.is_empty()) and insufficient children (raw_ids.is_empty() && children.len() < 2) is correct and consistent with the PR description. This prevents spawning useless aggregation jobs.

Performance & Resource Management

• Item 6: Historical hashes cloning (aggregation.rs:254)
  extended_historical_block_hashes clones the entire historical_block_hashes vector. For long-running chains, this is O(n) in chain length. Consider whether an Arc<[H256]> or slice view could be used, though likely acceptable given Ethereum's current historical root window.

• Item 7: MAX_AGGREGATION_JOBS cap (aggregation.rs:164)
  Capping at 3 jobs is a sensible DoS protection against unbounded prover work per slot.

Rust Best Practices

• Item 8: Error handling with expect (aggregation.rs:254-255, 263)
  The uses of expect for store.get_block_roots() and store.head() are acceptable only if these are truly invariant violations (store must be initialized). Ensure these cannot be triggered by adversarial input or corrupted state.

• Item 9: Test signature caching (aggregation.rs:806-817)
  The dummy_sig() helper correctly uses std::sync::LazyLock to cache the XMSS signature across tests, avoiding expensive re-generation. Good pattern.

Code Structure

• Item 10: Shared scoring logic (block_builder.rs:449-478)
  Extracting score_from_coverage and making it pub(crate) is a good refactor that ensures aggregation and block building can never drift on tiering logic (Finalize > Justify > Build).

Minor Suggestions

• Line 462 (aggregation.rs): The data_root parameter in resolve_job is unused except for keys_to_delete construction. This is fine, but could be documented.

• Line 624 (aggregation.rs): select_proofs_greedily now takes seed_covered: HashSet<u64> by value (moved). This is correct since the set is consumed, but document that the caller loses ownership.

Security Summary: No critical vulnerabilities introduced. The XMSS double-inclusion guard (Item 2) and chain-view fix (Item 1) are correctly implemented. The MAX_AGGREGATION_JOBS cap provides DoS resistance.

Approval: Approve with minor suggestion to handle duplicate validator IDs explicitly in resolve_job if the input isn't guaranteed unique.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

[github-actions]

🤖 Codex Code Review

1. High: raw XMSS inputs lose the required validator ordering in aggregation.rs. resolve_job() first copies validator_sigs into a HashMap, then rebuilds raw_pubkeys / raw_sigs / raw_ids by iterating that map at aggregation.rs. The storage layer explicitly documents that gossip signatures are kept in ascending validator order because XMSS aggregation requires it at store.rs, and the snapshot currently preserves that order via BTreeMap::iter() at store.rs. Replacing that with HashMap iteration makes the raw component order nondeterministic, which can yield invalid proofs or flaky behavior. Preserve the original validator_sigs order and use a side set/map only for trimming.

2. Medium: the early-aggregation path now snapshots the full pool, including stale groups and payload-only merges, at lib.rs and aggregation.rs, but an early-started session still suppresses the normal T2 session for that slot at lib.rs and is only started once per slot at lib.rs. Combined with the 3-job cap at aggregation.rs, this means signatures that arrive later in the same slot can no longer be picked up by a fresh T2 snapshot, while stale/payload-only work can consume the slot’s only aggregation session. That is a liveness regression for current-slot attestation coverage. I’d keep the early path current-slot-only, or allow a second refresh at T2.

3. Low: leansig and rand were added as normal dependencies in crates/blockchain/Cargo.toml, but they are only used by the test helper dummy_sig() under #[cfg(test)] at aggregation.rs. These should be dev-dependencies to avoid expanding the production dependency graph and compile surface.

I did not run tests locally: cargo is blocked here because the pinned Rust toolchain tries to write under a read-only ~/.rustup, and the environment has no network fallback.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[greptile-apps]

Greptile Summary

This PR adds value-based selection for interval-2 aggregation jobs. The main changes are:

• Resolves raw signatures and reusable proofs before selection.
• Filters and scores candidates against a projected consensus state.
• Prioritizes current-slot and higher-value jobs with a three-job cap.
• Shares coverage scoring with the block builder.
• Adds tests for resolution, ordering, projection, and snapshot behavior.

Confidence Score: 4/5

The interval-2 storage error path needs a fix before merging.

• Candidate resolution, coverage scoring, and projected-state updates preserve their main invariants.
• A failed live-chain root read currently panics from an optional background aggregation session.

crates/blockchain/src/aggregation.rs

Important Files Changed

Filename	Overview
crates/blockchain/src/aggregation.rs	Adds material resolution and greedy job selection, but a fallible block-root read can panic the actor.
crates/blockchain/src/block_builder.rs	Extracts shared coverage scoring while preserving proof-union scoring for block construction.
crates/blockchain/src/lib.rs	Routes interval-2 sessions through the new slot-aware aggregation selector.
crates/blockchain/Cargo.toml	Adds development dependencies used by the new aggregation tests.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Interval-2 aggregation trigger] --> B[Snapshot gossip and payload groups]
    B --> C[Resolve raw signatures and child proofs]
    C --> D[Build canonical chain view]
    D --> E[Filter and score candidates]
    E --> F{Candidate available and fewer than 3 jobs?}
    F -- Yes --> G[Advance projected state]
    G --> H[Add aggregation job]
    H --> E
    F -- No --> I[Run aggregation worker]
    I --> J[Store produced payload]
    J --> K[Delete represented gossip signatures]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Interval-2 aggregation trigger] --> B[Snapshot gossip and payload groups]
    B --> C[Resolve raw signatures and child proofs]
    C --> D[Build canonical chain view]
    D --> E[Filter and score candidates]
    E --> F{Candidate available and fewer than 3 jobs?}
    F -- Yes --> G[Advance projected state]
    G --> H[Add aggregation job]
    H --> E
    F -- No --> I[Run aggregation worker]
    I --> J[Store produced payload]
    J --> K[Delete represented gossip signatures]

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
crates/blockchain/src/aggregation.rs:299
**Block-Root Read Panics Actor**

When the storage backend fails while enumerating live block roots, this `expect` panics during the interval-2 actor path. A transient database read error can therefore terminate the node instead of skipping this optional aggregation session or returning an error.

_{Reviews (1): Last reviewed commit: "feat(aggregation): score interval-2 jobs..." | Re-trigger Greptile}

[greptile-apps]

Block-Root Read Panics Actor

When the storage backend fails while enumerating live block roots, this expect panics during the interval-2 actor path. A transient database read error can therefore terminate the node instead of skipping this optional aggregation session or returning an error.

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/blockchain/src/aggregation.rs
Line: 299

Comment:
**Block-Root Read Panics Actor**

When the storage backend fails while enumerating live block roots, this `expect` panics during the interval-2 actor path. A transient database read error can therefore terminate the node instead of skipping this optional aggregation session or returning an error.

How can I resolve this? If you propose a fix, please make it concise.