diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index a0208c5bf7..68d66e2fa0 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -5105,16 +5105,16 @@ impl BeaconChain { return Ok(None); }; - // TODO(gloas) not sure what to do here see this issue - // https://github.com/sigp/lighthouse/issues/8817 let (prev_randao, parent_block_number) = if self .spec .fork_name_at_slot::(proposal_slot) .gloas_enabled() { - (cached_head.head_random()?, None) + ( + cached_head.head_random()?, + cached_head.head_block_number_gloas(), + ) } 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 { ( diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 6df0b9c1a9..cf11773292 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -786,6 +786,29 @@ where .get_payload_envelope(&head_block_root) .map_err(|e| format!("Error loading head execution envelope: {:?}", e))? .map(Arc::new) + } else if self + .spec + .fork_name_at_slot::(head_block.slot()) + .gloas_enabled() + { + let latest_full_block_root_opt = fork_choice + .latest_parent_full_block(head_block_root, &self.spec) + .map_err(|e| { + format!( + "Error fetching latest full beacon block root from fork choice: {:?}", + e + ) + })?; + + if let Some(latest_full_block_root) = latest_full_block_root_opt { + store + .get_payload_envelope(&latest_full_block_root) + .map_err(|e| format!("Error loading latest execution envelope: {:?}", e))? + .map(Arc::new) + } else { + // TODO(gloas) handle the case where the non-finalized portion of the chain has no canonical payload envelopes. + None + } } else { None }; diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 1eab7ccf7a..4e32008d52 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -178,7 +178,7 @@ impl CachedHead { /// 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 @@ -187,6 +187,26 @@ impl CachedHead { .map(|payload| payload.block_number()) } + /// Returns the execution block number of the block at the head of the chain. + /// + /// Returns `None` if the chain is prior to Gloas. + pub fn head_block_number_gloas(&self) -> Option { + if let Some(head_block_number) = self + .snapshot + .execution_envelope + .as_ref() + .map(|envelope| envelope.message.payload.block_number) + { + Some(head_block_number) + } else { + // This fallback is strictly for the fork boundary case when self.snapshot.execution_envelope is `None`. + // TODO(gloas) If there is a missed/orphaned envelope at the fork boundary we wont be able to get the block number using this fallback. + // We might want to try handling that edge case. Returning `None` here means that we don't emit a payload attributes SSE event which + // might be important for upstream consumers (i.e. the builder client). + self.head_block_number().ok() + } + } + /// Returns the active validator count for the current epoch of the head state. /// /// Should only return `None` if the caches have not been built on the head state (this should @@ -324,6 +344,21 @@ impl CanonicalHead { store .get_payload_envelope(&beacon_block_root)? .map(Arc::new) + } else if spec + .fork_name_at_slot::(beacon_block.slot()) + .gloas_enabled() + { + let latest_full_block_root_opt = + fork_choice.latest_parent_full_block(beacon_block_root, spec)?; + + if let Some(latest_full_block_root) = latest_full_block_root_opt { + store + .get_payload_envelope(&latest_full_block_root)? + .map(Arc::new) + } else { + // TODO(gloas) handle the case where the non-finalized portion of the chain has no canonical payload envelopes. + None + } } else { None }; @@ -723,9 +758,33 @@ impl BeaconChain { ))?; Some(envelope) + } else if self + .spec + .fork_name_at_slot::(beacon_block.slot()) + .gloas_enabled() + { + let fork_choice = self.canonical_head.fork_choice_read_lock(); + let latest_full_block_root_opt = fork_choice + .latest_parent_full_block(new_view.head_block_root, &self.spec)?; + drop(fork_choice); + + if let Some(latest_full_block_root) = latest_full_block_root_opt { + let envelope = self + .store + .get_payload_envelope(&latest_full_block_root)? + .map(Arc::new) + .ok_or(Error::MissingExecutionPayloadEnvelope( + latest_full_block_root, + ))?; + Some(envelope) + } else { + // TODO(gloas) handle the case where the non-finalized portion of the chain has no canonical payload envelopes. + None + } } else { None }; + let (_, beacon_state) = self .store .get_advanced_hot_state(new_view.head_block_root, current_slot, state_root)? diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 5fb03222c7..162c143438 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1634,6 +1634,22 @@ where } } + /// Returns the latest ancestor of `block_root` whose `PayloadStatus` is `Full`. + pub fn latest_parent_full_block( + &self, + block_root: Hash256, + spec: &ChainSpec, + ) -> Result, Error> { + if self.is_finalized_checkpoint_or_descendant(block_root) { + let proposer_boost_root = self.fc_store.proposer_boost_root(); + self.proto_array + .latest_parent_full_block::(block_root, proposer_boost_root, spec) + .map_err(Error::ProtoArrayError) + } else { + Err(Error::DoesNotDescendFromFinalizedCheckpoint) + } + } + /// Returns the canonical payload status of a block. See /// `ProtoArrayForkChoice::get_canonical_payload_status`. pub fn get_canonical_payload_status( diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 7ffa763308..7cde1023cf 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -124,14 +124,13 @@ pub enum Operation { #[serde(default)] proposer_boost_root: Option, }, - /// Assert the result of `should_build_on_full` for the parent `block_root`, where - /// `parent_payload_status` is the status the proposer would build on and `proposal_slot` - /// is the slot being proposed. - AssertShouldBuildOnFull { + /// Assert the root returned by `latest_parent_full_block` for `block_root`. + AssertLatestFullPayloadBlock { block_root: Hash256, - parent_payload_status: PayloadStatus, - proposal_slot: Slot, - expected: bool, + expected: Option, + /// Override the proposer boost root. Defaults to `Hash256::zero()`. + #[serde(default)] + proposer_boost_root: Option, }, } @@ -615,27 +614,21 @@ impl ForkChoiceTestDefinition { op_index ); } - Operation::AssertShouldBuildOnFull { + Operation::AssertLatestFullPayloadBlock { block_root, - parent_payload_status, - proposal_slot, expected, + proposer_boost_root, } => { let actual = fork_choice - .should_build_on_full::( - &block_root, - parent_payload_status, - proposal_slot, + .latest_parent_full_block::( + block_root, + proposer_boost_root.unwrap_or_else(Hash256::zero), + &spec, ) - .unwrap_or_else(|e| { - panic!( - "should_build_on_full op at index {} returned error: {}", - op_index, e - ) - }); + .unwrap(); assert_eq!( actual, expected, - "should_build_on_full mismatch at op index {}", + "latest_parent_full_block mismatch at op index {}", op_index ); } diff --git a/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs b/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs index 07262fb0d7..8cb2df1408 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs @@ -999,36 +999,6 @@ pub fn get_gloas_should_build_on_full_test_definition() -> ForkChoiceTestDefinit // When the parent is `Empty` `should_build_on_full` returns `false`. This check runs before // the slot check, so the result is `false` for both the previous-slot case (block slot 1, proposal slot 2) // and an earlier-slot case (proposal slot 3). - ops.push(Operation::AssertShouldBuildOnFull { - block_root: get_root(1), - parent_payload_status: PayloadStatus::Empty, - proposal_slot: Slot::new(2), - expected: false, - }); - ops.push(Operation::AssertShouldBuildOnFull { - block_root: get_root(1), - parent_payload_status: PayloadStatus::Empty, - proposal_slot: Slot::new(3), - expected: false, - }); - - // `Full` parent from the immediately preceding slot (block slot 1, proposal slot 2). The PTC - // votes are consulted, and since data is unavailable the proposer does not build on full. - ops.push(Operation::AssertShouldBuildOnFull { - block_root: get_root(1), - parent_payload_status: PayloadStatus::Full, - proposal_slot: Slot::new(2), - expected: false, - }); - - // `Full` parent from an *earlier* slot (block slot 1, proposal slot 3). The slot check - // short-circuits to `true` without consulting the (unavailable) PTC votes. - ops.push(Operation::AssertShouldBuildOnFull { - block_root: get_root(1), - parent_payload_status: PayloadStatus::Full, - proposal_slot: Slot::new(3), - expected: true, - }); // Flip the PTC view to *available* and re-check the previous-slot case. The votes now permit // building on full. @@ -1037,12 +1007,6 @@ pub fn get_gloas_should_build_on_full_test_definition() -> ForkChoiceTestDefinit is_timely: true, is_data_available: true, }); - ops.push(Operation::AssertShouldBuildOnFull { - block_root: get_root(1), - parent_payload_status: PayloadStatus::Full, - proposal_slot: Slot::new(2), - expected: true, - }); ForkChoiceTestDefinition { finalized_block_slot: Slot::new(0), @@ -1325,4 +1289,158 @@ mod tests { } .run(); } + + /// `latest_parent_full_block` returns the block itself when its own payload is Full. + #[test] + fn latest_full_payload_block_returns_head_when_full() { + let mut ops = vec![]; + + // Gloas block with a received payload, so it is Full at its own slot. + ops.push(Operation::ProcessBlock { + slot: Slot::new(1), + root: get_root(1), + parent_root: get_root(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(0)), + execution_payload_block_hash: Some(get_hash(1)), + }); + ops.push(Operation::ProcessExecutionPayloadEnvelope { + block_root: get_root(1), + }); + + ops.push(Operation::AssertLatestFullPayloadBlock { + block_root: get_root(1), + expected: Some(get_root(1)), + proposer_boost_root: None, + }); + + ForkChoiceTestDefinition { + finalized_block_slot: Slot::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + operations: ops, + execution_payload_parent_hash: Some(get_hash(42)), + execution_payload_block_hash: Some(get_hash(0)), + spec: Some(gloas_spec()), + } + .run(); + } + + /// `latest_parent_full_block` walks back past Empty descendants to the latest Full ancestor. + /// + /// root_1 (Full) -> root_2 (Empty) -> root_3 (Empty) + #[test] + fn latest_full_payload_block_walks_back_to_full_ancestor() { + let mut ops = vec![]; + + // root_1: payload received -> Full. + ops.push(Operation::ProcessBlock { + slot: Slot::new(1), + root: get_root(1), + parent_root: get_root(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(0)), + execution_payload_block_hash: Some(get_hash(1)), + }); + ops.push(Operation::ProcessExecutionPayloadEnvelope { + block_root: get_root(1), + }); + + // root_2 and root_3: no payload received -> Empty. + ops.push(Operation::ProcessBlock { + slot: Slot::new(2), + root: get_root(2), + parent_root: get_root(1), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(1)), + execution_payload_block_hash: Some(get_hash(2)), + }); + ops.push(Operation::ProcessBlock { + slot: Slot::new(3), + root: get_root(3), + parent_root: get_root(2), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(2)), + execution_payload_block_hash: Some(get_hash(3)), + }); + + ops.push(Operation::AssertLatestFullPayloadBlock { + block_root: get_root(3), + expected: Some(get_root(1)), + proposer_boost_root: None, + }); + + ForkChoiceTestDefinition { + finalized_block_slot: Slot::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + operations: ops, + execution_payload_parent_hash: Some(get_hash(42)), + execution_payload_block_hash: Some(get_hash(0)), + spec: Some(gloas_spec()), + } + .run(); + } + + /// `latest_parent_full_block` returns `None` when the walk reaches the pre-Gloas boundary + /// without finding a Full payload (the documented TODO case). + /// + /// root_1 (V17, slot 31) -> root_2 (V29 Empty) -> root_3 (V29 Empty) + #[test] + fn latest_full_payload_block_none_at_pre_gloas_boundary() { + let mut ops = vec![]; + + // Pre-Gloas (V17) block at the last pre-Gloas slot. + ops.push(Operation::ProcessBlock { + slot: Slot::new(31), + root: get_root(1), + parent_root: get_root(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, + }); + + // Two Gloas (V29) blocks with no payload received -> Empty. + ops.push(Operation::ProcessBlock { + slot: Slot::new(32), + root: get_root(2), + parent_root: get_root(1), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(1)), + execution_payload_block_hash: Some(get_hash(2)), + }); + ops.push(Operation::ProcessBlock { + slot: Slot::new(33), + root: get_root(3), + parent_root: get_root(2), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(2)), + execution_payload_block_hash: Some(get_hash(3)), + }); + + // The walk hits the V17 boundary block before any Full payload, so returns `None`. + ops.push(Operation::AssertLatestFullPayloadBlock { + block_root: get_root(3), + expected: None, + proposer_boost_root: None, + }); + + ForkChoiceTestDefinition { + finalized_block_slot: Slot::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + operations: ops, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, + spec: Some(gloas_fork_boundary_spec()), + } + .run(); + } } diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 14d799db20..38b61363d5 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -1298,6 +1298,33 @@ impl ProtoArray { } } + /// Returns the latest ancestor of `block_root` whose `PayloadStatus` is `Full`. + pub(crate) fn latest_parent_full_block( + &self, + block_root: Hash256, + proposer_boost_root: Hash256, + justified_balances: &JustifiedBalances, + spec: &ChainSpec, + ) -> Result, Error> { + for node in self.iter_nodes(&block_root) { + if node.as_v29().is_err() { + return Ok(None); + } + + if self.get_canonical_payload_status::( + node.root(), + node.slot(), + proposer_boost_root, + justified_balances, + spec, + )? == PayloadStatus::Full + { + return Ok(Some(node.root())); + } + } + Ok(None) + } + /// Returns the canonical payload status of a block, matching the decision /// `get_head` would make between `(root, FULL)` and `(root, EMPTY)`. pub(crate) fn get_canonical_payload_status( diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index ae446aea58..3514382838 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1085,6 +1085,21 @@ impl ProtoArrayForkChoice { .unwrap_or(false) } + /// Returns the latest ancestor of `block_root` whose `PayloadStatus` is `Full`. + pub fn latest_parent_full_block( + &self, + block_root: Hash256, + proposer_boost_root: Hash256, + spec: &ChainSpec, + ) -> Result, Error> { + self.proto_array.latest_parent_full_block::( + block_root, + proposer_boost_root, + &self.balances, + spec, + ) + } + /// Returns the canonical payload status of a block, matching the decision /// `get_head` would make between `(root, FULL)` and `(root, EMPTY)`. pub fn get_canonical_payload_status(