From 472d153aec4b89550f26a5e3394d979e1d5c79c4 Mon Sep 17 00:00:00 2001 From: Emilia Hane Date: Sun, 19 Feb 2023 21:55:19 +0100 Subject: [PATCH] Decouple blobs using data_availability_handle --- beacon_node/beacon_chain/src/beacon_chain.rs | 127 ++++- .../beacon_chain/src/blob_verification.rs | 477 +++++++++--------- .../beacon_chain/src/block_verification.rs | 4 +- beacon_node/beacon_chain/src/kzg_utils.rs | 12 +- beacon_node/http_api/src/publish_blocks.rs | 2 +- .../beacon_processor/worker/sync_methods.rs | 2 +- consensus/types/src/blob_sidecar.rs | 4 +- 7 files changed, 360 insertions(+), 268 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 6dd9868daf8..965bc82b9cc 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -7,7 +7,10 @@ use crate::attester_cache::{AttesterCache, AttesterCacheKey}; use crate::beacon_proposer_cache::compute_proposer_duties_from_head; use crate::beacon_proposer_cache::BeaconProposerCache; use crate::blob_cache::BlobCache; -use crate::blob_verification::{AsBlock, AvailabilityPendingBlock, AvailableBlock, BlockWrapper, IntoAvailableBlock}; +use crate::blob_verification::{ + AsBlock, AvailabilityPendingBlock, AvailableBlock, BlockWrapper, ExecutedBlock, + ExecutedBlockAvailabiityHandle, IntoAvailablilityPendingBlock, +}; use crate::block_times_cache::BlockTimesCache; use crate::block_verification::{ check_block_is_finalized_descendant, check_block_relevancy, get_block_root, @@ -2684,7 +2687,10 @@ impl BeaconChain { /// /// Returns an `Err` if the given block was invalid, or an error was encountered during /// verification. - pub async fn process_block>( + pub async fn process_block< + A: IntoAvailabilityPendingBlock, + B: IntoExecutionPendingBlock, + >( self: &Arc, block_root: Hash256, unverified_block: B, @@ -2763,7 +2769,7 @@ impl BeaconChain { /// /// An error is returned if the block was unable to be imported. It may be partially imported /// (i.e., this function is not atomic). - async fn import_execution_pending_block( + async fn import_execution_pending_block( self: Arc, execution_pending_block: ExecutionPendingBlock, count_unrealized: CountUnrealized, @@ -2817,29 +2823,102 @@ impl BeaconChain { ); } - let available_block = block.into_available_block()?; - - let chain = self.clone(); - let block_hash = self - .spawn_blocking_handle( - move || { - chain.import_block( - available_block, - block_root, - state, - confirmed_state_roots, - payload_verification_status, - count_unrealized, - parent_block, - parent_eth1_finalization_data, - consensus_context, + let block = block.into_availability_pending_block(block_root, &self); + match block.try_into(&self) { + // If this block has blobs, at best they have all come over network while waiting on + // input from execution layer. + Ok(available_block) => { + let chain = self.clone(); + let block_hash = self + .spawn_blocking_handle( + move || { + chain.import_block( + available_block, + block_root, + state, + confirmed_state_roots, + payload_verification_status, + count_unrealized, + parent_block, + parent_eth1_finalization_data, + consensus_context, + ) + }, + "payload_verification_handle", ) - }, - "payload_verification_handle", - ) - .await??; + .await??; - Ok(block_hash) + Ok(block_hash) + } + Err(BlobError::PendingAvailability) => { + // move block to a background thread, like we use migrator, to do following. + + let (cache_block, blob_cache_update) = block.cache_item(); + self.block_pending_availability_cache + .put(&block_root, cache_block); + let ExecutedBlockAvailabilityHandle { expected_blobs, tx } = blob_cache_update; + + // atomically check if expected blobs number blobs have been received over + // network, otherwise update expected blobs cache item. Always check on + // insert into blobs cache item's blobs list if expected number of blobs have + // arrived. + let blob_cache_entry = self + .blobs_pending_availability_cache + .get(&block_root) + .write(); + if *blob.blobs.len() >= expected_blobs { + tx.send(()); + } else { + *blob_cahce_entry.expected_blobs = Some(expected_blobs); + *blob_cahce_entry.sender = Some(tx); + } + drop(blob_cache_entry); + + /// **** When blobs arrive over network, don't call `process_block` from the background thread again, but instead use + + /* + let ExecutedBlock { + block, + block_root, + state, + confirmed_state_roots, + payload_verification_status, + count_unrealized, + parent_block, + parent_eth1_finalization_data, + consensus_context, + } = cache_block.block; + + let available_block = block.try_into(&self.chain)?; + + let chain = self.chain.clone() + let block_hash = self + .spawn_blocking_handle( + move || { + chain.import_block( + available_block, + block_root, + state, + confirmed_state_roots, + payload_verification_status, + count_unrealized, + parent_block, + parent_eth1_finalization_data, + consensus_context, + ) + }, + "payload_verification_handle", + ) + .await??; + + Ok(block_hash) + */ + + /// **** + Err(BlobError::PendingAvailability) + } + Err(e) => Err(e), + } } /// Accepts a fully-verified block and imports it into the chain without performing any diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index e5a686a42e6..1a9759b4374 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -1,17 +1,26 @@ use derivative::Derivative; +use kzg::Kzg; use slot_clock::SlotClock; -use std::sync::Arc; -use tokio::task::JoinHandle; +use ssz_types::VariableList; +use std::{sync::Arc, task::Poll}; +use store::blob_sidecar::SignedBlobSidecar; +use tokio::{ + sync::oneshot, + task::JoinHandle, + time::{timeout, Duration, Timeout}, +}; use crate::beacon_chain::{BeaconChain, BeaconChainTypes, MAXIMUM_GOSSIP_CLOCK_DISPARITY}; use crate::block_verification::PayloadVerificationOutcome; use crate::{kzg_utils, BeaconChainError, BlockError}; -use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions; +use state_processing::{ + per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions, + ConsensusContext, +}; use types::signed_beacon_block::BlobReconstructionError; use types::{ - BeaconBlockRef, BeaconStateError, BlobsSidecar, EthSpec, Hash256, KzgCommitment, - SignedBeaconBlock, SignedBeaconBlockAndBlobsSidecar, SignedBeaconBlockHeader, Slot, - Transactions, + BeaconBlockRef, BeaconStateError, EthSpec, Hash256, KzgCommitment, SignedBeaconBlock, + SignedBeaconBlockHeader, SignedSignedBlobSidecar, Slot, Transactions, }; use types::{Epoch, ExecPayload}; @@ -62,8 +71,24 @@ pub enum BlobError { BeaconChainError(BeaconChainError), /// No blobs for the specified block where we would expect blobs. UnavailableBlobs, + /// Blobs are missing to verify availability. + PendingAvailability, /// Blobs provided for a pre-Eip4844 fork. InconsistentFork, + /// A cached pending-availability block failed to receive blobs. + ReceiveBlobsFailed(String), +} + +impl From for BlobError { + fn from(e: time::error::Elapsed) -> Self { + BlobError::ReceiveBlobsFailed(e.into_string()) + } +} + +impl From for BlobError { + fn from(e: oneshot::error::RecvError) -> Self { + BlobError::ReceiveBlobsFailed(e.into_string()) + } } impl From for BlobError { @@ -87,43 +112,46 @@ impl From for BlobError { } } +/// A wrapper around a [`SignedBlobSidecar`] that indicates it has been approved for re-gossiping +/// on the p2p network. +pub struct GossipVerifiedBlob { + pub blob: Arc>, +} + pub fn validate_blob_for_gossip( - block_wrapper: BlockWrapper, + blob: BlobWrapper, block_root: Hash256, chain: &BeaconChain, -) -> Result, BlobError> { - if let BlockWrapper::BlockAndBlobs(ref block, ref blobs_sidecar) = block_wrapper { - let blob_slot = blobs_sidecar.beacon_block_slot; - // Do not gossip or process blobs from future or past slots. - let latest_permissible_slot = chain - .slot_clock - .now_with_future_tolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY) - .ok_or(BeaconChainError::UnableToReadSlot)?; - if blob_slot > latest_permissible_slot { - return Err(BlobError::FutureSlot { - message_slot: latest_permissible_slot, - latest_permissible_slot: blob_slot, - }); - } - - if blob_slot != block.slot() { - return Err(BlobError::SlotMismatch { - blob_slot, - block_slot: block.slot(), - }); - } - } - - block_wrapper.into_availablilty_pending_block(block_root, chain) +) -> Result { + let blob_slot = blobs_sidecar.beacon_block_slot; + // Do not gossip or process blobs from future or past slots. + let latest_permissible_slot = chain + .slot_clock + .now_with_future_tolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY) + .ok_or(BeaconChainError::UnableToReadSlot)?; + if blob_slot > latest_permissible_slot { + return Err(BlobError::FutureSlot { + message_slot: latest_permissible_slot, + latest_permissible_slot: blob_slot, + }); + } + + if blob_slot != block.slot() { + return Err(BlobError::SlotMismatch { + blob_slot, + block_slot: block.slot(), + }); + } + GossipVerifiedBlob(blob) } fn verify_data_availability( - blob_sidecar: &BlobsSidecar, + blob_sidecar: &VariableList>, kzg_commitments: &[KzgCommitment], transactions: &Transactions, block_slot: Slot, block_root: Hash256, - chain: &BeaconChain, + kzg: &Kzg, ) -> Result<(), BlobError> { if verify_kzg_commitments_against_transactions::(transactions, kzg_commitments) .is_err() @@ -132,10 +160,7 @@ fn verify_data_availability( } // Validatate that the kzg proof is valid against the commitments and blobs - let kzg = chain - .kzg - .as_ref() - .ok_or(BlobError::TrustedSetupNotInitialized)?; + let kzg = kzg.ok_or(BlobError::TrustedSetupNotInitialized)?; if !kzg_utils::validate_blobs_sidecar( kzg, @@ -151,49 +176,29 @@ fn verify_data_availability( Ok(()) } -/// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobsSidecar`]. This makes no -/// claims about data availability and should not be used in consensus. This struct is useful in -/// networking when we want to send blocks around without consensus checks. +/// A wrapper over a [`SignedBeaconBlock`]. This makes no claims about data availability and +/// should not be used in consensus. This struct is useful in networking when we want to send +/// blocks around without consensus checks. #[derive(Clone, Debug, Derivative)] #[derivative(PartialEq, Hash(bound = "E: EthSpec"))] -pub enum BlockWrapper { - Block(Arc>), - BlockAndBlobs(Arc>, Arc>), - BlockAndBlobsFuture(Arc>, DataAvailabilityHandle), -} +pub struct BlockWrapper(Arc>); -impl BlockWrapper { - pub fn new( - block: Arc>, - blobs_sidecar: Option>>, - ) -> Self { - if let Some(blobs_sidecar) = blobs_sidecar { - BlockWrapper::BlockAndBlobs(block, blobs_sidecar) - } else { - BlockWrapper::Block(block) - } - } -} - -impl From> for BlockWrapper { - fn from(block: SignedBeaconBlock) -> Self { - BlockWrapper::Block(Arc::new(block)) +impl From>> for BlockWrapper { + fn from(block: Arc>) -> Self { + BlockWrapper(block) } } -impl From> for BlockWrapper { - fn from(block: SignedBeaconBlockAndBlobsSidecar) -> Self { - let SignedBeaconBlockAndBlobsSidecar { - beacon_block, - blobs_sidecar, - } = block; - BlockWrapper::BlockAndBlobs(beacon_block, blobs_sidecar) - } -} +/// A wrapper over a [`SignedBlobSidecar`]. This makes no claims about data availability and +/// should not be used in consensus. This struct is useful in networking when we want to send +/// blocks around without consensus checks. +#[derive(Clone, Debug, Derivative)] +#[derivative(PartialEq, Hash(bound = "E: EthSpec"))] +pub struct BlobWrapper(Arc>); -impl From>> for BlockWrapper { - fn from(block: Arc>) -> Self { - BlockWrapper::Block(block) +impl From>> for BlobWrapper { + fn from(blob: Arc>) -> Self { + BlobWrapper(blob) } } @@ -203,12 +208,20 @@ pub enum DataAvailabilityCheckRequired { No, } -impl BlockWrapper { +pub trait IntoAvailabilityPendingBlock { fn into_availablilty_pending_block( self, block_root: Hash256, chain: &BeaconChain, - ) -> Result, BlobError> { + ) -> AvailablilityPendingBlock; +} + +impl IntoAvailabilityPendingBlock for BlockWrapper { + fn into_availablilty_pending_block( + self, + block_root: Hash256, + chain: &BeaconChain, + ) -> AvailablilityPendingBlock { let data_availability_boundary = chain.data_availability_boundary(); let da_check_required = data_availability_boundary.map_or(DataAvailabilityCheckRequired::No, |boundary| { @@ -218,159 +231,52 @@ impl BlockWrapper { DataAvailabilityCheckRequired::No } }); - match self { - BlockWrapper::Block(block) => { - AvailabilityPendingBlock::new(block, block_root, da_check_required) - } - BlockWrapper::BlockAndBlobs(block, blobs_sidecar) => { - if matches!(da_check_required, DataAvailabilityCheckRequired::Yes) { - let kzg_commitments = block - .message() - .body() - .blob_kzg_commitments() - .map_err(|_| BlobError::KzgCommitmentMissing)?; - let transactions = block - .message() - .body() - .execution_payload_eip4844() - .map(|payload| payload.transactions()) - .map_err(|_| BlobError::TransactionsMissing)? - .ok_or(BlobError::TransactionsMissing)?; - verify_data_availability( - &blobs_sidecar, - kzg_commitments, - transactions, - block.slot(), - block_root, - chain, - )?; - } - - AvailabilityPendingBlock::new_with_blobs(block, blobs_sidecar, da_check_required) - } - } + Ok(AvailabilityPendingBlock { + block, + da_check_required, + }) } } -pub trait IntoAvailableBlock { - fn into_available_block( - self, - block_root: Hash256, - chain: &BeaconChain, - ) -> Result, BlobError>; -} - -/// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobsSidecar`]. An -/// `AvailableBlock` has passed any required data availability checks and should be used in -/// consensus. This newtype wraps `AvailableBlockInner` to ensure data availability checks -/// cannot be circumvented on construction. #[derive(Clone, Debug, Derivative)] #[derivative(PartialEq, Hash(bound = "E: EthSpec"))] pub struct AvailabilityPendingBlock { block: Arc>, - data_availability_handle: DataAvailabilityHandle, + da_check_required: bool, } -/// Used to await the result of data availability check. -type DataAvailabilityHandle = JoinHandle>>, BlobError>>; +pub struct AvailabilityPendingBlob(SignedBlobSidecar); + +/// Used to await blobs from the network. +type DataAvailabilityHandle = TimeOut>; +/// A wrapper over a [`SignedBeaconBlock`] and its blobs if it has any. An [`AvailableBlock`] has +/// passed any required data availability checks and should be used in consensus. This newtype +/// wraps [`AvailableBlockInner`] to ensure data availability checks cannot be circumvented on +/// construction. #[derive(Clone, Debug, Derivative)] #[derivative(PartialEq, Hash(bound = "E: EthSpec"))] -pub struct AvailableBlock { - block: Arc>, - blobs: Blobs, -} - -pub enum Blobs { - /// These blobs are available. - Available(Arc>), - /// 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, -} +pub struct AvailableBlock(AvailableBlockInner); /// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobsSidecar`]. #[derive(Clone, Debug, Derivative)] #[derivative(PartialEq, Hash(bound = "E: EthSpec"))] enum AvailableBlockInner { Block(Arc>), - BlockAndBlob(SignedBeaconBlockAndBlobsSidecar), + /// The container for any block which requires a data availability check at time of + /// construction. + BlockAndBlobs( + AvailableBlockAndBlobs, + VariableList, E::MaxBlobsPerBlock>, + ), } -impl AvailabilityPendingBlock { - pub fn new( - beacon_block: Arc>, - block_root: Hash256, - da_check_required: DataAvailabilityCheckRequired, - ) -> Result { - match beacon_block.as_ref() { - // No data availability check required prior to Eip4844. - SignedBeaconBlock::Base(_) - | SignedBeaconBlock::Altair(_) - | SignedBeaconBlock::Capella(_) - | SignedBeaconBlock::Merge(_) => { - Ok(AvailableBlock(AvailableBlockInner::Block(beacon_block))) - } - 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)?; - return Ok(AvailableBlock(AvailableBlockInner::BlockAndBlob( - SignedBeaconBlockAndBlobsSidecar { - beacon_block, - blobs_sidecar, - }, - ))); - } - DataAvailabilityCheckRequired::No => { - Ok(AvailableBlock(AvailableBlockInner::Block(beacon_block))) - } - } - } - } - } - - /// This function is private because an `AvailableBlock` should be - /// constructed via the `into_available_block` method. - fn new_with_blobs( - beacon_block: Arc>, - blobs_sidecar: Arc>, - da_check_required: DataAvailabilityCheckRequired, - ) -> Result { - 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( - AvailableBlockInner::BlockAndBlob(SignedBeaconBlockAndBlobsSidecar { - beacon_block, - 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(AvailableBlockInner::Block(beacon_block))) - } - } - } - } - } - +impl AvailableBlock { pub fn blobs(&self) -> Option>> { match &self.0 { AvailableBlockInner::Block(_) => None, - AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => { - Some(block_sidecar_pair.blobs_sidecar.clone()) + AvailableBlockInner::BlockAndBlobs(block_sidecar_pair) => { + Some(block_sidecar_pair.1.clone()) } } } @@ -378,17 +284,134 @@ impl AvailabilityPendingBlock { pub fn deconstruct(self) -> (Arc>, Option>>) { match self.0 { AvailableBlockInner::Block(block) => (block, None), - AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => { - let SignedBeaconBlockAndBlobsSidecar { - beacon_block, - blobs_sidecar, - } = block_sidecar_pair; - (beacon_block, Some(blobs_sidecar)) + AvailableBlockInner::BlockAndBlobs(block_sidecar_pair) => { + (block_sidecar_pair.0, Some(block_sidecar_pair.1)) } } } } +pub trait TryIntoAvailableBlock { + /// Verifies blobs against kzg-commitments in block if data availability check is required. + fn try_into(self, chain: &BeaconChain) -> Result, BlobError>; +} + +impl TryIntoAvailableBlock for &AvailabilityPendingBlock { + fn try_into(self, chain: &BeaconChain) -> Result, BlobError> { + let block = self.block; + if self.da_check_required { + let blobs = match chain.blobs_pending_availability_cache.get(&block_root) { + Some(blobs) => { + self.verify_blobs(&blobs, chain.kzg)?; + blobs + } + None => VariableList::empty(), + }; + Ok(AvailableBlock(AvailableBlockInner::BlockAndBlobs( + block, blobs, + ))) + } else { + Ok(AvailableBlock(AvailableBlockInner::Block(block))) + } + } +} + +/// The maximum time an [`AvailabilityPendingBlock`] is cached in seconds. +pub const AVAILABILITY_PENDING_BLOCK_CACHE_ITEM_TIMEOUT: u64 = 5; + +/// A block that has passed payload verification and is waiting for its blobs. +pub struct ExecutedBlock { + block: AvailabilityPendingBlock, + data_availability_handle: DataAvailabilityHandle, + state: BeaconState, + confirmed_state_roots: Vec, + payload_verification_status: PayloadVerificationStatus, + count_unrealized: CountUnrealized, + parent_block: SignedBeaconBlock, + parent_eth1_finalization_data: Eth1FinalizationData, + consensus_context: ConsensusContext, +} + +pub struct ExecutedBlockAvailabilityHandle { + /// Sender to advance [`DataAvailabilityHandle`] in an [`ExecutedBlock`]'s + /// [`AvailabilityPendingExecutedBlock`]. + tx: oneshot::Sender, + /// The number of blobs to wait for before sending blobs on tx. + expected_blobs: usize, +} + +impl AvailabilityPendingBlock { + pub fn verify_blobs( + &self, + blobs: &VariableList, E::MaxBlobsPerBlock>, + kzg: &Kzg, + ) -> Result<(), BlobError> { + let kzg_commitments = block + .message() + .body() + .blob_kzg_commitments() + .map_err(|_| BlobError::KzgCommitmentMissing)?; + if kzg_commitments.len() != blobs.len() { + return Err(BlobError::PendingAvailability); + } + let transactions = block + .message() + .body() + .execution_payload_eip4844() + .map(|payload| payload.transactions()) + .map_err(|_| BlobError::TransactionsMissing)? + .ok_or(BlobError::TransactionsMissing)?; + verify_data_availability( + blobs, + kzg_commitments, + transactions, + block.slot(), + block_root, + kzg, + ) + } + + /// Converts an [`AvailabilityPendingBlock`] to a cache item that stalls until receiving blobs + /// input from the network. Returns a pending-availability block cache item and a handle to + /// send it blobs. + pub fn cache_item( + self, + state: BeaconState, + confirmed_state_roots: Vec, + payload_verification_status: PayloadVerificationStatus, + count_unrealized: CountUnrealized, + parent_block: SignedBeaconBlock, + parent_eth1_finalization_data: Eth1FinalizationData, + consensus_context: ConsensusContext, + ) -> (ExecutedBlock, ExecutedBlockAvailabilityHandle) { + let (tx, rx) = oneshot::channel::<()>(); + let data_availability_handle = Some(tokio::time::timeout( + Duration::from_secs(AVAILABILITY_PENDING_BLOCK_CACHE_ITEM_TIMEOUT), + rx, + )); + let expected_blobs = block + .message() + .body() + .blob_kzg_commitments() + .map_err(|_| BlobError::KzgCommitmentMissing)? + .len(); + ( + ExecutedBlock { + block: self, + data_availability_handle, + state, + confirmed_state_roots, + payload_verification_status, + count_unrealized, + parent_block, + parent_eth1_finalization_data, + consensus_context, + }, + ExecutedBlockAvailabilityHandle { tx, expected_blobs }, + ) + } +} + pub trait IntoBlockWrapper: AsBlock { fn into_block_wrapper(self) -> BlockWrapper; } @@ -527,65 +550,57 @@ impl AsBlock for AvailabilityPendingBlock { fn slot(&self) -> Slot { match &self.0 { AvailableBlockInner::Block(block) => block.slot(), - AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => { - block_sidecar_pair.beacon_block.slot() - } + AvailableBlockInner::BlockAndBlobs(block_sidecar_pair) => block_sidecar_pair.0.slot(), } } fn epoch(&self) -> Epoch { match &self.0 { AvailableBlockInner::Block(block) => block.epoch(), - AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => { - block_sidecar_pair.beacon_block.epoch() - } + AvailableBlockInner::BlockAndBlobs(block_sidecar_pair) => block_sidecar_pair.0.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() + AvailableBlockInner::BlockAndBlobs(block_sidecar_pair) => { + block_sidecar_pair.0.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() + AvailableBlockInner::BlockAndBlobs(block_sidecar_pair) => { + block_sidecar_pair.0.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() + AvailableBlockInner::BlockAndBlobs(block_sidecar_pair) => { + block_sidecar_pair.0.signed_block_header() } } } fn message(&self) -> BeaconBlockRef { match &self.0 { AvailableBlockInner::Block(block) => block.message(), - AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => { - block_sidecar_pair.beacon_block.message() + AvailableBlockInner::BlockAndBlobs(block_sidecar_pair) => { + block_sidecar_pair.0.message() } } } fn as_block(&self) -> &SignedBeaconBlock { match &self.0 { AvailableBlockInner::Block(block) => &block, - AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => { - &block_sidecar_pair.beacon_block - } + AvailableBlockInner::BlockAndBlobs(block_sidecar_pair) => &block_sidecar_pair.0, } } fn block_cloned(&self) -> Arc> { match &self.0 { AvailableBlockInner::Block(block) => block.clone(), - AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => { - block_sidecar_pair.beacon_block.clone() - } + AvailableBlockInner::BlockAndBlobs(block_sidecar_pair) => block_sidecar_pair.0.clone(), } } } diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index fe853e1a5ab..a9143c9d8d8 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -45,7 +45,7 @@ //! ``` use crate::blob_verification::{ validate_blob_for_gossip, AsBlock, AvailabilityPendingBlock, AvailableBlock, BlobError, - BlockWrapper, IntoAvailableBlock, IntoBlockWrapper, + BlockWrapper, IntoAvailablilityPendingBlock, IntoBlockWrapper, }; use crate::eth1_finalization_cache::Eth1FinalizationData; use crate::execution_payload::{ @@ -679,7 +679,7 @@ pub struct ExecutionPendingBlock< /// Used to allow functions to accept blocks at various stages of verification. pub trait IntoExecutionPendingBlock< T: BeaconChainTypes, - B: IntoAvailableBlock = AvailableBlock, + B: IntoAvailablilityPendingBlock = AvailableBlock, >: Sized { fn into_execution_pending_block( diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs index 8589a6fe436..603c234d74f 100644 --- a/beacon_node/beacon_chain/src/kzg_utils.rs +++ b/beacon_node/beacon_chain/src/kzg_utils.rs @@ -13,16 +13,16 @@ pub fn validate_blobs_sidecar( slot: Slot, beacon_block_root: Hash256, expected_kzg_commitments: &[KzgCommitment], - blobs_sidecar: &BlobsSidecar, + blob_sidecars: Vec>, ) -> Result { - if slot != blobs_sidecar.beacon_block_slot - || beacon_block_root != blobs_sidecar.beacon_block_root - || blobs_sidecar.blobs.len() != expected_kzg_commitments.len() + if slot != blob_sidecars.beacon_block_slot + || beacon_block_root != blob_sidecars.beacon_block_root + || blob_sidecars.blobs.len() != expected_kzg_commitments.len() { return Ok(false); } - let blobs = blobs_sidecar + let blobs = blob_sidecars .blobs .into_iter() .map(|blob| ssz_blob_to_crypto_blob::(blob.clone())) // TODO(pawan): avoid this clone @@ -31,7 +31,7 @@ pub fn validate_blobs_sidecar( kzg.verify_aggregate_kzg_proof( &blobs, expected_kzg_commitments, - blobs_sidecar.kzg_aggregated_proof, + blob_sidecars.kzg_aggregated_proof, ) } diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index a9a0bc9c6be..7fd276cbb2c 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -1,5 +1,5 @@ use crate::metrics; -use beacon_chain::blob_verification::{AsBlock, BlockWrapper, IntoAvailableBlock}; +use beacon_chain::blob_verification::{AsBlock, BlockWrapper, IntoAvailablilityPendingBlock}; use beacon_chain::validator_monitor::{get_block_delay_ms, timestamp_now}; use beacon_chain::NotifyExecutionLayer; use beacon_chain::{BeaconChain, BeaconChainTypes, BlockError, CountUnrealized}; diff --git a/beacon_node/network/src/beacon_processor/worker/sync_methods.rs b/beacon_node/network/src/beacon_processor/worker/sync_methods.rs index 8d5bd53aea1..58344a588b8 100644 --- a/beacon_node/network/src/beacon_processor/worker/sync_methods.rs +++ b/beacon_node/network/src/beacon_processor/worker/sync_methods.rs @@ -7,7 +7,7 @@ use crate::beacon_processor::DuplicateCache; use crate::metrics; use crate::sync::manager::{BlockProcessType, SyncMessage}; use crate::sync::{BatchProcessResult, ChainId}; -use beacon_chain::blob_verification::{AsBlock, BlockWrapper, IntoAvailableBlock}; +use beacon_chain::blob_verification::{AsBlock, BlockWrapper, IntoAvailablilityPendingBlock}; use beacon_chain::CountUnrealized; use beacon_chain::{ BeaconChainError, BeaconChainTypes, BlockError, ChainSegmentResult, HistoricalBlockError, diff --git a/consensus/types/src/blob_sidecar.rs b/consensus/types/src/blob_sidecar.rs index e641b9bdd52..ae85a61e975 100644 --- a/consensus/types/src/blob_sidecar.rs +++ b/consensus/types/src/blob_sidecar.rs @@ -6,7 +6,6 @@ use kzg::KzgProof; use serde_derive::{Deserialize, Serialize}; use ssz::Encode; use ssz_derive::{Decode, Encode}; -use ssz_types::VariableList; use test_random_derive::TestRandom; use tree_hash_derive::TreeHash; @@ -57,7 +56,6 @@ impl BlobSidecar { Encode, Decode, TreeHash, - Default, TestRandom, Derivative, arbitrary::Arbitrary, @@ -66,7 +64,7 @@ impl BlobSidecar { #[arbitrary(bound = "T: EthSpec")] #[derivative(PartialEq, Hash(bound = "T: EthSpec"))] pub struct SignedBlobSidecar { - pub message: BlobsSidecar, + pub message: BlobSidecar, pub signature: Signature, }