test fixtures: improve forkchoice construction

CLAUDE.md

@@ -50,6 +50,10 @@ uvx tox                  # Everything (checks + tests + docs)
 - Test files/functions must start with `test_`
 - **No example code in docstrings**: Do not include `Example:` sections with code blocks in docstrings. Keep documentation concise and focused on explaining *what* and *why*, not *how to use*. Unit tests serve as usage examples.
 - **No section separator comments**: Never use banner-style separator comments (`# ====...`, `# ----...`, or similar). They add visual clutter with no value. Use blank lines to separate logical sections. If a section needs a heading, a single `#` comment line is enough.
+- **CRITICAL - Preserve existing documentation**: When refactoring or modifying code, NEVER remove or rewrite existing comments and docstrings unless they are directly invalidated by the code change. Removing documentation that still applies creates unnecessary noise in code review diffs and destroys context that was carefully written. Only modify documentation when:
+  - The documented behavior has actually changed
+  - The comment references code that no longer exists
+  - The comment is factually wrong after your change
 
 ### Import Style
 

packages/testing/src/consensus_testing/keys.py

@@ -45,16 +45,24 @@
     AttestationSignatures,
 )
 from lean_spec.subspecs.containers.slot import Slot
+from lean_spec.subspecs.koalabear import Fp
 from lean_spec.subspecs.xmss.aggregation import (
     AggregatedSignatureProof,
     SignatureKey,
 )
+from lean_spec.subspecs.xmss.constants import TARGET_CONFIG
 from lean_spec.subspecs.xmss.containers import KeyPair, PublicKey, Signature
 from lean_spec.subspecs.xmss.interface import (
     PROD_SIGNATURE_SCHEME,
     TEST_SIGNATURE_SCHEME,
     GeneralizedXmssScheme,
 )
+from lean_spec.subspecs.xmss.types import (
+    HashDigestList,
+    HashDigestVector,
+    HashTreeOpening,
+    Randomness,
+)
 from lean_spec.types import Uint64
 
 __all__ = [
@@ -64,6 +72,7 @@
     "LazyKeyDict",
     "NUM_VALIDATORS",
     "XmssKeyManager",
+    "create_dummy_signature",
     "download_keys",
     "get_keys_dir",
     "get_shared_key_manager",
@@ -94,6 +103,29 @@
 """Default max slot for the shared key manager."""
 
 
+def create_dummy_signature() -> Signature:
+    """
+    Create a structurally valid but cryptographically invalid individual signature.
+
+    The signature has proper structure (correct number of siblings, hashes, etc.)
+    but all values are zeros, so it will fail cryptographic verification.
+    """
+    # Create zero-filled hash digests with correct dimensions
+    zero_digest = HashDigestVector(data=[Fp(0) for _ in range(TARGET_CONFIG.HASH_LEN_FE)])
+
+    # Path needs LOG_LIFETIME siblings for the Merkle authentication path
+    siblings = HashDigestList(data=[zero_digest for _ in range(TARGET_CONFIG.LOG_LIFETIME)])
+
+    # Hashes need DIMENSION vectors for the Winternitz chain hashes
+    hashes = HashDigestList(data=[zero_digest for _ in range(TARGET_CONFIG.DIMENSION)])
+
+    return Signature(
+        path=HashTreeOpening(siblings=siblings),
+        rho=Randomness(data=[Fp(0) for _ in range(TARGET_CONFIG.RAND_LEN_FE)]),
+        hashes=hashes,
+    )
+
+
 def get_shared_key_manager(max_slot: Slot = _SHARED_MANAGER_MAX_SLOT) -> XmssKeyManager:
     """
     Get a shared XMSS key manager for reusing keys across tests.

packages/testing/src/consensus_testing/test_fixtures/verify_signatures.py

@@ -24,48 +24,16 @@
 from lean_spec.subspecs.containers.checkpoint import Checkpoint
 from lean_spec.subspecs.containers.state.state import State
 from lean_spec.subspecs.containers.validator import ValidatorIndex
-from lean_spec.subspecs.koalabear import Fp
 from lean_spec.subspecs.ssz import hash_tree_root
 from lean_spec.subspecs.xmss.aggregation import AggregatedSignatureProof
-from lean_spec.subspecs.xmss.constants import TARGET_CONFIG
-from lean_spec.subspecs.xmss.containers import Signature
-from lean_spec.subspecs.xmss.types import (
-    HashDigestList,
-    HashDigestVector,
-    HashTreeOpening,
-    Randomness,
-)
 from lean_spec.types import Bytes32
 from lean_spec.types.byte_arrays import ByteListMiB
 
-from ..keys import XmssKeyManager, get_shared_key_manager
+from ..keys import XmssKeyManager, create_dummy_signature, get_shared_key_manager
 from ..test_types import AggregatedAttestationSpec, BlockSpec
 from .base import BaseConsensusFixture
 
 
-def _create_dummy_signature() -> Signature:
-    """
-    Create a structurally valid but cryptographically invalid individual signature.
-
-    The signature has proper structure (correct number of siblings, hashes, etc.)
-    but all values are zeros, so it will fail cryptographic verification.
-    """
-    # Create zero-filled hash digests with correct dimensions
-    zero_digest = HashDigestVector(data=[Fp(0) for _ in range(TARGET_CONFIG.HASH_LEN_FE)])
-
-    # Path needs LOG_LIFETIME siblings for the Merkle authentication path
-    siblings = HashDigestList(data=[zero_digest for _ in range(TARGET_CONFIG.LOG_LIFETIME)])
-
-    # Hashes need DIMENSION vectors for the Winternitz chain hashes
-    hashes = HashDigestList(data=[zero_digest for _ in range(TARGET_CONFIG.DIMENSION)])
-
-    return Signature(
-        path=HashTreeOpening(siblings=siblings),
-        rho=Randomness(data=[Fp(0) for _ in range(TARGET_CONFIG.RAND_LEN_FE)]),
-        hashes=hashes,
-    )
-
-
 def _create_dummy_aggregated_proof(validator_ids: list[ValidatorIndex]) -> AggregatedSignatureProof:
     """
     Create a dummy aggregated signature proof with invalid proof data.
@@ -322,7 +290,7 @@ def _build_block_from_spec(
                 proposer_attestation.data,
             )
         else:
-            proposer_attestation_signature = _create_dummy_signature()
+            proposer_attestation_signature = create_dummy_signature()
 
         return SignedBlockWithAttestation(
             message=BlockWithAttestation(

packages/testing/src/consensus_testing/test_types/store_checks.py

@@ -431,7 +431,7 @@ def validate_against_store(
                     # Extract attestations from aggregated payloads
                     if check.location == "new":
                         extracted_attestations = (
-                            store._extract_attestations_from_aggregated_payloads(
+                            store.extract_attestations_from_aggregated_payloads(
                                 store.latest_new_aggregated_payloads
                             )
                         )
@@ -445,7 +445,7 @@ def validate_against_store(
 
                     else:  # check.location == "known"
                         extracted_attestations = (
-                            store._extract_attestations_from_aggregated_payloads(
+                            store.extract_attestations_from_aggregated_payloads(
                                 store.latest_known_aggregated_payloads
                             )
                         )
@@ -572,7 +572,7 @@ def validate_against_store(
                     # Calculate attestation weight: count attestations voting for this fork
                     # An attestation votes for this fork if its head is this block or a descendant
                     # Extract attestations from latest_known_aggregated_payloads
-                    known_attestations = store._extract_attestations_from_aggregated_payloads(
+                    known_attestations = store.extract_attestations_from_aggregated_payloads(
                         store.latest_known_aggregated_payloads
                     )
                     weight = 0

src/lean_spec/subspecs/forkchoice/store.py

@@ -690,7 +690,7 @@ def on_block(
 
         return store
 
-    def _extract_attestations_from_aggregated_payloads(
+    def extract_attestations_from_aggregated_payloads(
         self, aggregated_payloads: dict[SignatureKey, list[AggregatedSignatureProof]]
     ) -> dict[ValidatorIndex, AttestationData]:
         """
@@ -841,7 +841,7 @@ def update_head(self) -> "Store":
 
         """
         # Extract attestations from known aggregated payloads
-        attestations = self._extract_attestations_from_aggregated_payloads(
+        attestations = self.extract_attestations_from_aggregated_payloads(
             self.latest_known_aggregated_payloads
         )
 
@@ -930,7 +930,7 @@ def update_safe_target(self) -> "Store":
         min_target_score = -(-num_validators * 2 // 3)
 
         # Extract attestations from new aggregated payloads
-        attestations = self._extract_attestations_from_aggregated_payloads(
+        attestations = self.extract_attestations_from_aggregated_payloads(
             self.latest_new_aggregated_payloads
         )
 
@@ -1287,7 +1287,7 @@ def produce_block_with_signatures(
         # Extract attestations from known aggregated payloads.
         # These attestations have already influenced fork choice.
         # Including them in the block makes them permanent on-chain.
-        attestation_data_map = store._extract_attestations_from_aggregated_payloads(
+        attestation_data_map = store.extract_attestations_from_aggregated_payloads(
             store.latest_known_aggregated_payloads
         )
         available_attestations = [

tests/consensus/devnet/fc/test_signature_aggregation.py

@@ -308,13 +308,15 @@ def test_auto_collect_proposer_attestations(
                 ),
                 checks=StoreChecks(
                     head_slot=Slot(2),
-                    # Proposer 1's attestation should be auto-collected
+                    # Proposer 1's attestation should be auto-collected.
+                    # Target is genesis (slot 0) because the spec's attestation target
+                    # algorithm walks back from head toward safe_target.
                     block_attestation_count=1,
                     block_attestations=[
                         AggregatedAttestationCheck(
                             participants={1},
                             attestation_slot=Slot(1),
-                            target_slot=Slot(1),
+                            target_slot=Slot(0),
                         ),
                     ],
                 ),
@@ -337,10 +339,10 @@ def test_auto_collect_combined_with_explicit_attestations(
 
     Expected
     --------
-    Block body contains merged attestation from all sources:
-    - Proposer 1's attestation (auto-collected)
-    - Validators 0 and 3 (explicitly specified)
-    - All merged into single aggregation (same target)
+    Block body contains attestations from all sources.
+    Proposer 1's attestation targets genesis (slot 0) via the spec's target
+    walk-back algorithm, while explicit attestations target block_1 (slot 1).
+    Different targets produce separate aggregation groups.
     """
     fork_choice_test(
         steps=[
@@ -363,11 +365,16 @@ def test_auto_collect_combined_with_explicit_attestations(
                 ),
                 checks=StoreChecks(
                     head_slot=Slot(2),
-                    # All attestations merged: proposer 1 + explicit {0, 3}
-                    block_attestation_count=1,
+                    # Two separate groups: proposer targets genesis, explicit targets block_1
+                    block_attestation_count=2,
                     block_attestations=[
                         AggregatedAttestationCheck(
-                            participants={0, 1, 3},
+                            participants={1},
+                            attestation_slot=Slot(1),
+                            target_slot=Slot(0),
+                        ),
+                        AggregatedAttestationCheck(
+                            participants={0, 3},
                             attestation_slot=Slot(1),
                             target_slot=Slot(1),
                         ),

tests/lean_spec/subspecs/forkchoice/test_store_attestations.py

@@ -59,7 +59,7 @@ def test_on_block_processes_multi_validator_aggregations(key_manager: XmssKeyMan
     updated_store = consumer_store.on_block(signed_block)
 
     # Verify attestations can be extracted from aggregated payloads
-    extracted_attestations = updated_store._extract_attestations_from_aggregated_payloads(
+    extracted_attestations = updated_store.extract_attestations_from_aggregated_payloads(
         updated_store.latest_known_aggregated_payloads
     )
     assert ValidatorIndex(1) in extracted_attestations