From 58ac399ad73204eeb328c2797a15ab0438013d44 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Fri, 3 Apr 2026 08:05:05 -0700 Subject: [PATCH 01/15] Introduce logic to ensure served payload enveloeps past the split slot are canonical according to fork choice --- .../beacon_chain/src/canonical_head.rs | 19 +++++--- .../beacon_chain_adapter.rs | 7 +-- .../src/payload_envelope_streamer/mod.rs | 4 +- .../src/payload_envelope_streamer/tests.rs | 4 +- consensus/fork_choice/src/fork_choice.rs | 19 ++++++++ .../src/fork_choice_test_definition.rs | 23 ++++++++++ .../gloas_payload.rs | 44 +++++++++++++++++++ .../src/proto_array_fork_choice.rs | 32 ++++++++++++++ 8 files changed, 138 insertions(+), 14 deletions(-) diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index cd53d0ef7cf..a60cf1ae4f1 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -50,6 +50,7 @@ use itertools::process_results; use logging::crit; use parking_lot::{Mutex, RwLock, RwLockReadGuard, RwLockUpgradableReadGuard, RwLockWriteGuard}; +use proto_array::PayloadStatus; use slot_clock::SlotClock; use state_processing::AllCaches; use std::sync::Arc; @@ -381,11 +382,19 @@ impl CanonicalHead { Ok((head, execution_status)) } - // TODO(gloas) just a stub for now, implement this once we have fork choice. - /// Returns true if the payload for this block is canonical according to fork choice - /// Returns an error if the block root doesn't exist in fork choice. - pub fn block_has_canonical_payload(&self, _root: &Hash256) -> Result { - Ok(true) + /// Returns `Some(true)` if the payload for this block is canonical (Full) according to fork choice. + /// + /// Walks backwards from the head to determine the canonical payload status of the block. + /// Returns `None` if the block has already been pruned from fork choice or isn't part of the + /// canonical chain. + pub fn block_has_canonical_payload(&self, root: &Hash256) -> Option { + let cached_head = self.cached_head(); + let head_root = cached_head.head_block_root(); + let head_payload_status = cached_head.head_payload_status(); + + self.fork_choice_read_lock() + .get_canonical_payload_status(root, &head_root, head_payload_status) + .map(|status| status == PayloadStatus::Full) } /// Returns a clone of `self.cached_head`. diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs index 47c58f07b90..c9de2d09657 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs @@ -5,7 +5,7 @@ use mockall::automock; use task_executor::TaskExecutor; use types::{Hash256, SignedExecutionPayloadEnvelope, Slot}; -use crate::{BeaconChain, BeaconChainError, BeaconChainTypes}; +use crate::{BeaconChain, BeaconChainTypes}; /// An adapter to the `BeaconChain` functionalities to remove `BeaconChain` from direct dependency to enable testing envelope streamer logic. pub(crate) struct EnvelopeStreamerBeaconAdapter { @@ -33,10 +33,7 @@ impl EnvelopeStreamerBeaconAdapter { self.chain.store.get_split_info().slot } - pub(crate) fn block_has_canonical_payload( - &self, - root: &Hash256, - ) -> Result { + pub(crate) fn block_has_canonical_payload(&self, root: &Hash256) -> Option { self.chain.canonical_head.block_has_canonical_payload(root) } } diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs index d10e3762a4d..6700e7a4cf6 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs @@ -125,14 +125,14 @@ impl PayloadEnvelopeStreamer { } match streamer.adapter.block_has_canonical_payload(root) { - Ok(is_envelope_canonical) => { + Some(is_envelope_canonical) => { if is_envelope_canonical { results.push((*root, Ok(Some(envelope)))); } else { results.push((*root, Ok(None))); } } - Err(_) => { + None => { results.push(( *root, Err(BeaconChainError::EnvelopeStreamerError( diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/tests.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/tests.rs index 9e869a59b8b..bbed3017e3e 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/tests.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/tests.rs @@ -115,7 +115,7 @@ fn mock_canonical_head(mock: &mut MockEnvelopeStreamerBeaconAdapter, chain: & .map(|e| e.block_root) .collect(); mock.expect_block_has_canonical_payload() - .returning(move |root| Ok(!non_canonical.contains(root))); + .returning(move |root| Some(!non_canonical.contains(root))); } fn unwrap_result( @@ -288,7 +288,7 @@ async fn stream_envelopes_error() { mock.expect_get_split_slot().return_const(Slot::new(0)); mock_envelopes(&mut mock, &chain); mock.expect_block_has_canonical_payload() - .returning(|_| Err(BeaconChainError::CanonicalHeadLockTimeout)); + .returning(|_| None); let streamer = PayloadEnvelopeStreamer::new(mock, EnvelopeRequestSource::ByRange); let mut stream = streamer.launch_stream(roots(&chain)); diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 92fd4c1faf3..34bb717889f 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1502,6 +1502,25 @@ where } } + /// Returns the canonical payload status of a block. See + /// `ProtoArrayForkChoice::get_canonical_payload_status`. + pub fn get_canonical_payload_status( + &self, + block_root: &Hash256, + head_root: &Hash256, + head_payload_status: PayloadStatus, + ) -> Option { + if self.is_finalized_checkpoint_or_descendant(*block_root) { + self.proto_array.get_canonical_payload_status( + block_root, + head_root, + head_payload_status, + ) + } else { + None + } + } + /// Returns the weight for the given block root. pub fn get_block_weight(&self, block_root: &Hash256) -> Option { self.proto_array.get_weight(block_root) diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index c9764d3e44d..ec6741000d3 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -105,6 +105,12 @@ pub enum Operation { block_root: Hash256, expected: bool, }, + AssertCanonicalPayloadStatus { + block_root: Hash256, + head_root: Hash256, + head_payload_status: PayloadStatus, + expected_status: Option, + }, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -522,6 +528,23 @@ impl ForkChoiceTestDefinition { op_index ); } + Operation::AssertCanonicalPayloadStatus { + block_root, + head_root, + head_payload_status, + expected_status, + } => { + let actual = fork_choice.get_canonical_payload_status( + &block_root, + &head_root, + head_payload_status, + ); + assert_eq!( + actual, expected_status, + "canonical payload status 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 ea377807951..5bee2196f98 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 @@ -81,6 +81,31 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { expected_payload_status: None, }); + ops.push(Operation::AssertCanonicalPayloadStatus { + block_root: get_root(0), + head_root: get_root(3), + head_payload_status: PayloadStatus::Full, + expected_status: Some(PayloadStatus::Full), + }); + ops.push(Operation::AssertCanonicalPayloadStatus { + block_root: get_root(1), + head_root: get_root(3), + head_payload_status: PayloadStatus::Full, + expected_status: Some(PayloadStatus::Full), + }); + ops.push(Operation::AssertCanonicalPayloadStatus { + block_root: get_root(3), + head_root: get_root(3), + head_payload_status: PayloadStatus::Full, + expected_status: Some(PayloadStatus::Full), + }); + ops.push(Operation::AssertCanonicalPayloadStatus { + block_root: get_root(2), + head_root: get_root(3), + head_payload_status: PayloadStatus::Full, + expected_status: None, + }); + ops.push(Operation::SetPayloadTiebreak { block_root: get_root(0), is_timely: false, @@ -95,6 +120,25 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { expected_payload_status: None, }); + ops.push(Operation::AssertCanonicalPayloadStatus { + block_root: get_root(0), + head_root: get_root(4), + head_payload_status: PayloadStatus::Empty, + expected_status: Some(PayloadStatus::Empty), + }); + ops.push(Operation::AssertCanonicalPayloadStatus { + block_root: get_root(2), + head_root: get_root(4), + head_payload_status: PayloadStatus::Empty, + expected_status: Some(PayloadStatus::Empty), + }); + ops.push(Operation::AssertCanonicalPayloadStatus { + block_root: get_root(1), + head_root: get_root(4), + head_payload_status: PayloadStatus::Empty, + expected_status: None, + }); + ForkChoiceTestDefinition { finalized_block_slot: Slot::new(0), justified_checkpoint: get_checkpoint(0), diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 0ecaea39713..717cec5cbec 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1038,6 +1038,38 @@ impl ProtoArrayForkChoice { .unwrap_or(false) } + /// Returns the canonical payload status of a block by walking backwards from the head. + /// + /// For the head block, returns `head_payload_status` directly. For any ancestor of the + /// head, walks backwards until finding the child of `block_root` on the canonical chain + /// and returns that child's `parent_payload_status`. + /// + /// Returns `None` if the block is not an ancestor of head or has already been pruned from fork choice. + pub fn get_canonical_payload_status( + &self, + block_root: &Hash256, + head_root: &Hash256, + head_payload_status: PayloadStatus, + ) -> Option { + if block_root == head_root { + return Some(head_payload_status); + } + + let target_index = *self.proto_array.indices.get(block_root)?; + let mut current_index = *self.proto_array.indices.get(head_root)?; + + loop { + let node = self.proto_array.nodes.get(current_index)?; + let parent_index = node.parent()?; + + if parent_index == target_index { + return Some(node.get_parent_payload_status()); + } + + current_index = parent_index; + } + } + /// Returns the weight of a given block. pub fn get_weight(&self, block_root: &Hash256) -> Option { let block_index = self.proto_array.indices.get(block_root)?; From a0cd9f5401a063fe8072fd667194eef7930f66c1 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Fri, 3 Apr 2026 08:22:46 -0700 Subject: [PATCH 02/15] Small cleanup --- beacon_node/beacon_chain/src/canonical_head.rs | 6 +++++- consensus/fork_choice/src/fork_choice.rs | 8 ++------ .../src/fork_choice_test_definition.rs | 8 +------- .../fork_choice_test_definition/gloas_payload.rs | 12 ------------ .../proto_array/src/proto_array_fork_choice.rs | 15 +++++++-------- 5 files changed, 15 insertions(+), 34 deletions(-) diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index a60cf1ae4f1..cd693a954b4 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -392,8 +392,12 @@ impl CanonicalHead { let head_root = cached_head.head_block_root(); let head_payload_status = cached_head.head_payload_status(); + if *root == head_root { + return Some(head_payload_status == PayloadStatus::Full); + } + self.fork_choice_read_lock() - .get_canonical_payload_status(root, &head_root, head_payload_status) + .get_canonical_payload_status(root, &head_root) .map(|status| status == PayloadStatus::Full) } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 34bb717889f..15068812583 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1508,14 +1508,10 @@ where &self, block_root: &Hash256, head_root: &Hash256, - head_payload_status: PayloadStatus, ) -> Option { if self.is_finalized_checkpoint_or_descendant(*block_root) { - self.proto_array.get_canonical_payload_status( - block_root, - head_root, - head_payload_status, - ) + self.proto_array + .get_canonical_payload_status(block_root, head_root) } else { None } diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index ec6741000d3..b23327b580d 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -108,7 +108,6 @@ pub enum Operation { AssertCanonicalPayloadStatus { block_root: Hash256, head_root: Hash256, - head_payload_status: PayloadStatus, expected_status: Option, }, } @@ -531,14 +530,9 @@ impl ForkChoiceTestDefinition { Operation::AssertCanonicalPayloadStatus { block_root, head_root, - head_payload_status, expected_status, } => { - let actual = fork_choice.get_canonical_payload_status( - &block_root, - &head_root, - head_payload_status, - ); + let actual = fork_choice.get_canonical_payload_status(&block_root, &head_root); assert_eq!( actual, expected_status, "canonical payload status mismatch at 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 5bee2196f98..89be06dd331 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 @@ -84,25 +84,16 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::AssertCanonicalPayloadStatus { block_root: get_root(0), head_root: get_root(3), - head_payload_status: PayloadStatus::Full, expected_status: Some(PayloadStatus::Full), }); ops.push(Operation::AssertCanonicalPayloadStatus { block_root: get_root(1), head_root: get_root(3), - head_payload_status: PayloadStatus::Full, - expected_status: Some(PayloadStatus::Full), - }); - ops.push(Operation::AssertCanonicalPayloadStatus { - block_root: get_root(3), - head_root: get_root(3), - head_payload_status: PayloadStatus::Full, expected_status: Some(PayloadStatus::Full), }); ops.push(Operation::AssertCanonicalPayloadStatus { block_root: get_root(2), head_root: get_root(3), - head_payload_status: PayloadStatus::Full, expected_status: None, }); @@ -123,19 +114,16 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::AssertCanonicalPayloadStatus { block_root: get_root(0), head_root: get_root(4), - head_payload_status: PayloadStatus::Empty, expected_status: Some(PayloadStatus::Empty), }); ops.push(Operation::AssertCanonicalPayloadStatus { block_root: get_root(2), head_root: get_root(4), - head_payload_status: PayloadStatus::Empty, expected_status: Some(PayloadStatus::Empty), }); ops.push(Operation::AssertCanonicalPayloadStatus { block_root: get_root(1), head_root: get_root(4), - head_payload_status: PayloadStatus::Empty, expected_status: None, }); diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 717cec5cbec..5d008b0a310 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1038,21 +1038,20 @@ impl ProtoArrayForkChoice { .unwrap_or(false) } - /// Returns the canonical payload status of a block by walking backwards from the head. + /// Returns the canonical payload status of a block by walking backwards from the head to the + /// child of `block_root`. /// - /// For the head block, returns `head_payload_status` directly. For any ancestor of the - /// head, walks backwards until finding the child of `block_root` on the canonical chain - /// and returns that child's `parent_payload_status`. - /// - /// Returns `None` if the block is not an ancestor of head or has already been pruned from fork choice. + /// Returns `None` if the block is head, is not an ancestor of head, or has already been pruned + /// from fork choice. pub fn get_canonical_payload_status( &self, block_root: &Hash256, head_root: &Hash256, - head_payload_status: PayloadStatus, ) -> Option { + // Return `None` because the head root won't have + // a child in fork choice to check against. if block_root == head_root { - return Some(head_payload_status); + return None; } let target_index = *self.proto_array.indices.get(block_root)?; From 50f374c94acf5c9af417aea98f586f2390a86504 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Mon, 6 Apr 2026 22:54:52 -0700 Subject: [PATCH 03/15] get payload status by weight plus tests --- .../beacon_chain/src/canonical_head.rs | 2 +- consensus/fork_choice/src/fork_choice.rs | 13 ++-- .../src/fork_choice_test_definition.rs | 35 +++++++++-- .../gloas_payload.rs | 62 +++++++++++++------ .../src/proto_array_fork_choice.rs | 38 ++++-------- 5 files changed, 90 insertions(+), 60 deletions(-) diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index cd693a954b4..e08e8b3f203 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -397,7 +397,7 @@ impl CanonicalHead { } self.fork_choice_read_lock() - .get_canonical_payload_status(root, &head_root) + .get_payload_status_by_weight(root) .map(|status| status == PayloadStatus::Full) } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 15068812583..a9a8328395c 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1502,16 +1502,11 @@ where } } - /// Returns the canonical payload status of a block. See - /// `ProtoArrayForkChoice::get_canonical_payload_status`. - pub fn get_canonical_payload_status( - &self, - block_root: &Hash256, - head_root: &Hash256, - ) -> Option { + /// Returns the payload status of a block. See + /// `ProtoArrayForkChoice::get_payload_status_by_weight`. + pub fn get_payload_status_by_weight(&self, block_root: &Hash256) -> Option { if self.is_finalized_checkpoint_or_descendant(*block_root) { - self.proto_array - .get_canonical_payload_status(block_root, head_root) + self.proto_array.get_payload_status_by_weight(block_root) } else { None } diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index b23327b580d..fb2be25784f 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -61,6 +61,12 @@ pub enum Operation { block_root: Hash256, attestation_slot: Slot, }, + ProcessGloasAttestation { + validator_index: usize, + block_root: Hash256, + attestation_slot: Slot, + payload_present: bool, + }, ProcessPayloadAttestation { validator_index: usize, block_root: Hash256, @@ -105,9 +111,8 @@ pub enum Operation { block_root: Hash256, expected: bool, }, - AssertCanonicalPayloadStatus { + AssertPayloadStatusByWeight { block_root: Hash256, - head_root: Hash256, expected_status: Option, }, } @@ -313,6 +318,27 @@ impl ForkChoiceTestDefinition { }); check_bytes_round_trip(&fork_choice); } + Operation::ProcessGloasAttestation { + validator_index, + block_root, + attestation_slot, + payload_present, + } => { + fork_choice + .process_attestation( + validator_index, + block_root, + attestation_slot, + payload_present, + ) + .unwrap_or_else(|_| { + panic!( + "process_attestation op at index {} returned error", + op_index + ) + }); + check_bytes_round_trip(&fork_choice); + } Operation::ProcessPayloadAttestation { validator_index, block_root, @@ -527,12 +553,11 @@ impl ForkChoiceTestDefinition { op_index ); } - Operation::AssertCanonicalPayloadStatus { + Operation::AssertPayloadStatusByWeight { block_root, - head_root, expected_status, } => { - let actual = fork_choice.get_canonical_payload_status(&block_root, &head_root); + let actual = fork_choice.get_payload_status_by_weight(&block_root); assert_eq!( actual, expected_status, "canonical payload status mismatch at 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 89be06dd331..e178f336fae 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 @@ -81,50 +81,74 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { expected_payload_status: None, }); - ops.push(Operation::AssertCanonicalPayloadStatus { + // Cross-slot attestation with payload_present=true to Full branch (root 3, slot 2). + // vote_slot=3 != block_slot=2, payload_present=true → Full weight. + ops.push(Operation::ProcessGloasAttestation { + validator_index: 0, + block_root: get_root(3), + attestation_slot: Slot::new(3), + payload_present: true, + }); + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1], + expected_head: get_root(3), + current_slot: Slot::new(0), + expected_payload_status: None, + }); + + // Full weight propagated up: root 0 and root 1 should show Full. + ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(0), - head_root: get_root(3), expected_status: Some(PayloadStatus::Full), }); - ops.push(Operation::AssertCanonicalPayloadStatus { + ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(1), - head_root: get_root(3), expected_status: Some(PayloadStatus::Full), }); - ops.push(Operation::AssertCanonicalPayloadStatus { + // Root 2 (Empty branch) has no attestations → both weights zero → defaults to Full via >=. + ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(2), - head_root: get_root(3), - expected_status: None, + expected_status: Some(PayloadStatus::Full), }); - ops.push(Operation::SetPayloadTiebreak { - block_root: get_root(0), - is_timely: false, - is_data_available: false, + // Cross-slot attestations with payload_present=false to Empty branch (root 4, slot 2). + // Two validators so Empty branch outweighs Full branch. + ops.push(Operation::ProcessGloasAttestation { + validator_index: 1, + block_root: get_root(4), + attestation_slot: Slot::new(3), + payload_present: false, + }); + ops.push(Operation::ProcessGloasAttestation { + validator_index: 2, + block_root: get_root(4), + attestation_slot: Slot::new(3), + payload_present: false, }); ops.push(Operation::FindHead { justified_checkpoint: get_checkpoint(0), finalized_checkpoint: get_checkpoint(0), - justified_state_balances: vec![1], + justified_state_balances: vec![1, 1, 1], expected_head: get_root(4), current_slot: Slot::new(0), expected_payload_status: None, }); - ops.push(Operation::AssertCanonicalPayloadStatus { + // Empty weight now dominates: root 0 flips to Empty. + ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(0), - head_root: get_root(4), expected_status: Some(PayloadStatus::Empty), }); - ops.push(Operation::AssertCanonicalPayloadStatus { + ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(2), - head_root: get_root(4), expected_status: Some(PayloadStatus::Empty), }); - ops.push(Operation::AssertCanonicalPayloadStatus { + // Root 1 (Full branch) still has 1 Full vote, 0 Empty → Full. + ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(1), - head_root: get_root(4), - expected_status: None, + expected_status: Some(PayloadStatus::Full), }); ForkChoiceTestDefinition { diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 5d008b0a310..37307695ae2 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1038,35 +1038,21 @@ impl ProtoArrayForkChoice { .unwrap_or(false) } - /// Returns the canonical payload status of a block by walking backwards from the head to the - /// child of `block_root`. + /// Returns the payload status of a block by comparing full and empty payload weight. /// - /// Returns `None` if the block is head, is not an ancestor of head, or has already been pruned - /// from fork choice. - pub fn get_canonical_payload_status( - &self, - block_root: &Hash256, - head_root: &Hash256, - ) -> Option { - // Return `None` because the head root won't have - // a child in fork choice to check against. - if block_root == head_root { - return None; - } - - let target_index = *self.proto_array.indices.get(block_root)?; - let mut current_index = *self.proto_array.indices.get(head_root)?; - - loop { - let node = self.proto_array.nodes.get(current_index)?; - let parent_index = node.parent()?; - - if parent_index == target_index { - return Some(node.get_parent_payload_status()); + /// Returns `None` if a non-gloas node. + pub fn get_payload_status_by_weight(&self, block_root: &Hash256) -> Option { + let index = *self.proto_array.indices.get(block_root)?; + let node = self.proto_array.nodes.get(index)?; + + if let Ok(node) = node.as_v29() { + if node.full_payload_weight >= node.empty_payload_weight { + return Some(PayloadStatus::Full); } + return Some(PayloadStatus::Empty); + }; - current_index = parent_index; - } + None } /// Returns the weight of a given block. From 34cb92a6b60a96efc3740a69cff8b0733dc5c743 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 7 Apr 2026 02:49:47 -0700 Subject: [PATCH 04/15] Tiebreaker logic --- .../beacon_chain/src/canonical_head.rs | 2 +- consensus/fork_choice/src/fork_choice.rs | 14 ++++-- .../src/fork_choice_test_definition.rs | 8 +++- .../gloas_payload.rs | 4 +- consensus/proto_array/src/proto_array.rs | 2 +- .../src/proto_array_fork_choice.rs | 46 ++++++++++++++++--- 6 files changed, 60 insertions(+), 16 deletions(-) diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index e08e8b3f203..c4204f0a362 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -397,7 +397,7 @@ impl CanonicalHead { } self.fork_choice_read_lock() - .get_payload_status_by_weight(root) + .get_canonical_payload_status(root) .map(|status| status == PayloadStatus::Full) } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index a9a8328395c..f6b909c593f 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1502,11 +1502,17 @@ where } } - /// Returns the payload status of a block. See - /// `ProtoArrayForkChoice::get_payload_status_by_weight`. - pub fn get_payload_status_by_weight(&self, block_root: &Hash256) -> Option { + /// Returns the canonical payload status of a block. See + /// `ProtoArrayForkChoice::get_payload_status`. + pub fn get_canonical_payload_status(&self, block_root: &Hash256) -> Option { if self.is_finalized_checkpoint_or_descendant(*block_root) { - self.proto_array.get_payload_status_by_weight(block_root) + let current_slot = self.fc_store.get_current_slot(); + let proposer_boost_root = self.fc_store.proposer_boost_root(); + self.proto_array.get_canonical_payload_status::( + block_root, + current_slot, + proposer_boost_root, + ) } else { None } diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index fb2be25784f..a255c15a939 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -159,6 +159,7 @@ impl ForkChoiceTestDefinition { ) .expect("should create fork choice struct"); let equivocating_indices = BTreeSet::new(); + let mut last_current_slot = Slot::new(0); for (op_index, op) in self.operations.into_iter().enumerate() { match op.clone() { @@ -199,6 +200,7 @@ impl ForkChoiceTestDefinition { op_index, op ); } + last_current_slot = current_slot; check_bytes_round_trip(&fork_choice); } Operation::ProposerBoostFindHead { @@ -557,7 +559,11 @@ impl ForkChoiceTestDefinition { block_root, expected_status, } => { - let actual = fork_choice.get_payload_status_by_weight(&block_root); + let actual = fork_choice.get_canonical_payload_status::( + &block_root, + last_current_slot, + Hash256::zero(), + ); assert_eq!( actual, expected_status, "canonical payload status mismatch at 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 e178f336fae..94c921afcb8 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 @@ -107,10 +107,10 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { block_root: get_root(1), expected_status: Some(PayloadStatus::Full), }); - // Root 2 (Empty branch) has no attestations → both weights zero → defaults to Full via >=. + // Root 2 has no payload received → always Empty. ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(2), - expected_status: Some(PayloadStatus::Full), + expected_status: Some(PayloadStatus::Empty), }); // Cross-slot attestations with payload_present=false to Empty branch (root 4, slot 2). diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 1f7291b2602..6f2e1c3ae73 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -1419,7 +1419,7 @@ impl ProtoArray { } } - fn get_payload_status_tiebreaker( + pub fn get_payload_status_tiebreaker( &self, fc_node: &IndexedForkChoiceNode, proto_node: &ProtoNode, diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 37307695ae2..e207d2759d0 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1038,21 +1038,53 @@ impl ProtoArrayForkChoice { .unwrap_or(false) } - /// Returns the payload status of a block by comparing full and empty payload weight. + /// Returns the canonical payload status of a block by comparing full and empty payload weight. + /// When weights are equal, falls back to `get_payload_status_tiebreaker`. /// /// Returns `None` if a non-gloas node. - pub fn get_payload_status_by_weight(&self, block_root: &Hash256) -> Option { + pub fn get_canonical_payload_status( + &self, + block_root: &Hash256, + current_slot: Slot, + proposer_boost_root: Hash256, + ) -> Option { let index = *self.proto_array.indices.get(block_root)?; let node = self.proto_array.nodes.get(index)?; - if let Ok(node) = node.as_v29() { - if node.full_payload_weight >= node.empty_payload_weight { - return Some(PayloadStatus::Full); - } + let v29 = node.as_v29().ok()?; + + if !v29.payload_received { + return Some(PayloadStatus::Empty); + } + + if v29.full_payload_weight > v29.empty_payload_weight { + return Some(PayloadStatus::Full); + } else if v29.full_payload_weight < v29.empty_payload_weight { return Some(PayloadStatus::Empty); + } + + let tiebreaker_for_status = |status| { + let node_ref = IndexedForkChoiceNode { + root: node.root(), + proto_node_index: index, + payload_status: status, + }; + self.proto_array.get_payload_status_tiebreaker::( + &node_ref, + node, + current_slot, + proposer_boost_root, + ) }; - None + let full_tiebreaker = tiebreaker_for_status(PayloadStatus::Full).ok()?; + let empty_tiebreaker = tiebreaker_for_status(PayloadStatus::Empty).ok()?; + + if full_tiebreaker >= empty_tiebreaker { + Some(PayloadStatus::Full) + } else { + Some(PayloadStatus::Empty) + } } /// Returns the weight of a given block. From f6c288c7b3564c8affb6d6a925818dd6407e73c7 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Mon, 13 Apr 2026 05:17:00 -0700 Subject: [PATCH 05/15] propogate errors --- .../beacon_chain/src/canonical_head.rs | 11 +++--- .../beacon_chain_adapter.rs | 4 +-- .../src/payload_envelope_streamer/mod.rs | 15 ++++---- .../src/payload_envelope_streamer/tests.rs | 18 +++++----- consensus/fork_choice/src/fork_choice.rs | 7 ++-- .../src/fork_choice_test_definition.rs | 4 +-- .../gloas_payload.rs | 12 +++---- consensus/proto_array/src/proto_array.rs | 2 +- .../src/proto_array_fork_choice.rs | 34 ++++++++++++------- 9 files changed, 58 insertions(+), 49 deletions(-) diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index c4204f0a362..81949e66319 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -382,23 +382,20 @@ impl CanonicalHead { Ok((head, execution_status)) } - /// Returns `Some(true)` if the payload for this block is canonical (Full) according to fork choice. - /// - /// Walks backwards from the head to determine the canonical payload status of the block. - /// Returns `None` if the block has already been pruned from fork choice or isn't part of the - /// canonical chain. - pub fn block_has_canonical_payload(&self, root: &Hash256) -> Option { + /// Returns `true` if the payload for this block is canonical (Full) according to fork choice. + pub fn block_has_canonical_payload(&self, root: &Hash256) -> Result { let cached_head = self.cached_head(); let head_root = cached_head.head_block_root(); let head_payload_status = cached_head.head_payload_status(); if *root == head_root { - return Some(head_payload_status == PayloadStatus::Full); + return Ok(head_payload_status == PayloadStatus::Full); } self.fork_choice_read_lock() .get_canonical_payload_status(root) .map(|status| status == PayloadStatus::Full) + .map_err(Error::ForkChoiceError) } /// Returns a clone of `self.cached_head`. diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs index c9de2d09657..c6d0c797fdd 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs @@ -5,7 +5,7 @@ use mockall::automock; use task_executor::TaskExecutor; use types::{Hash256, SignedExecutionPayloadEnvelope, Slot}; -use crate::{BeaconChain, BeaconChainTypes}; +use crate::{BeaconChain, BeaconChainError, BeaconChainTypes}; /// An adapter to the `BeaconChain` functionalities to remove `BeaconChain` from direct dependency to enable testing envelope streamer logic. pub(crate) struct EnvelopeStreamerBeaconAdapter { @@ -33,7 +33,7 @@ impl EnvelopeStreamerBeaconAdapter { self.chain.store.get_split_info().slot } - pub(crate) fn block_has_canonical_payload(&self, root: &Hash256) -> Option { + pub(crate) fn block_has_canonical_payload(&self, root: &Hash256) -> Result { self.chain.canonical_head.block_has_canonical_payload(root) } } diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs index 6700e7a4cf6..3fc4cf4be71 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs @@ -11,7 +11,7 @@ use futures::Stream; use mockall_double::double; use tokio::sync::mpsc::{self, UnboundedSender}; use tokio_stream::wrappers::UnboundedReceiverStream; -use tracing::{debug, error, warn}; +use tracing::{debug, error, info, warn}; use types::{EthSpec, Hash256, SignedExecutionPayloadEnvelope}; #[cfg(not(test))] @@ -125,19 +125,22 @@ impl PayloadEnvelopeStreamer { } match streamer.adapter.block_has_canonical_payload(root) { - Some(is_envelope_canonical) => { + Ok(is_envelope_canonical) => { if is_envelope_canonical { results.push((*root, Ok(Some(envelope)))); } else { results.push((*root, Ok(None))); } } - None => { + Err(e) => { + info!( + error = ?e, + block_root = ?root, + "error verifying canonicity of payload envelope" + ); results.push(( *root, - Err(BeaconChainError::EnvelopeStreamerError( - Error::BlockMissingFromForkChoice, - )), + Err(e), )); } } diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/tests.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/tests.rs index bbed3017e3e..1c6883b659c 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/tests.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::beacon_chain::ForkChoiceError; use crate::payload_envelope_streamer::beacon_chain_adapter::MockEnvelopeStreamerBeaconAdapter; use crate::test_utils::EphemeralHarnessType; use bls::{FixedBytesExtended, Signature}; @@ -115,7 +116,7 @@ fn mock_canonical_head(mock: &mut MockEnvelopeStreamerBeaconAdapter, chain: & .map(|e| e.block_root) .collect(); mock.expect_block_has_canonical_payload() - .returning(move |root| Some(!non_canonical.contains(root))); + .returning(move |root| Ok(!non_canonical.contains(root))); } fn unwrap_result( @@ -280,7 +281,7 @@ async fn stream_envelopes_by_root() { } /// When `block_has_canonical_payload` returns an error, the streamer should -/// yield `Err(EnvelopeStreamerError(BlockMissingFromForkChoice))` for those roots. +/// propagate that error for those roots. #[tokio::test] async fn stream_envelopes_error() { let chain = build_chain(4, &[], &[], &[]); @@ -288,7 +289,9 @@ async fn stream_envelopes_error() { mock.expect_get_split_slot().return_const(Slot::new(0)); mock_envelopes(&mut mock, &chain); mock.expect_block_has_canonical_payload() - .returning(|_| None); + .returning(|_| Err(BeaconChainError::ForkChoiceError( + ForkChoiceError::DoesNotDescendFromFinalizedCheckpoint, + ))); let streamer = PayloadEnvelopeStreamer::new(mock, EnvelopeRequestSource::ByRange); let mut stream = streamer.launch_stream(roots(&chain)); @@ -300,13 +303,8 @@ async fn stream_envelopes_error() { .unwrap_or_else(|| panic!("stream ended early at index {i}")); assert_eq!(root, entry.block_root, "root mismatch at index {i}"); assert!( - matches!( - result.as_ref(), - Err(BeaconChainError::EnvelopeStreamerError( - Error::BlockMissingFromForkChoice - )) - ), - "expected BlockMissingFromForkChoice error at index {i}, got {:?}", + result.as_ref().is_err(), + "expected error at index {i}, got {:?}", result ); } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index f6b909c593f..5dd69638fc5 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -78,6 +78,7 @@ pub enum Error { UnrealizedVoteProcessing(state_processing::EpochProcessingError), ValidatorStatuses(BeaconStateError), ChainSpecError(String), + DoesNotDescendFromFinalizedCheckpoint, } impl From for Error { @@ -1504,7 +1505,7 @@ where /// Returns the canonical payload status of a block. See /// `ProtoArrayForkChoice::get_payload_status`. - pub fn get_canonical_payload_status(&self, block_root: &Hash256) -> Option { + pub fn get_canonical_payload_status(&self, block_root: &Hash256) -> Result> { if self.is_finalized_checkpoint_or_descendant(*block_root) { let current_slot = self.fc_store.get_current_slot(); let proposer_boost_root = self.fc_store.proposer_boost_root(); @@ -1512,9 +1513,9 @@ where block_root, current_slot, proposer_boost_root, - ) + ).map_err(Error::ProtoArrayError) } else { - None + Err(Error::DoesNotDescendFromFinalizedCheckpoint) } } diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index a255c15a939..43cc4bd086b 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -113,7 +113,7 @@ pub enum Operation { }, AssertPayloadStatusByWeight { block_root: Hash256, - expected_status: Option, + expected_status: PayloadStatus, }, } @@ -563,7 +563,7 @@ impl ForkChoiceTestDefinition { &block_root, last_current_slot, Hash256::zero(), - ); + ).unwrap(); assert_eq!( actual, expected_status, "canonical payload status mismatch at 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 94c921afcb8..74ffb806a65 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 @@ -101,16 +101,16 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { // Full weight propagated up: root 0 and root 1 should show Full. ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(0), - expected_status: Some(PayloadStatus::Full), + expected_status: PayloadStatus::Full, }); ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(1), - expected_status: Some(PayloadStatus::Full), + expected_status: PayloadStatus::Full, }); // Root 2 has no payload received → always Empty. ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(2), - expected_status: Some(PayloadStatus::Empty), + expected_status: PayloadStatus::Empty, }); // Cross-slot attestations with payload_present=false to Empty branch (root 4, slot 2). @@ -139,16 +139,16 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { // Empty weight now dominates: root 0 flips to Empty. ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(0), - expected_status: Some(PayloadStatus::Empty), + expected_status: PayloadStatus::Empty, }); ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(2), - expected_status: Some(PayloadStatus::Empty), + expected_status: PayloadStatus::Empty, }); // Root 1 (Full branch) still has 1 Full vote, 0 Empty → Full. ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(1), - expected_status: Some(PayloadStatus::Full), + expected_status: PayloadStatus::Full, }); ForkChoiceTestDefinition { diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 6f2e1c3ae73..7c4682199dc 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -1419,7 +1419,7 @@ impl ProtoArray { } } - pub fn get_payload_status_tiebreaker( + pub(crate) fn get_payload_status_tiebreaker( &self, fc_node: &IndexedForkChoiceNode, proto_node: &ProtoNode, diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index e207d2759d0..2aacee14a76 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1047,26 +1047,36 @@ impl ProtoArrayForkChoice { block_root: &Hash256, current_slot: Slot, proposer_boost_root: Hash256, - ) -> Option { - let index = *self.proto_array.indices.get(block_root)?; - let node = self.proto_array.nodes.get(index)?; + ) -> Result { + let block_index = self + .proto_array + .indices + .get(block_root) + .ok_or(Error::NodeUnknown(*block_root))?; + let node = self + .proto_array + .nodes + .get(*block_index) + .ok_or(Error::InvalidNodeIndex(*block_index))?; - let v29 = node.as_v29().ok()?; + let v29 = node.as_v29().map_err(|_| Error::InvalidNodeVariant { + block_root: *block_root, + })?; if !v29.payload_received { - return Some(PayloadStatus::Empty); + return Ok(PayloadStatus::Empty); } if v29.full_payload_weight > v29.empty_payload_weight { - return Some(PayloadStatus::Full); + return Ok(PayloadStatus::Full); } else if v29.full_payload_weight < v29.empty_payload_weight { - return Some(PayloadStatus::Empty); + return Ok(PayloadStatus::Empty); } let tiebreaker_for_status = |status| { let node_ref = IndexedForkChoiceNode { root: node.root(), - proto_node_index: index, + proto_node_index: *block_index, payload_status: status, }; self.proto_array.get_payload_status_tiebreaker::( @@ -1077,13 +1087,13 @@ impl ProtoArrayForkChoice { ) }; - let full_tiebreaker = tiebreaker_for_status(PayloadStatus::Full).ok()?; - let empty_tiebreaker = tiebreaker_for_status(PayloadStatus::Empty).ok()?; + let full_tiebreaker = tiebreaker_for_status(PayloadStatus::Full)?; + let empty_tiebreaker = tiebreaker_for_status(PayloadStatus::Empty)?; if full_tiebreaker >= empty_tiebreaker { - Some(PayloadStatus::Full) + Ok(PayloadStatus::Full) } else { - Some(PayloadStatus::Empty) + Ok(PayloadStatus::Empty) } } From 40e2ed63255e6768d92a7e57bda81c9e761dd020 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Mon, 13 Apr 2026 05:27:11 -0700 Subject: [PATCH 06/15] smol changes --- consensus/fork_choice/src/fork_choice.rs | 2 +- consensus/proto_array/src/fork_choice_test_definition.rs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 5dd69638fc5..4c5dc3ee9d8 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1504,7 +1504,7 @@ where } /// Returns the canonical payload status of a block. See - /// `ProtoArrayForkChoice::get_payload_status`. + /// `ProtoArrayForkChoice::get_canonical_payload_status`. pub fn get_canonical_payload_status(&self, block_root: &Hash256) -> Result> { if self.is_finalized_checkpoint_or_descendant(*block_root) { let current_slot = self.fc_store.get_current_slot(); diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 43cc4bd086b..5a66b3f3b75 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -30,6 +30,8 @@ pub enum Operation { justified_state_balances: Vec, expected_head: Hash256, current_slot: Slot, + // TODO(gloas): Make this non-optional. `find_head` always returns a `PayloadStatus` + // (Empty for pre-GLOAS), so every test should assert on it explicitly. #[serde(default)] expected_payload_status: Option, }, From 88fbe3a5249853e0c603439a306c31137fcbde16 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Mon, 13 Apr 2026 05:29:16 -0700 Subject: [PATCH 07/15] Update --- .../beacon_chain_adapter.rs | 5 ++++- .../src/payload_envelope_streamer/mod.rs | 5 +---- .../src/payload_envelope_streamer/tests.rs | 7 ++++--- consensus/fork_choice/src/fork_choice.rs | 13 +++++++------ .../proto_array/src/fork_choice_test_definition.rs | 12 +++++++----- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs index c6d0c797fdd..47c58f07b90 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs @@ -33,7 +33,10 @@ impl EnvelopeStreamerBeaconAdapter { self.chain.store.get_split_info().slot } - pub(crate) fn block_has_canonical_payload(&self, root: &Hash256) -> Result { + pub(crate) fn block_has_canonical_payload( + &self, + root: &Hash256, + ) -> Result { self.chain.canonical_head.block_has_canonical_payload(root) } } diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs index 3fc4cf4be71..1f7eacdd775 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs @@ -138,10 +138,7 @@ impl PayloadEnvelopeStreamer { block_root = ?root, "error verifying canonicity of payload envelope" ); - results.push(( - *root, - Err(e), - )); + results.push((*root, Err(e))); } } } else { diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/tests.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/tests.rs index 1c6883b659c..22c0fd542eb 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/tests.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/tests.rs @@ -288,10 +288,11 @@ async fn stream_envelopes_error() { let (mut mock, _runtime) = mock_adapter(); mock.expect_get_split_slot().return_const(Slot::new(0)); mock_envelopes(&mut mock, &chain); - mock.expect_block_has_canonical_payload() - .returning(|_| Err(BeaconChainError::ForkChoiceError( + mock.expect_block_has_canonical_payload().returning(|_| { + Err(BeaconChainError::ForkChoiceError( ForkChoiceError::DoesNotDescendFromFinalizedCheckpoint, - ))); + )) + }); let streamer = PayloadEnvelopeStreamer::new(mock, EnvelopeRequestSource::ByRange); let mut stream = streamer.launch_stream(roots(&chain)); diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 4c5dc3ee9d8..fccdf8c8a06 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1505,15 +1505,16 @@ where /// Returns the canonical payload status of a block. See /// `ProtoArrayForkChoice::get_canonical_payload_status`. - pub fn get_canonical_payload_status(&self, block_root: &Hash256) -> Result> { + pub fn get_canonical_payload_status( + &self, + block_root: &Hash256, + ) -> Result> { if self.is_finalized_checkpoint_or_descendant(*block_root) { let current_slot = self.fc_store.get_current_slot(); let proposer_boost_root = self.fc_store.proposer_boost_root(); - self.proto_array.get_canonical_payload_status::( - block_root, - current_slot, - proposer_boost_root, - ).map_err(Error::ProtoArrayError) + self.proto_array + .get_canonical_payload_status::(block_root, current_slot, proposer_boost_root) + .map_err(Error::ProtoArrayError) } else { Err(Error::DoesNotDescendFromFinalizedCheckpoint) } diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 5a66b3f3b75..9056800c0b1 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -561,11 +561,13 @@ impl ForkChoiceTestDefinition { block_root, expected_status, } => { - let actual = fork_choice.get_canonical_payload_status::( - &block_root, - last_current_slot, - Hash256::zero(), - ).unwrap(); + let actual = fork_choice + .get_canonical_payload_status::( + &block_root, + last_current_slot, + Hash256::zero(), + ) + .unwrap(); assert_eq!( actual, expected_status, "canonical payload status mismatch at op index {}", From 94d52350becb4b40185c4660c304b4be0f3de8a3 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Mon, 13 Apr 2026 05:44:36 -0700 Subject: [PATCH 08/15] make forward compatible --- .../src/proto_array_fork_choice.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 2aacee14a76..1204a15ee52 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1053,23 +1053,30 @@ impl ProtoArrayForkChoice { .indices .get(block_root) .ok_or(Error::NodeUnknown(*block_root))?; + let node = self .proto_array .nodes .get(*block_index) .ok_or(Error::InvalidNodeIndex(*block_index))?; - let v29 = node.as_v29().map_err(|_| Error::InvalidNodeVariant { - block_root: *block_root, - })?; + let (Ok(payload_received), Ok(full_payload_weight), Ok(empty_payload_weight)) = ( + node.payload_received(), + node.full_payload_weight(), + node.empty_payload_weight(), + ) else { + return Err(Error::InvalidNodeVariant { + block_root: *block_root, + }); + }; - if !v29.payload_received { + if !payload_received { return Ok(PayloadStatus::Empty); } - if v29.full_payload_weight > v29.empty_payload_weight { + if full_payload_weight > empty_payload_weight { return Ok(PayloadStatus::Full); - } else if v29.full_payload_weight < v29.empty_payload_weight { + } else if full_payload_weight < empty_payload_weight { return Ok(PayloadStatus::Empty); } From cbe8b7aecc41879bc9c8f61a43bc07c56f43bace Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Mon, 13 Apr 2026 05:58:26 -0700 Subject: [PATCH 09/15] Add test for payload status --- .../src/fork_choice_test_definition/gloas_payload.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) 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 74ffb806a65..b0fd8828225 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 @@ -612,6 +612,11 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef current_slot: Slot::new(1), expected_payload_status: None, }); + // Weights are tied (1 vote each branch), tiebreaker is Empty. + ops.push(Operation::AssertPayloadStatusByWeight { + block_root: get_root(0), + expected_status: PayloadStatus::Empty, + }); // Step 5: Flip tiebreaker to Full → Full branch wins. ops.push(Operation::SetPayloadTiebreak { @@ -627,6 +632,11 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef current_slot: Slot::new(100), expected_payload_status: None, }); + // Weights still tied, tiebreaker flipped to Full. + ops.push(Operation::AssertPayloadStatusByWeight { + block_root: get_root(0), + expected_status: PayloadStatus::Full, + }); // Step 6: Add extra CL weight to Empty branch → overrides Full tiebreaker. ops.push(Operation::ProcessAttestation { From 9b00020c46f18fb8b588f46b0e9cceeb22db4710 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Fri, 17 Apr 2026 12:48:50 +0900 Subject: [PATCH 10/15] outdated comment --- consensus/proto_array/src/proto_array_fork_choice.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 1204a15ee52..346f158850a 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1040,8 +1040,6 @@ impl ProtoArrayForkChoice { /// Returns the canonical payload status of a block by comparing full and empty payload weight. /// When weights are equal, falls back to `get_payload_status_tiebreaker`. - /// - /// Returns `None` if a non-gloas node. pub fn get_canonical_payload_status( &self, block_root: &Hash256, From 2d5f1937888aa13cc9742c8de4649fbcc2357697 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Fri, 17 Apr 2026 12:49:14 +0900 Subject: [PATCH 11/15] make it a warn{ --- beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs index 1f7eacdd775..949c6ce57f5 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs @@ -133,7 +133,7 @@ impl PayloadEnvelopeStreamer { } } Err(e) => { - info!( + warn!( error = ?e, block_root = ?root, "error verifying canonicity of payload envelope" From 19f7c01d8303a91cd262075242ca1eccaaf32ae4 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Fri, 17 Apr 2026 12:51:37 +0900 Subject: [PATCH 12/15] fmt --- beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs index 949c6ce57f5..c33579da20e 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs @@ -11,7 +11,7 @@ use futures::Stream; use mockall_double::double; use tokio::sync::mpsc::{self, UnboundedSender}; use tokio_stream::wrappers::UnboundedReceiverStream; -use tracing::{debug, error, info, warn}; +use tracing::{debug, error, warn}; use types::{EthSpec, Hash256, SignedExecutionPayloadEnvelope}; #[cfg(not(test))] From 0a2014cae791b44473503581255904ebefea32a6 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Fri, 17 Apr 2026 17:11:56 +0900 Subject: [PATCH 13/15] Fix block_has_canonical_payload impl and add test cases --- .../beacon_chain/src/canonical_head.rs | 8 +- .../beacon_chain_adapter.rs | 4 +- consensus/fork_choice/src/fork_choice.rs | 8 +- .../src/fork_choice_test_definition.rs | 65 ++++++- .../gloas_payload.rs | 173 ++++++++++++++++++ consensus/proto_array/src/proto_array.rs | 84 +++++++++ .../src/proto_array_fork_choice.rs | 66 +------ 7 files changed, 345 insertions(+), 63 deletions(-) diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 81949e66319..a80ce41ec8c 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -383,7 +383,11 @@ impl CanonicalHead { } /// Returns `true` if the payload for this block is canonical (Full) according to fork choice. - pub fn block_has_canonical_payload(&self, root: &Hash256) -> Result { + pub fn block_has_canonical_payload( + &self, + root: &Hash256, + spec: &ChainSpec, + ) -> Result { let cached_head = self.cached_head(); let head_root = cached_head.head_block_root(); let head_payload_status = cached_head.head_payload_status(); @@ -393,7 +397,7 @@ impl CanonicalHead { } self.fork_choice_read_lock() - .get_canonical_payload_status(root) + .get_canonical_payload_status(root, spec) .map(|status| status == PayloadStatus::Full) .map_err(Error::ForkChoiceError) } diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs index 47c58f07b90..4e36cf78951 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/beacon_chain_adapter.rs @@ -37,6 +37,8 @@ impl EnvelopeStreamerBeaconAdapter { &self, root: &Hash256, ) -> Result { - self.chain.canonical_head.block_has_canonical_payload(root) + self.chain + .canonical_head + .block_has_canonical_payload(root, &self.chain.spec) } } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index fccdf8c8a06..63828dffcda 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1508,12 +1508,18 @@ where pub fn get_canonical_payload_status( &self, block_root: &Hash256, + spec: &ChainSpec, ) -> Result> { if self.is_finalized_checkpoint_or_descendant(*block_root) { let current_slot = self.fc_store.get_current_slot(); let proposer_boost_root = self.fc_store.proposer_boost_root(); self.proto_array - .get_canonical_payload_status::(block_root, current_slot, proposer_boost_root) + .get_canonical_payload_status::( + block_root, + current_slot, + proposer_boost_root, + spec, + ) .map_err(Error::ProtoArrayError) } else { Err(Error::DoesNotDescendFromFinalizedCheckpoint) diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 9056800c0b1..d537f16bb27 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -4,6 +4,7 @@ mod gloas_payload; mod no_votes; mod votes; +use crate::error::Error; use crate::proto_array_fork_choice::{Block, ExecutionStatus, PayloadStatus, ProtoArrayForkChoice}; use crate::{InvalidationOperation, JustifiedBalances}; use fixed_bytes::FixedBytesExtended; @@ -116,6 +117,12 @@ pub enum Operation { AssertPayloadStatusByWeight { block_root: Hash256, expected_status: PayloadStatus, + /// Override `current_slot`. Defaults to the `current_slot` of the last `FindHead`. + #[serde(default)] + current_slot: Option, + /// Override the proposer boost root. Defaults to `Hash256::zero()`. + #[serde(default)] + proposer_boost_root: Option, }, } @@ -202,6 +209,15 @@ impl ForkChoiceTestDefinition { op_index, op ); } + assert_canonical_payload_status_matches_find_head( + &fork_choice, + &head, + current_slot, + Hash256::zero(), + &spec, + payload_status, + op_index, + ); last_current_slot = current_slot; check_bytes_round_trip(&fork_choice); } @@ -215,7 +231,7 @@ impl ForkChoiceTestDefinition { let justified_balances = JustifiedBalances::from_effective_balances(justified_state_balances) .unwrap(); - let (head, _payload_status) = fork_choice + let (head, payload_status) = fork_choice .find_head::( justified_checkpoint, finalized_checkpoint, @@ -234,6 +250,15 @@ impl ForkChoiceTestDefinition { "Operation at index {} failed head check. Operation: {:?}", op_index, op ); + assert_canonical_payload_status_matches_find_head( + &fork_choice, + &head, + Slot::new(0), + proposer_boost_root, + &spec, + payload_status, + op_index, + ); check_bytes_round_trip(&fork_choice); } Operation::InvalidFindHead { @@ -560,12 +585,15 @@ impl ForkChoiceTestDefinition { Operation::AssertPayloadStatusByWeight { block_root, expected_status, + current_slot, + proposer_boost_root, } => { let actual = fork_choice .get_canonical_payload_status::( &block_root, - last_current_slot, - Hash256::zero(), + current_slot.unwrap_or(last_current_slot), + proposer_boost_root.unwrap_or_else(Hash256::zero), + &spec, ) .unwrap(); assert_eq!( @@ -598,6 +626,37 @@ fn get_checkpoint(i: u64) -> Checkpoint { } } +/// Checks that `get_canonical_payload_status` agrees with the `payload_status` +/// returned by `find_head` for the head block. +fn assert_canonical_payload_status_matches_find_head( + fork_choice: &ProtoArrayForkChoice, + head: &Hash256, + current_slot: Slot, + proposer_boost_root: Hash256, + spec: &ChainSpec, + expected: PayloadStatus, + op_index: usize, +) { + match fork_choice.get_canonical_payload_status::( + head, + current_slot, + proposer_boost_root, + spec, + ) { + Ok(actual) => assert_eq!( + actual, expected, + "get_canonical_payload_status disagreed with find_head for head {:?} at op index {}", + head, op_index + ), + // Skip the check for pre-gloas nodes + Err(Error::InvalidNodeVariant { .. }) => {} + Err(e) => panic!( + "get_canonical_payload_status failed at op index {}: {:?}", + op_index, e + ), + } +} + fn check_bytes_round_trip(original: &ProtoArrayForkChoice) { let bytes = original.as_bytes(); let decoded = ProtoArrayForkChoice::from_bytes(&bytes, original.balances.clone()) 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 1b6c01feccd..f42a1dcc300 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 @@ -102,15 +102,21 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(0), expected_status: PayloadStatus::Full, + current_slot: None, + proposer_boost_root: None, }); ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(1), expected_status: PayloadStatus::Full, + current_slot: None, + proposer_boost_root: None, }); // Root 2 has no payload received → always Empty. ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(2), expected_status: PayloadStatus::Empty, + current_slot: None, + proposer_boost_root: None, }); // Cross-slot attestations with payload_present=false to Empty branch (root 4, slot 2). @@ -140,15 +146,21 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(0), expected_status: PayloadStatus::Empty, + current_slot: None, + proposer_boost_root: None, }); ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(2), expected_status: PayloadStatus::Empty, + current_slot: None, + proposer_boost_root: None, }); // Root 1 (Full branch) still has 1 Full vote, 0 Empty → Full. ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(1), expected_status: PayloadStatus::Full, + current_slot: None, + proposer_boost_root: None, }); ForkChoiceTestDefinition { @@ -616,6 +628,8 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(0), expected_status: PayloadStatus::Empty, + current_slot: None, + proposer_boost_root: None, }); // Step 5: Flip tiebreaker to Full → Full branch wins. @@ -636,6 +650,8 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(0), expected_status: PayloadStatus::Full, + current_slot: None, + proposer_boost_root: None, }); // Step 6: Add extra CL weight to Empty branch → overrides Full tiebreaker. @@ -780,6 +796,151 @@ pub fn get_gloas_payload_received_interleaving_test_definition() -> ForkChoiceTe } } +/// When `current_slot == node.slot + 1`, spec `get_weight` zeroes out Full and Empty +/// weights so the tiebreaker decides. Tests that the zero-out is applied and +/// doesn't just compare raw payload weights. +pub fn get_gloas_previous_slot_tiebreaker_test_definition() -> ForkChoiceTestDefinition { + let mut ops = vec![]; + + // Block 1 at slot 1 with its payload received. + 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), + }); + + // Block 2 at slot 2 with a mismatched EL parent hash → parent payload status 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(99)), + execution_payload_block_hash: Some(get_hash(2)), + }); + + // More Full weight than Empty on block 1. + ops.push(Operation::ProcessGloasAttestation { + validator_index: 0, + block_root: get_root(1), + attestation_slot: Slot::new(2), + payload_present: true, + }); + + // Before zero-out (current_slot == block 1's slot), raw weights decide → Full. + ops.push(Operation::AssertPayloadStatusByWeight { + block_root: get_root(1), + expected_status: PayloadStatus::Full, + current_slot: Some(Slot::new(1)), + proposer_boost_root: None, + }); + + // At current_slot == block 1's slot + 1, both weights zero out and the + // tiebreaker picks Empty (block 2 extends block 1 with an Empty parent + // payload status). + ops.push(Operation::AssertPayloadStatusByWeight { + block_root: get_root(1), + expected_status: PayloadStatus::Empty, + current_slot: Some(Slot::new(2)), + proposer_boost_root: Some(get_root(2)), + }); + + 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()), + } +} + +/// Proposer boost on a descendant can flip an ancestor's canonical payload status. +/// Boost supports the ancestor's Full variant (via the descendant's Full parent +/// payload status) but not Empty, so a large enough boost overrides raw Empty weight. +pub fn get_gloas_proposer_boost_flips_ancestor_test_definition() -> ForkChoiceTestDefinition { + let mut ops = vec![]; + + // Block 1 at slot 1 with payload received. + 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), + }); + + // Block 2 at slot 3 with a Full parent payload status (skip slot 2 so + // block 1's previous-slot zero-out doesn't fire at current_slot 3). + ops.push(Operation::ProcessBlock { + slot: Slot::new(3), + 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)), + }); + + // One Empty vote on block 1. Balance totals are chosen so the proposer + // boost score exceeds the single Empty voter's balance. + ops.push(Operation::ProcessGloasAttestation { + validator_index: 0, + block_root: get_root(1), + attestation_slot: Slot::new(2), + payload_present: false, + }); + + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![100, 10000], + expected_head: get_root(1), + current_slot: Slot::new(3), + expected_payload_status: Some(PayloadStatus::Empty), + }); + + // Without boost: raw weights decide → Empty. + ops.push(Operation::AssertPayloadStatusByWeight { + block_root: get_root(1), + expected_status: PayloadStatus::Empty, + current_slot: Some(Slot::new(3)), + proposer_boost_root: None, + }); + + // With boost on block 2: boost supports block 1's Full variant → Full. + ops.push(Operation::AssertPayloadStatusByWeight { + block_root: get_root(1), + expected_status: PayloadStatus::Full, + current_slot: Some(Slot::new(3)), + proposer_boost_root: Some(get_root(2)), + }); + + 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()), + } +} + #[cfg(test)] mod tests { use super::*; @@ -957,6 +1118,18 @@ mod tests { test.run(); } + #[test] + fn previous_slot_tiebreaker() { + let test = get_gloas_previous_slot_tiebreaker_test_definition(); + test.run(); + } + + #[test] + fn proposer_boost_flips_ancestor() { + let test = get_gloas_proposer_boost_flips_ancestor_test_definition(); + test.run(); + } + /// Test that execution payload invalidation propagates across the V17→V29 fork /// boundary: after invalidating a V17 parent, head must not select any descendant. /// diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index ffb7d5326b3..c36045857dc 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -1263,6 +1263,90 @@ impl ProtoArray { } } + /// 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( + &self, + root: Hash256, + current_slot: Slot, + proposer_boost_root: Hash256, + justified_balances: &JustifiedBalances, + spec: &ChainSpec, + ) -> Result { + let proto_node_index = *self.indices.get(&root).ok_or(Error::NodeUnknown(root))?; + let proto_node = self + .nodes + .get(proto_node_index) + .ok_or(Error::InvalidNodeIndex(proto_node_index))?; + + if !proto_node + .payload_received() + .map_err(|_| Error::InvalidNodeVariant { block_root: root })? + { + return Ok(PayloadStatus::Empty); + } + + let full_fc = IndexedForkChoiceNode { + root, + proto_node_index, + payload_status: PayloadStatus::Full, + }; + let empty_fc = IndexedForkChoiceNode { + root, + proto_node_index, + payload_status: PayloadStatus::Empty, + }; + + // Matches the hoisting optimization in `find_head`: `get_weight`'s spec-level + // `should_apply_proposer_boost` check is precomputed once. + let apply_proposer_boost = + self.should_apply_proposer_boost::(proposer_boost_root, justified_balances, spec)?; + + let full_weight = self.get_weight::( + &full_fc, + proto_node, + apply_proposer_boost, + proposer_boost_root, + current_slot, + justified_balances, + spec, + )?; + + let empty_weight = self.get_weight::( + &empty_fc, + proto_node, + apply_proposer_boost, + proposer_boost_root, + current_slot, + justified_balances, + spec, + )?; + + match full_weight.cmp(&empty_weight) { + std::cmp::Ordering::Greater => Ok(PayloadStatus::Full), + std::cmp::Ordering::Less => Ok(PayloadStatus::Empty), + std::cmp::Ordering::Equal => { + let full_tb = self.get_payload_status_tiebreaker::( + &full_fc, + proto_node, + current_slot, + proposer_boost_root, + )?; + let empty_tb = self.get_payload_status_tiebreaker::( + &empty_fc, + proto_node, + current_slot, + proposer_boost_root, + )?; + if full_tb >= empty_tb { + Ok(PayloadStatus::Full) + } else { + Ok(PayloadStatus::Empty) + } + } + } + } + /// Spec: `get_weight`. #[allow(clippy::too_many_arguments)] fn get_weight( diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 346f158850a..e2f16c4a29f 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1038,68 +1038,22 @@ impl ProtoArrayForkChoice { .unwrap_or(false) } - /// Returns the canonical payload status of a block by comparing full and empty payload weight. - /// When weights are equal, falls back to `get_payload_status_tiebreaker`. + /// 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( &self, block_root: &Hash256, current_slot: Slot, proposer_boost_root: Hash256, + spec: &ChainSpec, ) -> Result { - let block_index = self - .proto_array - .indices - .get(block_root) - .ok_or(Error::NodeUnknown(*block_root))?; - - let node = self - .proto_array - .nodes - .get(*block_index) - .ok_or(Error::InvalidNodeIndex(*block_index))?; - - let (Ok(payload_received), Ok(full_payload_weight), Ok(empty_payload_weight)) = ( - node.payload_received(), - node.full_payload_weight(), - node.empty_payload_weight(), - ) else { - return Err(Error::InvalidNodeVariant { - block_root: *block_root, - }); - }; - - if !payload_received { - return Ok(PayloadStatus::Empty); - } - - if full_payload_weight > empty_payload_weight { - return Ok(PayloadStatus::Full); - } else if full_payload_weight < empty_payload_weight { - return Ok(PayloadStatus::Empty); - } - - let tiebreaker_for_status = |status| { - let node_ref = IndexedForkChoiceNode { - root: node.root(), - proto_node_index: *block_index, - payload_status: status, - }; - self.proto_array.get_payload_status_tiebreaker::( - &node_ref, - node, - current_slot, - proposer_boost_root, - ) - }; - - let full_tiebreaker = tiebreaker_for_status(PayloadStatus::Full)?; - let empty_tiebreaker = tiebreaker_for_status(PayloadStatus::Empty)?; - - if full_tiebreaker >= empty_tiebreaker { - Ok(PayloadStatus::Full) - } else { - Ok(PayloadStatus::Empty) - } + self.proto_array.get_canonical_payload_status::( + *block_root, + current_slot, + proposer_boost_root, + &self.balances, + spec, + ) } /// Returns the weight of a given block. From 1f25b95d78084db35bd400b29bfad904d0aea8a6 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Fri, 17 Apr 2026 17:43:17 +0900 Subject: [PATCH 14/15] Clean up --- .../gloas_payload.rs | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) 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 f42a1dcc300..a8e610deabc 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 @@ -82,7 +82,7 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { }); // Cross-slot attestation with payload_present=true to Full branch (root 3, slot 2). - // vote_slot=3 != block_slot=2, payload_present=true → Full weight. + // vote_slot=3 differs from block_slot=2 and payload_present=true, so it counts as Full weight. ops.push(Operation::ProcessGloasAttestation { validator_index: 0, block_root: get_root(3), @@ -111,7 +111,7 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { current_slot: None, proposer_boost_root: None, }); - // Root 2 has no payload received → always Empty. + // Root 2 has no payload received, so it's always Empty. ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(2), expected_status: PayloadStatus::Empty, @@ -142,7 +142,7 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { expected_payload_status: None, }); - // Empty weight now dominates: root 0 flips to Empty. + // Empty weight now dominates, so root 0 flips to Empty. ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(0), expected_status: PayloadStatus::Empty, @@ -155,7 +155,7 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { current_slot: None, proposer_boost_root: None, }); - // Root 1 (Full branch) still has 1 Full vote, 0 Empty → Full. + // Root 1 (Full branch) still has 1 Full vote and 0 Empty, so it stays Full. ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(1), expected_status: PayloadStatus::Full, @@ -209,7 +209,7 @@ pub fn get_gloas_payload_probe_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: vec![1, 1], expected_head: get_root(1), current_slot: Slot::new(0), - // With MainnetEthSpec PTC_SIZE=512, 1 bit set out of 256 threshold → not timely → Empty. + // With MainnetEthSpec PTC_SIZE=512 and a 256-bit threshold, 1 bit set is not timely, so Empty. expected_payload_status: Some(PayloadStatus::Empty), }); // PTC votes write to bitfields only, not to full/empty weight. @@ -350,7 +350,7 @@ pub fn get_gloas_find_head_vote_transition_test_definition() -> ForkChoiceTestDe expected_payload_status: None, }); - // CL attestation to Empty branch (root 4) from validator 0 → head flips to 4. + // CL attestation to Empty branch (root 4) from validator 0 flips the head to 4. ops.push(Operation::ProcessAttestation { validator_index: 0, block_root: get_root(4), @@ -365,7 +365,7 @@ pub fn get_gloas_find_head_vote_transition_test_definition() -> ForkChoiceTestDe expected_payload_status: None, }); - // CL attestation back to Full branch (root 3) → head returns to 3. + // CL attestation back to Full branch (root 3) returns the head to 3. ops.push(Operation::ProcessAttestation { validator_index: 0, block_root: get_root(3), @@ -610,7 +610,7 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef block_root: get_root(1), }); - // Step 4: Set tiebreaker to Empty on genesis → Empty branch wins. + // Step 4: Set tiebreaker to Empty on genesis so the Empty branch wins. ops.push(Operation::SetPayloadTiebreak { block_root: get_root(0), is_timely: false, @@ -632,7 +632,7 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef proposer_boost_root: None, }); - // Step 5: Flip tiebreaker to Full → Full branch wins. + // Step 5: Flip tiebreaker to Full so the Full branch wins. ops.push(Operation::SetPayloadTiebreak { block_root: get_root(0), is_timely: true, @@ -654,7 +654,7 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef proposer_boost_root: None, }); - // Step 6: Add extra CL weight to Empty branch → overrides Full tiebreaker. + // Step 6: Add extra CL weight to the Empty branch; this overrides the Full tiebreaker. ops.push(Operation::ProcessAttestation { validator_index: 2, block_root: get_root(4), @@ -687,7 +687,7 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef /// - Block 1 (slot 1) extends genesis, Full chain /// - Block 2 (slot 1) extends genesis, Empty chain /// - Before payload arrives: payload_received is false for block 1 -/// - Process execution payload for block 1 → payload_received becomes true +/// - Process execution payload for block 1 so payload_received becomes true /// - Payload attestations arrive voting block 1's payload as timely + available /// - Head should follow block 1 because the PTC votes now count (payload_received = true) pub fn get_gloas_payload_received_interleaving_test_definition() -> ForkChoiceTestDefinition { @@ -744,8 +744,8 @@ pub fn get_gloas_payload_received_interleaving_test_definition() -> ForkChoiceTe attestation_slot: Slot::new(1), }); - // Equal weight, payload_received=true on genesis → tiebreaker uses - // payload_received (not previous slot, equal payload weights) → prefers Full. + // Weights are equal and payload_received=true on genesis, so the tiebreaker uses + // payload_received (not previous slot, equal payload weights) and prefers Full. // Block 1 (Full) wins because it matches the Full preference. ops.push(Operation::FindHead { justified_checkpoint: get_checkpoint(0), @@ -775,7 +775,7 @@ pub fn get_gloas_payload_received_interleaving_test_definition() -> ForkChoiceTe is_data_available: true, }); - // Still prefers Full via payload_received tiebreaker → Block 1 (Full) wins. + // Still prefers Full via payload_received tiebreaker. Block 1 (Full) wins. ops.push(Operation::FindHead { justified_checkpoint: get_checkpoint(0), finalized_checkpoint: get_checkpoint(0), @@ -816,7 +816,7 @@ pub fn get_gloas_previous_slot_tiebreaker_test_definition() -> ForkChoiceTestDef block_root: get_root(1), }); - // Block 2 at slot 2 with a mismatched EL parent hash → parent payload status Empty. + // Block 2 at slot 2 with a mismatched EL parent hash, giving it an Empty parent payload status. ops.push(Operation::ProcessBlock { slot: Slot::new(2), root: get_root(2), @@ -835,7 +835,17 @@ pub fn get_gloas_previous_slot_tiebreaker_test_definition() -> ForkChoiceTestDef payload_present: true, }); - // Before zero-out (current_slot == block 1's slot), raw weights decide → Full. + // Materialize the attestation into `full_payload_weight`. + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1], + expected_head: get_root(1), + current_slot: Slot::new(1), + expected_payload_status: Some(PayloadStatus::Full), + }); + + // Before zero-out (current_slot == block 1's slot), raw weights decide payload status (Full) ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(1), expected_status: PayloadStatus::Full, @@ -914,7 +924,7 @@ pub fn get_gloas_proposer_boost_flips_ancestor_test_definition() -> ForkChoiceTe expected_payload_status: Some(PayloadStatus::Empty), }); - // Without boost: raw weights decide → Empty. + // Without boost the raw weights decide and Empty wins. ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(1), expected_status: PayloadStatus::Empty, @@ -922,7 +932,7 @@ pub fn get_gloas_proposer_boost_flips_ancestor_test_definition() -> ForkChoiceTe proposer_boost_root: None, }); - // With boost on block 2: boost supports block 1's Full variant → Full. + // With boost on block 2 the boost supports block 1's Full variant, so Full wins. ops.push(Operation::AssertPayloadStatusByWeight { block_root: get_root(1), expected_status: PayloadStatus::Full, @@ -967,7 +977,7 @@ mod tests { let mut ops = vec![]; // Block at slot 31 — last pre-Gloas slot. Created as a V17 node because - // gloas_fork_epoch = 1 → Gloas starts at slot 32. + // gloas_fork_epoch = 1 means Gloas starts at slot 32. // // The test harness sets execution_status = Optimistic(ExecutionBlockHash::from_root(root)), // so this V17 node's EL block hash = ExecutionBlockHash::from_root(get_root(1)). From 863c2a704569b6495e4c06fef70bf0c100b71e10 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Fri, 17 Apr 2026 18:31:47 +0900 Subject: [PATCH 15/15] remove unneeded logs --- .../beacon_chain/src/payload_envelope_streamer/mod.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs index c33579da20e..5b1bda5dd57 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs @@ -133,11 +133,6 @@ impl PayloadEnvelopeStreamer { } } Err(e) => { - warn!( - error = ?e, - block_root = ?root, - "error verifying canonicity of payload envelope" - ); results.push((*root, Err(e))); } }