Rework block processing (#4092)

* introduce availability pending block * add intoavailableblock trait * small fixes * add 'gossip blob cache' and start to clean up processing and transition types * shard memory blob cache * Initial commit * Fix after rebase * Add gossip verification conditions * cache cleanup * general chaos * extended chaos * cargo fmt * more progress * more progress * tons of changes, just tryna compile * everything, everywhere, all at once * Reprocess an ExecutedBlock on unavailable blobs * Add sus gossip verification for blobs * Merge stuff * Remove reprocessing cache stuff * lint * Add a wrapper to allow construction of only valid `AvailableBlock`s * rename blob arc list to blob list * merge cleanuo * Revert "merge cleanuo" This reverts commit 5e98326878. * Revert "Revert "merge cleanuo"" This reverts commit 3a4009443a. * fix rpc methods * move beacon block and blob to eth2/types * rename gossip blob cache to data availability checker * lots of changes * fix some compilation issues * fix compilation issues * fix compilation issues * fix compilation issues * fix compilation issues * fix compilation issues * cargo fmt * use a common data structure for block import types * fix availability check on proposal import * refactor the blob cache and split the block wrapper into two types * add type conversion for signed block and block wrapper * fix beacon chain tests and do some renaming, add some comments * Partial processing (#4) * move beacon block and blob to eth2/types * rename gossip blob cache to data availability checker * lots of changes * fix some compilation issues * fix compilation issues * fix compilation issues * fix compilation issues * fix compilation issues * fix compilation issues * cargo fmt * use a common data structure for block import types * fix availability check on proposal import * refactor the blob cache and split the block wrapper into two types * add type conversion for signed block and block wrapper * fix beacon chain tests and do some renaming, add some comments * cargo update (#6) --------- Co-authored-by: realbigsean <sean@sigmaprime.io> Co-authored-by: realbigsean <seananderson33@gmail.com>
2026-05-30 04:37:13 +00:00 · 2023-03-25 03:00:41 +05:30
parent 25a2d8f078
commit b276af98b7
46 changed files with 2167 additions and 1487 deletions
--- a/beacon_node/beacon_chain/src/beacon_chain.rs
+++ b/beacon_node/beacon_chain/src/beacon_chain.rs
@@ -8,15 +8,19 @@ use crate::beacon_block_streamer::{BeaconBlockStreamer, CheckEarlyAttesterCache}
 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, AvailableBlock, BlockWrapper};
+use crate::blob_verification::{self, AsBlock, BlobError, BlockWrapper, GossipVerifiedBlob};
 use crate::block_times_cache::BlockTimesCache;
+use crate::block_verification::POS_PANDA_BANNER;
 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,
+    signature_verify_chain_segment, AvailableExecutedBlock, BlockError, BlockImportData,
+    ExecutedBlock, ExecutionPendingBlock, GossipVerifiedBlock, IntoExecutionPendingBlock,
 };
 pub use crate::canonical_head::{CanonicalHead, CanonicalHeadRwLock};
 use crate::chain_config::ChainConfig;
+use crate::data_availability_checker::{
+    Availability, AvailabilityCheckError, AvailableBlock, DataAvailabilityChecker,
+};
 use crate::early_attester_cache::EarlyAttesterCache;
 use crate::errors::{BeaconChainError as Error, BlockProductionError};
 use crate::eth1_chain::{Eth1Chain, Eth1ChainBackend};
@@ -109,8 +113,9 @@ use store::{
 use task_executor::{ShutdownReason, TaskExecutor};
 use tokio_stream::Stream;
 use tree_hash::TreeHash;
+use types::beacon_block_body::KzgCommitments;
 use types::beacon_state::CloneConfig;
-use types::blobs_sidecar::KzgCommitments;
+use types::blob_sidecar::{BlobIdentifier, BlobSidecarList, Blobs};
 use types::consts::eip4844::MIN_EPOCHS_FOR_BLOBS_SIDECARS_REQUESTS;
 use types::consts::merge::INTERVALS_PER_SLOT;
 use types::*;
@@ -183,6 +188,36 @@ pub enum WhenSlotSkipped {
    Prev,
 }

+#[derive(Debug, PartialEq)]
+pub enum AvailabilityProcessingStatus {
+    PendingBlobs(Vec<BlobIdentifier>),
+    PendingBlock(Hash256),
+    Imported(Hash256),
+}
+
+//TODO(sean) using this in tests for now
+impl TryInto<SignedBeaconBlockHash> for AvailabilityProcessingStatus {
+    type Error = ();
+
+    fn try_into(self) -> Result<SignedBeaconBlockHash, Self::Error> {
+        match self {
+            AvailabilityProcessingStatus::Imported(hash) => Ok(hash.into()),
+            _ => Err(()),
+        }
+    }
+}
+
+impl TryInto<Hash256> for AvailabilityProcessingStatus {
+    type Error = ();
+
+    fn try_into(self) -> Result<Hash256, Self::Error> {
+        match self {
+            AvailabilityProcessingStatus::Imported(hash) => Ok(hash),
+            _ => Err(()),
+        }
+    }
+}
+
 /// The result of a chain segment processing.
 pub enum ChainSegmentResult<T: EthSpec> {
    /// Processing this chain segment finished successfully.
@@ -433,8 +468,9 @@ pub struct BeaconChain<T: BeaconChainTypes> {
    pub slasher: Option<Arc<Slasher<T::EthSpec>>>,
    /// Provides monitoring of a set of explicitly defined validators.
    pub validator_monitor: RwLock<ValidatorMonitor<T::EthSpec>>,
-    pub blob_cache: BlobCache<T::EthSpec>,
-    pub kzg: Option<Arc<kzg::Kzg>>,
+    pub proposal_blob_cache: BlobCache<T::EthSpec>,
+    pub data_availability_checker: DataAvailabilityChecker<T::EthSpec, T::SlotClock>,
+    pub kzg: Option<Arc<Kzg>>,
 }

 type BeaconBlockAndState<T, Payload> = (BeaconBlock<T, Payload>, BeaconState<T>);
@@ -991,19 +1027,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
        &self,
        block_root: &Hash256,
    ) -> Result<Option<BlobSidecarList<T::EthSpec>>, Error> {
-        // If there is no data availability boundary, the Eip4844 fork is disabled.
-        if let Some(finalized_data_availability_boundary) =
-            self.finalized_data_availability_boundary()
-        {
-            self.early_attester_cache
-                .get_blobs(*block_root)
-                .map_or_else(
-                    || self.get_blobs(block_root, finalized_data_availability_boundary),
-                    |blobs| Ok(Some(blobs)),
-                )
-        } else {
-            Ok(None)
-        }
+        self.early_attester_cache
+            .get_blobs(*block_root)
+            .map_or_else(|| self.get_blobs(block_root), |blobs| Ok(Some(blobs)))
    }

    /// Returns the block at the given root, if any.
@@ -1086,35 +1112,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
    pub fn get_blobs(
        &self,
        block_root: &Hash256,
-        data_availability_boundary: Epoch,
    ) -> Result<Option<BlobSidecarList<T::EthSpec>>, Error> {
-        match self.store.get_blobs(block_root)? {
-            Some(blob_sidecar_list) => Ok(Some(blob_sidecar_list)),
-            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() {
-                            // TODO (mark): verify this
-                            Ok(BlobSidecarList::empty())
-                        } 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()
-            }
-        }
+        Ok(self.store.get_blobs(block_root)?)
    }

    pub fn get_blinded_block(
@@ -1936,6 +1935,15 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
        })
    }

+    pub fn verify_blob_sidecar_for_gossip(
+        self: &Arc<Self>,
+        blob_sidecar: SignedBlobSidecar<T::EthSpec>,
+        subnet_id: u64,
+    ) -> Result<GossipVerifiedBlob<T::EthSpec>, BlobError> // TODO(pawan): make a GossipVerifedBlob type
+    {
+        blob_verification::validate_blob_sidecar_for_gossip(blob_sidecar, subnet_id, self)
+    }
+
    /// Accepts some 'LightClientOptimisticUpdate' from the network and attempts to verify it
    pub fn verify_optimistic_update_for_gossip(
        self: &Arc<Self>,
@@ -2686,13 +2694,29 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
            .map_err(BeaconChainError::TokioJoin)?
    }

+    pub async fn process_blob(
+        self: &Arc<Self>,
+        blob: GossipVerifiedBlob<T::EthSpec>,
+        count_unrealized: CountUnrealized,
+    ) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
+        self.check_availability_and_maybe_import(
+            |chain| chain.data_availability_checker.put_gossip_blob(blob),
+            count_unrealized,
+        )
+        .await
+    }
+
    /// Returns `Ok(block_root)` if the given `unverified_block` was successfully verified and
    /// imported into the chain.
    ///
+    /// For post deneb blocks, this returns a `BlockError::AvailabilityPending` error
+    /// if the corresponding blobs are not in the required caches.
+    ///
    /// Items that implement `IntoExecutionPendingBlock` include:
    ///
    /// - `SignedBeaconBlock`
    /// - `GossipVerifiedBlock`
+    /// - `BlockWrapper`
    ///
    /// ## Errors
    ///
@@ -2704,105 +2728,67 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
        unverified_block: B,
        count_unrealized: CountUnrealized,
        notify_execution_layer: NotifyExecutionLayer,
-    ) -> Result<Hash256, BlockError<T::EthSpec>> {
+    ) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
        // Start the Prometheus timer.
        let _full_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_TIMES);

        // Increment the Prometheus counter for block processing requests.
        metrics::inc_counter(&metrics::BLOCK_PROCESSING_REQUESTS);

-        let slot = unverified_block.block().slot();
-
-        // A small closure to group the verification and import errors.
        let chain = self.clone();
-        let import_block = async move {
-            let execution_pending = unverified_block.into_execution_pending_block(
-                block_root,
-                &chain,
-                notify_execution_layer,
-            )?;
-            chain
-                .import_execution_pending_block(execution_pending, count_unrealized)
+
+        let execution_pending = unverified_block.into_execution_pending_block(
+            block_root,
+            &chain,
+            notify_execution_layer,
+        )?;
+
+        let executed_block = self
+            .clone()
+            .into_executed_block(execution_pending)
+            .await
+            .map_err(|e| self.handle_block_error(e))?;
+
+        match executed_block {
+            ExecutedBlock::Available(block) => {
+                self.import_available_block(Box::new(block), count_unrealized)
+                    .await
+            }
+            ExecutedBlock::AvailabilityPending(block) => {
+                self.check_availability_and_maybe_import(
+                    |chain| {
+                        chain
+                            .data_availability_checker
+                            .put_pending_executed_block(block)
+                    },
+                    count_unrealized,
+                )
                .await
-        };
-
-        // Verify and import the block.
-        match import_block.await {
-            // The block was successfully verified and imported. Yay.
-            Ok(block_root) => {
-                trace!(
-                    self.log,
-                    "Beacon block imported";
-                    "block_root" => ?block_root,
-                     "block_slot" => slot,
-                );
-
-                // Increment the Prometheus counter for block processing successes.
-                metrics::inc_counter(&metrics::BLOCK_PROCESSING_SUCCESSES);
-
-                Ok(block_root)
-            }
-            Err(e @ BlockError::BeaconChainError(BeaconChainError::TokioJoin(_))) => {
-                debug!(
-                    self.log,
-                    "Beacon block processing cancelled";
-                    "error" => ?e,
-                );
-                Err(e)
-            }
-            // There was an error whilst attempting to verify and import the block. The block might
-            // be partially verified or partially imported.
-            Err(BlockError::BeaconChainError(e)) => {
-                crit!(
-                    self.log,
-                    "Beacon block processing error";
-                    "error" => ?e,
-                );
-                Err(BlockError::BeaconChainError(e))
-            }
-            // The block failed verification.
-            Err(other) => {
-                trace!(
-                    self.log,
-                    "Beacon block rejected";
-                    "reason" => other.to_string(),
-                );
-                Err(other)
            }
        }
    }

-    /// Accepts a fully-verified block and imports it into the chain without performing any
-    /// additional verification.
+    /// Accepts a fully-verified block and awaits on it's payload verification handle to
+    /// get a fully `ExecutedBlock`
    ///
-    /// 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(
+    /// An error is returned if the verification handle couldn't be awaited.
+    async fn into_executed_block(
        self: Arc<Self>,
        execution_pending_block: ExecutionPendingBlock<T>,
-        count_unrealized: CountUnrealized,
-    ) -> Result<Hash256, BlockError<T::EthSpec>> {
+    ) -> Result<ExecutedBlock<T::EthSpec>, BlockError<T::EthSpec>> {
        let ExecutionPendingBlock {
            block,
-            block_root,
-            state,
-            parent_block,
-            confirmed_state_roots,
+            import_data,
            payload_verification_handle,
-            parent_eth1_finalization_data,
-            consensus_context,
        } = execution_pending_block;

-        let PayloadVerificationOutcome {
-            payload_verification_status,
-            is_valid_merge_transition_block,
-        } = payload_verification_handle
+        let payload_verification_outcome = payload_verification_handle
            .await
            .map_err(BeaconChainError::TokioJoin)?
            .ok_or(BeaconChainError::RuntimeShutdown)??;

        // Log the PoS pandas if a merge transition just occurred.
-        if is_valid_merge_transition_block {
+        if payload_verification_outcome.is_valid_merge_transition_block {
            info!(self.log, "{}", POS_PANDA_BANNER);
            info!(
                self.log,
@@ -2830,9 +2816,91 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                    .into_root()
            );
        }
+        Ok(ExecutedBlock::new(
+            block,
+            import_data,
+            payload_verification_outcome,
+        ))
+    }

+    fn handle_block_error(&self, e: BlockError<T::EthSpec>) -> BlockError<T::EthSpec> {
+        match e {
+            e @ BlockError::BeaconChainError(BeaconChainError::TokioJoin(_)) => {
+                debug!(
+                    self.log,
+                    "Beacon block processing cancelled";
+                    "error" => ?e,
+                );
+                e
+            }
+            BlockError::BeaconChainError(e) => {
+                crit!(
+                    self.log,
+                    "Beacon block processing error";
+                    "error" => ?e,
+                );
+                BlockError::BeaconChainError(e)
+            }
+            other => {
+                trace!(
+                    self.log,
+                    "Beacon block rejected";
+                    "reason" => other.to_string(),
+                );
+                other
+            }
+        }
+    }
+
+    /// Accepts a fully-verified, 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
+    /// (i.e., this function is not atomic).
+    pub async fn check_availability_and_maybe_import(
+        self: &Arc<Self>,
+        cache_fn: impl FnOnce(Arc<Self>) -> Result<Availability<T::EthSpec>, AvailabilityCheckError>,
+        count_unrealized: CountUnrealized,
+    ) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
+        let availability = cache_fn(self.clone())?;
+        match availability {
+            Availability::Available(block) => {
+                self.import_available_block(block, count_unrealized).await
+            }
+            Availability::PendingBlock(block_root) => {
+                Ok(AvailabilityProcessingStatus::PendingBlock(block_root))
+            }
+            Availability::PendingBlobs(blob_ids) => {
+                Ok(AvailabilityProcessingStatus::PendingBlobs(blob_ids))
+            }
+        }
+    }
+
+    async fn import_available_block(
+        self: &Arc<Self>,
+        block: Box<AvailableExecutedBlock<T::EthSpec>>,
+        count_unrealized: CountUnrealized,
+    ) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
+        let AvailableExecutedBlock {
+            block,
+            import_data,
+            payload_verification_outcome,
+        } = *block;
+
+        let BlockImportData {
+            block_root,
+            state,
+            parent_block,
+            parent_eth1_finalization_data,
+            confirmed_state_roots,
+            consensus_context,
+        } = import_data;
+
+        let slot = block.slot();
+
+        // import
        let chain = self.clone();
-        let block_hash = self
+        let result = self
            .spawn_blocking_handle(
                move || {
                    chain.import_block(
@@ -2840,7 +2908,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                        block_root,
                        state,
                        confirmed_state_roots,
-                        payload_verification_status,
+                        payload_verification_outcome.payload_verification_status,
                        count_unrealized,
                        parent_block,
                        parent_eth1_finalization_data,
@@ -2849,12 +2917,32 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                },
                "payload_verification_handle",
            )
-            .await??;
+            .await
+            .map_err(|e| {
+                let b = BlockError::from(e);
+                self.handle_block_error(b)
+            })?;

-        Ok(block_hash)
+        match result {
+            // The block was successfully verified and imported. Yay.
+            Ok(block_root) => {
+                trace!(
+                    self.log,
+                    "Beacon block imported";
+                    "block_root" => ?block_root,
+                     "block_slot" => slot,
+                );
+
+                // Increment the Prometheus counter for block processing successes.
+                metrics::inc_counter(&metrics::BLOCK_PROCESSING_SUCCESSES);
+
+                Ok(AvailabilityProcessingStatus::Imported(block_root))
+            }
+            Err(e) => Err(self.handle_block_error(e)),
+        }
    }

-    /// 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
@@ -3038,27 +3126,17 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
        ops.push(StoreOp::PutBlock(block_root, signed_block.clone()));
        ops.push(StoreOp::PutState(block.state_root(), &state));

-        // Only consider blobs if the eip4844 fork is enabled.
-        if let Some(data_availability_boundary) = self.data_availability_boundary() {
-            let block_epoch = block.slot().epoch(T::EthSpec::slots_per_epoch());
-            let margin_epochs = self.store.get_config().blob_prune_margin_epochs;
-            let import_boundary = data_availability_boundary - margin_epochs;
-
-            // Only store blobs at the data availability boundary, minus any configured epochs
-            // margin, or younger (of higher epoch number).
-            if block_epoch >= import_boundary {
-                if let Some(blobs) = blobs {
-                    if !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));
-                    }
-                }
+        if let Some(blobs) = blobs {
+            if !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));
            }
        }
+
        let txn_lock = self.store.hot_db.begin_rw_transaction();

        if let Err(e) = self.store.do_atomically_with_block_and_blobs_cache(ops) {
@@ -4782,12 +4860,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                .as_ref()
                .ok_or(BlockProductionError::TrustedSetupNotInitialized)?;
            let beacon_block_root = block.canonical_root();
-            let expected_kzg_commitments: &KzgCommitments<T::EthSpec> =
-                block.body().blob_kzg_commitments().map_err(|_| {
-                    BlockProductionError::InvalidBlockVariant(
-                        "EIP4844 block does not contain kzg commitments".to_string(),
-                    )
-                })?;
+            let expected_kzg_commitments = block.body().blob_kzg_commitments().map_err(|_| {
+                BlockProductionError::InvalidBlockVariant(
+                    "EIP4844 block does not contain kzg commitments".to_string(),
+                )
+            })?;

            if expected_kzg_commitments.len() != blobs.len() {
                return Err(BlockProductionError::MissingKzgCommitment(format!(
@@ -4836,7 +4913,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                    .collect::<Result<Vec<_>, BlockProductionError>>()?,
            );

-            self.blob_cache.put(beacon_block_root, blob_sidecars);
+            self.proposal_blob_cache
+                .put(beacon_block_root, blob_sidecars);
        }

        metrics::inc_counter(&metrics::BLOCK_PRODUCTION_SUCCESSES);
@@ -4854,8 +4932,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {

    fn compute_blob_kzg_proofs(
        kzg: &Arc<Kzg>,
-        blobs: &Blobs<<T as BeaconChainTypes>::EthSpec>,
-        expected_kzg_commitments: &KzgCommitments<<T as BeaconChainTypes>::EthSpec>,
+        blobs: &Blobs<T::EthSpec>,
+        expected_kzg_commitments: &KzgCommitments<T::EthSpec>,
        slot: Slot,
    ) -> Result<Vec<KzgProof>, BlockProductionError> {
        blobs
@@ -6119,18 +6197,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
        })
    }

-    /// The epoch that is a data availability boundary, or the latest finalized epoch.
-    /// `None` if the `Eip4844` fork is disabled.
-    pub fn finalized_data_availability_boundary(&self) -> Option<Epoch> {
-        self.data_availability_boundary().map(|boundary| {
-            std::cmp::max(
-                boundary,
-                self.canonical_head
-                    .cached_head()
-                    .finalized_checkpoint()
-                    .epoch,
-            )
-        })
+    /// Returns true if the given epoch lies within the da boundary and false otherwise.
+    pub fn block_needs_da_check(&self, block_epoch: Epoch) -> bool {
+        self.data_availability_boundary()
+            .map_or(false, |da_epoch| block_epoch >= da_epoch)
    }

    /// Returns `true` if we are at or past the `Eip4844` fork. This will always return `false` if