feat(aggregation): cap aggregation at one job before our proposal

[MegaRedHand]

Motivation

At interval 2 the aggregation worker runs up to MAX_AGGREGATION_JOBS leanVM proofs. When interval 4 of the same slot builds the next slot's block, that build runs its own proofs — so the two compete for the prover, and the build is the one with a hard deadline (it has to publish at the next slot's interval-0 tick).

Change

start_aggregation_session drops the session to a single job whenever one of our validators proposes the next slot:

let next_proposer = self
    .get_our_proposer(slot + 1)
    .filter(|_| self.sync_status.duties_allowed());
let max_jobs = if next_proposer.is_some() {
    1
} else {
    MAX_AGGREGATION_JOBS
};

• Condition mirrors the propose path (SlotInterval::EndOfSlot): proposer and duties_allowed(). A slot where duties are sync-suppressed keeps the full job budget, since no build will happen.
• Covers both entry points. The cap is computed inside start_aggregation_session, so it applies to the interval-2 tick and the early 2/3-threshold trigger. The early session is the slot's session, so exempting it would defeat the change.
• The retained job is the best-scoring candidate. snapshot_aggregation_inputs gained a max_jobs parameter that bounds the greedy selection loop; the pool is unchanged (groups_considered still counts every candidate), so the one job we run is the same one the uncapped selection picks first.
• MAX_AGGREGATION_JOBS lowered 3 → 2, trimming baseline prover work per session too.

Unchanged: AGGREGATION_DEADLINE, the early-trigger threshold, and the worker loop.

Tests

• Existing snapshot_caps_jobs_at_max_aggregation_jobs refactored to share a store fixture with a new snapshot_caps_jobs_at_one_for_proposer, which asserts the proposer cap yields exactly one job and that it is the top-scoring candidate (not an arbitrary one).
• make lint clean, cargo test --workspace --release green (122 fork-choice spec, 119 STF, all unit tests).

[github-actions]

🤖 Kimi Code Review

Overall Assessment: Solid PR with good defensive resource management. The change prevents leanVM prover contention when the node is about to propose, which is critical for block production latency.

Issues & Suggestions:

1. Defensive validation for max_jobs parameter (aggregation.rs:216)
The function snapshot_aggregation_inputs now accepts an arbitrary usize but assumes it will be > 0. If called with 0 (future refactoring hazard), it returns Some(snapshot_with_zero_jobs) rather than None, potentially spawning wasteful worker threads.

Suggestion: Add an early return or assert:

if max_jobs == 0 {
    return None;
}

Or document the precondition in the function docstring if callers are strictly controlled.

2. Documentation consistency (lib.rs:463-465)
The docstring says "capped at a single job when we propose next slot" but the logic actually checks slot + 1. This is correct (interval 4 builds slot+1), but the docstring could be slightly clearer that "next slot" means slot + 1 relative to the current aggregation slot.

3. Information leak consideration (lib.rs:500-509)
Throttling aggregation to 1 job when proposing at slot + 1 could theoretically leak proposer information via timing/side-channels if peer nodes can observe the node's aggregation output rate. However, since aggregation jobs are internal and only the final aggregates are gossiped, this appears to be a non-issue in practice. Consider adding a brief comment acknowledging this was considered and deemed safe.

4. Test robustness (aggregation.rs:1416)
The new test snapshot_caps_jobs_at_one_for_proposer verifies the cap works when there are NUM_GROUPS (5) candidates. Consider also testing the edge case where there is exactly 1 candidate with the proposer cap to ensure it doesn't cause issues (though the logic looks correct).

Positive Notes:

• Good separation of concerns: The aggregation module remains agnostic to proposer logic; it simply accepts a max_jobs parameter. This is proper layering.
• Test refactoring: Extracting store_with_competing_build_tier_groups reduces duplication between the existing cap test and the new proposer cap test.
• Clear logging: The info! log in start_aggregation_session includes both the slot and validator_id, making debugging easier.
• Mirroring conditions: The use of duties_allowed() filter correctly mirrors the propose path's conditions, preventing throttling when duties are suppressed.

Nitpick:
In lib.rs:495-498, the filter on Option is elegant but slightly dense. An explicit if might read easier, but this is subjective and the current code is idiomatic Rust.

// Current:
let next_proposer = self
    .get_our_proposer(slot + 1)
    .filter(|_| self.sync_status.duties_allowed());

// Alternative (clearer to some readers):
let next_proposer = if self.sync_status.duties_allowed() {
    self.get_our_proposer(slot + 1)
} else {
    None
};

Conclusion: Address Item 1 (defensive check for max_jobs == 0) before merge if possible; otherwise LGTM.

---

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

[github-actions]

🤖 Codex Code Review

1. crates/blockchain/src/lib.rs: capping proposer-adjacent aggregation sessions to 1 job can drop consensus-critical second-round proofs. snapshot_aggregation_inputs is intentionally multi-round and mutates ProjectedState after each pick so later candidates can be re-tiered to Justify/Finalize (crates/blockchain/src/aggregation.rs, crates/blockchain/src/block_builder.rs). The repo already has a block-builder regression test for exactly this cascading behavior, where attestation B is only admissible after attestation A projects slot 1 as justified (crates/blockchain/src/block_builder.rs). With this patch, the proposer-next-slot path may aggregate only A, never produce B, and the proposer cannot recover B from raw gossip later because aggregation outputs are what get promoted into the pool the block builder reads (crates/blockchain/src/aggregation.rs, crates/blockchain/src/store.rs, crates/blockchain/src/store.rs). That can reduce the next block’s justification/finalization progress and delay liveness on proposer slots.

No other material correctness, security, or memory-safety issues stood out in the touched code. I did not run tests.

---

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

[greptile-apps]

Duty transition bypasses cap

When the node is syncing at the interval-2 tick but becomes synced before interval 4, aggregation starts with three jobs while the later duty check permits the proposal, causing the original prover contention and potentially delaying block publication.

Knowledge Base Used: Blockchain core: fork choice, state transition, block building, sync

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/blockchain/src/lib.rs
Line: 499-500

Comment:
**Duty transition bypasses cap**

When the node is syncing at the interval-2 tick but becomes synced before interval 4, aggregation starts with three jobs while the later duty check permits the proposal, causing the original prover contention and potentially delaying block publication.

**Knowledge Base Used:** [Blockchain core: fork choice, state transition, block building, sync](https://app.greptile.com/lambdaclass/-/custom-context/knowledge-base/lambdaclass/ethlambda/-/docs/blockchain-core.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Greptile Summary

This PR introduces a proposer-aware aggregation budget.

• Adds one-job and normal aggregation caps.
• Passes the selected cap into the snapshot’s greedy selection loop.
• Adds coverage that the proposer cap retains the highest-scoring candidate.

Confidence Score: 3/5

The sync-status timing gap should be fixed before merging because it can leave three aggregation jobs competing with a newly enabled proposal.

Aggregation and proposal eligibility read mutable sync status at different ticks, so a node that catches up between intervals can start the uncapped worker and still build the next block.

Files Needing Attention: crates/blockchain/src/lib.rs

Important Files Changed

Filename	Overview
crates/blockchain/src/lib.rs	Selects the proposer-aware aggregation cap, but checking mutable duty status at session start leaves a syncing-to-synced contention window.
crates/blockchain/src/aggregation.rs	Parameterizes the greedy job limit and verifies that both configured caps retain score ordering.

Sequence Diagram

sequenceDiagram
    participant Tick2 as Interval-2 tick
    participant Sync as Sync status
    participant Agg as Aggregation worker
    participant Tick4 as Interval-4 tick
    participant Prover as leanVM prover
    Tick2->>Sync: "duties_allowed() = false"
    Tick2->>Agg: Start with 3 jobs
    Sync-->>Tick4: Node becomes synced
    Tick4->>Sync: "duties_allowed() = true"
    Tick4->>Prover: Build next-slot proposal
    Agg->>Prover: Aggregation proofs contend with build

Loading

Prompt To Fix All With AI

### Issue 1
crates/blockchain/src/lib.rs:499-500
**Duty transition bypasses cap**

When the node is syncing at the interval-2 tick but becomes synced before interval 4, aggregation starts with three jobs while the later duty check permits the proposal, causing the original prover contention and potentially delaying block publication.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "feat(aggregation): cap aggregation at on..." | Re-trigger Greptile}

[github-actions]

🤖 Claude Code Review

Review: PR #544 — cap aggregation at one job before our proposal

Overall: Sound, well-scoped change. The core fix — threading max_jobs through snapshot_aggregation_inputs to bound the greedy selection loop (crates/blockchain/src/aggregation.rs:286-287) — is a pure parameterization that doesn't touch candidate-pool computation (groups_considered still counts everything), so the "best job survives" guarantee holds. The new snapshot_caps_jobs_at_one_for_proposer test correctly asserts this against a shared fixture. No correctness bugs found in the selection logic itself.

A few things worth considering:

1. Duplicated proposer-prediction logic, no shared source of truth (crates/blockchain/src/lib.rs:441-444 vs crates/blockchain/src/lib.rs:498-500). Both the interval-4 EndOfSlot propose check and the interval-2 throttle check independently compute:

   self.get_our_proposer(slot + 1).filter(|_| self.sync_status.duties_allowed())

   The PR description explicitly relies on these staying in lockstep ("condition mirrors the propose path"), but nothing enforces that beyond comments. A future edit to one arm (e.g., adding another gating condition to the real propose path) could silently desync the throttle from actual propose behavior. Consider extracting a shared helper, e.g. fn next_slot_proposer(&self, slot: u64) -> Option<u64>.

2. Race between the two duties_allowed() reads (crates/blockchain/src/lib.rs:500 and crates/blockchain/src/lib.rs:444). sync_status is refreshed every tick via update_sync_status, so it can flip between interval 2 (throttle decision) and interval 4 (actual propose decision), ~1.6s later. In the narrow window where sync status changes mid-slot, the two decisions can disagree — most notably the case where duties become allowed only by interval 4: aggregation ran uncapped at interval 2, and the build now contends with a full 3-job session, i.e. exactly the race this PR sets out to eliminate. Self-correcting and narrow, but worth a one-line acknowledgment in the comment.

3. No test coverage for the actor-level wiring. The unit tests only exercise snapshot_aggregation_inputs with a hardcoded max_jobs (crates/blockchain/src/aggregation.rs:1414-1424); the decision of which constant start_aggregation_session passes — based on get_our_proposer(slot + 1) plus duties_allowed() (crates/blockchain/src/lib.rs:498-512) — is untested. A regression there (wrong slot offset, inverted filter) wouldn't be caught by CI. Understandably harder to unit-test given the actor/key_manager/sync_status setup required, but worth flagging as a gap.

4. Behavioral note for solo/small-validator devnets (not a bug): on any topology where this node's key_manager proposes every slot (e.g. a single-client devnet owning all validators), next_proposer is Some every slot, so the session is capped at 1 job on every single slot rather than only occasionally. That's a real throughput reduction for attestation aggregation (and potentially justification propagation speed) in such setups — worth calling out in release notes so it isn't mistaken for a regression during local testing.

5. Minor: no metric tracks how often the throttle activates — just an info! log (crates/blockchain/src/lib.rs:503-508). Given the codebase's existing pattern of instrumenting comparable branches (e.g., metrics::inc_aggregation_early_starts), a counter here would make the throttle's real-world frequency/impact observable via Prometheus rather than log-only.

Nothing here blocks merging; items 1–3 are the most actionable if you want to harden this before it ships.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt