Gloas builder API review follow-ups: direct-bid hash check and no-redirect block forwarding

beacon_node/beacon_chain/src/payload_bid_verification/direct_verified_bid.rs

@@ -267,6 +267,38 @@ mod tests {
         ));
     }
 
+    #[test]
+    fn rejects_block_hash_equal_to_parent_block_hash() {
+        let (state, spec) = state_and_spec();
+        // Passes every earlier check (slot, ancestor hash, parent root, RANDAO, gas limit), then
+        // claims a `block_hash` equal to its `parent_block_hash` — the consensus assert from
+        // `process_execution_payload_bid` that must be front-run before selection.
+        let executed_ancestor = ExecutionBlockHash::repeat_byte(7);
+        let mut bid = signed_bid(
+            Slot::new(1),
+            executed_ancestor,
+            Hash256::ZERO,
+            Hash256::ZERO,
+        );
+        bid.message.block_hash = executed_ancestor;
+        bid.message.gas_limit = EXECUTED_ANCESTOR_GAS_LIMIT;
+        let result = verify_direct_bid(
+            &bid,
+            Slot::new(1),
+            executed_ancestor,
+            Hash256::ZERO,
+            EXECUTED_ANCESTOR_GAS_LIMIT,
+            &BuilderPubkeys::default(),
+            &preferences(),
+            &state,
+            &spec,
+        );
+        assert!(matches!(
+            result,
+            Err(PayloadBidError::BlockHashEqualsParentBlockHash { .. })
+        ));
+    }
+
     #[test]
     fn rejects_gas_limit_incompatible_with_parent() {
         let (state, spec) = state_and_spec();
@@ -305,6 +337,9 @@ mod tests {
             Hash256::ZERO,
         );
         bid.message.gas_limit = EXECUTED_ANCESTOR_GAS_LIMIT;
+        // A default (zero) `block_hash` would equal the zero parent hash and trip the
+        // block-hash-equals-parent rejection before the checks this test targets.
+        bid.message.block_hash = ExecutionBlockHash::repeat_byte(1);
         let result = verify_direct_bid(
             &bid,
             Slot::new(1),

beacon_node/beacon_chain/src/payload_bid_verification/gossip_verified_bid.rs

@@ -42,14 +42,26 @@ fn verify_bid_payment_and_blobs<E: EthSpec>(
         });
     }
 
+    verify_bid_block_hash_not_parent(bid)?;
+
+    verify_bid_blobs(bid, spec)
+}
+
+/// Reject a bid whose `block_hash` equals its `parent_block_hash`.
+///
+/// `process_execution_payload_bid` enforces this in `per_block_processing`, so every bid intake —
+/// gossip *and* direct (builder-API) — must front-run it: a bid that fails only at block
+/// processing has already won selection and costs the proposer the slot.
+pub(crate) fn verify_bid_block_hash_not_parent<E: EthSpec>(
+    bid: &ExecutionPayloadBid<E>,
+) -> Result<(), PayloadBidError> {
     if bid.block_hash == bid.parent_block_hash {
         return Err(PayloadBidError::BlockHashEqualsParentBlockHash {
             slot: bid.slot,
             block_hash: bid.block_hash,
         });
     }
-
-    verify_bid_blobs(bid, spec)
+    Ok(())
 }
 
 fn verify_bid_blobs<E: EthSpec>(
@@ -87,6 +99,11 @@ pub(crate) fn verify_bid_consistency<E: EthSpec>(
         return Err(PayloadBidError::InvalidFeeRecipient);
     }
 
+    // Mirrors the consensus assert in `process_execution_payload_bid`. The gossip path applies
+    // this earlier (via `verify_bid_payment_and_blobs`); repeating it here keeps the direct path
+    // covered without depending on the gossip caller's composition.
+    verify_bid_block_hash_not_parent(bid)?;
+
     verify_bid_blobs(bid, spec)?;
 
     verify_bid_state_conditions(bid, head_state, spec)

beacon_node/builder_client/src/builder_http_client.rs

@@ -41,6 +41,11 @@ const DATE_MILLISECONDS: HeaderName = HeaderName::from_static("date-milliseconds
 #[derive(Clone)]
 pub struct BuilderHttpClient {
     client: reqwest::Client,
+    /// Client for `submitSignedBeaconBlock` only. The target URL arrives over the wire (the
+    /// `Eth-Builder-Url` request header echoed by the VC) and is an SSRF risk, so beacon-APIs
+    /// `publishBlock` requires that the forwarding request "MUST NOT follow redirects" — reqwest's
+    /// redirect policy is client-wide, hence a dedicated client with redirects disabled.
+    no_redirect_client: reqwest::Client,
     user_agent: String,
     /// Only use json for all request/response types.
     disable_ssz: bool,
@@ -50,8 +55,13 @@ impl BuilderHttpClient {
     pub fn new(user_agent: Option<String>, disable_ssz: bool) -> Result<Self, Error> {
         let user_agent = user_agent.unwrap_or_else(|| DEFAULT_USER_AGENT.to_string());
         let client = reqwest::Client::builder().user_agent(&user_agent).build()?;
+        let no_redirect_client = reqwest::Client::builder()
+            .user_agent(&user_agent)
+            .redirect(reqwest::redirect::Policy::none())
+            .build()?;
         Ok(Self {
             client,
+            no_redirect_client,
             user_agent,
             disable_ssz,
         })
@@ -234,6 +244,10 @@ impl BuilderHttpClient {
     ///
     /// `ssz_request` selects the request-body encoding: SSZ when `true` and the client has SSZ
     /// enabled, otherwise JSON.
+    ///
+    /// Sent via [`Self::no_redirect_client`]: `builder_url` is wire input (`Eth-Builder-Url`), and
+    /// the spec forbids following redirects on this request. A redirect response surfaces as
+    /// [`Error::StatusCode`] like any other non-202.
     pub async fn submit_signed_beacon_block<E: EthSpec>(
         &self,
         builder_url: &SensitiveUrl,
@@ -263,7 +277,7 @@ impl BuilderHttpClient {
                 HeaderValue::from_str(SSZ_CONTENT_TYPE_HEADER)
                     .map_err(|e| Error::InvalidHeaders(format!("{}", e)))?,
             );
-            self.client
+            self.no_redirect_client
                 .post(path)
                 .timeout(timeout)
                 .headers(headers)
@@ -274,7 +288,7 @@ impl BuilderHttpClient {
                 HeaderValue::from_str(JSON_CONTENT_TYPE_HEADER)
                     .map_err(|e| Error::InvalidHeaders(format!("{}", e)))?,
             );
-            self.client
+            self.no_redirect_client
                 .post(path)
                 .timeout(timeout)
                 .headers(headers)