From 9d65a4cfbb39d0fc9a90ef9aee451595bcd05ba5 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 15 Jan 2024 15:29:45 -0500 Subject: [PATCH] feat: add version_specific_fields method to EngineTypes (#6050) --- bin/reth/Cargo.toml | 1 + crates/node-api/Cargo.toml | 2 +- crates/node-api/src/engine/mod.rs | 85 ++++++++++++++++++++- crates/node-api/src/engine/traits.rs | 4 +- crates/node-api/src/lib.rs | 2 + crates/node-builder/Cargo.toml | 3 + crates/node-builder/src/engine.rs | 37 +++++++-- crates/node-builder/src/lib.rs | 5 +- crates/rpc/rpc-engine-api/src/engine_api.rs | 17 ++--- 9 files changed, 135 insertions(+), 21 deletions(-) diff --git a/bin/reth/Cargo.toml b/bin/reth/Cargo.toml index b74be8eedd01..0508764c85ab 100644 --- a/bin/reth/Cargo.toml +++ b/bin/reth/Cargo.toml @@ -154,6 +154,7 @@ optimism = [ "reth-optimism-payload-builder/optimism", "reth-ethereum-payload-builder/optimism", "reth-node-api/optimism", + "reth-node-builder/optimism", ] # no-op feature flag for switching between the `optimism` and default functionality in CI matrices diff --git a/crates/node-api/Cargo.toml b/crates/node-api/Cargo.toml index 6183d7d0ed82..c0d7b8c5017d 100644 --- a/crates/node-api/Cargo.toml +++ b/crates/node-api/Cargo.toml @@ -24,4 +24,4 @@ serde.workspace = true reth-payload-builder.workspace = true [features] -optimism = [] \ No newline at end of file +optimism = [] diff --git a/crates/node-api/src/engine/mod.rs b/crates/node-api/src/engine/mod.rs index 244711db06ee..89eabb362b9a 100644 --- a/crates/node-api/src/engine/mod.rs +++ b/crates/node-api/src/engine/mod.rs @@ -17,7 +17,7 @@ //! ```no_run //! # use reth_rpc_types::engine::{PayloadAttributes as EthPayloadAttributes, PayloadId, Withdrawal}; //! # use reth_primitives::{B256, ChainSpec, Address}; -//! # use reth_node_api::{EngineTypes, EngineApiMessageVersion, validate_version_specific_fields, AttributesValidationError, PayloadAttributes, PayloadBuilderAttributes}; +//! # use reth_node_api::{EngineTypes, EngineApiMessageVersion, validate_version_specific_fields, AttributesValidationError, PayloadAttributes, PayloadBuilderAttributes, PayloadOrAttributes}; //! # use reth_payload_builder::{EthPayloadBuilderAttributes, EthBuiltPayload}; //! # use serde::{Deserialize, Serialize}; //! # use thiserror::Error; @@ -58,7 +58,7 @@ //! chain_spec: &ChainSpec, //! version: EngineApiMessageVersion, //! ) -> Result<(), AttributesValidationError> { -//! validate_version_specific_fields(chain_spec, version, &self.into())?; +//! validate_version_specific_fields(chain_spec, version, self.into())?; //! //! // custom validation logic - ensure that the custom field is not zero //! if self.custom == 0 { @@ -122,6 +122,14 @@ //! type PayloadAttributes = CustomPayloadAttributes; //! type PayloadBuilderAttributes = CustomPayloadBuilderAttributes; //! type BuiltPayload = EthBuiltPayload; +//! +//! fn validate_version_specific_fields( +//! chain_spec: &ChainSpec, +//! version: EngineApiMessageVersion, +//! payload_or_attrs: PayloadOrAttributes<'_, CustomPayloadAttributes>, +//! ) -> Result<(), AttributesValidationError> { +//! validate_version_specific_fields(chain_spec, version, payload_or_attrs) +//! } //! } //! ``` @@ -152,6 +160,14 @@ pub trait EngineTypes: Send + Sync { /// The built payload type. type BuiltPayload: BuiltPayload + Clone + Unpin; + + /// 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: &ChainSpec, + version: EngineApiMessageVersion, + payload_or_attrs: PayloadOrAttributes<'_, Self::PayloadAttributes>, + ) -> Result<(), AttributesValidationError>; } /// Validates the timestamp depending on the version called: @@ -198,6 +214,44 @@ pub fn validate_payload_timestamp( Ok(()) } +#[cfg(feature = "optimism")] +/// Validates the presence of the `withdrawals` field according to the payload timestamp. +/// +/// After Canyon, withdrawals field must be [Some]. +/// Before Canyon, withdrawals field must be [None]; +/// +/// Canyon activates the Shanghai EIPs, see the Canyon specs for more details: +/// +pub fn optimism_validate_withdrawals_presence( + chain_spec: &ChainSpec, + version: EngineApiMessageVersion, + timestamp: u64, + has_withdrawals: bool, +) -> Result<(), AttributesValidationError> { + let is_shanghai = chain_spec.fork(Hardfork::Canyon).active_at_timestamp(timestamp); + + match version { + EngineApiMessageVersion::V1 => { + if has_withdrawals { + return Err(AttributesValidationError::WithdrawalsNotSupportedInV1) + } + if is_shanghai { + return Err(AttributesValidationError::NoWithdrawalsPostShanghai) + } + } + EngineApiMessageVersion::V2 | EngineApiMessageVersion::V3 => { + if is_shanghai && !has_withdrawals { + return Err(AttributesValidationError::NoWithdrawalsPostShanghai) + } + if !is_shanghai && has_withdrawals { + return Err(AttributesValidationError::HasWithdrawalsPreShanghai) + } + } + }; + + Ok(()) +} + /// Validates the presence of the `withdrawals` field according to the payload timestamp. /// After Shanghai, withdrawals field must be [Some]. /// Before Shanghai, withdrawals field must be [None]; @@ -290,7 +344,7 @@ pub fn validate_parent_beacon_block_root_presence( pub fn validate_version_specific_fields( chain_spec: &ChainSpec, version: EngineApiMessageVersion, - payload_or_attrs: &PayloadOrAttributes<'_, Type>, + payload_or_attrs: PayloadOrAttributes<'_, Type>, ) -> Result<(), AttributesValidationError> where Type: PayloadAttributes, @@ -309,6 +363,31 @@ where ) } +#[cfg(feature = "optimism")] +/// Validates the presence or exclusion of fork-specific fields based on the payload attributes +/// and the message version. +pub fn optimism_validate_version_specific_fields( + chain_spec: &ChainSpec, + version: EngineApiMessageVersion, + payload_or_attrs: PayloadOrAttributes<'_, Type>, +) -> Result<(), AttributesValidationError> +where + Type: PayloadAttributes, +{ + optimism_validate_withdrawals_presence( + chain_spec, + version, + payload_or_attrs.timestamp(), + payload_or_attrs.withdrawals().is_some(), + )?; + validate_parent_beacon_block_root_presence( + chain_spec, + version, + payload_or_attrs.timestamp(), + payload_or_attrs.parent_beacon_block_root().is_some(), + ) +} + /// The version of Engine API message. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum EngineApiMessageVersion { diff --git a/crates/node-api/src/engine/traits.rs b/crates/node-api/src/engine/traits.rs index 328ac21acb3e..c061a1c98c99 100644 --- a/crates/node-api/src/engine/traits.rs +++ b/crates/node-api/src/engine/traits.rs @@ -180,7 +180,7 @@ impl PayloadAttributes for EthPayloadAttributes { chain_spec: &ChainSpec, version: EngineApiMessageVersion, ) -> Result<(), AttributesValidationError> { - validate_version_specific_fields(chain_spec, version, &self.into()) + validate_version_specific_fields(chain_spec, version, self.into()) } } @@ -202,7 +202,7 @@ impl PayloadAttributes for OptimismPayloadAttributes { chain_spec: &ChainSpec, version: EngineApiMessageVersion, ) -> Result<(), AttributesValidationError> { - validate_version_specific_fields(chain_spec, version, &self.into())?; + validate_version_specific_fields(chain_spec, version, self.into())?; if self.gas_limit.is_none() && chain_spec.is_optimism() { return Err(AttributesValidationError::InvalidParams( diff --git a/crates/node-api/src/lib.rs b/crates/node-api/src/lib.rs index 718f67dc0991..7f490698d282 100644 --- a/crates/node-api/src/lib.rs +++ b/crates/node-api/src/lib.rs @@ -11,6 +11,8 @@ /// /// Notably contains the [EngineTypes] trait and implementations for ethereum mainnet types. pub mod engine; +#[cfg(feature = "optimism")] +pub use engine::optimism_validate_version_specific_fields; pub use engine::{ validate_payload_timestamp, validate_version_specific_fields, validate_withdrawals_presence, AttributesValidationError, BuiltPayload, EngineApiMessageVersion, EngineTypes, diff --git a/crates/node-builder/Cargo.toml b/crates/node-builder/Cargo.toml index 8ce1972bec90..bf0a7eb9bf83 100644 --- a/crates/node-builder/Cargo.toml +++ b/crates/node-builder/Cargo.toml @@ -19,3 +19,6 @@ reth-node-api.workspace = true # io serde.workspace = true + +[features] +optimism = ["reth-node-api/optimism"] diff --git a/crates/node-builder/src/engine.rs b/crates/node-builder/src/engine.rs index a0dfa4c4e996..05e3b77191be 100644 --- a/crates/node-builder/src/engine.rs +++ b/crates/node-builder/src/engine.rs @@ -1,8 +1,16 @@ -use reth_node_api::EngineTypes; -use reth_payload_builder::{ - EthBuiltPayload, EthPayloadBuilderAttributes, OptimismPayloadBuilderAttributes, +#[cfg(feature = "optimism")] +use reth_node_api::optimism_validate_version_specific_fields; +use reth_node_api::{ + validate_version_specific_fields, AttributesValidationError, EngineApiMessageVersion, + EngineTypes, PayloadOrAttributes, }; -use reth_rpc_types::engine::{OptimismPayloadAttributes, PayloadAttributes}; +#[cfg(feature = "optimism")] +use reth_payload_builder::OptimismPayloadBuilderAttributes; +use reth_payload_builder::{EthBuiltPayload, EthPayloadBuilderAttributes}; +use reth_primitives::ChainSpec; +#[cfg(feature = "optimism")] +use reth_rpc_types::engine::OptimismPayloadAttributes; +use reth_rpc_types::engine::PayloadAttributes as EthPayloadAttributes; /// The types used in the default mainnet ethereum beacon consensus engine. #[derive(Debug, Default, Clone)] @@ -10,18 +18,37 @@ use reth_rpc_types::engine::{OptimismPayloadAttributes, PayloadAttributes}; pub struct EthEngineTypes; impl EngineTypes for EthEngineTypes { - type PayloadAttributes = PayloadAttributes; + type PayloadAttributes = EthPayloadAttributes; type PayloadBuilderAttributes = EthPayloadBuilderAttributes; type BuiltPayload = EthBuiltPayload; + + fn validate_version_specific_fields( + chain_spec: &ChainSpec, + version: EngineApiMessageVersion, + payload_or_attrs: PayloadOrAttributes<'_, EthPayloadAttributes>, + ) -> Result<(), AttributesValidationError> { + validate_version_specific_fields(chain_spec, version, payload_or_attrs) + } } +#[cfg(feature = "optimism")] /// The types used in the optimism beacon consensus engine. #[derive(Debug, Default, Clone)] #[non_exhaustive] pub struct OptimismEngineTypes; +// TODO: remove cfg once Hardfork::Canyon can be used without the flag +#[cfg(feature = "optimism")] impl EngineTypes for OptimismEngineTypes { type PayloadAttributes = OptimismPayloadAttributes; type PayloadBuilderAttributes = OptimismPayloadBuilderAttributes; type BuiltPayload = EthBuiltPayload; + + fn validate_version_specific_fields( + chain_spec: &ChainSpec, + version: EngineApiMessageVersion, + payload_or_attrs: PayloadOrAttributes<'_, OptimismPayloadAttributes>, + ) -> Result<(), AttributesValidationError> { + optimism_validate_version_specific_fields(chain_spec, version, payload_or_attrs) + } } diff --git a/crates/node-builder/src/lib.rs b/crates/node-builder/src/lib.rs index 848101d5282b..c15e0132da5c 100644 --- a/crates/node-builder/src/lib.rs +++ b/crates/node-builder/src/lib.rs @@ -10,4 +10,7 @@ /// Exports commonly used concrete instances of the [EngineTypes](reth_node_api::EngineTypes) /// trait. pub mod engine; -pub use engine::{EthEngineTypes, OptimismEngineTypes}; +pub use engine::EthEngineTypes; + +#[cfg(feature = "optimism")] +pub use engine::OptimismEngineTypes; diff --git a/crates/rpc/rpc-engine-api/src/engine_api.rs b/crates/rpc/rpc-engine-api/src/engine_api.rs index d14118a989e4..d5592d20545d 100644 --- a/crates/rpc/rpc-engine-api/src/engine_api.rs +++ b/crates/rpc/rpc-engine-api/src/engine_api.rs @@ -4,9 +4,8 @@ use jsonrpsee_core::RpcResult; use reth_beacon_consensus::BeaconConsensusEngineHandle; use reth_interfaces::consensus::ForkchoiceState; use reth_node_api::{ - validate_payload_timestamp, validate_version_specific_fields, BuiltPayload, - EngineApiMessageVersion, EngineTypes, PayloadAttributes, PayloadBuilderAttributes, - PayloadOrAttributes, + validate_payload_timestamp, BuiltPayload, EngineApiMessageVersion, EngineTypes, + PayloadAttributes, PayloadBuilderAttributes, PayloadOrAttributes, }; use reth_payload_builder::PayloadStore; use reth_primitives::{BlockHash, BlockHashOrNumber, BlockNumber, ChainSpec, Hardfork, B256, U64}; @@ -100,10 +99,10 @@ where PayloadOrAttributes::<'_, EngineT::PayloadAttributes>::from_execution_payload( &payload, None, ); - validate_version_specific_fields( + EngineT::validate_version_specific_fields( &self.inner.chain_spec, EngineApiMessageVersion::V1, - &payload_or_attrs, + payload_or_attrs, )?; Ok(self.inner.beacon_consensus.new_payload(payload, None).await?) } @@ -118,10 +117,10 @@ where PayloadOrAttributes::<'_, EngineT::PayloadAttributes>::from_execution_payload( &payload, None, ); - validate_version_specific_fields( + EngineT::validate_version_specific_fields( &self.inner.chain_spec, EngineApiMessageVersion::V2, - &payload_or_attrs, + payload_or_attrs, )?; Ok(self.inner.beacon_consensus.new_payload(payload, None).await?) } @@ -139,10 +138,10 @@ where &payload, Some(parent_beacon_block_root), ); - validate_version_specific_fields( + EngineT::validate_version_specific_fields( &self.inner.chain_spec, EngineApiMessageVersion::V3, - &payload_or_attrs, + payload_or_attrs, )?; let cancun_fields = CancunPayloadFields { versioned_hashes, parent_beacon_block_root };