From 7136412062efef3a6c92de6bb08ef01855e0db76 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Sat, 25 May 2024 01:01:23 +0200 Subject: [PATCH] Simplify BlockProcessStatus --- .../beacon_chain/src/beacon_block_streamer.rs | 17 ++++----- beacon_node/beacon_chain/src/beacon_chain.rs | 38 +++++++++---------- .../src/data_availability_checker.rs | 8 ++-- .../overflow_lru_cache.rs | 16 ++------ .../state_lru_cache.rs | 4 ++ beacon_node/beacon_chain/src/metrics.rs | 3 +- .../network/src/sync/network_context.rs | 10 +---- 7 files changed, 41 insertions(+), 55 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_block_streamer.rs b/beacon_node/beacon_chain/src/beacon_block_streamer.rs index 0c92b7c1f62..f0a68b6be55 100644 --- a/beacon_node/beacon_chain/src/beacon_block_streamer.rs +++ b/beacon_node/beacon_chain/src/beacon_block_streamer.rs @@ -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; @@ -410,15 +410,14 @@ impl BeaconBlockStreamer { fn check_caches(&self, root: Hash256) -> Option>> { 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 } diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 3dcb1b01c99..0bc0a4a2d3d 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -337,20 +337,18 @@ struct PartialBeaconBlock { bls_to_execution_changes: Vec, } -pub enum BlockProcessStatus { +pub enum BlockProcessStatus { /// 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>), /// 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>), +} + +pub struct BeaconChainMetrics { + pub reqresp_pre_import_cache_len: usize, } pub type LightClientProducerEvent = (Hash256, Slot, SyncAggregate); @@ -1256,25 +1254,19 @@ impl BeaconChain { /// 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 { + 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 @@ -6673,6 +6665,12 @@ impl BeaconChain { ForkName::Base => Err(Error::UnsupportedFork), } } + + pub fn metrics(&self) -> BeaconChainMetrics { + BeaconChainMetrics { + reqresp_pre_import_cache_len: self.reqresp_pre_import_cache.read().len(), + } + } } impl Drop for BeaconChain { diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index d6f2d81de37..7efb6884d0f 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -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 @@ -88,12 +86,12 @@ impl DataAvailabilityChecker { /// 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 { + ) -> Option>> { self.availability_cache - .get_execution_valid_block_summary(block_root) + .get_execution_valid_block(block_root) } /// Return the required blobs `block_root` expects if the block is currenlty in the cache. 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 cfe12ba60dc..e350181c867 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 @@ -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 /// @@ -57,11 +57,6 @@ pub struct PendingComponents { pub executed_block: Option>, } -pub struct ExecutionValidBlockSummary { - pub slot: Slot, - pub blob_kzg_commitments_count: usize, -} - impl PendingComponents { /// Returns an immutable reference to the cached block. pub fn get_cached_block(&self) -> &Option> { @@ -549,10 +544,10 @@ impl OverflowLRUCache { } /// 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 { + ) -> Option>> { self.critical .read() .peek_pending_components(block_root) @@ -560,10 +555,7 @@ impl OverflowLRUCache { 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()) }) } 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 f8a243bd9e8..9775d54c024 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 @@ -37,6 +37,10 @@ impl DietAvailabilityPendingExecutedBlock { &self.block } + pub fn block_cloned(&self) -> Arc> { + self.block.clone() + } + pub fn num_blobs_expected(&self) -> usize { self.block .message() diff --git a/beacon_node/beacon_chain/src/metrics.rs b/beacon_node/beacon_chain/src/metrics.rs index 3b2453c3112..4ceaf675cec 100644 --- a/beacon_node/beacon_chain/src/metrics.rs +++ b/beacon_node/beacon_chain/src/metrics.rs @@ -1192,6 +1192,7 @@ pub fn scrape_for_metrics(beacon_chain: &BeaconChain) { } let attestation_stats = beacon_chain.op_pool.attestation_stats(); + let chain_metrics = beacon_chain.metrics(); set_gauge_by_usize( &BLOCK_PROCESSING_SNAPSHOT_CACHE_SIZE, @@ -1200,7 +1201,7 @@ pub fn scrape_for_metrics(beacon_chain: &BeaconChain) { 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(); diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 12b51a1341d..1b6c820a2f6 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -397,14 +397,8 @@ impl SyncNetworkContext { // 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