feat(proposer): prefer newer attestation targets in tiered scoring

[MegaRedHand]

What

Make the proposer's tiered attestation scorer (block_builder.rs) order candidates differently per tier, porting leanSpec PR #1149 (commit b2ab3c0).

Tier	Old (uniform)	New
Finalize / Justify	voters↓, target↑, att↑	target↓, att↓, voters↓
Build	voters↓, target↑, att↑	voters↓, target↓, att↓

tier still leads (lower tier wins) and data_root is still the final deterministic tiebreak.

Why

Finalize/justify entries already cross 2/3 on their target, so leading with a newer (larger) target slot pushes the justified slot as far forward as possible, which shortens recovery from a justification or finalization stall. The build tier only adds marginal voters toward the threshold, so coverage (more new voters) still leads there; a newer target is only a tiebreak.

Changes

• EntryScore::ordering_key is now tier-dependent; all slot priorities are Reverse-encoded ("larger is better").
• Factored the comparison tuple into a type OrderingKey alias (satisfies clippy::type_complexity).
• Added two unit tests covering the justify-tier and build-tier orderings (written TDD: both fail pre-change, pass after).

Testing

• make fmt / make lint clean.
• cargo test -p ethlambda-blockchain --lib block_builder — 10/10 pass.

https://claude.ai/code/session_01MiEYtTEFUjX8mqYKwrriYJ

[github-actions]

🤖 Kimi Code Review

Review: PR 446 - Tier-dependent fork choice ordering

The implementation correctly adopts the leanSpec PR #1149 logic for fork-choice candidate selection. The change from a uniform ordering to tier-dependent priorities (prioritizing chain progress for Finalize/Justify vs. voter coverage for Build) is sound for consensus liveness during stall recovery.

Specific notes:

crates/blockchain/src/block_builder.rs

• Line 13: Good cleanup importing Reverse directly rather than using fully-qualified syntax.
• Lines 459-483: The OrderingKey type alias and refactored ordering_key method improve readability. The tier-dependent match is exhaustive and correctly implements the specified priority inversions.
• Line 468: Casting new_voters (usize) to u64 is safe for Ethereum validator set sizes (<< 2^64) and improves cross-platform consistency by fixing the key width.
• Lines 784-851: Tests correctly verify the lexicographic ordering invariants for both tiers. The assertions validate that Reverse wrapping properly inverts the comparison so that larger values win within their priority slots.

Minor: Ensure Tier derives Ord with the desired variant ordering (presumably Finalize < Justify < Build or similar) since the tuple comparison depends on it as the primary key.

Verdict: Correct and well-tested. No blockers.

---

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

[github-actions]

🤖 Codex Code Review

No blocking findings in the diff.

The change in crates/blockchain/src/block_builder.rs:459 through crates/blockchain/src/block_builder.rs:488 is internally consistent with the projected-selection loop at crates/blockchain/src/block_builder.rs:165 through crates/blockchain/src/block_builder.rs:223: candidates that require newly justified sources are still gated by entry_passes_filters, so preferring newer targets in Justify/Finalize tiers should not produce block-body ordering that the STF later drops.

One non-blocking gap: the new tests at crates/blockchain/src/block_builder.rs:784 through crates/blockchain/src/block_builder.rs:849 only verify ordering_key() in isolation. I would add one end-to-end build_block regression covering two same-tier supermajority candidates so future changes cannot preserve the tuple ordering while regressing projected selection or final STF ordering.

I could not run the tests locally: the sandbox has no installed Rust 1.92.0 toolchain and network access is disabled, so verification was limited to static review.

---

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

[github-actions]

🤖 Claude Code Review

PR #446 Review: feat(proposer): prefer newer attestation targets in tiered scoring

Overview

This PR ports leanSpec PR #1149, making the attestation scorer in block_builder.rs use tier-dependent ordering within EntryScore::ordering_key. The change makes Finalize/Justify tiers prefer newer (larger) target slots, while Build tier keeps voters-first but also flips to prefer newer targets as a secondary. All priorities are now uniformly Reverse-encoded.

---

Correctness

Ordering logic is correct. The pick_best_candidate loop uses candidate_key < *k to select the best entry. Reverse wrappers invert the natural order so "larger is better" maps to "smaller tuple wins." The match arms correctly implement the documented per-tier policies:

• Tier::Finalize | Tier::Justify: (tier, Reverse(target_slot), Reverse(att_slot), Reverse(new_voters), data_root)
• Tier::Build: (tier, Reverse(new_voters), Reverse(target_slot), Reverse(att_slot), data_root)

Subtle concern in Tier::Finalize. Within a Finalize candidate set, target_slot leads — but finalization advances by source.slot, not target.slot. Consider two hypothetical Finalize candidates:

• A: source=5, target=6
• B: source=3, target=8

Under the new ordering, B (newer target) wins and is selected first, finalizing source=3. A would then finalize source=5 in the next round — assuming it's still admissible. This is probably fine in practice given the strict conditions for Tier::Finalize (source > finalized && no justifiable gaps between source and target), but the semantic mismatch between "prefer newer target" and "finalization = source advance" is worth documenting in the Finalize arm comment. This matches the ported spec behaviour, so it's not a blocker.

new_voters as u64 cast (line 469). EntryScore::new_voters is usize; the cast to u64 satisfies the uniform OrderingKey type alias. On 32-bit targets usize is 32 bits, so the cast is lossless. This works, but it's a minor type-coherence awkwardness: the alias forces three heterogeneous slot/count fields into the same type. If this ever becomes a concern, Reverse<usize> could be used for new_voters independently, though that breaks the clean alias.

---

Type Alias

type OrderingKey = (Tier, Reverse<u64>, Reverse<u64>, Reverse<u64>, H256);

Good choice. Satisfies clippy::type_complexity, makes the return type of ordering_key readable, and the comment on the alias explains the semantics. The variable names (more_new_voters, newer_target, newer_att) at the construction site are clear.

---

Test Coverage

The two new tests are well-targeted and written to the exact behavioral claims:

• ordering_key_justify_tier_prefers_newer_target_over_more_voters — covers the primary change for Finalize/Justify.
• ordering_key_build_tier_prefers_voters_then_newer_target — covers both the voters-first primary and the target-slot tiebreak.

Gaps worth considering:

1. Tier::Finalize is not directly tested (only Tier::Justify). Since both use the same match arm, this is low risk but a Finalize-tier variant of the first test would make the coverage explicit.
2. Neither test exercises the att_slot tiebreaker (the third Reverse slot). A case with equal new_voters and equal target_slot but differing att_slot would complete the picture.

---

Style & Conventions

• use std::cmp::Reverse at the top is cleaner than the previous inline std::cmp::Reverse — good cleanup.
• Variable naming in ordering_key is self-documenting; comments minimal and on-point — consistent with project style.
• Updated doc comment on EntryScore is accurate and matches the implementation.
• No comment about the Finalize arm in ordering_key itself — the type-level doc explains the intent, which is fine, but a one-liner above the Tier::Finalize | Tier::Justify => arm noting the target_slot-leads rationale (as in the doc comment) would aid future readers without being verbose.

---

Summary

The core logic correctly implements the leanSpec #1149 spec, the naming is clear, and the tests directly verify the two behavioral changes. The minor points are:

1. The Tier::Finalize arm semantics (target leads, but finalization is source-driven) could use a brief inline note; low priority since it mirrors the spec.
2. A test for Tier::Finalize ordering and for the att_slot tiebreaker would complete coverage.
3. The usize → u64 cast is harmless but implicit.

No correctness bugs or security issues found. The change is ready.

---

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

[greptile-apps]

Greptile Summary

This PR ports leanSpec PR #1149 to make EntryScore::ordering_key tier-dependent, so that Finalize/Justify entries (which already cross 2/3 on their target) lead with a newer target_slot rather than more voters, shortening recovery from justification stalls.

• Finalize / Justify tier: sort order changes to target_slot↓, att_slot↓, voters↓ — pushes the justified slot as far forward as possible instead of preferring older targets.
• Build tier: sort order changes to voters↓, target_slot↓, att_slot↓ — coverage still leads, but slot ordering flips from "older first" to "newer first" for the two tiebreaks.
• Introduces the OrderingKey type alias (satisfies clippy::type_complexity) and adds two targeted unit tests (TDD: both failed before the change).

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to the within-tier sort key, is spec-aligned, and is covered by new tests that failed before the patch.

The ordering logic is correct: Finalize/Justify entries get target↓, att↓, voters↓ and Build entries get voters↓, target↓, att↓, matching the leanSpec intent. The Finalize and Justify tiers share the same match arm, so the Justify-tier test covers both. The usize as u64 cast is safe in practice given validator-count bounds. No logic errors or missing cases found.

No files require special attention.

Important Files Changed

Filename	Overview
crates/blockchain/src/block_builder.rs	Introduces tier-dependent ordering in EntryScore::ordering_key, adds OrderingKey type alias, and ships two unit tests covering the Justify and Build tier orderings. Logic is consistent with the spec and well-documented.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[EntryScore::ordering_key] --> B{tier?}
    B -->|Finalize or Justify| C["(tier, Reverse(target_slot), Reverse(att_slot), Reverse(new_voters), data_root)"]
    B -->|Build| D["(tier, Reverse(new_voters), Reverse(target_slot), Reverse(att_slot), data_root)"]
    C --> E["Prefer newer target slot → newer att slot → more voters"]
    D --> F["Prefer more voters → newer target slot → newer att slot"]
    E --> G[data_root ascending as final deterministic tiebreak]
    F --> G

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[EntryScore::ordering_key] --> B{tier?}
    B -->|Finalize or Justify| C["(tier, Reverse(target_slot), Reverse(att_slot), Reverse(new_voters), data_root)"]
    B -->|Build| D["(tier, Reverse(new_voters), Reverse(target_slot), Reverse(att_slot), data_root)"]
    C --> E["Prefer newer target slot → newer att slot → more voters"]
    D --> F["Prefer more voters → newer target slot → newer att slot"]
    E --> G[data_root ascending as final deterministic tiebreak]
    F --> G

Loading

_{Reviews (1): Last reviewed commit: "feat(proposer): prefer newer attestation..." | Re-trigger Greptile}