fix(blockchain): source attestations from the head state's justified

[MegaRedHand]

Summary

Validators sourced their vote's source checkpoint from the store's global latest_justified, a highest-slot-wins maximum. When a minority fork justifies a slot the head chain never extended, that maximum latches onto an off-head sibling (always an ancestor of the head, but possibly at a higher slot than the head chain's own state has justified). Sourcing a vote from that higher slot pins every target above the canonical chain's justification ladder, so the canonical chain cannot finalize until it climbs past whatever slot the minority fork justified, and block production stalls (the "did not converge" stall).

This sources the vote's source from the head state's latest_justified instead, which always reflects what the head chain itself justified, so source and target stay on the same ladder and finalization can advance.

Matches leanSpec PR #1166

This is the same change leanSpec is landing in PR #1166, including its genesis handling, so it aligns ethlambda with the spec direction (the prior store.latest_justified sourcing came from leanSpec #595).

What changed

produce_attestation_data (vote production only):

	Before	After
source	store.latest_justified() (global max)	head_state.latest_justified (head chain's own)
target clamp	store.latest_justified()	same head-state justified (kept consistent with source)
genesis placeholder	n/a	zero-root justified rebased to the head (genesis) root

• The store's latest_justified checkpoint is left untouched.
• get_attestation_target(store) is unchanged and still reads the store checkpoints. It is what the forkchoice spec-test harness uses to validate attestationTargetSlot, so spec-test behavior is unchanged.
• Genesis handling: the raw genesis state carries a zero-root justified placeholder that the state transition rebases to the real genesis root on the first block. A vote cast directly on the genesis head normalizes the placeholder the same way, else validate_attestation_data would reject it with UnknownSourceBlock.

Testing

• produce_attestation_data_sources_from_head_state_not_store: with the store's global justified latched onto a higher off-head sibling, the produced source follows the head state's on-chain justified.
• produce_attestation_data_normalizes_genesis_placeholder_source: a genesis-head vote rebases the zero-root placeholder to the genesis root and passes validate_attestation_data.
• cargo test -p ethlambda-blockchain --lib — 42 passed.
• cargo fmt --all -- --check, cargo clippy -p ethlambda-blockchain --all-targets -- -D warnings — clean.
• Forkchoice spec fixtures were not downloaded in this environment; the change does not touch get_attestation_target, the only attestation path the spec harness exercises. CI will confirm against the full vectors.

https://claude.ai/code/session_01783X9yFrkXfA7uY1cyL8E4

[github-actions]

🤖 Kimi Code Review

Overall Assessment: This is a critical liveness fix for fork choice safety. The change correctly prevents attestation production from stalling when the store's global justified checkpoint latches onto a minority fork.

Issues and Suggestions:

1. Panic Risk in State Retrieval (Line 703-705)

let source = store
    .get_state(&head_root)
    .expect("head state exists")
    .latest_justified;

While the head state should always exist in a well-formed store, using expect in consensus-critical code introduces a denial-of-service vector if the store ever enters an inconsistent state (e.g., due to disk corruption or a bug). Consider returning a Result or using a more graceful error handling strategy.

2. Performance Consideration (Line 703)
The change introduces a state lookup (store.get_state) where previously only a field access (store.latest_justified()) was needed. Ensure the head state is cached in memory—if this triggers a database load, it could impact attestation production latency under load.

3. Documentation Quality
The comments clearly explain the divergence from leanSpec (referencing PR 595) and the "highest-slot-wins" behavior that causes the stall. Item 1's test coverage effectively captures the regression scenario.

Security Implications:
This fix addresses a subtle but serious liveness failure: if validators attest using a source checkpoint from a minority fork while targeting the head chain, is_valid_vote fails (target doesn't descend from source), preventing re-justification of the head chain. Sourcing from the head state ensures source-target chain alignment, allowing justification to advance.

Recommendation: Accept, but consider replacing expect with proper error propagation in a follow-up.

---

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

[github-actions]

🤖 Codex Code Review

No findings.

The change in crates/blockchain/src/store.rs looks internally consistent: produce_attestation_data now sources from the head state’s latest_justified, and the target helper is called with that same checkpoint, which matches the ancestry checks in validate_attestation_data and the STF rule that only requires the source slot to already be justified. That makes the minority-fork/off-head-global-justified liveness fix plausible and avoids the prior “source not on head chain” failure mode. The added regression test at store.rs:1221 covers the core scenario well.

Residual gap: the new test only asserts data.source, not that the resulting data.target still stays on the same chain as that source in the off-head-justified case. I’d consider adding that assertion, but I don’t see a concrete bug in the patch as written.

I could not run the targeted tests in this environment: cargo test was blocked by read-only rustup / cargo paths under /home/runner/.cargo and /home/runner/.rustup, so this review is based on static analysis only.

---

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

[github-actions]

🤖 Claude Code Review

Now I have a complete picture. Here is the review:

---

Review: fix(blockchain): source attestations from the head state's justified

Overview

This PR fixes a liveness bug where produce_attestation_data was reading the source checkpoint from store.latest_justified() — a highest-slot-wins global that can latch onto a minority-fork checkpoint. When the store's justified lies on a sibling fork, every head-chain target fails is_valid_vote (it no longer descends from the source), blocking re-justification and stalling block production. The fix reads source from head_state.latest_justified instead, which is always on the head chain.

The same source value is also forwarded to get_attestation_target_with_checkpoints so its clamp guard (target walked-back behind justified → clamp to justified) uses a consistent checkpoint.

---

Correctness

The core fix is logically sound. head_state.latest_justified is computed by the STF during block import on that specific chain, so it is guaranteed to lie on the head's ancestry. store.latest_justified() is a global max that can advance via any imported block (including on sibling forks) — the divergence from leanSpec PR #595 is real and well-described.

The symmetry with the existing finalized treatment in update_head (which already overrides the store's finalized with the head state's value, see store.rs:58–61) makes this the natural completion of that pattern.

Target clamp consistency. Previously get_attestation_target(store) passed store.latest_justified() to the clamp guard. Now get_attestation_target_with_checkpoints(store, source, ...) passes the same source = head_state.latest_justified used for the emitted source field. If the walk-back falls behind the justified checkpoint, the clamped target equals the source. This is correct — consistent source and target, though such an attestation would fail is_valid_vote Rule 5 (target.slot > source.slot) in the degenerate case where both land on the same slot. That degenerate case is a pre-existing limitation of the walk-back logic, not introduced by this PR.

---

Specific observations

Observation 1 — Residual store-level exposure in update_head

update_head (store.rs ~line 47) still passes store.latest_justified().root as the LMD-GHOST starting point:

let (new_head, weights) = ethlambda_fork_choice::compute_lmd_ghost_head(
    store.latest_justified().root,  // ← store's global (can be off-head)
    &blocks,
    &attestations,
    0,
);

If the store's justified root belongs to a sibling fork, the fork choice walk starts from a root that may not be reachable from the live chain, which could degrade fork choice behavior even after this PR. The PR description scopes the fix to vote production only, which is fair, but this residual is worth tracking as a follow-up. update_safe_target (line 109-ish) has the same pattern.

Observation 2 — New storage read in hot path

produce_attestation_data now calls store.get_state(&head_root) that wasn't there before. In production (RocksDB), this is a disk/block-cache read. Given how rarely attestation production fires (once per attestation interval per validator), this is not a meaningful concern — the same pattern is already used in update_safe_target for the same head-state lookup. Just worth knowing the cost exists.

Observation 3 — expect on head state

let source = store
    .get_state(&head_root)
    .expect("head state exists")
    .latest_justified;

The expect is consistent with the rest of the codebase (e.g. update_safe_target at line ~109, update_head finalized override). The invariant that the head always has a state is assumed throughout, so this is idiomatic rather than unsafe. Fine as-is.

---

Test

The unit test produce_attestation_data_sources_from_head_state_not_store correctly constructs the exact failure scenario: a head chain of two blocks, a head state with an on-chain justified, and a store global justified latched to a higher off-head checkpoint. The two assertions — one positive (source equals head-state justified) and one negative (source is not the store's off-head value) — together pin the behavior precisely.

One minor note on the test: in this setup the walk-back lands at genesis (slot 0), which is below the head-state justified (slot 1), so get_attestation_target_with_checkpoints clamps target to head_justified. The produced AttestationData therefore has target.slot == source.slot == 1, which would fail real is_valid_vote validation. The test doesn't exercise is_valid_vote and isn't testing target validity, so this is acceptable — the test's stated purpose is source derivation, and it achieves that. A comment noting why data.target isn't asserted on could add clarity, but is not required.

The store.set_time(2 * INTERVALS_PER_SLOT) call is unexplained in the test. It doesn't appear to affect produce_attestation_data directly (which doesn't read time), but consistency with other test cases suggests it may be there to satisfy preconditions inside get_attestation_target_with_checkpoints or future invariants. A short inline comment would help future readers.

---

Summary

The fix is correct, minimal, and well-documented. The deliberate spec divergence is clearly justified and the docstring explains it thoroughly. The implementation follows project conventions. The one structural concern worth a follow-up issue is the residual use of store.latest_justified().root as the LMD-GHOST root in update_head / update_safe_target — those functions have the same off-chain exposure that this PR fixes for vote production.

---

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

[greptile-apps]

Greptile Summary

This PR fixes a liveness stall in vote production by changing produce_attestation_data to source the source checkpoint from the head state's latest_justified instead of the store's global latest_justified. The global store checkpoint is a highest-slot-wins maximum that can latch onto an off-head sibling justified by a minority fork; using it as the vote source puts source and target on different chains, causing every is_valid_vote check to fail and blocking re-justification indefinitely.

• produce_attestation_data now calls store.get_state(&head_root).expect(\"head state exists\").latest_justified for both the emitted source field and the justified argument passed to get_attestation_target_with_checkpoints, keeping the clamp guard consistent.
• get_attestation_target (used by the spec-test harness) is left untouched so spec fixture behaviour is unchanged.
• A focused unit test exercises the minority-fork scenario: store global justified latched at slot 5 (off-head), head-state justified at slot 1 (on-chain), confirming the produced source tracks the head state.

Confidence Score: 4/5

Safe to merge; the change is well-scoped, well-tested, and consistent with how latest_finalized was already handled in this codebase.

The fix is targeted and correct: get_state for the head block always succeeds under normal operation because on_new_block stores the state before updating the head, and prune_old_data explicitly protects the head root. The get_attestation_target entry point used by spec fixtures is untouched. The new test directly models the problematic minority-fork scenario.

No files require special attention beyond the reviewed store.rs.

Important Files Changed

Filename	Overview
crates/blockchain/src/store.rs	Core fix: produce_attestation_data now sources source from the head state's latest_justified instead of the store's global checkpoint; target clamp argument updated to match; new unit test validates the minority-fork scenario.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant V as Validator
    participant PAD as produce_attestation_data
    participant Store
    participant HeadState as Head State

    V->>PAD: produce_attestation_data(store, slot)
    PAD->>Store: head()
    Store-->>PAD: head_root
    PAD->>HeadState: "get_state(&head_root)"
    HeadState-->>PAD: "State { latest_justified: source }"
    Note over PAD: source = head_state.latest_justified (always on-chain)
    PAD->>Store: "get_block_header(&head_root)"
    Store-->>PAD: head slot
    PAD->>Store: get_attestation_target_with_checkpoints(source, latest_finalized)
    Note over PAD: clamp guard uses same source for consistency
    Store-->>PAD: target_checkpoint
    PAD-->>V: "AttestationData { source, target, head }"

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"}}}%%
sequenceDiagram
    participant V as Validator
    participant PAD as produce_attestation_data
    participant Store
    participant HeadState as Head State

    V->>PAD: produce_attestation_data(store, slot)
    PAD->>Store: head()
    Store-->>PAD: head_root
    PAD->>HeadState: "get_state(&head_root)"
    HeadState-->>PAD: "State { latest_justified: source }"
    Note over PAD: source = head_state.latest_justified (always on-chain)
    PAD->>Store: "get_block_header(&head_root)"
    Store-->>PAD: head slot
    PAD->>Store: get_attestation_target_with_checkpoints(source, latest_finalized)
    Note over PAD: clamp guard uses same source for consistency
    Store-->>PAD: target_checkpoint
    PAD-->>V: "AttestationData { source, target, head }"

Loading

_{Reviews (1): Last reviewed commit: "fix(blockchain): source attestations fro..." | Re-trigger Greptile}