diff --git a/Cargo.lock b/Cargo.lock index 7b514a8c..47daf00b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2187,6 +2187,7 @@ dependencies = [ name = "ethlambda-test-fixtures" version = "0.1.0" dependencies = [ + "ethlambda-state-transition", "ethlambda-types", "hex", "libssz", diff --git a/crates/blockchain/src/spec_test_runner.rs b/crates/blockchain/src/spec_test_runner.rs index 0669ca71..51116db4 100644 --- a/crates/blockchain/src/spec_test_runner.rs +++ b/crates/blockchain/src/spec_test_runner.rs @@ -4,7 +4,7 @@ //! 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, @@ -12,11 +12,101 @@ use ethlambda_types::{ 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 { + 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 { + 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, -) -> 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 = 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}"))), } } diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 1a6d3884..d641cef2 100644 --- a/crates/blockchain/src/store.rs +++ b/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(); diff --git a/crates/blockchain/state_transition/tests/stf_spectests.rs b/crates/blockchain/state_transition/tests/stf_spectests.rs index 458cc2c0..4e76b8e6 100644 --- a/crates/blockchain/state_transition/tests/stf_spectests.rs +++ b/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!( diff --git a/crates/blockchain/state_transition/tests/types.rs b/crates/blockchain/state_transition/tests/types.rs index 262785b0..c14b80ff 100644 --- a/crates/blockchain/state_transition/tests/types.rs +++ b/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, - /// 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, + pub rejection_reason: Option, /// Aggregation proof regime (unused by the STF runner). Captured only so /// `deny_unknown_fields` accepts it. #[serde(rename = "proofSetting")] diff --git a/crates/blockchain/tests/forkchoice_spectests.rs b/crates/blockchain/tests/forkchoice_spectests.rs index e3c5cd67..1b5967be 100644 --- a/crates/blockchain/tests/forkchoice_spectests.rs +++ b/crates/blockchain/tests/forkchoice_spectests.rs @@ -4,7 +4,10 @@ use std::{ sync::Arc, }; -use ethlambda_blockchain::{spec_test_runner::apply_fork_choice_step, store}; +use ethlambda_blockchain::{ + spec_test_runner::{StepError, apply_fork_choice_step}, + store, +}; use ethlambda_storage::{Store, backend::InMemoryBackend}; use ethlambda_types::{ attestation::{AttestationData, validator_indices}, @@ -13,8 +16,10 @@ use ethlambda_types::{ state::{State, anchor_pair_is_consistent}, }; -use ethlambda_test_fixtures::fork_choice::{ - AttestationCheck, BlockAttestationCheck, ForkChoiceTestVector, StoreChecks, +use ethlambda_test_fixtures::{ + RejectionReason, + fork_choice::{AttestationCheck, BlockAttestationCheck, ForkChoiceTestVector, StoreChecks}, + rejection::check_rejection_reason, }; const SUPPORTED_FIXTURE_FORMAT: &str = "fork_choice_test"; @@ -56,6 +61,26 @@ fn run(path: &Path) -> datatest_stable::Result<()> { let anchor_block: Block = test.anchor_block.into(); let pair_ok = anchor_pair_is_consistent(&mut anchor_state, &anchor_block); if test.steps.is_empty() { + // The only anchor rejection the store can express is an inconsistent + // (state, block) pair, so any other reason means the fixture exercises + // a rule this runner does not model yet. + match test.rejection_reason.as_ref() { + Some(RejectionReason::AnchorStateRootMismatch) => {} + Some(other) => { + return Err(format!( + "Fixture '{name}' has no steps and expects anchor rejection \ + {other}, which this runner cannot assert" + ) + .into()); + } + None => { + return Err(format!( + "Fixture '{name}' has no steps (expects anchor rejection) \ + but names no rejectionReason" + ) + .into()); + } + } if pair_ok { return Err(format!( "Fixture '{name}' has no steps (expects anchor rejection) \ @@ -110,7 +135,7 @@ fn run(path: &Path) -> datatest_stable::Result<()> { } let result = apply_fork_choice_step(&mut store, &step, Some(proofs_are_mocked)); - assert_step_outcome(step_idx, step.valid, result)?; + assert_step_outcome(step_idx, step.valid, step.rejection_reason.as_ref(), result)?; // Fold this step's blocks into the cumulative tree before checks so // ancestry walks see blocks finalization may have just pruned from @@ -134,17 +159,41 @@ fn run(path: &Path) -> datatest_stable::Result<()> { Ok(()) } -fn assert_step_outcome( +/// Assert a step's outcome against its `valid` flag and, for expected +/// rejections, against the `rejectionReason` the fixture names. +/// +/// Checking only that the step failed lets a fixture pass on the wrong error, so +/// a named reason must match the reason the client's error classifies to. A +/// rejection the classifier does not recognise fails the step as well: silently +/// accepting it would restore exactly the "any error will do" behaviour. +fn assert_step_outcome( step_idx: usize, expected_valid: bool, - result: Result, + expected_reason: Option<&RejectionReason>, + result: Result<(), StepError>, ) -> datatest_stable::Result<()> { match (result, expected_valid) { - (Ok(_), false) => Err(format!("Step {step_idx} expected failure but got success").into()), + (Ok(()), true) => Ok(()), + (Ok(()), false) => Err(format!( + "Step {step_idx} expected failure{} but got success", + expected_reason + .map(|reason| format!(" ({reason})")) + .unwrap_or_default() + ) + .into()), (Err(err), true) => { Err(format!("Step {step_idx} expected success but got failure: {err:?}").into()) } - _ => Ok(()), + // Older fixtures mark a step invalid without naming a reason; the + // rejection itself is all they assert. + (Err(_), false) if expected_reason.is_none() => Ok(()), + (Err(err), false) => check_rejection_reason( + &format!("Step {step_idx}"), + expected_reason.expect("reason is set on this arm"), + err.rejection_reason().as_ref(), + &err, + ) + .map_err(Into::into), } } diff --git a/crates/blockchain/tests/signature_spectests.rs b/crates/blockchain/tests/signature_spectests.rs index a11a58cc..d6413d94 100644 --- a/crates/blockchain/tests/signature_spectests.rs +++ b/crates/blockchain/tests/signature_spectests.rs @@ -1,7 +1,7 @@ use std::path::Path; use std::sync::Arc; -use ethlambda_blockchain::{MILLISECONDS_PER_SLOT, store}; +use ethlambda_blockchain::{MILLISECONDS_PER_SLOT, spec_test_runner, store}; use ethlambda_storage::{Store, backend::InMemoryBackend}; use ethlambda_types::{ block::{Block, SignedBlock}, @@ -9,7 +9,9 @@ use ethlambda_types::{ state::State, }; -use ethlambda_test_fixtures::verify_signatures::VerifySignaturesTestVector; +use ethlambda_test_fixtures::{ + rejection::check_rejection_reason, verify_signatures::VerifySignaturesTestVector, +}; const SUPPORTED_FIXTURE_FORMAT: &str = "verify_signatures_test"; @@ -66,28 +68,27 @@ fn run(path: &Path) -> datatest_stable::Result<()> { // Process the block (this includes signature verification) let result = store::on_block(&mut st, signed_block); - // Step 3: Check that it succeeded or failed as expected - match (result.is_ok(), test.expect_exception.as_ref()) { - (true, None) => { + // Step 3: Check that it succeeded or failed as expected, and that a + // rejection is the one the fixture named rather than any failure at all. + match (result, test.rejection_reason.as_ref()) { + (Ok(_), None) => { // Expected success, got success } - (true, Some(expected_err)) => { + (Ok(_), Some(expected)) => { return Err(format!( - "Test '{}' failed: expected exception '{}' but got success", - name, expected_err + "Test '{name}' failed: expected rejection {expected} but got success" ) .into()); } - (false, None) => { + (Err(err), None) => { return Err(format!( - "Test '{}' failed: expected success but got failure: {:?}", - name, - result.err() + "Test '{name}' failed: expected success but got failure: {err:?}" ) .into()); } - (false, Some(_)) => { - // Expected failure, got failure + (Err(err), Some(expected)) => { + let actual = spec_test_runner::rejection_reason(&err); + check_rejection_reason(&name, expected, actual.as_ref(), &err)?; } } } diff --git a/crates/common/test-fixtures/Cargo.toml b/crates/common/test-fixtures/Cargo.toml index 4821b42d..fe9496cd 100644 --- a/crates/common/test-fixtures/Cargo.toml +++ b/crates/common/test-fixtures/Cargo.toml @@ -10,6 +10,7 @@ rust-version.workspace = true version.workspace = true [dependencies] +ethlambda-state-transition.workspace = true ethlambda-types.workspace = true libssz.workspace = true libssz-types.workspace = true diff --git a/crates/common/test-fixtures/src/fork_choice.rs b/crates/common/test-fixtures/src/fork_choice.rs index a6d2952f..c6c9e58c 100644 --- a/crates/common/test-fixtures/src/fork_choice.rs +++ b/crates/common/test-fixtures/src/fork_choice.rs @@ -4,8 +4,8 @@ //! endpoints, which receive the same JSON shapes from the lean spec-assets simulator. use crate::{ - AggregationBits, AttestationData, Block, BlockBody, Checkpoint, TestInfo, TestState, - deser_xmss_hex, + AggregationBits, AttestationData, Block, BlockBody, Checkpoint, RejectionReason, TestInfo, + TestState, deser_xmss_hex, }; use ethlambda_types::attestation::XmssSignature; use ethlambda_types::block::{MultiMessageAggregate, SignedBlock}; @@ -53,11 +53,10 @@ pub struct ForkChoiceTest { #[serde(rename = "maxSlot")] #[allow(dead_code)] pub max_slot: u64, - /// Top-level expected rejection reason for whole-vector negative tests. - /// Captured only so `deny_unknown_fields` accepts it. + /// Expected rejection reason for whole-vector negative tests, which carry no + /// steps: the store must refuse the anchor itself. #[serde(rename = "rejectionReason")] - #[allow(dead_code)] - pub rejection_reason: Option, + pub rejection_reason: Option, #[serde(rename = "_info")] pub info: TestInfo, } @@ -113,12 +112,10 @@ pub struct ForkChoiceStep { // gossip-signature groups) once the required Store plumbing exists. #[serde(rename = "storeSnapshot")] pub store_snapshot: Option, - /// Expected rejection reason for a step marked `valid: false`. Captured only - /// so `deny_unknown_fields` accepts it; step outcomes are asserted via the - /// `valid` flag. + /// Expected rejection reason for a step marked `valid: false`. When set, the + /// step must not just fail: it must fail for this reason. #[serde(rename = "rejectionReason")] - #[allow(dead_code)] - pub rejection_reason: Option, + pub rejection_reason: Option, } fn default_true() -> bool { diff --git a/crates/common/test-fixtures/src/lib.rs b/crates/common/test-fixtures/src/lib.rs index 8b52e0a6..32114c80 100644 --- a/crates/common/test-fixtures/src/lib.rs +++ b/crates/common/test-fixtures/src/lib.rs @@ -6,7 +6,9 @@ mod common; pub mod fork_choice; +pub mod rejection; pub mod state_transition; pub mod verify_signatures; pub use common::*; +pub use rejection::RejectionReason; diff --git a/crates/common/test-fixtures/src/rejection.rs b/crates/common/test-fixtures/src/rejection.rs new file mode 100644 index 00000000..c0533ba8 --- /dev/null +++ b/crates/common/test-fixtures/src/rejection.rs @@ -0,0 +1,311 @@ +//! Language-neutral rejection reasons carried by negative leanSpec fixtures. +//! +//! Fixtures that expect their input to be rejected name *why* in a +//! `rejectionReason` field. Asserting only that the client failed lets a test +//! pass for the wrong reason (a state-root mismatch standing in for the rule the +//! fixture meant to exercise), so the spec-test runners compare the reason the +//! client's error maps to against the reason the fixture names. + +use serde::{Deserialize, Deserializer}; +use std::fmt; + +/// Language-neutral reason the spec rejects an invalid input. +/// +/// Mirrors leanSpec's `RejectionReason` StrEnum +/// (`src/lean_spec/spec/forks/lstar/errors.py`), which is the vocabulary fixtures +/// use for their `rejectionReason` field. Clients match on the reason code, never +/// on a human-readable message. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum RejectionReason { + // Block validation + /// The block slot is not strictly greater than the current state slot. + BlockSlotNotInFuture, + /// The block slot runs so far beyond its parent it would force an unbounded + /// empty-slot walk. + BlockSlotGapTooLarge, + /// The block slot is beyond the store's accepted future horizon. + BlockTooFarInFuture, + /// The block slot is not newer than the latest block header. + BlockOlderThanLatestHeader, + /// The block slot disagrees with the state slot after slot processing. + BlockSlotMismatch, + /// The block parent root disagrees with the latest block header root. + ParentRootMismatch, + /// The block state root disagrees with the computed post-state root. + StateRootMismatch, + /// The block references a parent the store has never seen. + UnknownParentBlock, + /// The proposer index does not address any registered validator. + ProposerIndexOutOfRange, + /// The registry holds no validators, so no proposer can be scheduled. + EmptyValidatorRegistry, + /// The block proposer is not the scheduled proposer for its slot. + WrongProposer, + /// The block carries more distinct attestation data entries than allowed. + TooManyAttestationData, + /// The block carries the same attestation data entry more than once. + DuplicateAttestationData, + /// An aggregated attestation references no validator at all. + EmptyAggregationBits, + + // Attestation validation + /// The attestation source root is not a known block. + UnknownSourceBlock, + /// The attestation target root is not a known block. + UnknownTargetBlock, + /// The attestation head root is not a known block. + UnknownHeadBlock, + /// The attestation source checkpoint slot exceeds its target slot. + SourceAfterTarget, + /// The attestation head checkpoint is older than its target. + HeadOlderThanTarget, + /// The source checkpoint slot disagrees with the referenced block. + SourceSlotMismatch, + /// The target checkpoint slot disagrees with the referenced block. + TargetSlotMismatch, + /// The head checkpoint slot disagrees with the referenced block. + HeadSlotMismatch, + /// The attestation source checkpoint is not an ancestor of its target. + SourceNotAncestorOfTarget, + /// The attestation target checkpoint is not an ancestor of its head. + TargetNotAncestorOfHead, + /// The attestation head checkpoint does not descend from the finalized block. + HeadNotDescendantOfFinalized, + /// The attestation slot is beyond the store's acceptance horizon. + AttestationTooFarInFuture, + /// The attestation slot precedes its head block's slot. + AttestationSlotBeforeHead, + /// The referenced validator does not exist in the state registry. + ValidatorNotInState, + /// The validator index does not address any registered validator. + ValidatorIndexOutOfRange, + /// A justification query named a slot beyond the tracked window. + JustifiedSlotOutOfRange, + /// A tracked justification root is the zero hash, which is not a valid root. + ZeroHashJustificationRoot, + /// The flat vote list length is not the tracked-root count times the + /// validator count. + JustificationVotesLengthMismatch, + + // Cryptographic verification + /// An attestation signature or aggregate proof fails verification. + InvalidSignature, + /// The block's multi-message aggregate proof fails verification. + InvalidBlockProof, + + // Anchor initialization + /// The anchor block state root disagrees with the anchor state. + AnchorStateRootMismatch, + + // Wire decoding + /// The input bytes cannot be decoded into the expected structure. + DecodeError, + + /// A reason string this build does not know. + /// + /// Fixtures track leanSpec's latest release, so a new reason can arrive + /// before this enum learns it. Deserialization keeps it verbatim (the Hive + /// test driver must still answer such a step rather than reject the request) + /// and the offline runners fail on it, naming the string to add here. + Unknown(String), +} + +impl RejectionReason { + /// The wire spelling fixtures use for this reason. + pub fn as_str(&self) -> &str { + match self { + Self::BlockSlotNotInFuture => "BLOCK_SLOT_NOT_IN_FUTURE", + Self::BlockSlotGapTooLarge => "BLOCK_SLOT_GAP_TOO_LARGE", + Self::BlockTooFarInFuture => "BLOCK_TOO_FAR_IN_FUTURE", + Self::BlockOlderThanLatestHeader => "BLOCK_OLDER_THAN_LATEST_HEADER", + Self::BlockSlotMismatch => "BLOCK_SLOT_MISMATCH", + Self::ParentRootMismatch => "PARENT_ROOT_MISMATCH", + Self::StateRootMismatch => "STATE_ROOT_MISMATCH", + Self::UnknownParentBlock => "UNKNOWN_PARENT_BLOCK", + Self::ProposerIndexOutOfRange => "PROPOSER_INDEX_OUT_OF_RANGE", + Self::EmptyValidatorRegistry => "EMPTY_VALIDATOR_REGISTRY", + Self::WrongProposer => "WRONG_PROPOSER", + Self::TooManyAttestationData => "TOO_MANY_ATTESTATION_DATA", + Self::DuplicateAttestationData => "DUPLICATE_ATTESTATION_DATA", + Self::EmptyAggregationBits => "EMPTY_AGGREGATION_BITS", + Self::UnknownSourceBlock => "UNKNOWN_SOURCE_BLOCK", + Self::UnknownTargetBlock => "UNKNOWN_TARGET_BLOCK", + Self::UnknownHeadBlock => "UNKNOWN_HEAD_BLOCK", + Self::SourceAfterTarget => "SOURCE_AFTER_TARGET", + Self::HeadOlderThanTarget => "HEAD_OLDER_THAN_TARGET", + Self::SourceSlotMismatch => "SOURCE_SLOT_MISMATCH", + Self::TargetSlotMismatch => "TARGET_SLOT_MISMATCH", + Self::HeadSlotMismatch => "HEAD_SLOT_MISMATCH", + Self::SourceNotAncestorOfTarget => "SOURCE_NOT_ANCESTOR_OF_TARGET", + Self::TargetNotAncestorOfHead => "TARGET_NOT_ANCESTOR_OF_HEAD", + Self::HeadNotDescendantOfFinalized => "HEAD_NOT_DESCENDANT_OF_FINALIZED", + Self::AttestationTooFarInFuture => "ATTESTATION_TOO_FAR_IN_FUTURE", + Self::AttestationSlotBeforeHead => "ATTESTATION_SLOT_BEFORE_HEAD", + Self::ValidatorNotInState => "VALIDATOR_NOT_IN_STATE", + Self::ValidatorIndexOutOfRange => "VALIDATOR_INDEX_OUT_OF_RANGE", + Self::JustifiedSlotOutOfRange => "JUSTIFIED_SLOT_OUT_OF_RANGE", + Self::ZeroHashJustificationRoot => "ZERO_HASH_JUSTIFICATION_ROOT", + Self::JustificationVotesLengthMismatch => "JUSTIFICATION_VOTES_LENGTH_MISMATCH", + Self::InvalidSignature => "INVALID_SIGNATURE", + Self::InvalidBlockProof => "INVALID_BLOCK_PROOF", + Self::AnchorStateRootMismatch => "ANCHOR_STATE_ROOT_MISMATCH", + Self::DecodeError => "DECODE_ERROR", + Self::Unknown(reason) => reason, + } + } +} + +/// Parse a fixture's `rejectionReason`, keeping an unrecognised one verbatim. +/// +/// The catch-all is why a missing arm here cannot pass silently: an unmapped +/// reason becomes [`RejectionReason::Unknown`], which every runner reports as a +/// failure naming the string to add. +impl From<&str> for RejectionReason { + fn from(reason: &str) -> Self { + match reason { + "BLOCK_SLOT_NOT_IN_FUTURE" => Self::BlockSlotNotInFuture, + "BLOCK_SLOT_GAP_TOO_LARGE" => Self::BlockSlotGapTooLarge, + "BLOCK_TOO_FAR_IN_FUTURE" => Self::BlockTooFarInFuture, + "BLOCK_OLDER_THAN_LATEST_HEADER" => Self::BlockOlderThanLatestHeader, + "BLOCK_SLOT_MISMATCH" => Self::BlockSlotMismatch, + "PARENT_ROOT_MISMATCH" => Self::ParentRootMismatch, + "STATE_ROOT_MISMATCH" => Self::StateRootMismatch, + "UNKNOWN_PARENT_BLOCK" => Self::UnknownParentBlock, + "PROPOSER_INDEX_OUT_OF_RANGE" => Self::ProposerIndexOutOfRange, + "EMPTY_VALIDATOR_REGISTRY" => Self::EmptyValidatorRegistry, + "WRONG_PROPOSER" => Self::WrongProposer, + "TOO_MANY_ATTESTATION_DATA" => Self::TooManyAttestationData, + "DUPLICATE_ATTESTATION_DATA" => Self::DuplicateAttestationData, + "EMPTY_AGGREGATION_BITS" => Self::EmptyAggregationBits, + "UNKNOWN_SOURCE_BLOCK" => Self::UnknownSourceBlock, + "UNKNOWN_TARGET_BLOCK" => Self::UnknownTargetBlock, + "UNKNOWN_HEAD_BLOCK" => Self::UnknownHeadBlock, + "SOURCE_AFTER_TARGET" => Self::SourceAfterTarget, + "HEAD_OLDER_THAN_TARGET" => Self::HeadOlderThanTarget, + "SOURCE_SLOT_MISMATCH" => Self::SourceSlotMismatch, + "TARGET_SLOT_MISMATCH" => Self::TargetSlotMismatch, + "HEAD_SLOT_MISMATCH" => Self::HeadSlotMismatch, + "SOURCE_NOT_ANCESTOR_OF_TARGET" => Self::SourceNotAncestorOfTarget, + "TARGET_NOT_ANCESTOR_OF_HEAD" => Self::TargetNotAncestorOfHead, + "HEAD_NOT_DESCENDANT_OF_FINALIZED" => Self::HeadNotDescendantOfFinalized, + "ATTESTATION_TOO_FAR_IN_FUTURE" => Self::AttestationTooFarInFuture, + "ATTESTATION_SLOT_BEFORE_HEAD" => Self::AttestationSlotBeforeHead, + "VALIDATOR_NOT_IN_STATE" => Self::ValidatorNotInState, + "VALIDATOR_INDEX_OUT_OF_RANGE" => Self::ValidatorIndexOutOfRange, + "JUSTIFIED_SLOT_OUT_OF_RANGE" => Self::JustifiedSlotOutOfRange, + "ZERO_HASH_JUSTIFICATION_ROOT" => Self::ZeroHashJustificationRoot, + "JUSTIFICATION_VOTES_LENGTH_MISMATCH" => Self::JustificationVotesLengthMismatch, + "INVALID_SIGNATURE" => Self::InvalidSignature, + "INVALID_BLOCK_PROOF" => Self::InvalidBlockProof, + "ANCHOR_STATE_ROOT_MISMATCH" => Self::AnchorStateRootMismatch, + "DECODE_ERROR" => Self::DecodeError, + other => Self::Unknown(other.to_string()), + } + } +} + +impl fmt::Display for RejectionReason { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +impl<'de> Deserialize<'de> for RejectionReason { + fn deserialize>(deserializer: D) -> Result { + Ok(String::deserialize(deserializer)?.as_str().into()) + } +} + +/// Check a client rejection against the reason its fixture named. +/// +/// `context` names the failing case (a test name or step index) and `err` is +/// reported verbatim so a mismatch is debuggable. Returns the message the +/// spec-test runners surface as the test failure. +/// +/// Both an unclassified rejection and a reason this build does not know are +/// failures: accepting either would silently restore "any error will do", which +/// is what pinning the reason exists to prevent. +pub fn check_rejection_reason( + context: &str, + expected: &RejectionReason, + actual: Option<&RejectionReason>, + err: &dyn fmt::Debug, +) -> Result<(), String> { + if let RejectionReason::Unknown(reason) = expected { + return Err(format!( + "{context} expects rejection reason '{reason}', which this build does not know. \ + Add it to `RejectionReason` and classify the error that must produce it." + )); + } + match actual { + Some(actual) if actual == expected => Ok(()), + Some(actual) => Err(format!( + "{context} was rejected for the wrong reason: expected {expected}, got {actual} \ + ({err:?})" + )), + None => Err(format!( + "{context} expected rejection reason {expected} but the error carries no reason: \ + {err:?}. Classify it in the runner's `rejection_reason` mapping." + )), + } +} + +/// Classify a state-transition failure, mirroring leanSpec's +/// `classify_rejection`. +/// +/// Total on purpose: the match is exhaustive, so a new +/// [`ethlambda_state_transition::Error`] variant is a compile error here until +/// someone names the reason it corresponds to. +impl From<ðlambda_state_transition::Error> for RejectionReason { + fn from(err: ðlambda_state_transition::Error) -> Self { + use ethlambda_state_transition::Error; + + match err { + Error::StateSlotIsNewer { .. } => Self::BlockSlotNotInFuture, + Error::SlotMismatch { .. } => Self::BlockSlotMismatch, + Error::ParentSlotIsNewer { .. } => Self::BlockOlderThanLatestHeader, + Error::InvalidProposer { .. } => Self::WrongProposer, + Error::InvalidParent { .. } => Self::ParentRootMismatch, + Error::NoValidators => Self::EmptyValidatorRegistry, + Error::StateRootMismatch { .. } => Self::StateRootMismatch, + Error::SlotGapTooLarge { .. } => Self::BlockSlotGapTooLarge, + Error::ZeroHashInJustificationRoots => Self::ZeroHashJustificationRoot, + Error::JustificationVotesLengthMismatch { .. } => { + Self::JustificationVotesLengthMismatch + } + Error::EmptyAggregationBits => Self::EmptyAggregationBits, + // The spec indexes a per-root vote list with each participant index, + // so a bit set beyond the registry is an out-of-range validator + // index rather than a malformed bitlist. + Error::AggregationBitsOutOfBounds { .. } => Self::ValidatorIndexOutOfRange, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn known_reasons_round_trip_through_their_wire_spelling() { + let reason = RejectionReason::from("HEAD_NOT_DESCENDANT_OF_FINALIZED"); + assert_eq!(reason, RejectionReason::HeadNotDescendantOfFinalized); + assert_eq!(reason.as_str(), "HEAD_NOT_DESCENDANT_OF_FINALIZED"); + } + + #[test] + fn unknown_reasons_keep_their_string_verbatim() { + let reason = RejectionReason::from("REASON_FROM_A_NEWER_SPEC"); + assert_eq!( + reason, + RejectionReason::Unknown("REASON_FROM_A_NEWER_SPEC".to_string()) + ); + assert_eq!(reason.as_str(), "REASON_FROM_A_NEWER_SPEC"); + } + + #[test] + fn deserializes_from_a_json_string() { + let reason: RejectionReason = serde_json::from_str("\"SOURCE_AFTER_TARGET\"").unwrap(); + assert_eq!(reason, RejectionReason::SourceAfterTarget); + } +} diff --git a/crates/common/test-fixtures/src/verify_signatures.rs b/crates/common/test-fixtures/src/verify_signatures.rs index 038ee5d7..b28b0ed1 100644 --- a/crates/common/test-fixtures/src/verify_signatures.rs +++ b/crates/common/test-fixtures/src/verify_signatures.rs @@ -10,7 +10,7 @@ //! block: {...standard block fields...} //! proof: { proof: { data: "0x" } } -use crate::{Block, TestInfo, TestState}; +use crate::{Block, RejectionReason, TestInfo, TestState}; use ethlambda_types::block::{MultiMessageAggregate, SignedBlock}; use serde::Deserialize; use std::collections::HashMap; @@ -45,11 +45,11 @@ pub struct VerifySignaturesTest { pub anchor_state: TestState, #[serde(rename = "signedBlock")] pub signed_block: TestSignedBlock, - /// Expected rejection, when present. Newer fixtures name this field + /// Expected rejection reason, when present. Newer fixtures name this field /// `rejectionReason` (leanSpec replaced `expectException`); both /// spellings are accepted. - #[serde(default, rename = "expectException", alias = "rejectionReason")] - pub expect_exception: Option, + #[serde(default, rename = "rejectionReason", alias = "expectException")] + pub rejection_reason: Option, /// Aggregation proof regime (see [`crate::fork_choice::ForkChoiceTest`]). /// Captured only so `deny_unknown_fields` accepts it. #[serde(rename = "proofSetting")] @@ -122,7 +122,7 @@ impl std::error::Error for SignedBlockConvertError {} /// Lossy fixture-to-SignedBlock conversion that preserves the merged proof. /// /// The conversion is fallible because the proof bytes may not decode as hex -/// or may exceed the wire cap. Tests with `expectException` set tolerate +/// or may exceed the wire cap. Tests with `rejectionReason` set tolerate /// failures upstream; the From impl panics so test runners get a clear /// signal when fixture shape drifts. impl From for SignedBlock { diff --git a/crates/net/rpc/src/test_driver.rs b/crates/net/rpc/src/test_driver.rs index bd79d6a1..ba8ed584 100644 --- a/crates/net/rpc/src/test_driver.rs +++ b/crates/net/rpc/src/test_driver.rs @@ -231,7 +231,7 @@ async fn step_fork_choice( Ok(()) => (true, None), Err(err) => { debug!(%err, "fork-choice step rejected"); - (false, Some(err)) + (false, Some(err.to_string())) } }; let snapshot = snapshot_store(&guard);