Skip to content

fix: use head state's justified checkpoint as attestation source - #506

Merged
tcoratger merged 4 commits into
leanEthereum:mainfrom
GrapeBaBa:fix/attestation-source-use-head-state-justified-v2
Apr 1, 2026
Merged

fix: use head state's justified checkpoint as attestation source#506
tcoratger merged 4 commits into
leanEthereum:mainfrom
GrapeBaBa:fix/attestation-source-use-head-state-justified-v2

Conversation

@GrapeBaBa

@GrapeBaBa GrapeBaBa commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

Summary

produce_attestation_data was using store.latest_justified as the attestation source, while build_block filters attestations against head_state.latest_justified. When store.latest_justified diverges from head_state.latest_justified (caused by processing a non-head-chain block that crosses the 2/3 supermajority threshold), all attestations get filtered out during block building, producing blocks with 0 attestations.

Root Cause

store.latest_justified can advance past head_state.latest_justified when a non-head-chain block's state transition crosses the 2/3 threshold. This creates a mismatch:

  • Voting: produce_attestation_data used store.latest_justified → higher checkpoint
  • Block building: build_block filters with head_state.latest_justified → lower checkpoint
  • Result: All attestations fail the source filter → 0-attestation blocks

Fix

Align with the 3sf-mini reference design: the attestation source comes from head_state.latest_justified, not from the store-wide max.

Ref: ethereum/research 3sf-mini/p2p.pyStaker.vote() uses self.post_states[self.head].latest_justified_hash

The store-wide latest_justified (global max across all processed blocks) is intentionally used only for fork choice (recompute_head, compute_safe_target), never for voting — matching get_latest_justified_hash(post_states) in the reference.

produce_attestation_data was using store.latest_justified as the
attestation source, while build_block filters attestations against
head_state.latest_justified. When store.latest_justified diverges
from head_state.latest_justified (caused by processing a non-head-chain
block that crosses the 2/3 supermajority threshold), all attestations
get filtered out during block building, producing blocks with 0
attestations.

Align with the 3sf-mini reference: Staker.vote() uses
post_states[head].latest_justified_hash, not a global max. The
store-wide latest_justified is only used for fork choice (recompute_head,
compute_safe_target), never for voting.

Includes genesis root correction matching build_block's existing pattern:
the stored genesis state has latest_justified.root = Bytes32.zero(),
but attestation validation requires roots that exist in store.blocks.
Copilot AI review requested due to automatic review settings March 31, 2026 13:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an attestation-source mismatch that could lead to proposers building blocks with zero attestations when store.latest_justified advances on a non-head fork ahead of head_state.latest_justified.

Changes:

  • Switch produce_attestation_data to use the head state’s latest_justified checkpoint as the attestation source (aligning voting with block-building filters).
  • Add a genesis-specific root correction so the produced source.root exists in store.blocks (required for attestation validation).
  • Update the method docstring to document the intended fork-choice vs voting checkpoint usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lean_spec/subspecs/forkchoice/store.py
GrapeBaBa and others added 3 commits April 1, 2026 00:38
Verifies produce_attestation_data uses head_state.latest_justified
(not store.latest_justified) when the two diverge. This scenario
happens when a non-head fork advances store.latest_justified, which
would cause build_block to filter out all attestations.
Rewrite produce_attestation_data docstring and latest_justified field
docs to teach readers the invariant directly rather than referencing
past bugs or external implementations. Add line-by-line comments to
the unit test explaining each step of the divergence scenario.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fork choice test that creates a justified divergence between the
store-wide global max and the head state. A gossip attestation is
produced via the store's attestation production path and validated.

Fails without the fix: source.slot (1) > target.slot (0) violates
the monotonicity invariant. Passes with the fix: source comes from
the head state (slot 0) instead of the global max.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@tcoratger tcoratger left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for noticing the bug here @GrapeBaBa!

I've just pushed an additional commit on top with a dedicated test vector that fails on main branch and that passes here so that client teams can also be sure that this behaviour is fine on their side.

@tcoratger tcoratger added the bug Category: bug fix label Mar 31, 2026
zclawz pushed a commit to blockblaz/zeam that referenced this pull request Apr 1, 2026
constructAttestationData was using fcStore.latest_justified (the
store-wide global max) as the attestation source. Block building
filters attestations against head_state.latest_justified. When a
non-head fork crosses the 2/3 supermajority threshold,
fcStore.latest_justified can advance past the head chain's view,
causing every attestation to fail the source filter and producing
0-attestation blocks.

Fix: derive the attestation source from the head state's
latest_justified, matching the 3sf-mini reference where Staker.vote()
uses post_states[head].latest_justified_hash.

Genesis correction: at genesis the head state holds a zero-hash for
latest_justified.root (no real justified block yet). Since attestation
validation requires the source root to exist in the block registry,
substitute the actual genesis block root — the same correction already
applied in block building.

Also update getProposalAttestationsForHead in forkchoice to filter
attestations using the head state's justified checkpoint so that
produced attestations are actually included in blocks.

Ref: leanEthereum/leanSpec#506

@unnawut unnawut left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great and really good spotting. Plus I really like the informative description, really helpful.

I wonder if this is the reason for frequent reorgs and chain not justifying with longer devnet runs...

@tcoratger
tcoratger merged commit 9c30436 into leanEthereum:main Apr 1, 2026
13 checks passed
pablodeymo added a commit to lambdaclass/ethlambda that referenced this pull request Apr 1, 2026
## Summary

- Use `head_state.latest_justified` instead of
`store.latest_justified()` as attestation source in
`produce_attestation_data`, aligning voting with the block builder's
filter
- Handle genesis edge case (zero-hash root substitution) matching
existing `build_block` logic
- Add Rust unit test ported from leanSpec PR #506

## Root Cause

`produce_attestation_data` used `store.latest_justified` (the global max
across all forks) as the attestation source. `build_block` filters
attestations against `head_state.latest_justified` (the head chain's
view). When a non-head fork block advances the store-wide justified past
the head chain's justified — e.g. a minority fork carrying a
supermajority of attestations — every attestation is rejected during
block building. This produces blocks with 0 attestations, stalling
justification and finalization indefinitely.

**Ref**: [leanSpec PR
#506](leanEthereum/leanSpec#506), [3sf-mini
reference](https://github.com/ethereum/research/blob/master/3sf-mini/p2p.py)
(`Staker.vote()` uses
`self.post_states[self.head].latest_justified_hash`)

## Note on spec test fixtures

The leanSpec commit that includes PR #506 also introduces a validator
format change (`pubkey` → `attestationPubkey` + `proposalPubkey`) that
requires a domain model migration. The spec test fixture update is
deferred to a separate PR. The unit test covers the same invariant.

## Test plan

- [x] Unit test `produce_attestation_data_uses_head_state_justified`
passes
- [x] `cargo fmt --all -- --check` clean
- [x] `cargo clippy -p ethlambda-blockchain -- -D warnings` clean
- [ ] Full `make test` with existing fixtures (running)
GrapeBaBa added a commit to GrapeBaBa/leanSpec that referenced this pull request Apr 12, 2026
tcoratger added a commit to GrapeBaBa/leanSpec that referenced this pull request Apr 12, 2026
…ling test

Add an invariant check in update_head that walks backward from the
new head to verify it is a descendant of the justified root. This
documents the guarantee that LMD-GHOST provides by construction and
catches any future regression.

Add a fork choice filler test exercising the justified divergence
scenario: a minority fork (V1+V2+V3) justifies slot 1 while the
head chain (V0) has not seen those attestations. The next block on
the head chain self-heals via the fixed-point loop in build_block:

1. Pool contains fork B's attestations (source=0, target=1)
2. Builder starts with current_justified=0 (head state)
3. Fork B's attestations match -> justifies slot 1
4. Divergence closed in one block

This proves the revert of PR leanEthereum#506 is safe: the chain does not stall
because the fixed-point loop incorporates the justifying attestations.

Addresses leanEthereum#596

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tcoratger added a commit that referenced this pull request Apr 14, 2026
…rce" (#595)

* Revert "fix: use head state's justified checkpoint as attestation source (#506)"

This reverts commit 9c30436.

* fix(fc): add downstream assert in update_head and divergence self-healing test

Add an invariant check in update_head that walks backward from the
new head to verify it is a descendant of the justified root. This
documents the guarantee that LMD-GHOST provides by construction and
catches any future regression.

Add a fork choice filler test exercising the justified divergence
scenario: a minority fork (V1+V2+V3) justifies slot 1 while the
head chain (V0) has not seen those attestations. The next block on
the head chain self-heals via the fixed-point loop in build_block:

1. Pool contains fork B's attestations (source=0, target=1)
2. Builder starts with current_justified=0 (head state)
3. Fork B's attestations match -> justifies slot 1
4. Divergence closed in one block

This proves the revert of PR #506 is safe: the chain does not stall
because the fixed-point loop incorporates the justifying attestations.

Addresses #596

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(fc): move downstream assert to block production and add unit test

The assert in update_head was redundant — LMD-GHOST starts from the
justified root and descends, so the result is always downstream by
construction. Remove it.

Add the assert where it matters: in produce_block_with_signatures,
verifying the fixed-point loop in build_block closed any justified
divergence between the store and the head chain. Without this, a
produced block could omit justifying attestations, leaving other
nodes with a stale justified checkpoint.

- Move assert from update_head to produce_block_with_signatures
- Add block body checks to the divergence filler test (test vectors)
- Add unit test for build_block fixed-point convergence

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@unnawut unnawut added this to the pq-devnet-4 milestone Apr 16, 2026
@unnawut unnawut added the specs Scope: Changes to the specifications label Apr 16, 2026
MegaRedHand added a commit to lambdaclass/ethlambda that referenced this pull request Apr 21, 2026
…5) (#315)

## Summary

Applies
[leanSpec#595](leanEthereum/leanSpec#595), which
reverts
[leanSpec#506](leanEthereum/leanSpec#506).
Attestation production now uses the store's global `latest_justified`
checkpoint again; liveness under a justified-divergence is preserved by
the existing fixed-point loop in `build_block`, guarded by a new
invariant that blocks must not lag the store's justified slot.

- `produce_attestation_data` returns `source = store.latest_justified()`
(drops the head-state-derived source and the genesis ZERO-root patch;
`latest_justified` is seeded to the anchor block root at genesis, so no
correction is needed).
- `produce_block_with_signatures` checks
`post_checkpoints.justified.slot >= store.latest_justified().slot` and
otherwise returns a new `StoreError::JustifiedDivergenceNotClosed`
variant. The existing call site in `crates/blockchain/src/lib.rs`
already uses `inspect_err` + early return, so the node logs `Failed to
build block` and skips proposal rather than crashing.
- Removed the unit test
`produce_attestation_data_uses_head_state_justified` since it codified
the reverted behavior. No new tests were added; the self-healing
block-production path is exercised by the fork-choice spec tests against
the currently pinned leanSpec commit.

## Spec fixture compatibility

The pinned `LEAN_SPEC_COMMIT_HASH` (`bc17f7ae…`) is 30 commits ahead of
leanSpec#595's merge commit (`fcef7ef9…`), with the merge base equal to
that commit. No fixture regeneration is required: existing fixtures
already reflect the post-595 behavior.

## Test plan

- [x] `cargo fmt --all --check`
- [x] `cargo clippy --workspace --all-targets -- -D warnings`
- [x] `cargo test --workspace --release` (313 passed, 6 ignored, 29
suites), including `forkchoice_spectests` (70 tests) and `stf_spectests`
against the currently pinned leanSpec commit
- [ ] Devnet smoke test via
`.claude/skills/test-pr-devnet/scripts/test-branch.sh` to confirm
multi-client finalization still advances and the new invariant doesn't
fire under normal operation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Category: bug fix specs Scope: Changes to the specifications

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants