Skip to content

Commit

Permalink
Simplify BlockProcessStatus
Browse files Browse the repository at this point in the history
  • Loading branch information
dapplion committed May 24, 2024
1 parent 2b55017 commit 551f16a
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 55 deletions.
17 changes: 8 additions & 9 deletions beacon_node/beacon_chain/src/beacon_block_streamer.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{metrics, BeaconChain, BeaconChainError, BeaconChainTypes};
use crate::{metrics, BeaconChain, BeaconChainError, BeaconChainTypes, BlockProcessStatus};
use execution_layer::{ExecutionLayer, ExecutionPayloadBodyV1};
use slog::{crit, debug, error, Logger};
use std::collections::HashMap;
Expand Down Expand Up @@ -410,15 +410,14 @@ impl<T: BeaconChainTypes> BeaconBlockStreamer<T> {

fn check_caches(&self, root: Hash256) -> Option<Arc<SignedBeaconBlock<T::EthSpec>>> {
if self.check_caches == CheckCaches::Yes {
self.beacon_chain
.reqresp_pre_import_cache
.read()
.get(&root)
.map(|block| {
match self.beacon_chain.get_block_process_status(&root) {
BlockProcessStatus::Unknown => None,
BlockProcessStatus::NotValidated(block)
| BlockProcessStatus::ExecutionValidated(block) => {
metrics::inc_counter(&metrics::BEACON_REQRESP_PRE_IMPORT_CACHE_HITS);
block.clone()
})
.or(self.beacon_chain.early_attester_cache.get_block(root))
Some(block)
}
}
} else {
None
}
Expand Down
38 changes: 18 additions & 20 deletions beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,20 +337,18 @@ struct PartialBeaconBlock<E: EthSpec> {
bls_to_execution_changes: Vec<SignedBlsToExecutionChange>,
}

pub enum BlockProcessStatus {
pub enum BlockProcessStatus<E: EthSpec> {
/// Block is not in any pre-import cache. Block may be in the data-base or in the fork-choice.
Unknown,
/// Block is currently processing but not yet validated.
NotValidated {
slot: Slot,
blob_kzg_commitments_count: usize,
},
NotValidated(Arc<SignedBeaconBlock<E>>),
/// Block is fully valid, but not yet imported. It's cached in the da_checker while awaiting
/// missing block components.
ExecutionValidated {
slot: Slot,
blob_kzg_commitments_count: usize,
},
ExecutionValidated(Arc<SignedBeaconBlock<E>>),
}

pub struct BeaconChainMetrics {
pub reqresp_pre_import_cache_len: usize,
}

pub type LightClientProducerEvent<T> = (Hash256, Slot, SyncAggregate<T>);
Expand Down Expand Up @@ -1256,25 +1254,19 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
/// Return the status of a block as it progresses through the various caches of the beacon
/// chain. Used by sync to learn the status of a block and prevent repeated downloads /
/// processing attempts.
pub fn get_block_process_status(&self, block_root: &Hash256) -> BlockProcessStatus {
if let Some(execution_valid_block) = self
pub fn get_block_process_status(&self, block_root: &Hash256) -> BlockProcessStatus<T::EthSpec> {
if let Some(block) = self
.data_availability_checker
.get_execution_valid_block_summary(block_root)
.get_execution_valid_block(block_root)
{
return BlockProcessStatus::ExecutionValidated {
slot: execution_valid_block.slot,
blob_kzg_commitments_count: execution_valid_block.blob_kzg_commitments_count,
};
return BlockProcessStatus::ExecutionValidated(block);
}

if let Some(block) = self.reqresp_pre_import_cache.read().get(block_root) {
// A block is on the `reqresp_pre_import_cache` but NOT in the
// `data_availability_checker` only if it is actively processing. We can expect a future
// event with the result of processing
return BlockProcessStatus::NotValidated {
slot: block.slot(),
blob_kzg_commitments_count: block.num_expected_blobs(),
};
return BlockProcessStatus::NotValidated(block.clone());
}

BlockProcessStatus::Unknown
Expand Down Expand Up @@ -6673,6 +6665,12 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
ForkName::Base => Err(Error::UnsupportedFork),
}
}

pub fn metrics(&self) -> BeaconChainMetrics {
BeaconChainMetrics {
reqresp_pre_import_cache_len: self.reqresp_pre_import_cache.read().len(),
}
}
}

impl<T: BeaconChainTypes> Drop for BeaconChain<T> {
Expand Down
8 changes: 3 additions & 5 deletions beacon_node/beacon_chain/src/data_availability_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ mod state_lru_cache;
pub use error::{Error as AvailabilityCheckError, ErrorCategory as AvailabilityCheckErrorCategory};
use types::non_zero_usize::new_non_zero_usize;

use self::overflow_lru_cache::ExecutionValidBlockSummary;

/// The LRU Cache stores `PendingComponents` which can store up to
/// `MAX_BLOBS_PER_BLOCK = 6` blobs each. A `BlobSidecar` is 0.131256 MB. So
/// the maximum size of a `PendingComponents` is ~ 0.787536 MB. Setting this
Expand Down Expand Up @@ -88,12 +86,12 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {

/// Checks if the block root is currenlty in the availability cache awaiting import because
/// of missing components.
pub fn get_execution_valid_block_summary(
pub fn get_execution_valid_block(
&self,
block_root: &Hash256,
) -> Option<ExecutionValidBlockSummary> {
) -> Option<Arc<SignedBeaconBlock<T::EthSpec>>> {
self.availability_cache
.get_execution_valid_block_summary(block_root)
.get_execution_valid_block(block_root)
}

/// Return the set of imported blob indexes for `block_root`. Returns None if there is no block
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,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, Slot};
use types::{BlobSidecar, ChainSpec, Epoch, EthSpec, Hash256, SignedBeaconBlock};

/// This represents the components of a partially available block
///
Expand All @@ -57,11 +57,6 @@ pub struct PendingComponents<E: EthSpec> {
pub executed_block: Option<DietAvailabilityPendingExecutedBlock<E>>,
}

pub struct ExecutionValidBlockSummary {
pub slot: Slot,
pub blob_kzg_commitments_count: usize,
}

impl<E: EthSpec> PendingComponents<E> {
/// Returns an immutable reference to the cached block.
pub fn get_cached_block(&self) -> &Option<DietAvailabilityPendingExecutedBlock<E>> {
Expand Down Expand Up @@ -549,21 +544,18 @@ impl<T: BeaconChainTypes> OverflowLRUCache<T> {
}

/// Returns true if the block root is known, without altering the LRU ordering
pub fn get_execution_valid_block_summary(
pub fn get_execution_valid_block(
&self,
block_root: &Hash256,
) -> Option<ExecutionValidBlockSummary> {
) -> Option<Arc<SignedBeaconBlock<T::EthSpec>>> {
self.critical
.read()
.peek_pending_components(block_root)
.and_then(|pending_components| {
pending_components
.executed_block
.as_ref()
.map(|block| ExecutionValidBlockSummary {
slot: block.as_block().slot(),
blob_kzg_commitments_count: block.num_blobs_expected(),
})
.map(|block| block.block_cloned())
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ impl<E: EthSpec> DietAvailabilityPendingExecutedBlock<E> {
&self.block
}

pub fn block_cloned(&self) -> Arc<SignedBeaconBlock<E>> {
self.block.clone()
}

pub fn num_blobs_expected(&self) -> usize {
self.block
.message()
Expand Down
3 changes: 2 additions & 1 deletion beacon_node/beacon_chain/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1192,6 +1192,7 @@ pub fn scrape_for_metrics<T: BeaconChainTypes>(beacon_chain: &BeaconChain<T>) {
}

let attestation_stats = beacon_chain.op_pool.attestation_stats();
let chain_metrics = beacon_chain.metrics();

set_gauge_by_usize(
&BLOCK_PROCESSING_SNAPSHOT_CACHE_SIZE,
Expand All @@ -1200,7 +1201,7 @@ pub fn scrape_for_metrics<T: BeaconChainTypes>(beacon_chain: &BeaconChain<T>) {

set_gauge_by_usize(
&BEACON_REQRESP_PRE_IMPORT_CACHE_SIZE,
beacon_chain.reqresp_pre_import_cache.read().len(),
chain_metrics.reqresp_pre_import_cache_len,
);

let da_checker_metrics = beacon_chain.data_availability_checker.metrics();
Expand Down
10 changes: 2 additions & 8 deletions beacon_node/network/src/sync/network_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,14 +397,8 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
// can assert that this is the correct value of `blob_kzg_commitments_count`.
match self.chain.get_block_process_status(&block_root) {
BlockProcessStatus::Unknown => None,
BlockProcessStatus::NotValidated {
blob_kzg_commitments_count,
..
}
| BlockProcessStatus::ExecutionValidated {
blob_kzg_commitments_count,
..
} => Some(blob_kzg_commitments_count),
BlockProcessStatus::NotValidated(block)
| BlockProcessStatus::ExecutionValidated(block) => Some(block.num_expected_blobs()),
}
}) else {
// Wait to download the block before downloading blobs. Then we can be sure that the
Expand Down

0 comments on commit 551f16a

Please sign in to comment.