From a58c6ed9624eaad972e8826a72c47268783190e5 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Thu, 16 Mar 2023 20:55:21 +0530 Subject: [PATCH] more progress --- beacon_node/beacon_chain/src/beacon_chain.rs | 143 +++--- .../beacon_chain/src/blob_verification.rs | 415 +++++------------- .../beacon_chain/src/block_verification.rs | 101 ++--- consensus/types/src/signed_blob.rs | 21 +- 4 files changed, 246 insertions(+), 434 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index d403cbc597..e324a80ef4 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -7,12 +7,14 @@ 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, BlobError, BlockWrapper, IntoAvailableBlock, Blobs}; +use crate::blob_verification::{ + AsBlock, AvailableBlock, BlobError, Blobs, BlockWrapper, IntoAvailableBlock, +}; use crate::block_times_cache::BlockTimesCache; use crate::block_verification::{ check_block_is_finalized_checkpoint_or_descendant, check_block_relevancy, get_block_root, - signature_verify_chain_segment, BlockError, ExecutionPendingBlock, GossipVerifiedBlock, - IntoExecutionPendingBlock, PayloadVerificationOutcome, POS_PANDA_BANNER, ExecutedBlock, + signature_verify_chain_segment, BlockError, ExecutedBlock, ExecutionPendingBlock, + GossipVerifiedBlock, IntoExecutionPendingBlock, PayloadVerificationOutcome, POS_PANDA_BANNER, }; pub use crate::canonical_head::{CanonicalHead, CanonicalHeadRwLock}; use crate::chain_config::ChainConfig; @@ -82,6 +84,7 @@ use slasher::Slasher; use slog::{crit, debug, error, info, trace, warn, Logger}; use slot_clock::SlotClock; use ssz::Encode; +use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions; use state_processing::{ common::get_attesting_indices_from_state, per_block_processing, @@ -102,16 +105,14 @@ use std::io::prelude::*; use std::marker::PhantomData; use std::sync::Arc; use std::time::{Duration, Instant}; -use tokio::task::JoinHandle; -use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions; use store::iter::{BlockRootsIterator, ParentRootBlockIterator, StateRootsIterator}; use store::{ DatabaseBlock, Error as DBError, HotColdDB, KeyValueStore, KeyValueStoreOp, StoreItem, StoreOp, }; use task_executor::{ShutdownReason, TaskExecutor}; +use tokio::task::JoinHandle; use tree_hash::TreeHash; use types::beacon_state::CloneConfig; -use types::blobs_sidecar::KzgCommitments; use types::consts::eip4844::MIN_EPOCHS_FOR_BLOBS_SIDECARS_REQUESTS; use types::consts::merge::INTERVALS_PER_SLOT; use types::*; @@ -440,8 +441,7 @@ pub struct BeaconChain { /// Provides monitoring of a set of explicitly defined validators. pub validator_monitor: RwLock>, pub blob_cache: BlobCache, - pub blob_cache: BlobCache, - pub kzg: Option>, + pub kzg: Option>, } type BeaconBlockAndState = (BeaconBlock, BeaconState); @@ -1082,34 +1082,8 @@ impl BeaconChain { pub fn get_blobs( &self, block_root: &Hash256, - data_availability_boundary: Epoch, - ) -> Result>, Error> { - match self.store.get_blobs(block_root)? { - Some(blobs) => Ok(Some(blobs)), - None => { - // Check for the corresponding block to understand whether we *should* have blobs. - self.get_blinded_block(block_root)? - .map(|block| { - // If there are no KZG commitments in the block, we know the sidecar should - // be empty. - let expected_kzg_commitments = - match block.message().body().blob_kzg_commitments() { - Ok(kzg_commitments) => kzg_commitments, - Err(_) => return Err(Error::NoKzgCommitmentsFieldOnBlock), - }; - if expected_kzg_commitments.is_empty() { - Ok(BlobsSidecar::empty_from_parts(*block_root, block.slot())) - } else if data_availability_boundary <= block.epoch() { - // We should have blobs for all blocks younger than the boundary. - Err(Error::BlobsUnavailable) - } else { - // We shouldn't have blobs for blocks older than the boundary. - Err(Error::BlobsOlderThanDataAvailabilityBoundary(block.epoch())) - } - }) - .transpose() - } - } + ) -> Result>, Error> { + self.store.get_blobs(block_root) } pub fn get_blinded_block( @@ -2708,8 +2682,8 @@ impl BeaconChain { metrics::inc_counter(&metrics::BLOCK_PROCESSING_REQUESTS); let slot = unverified_block.block().slot(); + let chain = self.clone(); - let execution_pending = unverified_block.into_execution_pending_block( block_root, &chain, @@ -2721,11 +2695,13 @@ impl BeaconChain { .into_executed_block(execution_pending, count_unrealized) .await?; - let chain = self.clone(); - - // Check if the executed block has all it's blobs available to qualify as a fully + // Check if the executed block has all it's blobs available to qualify as a fully // available block - let import_block = if let Ok(blobs) = self.gossip_blob_cache.lock().blobs(executed_block.block_root) { + let import_block = if let Ok(blobs) = self + .gossip_blob_cache + .lock() + .blobs(executed_block.block_root) + { self.import_available_block(executed_block, blobs, count_unrealized) } else { return Ok(BlockProcessingResult::AvailabilityPending(executed_block)); @@ -2839,7 +2815,7 @@ impl BeaconChain { confirmed_state_roots, parent_eth1_finalization_data, consensus_context, - payload_verification_outcome + payload_verification_outcome, }) } @@ -2851,7 +2827,7 @@ impl BeaconChain { async fn import_available_block( self: Arc, executed_block: ExecutedBlock, - blobs: Blobs + blobs: Blobs, count_unrealized: CountUnrealized, ) -> Result> { let ExecutedBlock { @@ -2863,16 +2839,15 @@ impl BeaconChain { payload_verification_outcome, parent_eth1_finalization_data, consensus_context, - } = execution_pending_block; - + } = executed_block; let chain = self.clone(); - + let available_block = AvailableBlock { - block: block, - blobs: blobs + block: block.block_cloned(), + blobs: blobs, }; - + let block_hash = self .spawn_blocking_handle( move || { @@ -2881,7 +2856,7 @@ impl BeaconChain { block_root, state, confirmed_state_roots, - payload_verification_status, + payload_verification_outcome.payload_verification_status, count_unrealized, parent_block, parent_eth1_finalization_data, @@ -2895,7 +2870,7 @@ impl BeaconChain { Ok(block_hash) } - /// Accepts a fully-verified block and imports it into the chain without performing any + /// Accepts a fully-verified and available block and imports it into the chain without performing any /// additional verification. /// /// An error is returned if the block was unable to be imported. It may be partially imported @@ -2920,7 +2895,7 @@ impl BeaconChain { // ----------------------------------------------------------------------------------------- let current_slot = self.slot()?; let current_epoch = current_slot.epoch(T::EthSpec::slots_per_epoch()); - let block = signed_block.message(); + let block = signed_block.block.message(); let post_exec_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_POST_EXEC_PROCESSING); // Check against weak subjectivity checkpoint. @@ -2957,9 +2932,14 @@ impl BeaconChain { let mut fork_choice = self.canonical_head.fork_choice_write_lock(); // Do not import a block that doesn't descend from the finalized root. - let signed_block = - check_block_is_finalized_checkpoint_or_descendant(self, &fork_choice, signed_block)?; - let block = signed_block.message(); + let signed_block = check_block_is_finalized_checkpoint_or_descendant( + self, + &fork_choice, + BlockWrapper::from(signed_block), + )?; + // TODO(pawan): fix this atrocity + let signed_block = signed_block.into_available_block().unwrap(); + let block = signed_block.block.message(); // Register the new block with the fork choice service. { @@ -3089,14 +3069,12 @@ impl BeaconChain { // margin, or younger (of higher epoch number). if block_epoch >= import_boundary { if let Some(blobs) = blobs { - if !blobs.blobs.is_empty() { - //FIXME(sean) using this for debugging for now - info!( - self.log, "Writing blobs to store"; - "block_root" => ?block_root - ); - ops.push(StoreOp::PutBlobs(block_root, blobs)); - } + //FIXME(sean) using this for debugging for now + info!( + self.log, "Writing blobs to store"; + "block_root" => ?block_root + ); + ops.push(StoreOp::PutBlobs(block_root, blobs)); } } } @@ -4911,7 +4889,7 @@ impl BeaconChain { )), )?; - kzg_utils::compute_blob_kzg_proof::(kzg, blob, kzg_commitment.clone()) + kzg_utils::compute_blob_kzg_proof::(kzg, blob, *kzg_commitment) .map_err(BlockProductionError::KzgError) }) .collect::, BlockProductionError>>() @@ -6196,19 +6174,22 @@ impl BeaconChain { .unwrap_or(false)) } - pub async fn check_data_availability(&self, block: Arc>) -> Result, Error> { - 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)?; + pub async fn check_data_availability( + &self, + block: Arc>, + ) -> Result, Error> { + 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)?; if verify_kzg_commitments_against_transactions::(transactions, kzg_commitments) .is_err() @@ -6216,9 +6197,7 @@ impl BeaconChain { return Err(BlobError::TransactionCommitmentMismatch); } - self.blob_cache - - // Validatate that the kzg proof is valid against the commitments and blobs + // Validate that the kzg proof is valid against the commitments and blobs let kzg = self .kzg .as_ref() @@ -6231,7 +6210,7 @@ impl BeaconChain { kzg_commitments, blob_sidecar, ) - .map_err(BlobError::KzgError)? + .map_err(BlobError::KzgError)? { return Err(BlobError::InvalidKzgProof); } @@ -6287,4 +6266,4 @@ impl ChainSegmentResult { ChainSegmentResult::Successful { .. } => Ok(()), } } -} +} \ No newline at end of file diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index bfc0242208..35fb4c1429 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -1,8 +1,5 @@ -use derivative::Derivative; use slot_clock::SlotClock; use std::sync::Arc; -use tokio::task::JoinHandle; -use ssz_types::VariableList; use crate::beacon_chain::{ BeaconChain, BeaconChainTypes, MAXIMUM_GOSSIP_CLOCK_DISPARITY, @@ -11,12 +8,9 @@ use crate::beacon_chain::{ use crate::BeaconChainError; use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions; use types::{ - BeaconBlockRef, BeaconStateError, BlobsSidecar, EthSpec, Hash256, KzgCommitment, - SignedBeaconBlock, SignedBeaconBlockAndBlobsSidecar, SignedBeaconBlockHeader, - SignedBlobSidecar, Slot, Transactions, + BeaconBlockRef, BeaconStateError, BlobSidecarList, Epoch, EthSpec, Hash256, KzgCommitment, + SignedBeaconBlock, SignedBeaconBlockHeader, SignedBlobSidecar, Slot, Transactions, }; -use types::{Epoch, ExecPayload}; -use types::blob_sidecar::BlobSidecar; #[derive(Debug)] pub enum BlobError { @@ -191,9 +185,9 @@ pub fn validate_blob_sidecar_for_gossip( }; let blob_proposer_index = blob_sidecar.message.proposer_index; - if proposer_index != blob_proposer_index { + if proposer_index != blob_proposer_index as usize { return Err(BlobError::ProposerIndexMismatch { - sidecar: blob_proposer_index, + sidecar: blob_proposer_index as usize, local: proposer_index, }); } @@ -249,11 +243,11 @@ pub fn validate_blob_sidecar_for_gossip( } pub fn verify_data_availability( - blob_sidecar: &BlobsSidecar, + blob_sidecar: &BlobSidecarList, kzg_commitments: &[KzgCommitment], transactions: &Transactions, - block_slot: Slot, - block_root: Hash256, + _block_slot: Slot, + _block_root: Hash256, chain: &BeaconChain, ) -> Result<(), BlobError> { if verify_kzg_commitments_against_transactions::(transactions, kzg_commitments) @@ -263,7 +257,7 @@ pub fn verify_data_availability( } // Validatate that the kzg proof is valid against the commitments and blobs - let kzg = chain + let _kzg = chain .kzg .as_ref() .ok_or(BlobError::TrustedSetupNotInitialized)?; @@ -289,25 +283,6 @@ pub enum DataAvailabilityCheckRequired { No, } -impl BlockWrapper { - fn into_availablilty_pending_block( - self, - block_root: Hash256, - chain: &BeaconChain, - ) -> Result, BlobError> { - match self { - BlockWrapper::Block(block) => { - AvailabilityPendingBlock::new(block, block_root, da_check_required) - } - BlockWrapper::BlockAndBlobs(block, blobs_sidecar) => { - - - AvailabilityPendingBlock::new_with_blobs(block, blobs_sidecar, da_check_required) - } - } - } -} - pub trait IntoAvailableBlock { fn into_available_block( self, @@ -316,173 +291,49 @@ pub trait IntoAvailableBlock { ) -> 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. -#[derive(Clone, Debug, Derivative)] -#[derivative(PartialEq, Hash(bound = "T: BeaconChainTypes"))] -pub struct AvailabilityPendingBlock { - block: Arc>, - data_availability_handle: DataAvailabilityHandle, -} - -/// Used to await the result of data availability check. -type DataAvailabilityHandle = JoinHandle>, BlobError>>; - -#[derive(Clone, Debug, Derivative)] -#[derivative(PartialEq, Hash(bound = "T: BeaconChainTypes"))] -pub struct AvailableBlock { - block: Arc>, - blobs: Blobs, -} - -impl AvailableBlock { - pub fn blobs(&self) -> Option>> { - match &self.blobs { - Blobs::NotRequired | Blobs::None => None, - Blobs::Available(block_sidecar) => { - Some(block_sidecar.clone()) - } - } - } - - pub fn deconstruct(self) -> (Arc>, Option>>) { - match self.blobs { - Blobs::NotRequired | Blobs::None => (self.block, None), - Blobs::Available(blob_sidecars) => { - (self.block, Some(blob_sidecars)) - } - } - } -} - -pub enum Blobs { - /// These blobs are available. - Available(VariableList>, E::MaxBlobsPerBlock>), - /// 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, -} - -//TODO(sean) add block root to the availability pending block? -impl AvailabilityPendingBlock { - pub fn new( - beacon_block: Arc>, +impl IntoAvailableBlock for BlockWrapper { + fn into_available_block( + self, block_root: Hash256, chain: &BeaconChain, - ) -> Result { - let data_availability_boundary = chain.data_availability_boundary(); - let da_check_required = - data_availability_boundary.map_or(DataAvailabilityCheckRequired::No, |boundary| { - if chain.epoch()? >= boundary { - DataAvailabilityCheckRequired::Yes - } else { - DataAvailabilityCheckRequired::No - } - }); - - match beacon_block.as_ref() { - // No data availability check required prior to Eip4844. - SignedBeaconBlock::Base(_) - | SignedBeaconBlock::Altair(_) - | SignedBeaconBlock::Capella(_) - | SignedBeaconBlock::Merge(_) => { - Ok(AvailabilityPendingBlock { - block: beacon_block , - data_availability_handle: async{ Ok(Some(Blobs::NotRequired))} - }) - } - 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)?; - Ok(AvailableBlock(AvailableBlockInner::BlockAndBlob( - SignedBeaconBlockAndBlobsSidecar { - beacon_block, - blobs_sidecar, - }, - ))) - } - DataAvailabilityCheckRequired::No => { - AvailabilityPendingBlock { - block: beacon_block, - data_availability_handle: async{ Ok(Some(Blobs::NotRequired))} - } - } - } - } - } - } - - /// This function is private because an `AvailableBlock` should be - /// constructed via the `into_available_block` method. - //TODO(sean) do we want this to optionally cricumvent the beacon cache? - fn new_with_blobs( - beacon_block: Arc>, - blobs_sidecar: Arc>, - chain: &BeaconChain, - ) -> Result { - let data_availability_boundary = chain.data_availability_boundary(); - let da_check_required = - data_availability_boundary.map_or(DataAvailabilityCheckRequired::No, |boundary| { - if chain.epoch()? >= boundary { - DataAvailabilityCheckRequired::Yes - } else { - DataAvailabilityCheckRequired::No - } - }); - 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{ - block: beacon_block, - blobs: Blobs::Available(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{ - block: beacon_block, - blobs: Blobs::NotRequired - }) - } - } - } - } - } - -} - -pub trait IntoBlockWrapper: AsBlock { - fn into_block_wrapper(self) -> BlockWrapper; -} - -impl IntoBlockWrapper for BlockWrapper { - fn into_block_wrapper(self) -> BlockWrapper { - self + ) -> Result, BlobError> { + todo!() } } -impl IntoBlockWrapper for AvailabilityPendingBlock { - fn into_block_wrapper(self) -> BlockWrapper { - let (block, blobs) = self.deconstruct(); - if let Some(blobs) = blobs { - BlockWrapper::BlockAndBlobs(block, blobs) - } else { - BlockWrapper::Block(block) +#[derive(Clone, Debug, PartialEq)] +pub struct AvailableBlock { + pub block: Arc>, + pub blobs: Blobs, +} + +impl AvailableBlock { + pub fn blobs(&self) -> Option>> { + match &self.blobs { + Blobs::NotRequired | Blobs::None => None, + Blobs::Available(blobs) => Some(blobs.clone()), } } + + pub fn deconstruct(self) -> (Arc>, Option>>) { + match self.blobs { + Blobs::NotRequired | Blobs::None => (self.block, None), + Blobs::Available(blobs) => (self.block, Some(blobs)), + } + } +} + +#[derive(Clone, Debug, PartialEq)] +pub enum Blobs { + /// These blobs are available. + Available(Arc>), + /// This block is from outside the data availability boundary so doesn't require + /// a data availability check. + NotRequired, + /// The block's `kzg_commitments` field is empty so it does not contain any blobs. + EmptyBlobs, + /// This is a block prior to the 4844 fork, so doesn't require any blobs + None, } pub trait AsBlock { @@ -497,59 +348,78 @@ pub trait AsBlock { fn canonical_root(&self) -> Hash256; } +#[derive(Debug, Clone)] +pub enum BlockWrapper { + /// This variant is fully available. + /// i.e. for pre-4844 blocks, it contains a (`SignedBeaconBlock`, `Blobs::None`) and for + /// post-4844 blocks, it contains a `SignedBeaconBlock` and a Blobs variant other than `Blobs::None`. + Available(AvailableBlock), + /// This variant is not fully available and requires blobs to become fully available. + AvailabilityPending(Arc>), +} + +impl BlockWrapper { + pub fn into_available_block(self) -> Option> { + match self { + BlockWrapper::AvailabilityPending(_) => None, + BlockWrapper::Available(block) => Some(block), + } + } +} + impl AsBlock for BlockWrapper { fn slot(&self) -> Slot { match self { - BlockWrapper::Block(block) => block.slot(), - BlockWrapper::BlockAndBlobs(block, _) => block.slot(), + BlockWrapper::Available(block) => block.block.slot(), + BlockWrapper::AvailabilityPending(block) => block.slot(), } } fn epoch(&self) -> Epoch { match self { - BlockWrapper::Block(block) => block.epoch(), - BlockWrapper::BlockAndBlobs(block, _) => block.epoch(), + BlockWrapper::Available(block) => block.block.epoch(), + BlockWrapper::AvailabilityPending(block) => block.epoch(), } } fn parent_root(&self) -> Hash256 { match self { - BlockWrapper::Block(block) => block.parent_root(), - BlockWrapper::BlockAndBlobs(block, _) => block.parent_root(), + BlockWrapper::Available(block) => block.block.parent_root(), + BlockWrapper::AvailabilityPending(block) => block.parent_root(), } } fn state_root(&self) -> Hash256 { match self { - BlockWrapper::Block(block) => block.state_root(), - BlockWrapper::BlockAndBlobs(block, _) => block.state_root(), + BlockWrapper::Available(block) => block.block.state_root(), + BlockWrapper::AvailabilityPending(block) => block.state_root(), } } fn signed_block_header(&self) -> SignedBeaconBlockHeader { match &self { - BlockWrapper::Block(block) => block.signed_block_header(), - BlockWrapper::BlockAndBlobs(block, _) => block.signed_block_header(), + BlockWrapper::Available(block) => block.block.signed_block_header(), + BlockWrapper::AvailabilityPending(block) => block.signed_block_header(), } } fn message(&self) -> BeaconBlockRef { match &self { - BlockWrapper::Block(block) => block.message(), - BlockWrapper::BlockAndBlobs(block, _) => block.message(), + BlockWrapper::Available(block) => block.block.message(), + BlockWrapper::AvailabilityPending(block) => block.message(), } } fn as_block(&self) -> &SignedBeaconBlock { match &self { - BlockWrapper::Block(block) => &block, - BlockWrapper::BlockAndBlobs(block, _) => &block, + BlockWrapper::Available(block) => &block.block, + BlockWrapper::AvailabilityPending(block) => &block, } } fn block_cloned(&self) -> Arc> { match &self { - BlockWrapper::Block(block) => block.clone(), - BlockWrapper::BlockAndBlobs(block, _) => block.clone(), + BlockWrapper::Available(block) => block.block.clone(), + BlockWrapper::AvailabilityPending(block) => block.clone(), } } fn canonical_root(&self) -> Hash256 { match &self { - BlockWrapper::Block(block) => block.canonical_root(), - BlockWrapper::BlockAndBlob(block, _) => block.canonical_root(), + BlockWrapper::Available(block) => block.block.canonical_root(), + BlockWrapper::AvailabilityPending(block) => block.canonical_root(), } } } @@ -557,131 +427,74 @@ impl AsBlock for BlockWrapper { impl AsBlock for &BlockWrapper { fn slot(&self) -> Slot { match self { - BlockWrapper::Block(block) => block.slot(), - BlockWrapper::BlockAndBlobs(block, _) => block.slot(), + BlockWrapper::Available(block) => block.block.slot(), + BlockWrapper::AvailabilityPending(block) => block.slot(), } } fn epoch(&self) -> Epoch { match self { - BlockWrapper::Block(block) => block.epoch(), - BlockWrapper::BlockAndBlobs(block, _) => block.epoch(), + BlockWrapper::Available(block) => block.block.epoch(), + BlockWrapper::AvailabilityPending(block) => block.epoch(), } } fn parent_root(&self) -> Hash256 { match self { - BlockWrapper::Block(block) => block.parent_root(), - BlockWrapper::BlockAndBlobs(block, _) => block.parent_root(), + BlockWrapper::Available(block) => block.block.parent_root(), + BlockWrapper::AvailabilityPending(block) => block.parent_root(), } } fn state_root(&self) -> Hash256 { match self { - BlockWrapper::Block(block) => block.state_root(), - BlockWrapper::BlockAndBlobs(block, _) => block.state_root(), + BlockWrapper::Available(block) => block.block.state_root(), + BlockWrapper::AvailabilityPending(block) => block.state_root(), } } fn signed_block_header(&self) -> SignedBeaconBlockHeader { match &self { - BlockWrapper::Block(block) => block.signed_block_header(), - BlockWrapper::BlockAndBlobs(block, _) => block.signed_block_header(), + BlockWrapper::Available(block) => block.block.signed_block_header(), + BlockWrapper::AvailabilityPending(block) => block.signed_block_header(), } } fn message(&self) -> BeaconBlockRef { match &self { - BlockWrapper::Block(block) => block.message(), - BlockWrapper::BlockAndBlobs(block, _) => block.message(), + BlockWrapper::Available(block) => block.block.message(), + BlockWrapper::AvailabilityPending(block) => block.message(), } } fn as_block(&self) -> &SignedBeaconBlock { match &self { - BlockWrapper::Block(block) => &block, - BlockWrapper::BlockAndBlobs(block, _) => &block, + BlockWrapper::Available(block) => &block.block, + BlockWrapper::AvailabilityPending(block) => &block, } } fn block_cloned(&self) -> Arc> { match &self { - BlockWrapper::Block(block) => block.clone(), - BlockWrapper::BlockAndBlobs(block, _) => block.clone(), + BlockWrapper::Available(block) => block.block.clone(), + BlockWrapper::AvailabilityPending(block) => block.clone(), } } fn canonical_root(&self) -> Hash256 { match &self { - BlockWrapper::Block(block) => block.canonical_root(), - BlockWrapper::BlockAndBlob(block, _) => block.canonical_root(), + BlockWrapper::Available(block) => block.block.canonical_root(), + BlockWrapper::AvailabilityPending(block) => block.canonical_root(), } } } -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() - } - } - } - fn epoch(&self) -> Epoch { - match &self.0 { - AvailableBlockInner::Block(block) => block.epoch(), - AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => { - block_sidecar_pair.beacon_block.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() - } - } - } - 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() - } - } - } - 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() - } - } - } - fn message(&self) -> BeaconBlockRef { - match &self.0 { - AvailableBlockInner::Block(block) => block.message(), - AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => { - block_sidecar_pair.beacon_block.message() - } - } - } - fn as_block(&self) -> &SignedBeaconBlock { - match &self.0 { - AvailableBlockInner::Block(block) => block, - AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => { - &block_sidecar_pair.beacon_block - } - } - } - fn block_cloned(&self) -> Arc> { - match &self.0 { - AvailableBlockInner::Block(block) => block.clone(), - AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => { - block_sidecar_pair.beacon_block.clone() - } - } - } - fn canonical_root(&self) -> Hash256 { - match &self.0 { - AvailableBlockInner::Block(block) => block.canonical_root(), - AvailableBlockInner::BlockAndBlob(block_sidecar_pair) => { - block_sidecar_pair.beacon_block.canonical_root() - } - } +impl From> for BlockWrapper { + fn from(block: SignedBeaconBlock) -> Self { + BlockWrapper::AvailabilityPending(Arc::new(block)) + } +} + +impl From>> for BlockWrapper { + fn from(block: Arc>) -> Self { + BlockWrapper::AvailabilityPending(block) + } +} + +impl From> for BlockWrapper { + fn from(block: AvailableBlock) -> Self { + BlockWrapper::Available(block) } } diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 1e420f8a4b..544f484663 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -48,10 +48,7 @@ // returned alongside. #![allow(clippy::result_large_err)] -use crate::blob_verification::{ - validate_blob_for_gossip, AsBlock, AvailabilityPendingBlock, AvailableBlock, BlobError, - BlockWrapper, IntoAvailableBlock, IntoBlockWrapper, -}; +use crate::blob_verification::{AsBlock, AvailableBlock, BlobError, BlockWrapper}; use crate::eth1_finalization_cache::Eth1FinalizationData; use crate::execution_payload::{ is_optimistic_candidate_block, validate_execution_payload_for_gossip, validate_merge_block, @@ -67,7 +64,6 @@ use crate::{ }, metrics, BeaconChain, BeaconChainError, BeaconChainTypes, }; -use derivative::Derivative; use eth2::types::EventKind; use execution_layer::PayloadStatus; use fork_choice::{AttestationFromBlock, PayloadVerificationStatus}; @@ -596,13 +592,12 @@ pub fn signature_verify_chain_segment( signature_verifier.include_all_signatures(block.as_block(), &mut consensus_context)?; //FIXME(sean) batch kzg verification - let available_block = block.clone().into_available_block(*block_root, chain)?; consensus_context = consensus_context.set_kzg_commitments_consistent(true); // Save the block and its consensus context. The context will have had its proposer index // and attesting indices filled in, which can be used to accelerate later block processing. signature_verified_blocks.push(SignatureVerifiedBlock { - block: available_block, + block: block.clone(), block_root: *block_root, parent: None, consensus_context, @@ -624,10 +619,9 @@ pub fn signature_verify_chain_segment( /// A wrapper around a `SignedBeaconBlock` that indicates it has been approved for re-gossiping on /// the p2p network. -#[derive(Derivative)] -#[derivative(Debug(bound = "T: BeaconChainTypes"))] +#[derive(Debug)] pub struct GossipVerifiedBlock { - pub block: AvailabilityPendingBlock, + pub block: BlockWrapper, pub block_root: Hash256, parent: Option>, consensus_context: ConsensusContext, @@ -636,7 +630,7 @@ pub struct GossipVerifiedBlock { /// A wrapper around a `SignedBeaconBlock` that indicates that all signatures (except the deposit /// signatures) have been verified. pub struct SignatureVerifiedBlock { - block: AvailabilityPendingBlock, + block: BlockWrapper, block_root: Hash256, parent: Option>, consensus_context: ConsensusContext, @@ -659,7 +653,7 @@ type PayloadVerificationHandle = /// due to finality or some other event. A `ExecutionPendingBlock` should be imported into the /// `BeaconChain` immediately after it is instantiated. pub struct ExecutionPendingBlock { - pub block: Arc>, + pub block: BlockWrapper, pub block_root: Hash256, pub state: BeaconState, pub parent_block: SignedBeaconBlock>, @@ -670,7 +664,7 @@ pub struct ExecutionPendingBlock { } pub struct ExecutedBlock { - pub block: Arc>, + pub block: BlockWrapper, pub block_root: Hash256, pub state: BeaconState, pub parent_block: SignedBeaconBlock>, @@ -689,7 +683,7 @@ pub trait IntoExecutionPendingBlock: Sized { block_root: Hash256, chain: &Arc>, notify_execution_layer: NotifyExecutionLayer, - ) -> Result, BlockError> { + ) -> Result, BlockError> { self.into_execution_pending_block_slashable(block_root, chain, notify_execution_layer) .map(|execution_pending| { // Supply valid block to slasher. @@ -707,7 +701,7 @@ pub trait IntoExecutionPendingBlock: Sized { block_root: Hash256, chain: &Arc>, notify_execution_layer: NotifyExecutionLayer, - ) -> Result, BlockSlashInfo>>; + ) -> Result, BlockSlashInfo>>; fn block(&self) -> &SignedBeaconBlock; } @@ -930,16 +924,14 @@ impl GossipVerifiedBlock { // Validate the block's execution_payload (if any). validate_execution_payload_for_gossip(&parent_block, block.message(), chain)?; - let available_block = validate_blob_for_gossip(block, block_root, chain)?; - // Having checked the proposer index and the block root we can cache them. - let consensus_context = ConsensusContext::new(available_block.slot()) + let consensus_context = ConsensusContext::new(block.slot()) .set_current_block_root(block_root) - .set_proposer_index(available_block.as_block().message().proposer_index()) + .set_proposer_index(block.as_block().message().proposer_index()) .set_kzg_commitments_consistent(true); Ok(Self { - block: available_block, + block: block, block_root, parent, consensus_context, @@ -979,10 +971,11 @@ impl SignatureVerifiedBlock { /// /// Returns an error if the block is invalid, or if the block was unable to be verified. pub fn new( - block: AvailabilityPendingBlock, + block: Arc>, block_root: Hash256, chain: &BeaconChain, ) -> Result> { + let block = BlockWrapper::from(block); // Ensure the block is the correct structure for the fork at `block.slot()`. block .as_block() @@ -1029,7 +1022,7 @@ impl SignatureVerifiedBlock { /// As for `new` above but producing `BlockSlashInfo`. pub fn check_slashable( - block: AvailabilityPendingBlock, + block: Arc>, block_root: Hash256, chain: &BeaconChain, ) -> Result>> { @@ -1139,11 +1132,8 @@ impl IntoExecutionPendingBlock for Arc IntoExecutionPendingBlock for Arc IntoExecutionPendingBlock for AvailabilityPendingBlock { - /// Verifies the `SignedBeaconBlock` by first transforming it into a `SignatureVerifiedBlock` - /// and then using that implementation of `IntoExecutionPendingBlock` to complete verification. +impl IntoExecutionPendingBlock for BlockWrapper { fn into_execution_pending_block_slashable( self, block_root: Hash256, chain: &Arc>, notify_execution_layer: NotifyExecutionLayer, - ) -> Result, BlockSlashInfo>> { - // Perform an early check to prevent wasting time on irrelevant blocks. - let block_root = check_block_relevancy(self.as_block(), block_root, chain) - .map_err(|e| BlockSlashInfo::SignatureNotChecked(self.signed_block_header(), e))?; - - SignatureVerifiedBlock::check_slashable(self, block_root, chain)? - .into_execution_pending_block_slashable(block_root, chain, notify_execution_layer) + ) -> Result< + ExecutionPendingBlock, + BlockSlashInfo::EthSpec>>, + > { + match self { + BlockWrapper::AvailabilityPending(block) => block + .into_execution_pending_block_slashable(block_root, chain, notify_execution_layer), + BlockWrapper::Available(AvailableBlock { block, blobs }) => { + let execution_pending_block = block.into_execution_pending_block_slashable( + block_root, + chain, + notify_execution_layer, + )?; + let block = execution_pending_block.block.block_cloned(); + let available_execution_pending_block = + BlockWrapper::Available(AvailableBlock { block, blobs }); + std::mem::replace( + &mut execution_pending_block.block, + available_execution_pending_block, + ); + Ok(execution_pending_block) + } + } } - fn block(&self) -> &SignedBeaconBlock { + fn block(&self) -> &SignedBeaconBlock<::EthSpec> { self.as_block() } } @@ -1183,7 +1187,7 @@ impl ExecutionPendingBlock { /// /// Returns an error if the block is invalid, or if the block was unable to be verified. pub fn from_signature_verified_components( - block: AvailabilityPendingBlock, + block: BlockWrapper, block_root: Hash256, parent: PreProcessingSnapshot, mut consensus_context: ConsensusContext, @@ -1213,7 +1217,7 @@ impl ExecutionPendingBlock { // because it will revert finalization. Note that the finalized block is stored in fork // choice, so we will not reject any child of the finalized block (this is relevant during // genesis). - return Err(BlockError::ParentUnknown(block.into_block_wrapper())); + return Err(BlockError::ParentUnknown(block)); } // Reject any block that exceeds our limit on skipped slots. @@ -1648,14 +1652,11 @@ fn check_block_against_finalized_slot( /// ## Warning /// /// Taking a lock on the `chain.canonical_head.fork_choice` might cause a deadlock here. -pub fn check_block_is_finalized_checkpoint_or_descendant< - T: BeaconChainTypes, - B: IntoBlockWrapper, ->( +pub fn check_block_is_finalized_checkpoint_or_descendant( chain: &BeaconChain, fork_choice: &BeaconForkChoice, - block: B, -) -> Result> { + block: BlockWrapper, +) -> Result, BlockError> { if fork_choice.is_finalized_checkpoint_or_descendant(block.parent_root()) { Ok(block) } else { @@ -1676,7 +1677,7 @@ pub fn check_block_is_finalized_checkpoint_or_descendant< block_parent_root: block.parent_root(), }) } else { - Err(BlockError::ParentUnknown(block.into_block_wrapper())) + Err(BlockError::ParentUnknown(block)) } } } @@ -1766,11 +1767,11 @@ fn verify_parent_block_is_known( /// Returns `Err(BlockError::ParentUnknown)` if the parent is not found, or if an error occurs /// whilst attempting the operation. #[allow(clippy::type_complexity)] -fn load_parent>( +fn load_parent( block_root: Hash256, - block: B, + block: BlockWrapper, chain: &BeaconChain, -) -> Result<(PreProcessingSnapshot, B), BlockError> { +) -> Result<(PreProcessingSnapshot, BlockWrapper), BlockError> { let spec = &chain.spec; // Reject any block if its parent is not known to fork choice. @@ -1788,7 +1789,7 @@ fn load_parent>( .fork_choice_read_lock() .contains_block(&block.parent_root()) { - return Err(BlockError::ParentUnknown(block.into_block_wrapper())); + return Err(BlockError::ParentUnknown(block)); } let block_delay = chain diff --git a/consensus/types/src/signed_blob.rs b/consensus/types/src/signed_blob.rs index 4121b8b7f2..c295f6c6c1 100644 --- a/consensus/types/src/signed_blob.rs +++ b/consensus/types/src/signed_blob.rs @@ -1,4 +1,7 @@ -use crate::{test_utils::TestRandom, BlobSidecar, EthSpec, Signature}; +use crate::{ + test_utils::TestRandom, BlobSidecar, ChainSpec, EthSpec, Fork, Hash256, PublicKey, Signature, + SignedRoot, +}; use derivative::Derivative; use serde_derive::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; @@ -25,3 +28,19 @@ pub struct SignedBlobSidecar { pub message: BlobSidecar, pub signature: Signature, } + +impl SignedRoot for SignedBlobSidecar {} + +impl SignedBlobSidecar { + pub fn verify_signature( + &self, + _object_root_opt: Option, + _pubkey: &PublicKey, + _fork: &Fork, + _genesis_validators_root: Hash256, + _spec: &ChainSpec, + ) -> bool { + // TODO (pawan): fill up logic + unimplemented!() + } +}