fix: allow older-but-justified sources in build_block#716
Open
MegaRedHand wants to merge 11 commits into
Open
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.
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
3 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.