From e3f94e8cf6349892a2f859c94be26d4833611e19 Mon Sep 17 00:00:00 2001 From: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> Date: Sat, 4 Jul 2026 11:38:54 +0200 Subject: [PATCH 1/2] fix(fork-choice): state store invariants; reject attestations off the finalized subtree Four fork-choice store properties were relied on but never stated as invariants, each a clause a formal Store well-formedness predicate would have to assume. Three hold by construction today and are now documented; one is closed with a typed rejection. M-1 (justified descends from finalized): documented as an invariant on the justified checkpoint and on the slot-only checkpoint advance. It holds by construction: every post-state finalizes an ancestor of what it justifies, and the head walk anchors at the justified root. M-2 (admission mirrors pruning): attestation admission now rejects a vote whose head does not descend from the finalized block, mirroring the prune predicate. Without it a re-gossiped stale aggregate re-entered the pool after pruning dropped it. Such a vote never changed the head, so this is DoS and idempotence hardening, not a safety fix. Adds a typed rejection reason and a proving vector; the pruning test is reworked to seed stale entries without the now-rejected below-finalized gossip. M-3 (pruning against a reorg-mutable finalized): documented only. See the PR description for why monotone finalization is deliberately not adopted. M-4 (blocks and states share one key set): documented as an invariant on the store; the dependent asserts are relabelled as provably-unreachable internal checks. The block-builder test harness was synthesizing block aggregates through the gossip admission path, which the real block processing never uses. It now seeds the signature pool directly, so block-carried attestations are not gossip-validated against the current finalized checkpoint. Refs #1176 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test_types/block_spec.py | 41 ++--- .../spec/forks/lstar/containers/checkpoint.py | 4 + .../spec/forks/lstar/containers/store.py | 6 +- src/lean_spec/spec/forks/lstar/errors.py | 3 + src/lean_spec/spec/forks/lstar/fork_choice.py | 13 ++ .../test_prune_finalized_orphaned_branch.py | 169 +++++++++++++++++- .../lstar/fork_choice/test_store_pruning.py | 42 +---- 7 files changed, 216 insertions(+), 62 deletions(-) diff --git a/packages/testing/src/consensus_testing/test_types/block_spec.py b/packages/testing/src/consensus_testing/test_types/block_spec.py index 894eaa365..70afdc741 100644 --- a/packages/testing/src/consensus_testing/test_types/block_spec.py +++ b/packages/testing/src/consensus_testing/test_types/block_spec.py @@ -11,16 +11,16 @@ from lean_spec.base import CamelModel from lean_spec.spec.crypto.merkleization import hash_tree_root from lean_spec.spec.crypto.xmss.containers import PublicKey, Signature -from lean_spec.spec.forks import AggregationBits, Interval, Slot, ValidatorIndex +from lean_spec.spec.forks import AggregationBits, Slot, ValidatorIndex from lean_spec.spec.forks.lstar.containers import ( AggregatedAttestation, AggregatedAttestations, Attestation, AttestationData, + AttestationSignatureEntry, Block, BlockBody, MultiMessageAggregate, - SignedAttestation, SignedBlock, SingleMessageAggregate, State, @@ -418,16 +418,18 @@ def build_signed_block_with_store( parent_state, block_registry, key_manager ) - # In-body attestations carry the block's slot. - # The store's time check rejects votes whose slot has not yet started locally, - # so advance the local clock first. - block_slot_interval = Interval.from_slot(self.slot) - if store.time < block_slot_interval: - store, _ = spec.on_tick( - store, block_slot_interval, has_proposal=True, is_aggregator=True - ) - - # Gossip valid signatures so they run through the spec's verification path. + # Seed the aggregator's signature pool directly from the already-signed votes. + # A proposer includes attestations it collected earlier, not fresh gossip. + # The real block-import path never gossip-validates block-carried attestations. + # It verifies their proofs and runs the state transition instead. + # Routing these votes through the gossip admission guard would wrongly reject a + # legitimately-includable vote whose head no longer descends from the finalized block. + # Each vote here was signed by the builder itself, so it is valid by construction. + # Existing pool entries are preserved, mirroring how gossip merges into the pool. + grouped_signatures: dict[AttestationData, set[AttestationSignatureEntry]] = { + existing_data: set(existing_signatures) + for existing_data, existing_signatures in store.attestation_signatures.items() + } for attestation in valid_attestations: signatures_for_data = attestation_signatures.get(attestation.data) if ( @@ -435,18 +437,13 @@ def build_signed_block_with_store( or (signature := signatures_for_data.get(attestation.validator_index)) is None ): continue - store = spec.on_gossip_attestation( - store, - SignedAttestation( - validator_index=attestation.validator_index, - data=attestation.data, - signature=signature, - ), - is_aggregator=True, + grouped_signatures.setdefault(attestation.data, set()).add( + AttestationSignatureEntry(attestation.validator_index, signature) ) + store = store.model_copy(update={"attestation_signatures": grouped_signatures}) - # Aggregate gossip signatures into known payloads on the local clone. - # Gossip pools mutate here, but the caller's gossip view must not be consumed. + # Aggregate the pooled signatures into known payloads on the local clone. + # The pools mutate here, but the caller's signature pool must not be consumed. # Only the freshly aggregated payloads propagate back. aggregation_store, _ = spec.aggregate(store) merged_store = spec.accept_new_attestations(aggregation_store) diff --git a/src/lean_spec/spec/forks/lstar/containers/checkpoint.py b/src/lean_spec/spec/forks/lstar/containers/checkpoint.py index 672779dc4..ffc21ad99 100644 --- a/src/lean_spec/spec/forks/lstar/containers/checkpoint.py +++ b/src/lean_spec/spec/forks/lstar/containers/checkpoint.py @@ -26,6 +26,10 @@ def advance_to(self, candidate: "Checkpoint") -> "Checkpoint": The candidate replaces this checkpoint only when its slot is strictly higher. This enforces forward-only progression for justified and finalized checkpoints. + + Selection is by slot only. + It does not verify that the candidate's block descends from this one. + For justified and finalized checkpoints that ancestry is a separate store invariant. """ return candidate if candidate.slot > self.slot else self diff --git a/src/lean_spec/spec/forks/lstar/containers/store.py b/src/lean_spec/spec/forks/lstar/containers/store.py index 560a0ddb4..d296445ce 100644 --- a/src/lean_spec/spec/forks/lstar/containers/store.py +++ b/src/lean_spec/spec/forks/lstar/containers/store.py @@ -40,14 +40,14 @@ class Store[StateT: Container, BlockT: Container](StrictBaseModel): """Root of the block a validator is safe to attest to.""" latest_justified: Checkpoint - """Highest-slot justified checkpoint observed so far.""" + """Highest-slot justified checkpoint observed so far; the head walk starts here.""" latest_finalized: Checkpoint """ Finalization as seen from the canonical head, not irreversible economic finality. - This tracks the head chain's view and is reorg-mutable. - A reorg onto a fork that finalized a lower slot lowers this value. + Re-derived from the head each update, so it is reorg-mutable and can lower on a reorg. + Always an ancestor of the head, never monotone, never a safety guarantee. """ blocks: dict[Bytes32, BlockT] = Field(default_factory=dict) diff --git a/src/lean_spec/spec/forks/lstar/errors.py b/src/lean_spec/spec/forks/lstar/errors.py index 7dece283b..ca2e6e383 100644 --- a/src/lean_spec/spec/forks/lstar/errors.py +++ b/src/lean_spec/spec/forks/lstar/errors.py @@ -74,6 +74,9 @@ class RejectionReason(StrEnum): TARGET_NOT_ANCESTOR_OF_HEAD = "TARGET_NOT_ANCESTOR_OF_HEAD" """The attestation target checkpoint is not an ancestor of its head.""" + HEAD_NOT_DESCENDANT_OF_FINALIZED = "HEAD_NOT_DESCENDANT_OF_FINALIZED" + """The attestation head checkpoint does not descend from the finalized block.""" + ATTESTATION_TOO_FAR_IN_FUTURE = "ATTESTATION_TOO_FAR_IN_FUTURE" """The attestation slot is beyond the store's acceptance horizon.""" diff --git a/src/lean_spec/spec/forks/lstar/fork_choice.py b/src/lean_spec/spec/forks/lstar/fork_choice.py index 1f3292ad5..72ab6c803 100644 --- a/src/lean_spec/spec/forks/lstar/fork_choice.py +++ b/src/lean_spec/spec/forks/lstar/fork_choice.py @@ -142,6 +142,8 @@ def prune_stale_attestation_data(self, store: LstarStore) -> LstarStore: Drop attestation data whose head can no longer influence fork choice. Fork choice only ever descends from the latest finalized block. + This is sound only because the finalized checkpoint is re-derived from the head each update. + Pruning against a finalized checkpoint that drifted off the head chain would be unsound. A vote carries no fork-choice weight, and is dropped, when either holds: - Its head sits at or below the finalized slot. @@ -197,9 +199,13 @@ def validate_attestation(self, store: LstarStore, attestation_data: AttestationD Validate incoming attestation before processing. Ensures the vote respects the basic laws of time and topology. + The head must also descend from the finalized block. + Fork choice only descends from the finalized block, so an orphaned head carries no weight. + Rejecting it here stops a stale vote from re-entering after pruning drops it. Raises: SpecRejectionError: If the attestation fails any of the validation checks above. + SpecRejectionError: HEAD_NOT_DESCENDANT_OF_FINALIZED when the head is off finalized. """ source_checkpoint = attestation_data.source target_checkpoint = attestation_data.target @@ -270,6 +276,13 @@ def validate_attestation(self, store: LstarStore, attestation_data: AttestationD "Target checkpoint must be ancestor of head", ) + # Fork choice only ever descends from the finalized block. + if not self._checkpoint_is_ancestor(store, store.latest_finalized, head_checkpoint): + raise SpecRejectionError( + RejectionReason.HEAD_NOT_DESCENDANT_OF_FINALIZED, + "Head checkpoint must descend from the finalized block", + ) + # Head Consistency Check # # A vote cannot have observed its head before that head existed. diff --git a/tests/consensus/lstar/fork_choice/test_prune_finalized_orphaned_branch.py b/tests/consensus/lstar/fork_choice/test_prune_finalized_orphaned_branch.py index 0f670a8ea..1e34003d1 100644 --- a/tests/consensus/lstar/fork_choice/test_prune_finalized_orphaned_branch.py +++ b/tests/consensus/lstar/fork_choice/test_prune_finalized_orphaned_branch.py @@ -6,12 +6,13 @@ AggregatedAttestationSpec, BlockSpec, BlockStep, + ExpectedRejection, ForkChoiceTestFiller, GossipAggregatedAttestationStep, StoreChecks, build_genesis_state, ) -from lean_spec.spec.forks import Slot, ValidatorIndex +from lean_spec.spec.forks import RejectionReason, Slot, ValidatorIndex pytestmark = pytest.mark.valid_until("Lstar") @@ -164,3 +165,169 @@ def test_finalization_prunes_vote_on_orphaned_branch( ), ], ) + + +def test_re_gossip_of_pruned_orphaned_vote_is_rejected( + fork_choice_test: ForkChoiceTestFiller, +) -> None: + """ + Re-gossiping a vote pruned onto a finalized-orphaned branch is rejected, not re-admitted. + + Given + ----- + - 8 validators; a slot needs 6 votes (2/3) to be justified. + - the chain: + genesis(0) -> block_1(1) + - block_2(2) -> block_3(3) -> block_4(4) + - orph_2(2) -> orph_3(3) -> orph_4(4) + - an aggregate from V6 targets orph_4 at slot 4, admitted while finalized is slot 1. + - block_4 finalizes slot 2 on block_2. + - orph_4 is not a descendant of block_2, so finalization pruning drops that vote. + - the pending pool now holds no target slots. + - the counted pool holds only the canonical target slot 3. + + When + ---- + - V6 re-gossips the same aggregate targeting orph_4 at slot 4. + + Then + ---- + - validation fails because the head no longer descends from the finalized block. + - the rejection reason is head not descendant of finalized. + - the pending pool still holds no target slots. + - the counted pool still holds only the canonical target slot 3. + """ + fork_choice_test( + anchor_state=build_genesis_state(num_validators=8), + steps=[ + BlockStep( + block=BlockSpec(slot=Slot(1), parent_label="genesis", label="block_1"), + checks=StoreChecks(head_slot=Slot(1)), + ), + BlockStep( + block=BlockSpec( + slot=Slot(2), + parent_label="block_1", + label="block_2", + attestations=[ + AggregatedAttestationSpec( + validator_indices=[ValidatorIndex(i) for i in range(6)], + slot=Slot(2), + target_slot=Slot(1), + target_root_label="block_1", + ), + ], + ), + checks=StoreChecks( + head_slot=Slot(2), + head_root_label="block_2", + latest_justified_slot=Slot(1), + latest_finalized_slot=Slot(0), + ), + ), + BlockStep( + block=BlockSpec( + slot=Slot(3), + parent_label="block_2", + label="block_3", + attestations=[ + AggregatedAttestationSpec( + validator_indices=[ValidatorIndex(i) for i in range(6)], + slot=Slot(3), + target_slot=Slot(2), + target_root_label="block_2", + source_slot=Slot(1), + source_root_label="block_1", + ), + ], + ), + checks=StoreChecks( + head_slot=Slot(3), + head_root_label="block_3", + latest_justified_slot=Slot(2), + latest_finalized_slot=Slot(1), + ), + ), + BlockStep( + block=BlockSpec(slot=Slot(2), parent_label="block_1", label="orph_2"), + checks=StoreChecks(head_slot=Slot(3), head_root_label="block_3"), + ), + BlockStep( + block=BlockSpec(slot=Slot(3), parent_label="orph_2", label="orph_3"), + checks=StoreChecks(head_slot=Slot(3), head_root_label="block_3"), + ), + BlockStep( + block=BlockSpec(slot=Slot(4), parent_label="orph_3", label="orph_4"), + checks=StoreChecks( + head_slot=Slot(3), + head_root_label="block_3", + latest_justified_slot=Slot(2), + latest_finalized_slot=Slot(1), + ), + ), + GossipAggregatedAttestationStep( + attestation=AggregatedAttestationSpec( + validator_indices=[ValidatorIndex(6)], + slot=Slot(4), + target_slot=Slot(4), + target_root_label="orph_4", + head_root_label="orph_4", + head_slot=Slot(4), + source_slot=Slot(0), + source_root_label="genesis", + ), + checks=StoreChecks( + head_slot=Slot(3), + head_root_label="block_3", + latest_new_aggregated_target_slots=[Slot(4)], + ), + ), + BlockStep( + block=BlockSpec( + slot=Slot(4), + parent_label="block_3", + label="block_4", + attestations=[ + AggregatedAttestationSpec( + validator_indices=[ValidatorIndex(i) for i in range(6)], + slot=Slot(4), + target_slot=Slot(3), + target_root_label="block_3", + source_slot=Slot(2), + source_root_label="block_2", + ), + ], + ), + checks=StoreChecks( + head_slot=Slot(4), + head_root_label="block_4", + latest_justified_slot=Slot(3), + latest_finalized_slot=Slot(2), + latest_finalized_root_label="block_2", + latest_known_aggregated_target_slots=[Slot(3)], + latest_new_aggregated_target_slots=[], + ), + ), + GossipAggregatedAttestationStep( + attestation=AggregatedAttestationSpec( + validator_indices=[ValidatorIndex(6)], + slot=Slot(4), + target_slot=Slot(4), + target_root_label="orph_4", + head_root_label="orph_4", + head_slot=Slot(4), + source_slot=Slot(0), + source_root_label="genesis", + ), + valid=False, + expected_rejection=ExpectedRejection( + reason=RejectionReason.HEAD_NOT_DESCENDANT_OF_FINALIZED, + exact_message="Head checkpoint must descend from the finalized block", + ), + checks=StoreChecks( + latest_new_aggregated_target_slots=[], + latest_known_aggregated_target_slots=[Slot(3)], + ), + ), + ], + ) diff --git a/tests/consensus/lstar/fork_choice/test_store_pruning.py b/tests/consensus/lstar/fork_choice/test_store_pruning.py index 519f5b816..8b796897b 100644 --- a/tests/consensus/lstar/fork_choice/test_store_pruning.py +++ b/tests/consensus/lstar/fork_choice/test_store_pruning.py @@ -195,9 +195,10 @@ def test_finalization_prunes_stale_attestation_signatures( genesis(0) -> block_1(1) -> block_2(2) -> block_3(3) -> block_4(4) -> block_5(5) - block_2, block_3, block_4 each carry 6 votes, finalizing up to slot 2. - block_5 carries no votes and does not advance finalization. - - the raw signature pool holds votes for targets 1 through 5. - - the new aggregate pool holds votes for targets 1 through 5. - - the known aggregate pool holds votes for targets 1 through 5. + - each seeded vote heads a canonical block descending from the finalized block at slot 2. + - the raw signature pool holds votes for targets 2 through 5. + - the new aggregate pool holds votes for targets 2 through 5. + - the known aggregate pool holds votes for targets 2 through 5. When ---- @@ -206,10 +207,10 @@ def test_finalization_prunes_stale_attestation_signatures( Then ---- - finalized advances to slot 3. - - votes targeting slots 1, 2, 3 are pruned from all three pools. + - votes targeting slots 2, 3 are pruned from all three pools. - votes targeting slots 4, 5 survive in all three pools. """ - all_targets = [Slot(i) for i in range(1, 6)] + all_targets = [Slot(i) for i in range(2, 6)] fork_choice_test( anchor_state=build_genesis_state(num_validators=8), @@ -291,16 +292,6 @@ def test_finalization_prunes_stale_attestation_signatures( ), ), TickStep(time=23), - GossipAggregatedAttestationStep( - attestation=AggregatedAttestationSpec( - validator_indices=[ValidatorIndex(0), ValidatorIndex(1), ValidatorIndex(2)], - slot=Slot(5), - target_slot=Slot(1), - target_root_label="block_1", - source_root_label="genesis", - source_slot=Slot(0), - ), - ), GossipAggregatedAttestationStep( attestation=AggregatedAttestationSpec( validator_indices=[ValidatorIndex(0), ValidatorIndex(1), ValidatorIndex(2)], @@ -350,16 +341,6 @@ def test_finalization_prunes_stale_attestation_signatures( latest_known_aggregated_target_slots=all_targets, ), ), - GossipAggregatedAttestationStep( - attestation=AggregatedAttestationSpec( - validator_indices=[ValidatorIndex(3), ValidatorIndex(4), ValidatorIndex(5)], - slot=Slot(5), - target_slot=Slot(1), - target_root_label="block_1", - source_root_label="genesis", - source_slot=Slot(0), - ), - ), GossipAggregatedAttestationStep( attestation=AggregatedAttestationSpec( validator_indices=[ValidatorIndex(3), ValidatorIndex(4), ValidatorIndex(5)], @@ -400,17 +381,6 @@ def test_finalization_prunes_stale_attestation_signatures( source_slot=Slot(3), ), ), - AttestationStep( - attestation=GossipAttestationSpec( - validator_index=ValidatorIndex(6), - slot=Slot(5), - target_slot=Slot(1), - target_root_label="block_1", - source_root_label="genesis", - source_slot=Slot(0), - ), - is_aggregator=True, - ), AttestationStep( attestation=GossipAttestationSpec( validator_index=ValidatorIndex(6), From a8bc280aa71579472d618673fc4b3e077f07c1ea Mon Sep 17 00:00:00 2001 From: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> Date: Sat, 4 Jul 2026 11:43:01 +0200 Subject: [PATCH 2/2] docs(testing): trim the block-builder pool-seeding comment to its load-bearing reason Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/consensus_testing/test_types/block_spec.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/testing/src/consensus_testing/test_types/block_spec.py b/packages/testing/src/consensus_testing/test_types/block_spec.py index 70afdc741..c08d36b8e 100644 --- a/packages/testing/src/consensus_testing/test_types/block_spec.py +++ b/packages/testing/src/consensus_testing/test_types/block_spec.py @@ -418,14 +418,9 @@ def build_signed_block_with_store( parent_state, block_registry, key_manager ) - # Seed the aggregator's signature pool directly from the already-signed votes. - # A proposer includes attestations it collected earlier, not fresh gossip. - # The real block-import path never gossip-validates block-carried attestations. - # It verifies their proofs and runs the state transition instead. - # Routing these votes through the gossip admission guard would wrongly reject a - # legitimately-includable vote whose head no longer descends from the finalized block. - # Each vote here was signed by the builder itself, so it is valid by construction. - # Existing pool entries are preserved, mirroring how gossip merges into the pool. + # The real block-import path does not gossip-validate block-carried attestations. + # So seed the signature pool directly instead of routing through the gossip admission guard. + # That guard would wrongly reject a legitimately-includable vote whose head is now stale. grouped_signatures: dict[AttestationData, set[AttestationSignatureEntry]] = { existing_data: set(existing_signatures) for existing_data, existing_signatures in store.attestation_signatures.items()