diff --git a/Cargo.lock b/Cargo.lock index 24e52df2e..bfbaf3ac7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7401,6 +7401,7 @@ dependencies = [ "reth-consensus", "reth-db", "reth-db-common", + "reth-ethereum-engine-primitives", "reth-evm", "reth-execution-types", "reth-exex", diff --git a/crates/engine/primitives/src/lib.rs b/crates/engine/primitives/src/lib.rs index 8e1b6441a..2cf1366eb 100644 --- a/crates/engine/primitives/src/lib.rs +++ b/crates/engine/primitives/src/lib.rs @@ -31,9 +31,6 @@ pub trait EngineTypes: + Serialize + 'static { - /// The chain specification of the node. - type ChainSpec: Send + Sync; - /// Execution Payload V1 type. type ExecutionPayloadV1: DeserializeOwned + Serialize + Clone + Unpin + Send + Sync + 'static; /// Execution Payload V2 type. @@ -42,12 +39,22 @@ pub trait EngineTypes: type ExecutionPayloadV3: DeserializeOwned + Serialize + Clone + Unpin + Send + Sync + 'static; /// Execution Payload V4 type. type ExecutionPayloadV4: DeserializeOwned + Serialize + Clone + Unpin + Send + Sync + 'static; +} +/// Type that validates the payloads sent to the engine. +pub trait EngineValidator: Clone + Send + Sync + Unpin + 'static { /// Validates the presence or exclusion of fork-specific fields based on the payload attributes /// and the message version. fn validate_version_specific_fields( - chain_spec: &Self::ChainSpec, + &self, version: EngineApiMessageVersion, - payload_or_attrs: PayloadOrAttributes<'_, Self::PayloadAttributes>, + payload_or_attrs: PayloadOrAttributes<'_, ::PayloadAttributes>, + ) -> Result<(), EngineObjectValidationError>; + + /// Ensures that the payload attributes are valid for the given [`EngineApiMessageVersion`]. + fn ensure_well_formed_attributes( + &self, + version: EngineApiMessageVersion, + attributes: &::PayloadAttributes, ) -> Result<(), EngineObjectValidationError>; } diff --git a/crates/ethereum/engine-primitives/src/lib.rs b/crates/ethereum/engine-primitives/src/lib.rs index 955cdf8bb..92243d729 100644 --- a/crates/ethereum/engine-primitives/src/lib.rs +++ b/crates/ethereum/engine-primitives/src/lib.rs @@ -9,9 +9,11 @@ #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] mod payload; +use std::sync::Arc; + pub use payload::{EthBuiltPayload, EthPayloadBuilderAttributes}; use reth_chainspec::ChainSpec; -use reth_engine_primitives::EngineTypes; +use reth_engine_primitives::{EngineTypes, EngineValidator}; use reth_payload_primitives::{ validate_version_specific_fields, EngineApiMessageVersion, EngineObjectValidationError, PayloadOrAttributes, PayloadTypes, @@ -36,18 +38,42 @@ impl PayloadTypes for EthEngineTypes { } impl EngineTypes for EthEngineTypes { - type ChainSpec = ChainSpec; - type ExecutionPayloadV1 = ExecutionPayloadV1; type ExecutionPayloadV2 = ExecutionPayloadEnvelopeV2; type ExecutionPayloadV3 = ExecutionPayloadEnvelopeV3; type ExecutionPayloadV4 = ExecutionPayloadEnvelopeV4; +} +/// Validator for the ethereum engine API. +#[derive(Debug, Clone)] +pub struct EthereumEngineValidator { + chain_spec: Arc, +} + +impl EthereumEngineValidator { + /// Instantiates a new validator. + pub const fn new(chain_spec: Arc) -> Self { + Self { chain_spec } + } +} + +impl EngineValidator for EthereumEngineValidator +where + Types: EngineTypes, +{ fn validate_version_specific_fields( - chain_spec: &ChainSpec, + &self, version: EngineApiMessageVersion, payload_or_attrs: PayloadOrAttributes<'_, EthPayloadAttributes>, ) -> Result<(), EngineObjectValidationError> { - validate_version_specific_fields(chain_spec, version, payload_or_attrs) + validate_version_specific_fields(&self.chain_spec, version, payload_or_attrs) + } + + fn ensure_well_formed_attributes( + &self, + version: EngineApiMessageVersion, + attributes: &EthPayloadAttributes, + ) -> Result<(), EngineObjectValidationError> { + validate_version_specific_fields(&self.chain_spec, version, attributes.into()) } } diff --git a/crates/ethereum/node/src/node.rs b/crates/ethereum/node/src/node.rs index c7b8aee73..30e32c068 100644 --- a/crates/ethereum/node/src/node.rs +++ b/crates/ethereum/node/src/node.rs @@ -7,15 +7,15 @@ use reth_basic_payload_builder::{BasicPayloadJobGenerator, BasicPayloadJobGenera use reth_beacon_consensus::EthBeaconConsensus; use reth_chainspec::ChainSpec; use reth_ethereum_engine_primitives::{ - EthBuiltPayload, EthPayloadAttributes, EthPayloadBuilderAttributes, + EthBuiltPayload, EthPayloadAttributes, EthPayloadBuilderAttributes, EthereumEngineValidator, }; use reth_evm_ethereum::execute::EthExecutorProvider; use reth_network::NetworkHandle; -use reth_node_api::{ConfigureEvm, FullNodeComponents, NodeAddOns}; +use reth_node_api::{ConfigureEvm, EngineValidator, FullNodeComponents, NodeAddOns}; use reth_node_builder::{ components::{ - ComponentsBuilder, ConsensusBuilder, ExecutorBuilder, NetworkBuilder, - PayloadServiceBuilder, PoolBuilder, + ComponentsBuilder, ConsensusBuilder, EngineValidatorBuilder, ExecutorBuilder, + NetworkBuilder, PayloadServiceBuilder, PoolBuilder, }, node::{FullNodeTypes, NodeTypes, NodeTypesWithEngine}, BuilderContext, Node, PayloadBuilderConfig, PayloadTypes, @@ -46,6 +46,7 @@ impl EthereumNode { EthereumNetworkBuilder, EthereumExecutorBuilder, EthereumConsensusBuilder, + EthereumEngineValidatorBuilder, > where Node: FullNodeTypes>, @@ -62,6 +63,7 @@ impl EthereumNode { .network(EthereumNetworkBuilder::default()) .executor(EthereumExecutorBuilder::default()) .consensus(EthereumConsensusBuilder::default()) + .engine_validator(EthereumEngineValidatorBuilder::default()) } } @@ -94,6 +96,7 @@ where EthereumNetworkBuilder, EthereumExecutorBuilder, EthereumConsensusBuilder, + EthereumEngineValidatorBuilder, >; type AddOns = EthereumAddOns; @@ -316,3 +319,21 @@ where } } } + +/// Builder for [`EthereumEngineValidator`]. +#[derive(Debug, Default, Clone)] +#[non_exhaustive] +pub struct EthereumEngineValidatorBuilder; + +impl EngineValidatorBuilder for EthereumEngineValidatorBuilder +where + Types: NodeTypesWithEngine, + Node: FullNodeTypes, + EthereumEngineValidator: EngineValidator, +{ + type Validator = EthereumEngineValidator; + + async fn build_validator(self, ctx: &BuilderContext) -> eyre::Result { + Ok(EthereumEngineValidator::new(ctx.chain_spec())) + } +} diff --git a/crates/exex/test-utils/Cargo.toml b/crates/exex/test-utils/Cargo.toml index b5b62471b..e3c95fead 100644 --- a/crates/exex/test-utils/Cargo.toml +++ b/crates/exex/test-utils/Cargo.toml @@ -31,6 +31,7 @@ reth-primitives.workspace = true reth-provider = { workspace = true, features = ["test-utils"] } reth-tasks.workspace = true reth-transaction-pool = { workspace = true, features = ["test-utils"] } +reth-ethereum-engine-primitives.workspace = true ## async futures-util.workspace = true diff --git a/crates/exex/test-utils/src/lib.rs b/crates/exex/test-utils/src/lib.rs index ab1958cc8..4117c0c73 100644 --- a/crates/exex/test-utils/src/lib.rs +++ b/crates/exex/test-utils/src/lib.rs @@ -17,6 +17,7 @@ use reth_db::{ DatabaseEnv, }; use reth_db_common::init::init_genesis; +use reth_ethereum_engine_primitives::EthereumEngineValidator; use reth_evm::test_utils::MockExecutorProvider; use reth_execution_types::Chain; use reth_exex::{ExExContext, ExExEvent, ExExNotification, ExExNotifications}; @@ -33,7 +34,10 @@ use reth_node_builder::{ }; use reth_node_core::node_config::NodeConfig; use reth_node_ethereum::{ - node::{EthereumAddOns, EthereumNetworkBuilder, EthereumPayloadBuilder}, + node::{ + EthereumAddOns, EthereumEngineValidatorBuilder, EthereumNetworkBuilder, + EthereumPayloadBuilder, + }, EthEngineTypes, EthEvmConfig, }; use reth_payload_builder::noop::NoopPayloadBuilderService; @@ -133,6 +137,7 @@ where EthereumNetworkBuilder, TestExecutorBuilder, TestConsensusBuilder, + EthereumEngineValidatorBuilder, >; type AddOns = EthereumAddOns; @@ -144,6 +149,7 @@ where .network(EthereumNetworkBuilder::default()) .executor(TestExecutorBuilder::default()) .consensus(TestConsensusBuilder::default()) + .engine_validator(EthereumEngineValidatorBuilder::default()) } } @@ -246,7 +252,7 @@ pub async fn test_exex_context_with_chain_spec( let db = create_test_rw_db(); let provider_factory = ProviderFactory::new( db, - chain_spec, + chain_spec.clone(), StaticFileProvider::read_write(static_dir.into_path()).expect("static file provider"), ); @@ -266,6 +272,8 @@ pub async fn test_exex_context_with_chain_spec( let tasks = TaskManager::current(); let task_executor = tasks.executor(); + let engine_validator = EthereumEngineValidator::new(chain_spec.clone()); + let components = NodeAdapter::, _>, _> { components: Components { transaction_pool, @@ -274,6 +282,7 @@ pub async fn test_exex_context_with_chain_spec( consensus, network, payload_builder, + engine_validator, }, task_executor, provider, diff --git a/crates/node/builder/src/components/builder.rs b/crates/node/builder/src/components/builder.rs index ef9f303a7..ab8e29929 100644 --- a/crates/node/builder/src/components/builder.rs +++ b/crates/node/builder/src/components/builder.rs @@ -4,6 +4,7 @@ use std::{future::Future, marker::PhantomData}; use reth_consensus::Consensus; use reth_evm::execute::BlockExecutorProvider; +use reth_node_api::{EngineValidator, NodeTypesWithEngine}; use reth_primitives::Header; use reth_transaction_pool::TransactionPool; @@ -15,6 +16,8 @@ use crate::{ BuilderContext, ConfigureEvm, FullNodeTypes, }; +use super::EngineValidatorBuilder; + /// A generic, general purpose and customizable [`NodeComponentsBuilder`] implementation. /// /// This type is stateful and captures the configuration of the node's components. @@ -35,22 +38,23 @@ use crate::{ /// All component builders are captured in the builder state and will be consumed once the node is /// launched. #[derive(Debug)] -pub struct ComponentsBuilder { +pub struct ComponentsBuilder { pool_builder: PoolB, payload_builder: PayloadB, network_builder: NetworkB, executor_builder: ExecB, consensus_builder: ConsB, + engine_validator_builder: EVB, _marker: PhantomData, } -impl - ComponentsBuilder +impl + ComponentsBuilder { /// Configures the node types. pub fn node_types( self, - ) -> ComponentsBuilder + ) -> ComponentsBuilder where Types: FullNodeTypes, { @@ -60,6 +64,7 @@ impl network_builder, executor_builder: evm_builder, consensus_builder, + engine_validator_builder, _marker, } = self; ComponentsBuilder { @@ -68,6 +73,7 @@ impl payload_builder, network_builder, consensus_builder, + engine_validator_builder, _marker: Default::default(), } } @@ -80,6 +86,7 @@ impl network_builder: self.network_builder, executor_builder: self.executor_builder, consensus_builder: self.consensus_builder, + engine_validator_builder: self.engine_validator_builder, _marker: self._marker, } } @@ -92,6 +99,7 @@ impl network_builder: self.network_builder, executor_builder: self.executor_builder, consensus_builder: self.consensus_builder, + engine_validator_builder: self.engine_validator_builder, _marker: self._marker, } } @@ -104,6 +112,7 @@ impl network_builder: f(self.network_builder), executor_builder: self.executor_builder, consensus_builder: self.consensus_builder, + engine_validator_builder: self.engine_validator_builder, _marker: self._marker, } } @@ -116,6 +125,7 @@ impl network_builder: self.network_builder, executor_builder: f(self.executor_builder), consensus_builder: self.consensus_builder, + engine_validator_builder: self.engine_validator_builder, _marker: self._marker, } } @@ -128,13 +138,14 @@ impl network_builder: self.network_builder, executor_builder: self.executor_builder, consensus_builder: f(self.consensus_builder), + engine_validator_builder: self.engine_validator_builder, _marker: self._marker, } } } -impl - ComponentsBuilder +impl + ComponentsBuilder where Node: FullNodeTypes, { @@ -145,7 +156,7 @@ where pub fn pool( self, pool_builder: PB, - ) -> ComponentsBuilder + ) -> ComponentsBuilder where PB: PoolBuilder, { @@ -155,6 +166,7 @@ where network_builder, executor_builder: evm_builder, consensus_builder, + engine_validator_builder, _marker, } = self; ComponentsBuilder { @@ -163,13 +175,14 @@ where network_builder, executor_builder: evm_builder, consensus_builder, + engine_validator_builder, _marker, } } } -impl - ComponentsBuilder +impl + ComponentsBuilder where Node: FullNodeTypes, PoolB: PoolBuilder, @@ -181,7 +194,7 @@ where pub fn network( self, network_builder: NB, - ) -> ComponentsBuilder + ) -> ComponentsBuilder where NB: NetworkBuilder, { @@ -191,6 +204,7 @@ where network_builder: _, executor_builder: evm_builder, consensus_builder, + engine_validator_builder, _marker, } = self; ComponentsBuilder { @@ -199,6 +213,7 @@ where network_builder, executor_builder: evm_builder, consensus_builder, + engine_validator_builder, _marker, } } @@ -210,7 +225,7 @@ where pub fn payload( self, payload_builder: PB, - ) -> ComponentsBuilder + ) -> ComponentsBuilder where PB: PayloadServiceBuilder, { @@ -220,6 +235,7 @@ where network_builder, executor_builder: evm_builder, consensus_builder, + engine_validator_builder, _marker, } = self; ComponentsBuilder { @@ -228,6 +244,7 @@ where network_builder, executor_builder: evm_builder, consensus_builder, + engine_validator_builder, _marker, } } @@ -239,7 +256,7 @@ where pub fn executor( self, executor_builder: EB, - ) -> ComponentsBuilder + ) -> ComponentsBuilder where EB: ExecutorBuilder, { @@ -249,6 +266,7 @@ where network_builder, executor_builder: _, consensus_builder, + engine_validator_builder, _marker, } = self; ComponentsBuilder { @@ -257,6 +275,7 @@ where network_builder, executor_builder, consensus_builder, + engine_validator_builder, _marker, } } @@ -268,7 +287,7 @@ where pub fn consensus( self, consensus_builder: CB, - ) -> ComponentsBuilder + ) -> ComponentsBuilder where CB: ConsensusBuilder, { @@ -278,6 +297,7 @@ where network_builder, executor_builder, consensus_builder: _, + engine_validator_builder, _marker, } = self; ComponentsBuilder { @@ -286,13 +306,45 @@ where network_builder, executor_builder, consensus_builder, + engine_validator_builder, + _marker, + } + } + + /// Configures the consensus builder. + /// + /// This accepts a [`ConsensusBuilder`] instance that will be used to create the node's + /// components for consensus. + pub fn engine_validator( + self, + engine_validator_builder: EngineVB, + ) -> ComponentsBuilder + where + EngineVB: EngineValidatorBuilder, + { + let Self { + pool_builder, + payload_builder, + network_builder, + executor_builder, + consensus_builder, + engine_validator_builder: _, + _marker, + } = self; + ComponentsBuilder { + pool_builder, + payload_builder, + network_builder, + executor_builder, + consensus_builder, + engine_validator_builder, _marker, } } } -impl NodeComponentsBuilder - for ComponentsBuilder +impl NodeComponentsBuilder + for ComponentsBuilder where Node: FullNodeTypes, PoolB: PoolBuilder, @@ -300,8 +352,16 @@ where PayloadB: PayloadServiceBuilder, ExecB: ExecutorBuilder, ConsB: ConsensusBuilder, + EVB: EngineValidatorBuilder, { - type Components = Components; + type Components = Components< + Node, + PoolB::Pool, + ExecB::EVM, + ExecB::Executor, + ConsB::Consensus, + EVB::Validator, + >; async fn build_components( self, @@ -313,6 +373,7 @@ where network_builder, executor_builder: evm_builder, consensus_builder, + engine_validator_builder, _marker, } = self; @@ -321,6 +382,7 @@ where let network = network_builder.build_network(context, pool.clone()).await?; let payload_builder = payload_builder.spawn_payload_service(context, pool.clone()).await?; let consensus = consensus_builder.build_consensus(context).await?; + let engine_validator = engine_validator_builder.build_validator(context).await?; Ok(Components { transaction_pool: pool, @@ -329,11 +391,12 @@ where payload_builder, executor, consensus, + engine_validator, }) } } -impl Default for ComponentsBuilder<(), (), (), (), (), ()> { +impl Default for ComponentsBuilder<(), (), (), (), (), (), ()> { fn default() -> Self { Self { pool_builder: (), @@ -341,6 +404,7 @@ impl Default for ComponentsBuilder<(), (), (), (), (), ()> { network_builder: (), executor_builder: (), consensus_builder: (), + engine_validator_builder: (), _marker: Default::default(), } } @@ -366,17 +430,18 @@ pub trait NodeComponentsBuilder: Send { ) -> impl Future> + Send; } -impl NodeComponentsBuilder for F +impl NodeComponentsBuilder for F where Node: FullNodeTypes, F: FnOnce(&BuilderContext) -> Fut + Send, - Fut: Future>> + Send, + Fut: Future>> + Send, Pool: TransactionPool + Unpin + 'static, EVM: ConfigureEvm
, Executor: BlockExecutorProvider, Cons: Consensus + Clone + Unpin + 'static, + Val: EngineValidator<::Engine> + Clone + Unpin + 'static, { - type Components = Components; + type Components = Components; fn build_components( self, diff --git a/crates/node/builder/src/components/engine.rs b/crates/node/builder/src/components/engine.rs new file mode 100644 index 000000000..b3ee7cbbb --- /dev/null +++ b/crates/node/builder/src/components/engine.rs @@ -0,0 +1,38 @@ +//! Consensus component for the node builder. +use reth_node_api::{EngineValidator, NodeTypesWithEngine}; + +use crate::{BuilderContext, FullNodeTypes}; +use std::future::Future; + +/// A type that knows how to build the engine validator. +pub trait EngineValidatorBuilder: Send { + /// The consensus implementation to build. + type Validator: EngineValidator<::Engine> + + Clone + + Unpin + + 'static; + + /// Creates the engine validator. + fn build_validator( + self, + ctx: &BuilderContext, + ) -> impl Future> + Send; +} + +impl EngineValidatorBuilder for F +where + Node: FullNodeTypes, + Validator: + EngineValidator<::Engine> + Clone + Unpin + 'static, + F: FnOnce(&BuilderContext) -> Fut + Send, + Fut: Future> + Send, +{ + type Validator = Validator; + + fn build_validator( + self, + ctx: &BuilderContext, + ) -> impl Future> { + self(ctx) + } +} diff --git a/crates/node/builder/src/components/mod.rs b/crates/node/builder/src/components/mod.rs index 42001fc10..ff1646593 100644 --- a/crates/node/builder/src/components/mod.rs +++ b/crates/node/builder/src/components/mod.rs @@ -9,6 +9,7 @@ mod builder; mod consensus; +mod engine; mod execute; mod network; mod payload; @@ -16,6 +17,7 @@ mod pool; pub use builder::*; pub use consensus::*; +pub use engine::*; pub use execute::*; pub use network::*; pub use payload::*; @@ -25,7 +27,7 @@ use reth_consensus::Consensus; use reth_evm::execute::BlockExecutorProvider; use reth_network::NetworkHandle; use reth_network_api::FullNetwork; -use reth_node_api::NodeTypesWithEngine; +use reth_node_api::{EngineValidator, NodeTypesWithEngine}; use reth_payload_builder::PayloadBuilderHandle; use reth_primitives::Header; use reth_transaction_pool::TransactionPool; @@ -53,6 +55,9 @@ pub trait NodeComponents: Clone + Unpin + Send + Sync + 'stati /// Network API. type Network: FullNetwork; + /// Validator for the engine API. + type EngineValidator: EngineValidator<::Engine>; + /// Returns the transaction pool of the node. fn pool(&self) -> &Self::Pool; @@ -70,13 +75,16 @@ pub trait NodeComponents: Clone + Unpin + Send + Sync + 'stati /// Returns the handle to the payload builder service. fn payload_builder(&self) -> &PayloadBuilderHandle<::Engine>; + + /// Returns the engine validator. + fn engine_validator(&self) -> &Self::EngineValidator; } /// All the components of the node. /// /// This provides access to all the components of the node. #[derive(Debug)] -pub struct Components { +pub struct Components { /// The transaction pool of the node. pub transaction_pool: Pool, /// The node's EVM configuration, defining settings for the Ethereum Virtual Machine. @@ -89,22 +97,26 @@ pub struct Components { pub network: NetworkHandle, /// The handle to the payload builder service. pub payload_builder: PayloadBuilderHandle<::Engine>, + /// The validator for the engine API. + pub engine_validator: Validator, } -impl NodeComponents - for Components +impl NodeComponents + for Components where Node: FullNodeTypes, Pool: TransactionPool + Unpin + 'static, EVM: ConfigureEvm
, Executor: BlockExecutorProvider, Cons: Consensus + Clone + Unpin + 'static, + Val: EngineValidator<::Engine> + Clone + Unpin + 'static, { type Pool = Pool; type Evm = EVM; type Executor = Executor; type Consensus = Cons; type Network = NetworkHandle; + type EngineValidator = Val; fn pool(&self) -> &Self::Pool { &self.transaction_pool @@ -131,15 +143,21 @@ where ) -> &PayloadBuilderHandle<::Engine> { &self.payload_builder } + + fn engine_validator(&self) -> &Self::EngineValidator { + &self.engine_validator + } } -impl Clone for Components +impl Clone + for Components where Node: FullNodeTypes, Pool: TransactionPool, EVM: ConfigureEvm
, Executor: BlockExecutorProvider, Cons: Consensus + Clone, + Val: EngineValidator<::Engine>, { fn clone(&self) -> Self { Self { @@ -149,6 +167,7 @@ where consensus: self.consensus.clone(), network: self.network.clone(), payload_builder: self.payload_builder.clone(), + engine_validator: self.engine_validator.clone(), } } } diff --git a/crates/node/builder/src/launch/engine.rs b/crates/node/builder/src/launch/engine.rs index e8c5f7818..0f68772c4 100644 --- a/crates/node/builder/src/launch/engine.rs +++ b/crates/node/builder/src/launch/engine.rs @@ -275,6 +275,7 @@ where Box::new(ctx.task_executor().clone()), client, EngineCapabilities::default(), + ctx.components().engine_validator().clone(), ); info!(target: "reth::cli", "Engine API handler initialized"); diff --git a/crates/node/builder/src/launch/mod.rs b/crates/node/builder/src/launch/mod.rs index ee0215e08..eb2133543 100644 --- a/crates/node/builder/src/launch/mod.rs +++ b/crates/node/builder/src/launch/mod.rs @@ -351,6 +351,7 @@ where Box::new(ctx.task_executor().clone()), client, EngineCapabilities::default(), + ctx.components().engine_validator().clone(), ); info!(target: "reth::cli", "Engine API handler initialized"); diff --git a/crates/node/types/src/lib.rs b/crates/node/types/src/lib.rs index dda669817..2ad2f8abd 100644 --- a/crates/node/types/src/lib.rs +++ b/crates/node/types/src/lib.rs @@ -39,7 +39,7 @@ pub trait NodeTypes: Send + Sync + Unpin + 'static { /// The type that configures an Ethereum-like node with an engine for consensus. pub trait NodeTypesWithEngine: NodeTypes { /// The node's engine types, defining the interaction with the consensus engine. - type Engine: EngineTypes; + type Engine: EngineTypes; } /// A helper trait that is downstream of the [`NodeTypesWithEngine`] trait and adds database to the @@ -166,7 +166,7 @@ where impl NodeTypesWithEngine for AnyNodeTypesWithEngine where P: NodePrimitives + Send + Sync + Unpin + 'static, - E: EngineTypes + Send + Sync + Unpin, + E: EngineTypes + Send + Sync + Unpin, C: EthChainSpec, { type Engine = E; diff --git a/crates/optimism/node/src/engine.rs b/crates/optimism/node/src/engine.rs index 4bd6b78ea..bc5880034 100644 --- a/crates/optimism/node/src/engine.rs +++ b/crates/optimism/node/src/engine.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use op_alloy_rpc_types_engine::{ OptimismExecutionPayloadEnvelopeV3, OptimismExecutionPayloadEnvelopeV4, OptimismPayloadAttributes, @@ -9,7 +11,7 @@ use reth_node_api::{ EngineObjectValidationError, MessageValidationKind, PayloadOrAttributes, PayloadTypes, VersionSpecificValidationError, }, - EngineTypes, + validate_version_specific_fields, EngineTypes, EngineValidator, }; use reth_optimism_forks::OptimismHardfork; use reth_optimism_payload_builder::{OptimismBuiltPayload, OptimismPayloadBuilderAttributes}; @@ -20,6 +22,19 @@ use reth_rpc_types::{engine::ExecutionPayloadEnvelopeV2, ExecutionPayloadV1}; #[non_exhaustive] pub struct OptimismEngineTypes; +/// Validator for Optimism engine API. +#[derive(Debug, Clone)] +pub struct OptimismEngineValidator { + chain_spec: Arc, +} + +impl OptimismEngineValidator { + /// Instantiates a new validator. + pub const fn new(chain_spec: Arc) -> Self { + Self { chain_spec } + } +} + impl PayloadTypes for OptimismEngineTypes { type BuiltPayload = OptimismBuiltPayload; type PayloadAttributes = OptimismPayloadAttributes; @@ -27,33 +42,10 @@ impl PayloadTypes for OptimismEngineTypes { } impl EngineTypes for OptimismEngineTypes { - type ChainSpec = ChainSpec; - type ExecutionPayloadV1 = ExecutionPayloadV1; type ExecutionPayloadV2 = ExecutionPayloadEnvelopeV2; type ExecutionPayloadV3 = OptimismExecutionPayloadEnvelopeV3; type ExecutionPayloadV4 = OptimismExecutionPayloadEnvelopeV4; - - fn validate_version_specific_fields( - chain_spec: &ChainSpec, - version: EngineApiMessageVersion, - payload_or_attrs: PayloadOrAttributes<'_, Self::PayloadAttributes>, - ) -> Result<(), EngineObjectValidationError> { - validate_withdrawals_presence( - chain_spec, - version, - payload_or_attrs.message_validation_kind(), - payload_or_attrs.timestamp(), - payload_or_attrs.withdrawals().is_some(), - )?; - validate_parent_beacon_block_root_presence( - chain_spec, - version, - payload_or_attrs.message_validation_kind(), - payload_or_attrs.timestamp(), - payload_or_attrs.parent_beacon_block_root().is_some(), - ) - } } /// Validates the presence of the `withdrawals` field according to the payload timestamp. @@ -97,3 +89,45 @@ pub fn validate_withdrawals_presence( Ok(()) } + +impl EngineValidator for OptimismEngineValidator +where + Types: EngineTypes, +{ + fn validate_version_specific_fields( + &self, + version: EngineApiMessageVersion, + payload_or_attrs: PayloadOrAttributes<'_, OptimismPayloadAttributes>, + ) -> Result<(), EngineObjectValidationError> { + validate_withdrawals_presence( + &self.chain_spec, + version, + payload_or_attrs.message_validation_kind(), + payload_or_attrs.timestamp(), + payload_or_attrs.withdrawals().is_some(), + )?; + validate_parent_beacon_block_root_presence( + &self.chain_spec, + version, + payload_or_attrs.message_validation_kind(), + payload_or_attrs.timestamp(), + payload_or_attrs.parent_beacon_block_root().is_some(), + ) + } + + fn ensure_well_formed_attributes( + &self, + version: EngineApiMessageVersion, + attributes: &OptimismPayloadAttributes, + ) -> Result<(), EngineObjectValidationError> { + validate_version_specific_fields(&self.chain_spec, version, attributes.into())?; + + if attributes.gas_limit.is_none() { + return Err(EngineObjectValidationError::InvalidParams( + "MissingGasLimitInPayloadAttributes".to_string().into(), + )) + } + + Ok(()) + } +} diff --git a/crates/optimism/node/src/node.rs b/crates/optimism/node/src/node.rs index fec6d5065..aafdc8aaa 100644 --- a/crates/optimism/node/src/node.rs +++ b/crates/optimism/node/src/node.rs @@ -7,11 +7,11 @@ use reth_chainspec::ChainSpec; use reth_evm::ConfigureEvm; use reth_evm_optimism::{OpExecutorProvider, OptimismEvmConfig}; use reth_network::{NetworkHandle, NetworkManager}; -use reth_node_api::{FullNodeComponents, NodeAddOns}; +use reth_node_api::{EngineValidator, FullNodeComponents, NodeAddOns}; use reth_node_builder::{ components::{ - ComponentsBuilder, ConsensusBuilder, ExecutorBuilder, NetworkBuilder, - PayloadServiceBuilder, PoolBuilder, + ComponentsBuilder, ConsensusBuilder, EngineValidatorBuilder, ExecutorBuilder, + NetworkBuilder, PayloadServiceBuilder, PoolBuilder, }, node::{FullNodeTypes, NodeTypes, NodeTypesWithEngine}, BuilderContext, Node, PayloadBuilderConfig, @@ -30,6 +30,7 @@ use reth_transaction_pool::{ use crate::{ args::RollupArgs, + engine::OptimismEngineValidator, txpool::{OpTransactionPool, OpTransactionValidator}, OptimismEngineTypes, }; @@ -58,6 +59,7 @@ impl OptimismNode { OptimismNetworkBuilder, OptimismExecutorBuilder, OptimismConsensusBuilder, + OptimismEngineValidatorBuilder, > where Node: FullNodeTypes< @@ -75,6 +77,7 @@ impl OptimismNode { }) .executor(OptimismExecutorBuilder::default()) .consensus(OptimismConsensusBuilder::default()) + .engine_validator(OptimismEngineValidatorBuilder::default()) } } @@ -91,6 +94,7 @@ where OptimismNetworkBuilder, OptimismExecutorBuilder, OptimismConsensusBuilder, + OptimismEngineValidatorBuilder, >; type AddOns = OptimismAddOns; @@ -386,3 +390,21 @@ where } } } + +/// Builder for [`OptimismEngineValidator`]. +#[derive(Debug, Default, Clone)] +#[non_exhaustive] +pub struct OptimismEngineValidatorBuilder; + +impl EngineValidatorBuilder for OptimismEngineValidatorBuilder +where + Types: NodeTypesWithEngine, + Node: FullNodeTypes, + OptimismEngineValidator: EngineValidator, +{ + type Validator = OptimismEngineValidator; + + async fn build_validator(self, ctx: &BuilderContext) -> eyre::Result { + Ok(OptimismEngineValidator::new(ctx.chain_spec())) + } +} diff --git a/crates/payload/primitives/src/traits.rs b/crates/payload/primitives/src/traits.rs index 6e97ec2b9..9551b75a7 100644 --- a/crates/payload/primitives/src/traits.rs +++ b/crates/payload/primitives/src/traits.rs @@ -1,7 +1,4 @@ -use crate::{ - validate_version_specific_fields, EngineApiMessageVersion, EngineObjectValidationError, - PayloadBuilderError, PayloadEvents, PayloadTypes, -}; +use crate::{PayloadBuilderError, PayloadEvents, PayloadTypes}; use alloy_primitives::{Address, B256, U256}; use alloy_rpc_types::{ engine::{PayloadAttributes as EthPayloadAttributes, PayloadId}, @@ -9,7 +6,6 @@ use alloy_rpc_types::{ }; use op_alloy_rpc_types_engine::OptimismPayloadAttributes; use reth_chain_state::ExecutedBlock; -use reth_chainspec::ChainSpec; use reth_primitives::{SealedBlock, Withdrawals}; use std::{future::Future, pin::Pin}; use tokio::sync::oneshot; @@ -133,14 +129,6 @@ pub trait PayloadAttributes: /// Return the parent beacon block root for the payload attributes. fn parent_beacon_block_root(&self) -> Option; - - /// Ensures that the payload attributes are valid for the given [`ChainSpec`] and - /// [`EngineApiMessageVersion`]. - fn ensure_well_formed_attributes( - &self, - chain_spec: &ChainSpec, - version: EngineApiMessageVersion, - ) -> Result<(), EngineObjectValidationError>; } impl PayloadAttributes for EthPayloadAttributes { @@ -155,14 +143,6 @@ impl PayloadAttributes for EthPayloadAttributes { fn parent_beacon_block_root(&self) -> Option { self.parent_beacon_block_root } - - fn ensure_well_formed_attributes( - &self, - chain_spec: &ChainSpec, - version: EngineApiMessageVersion, - ) -> Result<(), EngineObjectValidationError> { - validate_version_specific_fields(chain_spec, version, self.into()) - } } impl PayloadAttributes for OptimismPayloadAttributes { @@ -177,22 +157,6 @@ impl PayloadAttributes for OptimismPayloadAttributes { fn parent_beacon_block_root(&self) -> Option { self.payload_attributes.parent_beacon_block_root } - - fn ensure_well_formed_attributes( - &self, - chain_spec: &ChainSpec, - version: EngineApiMessageVersion, - ) -> Result<(), EngineObjectValidationError> { - validate_version_specific_fields(chain_spec, version, self.into())?; - - if self.gas_limit.is_none() { - return Err(EngineObjectValidationError::InvalidParams( - "MissingGasLimitInPayloadAttributes".to_string().into(), - )) - } - - Ok(()) - } } /// A builder that can return the current payload attribute. diff --git a/crates/rpc/rpc-builder/tests/it/utils.rs b/crates/rpc/rpc-builder/tests/it/utils.rs index ee7a064ab..8121e480c 100644 --- a/crates/rpc/rpc-builder/tests/it/utils.rs +++ b/crates/rpc/rpc-builder/tests/it/utils.rs @@ -2,7 +2,7 @@ use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4}; use reth_beacon_consensus::BeaconConsensusEngineHandle; use reth_chainspec::MAINNET; -use reth_ethereum_engine_primitives::EthEngineTypes; +use reth_ethereum_engine_primitives::{EthEngineTypes, EthereumEngineValidator}; use reth_evm_ethereum::EthEvmConfig; use reth_network_api::noop::NoopNetwork; use reth_payload_builder::test_utils::spawn_test_payload_service; @@ -50,6 +50,7 @@ pub async fn launch_auth(secret: JwtSecret) -> AuthServerHandle { Box::::default(), client, EngineCapabilities::default(), + EthereumEngineValidator::new(MAINNET.clone()), ); let module = AuthRpcModule::new(engine_api); module.start_server(config).await.unwrap() diff --git a/crates/rpc/rpc-engine-api/src/engine_api.rs b/crates/rpc/rpc-engine-api/src/engine_api.rs index ed91c0102..431f1fd72 100644 --- a/crates/rpc/rpc-engine-api/src/engine_api.rs +++ b/crates/rpc/rpc-engine-api/src/engine_api.rs @@ -7,12 +7,12 @@ use async_trait::async_trait; use jsonrpsee_core::RpcResult; use reth_beacon_consensus::BeaconConsensusEngineHandle; use reth_chainspec::ChainSpec; -use reth_engine_primitives::EngineTypes; +use reth_engine_primitives::{EngineTypes, EngineValidator}; use reth_evm::provider::EvmEnvProvider; use reth_payload_builder::PayloadStore; use reth_payload_primitives::{ - validate_payload_timestamp, EngineApiMessageVersion, PayloadAttributes, - PayloadBuilderAttributes, PayloadOrAttributes, + validate_payload_timestamp, EngineApiMessageVersion, PayloadBuilderAttributes, + PayloadOrAttributes, }; use reth_primitives::{Block, BlockHashOrNumber, EthereumHardfork}; use reth_rpc_api::EngineApiServer; @@ -43,11 +43,11 @@ const MAX_BLOB_LIMIT: usize = 128; /// The Engine API implementation that grants the Consensus layer access to data and /// functions in the Execution layer that are crucial for the consensus process. -pub struct EngineApi { - inner: Arc>, +pub struct EngineApi { + inner: Arc>, } -struct EngineApiInner { +struct EngineApiInner { /// The provider to interact with the chain. provider: Provider, /// Consensus configuration @@ -66,13 +66,16 @@ struct EngineApiInner { capabilities: EngineCapabilities, /// Transaction pool. tx_pool: Pool, + /// Engine validator. + validator: Validator, } -impl EngineApi +impl EngineApi where Provider: HeaderProvider + BlockReader + StateProviderFactory + EvmEnvProvider + 'static, - EngineT: EngineTypes, + EngineT: EngineTypes, Pool: TransactionPool + 'static, + Validator: EngineValidator, { /// Create new instance of [`EngineApi`]. #[allow(clippy::too_many_arguments)] @@ -85,6 +88,7 @@ where task_spawner: Box, client: ClientVersionV1, capabilities: EngineCapabilities, + validator: Validator, ) -> Self { let inner = Arc::new(EngineApiInner { provider, @@ -96,6 +100,7 @@ where client, capabilities, tx_pool, + validator, }); Self { inner } } @@ -131,11 +136,9 @@ where PayloadOrAttributes::<'_, EngineT::PayloadAttributes>::from_execution_payload( &payload, None, ); - EngineT::validate_version_specific_fields( - &self.inner.chain_spec, - EngineApiMessageVersion::V1, - payload_or_attrs, - )?; + self.inner + .validator + .validate_version_specific_fields(EngineApiMessageVersion::V1, payload_or_attrs)?; Ok(self.inner.beacon_consensus.new_payload(payload, None).await?) } @@ -149,11 +152,9 @@ where PayloadOrAttributes::<'_, EngineT::PayloadAttributes>::from_execution_payload( &payload, None, ); - EngineT::validate_version_specific_fields( - &self.inner.chain_spec, - EngineApiMessageVersion::V2, - payload_or_attrs, - )?; + self.inner + .validator + .validate_version_specific_fields(EngineApiMessageVersion::V2, payload_or_attrs)?; Ok(self.inner.beacon_consensus.new_payload(payload, None).await?) } @@ -170,11 +171,9 @@ where &payload, Some(parent_beacon_block_root), ); - EngineT::validate_version_specific_fields( - &self.inner.chain_spec, - EngineApiMessageVersion::V3, - payload_or_attrs, - )?; + self.inner + .validator + .validate_version_specific_fields(EngineApiMessageVersion::V3, payload_or_attrs)?; let cancun_fields = CancunPayloadFields { versioned_hashes, parent_beacon_block_root }; @@ -194,11 +193,9 @@ where &payload, Some(parent_beacon_block_root), ); - EngineT::validate_version_specific_fields( - &self.inner.chain_spec, - EngineApiMessageVersion::V4, - payload_or_attrs, - )?; + self.inner + .validator + .validate_version_specific_fields(EngineApiMessageVersion::V4, payload_or_attrs)?; let cancun_fields = CancunPayloadFields { versioned_hashes, parent_beacon_block_root }; @@ -588,7 +585,7 @@ where ) -> EngineApiResult { if let Some(ref attrs) = payload_attrs { let attr_validation_res = - attrs.ensure_well_formed_attributes(&self.inner.chain_spec, version); + self.inner.validator.ensure_well_formed_attributes(version, attrs); // From the engine API spec: // @@ -619,11 +616,13 @@ where } #[async_trait] -impl EngineApiServer for EngineApi +impl EngineApiServer + for EngineApi where Provider: HeaderProvider + BlockReader + StateProviderFactory + EvmEnvProvider + 'static, - EngineT: EngineTypes, + EngineT: EngineTypes, Pool: TransactionPool + 'static, + Validator: EngineValidator, { /// Handler for `engine_newPayloadV1` /// See also @@ -933,7 +932,8 @@ where } } -impl std::fmt::Debug for EngineApi +impl std::fmt::Debug + for EngineApi where EngineT: EngineTypes, { @@ -948,7 +948,7 @@ mod tests { use assert_matches::assert_matches; use reth_beacon_consensus::{BeaconConsensusEngineEvent, BeaconEngineMessage}; use reth_chainspec::MAINNET; - use reth_ethereum_engine_primitives::EthEngineTypes; + use reth_ethereum_engine_primitives::{EthEngineTypes, EthereumEngineValidator}; use reth_payload_builder::test_utils::spawn_test_payload_service; use reth_primitives::SealedBlock; use reth_provider::test_utils::MockEthProvider; @@ -960,9 +960,15 @@ mod tests { use reth_transaction_pool::noop::NoopTransactionPool; use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver}; - fn setup_engine_api( - ) -> (EngineApiTestHandle, EngineApi, EthEngineTypes, NoopTransactionPool>) - { + fn setup_engine_api() -> ( + EngineApiTestHandle, + EngineApi< + Arc, + EthEngineTypes, + NoopTransactionPool, + EthereumEngineValidator, + >, + ) { let client = ClientVersionV1 { code: ClientCode::RH, name: "Reth".to_string(), @@ -985,6 +991,7 @@ mod tests { task_executor, client, EngineCapabilities::default(), + EthereumEngineValidator::new(chain_spec.clone()), ); let handle = EngineApiTestHandle { chain_spec, provider, from_api: engine_rx }; (handle, api) diff --git a/examples/custom-engine-types/src/main.rs b/examples/custom-engine-types/src/main.rs index ede84485d..a31bf1c34 100644 --- a/examples/custom-engine-types/src/main.rs +++ b/examples/custom-engine-types/src/main.rs @@ -17,7 +17,7 @@ #![cfg_attr(not(test), warn(unused_crate_dependencies))] -use std::convert::Infallible; +use std::{convert::Infallible, sync::Arc}; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -33,7 +33,7 @@ use alloy_rpc_types::{ use reth::{ api::PayloadTypes, builder::{ - components::{ComponentsBuilder, PayloadServiceBuilder}, + components::{ComponentsBuilder, EngineValidatorBuilder, PayloadServiceBuilder}, node::{NodeTypes, NodeTypesWithEngine}, BuilderContext, FullNodeTypes, Node, NodeBuilder, PayloadBuilderConfig, }, @@ -48,7 +48,8 @@ use reth_basic_payload_builder::{ use reth_chainspec::{Chain, ChainSpec}; use reth_node_api::{ payload::{EngineApiMessageVersion, EngineObjectValidationError, PayloadOrAttributes}, - validate_version_specific_fields, EngineTypes, PayloadAttributes, PayloadBuilderAttributes, + validate_version_specific_fields, EngineTypes, EngineValidator, PayloadAttributes, + PayloadBuilderAttributes, }; use reth_node_core::{args::RpcServerArgs, node_config::NodeConfig}; use reth_node_ethereum::{ @@ -94,23 +95,6 @@ impl PayloadAttributes for CustomPayloadAttributes { fn parent_beacon_block_root(&self) -> Option { self.inner.parent_beacon_block_root() } - - fn ensure_well_formed_attributes( - &self, - chain_spec: &ChainSpec, - version: EngineApiMessageVersion, - ) -> Result<(), EngineObjectValidationError> { - validate_version_specific_fields(chain_spec, version, self.into())?; - - // custom validation logic - ensure that the custom field is not zero - if self.custom == 0 { - return Err(EngineObjectValidationError::invalid_params( - CustomError::CustomFieldIsNotZero, - )) - } - - Ok(()) - } } /// New type around the payload builder attributes type @@ -167,19 +151,61 @@ impl PayloadTypes for CustomEngineTypes { } impl EngineTypes for CustomEngineTypes { - type ChainSpec = ChainSpec; - type ExecutionPayloadV1 = ExecutionPayloadV1; type ExecutionPayloadV2 = ExecutionPayloadEnvelopeV2; type ExecutionPayloadV3 = ExecutionPayloadEnvelopeV3; type ExecutionPayloadV4 = ExecutionPayloadEnvelopeV4; +} +/// Custom engine validator +#[derive(Debug, Clone)] +pub struct CustomEngineValidator { + chain_spec: Arc, +} + +impl EngineValidator for CustomEngineValidator +where + T: EngineTypes, +{ fn validate_version_specific_fields( - chain_spec: &ChainSpec, + &self, version: EngineApiMessageVersion, - payload_or_attrs: PayloadOrAttributes<'_, CustomPayloadAttributes>, + payload_or_attrs: PayloadOrAttributes<'_, T::PayloadAttributes>, ) -> Result<(), EngineObjectValidationError> { - validate_version_specific_fields(chain_spec, version, payload_or_attrs) + validate_version_specific_fields(&self.chain_spec, version, payload_or_attrs) + } + + fn ensure_well_formed_attributes( + &self, + version: EngineApiMessageVersion, + attributes: &T::PayloadAttributes, + ) -> Result<(), EngineObjectValidationError> { + validate_version_specific_fields(&self.chain_spec, version, attributes.into())?; + + // custom validation logic - ensure that the custom field is not zero + if attributes.custom == 0 { + return Err(EngineObjectValidationError::invalid_params( + CustomError::CustomFieldIsNotZero, + )) + } + + Ok(()) + } +} + +/// Custom engine validator builder +#[derive(Debug, Default, Clone, Copy)] +#[non_exhaustive] +pub struct CustomEngineValidatorBuilder; + +impl EngineValidatorBuilder for CustomEngineValidatorBuilder +where + N: FullNodeTypes>, +{ + type Validator = CustomEngineValidator; + + async fn build_validator(self, ctx: &BuilderContext) -> eyre::Result { + Ok(CustomEngineValidator { chain_spec: ctx.chain_spec() }) } } @@ -212,6 +238,7 @@ where EthereumNetworkBuilder, EthereumExecutorBuilder, EthereumConsensusBuilder, + CustomEngineValidatorBuilder, >; type AddOns = EthereumAddOns; @@ -223,6 +250,7 @@ where .network(EthereumNetworkBuilder::default()) .executor(EthereumExecutorBuilder::default()) .consensus(EthereumConsensusBuilder::default()) + .engine_validator(CustomEngineValidatorBuilder::default()) } }