-
Notifications
You must be signed in to change notification settings - Fork 85
Fix fee-less transactions, and refactor it to have fee-less addresses 🚀 #1389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
ad7584a
c11ab86
cf256e0
7530995
cf74fd8
84fea0e
263e28a
1a67c0d
2732133
e940bc0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,72 +2,94 @@ package ante | |
|
|
||
| import ( | ||
| "math" | ||
| "strings" | ||
|
|
||
| errorsmod "cosmossdk.io/errors" | ||
| sdkmath "cosmossdk.io/math" | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||
| authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" | ||
| govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" | ||
| parametertypes "github.com/elys-network/elys/v6/x/parameter/types" | ||
| oracletypes "github.com/ojo-network/ojo/x/oracle/types" | ||
| ) | ||
|
|
||
| // CheckTxFeeWithValidatorMinGasPrices implements the default fee logic, where the minimum price per | ||
| // unit of gas is fixed and set by each validator, can the tx priority is computed from the gas price. | ||
| func CheckTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) { | ||
| feeTx, ok := tx.(sdk.FeeTx) | ||
| if !ok { | ||
| return nil, 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") | ||
| } | ||
|
|
||
| feeCoins := feeTx.GetFee() | ||
| gas := feeTx.GetGas() | ||
| // GaslessAddrs contains only the governance module address. | ||
| // Governance can grant feegrants to operational addresses (price feeders, liquidation bots, etc.) | ||
| // When those addresses use feegrants, the fee payer becomes governance, enabling gasless transactions. | ||
| var GaslessAddrs = []string{ | ||
| authtypes.NewModuleAddress(govtypes.ModuleName).String(), // Governance module address | ||
| } | ||
|
|
||
| // Ensure that the provided fees meet a minimum threshold for the validator, | ||
| // if this is a CheckTx. This is only for local mempool purposes, and thus | ||
| // is only ran on check tx. | ||
| if ctx.IsCheckTx() { | ||
| minGasPrices := ctx.MinGasPrices() | ||
| // CheckTxFeeWithValidatorMinGasPrices checks fees in CheckTx (mempool). If the | ||
| // fee-payer is in GaslessAddrs, we force minGasPrices to zero so 0uelys txs pass. | ||
| func CheckTxFeeWithValidatorMinGasPrices( | ||
| ctx sdk.Context, tx sdk.Tx, | ||
| ) (sdk.Coins, int64, error) { | ||
| feeTx, ok := tx.(sdk.FeeTx) | ||
| if !ok { | ||
| return nil, 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, | ||
| "tx must implement FeeTx") | ||
| } | ||
|
|
||
| // Check for specific message types to adjust gas price | ||
| msgs := tx.GetMsgs() | ||
| if len(msgs) == 1 { | ||
| msgType := strings.ToLower(sdk.MsgTypeURL(msgs[0])) | ||
| if strings.Contains(msgType, sdk.MsgTypeURL(&oracletypes.MsgFeedPrice{})) || strings.Contains(msgType, sdk.MsgTypeURL(&oracletypes.MsgFeedMultiplePrices{})) { | ||
| // set the minimum gas price to 0 ELYS if the message is a feed price | ||
| minGasPrice := sdk.DecCoin{ | ||
| Denom: parametertypes.Elys, | ||
| Amount: sdkmath.LegacyZeroDec(), | ||
| } | ||
| if !minGasPrice.IsValid() { | ||
| return nil, 0, errorsmod.Wrap(sdkerrors.ErrLogic, "invalid gas price") | ||
| } | ||
| minGasPrices = sdk.NewDecCoins(minGasPrice) | ||
| feeCoins, gas := feeTx.GetFee(), feeTx.GetGas() | ||
| isGasless := false | ||
|
|
||
| // print minGasPrices | ||
| ctx.Logger().Info("Override minimum gas prices: " + minGasPrices.String()) | ||
| } | ||
| } | ||
| if ctx.IsCheckTx() { | ||
| // start with the validator's configured minGasPrices | ||
| minGasPrices := ctx.MinGasPrices() | ||
|
|
||
| if !minGasPrices.IsZero() { | ||
| requiredFees := make(sdk.Coins, len(minGasPrices)) | ||
| // Check if the fee payer is in the gasless whitelist | ||
| feePayerBytes := feeTx.FeePayer() | ||
| if len(feePayerBytes) > 0 { | ||
| feePayer := sdk.AccAddress(feePayerBytes).String() | ||
|
|
||
| for _, ga := range GaslessAddrs { | ||
| if feePayer == ga { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this allow address to do all txs gasless There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I mean this was just a design choice for flexibility. Allows governance to pick and choose what they want to allow as fee-less too. Then if it's abused the abuser can get removed it's not really a huge risk IMHO. But I mean I don't mind either way we can limit it to specific TXs. |
||
| isGasless = true | ||
| zero := sdkmath.LegacyZeroDec() | ||
| minGasPrices = sdk.NewDecCoins( | ||
| sdk.NewDecCoinFromDec(parametertypes.Elys, zero), | ||
| ) | ||
| ctx.Logger().Info( | ||
| "override minimum gas price to 0 for gasless address", | ||
| "address", ga, | ||
| ) | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Determine the required fees by multiplying each required minimum gas | ||
| // price by the gas limit, where fee = ceil(minGasPrice * gasLimit). | ||
| glDec := sdkmath.LegacyNewDec(int64(gas)) | ||
| for i, gp := range minGasPrices { | ||
| fee := gp.Amount.Mul(glDec) | ||
| requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) | ||
| } | ||
|
|
||
| if !feeCoins.IsAnyGTE(requiredFees) { | ||
| return nil, 0, errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) | ||
| } | ||
| } | ||
| } | ||
| // now enforce: feeCoins >= minGasPrices * gasLimit | ||
| if !minGasPrices.IsZero() { | ||
| required := make(sdk.Coins, len(minGasPrices)) | ||
| gdec := sdkmath.LegacyNewDec(int64(gas)) | ||
| for i, gp := range minGasPrices { | ||
| amt := gp.Amount.Mul(gdec).Ceil().RoundInt() | ||
| required[i] = sdk.NewCoin(gp.Denom, amt) | ||
| } | ||
| if !feeCoins.IsAnyGTE(required) { | ||
| return nil, 0, errorsmod.Wrapf( | ||
| sdkerrors.ErrInsufficientFee, | ||
| "insufficient fees; got: %s required: %s", | ||
| feeCoins, required, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| priority := getTxPriority(feeCoins, int64(gas)) | ||
| return feeCoins, priority, nil | ||
| // on DeliverTx/Simulate we just deduct whatever feeCoins was attached | ||
| // Give gasless transactions the highest priority | ||
| var priority int64 | ||
| if isGasless { | ||
| priority = math.MaxInt64 | ||
| ctx.Logger().Info( | ||
| "gasless transaction given highest priority", | ||
| "priority", priority, | ||
| ) | ||
| } else { | ||
| priority = getTxPriority(feeCoins, int64(gas)) | ||
| } | ||
|
|
||
| return feeCoins, priority, nil | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in this we are making all txs gases for hardcoded addresses, is that correct ? I guess we should only allow price txs as gasless otherwise someone can spam the chain with other txs if compromised. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The priority changes the priority of the TX, allowing our gasless TXs to execute before everyone else otherwise our gasless TXs always execute last. This logic also does not remove gas from the tx it simply makes it so the minimum fee is 0, so if you upload a TX with a fee 0 it can pass. But you can still pass in a fee. Also because this uses the feepayer address to identify who can send 0 uelys fees, we can actually make it one address then do a feegrant, from say governance. So we could control who has access. |
||
| } | ||
|
|
||
| // getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the gas price | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hard coding sender of tx makes tit difficult to change on the fly. Why not set it when chain starts by fetching params of respective module?
Only con is it might have to be done every block or we can do periodically or some other design
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean this makes it so governance can change it on the fly. If a wallet gets compromised then the governance module simply revokes the fee grant.