diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 97c06615..12bad732 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -8,7 +8,7 @@ use ethlambda_types::{ Attestation, AttestationData, HashedAttestationData, SignedAggregatedAttestation, SignedAttestation, validator_indices, }, - block::{AggregatedSignatureProof, Block, SignedBlock}, + block::{AggregatedSignatureProof, Block, BlockHeader, SignedBlock}, checkpoint::Checkpoint, primitives::{H256, HashTreeRoot as _}, signature::ValidatorSignature, @@ -116,6 +116,56 @@ fn update_safe_target(store: &mut Store) { store.set_safe_target(safe_target); } +/// Maximum number of parent links [`checkpoint_is_ancestor`] walks before +/// giving up. Bounds the per-attestation work on the gossip path; a vote whose +/// source/target sits more than this many slots below the descendant is +/// conservatively rejected. +const MAX_ANCESTRY_WALK_SLOTS: u64 = 128; + +/// Return whether `ancestor` lies on `descendant`'s parent chain. +/// +/// `descendant_header` is the descendant's already-fetched header, so the walk +/// starts from its parent without re-reading it. Walks parent links down to the +/// ancestor's slot, up to [`MAX_ANCESTRY_WALK_SLOTS`] steps. Empty (skipped) +/// slots on the path are traversed transparently since they carry no block. +/// Conservative: a block missing from the store, or exceeding the walk cap, +/// yields `false`. +fn checkpoint_is_ancestor( + store: &Store, + ancestor: &Checkpoint, + descendant: &Checkpoint, + descendant_header: &BlockHeader, +) -> bool { + if ancestor.slot >= descendant.slot { + // Equal slot: a checkpoint is its own ancestor only if the roots match. + // Greater: the ancestor cannot sit below the descendant in the chain. + return ancestor.slot == descendant.slot && ancestor.root == descendant.root; + } + + // The descendant header is already in hand, so begin the walk at its parent. + let mut current_root = descendant_header.parent_root; + let mut steps: u64 = 0; + while let Some(current_header) = store.get_block_header(¤t_root) { + if current_header.slot == ancestor.slot { + return current_root == ancestor.root; + } + if current_header.slot < ancestor.slot { + return false; + } + steps += 1; + if steps >= MAX_ANCESTRY_WALK_SLOTS { + warn!( + cap = MAX_ANCESTRY_WALK_SLOTS, + "Attestation ancestry walk exceeded cap, rejecting" + ); + return false; + } + current_root = current_header.parent_root; + } + + false +} + /// Validate incoming attestation before processing. /// /// Ensures the vote respects the basic laws of time and topology: @@ -123,7 +173,8 @@ fn update_safe_target(store: &mut Store) { /// 2. A vote cannot span backwards in time (source > target). /// 3. The head must be at least as recent as source and target. /// 4. Checkpoint slots must match the actual block slots. -/// 5. The vote's slot must have started locally (a small disparity margin is allowed). +/// 5. Source, target, and head must lie on one parent chain. +/// 6. The vote's slot must have started locally (a small disparity margin is allowed). fn validate_attestation_data(store: &Store, data: &AttestationData) -> Result<(), StoreError> { let _timing = metrics::time_attestation_validation(); @@ -170,6 +221,18 @@ fn validate_attestation_data(store: &Store, data: &AttestationData) -> Result<() }); } + // Ancestry Check - Source, target, and head must lie on one parent chain. + // + // Fork-choice weight accrues to every ancestor of the attested head. A sibling + // head would steer that weight onto a non-canonical branch, so reject a vote + // whose checkpoints are slot-ordered but not actually on the same chain. + if !checkpoint_is_ancestor(store, &data.source, &data.target, &target_header) { + return Err(StoreError::SourceNotAncestorOfTarget); + } + if !checkpoint_is_ancestor(store, &data.target, &data.head, &head_header) { + return Err(StoreError::TargetNotAncestorOfHead); + } + // Time Check - Honest validators emit votes only after their slot has begun. // Allow a small disparity margin for clock skew between peers. // @@ -792,6 +855,12 @@ pub enum StoreError { block_slot: u64, }, + #[error("Source checkpoint must be ancestor of target")] + SourceNotAncestorOfTarget, + + #[error("Target checkpoint must be ancestor of head")] + TargetNotAncestorOfHead, + #[error( "Attestation slot {attestation_slot} is too far in future (store time: {store_time} intervals)" )] @@ -1162,4 +1231,144 @@ mod tests { "Expected DuplicateAttestationData, got: {result:?}" ); } + + /// Insert a header-only block at `root` with the given slot and parent. + /// + /// Empty body means `get_block_header` resolves it without a body row, which + /// is all `validate_attestation_data` needs for the ancestry walk. + fn insert_test_block(store: &mut Store, root: H256, slot: u64, parent_root: H256) { + let signed_block = SignedBlock { + message: Block { + slot, + proposer_index: 0, + parent_root, + state_root: H256::ZERO, + body: BlockBody::default(), + }, + signature: BlockSignatures { + attestation_signatures: AttestationSignatures::try_from(vec![]).unwrap(), + proposer_signature: blank_xmss_signature(), + }, + }; + store.insert_signed_block(root, signed_block); + } + + fn new_test_store() -> Store { + use ethlambda_storage::backend::InMemoryBackend; + use std::sync::Arc; + let genesis_state = State::from_genesis(1000, vec![]); + let backend = Arc::new(InMemoryBackend::new()); + Store::from_anchor_state(backend, genesis_state) + } + + /// leanSpec #833: a vote whose head sits on a sibling fork of the target + /// must be rejected by gossip validation, even though every slot and + /// availability check passes. + #[test] + fn validate_attestation_rejects_head_on_sibling_fork() { + let mut store = new_test_store(); + let genesis = store.head(); + + let base = H256([1u8; 32]); + let fork_left = H256([2u8; 32]); + let fork_right = H256([3u8; 32]); + insert_test_block(&mut store, base, 1, genesis); + insert_test_block(&mut store, fork_left, 2, base); + insert_test_block(&mut store, fork_right, 3, base); + store.set_time(3 * INTERVALS_PER_SLOT); + + // source=base, target=fork_left, head=fork_right: target and head share a + // parent (base) but neither is an ancestor of the other. + let data = AttestationData { + slot: 3, + source: Checkpoint { + root: base, + slot: 1, + }, + target: Checkpoint { + root: fork_left, + slot: 2, + }, + head: Checkpoint { + root: fork_right, + slot: 3, + }, + }; + + let result = validate_attestation_data(&store, &data); + assert!( + matches!(result, Err(StoreError::TargetNotAncestorOfHead)), + "Expected TargetNotAncestorOfHead, got: {result:?}" + ); + } + + /// leanSpec #833: a vote whose source sits on a sibling fork of the target + /// must be rejected, even though `source.slot < target.slot`. + #[test] + fn validate_attestation_rejects_source_on_sibling_fork() { + let mut store = new_test_store(); + let genesis = store.head(); + + let base = H256([1u8; 32]); + let fork_left = H256([2u8; 32]); + let fork_right = H256([3u8; 32]); + let fork_right_head = H256([4u8; 32]); + insert_test_block(&mut store, base, 1, genesis); + insert_test_block(&mut store, fork_left, 2, base); + insert_test_block(&mut store, fork_right, 3, base); + insert_test_block(&mut store, fork_right_head, 4, fork_right); + store.set_time(4 * INTERVALS_PER_SLOT); + + // source=fork_left (abandoned branch), target=head=fork_right_head: + // source precedes target in slot but lies off the target's chain. + let data = AttestationData { + slot: 4, + source: Checkpoint { + root: fork_left, + slot: 2, + }, + target: Checkpoint { + root: fork_right_head, + slot: 4, + }, + head: Checkpoint { + root: fork_right_head, + slot: 4, + }, + }; + + let result = validate_attestation_data(&store, &data); + assert!( + matches!(result, Err(StoreError::SourceNotAncestorOfTarget)), + "Expected SourceNotAncestorOfTarget, got: {result:?}" + ); + } + + /// A vote whose source, target, and head form one parent chain validates. + #[test] + fn validate_attestation_accepts_proper_ancestor_chain() { + let mut store = new_test_store(); + let genesis = store.head(); + + let b1 = H256([1u8; 32]); + let b2 = H256([2u8; 32]); + insert_test_block(&mut store, b1, 1, genesis); + insert_test_block(&mut store, b2, 2, b1); + store.set_time(2 * INTERVALS_PER_SLOT); + + let data = AttestationData { + slot: 2, + source: Checkpoint { + root: genesis, + slot: 0, + }, + target: Checkpoint { root: b1, slot: 1 }, + head: Checkpoint { root: b2, slot: 2 }, + }; + + assert!( + validate_attestation_data(&store, &data).is_ok(), + "fully canonical vote must validate" + ); + } } diff --git a/crates/blockchain/state_transition/src/lib.rs b/crates/blockchain/state_transition/src/lib.rs index c6e8de87..ff43a27f 100644 --- a/crates/blockchain/state_transition/src/lib.rs +++ b/crates/blockchain/state_transition/src/lib.rs @@ -483,28 +483,39 @@ fn serialize_justifications( state.justifications_validators = justifications_validators; } -/// Whether both source and target checkpoints in `data` match the chain at -/// their slots. +/// Whether the source, target, and head 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. +/// +/// Checking `head` against the canonical view keeps counted votes internally +/// consistent: justification is keyed on `target.root`, so a sibling head must +/// not pair a canonical FFG vote with an LMD head on a non-canonical fork. pub fn attestation_data_matches_chain( historical_block_hashes: &[H256], data: &AttestationData, ) -> bool { - if data.source.root == H256::ZERO || data.target.root == H256::ZERO { + if data.source.root == H256::ZERO + || data.target.root == H256::ZERO + || data.head.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() + let head_slot = data.head.slot as usize; + if source_slot >= historical_block_hashes.len() + || target_slot >= historical_block_hashes.len() + || head_slot >= historical_block_hashes.len() { return false; } historical_block_hashes[source_slot] == data.source.root && historical_block_hashes[target_slot] == data.target.root + && historical_block_hashes[head_slot] == data.head.root } /// Checks if the slot is a valid candidate for justification after a given finalized slot. @@ -606,6 +617,70 @@ mod tests { } } + /// leanSpec #833: `attestation_data_matches_chain` must reject a vote whose + /// head sits off the canonical chain, even when source and target are both + /// canonical. Justification is keyed on `target.root`, so accepting a + /// canonical FFG vote paired with a sibling LMD head would decouple the two. + #[test] + fn matches_chain_rejects_off_canonical_head() { + let r1 = H256([1u8; 32]); + let r2 = H256([2u8; 32]); + let sibling = H256([0x99u8; 32]); + + // Canonical chain: slot 0 genesis (ZERO is fine for source here only if + // non-zero; use explicit roots), slot 1 -> r1, slot 2 -> r2. + let g = H256([7u8; 32]); + let hashes: Vec = vec![g, r1, r2]; + + // source=(0,g), target=(1,r1) both canonical; head=(2, sibling) is not. + let off_head = AttestationData { + slot: 2, + source: Checkpoint { slot: 0, root: g }, + target: Checkpoint { slot: 1, root: r1 }, + head: Checkpoint { + slot: 2, + root: sibling, + }, + }; + assert!( + !attestation_data_matches_chain(&hashes, &off_head), + "off-canonical head must be rejected" + ); + + // Control: the same vote with the canonical head=(2, r2) is accepted. + let canonical_head = AttestationData { + head: Checkpoint { slot: 2, root: r2 }, + ..off_head + }; + assert!( + attestation_data_matches_chain(&hashes, &canonical_head), + "fully canonical vote must be accepted" + ); + + // A zero-hash head is also rejected up front. + let zero_head = AttestationData { + head: Checkpoint { + slot: 2, + root: H256::ZERO, + }, + ..off_head + }; + assert!( + !attestation_data_matches_chain(&hashes, &zero_head), + "zero-hash head must be rejected" + ); + + // A head slot beyond the chain view is rejected. + let out_of_range_head = AttestationData { + head: Checkpoint { slot: 99, root: r2 }, + ..off_head + }; + assert!( + !attestation_data_matches_chain(&hashes, &out_of_range_head), + "out-of-range head slot must be rejected" + ); + } + /// Regression: `process_attestations` must not let `state.latest_justified` /// regress within a single block when attestations appear in body order /// whose target slots are not monotonically increasing.