From 42a499373fe1bc72a2c5885652827c6c00f7a135 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Mon, 6 May 2024 15:48:46 -0700 Subject: [PATCH 1/7] Add new engine api methods --- beacon_node/execution_layer/src/engine_api.rs | 11 +- .../execution_layer/src/engine_api/http.rs | 52 +++++++-- .../src/engine_api/json_structures.rs | 102 +++++++++++++++++- .../execution_layer/src/test_utils/mod.rs | 2 + 4 files changed, 155 insertions(+), 12 deletions(-) diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index ce1e0fec5d..6f6ec8b894 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -3,7 +3,8 @@ use crate::http::{ ENGINE_FORKCHOICE_UPDATED_V1, ENGINE_FORKCHOICE_UPDATED_V2, ENGINE_FORKCHOICE_UPDATED_V3, ENGINE_GET_CLIENT_VERSION_V1, 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_NEW_PAYLOAD_V1, ENGINE_NEW_PAYLOAD_V2, ENGINE_NEW_PAYLOAD_V3, + ENGINE_GET_PAYLOAD_V3, ENGINE_GET_PAYLOAD_V4, ENGINE_NEW_PAYLOAD_V1, ENGINE_NEW_PAYLOAD_V2, + ENGINE_NEW_PAYLOAD_V3, ENGINE_NEW_PAYLOAD_V4, }; use eth2::types::{ BlobsBundle, SsePayloadAttributes, SsePayloadAttributesV1, SsePayloadAttributesV2, @@ -652,6 +653,7 @@ pub struct EngineCapabilities { pub new_payload_v1: bool, pub new_payload_v2: bool, pub new_payload_v3: bool, + pub new_payload_v4: bool, pub forkchoice_updated_v1: bool, pub forkchoice_updated_v2: bool, pub forkchoice_updated_v3: bool, @@ -660,6 +662,7 @@ pub struct EngineCapabilities { pub get_payload_v1: bool, pub get_payload_v2: bool, pub get_payload_v3: bool, + pub get_payload_v4: bool, pub get_client_version_v1: bool, } @@ -675,6 +678,9 @@ impl EngineCapabilities { if self.new_payload_v3 { response.push(ENGINE_NEW_PAYLOAD_V3); } + if self.new_payload_v4 { + response.push(ENGINE_NEW_PAYLOAD_V4); + } if self.forkchoice_updated_v1 { response.push(ENGINE_FORKCHOICE_UPDATED_V1); } @@ -699,6 +705,9 @@ impl EngineCapabilities { if self.get_payload_v3 { response.push(ENGINE_GET_PAYLOAD_V3); } + if self.get_payload_v4 { + response.push(ENGINE_GET_PAYLOAD_V4); + } if self.get_client_version_v1 { response.push(ENGINE_GET_CLIENT_VERSION_V1); } diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index 93705a1692..e81027340f 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -34,11 +34,13 @@ pub const ETH_SYNCING_TIMEOUT: Duration = Duration::from_secs(1); 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_TIMEOUT: Duration = Duration::from_secs(8); pub const ENGINE_GET_PAYLOAD_V1: &str = "engine_getPayloadV1"; pub const ENGINE_GET_PAYLOAD_V2: &str = "engine_getPayloadV2"; pub const ENGINE_GET_PAYLOAD_V3: &str = "engine_getPayloadV3"; +pub const ENGINE_GET_PAYLOAD_V4: &str = "engine_getPayloadV4"; pub const ENGINE_GET_PAYLOAD_TIMEOUT: Duration = Duration::from_secs(2); pub const ENGINE_FORKCHOICE_UPDATED_V1: &str = "engine_forkchoiceUpdatedV1"; @@ -66,9 +68,11 @@ pub static LIGHTHOUSE_CAPABILITIES: &[&str] = &[ ENGINE_NEW_PAYLOAD_V1, ENGINE_NEW_PAYLOAD_V2, ENGINE_NEW_PAYLOAD_V3, + ENGINE_NEW_PAYLOAD_V4, ENGINE_GET_PAYLOAD_V1, ENGINE_GET_PAYLOAD_V2, ENGINE_GET_PAYLOAD_V3, + ENGINE_GET_PAYLOAD_V4, ENGINE_FORKCHOICE_UPDATED_V1, ENGINE_FORKCHOICE_UPDATED_V2, ENGINE_FORKCHOICE_UPDATED_V3, @@ -833,7 +837,7 @@ impl HttpJsonRpc { Ok(response.into()) } - pub async fn new_payload_v3_electra( + pub async fn new_payload_v4_electra( &self, new_payload_request_electra: NewPayloadRequestElectra<'_, E>, ) -> Result { @@ -845,7 +849,7 @@ impl HttpJsonRpc { let response: JsonPayloadStatusV1 = self .rpc_request( - ENGINE_NEW_PAYLOAD_V3, + ENGINE_NEW_PAYLOAD_V4, params, ENGINE_NEW_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, ) @@ -929,10 +933,40 @@ impl HttpJsonRpc { .await?; Ok(JsonGetPayloadResponse::V3(response).into()) } + ForkName::Base + | ForkName::Altair + | ForkName::Bellatrix + | ForkName::Capella + // TODO(pawan): make sure electra should not support getpayloadv3 + | ForkName::Electra => Err(Error::UnsupportedForkVariant(format!( + "called get_payload_v3 with {}", + fork_name + ))), + } + } + + pub async fn get_payload_v4( + &self, + fork_name: ForkName, + payload_id: PayloadId, + ) -> Result, Error> { + let params = json!([JsonPayloadIdRequest::from(payload_id)]); + + match fork_name { + ForkName::Deneb => { + let response: JsonGetPayloadResponseV3 = self + .rpc_request( + ENGINE_GET_PAYLOAD_V4, + params, + ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, + ) + .await?; + Ok(JsonGetPayloadResponse::V3(response).into()) + } ForkName::Electra => { let response: JsonGetPayloadResponseV4 = self .rpc_request( - ENGINE_GET_PAYLOAD_V3, + ENGINE_GET_PAYLOAD_V4, params, ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, ) @@ -940,7 +974,7 @@ impl HttpJsonRpc { Ok(JsonGetPayloadResponse::V4(response).into()) } ForkName::Base | ForkName::Altair | ForkName::Bellatrix | ForkName::Capella => Err( - Error::UnsupportedForkVariant(format!("called get_payload_v3 with {}", fork_name)), + Error::UnsupportedForkVariant(format!("called get_payload_v4 with {}", fork_name)), ), } } @@ -1067,6 +1101,7 @@ impl HttpJsonRpc { new_payload_v1: capabilities.contains(ENGINE_NEW_PAYLOAD_V1), 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), 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), @@ -1077,6 +1112,7 @@ impl HttpJsonRpc { get_payload_v1: capabilities.contains(ENGINE_GET_PAYLOAD_V1), get_payload_v2: capabilities.contains(ENGINE_GET_PAYLOAD_V2), get_payload_v3: capabilities.contains(ENGINE_GET_PAYLOAD_V3), + get_payload_v4: capabilities.contains(ENGINE_GET_PAYLOAD_V4), get_client_version_v1: capabilities.contains(ENGINE_GET_CLIENT_VERSION_V1), }) } @@ -1200,7 +1236,7 @@ impl HttpJsonRpc { } NewPayloadRequest::Electra(new_payload_request_electra) => { if engine_capabilities.new_payload_v3 { - self.new_payload_v3_electra(new_payload_request_electra) + self.new_payload_v4_electra(new_payload_request_electra) .await } else { Err(Error::RequiredMethodUnsupported("engine_newPayloadV3")) @@ -1221,14 +1257,16 @@ impl HttpJsonRpc { ForkName::Bellatrix | ForkName::Capella => { if engine_capabilities.get_payload_v2 { self.get_payload_v2(fork_name, payload_id).await - } else if engine_capabilities.new_payload_v1 { + } else if engine_capabilities.get_payload_v1 { self.get_payload_v1(payload_id).await } else { Err(Error::RequiredMethodUnsupported("engine_getPayload")) } } ForkName::Deneb | ForkName::Electra => { - if engine_capabilities.get_payload_v3 { + if engine_capabilities.get_payload_v4 { + self.get_payload_v4(fork_name, payload_id).await + } else if engine_capabilities.get_payload_v3 { self.get_payload_v3(fork_name, payload_id).await } else { Err(Error::RequiredMethodUnsupported("engine_getPayloadV3")) 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 50d3519e12..822a3802a8 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -4,7 +4,10 @@ use strum::EnumString; use superstruct::superstruct; use types::beacon_block_body::KzgCommitments; use types::blob_sidecar::BlobsList; -use types::{FixedVector, Unsigned}; +use types::{ + DepositReceipt, ExecutionLayerWithdrawalRequest, FixedVector, PublicKeyBytes, Signature, + Unsigned, +}; #[derive(Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -101,6 +104,11 @@ pub struct JsonExecutionPayload { #[superstruct(only(V3, V4))] #[serde(with = "serde_utils::u64_hex_be")] pub excess_blob_gas: u64, + #[superstruct(only(V4))] + pub deposit_requests: VariableList, + #[superstruct(only(V4))] + pub withdrawal_requests: + VariableList, } impl From> for JsonExecutionPayloadV1 { @@ -203,6 +211,18 @@ impl From> for JsonExecutionPayloadV4 .into(), blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, + deposit_requests: payload + .deposit_receipts + .into_iter() + .map(Into::into) + .collect::>() + .into(), + withdrawal_requests: payload + .withdrawal_requests + .into_iter() + .map(Into::into) + .collect::>() + .into(), } } } @@ -319,9 +339,18 @@ impl From> for ExecutionPayloadElectra .into(), blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, - // TODO(electra) - deposit_receipts: Default::default(), - withdrawal_requests: Default::default(), + deposit_receipts: payload + .deposit_requests + .into_iter() + .map(Into::into) + .collect::>() + .into(), + withdrawal_requests: payload + .withdrawal_requests + .into_iter() + .map(Into::into) + .collect::>() + .into(), } } } @@ -783,3 +812,68 @@ impl TryFrom for ClientVersionV1 { }) } } + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct JsonDepositRequest { + pub pubkey: PublicKeyBytes, + pub withdrawal_credentials: Hash256, + #[serde(with = "serde_utils::quoted_u64")] + pub amount: u64, + pub signature: Signature, + #[serde(with = "serde_utils::quoted_u64")] + pub index: u64, +} + +impl From for JsonDepositRequest { + fn from(deposit: DepositReceipt) -> Self { + Self { + pubkey: deposit.pubkey, + withdrawal_credentials: deposit.withdrawal_credentials, + amount: deposit.amount, + signature: deposit.signature, + index: deposit.index, + } + } +} + +impl From for DepositReceipt { + fn from(json_deposit: JsonDepositRequest) -> Self { + Self { + pubkey: json_deposit.pubkey, + withdrawal_credentials: json_deposit.withdrawal_credentials, + amount: json_deposit.amount, + signature: json_deposit.signature, + index: json_deposit.index, + } + } +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct JsonWithdrawalRequest { + pub source_address: Address, + pub validator_public_key: PublicKeyBytes, + #[serde(with = "serde_utils::quoted_u64")] + pub amount: u64, +} + +impl From for JsonWithdrawalRequest { + fn from(withdrawal_request: ExecutionLayerWithdrawalRequest) -> Self { + Self { + source_address: withdrawal_request.source_address, + validator_public_key: withdrawal_request.validator_pubkey, + amount: withdrawal_request.amount, + } + } +} + +impl From for ExecutionLayerWithdrawalRequest { + fn from(json_withdrawal_request: JsonWithdrawalRequest) -> Self { + Self { + source_address: json_withdrawal_request.source_address, + validator_pubkey: json_withdrawal_request.validator_public_key, + amount: json_withdrawal_request.amount, + } + } +} diff --git a/beacon_node/execution_layer/src/test_utils/mod.rs b/beacon_node/execution_layer/src/test_utils/mod.rs index a6d47995af..7b00ca9fbc 100644 --- a/beacon_node/execution_layer/src/test_utils/mod.rs +++ b/beacon_node/execution_layer/src/test_utils/mod.rs @@ -43,6 +43,7 @@ pub const DEFAULT_ENGINE_CAPABILITIES: EngineCapabilities = EngineCapabilities { new_payload_v1: true, new_payload_v2: true, new_payload_v3: true, + new_payload_v4: true, forkchoice_updated_v1: true, forkchoice_updated_v2: true, forkchoice_updated_v3: true, @@ -51,6 +52,7 @@ pub const DEFAULT_ENGINE_CAPABILITIES: EngineCapabilities = EngineCapabilities { get_payload_v1: true, get_payload_v2: true, get_payload_v3: true, + get_payload_v4: true, get_client_version_v1: true, }; From ca2a9461759e913dbd1902f55902820cf7415972 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 7 May 2024 14:42:51 -0700 Subject: [PATCH 2/7] Fix the versioning of v4 requests --- .../execution_layer/src/engine_api/http.rs | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index e81027340f..aea87dd7d4 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -937,7 +937,6 @@ impl HttpJsonRpc { | ForkName::Altair | ForkName::Bellatrix | ForkName::Capella - // TODO(pawan): make sure electra should not support getpayloadv3 | ForkName::Electra => Err(Error::UnsupportedForkVariant(format!( "called get_payload_v3 with {}", fork_name @@ -953,16 +952,6 @@ impl HttpJsonRpc { let params = json!([JsonPayloadIdRequest::from(payload_id)]); match fork_name { - ForkName::Deneb => { - let response: JsonGetPayloadResponseV3 = self - .rpc_request( - ENGINE_GET_PAYLOAD_V4, - params, - ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, - ) - .await?; - Ok(JsonGetPayloadResponse::V3(response).into()) - } ForkName::Electra => { let response: JsonGetPayloadResponseV4 = self .rpc_request( @@ -973,9 +962,14 @@ impl HttpJsonRpc { .await?; Ok(JsonGetPayloadResponse::V4(response).into()) } - ForkName::Base | ForkName::Altair | ForkName::Bellatrix | ForkName::Capella => Err( - Error::UnsupportedForkVariant(format!("called get_payload_v4 with {}", fork_name)), - ), + ForkName::Base + | ForkName::Altair + | ForkName::Bellatrix + | ForkName::Capella + | ForkName::Deneb => Err(Error::UnsupportedForkVariant(format!( + "called get_payload_v4 with {}", + fork_name + ))), } } @@ -1235,11 +1229,11 @@ impl HttpJsonRpc { } } NewPayloadRequest::Electra(new_payload_request_electra) => { - if engine_capabilities.new_payload_v3 { + if engine_capabilities.new_payload_v4 { self.new_payload_v4_electra(new_payload_request_electra) .await } else { - Err(Error::RequiredMethodUnsupported("engine_newPayloadV3")) + Err(Error::RequiredMethodUnsupported("engine_newPayloadV4")) } } } @@ -1263,13 +1257,18 @@ impl HttpJsonRpc { Err(Error::RequiredMethodUnsupported("engine_getPayload")) } } - ForkName::Deneb | ForkName::Electra => { - if engine_capabilities.get_payload_v4 { - self.get_payload_v4(fork_name, payload_id).await - } else if engine_capabilities.get_payload_v3 { + ForkName::Deneb => { + if engine_capabilities.get_payload_v3 { self.get_payload_v3(fork_name, payload_id).await } else { - Err(Error::RequiredMethodUnsupported("engine_getPayloadV3")) + Err(Error::RequiredMethodUnsupported("engine_getPayloadv3")) + } + } + ForkName::Electra => { + if engine_capabilities.get_payload_v4 { + self.get_payload_v4(fork_name, payload_id).await + } else { + Err(Error::RequiredMethodUnsupported("engine_getPayloadv4")) } } ForkName::Base | ForkName::Altair => Err(Error::UnsupportedForkVariant(format!( From 1ddd078d324fde76ca178a97321e22e569b5ff4f Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 7 May 2024 16:13:36 -0700 Subject: [PATCH 3/7] Handle new engine api methods in mock EL --- .../src/test_utils/handle_rpc.rs | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) 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 1dc8f0ab83..5e31646d31 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -111,20 +111,11 @@ pub async fn handle_rpc( .map(|jep| JsonExecutionPayload::V1(jep)) }) .map_err(|s| (s, BAD_PARAMS_ERROR_CODE))?, - ENGINE_NEW_PAYLOAD_V3 => get_param::>(params, 0) + ENGINE_NEW_PAYLOAD_V3 => get_param::>(params, 0) + .map(|jep| JsonExecutionPayload::V3(jep)) + .map_err(|s| (s, BAD_PARAMS_ERROR_CODE))?, + ENGINE_NEW_PAYLOAD_V4 => get_param::>(params, 0) .map(|jep| JsonExecutionPayload::V4(jep)) - .or_else(|_| { - 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)) - }) - }) - }) .map_err(|s| (s, BAD_PARAMS_ERROR_CODE))?, _ => unreachable!(), }; @@ -190,7 +181,10 @@ pub async fn handle_rpc( } } ForkName::Electra => { - if method == ENGINE_NEW_PAYLOAD_V1 || method == ENGINE_NEW_PAYLOAD_V2 { + if method == ENGINE_NEW_PAYLOAD_V1 + || method == ENGINE_NEW_PAYLOAD_V2 + || method == ENGINE_NEW_PAYLOAD_V3 + { return Err(( format!("{} called after Electra fork!", method), GENERIC_ERROR_CODE, @@ -259,7 +253,10 @@ pub async fn handle_rpc( Ok(serde_json::to_value(JsonPayloadStatusV1::from(response)).unwrap()) } - ENGINE_GET_PAYLOAD_V1 | ENGINE_GET_PAYLOAD_V2 | ENGINE_GET_PAYLOAD_V3 => { + ENGINE_GET_PAYLOAD_V1 + | ENGINE_GET_PAYLOAD_V2 + | ENGINE_GET_PAYLOAD_V3 + | ENGINE_GET_PAYLOAD_V4 => { let request: JsonPayloadIdRequest = get_param(params, 0).map_err(|s| (s, BAD_PARAMS_ERROR_CODE))?; let id = request.into(); @@ -309,7 +306,9 @@ pub async fn handle_rpc( .read() .get_fork_at_timestamp(response.timestamp()) == ForkName::Electra - && method == ENGINE_GET_PAYLOAD_V1 + && (method == ENGINE_GET_PAYLOAD_V1 + || method == ENGINE_GET_PAYLOAD_V2 + || method == ENGINE_GET_PAYLOAD_V3) { return Err(( format!("{} called after Electra fork!", method), @@ -353,6 +352,9 @@ pub async fn handle_rpc( }) .unwrap() } + _ => unreachable!(), + }), + ENGINE_GET_PAYLOAD_V4 => Ok(match JsonExecutionPayload::from(response) { JsonExecutionPayload::V4(execution_payload) => { serde_json::to_value(JsonGetPayloadResponseV4 { execution_payload, From 3ef7c9078e4a06b097963fdad51087ea7f869bc6 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 7 May 2024 16:21:21 -0700 Subject: [PATCH 4/7] Note todo --- beacon_node/execution_layer/src/engine_api/json_structures.rs | 1 + 1 file changed, 1 insertion(+) 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 822a3802a8..c9a7c072f8 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -105,6 +105,7 @@ pub struct JsonExecutionPayload { #[serde(with = "serde_utils::u64_hex_be")] pub excess_blob_gas: u64, #[superstruct(only(V4))] + // TODO(electra): Field name should be changed post devnet-0. See https://github.com/ethereum/execution-apis/pull/544 pub deposit_requests: VariableList, #[superstruct(only(V4))] pub withdrawal_requests: From 683de56f6e9185a2d091aff92db9dae2d66d117e Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Wed, 8 May 2024 13:42:29 -0700 Subject: [PATCH 5/7] Fix todos --- beacon_node/execution_layer/src/engine_api.rs | 18 +++++++++++++++ beacon_node/execution_layer/src/lib.rs | 23 +++++++++++++++---- .../types/src/execution_payload_header.rs | 1 - 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 6f6ec8b894..df6b30baa2 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -41,6 +41,8 @@ pub use new_payload_request::{ NewPayloadRequestDeneb, NewPayloadRequestElectra, }; +use self::json_structures::{JsonDepositRequest, JsonWithdrawalRequest}; + pub const LATEST_TAG: &str = "latest"; pub type PayloadId = [u8; 8]; @@ -64,6 +66,8 @@ pub enum Error { TransitionConfigurationMismatch, SszError(ssz_types::Error), DeserializeWithdrawals(ssz_types::Error), + DeserializeDepositReceipts(ssz_types::Error), + DeserializeWithdrawalRequests(ssz_types::Error), BuilderApi(builder_client::Error), IncorrectStateVariant, RequiredMethodUnsupported(&'static str), @@ -198,6 +202,10 @@ pub struct ExecutionBlockWithTransactions { #[superstruct(only(Deneb, Electra))] #[serde(with = "serde_utils::u64_hex_be")] pub excess_blob_gas: u64, + #[superstruct(only(Electra))] + pub deposit_receipts: Vec, + #[superstruct(only(Electra))] + pub withdrawal_requests: Vec, } impl TryFrom> for ExecutionBlockWithTransactions { @@ -305,6 +313,16 @@ impl TryFrom> for ExecutionBlockWithTransactions .collect(), blob_gas_used: block.blob_gas_used, excess_blob_gas: block.excess_blob_gas, + deposit_receipts: block + .deposit_receipts + .into_iter() + .map(|deposit| deposit.into()) + .collect(), + withdrawal_requests: block + .withdrawal_requests + .into_iter() + .map(|withdrawal| withdrawal.into()) + .collect(), }) } }; diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 11329259e9..15a1c19462 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -1985,6 +1985,22 @@ impl ExecutionLayer { .collect(), ) .map_err(ApiError::DeserializeWithdrawals)?; + let deposit_receipts = VariableList::new( + electra_block + .deposit_receipts + .into_iter() + .map(Into::into) + .collect(), + ) + .map_err(ApiError::DeserializeDepositReceipts)?; + let withdrawal_requests = VariableList::new( + electra_block + .withdrawal_requests + .into_iter() + .map(Into::into) + .collect(), + ) + .map_err(ApiError::DeserializeWithdrawalRequests)?; ExecutionPayload::Electra(ExecutionPayloadElectra { parent_hash: electra_block.parent_hash, fee_recipient: electra_block.fee_recipient, @@ -2003,11 +2019,8 @@ impl ExecutionLayer { withdrawals, blob_gas_used: electra_block.blob_gas_used, excess_blob_gas: electra_block.excess_blob_gas, - // TODO(electra) - // deposit_receipts: electra_block.deposit_receipts, - // withdrawal_requests: electra_block.withdrawal_requests, - deposit_receipts: <_>::default(), - withdrawal_requests: <_>::default(), + deposit_receipts, + withdrawal_requests, }) } }; diff --git a/consensus/types/src/execution_payload_header.rs b/consensus/types/src/execution_payload_header.rs index 2d7bc95071..a3df69652c 100644 --- a/consensus/types/src/execution_payload_header.rs +++ b/consensus/types/src/execution_payload_header.rs @@ -117,7 +117,6 @@ impl ExecutionPayloadHeader { #[allow(clippy::arithmetic_side_effects)] pub fn ssz_max_var_len_for_fork(fork_name: ForkName) -> usize { // Matching here in case variable fields are added in future forks. - // TODO(electra): review electra changes match fork_name { ForkName::Base | ForkName::Altair From dd5c9a8c8160e5cf744687b321293312469dedba Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Wed, 8 May 2024 16:22:01 -0700 Subject: [PATCH 6/7] Add support for electra fields in getPayloadBodies --- beacon_node/execution_layer/src/engine_api.rs | 60 ++++++++++--------- .../src/engine_api/json_structures.rs | 19 ++++++ .../src/test_utils/handle_rpc.rs | 8 +++ consensus/types/src/execution_payload.rs | 4 ++ 4 files changed, 64 insertions(+), 27 deletions(-) diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index df6b30baa2..09e36be41f 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -20,6 +20,7 @@ use reqwest::StatusCode; use serde::{Deserialize, Serialize}; use strum::IntoStaticStr; use superstruct::superstruct; +use types::execution_payload::{DepositReceipts, WithdrawalRequests}; pub use types::{ Address, BeaconBlockRef, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadHeader, ExecutionPayloadRef, FixedVector, ForkName, Hash256, Transactions, Uint256, VariableList, @@ -545,6 +546,8 @@ impl GetPayloadResponse { pub struct ExecutionPayloadBodyV1 { pub transactions: Transactions, pub withdrawals: Option>, + pub deposit_receipts: Option>, + pub withdrawal_requests: Option>, } impl ExecutionPayloadBodyV1 { @@ -632,35 +635,38 @@ impl ExecutionPayloadBodyV1 { } } ExecutionPayloadHeader::Electra(header) => { - if let Some(withdrawals) = self.withdrawals { - Ok(ExecutionPayload::Electra(ExecutionPayloadElectra { - parent_hash: header.parent_hash, - fee_recipient: header.fee_recipient, - state_root: header.state_root, - receipts_root: header.receipts_root, - logs_bloom: header.logs_bloom, - prev_randao: header.prev_randao, - block_number: header.block_number, - gas_limit: header.gas_limit, - gas_used: header.gas_used, - timestamp: header.timestamp, - extra_data: header.extra_data, - base_fee_per_gas: header.base_fee_per_gas, - block_hash: header.block_hash, - transactions: self.transactions, - withdrawals, - blob_gas_used: header.blob_gas_used, - excess_blob_gas: header.excess_blob_gas, - // TODO(electra) - deposit_receipts: <_>::default(), - withdrawal_requests: <_>::default(), - })) - } else { - Err(format!( - "block {} is post-capella but payload body doesn't have withdrawals", + let (Some(withdrawals), Some(deposit_receipts), Some(withdrawal_requests)) = ( + self.withdrawals, + self.deposit_receipts, + self.withdrawal_requests, + ) else { + return Err(format!( + "block {} is post-electra but payload body doesn't have withdrawals/deposit_receipts/withdrawal_requests \ + Check that ELs are returning receipts and withdrawal_requests in getPayloadBody requests", header.block_hash )) - } + }; + Ok(ExecutionPayload::Electra(ExecutionPayloadElectra { + parent_hash: header.parent_hash, + fee_recipient: header.fee_recipient, + state_root: header.state_root, + receipts_root: header.receipts_root, + logs_bloom: header.logs_bloom, + prev_randao: header.prev_randao, + block_number: header.block_number, + gas_limit: header.gas_limit, + gas_used: header.gas_used, + timestamp: header.timestamp, + extra_data: header.extra_data, + base_fee_per_gas: header.base_fee_per_gas, + block_hash: header.block_hash, + transactions: self.transactions, + withdrawals, + blob_gas_used: header.blob_gas_used, + excess_blob_gas: header.excess_blob_gas, + deposit_receipts, + withdrawal_requests, + })) } } } 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 c9a7c072f8..28eb90fd1f 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -724,6 +724,9 @@ pub struct JsonExecutionPayloadBodyV1 { #[serde(with = "ssz_types::serde_utils::list_of_hex_var_list")] pub transactions: Transactions, pub withdrawals: Option>, + pub deposit_receipts: Option>, + pub withdrawal_requests: + Option>, } impl From> for ExecutionPayloadBodyV1 { @@ -738,6 +741,22 @@ impl From> for ExecutionPayloadBodyV1< .collect::>(), ) }), + deposit_receipts: value.deposit_receipts.map(|json_receipts| { + DepositReceipts::::from( + json_receipts + .into_iter() + .map(Into::into) + .collect::>(), + ) + }), + withdrawal_requests: value.withdrawal_requests.map(|json_withdrawal_requests| { + WithdrawalRequests::::from( + json_withdrawal_requests + .into_iter() + .map(Into::into) + .collect::>(), + ) + }), } } } 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 5e31646d31..d2c16da799 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -580,6 +580,14 @@ pub async fn handle_rpc( .withdrawals() .ok() .map(|withdrawals| VariableList::from(withdrawals.clone())), + deposit_receipts: block.deposit_receipts().ok().map( + |deposit_receipts| VariableList::from(deposit_receipts.clone()), + ), + withdrawal_requests: block.withdrawal_requests().ok().map( + |withdrawal_requests| { + VariableList::from(withdrawal_requests.clone()) + }, + ), })); } None => response.push(None), diff --git a/consensus/types/src/execution_payload.rs b/consensus/types/src/execution_payload.rs index 0946b9ecff..cd6667b375 100644 --- a/consensus/types/src/execution_payload.rs +++ b/consensus/types/src/execution_payload.rs @@ -13,6 +13,10 @@ pub type Transactions = VariableList< >; pub type Withdrawals = VariableList::MaxWithdrawalsPerPayload>; +pub type DepositReceipts = + VariableList::MaxDepositReceiptsPerPayload>; +pub type WithdrawalRequests = + VariableList::MaxWithdrawalRequestsPerPayload>; #[superstruct( variants(Bellatrix, Capella, Deneb, Electra), From 5e1d5ff641b8eed551dc82e57a46a9db26461b0c Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Wed, 8 May 2024 16:46:23 -0700 Subject: [PATCH 7/7] Add comments for potential versioning confusion --- beacon_node/execution_layer/src/test_utils/handle_rpc.rs | 6 ++++++ 1 file changed, 6 insertions(+) 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 d2c16da799..b9d9d76186 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -111,6 +111,9 @@ pub async fn handle_rpc( .map(|jep| JsonExecutionPayload::V1(jep)) }) .map_err(|s| (s, BAD_PARAMS_ERROR_CODE))?, + // From v3 onwards, we use the newPayload version only for the corresponding + // ExecutionPayload version. So we return an error instead of falling back to + // older versions of newPayload ENGINE_NEW_PAYLOAD_V3 => get_param::>(params, 0) .map(|jep| JsonExecutionPayload::V3(jep)) .map_err(|s| (s, BAD_PARAMS_ERROR_CODE))?, @@ -337,6 +340,9 @@ pub async fn handle_rpc( } _ => unreachable!(), }), + // From v3 onwards, we use the getPayload version only for the corresponding + // ExecutionPayload version. So we return an error if the ExecutionPayload version + // we get does not correspond to the getPayload version. ENGINE_GET_PAYLOAD_V3 => Ok(match JsonExecutionPayload::from(response) { JsonExecutionPayload::V3(execution_payload) => { serde_json::to_value(JsonGetPayloadResponseV3 {