Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: streamline OpBlockExecutionError error type to replace a hard-coded BlockExecutionError #13696

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/reth/src/commands/debug_cmd/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl<C: ChainSpecParser<ChainSpec = ChainSpec>> Command<C> {
provider_factory: ProviderFactory<N>,
task_executor: &TaskExecutor,
static_file_producer: StaticFileProducer<ProviderFactory<N>>,
) -> eyre::Result<Pipeline<N>>
) -> eyre::Result<Pipeline<N, E>>
where
N: ProviderNodeTypes<ChainSpec = C::ChainSpec, Primitives = EthPrimitives> + CliNodeTypes,
Client: EthBlockClient + 'static,
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/commands/src/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ pub fn build_import_pipeline<N, C, E>(
static_file_producer: StaticFileProducer<ProviderFactory<N>>,
disable_exec: bool,
executor: E,
) -> eyre::Result<(Pipeline<N>, impl Stream<Item = NodeEvent<N::Primitives>>)>
) -> eyre::Result<(Pipeline<N, E>, impl Stream<Item = NodeEvent<N::Primitives>>)>
where
N: ProviderNodeTypes + CliNodeTypes,
C: Consensus<BlockTy<N>, Error = ConsensusError> + 'static,
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/commands/src/stage/unwind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl<C: ChainSpecParser<ChainSpec: EthChainSpec + EthereumHardforks>> Command<C>
self,
config: Config,
provider_factory: ProviderFactory<N>,
) -> Result<Pipeline<N>, eyre::Error> {
) -> Result<Pipeline<N, E>, eyre::Error> {
let stage_conf = &config.stages;
let prune_modes = config.prune.clone().map(|prune| prune.segments).unwrap_or_default();

Expand Down
2 changes: 1 addition & 1 deletion crates/engine/service/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ where
chain_spec: Arc<N::ChainSpec>,
client: Client,
incoming_requests: EngineMessageStream<N::Engine>,
pipeline: Pipeline<N>,
pipeline: Pipeline<N, E>,
pipeline_task_spawner: Box<dyn TaskSpawner>,
provider: ProviderFactory<N>,
blockchain_db: BlockchainProvider<N>,
Expand Down
9 changes: 5 additions & 4 deletions crates/engine/tree/src/backfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//! These modes are mutually exclusive and the node can only be in one mode at a time.

use futures::FutureExt;
use reth_errors::GenericBlockExecutionError;
use reth_provider::providers::ProviderNodeTypes;
use reth_stages_api::{ControlFlow, Pipeline, PipelineError, PipelineTarget, PipelineWithResult};
use reth_tasks::TaskSpawner;
Expand Down Expand Up @@ -90,7 +91,7 @@ pub struct PipelineSync<N: ProviderNodeTypes> {

impl<N: ProviderNodeTypes> PipelineSync<N> {
/// Create a new instance.
pub fn new(pipeline: Pipeline<N>, pipeline_task_spawner: Box<dyn TaskSpawner>) -> Self {
pub fn new(pipeline: Pipeline<N, E>, pipeline_task_spawner: Box<dyn TaskSpawner>) -> Self {
Self {
pipeline_task_spawner,
pipeline_state: PipelineState::Idle(Some(pipeline)),
Expand Down Expand Up @@ -212,11 +213,11 @@ impl<N: ProviderNodeTypes> BackfillSync for PipelineSync<N> {
/// blockchain tree any messages that would result in database writes, since it would result in a
/// deadlock.
#[derive(Debug)]
enum PipelineState<N: ProviderNodeTypes> {
enum PipelineState<N: ProviderNodeTypes, E: GenericBlockExecutionError> {
/// Pipeline is idle.
Idle(Option<Pipeline<N>>),
Idle(Option<Pipeline<N, E>>),
/// Pipeline is running and waiting for a response
Running(oneshot::Receiver<PipelineWithResult<N>>),
Running(oneshot::Receiver<PipelineWithResult<N, E>>),
}

impl<N: ProviderNodeTypes> PipelineState<N> {
Expand Down
4 changes: 3 additions & 1 deletion crates/errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ mod error;
pub use error::{RethError, RethResult};

pub use reth_consensus::ConsensusError;
pub use reth_execution_errors::{BlockExecutionError, BlockValidationError};
pub use reth_execution_errors::{
BlockExecutionError, BlockValidationError, GenericBlockExecutionError,
};
pub use reth_storage_errors::{
db::DatabaseError,
provider::{ProviderError, ProviderResult},
Expand Down
2 changes: 2 additions & 0 deletions crates/ethereum/evm/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ where
Transaction = reth_primitives::TransactionSigned,
>,
{
type Error = BlockExecutionError;

type Primitives = EthPrimitives;

type Strategy<DB: Database<Error: Into<ProviderError> + Display>> =
Expand Down
21 changes: 16 additions & 5 deletions crates/evm/execution-errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use alloc::{
};
use alloy_eips::BlockNumHash;
use alloy_primitives::B256;
use core::fmt::Display;
use reth_consensus::ConsensusError;
use reth_prune_types::PruneSegmentError;
use reth_storage_errors::provider::ProviderError;
Expand Down Expand Up @@ -134,6 +135,21 @@ pub enum BlockExecutionError {
Internal(#[from] InternalBlockExecutionError),
}

/// Generic block execution error.
pub trait GenericBlockExecutionError:
chungquantin marked this conversation as resolved.
Show resolved Hide resolved
Display + Sync + Send + std::fmt::Debug + 'static + From<ConsensusError> + From<BlockExecutionError>
{
/// Returns `true` if the error is a state root error.
fn is_state_root_error(&self) -> bool;
}

impl GenericBlockExecutionError for BlockExecutionError {
/// Returns `true` if the error is a state root error.
fn is_state_root_error(&self) -> bool {
matches!(self, Self::Validation(BlockValidationError::StateRoot(_)))
}
}

impl BlockExecutionError {
/// Create a new [`BlockExecutionError::Internal`] variant, containing a
/// [`InternalBlockExecutionError::Other`] error.
Expand All @@ -157,11 +173,6 @@ impl BlockExecutionError {
_ => None,
}
}

/// Returns `true` if the error is a state root error.
pub const fn is_state_root_error(&self) -> bool {
matches!(self, Self::Validation(BlockValidationError::StateRoot(_)))
}
}

impl From<ProviderError> for BlockExecutionError {
Expand Down
4 changes: 3 additions & 1 deletion crates/evm/src/either.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ use revm::State;
impl<A, B> BlockExecutorProvider for Either<A, B>
where
A: BlockExecutorProvider,
B: BlockExecutorProvider<Primitives = A::Primitives>,
B: BlockExecutorProvider<Primitives = A::Primitives, Error = A::Error>,
{
type Primitives = A::Primitives;

type Error = A::Error;

type Executor<DB: Database<Error: Into<ProviderError> + Display>> =
Either<A::Executor<DB>, B::Executor<DB>>;

Expand Down
25 changes: 18 additions & 7 deletions crates/evm/src/execute.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Traits for execution.

use alloy_consensus::BlockHeader;
use reth_execution_errors::GenericBlockExecutionError;
// Re-export execution types
pub use reth_execution_errors::{
BlockExecutionError, BlockValidationError, InternalBlockExecutionError,
Expand Down Expand Up @@ -80,7 +81,7 @@ pub trait BatchExecutor<DB> {
/// The output type for the executor.
type Output;
/// The error type returned by the executor.
type Error;
type Error: GenericBlockExecutionError;

/// Executes the next block in the batch, verifies the output and updates the state internally.
fn execute_and_verify_one(&mut self, input: Self::Input<'_>) -> Result<(), Self::Error>;
Expand Down Expand Up @@ -137,6 +138,9 @@ pub trait BlockExecutorProvider: Send + Sync + Clone + Unpin + 'static {
/// Receipt type.
type Primitives: NodePrimitives;

/// The error type returned by the executor.
type Error: GenericBlockExecutionError;

/// An executor that can execute a single block given a database.
///
/// # Verification
Expand All @@ -152,15 +156,15 @@ pub trait BlockExecutorProvider: Send + Sync + Clone + Unpin + 'static {
DB,
Input<'a> = &'a RecoveredBlock<<Self::Primitives as NodePrimitives>::Block>,
Output = BlockExecutionOutput<<Self::Primitives as NodePrimitives>::Receipt>,
Error = BlockExecutionError,
Error = Self::Error,
>;

/// An executor that can execute a batch of blocks given a database.
type BatchExecutor<DB: Database<Error: Into<ProviderError> + Display>>: for<'a> BatchExecutor<
DB,
Input<'a> = &'a RecoveredBlock<<Self::Primitives as NodePrimitives>::Block>,
Output = ExecutionOutcome<<Self::Primitives as NodePrimitives>::Receipt>,
Error = BlockExecutionError,
Error = Self::Error,
>;

/// Creates a new executor for single block execution.
Expand Down Expand Up @@ -197,7 +201,7 @@ pub trait BlockExecutionStrategy {
type Primitives: NodePrimitives;

/// The error type returned by this strategy's methods.
type Error: From<ProviderError> + core::error::Error;
type Error: GenericBlockExecutionError;

/// Initialize the strategy with the given transaction environment overrides.
fn init(&mut self, _tx_env_overrides: Box<dyn TxEnvOverrides>) {}
Expand Down Expand Up @@ -249,14 +253,17 @@ pub trait BlockExecutionStrategy {

/// A strategy factory that can create block execution strategies.
pub trait BlockExecutionStrategyFactory: Send + Sync + Clone + Unpin + 'static {
/// The error type returned by this strategy's methods.
type Error: GenericBlockExecutionError;

/// Primitive types used by the strategy.
type Primitives: NodePrimitives;

/// Associated strategy type.
type Strategy<DB: Database<Error: Into<ProviderError> + Display>>: BlockExecutionStrategy<
DB = DB,
Primitives = Self::Primitives,
Error = BlockExecutionError,
Error = Self::Error,
>;

/// Creates a strategy using the give database.
Expand Down Expand Up @@ -291,6 +298,8 @@ impl<F> BlockExecutorProvider for BasicBlockExecutorProvider<F>
where
F: BlockExecutionStrategyFactory,
{
type Error = F::Error;

type Primitives = F::Primitives;

type Executor<DB: Database<Error: Into<ProviderError> + Display>> =
Expand Down Expand Up @@ -421,12 +430,12 @@ where

impl<S, DB> BatchExecutor<DB> for BasicBatchExecutor<S>
where
S: BlockExecutionStrategy<DB = DB, Error = BlockExecutionError>,
S: BlockExecutionStrategy<DB = DB>,
DB: Database<Error: Into<ProviderError> + Display>,
{
type Input<'a> = &'a RecoveredBlock<<S::Primitives as NodePrimitives>::Block>;
type Output = ExecutionOutcome<<S::Primitives as NodePrimitives>::Receipt>;
type Error = BlockExecutionError;
type Error = S::Error;

fn execute_and_verify_one(&mut self, block: Self::Input<'_>) -> Result<(), Self::Error> {
if self.batch_record.first_block().is_none() {
Expand Down Expand Up @@ -525,6 +534,7 @@ mod tests {
struct TestExecutorProvider;

impl BlockExecutorProvider for TestExecutorProvider {
type Error = BlockExecutionError;
type Primitives = EthPrimitives;
type Executor<DB: Database<Error: Into<ProviderError> + Display>> = TestExecutor<DB>;
type BatchExecutor<DB: Database<Error: Into<ProviderError> + Display>> = TestExecutor<DB>;
Expand Down Expand Up @@ -623,6 +633,7 @@ mod tests {
}

impl BlockExecutionStrategyFactory for TestExecutorStrategyFactory {
type Error = BlockExecutionError;
type Primitives = EthPrimitives;
type Strategy<DB: Database<Error: Into<ProviderError> + Display>> =
TestExecutorStrategy<DB, TestEvmConfig>;
Expand Down
3 changes: 2 additions & 1 deletion crates/evm/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ mod tests {
use super::*;
use alloy_eips::eip7685::Requests;
use metrics_util::debugging::{DebugValue, DebuggingRecorder, Snapshotter};
use reth_execution_errors::BlockExecutionError;
use revm::db::BundleState;
use revm_primitives::{
Account, AccountInfo, AccountStatus, EvmState, EvmStorage, EvmStorageSlot, B256, U256,
Expand All @@ -162,7 +163,7 @@ mod tests {
where
Self: 'a;
type Output = BlockExecutionOutput<()>;
type Error = std::convert::Infallible;
type Error = BlockExecutionError;

fn execute(self, _input: Self::Input<'_>) -> Result<Self::Output, Self::Error> {
Ok(BlockExecutionOutput {
Expand Down
2 changes: 2 additions & 0 deletions crates/evm/src/noop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ pub struct NoopBlockExecutorProvider<P>(core::marker::PhantomData<P>);
impl<P: NodePrimitives> BlockExecutorProvider for NoopBlockExecutorProvider<P> {
type Primitives = P;

type Error = BlockExecutionError;

type Executor<DB: Database<Error: Into<ProviderError> + Display>> = Self;

type BatchExecutor<DB: Database<Error: Into<ProviderError> + Display>> = Self;
Expand Down
2 changes: 2 additions & 0 deletions crates/evm/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ impl MockExecutorProvider {
impl BlockExecutorProvider for MockExecutorProvider {
type Primitives = EthPrimitives;

type Error = BlockExecutionError;

type Executor<DB: Database<Error: Into<ProviderError> + Display>> = Self;

type BatchExecutor<DB: Database<Error: Into<ProviderError> + Display>> = Self;
Expand Down
20 changes: 16 additions & 4 deletions crates/exex/exex/src/backfill/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ pub struct BackfillJob<E, P> {

impl<E, P> Iterator for BackfillJob<E, P>
where
E: BlockExecutorProvider<Primitives: NodePrimitives<Block = P::Block>>,
E: BlockExecutorProvider<
Primitives: NodePrimitives<Block = P::Block>,
Error = BlockExecutionError,
>,
P: HeaderProvider + BlockReader<Transaction: SignedTransaction> + StateProviderFactory,
{
type Item = BackfillJobResult<Chain<E::Primitives>>;
Expand All @@ -55,7 +58,10 @@ where

impl<E, P> BackfillJob<E, P>
where
E: BlockExecutorProvider<Primitives: NodePrimitives<Block = P::Block>>,
E: BlockExecutorProvider<
Primitives: NodePrimitives<Block = P::Block>,
Error = BlockExecutionError,
>,
P: BlockReader<Transaction: SignedTransaction> + HeaderProvider + StateProviderFactory,
{
/// Converts the backfill job into a single block backfill job.
Expand Down Expand Up @@ -161,7 +167,10 @@ pub struct SingleBlockBackfillJob<E, P> {

impl<E, P> Iterator for SingleBlockBackfillJob<E, P>
where
E: BlockExecutorProvider<Primitives: NodePrimitives<Block = P::Block>>,
E: BlockExecutorProvider<
Primitives: NodePrimitives<Block = P::Block>,
Error = BlockExecutionError,
>,
P: HeaderProvider + BlockReader + StateProviderFactory,
{
type Item = BackfillJobResult<(
Expand All @@ -176,7 +185,10 @@ where

impl<E, P> SingleBlockBackfillJob<E, P>
where
E: BlockExecutorProvider<Primitives: NodePrimitives<Block = P::Block>>,
E: BlockExecutorProvider<
Primitives: NodePrimitives<Block = P::Block>,
Error = BlockExecutionError,
>,
P: HeaderProvider + BlockReader + StateProviderFactory,
{
/// Converts the single block backfill job into a stream.
Expand Down
12 changes: 10 additions & 2 deletions crates/exex/exex/src/backfill/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,11 @@ where

impl<E, P> Stream for StreamBackfillJob<E, P, SingleBlockStreamItem<E::Primitives>>
where
E: BlockExecutorProvider<Primitives: NodePrimitives<Block = P::Block>> + Clone + 'static,
E: BlockExecutorProvider<
Primitives: NodePrimitives<Block = P::Block>,
Error = BlockExecutionError,
> + Clone
+ 'static,
P: BlockReader + StateProviderFactory + Clone + Unpin + 'static,
{
type Item = BackfillJobResult<SingleBlockStreamItem<E::Primitives>>;
Expand Down Expand Up @@ -147,7 +151,11 @@ where

impl<E, P> Stream for StreamBackfillJob<E, P, BatchBlockStreamItem<E::Primitives>>
where
E: BlockExecutorProvider<Primitives: NodePrimitives<Block = P::Block>> + Clone + 'static,
E: BlockExecutorProvider<
Primitives: NodePrimitives<Block = P::Block>,
Error = BlockExecutionError,
> + Clone
+ 'static,
P: BlockReader + StateProviderFactory + Clone + Unpin + 'static,
{
type Item = BackfillJobResult<BatchBlockStreamItem<E::Primitives>>;
Expand Down
Loading
Loading