diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 3b6291dd..35b8dce9 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -2,7 +2,8 @@ use std::collections::{HashMap, HashSet}; use ethlambda_crypto::aggregate_proofs; use ethlambda_state_transition::{ - is_proposer, process_block, process_slots, slot_is_justifiable_after, + attestation_data_matches_chain, is_proposer, justified_slots_ops, process_block, process_slots, + slot_is_justifiable_after, }; use ethlambda_storage::{ForkCheckpoints, Store}; use ethlambda_types::{ @@ -1050,17 +1051,20 @@ fn build_block( let mut selected: Vec<(AggregatedAttestation, AggregatedSignatureProof)> = Vec::new(); if !aggregated_payloads.is_empty() { - // Genesis edge case: when building on genesis (slot 0), - // process_block_header will set latest_justified.root = parent_root. - // Derive this upfront so attestation filtering matches. - let mut current_justified = if head_state.latest_block_header.slot == 0 { - Checkpoint { - root: parent_root, - slot: head_state.latest_justified.slot, - } - } else { - head_state.latest_justified - }; + let mut current_justified = head_state.latest_justified; + let mut current_finalized_slot = head_state.latest_finalized.slot; + let mut current_justified_slots = head_state.justified_slots.clone(); + + // Chain view that `process_block_header` would produce on the candidate + // block: covering [0, slot - 1] with parent_root at parent.slot and + // ZERO_HASH for empty slots in between. Lets us validate source/target + // roots without waiting for the STF to drop mismatches. + let parent_slot = head_state.latest_block_header.slot; + let num_empty_slots = slot.saturating_sub(parent_slot).saturating_sub(1) as usize; + let mut extended_historical_block_hashes: Vec = + head_state.historical_block_hashes.iter().copied().collect(); + extended_historical_block_hashes.push(parent_root); + extended_historical_block_hashes.extend(std::iter::repeat_n(H256::ZERO, num_empty_slots)); let mut processed_data_roots: HashSet = HashSet::new(); @@ -1082,7 +1086,30 @@ fn build_block( if !known_block_roots.contains(&att_data.head.root) { continue; } - if att_data.source != current_justified { + if !justified_slots_ops::is_slot_justified( + ¤t_justified_slots, + current_finalized_slot, + att_data.source.slot, + ) { + continue; + } + + if !attestation_data_matches_chain(&extended_historical_block_hashes, att_data) { + continue; + } + + // Skip attestations whose target slot is already justified on + // this chain (they wouldn't change post-state). Allow the + // genesis self-vote (source=target=0) for fork-choice + // bootstrapping. + let is_genesis_self_vote = att_data.source.slot == 0 && att_data.target.slot == 0; + if !is_genesis_self_vote + && justified_slots_ops::is_slot_justified( + ¤t_justified_slots, + current_finalized_slot, + att_data.target.slot, + ) + { continue; } @@ -1096,7 +1123,7 @@ fn build_block( break; } - // Check if justification advanced + // Check if justification or finalization advanced let attestations: AggregatedAttestations = selected .iter() .map(|(att, _)| att.clone()) @@ -1114,8 +1141,12 @@ fn build_block( process_slots(&mut post_state, slot)?; process_block(&mut post_state, &candidate)?; - if post_state.latest_justified != current_justified { + if post_state.latest_justified != current_justified + || post_state.latest_finalized.slot != current_finalized_slot + { current_justified = post_state.latest_justified; + current_justified_slots = post_state.justified_slots.clone(); + current_finalized_slot = post_state.latest_finalized.slot; // Continue: new checkpoint may unlock more attestation data } else { break; @@ -1396,6 +1427,10 @@ mod tests { /// at MAX_ATTESTATIONS_DATA (16) and stays under the gossip size limit. #[test] fn build_block_caps_attestation_data_entries() { + use ethlambda_types::{ + block::BlockHeader, + state::{ChainConfig, JustificationValidators, JustifiedSlots}, + }; use libssz::SszEncode; use libssz_types::SszList; @@ -1404,7 +1439,9 @@ mod tests { const NUM_VALIDATORS: usize = 50; const NUM_PAYLOAD_ENTRIES: usize = 50; - // Create genesis state with NUM_VALIDATORS validators. + const HEAD_SLOT: u64 = 51; + const TARGET_SLOT: u64 = 5; + let validators: Vec<_> = (0..NUM_VALIDATORS) .map(|i| ethlambda_types::state::Validator { attestation_pubkey: [i as u8; 52], @@ -1412,47 +1449,75 @@ mod tests { index: i as u64, }) .collect(); - let head_state = State::from_genesis(1000, validators); - // process_slots fills in the genesis header's state_root before + // Build a head state at slot HEAD_SLOT with valid historical_block_hashes + // so attestations referencing in-range slots match the chain (the + // chain-match check in build_block now rejects mismatches). + let hashes: Vec = (0..HEAD_SLOT).map(|i| H256([(i + 1) as u8; 32])).collect(); + let historical_block_hashes = SszList::try_from(hashes.clone()).unwrap(); + + let head_header = BlockHeader { + slot: HEAD_SLOT, + proposer_index: 0, + parent_root: H256::ZERO, + state_root: H256::ZERO, + body_root: BlockBody::default().hash_tree_root(), + }; + + let head_state = State { + config: ChainConfig { genesis_time: 1000 }, + slot: HEAD_SLOT, + latest_block_header: head_header, + latest_justified: Checkpoint::default(), + latest_finalized: Checkpoint::default(), + historical_block_hashes, + justified_slots: JustifiedSlots::new(), + validators: SszList::try_from(validators).unwrap(), + justifications_roots: Default::default(), + justifications_validators: JustificationValidators::new(), + }; + + // process_slots fills in the parent header's state_root before // process_block_header computes the parent hash. Simulate that here. let mut header_for_root = head_state.latest_block_header.clone(); header_for_root.state_root = head_state.hash_tree_root(); let parent_root = header_for_root.hash_tree_root(); - // Proposer for slot 1 with NUM_VALIDATORS validators: 1 % 50 = 1 - let proposer_index = 1u64; - let slot = 1u64; + let slot = HEAD_SLOT + 1; + let proposer_index = slot % NUM_VALIDATORS as u64; - // The genesis edge case in build_block sets current_justified to: - // Checkpoint { root: parent_root, slot: 0 } + // Common source / target / head referencing valid chain entries so the + // chain-match check passes for every payload. We vary AttestationData.slot + // alone to produce 50 distinct data_roots. let source = Checkpoint { - root: parent_root, + root: hashes[0], + slot: 0, + }; + let target = Checkpoint { + root: hashes[TARGET_SLOT as usize], + slot: TARGET_SLOT, + }; + let head = Checkpoint { + root: hashes[0], slot: 0, }; let mut known_block_roots = HashSet::new(); known_block_roots.insert(parent_root); + known_block_roots.insert(hashes[0]); // Simulate a stall: populate the payload pool with many distinct entries. - // Each has a unique target (different slot) and a large proof payload. + // Each has a unique attestation slot and a large proof payload. let mut aggregated_payloads: HashMap< H256, (AttestationData, Vec), > = HashMap::new(); for i in 0..NUM_PAYLOAD_ENTRIES { - let target_slot = (i + 1) as u64; let att_data = AttestationData { - slot: target_slot, - head: Checkpoint { - root: parent_root, - slot: 0, - }, - target: Checkpoint { - root: H256([target_slot as u8; 32]), - slot: target_slot, - }, + slot: (i + 1) as u64, + head, + target, source, }; @@ -1513,6 +1578,134 @@ mod tests { ); } + /// Regression test for leanSpec PR #716: build_block must absorb + /// gap-closing attestations whose source is justified on the head + /// chain but older than `latest_justified` (e.g., a sibling fork + /// advanced the store's justified past what the canonical head has + /// proven). Without the relaxed `is_slot_justified(source.slot)` + /// filter, the exact-equality check would drop the attestation and + /// justification would never converge on this chain. + #[test] + fn build_block_absorbs_older_but_justified_source() { + use ethlambda_state_transition::justified_slots_ops; + use ethlambda_types::{ + block::BlockHeader, + state::{ChainConfig, JustificationValidators, JustifiedSlots}, + }; + use libssz_types::SszList; + + const NUM_VALIDATORS: usize = 50; + const SUPERMAJORITY: usize = 34; // ceil(2 * 50 / 3) + const HEAD_SLOT: u64 = 5; + const JUSTIFIED_SLOT: u64 = 1; + const GAP_TARGET_SLOT: u64 = 2; + + let validators: Vec<_> = (0..NUM_VALIDATORS) + .map(|i| ethlambda_types::state::Validator { + attestation_pubkey: [i as u8; 52], + proposal_pubkey: [i as u8; 52], + index: i as u64, + }) + .collect(); + + let hashes: Vec = (0..HEAD_SLOT).map(|i| H256([(i + 1) as u8; 32])).collect(); + + let mut justified_slots = JustifiedSlots::new(); + justified_slots_ops::extend_to_slot(&mut justified_slots, 0, JUSTIFIED_SLOT); + justified_slots_ops::set_justified(&mut justified_slots, 0, JUSTIFIED_SLOT); + + let head_header = BlockHeader { + slot: HEAD_SLOT, + proposer_index: 0, + parent_root: H256::ZERO, + state_root: H256::ZERO, + body_root: BlockBody::default().hash_tree_root(), + }; + + let head_state = State { + config: ChainConfig { genesis_time: 1000 }, + slot: HEAD_SLOT, + latest_block_header: head_header, + latest_justified: Checkpoint { + root: hashes[JUSTIFIED_SLOT as usize], + slot: JUSTIFIED_SLOT, + }, + latest_finalized: Checkpoint::default(), + historical_block_hashes: SszList::try_from(hashes.clone()).unwrap(), + justified_slots, + validators: SszList::try_from(validators).unwrap(), + justifications_roots: Default::default(), + justifications_validators: JustificationValidators::new(), + }; + + let mut header_for_root = head_state.latest_block_header.clone(); + header_for_root.state_root = head_state.hash_tree_root(); + let parent_root = header_for_root.hash_tree_root(); + + let slot = HEAD_SLOT + 1; + let proposer_index = slot % NUM_VALIDATORS as u64; + + // source = genesis (slot 0): older than head.latest_justified at + // slot 1. Pre-PR exact-equality filter would drop this; post-PR + // it's absorbed and the candidate justifies GAP_TARGET_SLOT. + let att_data = AttestationData { + slot, + head: Checkpoint { + root: hashes[0], + slot: 0, + }, + target: Checkpoint { + root: hashes[GAP_TARGET_SLOT as usize], + slot: GAP_TARGET_SLOT, + }, + source: Checkpoint { + root: hashes[0], + slot: 0, + }, + }; + let data_root = att_data.hash_tree_root(); + + let mut bits = AggregationBits::with_length(NUM_VALIDATORS).unwrap(); + for i in 0..SUPERMAJORITY { + bits.set(i, true).unwrap(); + } + let proof = AggregatedSignatureProof::new(bits, SszList::try_from(vec![0xAB; 64]).unwrap()); + + let mut aggregated_payloads = HashMap::new(); + aggregated_payloads.insert(data_root, (att_data.clone(), vec![proof])); + + let mut known_block_roots = HashSet::new(); + known_block_roots.insert(parent_root); + known_block_roots.insert(hashes[0]); + + let (block, _signatures, post_checkpoints) = build_block( + &head_state, + slot, + proposer_index, + parent_root, + &known_block_roots, + &aggregated_payloads, + ) + .expect("build_block should succeed"); + + let targets: Vec<_> = block + .body + .attestations + .iter() + .map(|att| att.data.target) + .collect(); + assert!( + targets.contains(&att_data.target), + "produced block missing gap-closing attestation: {targets:?}" + ); + + assert_eq!(post_checkpoints.justified.slot, GAP_TARGET_SLOT); + assert_eq!( + post_checkpoints.justified.root, + hashes[GAP_TARGET_SLOT as usize] + ); + } + fn make_att_data(slot: u64) -> AttestationData { AttestationData { slot, diff --git a/crates/blockchain/state_transition/src/lib.rs b/crates/blockchain/state_transition/src/lib.rs index df87f7a1..2435adc4 100644 --- a/crates/blockchain/state_transition/src/lib.rs +++ b/crates/blockchain/state_transition/src/lib.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use ethlambda_types::{ ShortRoot, + attestation::AttestationData, block::{AggregatedAttestations, Block, BlockHeader}, checkpoint::Checkpoint, primitives::{H256, HashTreeRoot as _}, @@ -9,7 +10,7 @@ use ethlambda_types::{ }; use tracing::{info, warn}; -mod justified_slots_ops; +pub mod justified_slots_ops; pub mod metrics; #[derive(Debug, thiserror::Error)] @@ -271,7 +272,7 @@ fn process_attestations( let source = attestation_data.source; let target = attestation_data.target; - if !is_valid_vote(state, source, target) { + if !is_valid_vote(state, attestation_data) { continue; } @@ -337,11 +338,14 @@ fn process_attestations( /// Checks (all must pass): /// 1. Source is already justified /// 2. Target is not yet justified -/// 3. Neither root is zero hash -/// 4. Both checkpoints exist in historical_block_hashes -/// 5. Target slot > source slot -/// 6. Target slot is justifiable after the finalized slot -fn is_valid_vote(state: &State, source: Checkpoint, target: Checkpoint) -> bool { +/// 3. Both checkpoints match the canonical chain at their slots (which also +/// rejects zero-hash source or target roots) +/// 4. Target slot > source slot +/// 5. Target slot is justifiable after the finalized slot +fn is_valid_vote(state: &State, data: &AttestationData) -> bool { + let source = data.source; + let target = data.target; + // Check that the source is already justified if !justified_slots_ops::is_slot_justified( &state.justified_slots, @@ -361,13 +365,9 @@ fn is_valid_vote(state: &State, source: Checkpoint, target: Checkpoint) -> bool return false; } - // Ignore votes that reference zero-hash slots. - if source.root == H256::ZERO || target.root == H256::ZERO { - return false; - } - - // Ensure the vote refers to blocks that actually exist on our chain - if !checkpoint_exists(state, source) || !checkpoint_exists(state, target) { + // Ensure the vote refers to blocks that actually exist on our chain; + // also rejects zero-hash source or target inline. + if !attestation_data_matches_chain(&state.historical_block_hashes, data) { return false; } @@ -473,12 +473,28 @@ fn serialize_justifications( state.justifications_validators = justifications_validators; } -fn checkpoint_exists(state: &State, checkpoint: Checkpoint) -> bool { - state - .historical_block_hashes - .get(checkpoint.slot as usize) - .map(|root| root == &checkpoint.root) - .unwrap_or(false) +/// Whether both source and target checkpoints in `data` match the chain at +/// their slots. +/// +/// Callers pass a chain view as it would appear after `process_block_header` +/// on the consuming block: covering `[0, block.slot - 1]` with `parent_root` +/// at the parent slot and `H256::ZERO` for empty slots between parent and the +/// candidate. +pub fn attestation_data_matches_chain( + historical_block_hashes: &[H256], + data: &AttestationData, +) -> bool { + if data.source.root == H256::ZERO || data.target.root == H256::ZERO { + return false; + } + let source_slot = data.source.slot as usize; + let target_slot = data.target.slot as usize; + if source_slot >= historical_block_hashes.len() || target_slot >= historical_block_hashes.len() + { + return false; + } + historical_block_hashes[source_slot] == data.source.root + && historical_block_hashes[target_slot] == data.target.root } /// Checks if the slot is a valid candidate for justification after a given finalized slot.