merge queue: queuing unstable (446f5b5) and #9405 together

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -5138,7 +5138,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             })
     }
 
-    // TODO(gloas): wrong for Gloas, needs an update
     pub fn overridden_forkchoice_update_params_or_failure_reason(
         &self,
         canonical_forkchoice_params: &ForkchoiceUpdateParameters,
@@ -5169,6 +5168,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             )
             .map_err(|e| e.map_inner_error(Error::ProposerHeadForkChoiceError))?;
 
+        // We don't need to override fork choice updates for Gloas.
+        if info.head_node.is_gloas() {
+            return Ok(*canonical_forkchoice_params);
+        }
+
         // The slot of our potential re-org block is always 1 greater than the head block because we
         // only attempt single-slot re-orgs.
         let head_slot = info.head_node.slot();
@@ -5302,9 +5306,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             return Err(Box::new(DoNotReOrg::NotProposing.into()));
         }
 
-        // TODO(gloas): V29 nodes don't carry execution_status, so this returns
-        // None for post-Gloas re-orgs. Need to source the EL block hash from
-        // the bid's block_hash instead. Re-org is disabled for Gloas for now.
+        // This only works pre-Gloas, but we don't run this code for Gloas anyway.
         let parent_head_hash = info
             .parent_node
             .execution_status()
@@ -6341,8 +6343,15 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     }
 
                     let canonical_fcu_params = cached_head.forkchoice_update_parameters();
-                    let fcu_params =
-                        chain.overridden_forkchoice_update_params(canonical_fcu_params)?;
+                    let fcu_params = if chain
+                        .spec
+                        .fork_name_at_slot::<T::EthSpec>(head_slot)
+                        .gloas_enabled()
+                    {
+                        canonical_fcu_params
+                    } else {
+                        chain.overridden_forkchoice_update_params(canonical_fcu_params)?
+                    };
                     let pre_payload_attributes = chain.get_pre_payload_attributes(
                         prepare_slot,
                         fcu_params.head_root,

beacon_node/beacon_chain/src/block_production/mod.rs

@@ -4,7 +4,7 @@ use fork_choice::PayloadStatus;
 use proto_array::{ProposerHeadError, ReOrgThreshold};
 use slot_clock::SlotClock;
 use tracing::{debug, error, info, instrument, warn};
-use types::{BeaconState, Epoch, Hash256, SignedExecutionPayloadEnvelope, Slot};
+use types::{BeaconState, Epoch, EthSpec, Hash256, SignedExecutionPayloadEnvelope, Slot};
 
 use crate::{
     BeaconChain, BeaconChainTypes, BlockProductionError, StateSkipConfig,
@@ -14,13 +14,21 @@ use crate::{
 mod gloas;
 
 /// State loaded from the database for block production.
-pub(crate) struct BlockProductionState<E: types::EthSpec> {
+pub(crate) struct BlockProductionState<E: EthSpec> {
     pub state: BeaconState<E>,
     pub state_root: Option<Hash256>,
     pub parent_payload_status: PayloadStatus,
     pub parent_envelope: Option<Arc<SignedExecutionPayloadEnvelope<E>>>,
 }
 
+/// Inputs assembled for producing a block via a proposer re-org.
+struct ReOrgInputs<E: EthSpec> {
+    state: BeaconState<E>,
+    state_root: Hash256,
+    parent_payload_status: PayloadStatus,
+    parent_envelope: Option<Arc<SignedExecutionPayloadEnvelope<E>>>,
+}
+
 impl<T: BeaconChainTypes> BeaconChain<T> {
     /// Load a beacon state from the database for block production. This is a long-running process
     /// that should not be performed in an `async` context.
@@ -50,39 +58,32 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 head.snapshot.execution_envelope.clone(),
             )
         };
+
         let result = if head_slot < slot {
             // Attempt an aggressive re-org if configured and the conditions are right.
-            // TODO(gloas): re-enable reorgs
-            let gloas_enabled = self
-                .spec
-                .fork_name_at_slot::<T::EthSpec>(slot)
-                .gloas_enabled();
-            if !gloas_enabled
-                && let Some((re_org_state, re_org_state_root)) =
-                    self.get_state_for_re_org(slot, head_slot, head_block_root)
-            {
+            if let Some(inputs) = self.get_state_for_re_org(slot, head_slot, head_block_root) {
                 info!(
                     %slot,
                     head_to_reorg = %head_block_root,
                     "Proposing block to re-org current head"
                 );
-                // TODO(gloas): ensure we use a sensible payload status when we enable reorgs
-                // for Gloas
                 BlockProductionState {
-                    state: re_org_state,
-                    state_root: Some(re_org_state_root),
-                    parent_payload_status: PayloadStatus::Pending,
-                    parent_envelope: None,
+                    state: inputs.state,
+                    state_root: Some(inputs.state_root),
+                    parent_payload_status: inputs.parent_payload_status,
+                    parent_envelope: inputs.parent_envelope,
                 }
             } else {
-                // Fetch the head state advanced through to `slot`, which should be present in the
-                // state cache thanks to the state advance timer.
+                // Continuation: the new block builds on the current head. Fetch the head state
+                // advanced through to `slot`, which should be present in the state cache thanks to
+                // the state advance timer.
                 let parent_state_root = head_state_root;
                 let (state_root, state) = self
                     .store
                     .get_advanced_hot_state(head_block_root, slot, parent_state_root)
                     .map_err(BlockProductionError::FailedToLoadState)?
                     .ok_or(BlockProductionError::UnableToProduceAtSlot(slot))?;
+
                 BlockProductionState {
                     state,
                     state_root: Some(state_root),
@@ -100,13 +101,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 .state_at_slot(slot - 1, StateSkipConfig::WithStateRoots)
                 .map_err(|_| BlockProductionError::UnableToProduceAtSlot(slot))?;
 
-            // TODO(gloas): update this to read payload canonicity from fork choice once ready
-            let parent_payload_status = PayloadStatus::Pending;
             BlockProductionState {
                 state,
                 state_root: None,
-                parent_payload_status,
-                parent_envelope: None,
+                parent_payload_status: head_payload_status,
+                parent_envelope: head_envelope,
             }
         };
 
@@ -173,7 +172,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         slot: Slot,
         head_slot: Slot,
         canonical_head: Hash256,
-    ) -> Option<(BeaconState<T::EthSpec>, Hash256)> {
+    ) -> Option<ReOrgInputs<T::EthSpec>> {
         let re_org_head_threshold = ReOrgThreshold(self.spec.reorg_head_weight_threshold);
         let re_org_parent_threshold = ReOrgThreshold(self.spec.reorg_parent_weight_threshold);
         let re_org_max_epochs_since_finalization =
@@ -237,9 +236,15 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 }
             })
             .ok()?;
+
         drop(proposer_head_timer);
         let re_org_parent_block = proposer_head.parent_node.root();
 
+        // The head uniquely determines the parent payload status for the re-org block, whichever
+        // variant (full or empty) it builds on must have more weight, or else we would have already
+        // re-orged away from this block naturally, and it would not be the head, by definition.
+        let parent_payload_status = proposer_head.head_node.get_parent_payload_status();
+
         let (state_root, state) = self
             .store
             .get_advanced_hot_state_from_cache(re_org_parent_block, slot)
@@ -248,6 +253,25 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 None
             })?;
 
+        let parent_envelope = if parent_payload_status == PayloadStatus::Full {
+            let envelope = self
+                .store
+                .get_payload_envelope(&re_org_parent_block)
+                .ok()
+                .flatten()
+                .map(Arc::new)
+                .or_else(|| {
+                    warn!(
+                        reason = "missing execution payload envelope",
+                        "Not attempting re-org"
+                    );
+                    None
+                })?;
+            Some(envelope)
+        } else {
+            None
+        };
+
         info!(
             weak_head = ?canonical_head,
             parent = ?re_org_parent_block,
@@ -256,6 +280,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             "Attempting re-org due to weak head"
         );
 
-        Some((state, state_root))
+        Some(ReOrgInputs {
+            state,
+            state_root,
+            parent_payload_status,
+            parent_envelope,
+        })
     }
 }