Fix stale and incorrectly versioned builder preference submissions

Cargo.toml

@@ -78,6 +78,7 @@ members = [
     "testing/web3signer_tests",
     "validator_client",
     "validator_client/beacon_node_fallback",
+    "validator_client/builder_store",
     "validator_client/doppelganger_service",
     "validator_client/graffiti_file",
     "validator_client/http_api",
@@ -120,6 +121,7 @@ bincode = "1"
 bitvec = "1"
 bls = { path = "crypto/bls" }
 builder_client = { path = "beacon_node/builder_client" }
+builder_store = { path = "validator_client/builder_store" }
 builder_types = { path = "common/builder_types" }
 byteorder = "1"
 bytes = "1.11.1"

beacon_node/beacon_chain/src/payload_bid_verification/direct_verified_bid.rs

@@ -1,6 +1,6 @@
 use crate::payload_bid_verification::{
     PayloadBidError,
-    gossip_verified_bid::{is_gas_limit_target_compatible, verify_bid_consistency},
+    gossip_verified_bid::{is_gas_limit_target_compatible, verify_direct_bid_consistency},
 };
 use eth2::types::BuilderPubkeys;
 use state_processing::signature_sets::{
@@ -14,7 +14,8 @@ use types::{
 /// Fully validate a bid fetched directly from a builder, for inclusion in a block being produced.
 ///
 /// This performs all validation a direct builder bid must pass before it can be selected:
-/// - the consensus-consistency checks shared with the gossip verifier via [`verify_bid_consistency`]
+/// - the consensus-consistency checks shared with the gossip verifier, bundled for this path in
+///   [`verify_direct_bid_consistency`]
 ///   (fee recipient, blob count, builder eligibility/version, and that the builder's collateral
 ///   covers the bid value),
 /// - that the bid matches the block being produced — the exact `proposal_slot`, the selected
@@ -81,7 +82,7 @@ pub fn verify_direct_bid<E: EthSpec>(
     }
 
     // Consensus-consistency checks shared with the gossip verifier.
-    verify_bid_consistency(bid, proposal_slot, proposer_preferences, state, spec)?;
+    verify_direct_bid_consistency(bid, proposal_slot, proposer_preferences, state, spec)?;
 
     // If the requesting `BuilderEntry` named builder pubkeys, the bid must come from one of them:
     // the builder at `bid.builder_index` must have one of those pubkeys (the `builder_pubkeys`
@@ -267,6 +268,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 +338,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>(
@@ -69,12 +81,17 @@ fn verify_bid_blobs<E: EthSpec>(
     Ok(())
 }
 
-/// Verify that an execution payload bid is consistent with the current chain state
-/// and proposer preferences.
+/// Verify that a direct (builder-API) bid is consistent with the current chain state
+/// and proposer preferences: the direct path's bundle of the shared bid checks.
 ///
-/// These checks are shared by gossip and direct bids. Source-specific checks (e.g. the gossip-only
-/// requirement that `execution_payment == 0`) are applied by the caller.
-pub(crate) fn verify_bid_consistency<E: EthSpec>(
+/// The individual checks are shared with gossip, but this bundle's only caller is
+/// [`verify_direct_bid`](crate::payload_bid_verification::direct_verified_bid::verify_direct_bid):
+/// the gossip verifier applies the same helpers (`verify_bid_slot`, `verify_bid_blobs`,
+/// `verify_bid_block_hash_not_parent`, `verify_bid_state_conditions`) piecewise, in gossip-spec
+/// order, interleaved with gossip-only work (cache checks, the preferences lookup, fork-choice
+/// rules). A check that must cover both intakes belongs in one of those shared helpers — adding
+/// it only here leaves gossip uncovered.
+pub(crate) fn verify_direct_bid_consistency<E: EthSpec>(
     bid: &ExecutionPayloadBid<E>,
     current_slot: Slot,
     proposer_preferences: &SignedProposerPreferences,
@@ -87,6 +104,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)