fix blob validation for empty blobs

beacon_node/beacon_chain/src/blob_verification.rs

@@ -5,6 +5,7 @@ use crate::{kzg_utils, BeaconChainError};
 use bls::PublicKey;
 use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions;
 use types::consts::eip4844::BLS_MODULUS;
+use types::signed_beacon_block::BlobReconstructionError;
 use types::{BeaconStateError, BlobsSidecar, Hash256, KzgCommitment, Slot, Transactions};
 
 #[derive(Debug)]
@@ -91,6 +92,12 @@ pub enum BlobError {
     MissingBlobs,
 }
 
+impl From<BlobReconstructionError> for BlobError {
+    fn from(_: BlobReconstructionError) -> Self {
+        BlobError::MissingBlobs
+    }
+}
+
 impl From<BeaconChainError> for BlobError {
     fn from(e: BeaconChainError) -> Self {
         BlobError::BeaconChainError(e)

beacon_node/beacon_chain/src/block_verification.rs

@@ -87,6 +87,7 @@ use std::time::Duration;
 use store::{Error as DBError, HotStateSummary, KeyValueStore, StoreOp};
 use task_executor::JoinHandle;
 use tree_hash::TreeHash;
+use types::signed_beacon_block::BlobReconstructionError;
 use types::signed_block_and_blobs::BlockWrapper;
 use types::{
     BeaconBlockRef, BeaconState, BeaconStateError, BlindedPayload, ChainSpec, CloneConfig, Epoch,
@@ -479,6 +480,12 @@ impl<T: EthSpec> From<ArithError> for BlockError<T> {
     }
 }
 
+impl<T: EthSpec> From<BlobReconstructionError> for BlockError<T> {
+    fn from(e: BlobReconstructionError) -> Self {
+        BlockError::BlobValidation(BlobError::from(e))
+    }
+}
+
 /// Stores information about verifying a payload against an execution engine.
 pub struct PayloadVerificationOutcome {
     pub payload_verification_status: PayloadVerificationStatus,
@@ -905,7 +912,7 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
         // Validate the block's execution_payload (if any).
         validate_execution_payload_for_gossip(&parent_block, block.message(), chain)?;
 
-        if let Some(blobs_sidecar) = block.blobs() {
+        if let Some(blobs_sidecar) = block.blobs(Some(block_root))? {
             let kzg_commitments = block
                 .message()
                 .body()
@@ -919,7 +926,7 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
                 .map_err(|_| BlockError::BlobValidation(BlobError::TransactionsMissing))?
                 .ok_or(BlockError::BlobValidation(BlobError::TransactionsMissing))?;
             validate_blob_for_gossip(
-                blobs_sidecar,
+                &blobs_sidecar,
                 kzg_commitments,
                 transactions,
                 block.slot(),
@@ -1134,11 +1141,7 @@ impl<T: BeaconChainTypes> IntoExecutionPendingBlock<T> for Arc<SignedBeaconBlock
         let block_root = check_block_relevancy(&self, block_root, chain)
             .map_err(|e| BlockSlashInfo::SignatureNotChecked(self.signed_block_header(), e))?;
 
-        SignatureVerifiedBlock::check_slashable(
+        SignatureVerifiedBlock::check_slashable(BlockWrapper::Block(self), block_root, chain)?
-            BlockWrapper::Block { block: self },
-            block_root,
-            chain,
-        )?
             .into_execution_pending_block_slashable(block_root, chain, notify_execution_layer)
     }
 
@@ -1563,7 +1566,7 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
         if let Some(data_availability_boundary) = chain.data_availability_boundary() {
             if block_slot.epoch(T::EthSpec::slots_per_epoch()) >= data_availability_boundary {
                 let sidecar = block
-                    .blobs()
+                    .blobs(Some(block_root))?
                     .ok_or(BlockError::BlobValidation(BlobError::MissingBlobs))?;
                 let kzg = chain.kzg.as_ref().ok_or(BlockError::BlobValidation(
                     BlobError::TrustedSetupNotInitialized,
@@ -1586,7 +1589,7 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
                         block.slot(),
                         block_root,
                         kzg_commitments,
-                        sidecar,
+                        &sidecar,
                     )
                     .map_err(|e| BlockError::BlobValidation(BlobError::KzgError(e)))?
                     {

beacon_node/http_api/src/publish_blocks.rs

@@ -43,9 +43,7 @@ pub async fn publish_block<T: BeaconChainTypes>(
                 network_tx,
                 PubsubMessage::BeaconBlockAndBlobsSidecars(block_and_blobs.clone()),
             )?;
-            BlockWrapper::BlockAndBlob {
+            BlockWrapper::BlockAndBlob(block_and_blobs)
-                block_sidecar_pair: block_and_blobs,
-            }
         } else {
             //FIXME(sean): This should probably return a specific no-blob-cached error code, beacon API coordination required
             return Err(warp_utils::reject::broadcast_without_import(format!(
@@ -54,7 +52,7 @@ pub async fn publish_block<T: BeaconChainTypes>(
         }
     } else {
         crate::publish_pubsub_message(network_tx, PubsubMessage::BeaconBlock(block.clone()))?;
-        BlockWrapper::Block { block }
+        BlockWrapper::Block(block)
     };
 
     // Determine the delay after the start of the slot, register it with metrics.

beacon_node/network/src/beacon_processor/mod.rs

@@ -1699,7 +1699,7 @@ impl<T: BeaconChainTypes> BeaconProcessor<T> {
                         message_id,
                         peer_id,
                         peer_client,
-                        BlockWrapper::Block { block },
+                        BlockWrapper::Block(block),
                         work_reprocessing_tx,
                         duplicate_cache,
                         seen_timestamp,
@@ -1721,7 +1721,7 @@ impl<T: BeaconChainTypes> BeaconProcessor<T> {
                         message_id,
                         peer_id,
                         peer_client,
-                        BlockWrapper::BlockAndBlob { block_sidecar_pair },
+                        BlockWrapper::BlockAndBlob(block_sidecar_pair),
                         work_reprocessing_tx,
                         duplicate_cache,
                         seen_timestamp,

beacon_node/network/src/beacon_processor/worker/sync_methods.rs

@@ -194,11 +194,9 @@ impl<T: BeaconChainTypes> Worker<T> {
                 let unwrapped = downloaded_blocks
                     .into_iter()
                     .map(|block| match block {
-                        BlockWrapper::Block { block } => block,
+                        BlockWrapper::Block(block) => block,
                         //FIXME(sean) handle blobs in backfill
-                        BlockWrapper::BlockAndBlob {
+                        BlockWrapper::BlockAndBlob(_) => todo!(),
-                            block_sidecar_pair: _,
-                        } => todo!(),
                     })
                     .collect();
 

beacon_node/network/src/sync/block_sidecar_coupling.rs

@@ -51,16 +51,14 @@ impl<T: EthSpec> BlockBlobRequestInfo<T> {
                 {
                     let blobs_sidecar =
                         accumulated_sidecars.pop_front().ok_or("missing sidecar")?;
-                    Ok(BlockWrapper::BlockAndBlob {
+                    Ok(BlockWrapper::BlockAndBlob(
-                        block_sidecar_pair: SignedBeaconBlockAndBlobsSidecar {
+                        SignedBeaconBlockAndBlobsSidecar {
                             beacon_block,
                             blobs_sidecar,
                         },
-                    })
+                    ))
                 } else {
-                    Ok(BlockWrapper::Block {
+                    Ok(BlockWrapper::Block(beacon_block))
-                        block: beacon_block,
-                    })
                 }
             })
             .collect::<Result<Vec<_>, _>>();

beacon_node/network/src/sync/manager.rs

@@ -734,14 +734,14 @@ impl<T: BeaconChainTypes> SyncManager<T> {
             RequestId::SingleBlock { id } => self.block_lookups.single_block_lookup_response(
                 id,
                 peer_id,
-                beacon_block.map(|block| BlockWrapper::Block { block }),
+                beacon_block.map(|block| BlockWrapper::Block(block)),
                 seen_timestamp,
                 &mut self.network,
             ),
             RequestId::ParentLookup { id } => self.block_lookups.parent_lookup_response(
                 id,
                 peer_id,
-                beacon_block.map(|block| BlockWrapper::Block { block }),
+                beacon_block.map(|block| BlockWrapper::Block(block)),
                 seen_timestamp,
                 &mut self.network,
             ),
@@ -756,7 +756,7 @@ impl<T: BeaconChainTypes> SyncManager<T> {
                         batch_id,
                         &peer_id,
                         id,
-                        beacon_block.map(|block| BlockWrapper::Block { block }),
+                        beacon_block.map(|block| BlockWrapper::Block(block)),
                     ) {
                         Ok(ProcessResult::SyncCompleted) => self.update_sync_state(),
                         Ok(ProcessResult::Successful) => {}
@@ -780,7 +780,7 @@ impl<T: BeaconChainTypes> SyncManager<T> {
                         chain_id,
                         batch_id,
                         id,
-                        beacon_block.map(|block| BlockWrapper::Block { block }),
+                        beacon_block.map(|block| BlockWrapper::Block(block)),
                     );
                     self.update_sync_state();
                 }
@@ -930,9 +930,11 @@ impl<T: BeaconChainTypes> SyncManager<T> {
             RequestId::SingleBlock { id } => self.block_lookups.single_block_lookup_response(
                 id,
                 peer_id,
-                block_sidecar_pair.map(|block_sidecar_pair| BlockWrapper::BlockAndBlob {
+                block_sidecar_pair.map(|block_sidecar_pair| {
+                    BlockWrapper::BlockAndBlob(
                         // TODO: why is this in an arc
-                    block_sidecar_pair: (*block_sidecar_pair).clone(),
+                        (*block_sidecar_pair).clone(),
+                    )
                 }),
                 seen_timestamp,
                 &mut self.network,
@@ -940,9 +942,11 @@ impl<T: BeaconChainTypes> SyncManager<T> {
             RequestId::ParentLookup { id } => self.block_lookups.parent_lookup_response(
                 id,
                 peer_id,
-                block_sidecar_pair.map(|block_sidecar_pair| BlockWrapper::BlockAndBlob {
+                block_sidecar_pair.map(|block_sidecar_pair| {
+                    BlockWrapper::BlockAndBlob(
                         // TODO: why is this in an arc
-                    block_sidecar_pair: (*block_sidecar_pair).clone(),
+                        (*block_sidecar_pair).clone(),
+                    )
                 }),
                 seen_timestamp,
                 &mut self.network,

beacon_node/network/src/sync/range_sync/batch.rs

@@ -25,11 +25,11 @@ impl<T: EthSpec> BatchTy<T> {
         match self {
             BatchTy::Blocks(blocks) => blocks
                 .into_iter()
-                .map(|block| BlockWrapper::Block { block })
+                .map(|block| BlockWrapper::Block(block))
                 .collect(),
             BatchTy::BlocksAndBlobs(block_sidecar_pair) => block_sidecar_pair
                 .into_iter()
-                .map(|block_sidecar_pair| BlockWrapper::BlockAndBlob { block_sidecar_pair })
+                .map(|block_sidecar_pair| BlockWrapper::BlockAndBlob(block_sidecar_pair))
                 .collect(),
         }
     }
@@ -413,7 +413,7 @@ impl<T: EthSpec, B: BatchConfig> BatchInfo<T, B> {
                 match self.batch_type {
                     ExpectedBatchTy::OnlyBlockBlobs => {
                         let blocks = blocks.into_iter().map(|block| {
-                            let BlockWrapper::BlockAndBlob { block_sidecar_pair: block_and_blob } = block else {
+                            let BlockWrapper::BlockAndBlob(block_and_blob) = block else {
                                 panic!("Batches should never have a mixed type. This is a bug. Contact D")
                             };
                             block_and_blob
@@ -422,7 +422,7 @@ impl<T: EthSpec, B: BatchConfig> BatchInfo<T, B> {
                     }
                     ExpectedBatchTy::OnlyBlock => {
                         let blocks = blocks.into_iter().map(|block| {
-                            let BlockWrapper::Block { block } = block else {
+                            let BlockWrapper::Block(block) = block else {
                                 panic!("Batches should never have a mixed type. This is a bug. Contact D")
                             };
                             block

consensus/state_processing/src/consensus_context.rs

@@ -21,8 +21,6 @@ pub struct ConsensusContext<T: EthSpec> {
     /// Cache of indexed attestations constructed during block processing.
     indexed_attestations:
         HashMap<(AttestationData, BitList<T::MaxValidatorsPerCommittee>), IndexedAttestation<T>>,
-    /// Should only be populated if the sidecar has not been validated.
-    blobs_sidecar: Option<Arc<BlobsSidecar<T>>>,
     /// Whether `validate_blobs_sidecar` has successfully passed.
     blobs_sidecar_validated: bool,
     /// Whether `verify_kzg_commitments_against_transactions` has successfully passed.
@@ -49,7 +47,6 @@ impl<T: EthSpec> ConsensusContext<T> {
             proposer_index: None,
             current_block_root: None,
             indexed_attestations: HashMap::new(),
-            blobs_sidecar: None,
             blobs_sidecar_validated: false,
             blobs_verified_vs_txs: false,
         }

consensus/types/src/blobs_sidecar.rs

@@ -28,6 +28,15 @@ impl<T: EthSpec> BlobsSidecar<T> {
         Self::default()
     }
 
+    pub fn empty_from_parts(beacon_block_root: Hash256, beacon_block_slot: Slot) -> Self {
+        Self {
+            beacon_block_root,
+            beacon_block_slot,
+            blobs: VariableList::empty(),
+            kzg_aggregated_proof: KzgProof::empty(),
+        }
+    }
+
     #[allow(clippy::integer_arithmetic)]
     pub fn max_size() -> usize {
         // Fixed part

consensus/types/src/signed_beacon_block.rs

@@ -36,6 +36,10 @@ impl From<SignedBeaconBlockHash> for Hash256 {
     }
 }
 
+pub enum BlobReconstructionError {
+    BlobsMissing,
+}
+
 /// A `BeaconBlock` and a signature from its proposer.
 #[superstruct(
     variants(Base, Altair, Merge, Capella, Eip4844),
@@ -235,6 +239,29 @@ impl<E: EthSpec, Payload: AbstractExecPayload<E>> SignedBeaconBlock<E, Payload>
     pub fn canonical_root(&self) -> Hash256 {
         self.message().tree_hash_root()
     }
+
+    /// Reconstructs an empty `BlobsSidecar`, using the given block root if provided, else calculates it.
+    /// If this block has kzg commitments, an error will be returned. If this block is from prior to the
+    /// Eip4844 fork, `None` will be returned.
+    pub fn reconstruct_empty_blobs(
+        &self,
+        block_root_opt: Option<Hash256>,
+    ) -> Result<Option<BlobsSidecar<E>>, BlobReconstructionError> {
+        self.message()
+            .body()
+            .blob_kzg_commitments()
+            .map(|kzg_commitments| {
+                if kzg_commitments.len() > 0 {
+                    Err(BlobReconstructionError::BlobsMissing)
+                } else {
+                    Ok(Some(BlobsSidecar::empty_from_parts(
+                        block_root_opt.unwrap_or(self.canonical_root()),
+                        self.slot(),
+                    )))
+                }
+            })
+            .unwrap_or(Ok(None))
+    }
 }
 
 // We can convert pre-Bellatrix blocks without payloads into blocks with payloads.

consensus/types/src/signed_block_and_blobs.rs

@@ -1,3 +1,4 @@
+use crate::signed_beacon_block::BlobReconstructionError;
 use crate::{BlobsSidecar, EthSpec, Hash256, SignedBeaconBlock, SignedBeaconBlockEip4844, Slot};
 use serde_derive::{Deserialize, Serialize};
 use ssz::{Decode, DecodeError};
@@ -35,60 +36,52 @@ impl<T: EthSpec> SignedBeaconBlockAndBlobsSidecar<T> {
 /// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobsSidecar`].
 #[derive(Clone, Debug)]
 pub enum BlockWrapper<T: EthSpec> {
-    Block {
+    Block(Arc<SignedBeaconBlock<T>>),
-        block: Arc<SignedBeaconBlock<T>>,
+    BlockAndBlob(SignedBeaconBlockAndBlobsSidecar<T>),
-    },
-    BlockAndBlob {
-        block_sidecar_pair: SignedBeaconBlockAndBlobsSidecar<T>,
-    },
 }
 
 impl<T: EthSpec> BlockWrapper<T> {
     pub fn slot(&self) -> Slot {
         match self {
-            BlockWrapper::Block { block } => block.slot(),
+            BlockWrapper::Block(block) => block.slot(),
-            BlockWrapper::BlockAndBlob { block_sidecar_pair } => {
+            BlockWrapper::BlockAndBlob(block_sidecar_pair) => {
                 block_sidecar_pair.beacon_block.slot()
             }
         }
     }
     pub fn block(&self) -> &SignedBeaconBlock<T> {
         match self {
-            BlockWrapper::Block { block } => &block,
+            BlockWrapper::Block(block) => &block,
-            BlockWrapper::BlockAndBlob { block_sidecar_pair } => &block_sidecar_pair.beacon_block,
+            BlockWrapper::BlockAndBlob(block_sidecar_pair) => &block_sidecar_pair.beacon_block,
         }
     }
     pub fn block_cloned(&self) -> Arc<SignedBeaconBlock<T>> {
         match self {
-            BlockWrapper::Block { block } => block.clone(),
+            BlockWrapper::Block(block) => block.clone(),
-            BlockWrapper::BlockAndBlob { block_sidecar_pair } => {
+            BlockWrapper::BlockAndBlob(block_sidecar_pair) => {
                 block_sidecar_pair.beacon_block.clone()
             }
         }
     }
 
-    pub fn blobs(&self) -> Option<&BlobsSidecar<T>> {
+    pub fn blobs(
+        &self,
+        block_root: Option<Hash256>,
+    ) -> Result<Option<Arc<BlobsSidecar<T>>>, BlobReconstructionError> {
         match self {
-            BlockWrapper::Block { .. } => None,
+            BlockWrapper::Block(block) => block
-            BlockWrapper::BlockAndBlob { block_sidecar_pair } => {
+                .reconstruct_empty_blobs(block_root)
-                Some(&block_sidecar_pair.blobs_sidecar)
+                .map(|blob_opt| blob_opt.map(Arc::new)),
-            }
+            BlockWrapper::BlockAndBlob(block_sidecar_pair) => {
-        }
+                Ok(Some(block_sidecar_pair.blobs_sidecar.clone()))
-    }
-
-    pub fn blobs_cloned(&self) -> Option<Arc<BlobsSidecar<T>>> {
-        match self {
-            BlockWrapper::Block { block: _ } => None,
-            BlockWrapper::BlockAndBlob { block_sidecar_pair } => {
-                Some(block_sidecar_pair.blobs_sidecar.clone())
             }
         }
     }
 
     pub fn message(&self) -> crate::BeaconBlockRef<T> {
         match self {
-            BlockWrapper::Block { block } => block.message(),
+            BlockWrapper::Block(block) => block.message(),
-            BlockWrapper::BlockAndBlob { block_sidecar_pair } => {
+            BlockWrapper::BlockAndBlob(block_sidecar_pair) => {
                 block_sidecar_pair.beacon_block.message()
             }
         }
@@ -100,8 +93,8 @@ impl<T: EthSpec> BlockWrapper<T> {
 
     pub fn deconstruct(self) -> (Arc<SignedBeaconBlock<T>>, Option<Arc<BlobsSidecar<T>>>) {
         match self {
-            BlockWrapper::Block { block } => (block, None),
+            BlockWrapper::Block(block) => (block, None),
-            BlockWrapper::BlockAndBlob { block_sidecar_pair } => {
+            BlockWrapper::BlockAndBlob(block_sidecar_pair) => {
                 let SignedBeaconBlockAndBlobsSidecar {
                     beacon_block,
                     blobs_sidecar,
@@ -112,29 +105,25 @@ impl<T: EthSpec> BlockWrapper<T> {
     }
 }
 
-// TODO: probably needes to be changed. This is needed because SignedBeaconBlockAndBlobsSidecar
+// FIXME(sean): probably needs to be changed. This is needed because SignedBeaconBlockAndBlobsSidecar
 // does not implement Hash
 impl<T: EthSpec> std::hash::Hash for BlockWrapper<T> {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
         match self {
-            BlockWrapper::Block { block } => block.hash(state),
+            BlockWrapper::Block(block) => block.hash(state),
-            BlockWrapper::BlockAndBlob {
+            BlockWrapper::BlockAndBlob(block_and_blob) => block_and_blob.beacon_block.hash(state),
-                block_sidecar_pair: block_and_blob,
-            } => block_and_blob.beacon_block.hash(state),
         }
     }
 }
 
 impl<T: EthSpec> From<SignedBeaconBlock<T>> for BlockWrapper<T> {
     fn from(block: SignedBeaconBlock<T>) -> Self {
-        BlockWrapper::Block {
+        BlockWrapper::Block(Arc::new(block))
-            block: Arc::new(block),
-        }
     }
 }
 
 impl<T: EthSpec> From<Arc<SignedBeaconBlock<T>>> for BlockWrapper<T> {
     fn from(block: Arc<SignedBeaconBlock<T>>) -> Self {
-        BlockWrapper::Block { block }
+        BlockWrapper::Block(block)
     }
 }

crypto/kzg/src/kzg_proof.rs

@@ -12,6 +12,14 @@ const KZG_PROOF_BYTES_LEN: usize = 48;
 #[ssz(struct_behaviour = "transparent")]
 pub struct KzgProof(pub [u8; KZG_PROOF_BYTES_LEN]);
 
+impl KzgProof {
+    pub fn empty() -> Self {
+        let mut bytes = [0; KZG_PROOF_BYTES_LEN];
+        bytes[0] = 192;
+        Self(bytes)
+    }
+}
+
 impl fmt::Display for KzgProof {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{}", eth2_serde_utils::hex::encode(self.0))