fix: tighten downtime anchoring, challenge lookup, and the challenge CLI - #72
Open
giunatale wants to merge 1 commit into
Open
Conversation
giunatale
requested review from
clockworkgr,
julienrbrt and
tbruyelle
as code owners
July 30, 2026 16:08
Contributor
Author
giunatale
force-pushed
the
giunatale/fix/downtime-precision-hardening
branch
from
July 31, 2026 18:41
d475996 to
c0dbaf1
Compare
windowEndTimestamp now rejects downtime evidence whose window-end height predates the client's retained consensus states -- i.e. when no consensus state at a height <= window-end is stored to bracket the upper anchor. A pruned or sparse client could otherwise anchor the age/grace checks to a state far above window-end, understating the evidence's age and letting genuinely-stale evidence pass. The timestamp itself now comes from the bracketing lower state rather than the upper one: consumer block times rise with height, so that state's timestamp is a verified lower bound on the true window-end time and the computed age can never come out smaller than the real one. That removes the forward skew entirely, which is what makes the published retention figure honest -- a challenger's worst case is DowntimeEvidenceMaxAge + DowntimeChallengeWindow plus the window's own span, not that plus an unbounded anchor skew. The upper state is still required, as proof the consumer actually reached the window end. The downtime challenge lookup no longer depends on the live consumer-addr to provider-addr key-assignment mapping being intact. The reporting validator's consumer consensus address is now stored on the pending slash and matched on when the fast-path (provider-addr keyed) lookup misses, so a legit slash stays cancellable after the mapping is pruned or the validator rotates its consumer key. The stored address is self-authenticated by the challenge's signature check. The challenge CLI now defaults the header's trusted height to the highest consensus-state height the consumer's client stores below claimed_height+1, instead of the client's latest height -- with an active relayer the latest height is usually already at or past the header, which the light client rejects, so the old default never verified. An explicit --trusted-height must be below claimed_height+1 or the command errors before broadcasting. Trusted validators are now fetched at trusted_height+1: the light client checks them against the trusted consensus state's NextValidatorsHash -- the hash of the next height's set -- so fetching at the trusted height itself only verified when the valset happened not to change across it. Double-vote evidence stays unbounded by age on purpose: it is a permanent cryptographic fault, punished whenever proven, made one-time by the tombstone, and already floored by the per-consumer min height. Document that asymmetry with the age-bounded downtime path. The infraction parameters are no longer frozen at genesis. A new MsgUpdateInfractionParams replaces them in full under the module authority, so every slash fraction, jail duration, tombstone flag, SLA window, challenge window and evidence max age can respond to an incident without a coordinated binary upgrade. They get their own message rather than riding on MsgUpdateParams because they are stored and exported as their own state item: a fee or epoch change need not restate the slashing policy, and vice versa. QueryParams now reports them alongside Params so a proposal author can read what is in force. Validation on that path is the genesis validation, plus the cross-parameter constraints the split creates. InfractionParameters.Validate covers the set itself; ValidateInfractionParamsAgainst then checks the challengeable interval (evidence max age + challenge window) against the incoming or stored trusting-period fraction, from both handlers -- so MsgUpdateParams can no longer lower the fraction out from under the stored infraction parameters, which genesis validates as a pair but nothing enforced at runtime. Widening the challengeable interval is additionally checked against the consumer clients already adopted: a proposal reaching past the trusting period of a pinned client would leave the oldest challengeable header unverifiable there, so pending slashes on that consumer would execute undefended, and the provider does not re-adopt a client to repair it. Narrowing stays unconditionally allowed, so a chain already holding a client too short for its parameters can still correct itself. This finally makes the change-tolerance machinery reachable. SetInfractionParams records the superseded SLA in PreviousDowntimeParams, and AcceptableDowntimeParams keeps accepting evidence echoing it for evidence-max-age + challenge-window after the change; neither had a non-genesis caller before. Evidence spanning a change is judged by the threshold its echoed values imply, not the live one. Tests: sparse-anchor rejection, bracketed-anchor acceptance, and the anchor landing on the highest state at or below window-end; challenge succeeds after the key-assignment mapping is pruned; an old but above-min-height double-sign still slashes a bonded validator; the trusted-height pick takes the highest stored height below the header and skips other revisions; authority gating and full validation of MsgUpdateInfractionParams; MsgUpdateParams rejecting a trusting fraction that strands the stored infraction parameters; widening past a pinned client's trusting period rejected while narrowing and a fitting widening pass; PreviousDowntimeParams written on an SLA change and not on any other; and end-to-end acceptance of in-flight evidence echoing a superseded SLA, with rejection once the tolerance horizon elapses.
giunatale
force-pushed
the
giunatale/fix/downtime-precision-hardening
branch
from
July 31, 2026 19:56
c0dbaf1 to
af83b05
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.
Precision fixes across the downtime and equivocation pipeline, plus two real
defects in the downtime-challenge CLI that made the default invocation unable
to ever verify.
Keeper fixes
windowEndTimestampresolved a window's timestampagainst the consumer client's consensus states; with sparse or pruned states
an unbracketed anchor could skew the window's age and mis-price a slash. An
evidence packet whose window end cannot be bracketed by stored consensus
states is now rejected rather than guessed at (parameter-free, fail-closed).
keyed off the current key-assignment mapping, which key rotation or pruning
can invalidate while a slash is pending. The accused validator's consumer
consensus address is now persisted on the pending slash itself and used as a
fallback, so a challenge always finds its target.
tested rather than implicit — equivocation is punished whenever proven (the
consumer's minimum evidence height is the only gate), and tombstoning makes
double-punishment impossible.
Infraction parameters become governable
Every slash fraction, jail duration, tombstone flag, SLA window, challenge
window, and evidence max-age was frozen at genesis:
SetInfractionParamshad nocaller outside
InitGenesis,MsgUpdateParamscarries only the other parameterset, and no other message existed. Governance could not respond to an incident,
or to a threshold that turned out to slash honest operators, without a
coordinated binary upgrade — and an unfortunate genesis (zero slash fraction,
tombstone false, zero jail duration all validate) was permanently unfixable.
Two pieces of machinery showed a runtime path had been intended and was
unreachable:
SetInfractionParamsmaintains a record of the superseded downtimeparameters, and the evidence path implements a change-tolerance horizon so
evidence echoing a just-superseded SLA is still judged fairly.
Adds
MsgUpdateInfractionParams(authority-gated, mirroring the existingMsgUpdateParamshandler), which makes that machinery reachable and tested:evidence echoing the superseded SLA is accepted within the horizon and refused
past it. Two cross-parameter hazards are closed at the same time:
MsgUpdateParamscould already lower
TrustingPeriodFractionout from under the stored infractionparameters, breaking a pairing genesis validates but no update path re-checked;
and growing the challengeable interval could push it past the trusting period of
clients that are already pinned, silently making queued accusations undefendable —
so growth is now validated against every adopted consumer client, while narrowing
stays unconditional.
Challenge CLI fixes
challenge-consumer-downtimeassembles the header/commit/validator material avalidator submits to disprove a downtime accusation. Two defects made it
effectively unusable exactly when needed:
client rejects a header at or below its trusted height, and with a live
relayer the latest height is virtually always past the infraction, so the
default never verified. It now defaults to the highest stored
consensus-state height strictly below
claimed_height+1, in the header'srevision, via a paged consensus-state-heights query; a clear error explains
when no usable state remains. An explicit
--trusted-heightat or above theheader height now fails fast client-side.
T, but the lightclient checks them against
NextValidatorsHash— the set ofT+1— soverification only succeeded when the consumer's set happened not to change.
They are now fetched at
T+1on both the default and override paths.Testing
Unit: unbracketed-anchor rejection, the consumer-address fallback lookup
(including after key rotation), the unbounded-age property, and the
trusted-height selection helper (revision filtering, bound exclusion,
unsorted input). Build, lint, and the module suite are green; a separate e2e
(in the test-coverage PR) drives this CLI end to end against real chain data.