From 24e5252a55148f8a49e806f2e0943cb834042b31 Mon Sep 17 00:00:00 2001 From: ethDreamer <37123614+ethDreamer@users.noreply.github.com> Date: Tue, 22 Nov 2022 12:27:48 -0600 Subject: [PATCH] Massive Update to Engine API (#3740) * Massive Update to Engine API * Update beacon_node/execution_layer/src/engine_api/json_structures.rs Co-authored-by: Michael Sproul * Update beacon_node/execution_layer/src/engine_api/json_structures.rs Co-authored-by: Michael Sproul * Update beacon_node/beacon_chain/src/execution_payload.rs Co-authored-by: realbigsean * Update beacon_node/execution_layer/src/engine_api.rs Co-authored-by: realbigsean Co-authored-by: Michael Sproul Co-authored-by: realbigsean --- beacon_node/beacon_chain/src/beacon_chain.rs | 75 ++- beacon_node/beacon_chain/src/errors.rs | 2 +- .../beacon_chain/src/execution_payload.rs | 17 + .../tests/payload_invalidation.rs | 8 +- beacon_node/execution_layer/Cargo.toml | 4 +- beacon_node/execution_layer/src/engine_api.rs | 46 +- .../execution_layer/src/engine_api/http.rs | 196 ++++++- .../src/engine_api/json_structures.rs | 485 ++++++++---------- beacon_node/execution_layer/src/engines.rs | 16 +- beacon_node/execution_layer/src/lib.rs | 59 ++- .../test_utils/execution_block_generator.rs | 19 +- .../src/test_utils/handle_rpc.rs | 9 +- .../src/test_utils/mock_builder.rs | 7 +- .../src/test_utils/mock_execution_layer.rs | 10 +- common/eth2/Cargo.toml | 2 + .../src/per_block_processing.rs | 6 +- testing/ef_tests/src/cases/operations.rs | 12 +- .../execution_engine_integration/Cargo.toml | 4 + .../src/test_rig.rs | 12 +- 19 files changed, 625 insertions(+), 364 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index e7c456ef1..efedfdae5 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -77,6 +77,8 @@ use slasher::Slasher; use slog::{crit, debug, error, info, trace, warn, Logger}; use slot_clock::SlotClock; use ssz::Encode; +#[cfg(feature = "withdrawals")] +use state_processing::per_block_processing::get_expected_withdrawals; use state_processing::{ common::{get_attesting_indices_from_state, get_indexed_attestation}, per_block_processing, @@ -4105,35 +4107,52 @@ impl BeaconChain { return Ok(()); } - let payload_attributes = match self.spec.fork_name_at_epoch(prepare_epoch) { + #[cfg(feature = "withdrawals")] + let head_state = &self.canonical_head.cached_head().snapshot.beacon_state; + #[cfg(feature = "withdrawals")] + let withdrawals = match self.spec.fork_name_at_epoch(prepare_epoch) { ForkName::Base | ForkName::Altair | ForkName::Merge => { - PayloadAttributes::V1(PayloadAttributesV1 { - timestamp: self - .slot_clock - .start_of(prepare_slot) - .ok_or(Error::InvalidSlot(prepare_slot))? - .as_secs(), - prev_randao: head_random, - suggested_fee_recipient: execution_layer - .get_suggested_fee_recipient(proposer as u64) - .await, - }) - } - ForkName::Capella | ForkName::Eip4844 => PayloadAttributes::V2(PayloadAttributesV2 { - timestamp: self - .slot_clock - .start_of(prepare_slot) - .ok_or(Error::InvalidSlot(prepare_slot))? - .as_secs(), - prev_randao: head_random, - suggested_fee_recipient: execution_layer - .get_suggested_fee_recipient(proposer as u64) - .await, - //FIXME(sean) - #[cfg(feature = "withdrawals")] - withdrawals: vec![], - }), - }; + None + }, + ForkName::Capella | ForkName::Eip4844 => match &head_state { + &BeaconState::Capella(_) | &BeaconState::Eip4844(_) => { + // The head_state is already BeaconState::Capella or later + // FIXME(mark) + // Might implement caching here in the future.. + Some(get_expected_withdrawals(head_state, &self.spec)) + } + &BeaconState::Base(_) | &BeaconState::Altair(_) | &BeaconState::Merge(_) => { + // We are the Capella transition block proposer, need advanced state + let mut prepare_state = self + .state_at_slot(prepare_slot, StateSkipConfig::WithoutStateRoots) + .or_else(|e| { + error!(self.log, "Capella Transition Proposer"; "Error Advancing State: " => ?e); + Err(e) + })?; + // FIXME(mark) + // Might implement caching here in the future.. + Some(get_expected_withdrawals(&prepare_state, &self.spec)) + } + }, + }.transpose().or_else(|e| { + error!(self.log, "Error preparing beacon proposer"; "while calculating expected withdrawals" => ?e); + Err(e) + }).map(|withdrawals_opt| withdrawals_opt.map(|w| w.into())) + .map_err(Error::PrepareProposerFailed)?; + + let payload_attributes = PayloadAttributes::V2(PayloadAttributesV2 { + timestamp: self + .slot_clock + .start_of(prepare_slot) + .ok_or(Error::InvalidSlot(prepare_slot))? + .as_secs(), + prev_randao: head_random, + suggested_fee_recipient: execution_layer + .get_suggested_fee_recipient(proposer as u64) + .await, + #[cfg(feature = "withdrawals")] + withdrawals, + }); debug!( self.log, diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index da944c102..e4d00d9ca 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -153,7 +153,7 @@ pub enum BeaconChainError { }, AddPayloadLogicError, ExecutionForkChoiceUpdateFailed(execution_layer::Error), - PrepareProposerBlockingFailed(execution_layer::Error), + PrepareProposerFailed(BlockProcessingError), ExecutionForkChoiceUpdateInvalid { status: PayloadStatus, }, diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 134e51e79..bf920a6da 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -17,6 +17,8 @@ use fork_choice::{InvalidationOperation, PayloadVerificationStatus}; use proto_array::{Block as ProtoBlock, ExecutionStatus}; use slog::debug; use slot_clock::SlotClock; +#[cfg(feature = "withdrawals")] +use state_processing::per_block_processing::get_expected_withdrawals; use state_processing::per_block_processing::{ compute_timestamp_at_slot, is_execution_enabled, is_merge_transition_complete, partially_verify_execution_payload, @@ -362,6 +364,15 @@ pub fn get_execution_payload< let random = *state.get_randao_mix(current_epoch)?; let latest_execution_payload_header_block_hash = state.latest_execution_payload_header()?.block_hash(); + #[cfg(feature = "withdrawals")] + let withdrawals = match state { + &BeaconState::Capella(_) | &BeaconState::Eip4844(_) => { + Some(get_expected_withdrawals(state, spec)?.into()) + } + &BeaconState::Merge(_) => None, + // These shouldn't happen but they're here to make the pattern irrefutable + &BeaconState::Base(_) | &BeaconState::Altair(_) => None, + }; // Spawn a task to obtain the execution payload from the EL via a series of async calls. The // `join_handle` can be used to await the result of the function. @@ -378,6 +389,8 @@ pub fn get_execution_payload< proposer_index, latest_execution_payload_header_block_hash, builder_params, + #[cfg(feature = "withdrawals")] + withdrawals, ) .await }, @@ -411,6 +424,7 @@ pub async fn prepare_execution_payload( proposer_index: u64, latest_execution_payload_header_block_hash: ExecutionBlockHash, builder_params: BuilderParams, + #[cfg(feature = "withdrawals")] withdrawals: Option>, ) -> Result, BlockProductionError> where T: BeaconChainTypes, @@ -480,6 +494,9 @@ where proposer_index, forkchoice_update_params, builder_params, + fork, + #[cfg(feature = "withdrawals")] + withdrawals, &chain.spec, ) .await diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 2336c3ba9..611b20988 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -12,9 +12,9 @@ use beacon_chain::{ INVALID_JUSTIFIED_PAYLOAD_SHUTDOWN_REASON, }; use execution_layer::{ - json_structures::{JsonForkChoiceStateV1, JsonPayloadAttributesV1}, + json_structures::{JsonForkchoiceStateV1, JsonPayloadAttributesV1}, test_utils::ExecutionBlockGenerator, - ExecutionLayer, ForkChoiceState, PayloadAttributes, + ExecutionLayer, ForkchoiceState, PayloadAttributes, }; use fork_choice::{ CountUnrealized, Error as ForkChoiceError, InvalidationOperation, PayloadVerificationStatus, @@ -117,7 +117,7 @@ impl InvalidPayloadRig { &self.harness.chain.canonical_head } - fn previous_forkchoice_update_params(&self) -> (ForkChoiceState, PayloadAttributes) { + fn previous_forkchoice_update_params(&self) -> (ForkchoiceState, PayloadAttributes) { let mock_execution_layer = self.harness.mock_execution_layer.as_ref().unwrap(); let json = mock_execution_layer .server @@ -126,7 +126,7 @@ impl InvalidPayloadRig { let params = json.get("params").expect("no params"); let fork_choice_state_json = params.get(0).expect("no payload param"); - let fork_choice_state: JsonForkChoiceStateV1 = + let fork_choice_state: JsonForkchoiceStateV1 = serde_json::from_value(fork_choice_state_json.clone()).unwrap(); let payload_param_json = params.get(1).expect("no payload param"); diff --git a/beacon_node/execution_layer/Cargo.toml b/beacon_node/execution_layer/Cargo.toml index 68a4f6a41..b3bdc54d0 100644 --- a/beacon_node/execution_layer/Cargo.toml +++ b/beacon_node/execution_layer/Cargo.toml @@ -5,8 +5,8 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [features] -withdrawals = ["state_processing/withdrawals", "types/withdrawals"] -withdrawals-processing = ["state_processing/withdrawals-processing"] +withdrawals = ["state_processing/withdrawals", "types/withdrawals", "eth2/withdrawals"] +withdrawals-processing = ["state_processing/withdrawals-processing", "eth2/withdrawals-processing"] [dependencies] types = { path = "../../consensus/types"} diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index ed940d4a8..128f23386 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -1,4 +1,4 @@ -use crate::engines::ForkChoiceState; +use crate::engines::ForkchoiceState; pub use ethers_core::types::Transaction; use ethers_core::utils::rlp::{Decodable, Rlp}; use http::deposit_methods::RpcError; @@ -7,10 +7,11 @@ use reqwest::StatusCode; use serde::{Deserialize, Serialize}; use strum::IntoStaticStr; use superstruct::superstruct; +#[cfg(feature = "withdrawals")] use types::Withdrawal; pub use types::{ Address, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadHeader, FixedVector, - Hash256, Uint256, VariableList, + ForkName, Hash256, Uint256, VariableList, }; pub mod auth; @@ -44,6 +45,9 @@ pub enum Error { DeserializeWithdrawals(ssz_types::Error), BuilderApi(builder_client::Error), IncorrectStateVariant, + RequiredMethodUnsupported(&'static str), + UnsupportedForkVariant(String), + BadConversion(String), } impl From for Error { @@ -255,7 +259,29 @@ pub struct PayloadAttributes { pub suggested_fee_recipient: Address, #[cfg(feature = "withdrawals")] #[superstruct(only(V2))] - pub withdrawals: Vec, + pub withdrawals: Option>, +} + +impl PayloadAttributes { + pub fn downgrade_to_v1(self) -> Result { + match self { + PayloadAttributes::V1(_) => Ok(self), + PayloadAttributes::V2(v2) => { + #[cfg(features = "withdrawals")] + if v2.withdrawals.is_some() { + return Err(Error::BadConversion( + "Downgrading from PayloadAttributesV2 with non-null withdrawaals" + .to_string(), + )); + } + Ok(PayloadAttributes::V1(PayloadAttributesV1 { + timestamp: v2.timestamp, + prev_randao: v2.prev_randao, + suggested_fee_recipient: v2.suggested_fee_recipient, + })) + } + } + } } #[derive(Clone, Debug, PartialEq)] @@ -277,3 +303,17 @@ pub struct ProposeBlindedBlockResponse { pub latest_valid_hash: Option, pub validation_error: Option, } + +// 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 +#[derive(Clone, Copy)] +pub struct SupportedApis { + pub new_payload_v1: bool, + pub new_payload_v2: bool, + pub forkchoice_updated_v1: bool, + pub forkchoice_updated_v2: bool, + pub get_payload_v1: bool, + pub get_payload_v2: 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 bd9f387e5..446623744 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -7,6 +7,7 @@ use reqwest::header::CONTENT_TYPE; use sensitive_url::SensitiveUrl; use serde::de::DeserializeOwned; use serde_json::json; +use tokio::sync::RwLock; use std::time::Duration; use types::EthSpec; @@ -29,15 +30,18 @@ pub const ETH_SYNCING: &str = "eth_syncing"; 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_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_TIMEOUT: Duration = Duration::from_secs(2); pub const ENGINE_GET_BLOBS_BUNDLE_V1: &str = "engine_getBlobsBundleV1"; pub const ENGINE_GET_BLOBS_BUNDLE_TIMEOUT: Duration = Duration::from_secs(2); pub const ENGINE_FORKCHOICE_UPDATED_V1: &str = "engine_forkchoiceUpdatedV1"; +pub const ENGINE_FORKCHOICE_UPDATED_V2: &str = "engine_forkchoiceUpdatedV2"; pub const ENGINE_FORKCHOICE_UPDATED_TIMEOUT: Duration = Duration::from_secs(8); pub const ENGINE_EXCHANGE_TRANSITION_CONFIGURATION_V1: &str = @@ -526,6 +530,7 @@ pub struct HttpJsonRpc { pub client: Client, pub url: SensitiveUrl, pub execution_timeout_multiplier: u32, + pub cached_supported_apis: RwLock>, auth: Option, } @@ -538,6 +543,7 @@ impl HttpJsonRpc { client: Client::builder().build()?, url, execution_timeout_multiplier: execution_timeout_multiplier.unwrap_or(1), + cached_supported_apis: Default::default(), auth: None, }) } @@ -551,6 +557,7 @@ impl HttpJsonRpc { client: Client::builder().build()?, url, execution_timeout_multiplier: execution_timeout_multiplier.unwrap_or(1), + cached_supported_apis: Default::default(), auth: Some(auth), }) } @@ -671,7 +678,7 @@ impl HttpJsonRpc { &self, execution_payload: ExecutionPayload, ) -> Result { - let params = json!([JsonExecutionPayload::from(execution_payload)]); + let params = json!([JsonExecutionPayloadV1::try_from(execution_payload)?]); let response: JsonPayloadStatusV1 = self .rpc_request( @@ -684,13 +691,31 @@ impl HttpJsonRpc { Ok(response.into()) } + pub async fn new_payload_v2( + &self, + execution_payload: ExecutionPayload, + ) -> Result { + let params = json!([JsonExecutionPayloadV2::try_from(execution_payload)?]); + + let response: JsonPayloadStatusV1 = self + .rpc_request( + ENGINE_NEW_PAYLOAD_V2, + params, + ENGINE_NEW_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, + ) + .await?; + + Ok(response.into()) + } + pub async fn get_payload_v1( &self, + fork_name: ForkName, payload_id: PayloadId, ) -> Result, Error> { let params = json!([JsonPayloadIdRequest::from(payload_id)]); - let response: JsonExecutionPayload = self + let payload_v1: JsonExecutionPayloadV1 = self .rpc_request( ENGINE_GET_PAYLOAD_V1, params, @@ -698,7 +723,25 @@ impl HttpJsonRpc { ) .await?; - Ok(response.into()) + JsonExecutionPayload::V1(payload_v1).try_into_execution_payload(fork_name) + } + + pub async fn get_payload_v2( + &self, + fork_name: ForkName, + payload_id: PayloadId, + ) -> Result, Error> { + let params = json!([JsonPayloadIdRequest::from(payload_id)]); + + let payload_v2: JsonExecutionPayloadV2 = self + .rpc_request( + ENGINE_GET_PAYLOAD_V2, + params, + ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier, + ) + .await?; + + JsonExecutionPayload::V2(payload_v2).try_into_execution_payload(fork_name) } pub async fn get_blobs_bundle_v1( @@ -720,11 +763,11 @@ impl HttpJsonRpc { pub async fn forkchoice_updated_v1( &self, - forkchoice_state: ForkChoiceState, + forkchoice_state: ForkchoiceState, payload_attributes: Option, ) -> Result { let params = json!([ - JsonForkChoiceStateV1::from(forkchoice_state), + JsonForkchoiceStateV1::from(forkchoice_state), payload_attributes.map(JsonPayloadAttributes::from) ]); @@ -739,6 +782,27 @@ impl HttpJsonRpc { Ok(response.into()) } + pub async fn forkchoice_updated_v2( + &self, + forkchoice_state: ForkchoiceState, + payload_attributes: Option, + ) -> Result { + let params = json!([ + JsonForkchoiceStateV1::from(forkchoice_state), + payload_attributes.map(JsonPayloadAttributes::from) + ]); + + let response: JsonForkchoiceUpdatedV1Response = self + .rpc_request( + ENGINE_FORKCHOICE_UPDATED_V2, + params, + ENGINE_FORKCHOICE_UPDATED_TIMEOUT * self.execution_timeout_multiplier, + ) + .await?; + + Ok(response.into()) + } + pub async fn exchange_transition_configuration_v1( &self, transition_configuration: TransitionConfigurationV1, @@ -756,6 +820,94 @@ impl HttpJsonRpc { Ok(response) } + + // this is a stub as this method hasn't been defined yet + pub async fn supported_apis_v1(&self) -> Result { + Ok(SupportedApis { + new_payload_v1: true, + new_payload_v2: cfg!(all(feature = "withdrawals", not(test))), + forkchoice_updated_v1: true, + forkchoice_updated_v2: cfg!(all(feature = "withdrawals", not(test))), + get_payload_v1: true, + get_payload_v2: cfg!(all(feature = "withdrawals", not(test))), + exchange_transition_configuration_v1: true, + }) + } + + pub async fn set_cached_supported_apis(&self, supported_apis: SupportedApis) { + *self.cached_supported_apis.write().await = Some(supported_apis); + } + + pub async fn get_cached_supported_apis(&self) -> Result { + let cached_opt = *self.cached_supported_apis.read().await; + if let Some(supported_apis) = cached_opt { + Ok(supported_apis) + } else { + let supported_apis = self.supported_apis_v1().await?; + self.set_cached_supported_apis(supported_apis).await; + Ok(supported_apis) + } + } + + // automatically selects the latest version of + // new_payload that the execution engine supports + pub async fn new_payload( + &self, + execution_payload: ExecutionPayload, + ) -> Result { + let supported_apis = self.get_cached_supported_apis().await?; + if supported_apis.new_payload_v2 { + // FIXME: I haven't thought at all about how to handle 4844.. + self.new_payload_v2(execution_payload).await + } else if supported_apis.new_payload_v1 { + self.new_payload_v1(execution_payload).await + } else { + Err(Error::RequiredMethodUnsupported("engine_newPayload")) + } + } + + // automatically selects the latest version of + // get_payload that the execution engine supports + pub async fn get_payload( + &self, + fork_name: ForkName, + payload_id: PayloadId, + ) -> Result, Error> { + let supported_apis = self.get_cached_supported_apis().await?; + if supported_apis.get_payload_v2 { + // FIXME: I haven't thought at all about how to handle 4844.. + 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 + } else { + Err(Error::RequiredMethodUnsupported("engine_getPayload")) + } + } + + // automatically selects the latest version of + // forkchoice_updated that the execution engine supports + pub async fn forkchoice_updated( + &self, + forkchoice_state: ForkchoiceState, + payload_attributes: Option, + ) -> Result { + let supported_apis = self.get_cached_supported_apis().await?; + if supported_apis.forkchoice_updated_v2 { + // FIXME: I haven't thought at all about how to handle 4844.. + 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 + } else { + Err(Error::RequiredMethodUnsupported("engine_forkchoiceUpdated")) + } + } } #[cfg(test)] @@ -767,8 +919,8 @@ mod test { use std::str::FromStr; use std::sync::Arc; use types::{ - AbstractExecPayload, ExecutionPayloadMerge, ForkName, FullPayload, MainnetEthSpec, - Transactions, Unsigned, VariableList, + ExecutionPayloadMerge, ForkName, FullPayload, MainnetEthSpec, Transactions, Unsigned, + VariableList, }; struct Tester { @@ -1052,7 +1204,7 @@ mod test { |client| async move { let _ = client .forkchoice_updated_v1( - ForkChoiceState { + ForkchoiceState { head_block_hash: ExecutionBlockHash::repeat_byte(1), safe_block_hash: ExecutionBlockHash::repeat_byte(1), finalized_block_hash: ExecutionBlockHash::zero(), @@ -1087,7 +1239,7 @@ mod test { .assert_auth_failure(|client| async move { client .forkchoice_updated_v1( - ForkChoiceState { + ForkchoiceState { head_block_hash: ExecutionBlockHash::repeat_byte(1), safe_block_hash: ExecutionBlockHash::repeat_byte(1), finalized_block_hash: ExecutionBlockHash::zero(), @@ -1108,7 +1260,9 @@ mod test { Tester::new(true) .assert_request_equals( |client| async move { - let _ = client.get_payload_v1::([42; 8]).await; + let _ = client + .get_payload_v1::(ForkName::Merge, [42; 8]) + .await; }, json!({ "id": STATIC_ID, @@ -1121,7 +1275,9 @@ mod test { Tester::new(false) .assert_auth_failure(|client| async move { - client.get_payload_v1::([42; 8]).await + client + .get_payload_v1::(ForkName::Merge, [42; 8]) + .await }) .await; } @@ -1209,7 +1365,7 @@ mod test { |client| async move { let _ = client .forkchoice_updated_v1( - ForkChoiceState { + ForkchoiceState { head_block_hash: ExecutionBlockHash::repeat_byte(0), safe_block_hash: ExecutionBlockHash::repeat_byte(0), finalized_block_hash: ExecutionBlockHash::repeat_byte(1), @@ -1235,7 +1391,7 @@ mod test { .assert_auth_failure(|client| async move { client .forkchoice_updated_v1( - ForkChoiceState { + ForkchoiceState { head_block_hash: ExecutionBlockHash::repeat_byte(0), safe_block_hash: ExecutionBlockHash::repeat_byte(0), finalized_block_hash: ExecutionBlockHash::repeat_byte(1), @@ -1274,7 +1430,7 @@ mod test { |client| async move { let _ = client .forkchoice_updated_v1( - ForkChoiceState { + ForkchoiceState { head_block_hash: ExecutionBlockHash::from_str("0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4a").unwrap(), safe_block_hash: ExecutionBlockHash::from_str("0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4a").unwrap(), finalized_block_hash: ExecutionBlockHash::zero(), @@ -1321,7 +1477,7 @@ mod test { |client| async move { let response = client .forkchoice_updated_v1( - ForkChoiceState { + ForkchoiceState { head_block_hash: ExecutionBlockHash::from_str("0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4a").unwrap(), safe_block_hash: ExecutionBlockHash::from_str("0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4a").unwrap(), finalized_block_hash: ExecutionBlockHash::zero(), @@ -1350,7 +1506,7 @@ mod test { // engine_getPayloadV1 REQUEST validation |client| async move { let _ = client - .get_payload_v1::(str_to_payload_id("0xa247243752eb10b4")) + .get_payload_v1::(ForkName::Merge,str_to_payload_id("0xa247243752eb10b4")) .await; }, json!({ @@ -1385,7 +1541,7 @@ mod test { })], |client| async move { let payload = client - .get_payload_v1::(str_to_payload_id("0xa247243752eb10b4")) + .get_payload_v1::(ForkName::Merge,str_to_payload_id("0xa247243752eb10b4")) .await .unwrap(); @@ -1468,7 +1624,7 @@ mod test { })], |client| async move { let response = client - .new_payload_v1::(FullPayload::default_at_fork(ForkName::Merge).into()) + .new_payload_v1::(ExecutionPayload::Merge(ExecutionPayloadMerge::default())) .await .unwrap(); @@ -1487,7 +1643,7 @@ mod test { |client| async move { let _ = client .forkchoice_updated_v1( - ForkChoiceState { + ForkchoiceState { head_block_hash: ExecutionBlockHash::from_str("0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858").unwrap(), safe_block_hash: ExecutionBlockHash::from_str("0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858").unwrap(), finalized_block_hash: ExecutionBlockHash::from_str("0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4a").unwrap(), @@ -1526,7 +1682,7 @@ mod test { |client| async move { let response = client .forkchoice_updated_v1( - ForkChoiceState { + ForkchoiceState { head_block_hash: ExecutionBlockHash::from_str("0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858").unwrap(), safe_block_hash: ExecutionBlockHash::from_str("0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858").unwrap(), finalized_block_hash: ExecutionBlockHash::from_str("0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4a").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 6d1d70e78..99459ec2b 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -3,11 +3,12 @@ use serde::{Deserialize, Serialize}; use strum::EnumString; use superstruct::superstruct; use types::{ - Blob, EthSpec, ExecutionBlockHash, ExecutionPayloadEip4844, ExecutionPayloadHeaderEip4844, - FixedVector, KzgCommitment, Transaction, Unsigned, VariableList, + Blob, EthSpec, ExecutionBlockHash, FixedVector, KzgCommitment, Transaction, Unsigned, + VariableList, Withdrawal, +}; +use types::{ + ExecutionPayload, ExecutionPayloadCapella, ExecutionPayloadEip4844, ExecutionPayloadMerge, }; -use types::{ExecutionPayload, ExecutionPayloadCapella, ExecutionPayloadMerge}; -use types::{ExecutionPayloadHeader, ExecutionPayloadHeaderCapella, ExecutionPayloadHeaderMerge}; #[derive(Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -62,169 +63,6 @@ pub struct JsonPayloadIdResponse { pub payload_id: PayloadId, } -// (V1,V2,V3) -> (Merge,Capella,EIP4844) -#[superstruct( - variants(V1, V2, V3), - variant_attributes( - derive(Debug, PartialEq, Default, 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", untagged)] -pub struct JsonExecutionPayloadHeader { - pub parent_hash: ExecutionBlockHash, - pub fee_recipient: Address, - pub state_root: Hash256, - pub receipts_root: Hash256, - #[serde(with = "serde_logs_bloom")] - pub logs_bloom: FixedVector, - pub prev_randao: Hash256, - #[serde(with = "eth2_serde_utils::u64_hex_be")] - pub block_number: u64, - #[serde(with = "eth2_serde_utils::u64_hex_be")] - pub gas_limit: u64, - #[serde(with = "eth2_serde_utils::u64_hex_be")] - pub gas_used: u64, - #[serde(with = "eth2_serde_utils::u64_hex_be")] - pub timestamp: u64, - #[serde(with = "ssz_types::serde_utils::hex_var_list")] - pub extra_data: VariableList, - #[serde(with = "eth2_serde_utils::u256_hex_be")] - pub base_fee_per_gas: Uint256, - #[serde(with = "eth2_serde_utils::u64_hex_be")] - #[superstruct(only(V3))] - pub excess_blobs: u64, - pub block_hash: ExecutionBlockHash, - pub transactions_root: Hash256, - #[cfg(feature = "withdrawals")] - #[superstruct(only(V2, V3))] - pub withdrawals_root: Hash256, -} - -impl From> for ExecutionPayloadHeader { - fn from(json_header: JsonExecutionPayloadHeader) -> Self { - match json_header { - JsonExecutionPayloadHeader::V1(v1) => Self::Merge(ExecutionPayloadHeaderMerge { - 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_root: v1.transactions_root, - }), - JsonExecutionPayloadHeader::V2(v2) => Self::Capella(ExecutionPayloadHeaderCapella { - 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_root: v2.transactions_root, - #[cfg(feature = "withdrawals")] - withdrawals_root: v2.withdrawals_root, - }), - JsonExecutionPayloadHeader::V3(v3) => Self::Eip4844(ExecutionPayloadHeaderEip4844 { - parent_hash: v3.parent_hash, - fee_recipient: v3.fee_recipient, - state_root: v3.state_root, - receipts_root: v3.receipts_root, - logs_bloom: v3.logs_bloom, - prev_randao: v3.prev_randao, - block_number: v3.block_number, - gas_limit: v3.gas_limit, - gas_used: v3.gas_used, - timestamp: v3.timestamp, - extra_data: v3.extra_data, - base_fee_per_gas: v3.base_fee_per_gas, - excess_blobs: v3.excess_blobs, - block_hash: v3.block_hash, - transactions_root: v3.transactions_root, - #[cfg(feature = "withdrawals")] - withdrawals_root: v3.withdrawals_root, - }), - } - } -} - -impl From> for JsonExecutionPayloadHeader { - fn from(header: ExecutionPayloadHeader) -> Self { - match header { - ExecutionPayloadHeader::Merge(merge) => Self::V1(JsonExecutionPayloadHeaderV1 { - 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_root: merge.transactions_root, - }), - ExecutionPayloadHeader::Capella(capella) => Self::V2(JsonExecutionPayloadHeaderV2 { - 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, - block_hash: capella.block_hash, - transactions_root: capella.transactions_root, - #[cfg(feature = "withdrawals")] - withdrawals_root: capella.withdrawals_root, - }), - ExecutionPayloadHeader::Eip4844(eip4844) => Self::V3(JsonExecutionPayloadHeaderV3 { - 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_blobs: eip4844.excess_blobs, - block_hash: eip4844.block_hash, - transactions_root: eip4844.transactions_root, - #[cfg(feature = "withdrawals")] - withdrawals_root: eip4844.withdrawals_root, - }), - } - } -} - -// (V1,V2, V2) -> (Merge,Capella,EIP4844) #[superstruct( variants(V1, V2, V3), variant_attributes( @@ -257,81 +95,173 @@ pub struct JsonExecutionPayload { #[serde(with = "eth2_serde_utils::u256_hex_be")] pub base_fee_per_gas: Uint256, #[superstruct(only(V3))] + // FIXME: can't easily make this an option because of custom deserialization.. #[serde(with = "eth2_serde_utils::u64_hex_be")] pub excess_blobs: u64, pub block_hash: ExecutionBlockHash, #[serde(with = "ssz_types::serde_utils::list_of_hex_var_list")] pub transactions: VariableList, T::MaxTransactionsPerPayload>, - #[cfg(feature = "withdrawals")] #[superstruct(only(V2, V3))] - pub withdrawals: VariableList, + pub withdrawals: Option>, } -impl From> for ExecutionPayload { - fn from(json_payload: JsonExecutionPayload) -> Self { - match json_payload { - JsonExecutionPayload::V1(v1) => Self::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, - }), - JsonExecutionPayload::V2(v2) => Self::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, - #[cfg(feature = "withdrawals")] - withdrawals: v2.withdrawals, - }), - JsonExecutionPayload::V3(v3) => Self::Eip4844(ExecutionPayloadEip4844 { - parent_hash: v3.parent_hash, - fee_recipient: v3.fee_recipient, - state_root: v3.state_root, - receipts_root: v3.receipts_root, - logs_bloom: v3.logs_bloom, - prev_randao: v3.prev_randao, - block_number: v3.block_number, - gas_limit: v3.gas_limit, - gas_used: v3.gas_used, - timestamp: v3.timestamp, - extra_data: v3.extra_data, - base_fee_per_gas: v3.base_fee_per_gas, - excess_blobs: v3.excess_blobs, - block_hash: v3.block_hash, - transactions: v3.transactions, - #[cfg(feature = "withdrawals")] - withdrawals: v3.withdrawals, - }), +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, + #[cfg(feature = "withdrawals")] + withdrawals: v2 + .withdrawals + .map(|v| { + Into::>::into(v) + .into_iter() + .map(Into::into) + .collect::>() + .into() + }) + .ok_or(Error::BadConversion("Null withdrawal field converting JsonExecutionPayloadV2 -> ExecutionPayloadCapella".to_string()))? + })), + ForkName::Eip4844 => Err(Error::UnsupportedForkVariant("JsonExecutionPayloadV2 -> ExecutionPayloadEip4844 not implemented yet as it might never be".to_string())), + _ => Err(Error::UnsupportedForkVariant(format!("Unsupported conversion from JsonExecutionPayloadV2 for {}", fork_name))), + } + JsonExecutionPayload::V3(v3) => match fork_name { + ForkName::Merge => Ok(ExecutionPayload::Merge(ExecutionPayloadMerge { + parent_hash: v3.parent_hash, + fee_recipient: v3.fee_recipient, + state_root: v3.state_root, + receipts_root: v3.receipts_root, + logs_bloom: v3.logs_bloom, + prev_randao: v3.prev_randao, + block_number: v3.block_number, + gas_limit: v3.gas_limit, + gas_used: v3.gas_used, + timestamp: v3.timestamp, + extra_data: v3.extra_data, + base_fee_per_gas: v3.base_fee_per_gas, + block_hash: v3.block_hash, + transactions: v3.transactions, + })), + ForkName::Capella => Ok(ExecutionPayload::Capella(ExecutionPayloadCapella { + parent_hash: v3.parent_hash, + fee_recipient: v3.fee_recipient, + state_root: v3.state_root, + receipts_root: v3.receipts_root, + logs_bloom: v3.logs_bloom, + prev_randao: v3.prev_randao, + block_number: v3.block_number, + gas_limit: v3.gas_limit, + gas_used: v3.gas_used, + timestamp: v3.timestamp, + extra_data: v3.extra_data, + base_fee_per_gas: v3.base_fee_per_gas, + block_hash: v3.block_hash, + transactions: v3.transactions, + #[cfg(feature = "withdrawals")] + withdrawals: v3 + .withdrawals + .map(|v| { + Into::>::into(v) + .into_iter() + .map(Into::into) + .collect::>() + .into() + }) + .ok_or(Error::BadConversion("Null withdrawal field converting JsonExecutionPayloadV3 -> ExecutionPayloadCapella".to_string()))? + })), + ForkName::Eip4844 => Ok(ExecutionPayload::Eip4844(ExecutionPayloadEip4844 { + parent_hash: v3.parent_hash, + fee_recipient: v3.fee_recipient, + state_root: v3.state_root, + receipts_root: v3.receipts_root, + logs_bloom: v3.logs_bloom, + prev_randao: v3.prev_randao, + block_number: v3.block_number, + gas_limit: v3.gas_limit, + gas_used: v3.gas_used, + timestamp: v3.timestamp, + extra_data: v3.extra_data, + base_fee_per_gas: v3.base_fee_per_gas, + // FIXME: excess_blobs probably will be an option whenever the engine API is finalized + excess_blobs: v3.excess_blobs, + block_hash: v3.block_hash, + transactions: v3.transactions, + #[cfg(feature = "withdrawals")] + withdrawals: v3 + .withdrawals + .map(|v| { + Vec::from(v) + .into_iter() + .map(Into::into) + .collect::>() + .into() + }) + .ok_or(Error::BadConversion("Null withdrawal field converting JsonExecutionPayloadV3 -> ExecutionPayloadEip4844".to_string()))?, + })), + _ => Err(Error::UnsupportedForkVariant(format!("Unsupported conversion from JsonExecutionPayloadV2 for {}", fork_name))), + } } } } -impl From> for JsonExecutionPayload { - fn from(payload: ExecutionPayload) -> Self { +impl TryFrom> for JsonExecutionPayloadV1 { + type Error = Error; + fn try_from(payload: ExecutionPayload) -> Result { match payload { - ExecutionPayload::Merge(merge) => Self::V1(JsonExecutionPayloadV1 { + ExecutionPayload::Merge(merge) => Ok(JsonExecutionPayloadV1 { parent_hash: merge.parent_hash, fee_recipient: merge.fee_recipient, state_root: merge.state_root, @@ -347,7 +277,40 @@ impl From> for JsonExecutionPayload { block_hash: merge.block_hash, transactions: merge.transactions, }), - ExecutionPayload::Capella(capella) => Self::V2(JsonExecutionPayloadV2 { + 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 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, + 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, @@ -363,27 +326,20 @@ impl From> for JsonExecutionPayload { block_hash: capella.block_hash, transactions: capella.transactions, #[cfg(feature = "withdrawals")] - withdrawals: capella.withdrawals, - }), - ExecutionPayload::Eip4844(eip4844) => Self::V3(JsonExecutionPayloadV3 { - 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_blobs: eip4844.excess_blobs, - block_hash: eip4844.block_hash, - transactions: eip4844.transactions, - #[cfg(feature = "withdrawals")] - withdrawals: eip4844.withdrawals, + withdrawals: Some( + Vec::from(capella.withdrawals) + .into_iter() + .map(Into::into) + .collect::>() + .into(), + ), + #[cfg(not(feature = "withdrawals"))] + withdrawals: None, }), + ExecutionPayload::Eip4844(_) => Err(Error::UnsupportedForkVariant(format!( + "Unsupported conversion to JsonExecutionPayloadV1 for {}", + ForkName::Eip4844 + ))), } } } @@ -424,12 +380,15 @@ impl From for Withdrawal { #[superstruct( variants(V1, V2), - variant_attributes(derive(Clone, Debug, PartialEq, Serialize, Deserialize),), + variant_attributes( + derive(Clone, Debug, PartialEq, Serialize, Deserialize), + serde(rename_all = "camelCase") + ), cast_error(ty = "Error", expr = "Error::IncorrectStateVariant"), partial_getter_error(ty = "Error", expr = "Error::IncorrectStateVariant") )] #[derive(Debug, PartialEq, Serialize, Deserialize)] -#[serde(rename_all = "camelCase", untagged)] +#[serde(untagged)] pub struct JsonPayloadAttributes { #[serde(with = "eth2_serde_utils::u64_hex_be")] pub timestamp: u64, @@ -437,7 +396,7 @@ pub struct JsonPayloadAttributes { pub suggested_fee_recipient: Address, #[cfg(feature = "withdrawals")] #[superstruct(only(V2))] - pub withdrawals: Vec, + pub withdrawals: Option>, } impl From for JsonPayloadAttributes { @@ -453,7 +412,9 @@ impl From for JsonPayloadAttributes { prev_randao: pa.prev_randao, suggested_fee_recipient: pa.suggested_fee_recipient, #[cfg(feature = "withdrawals")] - withdrawals: pa.withdrawals.into_iter().map(Into::into).collect(), + withdrawals: pa + .withdrawals + .map(|w| w.into_iter().map(Into::into).collect()), }), } } @@ -472,7 +433,9 @@ impl From for PayloadAttributes { prev_randao: jpa.prev_randao, suggested_fee_recipient: jpa.suggested_fee_recipient, #[cfg(feature = "withdrawals")] - withdrawals: jpa.withdrawals.into_iter().map(Into::into).collect(), + withdrawals: jpa + .withdrawals + .map(|jw| jw.into_iter().map(Into::into).collect()), }), } } @@ -488,16 +451,16 @@ pub struct JsonBlobBundles { #[derive(Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -pub struct JsonForkChoiceStateV1 { +pub struct JsonForkchoiceStateV1 { pub head_block_hash: ExecutionBlockHash, pub safe_block_hash: ExecutionBlockHash, pub finalized_block_hash: ExecutionBlockHash, } -impl From for JsonForkChoiceStateV1 { - fn from(f: ForkChoiceState) -> Self { +impl From for JsonForkchoiceStateV1 { + fn from(f: ForkchoiceState) -> Self { // Use this verbose deconstruction pattern to ensure no field is left unused. - let ForkChoiceState { + let ForkchoiceState { head_block_hash, safe_block_hash, finalized_block_hash, @@ -511,10 +474,10 @@ impl From for JsonForkChoiceStateV1 { } } -impl From for ForkChoiceState { - fn from(j: JsonForkChoiceStateV1) -> Self { +impl From for ForkchoiceState { + fn from(j: JsonForkchoiceStateV1) -> Self { // Use this verbose deconstruction pattern to ensure no field is left unused. - let JsonForkChoiceStateV1 { + let JsonForkchoiceStateV1 { head_block_hash, safe_block_hash, finalized_block_hash, diff --git a/beacon_node/execution_layer/src/engines.rs b/beacon_node/execution_layer/src/engines.rs index da77bd9cf..264303b5d 100644 --- a/beacon_node/execution_layer/src/engines.rs +++ b/beacon_node/execution_layer/src/engines.rs @@ -88,7 +88,7 @@ impl State { } #[derive(Copy, Clone, PartialEq, Debug)] -pub struct ForkChoiceState { +pub struct ForkchoiceState { pub head_block_hash: ExecutionBlockHash, pub safe_block_hash: ExecutionBlockHash, pub finalized_block_hash: ExecutionBlockHash, @@ -115,7 +115,7 @@ pub struct Engine { pub api: HttpJsonRpc, payload_id_cache: Mutex>, state: RwLock, - latest_forkchoice_state: RwLock>, + latest_forkchoice_state: RwLock>, executor: TaskExecutor, log: Logger, } @@ -161,13 +161,13 @@ impl Engine { pub async fn notify_forkchoice_updated( &self, - forkchoice_state: ForkChoiceState, + forkchoice_state: ForkchoiceState, payload_attributes: Option, log: &Logger, ) -> Result { let response = self .api - .forkchoice_updated_v1(forkchoice_state, payload_attributes.clone()) + .forkchoice_updated(forkchoice_state, payload_attributes.clone()) .await?; if let Some(payload_id) = response.payload_id { @@ -187,11 +187,11 @@ impl Engine { Ok(response) } - async fn get_latest_forkchoice_state(&self) -> Option { + async fn get_latest_forkchoice_state(&self) -> Option { *self.latest_forkchoice_state.read().await } - pub async fn set_latest_forkchoice_state(&self, state: ForkChoiceState) { + pub async fn set_latest_forkchoice_state(&self, state: ForkchoiceState) { *self.latest_forkchoice_state.write().await = Some(state); } @@ -216,7 +216,7 @@ impl Engine { // For simplicity, payload attributes are never included in this call. It may be // reasonable to include them in the future. - if let Err(e) = self.api.forkchoice_updated_v1(forkchoice_state, None).await { + if let Err(e) = self.api.forkchoice_updated(forkchoice_state, None).await { debug!( self.log, "Failed to issue latest head to engine"; @@ -349,7 +349,7 @@ impl Engine { // TODO: revisit this - do we need to key on withdrawals as well here? impl PayloadIdCacheKey { - fn new(state: &ForkChoiceState, attributes: &PayloadAttributes) -> Self { + fn new(state: &ForkchoiceState, attributes: &PayloadAttributes) -> Self { Self { head_block_hash: state.head_block_hash, timestamp: attributes.timestamp(), diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 04bdb4a20..2a19c4165 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -12,7 +12,7 @@ use engine_api::Error as ApiError; pub use engine_api::*; pub use engine_api::{http, http::deposit_methods, http::HttpJsonRpc}; use engines::{Engine, EngineError}; -pub use engines::{EngineState, ForkChoiceState}; +pub use engines::{EngineState, ForkchoiceState}; use fork_choice::ForkchoiceUpdateParameters; use lru::LruCache; use payload_status::process_payload_status; @@ -33,6 +33,8 @@ use tokio::{ time::sleep, }; use tokio_stream::wrappers::WatchStream; +#[cfg(feature = "withdrawals")] +use types::Withdrawal; use types::{AbstractExecPayload, Blob, ExecPayload, ExecutionPayloadEip4844, KzgCommitment}; use types::{ BlindedPayload, BlockType, ChainSpec, Epoch, ExecutionBlockHash, ForkName, @@ -613,6 +615,8 @@ impl ExecutionLayer { proposer_index: u64, forkchoice_update_params: ForkchoiceUpdateParameters, builder_params: BuilderParams, + current_fork: ForkName, + #[cfg(feature = "withdrawals")] withdrawals: Option>, spec: &ChainSpec, ) -> Result, Error> { let suggested_fee_recipient = self.get_suggested_fee_recipient(proposer_index).await; @@ -630,6 +634,9 @@ impl ExecutionLayer { suggested_fee_recipient, forkchoice_update_params, builder_params, + current_fork, + #[cfg(feature = "withdrawals")] + withdrawals, spec, ) .await @@ -645,6 +652,9 @@ impl ExecutionLayer { prev_randao, suggested_fee_recipient, forkchoice_update_params, + current_fork, + #[cfg(feature = "withdrawals")] + withdrawals, ) .await } @@ -660,6 +670,8 @@ impl ExecutionLayer { suggested_fee_recipient: Address, forkchoice_update_params: ForkchoiceUpdateParameters, builder_params: BuilderParams, + current_fork: ForkName, + #[cfg(feature = "withdrawals")] withdrawals: Option>, spec: &ChainSpec, ) -> Result, Error> { if let Some(builder) = self.builder() { @@ -683,6 +695,9 @@ impl ExecutionLayer { prev_randao, suggested_fee_recipient, forkchoice_update_params, + current_fork, + #[cfg(feature = "withdrawals")] + withdrawals, ) ); @@ -812,6 +827,9 @@ impl ExecutionLayer { prev_randao, suggested_fee_recipient, forkchoice_update_params, + current_fork, + #[cfg(feature = "withdrawals")] + withdrawals, ) .await } @@ -824,6 +842,8 @@ impl ExecutionLayer { prev_randao: Hash256, suggested_fee_recipient: Address, forkchoice_update_params: ForkchoiceUpdateParameters, + current_fork: ForkName, + #[cfg(feature = "withdrawals")] withdrawals: Option>, ) -> Result, Error> { self.get_full_payload_with( parent_hash, @@ -831,6 +851,9 @@ impl ExecutionLayer { prev_randao, suggested_fee_recipient, forkchoice_update_params, + current_fork, + #[cfg(feature = "withdrawals")] + withdrawals, noop, ) .await @@ -844,6 +867,8 @@ impl ExecutionLayer { prev_randao: Hash256, suggested_fee_recipient: Address, forkchoice_update_params: ForkchoiceUpdateParameters, + current_fork: ForkName, + #[cfg(feature = "withdrawals")] withdrawals: Option>, ) -> Result, Error> { self.get_full_payload_with( parent_hash, @@ -851,6 +876,9 @@ impl ExecutionLayer { prev_randao, suggested_fee_recipient, forkchoice_update_params, + current_fork, + #[cfg(feature = "withdrawals")] + withdrawals, Self::cache_payload, ) .await @@ -863,10 +891,14 @@ impl ExecutionLayer { prev_randao: Hash256, suggested_fee_recipient: Address, forkchoice_update_params: ForkchoiceUpdateParameters, + current_fork: ForkName, + #[cfg(feature = "withdrawals")] withdrawals: Option>, f: fn(&ExecutionLayer, &ExecutionPayload) -> Option>, ) -> Result, Error> { + #[cfg(feature = "withdrawals")] + let withdrawals_ref = &withdrawals; self.engine() - .request(|engine| async move { + .request(move |engine| async move { let payload_id = if let Some(id) = engine .get_payload_id(parent_hash, timestamp, prev_randao, suggested_fee_recipient) .await @@ -884,7 +916,7 @@ impl ExecutionLayer { &metrics::EXECUTION_LAYER_PRE_PREPARED_PAYLOAD_ID, &[metrics::MISS], ); - let fork_choice_state = ForkChoiceState { + let fork_choice_state = ForkchoiceState { head_block_hash: parent_hash, safe_block_hash: forkchoice_update_params .justified_hash @@ -893,12 +925,14 @@ impl ExecutionLayer { .finalized_hash .unwrap_or_else(ExecutionBlockHash::zero), }; - // FIXME: This will have to properly handle forks. To do that, - // withdrawals will need to be passed into this function - let payload_attributes = PayloadAttributes::V1(PayloadAttributesV1 { + // This must always be the latest PayloadAttributes + // FIXME: How to non-capella EIP4844 testnets handle this? + let payload_attributes = PayloadAttributes::V2(PayloadAttributesV2 { timestamp, prev_randao, suggested_fee_recipient, + #[cfg(feature = "withdrawals")] + withdrawals: withdrawals_ref.clone(), }); let response = engine @@ -925,7 +959,11 @@ impl ExecutionLayer { }; let blob_fut = async { - //FIXME(sean) do a fork check here and return None otherwise + //FIXME(sean) do a fork check here and return None otherwise + // ^ + // well now we have the fork in this function so + // it should be easier to do that now + // - Mark debug!( self.log(), "Issuing engine_getBlobsBundle"; @@ -945,9 +983,8 @@ impl ExecutionLayer { "timestamp" => timestamp, "parent_hash" => ?parent_hash, ); - engine.api.get_payload_v1::(payload_id).await + engine.api.get_payload::(current_fork, payload_id).await }; - let (blob, payload) = tokio::join!(blob_fut, payload_fut); let payload = payload.map(|full_payload| { if full_payload.fee_recipient() != suggested_fee_recipient { @@ -1020,7 +1057,7 @@ impl ExecutionLayer { let result = self .engine() - .request(|engine| engine.api.new_payload_v1(execution_payload.clone())) + .request(|engine| engine.api.new_payload(execution_payload.clone())) .await; if let Ok(status) = &result { @@ -1150,7 +1187,7 @@ impl ExecutionLayer { } } - let forkchoice_state = ForkChoiceState { + let forkchoice_state = ForkchoiceState { head_block_hash, safe_block_hash: justified_block_hash, finalized_block_hash, 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 37eb8ba8f..f2282c603 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 @@ -1,4 +1,4 @@ -use crate::engines::ForkChoiceState; +use crate::engines::ForkchoiceState; use crate::{ engine_api::{ json_structures::{ @@ -13,8 +13,7 @@ use std::collections::HashMap; use tree_hash::TreeHash; use tree_hash_derive::TreeHash; use types::{ - EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadCapella, ExecutionPayloadMerge, - Hash256, Uint256, + EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadMerge, Hash256, Uint256, }; const GAS_LIMIT: u64 = 16384; @@ -315,7 +314,7 @@ impl ExecutionBlockGenerator { pub fn forkchoice_updated_v1( &mut self, - forkchoice_state: ForkChoiceState, + forkchoice_state: ForkchoiceState, payload_attributes: Option, ) -> Result { if let Some(payload) = self @@ -369,7 +368,6 @@ impl ExecutionBlockGenerator { let id = payload_id_from_u64(self.next_payload_id); self.next_payload_id += 1; - // FIXME: think about how to test different forks let mut execution_payload = match &attributes { PayloadAttributes::V1(pa) => ExecutionPayload::Merge(ExecutionPayloadMerge { parent_hash: forkchoice_state.head_block_hash, @@ -388,7 +386,8 @@ impl ExecutionBlockGenerator { transactions: vec![].into(), }), PayloadAttributes::V2(pa) => { - ExecutionPayload::Capella(ExecutionPayloadCapella { + // FIXME: think about how to test different forks + ExecutionPayload::Merge(ExecutionPayloadMerge { parent_hash: forkchoice_state.head_block_hash, fee_recipient: pa.suggested_fee_recipient, receipts_root: Hash256::repeat_byte(42), @@ -403,14 +402,6 @@ impl ExecutionBlockGenerator { base_fee_per_gas: Uint256::one(), block_hash: ExecutionBlockHash::zero(), transactions: vec![].into(), - #[cfg(feature = "withdrawals")] - withdrawals: pa - .withdrawals - .iter() - .cloned() - .map(Into::into) - .collect::>() - .into(), }) } }; 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 ba26591ba..fe765cc94 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -4,7 +4,7 @@ use crate::json_structures::*; use serde::de::DeserializeOwned; use serde_json::Value as JsonValue; use std::sync::Arc; -use types::EthSpec; +use types::{EthSpec, ForkName}; pub async fn handle_rpc( body: JsonValue, @@ -97,7 +97,8 @@ pub async fn handle_rpc( Some( ctx.execution_block_generator .write() - .new_payload(request.into()), + // FIXME: should this worry about other forks? + .new_payload(request.try_into_execution_payload(ForkName::Merge).unwrap()), ) } else { None @@ -117,10 +118,10 @@ pub async fn handle_rpc( .get_payload(&id) .ok_or_else(|| format!("no payload for id {:?}", id))?; - Ok(serde_json::to_value(JsonExecutionPayload::from(response)).unwrap()) + Ok(serde_json::to_value(JsonExecutionPayloadV1::try_from(response).unwrap()).unwrap()) } ENGINE_FORKCHOICE_UPDATED_V1 => { - let forkchoice_state: JsonForkChoiceStateV1 = get_param(params, 0)?; + let forkchoice_state: JsonForkchoiceStateV1 = get_param(params, 0)?; let payload_attributes: Option = get_param(params, 1)?; let head_block_hash = forkchoice_state.head_block_hash; diff --git a/beacon_node/execution_layer/src/test_utils/mock_builder.rs b/beacon_node/execution_layer/src/test_utils/mock_builder.rs index 1323ea3e4..5c69fffbf 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_builder.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_builder.rs @@ -26,7 +26,8 @@ use task_executor::TaskExecutor; use tempfile::NamedTempFile; use tree_hash::TreeHash; use types::{ - Address, BeaconState, BlindedPayload, ChainSpec, EthSpec, ExecPayload, Hash256, Slot, Uint256, + Address, BeaconState, BlindedPayload, ChainSpec, EthSpec, ExecPayload, ForkName, Hash256, Slot, + Uint256, }; #[derive(Clone)] @@ -313,6 +314,10 @@ impl mev_build_rs::BlindedBlockProvider for MockBuilder { *prev_randao, fee_recipient, forkchoice_update_params, + // TODO: do we need to write a test for this if this is Capella fork? + ForkName::Merge, + #[cfg(feature = "withdrawals")] + None, ) .await .map_err(convert_err)? diff --git a/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs index 62336279b..cadeec1b3 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs @@ -114,7 +114,7 @@ impl MockExecutionLayer { suggested_fee_recipient: Address::repeat_byte(42), // FIXME: think about adding withdrawals here.. #[cfg(feature = "withdrawals")] - withdrawals: vec![], + withdrawals: Some(vec![]), }) } }, @@ -159,6 +159,10 @@ impl MockExecutionLayer { validator_index, forkchoice_update_params, builder_params, + // FIXME: do we need to consider other forks somehow? What about withdrawals? + ForkName::Merge, + #[cfg(feature = "withdrawals")] + Some(vec![]), &self.spec, ) .await @@ -191,6 +195,10 @@ impl MockExecutionLayer { validator_index, forkchoice_update_params, builder_params, + // FIXME: do we need to consider other forks somehow? What about withdrawals? + ForkName::Merge, + #[cfg(feature = "withdrawals")] + Some(vec![]), &self.spec, ) .await diff --git a/common/eth2/Cargo.toml b/common/eth2/Cargo.toml index eca086d83..6ee02b71b 100644 --- a/common/eth2/Cargo.toml +++ b/common/eth2/Cargo.toml @@ -35,3 +35,5 @@ procinfo = { version = "0.4.2", optional = true } [features] default = ["lighthouse"] lighthouse = ["proto_array", "psutil", "procinfo", "store", "slashing_protection"] +withdrawals = ["store/withdrawals"] +withdrawals-processing = ["store/withdrawals-processing"] \ No newline at end of file diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 5e59a0132..753a19398 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -462,7 +462,7 @@ pub fn compute_timestamp_at_slot( } /// FIXME: add link to this function once the spec is stable -#[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] +#[cfg(feature = "withdrawals")] pub fn get_expected_withdrawals( state: &BeaconState, spec: &ChainSpec, @@ -472,6 +472,10 @@ pub fn get_expected_withdrawals( let mut validator_index = state.next_withdrawal_validator_index()?; let mut withdrawals = vec![]; + if cfg!(not(feature = "withdrawals-processing")) { + return Ok(withdrawals.into()); + } + for _ in 0..state.validators().len() { let validator = state.get_validator(validator_index as usize)?; let balance = *state.balances().get(validator_index as usize).ok_or( diff --git a/testing/ef_tests/src/cases/operations.rs b/testing/ef_tests/src/cases/operations.rs index 8c2ddd136..9e3562bc7 100644 --- a/testing/ef_tests/src/cases/operations.rs +++ b/testing/ef_tests/src/cases/operations.rs @@ -4,15 +4,19 @@ use crate::case_result::compare_beacon_state_results_without_caches; use crate::decode::{ssz_decode_file, ssz_decode_file_with, ssz_decode_state, yaml_decode_file}; use crate::testing_spec; use serde_derive::Deserialize; +#[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] +use state_processing::per_block_processing::process_operations::{ + process_bls_to_execution_changes, process_bls_to_execution_changes, +}; use state_processing::{ per_block_processing::{ errors::BlockProcessingError, process_block_header, process_execution_payload, process_operations::{ - altair, base, process_attester_slashings, process_bls_to_execution_changes, - process_deposits, process_exits, process_proposer_slashings, + altair, base, process_attester_slashings, process_deposits, process_exits, + process_proposer_slashings, }, - process_sync_aggregate, process_withdrawals, VerifyBlockRoot, VerifySignatures, + process_sync_aggregate, VerifyBlockRoot, VerifySignatures, }, ConsensusContext, }; @@ -340,6 +344,7 @@ impl Operation for BlindedPayload { } } +#[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] impl Operation for WithdrawalsPayload { fn handler_name() -> String { "withdrawals".into() @@ -372,6 +377,7 @@ impl Operation for WithdrawalsPayload { } } +#[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] impl Operation for SignedBlsToExecutionChange { fn handler_name() -> String { "bls_to_execution_change".into() diff --git a/testing/execution_engine_integration/Cargo.toml b/testing/execution_engine_integration/Cargo.toml index a85138be9..b5923aafe 100644 --- a/testing/execution_engine_integration/Cargo.toml +++ b/testing/execution_engine_integration/Cargo.toml @@ -21,3 +21,7 @@ deposit_contract = { path = "../../common/deposit_contract" } reqwest = { version = "0.11.0", features = ["json"] } hex = "0.4.2" fork_choice = { path = "../../consensus/fork_choice" } + +[features] +default = [] +withdrawals = [] \ No newline at end of file diff --git a/testing/execution_engine_integration/src/test_rig.rs b/testing/execution_engine_integration/src/test_rig.rs index b3464ec98..9ef96687a 100644 --- a/testing/execution_engine_integration/src/test_rig.rs +++ b/testing/execution_engine_integration/src/test_rig.rs @@ -16,8 +16,8 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use task_executor::TaskExecutor; use tokio::time::sleep; use types::{ - Address, ChainSpec, EthSpec, ExecutionBlockHash, ExecutionPayload, FullPayload, Hash256, - MainnetEthSpec, PublicKeyBytes, Slot, Uint256, + Address, ChainSpec, EthSpec, ExecutionBlockHash, ExecutionPayload, ForkName, FullPayload, + Hash256, MainnetEthSpec, PublicKeyBytes, Slot, Uint256, }; const EXECUTION_ENGINE_START_TIMEOUT: Duration = Duration::from_secs(20); @@ -326,6 +326,10 @@ impl TestRig { proposer_index, forkchoice_update_params, builder_params, + // FIXME: think about how to test other forks + ForkName::Merge, + #[cfg(feature = "withdrawals")] + None, &self.spec, ) .await @@ -450,6 +454,10 @@ impl TestRig { proposer_index, forkchoice_update_params, builder_params, + // FIXME: think about how to test other forks + ForkName::Merge, + #[cfg(feature = "withdrawals")] + None, &self.spec, ) .await