Skip to content

feat(validator_store): fork-gate RegistrationService at GLOAS_FORK_EPOCH - #1137

Merged
mergify[bot] merged 2 commits into
sigp:epbsfrom
shane-moore:feat/gloas-gate-registration-service
Jul 16, 2026
Merged

feat(validator_store): fork-gate RegistrationService at GLOAS_FORK_EPOCH#1137
mergify[bot] merged 2 commits into
sigp:epbsfrom
shane-moore:feat/gloas-gate-registration-service

Conversation

@shane-moore

@shane-moore shane-moore commented Jul 14, 2026

Copy link
Copy Markdown
Member

Problem, Evidence, and Context (Required)

Change Overview (Required)

  • Adds a Gloas gate at the top of the registration spawning loop: once fork_name_at_slot(slot).gloas_enabled(), the loop logs at debug, sleeps to the next epoch, and re-evaluates. No BN call, no signing.
  • The gate decision is a pure should_skip_for_gloas helper so it is unit-testable without a runtime, mirroring is_scheduled_for_slot in the same file.
  • Intentionally unchanged: the service is not deleted and its startup is not gated; pre-Gloas behavior is identical apart from the pure check. Inbound ValidatorRegistration partial-sig rejection is out of scope (feat(message_validator): reject ValidatorRegistration partial-sig at/after Gloas #1115).

Risks, Trade-offs, and Mitigations (Required)

  • Sleep-and-recheck was chosen over terminating the task: a break would rest on a single wall-clock reading, so a transient forward clock step (bad NTP) on a pre-Gloas network would permanently stop registrations until restart. The sleeping loop self-heals next epoch and matches how the sibling fork-aware services (proposer preferences, payload attestation, metadata) gate.
  • Cost of keeping the task alive is one timer wakeup and one debug line per epoch.

Validation (Required)

  • Two runtime-free unit tests on the helper: the exact inclusive fork boundary (last pre-fork slot still registers, first fork-epoch slot skips) and the unscheduled-fork case.
  • cargo test -p anchor_validator_store: 58 passed. Workspace fmt and clippy clean.

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

  • Revert the single commit; no config, data, or wire impact.

Blockers / Dependencies (Optional)

N/A. Independent of #1062-#1064; all required LH symbols are already in the current pin.

Skip the periodic register_validators publish once the current slot's
fork is Gloas: SIP-94 §4 removes the relay-builder registration flow,
so the calls no longer feed block production. The service stays
spawned and re-evaluates each epoch, leaving pre-Gloas networks and
the transition window unaffected.
Consolidate the four gate tests into two: the fork-boundary test
already covers the before/after cases, and the unscheduled case stays.
Trim the helper doc so the call-site comment owns the SIP-94 §4
rationale.
@shane-moore

Copy link
Copy Markdown
Member Author

@claude, please take a look

@claude-code-actions-sigp

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

Copy link
Copy Markdown

Claude finished @shane-moore's task in 2m 32s —— View job


Review: fork-gate RegistrationService at GLOAS_FORK_EPOCH

  • Read the changed file and diff against origin/epbs
  • Analyze the Gloas gate logic and should_skip_for_gloas helper
  • Review the tests
  • Post review feedback

Solid, well-scoped change. The gate logic is correct and consistent with the established fork-gating pattern already used in qbft_manager/src/lib.rs:321 and validator_store/src/metadata_service.rs:134 (spec.fork_name_at_slot::<E>(slot).gloas_enabled()). Extracting should_skip_for_gloas as a pure helper mirrors is_scheduled_for_slot and makes the boundary unit-testable without a runtime — good call. The sleep-and-recheck rationale (self-healing vs. break on a single clock read) is sound and matches the sibling services.

No blocking issues found. A few observations:

1. Startup is not gated for Gloas-from-genesis networks (minor, and intentional)

The PR notes startup is intentionally left ungated. Worth flagging the consequence: on a network where Gloas is active from genesis, this task spawns and then loops forever doing nothing but one debug! + one timer wakeup per epoch, indefinitely. Compare PayloadAttestationService, which is gated at the call site on spec.is_gloas_scheduled() (client/src/lib.rs:825) precisely to "avoid spawning a perpetual idle task." The inverse gate here would be if !spec.is_gloas_scheduled_from_genesis() (or similar) — but since the per-epoch cost is trivial and the sleeping-loop design is deliberately chosen for transition-window safety, this is a defensible trade-off rather than a defect. Just calling out that the two sibling services now handle the "idle on wrong side of the fork" case differently.

2. PR description is stale on test count

The description says "Four runtime-free unit tests," but the current diff (after 405fbde refactor(validator_store): trim redundant Gloas gate tests) has two: should_skip_for_gloas_flips_exactly_at_fork_boundary and should_skip_for_gloas_returns_false_when_unscheduled. The remaining two cover the important cases (inclusive boundary + unscheduled), so coverage is fine — just update the PR body so it matches.

3. Fallback arithmetic (registration_service.rs:82) — correct, noting for the record

slot_duration * S::E::slots_per_epoch() as u32 relies on slots_per_epoch() (a u64) fitting in u32. It always does for real specs, and Duration's Mul<u32> is the right impl, so this is fine. No change needed.

Nothing here rises to a code suggestion. Nice, tight PR.
· feat/gloas-gate-registration-service

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.74194% with 10 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (epbs@3b17438). Learn more about missing BASE report.

Files with missing lines Patch % Lines
anchor/validator_store/src/registration_service.rs 67.74% 10 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             epbs    #1137   +/-   ##
=======================================
  Coverage        ?   65.67%           
=======================================
  Files           ?      163           
  Lines           ?    29058           
  Branches        ?        0           
=======================================
  Hits            ?    19083           
  Misses          ?     9975           
  Partials        ?        0           
Flag Coverage Δ
rust 65.67% <67.74%> (?)

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

shane-moore commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

@claude,

  1. Yes, deliberate: the spawn-gate polarity that works for PTC (is_gloas_scheduled(), post-fork service) has no safe analogue here, since a pre-fork service must spawn on every network today and the fork position is only knowable per-slot. The idle task costs one timer wakeup per epoch; the real cleanup is deleting the whole service once pre-Gloas networks are gone, which subsumes any startup gate.
  2. Description updated to match the two consolidated tests.
  3. Agreed, no change.

@claude-code-actions-sigp

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

Copy link
Copy Markdown

Claude finished @shane-moore's task in 33s —— View job


Re: review follow-ups

Thanks for the detailed responses — all three land well.

1. Startup gate polarity. Agreed, and the reasoning is airtight. The PTC gate (is_gloas_scheduled()) works because it's a post-fork service: it can safely refuse to spawn when Gloas isn't scheduled. A pre-fork service is the mirror image — it must spawn on every live network today, and the fork position is only knowable per-slot, so there's no genesis-time predicate that safely suppresses the spawn without also killing it on the networks that still need it. The idle-task cost (one timer wakeup + one debug line per epoch) is genuinely negligible, and framing the real cleanup as deletion of the whole service once pre-Gloas networks retire is the right end state — a startup gate would just be interim scaffolding that deletion subsumes. No objection.

2. Test count. Confirmed against the current file — two consolidated tests:

  • should_skip_for_gloas_flips_exactly_at_fork_boundary (registration_service.rs:265)
  • should_skip_for_gloas_returns_false_when_unscheduled (registration_service.rs:288)

They cover the inclusive boundary and the unscheduled-fork case, which are the two that matter. Description now matches.

3. 👍

No blocking issues, nothing further from me. LGTM.
· feat/gloas-gate-registration-service

@petarjuki7 petarjuki7 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.

Looks good!

@mergify

mergify Bot commented Jul 16, 2026

Copy link
Copy Markdown

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

@mergify

mergify Bot commented Jul 16, 2026

Copy link
Copy Markdown

Merge Queue Status

This pull request spent 12 minutes 45 seconds in the queue, including 11 minutes 31 seconds running CI.

Required conditions to merge
  • check-success=test-suite-success

mergify Bot added a commit that referenced this pull request Jul 16, 2026
@mergify
mergify Bot merged commit b41a4f6 into sigp:epbs Jul 16, 2026
25 checks passed
@mergify mergify Bot removed the queued label Jul 16, 2026
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