fix: align fork-choice finality and aggregate validation with leanSpec

MegaRedHand · MegaRedHand · commit 718768e3c360 · 2026-06-16T13:08:51.000-03:00
Three spec-test fixtures from the latest leanSpec release failed:

- test_losing_fork_higher_finalized_does_not_latch: finalized was latched
  monotonically in on_block_core. The spec derives the finalized checkpoint
  from the chosen head's post-state in update_head and pins it to the head's
  own ancestry, so a fork switch can move finalization downward. Move that
  computation into update_head; keep justified monotonic (advance_to) as the
  spec does.

- test_gossip_aggregated_attestation_empty_participants_rejected: an aggregate
  naming no participants slipped through the mocked-proof path. Reject empty
  participants (EMPTY_AGGREGATION_BITS) before bounds-check and verification,
  matching the spec's to_validator_indices().

- test_genesis_registry_holds_exact_validator_entries: the stf runner rejected
  the new post-state 'validators' registry check (deny_unknown_fields). Parse
  it and compare each validator's index and public keys.
diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs
@@ -52,7 +52,17 @@ pub fn update_head(store: &mut Store, log_tree: bool) {
         metrics::observe_fork_choice_reorg_depth(depth);
         info!(%old_head, %new_head, depth, "Fork choice reorg detected");
     }
-    store.update_checkpoints(ForkCheckpoints::head_only(new_head));
+
+    // Keep the finalized checkpoint on the head's chain (leanSpec `update_head`).
+    //
+    // Finalization is *not* a monotonic max over everything ever seen: it is
+    // derived from the chosen head's post-state and pinned to the block at that
+    // slot on the head's own ancestry. When fork choice switches to a fork that
+    // finalized less than a now-losing fork, finalized must follow the head
+    // downward rather than latch the higher value (which would let a losing
+    // fork's finalization persist on a chain that never contained it).
+    let finalized = recompute_finalized(store, new_head);
+    store.update_checkpoints(ForkCheckpoints::new(new_head, None, finalized));
 
     if old_head != new_head {
         let old_slot = store
@@ -88,6 +98,33 @@ pub fn update_head(store: &mut Store, log_tree: bool) {
     }
 }
 
+/// Resolve the finalized checkpoint that sits on `head`'s chain.
+///
+/// Mirrors leanSpec `update_head`: take the finalized slot recorded in the
+/// head's post-state, then climb the head's parent chain to the block at that
+/// slot. Returns `None` (caller keeps the existing checkpoint) when no block is
+/// stored at that slot, which happens only for a checkpoint-sync anchor whose
+/// pre-anchor history is absent.
+fn recompute_finalized(store: &Store, head: H256) -> Option<Checkpoint> {
+    let finalized_slot = store.get_state(&head)?.latest_finalized.slot;
+
+    let mut finalized_root = head;
+    while let Some(header) = store.get_block_header(&finalized_root) {
+        if header.slot <= finalized_slot {
+            break;
+        }
+        finalized_root = header.parent_root;
+    }
+
+    store
+        .get_block_header(&finalized_root)
+        .filter(|header| header.slot == finalized_slot)
+        .map(|_| Checkpoint {
+            root: finalized_root,
+            slot: finalized_slot,
+        })
+}
+
 /// Update the safe target for attestation.
 ///
 /// Safe target is an *availability* signal, not a durable-knowledge signal:
@@ -406,6 +443,12 @@ fn on_gossip_aggregated_attestation_core(
     let num_validators = validators.len() as u64;
 
     let participant_indices: Vec<u64> = aggregated.proof.participant_indices().collect();
+    // An aggregate must name at least one signer. The spec rejects empty
+    // participants (EMPTY_AGGREGATION_BITS) when resolving validator indices,
+    // before any bounds check or signature verification.
+    if participant_indices.is_empty() {
+        return Err(StoreError::EmptyAggregationBits);
+    }
     if participant_indices.iter().any(|&vid| vid >= num_validators) {
         return Err(StoreError::InvalidValidatorIndex);
     }
@@ -547,14 +590,14 @@ fn on_block_core(
     let state_root = block.state_root;
     post_state.latest_block_header.state_root = state_root;
 
-    // Update justified/finalized checkpoints if they have higher slots
+    // Advance the justified checkpoint when the post-state names a higher one
+    // (leanSpec `advance_to`: monotonic by slot). Finalized is intentionally not
+    // latched here; it is recomputed from the head's chain in `update_head`.
     let justified = (post_state.latest_justified.slot > store.latest_justified().slot)
         .then_some(post_state.latest_justified);
-    let finalized = (post_state.latest_finalized.slot > store.latest_finalized().slot)
-        .then_some(post_state.latest_finalized);
 
-    if justified.is_some() || finalized.is_some() {
-        store.update_checkpoints(ForkCheckpoints::new(store.head(), justified, finalized));
+    if let Some(justified) = justified {
+        store.update_checkpoints(ForkCheckpoints::new(store.head(), Some(justified), None));
     }
 
     // Store signed block and state
@@ -866,6 +909,9 @@ pub enum StoreError {
     #[error("Missing target state for block: {0}")]
     MissingTargetState(H256),
 
+    #[error("Aggregated attestation references no participants")]
+    EmptyAggregationBits,
+
     #[error("Validator {validator_index} is not the proposer for slot {slot}")]
     NotProposer { validator_index: u64, slot: u64 },
 
diff --git a/crates/blockchain/state_transition/tests/stf_spectests.rs b/crates/blockchain/state_transition/tests/stf_spectests.rs
@@ -110,6 +110,7 @@ fn compare_post_states(
         justifications_roots,
         justifications_validators,
         validator_count,
+        validators,
         latest_justified_root_label,
         latest_finalized_root_label,
         justifications_roots_labels,
@@ -275,6 +276,39 @@ fn compare_post_states(
             .into());
         }
     }
+    if let Some(validators) = validators {
+        let expected = &validators.data;
+        let post_validators: Vec<_> = post_state.validators.iter().collect();
+        if post_validators.len() != expected.len() {
+            return Err(format!(
+                "validators count mismatch: expected {}, got {}",
+                expected.len(),
+                post_validators.len()
+            )
+            .into());
+        }
+        for (i, expected_validator) in expected.iter().enumerate() {
+            let expected_domain: ethlambda_types::state::Validator =
+                expected_validator.clone().into();
+            let actual = post_validators[i];
+            if actual.index != expected_domain.index
+                || actual.attestation_pubkey != expected_domain.attestation_pubkey
+                || actual.proposal_pubkey != expected_domain.proposal_pubkey
+            {
+                return Err(format!(
+                    "validator[{i}] mismatch: expected index={} att={} prop={}, \
+                     got index={} att={} prop={}",
+                    expected_domain.index,
+                    hex::encode(expected_domain.attestation_pubkey),
+                    hex::encode(expected_domain.proposal_pubkey),
+                    actual.index,
+                    hex::encode(actual.attestation_pubkey),
+                    hex::encode(actual.proposal_pubkey),
+                )
+                .into());
+            }
+        }
+    }
     if let Some(label) = latest_justified_root_label {
         let expected = resolve_label(label, block_registry)?;
         if post_state.latest_justified.root != expected {
diff --git a/crates/blockchain/state_transition/tests/types.rs b/crates/blockchain/state_transition/tests/types.rs
@@ -82,6 +82,10 @@ pub struct PostState {
     #[serde(rename = "validatorCount")]
     pub validator_count: Option<u64>,
 
+    /// Full validator registry check: each entry's index and public keys must
+    /// match the post-state registry exactly.
+    pub validators: Option<Container<Validator>>,
+
     // Label-based root checks: "block_N" labels resolved to hash_tree_root of the Nth block.
     #[serde(rename = "latestJustifiedRootLabel")]
     pub latest_justified_root_label: Option<String>,
