fix(engine): align Amsterdam endpoint validation (#23625)

Author: mattsse

crates/payload/primitives/src/error.rs

@@ -126,9 +126,9 @@ pub enum VersionSpecificValidationError {
     /// root after Cancun
     #[error("no parent beacon block root post-cancun")]
     NoParentBeaconBlockRootPostCancun,
-    /// Thrown if the pre-V6 `PayloadAttributes` or `ExecutionPayload` contains a block access list
-    #[error("block access list not before V6")]
-    BlockAccessListNotSupportedBeforeV6,
+    /// Thrown if the current engine method version does not support a block access list
+    #[error("block access list not supported in this engine API version")]
+    BlockAccessListNotSupported,
     /// Thrown if  `engine_newPayload` contains no block access list
     /// after Amsterdam
     #[error("no block access list post-Amsterdam")]
@@ -137,9 +137,9 @@ pub enum VersionSpecificValidationError {
     /// before Amsterdam
     #[error("block access list pre-Amsterdam")]
     HasBlockAccessListPreAmsterdam,
-    /// Thrown if the pre-V6 `PayloadAttributes` or `ExecutionPayload` contains a slot number
-    #[error("slot number not before V6")]
-    SlotNumberNotSupportedBeforeV6,
+    /// Thrown if the current engine method version does not support a slot number
+    #[error("slot number not supported in this engine API version")]
+    SlotNumberNotSupported,
     /// Thrown if  `engine_newPayload` contains no slot number
     /// after Amsterdam
     #[error("no slot number post-Amsterdam")]

crates/payload/primitives/src/lib.rs

@@ -62,8 +62,10 @@ pub trait PayloadTypes: Send + Sync + Unpin + core::fmt::Debug + Clone + 'static
 /// * If V3, this ensures that the payload timestamp is within the Cancun timestamp.
 /// * If V4, this ensures that the payload timestamp is within the Prague timestamp.
 /// * If V5, this ensures that the payload timestamp is within the Osaka timestamp.
+/// * If V6, this ensures that the payload timestamp is within the Amsterdam timestamp.
 ///
-/// Additionally, it ensures that `engine_getPayloadV4` is not used for an Osaka payload.
+/// Additionally, it ensures that `engine_getPayloadV4` is not used for an Osaka payload and that
+/// staggered endpoint upgrades reject the next fork once a newer method version is required.
 ///
 /// Otherwise, this will return [`EngineObjectValidationError::UnsupportedFork`].
 pub fn validate_payload_timestamp(
@@ -151,12 +153,26 @@ pub fn validate_payload_timestamp(
         return Err(EngineObjectValidationError::UnsupportedFork)
     }
 
+    let is_amsterdam = chain_spec.is_amsterdam_active_at_timestamp(timestamp);
+
+    // Staggered endpoint upgrades must reject Amsterdam payloads until the Amsterdam-specific
+    // method version is used.
+    if is_amsterdam &&
+        matches!(
+            (version, kind),
+            (EngineApiMessageVersion::V3, MessageValidationKind::PayloadAttributes) |
+                (EngineApiMessageVersion::V4, MessageValidationKind::Payload) |
+                (EngineApiMessageVersion::V5, MessageValidationKind::GetPayload)
+        )
+    {
+        return Err(EngineObjectValidationError::UnsupportedFork)
+    }
+
     // `engine_getPayloadV4` MUST reject payloads with a timestamp >= Osaka.
     if version.is_v4() && kind == MessageValidationKind::GetPayload && is_osaka {
         return Err(EngineObjectValidationError::UnsupportedFork)
     }
 
-    let is_amsterdam = chain_spec.is_amsterdam_active_at_timestamp(timestamp);
     if version.is_v6() && !is_amsterdam {
         // From the Engine API spec:
         // <https://github.com/ethereum/execution-apis/blob/15399c2e2f16a5f800bf3f285640357e2c245ad9/src/engine/osaka.md#specification>
@@ -183,16 +199,30 @@ pub fn validate_block_access_list_presence<T: EthereumHardforks>(
     has_block_access_list: bool,
 ) -> Result<(), EngineObjectValidationError> {
     let is_amsterdam_active = chain_spec.is_amsterdam_active_at_timestamp(timestamp);
-
     match version {
         EngineApiMessageVersion::V1 |
         EngineApiMessageVersion::V2 |
         EngineApiMessageVersion::V3 |
-        EngineApiMessageVersion::V4 |
-        EngineApiMessageVersion::V5 => {
+        EngineApiMessageVersion::V4 => {
             if has_block_access_list {
                 return Err(message_validation_kind
-                    .to_error(VersionSpecificValidationError::BlockAccessListNotSupportedBeforeV6))
+                    .to_error(VersionSpecificValidationError::BlockAccessListNotSupported))
+            }
+        }
+
+        EngineApiMessageVersion::V5 => {
+            if message_validation_kind == MessageValidationKind::Payload {
+                if is_amsterdam_active && !has_block_access_list {
+                    return Err(message_validation_kind
+                        .to_error(VersionSpecificValidationError::NoBlockAccessListPostAmsterdam))
+                }
+                if !is_amsterdam_active && has_block_access_list {
+                    return Err(message_validation_kind
+                        .to_error(VersionSpecificValidationError::HasBlockAccessListPreAmsterdam))
+                }
+            } else if has_block_access_list {
+                return Err(message_validation_kind
+                    .to_error(VersionSpecificValidationError::BlockAccessListNotSupported))
             }
         }
 
@@ -224,14 +254,42 @@ pub fn validate_slot_number_presence<T: EthereumHardforks>(
     let is_amsterdam_active = chain_spec.is_amsterdam_active_at_timestamp(timestamp);
 
     match version {
-        EngineApiMessageVersion::V1 |
-        EngineApiMessageVersion::V2 |
-        EngineApiMessageVersion::V3 |
-        EngineApiMessageVersion::V4 |
-        EngineApiMessageVersion::V5 => {
+        EngineApiMessageVersion::V1 | EngineApiMessageVersion::V2 | EngineApiMessageVersion::V3 => {
             if has_slot_number {
                 return Err(message_validation_kind
-                    .to_error(VersionSpecificValidationError::SlotNumberNotSupportedBeforeV6))
+                    .to_error(VersionSpecificValidationError::SlotNumberNotSupported))
+            }
+        }
+
+        EngineApiMessageVersion::V4 => {
+            if message_validation_kind == MessageValidationKind::PayloadAttributes {
+                if is_amsterdam_active && !has_slot_number {
+                    return Err(message_validation_kind
+                        .to_error(VersionSpecificValidationError::NoSlotNumberPostAmsterdam))
+                }
+                if !is_amsterdam_active && has_slot_number {
+                    return Err(message_validation_kind
+                        .to_error(VersionSpecificValidationError::HasSlotNumberPreAmsterdam))
+                }
+            } else if has_slot_number {
+                return Err(message_validation_kind
+                    .to_error(VersionSpecificValidationError::SlotNumberNotSupported))
+            }
+        }
+
+        EngineApiMessageVersion::V5 => {
+            if message_validation_kind == MessageValidationKind::Payload {
+                if is_amsterdam_active && !has_slot_number {
+                    return Err(message_validation_kind
+                        .to_error(VersionSpecificValidationError::NoSlotNumberPostAmsterdam))
+                }
+                if !is_amsterdam_active && has_slot_number {
+                    return Err(message_validation_kind
+                        .to_error(VersionSpecificValidationError::HasSlotNumberPreAmsterdam))
+                }
+            } else if has_slot_number {
+                return Err(message_validation_kind
+                    .to_error(VersionSpecificValidationError::SlotNumberNotSupported))
             }
         }
 
@@ -651,6 +709,75 @@ mod tests {
         assert_matches!(res, Ok(()));
     }
 
+    #[test]
+    fn validate_amsterdam_staggered_version_restrictions() {
+        let chain_spec = ChainSpecBuilder::mainnet().amsterdam_activated().build();
+
+        let res = validate_payload_timestamp(
+            &chain_spec,
+            EngineApiMessageVersion::V3,
+            0,
+            MessageValidationKind::PayloadAttributes,
+        );
+        assert_matches!(res, Err(EngineObjectValidationError::UnsupportedFork));
+
+        let res = validate_payload_timestamp(
+            &chain_spec,
+            EngineApiMessageVersion::V4,
+            0,
+            MessageValidationKind::Payload,
+        );
+        assert_matches!(res, Err(EngineObjectValidationError::UnsupportedFork));
+
+        let res = validate_payload_timestamp(
+            &chain_spec,
+            EngineApiMessageVersion::V5,
+            0,
+            MessageValidationKind::GetPayload,
+        );
+        assert_matches!(res, Err(EngineObjectValidationError::UnsupportedFork));
+
+        let res = validate_payload_timestamp(
+            &chain_spec,
+            EngineApiMessageVersion::V6,
+            0,
+            MessageValidationKind::GetPayload,
+        );
+        assert_matches!(res, Ok(()));
+    }
+
+    #[test]
+    fn validate_amsterdam_slot_and_bal_presence() {
+        let chain_spec = ChainSpecBuilder::mainnet().amsterdam_activated().build();
+
+        let res = validate_slot_number_presence(
+            &chain_spec,
+            EngineApiMessageVersion::V4,
+            MessageValidationKind::PayloadAttributes,
+            0,
+            true,
+        );
+        assert_matches!(res, Ok(()));
+
+        let res = validate_slot_number_presence(
+            &chain_spec,
+            EngineApiMessageVersion::V5,
+            MessageValidationKind::Payload,
+            0,
+            true,
+        );
+        assert_matches!(res, Ok(()));
+
+        let res = validate_block_access_list_presence(
+            &chain_spec,
+            EngineApiMessageVersion::V5,
+            MessageValidationKind::Payload,
+            0,
+            true,
+        );
+        assert_matches!(res, Ok(()));
+    }
+
     #[test]
     fn execution_requests_validation() {
         assert_matches!(validate_execution_requests(&[]), Ok(()));

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

@@ -269,7 +269,7 @@ where
         >::from_execution_payload(&payload);
         self.inner
             .validator
-            .validate_version_specific_fields(EngineApiMessageVersion::V6, payload_or_attrs)?;
+            .validate_version_specific_fields(EngineApiMessageVersion::V5, payload_or_attrs)?;
         Ok(self.inner.beacon_consensus.new_payload(payload).await?)
     }
 
@@ -386,7 +386,7 @@ where
         state: ForkchoiceState,
         payload_attrs: Option<EngineT::PayloadAttributes>,
     ) -> EngineApiResult<ForkchoiceUpdated> {
-        self.validate_and_execute_forkchoice(EngineApiMessageVersion::V6, state, payload_attrs)
+        self.validate_and_execute_forkchoice(EngineApiMessageVersion::V4, state, payload_attrs)
             .await
     }
 
@@ -1438,9 +1438,10 @@ struct EngineApiInner<Provider, PayloadT: PayloadTypes, Pool, Validator, ChainSp
 #[cfg(test)]
 mod tests {
     use super::*;
-    use alloy_primitives::{Address, B256};
+    use alloy_eips::eip7685::Requests;
+    use alloy_primitives::{Address, Bytes, B256};
     use alloy_rpc_types_engine::{
-        ClientCode, ClientVersionV1, PayloadAttributes, PayloadStatusEnum,
+        ClientCode, ClientVersionV1, ExecutionPayloadV2, PayloadAttributes, PayloadStatusEnum,
     };
     use assert_matches::assert_matches;
     use reth_chainspec::{ChainSpec, ChainSpecBuilder, MAINNET};
@@ -1532,6 +1533,63 @@ mod tests {
         assert_matches!(handle.from_api.recv().await, Some(BeaconEngineMessage::NewPayload { .. }));
     }
 
+    #[tokio::test]
+    async fn new_payload_v5_accepts_amsterdam_payloads() {
+        let chain_spec = Arc::new(ChainSpecBuilder::mainnet().amsterdam_activated().build());
+        let provider = Arc::new(MockEthProvider::default());
+        let payload_store = spawn_test_payload_service::<EthEngineTypes>();
+        let (to_engine, mut engine_rx) = unbounded_channel();
+
+        let api = EngineApi::new(
+            provider,
+            chain_spec.clone(),
+            ConsensusEngineHandle::new(to_engine),
+            payload_store.into(),
+            NoopTransactionPool::default(),
+            Runtime::test(),
+            ClientVersionV1 {
+                code: ClientCode::RH,
+                name: "Reth".to_string(),
+                version: "v0.0.0-test".to_string(),
+                commit: "test".to_string(),
+            },
+            EngineCapabilities::default(),
+            EthereumEngineValidator::new(chain_spec),
+            false,
+            NoopNetwork::default(),
+        );
+
+        tokio::spawn(async move {
+            let payload_v1 = ExecutionPayloadV1::from_block_slow(&Block::default());
+            let payload = ExecutionPayloadV4 {
+                payload_inner: ExecutionPayloadV3 {
+                    payload_inner: ExecutionPayloadV2 {
+                        payload_inner: payload_v1,
+                        withdrawals: Vec::new(),
+                    },
+                    blob_gas_used: 0,
+                    excess_blob_gas: 0,
+                },
+                block_access_list: Bytes::from_static(b"bal"),
+                slot_number: 1,
+            };
+            let execution_data = ExecutionData {
+                payload: payload.into(),
+                sidecar: ExecutionPayloadSidecar::v4(
+                    CancunPayloadFields {
+                        versioned_hashes: Vec::new(),
+                        parent_beacon_block_root: B256::ZERO,
+                    },
+                    PraguePayloadFields { requests: RequestsOrHash::Requests(Requests::default()) },
+                ),
+            };
+
+            api.new_payload_v5(execution_data).await.unwrap();
+        });
+
+        assert_matches!(engine_rx.recv().await, Some(BeaconEngineMessage::NewPayload { .. }));
+    }
+
     #[derive(Clone)]
     struct TestNetworkInfo {
         syncing: bool,

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

@@ -121,12 +121,12 @@ impl From<EngineApiError> for jsonrpsee_types::error::ErrorObject<'static> {
                     VersionSpecificValidationError::WithdrawalsNotSupportedInV1 |
                     VersionSpecificValidationError::NoWithdrawalsPostShanghai |
                     VersionSpecificValidationError::HasWithdrawalsPreShanghai |
-                    VersionSpecificValidationError::BlockAccessListNotSupportedBeforeV6 |
+                    VersionSpecificValidationError::BlockAccessListNotSupported |
                     VersionSpecificValidationError::HasBlockAccessListPreAmsterdam |
                     VersionSpecificValidationError::NoBlockAccessListPostAmsterdam |
                     VersionSpecificValidationError::HasSlotNumberPreAmsterdam |
                     VersionSpecificValidationError::NoSlotNumberPostAmsterdam |
-                    VersionSpecificValidationError::SlotNumberNotSupportedBeforeV6,
+                    VersionSpecificValidationError::SlotNumberNotSupported,
                 ),
             ) |
             EngineApiError::UnexpectedRequestsHash => {