Gloas Forkchoice by hopinheimer · Pull Request #8906 · sigp/lighthouse

hopinheimer · 2026-02-26T09:44:34Z

Issue Addressed

This PR implements fork choice change for Gloas. note that the forkchoice tests for post-Gloas are ignored for the testing harness lacks support for new block production flow.

…roto

michaelsproul · 2026-03-04T23:12:27Z

                    justified_checkpoint,
                    finalized_checkpoint,
+                    execution_payload_parent_hash,
+                    execution_payload_block_hash,


The payload should maybe have its own variant in the Operation enum so we can test interleaving of blocks, payloads, and other events, e.g.

Process block 1

Process attestations

Process block 2 building on 1

Process payload for block 1

etc

eserilev

I've reviewed proto_array.rs. Looks pretty good so far. Will continue reviewing the rest of this PR

eserilev · 2026-03-04T03:58:35Z

+pub struct NodeDelta {
+    pub delta: i64,
+    pub empty_delta: i64,
+    pub full_delta: i64,
+    pub payload_tiebreaker: Option<PayloadTiebreak>,


can we add doc comments about each field

eserilev · 2026-03-10T05:41:46Z

+            } else {
+                PayloadStatus::Full
+            };


I can find two conditions where this could occur

starting a network w/ gloas at genesis.

the parent was pruned due to finalization

for (1) not sure how we're supposed to handle gloas at genesis. for (2) this situation could only happen if we've called on_block for a block that violates finality. Should never make it into this code path in that case.

So I guess we just need to understand what a gloas genesis block should do, either build on empty or full. Full probably makes sense? might be worth a comment here

eserilev · 2026-03-10T05:43:56Z

-        // `self`.
-        if let Some(parent_index) = node.parent {
+        // If the parent has an invalid execution status, return an error before adding the
+        // block to `self`. This applies when the parent is a V17 node with execution tracking.


Suggested change

// block to `self`. This applies when the parent is a V17 node with execution tracking.

// block to `self`. This applies only when the parent is a V17 node with execution tracking.

eserilev · 2026-03-10T06:21:16Z

+            let node_deltas = deltas
+                .get(node_index)
+                .copied()
+                .ok_or(Error::InvalidNodeDelta(node_index))?;

            let mut node_delta = if execution_status_is_invalid {


Nit: we have a node_deltas and a node_delta variable which is a bit confusing from a readability standpoint.

I think node_deltas should be renamed to node_delta (to match the struct name)

and node_delta should be renamed to delta (to match the field name its referencing)

eserilev · 2026-03-10T06:41:37Z

+        if let Ok(execution_status) = justified_node.execution_status()
+            && execution_status.is_invalid()
+        {


Nit: this would help make it clearer that this check is only relevant for v17 nodes

Suggested change

if let Ok(execution_status) = justified_node.execution_status()

&& execution_status.is_invalid()

{

if let Ok(node) = justified_node.as_v17()

&& node.execution_status.is_invalid()

{

eserilev · 2026-03-10T07:01:12Z

+impl PartialEq<i64> for NodeDelta {
+    fn eq(&self, other: &i64) -> bool {
+        self.delta == *other
+            && self.empty_delta == 0
+            && self.full_delta == 0
+            && self.payload_tiebreaker.is_none()
+    }
+}


is this dead code? are we using this anywhere?

eserilev · 2026-03-10T07:52:49Z

+                    } else {
+                        // Equal weights: for V29 parents, prefer the child whose
+                        // parent_payload_status matches the parent's payload preference.
+                        let child_matches = child_matches_parent_payload_preference(parent, child);
+                        let best_child_matches =
+                            child_matches_parent_payload_preference(parent, best_child);
+
+                        if child_matches && !best_child_matches {
                            change_to_child
-                        } else {
+                        } else if !child_matches && best_child_matches {
                            no_change
-                        }
-                    } else {
-                        // Choose the winner by weight.
-                        if child.weight > best_child.weight {
+                        } else if *child.root() >= *best_child.root() {
+                            // Final tie-breaker of equal weights by root.


I think we should add comments under each if statement to clarify what each condition means.

also should this code path have additional gloas related tie breaker logic from should_extend_payload?

eserilev · 2026-03-10T07:57:50Z

+/// When equal, the tiebreaker uses the parent's `payload_tiebreak`: prefer Full if the block
+/// was timely and data is available; otherwise prefer Empty.


isn't the full tie breaker this? :

(is_payload_timely(store, root) and is_payload_data_available(store, root)) or proposer_root == Root() or store.blocks[proposer_root].parent_root != root or is_parent_node_full(store, store.blocks[proposer_root])

eserilev

Made some more progress here. I'm about halfway through.

eserilev · 2026-03-11T05:09:35Z


+/// Used for queuing payload attestations (PTC votes) from the current slot.
+/// Payload attestations have different dequeue timing than regular attestations:
+/// non-block payload attestations need an extra slot of delay (slot + 1 < current_slot).


Suggested change

/// non-block payload attestations need an extra slot of delay (slot + 1 < current_slot).

/// Gossiped payload attestations need an extra slot of delay (slot + 1 < current_slot).

eserilev · 2026-03-11T05:18:51Z

+            } else if let Ok(signed_bid) =
+                anchor_block.message().body().signed_execution_payload_bid()
+            {
+                // Gloas: hashes come from the execution payload bid.


ExecutionStatus is always irrelevant post gloas right?

eserilev · 2026-03-11T05:36:15Z

        }

+        // Post-GLOAS: same-slot attestations with index != 0 indicate a payload-present vote.
+        // These must go through `on_payload_attestation`, not `on_attestation`.


I'm not sure what you mean by these must go through on_payload_attestation. aren't these just invalid attestations? a PayloadAttestation seems to be a different concept

I think you can just reword the comment to say that same slot attestations with index !=0 are invalid

eserilev · 2026-03-11T05:38:25Z

+            && indexed_attestation.data().slot == block.slot
+            && indexed_attestation.data().index != 0
+        {
+            return Err(InvalidAttestation::PayloadAttestationDuringSameSlot { slot: block.slot });


this error variant is misleading as well, since these are not PayloadAttestations, these are just regular attestations

eserilev · 2026-03-11T05:39:59Z

+            })?;
+
+        if block.slot > indexed_payload_attestation.data.slot {
+            return Err(InvalidAttestation::AttestsToFutureBlock {


I wonder if instead of re-using InvalidAttestation error variants we should look at introducing InvalidPayloadAttestation error variants

eserilev · 2026-03-11T05:41:18Z

+            && indexed_payload_attestation.data.slot != self.fc_store.get_current_slot()
+        {
+            return Err(InvalidAttestation::PayloadAttestationNotCurrentSlot {
+                attestation_slot: indexed_payload_attestation.data.slot,


if we introduced a separate InvalidPayloadAttestation error enum, we won't have to mix payload attestation specific error variants with attestation specific error variants

eserilev · 2026-03-11T06:00:31Z

+            });
+        }
+
+        if self.fc_store.get_current_slot() == block.slot


should this also check is_from_block == true? I think we might be inadvertently rejecting valid gossip payload attestations?

eserilev · 2026-03-11T06:02:41Z

+
+        let processing_slot = self.fc_store.get_current_slot();
+        // Payload attestations from blocks can be applied in the next slot (S+1 for data.slot=S),
+        // while non-block payload attestations are delayed one extra slot.


Suggested change

// while non-block payload attestations are delayed one extra slot.

// while gossiped payload attestations are delayed one extra slot.

…oas-fc-proto

…to gloas-fc-proto

dapplion · 2026-03-16T05:41:25Z

    },
    InvalidEpochOffset(u64),
    Arith(ArithError),
+    GloasNotImplemented,


Temp variant?

dapplion · 2026-03-16T05:43:37Z

-    variants(V17),
-    variant_attributes(derive(Clone, PartialEq, Debug, Encode, Decode, Serialize, Deserialize)),
-    no_enum
+    variants(V17, V29),


Do we really need to keep both versions implemented? Why not migrate all V17 nodes to V29 nodes once and not superstruct this? Then the API of the ProtoNode remains unchanged

The V17 variants will be gone once the Gloas fork epoch is finalized anyway. So they can be removed in a future schema migration.

dapplion · 2026-03-16T05:44:06Z

 }

+#[derive(Clone, PartialEq, Debug, Copy)]
+pub struct NodeDelta {


Also docs for the struct itself

…to gloas-fc-proto

Co-authored-by: Co-author hopinheimer <knmanas7@gmail.com> Co-authored-by: Co-author brech1 <11075677+brech1@users.noreply.github.com>

eserilev · 2026-03-21T10:58:52Z

+    pub fn head_payload_status<E: EthSpec>(
+        &self,
+        head_root: &Hash256,
+        _current_slot: Slot,
+    ) -> Option<PayloadStatus> {
+        let node = self.get_proto_node(head_root)?;
+        let v29 = node.as_v29().ok()?;
+        if v29.full_payload_weight > v29.empty_payload_weight {
+            Some(PayloadStatus::Full)
+        } else if v29.empty_payload_weight > v29.full_payload_weight {
+            Some(PayloadStatus::Empty)
+        } else if is_payload_timely(
+            &v29.payload_timeliness_votes,
+            E::ptc_size(),
+            v29.payload_received,
+        ) && is_payload_data_available(
+            &v29.payload_data_availability_votes,
+            E::ptc_size(),
+            v29.payload_received,
+        ) {
+            Some(PayloadStatus::Full)
+        } else {
+            Some(PayloadStatus::Empty)
+        }


I think were missing the payload tiebreaker logic here when node.slot + 1 == current_slot. I added this locally and got the on_execution_payload ef test to pass

oh it seems like this function is only being used in tests

…ef-tests

michaelsproul · 2026-03-24T03:38:53Z

+    #[superstruct(only(V29), partial_getter(copy))]
+    pub full_payload_weight: u64,
+    #[superstruct(only(V29), partial_getter(copy))]
+    pub execution_payload_block_hash: ExecutionBlockHash,


We should add a test that this equals the expected value from EthSpec.

eserilev · 2026-03-24T04:12:29Z

+                    let mut bv = BitVector::new();
+                    for i in 0..bv.len() {
+                        let _ = bv.set(i, true);
+                    }
+                    bv


nit: looks like we generate a full bit vector in the genesis case for both payload_timeliness_votes and payload_data_availability_votes. Would be nice to extract this out into separate helper method. Maybe even within the BitVector impl inside the ssz crate (e.g.BitVector::full())

eserilev · 2026-03-24T04:30:45Z

+        // Check if the parent is "weak" (low attestation weight).
+        // Parent weight currently includes the back-propagated boost, so subtract it.
+        let reorg_threshold = calculate_committee_fraction::<E>(
+            justified_balances,
+            spec.reorg_head_weight_threshold.unwrap_or(20),
+        )
+        .unwrap_or(0);
+
+        let parent_weight_without_boost = parent.weight().saturating_sub(proposer_score);
+        if parent_weight_without_boost >= reorg_threshold {
+            return Ok(true); // Parent is not weak — apply.
+        }


I believe this isn't consistent with the spec. is_head_weak counts attestation weight from equivocating validators. parent.weight() reflects post-equivocation and post-delta weight calculations.

I think this makes us consider the parent weaker than the spec intended, which can cause us to skip proposer boosts in certain edge cases. Don't think we need to fix it in this PR but worth a TODO

hopinheimer · 2026-04-03T01:36:28Z

closing this for now in favor of #9025

hopinheimer and others added 6 commits February 19, 2026 23:07

proto node versioning

491b69f

adding michael commits

3e3ccba

implement scoring mechanisms and plumbing

d5c5077

adding tests and payload changes

e04a8c3

Merge branch 'unstable' of github.com:sigp/lighthouse into gloas-fc-p…

6d74723

…roto

fixing test

eb1b810

hopinheimer added consensus An issue/PR that touches consensus code, such as state_processing or block verification. gloas fork-choice labels Feb 26, 2026

hopinheimer added 4 commits February 26, 2026 04:46

lint

59033a5

vote sanity and genesis epoch fix

e68cc03

lint

6f6da5b

test fixes

275ac11

michaelsproul reviewed Mar 4, 2026

View reviewed changes

Comment thread consensus/proto_array/src/ssz_container.rs Outdated

michaelsproul reviewed Mar 4, 2026

View reviewed changes

Comment thread beacon_node/beacon_chain/src/schema_change/migration_schema_v28.rs

michaelsproul reviewed Mar 4, 2026

View reviewed changes

Comment thread consensus/proto_array/src/proto_array.rs

hopinheimer mentioned this pull request Mar 5, 2026

Gloas devnet planning #8680

Open

40 tasks

hopinheimer and others added 2 commits March 9, 2026 19:06

fix migration SszContainer scripts

9c6f25c

Merge branch 'unstable' into gloas-fc-proto

ca1b3eb

eserilev reviewed Mar 10, 2026

View reviewed changes

eserilev reviewed Mar 11, 2026

View reviewed changes

eserilev and others added 3 commits March 13, 2026 04:52

Merge branch 'unstable' of https://github.com/sigp/lighthouse into gl…

6fbb931

…oas-fc-proto

addressing comments

5679994

Merge branch 'gloas-fc-proto' of github.com:hopinheimer/lighthouse in…

d89e7f7

…to gloas-fc-proto

brech1 mentioned this pull request Mar 14, 2026

feat: on_execution_payload fork choice test support #8986

Closed

dapplion reviewed Mar 16, 2026

View reviewed changes

Merge branch 'unstable' into gloas-fc-proto

cf9e463

dapplion reviewed Mar 16, 2026

View reviewed changes

hopinheimer and others added 11 commits March 16, 2026 02:30

bitfield for PTC votes

f747696

Merge branch 'gloas-fc-proto' of github.com:hopinheimer/lighthouse in…

97d1b7b

…to gloas-fc-proto

completing should_extend_payload implementation

0df749f

changes

916d9fb

addressing comments:

9ce88ea

enable ef tests @brech1 commit

a7bcf0f

Co-authored-by: Co-author hopinheimer <knmanas7@gmail.com> Co-authored-by: Co-author brech1 <11075677+brech1@users.noreply.github.com>

enable ef tests @brech1 commit

ffec1a1

unstable merge

5aa1192

changes

46a909b

Propagate weight to parent's full/empty variants

ab1305d

fixing recursive calls with caching

cc8466d

eserilev reviewed Mar 21, 2026

View reviewed changes

hopinheimer added 2 commits March 23, 2026 14:40

Merge branch 'unstable' of github.com:sigp/lighthouse into fix-gloas-…

cb35ba6

…ef-tests

passing ef tests ft. @dapplion

ce71471

michaelsproul reviewed Mar 24, 2026

View reviewed changes

eserilev reviewed Mar 24, 2026

View reviewed changes

hopinheimer closed this Apr 3, 2026

	// block to `self`. This applies when the parent is a V17 node with execution tracking.
	// block to `self`. This applies only when the parent is a V17 node with execution tracking.

		/// When equal, the tiebreaker uses the parent's `payload_tiebreak`: prefer Full if the block
		/// was timely and data is available; otherwise prefer Empty.

	/// non-block payload attestations need an extra slot of delay (slot + 1 < current_slot).
	/// Gossiped payload attestations need an extra slot of delay (slot + 1 < current_slot).

	// while non-block payload attestations are delayed one extra slot.
	// while gossiped payload attestations are delayed one extra slot.

Conversation

hopinheimer commented Feb 26, 2026

Issue Addressed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eserilev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eserilev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hopinheimer commented Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants