Strict match of errors in backfill sync (#6520)

* Strict match of errors in backfill sync

* Fix tests
This commit is contained in:
Lion - dapplion
2024-11-05 03:00:10 +02:00
committed by GitHub
parent 6a8d13e8a9
commit 38388979db
6 changed files with 113 additions and 143 deletions

View File

@@ -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),
})
}

View File

@@ -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);

View File

@@ -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,7 +10,11 @@ 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.
@@ -18,10 +22,8 @@ use types::{FixedBytesExtended, Hash256, Slot};
/// 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);

View File

@@ -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.

View File

@@ -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

View File

@@ -10,8 +10,8 @@ use beacon_chain::data_availability_checker::AvailabilityCheckError;
use beacon_chain::data_availability_checker::MaybeAvailableBlock;
use beacon_chain::data_column_verification::verify_kzg_for_data_column_list;
use beacon_chain::{
validator_monitor::get_slot_delay_ms, AvailabilityProcessingStatus, BeaconChainError,
BeaconChainTypes, BlockError, ChainSegmentResult, HistoricalBlockError, NotifyExecutionLayer,
validator_monitor::get_slot_delay_ms, AvailabilityProcessingStatus, BeaconChainTypes,
BlockError, ChainSegmentResult, HistoricalBlockError, NotifyExecutionLayer,
};
use beacon_processor::{
work_reprocessing_queue::{QueuedRpcBlock, ReprocessQueueMessage},
@@ -606,103 +606,75 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
);
(imported_blocks, Ok(()))
}
Err(error) => {
Err(e) => {
metrics::inc_counter(
&metrics::BEACON_PROCESSOR_BACKFILL_CHAIN_SEGMENT_FAILED_TOTAL,
);
let err = match error {
// Handle the historical block errors specifically
BeaconChainError::HistoricalBlockError(e) => match e {
HistoricalBlockError::MismatchedBlockRoot {
block_root,
expected_block_root,
} => {
debug!(
self.log,
"Backfill batch processing error";
"error" => "mismatched_block_root",
"block_root" => ?block_root,
"expected_root" => ?expected_block_root
);
ChainSegmentFailed {
message: String::from("mismatched_block_root"),
// The peer is faulty if they send blocks with bad roots.
peer_action: Some(PeerAction::LowToleranceError),
}
}
HistoricalBlockError::InvalidSignature
| HistoricalBlockError::SignatureSet(_) => {
warn!(
self.log,
"Backfill batch processing error";
"error" => ?e
);
ChainSegmentFailed {
message: "invalid_signature".into(),
// The peer is faulty if they bad signatures.
peer_action: Some(PeerAction::LowToleranceError),
}
}
HistoricalBlockError::ValidatorPubkeyCacheTimeout => {
warn!(
self.log,
"Backfill batch processing error";
"error" => "pubkey_cache_timeout"
);
ChainSegmentFailed {
message: "pubkey_cache_timeout".into(),
// This is an internal error, do not penalize the peer.
peer_action: None,
}
}
HistoricalBlockError::NoAnchorInfo => {
warn!(self.log, "Backfill not required");
ChainSegmentFailed {
message: String::from("no_anchor_info"),
// There is no need to do a historical sync, this is not a fault of
// the peer.
peer_action: None,
}
}
HistoricalBlockError::IndexOutOfBounds => {
error!(
self.log,
"Backfill batch OOB error";
"error" => ?e,
);
ChainSegmentFailed {
message: String::from("logic_error"),
// This should never occur, don't penalize the peer.
peer_action: None,
}
}
HistoricalBlockError::BlockOutOfRange { .. } => {
error!(
self.log,
"Backfill batch error";
"error" => ?e,
);
ChainSegmentFailed {
message: String::from("unexpected_error"),
// This should never occur, don't penalize the peer.
peer_action: None,
}
}
},
other => {
warn!(self.log, "Backfill batch processing error"; "error" => ?other);
ChainSegmentFailed {
message: format!("{:?}", other),
// This is an internal error, don't penalize the peer.
peer_action: None,
}
let peer_action = match &e {
HistoricalBlockError::MismatchedBlockRoot {
block_root,
expected_block_root,
} => {
debug!(
self.log,
"Backfill batch processing error";
"error" => "mismatched_block_root",
"block_root" => ?block_root,
"expected_root" => ?expected_block_root
);
// The peer is faulty if they send blocks with bad roots.
Some(PeerAction::LowToleranceError)
}
HistoricalBlockError::InvalidSignature
| HistoricalBlockError::SignatureSet(_) => {
warn!(
self.log,
"Backfill batch processing error";
"error" => ?e
);
// The peer is faulty if they bad signatures.
Some(PeerAction::LowToleranceError)
}
HistoricalBlockError::ValidatorPubkeyCacheTimeout => {
warn!(
self.log,
"Backfill batch processing error";
"error" => "pubkey_cache_timeout"
);
// This is an internal error, do not penalize the peer.
None
}
HistoricalBlockError::NoAnchorInfo => {
warn!(self.log, "Backfill not required");
// There is no need to do a historical sync, this is not a fault of
// the peer.
None
}
HistoricalBlockError::IndexOutOfBounds => {
error!(
self.log,
"Backfill batch OOB error";
"error" => ?e,
);
// This should never occur, don't penalize the peer.
None
}
HistoricalBlockError::StoreError(e) => {
warn!(self.log, "Backfill batch processing error"; "error" => ?e);
// This is an internal error, don't penalize the peer.
None
} //
// Do not use a fallback match, handle all errors explicitly
};
(0, Err(err))
let err_str: &'static str = e.into();
(
0,
Err(ChainSegmentFailed {
message: format!("{:?}", err_str),
// This is an internal error, don't penalize the peer.
peer_action,
}),
)
}
}
}