Split on_block() and on_gossip_attestation() into verified and unverified variants

CLAUDE.md

@@ -74,7 +74,7 @@ Fork choice head update
 ```bash
 make fmt                                     # Format code (cargo fmt --all)
 make lint                                    # Clippy with -D warnings
-make test                                    # All tests + forkchoice (with skip-signature-verification)
+make test                                    # All tests + forkchoice spec tests
 ```
 
 ### Common Operations
@@ -295,21 +295,22 @@ GENESIS_VALIDATORS:
 ### Test Categories
 1. **Unit tests**: Embedded in source files
 2. **Spec tests**: From `leanSpec/fixtures/consensus/`
-   - `forkchoice_spectests.rs` (requires `skip-signature-verification`)
+   - `forkchoice_spectests.rs` (uses `on_block_without_verification`)
    - `signature_spectests.rs`
    - `stf_spectests.rs` (state transition)
 
 ### Running Tests
 ```bash
 cargo test --workspace --release                                    # All workspace tests
-cargo test -p ethlambda-blockchain --features skip-signature-verification --test forkchoice_spectests
-cargo test -p ethlambda-blockchain --features skip-signature-verification --test forkchoice_spectests -- --test-threads=1  # Sequential
+cargo test -p ethlambda-blockchain --test forkchoice_spectests
+cargo test -p ethlambda-blockchain --test forkchoice_spectests -- --test-threads=1  # Sequential
 ```
 
 ## Common Gotchas
 
 ### Signature Verification
-- Tests require `skip-signature-verification` feature for performance
+- Fork choice tests use `on_block_without_verification()` to skip signature checks
+- Signature spec tests use `on_block()` which always verifies
 - Crypto tests marked `#[ignore]` (slow leanVM operations)
 
 ### Storage Architecture

Makefile

@@ -9,10 +9,9 @@ fmt: ## 🎨 Format all code using rustfmt
 lint: ## 🔍 Run clippy on all workspace crates
 	cargo clippy --workspace --all-targets -- -D warnings
 
-test: leanSpec/fixtures ## 🧪 Run all tests, then forkchoice tests with skip-signature-verification
+test: leanSpec/fixtures ## 🧪 Run all tests
 	# Tests need to be run on release to avoid stack overflows during signature verification/aggregation
 	cargo test --workspace --release
-	cargo test -p ethlambda-blockchain --features skip-signature-verification --test forkchoice_spectests
 
 GIT_COMMIT=$(shell git rev-parse HEAD)
 GIT_BRANCH=$(shell git rev-parse --abbrev-ref HEAD)

RELEASE.md

@@ -67,13 +67,13 @@ The Dockerfile accepts build arguments for customizing the build:
 | Argument | Default | Description |
 |----------|---------|-------------|
 | `BUILD_PROFILE` | `release` | Cargo build profile |
-| `FEATURES` | `""` | Extra Cargo features (e.g. `skip-signature-verification`) |
+| `FEATURES` | `""` | Extra Cargo features |
 | `RUSTFLAGS` | `""` | Extra Rust compiler flags |
 
 Example with custom args:
 
 ```bash
-docker build --build-arg FEATURES=skip-signature-verification -t ethlambda:test .
+docker build --build-arg BUILD_PROFILE=debug -t ethlambda:debug .
 ```
 
 `GIT_COMMIT` and `GIT_BRANCH` are also available but set automatically by CI.

crates/blockchain/Cargo.toml

@@ -10,10 +10,6 @@ rust-version.workspace = true
 version.workspace = true
 autotests = false
 
-[features]
-# To skip signature verification during tests
-skip-signature-verification = []
-
 [dependencies]
 ethlambda-storage.workspace = true
 ethlambda-state-transition.workspace = true
@@ -45,7 +41,6 @@ ssz_types = "0.14.0"
 name = "forkchoice_spectests"
 path = "tests/forkchoice_spectests.rs"
 harness = false
-required-features = ["skip-signature-verification"]
 
 [[test]]
 name = "signature_spectests"

crates/blockchain/src/store.rs

@@ -188,10 +188,36 @@ pub fn on_tick(store: &mut Store, timestamp: u64, has_proposal: bool) {
     }
 }
 
-/// Process a gossiped attestation.
+/// Process a gossiped attestation (with signature verification).
+///
+/// This is the safe default: it always verifies the validator's XMSS signature
+/// and stores it for future block building.
 pub fn on_gossip_attestation(
     store: &mut Store,
     signed_attestation: SignedAttestation,
+) -> Result<(), StoreError> {
+    on_gossip_attestation_core(store, signed_attestation, true)
+}
+
+/// Process a gossiped attestation without signature verification.
+///
+/// This skips all cryptographic checks and signature storage. Use only in tests
+/// where signatures are absent or irrelevant.
+pub fn on_gossip_attestation_without_verification(
+    store: &mut Store,
+    signed_attestation: SignedAttestation,
+) -> Result<(), StoreError> {
+    on_gossip_attestation_core(store, signed_attestation, false)
+}
+
+/// Core gossip attestation processing logic.
+///
+/// When `verify` is true, the validator's XMSS signature is checked and stored
+/// for future block building. When false, all signature checks are skipped.
+fn on_gossip_attestation_core(
+    store: &mut Store,
+    signed_attestation: SignedAttestation,
+    verify: bool,
 ) -> Result<(), StoreError> {
     let validator_id = signed_attestation.validator_id;
     let attestation = Attestation {
@@ -200,35 +226,36 @@ pub fn on_gossip_attestation(
     };
     validate_attestation(store, &attestation)
         .inspect_err(|_| metrics::inc_attestations_invalid("gossip"))?;
-    let target = attestation.data.target;
-    let target_state = store
-        .get_state(&target.root)
-        .ok_or(StoreError::MissingTargetState(target.root))?;
-    if validator_id >= target_state.validators.len() as u64 {
-        return Err(StoreError::InvalidValidatorIndex);
-    }
-    let validator_pubkey = target_state.validators[validator_id as usize]
-        .get_pubkey()
-        .map_err(|_| StoreError::PubkeyDecodingFailed(validator_id))?;
-    let message = attestation.data.tree_hash_root();
-    if cfg!(not(feature = "skip-signature-verification")) {
-        use ethlambda_types::signature::ValidatorSignature;
-        // Use attestation.data.slot as epoch (matching what Zeam and ethlambda use for signing)
+
+    if verify {
+        let target = attestation.data.target;
+        let target_state = store
+            .get_state(&target.root)
+            .ok_or(StoreError::MissingTargetState(target.root))?;
+        if validator_id >= target_state.validators.len() as u64 {
+            return Err(StoreError::InvalidValidatorIndex);
+        }
+        let validator_pubkey = target_state.validators[validator_id as usize]
+            .get_pubkey()
+            .map_err(|_| StoreError::PubkeyDecodingFailed(validator_id))?;
+        let message = attestation.data.tree_hash_root();
+
+        // Verify the validator's XMSS signature
         let epoch: u32 = attestation.data.slot.try_into().expect("slot exceeds u32");
         let signature = ValidatorSignature::from_bytes(&signed_attestation.signature)
             .map_err(|_| StoreError::SignatureDecodingFailed)?;
         if !signature.is_valid(&validator_pubkey, epoch, &message) {
             return Err(StoreError::SignatureVerificationFailed);
         }
-    }
-    on_attestation(store, attestation.clone(), false)?;
 
-    if cfg!(not(feature = "skip-signature-verification")) {
+        on_attestation(store, attestation.clone(), false)?;
+
         // Store signature for later lookup during block building
-        let signature = ValidatorSignature::from_bytes(&signed_attestation.signature)
-            .map_err(|_| StoreError::SignatureDecodingFailed)?;
         store.insert_gossip_signature(&attestation.data, validator_id, signature);
+    } else {
+        on_attestation(store, attestation.clone(), false)?;
     }
+
     metrics::inc_attestations_valid("gossip");
 
     let slot = attestation.data.slot;
@@ -312,23 +339,40 @@ fn on_attestation(
     Ok(())
 }
 
-/// Process a new block and update the forkchoice state.
+/// Process a new block and update the forkchoice state (with signature verification).
 ///
-/// This method integrates a block into the forkchoice store by:
-/// 1. Validating the block's parent exists
-/// 2. Computing the post-state via the state transition function
-/// 3. Processing attestations included in the block body (on-chain)
-/// 4. Updating the forkchoice head
-/// 5. Processing the proposer's attestation (as if gossiped)
+/// This is the safe default: it always verifies cryptographic signatures
+/// and stores them for future block building. Use this for all production paths.
 pub fn on_block(
     store: &mut Store,
     signed_block: SignedBlockWithAttestation,
+) -> Result<(), StoreError> {
+    on_block_core(store, signed_block, true)
+}
+
+/// Process a new block without signature verification.
+///
+/// This skips all cryptographic checks and signature storage. Use only in tests
+/// where signatures are absent or irrelevant (e.g., fork choice spec tests).
+pub fn on_block_without_verification(
+    store: &mut Store,
+    signed_block: SignedBlockWithAttestation,
+) -> Result<(), StoreError> {
+    on_block_core(store, signed_block, false)
+}
+
+/// Core block processing logic.
+///
+/// When `verify` is true, cryptographic signatures are validated and stored
+/// for future block building. When false, all signature checks are skipped.
+fn on_block_core(
+    store: &mut Store,
+    signed_block: SignedBlockWithAttestation,
+    verify: bool,
 ) -> Result<(), StoreError> {
     let _timing = metrics::time_fork_choice_block_processing();
 
-    // Unpack block components
-    let block = signed_block.message.block.clone();
-    let proposer_attestation = signed_block.message.proposer_attestation.clone();
+    let block = &signed_block.message.block;
     let block_root = block.tree_hash_root();
     let slot = block.slot;
 
@@ -348,15 +392,18 @@ pub fn on_block(
                 slot,
             })?;
 
-    // Validate cryptographic signatures
-    // TODO: extract signature verification to a pre-checks function
-    // to avoid the need for this
-    if cfg!(not(feature = "skip-signature-verification")) {
+    if verify {
+        // Validate cryptographic signatures
         verify_signatures(&parent_state, &signed_block)?;
     }
 
+    let block = signed_block.message.block.clone();
+    let proposer_attestation = signed_block.message.proposer_attestation.clone();
+    let block_root = block.tree_hash_root();
+    let slot = block.slot;
+
     // Execute state transition function to compute post-block state
-    let mut post_state = parent_state.clone();
+    let mut post_state = parent_state;
     ethlambda_state_transition::state_transition(&mut post_state, &block)?;
 
     // Cache the state root in the latest block header
@@ -414,11 +461,7 @@ pub fn on_block(
     // to prevent the proposer from gaining circular weight advantage.
     update_head(store);
 
-    // Process proposer attestation as if received via gossip
-    // The proposer's attestation should NOT affect this block's fork choice position.
-    // It is treated as pending until interval 3 (end of slot).
-
-    if cfg!(not(feature = "skip-signature-verification")) {
+    if verify {
         // Store the proposer's signature for potential future block building
         let proposer_sig =
             ValidatorSignature::from_bytes(&signed_block.signature.proposer_signature)
@@ -430,7 +473,9 @@ pub fn on_block(
         );
     }
 
-    // Process proposer attestation (enters "new" stage, not "known")
+    // Process proposer attestation as if received via gossip
+    // The proposer's attestation should NOT affect this block's fork choice position.
+    // It is treated as pending until interval 3 (end of slot).
     // TODO: validate attestations before processing
     let _ = on_attestation(store, proposer_attestation, false)
         .inspect(|_| metrics::inc_attestations_valid("gossip"))

crates/blockchain/tests/forkchoice_spectests.rs

@@ -63,7 +63,7 @@ fn run(path: &Path) -> datatest_stable::Result<()> {
 
                     // NOTE: the has_proposal argument is set to true, following the spec
                     store::on_tick(&mut store, block_time, true);
-                    let result = store::on_block(&mut store, signed_block);
+                    let result = store::on_block_without_verification(&mut store, signed_block);
 
                     match (result.is_ok(), step.valid) {
                         (true, false) => {
@@ -303,46 +303,46 @@ fn validate_attestation_check(
     })?;
 
     // Validate attestation slot if specified
-    if let Some(expected_slot) = check.attestation_slot {
-        if attestation.slot != expected_slot {
-            return Err(format!(
-                "Step {}: attestation slot mismatch for validator {}: expected {}, got {}",
-                step_idx, validator_id, expected_slot, attestation.slot
-            )
-            .into());
-        }
+    if let Some(expected_slot) = check.attestation_slot
+        && attestation.slot != expected_slot
+    {
+        return Err(format!(
+            "Step {}: attestation slot mismatch for validator {}: expected {}, got {}",
+            step_idx, validator_id, expected_slot, attestation.slot
+        )
+        .into());
     }
 
-    if let Some(expected_head_slot) = check.head_slot {
-        if attestation.head.slot != expected_head_slot {
-            return Err(format!(
-                "Step {}: attestation head slot mismatch: expected {}, got {}",
-                step_idx, expected_head_slot, attestation.head.slot
-            )
-            .into());
-        }
+    if let Some(expected_head_slot) = check.head_slot
+        && attestation.head.slot != expected_head_slot
+    {
+        return Err(format!(
+            "Step {}: attestation head slot mismatch: expected {}, got {}",
+            step_idx, expected_head_slot, attestation.head.slot
+        )
+        .into());
     }
 
     // Validate source slot if specified
-    if let Some(expected_source_slot) = check.source_slot {
-        if attestation.source.slot != expected_source_slot {
-            return Err(format!(
-                "Step {}: attestation source slot mismatch: expected {}, got {}",
-                step_idx, expected_source_slot, attestation.source.slot
-            )
-            .into());
-        }
+    if let Some(expected_source_slot) = check.source_slot
+        && attestation.source.slot != expected_source_slot
+    {
+        return Err(format!(
+            "Step {}: attestation source slot mismatch: expected {}, got {}",
+            step_idx, expected_source_slot, attestation.source.slot
+        )
+        .into());
     }
 
     // Validate target slot if specified
-    if let Some(expected_target_slot) = check.target_slot {
-        if attestation.target.slot != expected_target_slot {
-            return Err(format!(
-                "Step {}: attestation target slot mismatch: expected {}, got {}",
-                step_idx, expected_target_slot, attestation.target.slot
-            )
-            .into());
-        }
+    if let Some(expected_target_slot) = check.target_slot
+        && attestation.target.slot != expected_target_slot
+    {
+        return Err(format!(
+            "Step {}: attestation target slot mismatch: expected {}, got {}",
+            step_idx, expected_target_slot, attestation.target.slot
+        )
+        .into());
     }
 
     Ok(())