fix: allow older-but-justified sources in build_block#716
Merged
tcoratger merged 11 commits intoMay 14, 2026
Merged
Conversation
The fixed-point loop required `att.source` to equal `current_justified` exactly. That rejected legitimate gap-closing attestations whose source is at or before the head chain's latest justified slot (e.g. a genesis source when the head chain has already justified slot 1). Block production aborted with "Fixed-point attestation loop did not converge" when the store's justified checkpoint advanced via a sibling fork. Replace the Checkpoint-equality with a slot bound: skip atts whose source slot is past the current justified slot. This minimum change unblocks the gap-closing path; the right shape of the broader filter is left as an open question for review.
Build a fork tree where the canonical head (block_5) lags behind the store's justified checkpoint (block_2, advanced by sibling block_6). Assert that produce_block_with_signatures succeeds, the produced block's post-state catches up to block_2, and the body includes block_6's gap-closing attestation.
Layer additional filters onto the source-slot bound so build_block matches process_attestations more closely without re-introducing the strict source-equality rule: - Source and target roots must match the chain at their slots (historical_block_hashes for [0, parent.slot - 1], parent_root at parent.slot, ZERO_HASH for empty slots between parent and the candidate). Mirrors the STF's own source/target consistency check. - Target slot must not already be justified on the chain, with one exception: the degenerate source.slot == target.slot == 0 genesis self-vote stays selectable so existing fixtures keep working. The STF still drops these, but they're tolerated in the block body. Track the justified-slot bitfield and finalized slot through the fixed-point loop so the target check stays accurate after each iteration advances justification or finalization. With these additions, the previously failing test_build_block_skips_non_matching_source, test_block_builder_fixed_point_advances_justification, and test_block_builder_recovers_finality_after_non_zero_boundary_stall all pass without modification.
Selecting an attestation already adds its data to processed_att_data, and the cap check at the top of the loop short-circuits before doing anything with already-processed entries. The explicit duplicate-check before the add was redundant.
Trim the explanatory comments around the chain-match and already-justified checks now that the surrounding code is settled.
MegaRedHand
commented
May 13, 2026
|
|
||
|
|
||
| def test_build_block_skips_non_matching_source( | ||
| def test_build_block_skips_other_chain_source( |
Contributor
Author
There was a problem hiding this comment.
The original test's purpose no longer applies, but the actual assertions do, so we renamed it.
Both process_attestations and build_block need to verify that an attestation's source and target checkpoints reference the chain at their respective slots. Lift that logic into _attestation_data_matches_chain so the two call sites share a single implementation. build_block constructs the chain view that process_block_header would produce on the candidate block (parent_state.historical_block_hashes plus parent_root, plus ZERO_HASH for empty slots) and passes it to the helper, instead of branching inline on source.slot and target.slot.
…nature ty doesn't recognize the SSZ HistoricalBlockHashes container as a Sequence[Bytes32]. Use the concrete type to satisfy the typechecker without introducing structural-typing concessions.
MegaRedHand
changed the title
build_block
pablodeymo
approved these changes
May 13, 2026
4 tasks
Empty slots between blocks carry ZERO_HASH in historical_block_hashes. An attestation whose source or target root is ZERO_HASH could otherwise satisfy the chain-match helper by colliding with one of those entries, even though it doesn't reference a real block. Inline the explicit zero-hash rejection into the helper so both process_attestations and build_block benefit.
The previous filter only bounded source.slot by the latest justified slot. That admits attestations whose source lies on an unjustified slot before the latest justified one (gaps inside the bitfield). Match the STF: skip the attestation unless its source slot is set in the justified-slots bitfield.
3 tasks
pablodeymo
added a commit
to lambdaclass/ethlambda
that referenced
this pull request
May 14, 2026
…366) ## Summary Mirrors [leanSpec#716](leanEthereum/leanSpec#716). The fixed-point attestation loop in `build_block` required `att.source` to equal `current_justified` exactly. When the store's justified checkpoint has advanced via a sibling fork while the canonical head's chain has only proven an earlier slot, the head chain's pool holds a gap-closing attestation whose source differs from the head's `latest_justified` checkpoint. The exact-match filter dropped it and block production aborted with `JustifiedDivergenceNotClosed` ("Fixed-point attestation loop did not converge"). ### Scenario (from the spec PR) ``` block_4(4) -- block_5(5) <-- head / genesis -- 1 -- 2 -- 3 \ block_6(6) ``` - `block_4`: 6/8 attest `target=block_1` (justifies slot 1). - `block_5`: 2/8 attest `target=block_4` (fork-choice weight only). - `block_6` (sibling of block_4): 6/8 attest `target=block_2` (justifies slot 2 on the store). Fork choice picks `block_5` as head; the store's `latest_justified` is `block_2`, but `block_5`'s post-state still sits at `block_1`. The proposer at slot 7 builds on `block_5` and must absorb `block_6`'s attestation from the gossip pool to close the gap. ## Changes In `build_block` (`crates/blockchain/src/store.rs`): - Relax the source filter from `att.source != current_justified` to `att.source.slot > current_justified.slot`, so older-but-already-justified sources can flow through. - Build a chain view that `process_block_header` would produce on the candidate block (the head's `historical_block_hashes` plus `parent_root` at the parent slot plus `H256::ZERO` for each empty slot between parent and the candidate) and reject attestations whose `source.root` or `target.root` do not match it. Prevents pulling in attestations about other forks now that the exact source check is gone. - Skip attestations whose target slot is already justified on the chain, with a genesis self-vote (`source.slot == 0 && target.slot == 0`) exception for fork-choice bootstrapping. - Track `current_finalized_slot` and `current_justified_slots` alongside `current_justified`, refreshing them from `post_state` whenever the STF iteration advances either checkpoint. In `state_transition`: - Add a public `attestation_data_matches_chain(&[H256], &AttestationData) -> bool` helper. - Make the `justified_slots_ops` module `pub` so `is_slot_justified` can be reused from `build_block` without duplicating the relative-index logic. The existing process_attestations path already factored the source/target chain match into `checkpoint_exists` and is left unchanged. ### Test update `build_block_caps_attestation_data_entries` previously used fake target roots that don't appear in the chain. Restructured the setup to build a head state at slot 51 with populated `historical_block_hashes` and vary `AttestationData.slot` (rather than the target root) to produce distinct entries. Still verifies the `MAX_ATTESTATIONS_DATA` cap and gossip-size bound. ## Test plan - [x] `cargo fmt --all` - [x] `cargo clippy --workspace --all-targets -- -D warnings` - [x] `cargo test --workspace --release` (including `forkchoice_spectests`, `stf_spectests`, `signature_spectests` against the pinned leanSpec fixtures) - [ ] Multi-client devnet run to confirm no regression in steady-state finalization > Note: the leanSpec spec test for this scenario (`test_block_production_justification_gap.py`) is not yet in the pinned `LEAN_SPEC_COMMIT_HASH`. A follow-up commit hash bump in `Makefile` will pick it up automatically. --------- Co-authored-by: Pablo Deymonnaz <pdeymon@fi.uba.ar>
zclawz
added a commit
to blockblaz/zeam
that referenced
this pull request
May 16, 2026
…#716) Applies leanSpec commit 00556d850d5a6bce65b232b06c6603cabad6b1b0. The key change in the spec is that `build_block` (our `getProposalAttestationsUnlocked`) previously accepted attestations only when their source root exactly matched the current justified checkpoint. The new spec allows any attestation whose *source slot* is marked justified in the `current_justified_slots` bitfield — i.e. older-but-still-justified sources are now valid. Changes: 1. Add `isSlotJustifiedForBuild` helper Mirror of `utils.isSlotJustified` but returns `false` (never an error) for slots outside the current `justified_slots` window, as used in the build loop. 2. Add `attestationDataMatchesChainExtended` helper Mirrors leanSpec `_attestation_data_matches_chain`. Validates source and target roots against the extended historical block hashes view (pre_state hashes + parent_root + ZERO_HASH for empty slots) that `process_block_header` would produce for the candidate block. Also rejects zero-hash source/target roots inline. 3. Replace root-based source filter with slot-based justified check `getProposalAttestationsUnlocked` now tracks `current_finalized_slot`, `current_justified` (Checkpoint), and `current_justified_slots` (cloned bitfield) alongside the existing `processed_att_data` set. The inner filter loop now: - Accepts attestations whose source *slot* is justified (not just whose source root == latest_justified.root) - Validates source/target roots against the extended chain view - Skips attestations whose target slot is already justified (genesis self-votes excepted, for fork-choice bootstrapping) 4. Update loop restart condition Restart when either `latest_justified` or `latest_finalized.slot` changes (spec previously only restarted on justification change). On restart, all three tracking variables are refreshed from the candidate post-state. Spec ref: leanEthereum/leanSpec#716
zclawz
added a commit
to blockblaz/zeam
that referenced
this pull request
May 16, 2026
…#716) Applies leanSpec commit 00556d850d5a6bce65b232b06c6603cabad6b1b0. The key change in the spec is that `build_block` (our `getProposalAttestationsUnlocked`) previously accepted attestations only when their source root exactly matched the current justified checkpoint. The new spec allows any attestation whose *source slot* is marked justified in the `current_justified_slots` bitfield — i.e. older-but-still-justified sources are now valid. Changes: 1. Add `isSlotJustifiedForBuild` helper Mirror of `utils.isSlotJustified` but returns `false` (never an error) for slots outside the current `justified_slots` window, as used in the build loop. 2. Add `attestationDataMatchesChainExtended` helper Mirrors leanSpec `_attestation_data_matches_chain`. Validates source and target roots against the extended historical block hashes view (pre_state hashes + parent_root + ZERO_HASH for empty slots) that `process_block_header` would produce for the candidate block. Also rejects zero-hash source/target roots inline. 3. Replace root-based source filter with slot-based justified check `getProposalAttestationsUnlocked` now tracks `current_finalized_slot`, `current_justified` (Checkpoint), and `current_justified_slots` (cloned bitfield) alongside the existing `processed_att_data` set. The inner filter loop now: - Accepts attestations whose source *slot* is justified (not just whose source root == latest_justified.root) - Validates source/target roots against the extended chain view - Skips attestations whose target slot is already justified (genesis self-votes excepted, for fork-choice bootstrapping) 4. Update loop restart condition Restart when either `latest_justified` or `latest_finalized.slot` changes (spec previously only restarted on justification change). On restart, all three tracking variables are refreshed from the candidate post-state. Spec ref: leanEthereum/leanSpec#716
ch4r10t33r
pushed a commit
to blockblaz/zeam
that referenced
this pull request
May 16, 2026
…-sync in fc_initing (#892) * forkchoice: allow older-but-justified sources in build_block (leanSpec #716) Applies leanSpec commit 00556d850d5a6bce65b232b06c6603cabad6b1b0. The key change in the spec is that `build_block` (our `getProposalAttestationsUnlocked`) previously accepted attestations only when their source root exactly matched the current justified checkpoint. The new spec allows any attestation whose *source slot* is marked justified in the `current_justified_slots` bitfield — i.e. older-but-still-justified sources are now valid. Changes: 1. Add `isSlotJustifiedForBuild` helper Mirror of `utils.isSlotJustified` but returns `false` (never an error) for slots outside the current `justified_slots` window, as used in the build loop. 2. Add `attestationDataMatchesChainExtended` helper Mirrors leanSpec `_attestation_data_matches_chain`. Validates source and target roots against the extended historical block hashes view (pre_state hashes + parent_root + ZERO_HASH for empty slots) that `process_block_header` would produce for the candidate block. Also rejects zero-hash source/target roots inline. 3. Replace root-based source filter with slot-based justified check `getProposalAttestationsUnlocked` now tracks `current_finalized_slot`, `current_justified` (Checkpoint), and `current_justified_slots` (cloned bitfield) alongside the existing `processed_att_data` set. The inner filter loop now: - Accepts attestations whose source *slot* is justified (not just whose source root == latest_justified.root) - Validates source/target roots against the extended chain view - Skips attestations whose target slot is already justified (genesis self-votes excepted, for fork-choice bootstrapping) 4. Update loop restart condition Restart when either `latest_justified` or `latest_finalized.slot` changes (spec previously only restarted on justification change). On restart, all three tracking variables are refreshed from the candidate post-state. Spec ref: leanEthereum/leanSpec#716 * refactor: remove PR-number references and Mirror-style docstrings - Rename setup716TestChain -> setupJustifiedSourceTestChain in chain.zig - Remove '(leanSpec #716)' from test names in chain.zig - Replace 'Mirror of leanSpec...' docstring in forkchoice.zig with a description of what the function actually does Addresses review feedback from @GrapeBaBa on PR #891. * docs(forkchoice): add leanSpec commit 00556d8 refs to helper function docstrings isSlotJustifiedForBuild and attestationDataMatchesChainExtended were missing the commit hash in their doc comments. GrapeBaBa review on PR #891. * fix(node): retry unserved blocks_by_root roots on EOS/failure + range-sync in fc_initing Root cause of hive test-285 failure (sync: checkpoint sync fresh start, zeam_devnet4): The node checkpoint-synced to slot 17 and entered fc_initing. To exit fc_initing it must import blocks beyond the anchor until a new justified checkpoint is observed. With two connected peers — one at head_slot=23+ (the LeanSpec helper) and one at head_slot=0 (a mesh node with no blocks) — the node used random peer selection for every blocks_by_root parent-chain fetch. When the head_slot=0 peer was selected (50% probability per hop), it responded with EOS and zero chunks. The .completed handler called finalizePendingRequest which cleared the unserved root from pending_block_roots — but triggered no retry. The cached child blocks (e.g. slot 23, 22, …) sat in block_cache forever waiting for a parent nobody would ever fetch again. The node stayed at head_slot=17 for all 180s until the hive test declared failure. Fixes: 1. retryUnservedBlockRoots helper (primary fix): called from both the .completed and .failure arms of handleReqRespResponse for blocks_by_root requests. Collects roots still in pending_block_roots (not served by any prior chunk) BEFORE finalizePendingRequest removes them, then re-schedules each via fetchBlockByRoots so a new (hopefully different) peer can serve it. The warn log 'N/M root(s) unserved — scheduling retry' makes the event visible in hive logs. 2. fc_initing status handler: mirror the behind_peers bulk-sync path. When the peer gap exceeds BLOCKS_BY_RANGE_SYNC_THRESHOLD (64 slots), use blocks_by_range instead of the one-round-trip-per-block by-root parent-chain walk. Falls back to head-by-root on range failure. For the test-285 gap (6 slots) this path is not taken, but it future-proofs larger checkpoint gaps. Both sweepTimedOutRequests (timeout retry) and the new EOS retry now ensure that blocks_by_root requests are always retried regardless of how the peer terminates the RPC. The only difference: timeout retry requires 8 seconds; EOS retry is immediate. Verified: zig build clean, 152/152 unit tests pass. --------- Co-authored-by: zclawz <zclawz@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem Summary
The fixed-point loop in
build_blockrequiresatt.sourceto equalcurrent_justifiedexactly. When the store's justified checkpoint has advanced via a sibling fork (e.g. block_6 justifying block_2 while the canonical head is on a fork at block_1), while the current head justified an earlier slot, the head chain's pool holds an attestation that would close the gap between them, but its source is different from the head'slatest_justifiedcheckpoint so it is filtered out and block production aborts withFixed-point attestation loop did not converge.Reproduction
tests/lean_spec/forks/lstar/forkchoice/test_block_production_justification_gap.pybuilds the scenario explicitly:block_4: 6/8 attesttarget=block_1(justifies slot 1).block_5: 2/8 attesttarget=block_4(fork-choice weight only).block_6(sibling of block_4): 6/8 attesttarget=block_2(justifies slot 2 on the store).Fork choice keeps
block_5as head; the store'slatest_justifiedis nowblock_2, butblock_5's post-state still sits atblock_1. Producing on slot 7 must close the gap.PR Description
This PR relaxes that filter to a slot bound: an attestation is selectable as long as its source slot is at or before the head chain's latest justified slot.
Additionally, we also include some other, now required, sanity checks:
source.rootmatchessource.slot's expected root on the chaintarget.rootmatchestarget.slot's expected root on the chainThere was a specific test for the check we removed (
test_build_block_skips_non_matching_source), which was renamed totest_build_block_skips_other_chain_sourceand repurposed for the new checks on source and target.