chore(download): cleanup bodies downloader errors (#1026)

This commit is contained in:
Roman Krasiuk
2023-01-25 12:05:24 +02:00
committed by GitHub
parent 93194e8a0c
commit c5cd64bb0e
4 changed files with 40 additions and 46 deletions

1
Cargo.lock generated
View File

@ -4136,7 +4136,6 @@ dependencies = [
"reth-metrics-derive",
"reth-primitives",
"reth-tracing",
"thiserror",
"tokio",
"tracing",
]

View File

@ -90,6 +90,7 @@ pub type DownloadResult<T> = Result<T, DownloadError>;
/// The downloader error type
#[derive(Error, Debug, Clone)]
pub enum DownloadError {
/* ==================== HEADER ERRORS ==================== */
/// Header validation failed
#[error("Failed to validate header {hash}. Details: {error}.")]
HeaderValidation {
@ -99,18 +100,6 @@ pub enum DownloadError {
#[source]
error: consensus::Error,
},
/// Block validation failed
#[error("Failed to validate body for header {hash}. Details: {error}.")]
BlockValidation {
/// Hash of header failing validation
hash: H256,
/// The details of validation failure
#[source]
error: consensus::Error,
},
/// Timed out while waiting for request id response.
#[error("Timed out while getting headers for request.")]
Timeout,
/// Error when checking that the current [`Header`] has the parent's hash as the parent_hash
/// field, and that they have sequential block numbers.
#[error("Headers did not match, current number: {header_number} / current hash: {header_hash}, parent number: {parent_number} / parent_hash: {parent_hash}")]
@ -124,9 +113,6 @@ pub enum DownloadError {
/// The parent hash being evaluated
parent_hash: H256,
},
/// Received empty response while expecting headers
#[error("Received empty header response.")]
EmptyResponse,
/// Received an invalid tip
#[error("Received invalid tip: {received:?}. Expected {expected:?}.")]
InvalidTip {
@ -151,6 +137,31 @@ pub enum DownloadError {
/// How many headers we expected.
expected: u64,
},
/* ==================== BODIES ERRORS ==================== */
/// Block validation failed
#[error("Failed to validate body for header {hash}. Details: {error}.")]
BodyValidation {
/// Hash of header failing validation
hash: H256,
/// The details of validation failure
#[source]
error: consensus::Error,
},
/// Received more bodies than requested.
#[error("Received more bodies than requested. Expected: {expected}. Received: {received}")]
TooManyBodies {
/// How many bodies we received.
received: usize,
/// How many bodies we expected.
expected: usize,
},
/* ==================== COMMON ERRORS ==================== */
/// Timed out while waiting for request id response.
#[error("Timed out while waiting for response.")]
Timeout,
/// Received empty response while expecting non empty
#[error("Received empty response.")]
EmptyResponse,
/// Error while executing the request.
#[error(transparent)]
RequestError(#[from] RequestError),

View File

@ -21,7 +21,6 @@ futures-util = "0.3.25"
# misc
tracing = "0.1.37"
thiserror = "1.0"
metrics = "0.20.1"
[dev-dependencies]

View File

@ -1,10 +1,10 @@
use futures::{Future, FutureExt};
use reth_eth_wire::BlockBody;
use reth_interfaces::{
consensus::{Consensus as ConsensusTrait, Consensus, Error as ConsensusError},
consensus::{Consensus as ConsensusTrait, Consensus},
p2p::{
bodies::{client::BodiesClient, response::BlockResponse},
error::{PeerRequestResult, RequestError},
error::{DownloadError, PeerRequestResult},
},
};
use reth_primitives::{PeerId, SealedBlock, SealedHeader, H256};
@ -13,22 +13,9 @@ use std::{
sync::Arc,
task::{ready, Context, Poll},
};
use thiserror::Error;
type BodiesFut = Pin<Box<dyn Future<Output = PeerRequestResult<Vec<BlockBody>>> + Send>>;
#[derive(Error, Debug)]
enum BodyRequestError {
#[error("Received empty response")]
EmptyResponse,
#[error("Received more bodies than requested. Expected: {expected}. Received: {received}")]
TooManyBodies { expected: usize, received: usize },
#[error("Error validating body for header {hash:?}: {error}")]
Validation { hash: H256, error: ConsensusError },
#[error(transparent)]
Request(#[from] RequestError),
}
/// Body request implemented as a [Future].
///
/// The future will poll the underlying request until fullfilled.
@ -86,7 +73,7 @@ where
self
}
fn on_error(&mut self, error: BodyRequestError, peer_id: Option<PeerId>) {
fn on_error(&mut self, error: DownloadError, peer_id: Option<PeerId>) {
tracing::error!(target: "downloaders::bodies", ?peer_id, %error, "Error requesting bodies");
if let Some(peer_id) = peer_id {
self.client.report_bad_message(peer_id);
@ -114,7 +101,7 @@ where
///
/// If the number of buffered bodies does not equal the number of non empty headers.
#[allow(clippy::result_large_err)]
fn try_construct_blocks(&mut self) -> Result<Vec<BlockResponse>, (PeerId, BodyRequestError)> {
fn try_construct_blocks(&mut self) -> Result<Vec<BlockResponse>, (PeerId, DownloadError)> {
// Drop the allocated memory for the buffer. Optimistically, it will not be reused.
let mut bodies = std::mem::take(&mut self.buffer).into_iter();
let mut results = Vec::default();
@ -135,7 +122,7 @@ where
// This ensures that the TxRoot and OmmersRoot from the header match the
// ones calculated manually from the block body.
self.consensus.pre_validate_block(&block).map_err(|error| {
(peer_id, BodyRequestError::Validation { hash: header.hash(), error })
(peer_id, DownloadError::BodyValidation { hash: header.hash(), error })
})?;
results.push(BlockResponse::Full(block));
@ -161,16 +148,20 @@ where
match ready!(fut.poll_unpin(cx)) {
Ok(response) => {
let (peer_id, bodies) = response.split();
if bodies.is_empty() {
this.on_error(BodyRequestError::EmptyResponse, Some(peer_id));
let request_len = this.hashes_to_download.len();
let response_len = bodies.len();
// Malicious peers often return a single block. Mark responses with single
// block when more than 1 were requested invalid.
// TODO: Instead of marking single block responses invalid, calculate
// soft response size lower limit and use that for filtering.
if bodies.is_empty() || (request_len != 1 && response_len == 1) {
this.on_error(DownloadError::EmptyResponse, Some(peer_id));
continue
}
let request_len = this.hashes_to_download.len();
let response_len = bodies.len();
if response_len > request_len {
this.on_error(
BodyRequestError::TooManyBodies {
DownloadError::TooManyBodies {
expected: request_len,
received: response_len,
},
@ -179,12 +170,6 @@ where
continue
}
// TODO: Consider limiting
if request_len != 1 && response_len == 1 {
this.on_error(BodyRequestError::EmptyResponse, Some(peer_id));
continue
}
tracing::trace!(
target: "downloaders::bodies", request_len, response_len, ?peer_id, "Received bodies"
);