Strict match of errors in backfill sync (#6520)

* Strict match of errors in backfill sync

* Fix tests

Author: dapplion

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -34,7 +34,6 @@ use crate::execution_payload::{get_execution_payload, NotifyExecutionLayer, Prep
 use crate::fork_choice_signal::{ForkChoiceSignalRx, ForkChoiceSignalTx, ForkChoiceWaitResult};
 use crate::graffiti_calculator::GraffitiCalculator;
 use crate::head_tracker::{HeadTracker, HeadTrackerReader, SszHeadTracker};
-use crate::historical_blocks::HistoricalBlockError;
 use crate::light_client_finality_update_verification::{
     Error as LightClientFinalityUpdateError, VerifiedLightClientFinalityUpdate,
 };
@@ -755,12 +754,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     ) -> Result<impl Iterator<Item = Result<(Hash256, Slot), Error>> + '_, Error> {
         let oldest_block_slot = self.store.get_oldest_block_slot();
         if start_slot < oldest_block_slot {
-            return Err(Error::HistoricalBlockError(
-                HistoricalBlockError::BlockOutOfRange {
-                    slot: start_slot,
-                    oldest_block_slot,
-                },
-            ));
+            return Err(Error::HistoricalBlockOutOfRange {
+                slot: start_slot,
+                oldest_block_slot,
+            });
         }
 
         let local_head = self.head_snapshot();
@@ -785,12 +782,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     ) -> Result<impl Iterator<Item = Result<(Hash256, Slot), Error>> + '_, Error> {
         let oldest_block_slot = self.store.get_oldest_block_slot();
         if start_slot < oldest_block_slot {
-            return Err(Error::HistoricalBlockError(
-                HistoricalBlockError::BlockOutOfRange {
-                    slot: start_slot,
-                    oldest_block_slot,
-                },
-            ));
+            return Err(Error::HistoricalBlockOutOfRange {
+                slot: start_slot,
+                oldest_block_slot,
+            });
         }
 
         self.with_head(move |head| {
@@ -991,7 +986,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             WhenSlotSkipped::Prev => self.block_root_at_slot_skips_prev(request_slot),
         }
         .or_else(|e| match e {
-            Error::HistoricalBlockError(_) => Ok(None),
+            Error::HistoricalBlockOutOfRange { .. } => Ok(None),
             e => Err(e),
         })
     }

beacon_node/beacon_chain/src/errors.rs

@@ -4,7 +4,6 @@ use crate::beacon_chain::ForkChoiceError;
 use crate::beacon_fork_choice_store::Error as ForkChoiceStoreError;
 use crate::data_availability_checker::AvailabilityCheckError;
 use crate::eth1_chain::Error as Eth1ChainError;
-use crate::historical_blocks::HistoricalBlockError;
 use crate::migrate::PruningError;
 use crate::naive_aggregation_pool::Error as NaiveAggregationError;
 use crate::observed_aggregates::Error as ObservedAttestationsError;
@@ -123,7 +122,11 @@ pub enum BeaconChainError {
         block_slot: Slot,
         state_slot: Slot,
     },
-    HistoricalBlockError(HistoricalBlockError),
+    /// Block is not available (only returned when fetching historic blocks).
+    HistoricalBlockOutOfRange {
+        slot: Slot,
+        oldest_block_slot: Slot,
+    },
     InvalidStateForShuffling {
         state_epoch: Epoch,
         shuffling_epoch: Epoch,
@@ -245,7 +248,6 @@ easy_from_to!(BlockSignatureVerifierError, BeaconChainError);
 easy_from_to!(PruningError, BeaconChainError);
 easy_from_to!(ArithError, BeaconChainError);
 easy_from_to!(ForkChoiceStoreError, BeaconChainError);
-easy_from_to!(HistoricalBlockError, BeaconChainError);
 easy_from_to!(StateAdvanceError, BeaconChainError);
 easy_from_to!(BlockReplayError, BeaconChainError);
 easy_from_to!(InconsistentFork, BeaconChainError);

beacon_node/beacon_chain/src/historical_blocks.rs

@@ -1,5 +1,5 @@
 use crate::data_availability_checker::AvailableBlock;
-use crate::{errors::BeaconChainError as Error, metrics, BeaconChain, BeaconChainTypes};
+use crate::{metrics, BeaconChain, BeaconChainTypes};
 use itertools::Itertools;
 use slog::debug;
 use state_processing::{
@@ -10,18 +10,20 @@ use std::borrow::Cow;
 use std::iter;
 use std::time::Duration;
 use store::metadata::DataColumnInfo;
-use store::{chunked_vector::BlockRoots, AnchorInfo, BlobInfo, ChunkWriter, KeyValueStore};
+use store::{
+    chunked_vector::BlockRoots, AnchorInfo, BlobInfo, ChunkWriter, Error as StoreError,
+    KeyValueStore,
+};
+use strum::IntoStaticStr;
 use types::{FixedBytesExtended, Hash256, Slot};
 
 /// Use a longer timeout on the pubkey cache.
 ///
 /// It's ok if historical sync is stalled due to writes from forwards block processing.
 const PUBKEY_CACHE_LOCK_TIMEOUT: Duration = Duration::from_secs(30);
 
-#[derive(Debug)]
+#[derive(Debug, IntoStaticStr)]
 pub enum HistoricalBlockError {
-    /// Block is not available (only returned when fetching historic blocks).
-    BlockOutOfRange { slot: Slot, oldest_block_slot: Slot },
     /// Block root mismatch, caller should retry with different blocks.
     MismatchedBlockRoot {
         block_root: Hash256,
@@ -37,6 +39,14 @@ pub enum HistoricalBlockError {
     NoAnchorInfo,
     /// Logic error: should never occur.
     IndexOutOfBounds,
+    /// Internal store error
+    StoreError(StoreError),
+}
+
+impl From<StoreError> for HistoricalBlockError {
+    fn from(e: StoreError) -> Self {
+        Self::StoreError(e)
+    }
 }
 
 impl<T: BeaconChainTypes> BeaconChain<T> {
@@ -61,7 +71,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     pub fn import_historical_block_batch(
         &self,
         mut blocks: Vec<AvailableBlock<T::EthSpec>>,
-    ) -> Result<usize, Error> {
+    ) -> Result<usize, HistoricalBlockError> {
         let anchor_info = self
             .store
             .get_anchor_info()
@@ -127,8 +137,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 return Err(HistoricalBlockError::MismatchedBlockRoot {
                     block_root,
                     expected_block_root,
-                }
-                .into());
+                });
             }
 
             let blinded_block = block.clone_as_blinded();
@@ -212,7 +221,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
         let verify_timer = metrics::start_timer(&metrics::BACKFILL_SIGNATURE_VERIFY_TIMES);
         if !signature_set.verify() {
-            return Err(HistoricalBlockError::InvalidSignature.into());
+            return Err(HistoricalBlockError::InvalidSignature);
         }
         drop(verify_timer);
         drop(sig_timer);

beacon_node/beacon_chain/tests/store_tests.rs

@@ -2669,9 +2669,7 @@ async fn weak_subjectivity_sync_test(slots: Vec<Slot>, checkpoint_slot: Slot) {
     // Forwards iterator from 0 should fail as we lack blocks.
     assert!(matches!(
         beacon_chain.forwards_iter_block_roots(Slot::new(0)),
-        Err(BeaconChainError::HistoricalBlockError(
-            HistoricalBlockError::BlockOutOfRange { .. }
-        ))
+        Err(BeaconChainError::HistoricalBlockOutOfRange { .. })
     ));
 
     // Simulate processing of a `StatusMessage` with an older finalized epoch by calling
@@ -2739,7 +2737,7 @@ async fn weak_subjectivity_sync_test(slots: Vec<Slot>, checkpoint_slot: Slot) {
         beacon_chain
             .import_historical_block_batch(batch_with_invalid_first_block)
             .unwrap_err(),
-        BeaconChainError::HistoricalBlockError(HistoricalBlockError::InvalidSignature)
+        HistoricalBlockError::InvalidSignature
     ));
 
     // Importing the batch with valid signatures should succeed.

beacon_node/network/src/network_beacon_processor/rpc_methods.rs

@@ -2,7 +2,7 @@ use crate::network_beacon_processor::{NetworkBeaconProcessor, FUTURE_SLOT_TOLERA
 use crate::service::NetworkMessage;
 use crate::status::ToStatusMessage;
 use crate::sync::SyncMessage;
-use beacon_chain::{BeaconChainError, BeaconChainTypes, HistoricalBlockError, WhenSlotSkipped};
+use beacon_chain::{BeaconChainError, BeaconChainTypes, WhenSlotSkipped};
 use itertools::process_results;
 use lighthouse_network::discovery::ConnectionId;
 use lighthouse_network::rpc::methods::{
@@ -682,12 +682,10 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
             .forwards_iter_block_roots(Slot::from(*req.start_slot()))
         {
             Ok(iter) => iter,
-            Err(BeaconChainError::HistoricalBlockError(
-                HistoricalBlockError::BlockOutOfRange {
-                    slot,
-                    oldest_block_slot,
-                },
-            )) => {
+            Err(BeaconChainError::HistoricalBlockOutOfRange {
+                slot,
+                oldest_block_slot,
+            }) => {
                 debug!(self.log, "Range request failed during backfill";
                     "requested_slot" => slot,
                     "oldest_known_slot" => oldest_block_slot
@@ -941,12 +939,10 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
         let forwards_block_root_iter =
             match self.chain.forwards_iter_block_roots(request_start_slot) {
                 Ok(iter) => iter,
-                Err(BeaconChainError::HistoricalBlockError(
-                    HistoricalBlockError::BlockOutOfRange {
-                        slot,
-                        oldest_block_slot,
-                    },
-                )) => {
+                Err(BeaconChainError::HistoricalBlockOutOfRange {
+                    slot,
+                    oldest_block_slot,
+                }) => {
                     debug!(self.log, "Range request failed during backfill";
                         "requested_slot" => slot,
                         "oldest_known_slot" => oldest_block_slot
@@ -1147,12 +1143,10 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
         let forwards_block_root_iter =
             match self.chain.forwards_iter_block_roots(request_start_slot) {
                 Ok(iter) => iter,
-                Err(BeaconChainError::HistoricalBlockError(
-                    HistoricalBlockError::BlockOutOfRange {
-                        slot,
-                        oldest_block_slot,
-                    },
-                )) => {
+                Err(BeaconChainError::HistoricalBlockOutOfRange {
+                    slot,
+                    oldest_block_slot,
+                }) => {
                     debug!(self.log, "Range request failed during backfill";
                         "requested_slot" => slot,
                         "oldest_known_slot" => oldest_block_slot