diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 2d8427e30..54d773447 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -1007,9 +1007,7 @@ async fn payload_preparation() { .unwrap(), fee_recipient, None, - ) - .downgrade_to_v1() - .unwrap(); + ); assert_eq!(rig.previous_payload_attributes(), payload_attributes); } diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 4970361a5..21ca2b299 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -12,6 +12,7 @@ pub use types::{ Address, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadHeader, FixedVector, ForkName, Hash256, Uint256, VariableList, Withdrawal, }; +use types::{ExecutionPayloadCapella, ExecutionPayloadEip4844, ExecutionPayloadMerge}; pub mod auth; pub mod http; @@ -267,7 +268,7 @@ pub struct PayloadAttributes { #[superstruct(getter(copy))] pub suggested_fee_recipient: Address, #[superstruct(only(V2))] - pub withdrawals: Option>, + pub withdrawals: Vec, } impl PayloadAttributes { @@ -277,31 +278,18 @@ impl PayloadAttributes { suggested_fee_recipient: Address, withdrawals: Option>, ) -> Self { - // this should always return the highest version - PayloadAttributes::V2(PayloadAttributesV2 { - timestamp, - prev_randao, - suggested_fee_recipient, - withdrawals, - }) - } - - pub fn downgrade_to_v1(self) -> Result { - match self { - PayloadAttributes::V1(_) => Ok(self), - PayloadAttributes::V2(v2) => { - if v2.withdrawals.is_some() { - return Err(Error::BadConversion( - "Downgrading from PayloadAttributesV2 with non-null withdrawals" - .to_string(), - )); - } - Ok(PayloadAttributes::V1(PayloadAttributesV1 { - timestamp: v2.timestamp, - prev_randao: v2.prev_randao, - suggested_fee_recipient: v2.suggested_fee_recipient, - })) - } + match withdrawals { + Some(withdrawals) => PayloadAttributes::V2(PayloadAttributesV2 { + timestamp, + prev_randao, + suggested_fee_recipient, + withdrawals, + }), + None => PayloadAttributes::V1(PayloadAttributesV1 { + timestamp, + prev_randao, + suggested_fee_recipient, + }), } } } @@ -326,6 +314,39 @@ pub struct ProposeBlindedBlockResponse { pub validation_error: Option, } +#[superstruct( + variants(Merge, Capella, Eip4844), + variant_attributes(derive(Clone, Debug, PartialEq),), + cast_error(ty = "Error", expr = "Error::IncorrectStateVariant"), + partial_getter_error(ty = "Error", expr = "Error::IncorrectStateVariant") +)] +#[derive(Clone, Debug, PartialEq)] +pub struct GetPayloadResponse { + #[superstruct(only(Merge), partial_getter(rename = "execution_payload_merge"))] + pub execution_payload: ExecutionPayloadMerge, + #[superstruct(only(Capella), partial_getter(rename = "execution_payload_capella"))] + pub execution_payload: ExecutionPayloadCapella, + #[superstruct(only(Eip4844), partial_getter(rename = "execution_payload_eip4844"))] + pub execution_payload: ExecutionPayloadEip4844, + pub block_value: Uint256, +} + +impl GetPayloadResponse { + pub fn execution_payload(self) -> ExecutionPayload { + match self { + GetPayloadResponse::Merge(response) => { + ExecutionPayload::Merge(response.execution_payload) + } + GetPayloadResponse::Capella(response) => { + ExecutionPayload::Capella(response.execution_payload) + } + GetPayloadResponse::Eip4844(response) => { + ExecutionPayload::Eip4844(response.execution_payload) + } + } + } +} + // This name is work in progress, it could // change when this method is actually proposed // but I'm writing this as it has been described @@ -333,9 +354,11 @@ pub struct ProposeBlindedBlockResponse { pub struct SupportedApis { pub new_payload_v1: bool, pub new_payload_v2: bool, + pub new_payload_v3: bool, pub forkchoice_updated_v1: bool, pub forkchoice_updated_v2: bool, pub get_payload_v1: bool, pub get_payload_v2: bool, + pub get_payload_v3: bool, pub exchange_transition_configuration_v1: bool, } diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index dd06e2953..90f8f053d 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -548,11 +548,13 @@ impl HttpJsonRpc { let cached_supported_apis = RwLock::new(Some(SupportedApis { new_payload_v1: true, new_payload_v2: spec.capella_fork_epoch.is_some() || spec.eip4844_fork_epoch.is_some(), + new_payload_v3: spec.eip4844_fork_epoch.is_some(), forkchoice_updated_v1: true, forkchoice_updated_v2: spec.capella_fork_epoch.is_some() || spec.eip4844_fork_epoch.is_some(), get_payload_v1: true, get_payload_v2: spec.capella_fork_epoch.is_some() || spec.eip4844_fork_epoch.is_some(), + get_payload_v3: spec.eip4844_fork_epoch.is_some(), exchange_transition_configuration_v1: true, })); @@ -577,11 +579,13 @@ impl HttpJsonRpc { let cached_supported_apis = RwLock::new(Some(SupportedApis { new_payload_v1: true, new_payload_v2: spec.capella_fork_epoch.is_some() || spec.eip4844_fork_epoch.is_some(), + new_payload_v3: spec.eip4844_fork_epoch.is_some(), forkchoice_updated_v1: true, forkchoice_updated_v2: spec.capella_fork_epoch.is_some() || spec.eip4844_fork_epoch.is_some(), get_payload_v1: true, get_payload_v2: spec.capella_fork_epoch.is_some() || spec.eip4844_fork_epoch.is_some(), + get_payload_v3: spec.eip4844_fork_epoch.is_some(), exchange_transition_configuration_v1: true, })); @@ -737,7 +741,7 @@ impl HttpJsonRpc { &self, execution_payload: ExecutionPayload, ) -> Result { - let params = json!([JsonExecutionPayloadV1::try_from(execution_payload)?]); + let params = json!([JsonExecutionPayload::from(execution_payload)]); let response: JsonPayloadStatusV1 = self .rpc_request( @@ -754,7 +758,7 @@ impl HttpJsonRpc { &self, execution_payload: ExecutionPayload, ) -> Result { - let params = json!([JsonExecutionPayloadV2::try_from(execution_payload)?]); + let params = json!([JsonExecutionPayload::from(execution_payload)]); let response: JsonPayloadStatusV1 = self .rpc_request( @@ -771,7 +775,7 @@ impl HttpJsonRpc { &self, execution_payload: ExecutionPayload, ) -> Result { - let params = json!([JsonExecutionPayloadV2::try_from(execution_payload)?]); + let params = json!([JsonExecutionPayload::from(execution_payload)]); let response: JsonPayloadStatusV1 = self .rpc_request( @@ -786,7 +790,6 @@ impl HttpJsonRpc { pub async fn get_payload_v1( &self, - fork_name: ForkName, payload_id: PayloadId, ) -> Result, Error> { let params = json!([JsonPayloadIdRequest::from(payload_id)]); @@ -799,43 +802,86 @@ impl HttpJsonRpc { ) .await?; - JsonExecutionPayload::V1(payload_v1).try_into_execution_payload(fork_name) + Ok(JsonExecutionPayload::V1(payload_v1).into()) } pub async fn get_payload_v2( &self, fork_name: ForkName, payload_id: PayloadId, - ) -> Result, Error> { + ) -> Result, Error> { let params = json!([JsonPayloadIdRequest::from(payload_id)]); - let response: JsonGetPayloadResponse = self - .rpc_request( - ENGINE_GET_PAYLOAD_V2, - params, - ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, - ) - .await?; - - JsonExecutionPayload::V2(response.execution_payload).try_into_execution_payload(fork_name) + match fork_name { + ForkName::Merge => { + let response: JsonGetPayloadResponseV1 = self + .rpc_request( + ENGINE_GET_PAYLOAD_V2, + params, + ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, + ) + .await?; + Ok(JsonGetPayloadResponse::V1(response).into()) + } + ForkName::Capella => { + let response: JsonGetPayloadResponseV2 = self + .rpc_request( + ENGINE_GET_PAYLOAD_V2, + params, + ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, + ) + .await?; + Ok(JsonGetPayloadResponse::V2(response).into()) + } + ForkName::Base | ForkName::Altair | ForkName::Eip4844 => Err( + Error::UnsupportedForkVariant(format!("called get_payload_v2 with {}", fork_name)), + ), + } } pub async fn get_payload_v3( &self, fork_name: ForkName, payload_id: PayloadId, - ) -> Result, Error> { + ) -> Result, Error> { let params = json!([JsonPayloadIdRequest::from(payload_id)]); - let payload_v2: JsonExecutionPayloadV2 = self - .rpc_request( - ENGINE_GET_PAYLOAD_V3, - params, - ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, - ) - .await?; - - JsonExecutionPayload::V2(payload_v2).try_into_execution_payload(fork_name) + match fork_name { + ForkName::Merge => { + let response: JsonGetPayloadResponseV1 = self + .rpc_request( + ENGINE_GET_PAYLOAD_V2, + params, + ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, + ) + .await?; + Ok(JsonGetPayloadResponse::V1(response).into()) + } + ForkName::Capella => { + let response: JsonGetPayloadResponseV2 = self + .rpc_request( + ENGINE_GET_PAYLOAD_V2, + params, + ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, + ) + .await?; + Ok(JsonGetPayloadResponse::V2(response).into()) + } + ForkName::Eip4844 => { + let response: JsonGetPayloadResponseV3 = self + .rpc_request( + ENGINE_GET_PAYLOAD_V3, + params, + ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, + ) + .await?; + Ok(JsonGetPayloadResponse::V3(response).into()) + } + ForkName::Base | ForkName::Altair => Err(Error::UnsupportedForkVariant(format!( + "called get_payload_v3 with {}", + fork_name + ))), + } } pub async fn get_blobs_bundle_v1( @@ -924,10 +970,12 @@ impl HttpJsonRpc { Ok(SupportedApis { new_payload_v1: true, new_payload_v2: true, + new_payload_v3: true, forkchoice_updated_v1: true, forkchoice_updated_v2: true, get_payload_v1: true, get_payload_v2: true, + get_payload_v3: true, exchange_transition_configuration_v1: true, }) } @@ -954,7 +1002,9 @@ impl HttpJsonRpc { execution_payload: ExecutionPayload, ) -> Result { let supported_apis = self.get_cached_supported_apis().await?; - if supported_apis.new_payload_v2 { + if supported_apis.new_payload_v3 { + self.new_payload_v3(execution_payload).await + } else if supported_apis.new_payload_v2 { self.new_payload_v2(execution_payload).await } else if supported_apis.new_payload_v1 { self.new_payload_v1(execution_payload).await @@ -971,10 +1021,21 @@ impl HttpJsonRpc { payload_id: PayloadId, ) -> Result, Error> { let supported_apis = self.get_cached_supported_apis().await?; - if supported_apis.get_payload_v2 { - self.get_payload_v2(fork_name, payload_id).await - } else if supported_apis.new_payload_v1 { - self.get_payload_v1(fork_name, payload_id).await + if supported_apis.get_payload_v3 { + Ok(self + .get_payload_v3(fork_name, payload_id) + .await? + .execution_payload()) + } else if supported_apis.get_payload_v2 { + // TODO: modify this method to return GetPayloadResponse instead + // of throwing away the `block_value` and returning only the + // ExecutionPayload + Ok(self + .get_payload_v2(fork_name, payload_id) + .await? + .execution_payload()) + } else if supported_apis.get_payload_v1 { + self.get_payload_v1(payload_id).await } else { Err(Error::RequiredMethodUnsupported("engine_getPayload")) } @@ -992,13 +1053,8 @@ impl HttpJsonRpc { self.forkchoice_updated_v2(forkchoice_state, payload_attributes) .await } else if supported_apis.forkchoice_updated_v1 { - self.forkchoice_updated_v1( - forkchoice_state, - payload_attributes - .map(|pa| pa.downgrade_to_v1()) - .transpose()?, - ) - .await + self.forkchoice_updated_v1(forkchoice_state, payload_attributes) + .await } else { Err(Error::RequiredMethodUnsupported("engine_forkchoiceUpdated")) } @@ -1013,9 +1069,7 @@ mod test { use std::future::Future; use std::str::FromStr; use std::sync::Arc; - use types::{ - ExecutionPayloadMerge, ForkName, MainnetEthSpec, Transactions, Unsigned, VariableList, - }; + use types::{ExecutionPayloadMerge, MainnetEthSpec, Transactions, Unsigned, VariableList}; struct Tester { server: MockServer, @@ -1355,9 +1409,7 @@ mod test { Tester::new(true) .assert_request_equals( |client| async move { - let _ = client - .get_payload_v1::(ForkName::Merge, [42; 8]) - .await; + let _ = client.get_payload_v1::([42; 8]).await; }, json!({ "id": STATIC_ID, @@ -1370,9 +1422,7 @@ mod test { Tester::new(false) .assert_auth_failure(|client| async move { - client - .get_payload_v1::(ForkName::Merge, [42; 8]) - .await + client.get_payload_v1::([42; 8]).await }) .await; } @@ -1601,7 +1651,7 @@ mod test { // engine_getPayloadV1 REQUEST validation |client| async move { let _ = client - .get_payload_v1::(ForkName::Merge,str_to_payload_id("0xa247243752eb10b4")) + .get_payload_v1::(str_to_payload_id("0xa247243752eb10b4")) .await; }, json!({ @@ -1636,7 +1686,7 @@ mod test { })], |client| async move { let payload = client - .get_payload_v1::(ForkName::Merge,str_to_payload_id("0xa247243752eb10b4")) + .get_payload_v1::(str_to_payload_id("0xa247243752eb10b4")) .await .unwrap(); diff --git a/beacon_node/execution_layer/src/engine_api/json_structures.rs b/beacon_node/execution_layer/src/engine_api/json_structures.rs index dbcdcfb61..a2297b1f2 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -64,7 +64,7 @@ pub struct JsonPayloadIdResponse { } #[superstruct( - variants(V1, V2), + variants(V1, V2, V3), variant_attributes( derive(Debug, PartialEq, Default, Serialize, Deserialize,), serde(bound = "T: EthSpec", rename_all = "camelCase"), @@ -94,233 +94,234 @@ pub struct JsonExecutionPayload { pub extra_data: VariableList, #[serde(with = "eth2_serde_utils::u256_hex_be")] pub base_fee_per_gas: Uint256, - #[superstruct(only(V2))] - #[serde(with = "eth2_serde_utils::u256_hex_be_opt")] - pub excess_data_gas: Option, + #[superstruct(only(V3))] + #[serde(with = "eth2_serde_utils::u256_hex_be")] + pub excess_data_gas: Uint256, pub block_hash: ExecutionBlockHash, #[serde(with = "ssz_types::serde_utils::list_of_hex_var_list")] pub transactions: VariableList, T::MaxTransactionsPerPayload>, - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - #[superstruct(only(V2))] - pub withdrawals: Option>, + #[superstruct(only(V2, V3))] + pub withdrawals: VariableList, } -impl JsonExecutionPayload { - pub fn try_into_execution_payload( - self, - fork_name: ForkName, - ) -> Result, Error> { - match self { - JsonExecutionPayload::V1(v1) => match fork_name { - ForkName::Merge => Ok(ExecutionPayload::Merge(ExecutionPayloadMerge { - parent_hash: v1.parent_hash, - fee_recipient: v1.fee_recipient, - state_root: v1.state_root, - receipts_root: v1.receipts_root, - logs_bloom: v1.logs_bloom, - prev_randao: v1.prev_randao, - block_number: v1.block_number, - gas_limit: v1.gas_limit, - gas_used: v1.gas_used, - timestamp: v1.timestamp, - extra_data: v1.extra_data, - base_fee_per_gas: v1.base_fee_per_gas, - block_hash: v1.block_hash, - transactions: v1.transactions, - })), - _ => Err(Error::UnsupportedForkVariant(format!("Unsupported conversion from JsonExecutionPayloadV1 for {}", fork_name))), - } - JsonExecutionPayload::V2(v2) => match fork_name { - ForkName::Merge => Ok(ExecutionPayload::Merge(ExecutionPayloadMerge { - parent_hash: v2.parent_hash, - fee_recipient: v2.fee_recipient, - state_root: v2.state_root, - receipts_root: v2.receipts_root, - logs_bloom: v2.logs_bloom, - prev_randao: v2.prev_randao, - block_number: v2.block_number, - gas_limit: v2.gas_limit, - gas_used: v2.gas_used, - timestamp: v2.timestamp, - extra_data: v2.extra_data, - base_fee_per_gas: v2.base_fee_per_gas, - block_hash: v2.block_hash, - transactions: v2.transactions, - })), - ForkName::Capella => Ok(ExecutionPayload::Capella(ExecutionPayloadCapella { - parent_hash: v2.parent_hash, - fee_recipient: v2.fee_recipient, - state_root: v2.state_root, - receipts_root: v2.receipts_root, - logs_bloom: v2.logs_bloom, - prev_randao: v2.prev_randao, - block_number: v2.block_number, - gas_limit: v2.gas_limit, - gas_used: v2.gas_used, - timestamp: v2.timestamp, - extra_data: v2.extra_data, - base_fee_per_gas: v2.base_fee_per_gas, - block_hash: v2.block_hash, - transactions: v2.transactions, - withdrawals: v2 - .withdrawals - .map(|v| { - Into::>::into(v) - .into_iter() - .map(Into::into) - .collect::>() - .into() - }) - .ok_or_else(|| Error::BadConversion("Null withdrawal field converting JsonExecutionPayloadV2 -> ExecutionPayloadCapella".to_string()))? - })), - ForkName::Eip4844 => Ok(ExecutionPayload::Eip4844(ExecutionPayloadEip4844 { - parent_hash: v2.parent_hash, - fee_recipient: v2.fee_recipient, - state_root: v2.state_root, - receipts_root: v2.receipts_root, - logs_bloom: v2.logs_bloom, - prev_randao: v2.prev_randao, - block_number: v2.block_number, - gas_limit: v2.gas_limit, - gas_used: v2.gas_used, - timestamp: v2.timestamp, - extra_data: v2.extra_data, - base_fee_per_gas: v2.base_fee_per_gas, - excess_data_gas: v2.excess_data_gas.ok_or_else(|| Error::BadConversion("Null `excess_data_gas` field converting JsonExecutionPayloadV2 -> ExecutionPayloadEip4844".to_string()))?, - block_hash: v2.block_hash, - transactions: v2.transactions, - withdrawals: v2 - .withdrawals - .map(|v| { - Into::>::into(v) - .into_iter() - .map(Into::into) - .collect::>() - .into() - }) - .ok_or_else(|| Error::BadConversion("Null withdrawal field converting JsonExecutionPayloadV2 -> ExecutionPayloadEip4844".to_string()))? - })), - _ => Err(Error::UnsupportedForkVariant(format!("Unsupported conversion from JsonExecutionPayloadV2 for {}", fork_name))), - } +impl From> for JsonExecutionPayloadV1 { + fn from(payload: ExecutionPayloadMerge) -> Self { + JsonExecutionPayloadV1 { + parent_hash: payload.parent_hash, + fee_recipient: payload.fee_recipient, + state_root: payload.state_root, + receipts_root: payload.receipts_root, + logs_bloom: payload.logs_bloom, + prev_randao: payload.prev_randao, + block_number: payload.block_number, + gas_limit: payload.gas_limit, + gas_used: payload.gas_used, + timestamp: payload.timestamp, + extra_data: payload.extra_data, + base_fee_per_gas: payload.base_fee_per_gas, + block_hash: payload.block_hash, + transactions: payload.transactions, + } + } +} +impl From> for JsonExecutionPayloadV2 { + fn from(payload: ExecutionPayloadCapella) -> Self { + JsonExecutionPayloadV2 { + parent_hash: payload.parent_hash, + fee_recipient: payload.fee_recipient, + state_root: payload.state_root, + receipts_root: payload.receipts_root, + logs_bloom: payload.logs_bloom, + prev_randao: payload.prev_randao, + block_number: payload.block_number, + gas_limit: payload.gas_limit, + gas_used: payload.gas_used, + timestamp: payload.timestamp, + extra_data: payload.extra_data, + base_fee_per_gas: payload.base_fee_per_gas, + block_hash: payload.block_hash, + transactions: payload.transactions, + withdrawals: payload + .withdrawals + .into_iter() + .cloned() + .map(Into::into) + .collect::>() + .into(), + } + } +} +impl From> for JsonExecutionPayloadV3 { + fn from(payload: ExecutionPayloadEip4844) -> Self { + JsonExecutionPayloadV3 { + parent_hash: payload.parent_hash, + fee_recipient: payload.fee_recipient, + state_root: payload.state_root, + receipts_root: payload.receipts_root, + logs_bloom: payload.logs_bloom, + prev_randao: payload.prev_randao, + block_number: payload.block_number, + gas_limit: payload.gas_limit, + gas_used: payload.gas_used, + timestamp: payload.timestamp, + extra_data: payload.extra_data, + base_fee_per_gas: payload.base_fee_per_gas, + excess_data_gas: payload.excess_data_gas, + block_hash: payload.block_hash, + transactions: payload.transactions, + withdrawals: payload + .withdrawals + .into_iter() + .cloned() + .map(Into::into) + .collect::>() + .into(), } } } -impl TryFrom> for JsonExecutionPayloadV1 { - type Error = Error; - fn try_from(payload: ExecutionPayload) -> Result { - match payload { - ExecutionPayload::Merge(merge) => Ok(JsonExecutionPayloadV1 { - parent_hash: merge.parent_hash, - fee_recipient: merge.fee_recipient, - state_root: merge.state_root, - receipts_root: merge.receipts_root, - logs_bloom: merge.logs_bloom, - prev_randao: merge.prev_randao, - block_number: merge.block_number, - gas_limit: merge.gas_limit, - gas_used: merge.gas_used, - timestamp: merge.timestamp, - extra_data: merge.extra_data, - base_fee_per_gas: merge.base_fee_per_gas, - block_hash: merge.block_hash, - transactions: merge.transactions, - }), - ExecutionPayload::Capella(_) => Err(Error::UnsupportedForkVariant(format!( - "Unsupported conversion to JsonExecutionPayloadV1 for {}", - ForkName::Capella - ))), - ExecutionPayload::Eip4844(_) => Err(Error::UnsupportedForkVariant(format!( - "Unsupported conversion to JsonExecutionPayloadV1 for {}", - ForkName::Eip4844 - ))), +impl From> for JsonExecutionPayload { + fn from(execution_payload: ExecutionPayload) -> Self { + match execution_payload { + ExecutionPayload::Merge(payload) => JsonExecutionPayload::V1(payload.into()), + ExecutionPayload::Capella(payload) => JsonExecutionPayload::V2(payload.into()), + ExecutionPayload::Eip4844(payload) => JsonExecutionPayload::V3(payload.into()), } } } -impl TryFrom> for JsonExecutionPayloadV2 { - type Error = Error; - fn try_from(payload: ExecutionPayload) -> Result { - match payload { - ExecutionPayload::Merge(merge) => Ok(JsonExecutionPayloadV2 { - parent_hash: merge.parent_hash, - fee_recipient: merge.fee_recipient, - state_root: merge.state_root, - receipts_root: merge.receipts_root, - logs_bloom: merge.logs_bloom, - prev_randao: merge.prev_randao, - block_number: merge.block_number, - gas_limit: merge.gas_limit, - gas_used: merge.gas_used, - timestamp: merge.timestamp, - extra_data: merge.extra_data, - base_fee_per_gas: merge.base_fee_per_gas, - excess_data_gas: None, - block_hash: merge.block_hash, - transactions: merge.transactions, - withdrawals: None, - }), - ExecutionPayload::Capella(capella) => Ok(JsonExecutionPayloadV2 { - parent_hash: capella.parent_hash, - fee_recipient: capella.fee_recipient, - state_root: capella.state_root, - receipts_root: capella.receipts_root, - logs_bloom: capella.logs_bloom, - prev_randao: capella.prev_randao, - block_number: capella.block_number, - gas_limit: capella.gas_limit, - gas_used: capella.gas_used, - timestamp: capella.timestamp, - extra_data: capella.extra_data, - base_fee_per_gas: capella.base_fee_per_gas, - excess_data_gas: None, - block_hash: capella.block_hash, - transactions: capella.transactions, - withdrawals: Some( - Vec::from(capella.withdrawals) - .into_iter() - .map(Into::into) - .collect::>() - .into(), - ), - }), - ExecutionPayload::Eip4844(eip4844) => Ok(JsonExecutionPayloadV2 { - parent_hash: eip4844.parent_hash, - fee_recipient: eip4844.fee_recipient, - state_root: eip4844.state_root, - receipts_root: eip4844.receipts_root, - logs_bloom: eip4844.logs_bloom, - prev_randao: eip4844.prev_randao, - block_number: eip4844.block_number, - gas_limit: eip4844.gas_limit, - gas_used: eip4844.gas_used, - timestamp: eip4844.timestamp, - extra_data: eip4844.extra_data, - base_fee_per_gas: eip4844.base_fee_per_gas, - excess_data_gas: Some(eip4844.excess_data_gas), - block_hash: eip4844.block_hash, - transactions: eip4844.transactions, - withdrawals: Some( - Vec::from(eip4844.withdrawals) - .into_iter() - .map(Into::into) - .collect::>() - .into(), - ), - }), +impl From> for ExecutionPayloadMerge { + fn from(payload: JsonExecutionPayloadV1) -> Self { + ExecutionPayloadMerge { + parent_hash: payload.parent_hash, + fee_recipient: payload.fee_recipient, + state_root: payload.state_root, + receipts_root: payload.receipts_root, + logs_bloom: payload.logs_bloom, + prev_randao: payload.prev_randao, + block_number: payload.block_number, + gas_limit: payload.gas_limit, + gas_used: payload.gas_used, + timestamp: payload.timestamp, + extra_data: payload.extra_data, + base_fee_per_gas: payload.base_fee_per_gas, + block_hash: payload.block_hash, + transactions: payload.transactions, + } + } +} +impl From> for ExecutionPayloadCapella { + fn from(payload: JsonExecutionPayloadV2) -> Self { + ExecutionPayloadCapella { + parent_hash: payload.parent_hash, + fee_recipient: payload.fee_recipient, + state_root: payload.state_root, + receipts_root: payload.receipts_root, + logs_bloom: payload.logs_bloom, + prev_randao: payload.prev_randao, + block_number: payload.block_number, + gas_limit: payload.gas_limit, + gas_used: payload.gas_used, + timestamp: payload.timestamp, + extra_data: payload.extra_data, + base_fee_per_gas: payload.base_fee_per_gas, + block_hash: payload.block_hash, + transactions: payload.transactions, + withdrawals: payload + .withdrawals + .into_iter() + .cloned() + .map(Into::into) + .collect::>() + .into(), + } + } +} +impl From> for ExecutionPayloadEip4844 { + fn from(payload: JsonExecutionPayloadV3) -> Self { + ExecutionPayloadEip4844 { + parent_hash: payload.parent_hash, + fee_recipient: payload.fee_recipient, + state_root: payload.state_root, + receipts_root: payload.receipts_root, + logs_bloom: payload.logs_bloom, + prev_randao: payload.prev_randao, + block_number: payload.block_number, + gas_limit: payload.gas_limit, + gas_used: payload.gas_used, + timestamp: payload.timestamp, + extra_data: payload.extra_data, + base_fee_per_gas: payload.base_fee_per_gas, + excess_data_gas: payload.excess_data_gas, + block_hash: payload.block_hash, + transactions: payload.transactions, + withdrawals: payload + .withdrawals + .into_iter() + .cloned() + .map(Into::into) + .collect::>() + .into(), } } } +impl From> for ExecutionPayload { + fn from(json_execution_payload: JsonExecutionPayload) -> Self { + match json_execution_payload { + JsonExecutionPayload::V1(payload) => ExecutionPayload::Merge(payload.into()), + JsonExecutionPayload::V2(payload) => ExecutionPayload::Capella(payload.into()), + JsonExecutionPayload::V3(payload) => ExecutionPayload::Eip4844(payload.into()), + } + } +} + +#[superstruct( + variants(V1, V2, V3), + variant_attributes( + derive(Debug, PartialEq, Serialize, Deserialize), + serde(bound = "T: EthSpec", rename_all = "camelCase") + ), + cast_error(ty = "Error", expr = "Error::IncorrectStateVariant"), + partial_getter_error(ty = "Error", expr = "Error::IncorrectStateVariant") +)] #[derive(Debug, PartialEq, Serialize, Deserialize)] -#[serde(bound = "T: EthSpec", rename_all = "camelCase")] +#[serde(untagged)] pub struct JsonGetPayloadResponse { + #[superstruct(only(V1), partial_getter(rename = "execution_payload_v1"))] + pub execution_payload: JsonExecutionPayloadV1, + #[superstruct(only(V2), partial_getter(rename = "execution_payload_v2"))] pub execution_payload: JsonExecutionPayloadV2, - // uncomment this when geth fixes its serialization - //#[serde(with = "eth2_serde_utils::u256_hex_be")] - //pub block_value: Uint256, + #[superstruct(only(V3), partial_getter(rename = "execution_payload_v3"))] + pub execution_payload: JsonExecutionPayloadV3, + #[serde(with = "eth2_serde_utils::u256_hex_be")] + pub block_value: Uint256, +} + +impl From> for GetPayloadResponse { + fn from(json_get_payload_response: JsonGetPayloadResponse) -> Self { + match json_get_payload_response { + JsonGetPayloadResponse::V1(response) => { + GetPayloadResponse::Merge(GetPayloadResponseMerge { + execution_payload: response.execution_payload.into(), + block_value: response.block_value, + }) + } + JsonGetPayloadResponse::V2(response) => { + GetPayloadResponse::Capella(GetPayloadResponseCapella { + execution_payload: response.execution_payload.into(), + block_value: response.block_value, + }) + } + JsonGetPayloadResponse::V3(response) => { + GetPayloadResponse::Eip4844(GetPayloadResponseEip4844 { + execution_payload: response.execution_payload.into(), + block_value: response.block_value, + }) + } + } + } } #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] @@ -374,9 +375,7 @@ pub struct JsonPayloadAttributes { pub prev_randao: Hash256, pub suggested_fee_recipient: Address, #[superstruct(only(V2))] - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub withdrawals: Option>, + pub withdrawals: Vec, } impl From for JsonPayloadAttributes { @@ -391,9 +390,7 @@ impl From for JsonPayloadAttributes { timestamp: pa.timestamp, prev_randao: pa.prev_randao, suggested_fee_recipient: pa.suggested_fee_recipient, - withdrawals: pa - .withdrawals - .map(|w| w.into_iter().map(Into::into).collect()), + withdrawals: pa.withdrawals.into_iter().map(Into::into).collect(), }), } } @@ -411,9 +408,7 @@ impl From for PayloadAttributes { timestamp: jpa.timestamp, prev_randao: jpa.prev_randao, suggested_fee_recipient: jpa.suggested_fee_recipient, - withdrawals: jpa - .withdrawals - .map(|jw| jw.into_iter().map(Into::into).collect()), + withdrawals: jpa.withdrawals.into_iter().map(Into::into).collect(), }), } } diff --git a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs index 7790dcbed..63893375d 100644 --- a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs +++ b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs @@ -524,7 +524,7 @@ impl ExecutionBlockGenerator { base_fee_per_gas: Uint256::one(), block_hash: ExecutionBlockHash::zero(), transactions: vec![].into(), - withdrawals: pa.withdrawals.as_ref().unwrap().clone().into(), + withdrawals: pa.withdrawals.clone().into(), }) } ForkName::Eip4844 => { @@ -545,7 +545,7 @@ impl ExecutionBlockGenerator { excess_data_gas: Uint256::one(), block_hash: ExecutionBlockHash::zero(), transactions: vec![].into(), - withdrawals: pa.withdrawals.as_ref().unwrap().clone().into(), + withdrawals: pa.withdrawals.clone().into(), }) } _ => unreachable!(), diff --git a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs index a25b9e5ab..a0ce44450 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -79,12 +79,22 @@ pub async fn handle_rpc( ENGINE_NEW_PAYLOAD_V1 => { JsonExecutionPayload::V1(get_param::>(params, 0)?) } - ENGINE_NEW_PAYLOAD_V2 => { - JsonExecutionPayload::V2(get_param::>(params, 0)?) - } - ENGINE_NEW_PAYLOAD_V3 => { - JsonExecutionPayload::V2(get_param::>(params, 0)?) - } + ENGINE_NEW_PAYLOAD_V2 => get_param::>(params, 0) + .map(|jep| JsonExecutionPayload::V2(jep)) + .or_else(|_| { + get_param::>(params, 0) + .map(|jep| JsonExecutionPayload::V1(jep)) + })?, + ENGINE_NEW_PAYLOAD_V3 => get_param::>(params, 0) + .map(|jep| JsonExecutionPayload::V3(jep)) + .or_else(|_| { + get_param::>(params, 0) + .map(|jep| JsonExecutionPayload::V2(jep)) + .or_else(|_| { + get_param::>(params, 0) + .map(|jep| JsonExecutionPayload::V1(jep)) + }) + })?, _ => unreachable!(), }; @@ -95,9 +105,9 @@ pub async fn handle_rpc( // validate method called correctly according to shanghai fork time match fork { ForkName::Merge => { - if request.withdrawals().is_ok() && request.withdrawals().unwrap().is_some() { + if matches!(request, JsonExecutionPayload::V2(_)) { return Err(format!( - "{} called with `withdrawals` before capella fork!", + "{} called with `ExecutionPayloadV2` before capella fork!", method )); } @@ -106,36 +116,26 @@ pub async fn handle_rpc( if method == ENGINE_NEW_PAYLOAD_V1 { return Err(format!("{} called after capella fork!", method)); } - if request.withdrawals().is_err() - || (request.withdrawals().is_ok() - && request.withdrawals().unwrap().is_none()) - { + if matches!(request, JsonExecutionPayload::V1(_)) { return Err(format!( - "{} called without `withdrawals` after capella fork!", + "{} called with `ExecutionPayloadV1` after capella fork!", method )); } } ForkName::Eip4844 => { - //FIXME(sean) - if method == ENGINE_NEW_PAYLOAD_V1 { + if method == ENGINE_NEW_PAYLOAD_V1 || method == ENGINE_NEW_PAYLOAD_V2 { return Err(format!("{} called after capella fork!", method)); } - if request.withdrawals().is_err() - || (request.withdrawals().is_ok() - && request.withdrawals().unwrap().is_none()) - { + if matches!(request, JsonExecutionPayload::V1(_)) { return Err(format!( - "{} called without `withdrawals` after eip4844 fork!", + "{} called with `ExecutionPayloadV1` after eip4844 fork!", method )); } - if request.excess_data_gas().is_err() - || (request.excess_data_gas().is_ok() - && request.excess_data_gas().unwrap().is_none()) - { + if matches!(request, JsonExecutionPayload::V2(_)) { return Err(format!( - "{} called without `excess_data_gas` after eip4844 fork!", + "{} called with `ExecutionPayloadV2` after eip4844 fork!", method )); } @@ -163,7 +163,7 @@ pub async fn handle_rpc( Some( ctx.execution_block_generator .write() - .new_payload(request.try_into_execution_payload(fork).unwrap()), + .new_payload(request.into()), ) } else { None @@ -199,21 +199,55 @@ pub async fn handle_rpc( .read() .get_fork_at_timestamp(response.timestamp()) == ForkName::Eip4844 - //FIXME(sean) - && method == ENGINE_GET_PAYLOAD_V1 + && (method == ENGINE_GET_PAYLOAD_V1 || method == ENGINE_GET_PAYLOAD_V2) { - return Err(format!("{} called after capella fork!", method)); + return Err(format!("{} called after eip4844 fork!", method)); } match method { - ENGINE_GET_PAYLOAD_V1 => Ok(serde_json::to_value( - JsonExecutionPayloadV1::try_from(response).unwrap(), - ) - .unwrap()), - ENGINE_GET_PAYLOAD_V2 => Ok(serde_json::to_value(JsonGetPayloadResponse { - execution_payload: JsonExecutionPayloadV2::try_from(response).unwrap(), - }) - .unwrap()), + ENGINE_GET_PAYLOAD_V1 => { + Ok(serde_json::to_value(JsonExecutionPayload::from(response)).unwrap()) + } + ENGINE_GET_PAYLOAD_V2 => Ok(match JsonExecutionPayload::from(response) { + JsonExecutionPayload::V1(execution_payload) => { + serde_json::to_value(JsonGetPayloadResponseV1 { + execution_payload, + block_value: 0.into(), + }) + .unwrap() + } + JsonExecutionPayload::V2(execution_payload) => { + serde_json::to_value(JsonGetPayloadResponseV2 { + execution_payload, + block_value: 0.into(), + }) + .unwrap() + } + _ => unreachable!(), + }), + ENGINE_GET_PAYLOAD_V3 => Ok(match JsonExecutionPayload::from(response) { + JsonExecutionPayload::V1(execution_payload) => { + serde_json::to_value(JsonGetPayloadResponseV1 { + execution_payload, + block_value: 0.into(), + }) + .unwrap() + } + JsonExecutionPayload::V2(execution_payload) => { + serde_json::to_value(JsonGetPayloadResponseV2 { + execution_payload, + block_value: 0.into(), + }) + .unwrap() + } + JsonExecutionPayload::V3(execution_payload) => { + serde_json::to_value(JsonGetPayloadResponseV3 { + execution_payload, + block_value: 0.into(), + }) + .unwrap() + } + }), _ => unreachable!(), } } @@ -225,8 +259,31 @@ pub async fn handle_rpc( jpa1.map(JsonPayloadAttributes::V1) } ENGINE_FORKCHOICE_UPDATED_V2 => { - let jpa2: Option = get_param(params, 1)?; - jpa2.map(JsonPayloadAttributes::V2) + // we can't use `deny_unknown_fields` without breaking compatibility with some + // clients that haven't updated to the latest engine_api spec. So instead we'll + // need to deserialize based on timestamp + get_param::>(params, 1).and_then(|pa| { + pa.and_then(|pa| { + match ctx + .execution_block_generator + .read() + .get_fork_at_timestamp(*pa.timestamp()) + { + ForkName::Merge => { + get_param::>(params, 1) + .map(|opt| opt.map(JsonPayloadAttributes::V1)) + .transpose() + } + ForkName::Capella | ForkName::Eip4844 => { + get_param::>(params, 1) + .map(|opt| opt.map(JsonPayloadAttributes::V2)) + .transpose() + } + _ => unreachable!(), + } + }) + .transpose() + })? } _ => unreachable!(), }; @@ -239,36 +296,20 @@ pub async fn handle_rpc( .get_fork_at_timestamp(*pa.timestamp()) { ForkName::Merge => { - if pa.withdrawals().is_ok() && pa.withdrawals().unwrap().is_some() { + if matches!(pa, JsonPayloadAttributes::V2(_)) { return Err(format!( - "{} called with `withdrawals` before capella fork!", + "{} called with `JsonPayloadAttributesV2` before capella fork!", method )); } } - ForkName::Capella => { + ForkName::Capella | ForkName::Eip4844 => { if method == ENGINE_FORKCHOICE_UPDATED_V1 { return Err(format!("{} called after capella fork!", method)); } - if pa.withdrawals().is_err() - || (pa.withdrawals().is_ok() && pa.withdrawals().unwrap().is_none()) - { + if matches!(pa, JsonPayloadAttributes::V1(_)) { return Err(format!( - "{} called without `withdrawals` after capella fork!", - method - )); - } - } - ForkName::Eip4844 => { - //FIXME(sean) - if method == ENGINE_FORKCHOICE_UPDATED_V1 { - return Err(format!("{} called after capella fork!", method)); - } - if pa.withdrawals().is_err() - || (pa.withdrawals().is_ok() && pa.withdrawals().unwrap().is_none()) - { - return Err(format!( - "{} called without `withdrawals` after capella fork!", + "{} called with `JsonPayloadAttributesV1` after capella fork!", method )); }