Gloas envelope deduplication (issue #8948, sub-task 1)#9574
Open
MavenRain wants to merge 1 commit into
Open
Conversation
Implement the Gloas `execution_payload` gossip IGNORE rule: forward at most one valid `SignedExecutionPayloadEnvelope` for a given `(block_root, builder_index)`. Adds a `GossipVerifiedEnvelopeCache` (the envelope counterpart of `GossipVerifiedPayloadBidCache`) that is checked before the state load in `GossipVerifiedEnvelope::new` and populated only once an envelope has passed full gossip verification, so an earlier invalid envelope from a builder never suppresses a later valid one. Duplicates return the new non-penalizing `EnvelopeError::EnvelopeAlreadySeen` variant, which maps to `MessageAcceptance::Ignore`. The cache is pruned each slot alongside the bid cache. Addresses the deduplication portion of sigp#8948. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
|
|
Member
|
Can you please rewrite the PR description using your own words. And make sure all code comments are also using your own words and not written by AI. Per our LLM usage guidelines all descriptions and comments should be written by you At the moment everything I've read so far is too verbose and obviously written by AI |
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
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.
What
Implements the envelope-deduplication portion of #8948: the Gloas
execution_payloadgossip topic requireswhich was previously a TODO in
gossip_verified_envelope.rs. This PRadds the seen-envelope tracking and returns an IGNORE for duplicates.
How
GossipVerifiedEnvelopeCacheinpayload_envelope_verification/gossip_verified_envelope_cache.rs,keyed by
(block_root, builder_index)withSlotretained only forpruning. It is deliberately the seen-tracking half of the sibling
GossipVerifiedPayloadBidCache(bids and envelopes are the two halvesof the builder flow), and is non-generic because it only stores the
dedup identity, not a verified message.
GossipVerifiedEnvelope::new, mirror the bid path (check-first /insert-last): the
seen_envelopecheck runs before the full-blockload and signature verification (so duplicates stay cheap), and the
insert_seen_enveloperuns only after the envelope has fully passedverification. This keeps the spec's "another valid envelope"
semantics: an invalid envelope is never recorded, so it cannot
suppress a later valid one from the same builder.
EnvelopeError::EnvelopeAlreadySeenvariant,classified
penalize_peer() == falseand mapped toMessageAcceptance::Ignorein the gossip processor.BeaconChain::per_slot_task, right after the bidcache, using the same
retain(slot >= current_slot)policy.Tests
builder index; slot-based pruning).
duplicate_execution_payload_envelope_is_ignored(in
tests/events.rs): a valid envelope passes gossip verification andfires the
execution_payload_gossipevent; a second identical envelopereturns
EnvelopeAlreadySeenand fires no further event.Scope / deliberate follow-ups
The issue also lists two "related items to consider" that I have left as
follow-ups, because both are open design decisions rather than mechanical
work:
Block-validity distinction (IGNORE vs REJECT). Splitting the
current
BlockRootUnknowninto "block not seen yet" (IGNORE, keep thereprocess-queue deferral) and "block seen but invalid" (REJECT) needs a
signal that fork choice alone cannot provide, since it only holds valid
blocks. The in-code TODO sketches a fork-choice
Processed(Bid, bool)status table, and the issue notes the choice between an
Observed*cache and a block-status table is still open. I did not want to
pre-empt that decision in this PR; I'd be happy to take it on in a follow-up
once the preferred mechanism is settled.
Pre-execution caching. Caching a pre-executed envelope so it can be
evicted/retried on a transient EL failure would need a retry driver
that does not exist today (transient failures are currently dropped
with a debug log, not re-enqueued). That is a larger change than the
dedup and orthogonal to it.
Keeping this PR to the deduplication rule makes it self-contained and
reviewable; I can follow up on either item.
AI disclosure: This change was developed with significant AI
assistance (code, tests, and this description drafted with an AI coding
assistant), reviewed and authored for submission by me, per the
CONTRIBUTING "A Note on LLM usage" policy.