diff --git a/crates/blockchain/src/lib.rs b/crates/blockchain/src/lib.rs index e4a63b82..c7d9b609 100644 --- a/crates/blockchain/src/lib.rs +++ b/crates/blockchain/src/lib.rs @@ -192,6 +192,7 @@ impl BlockChain { pending_blocks: HashMap::new(), aggregator, pending_block_parents: HashMap::new(), + invalid_blocks: HashMap::new(), current_aggregation: None, last_tick_instant: None, attestation_committee_count, @@ -274,6 +275,16 @@ pub struct BlockChainServer { // a deeper missing parent after the entry was created. pending_block_parents: HashMap, + /// Roots of blocks that failed the state transition, with their slots. + /// + /// A child of one of these can never import, so it is rejected instead of + /// stored as pending. Only state transition failures are recorded: they + /// depend on the block message alone, which the root commits to, and the + /// block already passed signature verification, so only a misbehaving + /// validator can add an entry. Entries at or below the finalized slot are + /// pruned, since blocks there are rejected by slot anyway. + invalid_blocks: HashMap, + /// Whether this node acts as a committee aggregator. /// /// Read fresh on every tick and gossip event so runtime toggles via the @@ -969,6 +980,13 @@ impl BlockChainServer { self.store .prune_old_data() .expect("DB pruning should succeed"); + + let finalized_slot = self + .store + .latest_finalized() + .expect("latest finalized checkpoint exists") + .slot; + self.prune_invalid_blocks(finalized_slot); } /// Try to process a single block. If its parent state is missing, store it @@ -1025,6 +1043,34 @@ impl BlockChainServer { .has_state(&parent_root) .expect("DB read should succeed") { + // Pending blocks are persisted and served to peers, so check what + // can be checked without the parent state before storing. A + // rejected block can never import, and neither can any children + // that arrived ahead of it. + if self.invalid_blocks.contains_key(&parent_root) { + warn!( + %slot, + proposer, + block_root = %ShortRoot(&block_root.0), + parent_root = %ShortRoot(&parent_root.0), + "Rejecting block: parent failed the state transition" + ); + self.discard_pending_subtree(block_root); + return; + } + if let Err(err) = store::validate_pending_block(&self.store, &signed_block) { + warn!( + %slot, + proposer, + block_root = %ShortRoot(&block_root.0), + parent_root = %ShortRoot(&parent_root.0), + %err, + "Rejecting block with missing parent" + ); + self.discard_pending_subtree(block_root); + return; + } + info!(%slot, %parent_root, %block_root, "Block parent missing, storing as pending"); // Resolve the actual missing ancestor by walking the chain. A stale entry @@ -1123,6 +1169,7 @@ impl BlockChainServer { %err, "Failed to process block" ); + self.on_import_failure(block_root, slot, &err); } } } @@ -1195,11 +1242,43 @@ impl BlockChainServer { } } + /// React to a block whose import failed with `err`. + /// + /// Only a state transition failure condemns the root: it depends on the + /// block message alone, which the root commits to. The block is recorded + /// as invalid, and it and any children waiting on it are discarded along + /// with their rows. + /// + /// Every other failure is left alone. Some depend on the proof, which the + /// root does not cover, so a bad copy of a real block would otherwise + /// condemn the real one. Others depend on time, and may pass later. + fn on_import_failure(&mut self, block_root: H256, slot: u64, err: &StoreError) { + if !matches!(err, StoreError::StateTransitionFailed(_)) { + return; + } + self.invalid_blocks.insert(block_root, slot); + self.discard_pending_subtree(block_root); + } + + /// Forget invalid blocks at or below `finalized_slot`. Blocks there are + /// rejected by slot before the invalid set is consulted. + fn prune_invalid_blocks(&mut self, finalized_slot: u64) { + self.invalid_blocks.retain(|_, slot| *slot > finalized_slot); + } + /// Recursively discard a block and all its pending descendants. /// /// Used when a block is rejected (e.g., at/below finalized slot) to clean up /// children that would otherwise remain stuck in the pending maps indefinitely. + /// + /// Each discarded block's stored rows are deleted too, so peers stop being + /// served blocks we will never import. `delete_pending_block` leaves any + /// block with a state alone, which matters here: the root can be an + /// already-imported block at or below the finalized slot. fn discard_pending_subtree(&mut self, block_root: H256) { + self.store + .delete_pending_block(&block_root) + .expect("DB delete should succeed"); let Some(child_roots) = self.pending_blocks.remove(&block_root) else { return; }; @@ -1622,4 +1701,317 @@ mod tests { Duration::from_millis(1_600) ); } + + // ============ Pending Block Tests ============ + + use ethlambda_storage::backend::InMemoryBackend; + use ethlambda_types::{ + block::{Block, BlockBody, MultiMessageAggregate}, + state::State, + }; + use std::sync::Arc; + + /// Actor with no validators, no P2P handle, and default policies around + /// `store`. Enough to drive block import and the pending-block maps. + fn test_server(store: Store) -> BlockChainServer { + BlockChainServer { + store, + p2p: None, + key_manager: key_manager::KeyManager::new(HashMap::new()), + pending_blocks: HashMap::new(), + pending_block_parents: HashMap::new(), + invalid_blocks: HashMap::new(), + aggregator: AggregatorController::new(false), + current_aggregation: None, + last_tick_instant: None, + attestation_committee_count: 1, + subscribed_subnets: HashSet::new(), + aggregation_duty_subnet: 0, + skip_redundant_aggregation: false, + proposer_config: ProposerConfig { + enable_proposer_aggregation: false, + max_attestations_per_block: MAX_ATTESTATIONS_DATA, + }, + pre_merge_coverage: None, + sync_status: SyncStatusTracker::new(false), + sync_status_controller: SyncStatusController::default(), + events: EventBus::default(), + } + } + + /// Registry size for the pending-block tests: the proposer for slot `s` is + /// `s % PENDING_VALIDATORS`. + const PENDING_VALIDATORS: u64 = 4; + + /// A registry of `count` validators with placeholder keys, enough for the + /// proposer and index checks, which never decode a key. + fn make_validators(count: u64) -> Vec { + (0..count) + .map(|index| ethlambda_types::state::Validator { + attestation_pubkey: ethlambda_types::state::ValidatorPubkeyBytes::default(), + proposal_pubkey: ethlambda_types::state::ValidatorPubkeyBytes::default(), + index, + }) + .collect() + } + + /// Store anchored at a genesis of `PENDING_VALIDATORS` validators, with + /// the clock far enough ahead that the test blocks' slots have started. + fn pending_test_store() -> Store { + let backend = Arc::new(InMemoryBackend::new()); + let validators = make_validators(PENDING_VALIDATORS); + let genesis_state = State::from_genesis(GENESIS_TIME, validators); + let mut store = + Store::from_anchor_state(backend, genesis_state, DEFAULT_MILLISECONDS_PER_SLOT); + store + .set_time(100 * INTERVALS_PER_SLOT) + .expect("set store time"); + store + } + + /// An empty-bodied block from the slot's proposer. + fn empty_block(slot: u64, parent_root: H256) -> SignedBlock { + SignedBlock { + message: Block { + slot, + proposer_index: slot % PENDING_VALIDATORS, + parent_root, + state_root: H256::ZERO, + body: BlockBody::default(), + }, + proof: MultiMessageAggregate::default(), + } + } + + /// Record `block` as pending the way the pending path does, with + /// `missing_root` as its deepest missing ancestor, and return its root. + /// + /// Bypasses validation on purpose: a block can only pass the signature + /// check with a real proof from the leanVM prover, so tests that need a + /// block already pending set the state up directly. + fn insert_pending( + server: &mut BlockChainServer, + block: SignedBlock, + missing_root: H256, + ) -> H256 { + let root = block.message.hash_tree_root(); + let parent_root = block.message.parent_root; + server.store.insert_pending_block(root, block).unwrap(); + server + .pending_blocks + .entry(parent_root) + .or_default() + .insert(root); + server.pending_block_parents.insert(root, missing_root); + root + } + + /// Pend `a(5) <- b(6)` under a parent that is never stored, returning + /// `(missing_root, root_a, root_b)`. + fn pend_two_block_chain(server: &mut BlockChainServer) -> (H256, H256, H256) { + let missing_root = H256([0xAB; 32]); + let root_a = insert_pending(server, empty_block(5, missing_root), missing_root); + let root_b = insert_pending(server, empty_block(6, root_a), missing_root); + (missing_root, root_a, root_b) + } + + /// Discarding a pending subtree drops its blocks from disk as well as from + /// the in-memory maps, so they stop being served over BlocksByRoot. + #[test] + fn discard_pending_subtree_deletes_descendant_rows() { + let mut server = test_server(pending_test_store()); + let (missing_root, root_a, root_b) = pend_two_block_chain(&mut server); + + server.discard_pending_subtree(missing_root); + + assert!(server.store.get_block_header(&root_a).unwrap().is_none()); + assert!(server.store.get_block_header(&root_b).unwrap().is_none()); + assert!(server.pending_blocks.is_empty()); + assert!(server.pending_block_parents.is_empty()); + } + + /// A pending block whose slot finalization has passed is discarded when + /// its parent finally lands. Its own rows go too, not only its children's. + #[test] + fn discard_pending_subtree_deletes_pending_root_rows() { + let mut server = test_server(pending_test_store()); + let (_missing_root, root_a, root_b) = pend_two_block_chain(&mut server); + + server.discard_pending_subtree(root_a); + + assert!(server.store.get_block_header(&root_a).unwrap().is_none()); + assert!(server.store.get_block_header(&root_b).unwrap().is_none()); + } + + /// An orphan that fails pending validation is neither written to disk nor + /// tracked in the pending maps, and no parent fetch is started for it. + #[test] + fn invalid_orphan_is_neither_stored_nor_pended() { + let mut server = test_server(pending_test_store()); + let mut block = empty_block(5, H256([0xAB; 32])); + // Slot 5's proposer is 1; validator 2 is out of turn. + block.message.proposer_index = 2; + let block_root = block.message.hash_tree_root(); + + server.on_block(block); + + assert!( + server + .store + .get_block_header(&block_root) + .unwrap() + .is_none() + ); + assert!(server.pending_blocks.is_empty()); + assert!(server.pending_block_parents.is_empty()); + } + + /// Children can arrive before their parent. When the parent turns out to + /// be invalid, the children already waiting on it can never import, so + /// they are discarded along with their rows. + #[test] + fn invalid_orphan_discards_its_waiting_children() { + let mut server = test_server(pending_test_store()); + let mut parent = empty_block(5, H256([0xAB; 32])); + parent.message.proposer_index = 2; + let parent_root = parent.message.hash_tree_root(); + let child_root = insert_pending(&mut server, empty_block(6, parent_root), parent_root); + + server.on_block(parent); + + assert!( + server + .store + .get_block_header(&parent_root) + .unwrap() + .is_none() + ); + assert!( + server + .store + .get_block_header(&child_root) + .unwrap() + .is_none() + ); + assert!(server.pending_blocks.is_empty()); + assert!(server.pending_block_parents.is_empty()); + } + + /// Pend `child(6)` under `parent_root`, which is never stored, and return + /// the child's root. + fn pend_child_of(server: &mut BlockChainServer, parent_root: H256) -> H256 { + insert_pending(server, empty_block(6, parent_root), parent_root) + } + + /// A state transition failure that depends on the message alone. + fn state_transition_failure() -> StoreError { + StoreError::StateTransitionFailed(ethlambda_state_transition::Error::StateRootMismatch { + expected: H256::ZERO, + computed: H256([1; 32]), + }) + } + + /// A block whose parent already failed the state transition can never + /// import, so it is neither stored nor pended. + #[test] + fn child_of_invalid_block_is_rejected() { + let mut server = test_server(pending_test_store()); + let invalid_root = H256([0xCD; 32]); + server.invalid_blocks.insert(invalid_root, 5); + let child = empty_block(6, invalid_root); + let child_root = child.message.hash_tree_root(); + + server.on_block(child); + + assert!( + server + .store + .get_block_header(&child_root) + .unwrap() + .is_none() + ); + assert!(server.pending_blocks.is_empty()); + assert!(server.pending_block_parents.is_empty()); + } + + /// A state transition failure is bound to the root, so the block is + /// remembered as invalid and the children waiting on it are discarded. + #[test] + fn state_transition_failure_marks_block_invalid_and_discards_children() { + let mut server = test_server(pending_test_store()); + let failed_root = H256([0xCD; 32]); + let child_root = pend_child_of(&mut server, failed_root); + + server.on_import_failure(failed_root, 5, &state_transition_failure()); + + assert_eq!(server.invalid_blocks.get(&failed_root), Some(&5)); + assert!( + server + .store + .get_block_header(&child_root) + .unwrap() + .is_none() + ); + assert!(server.pending_blocks.is_empty()); + assert!(server.pending_block_parents.is_empty()); + } + + /// The root does not cover the proof, so a copy of a real block with a + /// bad proof fails verification under the real block's root. That + /// failure must not mark the root invalid or drop its children, or anyone + /// could kill a real block by racing a bad copy of it. + #[test] + fn signature_failure_neither_marks_invalid_nor_discards_children() { + let mut server = test_server(pending_test_store()); + let failed_root = H256([0xCD; 32]); + let child_root = pend_child_of(&mut server, failed_root); + + server.on_import_failure(failed_root, 5, &StoreError::SignatureVerificationFailed); + + assert!(server.invalid_blocks.is_empty()); + assert!( + server + .store + .get_block_header(&child_root) + .unwrap() + .is_some() + ); + assert_eq!( + server.pending_block_parents.get(&child_root), + Some(&failed_root) + ); + } + + /// Entries at or below the finalized slot are dropped; later ones stay. + #[test] + fn invalid_blocks_are_pruned_at_finalization() { + let mut server = test_server(pending_test_store()); + let at_finalized = H256([1; 32]); + let above_finalized = H256([2; 32]); + server.invalid_blocks.insert(at_finalized, 5); + server.invalid_blocks.insert(above_finalized, 6); + + server.prune_invalid_blocks(5); + + assert!(!server.invalid_blocks.contains_key(&at_finalized)); + assert!(server.invalid_blocks.contains_key(&above_finalized)); + } + + /// The subtree root can be an imported block, such as one at or below the + /// finalized slot that arrives again. Its rows must survive the discard. + #[test] + fn discard_pending_subtree_keeps_imported_root() { + let mut server = test_server(pending_test_store()); + let anchor_root = server.store.head().expect("head root"); + + server.discard_pending_subtree(anchor_root); + + assert!( + server + .store + .get_block_header(&anchor_root) + .unwrap() + .is_some() + ); + } } diff --git a/crates/blockchain/src/spec_test_runner.rs b/crates/blockchain/src/spec_test_runner.rs index cc2bebc2..6d4e6d82 100644 --- a/crates/blockchain/src/spec_test_runner.rs +++ b/crates/blockchain/src/spec_test_runner.rs @@ -95,6 +95,12 @@ pub fn rejection_reason(err: &StoreError) -> Option { | StoreError::SignatureAggregationFailed(_) | StoreError::MissingTargetState(_) | StoreError::SlotOutOfRange(_) => return None, + + // Raised only before a parentless block is stored as pending, a step + // the spec does not have, so fixtures never reach them. + StoreError::InvalidParentRoot { .. } + | StoreError::ParentSlotNotBefore { .. } + | StoreError::ParentConflictsWithFinalized { .. } => return None, }; Some(reason) } diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 011d33ee..35714930 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -680,24 +680,7 @@ fn on_block_core( }); } - // Each unique AttestationData must appear at most once per block. - let attestations = &signed_block.message.body.attestations; - let mut seen = HashSet::with_capacity(attestations.len()); - for att in attestations { - if !seen.insert(&att.data) { - return Err(StoreError::DuplicateAttestationData { - count: attestations.len(), - unique: seen.len(), - }); - } - } - // Reject blocks exceeding the per-block distinct-attestation-data cap (leanSpec #536). - if seen.len() > MAX_ATTESTATIONS_DATA { - return Err(StoreError::TooManyAttestationData { - count: seen.len(), - max: MAX_ATTESTATIONS_DATA, - }); - } + validate_block_attestations(&signed_block.message)?; let sig_verification_start = std::time::Instant::now(); if verify { @@ -1166,6 +1149,157 @@ pub enum StoreError { #[error("Block slot {block_slot} is beyond the future horizon (current slot: {current_slot})")] BlockTooFarInFuture { block_slot: u64, current_slot: u64 }, + + /// A block names the zero root or its own root as parent, so its parent + /// can never be fetched. + #[error("Block parent root {parent_root} is the zero root or the block's own root")] + InvalidParentRoot { parent_root: H256 }, + + /// A block's slot does not come after its stored parent's slot, so the + /// header check in the state transition would reject it. + #[error("Block slot {block_slot} is not after its parent's slot {parent_slot}")] + ParentSlotNotBefore { block_slot: u64, parent_slot: u64 }, + + /// A block's parent sits at or below the finalized slot but is not the + /// finalized block, so it is on a fork finality already discarded. + #[error( + "Parent {parent_root} at slot {parent_slot} conflicts with finalized block {finalized_root} at slot {finalized_slot}" + )] + ParentConflictsWithFinalized { + parent_root: H256, + parent_slot: u64, + finalized_root: H256, + finalized_slot: u64, + }, +} + +/// Check a block whose parent state is missing before it is stored as pending. +/// +/// Pending blocks are persisted and served to peers over BlocksByRoot, so a +/// block that can never import is rejected here instead. Only checks that +/// need no parent state can run, cheapest first. +/// +/// The validator registry comes from the head state rather than the +/// finalized one. The registry is fixed at genesis, so both give the same +/// answer, and the head state is almost always in the state cache. +pub fn validate_pending_block(store: &Store, signed_block: &SignedBlock) -> Result<(), StoreError> { + validate_pending_block_core(store, signed_block, true) +} + +/// [`validate_pending_block`], with the signature check skipped when `verify` +/// is false. Mirrors [`on_block_core`]: only tests skip verification, since a +/// real proof needs the leanVM prover. +fn validate_pending_block_core( + store: &Store, + signed_block: &SignedBlock, + verify: bool, +) -> Result<(), StoreError> { + let block = &signed_block.message; + let parent_root = block.parent_root; + + // A parent that is the zero root or the block itself can never be fetched. + if parent_root == H256::ZERO || parent_root == block.hash_tree_root() { + return Err(StoreError::InvalidParentRoot { parent_root }); + } + + validate_block_attestations(block)?; + + let head_state = store.head_state(); + let num_validators = head_state.validators.len() as u64; + if !is_proposer(block.proposer_index, block.slot, num_validators) { + return Err(StoreError::NotProposer { + validator_index: block.proposer_index, + slot: block.slot, + }); + } + validate_validator_indices(block, num_validators)?; + + // The parent's header is on disk when it is itself pending, which is how + // a deep gap is filled. Its slot and position relative to finality are + // then known even though its state is not. + if let Some(parent) = store + .get_block_header(&parent_root) + .expect("DB read should succeed") + { + if block.slot <= parent.slot { + return Err(StoreError::ParentSlotNotBefore { + block_slot: block.slot, + parent_slot: parent.slot, + }); + } + let finalized = store + .latest_finalized() + .expect("latest finalized checkpoint exists"); + if parent.slot <= finalized.slot && parent_root != finalized.root { + return Err(StoreError::ParentConflictsWithFinalized { + parent_root, + parent_slot: parent.slot, + finalized_root: finalized.root, + finalized_slot: finalized.slot, + }); + } + } + + // Last, since it is by far the most expensive check: a forged block is + // turned away by everything above without reaching the SNARK verifier. + // Signature verification reads only the registry, which the head state + // shares with any other. Import verifies again once the parent arrives. + if verify { + verify_block_signatures(&head_state, signed_block)?; + } + + Ok(()) +} + +/// Reject a block body that repeats an `AttestationData` or carries more +/// distinct ones than `MAX_ATTESTATIONS_DATA` (leanSpec #536). +/// +/// A repeated entry would let one block count the same validators' votes more +/// than once. Needs nothing but the block itself. +fn validate_block_attestations(block: &Block) -> Result<(), StoreError> { + let attestations = &block.body.attestations; + let mut seen = HashSet::with_capacity(attestations.len()); + for att in attestations { + if !seen.insert(&att.data) { + return Err(StoreError::DuplicateAttestationData { + count: attestations.len(), + unique: seen.len(), + }); + } + } + if seen.len() > MAX_ATTESTATIONS_DATA { + return Err(StoreError::TooManyAttestationData { + count: seen.len(), + max: MAX_ATTESTATIONS_DATA, + }); + } + Ok(()) +} + +/// Reject a block naming a validator outside a registry of `num_validators`: +/// any participant bit in an attestation, or the proposer. +/// +/// Attesters are checked first, then the proposer, and each gets its own +/// error, since the spec names them distinctly (`VALIDATOR_INDEX_OUT_OF_RANGE` +/// vs `PROPOSER_INDEX_OUT_OF_RANGE`). +fn validate_validator_indices(block: &Block, num_validators: u64) -> Result<(), StoreError> { + for attestation in block.body.attestations.iter() { + for vid in validator_indices(&attestation.aggregation_bits) { + if vid >= num_validators { + return Err(StoreError::AttesterIndexOutOfRange { + validator_index: vid, + num_validators, + }); + } + } + } + if block.proposer_index >= num_validators { + return Err(StoreError::ProposerIndexOutOfRange { + proposer_index: block.proposer_index, + num_validators, + }); + } + Ok(()) } /// Full verification of a signed block's merged multi-message aggregate proof. @@ -1198,22 +1332,7 @@ pub fn verify_block_signatures( // Per-component pubkeys are resolved from the block body itself; the // wire proof carries no separate participant declaration to cross-check // against (leanSpec PR #717). - for attestation in attestations.iter() { - for vid in validator_indices(&attestation.aggregation_bits) { - if vid >= num_validators { - return Err(StoreError::AttesterIndexOutOfRange { - validator_index: vid, - num_validators, - }); - } - } - } - if block.proposer_index >= num_validators { - return Err(StoreError::ProposerIndexOutOfRange { - proposer_index: block.proposer_index, - num_validators, - }); - } + validate_validator_indices(block, num_validators)?; let block_root = block.hash_tree_root(); let structural_elapsed = total_start.elapsed(); @@ -1480,6 +1599,49 @@ mod tests { ); } + /// One more distinct `AttestationData` than the cap is rejected before the + /// state transition runs. + #[test] + fn on_block_rejects_too_many_attestation_data() { + let mut store = new_test_store(); + let head_root = store.head().expect("store head exists"); + + // Distinct entries: each names a different slot. + let entries: Vec = (0..=MAX_ATTESTATIONS_DATA as u64) + .map(|slot| AggregatedAttestation { + aggregation_bits: make_bits(&[0]), + data: AttestationData { + slot, + head: Checkpoint::default(), + target: Checkpoint::default(), + source: Checkpoint::default(), + }, + }) + .collect(); + let signed_block = SignedBlock { + message: Block { + slot: 1, + proposer_index: 0, + parent_root: head_root, + state_root: H256::ZERO, + body: BlockBody { + attestations: AggregatedAttestations::try_from(entries).unwrap(), + }, + }, + proof: MultiMessageAggregate::default(), + }; + + let result = on_block_without_verification(&mut store, signed_block); + assert!( + matches!( + result, + Err(StoreError::TooManyAttestationData { count, max }) + if count == MAX_ATTESTATIONS_DATA + 1 && max == MAX_ATTESTATIONS_DATA + ), + "Expected TooManyAttestationData, 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 @@ -2160,4 +2322,255 @@ mod tests { "Expected ProposerIndexOutOfRange, got: {result:?}" ); } + + // ============ Pending Block Validation Tests ============ + + /// Registry size for the pending-block tests: the proposer for slot `s` is + /// `s % PENDING_VALIDATORS`. + const PENDING_VALIDATORS: u64 = 4; + + /// A parent root no test ever stores. + const UNKNOWN_PARENT: H256 = H256([0xAB; 32]); + + fn pending_test_store() -> Store { + use ethlambda_storage::backend::InMemoryBackend; + use std::sync::Arc; + let genesis_state = State::from_genesis(1000, make_validators(PENDING_VALIDATORS)); + let backend = Arc::new(InMemoryBackend::new()); + Store::from_anchor_state(backend, genesis_state, DEFAULT_MILLISECONDS_PER_SLOT) + } + + /// A block that passes every pending check: the slot's proposer, an empty + /// body, and a parent we have never seen. + fn orphan_block(slot: u64, parent_root: H256) -> SignedBlock { + SignedBlock { + message: Block { + slot, + proposer_index: slot % PENDING_VALIDATORS, + parent_root, + state_root: H256::ZERO, + body: BlockBody::default(), + }, + proof: MultiMessageAggregate::default(), + } + } + + fn attestation_at(slot: u64, validators: &[usize]) -> AggregatedAttestation { + AggregatedAttestation { + aggregation_bits: make_bits(validators), + data: AttestationData { + slot, + head: Checkpoint::default(), + target: Checkpoint::default(), + source: Checkpoint::default(), + }, + } + } + + fn with_attestations( + mut block: SignedBlock, + entries: Vec, + ) -> SignedBlock { + block.message.body.attestations = AggregatedAttestations::try_from(entries).unwrap(); + block + } + + /// Passes every check but the signature, which a placeholder proof cannot + /// satisfy, so verification is skipped here. + #[test] + fn validate_pending_block_accepts_well_formed_orphan() { + let store = pending_test_store(); + let block = with_attestations( + orphan_block(5, UNKNOWN_PARENT), + vec![attestation_at(4, &[0, 3])], + ); + + let result = validate_pending_block_core(&store, &block, false); + + assert!(result.is_ok(), "Expected Ok, got: {result:?}"); + } + + /// The normal deep-gap case: the parent is itself pending, stored but + /// without a state, at an earlier slot above finality. Verification is + /// skipped, as above. + #[test] + fn validate_pending_block_accepts_child_of_pending_parent() { + let mut store = pending_test_store(); + let parent = orphan_block(5, UNKNOWN_PARENT); + let parent_root = parent.message.hash_tree_root(); + store.insert_pending_block(parent_root, parent).unwrap(); + + let block = orphan_block(6, parent_root); + let result = validate_pending_block_core(&store, &block, false); + + assert!(result.is_ok(), "Expected Ok, got: {result:?}"); + } + + /// A block that passes every cheap check but carries no valid proof is a + /// forgery as far as we can tell, and must not be stored. + #[test] + fn validate_pending_block_rejects_invalid_proof() { + ethlambda_crypto::init_leanvm(false); + let store = pending_test_store(); + + let result = validate_pending_block(&store, &orphan_block(5, UNKNOWN_PARENT)); + + assert!( + matches!(result, Err(StoreError::BlockProofVerificationFailed(_))), + "Expected BlockProofVerificationFailed, got: {result:?}" + ); + } + + #[test] + fn validate_pending_block_rejects_zero_parent_root() { + let store = pending_test_store(); + + let result = validate_pending_block(&store, &orphan_block(5, H256::ZERO)); + + assert!( + matches!( + result, + Err(StoreError::InvalidParentRoot { parent_root }) if parent_root == H256::ZERO + ), + "Expected InvalidParentRoot, got: {result:?}" + ); + } + + #[test] + fn validate_pending_block_rejects_duplicate_attestation_data() { + let store = pending_test_store(); + let block = with_attestations( + orphan_block(5, UNKNOWN_PARENT), + vec![attestation_at(4, &[0]), attestation_at(4, &[1])], + ); + + let result = validate_pending_block(&store, &block); + + assert!( + matches!( + result, + Err(StoreError::DuplicateAttestationData { + count: 2, + unique: 1 + }) + ), + "Expected DuplicateAttestationData, got: {result:?}" + ); + } + + #[test] + fn validate_pending_block_rejects_too_many_attestation_data() { + let store = pending_test_store(); + let entries = (0..=MAX_ATTESTATIONS_DATA as u64) + .map(|slot| attestation_at(slot, &[0])) + .collect(); + let block = with_attestations(orphan_block(20, UNKNOWN_PARENT), entries); + + let result = validate_pending_block(&store, &block); + + assert!( + matches!( + result, + Err(StoreError::TooManyAttestationData { count, max }) + if count == MAX_ATTESTATIONS_DATA + 1 && max == MAX_ATTESTATIONS_DATA + ), + "Expected TooManyAttestationData, got: {result:?}" + ); + } + + #[test] + fn validate_pending_block_rejects_wrong_proposer() { + let store = pending_test_store(); + let mut block = orphan_block(5, UNKNOWN_PARENT); + // Slot 5's proposer is 1, so 2 is a real validator out of turn. + block.message.proposer_index = 2; + + let result = validate_pending_block(&store, &block); + + assert!( + matches!( + result, + Err(StoreError::NotProposer { + validator_index: 2, + slot: 5 + }) + ), + "Expected NotProposer, got: {result:?}" + ); + } + + #[test] + fn validate_pending_block_rejects_out_of_range_attester() { + let store = pending_test_store(); + // Bit 4 is one past the last of the four registered validators. + let block = with_attestations( + orphan_block(5, UNKNOWN_PARENT), + vec![attestation_at(4, &[4])], + ); + + let result = validate_pending_block(&store, &block); + + assert!( + matches!( + result, + Err(StoreError::AttesterIndexOutOfRange { + validator_index: 4, + num_validators: 4 + }) + ), + "Expected AttesterIndexOutOfRange, got: {result:?}" + ); + } + + #[test] + fn validate_pending_block_rejects_slot_not_after_parent() { + let mut store = pending_test_store(); + let parent = orphan_block(5, UNKNOWN_PARENT); + let parent_root = parent.message.hash_tree_root(); + store.insert_pending_block(parent_root, parent).unwrap(); + + // Same slot as the stored parent. + let result = validate_pending_block(&store, &orphan_block(5, parent_root)); + + assert!( + matches!( + result, + Err(StoreError::ParentSlotNotBefore { + block_slot: 5, + parent_slot: 5 + }) + ), + "Expected ParentSlotNotBefore, got: {result:?}" + ); + } + + /// A stored parent at the finalized slot that is not the finalized block + /// sits on a fork finality has discarded, so its child can never import. + #[test] + fn validate_pending_block_rejects_parent_conflicting_with_finalized() { + let mut store = pending_test_store(); + let finalized = store.latest_finalized().unwrap(); + // A slot-0 sibling of the genesis block, stored as pending. + let sibling = orphan_block(finalized.slot, UNKNOWN_PARENT); + let sibling_root = sibling.message.hash_tree_root(); + store.insert_pending_block(sibling_root, sibling).unwrap(); + + let result = validate_pending_block(&store, &orphan_block(1, sibling_root)); + + assert!( + matches!( + result, + Err(StoreError::ParentConflictsWithFinalized { + parent_root, + parent_slot, + finalized_root, + finalized_slot, + }) if parent_root == sibling_root + && parent_slot == finalized.slot + && finalized_root == finalized.root + && finalized_slot == finalized.slot + ), + "Expected ParentConflictsWithFinalized, got: {result:?}" + ); + } } diff --git a/crates/storage/src/store.rs b/crates/storage/src/store.rs index 9d12dea8..bf55fd01 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -1244,6 +1244,38 @@ impl Store { Ok(()) } + /// Delete a block that was stored by [`insert_pending_block`](Self::insert_pending_block) + /// but never imported. + /// + /// Removes the `BlockHeaders`/`BlockBodies`/`BlockProof` rows so a + /// discarded or invalid pending block stops being served over + /// `BlocksByRoot`. A block with a state was imported and is part of the + /// chain, so it is left untouched; the same goes for a root with no stored + /// header. Returns whether anything was deleted. + pub fn delete_pending_block(&mut self, root: &H256) -> Result { + if self.has_state(root)? { + return Ok(false); + } + let Some(header) = self.get_block_header(root)? else { + return Ok(false); + }; + + let root_key = root.to_ssz(); + let proof_key = encode_slot_root_key(header.slot, root); + let mut batch = self.backend.begin_write().expect("write batch"); + batch + .delete_batch(Table::BlockHeaders, vec![root_key.clone()]) + .expect("delete pending block header"); + batch + .delete_batch(Table::BlockBodies, vec![root_key]) + .expect("delete pending block body"); + batch + .delete_batch(Table::BlockProof, vec![proof_key]) + .expect("delete pending block proof"); + batch.commit().expect("commit"); + Ok(true) + } + /// Insert a signed block, storing the block and signatures separately. /// /// Blocks and signatures are stored in separate tables because the genesis @@ -3654,4 +3686,78 @@ mod tests { .is_none() ); } + + // ============ Pending Block Deletion Tests ============ + + /// A pending block's header, body, and proof rows are all removed, so it + /// is no longer served over BlocksByRoot. + #[test] + fn delete_pending_block_removes_all_block_rows() { + let backend = Arc::new(InMemoryBackend::new()); + let mut store = Store::from_anchor_state( + backend.clone(), + State::from_genesis(0, vec![]), + DEFAULT_MILLISECONDS_PER_SLOT, + ); + + // A non-empty body, so the BlockBodies row is written too. + let data = make_att_data_for_target(1, root(1)); + let block = signed_block_with_attestations( + 5, + root(99), + vec![AggregatedAttestation { + aggregation_bits: make_proof_for_validators(&[0]).participants, + data, + }], + ); + let block_root = block.message.hash_tree_root(); + store + .insert_pending_block(block_root, block) + .expect("insert pending block"); + assert!(has_key(backend.as_ref(), Table::BlockHeaders, &block_root)); + assert!(has_key(backend.as_ref(), Table::BlockBodies, &block_root)); + assert!(has_block_proof(backend.as_ref(), 5, &block_root)); + + let deleted = store + .delete_pending_block(&block_root) + .expect("delete pending block"); + + assert!(deleted); + assert!(!has_key(backend.as_ref(), Table::BlockHeaders, &block_root)); + assert!(!has_key(backend.as_ref(), Table::BlockBodies, &block_root)); + assert!(!has_block_proof(backend.as_ref(), 5, &block_root)); + assert!(store.get_signed_block(&block_root).unwrap().is_none()); + } + + /// A block with a state was imported, so it must survive: discarding a + /// pending subtree can be triggered by a canonical block's root. + #[test] + fn delete_pending_block_keeps_imported_block() { + let backend = Arc::new(InMemoryBackend::new()); + let mut store = Store::from_anchor_state( + backend.clone(), + State::from_genesis(0, vec![]), + DEFAULT_MILLISECONDS_PER_SLOT, + ); + let anchor_root = store.head().expect("head root"); + + let deleted = store + .delete_pending_block(&anchor_root) + .expect("delete call succeeds"); + + assert!(!deleted); + assert!(has_key(backend.as_ref(), Table::BlockHeaders, &anchor_root)); + } + + /// An unknown root is a no-op rather than an error. + #[test] + fn delete_pending_block_ignores_unknown_root() { + let mut store = Store::test_store(); + + let deleted = store + .delete_pending_block(&root(42)) + .expect("delete call succeeds"); + + assert!(!deleted); + } } diff --git a/docs/architecture.md b/docs/architecture.md index 7f889d4e..c2ebef25 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -176,6 +176,11 @@ walking back through already-stored pending blocks, in `pending_block_parents`. is what it asks the `P2PServer` to fetch. Once the ancestor lands, the actor cascades down the parent index and re-imports every block that was waiting. +Parked blocks are written to disk and served to peers, so the actor first runs every check +that needs no parent state, ending with the signature, and rejects any block whose parent +failed the state transition. A rejected block is discarded along with any children waiting on +it, and their stored rows are deleted. + ### Chain events The actor is the sole publisher on an `EventBus` (`crates/blockchain/src/events.rs`), which diff --git a/docs/data_storage.md b/docs/data_storage.md index 506fdb5c..be045ada 100644 --- a/docs/data_storage.md +++ b/docs/data_storage.md @@ -251,6 +251,15 @@ while the block waits for its parent. When the block is later processed, `insert_signed_block` overwrites the same keys (idempotent) and adds the `LiveChain` entry. +Pending rows are served to peers over BlocksByRoot like any other block, so +a block is checked before it is written this way (`validate_pending_block` +in `crates/blockchain/src/store.rs`). Every check that needs no parent state +runs: body structure, proposer, validator indices, the parent's slot and +fork when its header is stored, and finally the block signature. A pending +block that is later discarded, or that fails its state transition, has its +rows deleted by `delete_pending_block`, which never touches a block that has +a state. + ## State Storage: Snapshots + Diffs Storing a full `State` per block would be wasteful: most fields never change @@ -388,10 +397,20 @@ processed): not needed for fork choice, reorg safety, or re-aggregation once outside the window. +**When a pending block is discarded** (in the BlockChain actor): + +- `delete_pending_block`: deletes the `BlockHeaders`, `BlockBodies`, and + `BlockProof` rows of a block that was stored as pending but never + imported. This runs for a block rejected by slot or by validation, for one + that fails its state transition, and for every pending descendant of + either. A block with a state is left untouched. + **Never pruned:** `BlockHeaders`, `BlockBodies`, `BlockRoots`, `States`, `StateDiffs`, and `Metadata`. Headers, bodies, the canonical slot index, and the snapshot+diff chain are the full historical record; only the proof blobs -and the (non-finalized) fork choice index are disposable. +and the (non-finalized) fork choice index are disposable. The one exception +is the header and body of a pending block that never imports, which were +never part of that record. ## In-Memory Only (Lost on Restart)