Derive duty deadlines from Gloas spec - #428
Open
AntiD2ta wants to merge 7 commits into
Open
Conversation
The controller derives its duty deadlines from the chain specification only when the corresponding option is unset, but main.go declared hardcoded defaults of 4s/8s and passed them unconditionally. The zero check was therefore never true, and every spec-derived deadline was computed and discarded: a beacon node serving ATTESTATION_DUE_BPS, or a SLOT_DURATION_MS shorter than SECONDS_PER_SLOT, had no effect. This also predates Gloas — a minimal-preset chain with a 6s slot already got an 8s aggregation deadline. Default those four options to 0, the sentinel that selects derivation. This preserves mainnet behaviour exactly: the pre-Gloas derivation of slotDuration/3 and slotDuration*2/3 yields 4s and 8s on a 12-second slot, so the hardcoded constants were the derivation written out by hand. A test case pins that equivalence. Making the derivation reachable arms a second defect. The choice between the pre-Gloas and Gloas deadlines was frozen at construction, from CurrentEpoch() >= HardForkEpoch(...). The fork epoch is constant and fine to read once; the current epoch is not, so the comparison had a shelf life of one epoch while the process runs for months. Derive both deadline sets at construction and select between them per duty, from that duty's slot. The predicate keys on the duty's slot rather than the current epoch because duties are scheduled up to an epoch ahead: during the epoch before the fork the controller schedules duties for the fork epoch, and a current-epoch predicate would give those the pre-fork deadlines. This matches the proposer's existing fork gate. Both sets are immutable after construction, so the selector needs no synchronisation. Obtain the Gloas fork epoch through gloasDetails, following the electraDetails precedent: a chain without GLOAS_FORK_EPOCH yields the far-future sentinel and stays on the pre-Gloas deadlines rather than failing to start. Operator overrides keep their meaning and apply to both sets, as documented. Alongside this: - derive fallback deadlines from the Gloas slot duration rather than SECONDS_PER_SLOT; they were previously computed before the SLOT_DURATION_MS override was read, so a node serving SLOT_DURATION_MS=6000 without the *_DUE_BPS values produced an 8s deadline in a 6s slot - range-check served basis points; 0 produced a slot-start deadline and a value above 10000 produced one past the end of the slot, both silently - exercise the plain *_DUE_BPS key names in tests, not only the _GLOAS-suffixed variants, so the branch that runs against a real beacon node is covered - fix an off-by-one in createElectraAttestations, where len(validatorIndices) < i let i == len(validatorIndices) through and panicked
The duty timing selection changed this file, so the custom lint requires its copyright range to cover the current year.
The duty timing options described a third and two thirds of the slot duration as the derived default. That holds before Gloas, but from Gloas onwards those fractions are only the fallback used when the chain serves no basis-point value for the duty; the default is otherwise the served deadline, applied to SLOT_DURATION_MS rather than to SECONDS_PER_SLOT. Name the spec value behind each option, and state in the shared preamble that the two derivations are selected per duty from that duty's slot.
The Gloas derivation read the unsuffixed spec keys as a fallback when the _GLOAS-suffixed key was absent, and fell back to the pre-Gloas fractions when neither was served. Both fallbacks are the deadlines that Gloas moves away from, so a node that does not serve the full set produced post-fork duties on pre-fork timings. Against glamsterdam-devnet-7, which serves ATTESTATION_DUE_BPS_GLOAS but none of the other three suffixed keys, that meant aggregating at 8.0004s and generating sync committee messages at 3.9996s where Gloas requires 6s and 3s; the 8.0004s aggregation lands after the 6s payload reveal. Read the exact _GLOAS key for the four deadlines Gloas redefines, and make the fallbacks the Gloas values (a quarter and a half of the slot, being 2500 and 5000 basis points) rather than the pre-Gloas third and two thirds. This matches the convention the payload deadlines already use. Pin the derived values to the specification rather than to the formula, with a conformance test built from the configuration that beacon.glamsterdam-devnet-8.ethpandaops.io serves, and a regression test for the devnet-7 key set. Devnet-8 serves all six suffixed keys, so it was already correct; the fallback path is what changes. Record the pre-Gloas and Gloas values alongside each option in main.go and in docs/configuration.md.
AntiD2ta
force-pushed
the
gloas-attestation-deadlines
branch
from
August 21, 2026 09:32
788dfcb to
460197f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
0so the derivation is actually reachable_GLOAS-suffixed deadlines after the fork, and fall back to the Gloas valuesThe bug: the derived deadlines were unreachable
The controller derives its duty deadlines from the chain specification, but only when the
corresponding option is unset:
main.gomade that condition permanently false. It declared hardcoded defaults and then passedthem unconditionally:
Because viper always yields a non-zero duration, the fallback never ran and every spec-derived
deadline was computed and discarded. The four values were fixed at 4s/8s/4s/8s for the lifetime of
the process regardless of what the chain served — so a beacon node serving
ATTESTATION_DUE_BPS, ora
SLOT_DURATION_MSshorter thanSECONDS_PER_SLOT, had no effect.This was not Gloas-specific. Any chain whose slot is shorter than 12 seconds — the minimal preset
used by local devnets — already received deadlines derived from a 12-second slot, putting the 8s
aggregation deadline after the end of a 6s slot.
The bug: the fork decision was frozen at startup
The choice between pre-Gloas and Gloas deadlines was made once, during construction:
HardForkEpochis a constant and is fine to read once, butCurrentEpoch()is not. Freezing thecomparison gives the result a shelf life of one epoch while the process runs for months, so a vouch
started before
GLOAS_FORK_EPOCHwould keep the pre-Gloas deadlines for its entire lifetime.This had no observable effect while the derived values were being discarded. Making the derivation
reachable arms it, so the two must be fixed together.
The fix
Reachability. Default the four options to
0, the sentinel the controller already treats as"derive from the chain specification". This is behaviour-preserving on mainnet: the pre-Gloas
derivation is
slotDuration/3andslotDuration*2/3, which on a 12-second slot yields exactly the4s and 8s that were previously hardcoded — those constants were the derivation, written out by hand.
A dedicated test case pins that equivalence so the two cannot drift apart silently.
Fork selection. Both deadline sets are derived at construction and stored; which one applies is
decided per duty from that duty's slot:
Keying on the duty's slot rather than on the current epoch matters at the boundary: duties are
scheduled up to an epoch ahead, so during the epoch before the fork the controller is scheduling
duties for the fork epoch. A predicate on
CurrentEpoch()would hand those post-fork duties thepre-fork deadlines. This mirrors the existing fork gate in the proposer, which compares
SlotToEpoch(duty.Slot())against the fork epoch for the same reason.Both sets are immutable after construction, so the selector needs no synchronisation — the four
delay fields were previously written once before the scheduling goroutines start, and that property
is preserved.
gloasForkEpochis obtained throughgloasDetails, following the existingelectraDetailsandbellatrixDetailsprecedent: a chain that does not carryGLOAS_FORK_EPOCHyields the far-futuresentinel, the comparison is never true, and the pre-Gloas deadlines apply forever. Vouch starts and
logs rather than refusing, as it does for every other fork.
Operator overrides keep their existing meaning and are applied to both sets: an explicit value is
absolute and applies on both sides of the fork.
docs/configuration.mddocuments this, and notesthat an explicit value valid today can schedule duties past the end of the slot on a future fork.
The bug: the Gloas deadlines fell back to the pre-Gloas ones
The Gloas derivation looked up the
_GLOAS-suffixed key, then the unsuffixed key, then a ratio:Both fallbacks are the deadlines Gloas moves away from. The unsuffixed keys hold the pre-Gloas
values and keep being served after the fork, and the ratios were
slotDuration/3andslotDuration*2/3, which are the pre-Gloas3333and6667basis points. So a node that served anincomplete set produced post-fork duties on pre-fork timings — the same defect this PR exists to fix,
reintroduced one level down.
Against
glamsterdam-devnet-7, which servesATTESTATION_DUE_BPS_GLOASbut none of the other threesuffixed keys, that meant:
An 8.0004s aggregation lands after the 6s payload reveal, in a slot whose anatomy is propose at 0s,
attest at 3s, reveal at 6s, and vote on the payload at 9s.
The fix. The Gloas arm reads the exact
_GLOASkey and no longer falls through to the unsuffixedone, and its fallbacks are the Gloas values — a quarter and a half of the slot, being
2500and5000basis points. That matches the convention the payload deadlines already use.glamsterdam-devnet-8serves all six suffixed keys and was therefore already correct; what changesis the behaviour of every node that does not.
Also in this change
SECONDS_PER_SLOT.Previously the ratio defaults were computed before the
SLOT_DURATION_MSoverride was read, so anode serving
SLOT_DURATION_MS=6000without the*_DUE_BPSvalues produced an 8-second deadlineinside a 6-second slot.
ATTESTATION_DUE_BPS: 0previously produced aslot-start deadline (attesting before a block can arrive) and a value above 10000 produced a
deadline past the end of the slot, both silently. Out-of-range values now fall back to the ratio
default.
the derivation itself uses.
TestGloasSpecConformancepins them to the configuration served bybeacon.glamsterdam-devnet-8.ethpandaops.io, and a devnet-7 case covers the partial key set. Theprevious table derived every expectation from the same arithmetic as the code, so it could only
demonstrate that the code agreed with itself.
createElectraAttestationsguarded its validator-index lookup withlen(validatorIndices) < i, anoff-by-one that panics when
i == len(validatorIndices). It now matches the correcti >= len(validatorIndices)form.scheduleAttestationAggregationsis split out of the attester scheduling path. This is a pureextraction with no behaviour change, keeping the enclosing function within the complexity limit
now that the aggregation deadline is looked up per slot.
services/controller/standard/synccommitteemessenger.gois refreshed, asthe custom lint requires for a file this change modifies.
Known limitation
services/chaintime/standardderives its slot duration solely fromSECONDS_PER_SLOTand does notread
SLOT_DURATION_MS. Every deadline here is applied as an offset fromStartOfSlot(slot), so ona chain where the two values diverge the slot boundaries themselves would be wrong and correct
in-slot offsets would not be sufficient. That is out of scope here and needs resolving before any
network ships a Gloas slot duration that differs from
SECONDS_PER_SLOT.Relatedly,
SECONDS_PER_SLOThas been dropped from the devnet-8config.yamland fromconfigs/mainnet.yamlatv1.7.0-alpha.13. Live nodes still serve it through/eth/v1/config/spec, and vouch reads only that key —parameters.slotDurationreturns an errorwithout it, so vouch would refuse to start against a node that stopped serving it. That is a
fork-independent dependency on a compatibility alias and is not addressed here.
One residual restart requirement remains, shared with every other fork vouch handles: a beacon node
that begins serving
GLOAS_FORK_EPOCHonly after vouch has started requires a vouch restart, becausethe fork schedule is read once at construction and the spec is cached for the life of the process.
Reviewer note
Most of the diff in
services/controller/standard/service.gois gofmt realignment: removingsyncCommitteeAggregationDelay, the longest field name in theServicestruct, shifts thealignment column for every other field. Reviewing that file with
git show -wreduces it to the 30inserted and 8 deleted lines that actually change behaviour.
Validation
go build ./...— clean.gosilent test ./...— 989 tests, all pass../custom-gcl run --max-issues-per-linter=0 --max-same-issues=0— 14 issues, all pre-existing andoutside the changed lines.