From aad06c9ee5184f5a7a347ef5725b25c241353a27 Mon Sep 17 00:00:00 2001 From: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> Date: Wed, 10 Jun 2026 18:31:29 +0200 Subject: [PATCH 1/2] docs(testing): align aggregation-bits vectors with doc-writer conventions Follow-up to #899. Brings the aggregation-bits bounds vectors in line with the consensus test-vector documentation standard: - Drop all inline body comments from the test vectors; the Given/When/Then docstring is the single source of truth. - Strip the "a client that..." narration from every Then section so it states only the asserted outcome (rejection reason or post-state). - Move the bitfield layout and SSZ-encoding rationale into Given as atomic facts, and note the bitfield-limit vs registry distinction. - Tighten the aggregation_bits field and resolver docstrings to terse one-sentence-per-line phrasing. - Note in process_attestations that signature verification normally rejects an out-of-range bit first, so this guards the unsigned path. - Make is_final_block keyword-only on the block builder. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test_fixtures/state_transition.py | 1 + .../test_types/attestation_specs.py | 14 +++----- .../spec/forks/lstar/state_transition.py | 3 ++ .../state_transition/test_aggregation_bits.py | 32 ++++++------------- 4 files changed, 17 insertions(+), 33 deletions(-) diff --git a/packages/testing/src/consensus_testing/test_fixtures/state_transition.py b/packages/testing/src/consensus_testing/test_fixtures/state_transition.py index 88ecc7b2..13e42e8c 100644 --- a/packages/testing/src/consensus_testing/test_fixtures/state_transition.py +++ b/packages/testing/src/consensus_testing/test_fixtures/state_transition.py @@ -199,6 +199,7 @@ def _build_block_from_spec( block_spec: BlockSpec, state: State, block_registry: dict[str, Block], + *, is_final_block: bool, ) -> tuple[Block, State | None]: """ diff --git a/packages/testing/src/consensus_testing/test_types/attestation_specs.py b/packages/testing/src/consensus_testing/test_types/attestation_specs.py index 8f3f95a5..8c7d8574 100644 --- a/packages/testing/src/consensus_testing/test_types/attestation_specs.py +++ b/packages/testing/src/consensus_testing/test_types/attestation_specs.py @@ -163,19 +163,13 @@ class AggregatedAttestationSpec(AttestationSpec): """ Raw aggregation bits placed into the block body verbatim. - When None (default), the bits are derived from the validator indices, - producing the tightest bitfield that covers the highest set index. - Set this to author bit patterns the derivation cannot express: - a zero-length bitfield, all-false bits, or trailing padding past the - validator registry. - - Only honored by the state transition format's forced-attestation - path, which bypasses signing. Signed paths derive their bits from - the validator indices so proofs match the claimed participants. + When unset, bits are derived from the validator indices. + Only the state transition forced-attestation path honors an override. + Signed paths always derive bits from the validator indices. """ def resolve_aggregation_bits(self) -> AggregationBits: - """Return the explicit bits override, or bits derived from the validator indices.""" + """Return the bit override when present, else bits derived from the validator indices.""" if self.aggregation_bits is not None: return self.aggregation_bits return AggregationBits.from_indices(self.validator_indices) diff --git a/src/lean_spec/spec/forks/lstar/state_transition.py b/src/lean_spec/spec/forks/lstar/state_transition.py index 69da34f6..bedc43e4 100644 --- a/src/lean_spec/spec/forks/lstar/state_transition.py +++ b/src/lean_spec/spec/forks/lstar/state_transition.py @@ -456,6 +456,9 @@ def process_attestations( # A vote from a nonexistent validator has no flag to set, # so the whole block is invalid. # Trailing unset bits beyond the registry are harmless padding. + # + # Signature verification normally rejects such a bit first. + # This guards the unsigned path, which has no signature stage. for validator_index in voting_validator_indices: if not validator_index.is_within_registry(Uint64(len(state.validators))): raise SpecRejectionError( diff --git a/tests/consensus/lstar/state_transition/test_aggregation_bits.py b/tests/consensus/lstar/state_transition/test_aggregation_bits.py index 152a0612..b3bcec2e 100644 --- a/tests/consensus/lstar/state_transition/test_aggregation_bits.py +++ b/tests/consensus/lstar/state_transition/test_aggregation_bits.py @@ -27,8 +27,8 @@ def test_aggregation_bit_beyond_validator_registry_rejects_block( - 4 validators (indices 0-3); the vote tally has one flag per validator. - the chain: genesis -> block_1(1) -> block(2) - - block(2) carries an attestation for block_1 whose aggregation bits - name V0 and V1 plus nonexistent validator 4. + - block(2) carries an attestation for block_1 naming V0, V1, and V4. + - index 4 is within the bitfield limit but one past the registry. When ---- @@ -37,9 +37,6 @@ def test_aggregation_bit_beyond_validator_registry_rejects_block( Then ---- - the block is rejected with VALIDATOR_INDEX_OUT_OF_RANGE. - - a client that silently skips the attestation instead would accept a - block the rest of the network refuses: a consensus split on a single - crafted block. """ state_transition_test( blocks=[ @@ -48,8 +45,6 @@ def test_aggregation_bit_beyond_validator_registry_rejects_block( slot=Slot(2), parent_label="block_1", forced_attestations=[ - # Bits 0, 1, and 4 are set. - # Bit 4 points one past the 4-validator registry. AggregatedAttestationSpec( validator_indices=[ ValidatorIndex(0), @@ -89,9 +84,6 @@ def test_all_false_aggregation_bits_rejects_block( Then ---- - the block is rejected with EMPTY_AGGREGATION_BITS. - - a client that processes the attestation as a no-op instead leaves an - all-false tally entry in its post-state, diverging from clients that - reject the block. """ state_transition_test( blocks=[ @@ -128,6 +120,7 @@ def test_zero_length_aggregation_bits_rejects_block( genesis -> block_1(1) -> block(2) - block(2) carries an attestation for block_1 whose aggregation bits hold no bits at all. + - a zero-length bitfield is a distinct SSZ encoding from an all-false one. When ---- @@ -136,9 +129,6 @@ def test_zero_length_aggregation_bits_rejects_block( Then ---- - the block is rejected with EMPTY_AGGREGATION_BITS. - - the zero-length bitfield is a distinct SSZ encoding from an all-false - bitfield of registry size; both name no voter and both must reject - the block identically across clients. """ state_transition_test( blocks=[ @@ -173,8 +163,9 @@ def test_oversized_aggregation_bits_with_in_range_votes_processes_normally( - 4 validators; a slot needs 3 votes (2/3) to be justified. - the chain: genesis -> block_1(1) -> block(2) - - block(2) carries an attestation for block_1 whose bitfield is 6 bits - long (two bits past the registry) with only V0, V1, and V2 set. + - block(2) carries an attestation for block_1. + - the bitfield is 6 bits long with only V0, V1, V2 set. + - bits 4 and 5 are unset padding past the registry. When ---- @@ -182,12 +173,9 @@ def test_oversized_aggregation_bits_with_in_range_votes_processes_normally( Then ---- - - the attestation is processed normally: every set bit addresses a real - validator, and the trailing unset padding is harmless. - - block_1's slot is justified and its pending tally is cleared. - - a client that skips the attestation because of the bitfield length - computes a different post-state for a valid block; the pinned - post-state root catches that divergence directly. + - block_1's slot is justified. + - the pending tally for block_1 is cleared. + - finalization stays at genesis. """ state_transition_test( blocks=[ @@ -196,8 +184,6 @@ def test_oversized_aggregation_bits_with_in_range_votes_processes_normally( slot=Slot(2), parent_label="block_1", forced_attestations=[ - # Bits 0-2 are set; bits 3-5 are unset. - # Bits 4 and 5 pad past the 4-validator registry. AggregatedAttestationSpec( validator_indices=[], aggregation_bits=AggregationBits( From 72422c44b71cd1c32faa4d0660be134d36ce1471 Mon Sep 17 00:00:00 2001 From: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> Date: Wed, 10 Jun 2026 18:34:13 +0200 Subject: [PATCH 2/2] docs(testing): use bullets for the aggregation_bits field docstring Per the doc-writer rules, render the field's three behaviors as bullet points instead of prose, and fix the summary line so it describes the default (derived) case rather than only the verbatim override. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/consensus_testing/test_types/attestation_specs.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/testing/src/consensus_testing/test_types/attestation_specs.py b/packages/testing/src/consensus_testing/test_types/attestation_specs.py index 8c7d8574..075f990d 100644 --- a/packages/testing/src/consensus_testing/test_types/attestation_specs.py +++ b/packages/testing/src/consensus_testing/test_types/attestation_specs.py @@ -161,11 +161,11 @@ class AggregatedAttestationSpec(AttestationSpec): aggregation_bits: AggregationBits | None = None """ - Raw aggregation bits placed into the block body verbatim. + Raw aggregation bits for the block body, overriding index derivation. - When unset, bits are derived from the validator indices. - Only the state transition forced-attestation path honors an override. - Signed paths always derive bits from the validator indices. + - When unset, bits are derived from the validator indices. + - When set, the bits are used verbatim, even when zero-length or padded. + - Only the unsigned forced-attestation path honors the override. """ def resolve_aggregation_bits(self) -> AggregationBits: