test(spec): assert the rejection reason on expected-failure fixtures

crates/blockchain/src/spec_test_runner.rs

@@ -4,19 +4,109 @@
 //! functions so fixture replay cannot drift between the two entry points.
 
 use ethlambda_storage::Store;
-use ethlambda_test_fixtures::fork_choice::ForkChoiceStep;
+use ethlambda_test_fixtures::{RejectionReason, fork_choice::ForkChoiceStep};
 use ethlambda_types::{
     attestation::{
         AggregationBits, HashedAttestationData, SignedAggregatedAttestation, SignedAttestation,
     },
     block::{ByteList512KiB, SingleMessageAggregate},
 };
 
-use crate::{MILLISECONDS_PER_INTERVAL, MILLISECONDS_PER_SLOT, store};
+use crate::{
+    MILLISECONDS_PER_INTERVAL, MILLISECONDS_PER_SLOT,
+    store::{self, StoreError},
+};
 
 /// Prefix emitted by leanSpec's mocked aggregation prover.
 const MOCK_PROOF_PREFIX: &[u8] = b"\x00MOCKED-AGGREGATION-PROOF\x00";
 
+/// Why a fork-choice fixture step failed.
+///
+/// Distinguishes a client rejection, which negative fixtures assert against
+/// their `rejectionReason`, from a harness failure, which means the fixture
+/// asked for something this runner cannot replay.
+#[derive(Debug, thiserror::Error)]
+pub enum StepError {
+    /// The store rejected the step.
+    #[error(transparent)]
+    Store(#[from] StoreError),
+
+    /// The step is malformed or names something the runner cannot replay. Never
+    /// a client rejection, so it never satisfies an expected `rejectionReason`.
+    #[error("{0}")]
+    Harness(String),
+}
+
+impl StepError {
+    /// The leanSpec rejection reason this failure corresponds to, if any.
+    pub fn rejection_reason(&self) -> Option<RejectionReason> {
+        match self {
+            Self::Store(err) => rejection_reason(err),
+            Self::Harness(_) => None,
+        }
+    }
+}
+
+/// Classify a store rejection into the reason leanSpec would report for it,
+/// mirroring the spec's `classify_rejection`.
+///
+/// `None` means the variant has no spec counterpart, which the spec-test runners
+/// report as an unclassified rejection rather than accepting silently. The match
+/// is exhaustive so a new [`StoreError`] variant forces that decision here.
+///
+/// Two variants are context-dependent and classified for the gossip path they
+/// are reached through in fixtures:
+///
+/// * `InvalidValidatorIndex` is raised both for a gossip attestation from an
+///   unregistered validator (`VALIDATOR_NOT_IN_STATE`) and for a block-level
+///   participant bounds check (`VALIDATOR_INDEX_OUT_OF_RANGE`, spec
+///   `signatures.py`). Only the former has fixtures today.
+/// * `StateTransitionFailed` defers to the state-transition classification,
+///   which the STF runner asserts directly.
+pub fn rejection_reason(err: &StoreError) -> Option<RejectionReason> {
+    let reason = match err {
+        StoreError::MissingParentState { .. } => RejectionReason::UnknownParentBlock,
+        StoreError::InvalidValidatorIndex => RejectionReason::ValidatorNotInState,
+        StoreError::SignatureDecodingFailed | StoreError::SignatureVerificationFailed => {
+            RejectionReason::InvalidSignature
+        }
+        StoreError::StateTransitionFailed(err) => err.into(),
+        StoreError::UnknownSourceBlock(_) => RejectionReason::UnknownSourceBlock,
+        StoreError::UnknownTargetBlock(_) => RejectionReason::UnknownTargetBlock,
+        StoreError::UnknownHeadBlock(_) => RejectionReason::UnknownHeadBlock,
+        StoreError::SourceExceedsTarget => RejectionReason::SourceAfterTarget,
+        StoreError::HeadOlderThanTarget { .. } => RejectionReason::HeadOlderThanTarget,
+        StoreError::SourceSlotMismatch { .. } => RejectionReason::SourceSlotMismatch,
+        StoreError::TargetSlotMismatch { .. } => RejectionReason::TargetSlotMismatch,
+        StoreError::HeadSlotMismatch { .. } => RejectionReason::HeadSlotMismatch,
+        StoreError::SourceNotAncestorOfTarget => RejectionReason::SourceNotAncestorOfTarget,
+        StoreError::TargetNotAncestorOfHead => RejectionReason::TargetNotAncestorOfHead,
+        StoreError::HeadNotDescendantOfFinalized { .. } => {
+            RejectionReason::HeadNotDescendantOfFinalized
+        }
+        StoreError::AttestationSlotBeforeHead { .. } => RejectionReason::AttestationSlotBeforeHead,
+        StoreError::AttestationTooFarInFuture { .. } => RejectionReason::AttestationTooFarInFuture,
+        StoreError::AggregateVerificationFailed(_) => RejectionReason::InvalidSignature,
+        StoreError::BlockProofVerificationFailed(_) => RejectionReason::InvalidBlockProof,
+        StoreError::EmptyAggregationBits => RejectionReason::EmptyAggregationBits,
+        StoreError::NotProposer { .. } => RejectionReason::WrongProposer,
+        StoreError::DuplicateAttestationData { .. } => RejectionReason::DuplicateAttestationData,
+        StoreError::TooManyAttestationData { .. } => RejectionReason::TooManyAttestationData,
+        StoreError::BlockSlotGapTooLarge { .. } => RejectionReason::BlockSlotGapTooLarge,
+        StoreError::BlockTooFarInFuture { .. } => RejectionReason::BlockTooFarInFuture,
+
+        // Internal failures with no spec counterpart: the spec has no undecodable
+        // registry pubkey, no aggregation step inside validation, no state that
+        // can go missing behind a known block, and no slot width limit (its
+        // slots are unbounded where ours narrow to the XMSS epoch's u32).
+        StoreError::PubkeyDecodingFailed(_)
+        | StoreError::SignatureAggregationFailed(_)
+        | StoreError::MissingTargetState(_)
+        | StoreError::SlotOutOfRange(_) => return None,
+    };
+    Some(reason)
+}
+
 /// Apply one fork-choice fixture step.
 ///
 /// `proofs_are_mocked` is supplied by complete offline vectors through their
@@ -26,7 +116,7 @@ pub fn apply_fork_choice_step(
     store: &mut Store,
     step: &ForkChoiceStep,
     proofs_are_mocked: Option<bool>,
-) -> Result<(), String> {
+) -> Result<(), StepError> {
     match step.step_type.as_str() {
         "tick" => {
             let genesis_time = store.config().expect("config exists").genesis_time;
@@ -35,7 +125,11 @@ pub fn apply_fork_choice_step(
                 (None, Some(interval)) => {
                     genesis_time * 1000 + interval * MILLISECONDS_PER_INTERVAL
                 }
-                (None, None) => return Err("tick step missing time and interval".to_string()),
+                (None, None) => {
+                    return Err(StepError::Harness(
+                        "tick step missing time and interval".to_string(),
+                    ));
+                }
             };
             store::on_tick(store, timestamp_ms, step.has_proposal.unwrap_or(false));
             Ok(())
@@ -44,14 +138,14 @@ pub fn apply_fork_choice_step(
             let block_data = step
                 .block
                 .as_ref()
-                .ok_or_else(|| "block step missing block data".to_string())?;
+                .ok_or_else(|| StepError::Harness("block step missing block data".to_string()))?;
             let signed_block = block_data.to_blank_signed_block();
             if step.tick_to_slot {
                 let block_time_ms = store.config().expect("config exists").genesis_time * 1000
                     + signed_block.message.slot * MILLISECONDS_PER_SLOT;
                 store::on_tick(store, block_time_ms, true);
             }
-            store::on_block_without_verification(store, signed_block).map_err(|e| e.to_string())?;
+            store::on_block_without_verification(store, signed_block)?;
 
             let block = block_data.to_block();
             let entries = block.body.attestations.iter().map(|att| {
@@ -68,48 +162,45 @@ pub fn apply_fork_choice_step(
             let att = step
                 .attestation
                 .as_ref()
-                .ok_or_else(|| "attestation step missing data".to_string())?;
+                .ok_or_else(|| StepError::Harness("attestation step missing data".to_string()))?;
             let signed = SignedAttestation {
-                validator_id: att
-                    .validator_id
-                    .ok_or_else(|| "attestation step missing validatorId".to_string())?,
+                validator_id: att.validator_id.ok_or_else(|| {
+                    StepError::Harness("attestation step missing validatorId".to_string())
+                })?,
                 data: att.data.clone().into(),
-                signature: att
-                    .signature
-                    .clone()
-                    .ok_or_else(|| "attestation step missing signature".to_string())?,
+                signature: att.signature.clone().ok_or_else(|| {
+                    StepError::Harness("attestation step missing signature".to_string())
+                })?,
             };
-            store::on_gossip_attestation(store, &signed, step.is_aggregator.unwrap_or(false))
-                .map_err(|e| e.to_string())
+            store::on_gossip_attestation(store, &signed, step.is_aggregator.unwrap_or(false))?;
+            Ok(())
         }
         "gossipAggregatedAttestation" => {
-            let att = step
-                .attestation
-                .as_ref()
-                .ok_or_else(|| "gossipAggregatedAttestation step missing data".to_string())?;
-            let proof = att
-                .proof
-                .as_ref()
-                .ok_or_else(|| "gossipAggregatedAttestation step missing proof".to_string())?;
+            let att = step.attestation.as_ref().ok_or_else(|| {
+                StepError::Harness("gossipAggregatedAttestation step missing data".to_string())
+            })?;
+            let proof = att.proof.as_ref().ok_or_else(|| {
+                StepError::Harness("gossipAggregatedAttestation step missing proof".to_string())
+            })?;
             let participants: AggregationBits = proof.participants.clone().into();
             let proof_bytes: Vec<u8> = proof.proof.clone().into();
             let is_mocked =
                 proofs_are_mocked.unwrap_or_else(|| proof_bytes.starts_with(MOCK_PROOF_PREFIX));
-            let proof_data = ByteList512KiB::try_from(proof_bytes)
-                .map_err(|err| format!("aggregated proof data too large: {err:?}"))?;
+            let proof_data = ByteList512KiB::try_from(proof_bytes).map_err(|err| {
+                StepError::Harness(format!("aggregated proof data too large: {err:?}"))
+            })?;
             let aggregated = SignedAggregatedAttestation {
                 proof: SingleMessageAggregate::new(participants, proof_data),
                 data: att.data.clone().into(),
             };
             if is_mocked {
-                store::on_gossip_aggregated_attestation_without_verification(store, aggregated)
-                    .map_err(|e| e.to_string())
+                store::on_gossip_aggregated_attestation_without_verification(store, aggregated)?;
             } else {
-                store::on_gossip_aggregated_attestation(store, aggregated)
-                    .map_err(|e| e.to_string())
+                store::on_gossip_aggregated_attestation(store, aggregated)?;
             }
+            Ok(())
         }
         "checks" => Ok(()),
-        other => Err(format!("unknown step type: {other}")),
+        other => Err(StepError::Harness(format!("unknown step type: {other}"))),
     }
 }

crates/blockchain/src/store.rs

@@ -1058,6 +1058,9 @@ pub enum StoreError {
     #[error("Aggregated signature verification failed: {0}")]
     AggregateVerificationFailed(ethlambda_crypto::VerificationError),
 
+    #[error("Block proof verification failed: {0}")]
+    BlockProofVerificationFailed(ethlambda_crypto::VerificationError),
+
     #[error("Signature aggregation failed: {0}")]
     SignatureAggregationFailed(ethlambda_crypto::AggregationError),
 
@@ -1170,7 +1173,7 @@ pub fn verify_block_signatures(
         pubkeys_per_component,
         &expected_bindings,
     )
-    .map_err(StoreError::AggregateVerificationFailed)?;
+    .map_err(StoreError::BlockProofVerificationFailed)?;
     let crypto_elapsed = crypto_start.elapsed();
 
     let total_elapsed = total_start.elapsed();

crates/blockchain/state_transition/tests/stf_spectests.rs

@@ -2,6 +2,7 @@ use std::collections::HashMap;
 use std::path::Path;
 
 use ethlambda_state_transition::state_transition;
+use ethlambda_test_fixtures::{RejectionReason, rejection::check_rejection_reason};
 use ethlambda_types::{
     block::Block,
     primitives::{H256, HashTreeRoot as _},
@@ -12,6 +13,30 @@ use crate::types::PostState;
 
 const SUPPORTED_FIXTURE_FORMAT: &str = "state_transition_test";
 
+/// Fixtures this runner cannot replay, matched as substrings of the test name.
+///
+/// Both entries are authored against leanSpec's `BlockSpec.skip_slot_processing`,
+/// which drives the filler to call `process_block` alone. The flag never reaches
+/// the emitted fixture (`StateTransitionFixture` carries only `pre`, `blocks`,
+/// `post`, `postStateRoot` and `rejectionReason`), and the failing block is
+/// written with a zero `stateRoot`, so replaying `state_transition()` as the
+/// fixture format prescribes always advances the slot first and then dies on the
+/// state root instead of hitting the rule under test. Nothing in the JSON marks
+/// the entry point, so no client can reproduce these two: ream, zeam, gean,
+/// lantern and grandine all "pass" them on the state-root mismatch because they
+/// assert only that *some* error occurred.
+///
+/// Skip until leanSpec emits the entry point, or drops these vectors: they pin a
+/// Python-level API contract rather than cross-client behaviour.
+const SKIP_TESTS: &[&str] = &[
+    // Expects BLOCK_SLOT_MISMATCH from `process_block` on a state at slot 1 with
+    // a block claiming slot 2; `process_slots` makes the slots agree first.
+    "test_block_with_wrong_slot",
+    // Expects BLOCK_OLDER_THAN_LATEST_HEADER from a second block at the tip's
+    // slot; `process_slots` rejects the first block before the header check runs.
+    "test_block_at_parent_slot_rejected_when_slot_processing_skipped",
+];
+
 mod types;
 
 fn run(path: &Path) -> datatest_stable::Result<()> {
@@ -24,6 +49,10 @@ fn run(path: &Path) -> datatest_stable::Result<()> {
             )
             .into());
         }
+        if let Some(skip) = SKIP_TESTS.iter().find(|skip| name.contains(*skip)) {
+            println!("Skipping {skip} (see SKIP_TESTS comment)");
+            continue;
+        }
         println!("Running test: {}", name);
 
         // Fixtures with no blocks come from spec filler runs that raised
@@ -69,12 +98,24 @@ fn run(path: &Path) -> datatest_stable::Result<()> {
                 }
             }
             (Ok(_), None) => {
-                return Err(
-                    format!("Test '{name}' failed: expected failure but got success").into(),
-                );
+                let expected = test
+                    .rejection_reason
+                    .as_ref()
+                    .map(|reason| format!(" ({reason})"))
+                    .unwrap_or_default();
+                return Err(format!(
+                    "Test '{name}' failed: expected failure{expected} but got success"
+                )
+                .into());
             }
-            (Err(_), None) => {
-                // Expected failure
+            // Expected failure. When the fixture names why, the transition must
+            // have failed for that reason: a state-root mismatch standing in for
+            // the rule under test is a pass for the wrong reason.
+            (Err(err), None) => {
+                if let Some(expected) = test.rejection_reason.as_ref() {
+                    let actual = RejectionReason::from(&err);
+                    check_rejection_reason(&name, expected, Some(&actual), &err)?;
+                }
             }
             (Err(err), Some(_)) => {
                 return Err(format!(

crates/blockchain/state_transition/tests/types.rs

@@ -38,12 +38,10 @@ pub struct StateTransitionTest {
     /// any state field those checks don't enumerate is still pinned.
     #[serde(rename = "postStateRoot")]
     pub post_state_root: Option<H256>,
-    /// Expected rejection reason for negative cases. Captured only so
-    /// `deny_unknown_fields` accepts it; failure is asserted via a missing
-    /// `post`.
+    /// Expected rejection reason for negative cases. A missing `post` asserts
+    /// that the transition failed; this pins *why* it had to fail.
     #[serde(rename = "rejectionReason")]
-    #[allow(dead_code)]
-    pub rejection_reason: Option<String>,
+    pub rejection_reason: Option<RejectionReason>,
     /// Aggregation proof regime (unused by the STF runner). Captured only so
     /// `deny_unknown_fields` accepts it.
     #[serde(rename = "proofSetting")]