Skip to content

broadcast PTC votes early on payload availability - #9434

Merged
mergify[bot] merged 35 commits into
sigp:unstablefrom
AbolareRoheemah:broadcast-ptc-votes-once-payload-seen
Sep 10, 2026
Merged

mergify[bot] merged 35 commits into
sigp:unstablefrom
AbolareRoheemah:broadcast-ptc-votes-once-payload-seen

Conversation

@AbolareRoheemah

@AbolareRoheemah AbolareRoheemah commented Jun 8, 2026 •

Copy link
Copy Markdown
Contributor
  • Updated PayloadAttestationService to broadcast PTC votes as soon as the execution payload and data columns are available (via SSE execution_payload_available event).
  • Retained the existing deadline as fallback.
  • Follows the same early-trigger pattern used by AttestationService.

Addresses #9422 and #9584

@eserilev eserilev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good progress so far

Comment thread validator_client/beacon_node_fallback/src/beacon_head_monitor.rs
Comment on lines 181 to 187

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must guarantee that the BN that emitted the payload available event is the one performing the PTC duty. See the attestation service for an example, HeadEvent carries a beacon node index that is fed into the attestation service to ensure the node is requesting only the relevant BN

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted 👍

/// Returns the index of some validator in `self.shuffling`.
///
/// Always returns `None` for a non-initialized epoch.
#[allow(clippy::arithmetic_side_effects)]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually unrelated to this PR, but I added it because I was getting an error when I ran cargo clippy --all(before any of my changes).

Happy to remove it if you prefer.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can fix by changing p.get() - 1 to p.get().saturating_sub(1)

}

sleep(duration_to_next_slot + payload_attestation_due).await;
let _early_payload_event = if self.payload_available_rx.is_some() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we dont want the poller to run pre-gloas

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a check for this

@eserilev eserilev added gloas val-client Relates to the validator client binary waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Jun 10, 2026
@AbolareRoheemah

Copy link
Copy Markdown
Contributor Author

Hi @eserilev. I pushed some updates.

@chong-he

Copy link
Copy Markdown
Member

Some merge conflicts if you have time to fix it @AbolareRoheemah

@AbolareRoheemah
AbolareRoheemah requested a review from jxs as a code owner June 14, 2026 00:01
@AbolareRoheemah

Copy link
Copy Markdown
Contributor Author

Some merge conflicts if you have time to fix it @AbolareRoheemah

Hi @chong-he. Trust you're doing great. I resolved the conflicts and added some tests.

@eserilev

Copy link
Copy Markdown
Member

can you make sure to run
cargo fmt --all
+
make lint-full

and fix any linting issues

@AbolareRoheemah

Copy link
Copy Markdown
Contributor Author

Done @eserilev 🫡

@chong-he

Copy link
Copy Markdown
Member

Done @eserilev 🫡

cli check is failing. You would need to do make cli-local to update the cli help text in .md instead of manually editing it.

@AbolareRoheemah

Copy link
Copy Markdown
Contributor Author

Thanks @chong-he. I have now ran make cli-local 🙏

@eserilev

Copy link
Copy Markdown
Member

hi @AbolareRoheemah ive reverted a few changes that were not required. You might need to cargo clean and rustup update before running lint checks to ensure your local dev environment matches what our CI expects

I've also came across an edge case that we need to handle. Right now if ptc publishing fails we still update last_slot = attestation_slot. In the case where a payload available SSE event triggers the ptc service and fails, we still want to retry ptc publishing at the deadline.

Resolving this edge case nicely requires a refactor to the payload attestation service. I think i'm going to push up that refactor as a separate PR. The plan would be to merge that refactor first and then continue working on your changes here

@AbolareRoheemah

AbolareRoheemah commented Jun 17, 2026 •

Copy link
Copy Markdown
Contributor Author

hi @AbolareRoheemah ive reverted a few changes that were not required. You might need to cargo clean and rustup update before running lint checks to ensure your local dev environment matches what our CI expects

Thanks @eserilev. I added those changes to fix the lint errors I was getting. But now I'll try your recommendation and re-check linting before pushing again. 🤝

I've also came across an edge case that we need to handle. Right now if ptc publishing fails we still update last_slot = attestation_slot. In the case where a payload available SSE event triggers the ptc service and fails, we still want to retry ptc publishing at the deadline.
Resolving this edge case nicely requires a refactor to the payload attestation service. I think i'm going to push up that refactor as a separate PR. The plan would be to merge that refactor first and then continue working on your changes here

That makes a lot of sense. Happy work on the refactor too if you don't mind.

@eserilev

Copy link
Copy Markdown
Member

#9492 the refactor is here. the PR description gives some reasoning about why I think we need this. Wanted to do it in a separate PR to keep the diff minimal here. Lmk what you think

@AbolareRoheemah

Copy link
Copy Markdown
Contributor Author

#9492 the refactor is here. the PR description gives some reasoning about why I think we need this. Wanted to do it in a separate PR to keep the diff minimal here. Lmk what you think

Thanks for the refactor, @eserilev! I took a look at the changes. It will be very helpful in making the early broadcast logic cleaner. I will base my PR on top of it once merged.

Also let me know if you'd like me to test anything specific on top of this refactor.

@mergify

mergify Bot commented Jul 7, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@eserilev

eserilev commented Jul 7, 2026

Copy link
Copy Markdown
Member

@chong-he could I get another re-review from you when you get the chance? I made some tweaks and would love another set of eyes on it if possible.

@chong-he chong-he 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 Jul 10, 2026

@chong-he chong-he left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only some minor comments below.

On a side note, I recall there is a fallback test that we can assert the get endpoint beacon node to be called 2 times: #9434 (comment) but the test seems to have been removed?

Comment on lines +171 to +173
let mut data_result = self.produce_payload_attestation_data(trigger).await;

if triggered_early && !matches!(data_result, Ok(Some(_))) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When there is no duty, produce_payload_attestation_data returns Ok(None), so this loop resolves to true and will be executed. This will cause some unnecessary sleep in the loop. Maybe we can change this to only execute when data_result is Err(_)?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mergify

mergify Bot commented Jul 16, 2026

Copy link
Copy Markdown

This pull request has merge conflicts. Could you please resolve them @AbolareRoheemah? 🙏

@mergify 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 Jul 16, 2026
@chong-he

Copy link
Copy Markdown
Member

Some merge conflicts @AbolareRoheemah , also some comments above if you have time to look at it. Thanks

@eserilev

Copy link
Copy Markdown
Member

Thanks for all your work @AbolareRoheemah, sorry it took so long to get merged

@eserilev eserilev added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Sep 10, 2026
@mergify mergify Bot added the queued label Sep 10, 2026
@mergify

mergify Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Merge Queue Status

  • ✅ Entered queue — 2026-09-10 18:20 UTC · Rule: default · triggered by rule Add ready-to-merge labeled PRs to merge queue
  • ✅ Checks passed · on draft merge queue: checking #9434 on unstable (085034f) #10035
  • ✅ Merged — 2026-09-10 18:55 UTC · at 8760ed238a350689ac45c40c8681ce3f97454e8c · squash

This pull request spent 34 minutes 23 seconds in the queue, including 31 minutes 8 seconds running CI.

Required conditions to merge

@mergify
mergify Bot merged commit 8760ed2 into sigp:unstable Sep 10, 2026
39 checks passed
@mergify mergify Bot removed the queued label Sep 10, 2026
frisitano added a commit to eth-act/lighthouse that referenced this pull request Sep 16, 2026
* Avoid walking already indexed validators in the monitor (sigp#9995)

## Issue Addressed

The [validator monitor](https://github.com/sigp/lighthouse/blob/115bd16fb565c3df2b169a740fa5754adde00762/beacon_node/beacon_chain/src/validator_monitor.rs#L484-L502) walks the already indexed registry during recent block imports, including with default monitoring and zero monitored validators. Milhouse's `iter().skip(index)` traverses the skipped prefix even when there are no new validators.

## Proposed Changes

Start the iterator at the first unknown index with `iter_from`, keeping absolute validator indices. A regression test covers empty registries, growth, shorter forks, late registration and pending updates for both fixed and progressive lists.

## Additional Info

A local mainnet A/B on 7 Sep 2026 used two Lighthouse followers with separate mock ELs, zero monitored validators and a registry of about 2.36 million validators. Each follower had a 3-CPU quota on the same AMD EPYC-Milan VM. The builds used baseline `1256bd99`, with only this loop changed in the treatment. Each window followed one epoch of settling, with binaries swapped between nodes for the second window.

| Median `import_block` duration | Baseline | Treatment | Matched blocks |
| --- | ---: | ---: | ---: |
| 19:29-19:43 UTC | 199.87 ms | 38.47 ms | 66 |
| 19:53-20:07 UTC, after swap | 191.60 ms | 40.56 ms | 66 |

One first-window trace was unmatched and excluded; baseline logs confirm that block was received and became head. All slow outliers are included.

All 132 matched imports improved. Mean paired import savings were 159.70 ms and 152.68 ms; the monitor child span accounted for 158.24 ms and 150.67 ms respectively.

This measures local import completion with simulated execution validity. The monitor runs under the fork-choice write lock, normally after early attestability. These results do not establish earlier attestations, whole-client CPU savings or the same absolute saving in production.

* Remove buggy parent_root calculations from Gloas block production (sigp#9997)

## Issue Addressed

Gloas block production was calculating the parent_root twice, once before advancing the parent state and once after. The calculation prior to the state advance **would yield the wrong block root** in the case where the parent state was not already advanced. In practice, this didn't occur very often because the state advance timer would make the advanced state available in the state cache.

https://github.com/sigp/lighthouse/blob/1256bd99e1b6c5d1290241849354b7e27dbc0ed9/beacon_node/beacon_chain/src/block_production/gloas.rs#L185-L191

This potentially incorrect block root was used to calculate `should_build_on_full` on the next line:

https://github.com/sigp/lighthouse/blob/1256bd99e1b6c5d1290241849354b7e27dbc0ed9/beacon_node/beacon_chain/src/block_production/gloas.rs#L193-L199

As a result, we would sometimes end up reading `should_build_on_full` for the **grandparent** block rather than the parent. Often, this wouldn't make any difference. The impact of this bug was further mitigated by the fact that we re-calculated the `parent_root` a 2nd time _after_ the state advance:

https://github.com/sigp/lighthouse/blob/1256bd99e1b6c5d1290241849354b7e27dbc0ed9/beacon_node/beacon_chain/src/block_production/gloas.rs#L378-L398

This `parent_root` would always be correct, so the block would always be valid, just possibly building on the wrong empty/full variant.

## Proposed Changes

Calculating the `parent_root` at all is conceptually unnecessary. It was already decided and known when we called `load_state_for_block_production`. This PR threads that value through and removes both re-calculations.

A regression test is added in `gloas_block_production_parent_root_with_unadvanced_state`. I've verified that it fails on unstable and passes with this patch.

## Additional Info

Codex was used, but as you can see from the commit history, there was a lot of tweaking to arrive at the simplest fix.

Some more refactors and cleanups are possible (e.g. removing ReOrgInputs), but I'll do that in a separate PR to avoid muddying this bugfix.

This bug is similar to another one recently found in block prod. I'm going to try to weed them all out:

- sigp#9983

* Ignore exits for all withdrawing validators in Gloas block prod (sigp#9983)

## Issue Addressed

Closes:

- sigp#9981

## Proposed Changes

Filter out exits for validators which _could_ be impacted by any withdrawal triggered in the parent execution payload. See linked issue for details of the bug.

This fix prevents production of invalid blocks (!!) in some scenarios post-Gloas.

## Additional Info

Fix & comments written manually, tests fixed by Codex. Regression test by Codex with manual review.

* Add attestations test with `payload_present` in op pool (sigp#9531)

Add a test in operation pool about the index for the cases with and without payload_present. 

Written with Claude Code and did a self review

Currently blocked waiting for ethereum/consensus-specs#5399 fix
Update: the fix ethereum/consensus-specs#5473 is merged and included in [v1.7.0-alpha.13](https://github.com/ethereum/consensus-specs/releases/tag/v1.7.0-alpha.13), currently pending for Lighthouse update to alpha 13

* Don't return Pending node when fork choice reverts to justified block (sigp#9962)

Closes sigp#9544.

## Description

- `find_head_walk` was filtering virtual EMPTY/FULL children of a PENDING node against `viable_nodes`. Under deep non-finality the justified seed can be non-viable, so those children were dropped and `get_head` returned Pending, which breaks block production.
- Skip the filter for PENDING heads. Viability filtering still applies to real block children from EMPTY/FULL nodes.
- Adds `pending_head_resolves_when_justified_subtree_non_viable` in `gloas_payload.rs`.

* Fix flaky unknown block test (sigp#10001)

## Issue Addressed

Fix flaky CI test:

- sigp#9999

## Proposed Changes

The test was failing due to a race between importing the block on gossip vs importing the block as a result of its `getBlobs` call completing. The `getBlobs` codepath was missing a notification to the reprocess queue, which this PR adds.

There's also a new test added which prevents regression on the `getBlobs` codepath.

## Additional Info

Heavily Codex driven, manually reviewed. Change makes sense to me.

* Gloas spec beta 0 (sigp#10014)

## Issue Addressed

Each commit has a link to the relevant consensus spec PR

* Convert produceBlockV4 to POST and round-trip Eth-Builder-Url (Gloas builder API 4/5) (sigp#9806)

Fourth PR of the Gloas builder API stack (beacon-APIs sigp#630):

- convert `/eth/v4/validator/blocks/{slot}` to POST with an optional
  `BuilderConfig` body (min_bid, builder_boost_factor, direct builders)
- add `POST /eth/v1/validator/builder_preferences` for forwarding signed
  builder preferences
- set `Eth-Builder-Url` on produceBlockV4 responses when a direct-builder bid
  wins, accept it on `POST /eth/v2/beacon/blocks`, and forward the signed
  block to that builder

The validator client still uses the legacy GET methods at this point; it
migrates in the final PR of this stack.

* `engine_getBlobsV4` (sigp#9438)

## Issue Addressed

Implement EIP-8070's engine API calls: `engine_getBlobsV4`.

## Additional Info

Claude was heavily used for prototyping. Gloas support will be added after sigp#9325 is merged.

* Add 0x02 support to validator creation (sigp#9702)

## Issue Addressed

NA

## Proposed Changes

Adds the `--compounding` flag to `lighthouse validator_manager create` to allow creation of validators with 0x02-prefixed withdrawal credentials.

## Additional Info

I had to move some test vectors from Holesky to Hoodi as `ethstaker-deposit-cli` has stopped supporting Holesky.


Co-authored-by: Tan Chee Keong <tanck@sigmaprime.io>

* Standardise FCR metrics (sigp#9987)

## Issue Addressed

Closes sigp#9663

## Proposed Changes

Adopt [beacon-metrics](https://github.com/ethereum/beacon-metrics/blob/master/metrics.md#fast-confirmation) 

| Standard metric | Was | Fires |
|---|---|---|
| `beacon_fast_confirmation_slot` | `beacon_fcr_confirmed_root_slot` | after each FCR run |
| `beacon_fast_confirmation_reorgs_total` | new | head reorgs off a confirmed block |
| `beacon_fast_confirmation_fallbacks_total` | `beacon_fcr_revert_to_finalized_total` | confirmed root reverts to finalized |
| `beacon_fast_confirmation_restarts_total` | `beacon_fcr_restart_from_justified_total` | restart from the observed justified checkpoint |

Added a metric to track head reorgs out of the previous confirmed root:
- `_confirmed_root_reorgs_total`

* Enable FCR test cases (sigp#9964)

## Issue Addressed

Enable remaining FCR test cases

## Proposed Changes

Fix fake_crypto to produce correct states if the input pub keys are valid

## Additional Info

* broadcast PTC votes early on payload availability (sigp#9434)

- Updated PayloadAttestationService to broadcast PTC votes as soon as the execution payload and data columns are available (via SSE execution_payload_available event).
- Retained the existing deadline as fallback.
- Follows the same early-trigger pattern used by AttestationService.

Addresses sigp#9422 and sigp#9584


Co-authored-by: Eitan Seri- Levi <eserilev@gmail.com>
Co-authored-by: Eitan Seri-Levi <eserilev@ucsc.edu>
Co-authored-by: Tan Chee Keong <tanck@sigmaprime.io>

* Avoid copying unchanged justified balances (sigp#10023)

## Issue Addressed

- `find_head` clones `JustifiedBalances` on every call, even when it hasn't changed.
- During block import, this copy happens while holding the fork choice write lock.

## Proposed Changes

- Use the existing `PartialEq` implementation to only clone `JustifiedBalances` when it changes.

## Additional Info

- An instrumented mainnet run with a mock EL found unchanged balances in **130 of 131 calls**, including all 33 block imports.
- Tested against the parent commit with two mainnet beacon nodes in Kurtosis, using mock ELs and equal CPU and memory limits. Both stayed synced with matching block roots and finalized checkpoints.
- Across **96 block imports per node**, average fork choice time during block import dropped from **31.58 ms to 15.00 ms**, about **53% lower**. This includes `get_head` and the early attester cache update. Both nodes ran on the same dev box.

* Bump `rustls` to fix cargo audit failure (sigp#10052)

## Issue Addressed

Cargo audit failure from `rustls`: https://rustsec.org/advisories/RUSTSEC-2026-0285.html

## Proposed Changes

Bump locked `rustls` version to `0.23.45`

* Revert "Move `SlotAssignment` cache to `CanonicalHead` (sigp#9661)" (sigp#10047)

## Issue Addressed

Per this comment: sigp#9831 (comment), it turns out we don't need the `SlotAssignments` cache for regular fork choice. Attempting to use it in the general case is difficult because we need to deal with the fact that cache may be stale, and that building it while holding a lock could be very slow. This PR reverts dbd2824 so that the cache is used ONLY for FCR.

## Proposed Changes

Reverted the commit dbd2824 and resolved conflicts in `beacon_node/beacon_chain/src/canonical_head.rs` mostly relating to the fork choice poisoning (unrelated changes on nearby lines).

## Additional Info

Codex used for conflict res, results checked manually.

* fork choice compliance tests (sigp#9710)

## Issue Addressed

Add consensus spec fork choice compliance tests

Some tweaks were made to get tests passing:
1. For attestation related tests: If attestations are for a future-slot and queued we treat them as import failed.
2. Updated test runners to recompute head before every block import
3. Blocks rejected by block verification instead of fork choice skip fork-choice assertions.
4. We don't re-import block attestations after `on_block`. Lighthouse block import already does this, but via the from-block path, which skips the `validate_target_epoch_against_current_time` validation step
5. We only run complaince tests for fake_crypto + fulu/gloas minimal

* Add base support for Heze block production (sigp#9714)

## Proposed Changes

- Enable Heze block production by constructing `BeaconBlockHeze` in `complete_partial_beacon_block_gloas` (the body is identical to the Gloas beacon block variant)
- This work is a pre-requisite for testing the upcoming FOCIL PRs
- Route Heze block building through the existing Gloas `getPayload` path, because the [engine API spec for Bogota](https://github.com/ethereum/execution-apis/blob/main/src/engine/bogota.md) defines no new `getPayload` version for this fork. Heze responses are deliberately deserialized into the Gloas containers (see [TODO added](https://github.com/sigp/lighthouse/pull/9714/changes#diff-8b79597c7e1a33167a01f676e50f09dbc40e761dad5b294086ebda40828e1641R1064-R1065)), so the Heze-specific engine types stay unused until the payloads diverge. Likewise, envelope submission still uses the Gloas `newPayloadV5` path
- Add test producing and importing blocks across the Heze boundary epoch

This PR unblocks Heze testing through the beacon chain harness. Previously, with block production stubbed, the harness could not build or advance a Heze chain. This is a blocker for any Heze integration tests, a prerequisite for the upcoming FOCIL PRs' tests.

---------

Co-authored-by: Jimmy Chen <jchen.tc@gmail.com>
Co-authored-by: Michael Sproul <michaelsproul@users.noreply.github.com>
Co-authored-by: chonghe <44791194+chong-he@users.noreply.github.com>
Co-authored-by: Nikhil Sharma <nikhilsharma230303@gmail.com>
Co-authored-by: Eitan Seri-Levi <eserilev@gmail.com>
Co-authored-by: ethDreamer <37123614+ethDreamer@users.noreply.github.com>
Co-authored-by: Daniel Knopik <107140945+dknopik@users.noreply.github.com>
Co-authored-by: Paul Hauner <paul@paulhauner.com>
Co-authored-by: Tan Chee Keong <tanck@sigmaprime.io>
Co-authored-by: Lion - dapplion <35266934+dapplion@users.noreply.github.com>
Co-authored-by: Roheemah <60899500+AbolareRoheemah@users.noreply.github.com>
Co-authored-by: Eitan Seri-Levi <eserilev@ucsc.edu>
Co-authored-by: Mac L <mjladson@pm.me>
Co-authored-by: Cristian Conache <C.conache@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gloas ready-for-merge This PR is ready to merge. val-client Relates to the validator client binary

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants