Fix valid Gloas bids rejected with InvalidGasLimit after an empty parent

beacon_node/beacon_chain/src/payload_bid_verification/gossip_verified_bid.rs

@@ -17,10 +17,80 @@ use state_processing::signature_sets::{
 };
 use tracing::debug;
 use types::{
-    BeaconState, ChainSpec, EthSpec, ExecutionPayloadBid, SignedExecutionPayloadBid,
-    SignedProposerPreferences, Slot, consts::gloas::PAYLOAD_BUILDER_VERSION,
+    BeaconState, ChainSpec, EthSpec, ExecPayload, ExecutionBlockHash, ExecutionPayloadBid, Hash256,
+    SignedExecutionPayloadBid, SignedProposerPreferences, Slot,
+    consts::gloas::PAYLOAD_BUILDER_VERSION,
 };
 
+/// Find the beacon block carrying the known execution payload referenced by a bid.
+pub(crate) fn find_execution_payload_block_root<T: BeaconChainTypes>(
+    fork_choice_read: &ForkChoiceReadGuard<'_, T>,
+    parent_block_root: Hash256,
+    parent_block_hash: ExecutionBlockHash,
+) -> Result<Hash256, PayloadBidError> {
+    let mut block_root = Some(parent_block_root);
+    while let Some(root) = block_root {
+        let Some(block) = fork_choice_read.get_block(&root) else {
+            break;
+        };
+
+        if let Some(block_hash) = block.execution_payload_block_hash {
+            if fork_choice_read.is_payload_received(&root) && block_hash == parent_block_hash {
+                return Ok(root);
+            }
+        } else if !block.execution_status.is_invalid()
+            && block.execution_status.block_hash() == Some(parent_block_hash)
+        {
+            return Ok(root);
+        }
+
+        block_root = block.parent_root;
+    }
+    Err(PayloadBidError::ParentExecutionPayloadUnknown { parent_block_hash })
+}
+
+/// Return the gas limit committed by the block carrying an execution payload.
+pub(crate) fn get_parent_gas_limit<T: BeaconChainTypes>(
+    store: &BeaconStore<T>,
+    execution_payload_block_root: Hash256,
+    parent_block_hash: ExecutionBlockHash,
+) -> Result<u64, PayloadBidError> {
+    let block = store
+        .get_blinded_block(&execution_payload_block_root)
+        .map_err(|e| {
+            PayloadBidError::InternalError(format!(
+                "failed to load execution payload block {execution_payload_block_root:?}: {e:?}"
+            ))
+        })?
+        .ok_or_else(|| {
+            PayloadBidError::InternalError(format!(
+                "execution payload block {execution_payload_block_root:?} unavailable"
+            ))
+        })?;
+
+    if block.fork_name_unchecked().gloas_enabled() {
+        let bid = &block
+            .message()
+            .body()
+            .signed_execution_payload_bid()?
+            .message;
+        if bid.block_hash != parent_block_hash {
+            return Err(PayloadBidError::InternalError(format!(
+                "execution payload hash mismatch for block {execution_payload_block_root:?}"
+            )));
+        }
+        Ok(bid.gas_limit)
+    } else {
+        let payload = block.message().execution_payload()?;
+        if payload.block_hash() != parent_block_hash {
+            return Err(PayloadBidError::InternalError(format!(
+                "execution payload hash mismatch for block {execution_payload_block_root:?}"
+            )));
+        }
+        Ok(payload.gas_limit())
+    }
+}
+
 /// Verify that an execution payload bid is consistent with the current chain state
 /// and proposer preferences.
 pub(crate) fn verify_bid_consistency<E: EthSpec>(
@@ -302,26 +372,26 @@ impl<E: EthSpec> GossipVerifiedPayloadBid<E> {
             });
         }
 
-        // TODO(gloas): [IGNORE] bid.parent_block_hash is the block hash of a known execution
-        // payload in fork choice.
-
-        // TODO(gloas): This uses head state's bid gas_limit as parent_gas_limit, which is only
-        // correct when the bid's parent is the head. If the parent is an ancestor further back
-        // this check may be inaccurate. Fixing this requires storing
-        // gas_limit in fork choice or looking it up from the store by parent_block_hash. Taking the above
-        // TODO into consideration maybe should persist parent block hash and gas limit in fork choice?
-        if let Ok(parent_bid) = head_state.latest_execution_payload_bid()
-            && !is_gas_limit_target_compatible(
-                parent_bid.gas_limit,
-                signed_bid.message.gas_limit,
-                proposer_preferences.message.target_gas_limit,
-            )?
-        {
+        let execution_payload_block_root = find_execution_payload_block_root(
+            &fork_choice,
+            signed_bid.message.parent_block_root,
+            signed_bid.message.parent_block_hash,
+        )?;
+        drop(fork_choice);
+
+        let parent_gas_limit = get_parent_gas_limit::<T>(
+            ctx.store,
+            execution_payload_block_root,
+            signed_bid.message.parent_block_hash,
+        )?;
+        if !is_gas_limit_target_compatible(
+            parent_gas_limit,
+            signed_bid.message.gas_limit,
+            proposer_preferences.message.target_gas_limit,
+        )? {
             return Err(PayloadBidError::InvalidGasLimit);
         }
 
-        drop(fork_choice);
-
         verify_bid_consistency(
             &signed_bid.message,
             current_slot,

beacon_node/beacon_chain/src/payload_bid_verification/mod.rs

@@ -10,7 +10,7 @@
 //!    GossipVerifiedPayloadBid -------> Insert into GossipVerifiedPayloadBidCache
 //! ```
 
-use types::{BeaconStateError, Hash256, Slot};
+use types::{BeaconStateError, ExecutionBlockHash, Hash256, Slot};
 
 pub mod gossip_verified_bid;
 pub mod payload_bid_cache;
@@ -22,6 +22,10 @@ mod tests;
 pub enum PayloadBidError {
     /// The bid's parent block root is unknown.
     ParentBlockRootUnknown { parent_block_root: Hash256 },
+    /// The bid's parent execution payload is unknown.
+    ParentExecutionPayloadUnknown {
+        parent_block_hash: ExecutionBlockHash,
+    },
     /// The bid does not build on the head block or on the head block's parent.
     BidNotCompatibleWithHead { parent_block_root: Hash256 },
     /// The signature is invalid.

beacon_node/beacon_chain/src/payload_bid_verification/tests.rs

@@ -28,7 +28,10 @@ use crate::{
     chain_config::FastConfirmationMode,
     payload_bid_verification::{
         PayloadBidError,
-        gossip_verified_bid::{GossipVerificationContext, GossipVerifiedPayloadBid},
+        gossip_verified_bid::{
+            GossipVerificationContext, GossipVerifiedPayloadBid, find_execution_payload_block_root,
+            get_parent_gas_limit, is_gas_limit_target_compatible,
+        },
         payload_bid_cache::{BidParent, GossipVerifiedPayloadBidCache},
     },
     proposer_preferences_verification::{
@@ -148,6 +151,13 @@ impl TestContext {
             ForkChoice::from_anchor(fc_store, block_root, &signed_block, &state, None, &spec)
                 .expect("should create fork choice");
 
+        store
+            .put_block(&block_root, signed_block.clone())
+            .expect("should store genesis block");
+        fork_choice
+            .on_valid_payload_envelope_received(block_root)
+            .expect("should mark genesis payload as received");
+
         let (_, head_payload_status) = fork_choice
             .get_head(Slot::new(0), &spec)
             .expect("should run get_head");
@@ -509,6 +519,58 @@ fn gas_limit_mismatch() {
     assert!(matches!(result, Err(PayloadBidError::InvalidGasLimit)));
 }
 
+#[test]
+fn gas_limit_uses_known_execution_parent_after_unreceived_payload() {
+    if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) {
+        return;
+    }
+    let ctx = TestContext::new();
+    let rejected_payload_root = ctx.insert_non_canonical_block();
+    let fork_choice = ctx.canonical_head.fork_choice_read_lock();
+
+    let actual_parent_block_root = find_execution_payload_block_root(
+        &fork_choice,
+        rejected_payload_root,
+        ExecutionBlockHash::zero(),
+    )
+    .expect("should resolve the last received execution payload");
+    drop(fork_choice);
+    let actual_parent_gas_limit = get_parent_gas_limit::<T>(
+        &ctx.store,
+        actual_parent_block_root,
+        ExecutionBlockHash::zero(),
+    )
+    .expect("should load the last received execution payload gas limit");
+
+    let rejected_payload_gas_limit = 30_029_295;
+    let next_gas_limit = 30_029_295;
+    let target_gas_limit = 60_000_000;
+    assert_eq!(actual_parent_gas_limit, 30_000_000);
+    assert!(
+        is_gas_limit_target_compatible(actual_parent_gas_limit, next_gas_limit, target_gas_limit,)
+            .expect("gas limit calculation should succeed")
+    );
+    assert!(
+        !is_gas_limit_target_compatible(
+            rejected_payload_gas_limit,
+            next_gas_limit,
+            target_gas_limit,
+        )
+        .expect("gas limit calculation should succeed")
+    );
+
+    let fork_choice = ctx.canonical_head.fork_choice_read_lock();
+    let result = find_execution_payload_block_root(
+        &fork_choice,
+        rejected_payload_root,
+        ExecutionBlockHash::repeat_byte(0xab),
+    );
+    assert!(matches!(
+        result,
+        Err(PayloadBidError::ParentExecutionPayloadUnknown { .. })
+    ));
+}
+
 #[test]
 fn execution_payment_nonzero() {
     if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) {

beacon_node/network/src/network_beacon_processor/gossip_methods.rs

@@ -4238,6 +4238,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                 | PayloadBidError::BuilderAlreadySeen { .. }
                 | PayloadBidError::BidValueBelowCached { .. }
                 | PayloadBidError::ParentBlockRootUnknown { .. }
+                | PayloadBidError::ParentExecutionPayloadUnknown { .. }
                 | PayloadBidError::BidNotCompatibleWithHead { .. }
                 | PayloadBidError::BuilderCantCoverBid { .. }
                 | PayloadBidError::InvalidFeeRecipient