diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index b28d0b81f8d..dc4425b862e 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -234,6 +234,7 @@ pub struct PrePayloadAttributes { /// The parent block number is not part of the payload attributes sent to the EL, but *is* /// sent to builders via SSE. pub parent_block_number: u64, + pub parent_beacon_block_root: Hash256, } /// Information about a state/block at a specific slot. @@ -4171,6 +4172,7 @@ impl BeaconChain { proposer_index, prev_randao, parent_block_number, + parent_beacon_block_root: parent_block_root, })) } @@ -5229,7 +5231,8 @@ impl BeaconChain { { payload_attributes } else { - let withdrawals = match self.spec.fork_name_at_slot::(prepare_slot) { + let prepare_slot_fork = self.spec.fork_name_at_slot::(prepare_slot); + let withdrawals = match prepare_slot_fork { ForkName::Base | ForkName::Altair | ForkName::Merge => None, ForkName::Capella | ForkName::Deneb => { let chain = self.clone(); @@ -5244,6 +5247,11 @@ impl BeaconChain { } }; + let parent_beacon_block_root = match prepare_slot_fork { + ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => None, + ForkName::Deneb => Some(pre_payload_attributes.parent_beacon_block_root), + }; + let payload_attributes = PayloadAttributes::new( self.slot_clock .start_of(prepare_slot) @@ -5252,6 +5260,7 @@ impl BeaconChain { pre_payload_attributes.prev_randao, execution_layer.get_suggested_fee_recipient(proposer).await, withdrawals.map(Into::into), + parent_beacon_block_root, ); execution_layer diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 1166de1e563..237f2d7ecca 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -12,13 +12,15 @@ use crate::{ BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, BlockProductionError, ExecutionPayloadError, }; -use execution_layer::{BlockProposalContents, BuilderParams, PayloadAttributes, PayloadStatus}; +use execution_layer::{ + BlockProposalContents, BuilderParams, NewPayloadRequest, PayloadAttributes, PayloadStatus, +}; use fork_choice::{InvalidationOperation, PayloadVerificationStatus}; use proto_array::{Block as ProtoBlock, ExecutionStatus}; use slog::{debug, warn}; use slot_clock::SlotClock; use state_processing::per_block_processing::{ - self, compute_timestamp_at_slot, get_expected_withdrawals, is_execution_enabled, + compute_timestamp_at_slot, get_expected_withdrawals, is_execution_enabled, is_merge_transition_complete, partially_verify_execution_payload, }; use std::sync::Arc; @@ -139,23 +141,15 @@ async fn notify_new_payload<'a, T: BeaconChainTypes>( chain: &Arc>, block: BeaconBlockRef<'a, T::EthSpec>, ) -> Result> { - let execution_payload = block.execution_payload()?; - let versioned_hashes = block.body().blob_kzg_commitments().ok().map(|commitments| { - commitments - .into_iter() - .map(|commitment| { - per_block_processing::deneb::deneb::kzg_commitment_to_versioned_hash(commitment) - }) - .collect::>() - }); - let execution_layer = chain .execution_layer .as_ref() .ok_or(ExecutionPayloadError::NoExecutionConnection)?; + let new_payload_request: NewPayloadRequest = block.try_into()?; + let execution_block_hash = new_payload_request.block_hash(); let new_payload_response = execution_layer - .notify_new_payload(&execution_payload.into(), versioned_hashes) + .notify_new_payload(new_payload_request) .await; match new_payload_response { @@ -173,7 +167,7 @@ async fn notify_new_payload<'a, T: BeaconChainTypes>( "Invalid execution payload"; "validation_error" => ?validation_error, "latest_valid_hash" => ?latest_valid_hash, - "execution_block_hash" => ?execution_payload.block_hash(), + "execution_block_hash" => ?execution_block_hash, "root" => ?block.tree_hash_root(), "graffiti" => block.body().graffiti().as_utf8_lossy(), "proposer_index" => block.proposer_index(), @@ -219,7 +213,7 @@ async fn notify_new_payload<'a, T: BeaconChainTypes>( chain.log, "Invalid execution payload block hash"; "validation_error" => ?validation_error, - "execution_block_hash" => ?execution_payload.block_hash(), + "execution_block_hash" => ?execution_block_hash, "root" => ?block.tree_hash_root(), "graffiti" => block.body().graffiti().as_utf8_lossy(), "proposer_index" => block.proposer_index(), @@ -435,6 +429,12 @@ pub fn get_execution_payload< // These shouldn't happen but they're here to make the pattern irrefutable &BeaconState::Base(_) | &BeaconState::Altair(_) => None, }; + let parent_beacon_block_root = match state { + &BeaconState::Deneb(_) => Some(state.latest_block_header().canonical_root()), + &BeaconState::Merge(_) | &BeaconState::Capella(_) => 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. @@ -452,6 +452,7 @@ pub fn get_execution_payload< latest_execution_payload_header_block_hash, builder_params, withdrawals, + parent_beacon_block_root, ) .await }, @@ -486,6 +487,7 @@ pub async fn prepare_execution_payload( latest_execution_payload_header_block_hash: ExecutionBlockHash, builder_params: BuilderParams, withdrawals: Option>, + parent_beacon_block_root: Option, ) -> Result, BlockProductionError> where T: BeaconChainTypes, @@ -547,8 +549,13 @@ where let suggested_fee_recipient = execution_layer .get_suggested_fee_recipient(proposer_index) .await; - let payload_attributes = - PayloadAttributes::new(timestamp, random, suggested_fee_recipient, withdrawals); + let payload_attributes = PayloadAttributes::new( + timestamp, + random, + suggested_fee_recipient, + withdrawals, + parent_beacon_block_root, + ); // Note: the suggested_fee_recipient is stored in the `execution_layer`, it will add this parameter. // diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 60ba8f288e2..7d7525bc140 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -1017,6 +1017,7 @@ async fn payload_preparation() { .unwrap(), fee_recipient, None, + None, ); assert_eq!(rig.previous_payload_attributes(), payload_attributes); } diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 5b0c0d13643..45cf2004659 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -1,12 +1,15 @@ use crate::engines::ForkchoiceState; use crate::http::{ ENGINE_EXCHANGE_TRANSITION_CONFIGURATION_V1, ENGINE_FORKCHOICE_UPDATED_V1, - ENGINE_FORKCHOICE_UPDATED_V2, 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_FORKCHOICE_UPDATED_V2, ENGINE_FORKCHOICE_UPDATED_V3, + 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, }; use crate::BlobTxConversionError; -use eth2::types::{SsePayloadAttributes, SsePayloadAttributesV1, SsePayloadAttributesV2}; +use eth2::types::{ + SsePayloadAttributes, SsePayloadAttributesV1, SsePayloadAttributesV2, SsePayloadAttributesV3, +}; use ethers_core::types::Transaction; use ethers_core::utils::rlp::{self, Decodable, Rlp}; use http::deposit_methods::RpcError; @@ -14,17 +17,21 @@ pub use json_structures::{JsonWithdrawal, TransitionConfigurationV1}; use pretty_reqwest_error::PrettyReqwestError; use reqwest::StatusCode; use serde::{Deserialize, Serialize}; +use state_processing::per_block_processing::deneb::deneb::kzg_commitment_to_versioned_hash; use std::convert::TryFrom; use strum::IntoStaticStr; use superstruct::superstruct; use types::beacon_block_body::KzgCommitments; use types::blob_sidecar::Blobs; pub use types::{ - Address, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadHeader, + Address, BeaconBlockRef, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadHeader, ExecutionPayloadRef, FixedVector, ForkName, Hash256, Transactions, Uint256, VariableList, Withdrawal, Withdrawals, }; -use types::{ExecutionPayloadCapella, ExecutionPayloadDeneb, ExecutionPayloadMerge, KzgProofs}; +use types::{ + BeaconStateError, ExecutionPayloadCapella, ExecutionPayloadDeneb, ExecutionPayloadMerge, + KzgProofs, VersionedHash, +}; pub mod auth; pub mod http; @@ -280,7 +287,7 @@ impl TryFrom> for ExecutionBlockWithTransactions } #[superstruct( - variants(V1, V2), + variants(V1, V2, V3), variant_attributes(derive(Clone, Debug, Eq, Hash, PartialEq),), cast_error(ty = "Error", expr = "Error::IncorrectStateVariant"), partial_getter_error(ty = "Error", expr = "Error::IncorrectStateVariant") @@ -293,8 +300,10 @@ pub struct PayloadAttributes { pub prev_randao: Hash256, #[superstruct(getter(copy))] pub suggested_fee_recipient: Address, - #[superstruct(only(V2))] + #[superstruct(only(V2, V3))] pub withdrawals: Vec, + #[superstruct(only(V3), partial_getter(copy))] + pub parent_beacon_block_root: Hash256, } impl PayloadAttributes { @@ -303,14 +312,24 @@ impl PayloadAttributes { prev_randao: Hash256, suggested_fee_recipient: Address, withdrawals: Option>, + parent_beacon_block_root: Option, ) -> Self { match withdrawals { - Some(withdrawals) => PayloadAttributes::V2(PayloadAttributesV2 { - timestamp, - prev_randao, - suggested_fee_recipient, - withdrawals, - }), + Some(withdrawals) => match parent_beacon_block_root { + Some(parent_beacon_block_root) => PayloadAttributes::V3(PayloadAttributesV3 { + timestamp, + prev_randao, + suggested_fee_recipient, + withdrawals, + parent_beacon_block_root, + }), + None => PayloadAttributes::V2(PayloadAttributesV2 { + timestamp, + prev_randao, + suggested_fee_recipient, + withdrawals, + }), + }, None => PayloadAttributes::V1(PayloadAttributesV1 { timestamp, prev_randao, @@ -343,6 +362,19 @@ impl From for SsePayloadAttributes { suggested_fee_recipient, withdrawals, }), + PayloadAttributes::V3(PayloadAttributesV3 { + timestamp, + prev_randao, + suggested_fee_recipient, + withdrawals, + parent_beacon_block_root, + }) => Self::V3(SsePayloadAttributesV3 { + timestamp, + prev_randao, + suggested_fee_recipient, + withdrawals, + parent_beacon_block_root, + }), } } } @@ -392,27 +424,15 @@ pub struct GetPayloadResponse { impl GetPayloadResponse { pub fn fee_recipient(&self) -> Address { - match self { - GetPayloadResponse::Merge(inner) => inner.execution_payload.fee_recipient, - GetPayloadResponse::Capella(inner) => inner.execution_payload.fee_recipient, - GetPayloadResponse::Deneb(inner) => inner.execution_payload.fee_recipient, - } + ExecutionPayloadRef::from(self.to_ref()).fee_recipient() } pub fn block_hash(&self) -> ExecutionBlockHash { - match self { - GetPayloadResponse::Merge(inner) => inner.execution_payload.block_hash, - GetPayloadResponse::Capella(inner) => inner.execution_payload.block_hash, - GetPayloadResponse::Deneb(inner) => inner.execution_payload.block_hash, - } + ExecutionPayloadRef::from(self.to_ref()).block_hash() } pub fn block_number(&self) -> u64 { - match self { - GetPayloadResponse::Merge(inner) => inner.execution_payload.block_number, - GetPayloadResponse::Capella(inner) => inner.execution_payload.block_number, - GetPayloadResponse::Deneb(inner) => inner.execution_payload.block_number, - } + ExecutionPayloadRef::from(self.to_ref()).block_number() } } @@ -563,6 +583,110 @@ pub struct BlobsBundleV1 { pub blobs: Blobs, } +#[superstruct( + variants(Merge, Capella, Deneb), + variant_attributes(derive(Clone, Debug, PartialEq),), + map_into(ExecutionPayload), + map_ref_into(ExecutionPayloadRef), + cast_error( + ty = "BeaconStateError", + expr = "BeaconStateError::IncorrectStateVariant" + ), + partial_getter_error( + ty = "BeaconStateError", + expr = "BeaconStateError::IncorrectStateVariant" + ) +)] +#[derive(Clone, Debug, PartialEq)] +pub struct NewPayloadRequest { + #[superstruct(only(Merge), partial_getter(rename = "execution_payload_merge"))] + pub execution_payload: ExecutionPayloadMerge, + #[superstruct(only(Capella), partial_getter(rename = "execution_payload_capella"))] + pub execution_payload: ExecutionPayloadCapella, + #[superstruct(only(Deneb), partial_getter(rename = "execution_payload_deneb"))] + pub execution_payload: ExecutionPayloadDeneb, + #[superstruct(only(Deneb))] + pub versioned_hashes: Vec, + #[superstruct(only(Deneb))] + pub parent_beacon_block_root: Hash256, +} + +impl NewPayloadRequest { + pub fn parent_hash(&self) -> ExecutionBlockHash { + match self { + Self::Merge(payload) => payload.execution_payload.parent_hash, + Self::Capella(payload) => payload.execution_payload.parent_hash, + Self::Deneb(payload) => payload.execution_payload.parent_hash, + } + } + + pub fn block_hash(&self) -> ExecutionBlockHash { + match self { + Self::Merge(payload) => payload.execution_payload.block_hash, + Self::Capella(payload) => payload.execution_payload.block_hash, + Self::Deneb(payload) => payload.execution_payload.block_hash, + } + } + + pub fn block_number(&self) -> u64 { + match self { + Self::Merge(payload) => payload.execution_payload.block_number, + Self::Capella(payload) => payload.execution_payload.block_number, + Self::Deneb(payload) => payload.execution_payload.block_number, + } + } + + pub fn into_execution_payload(self) -> ExecutionPayload { + map_new_payload_request_into_execution_payload!(self, |request, cons| { + cons(request.execution_payload) + }) + } +} + +impl<'a, E: EthSpec> TryFrom> for NewPayloadRequest { + type Error = BeaconStateError; + + fn try_from(block: BeaconBlockRef<'a, E>) -> Result { + match block { + BeaconBlockRef::Base(_) | BeaconBlockRef::Altair(_) => { + Err(Self::Error::IncorrectStateVariant) + } + BeaconBlockRef::Merge(block_ref) => Ok(Self::Merge(NewPayloadRequestMerge { + execution_payload: block_ref.body.execution_payload.execution_payload.clone(), + })), + BeaconBlockRef::Capella(block_ref) => Ok(Self::Capella(NewPayloadRequestCapella { + execution_payload: block_ref.body.execution_payload.execution_payload.clone(), + })), + BeaconBlockRef::Deneb(block_ref) => Ok(Self::Deneb(NewPayloadRequestDeneb { + execution_payload: block_ref.body.execution_payload.execution_payload.clone(), + versioned_hashes: block_ref + .body + .blob_kzg_commitments + .iter() + .map(kzg_commitment_to_versioned_hash) + .collect(), + parent_beacon_block_root: block_ref.parent_root, + })), + } + } +} + +impl TryFrom> for NewPayloadRequest { + type Error = BeaconStateError; + + fn try_from(payload: ExecutionPayload) -> Result { + match payload { + ExecutionPayload::Merge(payload) => Ok(Self::Merge(NewPayloadRequestMerge { + execution_payload: payload, + })), + ExecutionPayload::Capella(payload) => Ok(Self::Capella(NewPayloadRequestCapella { + execution_payload: payload, + })), + ExecutionPayload::Deneb(_) => Err(Self::Error::IncorrectStateVariant), + } + } +} + #[derive(Clone, Copy, Debug)] pub struct EngineCapabilities { pub new_payload_v1: bool, @@ -570,6 +694,7 @@ pub struct EngineCapabilities { pub new_payload_v3: bool, pub forkchoice_updated_v1: bool, pub forkchoice_updated_v2: bool, + pub forkchoice_updated_v3: bool, pub get_payload_bodies_by_hash_v1: bool, pub get_payload_bodies_by_range_v1: bool, pub get_payload_v1: bool, @@ -596,6 +721,9 @@ impl EngineCapabilities { if self.forkchoice_updated_v2 { response.push(ENGINE_FORKCHOICE_UPDATED_V2); } + if self.forkchoice_updated_v3 { + response.push(ENGINE_FORKCHOICE_UPDATED_V3); + } if self.get_payload_bodies_by_hash_v1 { response.push(ENGINE_GET_PAYLOAD_BODIES_BY_HASH_V1); } diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index 9f33b378954..5b85a041dd7 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -11,7 +11,7 @@ use std::collections::HashSet; use tokio::sync::Mutex; use std::time::{Duration, Instant}; -use types::{EthSpec, VersionedHash}; +use types::EthSpec; pub use deposit_log::{DepositLog, Log}; pub use reqwest::Client; @@ -42,6 +42,7 @@ pub const ENGINE_GET_PAYLOAD_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_V3: &str = "engine_forkchoiceUpdatedV3"; pub const ENGINE_FORKCHOICE_UPDATED_TIMEOUT: Duration = Duration::from_secs(8); pub const ENGINE_GET_PAYLOAD_BODIES_BY_HASH_V1: &str = "engine_getPayloadBodiesByHashV1"; @@ -85,6 +86,7 @@ pub static PRE_CAPELLA_ENGINE_CAPABILITIES: EngineCapabilities = EngineCapabilit new_payload_v3: false, forkchoice_updated_v1: true, forkchoice_updated_v2: false, + forkchoice_updated_v3: false, get_payload_bodies_by_hash_v1: false, get_payload_bodies_by_range_v1: false, get_payload_v1: true, @@ -807,12 +809,12 @@ impl HttpJsonRpc { pub async fn new_payload_v3( &self, - execution_payload: ExecutionPayloadDeneb, - versioned_hashes: Vec, + new_payload_request_deneb: NewPayloadRequestDeneb, ) -> Result { let params = json!([ - JsonExecutionPayload::V3(execution_payload.into()), - versioned_hashes + JsonExecutionPayload::V3(new_payload_request_deneb.execution_payload.into()), + new_payload_request_deneb.versioned_hashes, + new_payload_request_deneb.parent_beacon_block_root, ]); let response: JsonPayloadStatusV1 = self @@ -950,6 +952,27 @@ impl HttpJsonRpc { Ok(response.into()) } + pub async fn forkchoice_updated_v3( + &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_V3, + params, + ENGINE_FORKCHOICE_UPDATED_TIMEOUT * self.execution_timeout_multiplier, + ) + .await?; + + Ok(response.into()) + } + pub async fn get_payload_bodies_by_hash_v1( &self, block_hashes: Vec, @@ -1037,6 +1060,7 @@ impl HttpJsonRpc { new_payload_v3: capabilities.contains(ENGINE_NEW_PAYLOAD_V3), 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), get_payload_bodies_by_hash_v1: capabilities .contains(ENGINE_GET_PAYLOAD_BODIES_BY_HASH_V1), get_payload_bodies_by_range_v1: capabilities @@ -1082,27 +1106,24 @@ impl HttpJsonRpc { // new_payload that the execution engine supports pub async fn new_payload( &self, - execution_payload: ExecutionPayload, - versioned_hashes_opt: Option>, + new_payload_request: NewPayloadRequest, ) -> Result { let engine_capabilities = self.get_engine_capabilities(None).await?; - match execution_payload { - ExecutionPayload::Merge(_) | ExecutionPayload::Capella(_) => { + match new_payload_request { + NewPayloadRequest::Merge(_) | NewPayloadRequest::Capella(_) => { if engine_capabilities.new_payload_v2 { - self.new_payload_v2(execution_payload).await + self.new_payload_v2(new_payload_request.into_execution_payload()) + .await } else if engine_capabilities.new_payload_v1 { - self.new_payload_v1(execution_payload).await + self.new_payload_v1(new_payload_request.into_execution_payload()) + .await } else { Err(Error::RequiredMethodUnsupported("engine_newPayload")) } } - ExecutionPayload::Deneb(execution_payload_deneb) => { - let Some(versioned_hashes) = versioned_hashes_opt else { - return Err(Error::IncorrectStateVariant); - }; + NewPayloadRequest::Deneb(new_payload_request_deneb) => { if engine_capabilities.new_payload_v3 { - self.new_payload_v3(execution_payload_deneb, versioned_hashes) - .await + self.new_payload_v3(new_payload_request_deneb).await } else { Err(Error::RequiredMethodUnsupported("engine_newPayloadV3")) } @@ -1147,14 +1168,41 @@ impl HttpJsonRpc { pub async fn forkchoice_updated( &self, forkchoice_state: ForkchoiceState, - payload_attributes: Option, + maybe_payload_attributes: Option, ) -> Result { let engine_capabilities = self.get_engine_capabilities(None).await?; - if engine_capabilities.forkchoice_updated_v2 { - self.forkchoice_updated_v2(forkchoice_state, payload_attributes) + if let Some(payload_attributes) = maybe_payload_attributes.as_ref() { + match payload_attributes { + PayloadAttributes::V1(_) | PayloadAttributes::V2(_) => { + if engine_capabilities.forkchoice_updated_v2 { + self.forkchoice_updated_v2(forkchoice_state, maybe_payload_attributes) + .await + } else if engine_capabilities.forkchoice_updated_v1 { + self.forkchoice_updated_v1(forkchoice_state, maybe_payload_attributes) + .await + } else { + Err(Error::RequiredMethodUnsupported("engine_forkchoiceUpdated")) + } + } + PayloadAttributes::V3(_) => { + if engine_capabilities.forkchoice_updated_v3 { + self.forkchoice_updated_v3(forkchoice_state, maybe_payload_attributes) + .await + } else { + Err(Error::RequiredMethodUnsupported( + "engine_forkchoiceUpdatedV3", + )) + } + } + } + } else if engine_capabilities.forkchoice_updated_v3 { + self.forkchoice_updated_v2(forkchoice_state, maybe_payload_attributes) + .await + } else if engine_capabilities.forkchoice_updated_v2 { + self.forkchoice_updated_v2(forkchoice_state, maybe_payload_attributes) .await } else if engine_capabilities.forkchoice_updated_v1 { - self.forkchoice_updated_v1(forkchoice_state, payload_attributes) + self.forkchoice_updated_v1(forkchoice_state, maybe_payload_attributes) .await } else { Err(Error::RequiredMethodUnsupported("engine_forkchoiceUpdated")) 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 e46e2133d0b..487bfa07867 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -364,7 +364,7 @@ impl From for Withdrawal { } #[superstruct( - variants(V1, V2), + variants(V1, V2, V3), variant_attributes( derive(Debug, Clone, PartialEq, Serialize, Deserialize), serde(rename_all = "camelCase") @@ -379,8 +379,10 @@ pub struct JsonPayloadAttributes { pub timestamp: u64, pub prev_randao: Hash256, pub suggested_fee_recipient: Address, - #[superstruct(only(V2))] + #[superstruct(only(V2, V3))] pub withdrawals: Vec, + #[superstruct(only(V3))] + pub parent_beacon_block_root: Hash256, } impl From for JsonPayloadAttributes { @@ -397,6 +399,13 @@ impl From for JsonPayloadAttributes { suggested_fee_recipient: pa.suggested_fee_recipient, withdrawals: pa.withdrawals.into_iter().map(Into::into).collect(), }), + PayloadAttributes::V3(pa) => Self::V3(JsonPayloadAttributesV3 { + timestamp: pa.timestamp, + prev_randao: pa.prev_randao, + suggested_fee_recipient: pa.suggested_fee_recipient, + withdrawals: pa.withdrawals.into_iter().map(Into::into).collect(), + parent_beacon_block_root: pa.parent_beacon_block_root, + }), } } } @@ -415,6 +424,13 @@ impl From for PayloadAttributes { suggested_fee_recipient: jpa.suggested_fee_recipient, withdrawals: jpa.withdrawals.into_iter().map(Into::into).collect(), }), + JsonPayloadAttributes::V3(jpa) => Self::V3(PayloadAttributesV3 { + timestamp: jpa.timestamp, + prev_randao: jpa.prev_randao, + suggested_fee_recipient: jpa.suggested_fee_recipient, + withdrawals: jpa.withdrawals.into_iter().map(Into::into).collect(), + parent_beacon_block_root: jpa.parent_beacon_block_root, + }), } } } diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index b337314ee99..1a7c36b68fc 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -45,10 +45,8 @@ use types::beacon_block_body::KzgCommitments; use types::blob_sidecar::Blobs; use types::KzgProofs; use types::{ - AbstractExecPayload, BeaconStateError, ExecPayload, ExecutionPayloadDeneb, VersionedHash, -}; -use types::{ - BlindedPayload, BlockType, ChainSpec, Epoch, ExecutionPayloadCapella, ExecutionPayloadMerge, + AbstractExecPayload, BeaconStateError, BlindedPayload, BlockType, ChainSpec, Epoch, + ExecPayload, ExecutionPayloadCapella, ExecutionPayloadDeneb, ExecutionPayloadMerge, }; use types::{ProposerPreparationData, PublicKeyBytes, Signature, Slot, Transaction}; @@ -1232,29 +1230,25 @@ impl ExecutionLayer { /// Maps to the `engine_newPayload` JSON-RPC call. pub async fn notify_new_payload( &self, - execution_payload: &ExecutionPayload, - versioned_hashes: Option>, + new_payload_request: NewPayloadRequest, ) -> Result { let _timer = metrics::start_timer_vec( &metrics::EXECUTION_LAYER_REQUEST_TIMES, &[metrics::NEW_PAYLOAD], ); + let block_hash = new_payload_request.block_hash(); trace!( self.log(), "Issuing engine_newPayload"; - "parent_hash" => ?execution_payload.parent_hash(), - "block_hash" => ?execution_payload.block_hash(), - "block_number" => execution_payload.block_number(), + "parent_hash" => ?new_payload_request.parent_hash(), + "block_hash" => ?block_hash, + "block_number" => ?new_payload_request.block_number(), ); let result = self .engine() - .request(|engine| { - engine - .api - .new_payload(execution_payload.clone(), versioned_hashes) - }) + .request(|engine| engine.api.new_payload(new_payload_request)) .await; if let Ok(status) = &result { @@ -1265,7 +1259,7 @@ impl ExecutionLayer { } *self.inner.last_new_payload_errored.write().await = result.is_err(); - process_payload_status(execution_payload.block_hash(), result, self.log()) + process_payload_status(block_hash, result, self.log()) .map_err(Box::new) .map_err(Error::EngineError) } 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 90ee04b4ddd..e0a9c7f7ec0 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 @@ -556,27 +556,27 @@ impl ExecutionBlockGenerator { transactions: vec![].into(), withdrawals: pa.withdrawals.clone().into(), }), - ForkName::Deneb => ExecutionPayload::Deneb(ExecutionPayloadDeneb { - parent_hash: forkchoice_state.head_block_hash, - fee_recipient: pa.suggested_fee_recipient, - receipts_root: Hash256::repeat_byte(42), - state_root: Hash256::repeat_byte(43), - logs_bloom: vec![0; 256].into(), - prev_randao: pa.prev_randao, - block_number: parent.block_number() + 1, - gas_limit: GAS_LIMIT, - gas_used: GAS_USED, - timestamp: pa.timestamp, - extra_data: "block gen was here".as_bytes().to_vec().into(), - base_fee_per_gas: Uint256::one(), - block_hash: ExecutionBlockHash::zero(), - transactions: vec![].into(), - withdrawals: pa.withdrawals.clone().into(), - data_gas_used: 0, - excess_data_gas: 0, - }), _ => unreachable!(), }, + PayloadAttributes::V3(pa) => ExecutionPayload::Deneb(ExecutionPayloadDeneb { + parent_hash: forkchoice_state.head_block_hash, + fee_recipient: pa.suggested_fee_recipient, + receipts_root: Hash256::repeat_byte(42), + state_root: Hash256::repeat_byte(43), + logs_bloom: vec![0; 256].into(), + prev_randao: pa.prev_randao, + block_number: parent.block_number() + 1, + gas_limit: GAS_LIMIT, + gas_used: GAS_USED, + timestamp: pa.timestamp, + extra_data: "block gen was here".as_bytes().to_vec().into(), + base_fee_per_gas: Uint256::one(), + block_hash: ExecutionBlockHash::zero(), + transactions: vec![].into(), + withdrawals: pa.withdrawals.clone().into(), + data_gas_used: 0, + excess_data_gas: 0, + }), }; match execution_payload.fork_name() { 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 6c5786a54e1..320b2a60dd1 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -314,7 +314,9 @@ pub async fn handle_rpc( _ => unreachable!(), } } - ENGINE_FORKCHOICE_UPDATED_V1 | ENGINE_FORKCHOICE_UPDATED_V2 => { + ENGINE_FORKCHOICE_UPDATED_V1 + | ENGINE_FORKCHOICE_UPDATED_V2 + | ENGINE_FORKCHOICE_UPDATED_V3 => { let forkchoice_state: JsonForkchoiceStateV1 = get_param(params, 0).map_err(|s| (s, BAD_PARAMS_ERROR_CODE))?; let payload_attributes = match method { @@ -352,10 +354,15 @@ pub async fn handle_rpc( }) .map_err(|s| (s, BAD_PARAMS_ERROR_CODE))? } + ENGINE_FORKCHOICE_UPDATED_V3 => { + get_param::>(params, 1) + .map(|opt| opt.map(JsonPayloadAttributes::V3)) + .map_err(|s| (s, BAD_PARAMS_ERROR_CODE))? + } _ => unreachable!(), }; - // validate method called correctly according to shanghai fork time + // validate method called correctly according to fork time if let Some(pa) = payload_attributes.as_ref() { match ctx .execution_block_generator @@ -373,13 +380,22 @@ pub async fn handle_rpc( )); } } - ForkName::Capella | ForkName::Deneb => { + ForkName::Capella => { if method == ENGINE_FORKCHOICE_UPDATED_V1 { return Err(( format!("{} called after Capella fork!", method), FORK_REQUEST_MISMATCH_ERROR_CODE, )); } + if method == ENGINE_FORKCHOICE_UPDATED_V3 { + return Err(( + format!( + "{} called with `JsonPayloadAttributesV3` before Deneb fork!", + method + ), + GENERIC_ERROR_CODE, + )); + } if matches!(pa, JsonPayloadAttributes::V1(_)) { return Err(( format!( @@ -390,6 +406,20 @@ pub async fn handle_rpc( )); } } + ForkName::Deneb => { + if method == ENGINE_FORKCHOICE_UPDATED_V1 { + return Err(( + format!("{} called after Deneb fork!", method), + FORK_REQUEST_MISMATCH_ERROR_CODE, + )); + } + if method == ENGINE_FORKCHOICE_UPDATED_V2 { + return Err(( + format!("{} called after Deneb fork!", method), + FORK_REQUEST_MISMATCH_ERROR_CODE, + )); + } + } _ => unreachable!(), }; } 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 93eb06de851..cd29a998b0f 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_builder.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_builder.rs @@ -402,13 +402,23 @@ impl mev_rs::BlindedBlockProvider for MockBuilder { let prev_randao = head_state .get_randao_mix(head_state.current_epoch()) .map_err(convert_err)?; + let parent_root = head_state.latest_block_header().parent_root; let payload_attributes = match fork { - ForkName::Merge => PayloadAttributes::new(timestamp, *prev_randao, fee_recipient, None), + ForkName::Merge => { + PayloadAttributes::new(timestamp, *prev_randao, fee_recipient, None, None) + } // the withdrawals root is filled in by operations - ForkName::Capella | ForkName::Deneb => { - PayloadAttributes::new(timestamp, *prev_randao, fee_recipient, Some(vec![])) + ForkName::Capella => { + PayloadAttributes::new(timestamp, *prev_randao, fee_recipient, Some(vec![]), None) } + ForkName::Deneb => PayloadAttributes::new( + timestamp, + *prev_randao, + fee_recipient, + Some(vec![]), + Some(parent_root), + ), ForkName::Base | ForkName::Altair => { return Err(MevError::InvalidFork); } 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 b4a7d247adf..d82aca3bc40 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 @@ -110,7 +110,8 @@ impl MockExecutionLayer { timestamp, prev_randao, Address::repeat_byte(42), - // FIXME: think about how to handle different forks / withdrawals here.. + // FIXME: think about how to handle different forks here.. + None, None, ); @@ -140,7 +141,7 @@ impl MockExecutionLayer { }; let suggested_fee_recipient = self.el.get_suggested_fee_recipient(validator_index).await; let payload_attributes = - PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None); + PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None, None); let payload: ExecutionPayload = self .el .get_payload::>( @@ -148,7 +149,7 @@ impl MockExecutionLayer { &payload_attributes, forkchoice_update_params, builder_params, - // FIXME: do we need to consider other forks somehow? What about withdrawals? + // FIXME: do we need to consider other forks somehow? ForkName::Merge, &self.spec, ) @@ -175,7 +176,7 @@ impl MockExecutionLayer { }; let suggested_fee_recipient = self.el.get_suggested_fee_recipient(validator_index).await; let payload_attributes = - PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None); + PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None, None); let payload_header = self .el .get_payload::>( @@ -204,7 +205,12 @@ impl MockExecutionLayer { Some(payload.clone()) ); - let status = self.el.notify_new_payload(&payload, None).await.unwrap(); + // TODO: again consider forks + let status = self + .el + .notify_new_payload(payload.try_into().unwrap()) + .await + .unwrap(); assert_eq!(status, PayloadStatus::Valid); // Use junk values for slot/head-root to ensure there is no payload supplied. diff --git a/beacon_node/execution_layer/src/test_utils/mod.rs b/beacon_node/execution_layer/src/test_utils/mod.rs index 7940cf9f71b..90bafdd3af3 100644 --- a/beacon_node/execution_layer/src/test_utils/mod.rs +++ b/beacon_node/execution_layer/src/test_utils/mod.rs @@ -44,6 +44,7 @@ pub const DEFAULT_ENGINE_CAPABILITIES: EngineCapabilities = EngineCapabilities { new_payload_v3: true, forkchoice_updated_v1: true, forkchoice_updated_v2: true, + forkchoice_updated_v3: true, get_payload_bodies_by_hash_v1: true, get_payload_bodies_by_range_v1: true, get_payload_v1: true, diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 29eb0ac6163..693e6117b5e 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -926,7 +926,7 @@ pub struct SseLateHead { } #[superstruct( - variants(V1, V2), + variants(V1, V2, V3), variant_attributes(derive(Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)) )] #[derive(Clone, Debug, Eq, Hash, PartialEq, Deserialize, Serialize)] @@ -939,8 +939,10 @@ pub struct SsePayloadAttributes { pub prev_randao: Hash256, #[superstruct(getter(copy))] pub suggested_fee_recipient: Address, - #[superstruct(only(V2))] + #[superstruct(only(V2, V3))] pub withdrawals: Vec, + #[superstruct(only(V3), partial_getter(copy))] + pub parent_beacon_block_root: Hash256, } #[derive(PartialEq, Debug, Deserialize, Serialize, Clone)] diff --git a/testing/execution_engine_integration/src/test_rig.rs b/testing/execution_engine_integration/src/test_rig.rs index 57fd7a55b38..efd60fd26ac 100644 --- a/testing/execution_engine_integration/src/test_rig.rs +++ b/testing/execution_engine_integration/src/test_rig.rs @@ -280,7 +280,13 @@ impl TestRig { head_root, proposer_index, // TODO: think about how to test different forks - PayloadAttributes::new(timestamp, prev_randao, Address::repeat_byte(42), None), + PayloadAttributes::new( + timestamp, + prev_randao, + Address::repeat_byte(42), + None, + None, + ), ) .await; @@ -319,7 +325,7 @@ impl TestRig { .get_suggested_fee_recipient(proposer_index) .await; let payload_attributes = - PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None); + PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None, None); let valid_payload = self .ee_a .execution_layer @@ -368,10 +374,11 @@ impl TestRig { * Provide the valid payload back to the EE again. */ + // TODO: again consider forks here let status = self .ee_a .execution_layer - .notify_new_payload(&valid_payload, None) + .notify_new_payload(valid_payload.clone().try_into().unwrap()) .await .unwrap(); assert_eq!(status, PayloadStatus::Valid); @@ -419,12 +426,13 @@ impl TestRig { * Provide an invalidated payload to the EE. */ + // TODO: again think about forks here let mut invalid_payload = valid_payload.clone(); *invalid_payload.prev_randao_mut() = Hash256::from_low_u64_be(42); let status = self .ee_a .execution_layer - .notify_new_payload(&invalid_payload, None) + .notify_new_payload(invalid_payload.try_into().unwrap()) .await .unwrap(); assert!(matches!( @@ -459,7 +467,7 @@ impl TestRig { .get_suggested_fee_recipient(proposer_index) .await; let payload_attributes = - PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None); + PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None, None); let second_payload = self .ee_a .execution_layer @@ -483,10 +491,11 @@ impl TestRig { * Provide the second payload back to the EE again. */ + // TODO: again consider forks here let status = self .ee_a .execution_layer - .notify_new_payload(&second_payload, None) + .notify_new_payload(second_payload.clone().try_into().unwrap()) .await .unwrap(); assert_eq!(status, PayloadStatus::Valid); @@ -503,7 +512,7 @@ impl TestRig { // To save sending proposer preparation data, just set the fee recipient // to the fee recipient configured for EE A. let payload_attributes = - PayloadAttributes::new(timestamp, prev_randao, Address::repeat_byte(42), None); + PayloadAttributes::new(timestamp, prev_randao, Address::repeat_byte(42), None, None); let slot = Slot::new(42); let head_block_root = Hash256::repeat_byte(100); let validator_index = 0; @@ -530,10 +539,11 @@ impl TestRig { * * Provide the second payload, without providing the first. */ + // TODO: again consider forks here let status = self .ee_b .execution_layer - .notify_new_payload(&second_payload, None) + .notify_new_payload(second_payload.clone().try_into().unwrap()) .await .unwrap(); // TODO: we should remove the `Accepted` status here once Geth fixes it @@ -571,10 +581,11 @@ impl TestRig { * Provide the first payload to the EE. */ + // TODO: again consider forks here let status = self .ee_b .execution_layer - .notify_new_payload(&valid_payload, None) + .notify_new_payload(valid_payload.clone().try_into().unwrap()) .await .unwrap(); assert_eq!(status, PayloadStatus::Valid); @@ -588,7 +599,7 @@ impl TestRig { let status = self .ee_b .execution_layer - .notify_new_payload(&second_payload, None) + .notify_new_payload(second_payload.clone().try_into().unwrap()) .await .unwrap(); assert_eq!(status, PayloadStatus::Valid);