Fork choice fixes

lean_client/fork_choice/src/handlers.rs

@@ -3,8 +3,8 @@ use std::sync::Arc;
 
 use anyhow::{Context, Result, anyhow, bail, ensure};
 use containers::{
-    AttestationData, Checkpoint, SignatureKey, SignedAggregatedAttestation, SignedAttestation,
-    SignedBlock, State,
+    AggregatedAttestation, AttestationData, Checkpoint, SignatureKey, SignedAggregatedAttestation,
+    SignedAttestation, SignedBlock, State,
 };
 use metrics::METRICS;
 use parking_lot::RwLock;
@@ -74,6 +74,14 @@ fn validate_attestation_data(store: &Store, data: &AttestationData) -> Result<()
         data.target.slot.0
     );
 
+    // A head in a future slot relative to the attestation slot is invalid.
+    ensure!(
+        data.slot >= data.head.slot,
+        "Attestation slot {} precedes head slot {}",
+        data.slot.0,
+        data.head.slot.0
+    );
+
     // Validate checkpoint slots match block slots.
     let source_block = &store.blocks[&data.source.root];
     let target_block = &store.blocks[&data.target.root];
@@ -226,6 +234,20 @@ pub fn on_gossip_attestation(
         });
     })?;
 
+    // Reject validators outside the registry (independent of signature checks).
+    let num_validators = store
+        .states
+        .get(&attestation_data.target.root)
+        .ok_or_else(|| anyhow!("no state for target block {}", attestation_data.target.root))?
+        .validators
+        .len_u64();
+    ensure!(
+        validator_id < num_validators,
+        "validator {} out of range (max {})",
+        validator_id,
+        num_validators
+    );
+
     // Non-aggregators validate attestation data but do not store or verify individual
     // signatures. Per leanSpec: only aggregators import gossip attestations for aggregation.
     // Subnet filtering is already enforced at the p2p subscription layer.
@@ -409,10 +431,13 @@ pub fn on_attestation(
 /// Verifies the aggregated XMSS proof against participant public keys and stores
 /// it in `latest_new_aggregated_payloads`. At interval 3, these are merged with
 /// `latest_known_aggregated_payloads` (from blocks) to compute safe target.
+///
+/// `verify_proof` gates only the XMSS SNARK check.
 #[inline]
 pub fn on_aggregated_attestation(
     store: &mut Store,
     signed_aggregated_attestation: SignedAggregatedAttestation,
+    verify_proof: bool,
 ) -> Result<()> {
     // Structure: { data: AttestationData, proof: AggregatedSignatureProof }
     let attestation_data = signed_aggregated_attestation.data.clone();
@@ -472,9 +497,11 @@ pub fn on_aggregated_attestation(
         })
         .collect::<Result<Vec<_>>>()?;
 
-    proof
-        .verify(public_keys, data_root, attestation_data.slot.0 as u32)
-        .context("aggregated attestation proof verification failed")?;
+    if verify_proof {
+        proof
+            .verify(public_keys, data_root, attestation_data.slot.0 as u32)
+            .context("aggregated attestation proof verification failed")?;
+    }
 
     let attestation_slot = attestation_data.slot;
     for vid in &validator_ids {
@@ -719,6 +746,13 @@ pub fn on_block(
         current_slot,
     );
 
+    // Reject block bodies carrying duplicate AttestationData (the state
+    // transition only bounds the distinct count, it does not dedup).
+    ensure!(
+        !AggregatedAttestation::has_duplicate_data(&signed_block.block.body.attestations),
+        "block body has duplicate AttestationData"
+    );
+
     process_block_internal(store, signed_block, block_root, verify_signatures)?;
     process_pending_blocks(store, cache, vec![block_root], verify_signatures);
 
@@ -796,7 +830,7 @@ pub fn apply_verified_block(
         .remove(&block_root)
         .unwrap_or_default();
     for signed_agg in pending_agg {
-        if let Err(err) = on_aggregated_attestation(store, signed_agg) {
+        if let Err(err) = on_aggregated_attestation(store, signed_agg, true) {
             warn!(%err, "Pending aggregated attestation retry failed after block arrival");
         }
     }

lean_client/http_api/src/test_driver.rs

@@ -34,6 +34,8 @@ use spec_test_fixtures::{
 use ssz::SszHash;
 use xmss::{AggregatedSignature, Signature};
 
+const MOCK_AGGREGATION_PROOF_MARKER: &[u8] = b"MOCKED-AGGREGATION-PROOF";
+
 /// Shared state for test-driver routes. Carries a writable handle to the
 /// fork-choice store plus the `BlockCache` that `on_block` requires.
 ///
@@ -375,20 +377,26 @@ fn apply_step(
             on_tick(store, target_time_millis, has_proposal.unwrap_or(false));
             Ok(())
         }
-        ForkChoiceStep::Block { block, .. } => {
+        ForkChoiceStep::Block {
+            block,
+            tick_to_slot,
+            ..
+        } => {
             let block: containers::Block = block.into();
             let signed = SignedBlock {
                 block,
                 proof: MultiMessageAggregate::default(),
             };
 
-            // Advance store time to the block's slot before applying it.
-            // Mirrors the local fork-choice test: attestations embedded in
-            // the block reference the slot, so the store needs to be at or
-            // past that interval.
-            let slot_time_millis =
-                (store.config.genesis_time + signed.block.slot.0 * SECONDS_PER_SLOT) * 1000;
-            on_tick(store, slot_time_millis, false);
+            // Advance store time to the block's slot before applying it, but
+            // only when the fixture requests it. Fixtures with
+            // `tickToSlot: false` deliberately keep the clock behind the block
+            // to exercise early-arrival and future-horizon handling.
+            if tick_to_slot {
+                let slot_time_millis =
+                    (store.config.genesis_time + signed.block.slot.0 * SECONDS_PER_SLOT) * 1000;
+                on_tick(store, slot_time_millis, false);
+            }
 
             // Skip XMSS signature verification — fork_choice fixtures ship
             // unsigned step blocks, so we apply them with a placeholder
@@ -412,8 +420,8 @@ fn apply_step(
                 // `Checks` steps still see a snapshot.
                 return Ok(());
             };
-            let signed = build_signed_aggregated_attestation(step)?;
-            on_aggregated_attestation(store, signed).map_err(|err| err.to_string())
+            let (signed, verify_proof) = build_signed_aggregated_attestation(step)?;
+            on_aggregated_attestation(store, signed, verify_proof).map_err(|err| err.to_string())
         }
         ForkChoiceStep::Checks { .. } => {
             // Pure-assertion step. The simulator validates against the
@@ -468,20 +476,29 @@ fn hex_root(root: &ssz::H256) -> String {
 /// string (the harness cannot re-aggregate without the signers' private
 /// keys), so we decode it directly into an [`AggregatedSignature`] and
 /// wrap with the participants bitfield from the same payload.
+///
+/// Returns the attestation and whether its proof should be XMSS-verified —
+/// `false` when the proof is the mocked sentinel.
 fn build_signed_aggregated_attestation(
     step: GossipAggregatedAttestationStep,
-) -> Result<SignedAggregatedAttestation, String> {
+) -> Result<(SignedAggregatedAttestation, bool), String> {
     let proof_hex = step.proof.proof.data.trim_start_matches("0x");
     let proof_bytes = hex::decode(proof_hex)
         .map_err(|err| format!("invalid hex in aggregate proof_data: {err}"))?;
+    let verify_proof = !proof_bytes
+        .windows(MOCK_AGGREGATION_PROOF_MARKER.len())
+        .any(|window| window == MOCK_AGGREGATION_PROOF_MARKER);
     let proof_data = AggregatedSignature::new(&proof_bytes)
         .map_err(|err| format!("failed to construct aggregated signature: {err}"))?;
 
-    Ok(SignedAggregatedAttestation {
-        data: step.data.into(),
-        proof: AggregatedSignatureProof {
-            participants: step.proof.participants.into(),
-            proof_data,
+    Ok((
+        SignedAggregatedAttestation {
+            data: step.data.into(),
+            proof: AggregatedSignatureProof {
+                participants: step.proof.participants.into(),
+                proof_data,
+            },
         },
-    })
+        verify_proof,
+    ))
 }

lean_client/spec_test_fixtures/src/fork_choice.rs

@@ -43,7 +43,7 @@ pub enum ForkChoiceStep {
         time: Option<u64>,
         #[serde(default)]
         interval: Option<u64>,
-        #[serde(default)]
+        #[serde(default, rename = "hasProposal")]
         has_proposal: Option<bool>,
         #[serde(default)]
         checks: Option<StoreChecks>,
@@ -58,7 +58,7 @@ pub enum ForkChoiceStep {
         time: Option<u64>,
         #[serde(default)]
         interval: Option<u64>,
-        #[serde(default)]
+        #[serde(default, rename = "hasProposal")]
         has_proposal: Option<bool>,
         #[serde(default)]
         checks: Option<StoreChecks>,
@@ -69,6 +69,8 @@ pub enum ForkChoiceStep {
         valid: bool,
         #[serde(default)]
         checks: Option<StoreChecks>,
+        #[serde(default, rename = "tickToSlot")]
+        tick_to_slot: bool,
         block: TestBlockWithAttestation,
     },
     /// Apply a single-validator gossip attestation to the store.