Skip to content

Gloas envelope deduplication (issue #8948, sub-task 1)#9574

Open
MavenRain wants to merge 1 commit into
sigp:unstablefrom
MavenRain:fix/gloas-envelope-dedup-status
Open

Gloas envelope deduplication (issue #8948, sub-task 1)#9574
MavenRain wants to merge 1 commit into
sigp:unstablefrom
MavenRain:fix/gloas-envelope-dedup-status

Conversation

@MavenRain

Copy link
Copy Markdown

What

Implements the envelope-deduplication portion of #8948: the Gloas
execution_payload gossip topic requires

[IGNORE] The node has not seen another valid
SignedExecutionPayloadEnvelope for this block root from this builder.

which was previously a TODO in gossip_verified_envelope.rs. This PR
adds the seen-envelope tracking and returns an IGNORE for duplicates.

How

  • New GossipVerifiedEnvelopeCache in
    payload_envelope_verification/gossip_verified_envelope_cache.rs,
    keyed by (block_root, builder_index) with Slot retained only for
    pruning. It is deliberately the seen-tracking half of the sibling
    GossipVerifiedPayloadBidCache (bids and envelopes are the two halves
    of the builder flow), and is non-generic because it only stores the
    dedup identity, not a verified message.
  • In GossipVerifiedEnvelope::new, mirror the bid path (check-first /
    insert-last): the seen_envelope check runs before the full-block
    load and signature verification (so duplicates stay cheap), and the
    insert_seen_envelope runs only after the envelope has fully passed
    verification. 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.
  • New non-penalizing EnvelopeError::EnvelopeAlreadySeen variant,
    classified penalize_peer() == false and mapped to
    MessageAcceptance::Ignore in the gossip processor.
  • Pruned per slot in BeaconChain::per_slot_task, right after the bid
    cache, using the same retain(slot >= current_slot) policy.

Tests

  • Unit tests on the cache (identity discrimination by block root and
    builder index; slot-based pruning).
  • End-to-end regression test duplicate_execution_payload_envelope_is_ignored
    (in tests/events.rs): a valid envelope passes gossip verification and
    fires the execution_payload_gossip event; a second identical envelope
    returns EnvelopeAlreadySeen and 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:

  1. Block-validity distinction (IGNORE vs REJECT). Splitting the
    current BlockRootUnknown into "block not seen yet" (IGNORE, keep the
    reprocess-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.

  2. 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.

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>
@MavenRain MavenRain requested a review from jxs as a code owner July 4, 2026 09:25
@cla-assistant

cla-assistant Bot commented Jul 4, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@cla-assistant

cla-assistant Bot commented Jul 4, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@eserilev

eserilev commented Jul 4, 2026

Copy link
Copy Markdown
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

@chong-he chong-he added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-on-author The reviewer has suggested changes and awaits thier implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants