refactor(engine-api): change payload fn arg with ExecutionPayload (#14673)

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

crates/payload/primitives/src/payload.rs

@@ -27,6 +27,9 @@ pub trait ExecutionPayload:
 
     /// Returns the timestamp to be used in the payload.
     fn timestamp(&self) -> u64;
+
+    /// Gas used by the payload
+    fn gas_used(&self) -> u64;
 }
 
 impl ExecutionPayload for ExecutionData {
@@ -53,6 +56,10 @@ impl ExecutionPayload for ExecutionData {
     fn timestamp(&self) -> u64 {
         self.payload.timestamp()
     }
+
+    fn gas_used(&self) -> u64 {
+        self.payload.as_v1().gas_used
+    }
 }
 
 /// Either a type that implements the [`ExecutionPayload`] or a type that implements the
@@ -152,4 +159,8 @@ impl ExecutionPayload for op_alloy_rpc_types_engine::OpExecutionData {
     fn timestamp(&self) -> u64 {
         self.payload.as_v2().timestamp()
     }
+
+    fn gas_used(&self) -> u64 {
+        self.payload.as_v2().payload_inner.gas_used
+    }
 }

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

@@ -21,8 +21,8 @@ use reth_chainspec::{EthereumHardfork, EthereumHardforks};
 use reth_engine_primitives::{BeaconConsensusEngineHandle, EngineTypes, EngineValidator};
 use reth_payload_builder::PayloadStore;
 use reth_payload_primitives::{
-    validate_payload_timestamp, EngineApiMessageVersion, PayloadBuilderAttributes,
-    PayloadOrAttributes,
+    validate_payload_timestamp, EngineApiMessageVersion, ExecutionPayload,
+    PayloadBuilderAttributes, PayloadOrAttributes,
 };
 use reth_primitives_traits::{Block, BlockBody};
 use reth_rpc_api::{EngineApiServer, IntoEngineApiRpcModule};
@@ -44,6 +44,16 @@ 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.
+///
+/// This type is generic over [`EngineTypes`] and intended to be used as the entrypoint for engine
+/// API processing. It can be reused by other non L1 engine APIs that deviate from the L1 spec but
+/// are still follow the engine API model.
+///
+/// ## Implementors
+///
+/// Implementing support for an engine API jsonrpsee RPC handler is done by defining the engine API
+/// server trait and implementing it on a type that can wrap this [`EngineApi`] type.
+/// See also [`EngineApiServer`] implementation for this type which is the L1 implementation.
 pub struct EngineApi<Provider, EngineT: EngineTypes, Pool, Validator, ChainSpec> {
     inner: Arc<EngineApiInner<Provider, EngineT, Pool, Validator, ChainSpec>>,
 }
@@ -549,7 +559,7 @@ impl<Provider, EngineT, Pool, Validator, ChainSpec>
     EngineApi<Provider, EngineT, Pool, Validator, ChainSpec>
 where
     Provider: HeaderProvider + BlockReader + StateProviderFactory + 'static,
-    EngineT: EngineTypes<ExecutionData = ExecutionData>,
+    EngineT: EngineTypes,
     Pool: TransactionPool + 'static,
     Validator: EngineValidator<EngineT>,
     ChainSpec: EthereumHardforks + Send + Sync + 'static,
@@ -558,14 +568,14 @@ where
     /// Caution: This should not accept the `withdrawals` field
     pub async fn new_payload_v1(
         &self,
-        payload: ExecutionPayloadV1,
+        payload: EngineT::ExecutionData,
     ) -> EngineApiResult<PayloadStatus> {
-        let payload =
-            ExecutionData { payload: payload.into(), sidecar: ExecutionPayloadSidecar::none() };
-        let payload_or_attrs =
-            PayloadOrAttributes::<'_, ExecutionData, EngineT::PayloadAttributes>::from_execution_payload(
-                &payload,
-            );
+        let payload_or_attrs = PayloadOrAttributes::<
+            '_,
+            EngineT::ExecutionData,
+            EngineT::PayloadAttributes,
+        >::from_execution_payload(&payload);
+
         self.inner
             .validator
             .validate_version_specific_fields(EngineApiMessageVersion::V1, payload_or_attrs)?;
@@ -581,10 +591,11 @@ where
     /// Metered version of `new_payload_v1`.
     async fn new_payload_v1_metered(
         &self,
-        payload: ExecutionPayloadV1,
+        payload: EngineT::ExecutionData,
     ) -> EngineApiResult<PayloadStatus> {
         let start = Instant::now();
-        let gas_used = payload.gas_used;
+        let gas_used = payload.gas_used();
+
         let res = Self::new_payload_v1(self, payload).await;
         let elapsed = start.elapsed();
         self.inner.metrics.latency.new_payload_v1.record(elapsed);
@@ -595,16 +606,13 @@ where
     /// See also <https://github.com/ethereum/execution-apis/blob/584905270d8ad665718058060267061ecfd79ca5/src/engine/shanghai.md#engine_newpayloadv2>
     pub async fn new_payload_v2(
         &self,
-        payload: ExecutionPayloadInputV2,
+        payload: EngineT::ExecutionData,
     ) -> EngineApiResult<PayloadStatus> {
-        let payload = ExecutionData {
-            payload: payload.into_payload(),
-            sidecar: ExecutionPayloadSidecar::none(),
-        };
-        let payload_or_attrs =
-            PayloadOrAttributes::<'_, ExecutionData, EngineT::PayloadAttributes>::from_execution_payload(
-                &payload,
-            );
+        let payload_or_attrs = PayloadOrAttributes::<
+            '_,
+            EngineT::ExecutionData,
+            EngineT::PayloadAttributes,
+        >::from_execution_payload(&payload);
         self.inner
             .validator
             .validate_version_specific_fields(EngineApiMessageVersion::V2, payload_or_attrs)?;
@@ -619,10 +627,11 @@ where
     /// Metered version of `new_payload_v2`.
     pub async fn new_payload_v2_metered(
         &self,
-        payload: ExecutionPayloadInputV2,
+        payload: EngineT::ExecutionData,
     ) -> EngineApiResult<PayloadStatus> {
         let start = Instant::now();
-        let gas_used = payload.execution_payload.gas_used;
+        let gas_used = payload.gas_used();
+
         let res = Self::new_payload_v2(self, payload).await;
         let elapsed = start.elapsed();
         self.inner.metrics.latency.new_payload_v2.record(elapsed);
@@ -633,21 +642,13 @@ where
     /// See also <https://github.com/ethereum/execution-apis/blob/fe8e13c288c592ec154ce25c534e26cb7ce0530d/src/engine/cancun.md#engine_newpayloadv3>
     pub async fn new_payload_v3(
         &self,
-        payload: ExecutionPayloadV3,
-        versioned_hashes: Vec<B256>,
-        parent_beacon_block_root: B256,
+        payload: EngineT::ExecutionData,
     ) -> EngineApiResult<PayloadStatus> {
-        let payload = ExecutionData {
-            payload: payload.into(),
-            sidecar: ExecutionPayloadSidecar::v3(CancunPayloadFields {
-                versioned_hashes,
-                parent_beacon_block_root,
-            }),
-        };
-        let payload_or_attrs =
-            PayloadOrAttributes::<'_, ExecutionData, EngineT::PayloadAttributes>::from_execution_payload(
-                &payload,
-            );
+        let payload_or_attrs = PayloadOrAttributes::<
+            '_,
+            EngineT::ExecutionData,
+            EngineT::PayloadAttributes,
+        >::from_execution_payload(&payload);
         self.inner
             .validator
             .validate_version_specific_fields(EngineApiMessageVersion::V3, payload_or_attrs)?;
@@ -663,14 +664,12 @@ where
     // Metrics version of `new_payload_v3`
     async fn new_payload_v3_metered(
         &self,
-        payload: ExecutionPayloadV3,
-        versioned_hashes: Vec<B256>,
-        parent_beacon_block_root: B256,
+        payload: EngineT::ExecutionData,
     ) -> RpcResult<PayloadStatus> {
         let start = Instant::now();
-        let gas_used = payload.payload_inner.payload_inner.gas_used;
-        let res =
-            Self::new_payload_v3(self, payload, versioned_hashes, parent_beacon_block_root).await;
+        let gas_used = payload.gas_used();
+
+        let res = Self::new_payload_v3(self, payload).await;
         let elapsed = start.elapsed();
         self.inner.metrics.latency.new_payload_v3.record(elapsed);
         self.inner.metrics.new_payload_response.update_response_metrics(&res, gas_used, elapsed);
@@ -680,30 +679,17 @@ where
     /// See also <https://github.com/ethereum/execution-apis/blob/7907424db935b93c2fe6a3c0faab943adebe8557/src/engine/prague.md#engine_newpayloadv4>
     pub async fn new_payload_v4(
         &self,
-        payload: ExecutionPayloadV3,
-        versioned_hashes: Vec<B256>,
-        parent_beacon_block_root: B256,
-        execution_requests: Requests,
+        payload: EngineT::ExecutionData,
     ) -> EngineApiResult<PayloadStatus> {
-        let payload = ExecutionData {
-            payload: payload.into(),
-            sidecar: ExecutionPayloadSidecar::v4(
-                CancunPayloadFields { versioned_hashes, parent_beacon_block_root },
-                PraguePayloadFields { requests: RequestsOrHash::Requests(execution_requests) },
-            ),
-        };
-        let payload_or_attrs =
-            PayloadOrAttributes::<'_, ExecutionData, EngineT::PayloadAttributes>::from_execution_payload(
-                &payload,
-            );
+        let payload_or_attrs = PayloadOrAttributes::<
+            '_,
+            EngineT::ExecutionData,
+            EngineT::PayloadAttributes,
+        >::from_execution_payload(&payload);
         self.inner
             .validator
             .validate_version_specific_fields(EngineApiMessageVersion::V4, payload_or_attrs)?;
 
-        if let Some(requests) = payload.sidecar.requests() {
-            self.inner.validator.validate_execution_requests(requests)?;
-        }
-
         Ok(self
             .inner
             .beacon_consensus
@@ -715,21 +701,13 @@ where
     /// Metrics version of `new_payload_v4`
     async fn new_payload_v4_metered(
         &self,
-        payload: ExecutionPayloadV3,
-        versioned_hashes: Vec<B256>,
-        parent_beacon_block_root: B256,
-        execution_requests: Requests,
+        payload: EngineT::ExecutionData,
     ) -> RpcResult<PayloadStatus> {
         let start = Instant::now();
-        let gas_used = payload.payload_inner.payload_inner.gas_used;
-        let res = Self::new_payload_v4(
-            self,
-            payload,
-            versioned_hashes,
-            parent_beacon_block_root,
-            execution_requests,
-        )
-        .await;
+        let gas_used = payload.gas_used();
+
+        let res = Self::new_payload_v4(self, payload).await;
+
         let elapsed = start.elapsed();
         self.inner.metrics.latency.new_payload_v4.record(elapsed);
         self.inner.metrics.new_payload_response.update_response_metrics(&res, gas_used, elapsed);
@@ -757,6 +735,7 @@ where
     }
 }
 
+// This is the concrete ethereum engine API implementation.
 #[async_trait]
 impl<Provider, EngineT, Pool, Validator, ChainSpec> EngineApiServer<EngineT>
     for EngineApi<Provider, EngineT, Pool, Validator, ChainSpec>
@@ -772,6 +751,8 @@ where
     /// Caution: This should not accept the `withdrawals` field
     async fn new_payload_v1(&self, payload: ExecutionPayloadV1) -> RpcResult<PayloadStatus> {
         trace!(target: "rpc::engine", "Serving engine_newPayloadV1");
+        let payload =
+            ExecutionData { payload: payload.into(), sidecar: ExecutionPayloadSidecar::none() };
         Ok(self.new_payload_v1_metered(payload).await?)
     }
 
@@ -779,6 +760,11 @@ where
     /// See also <https://github.com/ethereum/execution-apis/blob/584905270d8ad665718058060267061ecfd79ca5/src/engine/shanghai.md#engine_newpayloadv2>
     async fn new_payload_v2(&self, payload: ExecutionPayloadInputV2) -> RpcResult<PayloadStatus> {
         trace!(target: "rpc::engine", "Serving engine_newPayloadV2");
+        let payload = ExecutionData {
+            payload: payload.into_payload(),
+            sidecar: ExecutionPayloadSidecar::none(),
+        };
+
         Ok(self.new_payload_v2_metered(payload).await?)
     }
 
@@ -791,7 +777,15 @@ where
         parent_beacon_block_root: B256,
     ) -> RpcResult<PayloadStatus> {
         trace!(target: "rpc::engine", "Serving engine_newPayloadV3");
-        Ok(self.new_payload_v3_metered(payload, versioned_hashes, parent_beacon_block_root).await?)
+        let payload = ExecutionData {
+            payload: payload.into(),
+            sidecar: ExecutionPayloadSidecar::v3(CancunPayloadFields {
+                versioned_hashes,
+                parent_beacon_block_root,
+            }),
+        };
+
+        Ok(self.new_payload_v3_metered(payload).await?)
     }
 
     /// Handler for `engine_newPayloadV4`
@@ -804,14 +798,15 @@ where
         execution_requests: Requests,
     ) -> RpcResult<PayloadStatus> {
         trace!(target: "rpc::engine", "Serving engine_newPayloadV4");
-        Ok(self
-            .new_payload_v4_metered(
-                payload,
-                versioned_hashes,
-                parent_beacon_block_root,
-                execution_requests,
-            )
-            .await?)
+        let payload = ExecutionData {
+            payload: payload.into(),
+            sidecar: ExecutionPayloadSidecar::v4(
+                CancunPayloadFields { versioned_hashes, parent_beacon_block_root },
+                PraguePayloadFields { requests: RequestsOrHash::Requests(execution_requests) },
+            ),
+        };
+
+        Ok(self.new_payload_v4_metered(payload).await?)
     }
 
     /// Handler for `engine_forkchoiceUpdatedV1`
@@ -1147,9 +1142,13 @@ mod tests {
         let (mut handle, api) = setup_engine_api();
 
         tokio::spawn(async move {
-            api.new_payload_v1(ExecutionPayloadV1::from_block_slow(&Block::default()))
-                .await
-                .unwrap();
+            let payload_v1 = ExecutionPayloadV1::from_block_slow(&Block::default());
+            let execution_data = ExecutionData {
+                payload: payload_v1.into(),
+                sidecar: ExecutionPayloadSidecar::none(),
+            };
+
+            api.new_payload_v1(execution_data).await.unwrap();
         });
         assert_matches!(handle.from_api.recv().await, Some(BeaconEngineMessage::NewPayload { .. }));
     }