From 9ce88ea3c12c2256a3bfe2805621d35995a72926 Mon Sep 17 00:00:00 2001 From: hopinheimer Date: Mon, 16 Mar 2026 19:36:48 -0400 Subject: [PATCH] addressing comments: --- consensus/proto_array/src/error.rs | 1 - .../src/fork_choice_test_definition.rs | 5 +-- consensus/proto_array/src/proto_array.rs | 43 ++++++++++++++++--- .../src/proto_array_fork_choice.rs | 1 + 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/consensus/proto_array/src/error.rs b/consensus/proto_array/src/error.rs index d6bd7f2cbf..04e747f5f6 100644 --- a/consensus/proto_array/src/error.rs +++ b/consensus/proto_array/src/error.rs @@ -54,7 +54,6 @@ pub enum Error { }, InvalidEpochOffset(u64), Arith(ArithError), - GloasNotImplemented, InvalidNodeVariant { block_root: Hash256, }, diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index b36e9c2117..7f607c826f 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -460,10 +460,7 @@ impl ForkChoiceTestDefinition { current_slot, } => { let actual = fork_choice - .head_payload_status::( - &head_root, - current_slot, - ) + .head_payload_status::(&head_root, current_slot) .unwrap_or_else(|| { panic!( "AssertHeadPayloadStatus: head root not found at op index {}", diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 5a0f49e64d..09538f25eb 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -164,6 +164,22 @@ impl Default for ProposerBoost { } } +/// Accumulated score changes for a single proto-array node during a `find_head` pass. +/// +/// `delta` tracks the ordinary LMD-GHOST balance change applied to the concrete block node. +/// This is the same notion of weight that pre-GLOAS fork choice used. +/// +/// Under GLOAS we also need to track how votes contribute to the parent's virtual payload +/// branches: +/// +/// - `empty_delta` is the balance change attributable to votes that support the `Empty` payload +/// interpretation of the node +/// - `full_delta` is the balance change attributable to votes that support the `Full` payload +/// interpretation of the node +/// +/// Votes in `Pending` state only affect `delta`; they do not contribute to either payload bucket. +/// During score application these payload deltas are propagated independently up the tree so that +/// ancestors can compare children using payload-aware tie breaking. #[derive(Clone, PartialEq, Debug, Copy)] pub struct NodeDelta { pub delta: i64, @@ -172,8 +188,16 @@ pub struct NodeDelta { } impl NodeDelta { - /// Determine the payload bucket for a vote based on whether the vote's slot matches the - /// block's slot (Pending), or the vote's `payload_present` flag (Full/Empty). + /// Classify a vote into the payload bucket it contributes to for `block_slot`. + /// + /// Per the GLOAS model: + /// + /// - a same-slot vote is `Pending` + /// - a later vote with `payload_present = true` is `Full` + /// - a later vote with `payload_present = false` is `Empty` + /// + /// This classification is used only for payload-aware accounting; all votes still contribute to + /// the aggregate `delta`. pub fn payload_status( vote_slot: Slot, payload_present: bool, @@ -188,7 +212,9 @@ impl NodeDelta { } } - /// Add a balance to the appropriate payload status. + /// Add `balance` to the payload bucket selected by `status`. + /// + /// `Pending` votes do not affect payload buckets, so this becomes a no-op for that case. pub fn add_payload_delta( &mut self, status: PayloadStatus, @@ -206,7 +232,10 @@ impl NodeDelta { Ok(()) } - /// Create a delta that only affects the aggregate `delta` field. + /// Create a delta that only affects the aggregate block weight. + /// + /// This is useful for callers or tests that only care about ordinary LMD-GHOST weight changes + /// and do not need payload-aware accounting. pub fn from_delta(delta: i64) -> Self { Self { delta, @@ -215,7 +244,9 @@ impl NodeDelta { } } - /// Subtract a balance from the appropriate payload status. + /// Subtract `balance` from the payload bucket selected by `status`. + /// + /// `Pending` votes do not affect payload buckets, so this becomes a no-op for that case. pub fn sub_payload_delta( &mut self, status: PayloadStatus, @@ -1075,7 +1106,7 @@ impl ProtoArray { boost_node.parent() == Some(parent_index) && boost_node .parent_payload_status() - .map_or(false, |s| s != PayloadStatus::Full) + .is_ok_and(|s| s != PayloadStatus::Full) }); // These three variables are aliases to the three options that we may set the diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index b50db01561..ce634fbdbe 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -999,6 +999,7 @@ impl ProtoArrayForkChoice { /// - Previous slot (`slot + 1 == current_slot`): prefer Full only when timely and /// data available (per `should_extend_payload`). /// - Otherwise: prefer Full when payload has been received. + /// /// Returns `None` for V17 nodes. pub fn head_payload_status( &self,