tcoratger
diff --git a/‎src/lean_spec/forks/lstar/spec.py‎
Lines changed: 76 additions & 37 deletions b/‎src/lean_spec/forks/lstar/spec.py‎
Lines changed: 76 additions & 37 deletions
diff --git a/‎tests/lean_spec/forks/lstar/forkchoice/test_block_production_justification_gap.py‎
Lines changed: 44 additions & 82 deletions b/‎tests/lean_spec/forks/lstar/forkchoice/test_block_production_justification_gap.py‎
Lines changed: 44 additions & 82 deletions
@@ -1,7 +1,7 @@
 """Lstar fork — identity and construction facade."""
 
 from collections import defaultdict
-from collections.abc import Iterable
+from collections.abc import Iterable, Sequence
 from collections.abc import Set as AbstractSet
 from typing import Any, ClassVar
 
@@ -354,29 +354,41 @@ def process_block(self, state: State, block: Block) -> State:
 
         return self.process_attestations(state, block.body.attestations)
 
+    @staticmethod
     def _attestation_data_matches_chain(
-        self,
-        historical_block_hashes: HistoricalBlockHashes,
         attestation_data: AttestationData,
+        historical_block_hashes: Sequence[Bytes32],
     ) -> bool:
-        """Whether both source and target checkpoints match the chain at their slots.
+        """Check that source and target checkpoints point to blocks on a chain.
 
-        Callers pass the chain's historical block hashes as they would appear
-        after process_block_header on the consuming block: covering
-        [0, block.slot - 1] with parent_root at parent.slot and ZERO_HASH
-        for empty slots between parent and the candidate.
+        Args:
+            attestation_data: The attestation being validated.
+            historical_block_hashes: Chain view indexed by slot.
+                Empty slots carry the zero hash.
 
-        Empty slots carry ZERO_HASH; a vote referencing one is rejected here
-        even when the recorded root happens to compare equal.
+        Returns:
+            True when both checkpoint roots match the chain at their slot.
+            False when either root is the zero hash.
+            False when either checkpoint slot is past the end of the chain view.
         """
+        # Reject zero-hash checkpoints up front.
+        #
+        # Empty slots carry the zero hash on the chain.
+        # A vote whose recorded root equals the zero hash is meaningless.
         if attestation_data.source.root == ZERO_HASH or attestation_data.target.root == ZERO_HASH:
             return False
+
+        # Reject checkpoints whose slot is beyond the chain view.
+        #
+        # Without this guard, indexed access raises IndexError.
         source_slot = int(attestation_data.source.slot)
         target_slot = int(attestation_data.target.slot)
         if source_slot >= len(historical_block_hashes):
             return False
         if target_slot >= len(historical_block_hashes):
             return False
+
+        # Both checkpoint roots must match the chain at their slot.
         return (
             attestation_data.source.root == historical_block_hashes[source_slot]
             and attestation_data.target.root == historical_block_hashes[target_slot]
@@ -472,10 +484,15 @@ def process_attestations(
                 continue
 
             # Ensure the vote refers to blocks that actually exist on our chain.
-            # Prevents votes about unknown or conflicting forks; also rejects
-            # zero-hash source or target roots inline.
+            #
+            # The attestation must match our canonical chain.
+            # Both the source root and target root must equal the recorded block roots.
+            # The recorded roots are the ones stored for those slots in history.
+            #
+            # This prevents votes about unknown or conflicting forks.
+            # It also rejects zero-hash source or target roots.
             if not self._attestation_data_matches_chain(
-                state.historical_block_hashes, attestation.data
+                attestation.data, state.historical_block_hashes.data
             ):
                 continue
 
@@ -676,21 +693,24 @@ def build_block(
             else:
                 current_justified = state.latest_justified
 
-            # Track the justified-slot bitfield so we can skip attestations
-            # whose target slot is already justified on this chain. Extend
-            # so is_slot_justified doesn't raise for target slots between
-            # parent.slot and slot - 1.
+            # Track the justified-slot bitfield to skip already-justified targets.
+            #
+            # Extend the bitfield to cover every slot we might query.
+            # The range runs from the finalized boundary up to slot - 1 inclusive.
             current_finalized_slot = state.latest_finalized.slot
             current_justified_slots = state.justified_slots.extend_to_slot(
                 current_finalized_slot, slot - Slot(1)
             )
 
-            # Build the chain view that process_block_header would produce on
-            # the candidate block. Lets the chain-match helper validate source
-            # and target roots without waiting for the STF to drop mismatches.
+            # Build the chain view as it will appear on the candidate block.
+            #
+            # The view is the recorded history up to the parent.
+            # Then comes the parent root at the parent's slot.
+            # Then zero-hash entries for any skipped slots up to the new block.
+            # The chain-match helper uses this view to validate source and target roots.
             num_empty_slots = int(slot - state.latest_block_header.slot - Slot(1))
-            extended_historical_block_hashes = (
-                state.historical_block_hashes + [parent_root] + [ZERO_HASH] * num_empty_slots
+            extended_historical_block_hashes: list[Bytes32] = (
+                list(state.historical_block_hashes) + [parent_root] + [ZERO_HASH] * num_empty_slots
             )
 
             processed_att_data: set[AttestationData] = set()
@@ -701,32 +721,46 @@ def build_block(
                 for att_data, proofs in sorted(
                     aggregated_payloads.items(), key=lambda item: item[0].target.slot
                 ):
-                    if (
-                        Uint8(len(processed_att_data)) >= MAX_ATTESTATIONS_DATA
-                        and att_data not in processed_att_data
-                    ):
+                    if att_data in processed_att_data:
+                        continue
+
+                    if Uint8(len(processed_att_data)) >= MAX_ATTESTATIONS_DATA:
                         break
 
                     if att_data.head.root not in known_block_roots:
                         continue
 
-                    # Source slot must already be justified on this chain.
-                    if not current_justified_slots.is_slot_justified(
-                        current_finalized_slot, att_data.source.slot
+                    # Chain-match runs first.
+                    #
+                    # It rejects checkpoints whose slot is past the chain view.
+                    # That prevents the bounded queries below from indexing out of range.
+                    if not self._attestation_data_matches_chain(
+                        att_data, extended_historical_block_hashes
                     ):
                         continue
 
-                    if not self._attestation_data_matches_chain(
-                        extended_historical_block_hashes, att_data
+                    # The source slot must already be justified on this chain.
+                    if not current_justified_slots.is_slot_justified(
+                        current_finalized_slot, att_data.source.slot
                     ):
                         continue
 
-                    # Skip attestations whose target slot is already
-                    # justified on this chain. Ignore genesis self-votes
-                    # for fork-choice bootstrapping
-                    is_genesis_self_vote = att_data.source.slot == Slot(
-                        0
-                    ) and att_data.target.slot == Slot(0)
+                    # Genesis-anchored votes have source.slot = target.slot = 0.
+                    #
+                    # They cannot advance justification: the state transition drops them.
+                    # They still carry head-vote weight for fork choice.
+                    # Including them in the body propagates them into peers' payload pool.
+                    # The bypass below keeps them past the target-already-justified check,
+                    # since slot 0 is implicitly justified and would otherwise filter them.
+                    is_genesis_self_vote = att_data.source.slot == Slot(0) and (
+                        att_data.target.slot == Slot(0)
+                    )
+
+                    # Skip attestations whose target slot is already justified.
+                    #
+                    # Justification adds nothing for them.
+                    # Entries the state transition will later drop are still kept here.
+                    # They carry head-vote weight for fork choice.
                     if not is_genesis_self_vote and current_justified_slots.is_slot_justified(
                         current_finalized_slot, att_data.target.slot
                     ):
@@ -763,6 +797,11 @@ def build_block(
                 )
                 post_state = self.process_block(self.process_slots(state, slot), candidate_block)
 
+                # Re-run the filter when justification or finalization advanced.
+                #
+                # Both quantities are monotonic in 3SF-mini, so the loop is bounded.
+                # Finalization advancement shifts the justified window forward.
+                # That can unlock attestations whose target slot was outside it before.
                 if (
                     post_state.latest_justified != current_justified
                     or post_state.latest_finalized.slot != current_finalized_slot
 
@@ -1,12 +1,4 @@
-"""
-Block production closes the justification gap on a canonical head that lags.
-
-The scenario builds a fork tree where one branch advances the store's justified
-checkpoint past the level that the canonical head's chain has proven. The
-fixed-point attestation loop inside build_block picks up the gap-closing
-attestation from the gossip pool and produces a block whose post-state
-catches up to the store.
-"""
+"""Block production closes the justification gap when the canonical head lags."""
 
 from __future__ import annotations
 
@@ -28,63 +20,45 @@ def test_produce_block_on_head_with_lagging_justification(
     keyed_genesis_block: Block,
     key_manager: XmssKeyManager,
 ) -> None:
-    """
-    Closing the justification gap via the fixed-point attestation loop.
+    r"""Producer on a lagging head pulls a sibling's attestation to catch up.
 
-    Fork tree (labels are block names; slot numbers shown in parentheses)::
+    Fork tree::
 
-                               block_4(4) -- block_5(5)  <-- head
-                              /
+                              block_4(4) -- block_5(5)  <-- head
+                             /
         genesis -- 1 -- 2 -- 3
-                              \
-                               block_6(6)
-
-    Setup:
-
-    - block_4 carries 6 of 8 attestations targeting block_1, justifying block_1.
-    - block_5 carries 2 attestations (validators 6, 7) with head=block_4. These
-      give block_5's subtree fork-choice weight against block_6.
-    - block_6 (sibling of block_4) carries 6 of 8 attestations targeting block_2.
-      Block_6 alone moves the store's latest_justified to block_2.
-
-    After all six blocks are processed:
-
-    - store.latest_justified points at block_2 (slot 2)
-    - fork choice still picks block_5 as the head:
-        validators 0-5 vote head=block_2 (a common ancestor)
-        validators 6-7 vote head=block_4 (in block_5's subtree)
-        weight on block_3's child block_4 is 2; weight on block_6 is 0
-
-    Proposing at slot 7 builds on block_5, whose post-state has
-    latest_justified=block_1. The gossip pool holds block_6's attestation
-    (source=genesis, target=block_2). The fixed-point loop accepts that
-    attestation because genesis is justified on block_5's chain and
-    block_2 is on the common ancestor segment, so the produced block's
-    post-state catches up to the store's justified checkpoint.
+                             \
+                              block_6(6)
+
+    The head branch only justifies block_1.
+    The sibling branch justifies block_2 and pushes the store ahead of the head.
+    Producing on top of the head must reuse the sibling's attestation to close the gap.
     """
     store = keyed_store
     block_registry: dict[str, Block] = {"genesis": keyed_genesis_block}
 
     def add_block(block_spec: BlockSpec) -> None:
+        """Build the spec'd block on the current store and apply it."""
         nonlocal store
         signed_block = block_spec.build_signed_block_with_store(
             store, block_registry, key_manager, "test"
         )
         if block_spec.label is not None:
             block_registry[block_spec.label] = signed_block.block
-        # Re-align store time with the block's slot before processing.
-        # build_signed_block_with_store already ticks forward; this second tick
-        # is idempotent and mirrors the fork-choice spec-test fixture.
+        # The block builder helper ticks a local store copy and discards it.
+        # The outer store therefore still sits at genesis time.
+        # Without this tick, the block is rejected as too far in the future.
         target_interval = Interval.from_slot(signed_block.block.slot)
         store, _ = spec.on_tick(store, target_interval, has_proposal=True, is_aggregator=True)
         store = spec.on_block(store, signed_block)
 
-    # Linear chain through block_3.
+    # Phase 1: linear chain genesis -> block_1 -> block_2 -> block_3.
     add_block(BlockSpec(slot=Slot(1), label="block_1"))
     add_block(BlockSpec(slot=Slot(2), label="block_2"))
     add_block(BlockSpec(slot=Slot(3), label="block_3"))
 
-    # block_4: 6 of 8 validators attest target=block_1, justifying block_1.
+    # Phase 2a: block_4 carries 6/8 votes for target=block_1.
+    # Crosses the 2/3 threshold, so block_1 becomes justified.
     add_block(
         BlockSpec(
             slot=Slot(4),
@@ -100,12 +74,12 @@ def add_block(block_spec: BlockSpec) -> None:
             ],
         )
     )
-    assert store.latest_justified.slot == Slot(1)
-    assert store.latest_justified.root == hash_tree_root(block_registry["block_1"])
+    block_1_root = hash_tree_root(block_registry["block_1"])
+    assert store.latest_justified == Checkpoint(root=block_1_root, slot=Slot(1))
 
-    # block_5: validators 6, 7 attest target=block_4 with head=block_4.
-    # The head vote pulls fork-choice weight into block_5's subtree without
-    # advancing justification.
+    # Phase 2b: block_5 carries 2/8 head votes for block_4.
+    # Below the 2/3 threshold, so justification does not advance.
+    # The votes pull fork-choice weight into block_5's subtree.
     add_block(
         BlockSpec(
             slot=Slot(5),
@@ -121,11 +95,11 @@ def add_block(block_spec: BlockSpec) -> None:
             ],
         )
     )
-    assert store.latest_justified.slot == Slot(1)
+    assert store.latest_justified == Checkpoint(root=block_1_root, slot=Slot(1))
 
-    # block_6: sibling of block_4 (parent=block_3). 6 of 8 validators attest
-    # target=block_2 with source=genesis. After processing, the store's
-    # latest_justified advances to block_2.
+    # Phase 3: block_6 (sibling of block_4) carries 6/8 votes for target=block_2.
+    # The store learns block_2 is justified.
+    # block_5's chain still has latest_justified = block_1: the divergence.
     add_block(
         BlockSpec(
             slot=Slot(6),
@@ -142,46 +116,34 @@ def add_block(block_spec: BlockSpec) -> None:
         )
     )
 
-    # Sanity: block_5 is the head and the store's justified is block_2.
-    # Validators 0-5's latest vote is for head=block_2 (common ancestor).
-    # Validators 6, 7's latest vote is for head=block_4 (in block_5's subtree).
-    # block_4's subtree carries weight 2; block_6's subtree carries weight 0.
-    assert store.head == hash_tree_root(block_registry["block_5"])
-    assert store.latest_justified.slot == Slot(2)
-    assert store.latest_justified.root == hash_tree_root(block_registry["block_2"])
-
-    # block_6's body attestation justifying block_2 is merged into the
-    # store's known aggregated payload pool. Its source is genesis (the
-    # latest_justified at block_6's parent, block_3), so the filter must
-    # match on source-slot-justified, not full Checkpoint equality, to be
-    # able to reuse it from block_5's chain.
+    # Pre-condition: head is block_5, but the store's justified is ahead at block_2.
+    # Validators 0-5 vote head=block_2 (a common ancestor); validators 6-7 vote head=block_4.
+    # block_5's subtree wins fork choice with weight 2 vs 0.
     genesis_root = hash_tree_root(keyed_genesis_block)
     block_2_root = hash_tree_root(block_registry["block_2"])
+    block_2_checkpoint = Checkpoint(root=block_2_root, slot=Slot(2))
+    assert store.head == hash_tree_root(block_registry["block_5"])
+    assert store.latest_justified == block_2_checkpoint
+
+    # The gap-closing attestation is in the pool: source=genesis, target=block_2.
+    # Its source is NOT block_5's latest_justified (which is block_1).
+    # The filter must accept it on source-slot-justified, not full-Checkpoint equality.
     block_6_target_atts = [
-        att
-        for att in store.latest_known_aggregated_payloads
-        if att.target.root == block_2_root and att.target.slot == Slot(2)
+        att for att in store.latest_known_aggregated_payloads if att.target == block_2_checkpoint
     ]
     assert len(block_6_target_atts) == 1
     assert block_6_target_atts[0].source == Checkpoint(root=genesis_root, slot=Slot(0))
     assert block_6_target_atts[0].slot == Slot(6)
 
-    # Propose at slot 7 on top of block_5 (the head). The fixed-point loop
-    # picks up block_6's attestation (source=genesis matches the chain at
-    # slot 0) and advances the produced block's justified checkpoint to
-    # block_2 to match the store.
+    # Propose at slot 7 on top of block_5.
+    # The block builder picks up the gap-closing attestation and advances justification.
     new_store, new_block, _ = spec.produce_block_with_signatures(store, Slot(7), ValidatorIndex(7))
 
-    # The store's justified checkpoint stays at block_2; the new block
-    # closes the gap rather than leaving the producer behind.
-    block_2_checkpoint = Checkpoint(root=block_2_root, slot=Slot(2))
-    assert new_store.latest_justified == block_2_checkpoint
-
-    # The new block's post-state caught up to the store's justified checkpoint.
+    # The produced block's post-state caught up to the store's justified checkpoint.
+    # Its body carries the attestation that closed the gap.
     new_block_root = hash_tree_root(new_block)
+    body_targets = [att.data.target for att in new_block.body.attestations]
+    assert new_store.latest_justified == block_2_checkpoint
     assert new_block.parent_root == hash_tree_root(block_registry["block_5"])
     assert new_store.states[new_block_root].latest_justified == block_2_checkpoint
-
-    # The produced block must include the attestation that justifies block_2.
-    body_targets = [att.data.target for att in new_block.body.attestations]
     assert block_2_checkpoint in body_targets