refactor(spec): typed rejections replace prose-matched classification (leanEthereum#871)

Spec rejections previously surfaced as bare assertions whose English
messages the testing framework substring-matched into language-neutral
reasons. Any reword silently re-bucketed vectors or crashed fills, and
distinct conditions collapsed into shared buckets.

Spec side:

- Add a typed rejection error carrying its reason. It subclasses the
  assertion error so existing rejection handlers keep working.
- Convert every rejection assertion across state transition, fork
  choice, signatures, validator duties, and participation to raise the
  typed error. Messages are preserved verbatim.
- Split the block-level proof failure out of the signature bucket: a
  failing multi-message block proof now emits its own reason, distinct
  from a failing gossip attestation signature.
- Rename the reasons module to errors, reflecting that it now carries
  the error type alongside the vocabulary.

Framework side:

- The classifier reads the structured reason field; the fragment table
  is deleted. The aggregation error keeps its by-type fallback.
- Every step expected to fail must declare its expected rejection, and
  the invalid-anchor path requires one as well. A vector saying only
  "reject this" would let a client reject for the wrong reason.
- Exception catches narrow to spec rejections and aggregation errors.
  Harness bugs now propagate instead of being emitted as vectors.
- Block steps gain a flag controlling whether the store clock advances
  to the block's slot before import, emitted on the filled step so
  replaying clients know the timing. New vectors pin that block import
  has no arrival-time gate: an early block imports and becomes head.
- The unsupported signature fields on state transition specs fail at
  construction instead of deep inside generation, and the class
  documents that signatures are verified upstream.
- The duplicated decode-failure assertion logic collapses into one
  base helper that resolves the emitted reason centrally.

Documentation rules 6-8 added: one-line docstrings by default, no
question-style docstring openings, no member enumeration in container
docstrings.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: tcoratger

.claude/rules/documentation.md

@@ -74,6 +74,35 @@ If removing the comment would not confuse a future reader, do not write it.
 Never remove or rewrite a comment or docstring unless the documented behavior actually changed.
 Removing valid docs creates diff noise and destroys context.
 
+### 6. One-line docstrings by default
+
+A class, function, or constant of ordinary complexity gets exactly one summary line.
+Add a body only when a non-obvious design choice or invariant must be explained.
+A body that restates what the summary already implies is noise.
+
+### 7. Docstring summaries are noun phrases, not questions
+
+Never open a docstring with "Why ..." or any question-style phrasing.
+Name the thing plainly, the way a human would label it.
+
+### 8. Never enumerate the container's contents in its docstring
+
+Do not list members, variants, or fields in a class or enum docstring.
+The list rots the moment the class changes.
+Stay generic; each member documents itself.
+
+Bad:
+```python
+class RejectionReason(StrEnum):
+    """Why the spec rejects an invalid block, attestation, or wire message."""
+```
+
+Good:
+```python
+class RejectionReason(StrEnum):
+    """Language-neutral reason the spec rejects an invalid input."""
+```
+
 ## Style requirements
 
 ### Sentences

packages/testing/src/consensus_testing/rejection.py

@@ -1,55 +1,8 @@
 """Classification of spec rejections into language-neutral reasons."""
 
-from lean_spec.spec.forks import RejectionReason
+from lean_spec.spec.forks import RejectionReason, SpecRejectionError
 from lean_spec.spec.forks.lstar.containers import AggregationError
 
-_REASON_BY_MESSAGE_FRAGMENT: list[tuple[str, RejectionReason]] = [
-    ("Target slot must be in the future", RejectionReason.BLOCK_SLOT_NOT_IN_FUTURE),
-    ("Block is older than latest header", RejectionReason.BLOCK_OLDER_THAN_LATEST_HEADER),
-    ("Block slot mismatch", RejectionReason.BLOCK_SLOT_MISMATCH),
-    ("Block parent root mismatch", RejectionReason.PARENT_ROOT_MISMATCH),
-    ("Invalid block state root", RejectionReason.STATE_ROOT_MISMATCH),
-    ("Parent state not found", RejectionReason.UNKNOWN_PARENT_BLOCK),
-    ("Sync parent chain", RejectionReason.UNKNOWN_PARENT_BLOCK),
-    ("Proposer index out of range", RejectionReason.PROPOSER_INDEX_OUT_OF_RANGE),
-    ("is not the proposer for slot", RejectionReason.WRONG_PROPOSER),
-    ("Incorrect block proposer", RejectionReason.WRONG_PROPOSER),
-    (
-        "Aggregated attestation must reference at least one validator",
-        RejectionReason.EMPTY_AGGREGATION_BITS,
-    ),
-    ("distinct AttestationData entries", RejectionReason.TOO_MANY_ATTESTATION_DATA),
-    ("duplicate AttestationData", RejectionReason.DUPLICATE_ATTESTATION_DATA),
-    ("Unknown source block", RejectionReason.UNKNOWN_SOURCE_BLOCK),
-    ("Unknown target block", RejectionReason.UNKNOWN_TARGET_BLOCK),
-    ("Unknown head block", RejectionReason.UNKNOWN_HEAD_BLOCK),
-    ("Source checkpoint slot must not exceed target", RejectionReason.SOURCE_AFTER_TARGET),
-    ("Head checkpoint must not be older than target", RejectionReason.HEAD_OLDER_THAN_TARGET),
-    ("Source checkpoint slot mismatch", RejectionReason.SOURCE_SLOT_MISMATCH),
-    ("Target checkpoint slot mismatch", RejectionReason.TARGET_SLOT_MISMATCH),
-    ("Head checkpoint slot mismatch", RejectionReason.HEAD_SLOT_MISMATCH),
-    (
-        "Source checkpoint must be ancestor of target",
-        RejectionReason.SOURCE_NOT_ANCESTOR_OF_TARGET,
-    ),
-    (
-        "Target checkpoint must be ancestor of head",
-        RejectionReason.TARGET_NOT_ANCESTOR_OF_HEAD,
-    ),
-    ("Attestation too far in future", RejectionReason.ATTESTATION_TOO_FAR_IN_FUTURE),
-    ("not found in state", RejectionReason.VALIDATOR_NOT_IN_STATE),
-    ("Validator index out of range", RejectionReason.VALIDATOR_INDEX_OUT_OF_RANGE),
-    ("Signature verification failed", RejectionReason.INVALID_SIGNATURE),
-    ("Block proof verification failed", RejectionReason.INVALID_SIGNATURE),
-    ("Anchor block state root must match", RejectionReason.ANCHOR_STATE_ROOT_MISMATCH),
-]
-"""
-Ordered mapping from spec rejection messages to reasons.
-
-The first fragment contained in the exception message wins.
-Fragments mirror the spec's assertion messages one-to-one.
-"""
-
 
 def classify_rejection(exception: Exception) -> RejectionReason:
     """
@@ -62,19 +15,18 @@ def classify_rejection(exception: Exception) -> RejectionReason:
         The reason emitted into the test vector.
 
     Raises:
-        ValueError: If the rejection is not in the vocabulary yet.
+        ValueError: If the exception carries no reason.
     """
+    # Typed rejections carry their reason directly.
+    if isinstance(exception, SpecRejectionError):
+        return exception.reason
+
     # Aggregate proof failures carry library-specific messages.
     # The type alone identifies them as signature verification failures.
     if isinstance(exception, AggregationError):
         return RejectionReason.INVALID_SIGNATURE
 
-    message = str(exception)
-    for message_fragment, reason in _REASON_BY_MESSAGE_FRAGMENT:
-        if message_fragment in message:
-            return reason
-
     raise ValueError(
-        f"no rejection reason mapped for {type(exception).__name__}: {message}\n"
-        "Add the new rejection to the reason vocabulary and this mapping."
+        f"no rejection reason carried by {type(exception).__name__}: {exception}\n"
+        "Spec rejections must raise the typed rejection error with a reason."
     )

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

@@ -234,3 +234,34 @@ def resolve_rejection_reason(self, exception_raised: Exception) -> RejectionReas
                 f"but the test expects {self.expected_rejection.reason}"
             )
         return classified_reason
+
+    def assert_decode_rejection(
+        self,
+        exception_raised: Exception | None,
+        decoder_name: str,
+    ) -> RejectionReason:
+        """
+        Check a decode-failure outcome and resolve the emitted reason.
+
+        Decode failures never reach the rejection classifier.
+        The authored expectation is the only source of the emitted reason.
+
+        Args:
+            exception_raised: The exception the decoder raised, or None on success.
+            decoder_name: Decoder label for failure messages.
+
+        Returns:
+            The reason emitted into the test vector.
+
+        Raises:
+            ValueError: When the authored expectation is missing.
+            AssertionError: When decoding succeeds or contradicts the expectation.
+        """
+        if self.expected_rejection is None:
+            raise ValueError("decode-failure vectors require expected_rejection to be set")
+        if exception_raised is None:
+            raise AssertionError(
+                f"Expected {decoder_name} to reject the input, but decoding succeeded"
+            )
+        self.assert_expected_outcome(exception_raised)
+        return self.expected_rejection.reason

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

@@ -26,9 +26,16 @@
 from lean_spec.config import LEAN_ENV
 from lean_spec.node.chain.clock import SlotClock
 from lean_spec.spec.crypto.merkleization import hash_tree_root
-from lean_spec.spec.forks import Interval, RejectionReason, Slot, ValidatorIndex
+from lean_spec.spec.forks import (
+    Interval,
+    RejectionReason,
+    Slot,
+    SpecRejectionError,
+    ValidatorIndex,
+)
 from lean_spec.spec.forks.lstar.containers import (
     AggregatedAttestations,
+    AggregationError,
     Block,
     BlockBody,
     SignedAggregatedAttestation,
@@ -302,14 +309,15 @@ def generate(self) -> ForkChoiceFixture:
                                 )
                             block_registry[step.block.label] = filled_block
 
-                        # Advance time to the block's slot.
-                        # Store rejects blocks from the future.
+                        # Advance time to the block's slot unless the test
+                        # delivers the block ahead of the store clock.
                         # This tick includes a block (has proposal).
                         # Always act as aggregator to ensure gossip signatures are aggregated
-                        target_interval = Interval.from_slot(filled_block.slot)
-                        store, _ = spec.on_tick(
-                            store, target_interval, has_proposal=True, is_aggregator=True
-                        )
+                        if step.tick_to_slot:
+                            target_interval = Interval.from_slot(filled_block.slot)
+                            store, _ = spec.on_tick(
+                                store, target_interval, has_proposal=True, is_aggregator=True
+                            )
 
                         # Process the block through Store.
                         # This validates, applies state transition, and updates the store's head.
@@ -360,9 +368,10 @@ def generate(self) -> ForkChoiceFixture:
                         old_head=old_head,
                     )
 
-            except Exception as exception:
+            except (SpecRejectionError, AggregationError) as exception:
                 # Handle expected failures.
-                # Steps marked valid=False should raise exceptions.
+                # Steps marked valid=False should raise spec rejections.
+                # Harness bugs raise other types and propagate unswallowed.
                 if step.valid:
                     raise AssertionError(
                         f"Step {step_index} ({type(step).__name__}) "
@@ -404,6 +413,7 @@ def generate(self) -> ForkChoiceFixture:
                             rejection_reason=rejection_reason,
                             checks=step.checks,
                             store_snapshot=store_snapshot,
+                            tick_to_slot=step.tick_to_slot,
                             block=filled_block,
                             block_root_label=step.block.label,
                         )
@@ -462,18 +472,19 @@ def _generate_invalid_anchor(
             "steps must be empty when anchor_valid is False: "
             "Store.from_anchor is expected to fail before any step can run"
         )
+        # Why: a vector saying only "reject this anchor" lets a client
+        # reject for the wrong reason and still pass.
+        assert self.expected_rejection is not None, (
+            "anchor_valid=False requires expected_rejection to be set"
+        )
         try:
             spec.create_store(
                 self.anchor_state,
                 anchor_block,
                 validator_index=ValidatorIndex(0),
             )
-        except AssertionError as exception:
-            expected_substring = (
-                self.expected_rejection.message_substring
-                if self.expected_rejection is not None
-                else None
-            )
+        except SpecRejectionError as exception:
+            expected_substring = self.expected_rejection.message_substring
             if expected_substring is not None and expected_substring not in str(exception):
                 raise AssertionError(
                     "Store.from_anchor failed with wrong error.\n"

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

@@ -774,32 +774,12 @@ class NetworkingCodecTest(BaseTestSpec):
     def generate(self) -> NetworkingCodecFixture:
         """Run the codec case and emit the vector."""
         if isinstance(self.codec, DecodeFailure):
-            return self._generate_decode_failure(self.codec)
-        return NetworkingCodecFixture(codec=self.codec, output=self.codec.run())
-
-    def _generate_decode_failure(self, decode_failure: DecodeFailure) -> NetworkingCodecFixture:
-        """
-        Assert the decoder rejects the malformed input and emit the rejection vector.
-
-        Raises:
-            AssertionError: If the decoder succeeds or the rejection
-                contradicts the authored expectation.
-            ValueError: If the expected rejection is unset.
-        """
-        if self.expected_rejection is None:
-            raise ValueError("decode_failure codec requires expected_rejection to be set")
-
-        exception_raised = decode_failure.attempt_decode()
-        if exception_raised is None:
-            raise AssertionError(
-                f"Expected {decode_failure.decoder!r} decode to reject the input, "
-                "but decode succeeded"
+            # Emit the language-neutral reason clients assert against.
+            return NetworkingCodecFixture(
+                codec=self.codec,
+                output=DecodeFailureOutput(decoder=self.codec.decoder),
+                rejection_reason=self.assert_decode_rejection(
+                    self.codec.attempt_decode(), self.codec.decoder
+                ),
             )
-        self.assert_expected_outcome(exception_raised)
-
-        # Emit the language-neutral reason clients assert against.
-        return NetworkingCodecFixture(
-            codec=decode_failure,
-            output=DecodeFailureOutput(decoder=decode_failure.decoder),
-            rejection_reason=self.expected_rejection.reason,
-        )
+        return NetworkingCodecFixture(codec=self.codec, output=self.codec.run())

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

@@ -140,7 +140,6 @@ def _generate_decode_failure(self) -> SSZFixture:
             AssertionError: If the decoder succeeds.
             ValueError: If `raw_bytes` is missing.
         """
-        assert self.expected_rejection is not None
         if self.raw_bytes is None:
             raise ValueError("raw_bytes is required when expected_rejection is set")
 
@@ -152,18 +151,13 @@ def _generate_decode_failure(self) -> SSZFixture:
         except Exception as exception:
             exception_raised = exception
 
-        if exception_raised is None:
-            raise AssertionError(
-                f"Expected {decoder.__name__}.decode_bytes to reject the input, "
-                "but decode succeeded"
-            )
-        self.assert_expected_outcome(exception_raised)
-
         return SSZFixture(
             type_name=self.type_name,
             value=self.value,
             raw_bytes=self.raw_bytes,
             serialized="0x" + raw.hex(),
             root="",
-            rejection_reason=self.expected_rejection.reason,
+            rejection_reason=self.assert_decode_rejection(
+                exception_raised, f"{decoder.__name__}.decode_bytes"
+            ),
         )

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

@@ -2,14 +2,14 @@
 
 from typing import ClassVar
 
-from pydantic import Field
+from pydantic import Field, model_validator
 
 from consensus_testing.genesis import generate_pre_state
 from consensus_testing.keys import XmssKeyManager
 from consensus_testing.test_fixtures.base import BaseConsensusFixture, BaseTestSpec
 from consensus_testing.test_types import AggregatedAttestationSpec, BlockSpec, StateExpectation
 from lean_spec.spec.crypto.merkleization import hash_tree_root
-from lean_spec.spec.forks import AggregationBits
+from lean_spec.spec.forks import AggregationBits, SpecRejectionError
 from lean_spec.spec.forks.lstar.containers import (
     AggregatedAttestation,
     AggregatedAttestations,
@@ -62,6 +62,10 @@ class StateTransitionTest(BaseTestSpec):
     - Invalid blocks
 
     Tests everything through the main state_transition() public API.
+
+    The state transition assumes signatures were verified upstream.
+    Invalid-signature scenarios belong to the fork choice and signature
+    verification formats, never to this one.
     """
 
     format_name: ClassVar[str] = "state_transition_test"
@@ -94,6 +98,19 @@ class StateTransitionTest(BaseTestSpec):
     If None, no post-state validation is performed (e.g., for invalid tests).
     """
 
+    @model_validator(mode="after")
+    def validate_signatures_are_out_of_scope(self) -> "StateTransitionTest":
+        """Reject signature-flavored attestation fields at construction."""
+        for block_spec in self.blocks:
+            for attestation_spec in block_spec.attestations or []:
+                if not attestation_spec.valid_signature or attestation_spec.signer_ids is not None:
+                    raise ValueError(
+                        "state transition assumes signatures were verified upstream; "
+                        "author invalid-signature scenarios through the fork choice "
+                        "or signature verification formats"
+                    )
+        return self
+
     def generate(self) -> StateTransitionFixture:
         """
         Generate the fixture by running the spec.
@@ -140,7 +157,7 @@ def generate(self) -> StateTransitionFixture:
                     state = spec.state_transition(state, block=block)
 
             actual_post_state = state
-        except (AssertionError, ValueError) as exception:
+        except SpecRejectionError as exception:
             exception_raised = exception
 
         # Validate exception expectations
@@ -316,13 +333,6 @@ def _build_aggregated_payloads_from_spec(
         payloads: dict[AttestationData, set[SingleMessageAggregate]] = {}
 
         for spec in attestation_specs:
-            if not spec.valid_signature:
-                raise NotImplementedError(
-                    "valid_signature=False not yet supported in StateTransitionTest"
-                )
-            if spec.signer_ids is not None:
-                raise NotImplementedError("signer_ids not yet supported in StateTransitionTest")
-
             attestation_data = spec.build_attestation_data(block_registry, state.latest_justified)
             proof = key_manager.sign_and_aggregate(spec.validator_indices, attestation_data)
             payloads.setdefault(attestation_data, set()).add(proof)