more progress

beacon_node/beacon_chain/src/blob_verification.rs

@@ -1,8 +1,5 @@
-use derivative::Derivative;
 use slot_clock::SlotClock;
 use std::sync::Arc;
-use tokio::task::JoinHandle;
-use ssz_types::VariableList;
 
 use crate::beacon_chain::{
     BeaconChain, BeaconChainTypes, MAXIMUM_GOSSIP_CLOCK_DISPARITY,
@@ -11,12 +8,9 @@ use crate::beacon_chain::{
 use crate::BeaconChainError;
 use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions;
 use types::{
-    BeaconBlockRef, BeaconStateError, BlobsSidecar, EthSpec, Hash256, KzgCommitment,
-    SignedBeaconBlock, SignedBeaconBlockAndBlobsSidecar, SignedBeaconBlockHeader,
-    SignedBlobSidecar, Slot, Transactions,
+    BeaconBlockRef, BeaconStateError, BlobSidecarList, Epoch, EthSpec, Hash256, KzgCommitment,
+    SignedBeaconBlock, SignedBeaconBlockHeader, SignedBlobSidecar, Slot, Transactions,
 };
-use types::{Epoch, ExecPayload};
-use types::blob_sidecar::BlobSidecar;
 
 #[derive(Debug)]
 pub enum BlobError {
@@ -191,9 +185,9 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
     };
 
     let blob_proposer_index = blob_sidecar.message.proposer_index;
-    if proposer_index != blob_proposer_index {
+    if proposer_index != blob_proposer_index as usize {
         return Err(BlobError::ProposerIndexMismatch {
-            sidecar: blob_proposer_index,
+            sidecar: blob_proposer_index as usize,
             local: proposer_index,
         });
     }
@@ -249,11 +243,11 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
 }
 
 pub fn verify_data_availability<T: BeaconChainTypes>(
-    blob_sidecar: &BlobsSidecar<T::EthSpec>,
+    blob_sidecar: &BlobSidecarList<T::EthSpec>,
     kzg_commitments: &[KzgCommitment],
     transactions: &Transactions<T::EthSpec>,
-    block_slot: Slot,
-    block_root: Hash256,
+    _block_slot: Slot,
+    _block_root: Hash256,
     chain: &BeaconChain<T>,
 ) -> Result<(), BlobError> {
     if verify_kzg_commitments_against_transactions::<T::EthSpec>(transactions, kzg_commitments)
@@ -263,7 +257,7 @@ pub fn verify_data_availability<T: BeaconChainTypes>(
     }
 
     // Validatate that the kzg proof is valid against the commitments and blobs
-    let kzg = chain
+    let _kzg = chain
         .kzg
         .as_ref()
         .ok_or(BlobError::TrustedSetupNotInitialized)?;
@@ -289,25 +283,6 @@ pub enum DataAvailabilityCheckRequired {
     No,
 }
 
-impl<T: BeaconChainTypes> BlockWrapper<T::EthSpec> {
-    fn into_availablilty_pending_block(
-        self,
-        block_root: Hash256,
-        chain: &BeaconChain<T>,
-    ) -> Result<AvailabilityPendingBlock<T::EthSpec>, BlobError> {
-        match self {
-            BlockWrapper::Block(block) => {
-                AvailabilityPendingBlock::new(block, block_root, da_check_required)
-            }
-            BlockWrapper::BlockAndBlobs(block, blobs_sidecar) => {
-
-
-                AvailabilityPendingBlock::new_with_blobs(block, blobs_sidecar, da_check_required)
-            }
-        }
-    }
-}
-
 pub trait IntoAvailableBlock<T: BeaconChainTypes> {
     fn into_available_block(
         self,
@@ -316,173 +291,49 @@ pub trait IntoAvailableBlock<T: BeaconChainTypes> {
     ) -> Result<AvailableBlock<T::EthSpec>, BlobError>;
 }
 
-/// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobsSidecar`].  An
-/// `AvailableBlock` has passed any required data availability checks and should be used in
-/// consensus.
-#[derive(Clone, Debug, Derivative)]
-#[derivative(PartialEq, Hash(bound = "T: BeaconChainTypes"))]
-pub struct AvailabilityPendingBlock<T: BeaconChainTypes> {
-    block: Arc<SignedBeaconBlock<T::EthSpec>>,
-    data_availability_handle: DataAvailabilityHandle<T::EthSpec>,
-}
-
-/// Used to await the result of data availability check.
-type DataAvailabilityHandle<E> = JoinHandle<Result<Option<Blobs<E>>, BlobError>>;
-
-#[derive(Clone, Debug, Derivative)]
-#[derivative(PartialEq, Hash(bound = "T: BeaconChainTypes"))]
-pub struct AvailableBlock<T: BeaconChainTypes> {
-    block: Arc<SignedBeaconBlock<T::EthSpec>>,
-    blobs: Blobs<T::EthSpec>,
-}
-
-impl <T: BeaconChainTypes> AvailableBlock<T> {
-    pub fn blobs(&self) -> Option<Arc<BlobsSidecar<T>>> {
-        match &self.blobs {
-            Blobs::NotRequired | Blobs::None => None,
-            Blobs::Available(block_sidecar) => {
-                Some(block_sidecar.clone())
-            }
-        }
-    }
-
-    pub fn deconstruct(self) -> (Arc<SignedBeaconBlock<T::EthSpec>>, Option<Arc<BlobsSidecar<T::EthSpec>>>) {
-        match self.blobs {
-            Blobs::NotRequired | Blobs::None => (self.block, None),
-            Blobs::Available(blob_sidecars) => {
-                (self.block, Some(blob_sidecars))
-            }
-        }
-    }
-}
-
-pub enum Blobs<E: EthSpec> {
-    /// These blobs are available.
-    Available(VariableList<Arc<BlobSidecar<E>>, E::MaxBlobsPerBlock>),
-    /// This block is from outside the data availability boundary or the block is from prior
-    /// to the eip4844 fork.
-    NotRequired,
-    /// The block doesn't have any blob transactions.
-    None,
-}
-
-//TODO(sean) add block root to the availability pending block?
-impl<T: BeaconChainTypes> AvailabilityPendingBlock<T> {
-    pub fn new(
-        beacon_block: Arc<SignedBeaconBlock<T::EthSpec>>,
+impl<T: BeaconChainTypes> IntoAvailableBlock<T> for BlockWrapper<T::EthSpec> {
+    fn into_available_block(
+        self,
         block_root: Hash256,
         chain: &BeaconChain<T>,
-    ) -> Result<Self, BlobError> {
-        let data_availability_boundary = chain.data_availability_boundary();
-        let da_check_required =
-            data_availability_boundary.map_or(DataAvailabilityCheckRequired::No, |boundary| {
-                if chain.epoch()? >= boundary {
-                    DataAvailabilityCheckRequired::Yes
-                } else {
-                    DataAvailabilityCheckRequired::No
-                }
-            });
-
-        match beacon_block.as_ref() {
-            // No data availability check required prior to Eip4844.
-            SignedBeaconBlock::Base(_)
-            | SignedBeaconBlock::Altair(_)
-            | SignedBeaconBlock::Capella(_)
-            | SignedBeaconBlock::Merge(_) => {
-                Ok(AvailabilityPendingBlock {
-                    block: beacon_block ,
-                    data_availability_handle: async{ Ok(Some(Blobs::NotRequired))}
-                })
-            }
-            SignedBeaconBlock::Eip4844(_) => {
-                match da_check_required {
-                    DataAvailabilityCheckRequired::Yes => {
-                        // Attempt to reconstruct empty blobs here.
-                        let blobs_sidecar = beacon_block
-                            .reconstruct_empty_blobs(Some(block_root))
-                            .map(Arc::new)?;
-                        Ok(AvailableBlock(AvailableBlockInner::BlockAndBlob(
-                            SignedBeaconBlockAndBlobsSidecar {
-                                beacon_block,
-                                blobs_sidecar,
-                            },
-                        )))
-                    }
-                    DataAvailabilityCheckRequired::No => {
-                        AvailabilityPendingBlock {
-                            block: beacon_block,
-                            data_availability_handle: async{ Ok(Some(Blobs::NotRequired))}
-                        }
-                    }
-                }
-            }
-        }
-    }
-
-    /// This function is private because an `AvailableBlock` should be
-    /// constructed via the `into_available_block` method.
-    //TODO(sean) do we want this to optionally cricumvent the beacon cache?
-    fn new_with_blobs(
-        beacon_block: Arc<SignedBeaconBlock<T::EthSpec>>,
-        blobs_sidecar: Arc<BlobsSidecar<T::EthSpec>>,
-        chain: &BeaconChain<T>,
-    ) -> Result<Self, BlobError> {
-        let data_availability_boundary = chain.data_availability_boundary();
-        let da_check_required =
-            data_availability_boundary.map_or(DataAvailabilityCheckRequired::No, |boundary| {
-                if chain.epoch()? >= boundary {
-                    DataAvailabilityCheckRequired::Yes
-                } else {
-                    DataAvailabilityCheckRequired::No
-                }
-            });
-        match beacon_block.as_ref() {
-            // This method shouldn't be called with a pre-Eip4844 block.
-            SignedBeaconBlock::Base(_)
-            | SignedBeaconBlock::Altair(_)
-            | SignedBeaconBlock::Capella(_)
-            | SignedBeaconBlock::Merge(_) => Err(BlobError::InconsistentFork),
-            SignedBeaconBlock::Eip4844(_) => {
-                match da_check_required {
-                    DataAvailabilityCheckRequired::Yes => Ok(AvailableBlock{
-                            block: beacon_block,
-                            blobs: Blobs::Available(blobs_sidecar),
-                        }
-                    ),
-                    DataAvailabilityCheckRequired::No => {
-                        // Blobs were not verified so we drop them, we'll instead just pass around
-                        // an available `Eip4844` block without blobs.
-                        Ok(AvailableBlock{
-                           block: beacon_block,
-                            blobs: Blobs::NotRequired
-                        })
-                    }
-                }
-            }
-        }
-    }
-
-}
-
-pub trait IntoBlockWrapper<E: EthSpec>: AsBlock<E> {
-    fn into_block_wrapper(self) -> BlockWrapper<E>;
-}
-
-impl<E: EthSpec> IntoBlockWrapper<E> for BlockWrapper<E> {
-    fn into_block_wrapper(self) -> BlockWrapper<E> {
-        self
+    ) -> Result<AvailableBlock<T::EthSpec>, BlobError> {
+        todo!()
     }
 }
 
-impl<E: EthSpec> IntoBlockWrapper<E> for AvailabilityPendingBlock<E> {
-    fn into_block_wrapper(self) -> BlockWrapper<E> {
-        let (block, blobs) = self.deconstruct();
-        if let Some(blobs) = blobs {
-            BlockWrapper::BlockAndBlobs(block, blobs)
-        } else {
-            BlockWrapper::Block(block)
+#[derive(Clone, Debug, PartialEq)]
+pub struct AvailableBlock<T: EthSpec> {
+    pub block: Arc<SignedBeaconBlock<T>>,
+    pub blobs: Blobs<T>,
+}
+
+impl<T: EthSpec> AvailableBlock<T> {
+    pub fn blobs(&self) -> Option<Arc<BlobSidecarList<T>>> {
+        match &self.blobs {
+            Blobs::NotRequired | Blobs::None => None,
+            Blobs::Available(blobs) => Some(blobs.clone()),
         }
     }
+
+    pub fn deconstruct(self) -> (Arc<SignedBeaconBlock<T>>, Option<Arc<BlobSidecarList<T>>>) {
+        match self.blobs {
+            Blobs::NotRequired | Blobs::None => (self.block, None),
+            Blobs::Available(blobs) => (self.block, Some(blobs)),
+        }
+    }
+}
+
+#[derive(Clone, Debug, PartialEq)]
+pub enum Blobs<E: EthSpec> {
+    /// These blobs are available.
+    Available(Arc<BlobSidecarList<E>>),
+    /// This block is from outside the data availability boundary so doesn't require
+    /// a data availability check.
+    NotRequired,
+    /// The block's `kzg_commitments` field is empty so it does not contain any blobs.
+    EmptyBlobs,
+    /// This is a block prior to the 4844 fork, so doesn't require any blobs
+    None,
 }
 
 pub trait AsBlock<E: EthSpec> {
@@ -497,59 +348,78 @@ pub trait AsBlock<E: EthSpec> {
     fn canonical_root(&self) -> Hash256;
 }
 
+#[derive(Debug, Clone)]
+pub enum BlockWrapper<E: EthSpec> {
+    /// This variant is fully available.
+    /// i.e. for pre-4844 blocks, it contains a (`SignedBeaconBlock`, `Blobs::None`) and for
+    /// post-4844 blocks, it contains a `SignedBeaconBlock` and a Blobs variant other than `Blobs::None`.
+    Available(AvailableBlock<E>),
+    /// This variant is not fully available and requires blobs to become fully available.
+    AvailabilityPending(Arc<SignedBeaconBlock<E>>),
+}
+
+impl<E: EthSpec> BlockWrapper<E> {
+    pub fn into_available_block(self) -> Option<AvailableBlock<E>> {
+        match self {
+            BlockWrapper::AvailabilityPending(_) => None,
+            BlockWrapper::Available(block) => Some(block),
+        }
+    }
+}
+
 impl<E: EthSpec> AsBlock<E> for BlockWrapper<E> {
     fn slot(&self) -> Slot {
         match self {
-            BlockWrapper::Block(block) => block.slot(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.slot(),
+            BlockWrapper::Available(block) => block.block.slot(),
+            BlockWrapper::AvailabilityPending(block) => block.slot(),
         }
     }
     fn epoch(&self) -> Epoch {
         match self {
-            BlockWrapper::Block(block) => block.epoch(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.epoch(),
+            BlockWrapper::Available(block) => block.block.epoch(),
+            BlockWrapper::AvailabilityPending(block) => block.epoch(),
         }
     }
     fn parent_root(&self) -> Hash256 {
         match self {
-            BlockWrapper::Block(block) => block.parent_root(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.parent_root(),
+            BlockWrapper::Available(block) => block.block.parent_root(),
+            BlockWrapper::AvailabilityPending(block) => block.parent_root(),
         }
     }
     fn state_root(&self) -> Hash256 {
         match self {
-            BlockWrapper::Block(block) => block.state_root(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.state_root(),
+            BlockWrapper::Available(block) => block.block.state_root(),
+            BlockWrapper::AvailabilityPending(block) => block.state_root(),
         }
     }
     fn signed_block_header(&self) -> SignedBeaconBlockHeader {
         match &self {
-            BlockWrapper::Block(block) => block.signed_block_header(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.signed_block_header(),
+            BlockWrapper::Available(block) => block.block.signed_block_header(),
+            BlockWrapper::AvailabilityPending(block) => block.signed_block_header(),
         }
     }
     fn message(&self) -> BeaconBlockRef<E> {
         match &self {
-            BlockWrapper::Block(block) => block.message(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.message(),
+            BlockWrapper::Available(block) => block.block.message(),
+            BlockWrapper::AvailabilityPending(block) => block.message(),
         }
     }
     fn as_block(&self) -> &SignedBeaconBlock<E> {
         match &self {
-            BlockWrapper::Block(block) => &block,
-            BlockWrapper::BlockAndBlobs(block, _) => &block,
+            BlockWrapper::Available(block) => &block.block,
+            BlockWrapper::AvailabilityPending(block) => &block,
         }
     }
     fn block_cloned(&self) -> Arc<SignedBeaconBlock<E>> {
         match &self {
-            BlockWrapper::Block(block) => block.clone(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.clone(),
+            BlockWrapper::Available(block) => block.block.clone(),
+            BlockWrapper::AvailabilityPending(block) => block.clone(),
         }
     }
     fn canonical_root(&self) -> Hash256 {
         match &self {
-            BlockWrapper::Block(block) => block.canonical_root(),
-            BlockWrapper::BlockAndBlob(block, _) => block.canonical_root(),
+            BlockWrapper::Available(block) => block.block.canonical_root(),
+            BlockWrapper::AvailabilityPending(block) => block.canonical_root(),
         }
     }
 }
@@ -557,131 +427,74 @@ impl<E: EthSpec> AsBlock<E> for BlockWrapper<E> {
 impl<E: EthSpec> AsBlock<E> for &BlockWrapper<E> {
     fn slot(&self) -> Slot {
         match self {
-            BlockWrapper::Block(block) => block.slot(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.slot(),
+            BlockWrapper::Available(block) => block.block.slot(),
+            BlockWrapper::AvailabilityPending(block) => block.slot(),
         }
     }
     fn epoch(&self) -> Epoch {
         match self {
-            BlockWrapper::Block(block) => block.epoch(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.epoch(),
+            BlockWrapper::Available(block) => block.block.epoch(),
+            BlockWrapper::AvailabilityPending(block) => block.epoch(),
         }
     }
     fn parent_root(&self) -> Hash256 {
         match self {
-            BlockWrapper::Block(block) => block.parent_root(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.parent_root(),
+            BlockWrapper::Available(block) => block.block.parent_root(),
+            BlockWrapper::AvailabilityPending(block) => block.parent_root(),
         }
     }
     fn state_root(&self) -> Hash256 {
         match self {
-            BlockWrapper::Block(block) => block.state_root(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.state_root(),
+            BlockWrapper::Available(block) => block.block.state_root(),
+            BlockWrapper::AvailabilityPending(block) => block.state_root(),
         }
     }
     fn signed_block_header(&self) -> SignedBeaconBlockHeader {
         match &self {
-            BlockWrapper::Block(block) => block.signed_block_header(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.signed_block_header(),
+            BlockWrapper::Available(block) => block.block.signed_block_header(),
+            BlockWrapper::AvailabilityPending(block) => block.signed_block_header(),
         }
     }
     fn message(&self) -> BeaconBlockRef<E> {
         match &self {
-            BlockWrapper::Block(block) => block.message(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.message(),
+            BlockWrapper::Available(block) => block.block.message(),
+            BlockWrapper::AvailabilityPending(block) => block.message(),
         }
     }
     fn as_block(&self) -> &SignedBeaconBlock<E> {
         match &self {
-            BlockWrapper::Block(block) => &block,
-            BlockWrapper::BlockAndBlobs(block, _) => &block,
+            BlockWrapper::Available(block) => &block.block,
+            BlockWrapper::AvailabilityPending(block) => &block,
         }
     }
     fn block_cloned(&self) -> Arc<SignedBeaconBlock<E>> {
         match &self {
-            BlockWrapper::Block(block) => block.clone(),
-            BlockWrapper::BlockAndBlobs(block, _) => block.clone(),
+            BlockWrapper::Available(block) => block.block.clone(),
+            BlockWrapper::AvailabilityPending(block) => block.clone(),
         }
     }
     fn canonical_root(&self) -> Hash256 {
         match &self {
-            BlockWrapper::Block(block) => block.canonical_root(),
-            BlockWrapper::BlockAndBlob(block, _) => block.canonical_root(),
+            BlockWrapper::Available(block) => block.block.canonical_root(),
+            BlockWrapper::AvailabilityPending(block) => block.canonical_root(),
         }
     }
 }
 
-impl<E: EthSpec> AsBlock<E> for AvailabilityPendingBlock<E> {
-    fn slot(&self) -> Slot {
-        match &self.0 {
-            AvailableBlockInner::Block(block) => block.slot(),
-            AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => {
-                block_sidecar_pair.beacon_block.slot()
-            }
-        }
-    }
-    fn epoch(&self) -> Epoch {
-        match &self.0 {
-            AvailableBlockInner::Block(block) => block.epoch(),
-            AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => {
-                block_sidecar_pair.beacon_block.epoch()
-            }
-        }
-    }
-    fn parent_root(&self) -> Hash256 {
-        match &self.0 {
-            AvailableBlockInner::Block(block) => block.parent_root(),
-            AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => {
-                block_sidecar_pair.beacon_block.parent_root()
-            }
-        }
-    }
-    fn state_root(&self) -> Hash256 {
-        match &self.0 {
-            AvailableBlockInner::Block(block) => block.state_root(),
-            AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => {
-                block_sidecar_pair.beacon_block.state_root()
-            }
-        }
-    }
-    fn signed_block_header(&self) -> SignedBeaconBlockHeader {
-        match &self.0 {
-            AvailableBlockInner::Block(block) => block.signed_block_header(),
-            AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => {
-                block_sidecar_pair.beacon_block.signed_block_header()
-            }
-        }
-    }
-    fn message(&self) -> BeaconBlockRef<E> {
-        match &self.0 {
-            AvailableBlockInner::Block(block) => block.message(),
-            AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => {
-                block_sidecar_pair.beacon_block.message()
-            }
-        }
-    }
-    fn as_block(&self) -> &SignedBeaconBlock<E> {
-        match &self.0 {
-            AvailableBlockInner::Block(block) => block,
-            AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => {
-                &block_sidecar_pair.beacon_block
-            }
-        }
-    }
-    fn block_cloned(&self) -> Arc<SignedBeaconBlock<E>> {
-        match &self.0 {
-            AvailableBlockInner::Block(block) => block.clone(),
-            AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => {
-                block_sidecar_pair.beacon_block.clone()
-            }
-        }
-    }
-    fn canonical_root(&self) -> Hash256 {
-        match &self.0 {
-            AvailableBlockInner::Block(block) => block.canonical_root(),
-            AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => {
-                block_sidecar_pair.beacon_block.canonical_root()
-            }
-        }
+impl<E: EthSpec> From<SignedBeaconBlock<E>> for BlockWrapper<E> {
+    fn from(block: SignedBeaconBlock<E>) -> Self {
+        BlockWrapper::AvailabilityPending(Arc::new(block))
+    }
+}
+
+impl<E: EthSpec> From<Arc<SignedBeaconBlock<E>>> for BlockWrapper<E> {
+    fn from(block: Arc<SignedBeaconBlock<E>>) -> Self {
+        BlockWrapper::AvailabilityPending(block)
+    }
+}
+
+impl<E: EthSpec> From<AvailableBlock<E>> for BlockWrapper<E> {
+    fn from(block: AvailableBlock<E>) -> Self {
+        BlockWrapper::Available(block)
     }
 }