Skip to content

Fix block-carried payload votes discarded on late envelopes - #10101

Open
NikhilSharmaWe wants to merge 6 commits into
sigp:unstablefrom
NikhilSharmaWe:fix/block-carried-payload-votes-reprocess
Open

NikhilSharmaWe wants to merge 6 commits into
sigp:unstablefrom
NikhilSharmaWe:fix/block-carried-payload-votes-reprocess

Conversation

@NikhilSharmaWe

@NikhilSharmaWe NikhilSharmaWe commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Description

Block import discarded Gloas index-1 (payload_present) attestations when fork choice returned PayloadNotReceived because the envelope had not arrived yet. Those votes were swallowed as InvalidAttestation and never retried; envelope import only flipped payload_received.

Park block-carried votes inside fork choice when AttestationFromBlock::True hits PayloadNotReceived, then apply (or slot-queue) them from on_payload_envelope_received. That is the same path production and the harness use, so the drain cannot be skipped. Gossip still errors on PayloadNotReceived and keeps using the existing reprocess queue / UnknownPayloadEnvelope path.

Prune parked entries when the attested block leaves the proto-array. Parked votes are not persisted across restart (same class as other short-lived FC queues today) and are not deduped.

Fixes #10088

Test plan

  • cargo check -p fork_choice -p beacon_chain
  • cargo nextest run -p beacon_chain --features spec-minimal --release gloas_block_carried_payload_votes_applied_after_envelope

Signed-off-by: Nikhil Sharma <nikhilsharma230303@gmail.com>
@NikhilSharmaWe
NikhilSharmaWe requested a review from jxs as a code owner September 21, 2026 18:35
/// Optional hook notified when a payload envelope has been imported.
///
/// Set once by the client alongside `block_attestation_awaiting_payload`.
pub payload_envelope_imported: OnceLock<PayloadEnvelopeImportedFn>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we don't do such hooks in lighthouse

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.

Dropping the hooks, parking these on a BeaconChain pending map instead and applying them when the envelope imports.

Signed-off-by: Nikhil Sharma <nikhilsharma230303@gmail.com>
@michaelsproul michaelsproul added ready-for-review The code is ready for review v9.0.0 Gloas mainnet release gloas hardening labels Sep 24, 2026
@dapplion

dapplion commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

🤖 automated (dapplion's agent): Suggestion: park the votes inside fork_choice, not on BeaconChain — prototype dapplion@c2e3294, +81/−13 in one file vs +283/−4 across six. Gossip already parks this condition keyed by block root; the block path is the only gap. Separately, the drain in import_execution_payload_envelope is untested: the harness calls on_payload_envelope_received directly, so deleting it keeps the tests green.

@NikhilSharmaWe

NikhilSharmaWe commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor Author

agreed on the coverage hole with process_envelope.

I'm going to move park/drain into fork_choice as in your prototype: park QueuedAttestation on AttestationFromBlock::True + PayloadNotReceived, drain in on_payload_envelope_received, prune with contains_block. It will be a cleaner approach.

Will remove the BeaconChain pending map

@NikhilSharmaWe

NikhilSharmaWe commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor Author

@michaelsproul would like to know your thoughts on this, too.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gloas hardening ready-for-review The code is ready for review v9.0.0 Gloas mainnet release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants