Ensure payload envelope streamer always serves canonical envelopes after the split slot by eserilev · Pull Request #9085 · sigp/lighthouse

eserilev · 2026-04-03T15:30:32Z

If an envelope is after the split slot check fork choice to for envelope payload weight to see if the envelope is canonical or not

Note that we still need envelope pruning to ensure we serve canonical envelopes before the split slot

…t are canonical according to fork choice

mergify · 2026-04-07T06:21:30Z

Some required checks have failed. Could you please take a look @eserilev? 🙏

…oas-envelope-streamer-serves-canonical

michaelsproul · 2026-04-07T08:35:29Z

+            if node.full_payload_weight >= node.empty_payload_weight {
+                return Some(PayloadStatus::Full);
+            }


Sorry I lied saying it was just a weight comparison 😅

I think this also needs to call get_payload_status_tiebreaker to choose between the full and empty nodes when their weights are equal (which is likely for the head block before it receives attestations). See:

https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/fork-choice.md#modified-get_head

I think we have all the info needed here to make that call (the IndexedForkChoiceNodes can be synthesised, and the block roots &

We don't need to compare roots because both empty and full children have the same block root.

We also don't need to do anything if payload_received is false (then the status is always just Empty).

I was hoping to get away with the head block case further upstream here (by just looking at the cached head). Because if we're not checking the head root then full >= empty is the only check we need i believe

but maybe this is a bit of a footgun, wdyt?

I think it's a bit sketchy because there could definitely be non-head blocks with equal full and empty weights. I've seen a bunch of places where it would probably be good to use this function, so a general purpose + correct impl is worth it IMO

michaelsproul · 2026-04-07T08:36:27Z

        justified_state_balances: vec![1],
+        expected_head: get_root(3),
+        current_slot: Slot::new(0),
+        expected_payload_status: None,


Not sure why this is None, but haven't looked super closely. I would expect all Gloas blocks have a status?

maybe this doesn't need to be Option since find_head always return a payload status for pre-gloas (empty)?

it doesn't need to be None but I think it'd be better to handle this in a separate PR. theres like 80+ call sites that need to be updated if we make this change

jimmygchen · 2026-04-09T05:46:57Z

+        let index = *self.proto_array.indices.get(block_root)?;
+        let node = self.proto_array.nodes.get(index)?;
+
+        let v29 = node.as_v29().ok()?;


I think accessing the fields with the functions be more forward compatible?

node.payload_received()?

ok refactored things a bit

jimmygchen · 2026-04-09T05:57:37Z

+
+        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::<E>(
+                &node_ref,
+                node,
+                current_slot,
+                proposer_boost_root,
+            )
+        };
+
+        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)
+        }


I'm not sure if the tie breaker logic is covered in spec tests, can we add some unit tests if they are not?

oh yeah just read the tests in gloas_payload.rs, i think we can test this scenario there?

added a test
cbe8b7a

jimmygchen · 2026-04-09T06:34:12Z

+        let full_tiebreaker = tiebreaker_for_status(PayloadStatus::Full).ok()?;
+        let empty_tiebreaker = tiebreaker_for_status(PayloadStatus::Empty).ok()?;
+
+        if full_tiebreaker >= empty_tiebreaker {


this can never be equal right?

yeah it can never be equal

jimmygchen · 2026-04-09T06:58:16Z

        justified_state_balances: vec![1],
+        expected_head: get_root(3),
+        current_slot: Slot::new(0),
+        expected_payload_status: None,


maybe this doesn't need to be Option since find_head always return a payload status for pre-gloas (empty)?

…oas-envelope-streamer-serves-canonical

eserilev · 2026-04-17T09:30:27Z

the fork choice function to verify payload envelope canonicity was incorrect. I've updated it and added tests that verify specific edge cases that the previous impl wasn't handling correctly

michaelsproul

Looks good. Didn't review the fork choice test changes in depth, but can after the merge conflicts are resolved.

michaelsproul

LGTM

michaelsproul · 2026-04-23T11:04:43Z

@mergify queue

stale

mergify · 2026-04-23T11:05:18Z

Merge Queue Status

✅ Entered queue — 2026-04-23 11:05 UTC · Rule: default
✅ Checks passed · on draft merge queue: embarking unstable (cfc7483) and #9085 together #9168
✅ Merged — 2026-04-23 11:32 UTC · at 7594ab8ce4fa27c31121f9f4e79e46804347c17b · squash

This pull request spent 27 minutes 12 seconds in the queue, including 25 minutes 43 seconds running CI.

Required conditions to merge

#approved-reviews-by >= 1 [🛡 GitHub branch protection]
- Ensure payload envelope streamer always serves canonical envelopes after the split slot #9085
#changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
- Ensure payload envelope streamer always serves canonical envelopes after the split slot #9085
check-success=local-testnet-success
check-success=test-suite-success

eserilev added gloas ready-for-review The code is ready for review labels Apr 3, 2026

eserilev changed the title ~~Ensure payload envelope streamer always serves canonical envelope after the split slot~~ Ensure payload envelope streamer always serves canonical envelopes after the split slot Apr 3, 2026

eserilev added blocked and removed ready-for-review The code is ready for review labels Apr 4, 2026

eserilev mentioned this pull request Apr 4, 2026

Gloas devnet planning #8680

Open

40 tasks

eserilev added ready-for-review The code is ready for review and removed blocked labels Apr 7, 2026

eserilev added 3 commits April 6, 2026 23:01

Introduce logic to ensure served payload enveloeps past the split slo…

58ac399

…t are canonical according to fork choice

Small cleanup

a0cd9f5

get payload status by weight plus tests

50f374c

eserilev force-pushed the gloas-envelope-streamer-serves-canonical branch from 63cce7a to 50f374c Compare April 7, 2026 06:03

mergify Bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Apr 7, 2026

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

46c6238

…oas-envelope-streamer-serves-canonical

michaelsproul mentioned this pull request Apr 7, 2026

Gloas set AttestationData.index #9100

Merged

michaelsproul reviewed Apr 7, 2026

View reviewed changes

Tiebreaker logic

34cb92a

jimmygchen previously requested changes Apr 9, 2026

View reviewed changes

eserilev added 6 commits April 13, 2026 04:31

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

ec4e452

…oas-envelope-streamer-serves-canonical

propogate errors

f6c288c

smol changes

40e2ed6

Update

88fbe3a

make forward compatible

94d5235

Add test for payload status

cbe8b7a

eserilev added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Apr 13, 2026

eserilev requested a review from jimmygchen April 16, 2026 12:13

jimmygchen reviewed Apr 17, 2026

View reviewed changes

Comment thread beacon_node/beacon_chain/src/payload_envelope_streamer/mod.rs Outdated

jimmygchen reviewed Apr 17, 2026

View reviewed changes

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

eserilev added 6 commits April 17, 2026 12:48

outdated comment

9b00020

make it a warn{

2d5f193

Merge branch 'unstable' into gloas-envelope-streamer-serves-canonical

632496a

fmt

19f7c01

Fix block_has_canonical_payload impl and add test cases

0a2014c

Clean up

1f25b95

eserilev requested a review from jimmygchen April 17, 2026 09:30

remove unneeded logs

863c2a7

eserilev mentioned this pull request Apr 20, 2026

Gloas spec v1.7.0-alpha.5 and beacon_chain tests #8998

Merged

2 tasks

michaelsproul approved these changes Apr 21, 2026

View reviewed changes

Fix test

7594ab8

michaelsproul approved these changes Apr 23, 2026

View reviewed changes

michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Apr 23, 2026

mergify Bot added the queued label Apr 23, 2026

mergify Bot added a commit that referenced this pull request Apr 23, 2026

Merge of #9085

06c7688

mergify Bot mentioned this pull request Apr 23, 2026

merge queue: embarking unstable (cfc7483) and #9085 together #9168

Closed

10 tasks

mergify Bot merged commit 82dc8b4 into sigp:unstable Apr 23, 2026
39 checks passed

mergify Bot removed the queued label Apr 23, 2026

Conversation

eserilev commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mergify Bot commented Apr 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eserilev Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

eserilev Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eserilev commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelsproul left a comment

Choose a reason for hiding this comment

Uh oh!

michaelsproul left a comment

Choose a reason for hiding this comment

Uh oh!

michaelsproul commented Apr 23, 2026

Uh oh!

mergify Bot commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Queue Status

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eserilev commented Apr 3, 2026 •

edited

Loading

eserilev Apr 7, 2026 •

edited

Loading

eserilev Apr 13, 2026 •

edited

Loading

eserilev commented Apr 17, 2026 •

edited

Loading

mergify Bot commented Apr 23, 2026 •

edited

Loading