Parent envelope unknown lookup#9039
Open
eserilev wants to merge 29 commits intosigp:unstablefrom
Open
Conversation
…cessing and lookup the parent envelope
eserilev
changed the title
Closed
eserilev
commented
Apr 3, 2026
eserilev
commented
Apr 3, 2026
dapplion
added a commit
to dapplion/lighthouse
that referenced
this pull request
Apr 4, 2026
- Implement payload_lookup_request() (previously stubbed as NoRequestNeeded) - Add PayloadEnvelopesByRoot RPC request/response handling - Route envelope responses through router -> sync manager -> block lookups - Add envelope processing via verify_envelope_for_gossip + import - Add SinglePayloadEnvelope variant to SyncRequestId and BlockProcessType - Wire PayloadRequest state machine transitions (Downloaded -> Processing -> Done) Based on sigp#9039 by @eserilev, adapted for three-stream lookup model from gloas-lookup-sync-fixes.
dapplion
added a commit
to dapplion/lighthouse
that referenced
this pull request
Apr 4, 2026
- Implement payload_lookup_request() (previously stubbed as NoRequestNeeded) - Add PayloadEnvelopesByRoot RPC request/response handling - Route envelope responses through router -> sync manager -> block lookups - Add envelope processing via verify_envelope_for_gossip + import - Add SinglePayloadEnvelope variant to SyncRequestId and BlockProcessType - Wire PayloadRequest state machine transitions (Downloaded -> Processing -> Done) Based on sigp#9039 by @eserilev, adapted for three-stream lookup model from gloas-lookup-sync-fixes.
dapplion
added a commit
to dapplion/lighthouse
that referenced
this pull request
Apr 4, 2026
- Implement payload_lookup_request() (previously stubbed as NoRequestNeeded) - Add PayloadEnvelopesByRoot RPC request/response handling - Route envelope responses through router -> sync manager -> block lookups - Add envelope processing via verify_envelope_for_gossip + import - Add SinglePayloadEnvelope variant to SyncRequestId and BlockProcessType - Wire PayloadRequest state machine transitions (Downloaded -> Processing -> Done) Based on sigp#9039 by @eserilev, adapted for three-stream lookup model from gloas-lookup-sync-fixes.
michaelsproul
changed the title
Member
Author
|
I'd like to write tests for this before merging. I'll hold off marking this ready for review until the gloas beacon chain harness support lands |
eserilev
changed the title
eserilev
added
ready-for-review
|
This pull request has merge conflicts. Could you please resolve them @eserilev? 🙏 |
mergify
Bot
added
the
waiting-on-author
Closed
- block_verification: skip ParentEnvelopeUnknown check when parent is the proto-array anchor. The anchor's `payload_received` is intentionally false per spec (never added to `store.payloads`), but no envelope is expected for it; without this exception the check rejects every post-anchor gloas block. - network tests: disable `engineGetBlobs` in the TestRig harness. Under real crypto the mock EL's blob fetch raced the gossip path, importing via a spawned task that the test didn't await -- leaving `head_root()` unchanged when the assertion ran. The tests are designed to exercise the gossip + data-column path; the engine fetch was incidental. - network tests: relax `data_column_reconstruction_at_deadline` to allow trailing duplicate reconstruction work items. The reprocess queue removes its dedup entry on dispatch, so a column processed during an in-flight reconstruction can dispatch a second one. The second is a no-op via `reconstruction_started`, so accept >= 1 trailing event.
Replace `assert_event_journal_contains_at_least_ordered` helper with an inline drain that just counts the gossip + reconstruction events. The helper was carrying around `WORKER_FREED` bookkeeping and a strict prefix-match for one caller; counting the two relevant work types until both thresholds are met is the same check with much less code.
Wires the lookup test rig for Gloas:
- Capture per-block execution payload envelopes from the external harness
and serve them to peers via a new `network_envelopes_by_root` map.
- Handle `RequestType::PayloadEnvelopesByRoot` in `simulate_on_request` and
`Work::RpcPayloadEnvelope` in the simulator processor branch.
- Allow `TestRig` callers to override the genesis validator count and
bump initial balances to `max_effective_balance_electra` post-Electra,
which Gloas committee-selection requires for genesis init to converge.
Adds four tests for the parent-envelope-unknown flow (each verified
red/green by stubbing the corresponding source path):
- `creates_envelope_and_child_lookups` — `UnknownParentEnvelope` produces
exactly one envelope-only lookup for the parent root and one child
lookup awaiting that envelope.
- `idempotent_triggers` — repeated triggers for the same parent merge
into the existing envelope lookup; no duplicate lookups are created.
- `issues_payload_envelopes_by_root_rpc` — the envelope-only lookup
dispatches a `PayloadEnvelopesByRoot` RPC for the parent block_root.
- `drops_cascade_on_rpc_error` — when the envelope RPC errors, the
envelope lookup is dropped and the awaiting child cascades with it.
The end-to-end happy path (envelope arrives → child unblocks → block
imports → head advances) is gated on
`process_execution_payload_envelope` supporting `AvailabilityPending`,
which today returns `InternalError("Pending payload envelope not yet
implemented")`. That gap is independent of this PR's lookup machinery.
- bad_peer_wrong_envelope_response — peer responds with a different block_root than requested; request items raise UnrequestedBlockRoot, both lookups drop. - crypto_on_fail_with_bad_envelope_signature — signature corruption rejected in gossip verification, peer scored with lookup_envelope_processing_failure. Rename the four already-landed tests to match the existing happy_path / bad_peer / envelope_* / crypto_on_fail_with_* naming.
Implementation:
- payload_envelope_verification: implement the AvailabilityPending branch
in the envelope import flow. Previously returned
InternalError("Pending payload envelope not yet implemented") for any
envelope whose data columns hadn't yet been received, blocking the
end-to-end RPC import path. New `import_pending_execution_payload_envelope`
marks the payload as received in fork choice and persists the envelope to
the store; columns are still expected to arrive separately (gossip /
engineGetBlobs / reconstruction) and persist their own ops.
- sync manager: short-circuit `handle_unknown_parent_envelope` when the
parent's payload was received between gossip-verification and the trigger
reaching sync. No lookup is created; the trigger is treated as a no-op.
- gossip→sync hook: when a Gloas envelope is imported via the gossip path,
emit `SyncMessage::GossipEnvelopeImported { block_root }` so any lookups
awaiting that parent envelope unblock without depending on the in-flight
RPC response landing first. Closes the review-flagged race where a
gossip-imported envelope left child lookups pinned.
Tests (3 new):
- envelope_already_received_skips_lookup — trigger after envelope already
in fork choice creates zero lookups.
- happy_path_unknown_parent_envelope — end-to-end RPC import path: lookups
complete, head advances to the gossip block.
- happy_path_unknown_parent_envelope_via_gossip — pending envelope-only
lookup unblocked by a concurrent gossip import via the new sync hook.
Existing tests updated:
- bad_peer_envelope_rpc_failure / bad_peer_wrong_envelope_response now
expect the lookup to retry and succeed (mirroring `bad_peer_*` tests for
blocks/blobs/columns), reflecting the now-working import path.
dapplion
added
work-in-progress
…oas-parent-envelope-unknown-lookup
eserilev
commented
Apr 28, 2026
| /// store, but does not write data column ops. Columns are expected to arrive separately | ||
| /// (gossip, engineGetBlobs, or reconstruction). | ||
| #[instrument(skip_all)] | ||
| pub async fn import_pending_execution_payload_envelope( |
Member
Author
There was a problem hiding this comment.
this is a hack since we arent dealing with columns post-gloas at the moment. we should actually be sending this envelope to the da checker insteawd of just importing it immediately
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
#8994
When we attempt to import a block whose parent envelope is unknown push the block into the reprocessing queue and attempt to lookup the parent envelope