diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 5881ee7d9..f82fd7e28 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/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: Sized { chain: &BeaconChain, ) -> Result, BlockError>; fn inner(&self) -> &SignedBeaconBlock; - fn parts(self) -> ArcBlockContentsTuple; + fn blobs(&self) -> Option>; } impl IntoGossipVerifiedBlock @@ -827,8 +827,8 @@ impl IntoGossipVerifiedBlock fn inner(&self) -> &SignedBeaconBlock { self.0.block.as_block() } - fn parts(self) -> ArcBlockContentsTuple { - (self.0.block.block_cloned(), self.1) + fn blobs(&self) -> Option> { + self.1.clone() } } @@ -844,9 +844,8 @@ impl IntoGossipVerifiedBlock for SignedBlockContents ArcBlockContentsTuple { - let (block, blobs) = self.deconstruct(); - (Arc::new(block), blobs) + fn blobs(&self) -> Option> { + self.blobs_cloned() } } @@ -1069,7 +1068,9 @@ impl GossipVerifiedBlock { .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 => {} }; diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index ed472e0bf..719c1ebcb 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/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>( 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::>(), - ) - }); + // 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); diff --git a/beacon_node/http_api/tests/broadcast_validation_tests.rs b/beacon_node/http_api/tests/broadcast_validation_tests.rs index ccf7439b1..e8c97a77a 100644 --- a/beacon_node/http_api/tests/broadcast_validation_tests.rs +++ b/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 */ diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 9caf827ed..7bb43d8a9 100644 --- a/common/eth2/src/types.rs +++ b/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 = ( Option>, ); -pub type ArcBlockContentsTuple = (Arc>, Option>); - /// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobSidecars`]. #[derive(Clone, Debug, Serialize, Deserialize)] #[serde(untagged)] @@ -1445,6 +1442,15 @@ impl> SignedBlockContents Option> { + match self { + SignedBlockContents::BlockAndBlobSidecars(block_and_sidecars) => { + Some(block_and_sidecars.signed_blob_sidecars.clone()) + } + SignedBlockContents::Block(_block) => None, + } + } + pub fn deconstruct(self) -> BlockContentsTuple { match self { SignedBlockContents::BlockAndBlobSidecars(block_and_sidecars) => (