Skip to content

feat: add validator-scoped QBFT Role::EnvelopeProposer (#1120) - #1149

Merged
mergify[bot] merged 3 commits into
sigp:epbsfrom
jnhsigmap:feat/1120-role-envelopeproposer
Jul 22, 2026
Merged

feat: add validator-scoped QBFT Role::EnvelopeProposer (#1120)#1149
mergify[bot] merged 3 commits into
sigp:epbsfrom
jnhsigmap:feat/1120-role-envelopeproposer

Conversation

@jnhsigmap

Copy link
Copy Markdown
Contributor

Problem, Evidence, and Context (Required)

SIP-94 introduces ePBS self-build envelope signing (§6) as a second, proposer-scoped QBFT duty that runs after the §4 block is published. Anchor has no Role for it, so envelope messages cannot be classified, fork-gated, or validated. This PR adds that role and wires the message-validation contract SIP-94 §7 pins for it.

Because Role is a plain enum (not #[non_exhaustive]), adding a variant breaks every exhaustive match across ssv_types, message_validator, and qbft_manager. This change intentionally bundles those compile-coupled edits so the workspace stays buildable in one step, and leaves QBFT instance routing as a transient reject for the follow-up (#1122).

Change Overview (Required)

Role::EnvelopeProposer (wire byte [9, 0, 0, 0], matching SIP-94 RoleEnvelopeProposer = 9) is a validator-scoped, QBFT, monotonic-slot role bound to PartialSignatureKind::PostConsensus. It reuses the existing post-consensus machinery rather than adding a new partial-signature kind; the runner role discriminates routing. Validation is gated on the Ethereum Gloas fork.

The production surface is small (~42 lines): the new variant plus the arms every compile-coupled match site needs. The bulk of the diff is tests (~950 lines) that pin each SIP-94 §7 rule against the real validation entry points.

Issue Criteria Addressed:

  • Wire round-tripRole::EnvelopeProposer encodes/decodes as [9, 0, 0, 0] via From<Role> / TryFrom<&[u8]>. (msgid.rs)
  • Classificationis_committee_role() == false, is_qbft_role() == true, max_round() == Some(2) (its own arm — deliberately not grouped with Role::Proposer's Some(6), per §7's cut-off of 2). (msgid.rs)
  • Duty executorMessageId::duty_executor() resolves to DutyExecutor::Validator. (msgid.rs)
  • Partial-signature kind — only PostConsensus is accepted; any other kind is rejected with PartialSignatureTypeRoleMismatch. No new kind added. (partial_signature.rs)
  • Packet cardinality — same-packet message_count > 1 is rejected via the per-validator bound. (partial_signature.rs / lib.rs)
  • Replay guard — a repeated PostConsensus packet for the same signer/slot is rejected by the inherited post-consensus seen-message counter. (message_counts.rs path; no new arm needed)
  • Monotonic slot (shared state) — consensus and post-consensus share one role-specific MessageId state; after accepting at slot N, a lower-slot envelope message from the same signer returns SlotAlreadyAdvanced (→ Ignore). Ordinary Role::Proposer state stays isolated by its different role byte. (inherited non-committee checks; no envelope-specific stale branch)
  • Fork gate — pre-Gloas messages are rejected with RoleNotActiveBeforeEthFork { minimum_fork: ForkName::Gloas }. (lib.rs validate_role_for_fork)
  • TTL — short bucket 1 + LATE_SLOT_ALLOWANCE (3 slots), not the committee bucket. (lib.rs)
  • Beacon dutyvalidate_beacon_duty rejects a known-epoch non-proposer with NoDuty, and tolerates a not-yet-fetched proposer epoch (accepts). (lib.rs)
  • Duty capduty_limit(EnvelopeProposer) == Ok(Some(slots_per_epoch)). (lib.rs)
  • Gossip classificationNoDuty and duty-limit overflow → Ignore; fork-gate, kind mismatch, packet-count overflow → Reject. (lib.rs classification test)
  • QBFT routing — validator-executor EnvelopeProposer returns RoleNotActive with a TODO(#1122) marker (executor is correct, routing not yet wired — deliberately not InconsistentMessageId); committee-executor path handles it for exhaustiveness. (qbft_manager/lib.rs)

Intentionally unchanged: no new PartialSignatureKind; no envelope-specific stale-slot branch (the role inherits the existing monotonic checks); Role::Proposer behaviour and its max_round() == Some(6); QBFT instance routing (deferred to #1122).

Risks, Trade-offs, and Mitigations (Required)

  • Blast radius: touches shared Role match sites, so a missed arm would be a compile error, not a silent runtime bug — the type system is the safety net here. All new arms are additive.
  • Trade-off: QBFT routing is a transient RoleNotActive reject rather than a working instance. This is deliberate and marked TODO(#1122); envelope QBFT duties are inert until that lands, matching the issue's staged plan.
  • One deviation from the issue's suggested shape: the committee-executor EnvelopeProposer case cannot produce InconsistentMessageId as literally worded, because duty_executor() always decodes bytes 8–55 as a Validator pubkey — a committee-executor EnvelopeProposer MessageId is unconstructable. The role is instead handled in the committee arm for exhaustiveness, and a test documents this encoding constraint. Called out for reviewer awareness.
  • Mitigation: every §7 rule has a dedicated test exercising the real validation entry point (not a hand-rolled shortcut), so behaviour is pinned rather than assumed.

Validation (Required)

  • cargo test -p ssv_types — 111 passed
  • cargo test -p message_validator — 100 passed (all new EnvelopeProposer coverage + sibling roles)
  • cargo test -p qbft_manager — 36 passed
  • cargo fmt --check — clean; cargo clippy --tests -- -D warnings — clean
  • Coverage is new tests written for this change, exercising production entry points: role byte round-trip, duty executor, QBFT classification, kind binding, repeated-packet rejection, pre-Gloas fork gate, short TTL bucket, known/unknown proposer-epoch behaviour, duty cap, both monotonic-slot directions through the shared MessageId state, and the qbft_manager transient/permanent reject behaviour.

Rollback (Required for behavior or runtime changes; optional otherwise)

Revert the single commit. The role is fork-gated to Gloas (not yet active) and QBFT routing is a no-op reject, so there is no runtime or data impact on current networks. No config or migration involved.

Additional Info / Next Steps (Optional)

Reviewer guide — production vs test. The diff is ~4% production, ~96% test. Read the production arms first (small, mechanical), then spot-check the tests.

Production (~42 lines) — read these first:

  • ssv_types/src/msgid.rs (+6) — the variant, [9,0,0,0] codec, max_round() = Some(2), Validator executor.
  • message_validator/src/lib.rs (+22) — committee lookup, fork gate, short TTL, duty_limit, and the validate_beacon_duty proposer-assignment arm.
  • qbft_manager/src/lib.rs (+11) — transient RoleNotActive reject + TODO(#1122); exhaustiveness in the committee arm.
  • message_validator/src/partial_signature.rs (+3 production) — the role added to the PostConsensus bind and the per-validator packet bound.

Tests (~950 lines) — verify the contract:

  • message_validator/src/partial_signature.rs (+577) — the §7 partial-signature contract: kind bind, packet cap, short TTL (accept + reject), proposer-assignment (3 cases), monotonic equality boundary + both stale-slot directions, replay rejection. A shared four_node_committee_and_keypair fixture removes repeated setup across these.
  • message_validator/src/consensus_message.rs (+111) — duty-cap and the pre-Gloas fork-gate rejection via the consensus entry point.
  • message_validator/src/lib.rs (+46) — gossip MessageAcceptance classification.
  • qbft_manager/src/tests.rs (+168) — transient/permanent reject behaviour and the committee-executor encoding constraint noted above.
  • ssv_types/src/msgid.rs (+48) — role byte round-trip and classification pinning.

The single commit is feat: add validator-scoped QBFT Role::EnvelopeProposer (#1120).

@jnhsigmap
jnhsigmap marked this pull request as ready for review July 20, 2026 12:51
@codecov-commenter

codecov-commenter commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.01575% with 5 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (epbs@2bf9ec4). Learn more about missing BASE report.

Files with missing lines Patch % Lines
anchor/message_validator/src/partial_signature.rs 99.00% 4 Missing ⚠️
anchor/message_validator/src/consensus_message.rs 98.57% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             epbs    #1149   +/-   ##
=======================================
  Coverage        ?   68.53%           
=======================================
  Files           ?      164           
  Lines           ?    31434           
  Branches        ?        0           
=======================================
  Hits            ?    21544           
  Misses          ?     9890           
  Partials        ?        0           
Flag Coverage Δ
rust 68.53% <99.01%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jnhsigmap

Copy link
Copy Markdown
Contributor Author

@claude, review this PR

@claude-code-actions-sigp

claude-code-actions-sigp Bot commented Jul 20, 2026

Copy link
Copy Markdown

Claude finished @jnhsigmap's task in 6m 7s —— View job


PR Review — Role::EnvelopeProposer (#1149)

  • Gather context (diff vs origin/epbs)
  • Review ssv_types/src/msgid.rs
  • Review message_validator/src/lib.rs
  • Review message_validator/src/partial_signature.rs
  • Review message_validator/src/consensus_message.rs
  • Review qbft_manager/src/lib.rs
  • Check untouched Role match sites for silent mishandling
  • Post review feedback

Verdict: LGTM — no blocking issues. The production surface is ~42 lines, all additive arms, and I verified the non-obvious interactions with the match sites the diff doesn't touch. Test coverage against the real entry points is thorough.

What I verified

Exhaustiveness safety net holds. Role isn't #[non_exhaustive], so every explicit match must name the new variant or the build fails — that's the intended net. The risk is the wildcard (_ =>) arms that compile silently but could mis-handle EnvelopeProposer. I checked the two in message_validator/src/lib.rs that the diff doesn't touch, and both defaults are correct for this role:

  • early_slot_allowance (lib.rs:1016) — falls through to Duration::ZERO. Correct: the envelope is signed at the proposal slot after the §4 block is published, not at a future lookahead slot, so the strict no-future rule is right. This is the key semantic divergence from ProposerPreferences, which needs the lookahead window.
  • stored_slot_count (lib.rs:710) — falls through to the 2-epoch default. Consistent with the above: no future-slot writes, so the dedup ring doesn't need the lookahead extension.

Doppelgänger handling is correctly left alone. operator_doppelganger/src/service.rs:154 exempts only ProposerPreferences from slot-based twin detection, because its slot is a future proposal_slot and can't be distinguished from a replayed twin. Since EnvelopeProposer's message slot is its emission slot, msg_slot > startup_slot is genuine twin evidence — so not exempting it is the right call. Worth a mention because it's the kind of thing that's silently correct by omission.

Wire/classification correct. Byte [9,0,0,0] matches SIP-94 RoleEnvelopeProposer = 9; max_round() == Some(2) gets its own arm (not folded into Proposer's Some(6)); duty_executor() resolves to Validator.

QBFT transient reject is sound. The validator-executor arm returns RoleNotActive with TODO(#1122) rather than InconsistentMessageId (routing pending, not malformed), and the committee-executor arm handles it for exhaustiveness. The stated reason a committee-executor EnvelopeProposer MessageId is unconstructable checks out — duty_executor() decodes bytes 8–55 as a Validator pubkey for this role.

Notes for the reviewer (non-blocking)

  1. Design confirmation, not a defect: the correctness of the two wildcard defaults above rests entirely on "envelope slot == proposal slot (present), never future." If that assumption is ever wrong, early_slot_allowance/stored_slot_count would both silently under-provision. It's correct today; just flagging that these two behaviors are load-bearing and not pinned by an explicit EnvelopeProposer arm or a dedicated test. A one-line assertion that the envelope slot is treated as present-time would lock the assumption in.

  2. Duty cap = slots_per_epoch is a sensible upper bound (a validator proposes at most once per slot), and mirrors ProposerPreferences. No concern.

  3. Codecov flags 5 uncovered lines in partial_signature.rs/consensus_message.rs — given these are the new arms, likely an unexercised branch inside a test helper rather than a real gap, but worth a glance.

Nothing here needs to block the merge.
· branch feat/1120-role-envelopeproposer

Comment thread anchor/qbft_manager/src/lib.rs Outdated
Comment thread anchor/qbft_manager/src/tests.rs Outdated
Comment thread anchor/message_validator/src/partial_signature.rs
Comment thread anchor/message_validator/src/lib.rs Outdated
Comment thread anchor/message_validator/src/partial_signature.rs Outdated
Comment thread anchor/message_validator/src/lib.rs

@shane-moore shane-moore left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@mergify

mergify Bot commented Jul 21, 2026

Copy link
Copy Markdown

Queued — the merge queue status continues in this comment ↓.

@mergify

mergify Bot commented Jul 22, 2026

Copy link
Copy Markdown

Merge Queue Status

  • Entered queue2026-07-22 01:44 UTC · Rule: default · triggered by rule Add outdated, ready-to-merge PRs to merge queue
  • Checks passed · on draft merge queue: checking epbs (2bf9ec4) and #1149 together #1162
  • Merged2026-07-22 02:14 UTC · at b426fa97f2b5bcfde14e54e5d7a55fefc99c042b · squash

This pull request spent 29 minutes 20 seconds in the queue, including 27 minutes 10 seconds running CI.

Required conditions to merge
  • check-success=cli-reference-check
  • check-success=run-local-testnet
  • check-success=test-suite-success

@mergify
mergify Bot merged commit ceaa8c0 into sigp:epbs Jul 22, 2026
23 checks passed
@mergify mergify Bot removed the queued label Jul 22, 2026
@jnhsigmap
jnhsigmap deleted the feat/1120-role-envelopeproposer branch July 28, 2026 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants