diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index bd3716ea54..7eff6fb99a 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -115,6 +115,7 @@ use parking_lot::{Mutex, RwLock, RwLockWriteGuard}; use proto_array::{DoNotReOrg, ProposerHeadError, ReOrgThreshold}; use rand::RngCore; use safe_arith::SafeArith; +use serde_utils::quoted_u64::Quoted; use slasher::Slasher; use slot_clock::SlotClock; use ssz::Encode; @@ -5057,16 +5058,19 @@ impl BeaconChain { return Ok(None); }; - // TODO(gloas) not sure what to do here see this issue - // https://github.com/sigp/lighthouse/issues/8817 + // TODO(gloas) once we fork to gloas, we can remove `parent_block_number`. + // In the meantime we are just setting it to `None`. let (prev_randao, parent_block_number) = if self .spec .fork_name_at_slot::(proposal_slot) .gloas_enabled() { - (cached_head.head_random()?, None) + if proposer_head == head_parent_block_root { + (cached_head.parent_random()?, None) + } else { + (cached_head.head_random()?, None) + } } else { - // Get the `prev_randao` and parent block number. let head_block_number = cached_head.head_block_number()?; if proposer_head == head_parent_block_root { ( @@ -6543,14 +6547,15 @@ impl BeaconChain { // Push a server-sent event (probably to a block builder or relay). if let Some(event_handler) = &self.event_handler && event_handler.has_payload_attributes_subscribers() - && let Some(parent_block_number) = pre_payload_attributes.parent_block_number { event_handler.register(EventKind::PayloadAttributes(ForkVersionedResponse { data: SseExtendedPayloadAttributes { proposal_slot: prepare_slot, proposer_index: proposer, parent_block_root: head_root, - parent_block_number, + parent_block_number: pre_payload_attributes + .parent_block_number + .map(|value| Quoted { value }), parent_block_hash: forkchoice_update_params.head_hash.unwrap_or_default(), payload_attributes: payload_attributes.into(), }, diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 5d7a7f1527..db1a417775 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -163,22 +163,29 @@ impl CachedHead { /// Returns the randao mix for the parent of the block at the head of the chain. /// - /// This is useful for re-orging the current head. The parent's RANDAO value is read from - /// the head's execution payload because it is unavailable in the beacon state's RANDAO mixes - /// array after being overwritten by the head block's RANDAO mix. + /// This is useful for re-orging the current head. /// + /// Pre-gloas: The parent's RANDAO value is read from the head's execution payload because + /// it is unavailable in the beacon state's RANDAO mixes array after being overwritten by the head + /// block's RANDAO mix. + /// + /// Post-gloas: The parent's RANDAO value is read from the beacon states latest execution payload bid. /// This will error if the head block is not execution-enabled (post Bellatrix). pub fn parent_random(&self) -> Result { - self.snapshot - .beacon_block - .message() - .execution_payload() - .map(|payload| payload.prev_randao()) + let block = self.snapshot.beacon_block.message(); + if block.fork_name_unchecked().gloas_enabled() { + self.snapshot + .beacon_state + .latest_execution_payload_bid() + .map(|bid| bid.prev_randao) + } else { + block.body().execution_payload().map(|p| p.prev_randao()) + } } /// Returns the execution block number of the block at the head of the chain. /// - /// Returns an error if the chain is prior to Bellatrix. + /// Returns an error if the chain is prior to Bellatrix or post-Gloas pub fn head_block_number(&self) -> Result { self.snapshot .beacon_block @@ -726,6 +733,7 @@ impl BeaconChain { } else { None }; + let (_, beacon_state) = self .store .get_advanced_hot_state(new_view.head_block_root, current_slot, state_root)? diff --git a/beacon_node/beacon_chain/tests/prepare_payload.rs b/beacon_node/beacon_chain/tests/prepare_payload.rs index ad23b77a55..4a2b8121a4 100644 --- a/beacon_node/beacon_chain/tests/prepare_payload.rs +++ b/beacon_node/beacon_chain/tests/prepare_payload.rs @@ -689,3 +689,53 @@ async fn gloas_block_production_caches_blobs_for_column_publishing() { "Envelope should remain in cache after taking blobs" ); } + +/// A Gloas proposer re-orging must use the parent's `prev_randao` +#[tokio::test] +async fn gloas_pre_payload_attributes_reorg_uses_parent_randao() { + let spec = Arc::new(test_spec::()); + if !spec.fork_name_at_slot::(Slot::new(0)).gloas_enabled() { + return; + } + + let db_path = tempdir().unwrap(); + let store = get_store(&db_path, spec.clone()); + let harness = get_harness(store.clone(), LOW_VALIDATOR_COUNT); + + harness + .extend_chain( + 5, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + ) + .await; + + let cached_head = harness.chain.canonical_head.cached_head(); + let head_block_root = cached_head.head_block_root(); + let head_parent_block_root = cached_head.parent_block_root(); + let proposal_slot = cached_head.head_slot() + 1; + + let head_random = cached_head.head_random().unwrap(); + let parent_random = cached_head.parent_random().unwrap(); + assert_ne!(head_random, parent_random); + + let on_head = harness + .chain + .get_pre_payload_attributes(proposal_slot, head_block_root, &cached_head) + .unwrap() + .unwrap(); + assert_eq!(on_head.prev_randao, head_random); + + // value should always be none post gloas + assert_eq!(on_head.parent_block_number, None); + + let on_parent = harness + .chain + .get_pre_payload_attributes(proposal_slot, head_parent_block_root, &cached_head) + .unwrap() + .unwrap(); + assert_eq!(on_parent.prev_randao, parent_random); + + // value should always be none post gloas + assert_eq!(on_parent.parent_block_number, None); +} diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 4d0bb48f54..f2b9a5ca64 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -1154,9 +1154,9 @@ pub struct SseExtendedPayloadAttributesGeneric { #[serde(with = "serde_utils::quoted_u64")] pub proposer_index: u64, pub parent_block_root: Hash256, - #[serde(with = "serde_utils::quoted_u64")] - pub parent_block_number: u64, - + // TODO(gloas) can remove this field once we fork to gloas + #[serde(default, skip_serializing_if = "Option::is_none")] + pub parent_block_number: Option>, pub parent_block_hash: ExecutionBlockHash, pub payload_attributes: T, }