fix tests

beacon_node/beacon_chain/src/block_verification.rs

@@ -69,7 +69,7 @@ use crate::{
     metrics, BeaconChain, BeaconChainError, BeaconChainTypes,
 };
 use derivative::Derivative;
-use eth2::types::{ArcBlockContentsTuple, EventKind, SignedBlockContents};
+use eth2::types::{EventKind, SignedBlockContents};
 use execution_layer::PayloadStatus;
 pub use fork_choice::{AttestationFromBlock, PayloadVerificationStatus};
 use parking_lot::RwLockReadGuard;
@@ -809,7 +809,7 @@ pub trait IntoGossipVerifiedBlock<T: BeaconChainTypes>: Sized {
         chain: &BeaconChain<T>,
     ) -> Result<GossipVerifiedBlock<T>, BlockError<T::EthSpec>>;
     fn inner(&self) -> &SignedBeaconBlock<T::EthSpec>;
-    fn parts(self) -> ArcBlockContentsTuple<T::EthSpec>;
+    fn blobs(&self) -> Option<SignedBlobSidecarList<T::EthSpec>>;
 }
 
 impl<T: BeaconChainTypes> IntoGossipVerifiedBlock<T>
@@ -827,8 +827,8 @@ impl<T: BeaconChainTypes> IntoGossipVerifiedBlock<T>
     fn inner(&self) -> &SignedBeaconBlock<T::EthSpec> {
         self.0.block.as_block()
     }
-    fn parts(self) -> ArcBlockContentsTuple<T::EthSpec> {
-        (self.0.block.block_cloned(), self.1)
+    fn blobs(&self) -> Option<SignedBlobSidecarList<T::EthSpec>> {
+        self.1.clone()
     }
 }
 
@@ -844,9 +844,8 @@ impl<T: BeaconChainTypes> IntoGossipVerifiedBlock<T> for SignedBlockContents<T::
         self.signed_block()
     }
 
-    fn parts(self) -> ArcBlockContentsTuple<T::EthSpec> {
-        let (block, blobs) = self.deconstruct();
-        (Arc::new(block), blobs)
+    fn blobs(&self) -> Option<SignedBlobSidecarList<T::EthSpec>> {
+        self.blobs_cloned()
     }
 }
 
@@ -1069,7 +1068,9 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
             .observe_proposal(block_root, block.message())
             .map_err(|e| BlockError::BeaconChainError(e.into()))?
         {
-            SeenBlock::Slashable => return Err(BlockError::Slashable),
+            SeenBlock::Slashable => {
+                return Err(BlockError::Slashable);
+            }
             SeenBlock::Duplicate => return Err(BlockError::BlockIsAlreadyKnown),
             SeenBlock::UniqueNonSlashable => {}
         };

beacon_node/http_api/src/publish_blocks.rs

@@ -1,10 +1,10 @@
 use crate::metrics;
 
-use beacon_chain::blob_verification::BlockWrapper;
+use beacon_chain::blob_verification::AsBlock;
 use beacon_chain::validator_monitor::{get_block_delay_ms, timestamp_now};
 use beacon_chain::{
     AvailabilityProcessingStatus, BeaconChain, BeaconChainError, BeaconChainTypes, BlockError,
-    GossipVerifiedBlock, IntoGossipVerifiedBlock, NotifyExecutionLayer,
+    IntoGossipVerifiedBlock, NotifyExecutionLayer,
 };
 use eth2::types::BroadcastValidation;
 use eth2::types::SignedBlockContents;
@@ -20,7 +20,7 @@ use tokio::sync::mpsc::UnboundedSender;
 use tree_hash::TreeHash;
 use types::{
     AbstractExecPayload, BeaconBlockRef, BlindedPayload, EthSpec, ExecPayload, ExecutionBlockHash,
-    FullPayload, Hash256, SignedBeaconBlock, SignedBlobSidecarList, VariableList,
+    FullPayload, Hash256, SignedBeaconBlock, SignedBlobSidecarList,
 };
 use warp::Rejection;
 
@@ -109,26 +109,21 @@ pub async fn publish_block<T: BeaconChainTypes, B: IntoGossipVerifiedBlock<T>>(
     let sender_clone = network_tx.clone();
     let log_clone = log.clone();
 
-    let (block, blobs_opt) = block_contents.parts();
-
-    let mapped_blobs = blobs_opt.clone().map(|blobs| {
-        VariableList::from(
-            blobs
-                .into_iter()
-                .map(|blob| blob.message)
-                .collect::<Vec<_>>(),
-        )
-    });
+    // We can clone this because the blobs are `Arc`'d in `BlockContents`, but the block is not,
+    // so we avoid cloning the block at this point.
+    let blobs_opt = block_contents.blobs();
 
     /* if we can form a `GossipVerifiedBlock`, we've passed our basic gossip checks */
-    let gossip_verified_block = GossipVerifiedBlock::new(
-        BlockWrapper::new(block.clone(), mapped_blobs),
-        &chain,
-    )
-    .map_err(|e| {
-        warn!(log, "Not publishing block, not gossip verified"; "slot" => slot, "error" => ?e);
-        warp_utils::reject::custom_bad_request(e.to_string())
-    })?;
+    let gossip_verified_block = block_contents
+        .into_gossip_verified_block(&chain)
+        .map_err(|e| {
+            warn!(log, "Not publishing block, not gossip verified"; "slot" => slot, "error" => ?e);
+            warp_utils::reject::custom_bad_request(e.to_string())
+        })?;
+
+    // Clone here, so we can take advantage of the `Arc`. The block in `BlockContents` is not,
+    // `Arc`'d but blobs are.
+    let block = gossip_verified_block.block.block_cloned();
 
     let block_root = block_root.unwrap_or(gossip_verified_block.block_root);
 

beacon_node/http_api/tests/broadcast_validation_tests.rs

@@ -566,9 +566,11 @@ pub async fn equivocation_gossip() {
     );
 }
 
-/// This test checks that a block that is valid from both a gossip and consensus perspective but that equivocates **late** is rejected when using `broadcast_validation=consensus_and_equivocation`.
+/// This test checks that a block that is valid from both a gossip and consensus perspective but
+/// that equivocates **late** is rejected when using `broadcast_validation=consensus_and_equivocation`.
 ///
-/// This test is unique in that we can't actually test the HTTP API directly, but instead have to hook into the `publish_blocks` code manually. This is in order to handle the late equivocation case.
+/// This test is unique in that we can't actually test the HTTP API directly, but instead have to
+/// hook into the `publish_blocks` code manually. This is in order to handle the late equivocation case.
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 pub async fn equivocation_consensus_late_equivocation() {
     /* this test targets gossip-level validation */

common/eth2/src/types.rs

@@ -9,7 +9,6 @@ use ssz_derive::Encode;
 use std::convert::TryFrom;
 use std::fmt::{self, Display};
 use std::str::{from_utf8, FromStr};
-use std::sync::Arc;
 use std::time::Duration;
 pub use types::*;
 
@@ -1410,8 +1409,6 @@ pub type BlockContentsTuple<T, Payload> = (
     Option<SignedBlobSidecarList<T>>,
 );
 
-pub type ArcBlockContentsTuple<T> = (Arc<SignedBeaconBlock<T>>, Option<SignedBlobSidecarList<T>>);
-
 /// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobSidecars`].
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(untagged)]
@@ -1445,6 +1442,15 @@ impl<T: EthSpec, Payload: AbstractExecPayload<T>> SignedBlockContents<T, Payload
         }
     }
 
+    pub fn blobs_cloned(&self) -> Option<SignedBlobSidecarList<T>> {
+        match self {
+            SignedBlockContents::BlockAndBlobSidecars(block_and_sidecars) => {
+                Some(block_and_sidecars.signed_blob_sidecars.clone())
+            }
+            SignedBlockContents::Block(_block) => None,
+        }
+    }
+
     pub fn deconstruct(self) -> BlockContentsTuple<T, Payload> {
         match self {
             SignedBlockContents::BlockAndBlobSidecars(block_and_sidecars) => (