Add a wrapper to allow construction of only valid AvailableBlocks

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -2714,7 +2714,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             |chain| {
                 chain
                     .data_availability_checker
-                    .check_block_availability(executed_block)
+                    .check_block_availability(executed_block, |epoch| {
+                        chain.block_needs_da_check(epoch)
+                    })
             },
             count_unrealized,
         )
@@ -2846,7 +2848,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     }
                 };
 
-                let slot = available_block.block.slot();
+                let slot = available_block.slot();
 
                 // import
                 let chain = self.clone();
@@ -2925,7 +2927,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         // -----------------------------------------------------------------------------------------
         let current_slot = self.slot()?;
         let current_epoch = current_slot.epoch(T::EthSpec::slots_per_epoch());
-        let block = signed_block.block.message();
+        let block = signed_block.message();
         let post_exec_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_POST_EXEC_PROCESSING);
 
         // Check against weak subjectivity checkpoint.
@@ -2969,7 +2971,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         )?;
         // TODO(pawan): fix this atrocity
         let signed_block = signed_block.into_available_block().unwrap();
-        let block = signed_block.block.message();
+        let block = signed_block.message();
 
         // Register the new block with the fork choice service.
         {
@@ -6169,6 +6171,12 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         })
     }
 
+    /// Returns true if the given epoch lies within the da boundary and false otherwise.
+    pub fn block_needs_da_check(&self, block_epoch: Epoch) -> bool {
+        self.data_availability_boundary()
+            .map_or(false, |da_epoch| block_epoch >= da_epoch)
+    }
+
     /// The epoch that is a data availability boundary, or the latest finalized epoch.
     /// `None` if the `Eip4844` fork is disabled.
     pub fn finalized_data_availability_boundary(&self) -> Option<Epoch> {

beacon_node/beacon_chain/src/blob_verification.rs

@@ -322,15 +322,78 @@ impl<T: BeaconChainTypes> IntoAvailableBlock<T> for BlockWrapper<T::EthSpec> {
 }
 
 #[derive(Clone, Debug, PartialEq, Derivative)]
-#[derivative(Hash(bound = "T: EthSpec"))]
-pub struct AvailableBlock<T: EthSpec> {
-    pub block: Arc<SignedBeaconBlock<T>>,
-    pub blobs: VerifiedBlobs<T>,
+#[derivative(Hash(bound = "E: EthSpec"))]
+pub struct AvailableBlock<E: EthSpec>(AvailableBlockInner<E>);
+
+#[derive(Clone, Debug, PartialEq, Derivative)]
+#[derivative(Hash(bound = "E: EthSpec"))]
+struct AvailableBlockInner<E: EthSpec> {
+    block: Arc<SignedBeaconBlock<E>>,
+    blobs: VerifiedBlobs<E>,
 }
 
-impl<T: EthSpec> AvailableBlock<T> {
-    pub fn blobs(&self) -> Option<BlobSidecarArcList<T>> {
-        match &self.blobs {
+impl<E: EthSpec> AvailableBlock<E> {
+    pub fn new(
+        block: Arc<SignedBeaconBlock<E>>,
+        blobs: Vec<Arc<BlobSidecar<E>>>,
+        da_check: impl FnOnce(Epoch) -> bool,
+    ) -> Result<Self, String> {
+        if let Ok(block_kzg_commitments) = block.message().body().blob_kzg_commitments() {
+            if blobs.is_empty() && block_kzg_commitments.is_empty() {
+                return Ok(Self(AvailableBlockInner {
+                    block,
+                    blobs: VerifiedBlobs::EmptyBlobs,
+                }));
+            }
+
+            if blobs.is_empty() {
+                if da_check(block.epoch()) {
+                    return Ok(Self(AvailableBlockInner {
+                        block,
+                        blobs: VerifiedBlobs::NotRequired,
+                    }));
+                } else {
+                    return Err("Block within DA boundary but no blobs provided".to_string());
+                }
+            }
+
+            if blobs.len() != block_kzg_commitments.len() {
+                return Err(format!(
+                    "Block commitments and blobs length must be the same. 
+                        Block commitments len: {}, blobs length: {}",
+                    block_kzg_commitments.len(),
+                    blobs.len()
+                ));
+            }
+
+            for (block_commitment, blob) in block_kzg_commitments.iter().zip(blobs.iter()) {
+                if *block_commitment != blob.kzg_commitment {
+                    return Err(format!(
+                        "Invalid input. Blob and block commitment mismatch at index {}",
+                        blob.index
+                    ));
+                }
+            }
+            Ok(Self(AvailableBlockInner {
+                block,
+                blobs: VerifiedBlobs::Available(blobs.into()),
+            }))
+        }
+        // This is a pre 4844 block
+        else {
+            Ok(Self(AvailableBlockInner {
+                block,
+                blobs: VerifiedBlobs::PreEip4844,
+            }))
+        }
+    }
+
+    pub fn block(&self) -> &SignedBeaconBlock<E> {
+        &self.0.block
+    }
+
+    pub fn blobs(&self) -> Option<BlobSidecarArcList<E>> {
+        match &self.0.blobs {
             VerifiedBlobs::EmptyBlobs | VerifiedBlobs::NotRequired | VerifiedBlobs::PreEip4844 => {
                 None
             }
@@ -338,12 +401,12 @@ impl<T: EthSpec> AvailableBlock<T> {
         }
     }
 
-    pub fn deconstruct(self) -> (Arc<SignedBeaconBlock<T>>, Option<BlobSidecarArcList<T>>) {
-        match self.blobs {
+    pub fn deconstruct(self) -> (Arc<SignedBeaconBlock<E>>, Option<BlobSidecarArcList<E>>) {
+        match self.0.blobs {
             VerifiedBlobs::EmptyBlobs | VerifiedBlobs::NotRequired | VerifiedBlobs::PreEip4844 => {
-                (self.block, None)
+                (self.0.block, None)
             }
-            VerifiedBlobs::Available(blobs) => (self.block, Some(blobs)),
+            VerifiedBlobs::Available(blobs) => (self.0.block, Some(blobs)),
         }
     }
 }
@@ -394,117 +457,113 @@ impl<E: EthSpec> BlockWrapper<E> {
     }
 }
 
+impl<E: EthSpec> AsBlock<E> for AvailableBlock<E> {
+    fn slot(&self) -> Slot {
+        self.0.block.slot()
+    }
+
+    fn epoch(&self) -> Epoch {
+        self.0.block.epoch()
+    }
+
+    fn parent_root(&self) -> Hash256 {
+        self.0.block.parent_root()
+    }
+
+    fn state_root(&self) -> Hash256 {
+        self.0.block.state_root()
+    }
+
+    fn signed_block_header(&self) -> SignedBeaconBlockHeader {
+        self.0.block.signed_block_header()
+    }
+
+    fn message(&self) -> BeaconBlockRef<E> {
+        self.0.block.message()
+    }
+
+    fn as_block(&self) -> &SignedBeaconBlock<E> {
+        &self.0.block
+    }
+
+    fn block_cloned(&self) -> Arc<SignedBeaconBlock<E>> {
+        self.0.block.clone()
+    }
+
+    fn canonical_root(&self) -> Hash256 {
+        self.0.block.canonical_root()
+    }
+}
+
 impl<E: EthSpec> AsBlock<E> for BlockWrapper<E> {
     fn slot(&self) -> Slot {
-        match self {
-            BlockWrapper::Available(block) => block.block.slot(),
-            BlockWrapper::AvailabilityPending(block) => block.slot(),
-        }
+        self.as_block().slot()
     }
     fn epoch(&self) -> Epoch {
-        match self {
-            BlockWrapper::Available(block) => block.block.epoch(),
-            BlockWrapper::AvailabilityPending(block) => block.epoch(),
-        }
+        self.as_block().epoch()
     }
     fn parent_root(&self) -> Hash256 {
-        match self {
-            BlockWrapper::Available(block) => block.block.parent_root(),
-            BlockWrapper::AvailabilityPending(block) => block.parent_root(),
-        }
+        self.as_block().parent_root()
     }
     fn state_root(&self) -> Hash256 {
-        match self {
-            BlockWrapper::Available(block) => block.block.state_root(),
-            BlockWrapper::AvailabilityPending(block) => block.state_root(),
-        }
+        self.as_block().state_root()
     }
     fn signed_block_header(&self) -> SignedBeaconBlockHeader {
-        match &self {
-            BlockWrapper::Available(block) => block.block.signed_block_header(),
-            BlockWrapper::AvailabilityPending(block) => block.signed_block_header(),
-        }
+        self.as_block().signed_block_header()
     }
     fn message(&self) -> BeaconBlockRef<E> {
-        match &self {
-            BlockWrapper::Available(block) => block.block.message(),
-            BlockWrapper::AvailabilityPending(block) => block.message(),
-        }
+        self.as_block().message()
     }
     fn as_block(&self) -> &SignedBeaconBlock<E> {
         match &self {
-            BlockWrapper::Available(block) => &block.block,
+            BlockWrapper::Available(block) => &block.0.block,
             BlockWrapper::AvailabilityPending(block) => block,
         }
     }
     fn block_cloned(&self) -> Arc<SignedBeaconBlock<E>> {
         match &self {
-            BlockWrapper::Available(block) => block.block.clone(),
+            BlockWrapper::Available(block) => block.0.block.clone(),
             BlockWrapper::AvailabilityPending(block) => block.clone(),
         }
     }
     fn canonical_root(&self) -> Hash256 {
-        match &self {
-            BlockWrapper::Available(block) => block.block.canonical_root(),
-            BlockWrapper::AvailabilityPending(block) => block.canonical_root(),
-        }
+        self.as_block().canonical_root()
     }
 }
 
 impl<E: EthSpec> AsBlock<E> for &BlockWrapper<E> {
     fn slot(&self) -> Slot {
-        match self {
-            BlockWrapper::Available(block) => block.block.slot(),
-            BlockWrapper::AvailabilityPending(block) => block.slot(),
-        }
+        self.as_block().slot()
     }
     fn epoch(&self) -> Epoch {
-        match self {
-            BlockWrapper::Available(block) => block.block.epoch(),
-            BlockWrapper::AvailabilityPending(block) => block.epoch(),
-        }
+        self.as_block().epoch()
     }
     fn parent_root(&self) -> Hash256 {
-        match self {
-            BlockWrapper::Available(block) => block.block.parent_root(),
-            BlockWrapper::AvailabilityPending(block) => block.parent_root(),
-        }
+        self.as_block().parent_root()
     }
     fn state_root(&self) -> Hash256 {
-        match self {
-            BlockWrapper::Available(block) => block.block.state_root(),
-            BlockWrapper::AvailabilityPending(block) => block.state_root(),
-        }
+        self.as_block().state_root()
     }
     fn signed_block_header(&self) -> SignedBeaconBlockHeader {
-        match &self {
-            BlockWrapper::Available(block) => block.block.signed_block_header(),
-            BlockWrapper::AvailabilityPending(block) => block.signed_block_header(),
-        }
+        self.as_block().signed_block_header()
     }
     fn message(&self) -> BeaconBlockRef<E> {
-        match &self {
-            BlockWrapper::Available(block) => block.block.message(),
-            BlockWrapper::AvailabilityPending(block) => block.message(),
-        }
+        self.as_block().message()
     }
     fn as_block(&self) -> &SignedBeaconBlock<E> {
         match &self {
-            BlockWrapper::Available(block) => &block.block,
+            BlockWrapper::Available(block) => &block.0.block,
             BlockWrapper::AvailabilityPending(block) => block,
         }
     }
     fn block_cloned(&self) -> Arc<SignedBeaconBlock<E>> {
         match &self {
-            BlockWrapper::Available(block) => block.block.clone(),
+            BlockWrapper::Available(block) => block.0.block.clone(),
             BlockWrapper::AvailabilityPending(block) => block.clone(),
         }
     }
     fn canonical_root(&self) -> Hash256 {
-        match &self {
-            BlockWrapper::Available(block) => block.block.canonical_root(),
-            BlockWrapper::AvailabilityPending(block) => block.canonical_root(),
-        }
+        self.as_block().canonical_root()
     }
 }
 

beacon_node/beacon_chain/src/block_verification.rs

@@ -93,9 +93,9 @@ use task_executor::JoinHandle;
 use tree_hash::TreeHash;
 use types::ExecPayload;
 use types::{
-    BeaconBlockRef, BeaconState, BeaconStateError, BlindedPayload, ChainSpec, CloneConfig, Epoch,
-    EthSpec, ExecutionBlockHash, Hash256, InconsistentFork, PublicKey, PublicKeyBytes,
-    RelativeEpoch, SignedBeaconBlock, SignedBeaconBlockHeader, Slot,
+    BeaconBlockRef, BeaconState, BeaconStateError, BlindedPayload, BlobSidecar, ChainSpec,
+    CloneConfig, Epoch, EthSpec, ExecutionBlockHash, Hash256, InconsistentFork, PublicKey,
+    PublicKeyBytes, RelativeEpoch, SignedBeaconBlock, SignedBeaconBlockHeader, Slot,
 };
 
 pub const POS_PANDA_BANNER: &str = r#"
@@ -1171,16 +1171,30 @@ impl<T: BeaconChainTypes> IntoExecutionPendingBlock<T> for BlockWrapper<T::EthSp
         match self {
             BlockWrapper::AvailabilityPending(block) => block
                 .into_execution_pending_block_slashable(block_root, chain, notify_execution_layer),
-            BlockWrapper::Available(AvailableBlock { block, blobs }) => {
+            BlockWrapper::Available(available_block) => {
+                let (block, blobs) = available_block.deconstruct();
                 let mut execution_pending_block = block.into_execution_pending_block_slashable(
                     block_root,
                     chain,
                     notify_execution_layer,
                 )?;
                 let block = execution_pending_block.block.block_cloned();
+
+                let blobs: Vec<Arc<BlobSidecar<_>>> = match blobs {
+                    Some(blob_list) => blob_list.into(),
+                    None => vec![],
+                };
                 let available_execution_pending_block =
-                    BlockWrapper::Available(AvailableBlock { block, blobs });
-                execution_pending_block.block = available_execution_pending_block;
+                    AvailableBlock::new(block, blobs, |epoch| chain.block_needs_da_check(epoch))
+                        .map_err(|e| {
+                            BlockSlashInfo::SignatureValid(
+                                execution_pending_block.block.signed_block_header(),
+                                BlockError::AvailabilityCheck(
+                                    AvailabilityCheckError::InvalidBlockOrBlob(e),
+                                ),
+                            )
+                        })?;
+                execution_pending_block.block = available_execution_pending_block.into();
                 Ok(execution_pending_block)
             }
         }

beacon_node/beacon_chain/src/gossip_blob_cache.rs

@@ -1,21 +1,22 @@
-use crate::blob_verification::{AsBlock, AvailableBlock, BlockWrapper, VerifiedBlobs};
+use crate::blob_verification::{AsBlock, AvailableBlock, BlockWrapper};
 use crate::block_verification::ExecutedBlock;
 use crate::kzg_utils::validate_blob;
 use kzg::Error as KzgError;
 use kzg::Kzg;
 use parking_lot::{Mutex, RwLock};
-use ssz_types::{Error, VariableList};
+use ssz_types::Error;
 use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 use std::sync::Arc;
 use types::blob_sidecar::{BlobIdentifier, BlobSidecar};
-use types::{EthSpec, Hash256};
+use types::{Epoch, EthSpec, Hash256};
 
 #[derive(Debug)]
 pub enum AvailabilityCheckError {
     DuplicateBlob(Hash256),
     Kzg(KzgError),
     SszTypes(ssz_types::Error),
+    InvalidBlockOrBlob(String),
 }
 
 impl From<ssz_types::Error> for AvailabilityCheckError {
@@ -131,6 +132,7 @@ impl<T: EthSpec> DataAvailabilityChecker<T> {
     pub fn check_block_availability(
         &self,
         executed_block: ExecutedBlock<T>,
+        da_check_fn: impl FnOnce(Epoch) -> bool,
     ) -> Result<Availability<T>, AvailabilityCheckError> {
         let block_clone = executed_block.block.clone();
 
@@ -166,12 +168,10 @@ impl<T: EthSpec> DataAvailabilityChecker<T> {
                                     payload_verification_outcome,
                                 } = executed_block;
 
-                                let available_block = BlockWrapper::Available(AvailableBlock {
-                                    block,
-                                    blobs: VerifiedBlobs::Available(VariableList::new(
-                                        removed.verified_blobs,
-                                    )?),
-                                });
+                                let available_block =
+                                    AvailableBlock::new(block, removed.verified_blobs, da_check_fn)
+                                        .map_err(AvailabilityCheckError::InvalidBlockOrBlob)?
+                                        .into();
 
                                 let available_executed = ExecutedBlock {
                                     block: available_block,