feat(x/provider): withdraw funds from consumer pool address at each block#10
feat(x/provider): withdraw funds from consumer pool address at each block#10
Conversation
…e-payment # Conflicts: # x/vaas/provider/types/params.go
tbruyelle
left a comment
There was a problem hiding this comment.
Some small comments but overall LGTM, thanks !
…e-payment # Conflicts: # x/vaas/provider/types/provider.pb.go
…ndary Drop the mid-epoch debt-only VSC packet machinery. Every epoch, each launched consumer receives exactly one VSC packet carrying the current ConsumerInDebt flag — even when the validator set did not change. This guarantees debt transitions reach the consumer within one epoch without a separate notification mechanism. Latency is at most blocks_per_epoch blocks (~1 hour at default 600 blocks), which is acceptable given the consumer is validated by the same validator set during the whole epoch anyway. Removed: - PendingConsumerDebtPackets map + Set/Has/Clear helpers - QueuePendingConsumerDebtPackets / HasQueuedVSCPackets - Mid-epoch SendVSCPackets path in EndBlockVSU - PendingConsumerDebtPacketsPrefix and its key name - Associated tests and assertions Changed: - QueueVSCPackets now enqueues one packet per consumer per epoch, regardless of whether the validator set changed. - EndBlockVSU reverts to the original epoch-gated flow. - Validate() comment on ValidatorSetChangePacketData reflects the new contract (empty updates are legal and mean "no valset changes this epoch").
Relayers sign /ibc.core.* messages directly with their own keys, so unwrapping nested authz MsgExec to detect IBC liveness traffic defends against a workflow no relayer actually uses. Removing the recursion removes the maxAuthzExecDepth magic number and cuts ante surface area. Authz-wrapped IBC core txs are now treated as non-IBC and blocked while the consumer is in debt; the replacement test asserts this.
Most txs on a healthy consumer chain are not IBC-core and the chain is not in debt. Check IsConsumerInDebt first so the hot path avoids the per-message type-URL walk entirely. Behavior is unchanged; only non-debt path gets faster.
validateFeesPerBlock and ValidateTemplateClient were shaped for the old
x/params subspace validator interface. With typed Params.Validate() the
interface{} + type-assertion dance is dead code. Retype both to concrete
types for compile-time safety. Also drop the redundant IsNegative check
in validateFeesPerBlock since IsValid already rejects negative amounts.
Replace the single sdk.NewCoin var with two typed consts (denom + int64 amount) and build the Coin in DefaultParams via NewInt64Coin. Fixes the denom from "photon" to "uphoton" to match the base-unit convention used elsewhere (cf. uatone in the consumer staking query).
Align test fixtures with the base-unit convention (uatone, uphoton) used elsewhere. No logic change.
Restore .gitignore to match main exactly, and normalize app.go imports via lint-fix so the only remaining diff versus main is the load-bearing providertypes.ModuleName entry in maccPerms.
Extend the debt-gate allowlist to /cosmos.gov.* so the community can vote on recovery (emergency funding, param changes) without requiring off-chain coordination. Pairs with the existing /ibc.core.* allowance which covers CCV liveness and ICS-20 v2 transfers via MsgSendPacket.
…t id Main's IBC v1 removal dropped Keeper.GetProviderChannel; the ConsumerFundsKeeper interface still referenced it. Switch to GetProviderClientID so the ante gate compiles against the post-merge consumer keeper and still skips the debt check until the provider client is established.
…rice The add-gas-price subcommand of ibc-v2-ts-relayer:latest no longer accepts --gas-adjustment. Leaving it in the call aborts the suite at setup with exit code 1.
Exercises the full fee-debt lifecycle: 1. Empty consumer fee pool → bank send on consumer rejected with ErrConsumerInDebt (probed via --dry-run which runs the ante chain). 2. Fund the pool on the provider with the deterministic fee-pool address (mirrors keeper.GetConsumerFeePoolAddress). 3. Next epoch clears the debt and the bank send is accepted again. Also sets provider genesis fees_per_block to the bond denom so the test can fund the pool from existing genesis accounts without having to mint uphoton. The test currently cannot run end-to-end because of a separate pre-existing ts-relayer setup breakage in "add-path" (reproduces on main). Once that is resolved the debt-flow test runs as part of TestVAAS.
…dd-gas-price" This reverts commit f42972f.
The isAllowedDuringDebtTx helper returns false on zero-length msg slices. Empty txs normally get caught later in the ante chain but ConsumerFundsDecorator runs first, so this defensive branch deserves explicit coverage.
Annotate validator_updates, valset_update_id, and consumer_in_debt to describe what each field carries, when it is expected to be empty, and how the consumer reacts. Also drop unused ErrNoProposerChannelId which became dead after main's IBC v1 removal.
|
@julienrbrt here is the list of the feature changes I've made compared to the initial version of the PR:
|
Drop voting-power-weighted distribution in favor of an equal split. VAAS consumers pay for the service of being validated, and each validator delivers roughly the same service regardless of stake, so the pay should match the work rather than the stake. This also sidesteps the rounding-to-zero starvation small validators suffered under proportional math with skewed voting power. Followup (tracked separately): filter to validators that actually signed the previous block so downtime is economically penalized, matching Cosmos SDK x/distribution. Also: - Drop GetLastTotalPower from the expected StakingKeeper interface (fees.go was the only caller). - Fix make mocks-gen to run mockgen through devdeps instead of the main module. - Regenerate testutil/keeper/mocks.go accordingly.
…lock Read VoteInfos from BeginBlock and filter to committing signers whose consensus address is in the current bonded set. Validators that were absent, voted nil, or have unbonded since signing forfeit their share. This penalizes downtime in the same way Cosmos SDK x/distribution does. Implementation: fetch the bonded set once and index by consensus address, then walk VoteInfos and look up each signer in the map. One KV scan of the bonded set instead of N per-signer indexed reads; also collapses "skip unbonded" and "skip removed" into a single map-miss branch. Tests cover: equal split, zero pool, share rounds to zero, remainder pooled, empty VoteInfos, BlockIDFlagAbsent filtered, signer no longer bonded, staking-keeper fetch error propagated.
| // Equal split. Any integer-division remainder stays in the provider | ||
| // module account and is picked up by the next block's GetBalance. | ||
| share := totalFees.Amount.Quo(math.NewInt(int64(len(validators)))) | ||
| share := totalFees.Amount.Quo(math.NewInt(int64(len(eligible)))) |
There was a problem hiding this comment.
now you are incentived to ddos your fellow validators 😆
but we need this anyway, just funny to think about it.
…unds
Previously any error from SendCoinsFromAccountToModule flipped the
consumer's debt flag, conflating chain-config issues (blocked address,
denom send-disabled, send restrictions) with real underfunding. That
misled operators whose fee pool was full but were told they were in
debt, with no way to recover.
Check errorsmod.IsOf(err, sdkerrors.ErrInsufficientFunds) to handle the
two cases separately:
- ErrInsufficientFunds → real debt (e.g. vesting lockup reduced
spendable below the amount GetBalance reported).
- anything else → log and skip without touching the debt flag.
Tests: rename the generic-error case to assert debt stays false, and
add a new ErrInsufficientFunds case asserting debt is flipped to true.
SendCoins already enforces spendable balance and returns ErrInsufficientFunds on underfunding, which is the same signal used to mark a consumer in debt. The GetBalance pre-check was both redundant (one extra bank call per consumer per block on the happy path) and strictly weaker than SendCoins (it ignored vesting lockups). CollectFeesFromConsumers also no longer returns an error since no path produced one; the caller's rollback branch is removed.
The decorator previously ran before ValidateBasicDecorator, which meant empty/malformed txs in debt surfaced "consumer chain is in debt" instead of their actual validation error. Moving it after ValidateBasic lets the proper error reach the user, and lets us drop the unreachable empty-msgs guard (and the test that documented the old workaround).
ConsumerFundsDecorator and MsgFilterDecorator were both consumer-state admission gates differing only in phase: MsgFilter handled the pre-CCV window (only /ibc.* allowed), ConsumerFunds handled the in-debt window (only /ibc.core.* and /cosmos.gov.* allowed). Folding them together expresses the three modes (pre-CCV / normal / in-debt) as one switch and drops a redundant GetProviderClientID call per tx in the hot path.
close #9
close #21
Summary
This PR adds the fee-payment flow for consumer chains and enforces debt status when a consumer can no longer cover its per-block fee.
On the provider side, launched consumers are charged
fees_per_blockfrom a deterministic fee-pool account and the collected fees are distributed to provider validators. If a consumer does not have enough funds, the provider marks it as in debt and propagates that status to the consumer over the existing VSC channel.On the consumer side, the debt flag is persisted in consumer chain state and used by the existing
MsgFilterDecoratorto reject non-/ibc.core.*and non-/cosmos.gov.*transactions while the chain is underfunded. This keeps IBC/CCV traffic alive (so the chain can recover once funded again) and governance available for community-driven recovery, without attempting to halt consensus.Changes
fees_per_blockand consumer debt trackingconsumer_in_debtMsgFilterDecoratorto gate user transactions while in debt