Sidecar inclusion api updates

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -122,7 +122,6 @@ use tree_hash::TreeHash;
 use types::beacon_state::CloneConfig;
 use types::blob_sidecar::{BlobSidecarList, FixedBlobSidecarList};
 use types::payload::BlockProductionVersion;
-use types::sidecar::BlobItems;
 use types::*;
 
 pub type ForkChoiceError = fork_choice::Error<crate::ForkChoiceStoreError>;
@@ -490,10 +489,37 @@ pub enum BeaconBlockResponseType<T: EthSpec> {
     Blinded(BeaconBlockResponse<T, BlindedPayload<T>>),
 }
 
+impl<E: EthSpec> BeaconBlockResponseType<E> {
+    pub fn fork_name(&self, spec: &ChainSpec) -> Result<ForkName, InconsistentFork> {
+        Ok(match self {
+            BeaconBlockResponseType::Full(resp) => resp.block.to_ref().fork_name(spec)?,
+            BeaconBlockResponseType::Blinded(resp) => resp.block.to_ref().fork_name(spec)?,
+        })
+    }
+
+    pub fn execution_payload_value(&self) -> Option<Uint256> {
+        match self {
+            BeaconBlockResponseType::Full(resp) => resp.execution_payload_value,
+            BeaconBlockResponseType::Blinded(resp) => resp.execution_payload_value,
+        }
+    }
+
+    pub fn consensus_block_value(&self) -> Option<u64> {
+        match self {
+            BeaconBlockResponseType::Full(resp) => resp.consensus_block_value,
+            BeaconBlockResponseType::Blinded(resp) => resp.consensus_block_value,
+        }
+    }
+
+    pub fn is_blinded(&self) -> bool {
+        matches!(self, BeaconBlockResponseType::Blinded(_))
+    }
+}
+
 pub struct BeaconBlockResponse<T: EthSpec, Payload: AbstractExecPayload<T>> {
     pub block: BeaconBlock<T, Payload>,
     pub state: BeaconState<T>,
-    pub blob_items: Option<(KzgProofs<T>, <Payload::Sidecar as Sidecar<T>>::BlobItems)>,
+    pub blob_items: Option<(KzgProofs<T>, BlobsList<T>)>,
     pub execution_payload_value: Option<Uint256>,
     pub consensus_block_value: Option<u64>,
 }
@@ -4384,23 +4410,27 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         &self,
         canonical_forkchoice_params: ForkchoiceUpdateParameters,
     ) -> Result<ForkchoiceUpdateParameters, Error> {
-        self.overridden_forkchoice_update_params_or_failure_reason(&canonical_forkchoice_params)
-            .or_else(|e| match e {
-                ProposerHeadError::DoNotReOrg(reason) => {
-                    trace!(
-                        self.log,
-                        "Not suppressing fork choice update";
-                        "reason" => %reason,
-                    );
-                    Ok(canonical_forkchoice_params)
-                }
-                ProposerHeadError::Error(e) => Err(e),
-            })
+        self.overridden_forkchoice_update_params_or_failure_reason(
+            &canonical_forkchoice_params,
+            false,
+        )
+        .or_else(|e| match e {
+            ProposerHeadError::DoNotReOrg(reason) => {
+                trace!(
+                    self.log,
+                    "Not suppressing fork choice update";
+                    "reason" => %reason,
+                );
+                Ok(canonical_forkchoice_params)
+            }
+            ProposerHeadError::Error(e) => Err(e),
+        })
     }
 
-    fn overridden_forkchoice_update_params_or_failure_reason(
+    pub fn overridden_forkchoice_update_params_or_failure_reason(
         &self,
         canonical_forkchoice_params: &ForkchoiceUpdateParameters,
+        testing: bool,
     ) -> Result<ForkchoiceUpdateParameters, ProposerHeadError<Error>> {
         let _timer = metrics::start_timer(&metrics::FORK_CHOICE_OVERRIDE_FCU_TIMES);
 
@@ -4452,7 +4482,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         }
 
         // Only attempt a re-org if we have a proposer registered for the re-org slot.
-        let proposing_at_re_org_slot = {
+        let proposing_at_re_org_slot = testing || {
             // The proposer shuffling has the same decision root as the next epoch attestation
             // shuffling. We know our re-org block is not on the epoch boundary, so it has the
             // same proposer shuffling as the head (but not necessarily the parent which may lie
@@ -4507,7 +4537,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         // current slot, which would be necessary for determining its weight.
         let head_block_late =
             self.block_observed_after_attestation_deadline(head_block_root, head_slot);
-        if !head_block_late {
+        if !head_block_late && !testing {
             return Err(DoNotReOrg::HeadNotLate.into());
         }
 
@@ -5148,40 +5178,38 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         let blobs_verification_timer =
             metrics::start_timer(&metrics::BLOCK_PRODUCTION_BLOBS_VERIFICATION_TIMES);
         let blob_item = match (blobs_opt, proofs_opt) {
-            (Some(blobs_or_blobs_roots), Some(proofs)) => {
+            (Some(blobs), Some(proofs)) => {
                 let expected_kzg_commitments =
                     block.body().blob_kzg_commitments().map_err(|_| {
                         BlockProductionError::InvalidBlockVariant(
                             "deneb block does not contain kzg commitments".to_string(),
                         )
                     })?;
 
-                if expected_kzg_commitments.len() != blobs_or_blobs_roots.len() {
+                if expected_kzg_commitments.len() != blobs.len() {
                     return Err(BlockProductionError::MissingKzgCommitment(format!(
                         "Missing KZG commitment for slot {}. Expected {}, got: {}",
                         block.slot(),
-                        blobs_or_blobs_roots.len(),
+                        blobs.len(),
                         expected_kzg_commitments.len()
                     )));
                 }
 
                 let kzg_proofs = Vec::from(proofs);
 
-                if let Some(blobs) = blobs_or_blobs_roots.blobs() {
-                    let kzg = self
-                        .kzg
-                        .as_ref()
-                        .ok_or(BlockProductionError::TrustedSetupNotInitialized)?;
-                    kzg_utils::validate_blobs::<T::EthSpec>(
-                        kzg,
-                        expected_kzg_commitments,
-                        blobs.iter().collect(),
-                        &kzg_proofs,
-                    )
-                    .map_err(BlockProductionError::KzgError)?;
-                }
+                let kzg = self
+                    .kzg
+                    .as_ref()
+                    .ok_or(BlockProductionError::TrustedSetupNotInitialized)?;
+                kzg_utils::validate_blobs::<T::EthSpec>(
+                    kzg,
+                    expected_kzg_commitments,
+                    blobs.iter().collect(),
+                    &kzg_proofs,
+                )
+                .map_err(BlockProductionError::KzgError)?;
 
-                Some((kzg_proofs.into(), blobs_or_blobs_roots))
+                Some((kzg_proofs.into(), blobs))
             }
             _ => None,
         };

beacon_node/beacon_chain/src/blob_verification.rs

@@ -8,6 +8,7 @@ use crate::block_verification::cheap_state_advance_to_obtain_committees;
 use crate::kzg_utils::{validate_blob, validate_blobs};
 use crate::{metrics, BeaconChainError};
 use kzg::{Error as KzgError, Kzg, KzgCommitment};
+use merkle_proof::MerkleTreeError;
 use slog::{debug, warn};
 use ssz_derive::{Decode, Encode};
 use ssz_types::VariableList;
@@ -123,6 +124,13 @@ pub enum GossipBlobError<T: EthSpec> {
     ///
     /// The blob sidecar is invalid and the peer is faulty.
     KzgError(kzg::Error),
+
+    /// The kzg commitment inclusion proof failed.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The blob sidecar is invalid
+    InclusionProof(MerkleTreeError),
 }
 
 impl<T: EthSpec> std::fmt::Display for GossipBlobError<T> {
@@ -184,7 +192,7 @@ impl<T: BeaconChainTypes> GossipVerifiedBlob<T> {
     /// This should ONLY be used for testing.
     pub fn __assumed_valid(blob: Arc<BlobSidecar<T::EthSpec>>) -> Self {
         Self {
-            blob: KzgVerifiedBlob { blob: blob },
+            blob: KzgVerifiedBlob { blob },
         }
     }
     pub fn id(&self) -> BlobIdentifier {
@@ -265,8 +273,8 @@ pub fn verify_kzg_for_blob<T: EthSpec>(
     blob: Arc<BlobSidecar<T>>,
     kzg: &Kzg,
 ) -> Result<KzgVerifiedBlob<T>, KzgError> {
-    let _timer = crate::metrics::start_timer(&crate::metrics::KZG_VERIFICATION_SINGLE_TIMES);
-    let _ = validate_blob::<T>(kzg, &blob.blob, blob.kzg_commitment, blob.kzg_proof)?;
+    let _timer = metrics::start_timer(&crate::metrics::KZG_VERIFICATION_SINGLE_TIMES);
+    validate_blob::<T>(kzg, &blob.blob, blob.kzg_commitment, blob.kzg_proof)?;
 
     Ok(KzgVerifiedBlob { blob })
 }
@@ -359,7 +367,10 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
     }
 
     // Verify the inclusion proof in the sidecar
-    if !blob_sidecar.verify_blob_sidecar_inclusion_proof() {
+    if !blob_sidecar
+        .verify_blob_sidecar_inclusion_proof()
+        .map_err(GossipBlobError::InclusionProof)?
+    {
         return Err(GossipBlobError::InvalidInclusionProof);
     }
 
@@ -521,7 +532,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
         .as_ref()
         .ok_or(GossipBlobError::KzgNotInitialized)?;
     let kzg_verified_blob =
-        verify_kzg_for_blob(blob_sidecar, &kzg).map_err(GossipBlobError::KzgError)?;
+        verify_kzg_for_blob(blob_sidecar, kzg).map_err(GossipBlobError::KzgError)?;
 
     Ok(GossipVerifiedBlob {
         blob: kzg_verified_blob,

beacon_node/beacon_chain/src/block_verification.rs

@@ -99,11 +99,11 @@ use store::{Error as DBError, HotStateSummary, KeyValueStore, StoreOp};
 use task_executor::JoinHandle;
 use tree_hash::TreeHash;
 use types::{
-    BeaconBlockRef, BeaconState, BeaconStateError, BlobSidecarList, ChainSpec, CloneConfig, Epoch,
-    EthSpec, ExecutionBlockHash, Hash256, InconsistentFork, PublicKey, PublicKeyBytes,
-    RelativeEpoch, SignedBeaconBlock, SignedBeaconBlockHeader, Slot,
+    BeaconBlockRef, BeaconState, BeaconStateError, ChainSpec, CloneConfig, Epoch, EthSpec,
+    ExecutionBlockHash, Hash256, InconsistentFork, PublicKey, PublicKeyBytes, RelativeEpoch,
+    SignedBeaconBlock, SignedBeaconBlockHeader, Slot,
 };
-use types::{BlobSidecar, ExecPayload, Sidecar};
+use types::{BlobSidecar, ExecPayload};
 
 pub const POS_PANDA_BANNER: &str = r#"
     ,,,         ,,,                                               ,,,         ,,,
@@ -669,7 +669,6 @@ pub trait IntoGossipVerifiedBlockContents<T: BeaconChainTypes>: Sized {
         chain: &BeaconChain<T>,
     ) -> Result<GossipVerifiedBlockContents<T>, BlockContentsError<T::EthSpec>>;
     fn inner_block(&self) -> &SignedBeaconBlock<T::EthSpec>;
-    fn inner_blobs(&self) -> Option<BlobSidecarList<T::EthSpec>>;
 }
 
 impl<T: BeaconChainTypes> IntoGossipVerifiedBlockContents<T> for GossipVerifiedBlockContents<T> {
@@ -682,46 +681,28 @@ impl<T: BeaconChainTypes> IntoGossipVerifiedBlockContents<T> for GossipVerifiedB
     fn inner_block(&self) -> &SignedBeaconBlock<T::EthSpec> {
         self.0.block.as_block()
     }
-    fn inner_blobs(&self) -> Option<BlobSidecarList<T::EthSpec>> {
-        self.1.as_ref().map(|blobs| {
-            VariableList::from(
-                blobs
-                    .into_iter()
-                    .map(GossipVerifiedBlob::cloned)
-                    .collect::<Vec<_>>(),
-            )
-        })
-    }
 }
 
 impl<T: BeaconChainTypes> IntoGossipVerifiedBlockContents<T> for SignedBlockContents<T::EthSpec> {
     fn into_gossip_verified_block(
         self,
         chain: &BeaconChain<T>,
     ) -> Result<GossipVerifiedBlockContents<T>, BlockContentsError<T::EthSpec>> {
-        let (block, blob_items) = self.deconstruct();
+        let (block, blobs) = self.deconstruct();
 
-        let gossip_verified_blobs = blob_items
+        let gossip_verified_blobs = blobs
             .map(|(kzg_proofs, blobs)| {
-                let expected_kzg_commitments =
-                    block.message().body().blob_kzg_commitments().map_err(|e| {
-                        BlockContentsError::BlockError(BlockError::BeaconChainError(
-                            BeaconChainError::BeaconStateError(e),
-                        ))
-                    })?;
-                let sidecars = BlobSidecar::build_sidecar(
-                    blobs,
-                    &block,
-                    expected_kzg_commitments,
-                    kzg_proofs.into(),
-                )
-                .map_err(BlockContentsError::SidecarError)?;
-                Ok::<_, BlockContentsError<T::EthSpec>>(VariableList::from(
-                    sidecars
-                        .into_iter()
-                        .map(|blob| GossipVerifiedBlob::new(blob, chain))
-                        .collect::<Result<Vec<_>, _>>()?,
-                ))
+                let signed_block_header = block.signed_block_header();
+                let mut gossip_verified_blobs = vec![];
+                for (i, (kzg_proof, blob)) in kzg_proofs.iter().zip(blobs).enumerate() {
+                    let blob =
+                        BlobSidecar::new(i, blob, signed_block_header.clone(), &block, *kzg_proof)
+                            .map_err(BlockContentsError::SidecarError)?;
+                    let gossip_verified_blob = GossipVerifiedBlob::new(Arc::new(blob), chain)?;
+                    gossip_verified_blobs.push(gossip_verified_blob);
+                }
+                let gossip_verified_blobs = VariableList::from(gossip_verified_blobs);
+                Ok::<_, BlockContentsError<T::EthSpec>>(gossip_verified_blobs)
             })
             .transpose()?;
         let gossip_verified_block = GossipVerifiedBlock::new(Arc::new(block), chain)?;
@@ -732,10 +713,6 @@ impl<T: BeaconChainTypes> IntoGossipVerifiedBlockContents<T> for SignedBlockCont
     fn inner_block(&self) -> &SignedBeaconBlock<T::EthSpec> {
         self.signed_block()
     }
-
-    fn inner_blobs(&self) -> Option<BlobSidecarList<T::EthSpec>> {
-        self.blobs_sidecar_list()
-    }
 }
 
 /// Implemented on types that can be converted into a `ExecutionPendingBlock`.

beacon_node/beacon_chain/src/block_verification_types.rs

@@ -8,7 +8,7 @@ use derivative::Derivative;
 use ssz_types::VariableList;
 use state_processing::ConsensusContext;
 use std::sync::Arc;
-use types::blob_sidecar::{BlobIdentifier, FixedBlobSidecarList};
+use types::blob_sidecar::{BlobIdentifier, BlobSidecarError, FixedBlobSidecarList};
 use types::{
     BeaconBlockRef, BeaconState, BlindedPayload, BlobSidecarList, Epoch, EthSpec, Hash256,
     SignedBeaconBlock, SignedBeaconBlockHeader, Slot,
@@ -295,8 +295,7 @@ pub type GossipVerifiedBlockContents<T> =
 pub enum BlockContentsError<T: EthSpec> {
     BlockError(BlockError<T>),
     BlobError(GossipBlobError<T>),
-    // TODO(pawan): absorb into one of the above types
-    SidecarError(String),
+    SidecarError(BlobSidecarError),
 }
 
 impl<T: EthSpec> From<BlockError<T>> for BlockContentsError<T> {
@@ -321,7 +320,7 @@ impl<T: EthSpec> std::fmt::Display for BlockContentsError<T> {
                 write!(f, "BlobError({})", err)
             }
             BlockContentsError::SidecarError(err) => {
-                write!(f, "SidecarError({})", err)
+                write!(f, "SidecarError({:?})", err)
             }
         }
     }

beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs

@@ -753,7 +753,7 @@ mod test {
     use std::ops::AddAssign;
     use store::{HotColdDB, ItemStore, LevelDB, StoreConfig};
     use tempfile::{tempdir, TempDir};
-    use types::{ChainSpec, ExecPayload, MinimalEthSpec, Sidecar};
+    use types::{ChainSpec, ExecPayload, MinimalEthSpec};
 
     const LOW_VALIDATOR_COUNT: usize = 32;
 
@@ -919,17 +919,7 @@ mod test {
         info!(log, "done printing kzg commitments");
 
         let gossip_verified_blobs = if let Some((kzg_proofs, blobs)) = maybe_blobs {
-            let sidecars = BlobSidecar::build_sidecar(
-                blobs,
-                &block,
-                block
-                    .message()
-                    .body()
-                    .blob_kzg_commitments()
-                    .expect("should be deneb fork"),
-                kzg_proofs.into(),
-            )
-            .unwrap();
+            let sidecars = BlobSidecar::build_sidecars(blobs, &block, kzg_proofs).unwrap();
             Vec::from(sidecars)
                 .into_iter()
                 .map(|sidecar| {