feat: add Role::PTCCommittee with message-validator fork-gate

[shane-moore]

Problem, Evidence, and Context

• SIP-94 §3 (ssvlabs/SIPs#94) adds the Payload Timeliness Committee duty at the Gloas / EIP-7732 / ePBS fork. Anchor needs a new Role::PTCCommittee wire surface before any other PTC work can land.
• Adding the variant breaks five exhaustive match sites in message_validator and qbft_manager; those edits ship together so the workspace stays green between PRs. The qbft_manager change is a one-line stub that the PTC instance routing PR replaces.
• The role is wire-incompatible with pre-CStar operators (pre-CStar nodes decode the byte as Err(NoMatchingVariant)). The message-validator fork-gate is defense-in-depth for CStar-capable nodes that might receive a PTC message before their local active_fork >= Fork::CStar.
• Issue: feat: add Role::PTCCommittee with message-validator fork-gate #1032
• Milestone: ePBS: Payload Timeliness Committee (PTC) Attestation
• Spec: consensus-specs Gloas compute_ptc

Change Overview

Diff shape: +553 / -9 across 5 files. ~69 lines of production code, ~484 lines of tests (≈7:1). Most of the test volume is ValidationContext construction boilerplate (matches the existing house style for full-pipeline integration tests in partial_signature.rs).

ssv_types — wire surface:

• Role::PTCCommittee variant: byte [7, 0, 0, 0] (matches SIP-94 BNRolePTCAttester = 7), is_committee_role = true, duty_executor = Committee, max_round = Some(4).
• No new PartialSignatureKind variant. PTC is structurally a post-consensus signature (signed after QBFT decides PayloadAttestationData, same shape as Role::Committee's post-consensus path). Reusing PostConsensus eliminates a wire-discriminant slot and ~30 LOC of test boilerplate. The role-to-kind binding lives in partial_signature_type_matches_role.

message_validator — fork-gate + bounds:

• validate_role_for_fork: pre-CStar reject mirroring the AggregatorCommittee/pre-Boole safety net.
• message_lateness: PTC bucketed in the long-TTL arm (committee-scoped messages need cross-cluster propagation time, even with mid-slot deadlines).
• duty_limit: Some(min(slots_per_epoch, V)) where V = local validator count. PTC pool for slot S = union of beacon committees for slot S, so each validator is PTC-eligible in at most one slot per epoch. Distinct from Committee/AggregatorCommittee's min(slots_per_epoch, 2*V) — PTC has no sync-committee component and no attest+sync multiplier.
• partial_signature_type_matches_role: binds PTC to PostConsensus (folded into the existing Committee arm).
• Per-role bounds: new Role::PTCCommittee arm with message_count <= V and validator-index occurrence cap 1. Structural maximum — PTC produces exactly one partial-sig per locally-assigned validator per slot. Distinct from Committee's caps (2V, 2) and AggregatorCommittee's (5V, 5).

qbft_manager — transient stub:

• Adds Role::PTCCommittee to the Some(DutyExecutor::Validator(_)) invalid-combination group.
• New committee-role arm returns Err(QbftError::RoleNotActive) with a warn-log, marked //todo(cstar). The PTC instance routing PR replaces both.

Tests delivered (10): 3 in ssv_types, 7 in message_validator. Each maps one-to-one to a design decision documented in the local milestone plan.

Intentionally not changed. partial_sig.rs and message_counts.rs (no new PartialSignatureKind variant). Slot-advancement-skip and validator-index-mismatch-skip behavior is inherited via is_committee_role() == true and structurally covered by existing AggregatorCommittee skip tests; adding PTC-specific copies would duplicate ~150 LOC with no new signal. The ptc_is_committee_role unit test asserts the boolean that drives both skips.

Risks, Trade-offs, and Mitigations

• Wire-byte collision audit. Anchor's Role::AggregatorCommittee = [6, 0, 0, 0] is Anchor-internal; PTC = 7 is upstream SIP-94. Manual audit for upstream go-ssv role conflict is required before merge. Byte 7 matches the SIP-assigned value and Anchor's byte 6 sits below the SIP allocation.
• qbft_manager stub is transient. PTC messages reaching receive_data are rejected with RoleNotActive + warn-log. Not a safety issue (no PTC QBFT instance exists yet — rejection is the correct response). The //todo(cstar) marker and warn-log must not survive the PTC instance routing PR.
• max_round = Some(4) is timing-derived, not measured. Follows from the 75% slot start and QUICK_TIMEOUT = 2s: round 1 finishes within slot, rounds 2-4 cover operator clock skew, rounds 5+ are dead weight. Retune only if Lighthouse moves the PTC service start time off 75% slot.

Validation

• cargo test -p ssv_types -p message_validator — 64 + 83 tests pass (10 new PTC tests; 0 regressions).
• make cargo-fmt-check, make lint — green.
• Each of the 10 new tests targets a specific design decision (PostConsensus binding, fork gate, TTL bucketing, duty_limit formula, per-role bounds, committee-role semantics, max_round value, committee-style duty-executor).
• Production code comments at three non-obvious sites document the WHY for reviewers reading the diff cold: lib.rs::message_lateness (long-TTL rationale), lib.rs::duty_limit (PTC selection mechanics + consensus-specs permalink to compute_ptc), partial_signature.rs::partial_signature_type_matches_role (PostConsensus reuse).

Rollback

• Single-commit revert. The variant is wire-incompatible with pre-CStar operators by design; pre-CStar peers reject the byte already. No data, config, or operational impact. Until CStar activates on a live SSV network, the new role is unreachable from any production code path.

Blockers / Dependencies

• None for this PR.
• Follow-up issues (same milestone): PTC instance routing in qbft_manager (replaces //todo(cstar) stub), PayloadAttestationVote SSZ container + value checker, metadata_service PTC voting-context phase, sign_payload_attestation impl, Lighthouse PayloadAttestationService spawn.

Additional Info

Base: epbs. The milestone bundles original M3.1 (ssv_types) + M3.7 (message-validator) because the exhaustive-match cascade made splitting them produce double-touched files. The follow-up issues land independently against this base.

[shane-moore]

@claude, please take a look. focus on the production code changes first and then the tests in another pass

[claude-code-actions-sigp]

Claude finished @shane-moore's task in 4m 20s —— View job

---

Review

• Read production code changes
• Cross-check against existing patterns
• Tests pass

Overall the production diff is tight and well-reasoned. The PostConsensus reuse is the right call, the per-role bounds match SIP-94 mechanics, and the fork gate mirrors the existing AggregatorCommittee/Boole pattern correctly. Nothing blocks merge for me.

---

Pass 1 — production code

anchor/common/ssv_types/src/msgid.rs

Looks correct. Byte 7 matches SIP-94 BNRolePTCAttester = 7. The [7,0,0,0] encoding, try_from decode, is_committee_role extension, and duty_executor() Committee arm are all consistent with the existing AggregatorCommittee pattern.

max_round = Some(4) — agree the rationale (75% slot start, QUICK_TIMEOUT = 2s, 1 useful round + skew margin) is sound. Comment at msgid.rs:81-83 documents the WHY clearly.

Nit (non-blocking): The match in From<Role> and TryFrom<&[u8]> (msgid.rs:30-39, 46-57) is now 8-armed and growing. Not worth refactoring in this PR but a const ROLE_ENCODINGS: [(Role, [u8;4]); 8] table-driven approach would compress these and make encoding/decoding mismatches structurally impossible. File for later.

anchor/message_validator/src/lib.rs

validate_role_for_fork (lib.rs:862-869) — Correctly mirrors the AggregatorCommittee/Boole guard. Order is irrelevant since each if matches a disjoint Role variant. ✓

message_lateness (lib.rs:937-938) — Bucketing PTC into the long-TTL arm is consistent with other committee-scoped messages. However, PTC has a mid-slot deadline by design (the timeliness in "Payload Timeliness Committee"). It's worth a code comment here clarifying: the long TTL is the message-validator's outer bound for gossipsub propagation; the actual PTC voting deadline is enforced at duty execution time, not here. Without that, a future reader could think long-TTL contradicts SIP-94 timing.

duty_limit for PTCCommittee (lib.rs:1052-1055) — min(slots_per_epoch, V) is structurally correct. The comment at lib.rs:1046-1051 linking to compute_ptc is exactly the kind of WHY-not-WHAT documentation reviewers need. ✓

Minor: there's no early-out analogous to the Committee/AggregatorCommittee if validator_index_count < slots_per_epoch_val / 2 short-circuit. For PTC that branch isn't needed (no sync-committee multiplier), so the simpler arm is right. Confirming this is intentional.

anchor/message_validator/src/partial_signature.rs (production portion)

partial_signature_type_matches_role (line 147) — Folding PTC into the Committee arm is the right reuse: both bind to PostConsensus only. This is the load-bearing decision behind "no new PartialSignatureKind variant" — keeps the wire surface minimal and the message-counts/duty-state machinery role-agnostic. ✓

Role::PTCCommittee arm in validate_partial_sig_messages_by_duty_logic (lines 317-341) — message_count <= V and per-validator-index occurrence cap of 1 are structurally exact for PTC. Distinct from Committee (2V, 2) and AggregatorCommittee (5V, 5).

Suggestion (non-blocking): The PTC arm duplicates the HashMap occurrence-counting loop pattern from the Committee and AggregatorCommittee arms with only the constant changing (1 vs 2 vs 5). With three call sites you're at the threshold where extracting a helper check_validator_index_occurrence_limit(messages, limit) is justified — eliminates ~30 LOC of triplicated logic. File for follow-up if you don't want to expand scope here. Fix this →

anchor/qbft_manager/src/lib.rs

Stub at lines 333-338 — Acceptable as transient. Err(QbftError::RoleNotActive) is the correct response: no PTC instance type exists yet, so any PTC consensus message at this layer is unrouteable. The //todo(cstar) marker is visible.

Concerns:

1. The warn-log will fire for every PTC message that reaches receive_data at CStar+. If PTC traffic is non-trivial this is a potential log flood between this PR and the routing follow-up. Consider downgrading to debug! or removing the log entirely until routing is wired — RoleNotActive is already returned and observable upstream.
2. No fork gate on the stub arm itself. AggregatorCommittee gates on < Fork::Boole here (lines 315-319) before its routing. Because the message validator already fork-gates PTC pre-CStar, defense-in-depth at this layer would be consistent with the AggregatorCommittee precedent. If you intend the routing PR to add it, fine; just flag.

---

Pass 2 — tests

Coverage is good. The 10 new tests map cleanly to design decisions:

ssv_types/src/msgid.rs:

• ptc_uses_committee_duty_executor — guards against future variant→duty-executor binding regressions.
• ptc_max_round_is_four — guards the deliberate Some(4) vs Some(12). The comment "regressions copying the Committee value would compile silently — this guards against that" is exactly right.
• ptc_is_committee_role — guards the boolean that drives is_committee_role-conditional skips. Important because this PR explicitly does NOT add PTC-specific tests for slot-advancement skip / validator-index-mismatch skip; that coverage is inherited via this boolean.
• role_roundtrip_all_variants — uses EnumIter and now exercises PTC automatically. Good design.

message_validator/src/consensus_message.rs:

• test_duty_limit_ptc_committee — covers both min branches (Vspe). Adequate.

message_validator/src/partial_signature.rs:

• test_ptc_committee_validator_index_occurrence_limit — boundary at 1 vs 2.
• test_ptc_committee_message_count_exceeds_validator_count — exactly V+1 distinct validators, confirming the structural limit.
• test_ptc_committee_rejected_before_cstar — confirms RoleNotActiveBeforeFork { minimum_fork: Fork::CStar, .. }.
• test_ptc_committee_within_ttl_accepted / test_ptc_committee_beyond_ttl_rejected — TTL boundary covered both sides.
• test_ptc_committee_invalid_partial_sig_kind_rejected — rejects RandaoPartialSig for PTC. This is the test that locks down the PostConsensus binding.

Test-quality observations

1. test_ptc_committee_message_count_exceeds_validator_count ordering. Test description claims "must trip the message_count > V check before the per-validator-index occurrence check fires." The test uses distinct validator indices, which means the per-index check would not fire anyway. The test correctly asserts on TooManyPartialSignatureMessages, but the comment slightly misrepresents what's being verified. If you want to actually verify ordering (count check runs before per-index check), the test would need V+1 messages with the same index — that would currently fail on either check, and a structural ordering test would need to disambiguate. As written the test verifies only that distinct-indices over V is rejected. Minor.

2. test_ptc_committee_validator_index_occurrence_limit is large (~100 LOC of ValidationContext + signing scaffolding for a "1 vs 2" boundary). The PR description acknowledges this is "house style" — fine — but if the helper extraction in pass-1 suggestion Add basic repository structure #2 happens, this test could shrink considerably. Not a blocker.

3. No PTC test for is_committee_role-driven slot-advancement skip / validator-index-mismatch skip. The PR explicitly justifies omission ("inherited via is_committee_role() == true and structurally covered by existing AggregatorCommittee skip tests; adding PTC-specific copies would duplicate ~150 LOC with no new signal"). Reasonable trade-off — the ptc_is_committee_role unit test is the right minimal guard for the boolean. If skip behavior diverges between committee roles in the future this becomes inadequate, but that's a future-PR concern.

4. test_ptc_committee_rejected_before_cstar uses Fork::Boole — good choice. Verifies the < boundary against the immediate predecessor fork rather than something farther away. If Fork::Boole < Fork::CStar weren't true (it is, from fork.rs:97-98), this test would catch it.

Test-quality summary

#	Severity	Item
T1	nit	test_ptc_committee_message_count_exceeds_validator_count comment overstates the ordering it verifies
T2	nit	Large test boilerplate (helper extraction would shrink)

No test concerns block merge. Coverage is proportional to the production changes.

---

Summary

Overall verdict: approve (informally — I can't submit formal reviews). Production decisions are correct, tests target the right invariants, and the WHY-comments at non-obvious sites are unusually well done. The qbft_manager warn-log and the duplicated occurrence-counting loop are the only things I'd consider addressing in this PR; everything else is fine as follow-up.
· branch: feat/ptc-role

[codecov-commenter]

Codecov Report

❌ Patch coverage is 96.74419% with 7 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (epbs@80d25de). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
anchor/qbft_manager/src/lib.rs	0.00%	3 Missing ⚠️
anchor/common/ssv_types/src/msgid.rs	90.90%	2 Missing ⚠️
anchor/message_validator/src/partial_signature.rs	98.42%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             epbs    #1033   +/-   ##
=======================================
  Coverage        ?   61.91%           
=======================================
  Files           ?      156           
  Lines           ?    26141           
  Branches        ?        0           
=======================================
  Hits            ?    16185           
  Misses          ?     9956           
  Partials        ?        0           

Flag	Coverage Δ
rust	61.91% <96.74%> (?)

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

☔ View full report in Codecov by Sentry.
📢 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.

[shane-moore]

@claude, thanks for the thorough pass. Walking through:

Table-driven Role encoding (msgid.rs): Not pursuing. Two properties matter for Role: (1) compiler catches missing arms on variant add/remove, (2) wire bytes for existing variants stay stable across refactors (go-ssv interop). The current explicit-match pattern gives both by construction. Wire bytes live in match arm bodies, pinned to their variant binding, so enum reorders or renames cannot silently change what byte gets emitted. A const table or repr(u8)+FromRepr approach moves wire bytes into a separate structure (table entries or = N annotations) that someone could silently reshuffle during a reorder, with round-trip tests still passing because they verify internal consistency, not absolute bytes. Since Role is actively churning during the ePBS arc (AggregatorCommittee landed recently, PTC just now, ProposerPreferences planned for a later milestone), the trade weakens (2) more than it strengthens (1). Worth revisiting once the enum stabilizes.

message_lateness PTC comment: Considered, then reverted. The match arm covers 6 roles; a PTC-specific clarification was awkward there, and the TTL-vs-duty-deadline distinction applies equally to Committee/Aggregator/AggregatorCommittee. Function name message_lateness + variable ttl already convey "message ageing, not duty timing" without a comment.

duty_limit sync-committee short-circuit confirmation: Intentional. The Committee/AggregatorCommittee short-circuit gates a sync-committee membership lookup. PTC has no analog (PTC pool = union of beacon committees per compute_ptc, no sync multiplier), so there is no lookup to gate and the simpler arm is correct.

Occurrence-counting helper extraction: Agreed, filed as #1039. The triplication pre-dates this PR; PTC is the third instance but extracting in this PR expands scope outside the wire surface. Test boilerplate (your Pass 2 #2) bundled into the same issue.

warn! -> debug! in qbft_manager stub: Keeping warn!. Matches the AggregatorCommittee precedent at qbft_manager/src/lib.rs:317, and the upstream error! at message_receiver/src/manager.rs:187 fires for every QBFT error including this stub's RoleNotActive anyway, so the dominant log noise source is not here. The whole arm goes away when routing lands.

Fork gate on qbft_manager stub: Already addressed by the expanded TODO in 364fc03: // TODO(cstar): wire PTC instance routing and add pre-CStar fork gate (mirror AggregatorCommittee arm above). The routing follow-up will add both. Note this layer is defense-in-depth only; the message-validator fork gate at lib.rs:862 already rejects pre-CStar PTC upstream of qbft_manager (single production caller, gated by Validator::validate).

Test comment overstates ordering (Pass 2 #1): Good catch, applied in a0bc8f1. Trimmed the wrong claim; kept the "V+1 distinct validator indices" hint since it explains why the loop is shaped that way. Comment now reads: // V+1 distinct validator indices: verifies the structural message-count cap (V).

Follow-up issue: #1039.

[claude-code-actions-sigp]

Claude finished @shane-moore's task in 1m 27s —— View job

---

Acknowledged

All dispositions verified against the tree.

Table-driven Role encoding. Reasoning accepted. The "wire bytes pinned to variant binding by syntactic position in the match arm body" property is exactly what makes the explicit-match form refactor-safe in a way #[repr(u8)] + FromRepr is not. Agree the trade flips once Role stops churning; not this PR.

message_lateness comment. Fair. The TTL-vs-deadline distinction is general to all committee-scoped roles, not PTC-specific — pinning a comment to one arm would be misleading. Withdrawing.

duty_limit short-circuit. Confirmed. The Committee/AggregatorCommittee branch gates a sync-committee membership lookup; PTC has no such lookup, so there's nothing to short-circuit. Simpler arm is correct.

Helper extraction → #1039. Right call to keep the wire-surface PR scoped. Bundling the test boilerplate into the same issue makes sense — the helper shrinks both production and test code in one pass.

warn! in qbft_manager stub. Verified the precedent: qbft_manager/src/lib.rs:317 uses warn! for the analogous AggregatorCommittee pre-Boole reject, and message_receiver/src/manager.rs:187 already emits error! for every receive_data failure including this one. Withdrawing the downgrade suggestion — the stub matches the established pattern and isn't the dominant log source.

Fork gate on stub. Verified 364fc03: TODO now reads // TODO(cstar): wire PTC instance routing and add pre-CStar fork gate (mirror AggregatorCommittee arm above). at qbft_manager/src/lib.rs:334-335. Combined with the upstream gate at message_validator/src/lib.rs:862-869 (single production caller path through Validator::validate), defense-in-depth is correctly deferred to the routing PR. Resolved.

T1 — test comment. Verified a0bc8f1. New comment // V+1 distinct validator indices: verifies the structural message-count cap (V). accurately describes what the test asserts (TooManyPartialSignatureMessages on V+1 distinct indices) without overclaiming ordering. Resolved.

No further blockers from my side. Production and test diff stand as-is for merge.

---

• Verify a0bc8f1 test comment change
• Verify 364fc03 qbft_manager TODO expansion
• Verify AggregatorCommittee warn! precedent at qbft_manager/src/lib.rs:317
• Verify upstream error! at message_receiver/src/manager.rs:187
• Verify message-validator fork gate at lib.rs:862
• Acknowledge dispositions
  · branch: feat/ptc-role

[diegomrsantos]

I don’t think this comment explains the Some(4) choice clearly enough. Starting PTC near 75% of a 12s slot leaves about 3s, so only round 1 is likely to complete before slot end with a 2s quick timeout. Rounds 2-4 are a local grace window rather than something derived from the protocol, and the current wording makes that sound more precise than it is.

Could we soften it to something like this?

// PTC is expected near 75% of a 12s slot, leaving about 3s.
// With 2s quick round timeouts, only round 1 is likely to finish
// before slot end. Allow up to 4 rounds as a small local grace
// window for delayed starts or message loss; this is not a
// consensus-spec requirement.

[shane-moore]

Applied verbatim in 4d98a50.

[diegomrsantos]

I think these are the right behaviors to cover, but the new validation tests are much heavier than the rules they assert. Most of the PTC cases rebuild a full signed message, RSA key pair, fork schedule, clock, committee info, and operator key map just to exercise a small branch like message_count > V, duplicate validator-index counting, or PartialSignatureKind matching.

Could we factor the repeated setup into test-only helpers and keep only one or two full-pipeline PTC smoke tests? The count and duplicate-index cases should ideally read mostly as "build these partial sig messages, validate PTC bounds, expect this error." That would keep the coverage while making the rule under test much easier to review.

The TTL coverage could also be more targeted: use a receive time that would fail under the short proposer/sync TTL but pass under the long committee TTL, so the test proves PTC is intentionally in the committee-style TTL bucket rather than only proving one accepted and one rejected timestamp.

[shane-moore]

Pushed 4d98a50. The three small branches you flagged now read as pure-function asserts:

• message_count > V → validate_ptc_committee_message_count(count, V), mirroring the existing validate_aggregator_committee_message_count shape that the small message-count tests at lines 519-554 already use.
• Duplicate validator-index → validate_validator_index_occurrence_limit(messages, limit).
• PartialSignatureKind matching → direct asserts on the existing pure function partial_signature_type_matches_role. The integration mapping (predicate false → PartialSignatureTypeRoleMismatch) is already covered end-to-end by test_partial_signature_message_with_invalid_type_for_role at line 578, and PTC shares that arm.

TTL is now bucket-specific via COMMITTEE_TTL_BUCKET_SLOTS = 20 (past proposer/sync TTL of 3, inside committee TTL of 34). Per your "rather than only proving one accepted and one rejected timestamp" framing I also dropped test_ptc_committee_beyond_ttl_rejected; the bucket-specific accepted test covers it and the generic LateSlotMessage upper-bound is exercised for other roles.

Remaining full-pipeline smoke tests: pre-CStar fork gate + bucket-specific TTL accepted. Two total, all using existing helpers rather than raw signing/key construction.

What I deliberately did not do: migrate the Committee and AggregatorCommittee inline arms (and the 152-LOC AggregatorCommittee occurrence test) to use the new helpers. That migration is a clean standalone refactor; doing it here mixes refactor with new code. The helpers ship with their first consumer in this PR and the rest moves in #1039.

Inline msgid.rs:81 thread resolved separately with your suggested wording.

[diegomrsantos]

Approved. Re-reviewed after the latest changes; the PTC round-limit comment and validation test refactor address my remaining concerns.

[mergify]

Merge Queue Status

• ✅ Entered queue — 2026-05-21 14:25 UTC · Rule: default
• ✅ Checks passed · on draft merge queue: embarking epbs (39f5ba1) and #1033 together #1049
• ✅ Merged — 2026-05-21 14:36 UTC · at 4d98a5059ddec4357a1892ac2441ed4af1ccd95d · squash

This pull request spent 11 minutes 16 seconds in the queue, including 9 minutes 46 seconds running CI.

Required conditions to merge

• check-success=test-suite-success