fix: F-2026-18148 | [Dual Defense] Removed Universal Validator Retains ChainMeta Vote Authority - #348
Merged
Merged
Conversation
…s ChainMeta Vote Authority Gate MsgVoteChainMeta on GetEligibleVoters (ACTIVE/PENDING_JOIN + bonded + not tombstoned) so a removed, still-bonded validator cannot reinsert votes.
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.
F-2026-18148 — Removed Universal Validator Retains ChainMeta Vote Authority
Severity: Info · Scope: Core · Base:
audit-fixes(fresh-genesis — no upgrade handler / state migration needed)Implements Recommendation 1 only. Recs 2 (revoke/expire voting AuthZ on removal), 3 (bound price/height), and 4 (admin reset for
LastAppliedChainHeight) are deliberately not in scope — see Residual below.The vector
MsgVoteChainMetaadmitted viaIsBondedUniversalValidator(x/uexecutor/keeper/msg_server.go:156), which is lifecycle-blind: any bonded, registered universal validator was accepted regardless of ACTIVE / PENDING_LEAVE / INACTIVE.Admin removal moves a universal validator to PENDING_LEAVE while its stake stays bonded.
AfterValidatorRemovedprunes that validator's ChainMeta rows, but it revokes neither its AuthZ grant nor its membership in the universal validator set. So the removed hotkey could turn straight around and reinsert a vote after the prune — and, because the median is recomputed over whoever is currently fresh, that reinserted vote immediately moved the oracle.Fix
VoteChainMetanow gates on the same eligibility predicate that uvalidator uses when it snapshots a ballot's voters — lifecycle ACTIVE / PENDING_JOIN and bonded and not tombstoned — by callingGetEligibleVotersdirectly rather than re-deriving the checks, so ChainMeta admission cannot drift away from that definition.The
IsTombstonedUniversalValidatorcheck is kept and moved ahead of the new gate so it retains its specific error message (and its "not present in the registered universal validators set" error for unregistered signers) instead of being shadowed into dead code.Why tightening admission is safe here — and only here
ChainMeta is median-based, not ballot-based (
x/uexecutor/keeper/chain_meta.go):There is no
CreateBallot, no snapshottedEligibleVoters, and no frozenVotingThreshold. Every vote recomputes the median over whichever votes are currently fresh, so a narrower admitted set cannot strand anything in flight.Scope is deliberately narrow.
VoteInbound(:83) andVoteOutbound(:123) are untouched, as are the utss vote entry points. Those are ballot-based with frozen thresholds — tightening their admission would strand in-flight ballots and reintroduce the F-2026-16991 deadlock. They belong to the parked F-2026-18192 / RC-13 work.Residual after Rec 1 — accepted knowingly
From Hacken's own text: "The same
MaxUint64median outcome is already reachable by an ACTIVE UV casting the identical vote on the current two-UV topology; the incremental defect addressed here is incomplete revocation after removal."Rec 1 closes only the removed-validator vector. An ACTIVE universal validator can still push a
MaxUint64price/height and stickLastAppliedChainHeight, leaving revert-outbound gas quotes unfunded. Rec 3 is what closes that, and it is deliberately not taken here.chainMetaMinVotesForFirstWrite = 3— bootstrap interactionAsked and answered, with a test: yes, tightening admission can keep a small set below the bootstrap minimum — but it does not introduce a new class of problem.
len(fresh)counts fresh vote rows, and there is at most one row per validator. Bootstrap therefore needs three distinct validators voting within the 300s window. Tightening admission can only shrink the pool that can produce a row, so a topology with fewer than three eligible universal validators can never bootstrap the oracle. That was already true of any topology with fewer than three bonded universal validators; this gate simply makes lifecycle state count toward the same threshold.Practically: a 3-UV set where one validator is removed (PENDING_LEAVE) or INACTIVE drops to 2 eligible voters and can no longer perform a cold-start bootstrap for a new observed chain. Already-bootstrapped chains (
LastAppliedChainHeight > 0) are unaffected — they take the normal median-on-each-fresh-vote path, which has no minimum. The constant is not changed in this PR; the last subtest documents the behaviour so it is visible rather than surprising.Tests
test/integration/uexecutor/vote_chain_meta_eligibility_test.go:RemoveUniversalValidator, the test asserts the validator is in PENDING_LEAVE, still bonded, and pruned from ChainMeta — then the removed hotkey tries to reinsert a price of 250, chosen to sit between the surviving 200 and 300 so it would drag the recorded upper median from 300 down to 250.State assertions are ordered before the
require.Error/require.Containsin every rejection subtest, so an aborting error assertion cannot mask a vote that actually landed.Mutation check
With
IsBondedUniversalValidatorrestored in place of the eligibility gate and the tests unchanged, 3 of the 5 subtests fail:0x12cis 300 and0xfais 250 — under mutation the removed validator's vote lands and moves the median, which is precisely the finding.The two positive-path subtests (ACTIVE / PENDING_JOIN still accepted) pass under mutation by design — they exist to prove the gate does not lock out legitimate voters.
Interaction with F-2026-18133
F-2026-18133 (separate PR) adds a
Jailedexclusion insideGetEligibleVoters; this PR makesVoteChainMetauseGetEligibleVoters. Composed, a jailed validator also loses ChainMeta vote authority. Different files, no merge conflict — each change is correct without the other.