feat: validate engine cancun fields based on method version (#4256)

crates/rpc/rpc-engine-api/src/engine_api.rs

@@ -1,4 +1,6 @@
-use crate::{EngineApiError, EngineApiMessageVersion, EngineApiResult};
+use crate::{
+    payload::PayloadOrAttributes, EngineApiError, EngineApiMessageVersion, EngineApiResult,
+};
 use async_trait::async_trait;
 use jsonrpsee_core::RpcResult;
 use reth_beacon_consensus::BeaconConsensusEngineHandle;
@@ -69,10 +71,9 @@ where
         &self,
         payload: ExecutionPayload,
     ) -> EngineApiResult<PayloadStatus> {
-        self.validate_withdrawals_presence(
+        self.validate_version_specific_fields(
             EngineApiMessageVersion::V1,
-            payload.timestamp.as_u64(),
-            payload.withdrawals.is_some(),
+            PayloadOrAttributes::from_execution_payload(&payload, None),
         )?;
         Ok(self.inner.beacon_consensus.new_payload(payload).await?)
     }
@@ -82,14 +83,29 @@ where
         &self,
         payload: ExecutionPayload,
     ) -> EngineApiResult<PayloadStatus> {
-        self.validate_withdrawals_presence(
+        self.validate_version_specific_fields(
             EngineApiMessageVersion::V2,
-            payload.timestamp.as_u64(),
-            payload.withdrawals.is_some(),
+            PayloadOrAttributes::from_execution_payload(&payload, None),
         )?;
         Ok(self.inner.beacon_consensus.new_payload(payload).await?)
     }
 
+    /// See also <https://github.com/ethereum/execution-apis/blob/fe8e13c288c592ec154ce25c534e26cb7ce0530d/src/engine/cancun.md#engine_newpayloadv3>
+    pub async fn new_payload_v3(
+        &self,
+        payload: ExecutionPayload,
+        _versioned_hashes: Vec<H256>,
+        parent_beacon_block_root: H256,
+    ) -> EngineApiResult<PayloadStatus> {
+        self.validate_version_specific_fields(
+            EngineApiMessageVersion::V3,
+            PayloadOrAttributes::from_execution_payload(&payload, Some(parent_beacon_block_root)),
+        )?;
+
+        // TODO: validate versioned hashes and figure out what to do with parent_beacon_block_root
+        Ok(self.inner.beacon_consensus.new_payload(payload).await?)
+    }
+
     /// Sends a message to the beacon consensus engine to update the fork choice _without_
     /// withdrawals.
     ///
@@ -102,11 +118,7 @@ where
         payload_attrs: Option<PayloadAttributes>,
     ) -> EngineApiResult<ForkchoiceUpdated> {
         if let Some(ref attrs) = payload_attrs {
-            self.validate_withdrawals_presence(
-                EngineApiMessageVersion::V1,
-                attrs.timestamp.as_u64(),
-                attrs.withdrawals.is_some(),
-            )?;
+            self.validate_version_specific_fields(EngineApiMessageVersion::V1, attrs.into())?;
         }
         Ok(self.inner.beacon_consensus.fork_choice_updated(state, payload_attrs).await?)
     }
@@ -121,11 +133,7 @@ where
         payload_attrs: Option<PayloadAttributes>,
     ) -> EngineApiResult<ForkchoiceUpdated> {
         if let Some(ref attrs) = payload_attrs {
-            self.validate_withdrawals_presence(
-                EngineApiMessageVersion::V2,
-                attrs.timestamp.as_u64(),
-                attrs.withdrawals.is_some(),
-            )?;
+            self.validate_version_specific_fields(EngineApiMessageVersion::V2, attrs.into())?;
         }
         Ok(self.inner.beacon_consensus.fork_choice_updated(state, payload_attrs).await?)
     }
@@ -140,11 +148,7 @@ where
         payload_attrs: Option<PayloadAttributes>,
     ) -> EngineApiResult<ForkchoiceUpdated> {
         if let Some(ref attrs) = payload_attrs {
-            self.validate_withdrawals_presence(
-                EngineApiMessageVersion::V3,
-                attrs.timestamp.as_u64(),
-                attrs.withdrawals.is_some(),
-            )?;
+            self.validate_version_specific_fields(EngineApiMessageVersion::V3, attrs.into())?;
         }
 
         Ok(self.inner.beacon_consensus.fork_choice_updated(state, payload_attrs).await?)
@@ -353,6 +357,70 @@ where
 
         Ok(())
     }
+
+    /// Validate the presence of the `parentBeaconBlockRoot` field according to the payload
+    /// timestamp.
+    ///
+    /// After Cancun, `parentBeaconBlockRoot` field must be [Some].
+    /// Before Cancun, `parentBeaconBlockRoot` field must be [None].
+    ///
+    /// If the payload attribute's timestamp is before the Cancun fork and the engine API message
+    /// version is V3, then this will return [EngineApiError::UnsupportedFork].
+    ///
+    /// If the engine API message version is V1 or V2, and the payload attribute's timestamp is
+    /// post-Cancun, then this will return [EngineApiError::NoParentBeaconBlockRootPostCancun].
+    ///
+    /// Implements the following Engine API spec rule:
+    ///
+    /// * Client software MUST return `-38005: Unsupported fork` error if the timestamp of the
+    /// payload does not fall within the time frame of the Cancun fork.
+    fn validate_parent_beacon_block_root_presence(
+        &self,
+        version: EngineApiMessageVersion,
+        timestamp: u64,
+        has_parent_beacon_block_root: bool,
+    ) -> EngineApiResult<()> {
+        let is_cancun = self.inner.chain_spec.fork(Hardfork::Cancun).active_at_timestamp(timestamp);
+
+        match version {
+            EngineApiMessageVersion::V1 | EngineApiMessageVersion::V2 => {
+                if has_parent_beacon_block_root {
+                    return Err(EngineApiError::ParentBeaconBlockRootNotSupportedBeforeV3)
+                }
+                if is_cancun {
+                    return Err(EngineApiError::NoParentBeaconBlockRootPostCancun)
+                }
+            }
+            EngineApiMessageVersion::V3 => {
+                if !is_cancun {
+                    return Err(EngineApiError::UnsupportedFork)
+                } else if !has_parent_beacon_block_root {
+                    return Err(EngineApiError::NoParentBeaconBlockRootPostCancun)
+                }
+            }
+        };
+
+        Ok(())
+    }
+
+    /// Validates the presence or exclusion of fork-specific fields based on the payload attributes
+    /// and the message version.
+    fn validate_version_specific_fields(
+        &self,
+        version: EngineApiMessageVersion,
+        payload_or_attrs: PayloadOrAttributes<'_>,
+    ) -> EngineApiResult<()> {
+        self.validate_withdrawals_presence(
+            version,
+            payload_or_attrs.timestamp(),
+            payload_or_attrs.withdrawals().is_some(),
+        )?;
+        self.validate_parent_beacon_block_root_presence(
+            version,
+            payload_or_attrs.timestamp(),
+            payload_or_attrs.parent_beacon_block_root().is_some(),
+        )
+    }
 }
 
 #[async_trait]

crates/rpc/rpc-engine-api/src/error.rs

@@ -7,6 +7,8 @@ use thiserror::Error;
 /// The Engine API result type
 pub type EngineApiResult<Ok> = Result<Ok, EngineApiError>;
 
+/// Payload unsupported fork code.
+pub const UNSUPPORTED_FORK_CODE: i32 = -38005;
 /// Payload unknown error code.
 pub const UNKNOWN_PAYLOAD_CODE: i32 = -38001;
 /// Request too large error code.
@@ -34,6 +36,10 @@ pub enum EngineApiError {
         /// requested number of items
         count: u64,
     },
+    /// Thrown if `PayloadAttributes` provided in engine_forkchoiceUpdated before V3 contains a
+    /// parent beacon block root
+    #[error("parent beacon block root not supported before V3")]
+    ParentBeaconBlockRootNotSupportedBeforeV3,
     /// Thrown if engine_forkchoiceUpdatedV1 contains withdrawals
     #[error("withdrawals not supported in V1")]
     WithdrawalsNotSupportedInV1,
@@ -43,6 +49,14 @@ pub enum EngineApiError {
     /// Thrown if engine_forkchoiceUpdated contains withdrawals before Shanghai
     #[error("withdrawals pre-shanghai")]
     HasWithdrawalsPreShanghai,
+    /// Thrown if the `PayloadAttributes` provided in engine_forkchoiceUpdated contains no parent
+    /// beacon block root after Cancun
+    #[error("no parent beacon block root post-cancun")]
+    NoParentBeaconBlockRootPostCancun,
+    /// Thrown if `PayloadAttributes` were provided with a timestamp, but the version of the engine
+    /// method called is meant for a fork that occurs after the provided timestamp.
+    #[error("unsupported fork")]
+    UnsupportedFork,
     /// Terminal total difficulty mismatch during transition configuration exchange.
     #[error(
         "Invalid transition terminal total difficulty. Execution: {execution}. Consensus: {consensus}"
@@ -82,10 +96,13 @@ impl From<EngineApiError> for jsonrpsee_types::error::ErrorObject<'static> {
         let code = match error {
             EngineApiError::InvalidBodiesRange { .. } |
             EngineApiError::WithdrawalsNotSupportedInV1 |
+            EngineApiError::ParentBeaconBlockRootNotSupportedBeforeV3 |
+            EngineApiError::NoParentBeaconBlockRootPostCancun |
             EngineApiError::NoWithdrawalsPostShanghai |
             EngineApiError::HasWithdrawalsPreShanghai => INVALID_PARAMS_CODE,
             EngineApiError::UnknownPayload => UNKNOWN_PAYLOAD_CODE,
             EngineApiError::PayloadRequestTooLarge { .. } => REQUEST_TOO_LARGE_CODE,
+            EngineApiError::UnsupportedFork => UNSUPPORTED_FORK_CODE,
 
             // Error responses from the consensus engine
             EngineApiError::ForkChoiceUpdate(ref err) => match err {

crates/rpc/rpc-engine-api/src/lib.rs

@@ -20,6 +20,9 @@ mod engine_api;
 /// The Engine API message type.
 mod message;
 
+/// An type representing either an execution payload or payload attributes.
+mod payload;
+
 /// Engine API error.
 mod error;
 

crates/rpc/rpc-engine-api/src/payload.rs

@@ -0,0 +1,56 @@
+use reth_primitives::{Withdrawal, H256};
+use reth_rpc_types::engine::{ExecutionPayload, PayloadAttributes};
+
+/// Either an [ExecutionPayload] or a [PayloadAttributes].
+pub(crate) enum PayloadOrAttributes<'a> {
+    /// An [ExecutionPayload] and optional parent beacon block root.
+    ExecutionPayload {
+        /// The inner execution payload
+        payload: &'a ExecutionPayload,
+        /// The parent beacon block root
+        parent_beacon_block_root: Option<H256>,
+    },
+    /// A [PayloadAttributes].
+    PayloadAttributes(&'a PayloadAttributes),
+}
+
+impl<'a> PayloadOrAttributes<'a> {
+    /// Construct a [PayloadOrAttributes] from an [ExecutionPayload] and optional parent beacon
+    /// block root.
+    pub(crate) fn from_execution_payload(
+        payload: &'a ExecutionPayload,
+        parent_beacon_block_root: Option<H256>,
+    ) -> Self {
+        Self::ExecutionPayload { payload, parent_beacon_block_root }
+    }
+
+    /// Return the withdrawals for the payload or attributes.
+    pub(crate) fn withdrawals(&self) -> &Option<Vec<Withdrawal>> {
+        match self {
+            Self::ExecutionPayload { payload, .. } => &payload.withdrawals,
+            Self::PayloadAttributes(attributes) => &attributes.withdrawals,
+        }
+    }
+
+    /// Return the timestamp for the payload or attributes.
+    pub(crate) fn timestamp(&self) -> u64 {
+        match self {
+            Self::ExecutionPayload { payload, .. } => payload.timestamp.as_u64(),
+            Self::PayloadAttributes(attributes) => attributes.timestamp.as_u64(),
+        }
+    }
+
+    /// Return the parent beacon block root for the payload or attributes.
+    pub(crate) fn parent_beacon_block_root(&self) -> Option<H256> {
+        match self {
+            Self::ExecutionPayload { parent_beacon_block_root, .. } => *parent_beacon_block_root,
+            Self::PayloadAttributes(attributes) => attributes.parent_beacon_block_root,
+        }
+    }
+}
+
+impl<'a> From<&'a PayloadAttributes> for PayloadOrAttributes<'a> {
+    fn from(attributes: &'a PayloadAttributes) -> Self {
+        Self::PayloadAttributes(attributes)
+    }
+}