Type sync network context send errors (#5808)

* Type sync network context send errors

* Consisntent naming
This commit is contained in:
Lion - dapplion
2024-05-17 14:34:21 +03:00
committed by GitHub
parent 319b4a2467
commit 8006418d80
6 changed files with 119 additions and 103 deletions

View File

@@ -952,7 +952,7 @@ impl<T: BeaconChainTypes> BackFillSync<T> {
Err(e) => { Err(e) => {
// NOTE: under normal conditions this shouldn't happen but we handle it anyway // NOTE: under normal conditions this shouldn't happen but we handle it anyway
warn!(self.log, "Could not send batch request"; warn!(self.log, "Could not send batch request";
"batch_id" => batch_id, "error" => e, &batch); "batch_id" => batch_id, "error" => ?e, &batch);
// register the failed download and check if the batch can be retried // register the failed download and check if the batch can be retried
if let Err(e) = batch.start_downloading_from_peer(peer, 1) { if let Err(e) = batch.start_downloading_from_peer(peer, 1) {
return self.fail_sync(BackFillError::BatchInvalidState(batch_id, e.0)); return self.fail_sync(BackFillError::BatchInvalidState(batch_id, e.0));

View File

@@ -82,7 +82,7 @@ impl<T: BeaconChainTypes> RequestState<T> for BlockRequestState<T::EthSpec> {
cx: &mut SyncNetworkContext<T>, cx: &mut SyncNetworkContext<T>,
) -> Result<LookupRequestResult, LookupRequestError> { ) -> Result<LookupRequestResult, LookupRequestError> {
cx.block_lookup_request(id, peer_id, self.requested_block_root) cx.block_lookup_request(id, peer_id, self.requested_block_root)
.map_err(LookupRequestError::SendFailed) .map_err(LookupRequestError::SendFailedNetwork)
} }
fn send_for_processing( fn send_for_processing(
@@ -102,7 +102,7 @@ impl<T: BeaconChainTypes> RequestState<T> for BlockRequestState<T::EthSpec> {
RpcBlock::new_without_blobs(Some(block_root), value), RpcBlock::new_without_blobs(Some(block_root), value),
seen_timestamp, seen_timestamp,
) )
.map_err(LookupRequestError::SendFailed) .map_err(LookupRequestError::SendFailedProcessor)
} }
fn response_type() -> ResponseType { fn response_type() -> ResponseType {
@@ -135,7 +135,7 @@ impl<T: BeaconChainTypes> RequestState<T> for BlobRequestState<T::EthSpec> {
self.block_root, self.block_root,
downloaded_block_expected_blobs, downloaded_block_expected_blobs,
) )
.map_err(LookupRequestError::SendFailed) .map_err(LookupRequestError::SendFailedNetwork)
} }
fn send_for_processing( fn send_for_processing(
@@ -150,7 +150,7 @@ impl<T: BeaconChainTypes> RequestState<T> for BlobRequestState<T::EthSpec> {
peer_id: _, peer_id: _,
} = download_result; } = download_result;
cx.send_blobs_for_processing(id, block_root, value, seen_timestamp) cx.send_blobs_for_processing(id, block_root, value, seen_timestamp)
.map_err(LookupRequestError::SendFailed) .map_err(LookupRequestError::SendFailedProcessor)
} }
fn response_type() -> ResponseType { fn response_type() -> ResponseType {

View File

@@ -2,7 +2,7 @@ use self::parent_chain::{compute_parent_chains, NodeChain};
pub use self::single_block_lookup::DownloadResult; pub use self::single_block_lookup::DownloadResult;
use self::single_block_lookup::{LookupRequestError, LookupResult, SingleBlockLookup}; use self::single_block_lookup::{LookupRequestError, LookupResult, SingleBlockLookup};
use super::manager::{BlockProcessType, BlockProcessingResult}; use super::manager::{BlockProcessType, BlockProcessingResult};
use super::network_context::{RpcProcessingResult, SyncNetworkContext}; use super::network_context::{RpcResponseResult, SyncNetworkContext};
use crate::metrics; use crate::metrics;
use crate::sync::block_lookups::common::{ResponseType, PARENT_DEPTH_TOLERANCE}; use crate::sync::block_lookups::common::{ResponseType, PARENT_DEPTH_TOLERANCE};
use crate::sync::block_lookups::parent_chain::find_oldest_fork_ancestor; use crate::sync::block_lookups::parent_chain::find_oldest_fork_ancestor;
@@ -313,7 +313,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
&mut self, &mut self,
id: SingleLookupReqId, id: SingleLookupReqId,
peer_id: PeerId, peer_id: PeerId,
response: RpcProcessingResult<R::VerifiedResponseType>, response: RpcResponseResult<R::VerifiedResponseType>,
cx: &mut SyncNetworkContext<T>, cx: &mut SyncNetworkContext<T>,
) { ) {
let result = self.on_download_response_inner::<R>(id, peer_id, response, cx); let result = self.on_download_response_inner::<R>(id, peer_id, response, cx);
@@ -325,7 +325,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
&mut self, &mut self,
id: SingleLookupReqId, id: SingleLookupReqId,
peer_id: PeerId, peer_id: PeerId,
response: RpcProcessingResult<R::VerifiedResponseType>, response: RpcResponseResult<R::VerifiedResponseType>,
cx: &mut SyncNetworkContext<T>, cx: &mut SyncNetworkContext<T>,
) -> Result<LookupResult, LookupRequestError> { ) -> Result<LookupResult, LookupRequestError> {
// Note: no need to downscore peers here, already downscored on network context // Note: no need to downscore peers here, already downscored on network context

View File

@@ -2,7 +2,9 @@ use super::common::ResponseType;
use super::{BlockComponent, PeerId, SINGLE_BLOCK_LOOKUP_MAX_ATTEMPTS}; use super::{BlockComponent, PeerId, SINGLE_BLOCK_LOOKUP_MAX_ATTEMPTS};
use crate::sync::block_lookups::common::RequestState; use crate::sync::block_lookups::common::RequestState;
use crate::sync::block_lookups::Id; use crate::sync::block_lookups::Id;
use crate::sync::network_context::{LookupRequestResult, ReqId, SyncNetworkContext}; use crate::sync::network_context::{
LookupRequestResult, ReqId, RpcRequestSendError, SendErrorProcessor, SyncNetworkContext,
};
use beacon_chain::BeaconChainTypes; use beacon_chain::BeaconChainTypes;
use derivative::Derivative; use derivative::Derivative;
use itertools::Itertools; use itertools::Itertools;
@@ -34,8 +36,10 @@ pub enum LookupRequestError {
}, },
/// No peers left to serve this lookup /// No peers left to serve this lookup
NoPeers, NoPeers,
/// Error sending event to network or beacon processor /// Error sending event to network
SendFailed(&'static str), SendFailedNetwork(RpcRequestSendError),
/// Error sending event to processor
SendFailedProcessor(SendErrorProcessor),
/// Inconsistent lookup request state /// Inconsistent lookup request state
BadState(String), BadState(String),
/// Lookup failed for some other reason and should be dropped /// Lookup failed for some other reason and should be dropped

View File

@@ -52,31 +52,43 @@ pub enum RpcEvent<T> {
RPCError(RPCError), RPCError(RPCError),
} }
pub type RpcProcessingResult<T> = Result<(T, Duration), LookupFailure>; pub type RpcResponseResult<T> = Result<(T, Duration), RpcResponseError>;
pub enum LookupFailure { pub enum RpcResponseError {
RpcError(RPCError), RpcError(RPCError),
LookupVerifyError(LookupVerifyError), VerifyError(LookupVerifyError),
} }
impl std::fmt::Display for LookupFailure { #[derive(Debug, PartialEq, Eq)]
pub enum RpcRequestSendError {
/// Network channel send failed
NetworkSendError,
}
#[derive(Debug, PartialEq, Eq)]
pub enum SendErrorProcessor {
SendError,
ProcessorNotAvailable,
}
impl std::fmt::Display for RpcResponseError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match self { match self {
LookupFailure::RpcError(e) => write!(f, "RPC Error: {:?}", e), RpcResponseError::RpcError(e) => write!(f, "RPC Error: {:?}", e),
LookupFailure::LookupVerifyError(e) => write!(f, "Lookup Verify Error: {:?}", e), RpcResponseError::VerifyError(e) => write!(f, "Lookup Verify Error: {:?}", e),
} }
} }
} }
impl From<RPCError> for LookupFailure { impl From<RPCError> for RpcResponseError {
fn from(e: RPCError) -> Self { fn from(e: RPCError) -> Self {
LookupFailure::RpcError(e) RpcResponseError::RpcError(e)
} }
} }
impl From<LookupVerifyError> for LookupFailure { impl From<LookupVerifyError> for RpcResponseError {
fn from(e: LookupVerifyError) -> Self { fn from(e: LookupVerifyError) -> Self {
LookupFailure::LookupVerifyError(e) RpcResponseError::VerifyError(e)
} }
} }
@@ -209,7 +221,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
peer_id: PeerId, peer_id: PeerId,
batch_type: ByRangeRequestType, batch_type: ByRangeRequestType,
request: BlocksByRangeRequest, request: BlocksByRangeRequest,
) -> Result<Id, &'static str> { ) -> Result<Id, RpcRequestSendError> {
let id = self.next_id(); let id = self.next_id();
trace!( trace!(
self.log, self.log,
@@ -218,11 +230,13 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
"count" => request.count(), "count" => request.count(),
"peer" => %peer_id, "peer" => %peer_id,
); );
self.send_network_msg(NetworkMessage::SendRequest { self.network_send
peer_id, .send(NetworkMessage::SendRequest {
request: Request::BlocksByRange(request.clone()), peer_id,
request_id: RequestId::Sync(SyncRequestId::RangeBlockAndBlobs { id }), request: Request::BlocksByRange(request.clone()),
})?; request_id: RequestId::Sync(SyncRequestId::RangeBlockAndBlobs { id }),
})
.map_err(|_| RpcRequestSendError::NetworkSendError)?;
if matches!(batch_type, ByRangeRequestType::BlocksAndBlobs) { if matches!(batch_type, ByRangeRequestType::BlocksAndBlobs) {
debug!( debug!(
@@ -234,14 +248,16 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
); );
// Create the blob request based on the blocks request. // Create the blob request based on the blocks request.
self.send_network_msg(NetworkMessage::SendRequest { self.network_send
peer_id, .send(NetworkMessage::SendRequest {
request: Request::BlobsByRange(BlobsByRangeRequest { peer_id,
start_slot: *request.start_slot(), request: Request::BlobsByRange(BlobsByRangeRequest {
count: *request.count(), start_slot: *request.start_slot(),
}), count: *request.count(),
request_id: RequestId::Sync(SyncRequestId::RangeBlockAndBlobs { id }), }),
})?; request_id: RequestId::Sync(SyncRequestId::RangeBlockAndBlobs { id }),
})
.map_err(|_| RpcRequestSendError::NetworkSendError)?;
} }
Ok(id) Ok(id)
@@ -254,7 +270,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
batch_type: ByRangeRequestType, batch_type: ByRangeRequestType,
request: BlocksByRangeRequest, request: BlocksByRangeRequest,
sender_id: RangeRequestId, sender_id: RangeRequestId,
) -> Result<Id, &'static str> { ) -> Result<Id, RpcRequestSendError> {
let id = self.blocks_by_range_request(peer_id, batch_type, request)?; let id = self.blocks_by_range_request(peer_id, batch_type, request)?;
self.range_blocks_and_blobs_requests self.range_blocks_and_blobs_requests
.insert(id, (sender_id, BlocksAndBlobsRequestInfo::new(batch_type))); .insert(id, (sender_id, BlocksAndBlobsRequestInfo::new(batch_type)));
@@ -320,7 +336,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
lookup_id: SingleLookupId, lookup_id: SingleLookupId,
peer_id: PeerId, peer_id: PeerId,
block_root: Hash256, block_root: Hash256,
) -> Result<LookupRequestResult, &'static str> { ) -> Result<LookupRequestResult, RpcRequestSendError> {
// da_checker includes block that are execution verified, but are missing components // da_checker includes block that are execution verified, but are missing components
if self if self
.chain .chain
@@ -357,11 +373,13 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
let request = BlocksByRootSingleRequest(block_root); let request = BlocksByRootSingleRequest(block_root);
self.send_network_msg(NetworkMessage::SendRequest { self.network_send
peer_id, .send(NetworkMessage::SendRequest {
request: Request::BlocksByRoot(request.into_request(&self.chain.spec)), peer_id,
request_id: RequestId::Sync(SyncRequestId::SingleBlock { id }), request: Request::BlocksByRoot(request.into_request(&self.chain.spec)),
})?; request_id: RequestId::Sync(SyncRequestId::SingleBlock { id }),
})
.map_err(|_| RpcRequestSendError::NetworkSendError)?;
self.blocks_by_root_requests self.blocks_by_root_requests
.insert(id, ActiveBlocksByRootRequest::new(request)); .insert(id, ActiveBlocksByRootRequest::new(request));
@@ -381,7 +399,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
peer_id: PeerId, peer_id: PeerId,
block_root: Hash256, block_root: Hash256,
downloaded_block_expected_blobs: Option<usize>, downloaded_block_expected_blobs: Option<usize>,
) -> Result<LookupRequestResult, &'static str> { ) -> Result<LookupRequestResult, RpcRequestSendError> {
let Some(expected_blobs) = downloaded_block_expected_blobs.or_else(|| { let Some(expected_blobs) = downloaded_block_expected_blobs.or_else(|| {
self.chain self.chain
.data_availability_checker .data_availability_checker
@@ -428,11 +446,13 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
indices, indices,
}; };
self.send_network_msg(NetworkMessage::SendRequest { self.network_send
peer_id, .send(NetworkMessage::SendRequest {
request: Request::BlobsByRoot(request.clone().into_request(&self.chain.spec)), peer_id,
request_id: RequestId::Sync(SyncRequestId::SingleBlob { id }), request: Request::BlobsByRoot(request.clone().into_request(&self.chain.spec)),
})?; request_id: RequestId::Sync(SyncRequestId::SingleBlob { id }),
})
.map_err(|_| RpcRequestSendError::NetworkSendError)?;
self.blobs_by_root_requests self.blobs_by_root_requests
.insert(id, ActiveBlobsByRootRequest::new(request)); .insert(id, ActiveBlobsByRootRequest::new(request));
@@ -549,7 +569,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
request_id: SingleLookupReqId, request_id: SingleLookupReqId,
peer_id: PeerId, peer_id: PeerId,
block: RpcEvent<Arc<SignedBeaconBlock<T::EthSpec>>>, block: RpcEvent<Arc<SignedBeaconBlock<T::EthSpec>>>,
) -> Option<RpcProcessingResult<Arc<SignedBeaconBlock<T::EthSpec>>>> { ) -> Option<RpcResponseResult<Arc<SignedBeaconBlock<T::EthSpec>>>> {
let Entry::Occupied(mut request) = self.blocks_by_root_requests.entry(request_id) else { let Entry::Occupied(mut request) = self.blocks_by_root_requests.entry(request_id) else {
return None; return None;
}; };
@@ -575,7 +595,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
} }
}; };
if let Err(LookupFailure::LookupVerifyError(e)) = &resp { if let Err(RpcResponseError::VerifyError(e)) = &resp {
self.report_peer(peer_id, PeerAction::LowToleranceError, e.into()); self.report_peer(peer_id, PeerAction::LowToleranceError, e.into());
} }
Some(resp) Some(resp)
@@ -586,7 +606,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
request_id: SingleLookupReqId, request_id: SingleLookupReqId,
peer_id: PeerId, peer_id: PeerId,
blob: RpcEvent<Arc<BlobSidecar<T::EthSpec>>>, blob: RpcEvent<Arc<BlobSidecar<T::EthSpec>>>,
) -> Option<RpcProcessingResult<FixedBlobSidecarList<T::EthSpec>>> { ) -> Option<RpcResponseResult<FixedBlobSidecarList<T::EthSpec>>> {
let Entry::Occupied(mut request) = self.blobs_by_root_requests.entry(request_id) else { let Entry::Occupied(mut request) = self.blobs_by_root_requests.entry(request_id) else {
return None; return None;
}; };
@@ -618,7 +638,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
// catch if a peer is returning more blobs than requested or if the excess blobs are // catch if a peer is returning more blobs than requested or if the excess blobs are
// invalid. // invalid.
Err((e, resolved)) => { Err((e, resolved)) => {
if let LookupFailure::LookupVerifyError(e) = &e { if let RpcResponseError::VerifyError(e) = &e {
self.report_peer(peer_id, PeerAction::LowToleranceError, e.into()); self.report_peer(peer_id, PeerAction::LowToleranceError, e.into());
} }
if resolved { if resolved {
@@ -636,31 +656,27 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
block_root: Hash256, block_root: Hash256,
block: RpcBlock<T::EthSpec>, block: RpcBlock<T::EthSpec>,
duration: Duration, duration: Duration,
) -> Result<(), &'static str> { ) -> Result<(), SendErrorProcessor> {
match self.beacon_processor_if_enabled() { let beacon_processor = self
Some(beacon_processor) => { .beacon_processor_if_enabled()
debug!(self.log, "Sending block for processing"; "block" => ?block_root, "id" => id); .ok_or(SendErrorProcessor::ProcessorNotAvailable)?;
if let Err(e) = beacon_processor.send_rpc_beacon_block(
block_root, debug!(self.log, "Sending block for processing"; "block" => ?block_root, "id" => id);
block, beacon_processor
duration, .send_rpc_beacon_block(
BlockProcessType::SingleBlock { id }, block_root,
) { block,
error!( duration,
self.log, BlockProcessType::SingleBlock { id },
"Failed to send sync block to processor"; )
"error" => ?e .map_err(|e| {
); error!(
Err("beacon processor send failure") self.log,
} else { "Failed to send sync block to processor";
Ok(()) "error" => ?e
} );
} SendErrorProcessor::SendError
None => { })
trace!(self.log, "Dropping block ready for processing. Beacon processor not available"; "block" => %block_root);
Err("beacon processor unavailable")
}
}
} }
pub fn send_blobs_for_processing( pub fn send_blobs_for_processing(
@@ -669,31 +685,27 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
block_root: Hash256, block_root: Hash256,
blobs: FixedBlobSidecarList<T::EthSpec>, blobs: FixedBlobSidecarList<T::EthSpec>,
duration: Duration, duration: Duration,
) -> Result<(), &'static str> { ) -> Result<(), SendErrorProcessor> {
match self.beacon_processor_if_enabled() { let beacon_processor = self
Some(beacon_processor) => { .beacon_processor_if_enabled()
debug!(self.log, "Sending blobs for processing"; "block" => ?block_root, "id" => id); .ok_or(SendErrorProcessor::ProcessorNotAvailable)?;
if let Err(e) = beacon_processor.send_rpc_blobs(
block_root, debug!(self.log, "Sending blobs for processing"; "block" => ?block_root, "id" => id);
blobs, beacon_processor
duration, .send_rpc_blobs(
BlockProcessType::SingleBlob { id }, block_root,
) { blobs,
error!( duration,
self.log, BlockProcessType::SingleBlob { id },
"Failed to send sync blobs to processor"; )
"error" => ?e .map_err(|e| {
); error!(
Err("beacon processor send failure") self.log,
} else { "Failed to send sync blobs to processor";
Ok(()) "error" => ?e
} );
} SendErrorProcessor::SendError
None => { })
trace!(self.log, "Dropping blobs ready for processing. Beacon processor not available"; "block_root" => %block_root);
Err("beacon processor unavailable")
}
}
} }
} }

View File

@@ -923,7 +923,7 @@ impl<T: BeaconChainTypes> SyncingChain<T> {
Err(e) => { Err(e) => {
// NOTE: under normal conditions this shouldn't happen but we handle it anyway // NOTE: under normal conditions this shouldn't happen but we handle it anyway
warn!(self.log, "Could not send batch request"; warn!(self.log, "Could not send batch request";
"batch_id" => batch_id, "error" => e, &batch); "batch_id" => batch_id, "error" => ?e, &batch);
// register the failed download and check if the batch can be retried // register the failed download and check if the batch can be retried
batch.start_downloading_from_peer(peer, 1)?; // fake request_id is not relevant batch.start_downloading_from_peer(peer, 1)?; // fake request_id is not relevant
self.peers self.peers