diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index f6fcd62f33..aa7f079c20 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -2940,18 +2940,8 @@ impl BeaconChain { unverified_block: B, notify_execution_layer: NotifyExecutionLayer, ) -> Result> { - if let Ok(commitments) = unverified_block - .block() - .message() - .body() - .blob_kzg_commitments() - { - self.data_availability_checker.notify_block_commitments( - unverified_block.block().slot(), - block_root, - commitments.clone(), - ); - }; + self.data_availability_checker + .notify_block(block_root, unverified_block.block_cloned()); let r = self .process_block(block_root, unverified_block, notify_execution_layer, || { Ok(()) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index e8df5b811e..ac3d3e3ab8 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -764,6 +764,7 @@ pub trait IntoExecutionPendingBlock: Sized { ) -> Result, BlockSlashInfo>>; fn block(&self) -> &SignedBeaconBlock; + fn block_cloned(&self) -> Arc>; } impl GossipVerifiedBlock { @@ -1017,6 +1018,10 @@ impl IntoExecutionPendingBlock for GossipVerifiedBlock &SignedBeaconBlock { self.block.as_block() } + + fn block_cloned(&self) -> Arc> { + self.block.clone() + } } impl SignatureVerifiedBlock { @@ -1168,6 +1173,10 @@ impl IntoExecutionPendingBlock for SignatureVerifiedBloc fn block(&self) -> &SignedBeaconBlock { self.block.as_block() } + + fn block_cloned(&self) -> Arc> { + self.block.block_cloned() + } } impl IntoExecutionPendingBlock for Arc> { @@ -1198,6 +1207,10 @@ impl IntoExecutionPendingBlock for Arc &SignedBeaconBlock { self } + + fn block_cloned(&self) -> Arc> { + self.clone() + } } impl IntoExecutionPendingBlock for RpcBlock { @@ -1228,6 +1241,10 @@ impl IntoExecutionPendingBlock for RpcBlock fn block(&self) -> &SignedBeaconBlock { self.as_block() } + + fn block_cloned(&self) -> Arc> { + self.block_cloned() + } } impl ExecutionPendingBlock { diff --git a/beacon_node/beacon_chain/src/block_verification_types.rs b/beacon_node/beacon_chain/src/block_verification_types.rs index a6840ed764..edba7a211c 100644 --- a/beacon_node/beacon_chain/src/block_verification_types.rs +++ b/beacon_node/beacon_chain/src/block_verification_types.rs @@ -46,6 +46,13 @@ impl RpcBlock { } } + pub fn block_cloned(&self) -> Arc> { + match &self.block { + RpcBlockInner::Block(block) => block.clone(), + RpcBlockInner::BlockAndBlobs(block, _) => block.clone(), + } + } + pub fn blobs(&self) -> Option<&BlobSidecarList> { match &self.block { RpcBlockInner::Block(_) => None, diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 07ee55b551..c60cca57be 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -20,7 +20,7 @@ use std::fmt::Debug; use std::num::NonZeroUsize; use std::sync::Arc; use task_executor::TaskExecutor; -use types::beacon_block_body::{KzgCommitmentOpts, KzgCommitments}; +use types::beacon_block_body::KzgCommitmentOpts; use types::blob_sidecar::{BlobIdentifier, BlobSidecar, FixedBlobSidecarList}; use types::{BlobSidecarList, ChainSpec, Epoch, EthSpec, Hash256, SignedBeaconBlock, Slot}; @@ -195,7 +195,10 @@ impl DataAvailabilityChecker { /// Get a block from the availability cache. Only checks for blocks stored in memory. Useful /// for serving RPC requests. pub fn get_block(&self, block_root: &Hash256) -> Option>> { - self.availability_cache.peek_block(block_root) + self.processing_cache + .read() + .get(block_root) + .and_then(|cached| cached.block.clone()) } /// Put a list of blobs received via RPC into the availability cache. This performs KZG @@ -350,20 +353,16 @@ impl DataAvailabilityChecker { block.num_expected_blobs() > 0 && self.da_check_required_for_epoch(block.epoch()) } - /// Adds block commitments to the processing cache. These commitments are unverified but caching + /// Adds a block to the processing cache. This block's commitments are unverified but caching /// them here is useful to avoid duplicate downloads of blocks, as well as understanding - /// our blob download requirements. - pub fn notify_block_commitments( - &self, - slot: Slot, - block_root: Hash256, - commitments: KzgCommitments, - ) { + /// our blob download requirements. We will also serve this over RPC. + pub fn notify_block(&self, block_root: Hash256, block: Arc>) { + let slot = block.slot(); self.processing_cache .write() .entry(block_root) .or_insert_with(|| ProcessingComponents::new(slot)) - .merge_block(commitments); + .merge_block(block); } /// Add a single blob commitment to the processing cache. This commitment is unverified but caching @@ -591,6 +590,15 @@ pub enum MaybeAvailableBlock { }, } +impl MaybeAvailableBlock { + pub fn block_cloned(&self) -> Arc> { + match self { + Self::Available(block) => block.block_cloned(), + Self::AvailabilityPending { block, .. } => block.clone(), + } + } +} + #[derive(Debug, Clone)] pub enum MissingBlobs { /// We know for certain these blobs are missing. diff --git a/beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs b/beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs index 776f81ee54..65093db26b 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs @@ -182,9 +182,9 @@ macro_rules! impl_availability_view { impl_availability_view!( ProcessingComponents, - KzgCommitments, + Arc>, KzgCommitment, - block_commitments, + block, blob_commitments ); @@ -212,12 +212,6 @@ pub trait GetCommitment { fn get_commitment(&self) -> &KzgCommitment; } -// These implementations are required to implement `AvailabilityView` for `ProcessingView`. -impl GetCommitments for KzgCommitments { - fn get_commitments(&self) -> KzgCommitments { - self.clone() - } -} impl GetCommitment for KzgCommitment { fn get_commitment(&self) -> &KzgCommitment { self @@ -310,7 +304,7 @@ pub mod tests { } type ProcessingViewSetup = ( - KzgCommitments, + Arc>, FixedVector, ::MaxBlobsPerBlock>, FixedVector, ::MaxBlobsPerBlock>, ); @@ -320,12 +314,6 @@ pub mod tests { valid_blobs: FixedVector>>, ::MaxBlobsPerBlock>, invalid_blobs: FixedVector>>, ::MaxBlobsPerBlock>, ) -> ProcessingViewSetup { - let commitments = block - .message() - .body() - .blob_kzg_commitments() - .unwrap() - .clone(); let blobs = FixedVector::from( valid_blobs .iter() @@ -338,7 +326,7 @@ pub mod tests { .map(|blob_opt| blob_opt.as_ref().map(|blob| blob.kzg_commitment)) .collect::>(), ); - (commitments, blobs, invalid_blobs) + (Arc::new(block), blobs, invalid_blobs) } type PendingComponentsSetup = ( diff --git a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs index 3839f5e2f4..34c9bc76f6 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs @@ -45,7 +45,7 @@ use ssz_types::{FixedVector, VariableList}; use std::num::NonZeroUsize; use std::{collections::HashSet, sync::Arc}; use types::blob_sidecar::BlobIdentifier; -use types::{BlobSidecar, ChainSpec, Epoch, EthSpec, Hash256, SignedBeaconBlock}; +use types::{BlobSidecar, ChainSpec, Epoch, EthSpec, Hash256}; /// This represents the components of a partially available block /// @@ -322,18 +322,6 @@ impl Critical { } } - /// This only checks for the blocks in memory - pub fn peek_block(&self, block_root: &Hash256) -> Option>> { - self.in_memory - .peek(block_root) - .and_then(|pending_components| { - pending_components - .executed_block - .as_ref() - .map(|block| block.block_cloned()) - }) - } - /// Puts the pending components in the LRU cache. If the cache /// is at capacity, the LRU entry is written to the store first pub fn put_pending_components( @@ -411,11 +399,6 @@ impl OverflowLRUCache { }) } - /// Just checks for blocks stored in memory - pub fn peek_block(&self, block_root: &Hash256) -> Option>> { - self.critical.read().peek_block(block_root) - } - /// Fetch a blob from the cache without affecting the LRU ordering pub fn peek_blob( &self, diff --git a/beacon_node/beacon_chain/src/data_availability_checker/processing_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker/processing_cache.rs index 969034c657..3bdc74e630 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/processing_cache.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/processing_cache.rs @@ -1,8 +1,9 @@ use crate::data_availability_checker::AvailabilityView; use std::collections::hash_map::Entry; use std::collections::HashMap; -use types::beacon_block_body::{KzgCommitmentOpts, KzgCommitments}; -use types::{EthSpec, Hash256, Slot}; +use std::sync::Arc; +use types::beacon_block_body::KzgCommitmentOpts; +use types::{EthSpec, Hash256, SignedBeaconBlock, Slot}; /// This cache is used only for gossip blocks/blobs and single block/blob lookups, to give req/resp /// a view of what we have and what we require. This cache serves a slightly different purpose than @@ -45,7 +46,7 @@ pub struct ProcessingComponents { /// Blobs required for a block can only be known if we have seen the block. So `Some` here /// means we've seen it, a `None` means we haven't. The `kzg_commitments` value helps us figure /// out whether incoming blobs actually match the block. - pub block_commitments: Option>, + pub block: Option>>, /// `KzgCommitments` for blobs are always known, even if we haven't seen the block. See /// `AvailabilityView`'s trait definition for more details. pub blob_commitments: KzgCommitmentOpts, @@ -55,7 +56,7 @@ impl ProcessingComponents { pub fn new(slot: Slot) -> Self { Self { slot, - block_commitments: None, + block: None, blob_commitments: KzgCommitmentOpts::::default(), } } @@ -67,7 +68,7 @@ impl ProcessingComponents { pub fn empty(_block_root: Hash256) -> Self { Self { slot: Slot::new(0), - block_commitments: None, + block: None, blob_commitments: KzgCommitmentOpts::::default(), } } diff --git a/beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs index e9575d2b97..bd125a7f42 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs @@ -35,10 +35,6 @@ impl DietAvailabilityPendingExecutedBlock { &self.block } - pub fn block_cloned(&self) -> Arc> { - self.block.clone() - } - pub fn num_blobs_expected(&self) -> usize { self.block .message()