refactor(blockchain): move projection and scoring onto ProjectedState

[MegaRedHand]

What

Pure, behavior-preserving refactor of the block builder: pull the round-by-round
justification/finalization projection out of select_attestations into a
reusable ProjectedState with from_head_state() and advance(), and widen
entry_passes_filters, Tier, EntryScore, OrderingKey, and
EntryScore::ordering_key to pub(crate).

Why

The interval-2 aggregation selector duplicates the block builder's
justify/finalize projection verbatim. This PR extracts that projection so a
follow-up can reuse it instead of copy-pasting the round-by-round logic, keeping
proposer and aggregator from ever drifting on how they project the post-state.

Behavior

Block building is unchanged:

• advance()'s body is the former inline projection.
• The current_votes update simply moves after the trace! it never fed.

Notes

• score_entry is intentionally left as-is (proofs-based).
• Draft: base of the stacked aggregation PR that consumes these primitives.

Checks

• cargo fmt clean
• cargo clippy --all-targets -- -D warnings clean
• cargo test -p ethlambda-blockchain --lib green (incl. projection-cascade tests)

[github-actions]

🤖 Kimi Code Review

The PR refactors attestation selection logic by extracting ProjectedState into a proper struct with methods, enabling code sharing between block proposal and the upcoming aggregation pipeline. The consensus logic (3SF-mini justification/finalization conditions, vote filtering, and tier scoring) is preserved correctly.

High-level assessment: Correct and well-structured refactoring. No security issues detected.

Specific observations:

1. Code deduplication & API design (crates/blockchain/src/block_builder.rs:302-496)
   Converting free functions into ProjectedState methods (from_head_state, advance, score_entry, entry_passes_filters) is idiomatic. Moving coverage calculation to the caller (line 271-274) allows the aggregation module to reuse score_entry without duplicating the tiering logic, as noted in the doc comments.

2. Visibility changes (lines 314, 499, 527, 540)
   Changing Tier, EntryScore, OrderingKey, and ProjectedState to pub(crate) is appropriate given the stated goal of sharing projection logic with the aggregation module. The encapsulation of target_slot and att_slot within EntryScore (private fields with ordering_key as the only accessor) is good defensive programming.

3. Logic preservation in advance (lines 320-368)
   The method preserves the exact sequence of the original code:

   1. Extend current_votes with new voters (line 325-327)
   2. If Justify/Finalize: extend justification bitfield and mark target justified (lines 333-343)
   3. Remove justified target from current_votes to prevent redundant tracking (line 347)
   4. If Finalize: shift justification window and update finalized_slot (lines 350-353)

   The finalizes condition in score_entry (lines 411-415) correctly maintains the 3SF-mini requirement that no slot strictly between source and target remains justifiable relative to the projected finalized slot.

4. Filter correctness (lines 437-470)
   entry_passes_filters correctly preserves the genesis self-vote exemption (lines 457, 461, 467) and validates the attestation against the projected justification/finalization state.

Minor suggestions:

• Documentation consistency: The doc comment for ProjectedState (lines 308-312) mentions "interval-2 aggregation" which appears to be internal terminology. Consider adding a brief clarification or ensuring this is documented in the aggregation module's architecture docs.

• Performance note: In score_entry (lines 394-397), the new_voters calculation iterates the full coverage set. Since coverage is already a HashSet, the filter operation is O(coverage). This is equivalent to the old logic's complexity but allocates a new HashSet for the result. This is acceptable given that marginal voters are needed by the caller for advance, but consider documenting that this allocation is intentional for the returned set.

No changes required – the PR is ready to merge.

---

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

[github-actions]

🤖 Codex Code Review

1. crates/blockchain/src/block_builder.rs:274 and crates/blockchain/src/block_builder.rs:399: this refactor adds an avoidable double allocation/hash pass in the proposer hot path. pick_best_candidate now builds a full coverage: HashSet<u64> for every candidate, and ProjectedState::score_entry immediately walks it again to build a second HashSet for new_voters. Before this PR, scoring computed the marginal set in one pass over the proofs. On dense heads with many candidates/proofs, that increases per-round CPU and allocation pressure right where proposer/aggregation deadlines are tight. I’d keep proof iteration inside score_entry, or pass an iterator/bitset form that lets new_voters be computed once.

No consensus-correctness or security regression stood out from the refactor itself. The extracted entry_passes_filters and finalization scoring still line up with STF is_valid_vote / try_finalize in state_transition/src/lib.rs:395 and state_transition/src/lib.rs:445.

I couldn’t run cargo test in this environment because the pinned toolchain (1.92.0) could not be fetched here, so this review is source-only.

---

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

[greptile-apps]

Greptile Summary

This PR moves attestation projection and scoring into a reusable ProjectedState. The main changes are:

• Extracts projection initialization and advancement from the selection loop.
• Moves filtering and scoring onto ProjectedState.
• Passes deduplicated participant coverage into the scorer.
• Widens projection and ordering types to crate-level visibility.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.
• Participant coverage remains equivalent to the previous proof iteration.
• Projection updates retain the previous justification and finalization order.
• The new crate-visible methods have no additional production callers in this change.

Important Files Changed

Filename	Overview
crates/blockchain/src/block_builder.rs	Extracts the existing projection, filtering, and scoring flow into crate-visible types and methods without changing the current block-builder behavior.

_{Reviews (1): Last reviewed commit: "Merge branch 'main' into refactor/block-..." | Re-trigger Greptile}

[github-actions]

🤖 Claude Code Review

Review: PR 525 — refactor(blockchain): move projection and scoring onto ProjectedState

Summary

This is a clean, mechanical extraction. ProjectedState gains from_head_state, advance, score_entry, and entry_passes_filters as methods, replacing free functions that took the same fields as parameters. I compared the extracted method bodies against the pre-PR free functions line-by-line — the logic is byte-for-byte preserved (the Finalize implies Justify tiering, the crosses_2_3/finalizes computation, the filter ordering in entry_passes_filters). The existing regression tests (score_entry_does_not_finalize_source_at_boundary, the build_block_* suite, keep_best_proof_per_data_*) all still exercise the same code paths and should pass unchanged. Good use of pub(crate) scoped narrowly (e.g., EntryScore.target_slot/att_slot stay private, only fields the future caller needs are widened).

Two minor items worth a look before merge — neither blocks the refactor, both are cheap to fix.

Findings

1. pick_best_candidate now materializes a full coverage set before filtering to new voters — minor perf regression (crates/blockchain/src/block_builder.rs:274-279)

let coverage: HashSet<u64> = proofs
    .iter()
    .flat_map(|proof| proof.participant_indices())
    .collect();
let Some((score, new_voters)) =
    projected.score_entry(att_data, &coverage, chain.validator_count)

The old score_entry took proofs directly and built new_voters in a single pass, checking prior_voters.contains(&vid) per participant without ever collecting the full (unfiltered) coverage set. The new version does two passes: build coverage (all participants, no filtering) in pick_best_candidate, then filter it against prior_voters inside score_entry. This runs once per unprocessed candidate per round, so for the stress-test scenario already in this file's tests (50 entries, many rounds) it's a real if small extra allocation + iteration cost, not just a wash.

This tradeoff is presumably intentional — the doc comment on score_entry explains the aggregator needs to pass in "realized raw + child participants" as coverage rather than raw proofs, so a shared signature is needed. That's a reasonable reason to accept the extra pass, but it's worth calling out explicitly (e.g., in the PR description or a comment at the call site) so it doesn't read as an accidental regression during future profiling.

2. Doc comments describe the aggregator integration as already wired up, but it isn't yet (crates/blockchain/src/block_builder.rs:309-314, 385-389)

/// Shared by `select_attestations` (block proposal) and
/// `aggregation::snapshot_aggregation_inputs` (interval-2 aggregation) so the
/// two selectors project justification/finalization identically.

and

/// The caller resolves `coverage` and passes it in: block building unions a
/// data's proof participants (see `pick_best_candidate`); committee-signature
/// aggregation passes a job's realized raw + child participants.

I checked crates/blockchain/src/aggregation.rs — snapshot_aggregation_inputs doesn't reference ProjectedState, Tier, EntryScore, or any of the extracted methods today. The PR description confirms this is a draft base for a follow-up PR that hasn't landed. As written, the doc comments state present-tense fact ("Shared by X and Y") about code that doesn't exist yet, which will mislead anyone who greps for the actual call site and finds nothing. Suggest rephrasing to future/intended tense (e.g., "Intended to be shared by... once the follow-up aggregation PR lands") until the consuming code actually exists.

Non-findings (things that look right)

• Tier ordering and the tier <= Tier::Justify / tier == Tier::Finalize branches in advance() correctly preserve "Finalize implies Justify" given Finalize = 1 < Justify = 2 < Build = 3.
• advance() taking new_voters: impl IntoIterator<Item = u64> instead of &HashSet<u64> avoids a clone at the one call site that already owns the set — consistent with the repo's ownership-over-cloning convention.
• The entry_passes_filters extraction changes call convention (projected.entry_passes_filters(...) vs. free function with &projected.justified_slots, projected.finalized_slot) but preserves check order and error strings exactly.
• No new unwrap/expect introduced, no unsafe code, no changes to SSZ encoding, signature verification, or fork-choice logic — this PR touches only the block-builder's internal candidate-selection bookkeeping.

---

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