Gossip verification compiles

beacon_node/beacon_chain/src/blob_verification.rs

@@ -16,10 +16,7 @@ use ssz_derive::{Decode, Encode};
 use ssz_types::VariableList;
 use tree_hash::TreeHash;
 use types::blob_sidecar::BlobIdentifier;
-use types::{
-    BeaconStateError, BlobSidecar, BlobSidecarList, CloneConfig, EthSpec, Hash256,
-    SignedBlobSidecar, Slot,
-};
+use types::{BeaconStateError, BlobSidecar, BlobSidecarList, CloneConfig, EthSpec, Hash256, Slot};
 
 /// An error occurred while validating a gossip blob.
 #[derive(Debug)]
@@ -118,7 +115,7 @@ impl<T: EthSpec> std::fmt::Display for GossipBlobError<T> {
                 write!(
                     f,
                     "BlobParentUnknown(parent_root:{})",
-                    blob_sidecar.block_parent_root
+                    blob_sidecar.block_parent_root()
                 )
             }
             other => write!(f, "{:?}", other),
@@ -147,62 +144,56 @@ pub type GossipVerifiedBlobList<T> = VariableList<
 /// the p2p network.
 #[derive(Debug)]
 pub struct GossipVerifiedBlob<T: BeaconChainTypes> {
-    blob: SignedBlobSidecar<T::EthSpec>,
+    blob: Arc<BlobSidecar<T::EthSpec>>,
 }
 
 impl<T: BeaconChainTypes> GossipVerifiedBlob<T> {
     pub fn new(
-        blob: SignedBlobSidecar<T::EthSpec>,
+        blob: Arc<BlobSidecar<T::EthSpec>>,
         chain: &BeaconChain<T>,
     ) -> Result<Self, GossipBlobError<T::EthSpec>> {
-        let blob_index = blob.message.index;
+        let blob_index = blob.index;
         validate_blob_sidecar_for_gossip(blob, blob_index, chain)
     }
     /// Construct a `GossipVerifiedBlob` that is assumed to be valid.
     ///
     /// This should ONLY be used for testing.
-    pub fn __assumed_valid(blob: SignedBlobSidecar<T::EthSpec>) -> Self {
+    pub fn __assumed_valid(blob: Arc<BlobSidecar<T::EthSpec>>) -> Self {
         Self { blob }
     }
     pub fn id(&self) -> BlobIdentifier {
-        self.blob.message.id()
+        self.blob.id()
     }
     pub fn block_root(&self) -> Hash256 {
-        self.blob.message.block_root
-    }
-    pub fn to_blob(self) -> Arc<BlobSidecar<T::EthSpec>> {
-        self.blob.message
-    }
-    pub fn as_blob(&self) -> &BlobSidecar<T::EthSpec> {
-        &self.blob.message
-    }
-    pub fn signed_blob(&self) -> SignedBlobSidecar<T::EthSpec> {
-        self.blob.clone()
+        self.blob.block_root()
     }
     pub fn slot(&self) -> Slot {
-        self.blob.message.slot
+        self.blob.slot()
     }
     pub fn index(&self) -> u64 {
-        self.blob.message.index
+        self.blob.index
     }
     pub fn kzg_commitment(&self) -> KzgCommitment {
-        self.blob.message.kzg_commitment
+        self.blob.kzg_commitment
+    }
+    pub fn cloned(&self) -> Arc<BlobSidecar<T::EthSpec>> {
+        self.blob.clone()
     }
-    pub fn proposer_index(&self) -> u64 {
-        self.blob.message.proposer_index
+    pub fn into_inner(self) -> Arc<BlobSidecar<T::EthSpec>> {
+        self.blob
     }
 }
 
 pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
-    signed_blob_sidecar: SignedBlobSidecar<T::EthSpec>,
+    blob_sidecar: Arc<BlobSidecar<T::EthSpec>>,
     subnet: u64,
     chain: &BeaconChain<T>,
 ) -> Result<GossipVerifiedBlob<T>, GossipBlobError<T::EthSpec>> {
-    let blob_slot = signed_blob_sidecar.message.slot;
-    let blob_index = signed_blob_sidecar.message.index;
-    let block_parent_root = signed_blob_sidecar.message.block_parent_root;
-    let blob_proposer_index = signed_blob_sidecar.message.proposer_index;
-    let block_root = signed_blob_sidecar.message.block_root;
+    let blob_slot = blob_sidecar.slot();
+    let blob_index = blob_sidecar.index;
+    let block_parent_root = blob_sidecar.block_parent_root();
+    let blob_proposer_index = blob_sidecar.block_proposer_index();
+    let block_root = blob_sidecar.block_root();
     let blob_epoch = blob_slot.epoch(T::EthSpec::slots_per_epoch());
 
     // Verify that the blob_sidecar was received on the correct subnet.
@@ -213,7 +204,16 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
         });
     }
 
-    let blob_root = get_blob_root(&signed_blob_sidecar);
+    // This condition is not possible if we have received the blob from the network
+    // since we only subscribe to `MaxBlobsPerBlock` subnets over gossip network.
+    // We include this check only for completeness.
+    // Getting this error would imply something very wrong with our networking decoding logic.
+    if blob_index >= T::EthSpec::max_blobs_per_block() as u64 {
+        return Err(GossipBlobError::InvalidSubnet {
+            expected: subnet,
+            received: blob_index,
+        });
+    }
 
     // Verify that the sidecar is not from a future slot.
     let latest_permissible_slot = chain
@@ -244,7 +244,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
     if chain
         .observed_blob_sidecars
         .read()
-        .is_known(&signed_blob_sidecar.message)
+        .is_known(&blob_sidecar)
         .map_err(|e| GossipBlobError::BeaconChainError(e.into()))?
     {
         return Err(GossipBlobError::RepeatBlob {
@@ -261,9 +261,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
         .fork_choice_read_lock()
         .get_block(&block_parent_root)
     else {
-        return Err(GossipBlobError::BlobParentUnknown(
-            signed_blob_sidecar.message,
-        ));
+        return Err(GossipBlobError::BlobParentUnknown(blob_sidecar));
     };
 
     if parent_block.slot >= blob_slot {
@@ -392,31 +390,6 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
         });
     }
 
-    // Signature verification
-    let signature_is_valid = {
-        let pubkey_cache = chain
-            .validator_pubkey_cache
-            .try_read_for(VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT)
-            .ok_or(BeaconChainError::ValidatorPubkeyCacheLockTimeout)
-            .map_err(GossipBlobError::BeaconChainError)?;
-
-        let pubkey = pubkey_cache
-            .get(proposer_index)
-            .ok_or_else(|| GossipBlobError::UnknownValidator(proposer_index as u64))?;
-
-        signed_blob_sidecar.verify_signature(
-            Some(blob_root),
-            pubkey,
-            &fork,
-            chain.genesis_validators_root,
-            &chain.spec,
-        )
-    };
-
-    if !signature_is_valid {
-        return Err(GossipBlobError::ProposerSignatureInvalid);
-    }
-
     // Now the signature is valid, store the proposal so we don't accept another blob sidecar
     // with the same `BlobIdentifier`.
     // It's important to double-check that the proposer still hasn't been observed so we don't
@@ -431,7 +404,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
     if chain
         .observed_blob_sidecars
         .write()
-        .observe_sidecar(&signed_blob_sidecar.message)
+        .observe_sidecar(&blob_sidecar)
         .map_err(|e| GossipBlobError::BeaconChainError(e.into()))?
     {
         return Err(GossipBlobError::RepeatBlob {
@@ -441,9 +414,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
         });
     }
 
-    Ok(GossipVerifiedBlob {
-        blob: signed_blob_sidecar,
-    })
+    Ok(GossipVerifiedBlob { blob: blob_sidecar })
 }
 
 /// Wrapper over a `BlobSidecar` for which we have completed kzg verification.
@@ -478,7 +449,7 @@ impl<T: EthSpec> KzgVerifiedBlob<T> {
         self.blob.clone()
     }
     pub fn block_root(&self) -> Hash256 {
-        self.blob.block_root
+        self.blob.block_root()
     }
     pub fn blob_index(&self) -> u64 {
         self.blob.index
@@ -537,10 +508,10 @@ pub fn verify_kzg_for_blob_list<T: EthSpec>(
 /// Returns the canonical root of the given `blob`.
 ///
 /// Use this function to ensure that we report the blob hashing time Prometheus metric.
-pub fn get_blob_root<E: EthSpec>(blob: &SignedBlobSidecar<E>) -> Hash256 {
+pub fn get_blob_root<E: EthSpec>(blob: &BlobSidecar<E>) -> Hash256 {
     let blob_root_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_BLOB_ROOT);
 
-    let blob_root = blob.message.tree_hash_root();
+    let blob_root = blob.tree_hash_root();
 
     metrics::stop_timer(blob_root_timer);
 

beacon_node/beacon_chain/src/block_verification.rs

@@ -682,12 +682,12 @@ impl<T: BeaconChainTypes> IntoGossipVerifiedBlockContents<T> for GossipVerifiedB
     fn inner_block(&self) -> &SignedBeaconBlock<T::EthSpec> {
         self.0.block.as_block()
     }
-    fn inner_blobs(&self) -> Option<SignedBlobSidecarList<T::EthSpec>> {
+    fn inner_blobs(&self) -> Option<BlobSidecarList<T::EthSpec>> {
         self.1.as_ref().map(|blobs| {
             VariableList::from(
                 blobs
                     .into_iter()
-                    .map(GossipVerifiedBlob::signed_blob)
+                    .map(GossipVerifiedBlob::cloned)
                     .collect::<Vec<_>>(),
             )
         })

beacon_node/beacon_chain/src/block_verification_types.rs

@@ -91,13 +91,6 @@ impl<E: EthSpec> RpcBlock<E> {
                 return Err(AvailabilityCheckError::MissingBlobs);
             }
             for (blob, &block_commitment) in blobs.iter().zip(block_commitments.iter()) {
-                let blob_block_root = blob.block_root;
-                if blob_block_root != block_root {
-                    return Err(AvailabilityCheckError::InconsistentBlobBlockRoots {
-                        block_root,
-                        blob_block_root,
-                    });
-                }
                 let blob_commitment = blob.kzg_commitment;
                 if blob_commitment != block_commitment {
                     return Err(AvailabilityCheckError::KzgCommitmentMismatch {

beacon_node/beacon_chain/src/data_availability_checker.rs

@@ -221,7 +221,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
     ) -> Result<Availability<T::EthSpec>, AvailabilityCheckError> {
         // Verify the KZG commitments.
         let kzg_verified_blob = if let Some(kzg) = self.kzg.as_ref() {
-            verify_kzg_for_blob(gossip_blob.to_blob(), kzg)?
+            verify_kzg_for_blob(gossip_blob.into_inner(), kzg)?
         } else {
             return Err(AvailabilityCheckError::KzgNotInitialized);
         };
@@ -310,8 +310,8 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
         block_root: Hash256,
         blob: &GossipVerifiedBlob<T>,
     ) {
-        let index = blob.as_blob().index;
-        let commitment = blob.as_blob().kzg_commitment;
+        let index = blob.index();
+        let commitment = blob.kzg_commitment();
         self.processing_cache
             .write()
             .entry(block_root)

beacon_node/beacon_chain/src/data_availability_checker/error.rs

@@ -16,10 +16,6 @@ pub enum Error {
     BlobIndexInvalid(u64),
     StoreError(store::Error),
     DecodeError(ssz::DecodeError),
-    InconsistentBlobBlockRoots {
-        block_root: Hash256,
-        blob_block_root: Hash256,
-    },
     ParentStateMissing(Hash256),
     BlockReplayError(state_processing::BlockReplayError),
     RebuildingStateCaches(BeaconStateError),
@@ -47,8 +43,7 @@ impl Error {
             Error::Kzg(_)
             | Error::BlobIndexInvalid(_)
             | Error::KzgCommitmentMismatch { .. }
-            | Error::KzgVerificationFailed
-            | Error::InconsistentBlobBlockRoots { .. } => ErrorCategory::Malicious,
+            | Error::KzgVerificationFailed => ErrorCategory::Malicious,
         }
     }
 }

beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs

@@ -125,7 +125,10 @@ impl<T: EthSpec> PendingComponents<T> {
                 for maybe_blob in self.verified_blobs.iter() {
                     if maybe_blob.is_some() {
                         return maybe_blob.as_ref().map(|kzg_verified_blob| {
-                            kzg_verified_blob.as_blob().slot.epoch(T::slots_per_epoch())
+                            kzg_verified_blob
+                                .as_blob()
+                                .slot()
+                                .epoch(T::slots_per_epoch())
                         });
                     }
                 }
@@ -421,12 +424,6 @@ impl<T: BeaconChainTypes> OverflowLRUCache<T> {
         // Initial check to ensure all provided blobs have a consistent block root.
         for blob in kzg_verified_blobs {
             let blob_block_root = blob.block_root();
-            if blob_block_root != block_root {
-                return Err(AvailabilityCheckError::InconsistentBlobBlockRoots {
-                    block_root,
-                    blob_block_root,
-                });
-            }
             if let Some(blob_opt) = fixed_blobs.get_mut(blob.blob_index() as usize) {
                 *blob_opt = Some(blob);
             }
@@ -651,7 +648,7 @@ impl<T: BeaconChainTypes> OverflowLRUCache<T> {
                         OverflowKey::Blob(_, _) => {
                             KzgVerifiedBlob::<T::EthSpec>::from_ssz_bytes(value_bytes.as_slice())?
                                 .as_blob()
-                                .slot
+                                .slot()
                                 .epoch(T::EthSpec::slots_per_epoch())
                         }
                     };

beacon_node/beacon_chain/src/observed_blob_sidecars.rs

@@ -50,36 +50,36 @@ impl<T: EthSpec> ObservedBlobSidecars<T> {
     /// This will update `self` so future calls to it indicate that this `blob_sidecar` is known.
     ///
     /// The supplied `blob_sidecar` **MUST** have completed proposer signature verification.
-    pub fn observe_sidecar(&mut self, blob_sidecar: &Arc<BlobSidecar<T>>) -> Result<bool, Error> {
+    pub fn observe_sidecar(&mut self, blob_sidecar: &BlobSidecar<T>) -> Result<bool, Error> {
         self.sanitize_blob_sidecar(blob_sidecar)?;
 
         let did_not_exist = self
             .items
-            .entry((blob_sidecar.block_root, blob_sidecar.slot))
+            .entry((blob_sidecar.block_root(), blob_sidecar.slot()))
             .or_insert_with(|| HashSet::with_capacity(T::max_blobs_per_block()))
             .insert(blob_sidecar.index);
 
         Ok(!did_not_exist)
     }
 
     /// Returns `true` if the `blob_sidecar` has already been observed in the cache within the prune window.
-    pub fn is_known(&self, blob_sidecar: &Arc<BlobSidecar<T>>) -> Result<bool, Error> {
+    pub fn is_known(&self, blob_sidecar: &BlobSidecar<T>) -> Result<bool, Error> {
         self.sanitize_blob_sidecar(blob_sidecar)?;
         let is_known = self
             .items
-            .get(&(blob_sidecar.block_root, blob_sidecar.slot))
+            .get(&(blob_sidecar.block_root(), blob_sidecar.slot()))
             .map_or(false, |set| set.contains(&blob_sidecar.index));
         Ok(is_known)
     }
 
-    fn sanitize_blob_sidecar(&self, blob_sidecar: &Arc<BlobSidecar<T>>) -> Result<(), Error> {
+    fn sanitize_blob_sidecar(&self, blob_sidecar: &BlobSidecar<T>) -> Result<(), Error> {
         if blob_sidecar.index >= T::max_blobs_per_block() as u64 {
             return Err(Error::InvalidBlobIndex(blob_sidecar.index));
         }
         let finalized_slot = self.finalized_slot;
-        if finalized_slot > 0 && blob_sidecar.slot <= finalized_slot {
+        if finalized_slot > 0 && blob_sidecar.slot() <= finalized_slot {
             return Err(Error::FinalizedBlob {
-                slot: blob_sidecar.slot,
+                slot: blob_sidecar.slot(),
                 finalized_slot,
             });
         }
@@ -107,8 +107,9 @@ mod tests {
 
     fn get_blob_sidecar(slot: u64, block_root: Hash256, index: u64) -> Arc<BlobSidecar<E>> {
         let mut blob_sidecar = BlobSidecar::empty();
-        blob_sidecar.block_root = block_root;
-        blob_sidecar.slot = slot.into();
+        // TODO(pawan): have a block root setter for tests
+        // blob_sidecar.block_root = block_root;
+        blob_sidecar.signed_block_header.message.slot = slot.into();
         blob_sidecar.index = index;
         Arc::new(blob_sidecar)
     }