Skip to content

fix: reserve the withheld-fee escrow and fix fee-pool distribution edge cases - #70

Open
giunatale wants to merge 1 commit into
giunatale/feat/offline-detectionfrom
giunatale/fix/fee-pool-correctness
Open

fix: reserve the withheld-fee escrow and fix fee-pool distribution edge cases#70
giunatale wants to merge 1 commit into
giunatale/feat/offline-detectionfrom
giunatale/fix/fee-pool-correctness

Conversation

@giunatale

@giunatale giunatale commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

The consumer fee pool doubles as the escrow for downtime-accusation refunds,
but nothing actually reserved the escrowed amounts — plus three smaller
distribution edges.

Quotes and payouts now come from one definition

The fee-claim queries recomputed a claim inline as shares * balance / total,
without reserving the outstanding escrow, while the helper that did apply the
cap had no production caller — so a depositor querying during an open challenge
window was quoted more than a withdrawal would pay. Capping the queries is not
quite enough either: an escrow-limited withdrawal burns whole shares and
re-derives tokens from them, so a naive min(claim, available) can still
over-quote by a truncation unit. Both the withdrawal and both queries now go
through a single feePoolDraw definition, so a quote and a payout cannot
disagree at all.

An unaccounted pool balance can no longer halt a block

The send restriction deliberately exempts the distribution module account
(governance funding routes through it), which means a community-pool spend
addressed straight at a consumer fee-pool address is waved through and creates a
balance no shares account for — exactly the state the restriction exists to
prevent. That state then panicked in three places, one of them reachable from
BeginBlock through consumer deletion, so a governance transaction could arm a
later chain halt; a third site refused to import such a balance, so a state
export taken afterwards could not be restarted. All three now absorb the balance
deterministically, crediting the distribution module account — the same
accounting the proper funding path would have produced, so the funds return to
the community pool on sweep — and log it. Genuine store or codec failures still
panic, so a deletion can never be silently aborted. The consistency invariant
still reports the state, with its godoc explaining that it is reachable and
self-healing.

Escrow reservation

When downtime evidence is accepted, the accused validator's fee share is
withheld and recorded; a successful challenge retro-pays it from the pool. But
ordinary distribution and funder withdrawal operated against the full pool
balance, so the money backing a pending challenge could be spent out from under
it and a proven-innocent validator's refund could bounce.
DistributeConsumerFees and WithdrawShares now operate against
balance - outstandingWithheld, withdrawal caps tokens (not shares) so the
share-accounting invariant holds, and the post-stop sweep deliberately does not
reserve (a stopped consumer's pending slashes are already cancelled, so its
escrow is dead and the full balance is released).

Refunds never pay a matured accusation

PayWithheldFees refunds every record for the consumer. A record whose
challenge window has already elapsed backs a slash that has since matured and
executed; it is now cleared without payment, so a later same-consumer challenge
can never refund an already-slashed validator.

Distribution edges

  • A positive per-epoch fee smaller than the bonded-validator count truncated to
    a zero share and skipped both charging and the debt flag — a free epoch. It
    now flags debt; a genuinely zero fee still owes nothing.
  • Downtime fee exclusion applies to the epoch the infraction window falls in,
    not the epoch the evidence arrived in: excluding on arrival could dock a
    later epoch the validator was genuinely owed. When the window's epoch has
    already paid out, only the (separately priced) pending slash applies.
  • ResolveDowntimeSlashTokens rejects a non-positive photon conversion rate
    (previously only zero); a negative rate would have produced a negative slash.

Testing

Unit coverage for every path above, including the full
exclude -> distribute -> challenge -> refund-paid-in-full sequence, the
truncation debt flag, epoch-targeted exclusion, and the expired-record refund
guard. Build, lint, and the module suite are green.

@giunatale

Copy link
Copy Markdown
Contributor Author

Branched from giunatale/feat/offline-detection (#63).
Opened against that branch so only this PR's commit shows; will retarget to main after #63 lands and this rebases.

- reserve outstanding withheld-fee escrow (balance minus outstandingWithheld)
  in DistributeConsumerFees, WithdrawShares, and ComputeClaim so ordinary
  distribution or withdrawal can no longer spend the funds backing a pending
  downtime challenge; a proven-innocent validator's withheld share is then
  paid back in full. WithdrawShares caps the tokens (not the shares) so the
  no-orphan-balance invariant still holds. Sweep runs only post-stop, where the
  escrow is already dead, so it releases the full balance (documented in place).
- flag debt instead of granting a free epoch when a positive fees_per_epoch is
  smaller than the bonded-validator count (the even share truncates to zero); a
  genuinely zero fee still owes nothing and is not flagged.
- apply the downtime fee exclusion to the infraction epoch, not the epoch the
  evidence arrived in: mark epoch downtime only when the window falls in the
  current, not-yet-distributed epoch.
- reject a non-positive photon conversion rate in ResolveDowntimeSlashTokens
  (previously only a zero rate), since a negative rate would yield a negative
  slash.
- guard PayWithheldFees so an expired record -- one backing a downtime slash
  that has already matured and executed -- is cleared but never refunded, even
  if a later same-consumer challenge lands before the record is swept.
- stop requiring the withheld-fee record in the downtime-slash e2e: with the
  exclusion now gated on the infraction epoch, the suite's 5-block epochs make
  the already-distributed path (clawback via the slash) the common timing, so
  the e2e observes the record when present and asserts the slash execution
  unconditionally; withholding mechanics stay covered by unit tests.

- quote the reserved escrow in the fee-claim queries: ComputeClaim, WithdrawShares
  and both claim queries now share one feePoolDraw computation, so
  ConsumerFeePoolClaim and ConsumerFeePoolClaims can no longer quote a depositor
  more than a withdrawal pays during an open challenge window. The queries used to
  recompute shares * balance / total inline with no escrow subtraction while
  ComputeClaim -- the version that applies the reservation -- had no caller.
- credit the community pool with a fee-pool balance that no shares account for,
  instead of panicking on it. The send restriction deliberately exempts the
  distribution module so community-pool spends can reach a pool, so a spend
  addressed straight at a pool address lands funds with nothing minted against
  them; MintShares, the sweep and genesis import now book that balance to the
  distribution module account -- the same accounting a gov-signed
  MsgFundConsumerFeePool would have produced -- and log it at error level. The
  sweep runs from BeginBlock on consumer deletion, where the old panic would have
  halted the provider; store failures there still panic, so a deletion is never
  silently aborted. The invariant keeps reporting the state, since shares do not
  cover the balance until something touches that pool.
@giunatale
giunatale force-pushed the giunatale/fix/fee-pool-correctness branch from 7bd0bf4 to d5eb65b Compare July 31, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant