fix: F-2026-18133 | [Dual Defense] Same-Block Validator Jailing Can Snapshot an Oversized Ballot Quorum - #347
Merged
Merged
Conversation
…napshot an Oversized Ballot Quorum Exclude jailed validators from GetEligibleVoters so a validator jailed in BeginBlock is not snapshotted into ballots created later in the same block.
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-18133 — Same-Block Validator Jailing Can Snapshot an Oversized Ballot Quorum
Severity: Info · Scope: Core · Base:
audit-fixes(fresh-genesis — no upgrade handler / state migration needed)Implements Recommendation 1 only. Recs 2 (auto-recompute on UV leave hooks) and 3 (ops monitoring) are deliberately not in scope here.
Root cause
GetEligibleVoters(x/uvalidator/keeper/validator.go) gated on UV lifecycle,IsBonded()andIsTombstoned()— but not onJailed.In the Cosmos SDK, jailing and unbonding are two different things that happen at two different times:
x/slashingjails in BeginBlock; the bonded → unbonding move happens in staking's EndBlocker. So for the entire tx-processing phase in between, a jailed validator is bothJailed == trueandIsBonded() == true— and was therefore snapshotted into any ballot created during that block.Reachability
The snapshot is frozen at ballot creation, and the threshold is
(2*N)/3 + 1(x/uvalidator/keeper/ballot.go:283) computed on the inflatedN, while onlyN-1signers can actually vote:Recovery required an admin
MsgRecomputeBallotQuorum;DefaultExpiryAfterBlocks = 100_000_000means a stranded ballot effectively never ages out on its own.Fix
One condition added to
GetEligibleVoters, alongside the existingIsBonded()/IsTombstoned()gates:This tightens at ballot creation, so the threshold is computed on the smaller
N— which improves liveness. It is the opposite direction from tightening vote admission against an already-frozen threshold, so it carries none of the F-2026-16991 deadlock coupling and needs no companion auto-recompute.Existing ballots are unaffected:
VoteOnBallot→GetOrCreateBallotonly consults the passed voter set when it creates a ballot, so already-frozen snapshots keep their membership and threshold.msgServer.VoteInbound/VoteOutboundadmission is untouched, so a validator that was already inside a frozen snapshot before being jailed can still cast its vote there.Blast radius — please review
Six call sites inherit the new predicate on this branch:
x/uexecutor/keeper/voting.go:23x/uexecutor/keeper/voting.go:87x/utss/keeper/voting.go:113VoteOnFundMigrationBallot— samex/utss/keeper/initiate_tss_key_process.go:145x/utss/keeper/hooks.go:45handleEligibleValidatorSetChangecount gatex/uvalidator/keeper/ballot.go:254RecomputeBallotQuorum— admin escape hatch now also drops jailed validatorsRecomputeBallotQuorumalready marks a ballotEXPIREDwhen the recomputed eligible count reaches zero. Jailing every validator at once and then recomputing would now hit that branch, where previously the jailed set still counted. That matches how the same function already treats a fully unbonded / tombstoned set.Correction to the finding text
The report lists seven inheriting call sites, including
x/ucallback/keeper/voting.go:32.x/ucallbackdoes not exist on this branch — it lives on the read-state line of development and will inherit the fix when it lands here. The report also cites Cosmos SDK v0.53.0, whereas this branch pins v0.50.10 (go.modline 19); the mechanism is byte-for-byte identical in v0.50.10 (jailValidatorsetsJailedand leavesStatusalone;IsBonded()isGetStatus() == Bonded), so the finding stands as written.Tests
test/integration/uvalidator/jailed_voter_quorum_test.go— jails a validator through staking's realKeeper.Jail(exactly what slashing calls in BeginBlock) without running the EndBlocker, then exercises the same block:Jail, the validator isIsJailed()and stillStatus == Bonded/IsBonded(). If this ever stops holding, the test says so.GetEligibleVoters(3 → 2)PENDING_JOINvalidatorVoteOnInboundBallotpath freezesVotingThreshold == 2(on N-1) and does not contain the jailed validatorMutation check
With the
IsJailed()gate disabled and the tests unchanged, 4 of the 5 subtests fail:The
preconditionsubtest passes under mutation by design — it asserts SDK behaviour, not this fix.