From cb22b17b3e6da030ac74cf4ae83913f8af1714b3 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 8 Jan 2025 14:34:28 +0100 Subject: [PATCH] chore: rm blockchaintree dep from engine-tree (#13729) --- Cargo.lock | 1 - crates/engine/tree/Cargo.toml | 1 - crates/engine/tree/src/lib.rs | 3 - crates/engine/tree/src/tree/error.rs | 199 +++++++++++++++++++++++++++ crates/engine/tree/src/tree/mod.rs | 49 +++++-- 5 files changed, 233 insertions(+), 20 deletions(-) create mode 100644 crates/engine/tree/src/tree/error.rs diff --git a/Cargo.lock b/Cargo.lock index 0e3c08dc5..23c3eabb2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7195,7 +7195,6 @@ dependencies = [ "proptest", "rand 0.8.5", "rayon", - "reth-blockchain-tree-api", "reth-chain-state", "reth-chainspec", "reth-consensus", diff --git a/crates/engine/tree/Cargo.toml b/crates/engine/tree/Cargo.toml index 7376bf238..bd5e70319 100644 --- a/crates/engine/tree/Cargo.toml +++ b/crates/engine/tree/Cargo.toml @@ -12,7 +12,6 @@ workspace = true [dependencies] # reth -reth-blockchain-tree-api.workspace = true reth-chain-state.workspace = true reth-chainspec = { workspace = true, optional = true } reth-consensus.workspace = true diff --git a/crates/engine/tree/src/lib.rs b/crates/engine/tree/src/lib.rs index 19eecf8d6..f197dd764 100644 --- a/crates/engine/tree/src/lib.rs +++ b/crates/engine/tree/src/lib.rs @@ -92,9 +92,6 @@ #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] #![cfg_attr(not(test), warn(unused_crate_dependencies))] -/// Re-export of the blockchain tree API. -pub use reth_blockchain_tree_api::*; - /// Support for backfill sync mode. pub mod backfill; /// The type that drives the chain forward. diff --git a/crates/engine/tree/src/tree/error.rs b/crates/engine/tree/src/tree/error.rs new file mode 100644 index 000000000..025655315 --- /dev/null +++ b/crates/engine/tree/src/tree/error.rs @@ -0,0 +1,199 @@ +//! Internal errors for the tree module. + +use alloy_consensus::BlockHeader; +use reth_consensus::ConsensusError; +use reth_errors::{BlockExecutionError, BlockValidationError, ProviderError}; +use reth_evm::execute::InternalBlockExecutionError; +use reth_primitives::SealedBlockFor; +use reth_primitives_traits::{Block, BlockBody}; +use tokio::sync::oneshot::error::TryRecvError; + +/// This is an error that can come from advancing persistence. Either this can be a +/// [`TryRecvError`], or this can be a [`ProviderError`] +#[derive(Debug, thiserror::Error)] +pub enum AdvancePersistenceError { + /// An error that can be from failing to receive a value from persistence + #[error(transparent)] + RecvError(#[from] TryRecvError), + /// A provider error + #[error(transparent)] + Provider(#[from] ProviderError), +} + +#[derive(thiserror::Error)] +#[error("Failed to insert block (hash={}, number={}, parent_hash={}): {}", + .block.hash(), + .block.number(), + .block.parent_hash(), + .kind)] +struct InsertBlockErrorDataTwo { + block: SealedBlockFor, + #[source] + kind: InsertBlockErrorKindTwo, +} + +impl std::fmt::Debug for InsertBlockErrorDataTwo { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("InsertBlockError") + .field("error", &self.kind) + .field("hash", &self.block.hash()) + .field("number", &self.block.number()) + .field("parent_hash", &self.block.parent_hash()) + .field("num_txs", &self.block.body().transactions().len()) + .finish_non_exhaustive() + } +} + +impl InsertBlockErrorDataTwo { + const fn new(block: SealedBlockFor, kind: InsertBlockErrorKindTwo) -> Self { + Self { block, kind } + } + + fn boxed(block: SealedBlockFor, kind: InsertBlockErrorKindTwo) -> Box { + Box::new(Self::new(block, kind)) + } +} + +/// Error thrown when inserting a block failed because the block is considered invalid. +#[derive(thiserror::Error)] +#[error(transparent)] +pub struct InsertBlockErrorTwo { + inner: Box>, +} + +// === impl InsertBlockErrorTwo === + +impl InsertBlockErrorTwo { + /// Create a new `InsertInvalidBlockErrorTwo` + pub fn new(block: SealedBlockFor, kind: InsertBlockErrorKindTwo) -> Self { + Self { inner: InsertBlockErrorDataTwo::boxed(block, kind) } + } + + /// Create a new `InsertInvalidBlockError` from a consensus error + pub fn consensus_error(error: ConsensusError, block: SealedBlockFor) -> Self { + Self::new(block, InsertBlockErrorKindTwo::Consensus(error)) + } + + /// Create a new `InsertInvalidBlockError` from a consensus error + pub fn sender_recovery_error(block: SealedBlockFor) -> Self { + Self::new(block, InsertBlockErrorKindTwo::SenderRecovery) + } + + /// Consumes the error and returns the block that resulted in the error + #[inline] + pub fn into_block(self) -> SealedBlockFor { + self.inner.block + } + + /// Returns the error kind + #[inline] + pub const fn kind(&self) -> &InsertBlockErrorKindTwo { + &self.inner.kind + } + + /// Returns the block that resulted in the error + #[inline] + pub const fn block(&self) -> &SealedBlockFor { + &self.inner.block + } + + /// Consumes the type and returns the block and error kind. + #[inline] + pub fn split(self) -> (SealedBlockFor, InsertBlockErrorKindTwo) { + let inner = *self.inner; + (inner.block, inner.kind) + } +} + +impl std::fmt::Debug for InsertBlockErrorTwo { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Debug::fmt(&self.inner, f) + } +} + +/// All error variants possible when inserting a block +#[derive(Debug, thiserror::Error)] +pub enum InsertBlockErrorKindTwo { + /// Failed to recover senders for the block + #[error("failed to recover senders for block")] + SenderRecovery, + /// Block violated consensus rules. + #[error(transparent)] + Consensus(#[from] ConsensusError), + /// Block execution failed. + #[error(transparent)] + Execution(#[from] BlockExecutionError), + /// Provider error. + #[error(transparent)] + Provider(#[from] ProviderError), + /// Other errors. + #[error(transparent)] + Other(#[from] Box), +} + +impl InsertBlockErrorKindTwo { + /// Returns an [`InsertBlockValidationError`] if the error is caused by an invalid block. + /// + /// Returns an [`InsertBlockFatalError`] if the error is caused by an error that is not + /// validation related or is otherwise fatal. + /// + /// This is intended to be used to determine if we should respond `INVALID` as a response when + /// processing a new block. + pub fn ensure_validation_error( + self, + ) -> Result { + match self { + Self::SenderRecovery => Ok(InsertBlockValidationError::SenderRecovery), + Self::Consensus(err) => Ok(InsertBlockValidationError::Consensus(err)), + // other execution errors that are considered internal errors + Self::Execution(err) => { + match err { + BlockExecutionError::Validation(err) => { + Ok(InsertBlockValidationError::Validation(err)) + } + BlockExecutionError::Consensus(err) => { + Ok(InsertBlockValidationError::Consensus(err)) + } + // these are internal errors, not caused by an invalid block + BlockExecutionError::Internal(error) => { + Err(InsertBlockFatalError::BlockExecutionError(error)) + } + } + } + Self::Provider(err) => Err(InsertBlockFatalError::Provider(err)), + Self::Other(err) => Err(InternalBlockExecutionError::Other(err).into()), + } + } +} + +/// Error variants that are not caused by invalid blocks +#[derive(Debug, thiserror::Error)] +pub enum InsertBlockFatalError { + /// A provider error + #[error(transparent)] + Provider(#[from] ProviderError), + /// An internal / fatal block execution error + #[error(transparent)] + BlockExecutionError(#[from] InternalBlockExecutionError), +} + +/// Error variants that are caused by invalid blocks +#[derive(Debug, thiserror::Error)] +pub enum InsertBlockValidationError { + /// Failed to recover senders for the block + #[error("failed to recover senders for block")] + SenderRecovery, + /// Block violated consensus rules. + #[error(transparent)] + Consensus(#[from] ConsensusError), + /// Validation error, transparently wrapping [`BlockValidationError`] + #[error(transparent)] + Validation(#[from] BlockValidationError), +} + +impl InsertBlockValidationError { + /// Returns true if this is a block pre merge error. + pub const fn is_block_pre_merge(&self) -> bool { + matches!(self, Self::Validation(BlockValidationError::BlockPreMerge { .. })) + } +} diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index 1db3e4a70..c678e290f 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -16,10 +16,7 @@ use alloy_rpc_types_engine::{ PayloadValidationError, }; use block_buffer::BlockBuffer; -use reth_blockchain_tree_api::{ - error::{InsertBlockErrorKindTwo, InsertBlockErrorTwo, InsertBlockFatalError}, - BlockStatus2, InsertPayloadOk2, -}; +use error::{InsertBlockErrorKindTwo, InsertBlockErrorTwo, InsertBlockFatalError}; use reth_chain_state::{ CanonicalInMemoryState, ExecutedBlock, MemoryOverlayStateProvider, NewCanonicalChain, }; @@ -78,6 +75,7 @@ use tracing::*; mod block_buffer; pub mod config; +pub mod error; mod invalid_block_hook; mod invalid_headers; mod metrics; @@ -85,7 +83,10 @@ mod persistence_state; pub mod root; mod trie_updates; -use crate::tree::{config::MIN_BLOCKS_FOR_PIPELINE_RUN, invalid_headers::InvalidHeaderCache}; +use crate::tree::{ + config::MIN_BLOCKS_FOR_PIPELINE_RUN, error::AdvancePersistenceError, + invalid_headers::InvalidHeaderCache, +}; pub use config::TreeConfig; pub use invalid_block_hook::{InvalidBlockHooks, NoopInvalidBlockHook}; pub use persistence_state::PersistenceState; @@ -2764,16 +2765,34 @@ where } } -/// This is an error that can come from advancing persistence. Either this can be a -/// [`TryRecvError`], or this can be a [`ProviderError`] -#[derive(Debug, thiserror::Error)] -pub enum AdvancePersistenceError { - /// An error that can be from failing to receive a value from persistence - #[error(transparent)] - RecvError(#[from] TryRecvError), - /// A provider error - #[error(transparent)] - Provider(#[from] ProviderError), +/// Block inclusion can be valid, accepted, or invalid. Invalid blocks are returned as an error +/// variant. +/// +/// If we don't know the block's parent, we return `Disconnected`, as we can't claim that the block +/// is valid or not. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum BlockStatus2 { + /// The block is valid and block extends canonical chain. + Valid, + /// The block may be valid and has an unknown missing ancestor. + Disconnected { + /// Current canonical head. + head: BlockNumHash, + /// The lowest ancestor block that is not connected to the canonical chain. + missing_ancestor: BlockNumHash, + }, +} + +/// How a payload was inserted if it was valid. +/// +/// If the payload was valid, but has already been seen, [`InsertPayloadOk2::AlreadySeen(_)`] is +/// returned, otherwise [`InsertPayloadOk2::Inserted(_)`] is returned. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum InsertPayloadOk2 { + /// The payload was valid, but we have already seen it. + AlreadySeen(BlockStatus2), + /// The payload was valid and inserted into the tree. + Inserted(BlockStatus2), } #[cfg(test)]