From e7f027baddc6765ba4491790d406a837812a8e86 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Wed, 25 Mar 2026 19:36:14 -0500 Subject: [PATCH] O(n) children index, fix load_parent for gloas blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Build parent->children index once per find_head call, replacing O(n) scans in filter_block_tree and get_node_children with O(1) lookups. - Skip zero block_hash in is_parent_block_full check — default/zero hashes don't indicate a real payload relationship. - Fall back to block state_root for genesis when envelope not stored. - Store execution payload envelope in EF test harness during on_execution_payload step. --- .../beacon_chain/src/block_verification.rs | 28 +++++--- consensus/proto_array/src/proto_array.rs | 70 ++++++++++++------- testing/ef_tests/src/cases/fork_choice.rs | 25 ++++++- 3 files changed, 88 insertions(+), 35 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index bc29486326..d09ac291ab 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1964,15 +1964,27 @@ fn load_parent>( if block.as_block().fork_name_unchecked().gloas_enabled() && let Ok(parent_bid_block_hash) = parent_block.payload_bid_block_hash() { - if block.as_block().is_parent_block_full(parent_bid_block_hash) { + if !parent_bid_block_hash.into_root().is_zero() + && block.as_block().is_parent_block_full(parent_bid_block_hash) + { // TODO(gloas): loading the envelope here is not very efficient - // TODO(gloas): check parent payload existence prior to this point? - let envelope = chain.store.get_payload_envelope(&root)?.ok_or_else(|| { - BeaconChainError::DBInconsistent(format!( - "Missing envelope for parent block {root:?}", - )) - })?; - (StatePayloadStatus::Full, envelope.message.state_root) + let envelope = chain.store.get_payload_envelope(&root)?; + let state_root = if let Some(env) = envelope { + env.message.state_root + } else { + // The envelope may not be stored yet for the genesis/anchor + // block. Fall back to the block's state_root which is the + // post-payload state for the anchor per get_forkchoice_store. + if parent_block.slot() == chain.spec.genesis_slot { + parent_block.state_root() + } else { + return Err(BeaconChainError::DBInconsistent(format!( + "Missing envelope for parent block {root:?}", + )) + .into()); + } + }; + (StatePayloadStatus::Full, state_root) } else { (StatePayloadStatus::Pending, parent_block.state_root()) } diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 374190f9ed..8e59071baf 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -1188,6 +1188,26 @@ impl ProtoArray { Ok((best_fc_node.root, best_fc_node.payload_status)) } + /// Build a parent->children index. Invalid nodes are excluded + /// (they aren't in store.blocks in the spec). + fn build_children_index(&self) -> Vec> { + let mut children = vec![vec![]; self.nodes.len()]; + for (i, node) in self.nodes.iter().enumerate() { + if node + .execution_status() + .is_ok_and(|status| status.is_invalid()) + { + continue; + } + if let Some(parent) = node.parent() { + if parent < children.len() { + children[parent].push(i); + } + } + } + children + } + /// Spec: `get_filtered_block_tree`. /// /// Returns the set of node indices on viable branches — those with at least @@ -1198,6 +1218,7 @@ impl ProtoArray { current_slot: Slot, best_justified_checkpoint: Checkpoint, best_finalized_checkpoint: Checkpoint, + children_index: &[Vec], ) -> HashSet { let mut viable = HashSet::new(); self.filter_block_tree::( @@ -1205,6 +1226,7 @@ impl ProtoArray { current_slot, best_justified_checkpoint, best_finalized_checkpoint, + children_index, &mut viable, ); viable @@ -1217,25 +1239,17 @@ impl ProtoArray { current_slot: Slot, best_justified_checkpoint: Checkpoint, best_finalized_checkpoint: Checkpoint, + children_index: &[Vec], viable: &mut HashSet, ) -> bool { let Some(node) = self.nodes.get(node_index) else { return false; }; - // Skip invalid children — they aren't in store.blocks in the spec. - let children: Vec = self - .nodes - .iter() - .enumerate() - .filter(|(_, child)| { - child.parent() == Some(node_index) - && !child - .execution_status() - .is_ok_and(|status| status.is_invalid()) - }) - .map(|(i, _)| i) - .collect(); + let children = children_index + .get(node_index) + .map(|c| c.as_slice()) + .unwrap_or(&[]); if !children.is_empty() { // Evaluate ALL children (no short-circuit) to mark all viable branches. @@ -1247,6 +1261,7 @@ impl ProtoArray { current_slot, best_justified_checkpoint, best_finalized_checkpoint, + children_index, viable, ) }) @@ -1291,12 +1306,16 @@ impl ProtoArray { payload_status: PayloadStatus::Pending, }; + // Build parent->children index once for O(1) lookups. + let children_index = self.build_children_index(); + // Spec: `get_filtered_block_tree`. let viable_nodes = self.get_filtered_block_tree::( start_index, current_slot, best_justified_checkpoint, best_finalized_checkpoint, + &children_index, ); // Compute once rather than per-child per-level. @@ -1305,7 +1324,7 @@ impl ProtoArray { loop { let children: Vec<_> = self - .get_node_children(&head)? + .get_node_children(&head, &children_index)? .into_iter() .filter(|(fc_node, _)| viable_nodes.contains(&fc_node.proto_node_index)) .collect(); @@ -1468,6 +1487,7 @@ impl ProtoArray { fn get_node_children( &self, node: &IndexedForkChoiceNode, + children_index: &[Vec], ) -> Result, Error> { if node.payload_status == PayloadStatus::Pending { let proto_node = self @@ -1481,23 +1501,25 @@ impl ProtoArray { } Ok(children) } else { - Ok(self - .nodes + let child_indices = children_index + .get(node.proto_node_index) + .map(|c| c.as_slice()) + .unwrap_or(&[]); + Ok(child_indices .iter() - .enumerate() - .filter(|(_, child_node)| { - child_node.parent() == Some(node.proto_node_index) - && child_node.get_parent_payload_status() == node.payload_status - }) - .map(|(child_index, child_node)| { - ( + .filter_map(|&child_index| { + let child_node = self.nodes.get(child_index)?; + if child_node.get_parent_payload_status() != node.payload_status { + return None; + } + Some(( IndexedForkChoiceNode { root: child_node.root(), proto_node_index: child_index, payload_status: PayloadStatus::Pending, }, child_node.clone(), - ) + )) }) .collect()) } diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 9f0e6de2ea..0c95d1c2d2 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -449,8 +449,7 @@ impl Case for ForkChoiceTest { execution_payload, valid, } => { - tester - .process_execution_payload(execution_payload.beacon_block_root(), *valid)?; + tester.process_execution_payload(execution_payload, *valid)?; } } } @@ -993,7 +992,27 @@ impl Tester { check_equal("proposer_head", proposer_head, expected_proposer_head) } - pub fn process_execution_payload(&self, block_root: Hash256, valid: bool) -> Result<(), Error> { + pub fn process_execution_payload( + &self, + signed_envelope: &SignedExecutionPayloadEnvelope, + valid: bool, + ) -> Result<(), Error> { + let block_root = signed_envelope.message.beacon_block_root; + + // Store the envelope in the database so that child blocks extending + // the FULL path can load the parent's post-payload state. + if valid { + self.harness + .chain + .store + .put_payload_envelope(&block_root, signed_envelope.clone()) + .map_err(|e| { + Error::InternalError(format!( + "Failed to store payload envelope for {block_root:?}: {e:?}", + )) + })?; + } + let result = self .harness .chain