fix: F-2026-18795 | [Dual Defense] Unbounded Active-Ballot Expiry Scan on Create + Disabled 100M Expiry

[0xNilesh]

F-2026-18795 — Unbounded Active-Ballot Expiry Scan on Create + Disabled 100M Expiry

Severity: Low (Impact 1 / Likelihood 1) · Base: audit-fixes

The bug

CreateBallot unconditionally called ExpireBallotsBeforeHeight before creating anything
(x/uvalidator/keeper/ballot.go:37). That walked the entire ActiveBallotIDs set and did a
Ballots.Get per entry.

The cost was not an in-memory walk — it was n IAVL reads + n protobuf unmarshals, gas-metered, on
a consensus path, once per ballot creation. The reason it had to open every ballot is that
ActiveBallotIDs is keyed by ballot id (a hash), so the expiry height is not in the key; the only
way to decide whether a ballot was due was to load it.

And it was guaranteed to find nothing. Expiry is deliberately disabled:
DefaultExpiryAfterBlocks = 100_000_000 (~19 years at 6s blocks,
x/uexecutor/types/constants.go). So every create paid O(|Active|) reads to expire exactly zero
ballots. Cumulative cost across the set's lifetime is ~O(n²).

Live donut carries ~556 active ballots (decoded AllActiveBallotIDs at height 22061950 — treat
as "hundreds, unbounded", not an exact figure). The set only shrinks on quorum finalization, so
never-quorum stragglers are permanent residue while expiry stays disabled.

The fix (four parts)

1. PendingByExpiry index. New collections.KeySet[collections.Pair[int64, string]] keyed by
(expiryHeight, ballotID), on the next free prefix in the module (7).
collections.Pair orders by the first component, so the sweep ranges [0, currentHeight] via
NewPrefixUntilPairRange and stops at the first row beyond it. Two wins: only due ballots are
visited (zero today), and Ballots.Get is no longer needed to decide — the height is the key,
so only ballots actually being expired are ever loaded.

2. Mirrored at every ActiveBallotIDs writer. All six, verified by grep:

Site	Op
ballot.go CreateBallot	Set → indexPending
ballot.go DeleteBallot	Remove → unindexPending
ballot.go MarkBallotExpired	Remove → unindexPending
ballot.go MarkBallotFinalized	Remove → unindexPending
keeper.go InitGenesis	Set → index rebuilt from the ballots restored just above
voting.go VoteOnBallot (new-ballot path)	Set → indexPending

DeleteBallot now reads the record before removing it, because the index row is keyed by the
ballot's expiry height. It stays idempotent on absent IDs.

3. Sweep moved from CreateBallot into a new x/uvalidator EndBlocker. Expiry should not
depend on inbound traffic arriving. The module had no EndBlock; it now has one, mirroring the
existing BeginBlock/BeginBlocker split. x/uvalidator was already listed in
app.ModuleManager.SetOrderEndBlockers (app/app.go:1108) — listing alone is not enough, the
module manager only calls modules that satisfy appmodule.HasEndBlocker, so the interface
assertion is declared in module.go and an integration test drives the app's real EndBlocker and
observes the sweep to guard against a silently-never-called hook.

A failed sweep is logged, not returned: expiry must never halt the chain, and the work is
idempotent since anything missed stays in the index for the next block.

4. MaxExpiriesPerBlock = 50. Bounds per-block work; leftovers stay in the index and are picked
up by the next block's sweep.

ExpireBallotsBeforeHeight keeps its two-phase shape — collect while iterating, mutate only after
the iterator is closed. Mutating a collection mid-iteration skips entries at best and panics at
worst.

Deliberately NOT changed

• DefaultExpiryAfterBlocks stays at 100M. The constant's own comment says it waits on an
  escape hatch for stuck pending items (see F-2026-18801, F-2026-18796); re-enabling expiry without
  one would strand inbounds. This change makes that future flip a one-line constant change rather
  than a performance cliff — the index is what removes the cost of having expiry on at all.
• No cap on ActiveBallotIDs size (Hacken rec 3). Rejecting creates past a threshold makes
  honest inbounds unobservable once full, trading a gas-cost problem for a liveness problem.
• No per-UV create rate limiting (rec 4). Creation is bonded-UV-gated already; this would add
  consensus state for a Low.

No migration / upgrade handler

audit-fixes targets mainnet, which starts from a fresh genesis — the index is built as ballots
are created, and InitGenesis rebuilds it on import (covered by a test). Same precedent as PR #317
dropping its remove-group handler.

Donut backfill required. When this is merged to the testnet branch, donut's ~556 existing
ActiveBallotIDs rows will have no PendingByExpiry entries and would therefore be invisible
to the sweep. That branch needs an upgrade handler that walks ActiveBallotIDs once and writes
(BlockHeightExpiry, id) for each. Harmless while expiry is at 100M, mandatory before it is ever
lowered.

Tests

x/uvalidator/keeper/ballot_test.go, x/uvalidator/keeper/genesis_test.go,
test/integration/uvalidator/ballot_voting_test.go:

• TestPendingByExpiryIndex_MirrorsEveryActiveSetWriter — index correctness at each writer;
  the finalize subtest also asserts the behavioural consequence of a leaked row (the sweep would
  overwrite PASSED with EXPIRED).
• TestExpireBallotsBeforeHeight_OnlyDueRowsAreTouched — past/boundary/future mix; not-yet-due rows
  stay indexed and pending.
• TestExpireBallotsBeforeHeight_CapsAtMaxExpiriesPerBlock — exactly 50 in one call, remainder
  survives, second call drains it.
• TestExpireBallotsBeforeHeight_OrphanedIndexRow — a row whose ballot record is gone is dropped,
  not retried forever, and does not block the ballot queued behind it.
• TestCreateBallot_DoesNotExpireOnCreate / _DoesNotExpireMultipleOldOnCreate — the two existing
  "expires on create" tests, rewritten for the new design: create expires nothing; the sweep does.
• TestInitGenesisRebuildsPendingByExpiry / TestInitGenesisRejectsDanglingActiveBallotID — why no
  migration is needed.
• TestIntegration_UvalidatorEndBlockerRuns — drives chainApp.EndBlocker (the real module-manager
  chain) and observes the sweep; also asserts the module is in OrderEndBlockers and implements
  appmodule.HasEndBlocker.

Assertions that catch a regression are placed first in each subtest: require aborts on
failure, so an ordering that checked something else first would never reach them.

Mutation-verified. Three mutations, each caught by the test that is supposed to catch it:

1. unindexPending removed from the MarkBallotFinalized removal site:

--- FAIL: TestPendingByExpiryIndex_MirrorsEveryActiveSetWriter/MarkBallotFinalized_unindexes
    Error:      Should be false
    Messages:   MarkBallotFinalized must remove the PendingByExpiry row

2. MaxExpiriesPerBlock bound removed from the sweep:

--- FAIL: TestExpireBallotsBeforeHeight_CapsAtMaxExpiriesPerBlock
    Error:      Not equal: expected: 50   actual: 57
    Messages:   a single sweep must expire at most MaxExpiriesPerBlock ballots

3. AppModule.EndBlock and the HasEndBlocker assertion removed (the
   never-called-EndBlock regression this change is most exposed to):

--- FAIL: TestIntegration_UvalidatorEndBlockerRuns/expires_due_ballots_and_leaves_the_rest_alone
    Error:      Not equal: expected: 4   actual: 1
    Messages:   the x/uvalidator EndBlocker did not run the ballot expiry sweep
--- FAIL: .../caps_a_large_backlog_at_MaxExpiriesPerBlock_per_block
    Error:      Not equal: expected: 50   actual: 0
--- FAIL: .../module_is_wired_into_the_EndBlocker_ordering
    Error:      Should be true
    Messages:   x/uvalidator must implement appmodule.HasEndBlocker; the module manager skips modules that do not

Full suite green with all mutations reverted:
go test -tags="ledger test_ledger_mock test" ./x/... ./app/... ./test/integration/...
-> 20 ok, 0 FAIL.