diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index d42855794b..9acab8d360 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -323,6 +323,16 @@ pub enum BlockError { /// We were unable to process this block due to an internal error. It's unclear if the block is /// valid. InternalError(String), + /// The number of kzg commitments in the block exceed the max allowed blobs per block for + /// the block's epoch. + /// + /// ## Peer scoring + /// + /// This block is invalid and the peer should be penalised. + InvalidBlobCount { + max_blobs_at_epoch: usize, + block: usize, + }, } /// Which specific signature(s) are invalid in a SignedBeaconBlock @@ -856,6 +866,21 @@ impl GossipVerifiedBlock { }); } + // Do not gossip blocks that claim to contain more blobs than the max allowed + // at the given block epoch. + if let Ok(commitments) = block.message().body().blob_kzg_commitments() { + let max_blobs_at_epoch = chain + .spec + .max_blobs_per_block(block.slot().epoch(T::EthSpec::slots_per_epoch())) + as usize; + if commitments.len() > max_blobs_at_epoch { + return Err(BlockError::InvalidBlobCount { + max_blobs_at_epoch, + block: commitments.len(), + }); + } + } + let block_root = get_block_header_root(block_header); // Do not gossip a block from a finalized slot. diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 3ff5f772aa..9a6a789b42 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -1247,6 +1247,38 @@ async fn block_gossip_verification() { ), "the second proposal by this validator should be rejected" ); + + /* + * This test ensures that: + * + * We do not accept blocks with blob_kzg_commitments length larger than the max_blobs for that epoch. + */ + let (mut block, signature) = chain_segment[block_index] + .beacon_block + .as_ref() + .clone() + .deconstruct(); + + let kzg_commitments_len = harness + .chain + .spec + .max_blobs_per_block(block.slot().epoch(E::slots_per_epoch())) + as usize; + + if let Ok(kzg_commitments) = block.body_mut().blob_kzg_commitments_mut() { + *kzg_commitments = vec![KzgCommitment::empty_for_testing(); kzg_commitments_len + 1].into(); + assert!( + matches!( + unwrap_err(harness.chain.verify_block_for_gossip(Arc::new(SignedBeaconBlock::from_block(block, signature))).await), + BlockError::InvalidBlobCount { + max_blobs_at_epoch, + block, + } + if max_blobs_at_epoch == kzg_commitments_len && block == kzg_commitments_len + 1 + ), + "should not import a block with higher blob_kzg_commitment length than the max_blobs at epoch" + ); + } } async fn verify_and_process_gossip_data_sidecars( diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index ea9a5db1af..6bdcd02197 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -1304,7 +1304,8 @@ impl NetworkBeaconProcessor { | Err(e @ BlockError::ExecutionPayloadError(_)) | Err(e @ BlockError::ParentExecutionPayloadInvalid { .. }) | Err(e @ BlockError::KnownInvalidExecutionPayload(_)) - | Err(e @ BlockError::GenesisBlock) => { + | Err(e @ BlockError::GenesisBlock) + | Err(e @ BlockError::InvalidBlobCount { .. }) => { warn!(error = %e, "Could not verify block for gossip. Rejecting the block"); self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject); self.gossip_penalize_peer( diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index 80974df6e7..f5e44f7ac9 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -3538,7 +3538,7 @@ pub fn get_ancestor_state_root<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStor .get_cold_state_root(target_slot) .map_err(Box::new) .map_err(StateSummaryIteratorError::LoadStateRootError)? - .ok_or_else(|| StateSummaryIteratorError::MissingStateRoot { + .ok_or(StateSummaryIteratorError::MissingStateRoot { target_slot, state_upper_limit, });