cosmos / gaia

Cosmos Hub
https://hub.cosmos.network
Apache License 2.0
465 stars 679 forks source link

Refactor: fee antehandler refactor #2351

Closed yaruwangway closed 1 year ago

yaruwangway commented 1 year ago

Summary

Gaia fee antehandler introduction

Gaia fee antehandler takes min_gas_price, Global fee into a combined fee requirement. The combined fee:

  1. takes the higher fees between min_gas_price and global fee if min_gas_price and global_fee have the same denom. Otherwise, take the global fee.
  2. consider the global fee denom as required denom even if global fee is zero coin.

For point 2, present gaia fee antehandler allows zero coins globalfee, for example, globalfee=[0stake] means the chain does not want to charge transaction fees, but if there is volunteer paid fee, it should be in the denom of stake.

Concerns

Globalfee is sdk.DecCoins type, but cosmos-sdk sdk.DecCoins are sanitized to remove zero coins in it, so several methods from sdk.DecCoins are over-written in gaia fee antehandler. Even though present fee antehandler logic works, allowing zero coins in global fee makes the fee check logic in antehandler complicated and difficult to maintain.

Type

Code Debt Design Debt

Impact

Simplify gaia fee antehandler, the present logic is complex and hard to maintain, need to maintain extra methods that overwritten sdk's methods.

### Potential state-breaking For the hub, this refactor means users cannot propose zero coins in globalfee through gov proposal. If global fee is empty, the bond_denom will be used to check the paid fee denoms. This is potential state-breaking if in param store, globalfee params already contain zero coins (assume someone proposed zero coins as globalfee before this refactor). Please see below solutions, step4: upgrade handler, for how to deal with this scenarios.

Proposed Solution

Proposal:

## proposal 1: Globalfee does not allow zero coins, this means globalfee=[0stake] is invalid setup. - If the chain does not want to charge fees (above mentioned point 2), it can set globalfee=[], if globalfee empty, we can use bonding denom to check the denoms of the global fee. - If the chain allows any other denoms as paid fee denom, it has to set a value in global fee, for example, globalfee=[0.0001stake]

This can simplify the logic, but disables one possibility: bonding denom = uatom, but the chain want to set 0stake as globalfee (the chain does not charge tx fee, but tx still pays, it has to be stake (not a bonding denom of the chain.) )

So in summary: after refactor, globalfee does not allow zero coins. - if chain want to set globalfee=[0uatom] (uatom is bond-denom), it can set globalfee=[] - if chain want to set globalfee=[0stake] (uatom is bond-denom), it has to setup as globalfee=[x stake], (x > 0)

Proposal 2:

globalfee can contain zero coins, in the fee check antehandler: split the combinedFeeRequirement(get a combined fee requirement from local fee(min-gas-prices in app.toml) and global fee) into zero and non-zero coins: combinedFeeRequirement = nonZeroCoinFeesReq + zeroCoinFeesDenomReq

split the paidfee according to the nonZeroCoinFeesReq and zeroCoinFeesDenomReq denoms: paidfee= feeCoinsNoZeroDenom + feeCoinsZeroDenom since nonZeroCoinFeesReq does not contain zero coins, sdk DenomsSubsetOf and IsAnyGTE can be called.

example: globalfee=[1photon, 0uatom, 1stake] local min-gas-prices = [0.5stake]

get combinedFeeRequirement = [1photon, 0uatom, 1stake], split the combinedFeeRequirement into [0uatom], and [1photon, 1stake]

paidFee =[1uatom, 0.5photon] split the paidFee into [1uatom] (the same denom as zero coins in combinedFeeRequirement), and [0.5stake]

steps:

  1. check paidfee denoms
  2. if bypass-msg, tx pass
  3. if not bypass-msg, if paidfee contains feeCoinsZeroDenom, tx pass. feeCoinsZeroDenom means paidfees contain denoms from zero coins in combinedFeeRequirement.
  4. if not bypass-msg, if paidfee does not contain feeCoinsZeroDenom, check feeCoinsNoZeroDenom 's amount IsAnyGTE than nonZeroCoinFeesReq
  5. special case: when paidfee=[], and there is zero coins in combinedFeeRequirement, tx pass.

example:

globalfee=[0uatom,1stake,1photon], min-gas-prices=[] in app.toml. combinedFeeRequirement=[0uatom,1stake,1photon]. nonZeroCoinFeesReq=[1stake,1photon], zeroCoinFeesReq=[0uatom].

  1. paidfee=[1uatom,3quark], fail in step 1.
  2. paidfee=[1uatom,2photon], pass denom checks, then split paid fee,feeCoinsZeroDenom=[1uatom], feeCoinsNoZeroDenom=[2photon], feeCoinsZeroDenom is not empty, direct pass tx.
  3. paidfee=[2photon] pass denom checks, then split paid fee,feeCoinsZeroDenom=[], feeCoinsNoZeroDenom=[2photon], feeCoinsZeroDenom empty, check feeCoinsNoZeroDenom IsAnyGTE than nonZeroCoinFeesReq, pass
  4. paidfee=[] pass denoms checks, then split paid fee,feeCoinsZeroDenom=[], feeCoinsNoZeroDenom=[], feeCoinsZeroDenom empty, if directly check feeCoinsNoZeroDenom IsAnyGTE than nonZeroCoinFeesReq, this tx will fail, so check if combinedFeeRequirement is also empty first before check IsAnyGTE
  5. paidfee=[0.5photon], pass denom checks, then split paid fee,feeCoinsZeroDenom=[], feeCoinsNoZeroDenom=[0.5photon], feeCoinsZeroDenom empty, check feeCoinsNoZeroDenom IsAnyGTE then nonZeroCoinFeesReq fail.

For Admin Use

mmulji-ic commented 1 year ago

Under proposal 2:

paidFee =[1uatom, 0.5photon]
split the paidFee into [1uatom] (the same denom as zero coins in combinedFeeRequirement), and [0.5stake]

The [0.5stake] should read [0.5photon]