refactor!: gate ePBS on Ethereum Gloas fork, remove SSV Fork::CStar

[shane-moore]

Problem, Evidence, and Context

ePBS (Gloas) behavior on the epbs branch is gated behind a placeholder SSV protocol fork, Fork::CStar, that has never had a real schedule entry (only commented-out examples in the built-in ssv_fork_schedule.yaml files). That couples an Ethereum consensus-layer transition (Gloas / EIP-7732) to an SSV-invented fork ordinal, so the fork boundary effectively lives in two places (the SSV fork schedule and the eth2 config.yaml) and can drift, risking ePBS activating at the wrong slot.

Our pinned Lighthouse ChainSpec already exposes the Gloas boundary directly (gloas_fork_epoch, fork_name_at_slot / fork_name_at_epoch, gloas_enabled()), so the canonical source of truth is already in hand. This aligns with SIP-94, which frames ePBS as an Ethereum-fork-driven change rather than a new SSV network fork.

Change Overview

Gate the Gloas-specific paths on the Ethereum Gloas fork from the ChainSpec instead of the SSV fork:

active_fork(epoch) >= Fork::CStar
  ->  spec.fork_name_at_slot::<E>(slot).gloas_enabled()

• Remove the Fork::CStar variant; Fork is now {Alan, Boole}.
• Thread spec: Arc<ChainSpec> into QbftManager and the message Validator (validator_store already held one).
• Add a RoleNotActiveBeforeEthFork validation failure for PTCAttester messages received before the Gloas fork.
• Drop the cstar entries from the built-in ssv_fork_schedule.yaml files.

Reading order: start with common/fork/src/fork.rs + schedule.rs (the variant removal), then qbft_manager/src/lib.rs and validator_store/src/lib.rs (gate swap + dispatch), then message_validator/src/lib.rs (role gate + new failure variant), then the tests.

What did NOT change: BeaconVote <-> GloasBeaconVote dispatch behavior, domain types, and gossip topic prefixes. The SSV Fork schedule still governs SSV-only concerns (Alan/Boole gates, deprecated roles). Only the source of the Gloas boundary moved, from the SSV fork ordinal to the eth ChainSpec.

Risks, Trade-offs, and Mitigations

• Blast radius: 7 crates, but the change is an atomic enum-variant removal plus signature threading; it cannot be split without breaking compilation at intermediate steps. Net diff is small (+145 / -181).
• Operational: ePBS now activates from GLOAS_FORK_EPOCH in the eth2 network config. This is the same config that already drives every other Ethereum fork transition (Electra, Fulu), so there is no new operational surface, and it removes the prior risk of the SSV schedule and eth2 config disagreeing.
• Trade-off: the spec.fork_name_at_*(...).gloas_enabled() gate is open-coded at 3 sites rather than wrapped in a helper. This deliberately matches the existing house idiom (metadata_service gates Electra the same way); a wrapper was not added for 3 callers.

Validation

• cargo +nightly fmt --all -- --check: clean.
• cargo check --workspace --all-targets: clean.
• Tests pass: anchor_validator_store 46, message_validator 63, qbft_manager 31. The qbft_manager Gloas-dispatch and message_validator PTCAttester tests were rewritten to drive the boundary via a spec_with_gloas(Option<epoch>) helper that sets ChainSpec::gloas_fork_epoch, covering pre-Gloas, at-activation-boundary, and post-Gloas.
• Rebased on the latest epbs (including feat(validator_store): implement sign_payload_attestation #1082 sign_payload_attestation) before pushing.

Rollback

Pure git revert restores the Fork::CStar gate. No migrations, persisted state, or config changes are involved, and CStar had no real schedule entry, so there is no runtime/operational impact.

Additional Info / Next Steps

Follow-on ePBS attestation/sync work can gate on the same gloas_enabled() predicate rather than CStar, which also removes the need to keep Gloas on the SSV Fork ordinal axis.

🤖 Generated with Claude Code

[shane-moore]

@claude, please take a look

[claude-code-actions-sigp]

Claude finished @shane-moore's task in 5m 31s —— View job

---

PR Review

Solid, focused refactor. The Gloas boundary genuinely belongs on ChainSpec rather than an SSV-internal fork ordinal, the migration is mechanical, and the test rewrites preserve the meaningful boundary coverage. I have a couple of small observations and one suggestion, none of which are blocking.

---

What looks good

• Source-of-truth migration is clean. self.spec.fork_name_at_slot::<E>(slot).gloas_enabled() matches the existing house idiom in validator_store/src/metadata_service.rs:849, :899 and the attestation_due_switches_at_gloas_boundary test in validator_store/src/lib.rs:3713-3718. Not adding a wrapper helper for 3 callers is the right call given that pattern.
• Fork::CStar removal is complete. grep -rn "CStar\|cstar" across anchor/**/*.{rs,yaml,toml} returns no hits. The remaining Fork::Boole matches in subnet_service (service.rs:105, subnet.rs:121) collapse cleanly to non-| arms now that Fork is {Alan, Boole}.
• Boundary test is preserved with stronger semantics. qbft_manager/src/tests/gloas_dispatch_tests.rs:160-189 (test_committee_message_routes_at_gloas_activation_boundary) now drives the boundary directly via ChainSpec::gloas_fork_epoch, which is exactly the predicate production code reads. The last_pre_gloas / first_gloas pair pins fork_name_at_slot + gloas_enabled() simultaneously — an off-by-one in either flips one assertion.
• MessageAcceptance routing is consistent. ValidationFailure::RoleNotActiveBeforeEthFork falls through to _ => Reject in message_validator/src/lib.rs:232-258, matching how the SSV-side RoleNotActiveBeforeFork is handled. Rejecting a role that doesn't exist yet on the live chain is the right acceptance.
• AggregatorCommittee/Aggregator/SyncCommittee gates correctly stay on the SSV Fork schedule (message_validator/src/lib.rs:855-871, qbft_manager/src/lib.rs:333). The PR is appropriately surgical — only the Gloas gate moved.

---

Observations / suggestions (non-blocking)

1. RoleNotActiveBeforeEthFork drops current_fork (asymmetry with RoleNotActiveBeforeFork).

message_validator/src/lib.rs:226-229:

RoleNotActiveBeforeEthFork {
    role: Role,
    minimum_fork: ForkName,
},

The SSV-side variant carries current_fork: fork::Fork (:218-223). For ops/triage, knowing what the spec actually thought the active fork was at the offending epoch is useful — particularly because the failure path is invisible in the error otherwise. Consider:

RoleNotActiveBeforeEthFork {
    role: Role,
    current_fork: ForkName,   // spec.fork_name_at_epoch(epoch)
    minimum_fork: ForkName,
},

message_validator/src/lib.rs:874-884 already computes the predicate; carrying the ForkName adds one line and matches the existing variant's diagnostic shape. Fix this →

2. spec_with_gloas is duplicated across three crates.

The same 4-line helper exists in:

• qbft_manager/src/tests/gloas_dispatch_tests.rs:27-32
• message_validator/src/partial_signature.rs:493-498
• inline at validator_store/src/lib.rs:3716-3718

Cross-crate test helpers are awkward to share without a dev-dependency on a shared test-utils crate, so I would not block on it. Worth a one-line // TODO: extract to shared test util when a third copy lands comment, or just leaving as-is. Flagging only because the DRY rule in .claude/rules/rust-style.md explicitly calls out the third-copy threshold — and this PR creates the second and third copies. (The pattern itself — spec.gloas_fork_epoch = Option<Epoch> — is the right one.)

3. Doc comment on validate_role_for_fork is now incomplete.

message_validator/src/lib.rs:842-846:

/// Validates that a role is allowed for the fork active at the given slot.
///
/// Rejects:
/// - AggregatorCommittee before Boole fork (not yet active)
/// - Aggregator and SyncCommittee after Boole fork (deprecated)

The function now also rejects PTCAttester before Gloas. A one-line addition keeps the docstring honest. Fix this →

4. qbft_manager import note (verify only).

qbft_manager/src/lib.rs still imports Fork (line 6) and Epoch (line 34). Fork::Boole is used at :333 and Epoch is used elsewhere in the file — so both are still needed, no dead import. ✅ (Mentioning explicitly because the diff removed the only Fork::CStar use; just confirming the remaining imports are live.)

---

Verification

I did not run make test / make lint here (CI will). The PR description claims:

• cargo +nightly fmt --all -- --check: clean
• cargo check --workspace --all-targets: clean
• anchor_validator_store 46, message_validator 63, qbft_manager 31 tests pass

Recommend confirming make lint and make test are green in CI before merging given the cross-crate signature threading.

---

Tasks

• Read PR diff against origin/epbs
• Review fork variant removal (common/fork/)
• Review qbft_manager and validator_store gate swap
• Review message_validator role gate + new failure variant
• Review tests
• Post review feedback
  · branch: rip-cstar

[codecov-commenter]

Codecov Report

❌ Patch coverage is 86.66667% with 8 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (epbs@4db2c7f). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
anchor/message_validator/src/lib.rs	75.00%	3 Missing ⚠️
anchor/client/src/lib.rs	0.00%	2 Missing ⚠️
anchor/subnet_service/src/service.rs	0.00%	2 Missing ⚠️
anchor/subnet_service/src/subnet.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             epbs    #1090   +/-   ##
=======================================
  Coverage        ?   63.92%           
=======================================
  Files           ?      163           
  Lines           ?    28252           
  Branches        ?        0           
=======================================
  Hits            ?    18061           
  Misses          ?    10191           
  Partials        ?        0           

Flag	Coverage Δ
rust	63.92% <86.66%> (?)

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.

[shane-moore]

@claude, thanks for the review. Addressed in 942c551:

1. current_fork on RoleNotActiveBeforeEthFork — done. Worth noting this was a field the refactor dropped: the pre-refactor PTCAttester gate used RoleNotActiveBeforeFork, which already carried current_fork. Restored it, now carrying the Ethereum ForkName active at the offending epoch (spec.fork_name_at_epoch(epoch)), computed once and reused for the gloas_enabled() check — the correct axis for an eth-fork gate, and back in parity with the sibling variants.

2. spec_with_gloas duplication — declining, as convention. Sharing it cross-crate needs a dev-dependency test-utils crate (or a feature-gated fork::testing module); the repo currently keeps per-file test helpers (e.g. generate_fork_schedule is duplicated the same way). Happy to extract into a shared test util in a follow-up if/when a fourth copy lands.

3. validate_role_for_fork doc comment — done. Added the PTCAttester before the Ethereum Gloas (ePBS) fork bullet. (Minor note: the PTCAttester rejection itself predates this PR — it was previously CStar-gated — so the doc was already missing it; corrected now.)

4. qbft_manager imports — confirmed, no change needed. Fork stays live via Fork::Boole (lib.rs:223, :333) and Epoch via Epoch::new (lib.rs:222).

Verification: cargo clippy --workspace --tests -- -D warnings -D clippy::allow_attributes is clean, cargo +nightly fmt --all -- --check is clean, and message_validator 63/63 pass (the RoleNotActiveBeforeEthFork test uses .., so the new field needed no test change).

[claude-code-actions-sigp]

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

---

Re-review of 942c551

All four points land cleanly. Confirmed in the diff:

1. current_fork restored ✅ — message_validator/src/lib.rs:225-229 now carries current_fork: ForkName, and :876-883 computes it once via validation_context.spec.fork_name_at_epoch(epoch) and reuses it for both the gloas_enabled() predicate and the failure construction. This is strictly better than my original suggestion: I asked for the field; you also de-duplicated the spec lookup. Diagnostic parity with RoleNotActiveBeforeFork (:218-224) is restored, and the value is the Ethereum ForkName — the right axis for an eth-fork gate. Good catch on framing it as "restoring a field the refactor dropped" — the original CStar gate carried it, and losing it was a regression of the refactor, not a new omission.

2. spec_with_gloas duplication — accepted. Convention argument is fair. generate_fork_schedule is the local precedent, and a dev-dependency test-utils crate for two callsites is the wrong trade. Follow-up-on-fourth-copy is the right disposition.

3. Doc comment ✅ — :848 adds the PTCAttester before the Ethereum Gloas (ePBS) fork (not yet active) bullet, matching the format of the two existing entries. The "this rejection predates the PR" framing is correct — the doc was already stale against the CStar-era behavior, and this PR is what surfaces it. Worth fixing here regardless.

4. qbft_manager imports ✅ — confirmed live (Fork::Boole at lib.rs:333, Epoch::new at :222). No action needed.

Test compatibility ✅ — partial_signature.rs:1772-1778 uses matches!(.., RoleNotActiveBeforeEthFork { minimum_fork: ForkName::Gloas, .. }), so the .. rest-pattern absorbs the new field without churn. The test still asserts the meaningful invariant (minimum_fork == Gloas).

LGTM. Nothing else from me — recommend merging once CI is green.

Tasks

• Read commit 942c551 to verify the addressed feedback
• Verify current_fork field on RoleNotActiveBeforeEthFork
• Verify validate_role_for_fork doc comment update
• Verify qbft_manager imports are still live
• Post response
  · branch: rip-cstar

[petarjuki7]

Makes sense, lgtm!