feat: add payload integrity tests

bolt-sidecar/src/api/builder.rs

@@ -17,6 +17,7 @@ use parking_lot::Mutex;
 use reqwest::Url;
 use serde::Deserialize;
 use std::{sync::Arc, time::Duration};
+use thiserror::Error;
 use tokio::net::TcpListener;
 
 use super::spec::{
@@ -140,15 +141,12 @@ where
         // On ANY error, we fall back to locally built block
         tracing::warn!(slot, elapsed = ?start.elapsed(), err = ?err, "Proxy error, fetching local payload instead");
 
-        let payload_and_bid = match server.payload_fetcher.fetch_payload(slot).await {
-            Some(payload_and_bid) => payload_and_bid,
-            None => {
-                // TODO: handle failure? In this case, we don't have a fallback block
-                // which means we haven't made any commitments. This means the EL should
-                // fallback to local block building.
-                tracing::error!("No local payload produced for slot {slot}");
-                return Err(BuilderApiError::FailedToFetchLocalPayload(slot));
-            }
+        let Some(payload_and_bid) = server.payload_fetcher.fetch_payload(slot).await else {
+            // TODO: handle failure? In this case, we don't have a fallback block
+            // which means we haven't made any commitments. This means the EL should
+            // fallback to local block building.
+            tracing::debug!("No local payload with commitments produced for slot {slot}");
+            return Err(BuilderApiError::FailedToFetchLocalPayload(slot));
         };
 
         let hash = payload_and_bid.bid.message.header.block_hash.clone();
@@ -195,30 +193,11 @@ where
 
         // If we have a locally built payload, it means we signed a local header.
         // Return it and clear the cache.
-        if let Some(payload) = server.local_payload.lock().take() {
-            let requested_block = &signed_blinded_block
-                .message
-                .body
-                .execution_payload_header
-                .block_hash;
-
-            // WARNING: this is an important check. If the local block does not match what the
-            // beacon node has signed, we are at risk of equivocation and slashing.
-            if payload.block_hash() != requested_block {
-                tracing::error!(
-                    expected = %requested_block.to_string(),
-                    have = %payload.block_hash().to_string(),
-                    "Local block hash does not match requested block hash"
-                );
-
-                return Err(BuilderApiError::InvalidLocalPayloadBlockHash {
-                    expected: requested_block.to_string(),
-                    have: payload.block_hash().to_string(),
-                });
-            };
-
-            tracing::debug!("Local block found, returning: {payload:?}");
-            return Ok(Json(payload));
+        if let Some(local_payload) = server.local_payload.lock().take() {
+            check_locally_built_payload_integrity(&signed_blinded_block, &local_payload)?;
+
+            tracing::debug!("Valid local block found, returning: {local_payload:?}");
+            return Ok(Json(local_payload));
         }
 
         // TODO: how do we deal with failures here? What if we submit the signed blinded block but don't get a response?
@@ -285,3 +264,147 @@ where
 async fn index() -> Html<&'static str> {
     Html("Hello")
 }
+
+#[derive(Error, Debug, Clone)]
+pub enum LocalPayloadIntegrityError {
+    #[error(
+        "Locally built payload does not match signed header. 
+        {field_name} mismatch: expected {expected}, have {have}"
+    )]
+    FieldMismatch {
+        field_name: String,
+        expected: String,
+        have: String,
+    },
+}
+
+/// Helper macro to compare fields of the signed header and the local block.
+macro_rules! assert_payload_fields_eq {
+    ($expected:expr, $have:expr, $field_name:ident) => {
+        if $expected != $have {
+            tracing::error!(
+                field_name = stringify!($field_name),
+                expected = %$expected,
+                have = %$have,
+                "Local block does not match signed header"
+            );
+            return Err(LocalPayloadIntegrityError::FieldMismatch {
+                field_name: stringify!($field_name).to_string(),
+                expected: $expected.to_string(),
+                have: $have.to_string(),
+            });
+        }
+    };
+}
+
+/// Perform some integrity checks on the locally built payload.
+/// This is to ensure that the beacon node will accept the header that was signed
+/// when we submit the full payload.
+#[inline]
+fn check_locally_built_payload_integrity(
+    signed_blinded_block: &SignedBlindedBeaconBlock,
+    local_payload: &GetPayloadResponse,
+) -> Result<(), LocalPayloadIntegrityError> {
+    let header_signed_by_cl = &signed_blinded_block.message.body.execution_payload_header;
+    let local_execution_payload = local_payload.execution_payload();
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.block_hash,
+        local_execution_payload.block_hash(),
+        BlockHash
+    );
+
+    assert_payload_fields_eq!(
+        header_signed_by_cl.block_number,
+        local_execution_payload.block_number(),
+        BlockNumber
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.state_root,
+        local_execution_payload.state_root(),
+        StateRoot
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.receipts_root,
+        local_execution_payload.receipts_root(),
+        ReceiptsRoot
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.prev_randao,
+        local_execution_payload.prev_randao(),
+        PrevRandao
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.gas_limit,
+        &local_execution_payload.gas_limit(),
+        GasLimit
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.gas_used,
+        &local_execution_payload.gas_used(),
+        GasUsed
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.timestamp,
+        &local_execution_payload.timestamp(),
+        Timestamp
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.extra_data,
+        local_execution_payload.extra_data(),
+        ExtraData
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.base_fee_per_gas,
+        local_execution_payload.base_fee_per_gas(),
+        BaseFeePerGas
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.parent_hash,
+        local_execution_payload.parent_hash(),
+        ParentHash
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.fee_recipient,
+        local_execution_payload.fee_recipient(),
+        FeeRecipient
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.logs_bloom,
+        local_execution_payload.logs_bloom(),
+        LogsBloom
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.blob_gas_used,
+        &local_execution_payload.blob_gas_used().unwrap_or_default(),
+        BlobGasUsed
+    );
+
+    assert_payload_fields_eq!(
+        &header_signed_by_cl.excess_blob_gas,
+        &local_execution_payload
+            .excess_blob_gas()
+            .unwrap_or_default(),
+        ExcessBlobGas
+    );
+
+    // TODO: Sanity check: recalculate transactions and withdrawals roots
+    // and assert them against the header
+
+    // TODO: Sanity check: verify the validator signature
+    // signed_blinded_block.verify_signature()?;
+
+    Ok(())
+}

bolt-sidecar/src/api/spec.rs

@@ -64,8 +64,8 @@ pub enum BuilderApiError {
     Timeout(#[from] tokio::time::error::Elapsed),
     #[error("Invalid fork: {0}")]
     InvalidFork(String),
-    #[error("Invalid local payload block hash. expected: {expected}, got: {have}")]
-    InvalidLocalPayloadBlockHash { expected: String, have: String },
+    #[error("Locally-built payload does not match expected signed header")]
+    LocalPayloadIntegrity(#[from] super::builder::LocalPayloadIntegrityError),
     #[error("Generic error: {0}")]
     Generic(String),
 }
@@ -109,8 +109,8 @@ impl IntoResponse for BuilderApiError {
             BuilderApiError::InvalidFork(err) => {
                 (StatusCode::BAD_REQUEST, Json(err)).into_response()
             }
-            BuilderApiError::InvalidLocalPayloadBlockHash { .. } => {
-                (StatusCode::BAD_REQUEST, self.to_string()).into_response()
+            BuilderApiError::LocalPayloadIntegrity(err) => {
+                (StatusCode::BAD_REQUEST, err.to_string()).into_response()
             }
             BuilderApiError::Generic(err) => {
                 (StatusCode::INTERNAL_SERVER_ERROR, Json(err)).into_response()

bolt-sidecar/src/builder/mod.rs

@@ -95,7 +95,6 @@ impl LocalBuilder {
 
     /// Build a new payload with the given transactions. This method will
     /// cache the payload in the local builder instance, and make it available
-    ///
     pub async fn build_new_local_payload(
         &mut self,
         template: &BlockTemplate,
@@ -160,8 +159,6 @@ impl LocalBuilder {
 
     /// transform a sealed header into a signed builder bid using
     /// the local builder's BLS key.
-    ///
-    /// TODO: add blobs bundle
     fn create_signed_builder_bid(
         &self,
         value: U256,

bolt-sidecar/src/builder/payload_builder.rs

@@ -196,29 +196,14 @@ impl FallbackPayloadBuilder {
             withdrawals: Some(Withdrawals::new(withdrawals)),
         };
 
-        // If there are some blob transactions, send them to the mempool
-        let blob_txs = transactions.iter().filter(|tx| tx.as_eip4844().is_some());
-        for tx in blob_txs {
-            tracing::debug!(?tx.hash, "Sending blob tx to mempool");
-            let mut bytes: Vec<u8> = Vec::new();
-            tx.encode_enveloped(&mut bytes);
-            if let Err(e) = self
-                .execution_rpc_client
-                .send_raw_transaction(bytes.into())
-                .await
-            {
-                tracing::error!(error = ?e, ?tx.hash, "Failed to send blob tx to mempool");
-            }
-        }
-
         let mut hints = Hints::default();
         let max_iterations = 20;
         let mut i = 0;
         loop {
             let header = build_header_with_hints_and_context(&latest_block, &hints, &ctx);
 
             let sealed_header = header.seal_slow();
-            let sealed_block = SealedBlock::new(sealed_header.clone(), body.clone());
+            let sealed_block = SealedBlock::new(sealed_header, body.clone());
 
             let block_hash = hints.block_hash.unwrap_or(sealed_block.hash());
 

bolt-sidecar/src/primitives/mod.rs

@@ -195,6 +195,14 @@ impl GetPayloadResponse {
             GetPayloadResponse::Deneb(payload) => payload.execution_payload.block_hash(),
         }
     }
+
+    pub fn execution_payload(&self) -> &ExecutionPayload {
+        match self {
+            GetPayloadResponse::Capella(payload) => payload,
+            GetPayloadResponse::Bellatrix(payload) => payload,
+            GetPayloadResponse::Deneb(payload) => &payload.execution_payload,
+        }
+    }
 }
 
 /// A struct representing the current chain head.