Gloas attestation payload reprocess tests - #9490
Open
eserilev wants to merge 32 commits into
Open
Conversation
Scaffolding for the Gloas beacon_attestation `index == 1` (payload-present) rule: a
payload-present attestation for a block whose execution payload envelope hasn't been seen
should be IGNOREd-pending-payload and trigger an ExecutionPayloadEnvelopesByRoot fetch.
Adds (not yet wired; the gloas lookup-sync PR connects these):
- attestation_verification: Error::UnknownPayloadEnvelope { beacon_block_root }
- gossip handle_attestation_verification_failure: IGNORE arm that emits
SyncMessage::UnknownPayloadEnvelopeFromAttestation and queues the attestation for reprocess
- SyncMessage::UnknownPayloadEnvelopeFromAttestation (+ stub handler, TODO(gloas))
- reprocess queue: UnknownPayload{Unaggregate,Aggregate} + PayloadEnvelopeImported wake event +
awaiting_attestations_per_payload map (reuses QueuedUnaggregate/Aggregate/ReadyWork)
The ReadyAttestation expiry handler pruned only awaiting_attestations_per_root, leaking awaiting_attestations_per_payload entries on expiry (same class of leak as PR sigp#8065). Prune both maps on expiry. Add unit tests: prune_awaiting_attestations_per_payload and release_awaiting_attestations_on_payload_envelope_imported.
Make the path live (only the SyncMessage handler -> lookup is left for sigp#9155): - attestation_verification::verify_early_checks: a gloas index==1 attestation for a block whose payload envelope has not been received (!is_payload_received) returns UnknownPayloadEnvelope. - process_gossip_verified_execution_payload_envelope: on Imported, send ReprocessQueueMessage::PayloadEnvelopeImported to release attestations awaiting the payload.
Add the block slot to AvailabilityProcessingStatus::Imported so the import path knows the fork without re-deriving it. Extract send_reprocess_block/send_reprocess_envelope and route column imports through maybe_reprocess_after_column_import, which emits PayloadEnvelopeImported post-gloas (releasing attestations awaiting the payload) and BlockImported otherwise. Drop the now-unused parent_root from BlockImported.
Name the helpers after the event they announce rather than the channel: send_reprocess_block -> notify_block_imported, send_reprocess_envelope -> notify_payload_envelope_imported, matching the ReprocessQueueMessage variants.
Route the sync notification through the send_sync_message helper, matching the sibling UnknownBlockHashFromAttestation path instead of hand-rolling sync_tx.send + unwrap_or_else.
It always notifies; the fork only selects which event (block vs payload envelope), so drop the misleading maybe_ prefix.
…ess' into gloas-attestation-payload-reprocess
# Conflicts: # beacon_node/beacon_chain/src/beacon_chain.rs
…lease - Add UnknownPayloadEnvelope check to the aggregated attestation path so index==1 aggregates referencing an unseen payload envelope are parked and re-processed (the reprocess-queue plumbing already supported aggregates). - Release parked attestations when an envelope is imported via the lookup/RPC path, matching the gossip import path. - Reword the UnknownPayloadEnvelope doc comment per review.
- Factor the four near-identical UnknownBlock/UnknownPayload x aggregate/unaggregate parking arms into a single queue_awaiting_attestation helper, parameterised by QueuedAttestation and AwaitingComponent. - Add aggregate-path tests for the payload-envelope park->prune and park->release flows (UnknownPayloadAggregate), mirroring the existing unaggregate tests.
…free is_payload_received - Dedup UnknownPayloadEnvelopeFromAttestation debug logs via a dedicated notified_unknown_payload_roots LRU cache, mirroring notified_unknown_roots. - Add allow_payload_reprocess flag distinct from allow_reprocess so a payload-present (index==1) attestation parked first for an unknown block can still be parked once for an unknown payload envelope after the block arrives (was dropped before). - Carry payload_received on proto_array Block/ProtoBlock and read it from the ProtoBlock returned by verify_head_block_is_known, avoiding a second fork-choice read lock in the attestation verification path (both aggregate and unaggregate).
A payload-present (index==1) attestation submitted via the beacon API whose block's execution payload envelope has not been seen is now parked for re-processing (UnknownPayloadUnaggregate), mirroring the UnknownHeadBlock path, instead of being reported as a failure.
…tion-payload-reprocess-tests
eserilev
added
test improvement
…oas-attestation-payload-reprocess-tests # Conflicts: # beacon_node/beacon_processor/src/scheduler/work_reprocessing_queue.rs # beacon_node/network/src/network_beacon_processor/gossip_methods.rs # beacon_node/network/src/network_beacon_processor/sync_methods.rs # beacon_node/network/src/network_beacon_processor/tests.rs
…ithub.com/eserilev/lighthouse into gloas-attestation-payload-reprocess-tests
eserilev
added
ready-for-review
|
Some required checks have failed. Could you please take a look @eserilev? 🙏 |
mergify
Bot
added
waiting-on-author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue Addressed
This is a follow up to #9440. This just adds some additional test cases for the new payload reprocess code paths.