Use Fork variants instead of version for JsonPayload types (#7909)

With Fulu, we increment the engine API version for `get_payload` but we do not also increment `new_payload`.
In Lighthouse, we have a tight coupling between these version numbers and the Fork variants.
For example, both `get_payload_v3` and `new_payload_v3` correspond to Deneb, `v4` to Electra, etc.

However this coupling breaks with Fulu where only `get_payload_v5` is related to Fulu and `new_payload_v4` now also corresponds to Fulu (new_payload_v5 does not exist). While we can work around this, it creates a confusing situation where the versions and Fork variants are out of sync.

See the following code snippet where we are using the v4 endpoint, and yet passing a `V5` payload variant: https://github.com/sigp/lighthouse/blob/522bd9e9c6ac167f2231525e937c9ebbcb86cf6e/beacon_node/execution_layer/src/engine_api/http.rs#L849-L870


  1. Remove `new_payload_v5` as it is unused in Fulu.
2. Rename the `JsonExecutionPayload` and `JsonGetExecutionPayloadResponse` types to use Fork variants instead of version variants. This clarifies the relationship between them.

Author: macladson

beacon_node/execution_layer/src/engine_api.rs

@@ -5,7 +5,7 @@ use crate::http::{
     ENGINE_GET_PAYLOAD_BODIES_BY_HASH_V1, ENGINE_GET_PAYLOAD_BODIES_BY_RANGE_V1,
     ENGINE_GET_PAYLOAD_V1, ENGINE_GET_PAYLOAD_V2, ENGINE_GET_PAYLOAD_V3, ENGINE_GET_PAYLOAD_V4,
     ENGINE_GET_PAYLOAD_V5, ENGINE_NEW_PAYLOAD_V1, ENGINE_NEW_PAYLOAD_V2, ENGINE_NEW_PAYLOAD_V3,
-    ENGINE_NEW_PAYLOAD_V4, ENGINE_NEW_PAYLOAD_V5,
+    ENGINE_NEW_PAYLOAD_V4,
 };
 use eth2::types::{
     BlobsBundle, SsePayloadAttributes, SsePayloadAttributesV1, SsePayloadAttributesV2,
@@ -541,7 +541,6 @@ pub struct EngineCapabilities {
     pub new_payload_v2: bool,
     pub new_payload_v3: bool,
     pub new_payload_v4: bool,
-    pub new_payload_v5: bool,
     pub forkchoice_updated_v1: bool,
     pub forkchoice_updated_v2: bool,
     pub forkchoice_updated_v3: bool,
@@ -572,9 +571,6 @@ impl EngineCapabilities {
         if self.new_payload_v4 {
             response.push(ENGINE_NEW_PAYLOAD_V4);
         }
-        if self.new_payload_v5 {
-            response.push(ENGINE_NEW_PAYLOAD_V5);
-        }
         if self.forkchoice_updated_v1 {
             response.push(ENGINE_FORKCHOICE_UPDATED_V1);
         }

beacon_node/execution_layer/src/engine_api/http.rs

@@ -35,7 +35,6 @@ pub const ENGINE_NEW_PAYLOAD_V1: &str = "engine_newPayloadV1";
 pub const ENGINE_NEW_PAYLOAD_V2: &str = "engine_newPayloadV2";
 pub const ENGINE_NEW_PAYLOAD_V3: &str = "engine_newPayloadV3";
 pub const ENGINE_NEW_PAYLOAD_V4: &str = "engine_newPayloadV4";
-pub const ENGINE_NEW_PAYLOAD_V5: &str = "engine_newPayloadV5";
 pub const ENGINE_NEW_PAYLOAD_TIMEOUT: Duration = Duration::from_secs(8);
 
 pub const ENGINE_GET_PAYLOAD_V1: &str = "engine_getPayloadV1";
@@ -75,7 +74,6 @@ pub static LIGHTHOUSE_CAPABILITIES: &[&str] = &[
     ENGINE_NEW_PAYLOAD_V2,
     ENGINE_NEW_PAYLOAD_V3,
     ENGINE_NEW_PAYLOAD_V4,
-    ENGINE_NEW_PAYLOAD_V5,
     ENGINE_GET_PAYLOAD_V1,
     ENGINE_GET_PAYLOAD_V2,
     ENGINE_GET_PAYLOAD_V3,
@@ -805,7 +803,7 @@ impl HttpJsonRpc {
         new_payload_request_deneb: NewPayloadRequestDeneb<'_, E>,
     ) -> Result<PayloadStatusV1, Error> {
         let params = json!([
-            JsonExecutionPayload::V3(new_payload_request_deneb.execution_payload.clone().into()),
+            JsonExecutionPayload::Deneb(new_payload_request_deneb.execution_payload.clone().into()),
             new_payload_request_deneb.versioned_hashes,
             new_payload_request_deneb.parent_beacon_block_root,
         ]);
@@ -826,7 +824,9 @@ impl HttpJsonRpc {
         new_payload_request_electra: NewPayloadRequestElectra<'_, E>,
     ) -> Result<PayloadStatusV1, Error> {
         let params = json!([
-            JsonExecutionPayload::V4(new_payload_request_electra.execution_payload.clone().into()),
+            JsonExecutionPayload::Electra(
+                new_payload_request_electra.execution_payload.clone().into()
+            ),
             new_payload_request_electra.versioned_hashes,
             new_payload_request_electra.parent_beacon_block_root,
             new_payload_request_electra
@@ -850,7 +850,7 @@ impl HttpJsonRpc {
         new_payload_request_fulu: NewPayloadRequestFulu<'_, E>,
     ) -> Result<PayloadStatusV1, Error> {
         let params = json!([
-            JsonExecutionPayload::V5(new_payload_request_fulu.execution_payload.clone().into()),
+            JsonExecutionPayload::Fulu(new_payload_request_fulu.execution_payload.clone().into()),
             new_payload_request_fulu.versioned_hashes,
             new_payload_request_fulu.parent_beacon_block_root,
             new_payload_request_fulu
@@ -875,7 +875,7 @@ impl HttpJsonRpc {
     ) -> Result<GetPayloadResponse<E>, Error> {
         let params = json!([JsonPayloadIdRequest::from(payload_id)]);
 
-        let payload_v1: JsonExecutionPayloadV1<E> = self
+        let payload_v1: JsonExecutionPayloadBellatrix<E> = self
             .rpc_request(
                 ENGINE_GET_PAYLOAD_V1,
                 params,
@@ -901,26 +901,26 @@ impl HttpJsonRpc {
 
         match fork_name {
             ForkName::Bellatrix => {
-                let response: JsonGetPayloadResponseV1<E> = self
+                let response: JsonGetPayloadResponseBellatrix<E> = self
                     .rpc_request(
                         ENGINE_GET_PAYLOAD_V2,
                         params,
                         ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier,
                     )
                     .await?;
-                JsonGetPayloadResponse::V1(response)
+                JsonGetPayloadResponse::Bellatrix(response)
                     .try_into()
                     .map_err(Error::BadResponse)
             }
             ForkName::Capella => {
-                let response: JsonGetPayloadResponseV2<E> = self
+                let response: JsonGetPayloadResponseCapella<E> = self
                     .rpc_request(
                         ENGINE_GET_PAYLOAD_V2,
                         params,
                         ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier,
                     )
                     .await?;
-                JsonGetPayloadResponse::V2(response)
+                JsonGetPayloadResponse::Capella(response)
                     .try_into()
                     .map_err(Error::BadResponse)
             }
@@ -940,14 +940,14 @@ impl HttpJsonRpc {
 
         match fork_name {
             ForkName::Deneb => {
-                let response: JsonGetPayloadResponseV3<E> = self
+                let response: JsonGetPayloadResponseDeneb<E> = self
                     .rpc_request(
                         ENGINE_GET_PAYLOAD_V3,
                         params,
                         ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier,
                     )
                     .await?;
-                JsonGetPayloadResponse::V3(response)
+                JsonGetPayloadResponse::Deneb(response)
                     .try_into()
                     .map_err(Error::BadResponse)
             }
@@ -967,14 +967,14 @@ impl HttpJsonRpc {
 
         match fork_name {
             ForkName::Electra => {
-                let response: JsonGetPayloadResponseV4<E> = self
+                let response: JsonGetPayloadResponseElectra<E> = self
                     .rpc_request(
                         ENGINE_GET_PAYLOAD_V4,
                         params,
                         ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier,
                     )
                     .await?;
-                JsonGetPayloadResponse::V4(response)
+                JsonGetPayloadResponse::Electra(response)
                     .try_into()
                     .map_err(Error::BadResponse)
             }
@@ -994,14 +994,14 @@ impl HttpJsonRpc {
 
         match fork_name {
             ForkName::Fulu => {
-                let response: JsonGetPayloadResponseV5<E> = self
+                let response: JsonGetPayloadResponseFulu<E> = self
                     .rpc_request(
                         ENGINE_GET_PAYLOAD_V5,
                         params,
                         ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier,
                     )
                     .await?;
-                JsonGetPayloadResponse::V5(response)
+                JsonGetPayloadResponse::Fulu(response)
                     .try_into()
                     .map_err(Error::BadResponse)
             }
@@ -1135,7 +1135,6 @@ impl HttpJsonRpc {
             new_payload_v2: capabilities.contains(ENGINE_NEW_PAYLOAD_V2),
             new_payload_v3: capabilities.contains(ENGINE_NEW_PAYLOAD_V3),
             new_payload_v4: capabilities.contains(ENGINE_NEW_PAYLOAD_V4),
-            new_payload_v5: capabilities.contains(ENGINE_NEW_PAYLOAD_V5),
             forkchoice_updated_v1: capabilities.contains(ENGINE_FORKCHOICE_UPDATED_V1),
             forkchoice_updated_v2: capabilities.contains(ENGINE_FORKCHOICE_UPDATED_V2),
             forkchoice_updated_v3: capabilities.contains(ENGINE_FORKCHOICE_UPDATED_V3),
@@ -1501,10 +1500,11 @@ mod test {
     fn encode_transactions<E: EthSpec>(
         transactions: Transactions<E>,
     ) -> Result<serde_json::Value, serde_json::Error> {
-        let ep: JsonExecutionPayload<E> = JsonExecutionPayload::V1(JsonExecutionPayloadV1 {
-            transactions,
-            ..<_>::default()
-        });
+        let ep: JsonExecutionPayload<E> =
+            JsonExecutionPayload::Bellatrix(JsonExecutionPayloadBellatrix {
+                transactions,
+                ..<_>::default()
+            });
         let json = serde_json::to_value(ep)?;
         Ok(json.get("transactions").unwrap().clone())
     }