Verify Versioned Hashes During Optimistic Sync

beacon_node/beacon_chain/src/execution_payload.rs

@@ -81,14 +81,10 @@ impl<T: BeaconChainTypes> PayloadNotifier<T> {
 
             match notify_execution_layer {
                 NotifyExecutionLayer::No if chain.config.optimistic_finalized_sync => {
-                    // Verify the block hash here in Lighthouse and immediately mark the block as
-                    // optimistically imported. This saves a lot of roundtrips to the EL.
-                    let execution_layer = chain
-                        .execution_layer
-                        .as_ref()
-                        .ok_or(ExecutionPayloadError::NoExecutionConnection)?;
-
-                    if let Err(e) = execution_layer.verify_payload_block_hash(block_message) {
+                    // Create a NewPayloadRequest (no clones required) and check optimistic sync verifications
+                    let new_payload_request: NewPayloadRequest<T::EthSpec> =
+                        block_message.try_into()?;
+                    if let Err(e) = new_payload_request.perform_optimistic_sync_verifications() {
                         warn!(
                             chain.log,
                             "Falling back to slow block hash verification";
@@ -143,11 +139,8 @@ async fn notify_new_payload<'a, T: BeaconChainTypes>(
         .as_ref()
         .ok_or(ExecutionPayloadError::NoExecutionConnection)?;
 
-    let new_payload_request: NewPayloadRequest<T::EthSpec> = block.try_into()?;
-    let execution_block_hash = new_payload_request.block_hash();
-    let new_payload_response = execution_layer
-        .notify_new_payload(new_payload_request)
-        .await;
+    let execution_block_hash = block.execution_payload()?.block_hash();
+    let new_payload_response = execution_layer.notify_new_payload(block.try_into()?).await;
 
     match new_payload_response {
         Ok(status) => match status {

beacon_node/execution_layer/Cargo.toml

@@ -51,3 +51,5 @@ hash-db = "0.15.2"
 pretty_reqwest_error = { workspace = true }
 arc-swap = "1.6.0"
 eth2_network_config = { workspace = true }
+alloy-rlp = "0.3"
+alloy-consensus = { git = "https://github.com/alloy-rs/alloy.git" }

beacon_node/execution_layer/src/block_hash.rs

@@ -1,92 +1,61 @@
 use crate::{
     json_structures::JsonWithdrawal,
     keccak::{keccak256, KeccakHasher},
-    metrics, Error, ExecutionLayer,
 };
 use ethers_core::utils::rlp::RlpStream;
 use keccak_hash::KECCAK_EMPTY_LIST_RLP;
 use triehash::ordered_trie_root;
 use types::{
-    map_execution_block_header_fields_base, Address, BeaconBlockRef, EthSpec, ExecutionBlockHash,
+    map_execution_block_header_fields_base, Address, EthSpec, ExecutionBlockHash,
     ExecutionBlockHeader, ExecutionPayloadRef, Hash256, Hash64, Uint256,
 };
 
-impl<T: EthSpec> ExecutionLayer<T> {
-    /// Calculate the block hash of an execution block.
-    ///
-    /// Return `(block_hash, transactions_root)`, where `transactions_root` is the root of the RLP
-    /// transactions.
-    pub fn calculate_execution_block_hash(
-        payload: ExecutionPayloadRef<T>,
-        parent_beacon_block_root: Hash256,
-    ) -> (ExecutionBlockHash, Hash256) {
-        // Calculate the transactions root.
-        // We're currently using a deprecated Parity library for this. We should move to a
-        // better alternative when one appears, possibly following Reth.
-        let rlp_transactions_root = ordered_trie_root::<KeccakHasher, _>(
-            payload.transactions().iter().map(|txn_bytes| &**txn_bytes),
-        );
-
-        // Calculate withdrawals root (post-Capella).
-        let rlp_withdrawals_root = if let Ok(withdrawals) = payload.withdrawals() {
-            Some(ordered_trie_root::<KeccakHasher, _>(
-                withdrawals.iter().map(|withdrawal| {
-                    rlp_encode_withdrawal(&JsonWithdrawal::from(withdrawal.clone()))
-                }),
-            ))
-        } else {
-            None
-        };
-
-        let rlp_blob_gas_used = payload.blob_gas_used().ok();
-        let rlp_excess_blob_gas = payload.excess_blob_gas().ok();
-
-        // Calculate parent beacon block root (post-Deneb).
-        let rlp_parent_beacon_block_root = rlp_excess_blob_gas
-            .as_ref()
-            .map(|_| parent_beacon_block_root);
-
-        // Construct the block header.
-        let exec_block_header = ExecutionBlockHeader::from_payload(
-            payload,
-            KECCAK_EMPTY_LIST_RLP.as_fixed_bytes().into(),
-            rlp_transactions_root,
-            rlp_withdrawals_root,
-            rlp_blob_gas_used,
-            rlp_excess_blob_gas,
-            rlp_parent_beacon_block_root,
-        );
-
-        // Hash the RLP encoding of the block header.
-        let rlp_block_header = rlp_encode_block_header(&exec_block_header);
-        (
-            ExecutionBlockHash::from_root(keccak256(&rlp_block_header)),
-            rlp_transactions_root,
-        )
-    }
-
-    /// Verify `payload.block_hash` locally within Lighthouse.
-    ///
-    /// No remote calls to the execution client will be made, so this is quite a cheap check.
-    pub fn verify_payload_block_hash(&self, block: BeaconBlockRef<T>) -> Result<(), Error> {
-        let payload = block.execution_payload()?.execution_payload_ref();
-        let parent_beacon_block_root = block.parent_root();
-
-        let _timer = metrics::start_timer(&metrics::EXECUTION_LAYER_VERIFY_BLOCK_HASH);
-
-        let (header_hash, rlp_transactions_root) =
-            Self::calculate_execution_block_hash(payload, parent_beacon_block_root);
-
-        if header_hash != payload.block_hash() {
-            return Err(Error::BlockHashMismatch {
-                computed: header_hash,
-                payload: payload.block_hash(),
-                transactions_root: rlp_transactions_root,
-            });
-        }
-
-        Ok(())
-    }
+/// Calculate the block hash of an execution block.
+///
+/// Return `(block_hash, transactions_root)`, where `transactions_root` is the root of the RLP
+/// transactions.
+pub fn calculate_execution_block_hash<T: EthSpec>(
+    payload: ExecutionPayloadRef<T>,
+    parent_beacon_block_root: Option<Hash256>,
+) -> (ExecutionBlockHash, Hash256) {
+    // Calculate the transactions root.
+    // We're currently using a deprecated Parity library for this. We should move to a
+    // better alternative when one appears, possibly following Reth.
+    let rlp_transactions_root = ordered_trie_root::<KeccakHasher, _>(
+        payload.transactions().iter().map(|txn_bytes| &**txn_bytes),
+    );
+
+    // Calculate withdrawals root (post-Capella).
+    let rlp_withdrawals_root = if let Ok(withdrawals) = payload.withdrawals() {
+        Some(ordered_trie_root::<KeccakHasher, _>(
+            withdrawals
+                .iter()
+                .map(|withdrawal| rlp_encode_withdrawal(&JsonWithdrawal::from(withdrawal.clone()))),
+        ))
+    } else {
+        None
+    };
+
+    let rlp_blob_gas_used = payload.blob_gas_used().ok();
+    let rlp_excess_blob_gas = payload.excess_blob_gas().ok();
+
+    // Construct the block header.
+    let exec_block_header = ExecutionBlockHeader::from_payload(
+        payload,
+        KECCAK_EMPTY_LIST_RLP.as_fixed_bytes().into(),
+        rlp_transactions_root,
+        rlp_withdrawals_root,
+        rlp_blob_gas_used,
+        rlp_excess_blob_gas,
+        parent_beacon_block_root,
+    );
+
+    // Hash the RLP encoding of the block header.
+    let rlp_block_header = rlp_encode_block_header(&exec_block_header);
+    (
+        ExecutionBlockHash::from_root(keccak256(&rlp_block_header)),
+        rlp_transactions_root,
+    )
 }
 
 /// RLP encode a withdrawal.

beacon_node/execution_layer/src/engine_api.rs

@@ -17,7 +17,6 @@ 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::kzg_commitment_to_versioned_hash;
 use std::convert::TryFrom;
 use strum::IntoStaticStr;
 use superstruct::superstruct;
@@ -26,14 +25,16 @@ pub use types::{
     ExecutionPayloadRef, FixedVector, ForkName, Hash256, Transactions, Uint256, VariableList,
     Withdrawal, Withdrawals,
 };
-use types::{
-    BeaconStateError, ExecutionPayloadCapella, ExecutionPayloadDeneb, ExecutionPayloadMerge,
-    KzgProofs, VersionedHash,
-};
+use types::{ExecutionPayloadCapella, ExecutionPayloadDeneb, ExecutionPayloadMerge, KzgProofs};
 
 pub mod auth;
 pub mod http;
 pub mod json_structures;
+mod new_payload_request;
+
+pub use new_payload_request::{
+    NewPayloadRequest, NewPayloadRequestCapella, NewPayloadRequestDeneb, NewPayloadRequestMerge,
+};
 
 pub const LATEST_TAG: &str = "latest";
 
@@ -571,110 +572,6 @@ impl<E: EthSpec> ExecutionPayloadBodyV1<E> {
     }
 }
 
-#[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<E: EthSpec> {
-    #[superstruct(only(Merge), partial_getter(rename = "execution_payload_merge"))]
-    pub execution_payload: ExecutionPayloadMerge<E>,
-    #[superstruct(only(Capella), partial_getter(rename = "execution_payload_capella"))]
-    pub execution_payload: ExecutionPayloadCapella<E>,
-    #[superstruct(only(Deneb), partial_getter(rename = "execution_payload_deneb"))]
-    pub execution_payload: ExecutionPayloadDeneb<E>,
-    #[superstruct(only(Deneb))]
-    pub versioned_hashes: Vec<VersionedHash>,
-    #[superstruct(only(Deneb))]
-    pub parent_beacon_block_root: Hash256,
-}
-
-impl<E: EthSpec> NewPayloadRequest<E> {
-    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<E> {
-        map_new_payload_request_into_execution_payload!(self, |request, cons| {
-            cons(request.execution_payload)
-        })
-    }
-}
-
-impl<'a, E: EthSpec> TryFrom<BeaconBlockRef<'a, E>> for NewPayloadRequest<E> {
-    type Error = BeaconStateError;
-
-    fn try_from(block: BeaconBlockRef<'a, E>) -> Result<Self, Self::Error> {
-        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<E: EthSpec> TryFrom<ExecutionPayload<E>> for NewPayloadRequest<E> {
-    type Error = BeaconStateError;
-
-    fn try_from(payload: ExecutionPayload<E>) -> Result<Self, Self::Error> {
-        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,

beacon_node/execution_layer/src/engine_api/http.rs

@@ -803,10 +803,10 @@ impl HttpJsonRpc {
 
     pub async fn new_payload_v3<T: EthSpec>(
         &self,
-        new_payload_request_deneb: NewPayloadRequestDeneb<T>,
+        new_payload_request_deneb: NewPayloadRequestDeneb<'_, T>,
     ) -> Result<PayloadStatusV1, Error> {
         let params = json!([
-            JsonExecutionPayload::V3(new_payload_request_deneb.execution_payload.into()),
+            JsonExecutionPayload::V3(new_payload_request_deneb.execution_payload.clone().into()),
             new_payload_request_deneb.versioned_hashes,
             new_payload_request_deneb.parent_beacon_block_root,
         ]);
@@ -1079,7 +1079,7 @@ impl HttpJsonRpc {
     // new_payload that the execution engine supports
     pub async fn new_payload<T: EthSpec>(
         &self,
-        new_payload_request: NewPayloadRequest<T>,
+        new_payload_request: NewPayloadRequest<'_, T>,
     ) -> Result<PayloadStatusV1, Error> {
         let engine_capabilities = self.get_engine_capabilities(None).await?;
         match new_payload_request {