fix(state-transition): bound distinct attestation data in the transition

The per-block cap on distinct AttestationData is a transition rule in
leanSpec (`process_attestations`), and `fork_choice.on_block` says so
explicitly: "The transition itself bounds the distinct-data count. Only the
wire-level duplicate prohibition lives here."

We enforced it only at the import boundary in `on_block`, so
`state_transition()` accepted an over-cap block and then failed on the state
root instead. Both block production (`build_block` -> `process_block`) and
spec-fixture replay call the transition without going through `on_block`, so
neither was bounded.

Check it at the top of `process_attestations`, ahead of the
justification-bookkeeping guards as the spec does. The `on_block` check stays
for now: it runs before signature verification, so an over-cap block is still
rejected without paying for proof verification.

(leanSpec #536)

Author: MegaRedHand

crates/blockchain/state_transition/src/lib.rs

@@ -1,9 +1,9 @@
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 
 use ethlambda_types::{
     ShortRoot,
     attestation::AttestationData,
-    block::{AggregatedAttestations, Block, BlockHeader},
+    block::{AggregatedAttestations, Block, BlockHeader, MAX_ATTESTATIONS_DATA},
     checkpoint::Checkpoint,
     primitives::{H256, HashTreeRoot as _},
     state::{HISTORICAL_ROOTS_LIMIT, JustificationValidators, State},
@@ -48,6 +48,8 @@ pub enum Error {
         index: usize,
         validator_count: usize,
     },
+    #[error("block carries {count} distinct AttestationData entries; maximum is {max}")]
+    TooManyAttestationData { count: usize, max: usize },
 }
 
 /// Transition the given pre-state to the block's post-state.
@@ -243,6 +245,24 @@ fn process_attestations(
 ) -> Result<(), Error> {
     let _timing = metrics::time_attestations_processing();
 
+    // Cap the distinct attestation data a block may carry (leanSpec #536).
+    //
+    // Each distinct data allocates a tally sized to the validator set below, so the
+    // distinct count, not the attestation count, is what drives the work here. Split
+    // aggregates over one data share their tally and count once.
+    //
+    // `on_block` re-checks this before signature verification so a crafted block is
+    // rejected cheaply, but the bound belongs to the transition: this is the only
+    // entry point block production and fixture replay share with block import.
+    let distinct_attestation_data: HashSet<&AttestationData> =
+        attestations.iter().map(|att| &att.data).collect();
+    if distinct_attestation_data.len() > MAX_ATTESTATIONS_DATA {
+        return Err(Error::TooManyAttestationData {
+            count: distinct_attestation_data.len(),
+            max: MAX_ATTESTATIONS_DATA,
+        });
+    }
+
     // Validate the justification bookkeeping before unpacking the flat vote list
     // (leanSpec #1178). A `State` decoded from untrusted bytes (e.g. checkpoint
     // sync) can satisfy SSZ yet still violate these cross-field invariants; without
@@ -973,4 +993,99 @@ mod tests {
             "expected NoValidators, got {err:?}"
         );
     }
+
+    /// Build `count` attestations carrying pairwise-distinct `AttestationData`.
+    fn distinct_attestations(count: usize, validator_count: usize) -> AggregatedAttestations {
+        let r1 = H256([1u8; 32]);
+        let attestations: Vec<AggregatedAttestation> = (0..count)
+            .map(|i| {
+                make_attestation(
+                    i as u64 + 1,
+                    (0, H256::ZERO),
+                    (1, r1),
+                    (1, r1),
+                    &[0],
+                    validator_count,
+                )
+            })
+            .collect();
+        attestations
+            .try_into()
+            .expect("count is under the SSZ limit")
+    }
+
+    /// A state whose only interesting property is its validator count, so the
+    /// cap can be exercised without satisfying the vote-tracking invariants.
+    fn state_for_cap_tests(validator_count: usize) -> State {
+        let r1 = H256([1u8; 32]);
+        State {
+            config: ChainConfig { genesis_time: 0 },
+            slot: 2,
+            latest_block_header: BlockHeader {
+                slot: 1,
+                proposer_index: 0,
+                parent_root: H256::ZERO,
+                state_root: H256::ZERO,
+                body_root: BlockBody::default().hash_tree_root(),
+            },
+            latest_justified: Checkpoint {
+                slot: 0,
+                root: H256::ZERO,
+            },
+            latest_finalized: Checkpoint {
+                slot: 0,
+                root: H256::ZERO,
+            },
+            historical_block_hashes: SszList::try_from(vec![H256::ZERO, r1]).unwrap(),
+            justified_slots: JustifiedSlots::new(),
+            validators: SszList::try_from(make_validators(validator_count)).unwrap(),
+            justifications_roots: Default::default(),
+            justifications_validators: JustificationValidators::new(),
+        }
+    }
+
+    /// leanSpec #536: the distinct-attestation-data cap belongs to the
+    /// transition, not only to the import boundary in `on_block`. Each distinct
+    /// data allocates a per-validator tally, so a block that slips past the
+    /// proposer-side clamp must be rejected here too.
+    #[test]
+    fn process_attestations_rejects_more_distinct_data_than_the_cap() {
+        const NUM_VALIDATORS: usize = 4;
+
+        let atts = distinct_attestations(MAX_ATTESTATIONS_DATA + 1, NUM_VALIDATORS);
+        let err =
+            process_attestations(&mut state_for_cap_tests(NUM_VALIDATORS), &atts).unwrap_err();
+        assert!(
+            matches!(
+                err,
+                Error::TooManyAttestationData { count, max }
+                    if count == MAX_ATTESTATIONS_DATA + 1 && max == MAX_ATTESTATIONS_DATA
+            ),
+            "expected TooManyAttestationData over the cap, got {err:?}"
+        );
+
+        // The bound is inclusive: a block exactly at the cap clears this check.
+        let atts = distinct_attestations(MAX_ATTESTATIONS_DATA, NUM_VALIDATORS);
+        let result = process_attestations(&mut state_for_cap_tests(NUM_VALIDATORS), &atts);
+        assert!(
+            !matches!(result, Err(Error::TooManyAttestationData { .. })),
+            "a block at the cap must not be rejected for exceeding it, got {result:?}"
+        );
+    }
+
+    /// The cap is checked before the justification-bookkeeping guards, matching
+    /// the spec's order inside `process_attestations`: an over-cap block reports
+    /// the cap even when the state would also fail a later guard.
+    #[test]
+    fn attestation_data_cap_precedes_the_justification_guards() {
+        // An empty registry is rejected by the guard immediately after the cap.
+        let mut state = state_for_cap_tests(0);
+        let atts = distinct_attestations(MAX_ATTESTATIONS_DATA + 1, 4);
+
+        let err = process_attestations(&mut state, &atts).unwrap_err();
+        assert!(
+            matches!(err, Error::TooManyAttestationData { .. }),
+            "expected the cap to be reported ahead of NoValidators, got {err:?}"
+        );
+    }
 }