From 29756173d8aca246ec807a69bc031268334176e9 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 10:59:56 -0500 Subject: [PATCH 01/55] go mod --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 21e89a2..d2a35c1 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( cosmossdk.io/core v0.11.0 cosmossdk.io/depinject v1.0.0-alpha.4 cosmossdk.io/log v1.2.1 + cosmossdk.io/math v1.1.3-rc.1 github.com/client9/misspell v0.3.4 github.com/cometbft/cometbft v0.37.2 github.com/cometbft/cometbft-db v0.8.0 @@ -35,7 +36,6 @@ require ( cloud.google.com/go/iam v1.1.4 // indirect cloud.google.com/go/storage v1.30.1 // indirect cosmossdk.io/errors v1.0.0 // indirect - cosmossdk.io/math v1.1.3-rc.1 // indirect cosmossdk.io/tools/rosetta v0.2.1 // indirect filippo.io/edwards25519 v1.0.0 // indirect github.com/4meepo/tagalign v1.3.3 // indirect From 7727eee3d7e5ff74d85a5df112d599844445a247 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 11:00:04 -0500 Subject: [PATCH 02/55] add ante --- x/feemarket/ante/ante.go | 46 ++++++++++++++++++++++++++++ x/feemarket/ante/expected_keepers.go | 27 ++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 x/feemarket/ante/ante.go create mode 100644 x/feemarket/ante/expected_keepers.go diff --git a/x/feemarket/ante/ante.go b/x/feemarket/ante/ante.go new file mode 100644 index 0000000..a34b6ca --- /dev/null +++ b/x/feemarket/ante/ante.go @@ -0,0 +1,46 @@ +package ante + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + authante "github.com/cosmos/cosmos-sdk/x/auth/ante" +) + +// HandlerOptions are the options required for constructing an SDK AnteHandler with the fee market injected. +type HandlerOptions struct { + BaseOptions authante.HandlerOptions +} + +// NewAnteHandler returns an AnteHandler that checks and increments sequence +// numbers, checks signatures & account numbers, and deducts fees from the first +// signer. +func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { + if options.BaseOptions.AccountKeeper == nil { + return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "account keeper is required for ante builder") + } + + if options.BaseOptions.BankKeeper == nil { + return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "bank keeper is required for ante builder") + } + + if options.BaseOptions.SignModeHandler == nil { + return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for ante builder") + } + + anteDecorators := []sdk.AnteDecorator{ + authante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first + authante.NewExtensionOptionsDecorator(options.BaseOptions.ExtensionOptionChecker), + authante.NewValidateBasicDecorator(), + authante.NewTxTimeoutHeightDecorator(), + authante.NewValidateMemoDecorator(options.BaseOptions.AccountKeeper), + authante.NewConsumeGasForTxSizeDecorator(options.BaseOptions.AccountKeeper), + authante.NewDeductFeeDecorator(options.BaseOptions.AccountKeeper, options.BaseOptions.BankKeeper, options.BaseOptions.FeegrantKeeper, options.BaseOptions.TxFeeChecker), + authante.NewSetPubKeyDecorator(options.BaseOptions.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators + authante.NewValidateSigCountDecorator(options.BaseOptions.AccountKeeper), + authante.NewSigGasConsumeDecorator(options.BaseOptions.AccountKeeper, options.BaseOptions.SigGasConsumer), + authante.NewSigVerificationDecorator(options.BaseOptions.AccountKeeper, options.BaseOptions.SignModeHandler), + authante.NewIncrementSequenceDecorator(options.BaseOptions.AccountKeeper), + } + + return sdk.ChainAnteDecorators(anteDecorators...), nil +} diff --git a/x/feemarket/ante/expected_keepers.go b/x/feemarket/ante/expected_keepers.go new file mode 100644 index 0000000..bd31753 --- /dev/null +++ b/x/feemarket/ante/expected_keepers.go @@ -0,0 +1,27 @@ +package ante + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +// AccountKeeper defines the contract needed for AccountKeeper related APIs. +// Interface provides support to use non-sdk AccountKeeper for AnteHandler's decorators. +type AccountKeeper interface { + GetParams(ctx sdk.Context) (params types.Params) + GetAccount(ctx sdk.Context, addr sdk.AccAddress) types.AccountI + SetAccount(ctx sdk.Context, acc types.AccountI) + GetModuleAddress(moduleName string) sdk.AccAddress +} + +// FeegrantKeeper defines the expected feegrant keeper. +type FeegrantKeeper interface { + UseGrantedFees(ctx sdk.Context, granter, grantee sdk.AccAddress, fee sdk.Coins, msgs []sdk.Msg) error +} + +// BankKeeper defines the contract needed for supply related APIs (noalias) +type BankKeeper interface { + IsSendEnabledCoins(ctx sdk.Context, coins ...sdk.Coin) error + SendCoins(ctx sdk.Context, from, to sdk.AccAddress, amt sdk.Coins) error + SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error +} From 028f302af83b9cf9d7d59621c490383129b32069 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 11:00:12 -0500 Subject: [PATCH 03/55] set antehandler --- tests/simapp/app.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/simapp/app.go b/tests/simapp/app.go index 171d55a..73b40e7 100644 --- a/tests/simapp/app.go +++ b/tests/simapp/app.go @@ -4,6 +4,7 @@ package simapp import ( "encoding/json" + "github.com/cosmos/cosmos-sdk/x/auth/ante" "io" "os" "path/filepath" @@ -65,6 +66,8 @@ import ( "github.com/cosmos/cosmos-sdk/x/upgrade" upgradeclient "github.com/cosmos/cosmos-sdk/x/upgrade/client" upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper" + + feemarketante "github.com/skip-mev/feemarket/x/feemarket/ante" ) var ( @@ -252,6 +255,26 @@ func NewSimApp( app.App = appBuilder.Build(logger, db, traceStore, baseAppOptions...) + // ---------------------------------------------------------------------------- // + // ------------------------- Begin Custom Code -------------------------------- // + // ---------------------------------------------------------------------------- // + + handlerOptions := ante.HandlerOptions{ + AccountKeeper: app.AccountKeeper, + BankKeeper: app.BankKeeper, + FeegrantKeeper: app.FeeGrantKeeper, + SigGasConsumer: ante.DefaultSigVerificationGasConsumer, + SignModeHandler: app.txConfig.SignModeHandler(), + } + options := feemarketante.HandlerOptions{ + BaseOptions: handlerOptions, + } + anteHandler, err := feemarketante.NewAnteHandler(options) + if err != nil { + panic(err) + } + app.App.SetAnteHandler(anteHandler) + /**** Module Options ****/ app.ModuleManager.RegisterInvariants(app.CrisisKeeper) From 560d930a5698e3b44e767d612ce09f79da57c031 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 11:02:05 -0500 Subject: [PATCH 04/55] cute --- tests/simapp/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/simapp/app.go b/tests/simapp/app.go index 73b40e7..e7e378b 100644 --- a/tests/simapp/app.go +++ b/tests/simapp/app.go @@ -4,7 +4,6 @@ package simapp import ( "encoding/json" - "github.com/cosmos/cosmos-sdk/x/auth/ante" "io" "os" "path/filepath" @@ -25,6 +24,7 @@ import ( storetypes "github.com/cosmos/cosmos-sdk/store/types" "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/cosmos-sdk/x/auth" + "github.com/cosmos/cosmos-sdk/x/auth/ante" authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" _ "github.com/cosmos/cosmos-sdk/x/auth/tx/config" // import for side-effects "github.com/cosmos/cosmos-sdk/x/auth/vesting" From 5789e63730d9fb743879c2aff96fe1de3b482249 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 11:09:16 -0500 Subject: [PATCH 05/55] lint fix --- x/feemarket/ante/ante.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/x/feemarket/ante/ante.go b/x/feemarket/ante/ante.go index a34b6ca..7130bf4 100644 --- a/x/feemarket/ante/ante.go +++ b/x/feemarket/ante/ante.go @@ -1,6 +1,8 @@ package ante import ( + errorsmod "cosmossdk.io/errors" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" authante "github.com/cosmos/cosmos-sdk/x/auth/ante" @@ -16,15 +18,15 @@ type HandlerOptions struct { // signer. func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { if options.BaseOptions.AccountKeeper == nil { - return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "account keeper is required for ante builder") + return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "account keeper is required for ante builder") } if options.BaseOptions.BankKeeper == nil { - return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "bank keeper is required for ante builder") + return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "bank keeper is required for ante builder") } if options.BaseOptions.SignModeHandler == nil { - return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for ante builder") + return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for ante builder") } anteDecorators := []sdk.AnteDecorator{ From ed2437736a08b905ea9abe206400d1ad91d4fb78 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 11:21:24 -0500 Subject: [PATCH 06/55] setup and wire --- tests/simapp/app.go | 5 +- x/feemarket/ante/ante.go | 5 +- x/feemarket/ante/expected_keepers.go | 8 +- x/feemarket/ante/fee.go | 136 +++++++++++++++++++++++++++ x/feemarket/ante/validator_fee.go | 66 +++++++++++++ 5 files changed, 215 insertions(+), 5 deletions(-) create mode 100644 x/feemarket/ante/fee.go create mode 100644 x/feemarket/ante/validator_fee.go diff --git a/tests/simapp/app.go b/tests/simapp/app.go index 5e9149d..32bd226 100644 --- a/tests/simapp/app.go +++ b/tests/simapp/app.go @@ -142,7 +142,7 @@ type SimApp struct { GroupKeeper groupkeeper.Keeper NFTKeeper nftkeeper.Keeper ConsensusParamsKeeper consensuskeeper.Keeper - FeeMarketKeeper feemarketkeeper.Keeper + FeeMarketKeeper *feemarketkeeper.Keeper // simulation manager sm *module.SimulationManager @@ -272,7 +272,8 @@ func NewSimApp( SignModeHandler: app.txConfig.SignModeHandler(), } options := feemarketante.HandlerOptions{ - BaseOptions: handlerOptions, + BaseOptions: handlerOptions, + FeeMarketKeeper: app.FeeMarketKeeper, } anteHandler, err := feemarketante.NewAnteHandler(options) if err != nil { diff --git a/x/feemarket/ante/ante.go b/x/feemarket/ante/ante.go index 7130bf4..ac01c09 100644 --- a/x/feemarket/ante/ante.go +++ b/x/feemarket/ante/ante.go @@ -10,7 +10,8 @@ import ( // HandlerOptions are the options required for constructing an SDK AnteHandler with the fee market injected. type HandlerOptions struct { - BaseOptions authante.HandlerOptions + BaseOptions authante.HandlerOptions + FeeMarketKeeper FeeMarketKeeper } // NewAnteHandler returns an AnteHandler that checks and increments sequence @@ -36,7 +37,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { authante.NewTxTimeoutHeightDecorator(), authante.NewValidateMemoDecorator(options.BaseOptions.AccountKeeper), authante.NewConsumeGasForTxSizeDecorator(options.BaseOptions.AccountKeeper), - authante.NewDeductFeeDecorator(options.BaseOptions.AccountKeeper, options.BaseOptions.BankKeeper, options.BaseOptions.FeegrantKeeper, options.BaseOptions.TxFeeChecker), + NewDeductFeeDecorator(options.BaseOptions.AccountKeeper, options.BaseOptions.BankKeeper, options.BaseOptions.FeegrantKeeper, TxFeeChecker(options.BaseOptions.TxFeeChecker)), authante.NewSetPubKeyDecorator(options.BaseOptions.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators authante.NewValidateSigCountDecorator(options.BaseOptions.AccountKeeper), authante.NewSigGasConsumeDecorator(options.BaseOptions.AccountKeeper, options.BaseOptions.SigGasConsumer), diff --git a/x/feemarket/ante/expected_keepers.go b/x/feemarket/ante/expected_keepers.go index bd31753..04266e9 100644 --- a/x/feemarket/ante/expected_keepers.go +++ b/x/feemarket/ante/expected_keepers.go @@ -3,6 +3,7 @@ package ante import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth/types" + feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" ) // AccountKeeper defines the contract needed for AccountKeeper related APIs. @@ -19,9 +20,14 @@ type FeegrantKeeper interface { UseGrantedFees(ctx sdk.Context, granter, grantee sdk.AccAddress, fee sdk.Coins, msgs []sdk.Msg) error } -// BankKeeper defines the contract needed for supply related APIs (noalias) +// BankKeeper defines the contract needed for supply related APIs. type BankKeeper interface { IsSendEnabledCoins(ctx sdk.Context, coins ...sdk.Coin) error SendCoins(ctx sdk.Context, from, to sdk.AccAddress, amt sdk.Coins) error SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error } + +// FeeMarketKeeper defines the expected feemarket keeper. +type FeeMarketKeeper interface { + GetState(ctx sdk.Context) (feemarkettypes.State, error) +} diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go new file mode 100644 index 0000000..66b2ba3 --- /dev/null +++ b/x/feemarket/ante/fee.go @@ -0,0 +1,136 @@ +package ante + +import ( + "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +// TxFeeChecker check if the provided fee is enough and returns the effective fee and tx priority, +// the effective fee should be deducted later, and the priority should be returned in abci response. +type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) + +// DeductFeeDecorator deducts fees from the fee payer. The fee payer is the fee granter (if specified) or first signer of the tx. +// If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. +// Call next AnteHandler if fees successfully deducted. +// CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator +type DeductFeeDecorator struct { + accountKeeper AccountKeeper + bankKeeper types.BankKeeper + feegrantKeeper FeegrantKeeper + txFeeChecker TxFeeChecker +} + +func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, tfc TxFeeChecker) DeductFeeDecorator { + if tfc == nil { + tfc = checkTxFeeWithValidatorMinGasPrices + } + + return DeductFeeDecorator{ + accountKeeper: ak, + bankKeeper: bk, + feegrantKeeper: fk, + txFeeChecker: tfc, + } +} + +func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + feeTx, ok := tx.(sdk.FeeTx) + if !ok { + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + } + + if !simulate && ctx.BlockHeight() > 0 && feeTx.GetGas() == 0 { + return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") + } + + var ( + priority int64 + err error + ) + + fee := feeTx.GetFee() + if !simulate { + fee, priority, err = dfd.txFeeChecker(ctx, tx) + if err != nil { + return ctx, err + } + } + if err := dfd.checkDeductFee(ctx, tx, fee); err != nil { + return ctx, err + } + + newCtx := ctx.WithPriority(priority) + + return next(newCtx, tx, simulate) +} + +func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee sdk.Coins) error { + feeTx, ok := sdkTx.(sdk.FeeTx) + if !ok { + return sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + } + + if addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { + return fmt.Errorf("fee collector module account (%s) has not been set", types.FeeCollectorName) + } + + feePayer := feeTx.FeePayer() + feeGranter := feeTx.FeeGranter() + deductFeesFrom := feePayer + + // if feegranter set deduct fee from feegranter account. + // this works with only when feegrant enabled. + if feeGranter != nil { + if dfd.feegrantKeeper == nil { + return sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled") + } else if !feeGranter.Equals(feePayer) { + err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, sdkTx.GetMsgs()) + if err != nil { + return sdkerrors.Wrapf(err, "%s does not allow to pay fees for %s", feeGranter, feePayer) + } + } + + deductFeesFrom = feeGranter + } + + deductFeesFromAcc := dfd.accountKeeper.GetAccount(ctx, deductFeesFrom) + if deductFeesFromAcc == nil { + return sdkerrors.ErrUnknownAddress.Wrapf("fee payer address: %s does not exist", deductFeesFrom) + } + + // deduct the fees + if !fee.IsZero() { + err := DeductFees(dfd.bankKeeper, ctx, deductFeesFromAcc, fee) + if err != nil { + return err + } + } + + events := sdk.Events{ + sdk.NewEvent( + sdk.EventTypeTx, + sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()), + sdk.NewAttribute(sdk.AttributeKeyFeePayer, deductFeesFrom.String()), + ), + } + ctx.EventManager().EmitEvents(events) + + return nil +} + +// DeductFees deducts fees from the given account. +func DeductFees(bankKeeper types.BankKeeper, ctx sdk.Context, acc types.AccountI, fees sdk.Coins) error { + if !fees.IsValid() { + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) + } + + err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) + if err != nil { + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) + } + + return nil +} diff --git a/x/feemarket/ante/validator_fee.go b/x/feemarket/ante/validator_fee.go new file mode 100644 index 0000000..e8c865b --- /dev/null +++ b/x/feemarket/ante/validator_fee.go @@ -0,0 +1,66 @@ +package ante + +import ( + "math" + + sdkmath "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +// 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, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + } + + feeCoins := feeTx.GetFee() + gas := feeTx.GetGas() + + // 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() + if !minGasPrices.IsZero() { + requiredFees := make(sdk.Coins, len(minGasPrices)) + + // 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, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) + } + } + } + + priority := getTxPriority(feeCoins, int64(gas)) + return feeCoins, priority, nil +} + +// getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the gas price +// provided in a transaction. +// NOTE: This implementation should be used with a great consideration as it opens potential attack vectors +// where txs with multiple coins could not be prioritize as expected. +func getTxPriority(fee sdk.Coins, gas int64) int64 { + var priority int64 + for _, c := range fee { + p := int64(math.MaxInt64) + gasPrice := c.Amount.QuoRaw(gas) + if gasPrice.IsInt64() { + p = gasPrice.Int64() + } + if priority == 0 || p < priority { + priority = p + } + } + + return priority +} From 5aec66a956301631f534fdf43ab1abb29d537228 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 11:23:20 -0500 Subject: [PATCH 07/55] wire fmk --- x/feemarket/ante/ante.go | 8 +++++++- x/feemarket/ante/fee.go | 20 +++++++++++--------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/x/feemarket/ante/ante.go b/x/feemarket/ante/ante.go index ac01c09..0c06b21 100644 --- a/x/feemarket/ante/ante.go +++ b/x/feemarket/ante/ante.go @@ -37,7 +37,13 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { authante.NewTxTimeoutHeightDecorator(), authante.NewValidateMemoDecorator(options.BaseOptions.AccountKeeper), authante.NewConsumeGasForTxSizeDecorator(options.BaseOptions.AccountKeeper), - NewDeductFeeDecorator(options.BaseOptions.AccountKeeper, options.BaseOptions.BankKeeper, options.BaseOptions.FeegrantKeeper, TxFeeChecker(options.BaseOptions.TxFeeChecker)), + NewDeductFeeDecorator( + options.BaseOptions.AccountKeeper, + options.BaseOptions.BankKeeper, + options.BaseOptions.FeegrantKeeper, + options.FeeMarketKeeper, + TxFeeChecker(options.BaseOptions.TxFeeChecker), + ), authante.NewSetPubKeyDecorator(options.BaseOptions.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators authante.NewValidateSigCountDecorator(options.BaseOptions.AccountKeeper), authante.NewSigGasConsumeDecorator(options.BaseOptions.AccountKeeper, options.BaseOptions.SigGasConsumer), diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 66b2ba3..d276b0b 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -17,22 +17,24 @@ type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) // Call next AnteHandler if fees successfully deducted. // CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator type DeductFeeDecorator struct { - accountKeeper AccountKeeper - bankKeeper types.BankKeeper - feegrantKeeper FeegrantKeeper - txFeeChecker TxFeeChecker + accountKeeper AccountKeeper + bankKeeper types.BankKeeper + feegrantKeeper FeegrantKeeper + feemarketKeeper FeeMarketKeeper + txFeeChecker TxFeeChecker } -func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, tfc TxFeeChecker) DeductFeeDecorator { +func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, fmk FeeMarketKeeper, tfc TxFeeChecker) DeductFeeDecorator { if tfc == nil { tfc = checkTxFeeWithValidatorMinGasPrices } return DeductFeeDecorator{ - accountKeeper: ak, - bankKeeper: bk, - feegrantKeeper: fk, - txFeeChecker: tfc, + accountKeeper: ak, + bankKeeper: bk, + feegrantKeeper: fk, + feemarketKeeper: fmk, + txFeeChecker: tfc, } } From 5b1e5bf6358ea85f74e5a4ac8466e29d71c7696f Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 11:24:40 -0500 Subject: [PATCH 08/55] clean --- x/feemarket/ante/fee.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index d276b0b..585fc47 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -5,7 +5,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - "github.com/cosmos/cosmos-sdk/x/auth/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) // TxFeeChecker check if the provided fee is enough and returns the effective fee and tx priority, @@ -18,13 +18,13 @@ type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) // CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator type DeductFeeDecorator struct { accountKeeper AccountKeeper - bankKeeper types.BankKeeper + bankKeeper BankKeeper feegrantKeeper FeegrantKeeper feemarketKeeper FeeMarketKeeper txFeeChecker TxFeeChecker } -func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, fmk FeeMarketKeeper, tfc TxFeeChecker) DeductFeeDecorator { +func NewDeductFeeDecorator(ak AccountKeeper, bk BankKeeper, fk FeegrantKeeper, fmk FeeMarketKeeper, tfc TxFeeChecker) DeductFeeDecorator { if tfc == nil { tfc = checkTxFeeWithValidatorMinGasPrices } @@ -75,8 +75,8 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee return sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } - if addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { - return fmt.Errorf("fee collector module account (%s) has not been set", types.FeeCollectorName) + if addr := dfd.accountKeeper.GetModuleAddress(authtypes.FeeCollectorName); addr == nil { + return fmt.Errorf("fee collector module account (%s) has not been set", authtypes.FeeCollectorName) } feePayer := feeTx.FeePayer() @@ -124,12 +124,12 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee } // DeductFees deducts fees from the given account. -func DeductFees(bankKeeper types.BankKeeper, ctx sdk.Context, acc types.AccountI, fees sdk.Coins) error { +func DeductFees(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error { if !fees.IsValid() { return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) } - err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) + err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), authtypes.FeeCollectorName, fees) if err != nil { return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } From e2c15bff5b14bee56a6b3145104fbb0619b9e067 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 11:24:50 -0500 Subject: [PATCH 09/55] format --- x/feemarket/ante/expected_keepers.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/feemarket/ante/expected_keepers.go b/x/feemarket/ante/expected_keepers.go index 04266e9..aae1417 100644 --- a/x/feemarket/ante/expected_keepers.go +++ b/x/feemarket/ante/expected_keepers.go @@ -3,6 +3,7 @@ package ante import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth/types" + feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" ) From 12457ce93cbcc048542a0acef6861f724ea92780 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 11:25:12 -0500 Subject: [PATCH 10/55] todo --- x/feemarket/ante/fee.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 585fc47..c2e374d 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -48,6 +48,8 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } + // TODO refactor with new logic + var ( priority int64 err error From a76474c7a8fce3ab366a85abc9ed129854214905 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 11:29:28 -0500 Subject: [PATCH 11/55] lint fix --- x/feemarket/ante/fee.go | 14 ++++++++------ x/feemarket/ante/validator_fee.go | 6 ++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index c2e374d..6cf5bf0 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -3,6 +3,8 @@ package ante import ( "fmt" + errorsmod "cosmossdk.io/errors" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" @@ -41,11 +43,11 @@ func NewDeductFeeDecorator(ak AccountKeeper, bk BankKeeper, fk FeegrantKeeper, f func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { - return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } if !simulate && ctx.BlockHeight() > 0 && feeTx.GetGas() == 0 { - return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") + return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } // TODO refactor with new logic @@ -74,7 +76,7 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee sdk.Coins) error { feeTx, ok := sdkTx.(sdk.FeeTx) if !ok { - return sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } if addr := dfd.accountKeeper.GetModuleAddress(authtypes.FeeCollectorName); addr == nil { @@ -93,7 +95,7 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee } else if !feeGranter.Equals(feePayer) { err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, sdkTx.GetMsgs()) if err != nil { - return sdkerrors.Wrapf(err, "%s does not allow to pay fees for %s", feeGranter, feePayer) + return errorsmod.Wrapf(err, "%s does not allow to pay fees for %s", feeGranter, feePayer) } } @@ -128,12 +130,12 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee // DeductFees deducts fees from the given account. func DeductFees(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error { if !fees.IsValid() { - return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) } err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), authtypes.FeeCollectorName, fees) if err != nil { - return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } return nil diff --git a/x/feemarket/ante/validator_fee.go b/x/feemarket/ante/validator_fee.go index e8c865b..027151f 100644 --- a/x/feemarket/ante/validator_fee.go +++ b/x/feemarket/ante/validator_fee.go @@ -3,6 +3,8 @@ package ante import ( "math" + errorsmod "cosmossdk.io/errors" + sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -13,7 +15,7 @@ import ( func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { - return nil, 0, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + return nil, 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } feeCoins := feeTx.GetFee() @@ -36,7 +38,7 @@ func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, } if !feeCoins.IsAnyGTE(requiredFees) { - return nil, 0, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) + return nil, 0, errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) } } } From 402b9b1875b023533db4a7aa77a6d45a8e342ab5 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 11:32:17 -0500 Subject: [PATCH 12/55] rename --- x/feemarket/ante/ante.go | 2 +- x/feemarket/ante/fee.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/x/feemarket/ante/ante.go b/x/feemarket/ante/ante.go index 0c06b21..2c661a5 100644 --- a/x/feemarket/ante/ante.go +++ b/x/feemarket/ante/ante.go @@ -37,7 +37,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { authante.NewTxTimeoutHeightDecorator(), authante.NewValidateMemoDecorator(options.BaseOptions.AccountKeeper), authante.NewConsumeGasForTxSizeDecorator(options.BaseOptions.AccountKeeper), - NewDeductFeeDecorator( + NewFeeMarketDecorator( options.BaseOptions.AccountKeeper, options.BaseOptions.BankKeeper, options.BaseOptions.FeegrantKeeper, diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 6cf5bf0..c860738 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -18,7 +18,7 @@ type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) // If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. // Call next AnteHandler if fees successfully deducted. // CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator -type DeductFeeDecorator struct { +type FeeMarketDecorator struct { accountKeeper AccountKeeper bankKeeper BankKeeper feegrantKeeper FeegrantKeeper @@ -26,12 +26,12 @@ type DeductFeeDecorator struct { txFeeChecker TxFeeChecker } -func NewDeductFeeDecorator(ak AccountKeeper, bk BankKeeper, fk FeegrantKeeper, fmk FeeMarketKeeper, tfc TxFeeChecker) DeductFeeDecorator { +func NewFeeMarketDecorator(ak AccountKeeper, bk BankKeeper, fk FeegrantKeeper, fmk FeeMarketKeeper, tfc TxFeeChecker) FeeMarketDecorator { if tfc == nil { tfc = checkTxFeeWithValidatorMinGasPrices } - return DeductFeeDecorator{ + return FeeMarketDecorator{ accountKeeper: ak, bankKeeper: bk, feegrantKeeper: fk, @@ -40,7 +40,7 @@ func NewDeductFeeDecorator(ak AccountKeeper, bk BankKeeper, fk FeegrantKeeper, f } } -func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { +func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") @@ -73,7 +73,7 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return next(newCtx, tx, simulate) } -func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee sdk.Coins) error { +func (dfd FeeMarketDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee sdk.Coins) error { feeTx, ok := sdkTx.(sdk.FeeTx) if !ok { return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") From 2f464f408ff7863a984c92c9b8bbbfadc81ddf2e Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 12:11:06 -0500 Subject: [PATCH 13/55] wip --- x/feemarket/ante/expected_keepers.go | 1 + x/feemarket/ante/fee.go | 21 ++++++++++----------- x/feemarket/ante/validator_fee.go | 10 +++++----- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/x/feemarket/ante/expected_keepers.go b/x/feemarket/ante/expected_keepers.go index aae1417..7a75a79 100644 --- a/x/feemarket/ante/expected_keepers.go +++ b/x/feemarket/ante/expected_keepers.go @@ -31,4 +31,5 @@ type BankKeeper interface { // FeeMarketKeeper defines the expected feemarket keeper. type FeeMarketKeeper interface { GetState(ctx sdk.Context) (feemarkettypes.State, error) + GetMinGasPrices(ctx sdk.Context) (sdk.DecCoins, error) } diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index c860738..f2e3d0a 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -23,20 +23,14 @@ type FeeMarketDecorator struct { bankKeeper BankKeeper feegrantKeeper FeegrantKeeper feemarketKeeper FeeMarketKeeper - txFeeChecker TxFeeChecker } -func NewFeeMarketDecorator(ak AccountKeeper, bk BankKeeper, fk FeegrantKeeper, fmk FeeMarketKeeper, tfc TxFeeChecker) FeeMarketDecorator { - if tfc == nil { - tfc = checkTxFeeWithValidatorMinGasPrices - } - +func NewFeeMarketDecorator(ak AccountKeeper, bk BankKeeper, fk FeegrantKeeper, fmk FeeMarketKeeper) FeeMarketDecorator { return FeeMarketDecorator{ accountKeeper: ak, bankKeeper: bk, feegrantKeeper: fk, feemarketKeeper: fmk, - txFeeChecker: tfc, } } @@ -50,26 +44,31 @@ func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } - // TODO refactor with new logic - var ( priority int64 err error ) + minGasPrices, err := dfd.feemarketKeeper.GetMinGasPrices(ctx) + if err != nil { + return ctx, errorsmod.Wrapf(err, "unable to get fee market state") + } + + ctx = ctx.WithMinGasPrices(minGasPrice) + fee := feeTx.GetFee() if !simulate { - fee, priority, err = dfd.txFeeChecker(ctx, tx) + fee, priority, err = checkTxFees(ctx, minGasPrice, tx) if err != nil { return ctx, err } } + if err := dfd.checkDeductFee(ctx, tx, fee); err != nil { return ctx, err } newCtx := ctx.WithPriority(priority) - return next(newCtx, tx, simulate) } diff --git a/x/feemarket/ante/validator_fee.go b/x/feemarket/ante/validator_fee.go index 027151f..096ef3e 100644 --- a/x/feemarket/ante/validator_fee.go +++ b/x/feemarket/ante/validator_fee.go @@ -10,9 +10,9 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) -// 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) { +// checkTxFees implements the logic for the fee market to check if a Tx has provided suffucient +// fees given the current state of the fee market. Returns an error if insufficient fees. +func checkTxFees(ctx sdk.Context, fees sdk.DecCoins, 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") @@ -21,11 +21,11 @@ func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, feeCoins := feeTx.GetFee() gas := feeTx.GetGas() - // Ensure that the provided fees meet a minimum threshold for the validator, + // Ensure that the provided fees meet the minimum // 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() + minGasPrices := fees if !minGasPrices.IsZero() { requiredFees := make(sdk.Coins, len(minGasPrices)) From 9f4d21b2e97bcacae44ff08ee5bf933e35df659f Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 12:17:18 -0500 Subject: [PATCH 14/55] proto --- proto/feemarket/feemarket/v1/genesis.proto | 8 +- proto/feemarket/feemarket/v1/params.proto | 5 +- x/feemarket/types/genesis.pb.go | 17 ++-- x/feemarket/types/params.pb.go | 112 +++++++++++++++------ 4 files changed, 100 insertions(+), 42 deletions(-) diff --git a/proto/feemarket/feemarket/v1/genesis.proto b/proto/feemarket/feemarket/v1/genesis.proto index d13cd97..4b89195 100644 --- a/proto/feemarket/feemarket/v1/genesis.proto +++ b/proto/feemarket/feemarket/v1/genesis.proto @@ -45,11 +45,11 @@ message State { // Index is the index of the current block in the block utilization window. uint64 index = 4; - // MaxBlockUtilization is the maximum utilization of a given block. This is denominated - // in the number of gas units consumed per block. + // MaxBlockUtilization is the maximum utilization of a given block. This is + // denominated in the number of gas units consumed per block. uint64 max_block_utilization = 5; - // TargetBlockUtilization is the target utilization of a given block. This is denominated - // in the number of gas units consumed per block. + // TargetBlockUtilization is the target utilization of a given block. This is + // denominated in the number of gas units consumed per block. uint64 target_block_utilization = 6; } diff --git a/proto/feemarket/feemarket/v1/params.proto b/proto/feemarket/feemarket/v1/params.proto index 8b294f3..11bd39e 100644 --- a/proto/feemarket/feemarket/v1/params.proto +++ b/proto/feemarket/feemarket/v1/params.proto @@ -77,7 +77,10 @@ message Params { // over a moving window of blocks. uint64 window = 10; + // FeeDenom is the denom that will be used for all fee payments. + string fee_denom = 11; + // Enabled is a boolean that determines whether the EIP1559 fee market is // enabled. - bool enabled = 11; + bool enabled = 12; } diff --git a/x/feemarket/types/genesis.pb.go b/x/feemarket/types/genesis.pb.go index c7b038e..83bf3ed 100644 --- a/x/feemarket/types/genesis.pb.go +++ b/x/feemarket/types/genesis.pb.go @@ -86,21 +86,22 @@ func (m *GenesisState) GetState() State { // the current base fee, learning rate, and block utilization within the // specified AIMD window. type State struct { - // BaseFee is the current base fee. This is denominated in the fee per gas unit. + // BaseFee is the current base fee. This is denominated in the fee per gas + // unit. BaseFee cosmossdk_io_math.Int `protobuf:"bytes,1,opt,name=base_fee,json=baseFee,proto3,customtype=cosmossdk.io/math.Int" json:"base_fee"` // LearningRate is the current learning rate. LearningRate cosmossdk_io_math.LegacyDec `protobuf:"bytes,2,opt,name=learning_rate,json=learningRate,proto3,customtype=cosmossdk.io/math.LegacyDec" json:"learning_rate"` - // Window contains a list of the last blocks' utilization values. This is used to - // calculate the next base fee. This stores the number of units of gas consumed - // per block. + // Window contains a list of the last blocks' utilization values. This is used + // to calculate the next base fee. This stores the number of units of gas + // consumed per block. Window []uint64 `protobuf:"varint,3,rep,packed,name=window,proto3" json:"window,omitempty"` // Index is the index of the current block in the block utilization window. Index uint64 `protobuf:"varint,4,opt,name=index,proto3" json:"index,omitempty"` - // MaxBlockUtilization is the maximum utilization of a given block. This is denominated - // in the number of gas units consumed per block. + // MaxBlockUtilization is the maximum utilization of a given block. This is + // denominated in the number of gas units consumed per block. MaxBlockUtilization uint64 `protobuf:"varint,5,opt,name=max_block_utilization,json=maxBlockUtilization,proto3" json:"max_block_utilization,omitempty"` - // TargetBlockUtilization is the target utilization of a given block. This is denominated - // in the number of gas units consumed per block. + // TargetBlockUtilization is the target utilization of a given block. This is + // denominated in the number of gas units consumed per block. TargetBlockUtilization uint64 `protobuf:"varint,6,opt,name=target_block_utilization,json=targetBlockUtilization,proto3" json:"target_block_utilization,omitempty"` } diff --git a/x/feemarket/types/params.pb.go b/x/feemarket/types/params.pb.go index 7be2a5f..0809db5 100644 --- a/x/feemarket/types/params.pb.go +++ b/x/feemarket/types/params.pb.go @@ -58,9 +58,11 @@ type Params struct { // Window defines the window size for calculating an adaptive learning rate // over a moving window of blocks. Window uint64 `protobuf:"varint,10,opt,name=window,proto3" json:"window,omitempty"` + // FeeDenom is the denom that will be used for all fee payments. + FeeDenom string `protobuf:"bytes,11,opt,name=fee_denom,json=feeDenom,proto3" json:"fee_denom,omitempty"` // Enabled is a boolean that determines whether the EIP1559 fee market is // enabled. - Enabled bool `protobuf:"varint,11,opt,name=enabled,proto3" json:"enabled,omitempty"` + Enabled bool `protobuf:"varint,12,opt,name=enabled,proto3" json:"enabled,omitempty"` } func (m *Params) Reset() { *m = Params{} } @@ -117,6 +119,13 @@ func (m *Params) GetWindow() uint64 { return 0 } +func (m *Params) GetFeeDenom() string { + if m != nil { + return m.FeeDenom + } + return "" +} + func (m *Params) GetEnabled() bool { if m != nil { return m.Enabled @@ -133,34 +142,36 @@ func init() { } var fileDescriptor_3907de4df2e1c66e = []byte{ - // 430 bytes of a gzipped FileDescriptorProto + // 451 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x9c, 0xd3, 0x41, 0x8b, 0xd3, 0x40, - 0x14, 0x07, 0xf0, 0x46, 0xdb, 0xee, 0xee, 0x28, 0x88, 0xd1, 0x2d, 0xe3, 0x0a, 0xd9, 0xa2, 0x97, - 0x82, 0x6c, 0x42, 0xf5, 0xe2, 0xb9, 0xac, 0x4a, 0x61, 0x05, 0x09, 0x78, 0x11, 0x24, 0xbc, 0xa4, - 0x6f, 0xd3, 0xa1, 0x99, 0x99, 0x90, 0xbc, 0xed, 0x76, 0xfd, 0x14, 0x7e, 0x18, 0x3f, 0xc4, 0x1e, - 0x17, 0x2f, 0x8a, 0x87, 0x22, 0xed, 0x17, 0x91, 0xcc, 0x44, 0x5b, 0xb7, 0xb7, 0xdc, 0xde, 0x9b, - 0x37, 0xff, 0xdf, 0xbc, 0xcb, 0xb0, 0xe7, 0xe7, 0x88, 0x12, 0x8a, 0x19, 0x52, 0xb0, 0xa9, 0xe6, - 0xc3, 0x20, 0x87, 0x02, 0x64, 0xe9, 0xe7, 0x85, 0x26, 0xed, 0xf6, 0xfe, 0x8d, 0xfc, 0x4d, 0x35, - 0x1f, 0x1e, 0x3d, 0x49, 0x74, 0x29, 0x75, 0x19, 0x99, 0x5b, 0x81, 0x6d, 0x6c, 0xe4, 0xe8, 0x71, - 0xaa, 0x53, 0x6d, 0xcf, 0xab, 0xca, 0x9e, 0x3e, 0xfb, 0xd1, 0x61, 0xdd, 0x0f, 0x46, 0x76, 0xdf, - 0xb1, 0x0e, 0x64, 0xf9, 0x14, 0xb8, 0xd3, 0x77, 0x06, 0x07, 0xa3, 0xe1, 0xf5, 0xf2, 0xb8, 0xf5, - 0x6b, 0x79, 0xfc, 0xd4, 0x2a, 0xe5, 0x64, 0xe6, 0x0b, 0x1d, 0x48, 0xa0, 0xa9, 0x7f, 0x86, 0x29, - 0x24, 0x57, 0xa7, 0x98, 0x7c, 0xff, 0x76, 0xc2, 0xea, 0x47, 0x4e, 0x31, 0x09, 0x6d, 0xde, 0x7d, - 0xc3, 0xda, 0x31, 0x12, 0xf0, 0x3b, 0x4d, 0x1d, 0x13, 0xaf, 0xf6, 0xa1, 0x69, 0xe5, 0xdc, 0x6d, - 0xbc, 0x8f, 0xc9, 0x57, 0xd0, 0x04, 0x33, 0x02, 0xde, 0x6e, 0x0c, 0x99, 0xbc, 0xfb, 0x9e, 0xdd, - 0x97, 0x42, 0x45, 0x31, 0x94, 0x18, 0x9d, 0x23, 0xf2, 0x8e, 0xf1, 0x5e, 0xd4, 0xde, 0xe1, 0xae, - 0x37, 0x56, 0xb4, 0x25, 0x8d, 0x15, 0x85, 0x4c, 0x0a, 0x35, 0x82, 0x12, 0xdf, 0x22, 0xba, 0x9f, - 0xd9, 0xc3, 0x8a, 0xcb, 0x10, 0x0a, 0x25, 0x54, 0x1a, 0x15, 0x40, 0xc8, 0xbb, 0x4d, 0x77, 0x7c, - 0x20, 0x85, 0x3a, 0xab, 0xa9, 0x10, 0xc8, 0xf2, 0xb0, 0xb8, 0xc5, 0xef, 0x35, 0xe7, 0x61, 0xf1, - 0x1f, 0xff, 0x9a, 0x71, 0x82, 0x22, 0x45, 0x8a, 0xe2, 0x4c, 0x27, 0xb3, 0xe8, 0x82, 0x44, 0x26, - 0xbe, 0x00, 0x09, 0xad, 0xf8, 0x7e, 0xdf, 0x19, 0xb4, 0xc3, 0x9e, 0x9d, 0x8f, 0xaa, 0xf1, 0xc7, - 0xcd, 0xd4, 0x7d, 0xc9, 0x0e, 0xab, 0xc5, 0x76, 0x63, 0x07, 0x26, 0xf6, 0x48, 0xc2, 0x62, 0x27, - 0xd3, 0x63, 0xdd, 0x4b, 0xa1, 0x26, 0xfa, 0x92, 0x33, 0x73, 0xa9, 0xee, 0x5c, 0xce, 0xf6, 0x50, - 0x41, 0x9c, 0xe1, 0x84, 0xdf, 0xeb, 0x3b, 0x83, 0xfd, 0xf0, 0x6f, 0x3b, 0x1a, 0x5f, 0xaf, 0x3c, - 0xe7, 0x66, 0xe5, 0x39, 0xbf, 0x57, 0x9e, 0xf3, 0x75, 0xed, 0xb5, 0x6e, 0xd6, 0x5e, 0xeb, 0xe7, - 0xda, 0x6b, 0x7d, 0x0a, 0x52, 0x41, 0xd3, 0x8b, 0xd8, 0x4f, 0xb4, 0x0c, 0xca, 0x99, 0xc8, 0x4f, - 0x24, 0xce, 0xb7, 0xfe, 0xda, 0x62, 0xab, 0xa6, 0xab, 0x1c, 0xcb, 0xb8, 0x6b, 0xfe, 0xca, 0xab, - 0x3f, 0x01, 0x00, 0x00, 0xff, 0xff, 0xcb, 0xbf, 0x43, 0x86, 0x9b, 0x03, 0x00, 0x00, + 0x14, 0x07, 0xf0, 0x46, 0xdb, 0x6e, 0x3b, 0x2e, 0x88, 0xd1, 0x2d, 0xe3, 0x2e, 0x64, 0x8b, 0x5e, + 0x0a, 0xb2, 0x09, 0xd5, 0x8b, 0xe7, 0x52, 0x95, 0xc2, 0x0a, 0x12, 0xf0, 0x22, 0x48, 0x78, 0x49, + 0x5e, 0xd3, 0xa1, 0x99, 0x99, 0x90, 0xcc, 0x76, 0xbb, 0x7e, 0x0a, 0x3f, 0x8c, 0x1f, 0x62, 0x8f, + 0x8b, 0x27, 0xf1, 0xb0, 0x48, 0xfb, 0x15, 0xfc, 0x00, 0x32, 0x33, 0xd5, 0xd6, 0xed, 0x2d, 0xb7, + 0xf7, 0xe6, 0xcd, 0xff, 0x97, 0x97, 0xc3, 0x90, 0xe7, 0x53, 0x44, 0x0e, 0xe5, 0x1c, 0x55, 0xb0, + 0xad, 0x16, 0xc3, 0xa0, 0x80, 0x12, 0x78, 0xe5, 0x17, 0xa5, 0x54, 0xd2, 0xed, 0xfd, 0x1b, 0xf9, + 0xdb, 0x6a, 0x31, 0x3c, 0x7e, 0x9a, 0xc8, 0x8a, 0xcb, 0x2a, 0x32, 0xb7, 0x02, 0xdb, 0xd8, 0xc8, + 0xf1, 0x93, 0x4c, 0x66, 0xd2, 0x9e, 0xeb, 0xca, 0x9e, 0x3e, 0xfb, 0xdd, 0x22, 0xed, 0x0f, 0x46, + 0x76, 0xdf, 0x91, 0x16, 0xe4, 0xc5, 0x0c, 0xa8, 0xd3, 0x77, 0x06, 0xdd, 0xd1, 0xf0, 0xfa, 0xf6, + 0xb4, 0xf1, 0xf3, 0xf6, 0xf4, 0xc4, 0x2a, 0x55, 0x3a, 0xf7, 0x99, 0x0c, 0x38, 0xa8, 0x99, 0x7f, + 0x8e, 0x19, 0x24, 0x57, 0x63, 0x4c, 0xbe, 0x7f, 0x3b, 0x23, 0x9b, 0x8f, 0x8c, 0x31, 0x09, 0x6d, + 0xde, 0x7d, 0x43, 0x9a, 0x31, 0x2a, 0xa0, 0xf7, 0xea, 0x3a, 0x26, 0xae, 0xf7, 0x51, 0x33, 0xed, + 0xdc, 0xaf, 0xbd, 0x8f, 0xc9, 0x6b, 0x28, 0xc5, 0x5c, 0x01, 0x6d, 0xd6, 0x86, 0x4c, 0xde, 0x7d, + 0x4f, 0x0e, 0x39, 0x13, 0x51, 0x0c, 0x15, 0x46, 0x53, 0x44, 0xda, 0x32, 0xde, 0x8b, 0x8d, 0x77, + 0xb4, 0xef, 0x4d, 0x84, 0xda, 0x91, 0x26, 0x42, 0x85, 0x84, 0x33, 0x31, 0x82, 0x0a, 0xdf, 0x22, + 0xba, 0x9f, 0xc9, 0x23, 0xcd, 0xe5, 0x08, 0xa5, 0x60, 0x22, 0x8b, 0x4a, 0x50, 0x48, 0xdb, 0x75, + 0x77, 0x7c, 0xc8, 0x99, 0x38, 0xdf, 0x50, 0x21, 0x28, 0xcb, 0xc3, 0xf2, 0x0e, 0x7f, 0x50, 0x9f, + 0x87, 0xe5, 0x7f, 0xfc, 0x6b, 0x42, 0x15, 0x94, 0x19, 0xaa, 0x28, 0xce, 0x65, 0x32, 0x8f, 0x2e, + 0x14, 0xcb, 0xd9, 0x17, 0x50, 0x4c, 0x0a, 0xda, 0xe9, 0x3b, 0x83, 0x66, 0xd8, 0xb3, 0xf3, 0x91, + 0x1e, 0x7f, 0xdc, 0x4e, 0xdd, 0x97, 0xe4, 0x48, 0x2f, 0xb6, 0x1f, 0xeb, 0x9a, 0xd8, 0x63, 0x0e, + 0xcb, 0xbd, 0x4c, 0x8f, 0xb4, 0x2f, 0x99, 0x48, 0xe5, 0x25, 0x25, 0xe6, 0xd2, 0xa6, 0x73, 0x4f, + 0x48, 0x77, 0x8a, 0x18, 0xa5, 0x28, 0x24, 0xa7, 0x0f, 0xf4, 0xcf, 0x85, 0x9d, 0x29, 0xe2, 0x58, + 0xf7, 0x2e, 0x25, 0x07, 0x28, 0x20, 0xce, 0x31, 0xa5, 0x87, 0x7d, 0x67, 0xd0, 0x09, 0xff, 0xb6, + 0xa3, 0xc9, 0xf5, 0xca, 0x73, 0x6e, 0x56, 0x9e, 0xf3, 0x6b, 0xe5, 0x39, 0x5f, 0xd7, 0x5e, 0xe3, + 0x66, 0xed, 0x35, 0x7e, 0xac, 0xbd, 0xc6, 0xa7, 0x20, 0x63, 0x6a, 0x76, 0x11, 0xfb, 0x89, 0xe4, + 0x41, 0x35, 0x67, 0xc5, 0x19, 0xc7, 0xc5, 0xce, 0x43, 0x5c, 0xee, 0xd4, 0xea, 0xaa, 0xc0, 0x2a, + 0x6e, 0x9b, 0x87, 0xf4, 0xea, 0x4f, 0x00, 0x00, 0x00, 0xff, 0xff, 0x7e, 0xef, 0x08, 0x09, 0xb8, + 0x03, 0x00, 0x00, } func (m *Params) Marshal() (dAtA []byte, err error) { @@ -191,7 +202,14 @@ func (m *Params) MarshalToSizedBuffer(dAtA []byte) (int, error) { dAtA[i] = 0 } i-- - dAtA[i] = 0x58 + dAtA[i] = 0x60 + } + if len(m.FeeDenom) > 0 { + i -= len(m.FeeDenom) + copy(dAtA[i:], m.FeeDenom) + i = encodeVarintParams(dAtA, i, uint64(len(m.FeeDenom))) + i-- + dAtA[i] = 0x5a } if m.Window != 0 { i = encodeVarintParams(dAtA, i, uint64(m.Window)) @@ -321,6 +339,10 @@ func (m *Params) Size() (n int) { if m.Window != 0 { n += 1 + sovParams(uint64(m.Window)) } + l = len(m.FeeDenom) + if l > 0 { + n += 1 + l + sovParams(uint64(l)) + } if m.Enabled { n += 2 } @@ -658,6 +680,38 @@ func (m *Params) Unmarshal(dAtA []byte) error { } } case 11: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field FeeDenom", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowParams + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthParams + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthParams + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.FeeDenom = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + case 12: if wireType != 0 { return fmt.Errorf("proto: wrong wireType = %d for field Enabled", wireType) } From 364aaa1367484f394307e4dd5c39ffa5aea2e7e2 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 12:25:20 -0500 Subject: [PATCH 15/55] add param --- x/feemarket/types/eip1559.go | 23 +++--- x/feemarket/types/eip1559_aimd.go | 18 +++-- x/feemarket/types/params.go | 6 ++ x/feemarket/types/params_test.go | 112 ++++++++++++++++++++---------- 4 files changed, 107 insertions(+), 52 deletions(-) diff --git a/x/feemarket/types/eip1559.go b/x/feemarket/types/eip1559.go index 40ed3ed..b2a2360 100644 --- a/x/feemarket/types/eip1559.go +++ b/x/feemarket/types/eip1559.go @@ -1,6 +1,9 @@ package types -import "cosmossdk.io/math" +import ( + "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" +) // Note: We use the same default values as Ethereum for the EIP-1559 // fee market implementation. These parameters do not implement the @@ -13,16 +16,16 @@ var ( DefaultWindow uint64 = 1 // DefaultAlpha is not used in the base EIP-1559 implementation. - DefaultAlpha math.LegacyDec = math.LegacyMustNewDecFromStr("0.0") + DefaultAlpha = math.LegacyMustNewDecFromStr("0.0") // DefaultBeta is not used in the base EIP-1559 implementation. - DefaultBeta math.LegacyDec = math.LegacyMustNewDecFromStr("1.0") + DefaultBeta = math.LegacyMustNewDecFromStr("1.0") // DefaultTheta is not used in the base EIP-1559 implementation. - DefaultTheta math.LegacyDec = math.LegacyMustNewDecFromStr("0.0") + DefaultTheta = math.LegacyMustNewDecFromStr("0.0") // DefaultDelta is not used in the base EIP-1559 implementation. - DefaultDelta math.LegacyDec = math.LegacyMustNewDecFromStr("0.0") + DefaultDelta = math.LegacyMustNewDecFromStr("0.0") // DefaultTargetBlockSize is the default target block utilization. This is the default // on Ethereum. This denominated in units of gas consumed in a block. @@ -35,13 +38,16 @@ var ( // DefaultMinBaseFee is the default minimum base fee. This is the default // on Ethereum. Note that Ethereum is denominated in 1e18 wei. Cosmos chains will // likely want to change this to 1e6. - DefaultMinBaseFee math.Int = math.NewInt(1_000_000_000) + DefaultMinBaseFee = math.NewInt(1_000_000_000) // DefaultMinLearningRate is not used in the base EIP-1559 implementation. - DefaultMinLearningRate math.LegacyDec = math.LegacyMustNewDecFromStr("0.125") + DefaultMinLearningRate = math.LegacyMustNewDecFromStr("0.125") // DefaultMaxLearningRate is not used in the base EIP-1559 implementation. - DefaultMaxLearningRate math.LegacyDec = math.LegacyMustNewDecFromStr("0.125") + DefaultMaxLearningRate = math.LegacyMustNewDecFromStr("0.125") + + // DefaultFeeDenom is the Cosmos SDK default bond denom. + DefaultFeeDenom = sdk.DefaultBondDenom ) // DefaultParams returns a default set of parameters that implements @@ -59,6 +65,7 @@ func DefaultParams() Params { DefaultMinBaseFee, DefaultMinLearningRate, DefaultMaxLearningRate, + DefaultFeeDenom, true, ) } diff --git a/x/feemarket/types/eip1559_aimd.go b/x/feemarket/types/eip1559_aimd.go index 4153739..8cd102d 100644 --- a/x/feemarket/types/eip1559_aimd.go +++ b/x/feemarket/types/eip1559_aimd.go @@ -14,24 +14,24 @@ var ( // DefaultAIMDAlpha is the default alpha value for the learning // rate calculation. This value determines how much we want to additively // increase the learning rate when the target block size is exceeded. - DefaultAIMDAlpha math.LegacyDec = math.LegacyMustNewDecFromStr("0.025") + DefaultAIMDAlpha = math.LegacyMustNewDecFromStr("0.025") // DefaultAIMDBeta is the default beta value for the learning rate // calculation. This value determines how much we want to multiplicatively // decrease the learning rate when the target utilization is not met. - DefaultAIMDBeta math.LegacyDec = math.LegacyMustNewDecFromStr("0.95") + DefaultAIMDBeta = math.LegacyMustNewDecFromStr("0.95") // DefaultAIMDTheta is the default threshold for determining whether // to increase or decrease the learning rate. In this case, we increase // the learning rate if the block utilization within the window is greater // than 0.75 or less than 0.25. Otherwise, we multiplicatively decrease // the learning rate. - DefaultAIMDTheta math.LegacyDec = math.LegacyMustNewDecFromStr("0.25") + DefaultAIMDTheta = math.LegacyMustNewDecFromStr("0.25") // DefaultAIMDDelta is the default delta value for how much we additively // increase or decrease the base fee when the net block utilization within // the window is not equal to the target utilization. - DefaultAIMDDelta math.LegacyDec = math.LegacyMustNewDecFromStr("0.0") + DefaultAIMDDelta = math.LegacyMustNewDecFromStr("0.0") // DefaultAIMDTargetBlockSize is the default target block utilization. This // is the default on Ethereum. This denominated in units of gas consumed in @@ -46,13 +46,16 @@ var ( // DefaultAIMDMinBaseFee is the default minimum base fee. This is the // default on Ethereum. Note that ethereum is denominated in 1e18 wei. // Cosmos chains will likely want to change this to 1e6. - DefaultAIMDMinBaseFee math.Int = math.NewInt(1_000_000_000) + DefaultAIMDMinBaseFee = math.NewInt(1_000_000_000) // DefaultAIMDMinLearningRate is the default minimum learning rate. - DefaultAIMDMinLearningRate math.LegacyDec = math.LegacyMustNewDecFromStr("0.01") + DefaultAIMDMinLearningRate = math.LegacyMustNewDecFromStr("0.01") // DefaultAIMDMaxLearningRate is the default maximum learning rate. - DefaultAIMDMaxLearningRate math.LegacyDec = math.LegacyMustNewDecFromStr("0.50") + DefaultAIMDMaxLearningRate = math.LegacyMustNewDecFromStr("0.50") + + // DefaultAIMDFeeDenom is the Cosmos SDK default bond denom. + DefaultAIMDFeeDenom = DefaultFeeDenom ) // DefaultAIMDParams returns a default set of parameters that implements @@ -71,6 +74,7 @@ func DefaultAIMDParams() Params { DefaultAIMDMinBaseFee, DefaultAIMDMinLearningRate, DefaultAIMDMaxLearningRate, + DefaultAIMDFeeDenom, true, ) } diff --git a/x/feemarket/types/params.go b/x/feemarket/types/params.go index a00f438..4e2d41c 100644 --- a/x/feemarket/types/params.go +++ b/x/feemarket/types/params.go @@ -19,6 +19,7 @@ func NewParams( minBaseFee math.Int, minLearingRate math.LegacyDec, maxLearningRate math.LegacyDec, + feeDenom string, enabled bool, ) Params { return Params{ @@ -32,6 +33,7 @@ func NewParams( TargetBlockUtilization: targetBlockSize, MaxBlockUtilization: maxBlockSize, Window: window, + FeeDenom: feeDenom, Enabled: enabled, } } @@ -86,5 +88,9 @@ func (p *Params) ValidateBasic() error { return fmt.Errorf("min learning rate cannot be greater than max learning rate") } + if p.FeeDenom == "" { + return fmt.Errorf("fee denom must be set") + } + return nil } diff --git a/x/feemarket/types/params_test.go b/x/feemarket/types/params_test.go index 2c44884..a52c322 100644 --- a/x/feemarket/types/params_test.go +++ b/x/feemarket/types/params_test.go @@ -31,102 +31,115 @@ func TestParams(t *testing.T) { expectedErr: true, }, { - name: "nil alpha", - p: types.Params{Window: 1}, + name: "nil alpha", + p: types.Params{ + Window: 1, + FeeDenom: types.DefaultFeeDenom, + }, expectedErr: true, }, { name: "negative alpha", p: types.Params{ - Window: 1, - Alpha: math.LegacyMustNewDecFromStr("-0.1"), + Window: 1, + Alpha: math.LegacyMustNewDecFromStr("-0.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, { name: "beta is nil", p: types.Params{ - Window: 1, - Alpha: math.LegacyMustNewDecFromStr("0.1"), + Window: 1, + Alpha: math.LegacyMustNewDecFromStr("0.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, { name: "beta is negative", p: types.Params{ - Window: 1, - Alpha: math.LegacyMustNewDecFromStr("0.1"), - Beta: math.LegacyMustNewDecFromStr("-0.1"), + Window: 1, + Alpha: math.LegacyMustNewDecFromStr("0.1"), + Beta: math.LegacyMustNewDecFromStr("-0.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, { name: "beta is greater than 1", p: types.Params{ - Window: 1, - Alpha: math.LegacyMustNewDecFromStr("0.1"), - Beta: math.LegacyMustNewDecFromStr("1.1"), + Window: 1, + Alpha: math.LegacyMustNewDecFromStr("0.1"), + Beta: math.LegacyMustNewDecFromStr("1.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, { name: "theta is nil", p: types.Params{ - Window: 1, - Alpha: math.LegacyMustNewDecFromStr("0.1"), - Beta: math.LegacyMustNewDecFromStr("0.1"), + Window: 1, + Alpha: math.LegacyMustNewDecFromStr("0.1"), + Beta: math.LegacyMustNewDecFromStr("0.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, { name: "theta is negative", p: types.Params{ - Window: 1, - Alpha: math.LegacyMustNewDecFromStr("0.1"), - Beta: math.LegacyMustNewDecFromStr("0.1"), - Theta: math.LegacyMustNewDecFromStr("-0.1"), + Window: 1, + Alpha: math.LegacyMustNewDecFromStr("0.1"), + Beta: math.LegacyMustNewDecFromStr("0.1"), + Theta: math.LegacyMustNewDecFromStr("-0.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, { name: "theta is greater than 1", p: types.Params{ - Window: 1, - Alpha: math.LegacyMustNewDecFromStr("0.1"), - Beta: math.LegacyMustNewDecFromStr("0.1"), - Theta: math.LegacyMustNewDecFromStr("1.1"), + Window: 1, + Alpha: math.LegacyMustNewDecFromStr("0.1"), + Beta: math.LegacyMustNewDecFromStr("0.1"), + Theta: math.LegacyMustNewDecFromStr("1.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, { name: "delta is nil", p: types.Params{ - Window: 1, - Alpha: math.LegacyMustNewDecFromStr("0.1"), - Beta: math.LegacyMustNewDecFromStr("0.1"), - Theta: math.LegacyMustNewDecFromStr("0.1"), + Window: 1, + Alpha: math.LegacyMustNewDecFromStr("0.1"), + Beta: math.LegacyMustNewDecFromStr("0.1"), + Theta: math.LegacyMustNewDecFromStr("0.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, { name: "delta is negative", p: types.Params{ - Window: 1, - Alpha: math.LegacyMustNewDecFromStr("0.1"), - Beta: math.LegacyMustNewDecFromStr("0.1"), - Theta: math.LegacyMustNewDecFromStr("0.1"), - Delta: math.LegacyMustNewDecFromStr("-0.1"), + Window: 1, + Alpha: math.LegacyMustNewDecFromStr("0.1"), + Beta: math.LegacyMustNewDecFromStr("0.1"), + Theta: math.LegacyMustNewDecFromStr("0.1"), + Delta: math.LegacyMustNewDecFromStr("-0.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, { name: "target block size is zero", p: types.Params{ - Window: 1, - Alpha: math.LegacyMustNewDecFromStr("0.1"), - Beta: math.LegacyMustNewDecFromStr("0.1"), - Theta: math.LegacyMustNewDecFromStr("0.1"), - Delta: math.LegacyMustNewDecFromStr("0.1"), + Window: 1, + Alpha: math.LegacyMustNewDecFromStr("0.1"), + Beta: math.LegacyMustNewDecFromStr("0.1"), + Theta: math.LegacyMustNewDecFromStr("0.1"), + Delta: math.LegacyMustNewDecFromStr("0.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, @@ -139,6 +152,7 @@ func TestParams(t *testing.T) { Theta: math.LegacyMustNewDecFromStr("0.1"), Delta: math.LegacyMustNewDecFromStr("0.1"), TargetBlockUtilization: 1, + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, @@ -152,6 +166,7 @@ func TestParams(t *testing.T) { Delta: math.LegacyMustNewDecFromStr("0.1"), TargetBlockUtilization: 2, MaxBlockUtilization: 1, + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, @@ -165,6 +180,7 @@ func TestParams(t *testing.T) { Delta: math.LegacyMustNewDecFromStr("0.1"), TargetBlockUtilization: 2, MaxBlockUtilization: 3, + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, @@ -179,6 +195,7 @@ func TestParams(t *testing.T) { TargetBlockUtilization: 2, MaxBlockUtilization: 3, MinBaseFee: math.NewInt(-1), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, @@ -193,6 +210,7 @@ func TestParams(t *testing.T) { TargetBlockUtilization: 2, MaxBlockUtilization: 3, MinBaseFee: math.NewInt(1), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, @@ -208,6 +226,7 @@ func TestParams(t *testing.T) { MaxBlockUtilization: 3, MinBaseFee: math.NewInt(1), MinLearningRate: math.LegacyMustNewDecFromStr("-0.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, @@ -223,6 +242,7 @@ func TestParams(t *testing.T) { MaxBlockUtilization: 3, MinBaseFee: math.NewInt(1), MinLearningRate: math.LegacyMustNewDecFromStr("0.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, @@ -239,6 +259,7 @@ func TestParams(t *testing.T) { MinBaseFee: math.NewInt(1), MinLearningRate: math.LegacyMustNewDecFromStr("0.1"), MaxLearningRate: math.LegacyMustNewDecFromStr("-0.1"), + FeeDenom: types.DefaultFeeDenom, }, expectedErr: true, }, @@ -255,6 +276,23 @@ func TestParams(t *testing.T) { MinBaseFee: math.NewInt(1), MinLearningRate: math.LegacyMustNewDecFromStr("0.1"), MaxLearningRate: math.LegacyMustNewDecFromStr("0.05"), + FeeDenom: types.DefaultFeeDenom, + }, + expectedErr: true, + }, + { + name: "fee denom is empty", + p: types.Params{ + Window: 1, + Alpha: math.LegacyMustNewDecFromStr("0.1"), + Beta: math.LegacyMustNewDecFromStr("0.1"), + Theta: math.LegacyMustNewDecFromStr("0.1"), + Delta: math.LegacyMustNewDecFromStr("0.1"), + TargetBlockUtilization: 2, + MaxBlockUtilization: 3, + MinBaseFee: math.NewInt(1), + MinLearningRate: math.LegacyMustNewDecFromStr("0.01"), + MaxLearningRate: math.LegacyMustNewDecFromStr("0.05"), }, expectedErr: true, }, From 018da67ed1d0db100eb6bf22b58806cad5185268 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 12:27:42 -0500 Subject: [PATCH 16/55] add get --- x/feemarket/keeper/feemarket.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/x/feemarket/keeper/feemarket.go b/x/feemarket/keeper/feemarket.go index e973736..7c3feac 100644 --- a/x/feemarket/keeper/feemarket.go +++ b/x/feemarket/keeper/feemarket.go @@ -71,3 +71,21 @@ func (k *Keeper) GetLearningRate(ctx sdk.Context) (math.LegacyDec, error) { return state.LearningRate, nil } + +// GetMinGasPrices returns the mininum gas prices as sdk.Coins from the fee market state. +func (k *Keeper) GetMinGasPrices(ctx sdk.Context) (sdk.Coins, error) { + baseFee, err := k.GetBaseFee(ctx) + if err != nil { + return sdk.NewCoins(), err + } + + params, err := k.GetParams(ctx) + if err != nil { + return sdk.NewCoins(), err + } + + fee := sdk.NewCoin(params.FeeDenom, baseFee) + minGasPrices := sdk.NewCoins(fee) + + return minGasPrices, nil +} From 81ed0fbb22a60ba4f007a469eac9cfdc4996e7f0 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 12:29:27 -0500 Subject: [PATCH 17/55] test --- x/feemarket/keeper/feemarket_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/x/feemarket/keeper/feemarket_test.go b/x/feemarket/keeper/feemarket_test.go index 5fc0298..f42943d 100644 --- a/x/feemarket/keeper/feemarket_test.go +++ b/x/feemarket/keeper/feemarket_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/skip-mev/feemarket/x/feemarket/types" ) @@ -47,3 +48,27 @@ func (s *KeeperTestSuite) TestGetLearningRate() { s.Require().Equal(lr, gs.State.LearningRate) }) } + +func (s *KeeperTestSuite) TestGetMinGasPrices() { + s.Run("can retrieve min gas prices with default eip-1559", func() { + gs := types.DefaultGenesisState() + s.feemarketKeeper.InitGenesis(s.ctx, *gs) + + expected := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, gs.State.BaseFee)) + + mgp, err := s.feemarketKeeper.GetMinGasPrices(s.ctx) + s.Require().NoError(err) + s.Require().Equal(expected, mgp) + }) + + s.Run("can retrieve min gas prices with aimd eip-1559", func() { + gs := types.DefaultAIMDGenesisState() + s.feemarketKeeper.InitGenesis(s.ctx, *gs) + + expected := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, gs.State.BaseFee)) + + mgp, err := s.feemarketKeeper.GetMinGasPrices(s.ctx) + s.Require().NoError(err) + s.Require().Equal(expected, mgp) + }) +} From 7549ced8304ba4ff33e20513ed1023f013e7c675 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 12:30:47 -0500 Subject: [PATCH 18/55] format --- x/feemarket/keeper/feemarket_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/feemarket/keeper/feemarket_test.go b/x/feemarket/keeper/feemarket_test.go index f42943d..ec074c5 100644 --- a/x/feemarket/keeper/feemarket_test.go +++ b/x/feemarket/keeper/feemarket_test.go @@ -2,6 +2,7 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/skip-mev/feemarket/x/feemarket/types" ) From cb00ab4b9e5395549ab8c20d76e38bbc5064594c Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 12:41:03 -0500 Subject: [PATCH 19/55] re-wire --- x/feemarket/ante/ante.go | 1 - x/feemarket/ante/expected_keepers.go | 2 +- x/feemarket/ante/fee.go | 5 +++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x/feemarket/ante/ante.go b/x/feemarket/ante/ante.go index 2c661a5..ea86fc8 100644 --- a/x/feemarket/ante/ante.go +++ b/x/feemarket/ante/ante.go @@ -42,7 +42,6 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { options.BaseOptions.BankKeeper, options.BaseOptions.FeegrantKeeper, options.FeeMarketKeeper, - TxFeeChecker(options.BaseOptions.TxFeeChecker), ), authante.NewSetPubKeyDecorator(options.BaseOptions.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators authante.NewValidateSigCountDecorator(options.BaseOptions.AccountKeeper), diff --git a/x/feemarket/ante/expected_keepers.go b/x/feemarket/ante/expected_keepers.go index 7a75a79..6fc7fe6 100644 --- a/x/feemarket/ante/expected_keepers.go +++ b/x/feemarket/ante/expected_keepers.go @@ -31,5 +31,5 @@ type BankKeeper interface { // FeeMarketKeeper defines the expected feemarket keeper. type FeeMarketKeeper interface { GetState(ctx sdk.Context) (feemarkettypes.State, error) - GetMinGasPrices(ctx sdk.Context) (sdk.DecCoins, error) + GetMinGasPrices(ctx sdk.Context) (sdk.Coins, error) } diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index f2e3d0a..4a18a53 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -54,11 +54,12 @@ func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return ctx, errorsmod.Wrapf(err, "unable to get fee market state") } - ctx = ctx.WithMinGasPrices(minGasPrice) + minGasPricesDec := sdk.NewDecCoinsFromCoins(minGasPrices...) + ctx = ctx.WithMinGasPrices(minGasPricesDec) fee := feeTx.GetFee() if !simulate { - fee, priority, err = checkTxFees(ctx, minGasPrice, tx) + fee, priority, err = checkTxFees(ctx, minGasPricesDec, tx) if err != nil { return ctx, err } From 36ebe05bd237ceb445d66f57cc7f276b8dafaefd Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 12:53:54 -0500 Subject: [PATCH 20/55] add ak --- x/feemarket/ante/fee.go | 3 +-- x/feemarket/keeper/keeper.go | 14 ++++++++++++++ x/feemarket/types/expected_keepers.go | 14 ++++++++++++++ x/feemarket/types/keys.go | 3 +++ 4 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 x/feemarket/types/expected_keepers.go diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 4a18a53..6cd535b 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -55,7 +55,6 @@ func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo } minGasPricesDec := sdk.NewDecCoinsFromCoins(minGasPrices...) - ctx = ctx.WithMinGasPrices(minGasPricesDec) fee := feeTx.GetFee() if !simulate { @@ -69,7 +68,7 @@ func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return ctx, err } - newCtx := ctx.WithPriority(priority) + newCtx := ctx.WithPriority(priority).WithMinGasPrices(minGasPricesDec) return next(newCtx, tx, simulate) } diff --git a/x/feemarket/keeper/keeper.go b/x/feemarket/keeper/keeper.go index fb59bcc..90a8cb7 100644 --- a/x/feemarket/keeper/keeper.go +++ b/x/feemarket/keeper/keeper.go @@ -1,6 +1,8 @@ package keeper import ( + "fmt" + "github.com/cometbft/cometbft/libs/log" "github.com/cosmos/cosmos-sdk/codec" storetypes "github.com/cosmos/cosmos-sdk/store/types" @@ -12,6 +14,7 @@ import ( type Keeper struct { cdc codec.BinaryCodec storeKey storetypes.StoreKey + ak types.AccountKeeper // The address that is capable of executing a MsgParams message. // Typically, this will be the governance module's address. @@ -22,11 +25,22 @@ type Keeper struct { func NewKeeper( cdc codec.BinaryCodec, storeKey storetypes.StoreKey, + authKeeper types.AccountKeeper, authority string, ) *Keeper { + // ensure governance module account is set + if addr := authKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { + panic(fmt.Sprintf("%s module account has not been set", types.ModuleName)) + } + + if _, err := sdk.AccAddressFromBech32(authority); err != nil { + panic(fmt.Sprintf("invalid authority address: %s", authority)) + } + k := &Keeper{ cdc, storeKey, + authKeeper, authority, } diff --git a/x/feemarket/types/expected_keepers.go b/x/feemarket/types/expected_keepers.go new file mode 100644 index 0000000..0f9f506 --- /dev/null +++ b/x/feemarket/types/expected_keepers.go @@ -0,0 +1,14 @@ +package types + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +// AccountKeeper defines the expected account keeper (noalias) +type AccountKeeper interface { + GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI + + GetModuleAddress(name string) sdk.AccAddress + GetModuleAccount(ctx sdk.Context, name string) authtypes.ModuleAccountI +} diff --git a/x/feemarket/types/keys.go b/x/feemarket/types/keys.go index a0cc483..6bf6abd 100644 --- a/x/feemarket/types/keys.go +++ b/x/feemarket/types/keys.go @@ -5,6 +5,9 @@ const ( ModuleName = "feemarket" // StoreKey is the store key string for the feemarket module. StoreKey = ModuleName + + // FeeCollectorName the root string for the fee market fee collector account address. + FeeCollectorName = "feemarket-fee-collector" ) const ( From 099fc86cadee7ac499264a570aaee26cb80056c2 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 13:30:11 -0500 Subject: [PATCH 21/55] fee decorator --- tests/simapp/config.go | 2 + x/feemarket/ante/fee.go | 75 +++++++++++++++++++++++++++---- x/feemarket/ante/fee_test.go | 3 ++ x/feemarket/ante/validator_fee.go | 68 ---------------------------- 4 files changed, 72 insertions(+), 76 deletions(-) create mode 100644 x/feemarket/ante/fee_test.go delete mode 100644 x/feemarket/ante/validator_fee.go diff --git a/tests/simapp/config.go b/tests/simapp/config.go index 341789e..cb23b42 100644 --- a/tests/simapp/config.go +++ b/tests/simapp/config.go @@ -83,6 +83,7 @@ var ( // module account permissions moduleAccPerms = []*authmodulev1.ModuleAccountPermission{ + {Account: feemarkettypes.FeeCollectorName, Permissions: []string{authtypes.Burner}}, // allow fee market to burn {Account: authtypes.FeeCollectorName}, {Account: distrtypes.ModuleName}, {Account: feemarkettypes.ModuleName}, @@ -99,6 +100,7 @@ var ( minttypes.ModuleName, stakingtypes.BondedPoolName, stakingtypes.NotBondedPoolName, + feemarkettypes.FeeCollectorName, // We allow the following module accounts to receive funds: // govtypes.ModuleName } diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 6cd535b..7b076ef 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -2,19 +2,24 @@ package ante import ( "fmt" + "math" 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" + + feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" ) // TxFeeChecker check if the provided fee is enough and returns the effective fee and tx priority, // the effective fee should be deducted later, and the priority should be returned in abci response. type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) -// DeductFeeDecorator deducts fees from the fee payer. The fee payer is the fee granter (if specified) or first signer of the tx. +// FeeMarketDecorator deducts fees from the fee payer based off of the current state of the feemarket. +// The fee payer is the fee granter (if specified) or first signer of the tx. // If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. // Call next AnteHandler if fees successfully deducted. // CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator @@ -44,10 +49,7 @@ func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } - var ( - priority int64 - err error - ) + var priority int64 minGasPrices, err := dfd.feemarketKeeper.GetMinGasPrices(ctx) if err != nil { @@ -78,8 +80,8 @@ func (dfd FeeMarketDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } - if addr := dfd.accountKeeper.GetModuleAddress(authtypes.FeeCollectorName); addr == nil { - return fmt.Errorf("fee collector module account (%s) has not been set", authtypes.FeeCollectorName) + if addr := dfd.accountKeeper.GetModuleAddress(feemarkettypes.FeeCollectorName); addr == nil { + return fmt.Errorf("fee collector module account (%s) has not been set", feemarkettypes.FeeCollectorName) } feePayer := feeTx.FeePayer() @@ -132,10 +134,67 @@ func DeductFees(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) } - err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), authtypes.FeeCollectorName, fees) + err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), feemarkettypes.FeeCollectorName, fees) if err != nil { return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } return nil } + +// checkTxFees implements the logic for the fee market to check if a Tx has provided suffucient +// fees given the current state of the fee market. Returns an error if insufficient fees. +func checkTxFees(ctx sdk.Context, fees sdk.DecCoins, 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() + + // Ensure that the provided fees meet the minimum + // if this is a CheckTx. This is only for local mempool purposes, and thus + // is only ran on check tx. + if ctx.IsCheckTx() { + minGasPrices := fees + if !minGasPrices.IsZero() { + requiredFees := make(sdk.Coins, len(minGasPrices)) + + // 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) + } + } + } + + priority := getTxPriority(feeCoins, int64(gas)) + return feeCoins, priority, nil +} + +// getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the gas price +// provided in a transaction. +// NOTE: This implementation should be used with a great consideration as it opens potential attack vectors +// where txs with multiple coins could not be prioritize as expected. +func getTxPriority(fee sdk.Coins, gas int64) int64 { + var priority int64 + for _, c := range fee { + p := int64(math.MaxInt64) + gasPrice := c.Amount.QuoRaw(gas) + if gasPrice.IsInt64() { + p = gasPrice.Int64() + } + if priority == 0 || p < priority { + priority = p + } + } + + return priority +} diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go new file mode 100644 index 0000000..25311cd --- /dev/null +++ b/x/feemarket/ante/fee_test.go @@ -0,0 +1,3 @@ +package ante_test + +// TODO add test diff --git a/x/feemarket/ante/validator_fee.go b/x/feemarket/ante/validator_fee.go deleted file mode 100644 index 096ef3e..0000000 --- a/x/feemarket/ante/validator_fee.go +++ /dev/null @@ -1,68 +0,0 @@ -package ante - -import ( - "math" - - errorsmod "cosmossdk.io/errors" - - sdkmath "cosmossdk.io/math" - sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" -) - -// checkTxFees implements the logic for the fee market to check if a Tx has provided suffucient -// fees given the current state of the fee market. Returns an error if insufficient fees. -func checkTxFees(ctx sdk.Context, fees sdk.DecCoins, 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() - - // Ensure that the provided fees meet the minimum - // if this is a CheckTx. This is only for local mempool purposes, and thus - // is only ran on check tx. - if ctx.IsCheckTx() { - minGasPrices := fees - if !minGasPrices.IsZero() { - requiredFees := make(sdk.Coins, len(minGasPrices)) - - // 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) - } - } - } - - priority := getTxPriority(feeCoins, int64(gas)) - return feeCoins, priority, nil -} - -// getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the gas price -// provided in a transaction. -// NOTE: This implementation should be used with a great consideration as it opens potential attack vectors -// where txs with multiple coins could not be prioritize as expected. -func getTxPriority(fee sdk.Coins, gas int64) int64 { - var priority int64 - for _, c := range fee { - p := int64(math.MaxInt64) - gasPrice := c.Amount.QuoRaw(gas) - if gasPrice.IsInt64() { - p = gasPrice.Int64() - } - if priority == 0 || p < priority { - priority = p - } - } - - return priority -} From 061498063ad2fd21ee4c31a5c1d2291477af8c41 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 13:44:36 -0500 Subject: [PATCH 22/55] mock --- x/feemarket/keeper/keeper_test.go | 6 ++ x/feemarket/module.go | 8 +- x/feemarket/types/expected_keepers.go | 2 + .../types/mocks/mock_account_keeper.go | 79 +++++++++++++++++++ 4 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 x/feemarket/types/mocks/mock_account_keeper.go diff --git a/x/feemarket/keeper/keeper_test.go b/x/feemarket/keeper/keeper_test.go index 461bbba..75694ef 100644 --- a/x/feemarket/keeper/keeper_test.go +++ b/x/feemarket/keeper/keeper_test.go @@ -12,11 +12,13 @@ import ( "github.com/skip-mev/feemarket/testutils" "github.com/skip-mev/feemarket/x/feemarket/keeper" "github.com/skip-mev/feemarket/x/feemarket/types" + "github.com/skip-mev/feemarket/x/feemarket/types/mocks" ) type KeeperTestSuite struct { suite.Suite + accountKeeper *mocks.AccountKeeper feemarketKeeper *keeper.Keeper encCfg testutils.EncodingConfig ctx sdk.Context @@ -41,9 +43,13 @@ func (s *KeeperTestSuite) SetupTest() { s.ctx = testCtx.Ctx s.authorityAccount = []byte("authority") + s.accountKeeper = mocks.NewAccountKeeper(s.T()) + s.accountKeeper.On("GetModuleAddress", "feemarket-fee-collector").Return(sdk.AccAddress("feemarket-fee-collector")) + s.feemarketKeeper = keeper.NewKeeper( s.encCfg.Codec, s.key, + s.accountKeeper, s.authorityAccount.String(), ) diff --git a/x/feemarket/module.go b/x/feemarket/module.go index 9080ae1..d531d9a 100644 --- a/x/feemarket/module.go +++ b/x/feemarket/module.go @@ -158,9 +158,10 @@ func init() { type Inputs struct { depinject.In - Config *modulev1.Module - Cdc codec.Codec - Key *store.KVStoreKey + Config *modulev1.Module + Cdc codec.Codec + Key *store.KVStoreKey + AccountKeeper types.AccountKeeper } type Outputs struct { @@ -187,6 +188,7 @@ func ProvideModule(in Inputs) Outputs { Keeper := keeper.NewKeeper( in.Cdc, in.Key, + in.AccountKeeper, authority.String(), ) diff --git a/x/feemarket/types/expected_keepers.go b/x/feemarket/types/expected_keepers.go index 0f9f506..ff5f6f6 100644 --- a/x/feemarket/types/expected_keepers.go +++ b/x/feemarket/types/expected_keepers.go @@ -6,6 +6,8 @@ import ( ) // AccountKeeper defines the expected account keeper (noalias) +// +//go:generate mockery --name AccountKeeper --filename mock_account_keeper.go type AccountKeeper interface { GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI diff --git a/x/feemarket/types/mocks/mock_account_keeper.go b/x/feemarket/types/mocks/mock_account_keeper.go new file mode 100644 index 0000000..a4f040a --- /dev/null +++ b/x/feemarket/types/mocks/mock_account_keeper.go @@ -0,0 +1,79 @@ +// Code generated by mockery v0.0.0-dev. DO NOT EDIT. + +package mocks + +import ( + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + + mock "github.com/stretchr/testify/mock" + + types "github.com/cosmos/cosmos-sdk/types" +) + +// AccountKeeper is an autogenerated mock type for the AccountKeeper type +type AccountKeeper struct { + mock.Mock +} + +// GetAccount provides a mock function with given fields: ctx, addr +func (_m *AccountKeeper) GetAccount(ctx types.Context, addr types.AccAddress) authtypes.AccountI { + ret := _m.Called(ctx, addr) + + var r0 authtypes.AccountI + if rf, ok := ret.Get(0).(func(types.Context, types.AccAddress) authtypes.AccountI); ok { + r0 = rf(ctx, addr) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(authtypes.AccountI) + } + } + + return r0 +} + +// GetModuleAccount provides a mock function with given fields: ctx, name +func (_m *AccountKeeper) GetModuleAccount(ctx types.Context, name string) authtypes.ModuleAccountI { + ret := _m.Called(ctx, name) + + var r0 authtypes.ModuleAccountI + if rf, ok := ret.Get(0).(func(types.Context, string) authtypes.ModuleAccountI); ok { + r0 = rf(ctx, name) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(authtypes.ModuleAccountI) + } + } + + return r0 +} + +// GetModuleAddress provides a mock function with given fields: name +func (_m *AccountKeeper) GetModuleAddress(name string) types.AccAddress { + ret := _m.Called(name) + + var r0 types.AccAddress + if rf, ok := ret.Get(0).(func(string) types.AccAddress); ok { + r0 = rf(name) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(types.AccAddress) + } + } + + return r0 +} + +// NewAccountKeeper creates a new instance of AccountKeeper. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewAccountKeeper(t interface { + mock.TestingT + Cleanup(func()) +}, +) *AccountKeeper { + mock := &AccountKeeper{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} From 8364cbd5fee6e948410f09656cdabe6c9b7c4917 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 13:52:11 -0500 Subject: [PATCH 23/55] always check and use fee --- x/feemarket/ante/fee.go | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 7b076ef..8a8dc16 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -154,25 +154,23 @@ func checkTxFees(ctx sdk.Context, fees sdk.DecCoins, tx sdk.Tx) (sdk.Coins, int6 gas := feeTx.GetGas() // Ensure that the provided fees meet the minimum - // if this is a CheckTx. This is only for local mempool purposes, and thus - // is only ran on check tx. - if ctx.IsCheckTx() { - minGasPrices := fees - if !minGasPrices.IsZero() { - requiredFees := make(sdk.Coins, len(minGasPrices)) - - // 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()) - } + minGasPrices := fees + if !minGasPrices.IsZero() { + requiredFees := make(sdk.Coins, len(minGasPrices)) + + // 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) - } + if !feeCoins.IsAnyGTE(requiredFees) { + return nil, 0, errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) } + + feeCoins = requiredFees } priority := getTxPriority(feeCoins, int64(gas)) From 477dcdd1333d7c776c286175c9b5935a282b601d Mon Sep 17 00:00:00 2001 From: aljo242 Date: Wed, 15 Nov 2023 14:05:04 -0500 Subject: [PATCH 24/55] extend with tip --- x/feemarket/ante/fee.go | 53 ++++++++++++++++++++++++++++++--------- x/feemarket/types/keys.go | 3 +++ 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 8a8dc16..e6b53ac 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -49,7 +49,10 @@ func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } - var priority int64 + var ( + priority int64 + tip sdk.Coins + ) minGasPrices, err := dfd.feemarketKeeper.GetMinGasPrices(ctx) if err != nil { @@ -60,13 +63,13 @@ func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo fee := feeTx.GetFee() if !simulate { - fee, priority, err = checkTxFees(ctx, minGasPricesDec, tx) + fee, tip, priority, err = checkTxFees(ctx, minGasPricesDec, tx) if err != nil { return ctx, err } } - if err := dfd.checkDeductFee(ctx, tx, fee); err != nil { + if err := dfd.checkDeductFeeAndTip(ctx, tx, fee, tip); err != nil { return ctx, err } @@ -74,7 +77,7 @@ func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return next(newCtx, tx, simulate) } -func (dfd FeeMarketDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee sdk.Coins) error { +func (dfd FeeMarketDecorator) checkDeductFeeAndTip(ctx sdk.Context, sdkTx sdk.Tx, fee, tip sdk.Coins) error { feeTx, ok := sdkTx.(sdk.FeeTx) if !ok { return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") @@ -116,11 +119,21 @@ func (dfd FeeMarketDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee } } + // deduct the tip + if !tip.IsZero() { + err := DeductTip(dfd.bankKeeper, ctx, deductFeesFromAcc, tip) + if err != nil { + return err + } + } + events := sdk.Events{ sdk.NewEvent( sdk.EventTypeTx, sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()), sdk.NewAttribute(sdk.AttributeKeyFeePayer, deductFeesFrom.String()), + sdk.NewAttribute(feemarkettypes.AttributeKeyTip, tip.String()), + sdk.NewAttribute(feemarkettypes.AttributeKeyTipPayer, sdk.AccAddress(ctx.BlockHeader().ProposerAddress).String()), ), } ctx.EventManager().EmitEvents(events) @@ -128,7 +141,22 @@ func (dfd FeeMarketDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee return nil } -// DeductFees deducts fees from the given account. +// DeductTip deducts tips from the given account. +func DeductTip(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, tips sdk.Coins) error { + if !tips.IsValid() { + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", tips) + } + + proposer := sdk.AccAddress(ctx.BlockHeader().ProposerAddress) + err := bankKeeper.SendCoins(ctx, acc.GetAddress(), proposer, tips) + if err != nil { + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) + } + + return nil +} + +// DeductFee deducts fees from the given account. func DeductFees(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error { if !fees.IsValid() { return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) @@ -144,13 +172,13 @@ func DeductFees(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, // checkTxFees implements the logic for the fee market to check if a Tx has provided suffucient // fees given the current state of the fee market. Returns an error if insufficient fees. -func checkTxFees(ctx sdk.Context, fees sdk.DecCoins, tx sdk.Tx) (sdk.Coins, int64, error) { +func checkTxFees(ctx sdk.Context, fees sdk.DecCoins, tx sdk.Tx) (feeCoins sdk.Coins, tip sdk.Coins, priority int64, err error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { - return nil, 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + return nil, nil, 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } - feeCoins := feeTx.GetFee() + feeCoins = feeTx.GetFee() gas := feeTx.GetGas() // Ensure that the provided fees meet the minimum @@ -167,14 +195,15 @@ func checkTxFees(ctx sdk.Context, fees sdk.DecCoins, tx sdk.Tx) (sdk.Coins, int6 } if !feeCoins.IsAnyGTE(requiredFees) { - return nil, 0, errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) + return nil, nil, 0, errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) } - feeCoins = requiredFees + tip = feeCoins.Sub(requiredFees...) // tip is whatever is left + feeCoins = requiredFees // set free coins to be ONLY the required amount } - priority := getTxPriority(feeCoins, int64(gas)) - return feeCoins, priority, nil + priority = getTxPriority(feeCoins, int64(gas)) + return feeCoins, tip, priority, nil } // getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the gas price diff --git a/x/feemarket/types/keys.go b/x/feemarket/types/keys.go index 6bf6abd..c2aaf09 100644 --- a/x/feemarket/types/keys.go +++ b/x/feemarket/types/keys.go @@ -21,4 +21,7 @@ var ( // KeyState is the store key for the feemarket module's data. KeyState = []byte{prefixState} + + AttributeKeyTip = "tip" + AttributeKeyTipPayer = "tip_payer" ) From 7f50ce6f89e99f2e1d89651ae7b63320a9e3af24 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 20 Nov 2023 14:03:09 -0500 Subject: [PATCH 25/55] fix lint --- x/feemarket/ante/fee.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index e6b53ac..5eab745 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -172,7 +172,7 @@ func DeductFees(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, // checkTxFees implements the logic for the fee market to check if a Tx has provided suffucient // fees given the current state of the fee market. Returns an error if insufficient fees. -func checkTxFees(ctx sdk.Context, fees sdk.DecCoins, tx sdk.Tx) (feeCoins sdk.Coins, tip sdk.Coins, priority int64, err error) { +func checkTxFees(_ sdk.Context, fees sdk.DecCoins, tx sdk.Tx) (feeCoins sdk.Coins, tip sdk.Coins, priority int64, err error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { return nil, nil, 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") From 37344a1e3e5a5ac3815c32daca1c20bdaf9e7fd9 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 20 Nov 2023 14:39:54 -0500 Subject: [PATCH 26/55] refactor --- x/feemarket/ante/fee.go | 29 +++++++---------------------- x/feemarket/ante/fee_test.go | 6 +++++- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 5eab745..8301bdf 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -113,7 +113,7 @@ func (dfd FeeMarketDecorator) checkDeductFeeAndTip(ctx sdk.Context, sdkTx sdk.Tx // deduct the fees if !fee.IsZero() { - err := DeductFees(dfd.bankKeeper, ctx, deductFeesFromAcc, fee) + err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, fee) if err != nil { return err } @@ -121,7 +121,7 @@ func (dfd FeeMarketDecorator) checkDeductFeeAndTip(ctx sdk.Context, sdkTx sdk.Tx // deduct the tip if !tip.IsZero() { - err := DeductTip(dfd.bankKeeper, ctx, deductFeesFromAcc, tip) + err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, tip) if err != nil { return err } @@ -141,28 +141,13 @@ func (dfd FeeMarketDecorator) checkDeductFeeAndTip(ctx sdk.Context, sdkTx sdk.Tx return nil } -// DeductTip deducts tips from the given account. -func DeductTip(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, tips sdk.Coins) error { - if !tips.IsValid() { - return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", tips) +// DeductCoins deducts coins from the given account. Coins are sent to the feemarket module account. +func DeductCoins(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, coins sdk.Coins) error { + if !coins.IsValid() { + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid coin amount: %s", coins) } - proposer := sdk.AccAddress(ctx.BlockHeader().ProposerAddress) - err := bankKeeper.SendCoins(ctx, acc.GetAddress(), proposer, tips) - if err != nil { - return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) - } - - return nil -} - -// DeductFee deducts fees from the given account. -func DeductFees(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error { - if !fees.IsValid() { - return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) - } - - err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), feemarkettypes.FeeCollectorName, fees) + err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), feemarkettypes.FeeCollectorName, coins) if err != nil { return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go index 25311cd..7c97a84 100644 --- a/x/feemarket/ante/fee_test.go +++ b/x/feemarket/ante/fee_test.go @@ -1,3 +1,7 @@ package ante_test -// TODO add test +import "testing" + +func DeductCoins(t *testing.T) { + +} From 47a2c2bae12c92b6b30aff0823edacc04a196bcf Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 20 Nov 2023 15:28:22 -0500 Subject: [PATCH 27/55] utd --- x/feemarket/ante/expected_keepers.go | 22 ++- x/feemarket/ante/fee.go | 4 +- x/feemarket/ante/fee_test.go | 141 +++++++++++++++++- x/feemarket/ante/mocks/mock_account_keeper.go | 112 ++++++++++++++ x/feemarket/ante/mocks/mock_bank_keeper.go | 76 ++++++++++ .../ante/mocks/mock_feegrant_keeper.go | 41 +++++ .../ante/mocks/mock_feemarket_keeper.go | 79 ++++++++++ 7 files changed, 465 insertions(+), 10 deletions(-) create mode 100644 x/feemarket/ante/mocks/mock_account_keeper.go create mode 100644 x/feemarket/ante/mocks/mock_bank_keeper.go create mode 100644 x/feemarket/ante/mocks/mock_feegrant_keeper.go create mode 100644 x/feemarket/ante/mocks/mock_feemarket_keeper.go diff --git a/x/feemarket/ante/expected_keepers.go b/x/feemarket/ante/expected_keepers.go index 6fc7fe6..c3c2592 100644 --- a/x/feemarket/ante/expected_keepers.go +++ b/x/feemarket/ante/expected_keepers.go @@ -2,26 +2,34 @@ package ante import ( sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/auth/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" ) // AccountKeeper defines the contract needed for AccountKeeper related APIs. // Interface provides support to use non-sdk AccountKeeper for AnteHandler's decorators. +// +//go:generate mockery --name AccountKeeper --filename mock_account_keeper.go type AccountKeeper interface { - GetParams(ctx sdk.Context) (params types.Params) - GetAccount(ctx sdk.Context, addr sdk.AccAddress) types.AccountI - SetAccount(ctx sdk.Context, acc types.AccountI) + GetParams(ctx sdk.Context) (params authtypes.Params) + GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI + SetAccount(ctx sdk.Context, acc authtypes.AccountI) GetModuleAddress(moduleName string) sdk.AccAddress + GetModuleAccount(ctx sdk.Context, name string) authtypes.ModuleAccountI + NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI } -// FeegrantKeeper defines the expected feegrant keeper. -type FeegrantKeeper interface { +// FeeGrantKeeper defines the expected feegrant keeper. +// +//go:generate mockery --name FeeGrantKeeper --filename mock_feegrant_keeper.go +type FeeGrantKeeper interface { UseGrantedFees(ctx sdk.Context, granter, grantee sdk.AccAddress, fee sdk.Coins, msgs []sdk.Msg) error } // BankKeeper defines the contract needed for supply related APIs. +// +//go:generate mockery --name BankKeeper --filename mock_bank_keeper.go type BankKeeper interface { IsSendEnabledCoins(ctx sdk.Context, coins ...sdk.Coin) error SendCoins(ctx sdk.Context, from, to sdk.AccAddress, amt sdk.Coins) error @@ -29,6 +37,8 @@ type BankKeeper interface { } // FeeMarketKeeper defines the expected feemarket keeper. +// +//go:generate mockery --name FeeMarketKeeper --filename mock_feemarket_keeper.go type FeeMarketKeeper interface { GetState(ctx sdk.Context) (feemarkettypes.State, error) GetMinGasPrices(ctx sdk.Context) (sdk.Coins, error) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 8301bdf..906e033 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -26,11 +26,11 @@ type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) type FeeMarketDecorator struct { accountKeeper AccountKeeper bankKeeper BankKeeper - feegrantKeeper FeegrantKeeper + feegrantKeeper FeeGrantKeeper feemarketKeeper FeeMarketKeeper } -func NewFeeMarketDecorator(ak AccountKeeper, bk BankKeeper, fk FeegrantKeeper, fmk FeeMarketKeeper) FeeMarketDecorator { +func NewFeeMarketDecorator(ak AccountKeeper, bk BankKeeper, fk FeeGrantKeeper, fmk FeeMarketKeeper) FeeMarketDecorator { return FeeMarketDecorator{ accountKeeper: ak, bankKeeper: bk, diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go index 7c97a84..d6f8e8f 100644 --- a/x/feemarket/ante/fee_test.go +++ b/x/feemarket/ante/fee_test.go @@ -1,7 +1,144 @@ package ante_test -import "testing" +import ( + "testing" -func DeductCoins(t *testing.T) { + "github.com/cosmos/cosmos-sdk/client" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + storetypes "github.com/cosmos/cosmos-sdk/store/types" + "github.com/cosmos/cosmos-sdk/testutil" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + sdk "github.com/cosmos/cosmos-sdk/types" + authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/stretchr/testify/suite" + "github.com/skip-mev/feemarket/testutils" + "github.com/skip-mev/feemarket/x/feemarket/ante" + "github.com/skip-mev/feemarket/x/feemarket/ante/mocks" + "github.com/skip-mev/feemarket/x/feemarket/keeper" + "github.com/skip-mev/feemarket/x/feemarket/types" +) + +type AnteTestSuite struct { + suite.Suite + anteHandler sdk.AnteHandler + ctx sdk.Context + clientCtx client.Context + txBuilder client.TxBuilder + + accountKeeper authkeeper.AccountKeeper + bankKeeper *mocks.BankKeeper + feeGrantKeeper *mocks.FeeGrantKeeper + feemarketKeeper *keeper.Keeper + encCfg testutils.EncodingConfig + key *storetypes.KVStoreKey + authorityAccount sdk.AccAddress + + // Message server variables + msgServer types.MsgServer + + // Query server variables + queryServer types.QueryServer +} + +func TestAnteTestSuite(t *testing.T) { + suite.Run(t, new(AnteTestSuite)) +} + +// TestAccount represents an account used in the tests in x/auth/ante. +type TestAccount struct { + acc authtypes.AccountI + priv cryptotypes.PrivKey +} + +func (s *AnteTestSuite) CreateTestAccounts(numAccs int) []TestAccount { + var accounts []TestAccount + + for i := 0; i < numAccs; i++ { + priv, _, addr := testdata.KeyTestPubAddr() + acc := s.accountKeeper.NewAccountWithAddress(s.ctx, addr) + err := acc.SetAccountNumber(uint64(i + 1000)) + if err != nil { + panic(err) + } + s.accountKeeper.SetAccount(s.ctx, acc) + accounts = append(accounts, TestAccount{acc, priv}) + } + + return accounts +} + +func (s *AnteTestSuite) SetupTest() { + s.encCfg = testutils.CreateTestEncodingConfig() + s.key = storetypes.NewKVStoreKey(types.StoreKey) + tkey := storetypes.NewTransientStoreKey("transient_test_feemarket") + testCtx := testutil.DefaultContextWithDB(s.T(), s.key, tkey) + s.ctx = testCtx.Ctx.WithIsCheckTx(true).WithBlockHeight(1) + cms, db := testCtx.CMS, testCtx.DB + + authKey := storetypes.NewKVStoreKey(authtypes.StoreKey) + tkey = storetypes.NewTransientStoreKey("transient_test_auth") + cms.MountStoreWithDB(authKey, storetypes.StoreTypeIAVL, db) + cms.MountStoreWithDB(tkey, storetypes.StoreTypeTransient, db) + err := cms.LoadLatestVersion() + s.Require().NoError(err) + + maccPerms := map[string][]string{ + "fee_collector": nil, + types.ModuleName: nil, + types.FeeCollectorName: {"burner"}, + "mint": {"minter"}, + "bonded_tokens_pool": {"burner", "staking"}, + "not_bonded_tokens_pool": {"burner", "staking"}, + "multiPerm": {"burner", "minter", "staking"}, + "random": {"random"}, + } + + s.authorityAccount = authtypes.NewModuleAddress("gov") + s.accountKeeper = authkeeper.NewAccountKeeper( + s.encCfg.Codec, authKey, authtypes.ProtoBaseAccount, maccPerms, sdk.Bech32MainPrefix, s.authorityAccount.String(), + ) + + s.feemarketKeeper = keeper.NewKeeper( + s.encCfg.Codec, + s.key, + s.accountKeeper, + s.authorityAccount.String(), + ) + + err = s.feemarketKeeper.SetParams(s.ctx, types.DefaultParams()) + s.Require().NoError(err) + + s.msgServer = keeper.NewMsgServer(*s.feemarketKeeper) + s.queryServer = keeper.NewQueryServer(*s.feemarketKeeper) +} + +func (s *AnteTestSuite) DeductCoins(t *testing.T) { + +} + +func (s *AnteTestSuite) TestDeductCoins() { + accs := s.CreateTestAccounts(1) + + tests := []struct { + name string + acc TestAccount + coins sdk.Coins + wantErr bool + }{ + { + name: "valid no coins", + acc: accs[0], + }, + } + for _, tc := range tests { + s.Run(tc.name, func() { + s.bankKeeper.On("SendCoinsFromAccountToModule", s.ctx, tc.acc.acc.GetAddress(), types.FeeCollectorName, tc.coins).Return(nil) + + if err := ante.DeductCoins(s.bankKeeper, s.ctx, tc.acc.acc, tc.coins); (err != nil) != tc.wantErr { + s.Errorf(err, "DeductCoins() error = %v, wantErr %v", err, tc.wantErr) + } + }) + } } diff --git a/x/feemarket/ante/mocks/mock_account_keeper.go b/x/feemarket/ante/mocks/mock_account_keeper.go new file mode 100644 index 0000000..207e219 --- /dev/null +++ b/x/feemarket/ante/mocks/mock_account_keeper.go @@ -0,0 +1,112 @@ +// Code generated by mockery v0.0.0-dev. DO NOT EDIT. + +package mocks + +import ( + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + mock "github.com/stretchr/testify/mock" + + types "github.com/cosmos/cosmos-sdk/types" +) + +// AccountKeeper is an autogenerated mock type for the AccountKeeper type +type AccountKeeper struct { + mock.Mock +} + +// GetAccount provides a mock function with given fields: ctx, addr +func (_m *AccountKeeper) GetAccount(ctx types.Context, addr types.AccAddress) authtypes.AccountI { + ret := _m.Called(ctx, addr) + + var r0 authtypes.AccountI + if rf, ok := ret.Get(0).(func(types.Context, types.AccAddress) authtypes.AccountI); ok { + r0 = rf(ctx, addr) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(authtypes.AccountI) + } + } + + return r0 +} + +// GetModuleAccount provides a mock function with given fields: ctx, name +func (_m *AccountKeeper) GetModuleAccount(ctx types.Context, name string) authtypes.ModuleAccountI { + ret := _m.Called(ctx, name) + + var r0 authtypes.ModuleAccountI + if rf, ok := ret.Get(0).(func(types.Context, string) authtypes.ModuleAccountI); ok { + r0 = rf(ctx, name) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(authtypes.ModuleAccountI) + } + } + + return r0 +} + +// GetModuleAddress provides a mock function with given fields: moduleName +func (_m *AccountKeeper) GetModuleAddress(moduleName string) types.AccAddress { + ret := _m.Called(moduleName) + + var r0 types.AccAddress + if rf, ok := ret.Get(0).(func(string) types.AccAddress); ok { + r0 = rf(moduleName) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(types.AccAddress) + } + } + + return r0 +} + +// GetParams provides a mock function with given fields: ctx +func (_m *AccountKeeper) GetParams(ctx types.Context) authtypes.Params { + ret := _m.Called(ctx) + + var r0 authtypes.Params + if rf, ok := ret.Get(0).(func(types.Context) authtypes.Params); ok { + r0 = rf(ctx) + } else { + r0 = ret.Get(0).(authtypes.Params) + } + + return r0 +} + +// NewAccountWithAddress provides a mock function with given fields: ctx, addr +func (_m *AccountKeeper) NewAccountWithAddress(ctx types.Context, addr types.AccAddress) authtypes.AccountI { + ret := _m.Called(ctx, addr) + + var r0 authtypes.AccountI + if rf, ok := ret.Get(0).(func(types.Context, types.AccAddress) authtypes.AccountI); ok { + r0 = rf(ctx, addr) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(authtypes.AccountI) + } + } + + return r0 +} + +// SetAccount provides a mock function with given fields: ctx, acc +func (_m *AccountKeeper) SetAccount(ctx types.Context, acc authtypes.AccountI) { + _m.Called(ctx, acc) +} + +// NewAccountKeeper creates a new instance of AccountKeeper. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewAccountKeeper(t interface { + mock.TestingT + Cleanup(func()) +}) *AccountKeeper { + mock := &AccountKeeper{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/x/feemarket/ante/mocks/mock_bank_keeper.go b/x/feemarket/ante/mocks/mock_bank_keeper.go new file mode 100644 index 0000000..351bafa --- /dev/null +++ b/x/feemarket/ante/mocks/mock_bank_keeper.go @@ -0,0 +1,76 @@ +// Code generated by mockery v0.0.0-dev. DO NOT EDIT. + +package mocks + +import ( + types "github.com/cosmos/cosmos-sdk/types" + mock "github.com/stretchr/testify/mock" +) + +// BankKeeper is an autogenerated mock type for the BankKeeper type +type BankKeeper struct { + mock.Mock +} + +// IsSendEnabledCoins provides a mock function with given fields: ctx, coins +func (_m *BankKeeper) IsSendEnabledCoins(ctx types.Context, coins ...types.Coin) error { + _va := make([]interface{}, len(coins)) + for _i := range coins { + _va[_i] = coins[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + var r0 error + if rf, ok := ret.Get(0).(func(types.Context, ...types.Coin) error); ok { + r0 = rf(ctx, coins...) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// SendCoins provides a mock function with given fields: ctx, from, to, amt +func (_m *BankKeeper) SendCoins(ctx types.Context, from types.AccAddress, to types.AccAddress, amt types.Coins) error { + ret := _m.Called(ctx, from, to, amt) + + var r0 error + if rf, ok := ret.Get(0).(func(types.Context, types.AccAddress, types.AccAddress, types.Coins) error); ok { + r0 = rf(ctx, from, to, amt) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// SendCoinsFromAccountToModule provides a mock function with given fields: ctx, senderAddr, recipientModule, amt +func (_m *BankKeeper) SendCoinsFromAccountToModule(ctx types.Context, senderAddr types.AccAddress, recipientModule string, amt types.Coins) error { + ret := _m.Called(ctx, senderAddr, recipientModule, amt) + + var r0 error + if rf, ok := ret.Get(0).(func(types.Context, types.AccAddress, string, types.Coins) error); ok { + r0 = rf(ctx, senderAddr, recipientModule, amt) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// NewBankKeeper creates a new instance of BankKeeper. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewBankKeeper(t interface { + mock.TestingT + Cleanup(func()) +}) *BankKeeper { + mock := &BankKeeper{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/x/feemarket/ante/mocks/mock_feegrant_keeper.go b/x/feemarket/ante/mocks/mock_feegrant_keeper.go new file mode 100644 index 0000000..355ecd5 --- /dev/null +++ b/x/feemarket/ante/mocks/mock_feegrant_keeper.go @@ -0,0 +1,41 @@ +// Code generated by mockery v0.0.0-dev. DO NOT EDIT. + +package mocks + +import ( + types "github.com/cosmos/cosmos-sdk/types" + mock "github.com/stretchr/testify/mock" +) + +// FeeGrantKeeper is an autogenerated mock type for the FeeGrantKeeper type +type FeeGrantKeeper struct { + mock.Mock +} + +// UseGrantedFees provides a mock function with given fields: ctx, granter, grantee, fee, msgs +func (_m *FeeGrantKeeper) UseGrantedFees(ctx types.Context, granter types.AccAddress, grantee types.AccAddress, fee types.Coins, msgs []types.Msg) error { + ret := _m.Called(ctx, granter, grantee, fee, msgs) + + var r0 error + if rf, ok := ret.Get(0).(func(types.Context, types.AccAddress, types.AccAddress, types.Coins, []types.Msg) error); ok { + r0 = rf(ctx, granter, grantee, fee, msgs) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// NewFeeGrantKeeper creates a new instance of FeeGrantKeeper. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewFeeGrantKeeper(t interface { + mock.TestingT + Cleanup(func()) +}) *FeeGrantKeeper { + mock := &FeeGrantKeeper{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/x/feemarket/ante/mocks/mock_feemarket_keeper.go b/x/feemarket/ante/mocks/mock_feemarket_keeper.go new file mode 100644 index 0000000..3b1cd67 --- /dev/null +++ b/x/feemarket/ante/mocks/mock_feemarket_keeper.go @@ -0,0 +1,79 @@ +// Code generated by mockery v0.0.0-dev. DO NOT EDIT. + +package mocks + +import ( + feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" + mock "github.com/stretchr/testify/mock" + + types "github.com/cosmos/cosmos-sdk/types" +) + +// FeeMarketKeeper is an autogenerated mock type for the FeeMarketKeeper type +type FeeMarketKeeper struct { + mock.Mock +} + +// GetMinGasPrices provides a mock function with given fields: ctx +func (_m *FeeMarketKeeper) GetMinGasPrices(ctx types.Context) (types.Coins, error) { + ret := _m.Called(ctx) + + var r0 types.Coins + var r1 error + if rf, ok := ret.Get(0).(func(types.Context) (types.Coins, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(types.Context) types.Coins); ok { + r0 = rf(ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(types.Coins) + } + } + + if rf, ok := ret.Get(1).(func(types.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetState provides a mock function with given fields: ctx +func (_m *FeeMarketKeeper) GetState(ctx types.Context) (feemarkettypes.State, error) { + ret := _m.Called(ctx) + + var r0 feemarkettypes.State + var r1 error + if rf, ok := ret.Get(0).(func(types.Context) (feemarkettypes.State, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(types.Context) feemarkettypes.State); ok { + r0 = rf(ctx) + } else { + r0 = ret.Get(0).(feemarkettypes.State) + } + + if rf, ok := ret.Get(1).(func(types.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewFeeMarketKeeper creates a new instance of FeeMarketKeeper. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewFeeMarketKeeper(t interface { + mock.TestingT + Cleanup(func()) +}) *FeeMarketKeeper { + mock := &FeeMarketKeeper{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} From 53437a2887112591fac6713e16fec722df0de3e7 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 20 Nov 2023 15:30:00 -0500 Subject: [PATCH 28/55] utd --- testutils/testutils.go | 12 +++++++----- x/feemarket/ante/fee_test.go | 1 + 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/testutils/testutils.go b/testutils/testutils.go index 5bc9d8c..ef1eefb 100644 --- a/testutils/testutils.go +++ b/testutils/testutils.go @@ -1,6 +1,7 @@ package testutils import ( + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "math/rand" "github.com/cosmos/cosmos-sdk/client" @@ -28,21 +29,22 @@ type EncodingConfig struct { } func CreateTestEncodingConfig() EncodingConfig { - cdc := codec.NewLegacyAmino() + amino := codec.NewLegacyAmino() interfaceRegistry := types.NewInterfaceRegistry() banktypes.RegisterInterfaces(interfaceRegistry) + authtypes.RegisterInterfaces(interfaceRegistry) cryptocodec.RegisterInterfaces(interfaceRegistry) feemarkettypes.RegisterInterfaces(interfaceRegistry) stakingtypes.RegisterInterfaces(interfaceRegistry) - codec := codec.NewProtoCodec(interfaceRegistry) + cdc := codec.NewProtoCodec(interfaceRegistry) return EncodingConfig{ InterfaceRegistry: interfaceRegistry, - Codec: codec, - TxConfig: tx.NewTxConfig(codec, tx.DefaultSignModes), - Amino: cdc, + Codec: cdc, + TxConfig: tx.NewTxConfig(cdc, tx.DefaultSignModes), + Amino: amino, } } diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go index d6f8e8f..b2df919 100644 --- a/x/feemarket/ante/fee_test.go +++ b/x/feemarket/ante/fee_test.go @@ -9,6 +9,7 @@ import ( "github.com/cosmos/cosmos-sdk/testutil" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" + _ "github.com/cosmos/cosmos-sdk/x/auth" authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/stretchr/testify/suite" From 22735e4be4270ced09d8e463f87b3ee67c4d6fc9 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 20 Nov 2023 15:59:23 -0500 Subject: [PATCH 29/55] setup test --- x/feemarket/ante/fee_test.go | 38 +++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go index b2df919..6eb19eb 100644 --- a/x/feemarket/ante/fee_test.go +++ b/x/feemarket/ante/fee_test.go @@ -111,6 +111,9 @@ func (s *AnteTestSuite) SetupTest() { err = s.feemarketKeeper.SetParams(s.ctx, types.DefaultParams()) s.Require().NoError(err) + s.bankKeeper = mocks.NewBankKeeper(s.T()) + s.feeGrantKeeper = mocks.NewFeeGrantKeeper(s.T()) + s.msgServer = keeper.NewMsgServer(*s.feemarketKeeper) s.queryServer = keeper.NewQueryServer(*s.feemarketKeeper) } @@ -123,19 +126,37 @@ func (s *AnteTestSuite) TestDeductCoins() { accs := s.CreateTestAccounts(1) tests := []struct { - name string - acc TestAccount - coins sdk.Coins - wantErr bool + name string + acc TestAccount + coins sdk.Coins + wantErr bool + invalidCoin bool }{ { - name: "valid no coins", - acc: accs[0], + name: "valid", + acc: accs[0], + coins: sdk.NewCoins(sdk.NewCoin("test", sdk.NewInt(10))), + wantErr: false, + }, + { + name: "valid no coins", + acc: accs[0], + coins: sdk.NewCoins(), + wantErr: false, + }, + { + name: "invalid coins", + acc: accs[0], + coins: sdk.Coins{sdk.Coin{Amount: sdk.NewInt(-1)}}, + wantErr: true, + invalidCoin: true, }, } for _, tc := range tests { s.Run(tc.name, func() { - s.bankKeeper.On("SendCoinsFromAccountToModule", s.ctx, tc.acc.acc.GetAddress(), types.FeeCollectorName, tc.coins).Return(nil) + if !tc.invalidCoin { + s.bankKeeper.On("SendCoinsFromAccountToModule", s.ctx, tc.acc.acc.GetAddress(), types.FeeCollectorName, tc.coins).Return(nil).Once() + } if err := ante.DeductCoins(s.bankKeeper, s.ctx, tc.acc.acc, tc.coins); (err != nil) != tc.wantErr { s.Errorf(err, "DeductCoins() error = %v, wantErr %v", err, tc.wantErr) @@ -143,3 +164,6 @@ func (s *AnteTestSuite) TestDeductCoins() { }) } } + +func (s *AnteTestSuite) TestAnteHandle() { +} From b62d82c392fb7c4261dd3e271755179b5fea929e Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 20 Nov 2023 17:09:21 -0500 Subject: [PATCH 30/55] attempt test --- x/feemarket/ante/fee.go | 6 +- x/feemarket/ante/fee_test.go | 161 ++++++++++++++++++++++++++++++++++- 2 files changed, 162 insertions(+), 5 deletions(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 906e033..2e22865 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -59,11 +59,11 @@ func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return ctx, errorsmod.Wrapf(err, "unable to get fee market state") } - minGasPricesDec := sdk.NewDecCoinsFromCoins(minGasPrices...) + minGasPricesDecCoins := sdk.NewDecCoinsFromCoins(minGasPrices...) fee := feeTx.GetFee() if !simulate { - fee, tip, priority, err = checkTxFees(ctx, minGasPricesDec, tx) + fee, tip, priority, err = checkTxFees(ctx, minGasPricesDecCoins, tx) if err != nil { return ctx, err } @@ -73,7 +73,7 @@ func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return ctx, err } - newCtx := ctx.WithPriority(priority).WithMinGasPrices(minGasPricesDec) + newCtx := ctx.WithPriority(priority).WithMinGasPrices(minGasPricesDecCoins) return next(newCtx, tx, simulate) } diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go index 6eb19eb..c09aa0b 100644 --- a/x/feemarket/ante/fee_test.go +++ b/x/feemarket/ante/fee_test.go @@ -1,6 +1,10 @@ package ante_test import ( + "github.com/cosmos/cosmos-sdk/client/tx" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + "github.com/stretchr/testify/require" "testing" "github.com/cosmos/cosmos-sdk/client" @@ -23,8 +27,8 @@ import ( type AnteTestSuite struct { suite.Suite - anteHandler sdk.AnteHandler ctx sdk.Context + anteHandler sdk.AnteHandler clientCtx client.Context txBuilder client.TxBuilder @@ -118,8 +122,115 @@ func (s *AnteTestSuite) SetupTest() { s.queryServer = keeper.NewQueryServer(*s.feemarketKeeper) } -func (s *AnteTestSuite) DeductCoins(t *testing.T) { +// TestCase represents a test case used in test tables. +type TestCase struct { + desc string + malleate func(*AnteTestSuite) TestCaseArgs + simulate bool + expPass bool + expErr error +} + +type TestCaseArgs struct { + chainID string + accNums []uint64 + accSeqs []uint64 + feeAmount sdk.Coins + gasLimit uint64 + msgs []sdk.Msg + privs []cryptotypes.PrivKey +} + +// DeliverMsgs constructs a tx and runs it through the ante handler. This is used to set the context for a test case, for +// example to test for replay protection. +func (s *AnteTestSuite) DeliverMsgs(t *testing.T, privs []cryptotypes.PrivKey, msgs []sdk.Msg, feeAmount sdk.Coins, gasLimit uint64, accNums, accSeqs []uint64, chainID string, simulate bool) (sdk.Context, error) { + require.NoError(t, s.txBuilder.SetMsgs(msgs...)) + s.txBuilder.SetFeeAmount(feeAmount) + s.txBuilder.SetGasLimit(gasLimit) + tx, txErr := s.CreateTestTx(privs, accNums, accSeqs, chainID) + require.NoError(t, txErr) + return s.anteHandler(s.ctx, tx, simulate) +} + +func (s *AnteTestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { + require.NoError(t, s.txBuilder.SetMsgs(args.msgs...)) + s.txBuilder.SetFeeAmount(args.feeAmount) + s.txBuilder.SetGasLimit(args.gasLimit) + + // Theoretically speaking, ante handler unit tests should only test + // ante handlers, but here we sometimes also test the tx creation + // process. + tx, txErr := s.CreateTestTx(args.privs, args.accNums, args.accSeqs, args.chainID) + newCtx, anteErr := s.anteHandler(s.ctx, tx, tc.simulate) + + if tc.expPass { + require.NoError(t, txErr) + require.NoError(t, anteErr) + require.NotNil(t, newCtx) + + s.ctx = newCtx + } else { + switch { + case txErr != nil: + require.Error(t, txErr) + require.ErrorIs(t, txErr, tc.expErr) + + case anteErr != nil: + require.Error(t, anteErr) + require.ErrorIs(t, anteErr, tc.expErr) + + default: + t.Fatal("expected one of txErr, anteErr to be an error") + } + } +} + +// CreateTestTx is a helper function to create a tx given multiple inputs. +func (s *AnteTestSuite) CreateTestTx(privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (xauthsigning.Tx, error) { + // First round: we gather all the signer infos. We use the "set empty + // signature" hack to do that. + var sigsV2 []signing.SignatureV2 + for i, priv := range privs { + sigV2 := signing.SignatureV2{ + PubKey: priv.PubKey(), + Data: &signing.SingleSignatureData{ + SignMode: s.clientCtx.TxConfig.SignModeHandler().DefaultMode(), + Signature: nil, + }, + Sequence: accSeqs[i], + } + + sigsV2 = append(sigsV2, sigV2) + } + err := s.txBuilder.SetSignatures(sigsV2...) + if err != nil { + return nil, err + } + + // Second round: all signer infos are set, so each signer can sign. + sigsV2 = []signing.SignatureV2{} + for i, priv := range privs { + signerData := xauthsigning.SignerData{ + ChainID: chainID, + AccountNumber: accNums[i], + Sequence: accSeqs[i], + } + sigV2, err := tx.SignWithPrivKey( + s.clientCtx.TxConfig.SignModeHandler().DefaultMode(), signerData, + s.txBuilder, priv, s.clientCtx.TxConfig, accSeqs[i]) + if err != nil { + return nil, err + } + + sigsV2 = append(sigsV2, sigV2) + } + err = s.txBuilder.SetSignatures(sigsV2...) + if err != nil { + return nil, err + } + + return s.txBuilder.GetTx(), nil } func (s *AnteTestSuite) TestDeductCoins() { @@ -166,4 +277,50 @@ func (s *AnteTestSuite) TestDeductCoins() { } func (s *AnteTestSuite) TestAnteHandle() { + // Same data for every test cases + feeAmount := testdata.NewTestFeeAmount() + gasLimit := testdata.NewTestGasLimit() + + testCases := []TestCase{ + { + "signer has no funds", + func(suite *AnteTestSuite) TestCaseArgs { + accs := suite.CreateTestAccounts(1) + suite.bankKeeper.On("SendCoinsFromAccountToModule", s.ctx, accs[0].acc.GetAddress(), types.FeeCollectorName, feeAmount).Return(sdkerrors.ErrInsufficientFunds) + + return TestCaseArgs{ + msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].acc.GetAddress())}, + }.WithAccountsInfo(accs) + }, + false, + false, + sdkerrors.ErrInsufficientFunds, + }, + { + "signer has enough funds, should pass", + func(suite *AnteTestSuite) TestCaseArgs { + accs := suite.CreateTestAccounts(1) + suite.bankKeeper.On("SendCoinsFromAccountToModule", s.ctx, accs[0].acc.GetAddress(), types.FeeCollectorName, feeAmount).Return(nil) + + return TestCaseArgs{ + msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].acc.GetAddress())}, + }.WithAccountsInfo(accs) + }, + false, + true, + nil, + }, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("Case %s", tc.desc), func(t *testing.T) { + suite := SetupTestSuite(t, false) + suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() + args := tc.malleate(suite) + args.feeAmount = feeAmount + args.gasLimit = gasLimit + + suite.RunTestCase(t, tc, args) + }) + } } From dcd74615e3db17bb2c40a17793f35d90e589c267 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Tue, 21 Nov 2023 10:12:43 -0500 Subject: [PATCH 31/55] fmt --- tests/simapp/ante.go | 19 +- tests/simapp/app.go | 2 +- testutils/testutils.go | 3 +- x/feemarket/ante/ante_test.go | 273 ++++++++++++++++++ x/feemarket/ante/fee_test.go | 268 ++--------------- x/feemarket/ante/mocks/mock_account_keeper.go | 3 +- x/feemarket/ante/mocks/mock_bank_keeper.go | 3 +- .../ante/mocks/mock_feegrant_keeper.go | 3 +- .../ante/mocks/mock_feemarket_keeper.go | 6 +- x/feemarket/types/eip1559.go | 2 +- 10 files changed, 320 insertions(+), 262 deletions(-) create mode 100644 x/feemarket/ante/ante_test.go diff --git a/tests/simapp/ante.go b/tests/simapp/ante.go index f1eee9e..24ae0a4 100644 --- a/tests/simapp/ante.go +++ b/tests/simapp/ante.go @@ -13,6 +13,7 @@ import ( // HandlerOptions are the options required for constructing an SDK AnteHandler with the fee market injected. type HandlerOptions struct { BaseOptions authante.HandlerOptions + AccountKeeper feemarketante.AccountKeeper FeeMarketKeeper feemarketante.FeeMarketKeeper } @@ -20,7 +21,7 @@ type HandlerOptions struct { // numbers, checks signatures & account numbers, and deducts fees from the first // signer. func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { - if options.BaseOptions.AccountKeeper == nil { + if options.AccountKeeper == nil { return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "account keeper is required for ante builder") } @@ -37,19 +38,19 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { authante.NewExtensionOptionsDecorator(options.BaseOptions.ExtensionOptionChecker), authante.NewValidateBasicDecorator(), authante.NewTxTimeoutHeightDecorator(), - authante.NewValidateMemoDecorator(options.BaseOptions.AccountKeeper), - authante.NewConsumeGasForTxSizeDecorator(options.BaseOptions.AccountKeeper), + authante.NewValidateMemoDecorator(options.AccountKeeper), + authante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper), feemarketante.NewFeeMarketDecorator( // fee market replaces fee deduct decorator - options.BaseOptions.AccountKeeper, + options.AccountKeeper, options.BaseOptions.BankKeeper, options.BaseOptions.FeegrantKeeper, options.FeeMarketKeeper, ), - authante.NewSetPubKeyDecorator(options.BaseOptions.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators - authante.NewValidateSigCountDecorator(options.BaseOptions.AccountKeeper), - authante.NewSigGasConsumeDecorator(options.BaseOptions.AccountKeeper, options.BaseOptions.SigGasConsumer), - authante.NewSigVerificationDecorator(options.BaseOptions.AccountKeeper, options.BaseOptions.SignModeHandler), - authante.NewIncrementSequenceDecorator(options.BaseOptions.AccountKeeper), + authante.NewSetPubKeyDecorator(options.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators + authante.NewValidateSigCountDecorator(options.AccountKeeper), + authante.NewSigGasConsumeDecorator(options.AccountKeeper, options.BaseOptions.SigGasConsumer), + authante.NewSigVerificationDecorator(options.AccountKeeper, options.BaseOptions.SignModeHandler), + authante.NewIncrementSequenceDecorator(options.AccountKeeper), } return sdk.ChainAnteDecorators(anteDecorators...), nil diff --git a/tests/simapp/app.go b/tests/simapp/app.go index a809174..607ce2d 100644 --- a/tests/simapp/app.go +++ b/tests/simapp/app.go @@ -264,7 +264,6 @@ func NewSimApp( // ---------------------------------------------------------------------------- // handlerOptions := ante.HandlerOptions{ - AccountKeeper: app.AccountKeeper, BankKeeper: app.BankKeeper, FeegrantKeeper: app.FeeGrantKeeper, SigGasConsumer: ante.DefaultSigVerificationGasConsumer, @@ -273,6 +272,7 @@ func NewSimApp( options := HandlerOptions{ BaseOptions: handlerOptions, + AccountKeeper: app.AccountKeeper, FeeMarketKeeper: app.FeeMarketKeeper, } anteHandler, err := NewAnteHandler(options) diff --git a/testutils/testutils.go b/testutils/testutils.go index ef1eefb..2ed04ae 100644 --- a/testutils/testutils.go +++ b/testutils/testutils.go @@ -1,9 +1,10 @@ package testutils import ( - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "math/rand" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/types" diff --git a/x/feemarket/ante/ante_test.go b/x/feemarket/ante/ante_test.go new file mode 100644 index 0000000..a6bb4bf --- /dev/null +++ b/x/feemarket/ante/ante_test.go @@ -0,0 +1,273 @@ +package ante_test + +import ( + "testing" + + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/client/tx" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + storetypes "github.com/cosmos/cosmos-sdk/store/types" + "github.com/cosmos/cosmos-sdk/testutil" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + authante "github.com/cosmos/cosmos-sdk/x/auth/ante" + authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/skip-mev/feemarket/testutils" + feemarketante "github.com/skip-mev/feemarket/x/feemarket/ante" + "github.com/skip-mev/feemarket/x/feemarket/ante/mocks" + "github.com/skip-mev/feemarket/x/feemarket/keeper" + "github.com/skip-mev/feemarket/x/feemarket/types" +) + +type AnteTestSuite struct { + suite.Suite + ctx sdk.Context + anteHandler sdk.AnteHandler + clientCtx client.Context + txBuilder client.TxBuilder + + accountKeeper authkeeper.AccountKeeper + bankKeeper *mocks.BankKeeper + feeGrantKeeper *mocks.FeeGrantKeeper + feemarketKeeper *keeper.Keeper + encCfg testutils.EncodingConfig + key *storetypes.KVStoreKey + authorityAccount sdk.AccAddress + + // Message server variables + msgServer types.MsgServer + + // Query server variables + queryServer types.QueryServer +} + +// TestAccount represents an account used in the tests in x/auth/ante. +type TestAccount struct { + acc authtypes.AccountI + priv cryptotypes.PrivKey +} + +func (s *AnteTestSuite) CreateTestAccounts(numAccs int) []TestAccount { + var accounts []TestAccount + + for i := 0; i < numAccs; i++ { + priv, _, addr := testdata.KeyTestPubAddr() + acc := s.accountKeeper.NewAccountWithAddress(s.ctx, addr) + err := acc.SetAccountNumber(uint64(i + 1000)) + if err != nil { + panic(err) + } + s.accountKeeper.SetAccount(s.ctx, acc) + accounts = append(accounts, TestAccount{acc, priv}) + } + + return accounts +} + +// SetupTest setups a new test, with new app, context, and anteHandler. +func SetupTestSuite(t *testing.T, isCheckTx bool) *AnteTestSuite { + s := &AnteTestSuite{} + + s.encCfg = testutils.CreateTestEncodingConfig() + s.key = storetypes.NewKVStoreKey(types.StoreKey) + tkey := storetypes.NewTransientStoreKey("transient_test_feemarket") + testCtx := testutil.DefaultContextWithDB(t, s.key, tkey) + s.ctx = testCtx.Ctx.WithIsCheckTx(isCheckTx).WithBlockHeight(1) + cms, db := testCtx.CMS, testCtx.DB + + authKey := storetypes.NewKVStoreKey(authtypes.StoreKey) + tkey = storetypes.NewTransientStoreKey("transient_test_auth") + cms.MountStoreWithDB(authKey, storetypes.StoreTypeIAVL, db) + cms.MountStoreWithDB(tkey, storetypes.StoreTypeTransient, db) + err := cms.LoadLatestVersion() + require.NoError(t, err) + + maccPerms := map[string][]string{ + "fee_collector": nil, + types.ModuleName: nil, + types.FeeCollectorName: {"burner"}, + "mint": {"minter"}, + "bonded_tokens_pool": {"burner", "staking"}, + "not_bonded_tokens_pool": {"burner", "staking"}, + "multiPerm": {"burner", "minter", "staking"}, + "random": {"random"}, + } + + s.authorityAccount = authtypes.NewModuleAddress("gov") + s.accountKeeper = authkeeper.NewAccountKeeper( + s.encCfg.Codec, authKey, authtypes.ProtoBaseAccount, maccPerms, sdk.Bech32MainPrefix, s.authorityAccount.String(), + ) + + s.feemarketKeeper = keeper.NewKeeper( + s.encCfg.Codec, + s.key, + s.accountKeeper, + s.authorityAccount.String(), + ) + + err = s.feemarketKeeper.SetParams(s.ctx, types.DefaultParams()) + require.NoError(t, err) + + err = s.feemarketKeeper.SetState(s.ctx, types.DefaultState()) + require.NoError(t, err) + + s.bankKeeper = mocks.NewBankKeeper(t) + s.feeGrantKeeper = mocks.NewFeeGrantKeeper(t) + + s.msgServer = keeper.NewMsgServer(*s.feemarketKeeper) + s.queryServer = keeper.NewQueryServer(*s.feemarketKeeper) + + s.clientCtx = client.Context{}.WithTxConfig(s.encCfg.TxConfig) + s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() + + anteDecorators := []sdk.AnteDecorator{ + authante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first + // authante.NewExtensionOptionsDecorator(nil), + // authante.NewValidateBasicDecorator(), + // authante.NewTxTimeoutHeightDecorator(), + // authante.NewValidateMemoDecorator(s.accountKeeper), + // authante.NewConsumeGasForTxSizeDecorator(s.accountKeeper), + feemarketante.NewFeeMarketDecorator( // fee market replaces fee deduct decorator + s.accountKeeper, + s.bankKeeper, + s.feeGrantKeeper, + s.feemarketKeeper, + ), + // authante.NewSetPubKeyDecorator(s.accountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators + // authante.NewValidateSigCountDecorator(s.accountKeeper), + authante.NewSigGasConsumeDecorator(s.accountKeeper, authante.DefaultSigVerificationGasConsumer), + // authante.NewSigVerificationDecorator(s.accountKeeper, s.clientCtx.TxConfig.SignModeHandler()), + // authante.NewIncrementSequenceDecorator(s.accountKeeper), + } + + s.anteHandler = sdk.ChainAnteDecorators(anteDecorators...) + + return s +} + +// TestCase represents a test case used in test tables. +type TestCase struct { + name string + malleate func(*AnteTestSuite) TestCaseArgs + simulate bool + expPass bool + expErr error +} + +type TestCaseArgs struct { + chainID string + accNums []uint64 + accSeqs []uint64 + feeAmount sdk.Coins + gasLimit uint64 + msgs []sdk.Msg + privs []cryptotypes.PrivKey +} + +// DeliverMsgs constructs a tx and runs it through the ante handler. This is used to set the context for a test case, for +// example to test for replay protection. +func (s *AnteTestSuite) DeliverMsgs(t *testing.T, privs []cryptotypes.PrivKey, msgs []sdk.Msg, feeAmount sdk.Coins, gasLimit uint64, accNums, accSeqs []uint64, chainID string, simulate bool) (sdk.Context, error) { + require.NoError(t, s.txBuilder.SetMsgs(msgs...)) + s.txBuilder.SetFeeAmount(feeAmount) + s.txBuilder.SetGasLimit(gasLimit) + + tx, txErr := s.CreateTestTx(privs, accNums, accSeqs, chainID) + require.NoError(t, txErr) + return s.anteHandler(s.ctx, tx, simulate) +} + +func (s *AnteTestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { + require.NoError(t, s.txBuilder.SetMsgs(args.msgs...)) + s.txBuilder.SetFeeAmount(args.feeAmount) + s.txBuilder.SetGasLimit(args.gasLimit) + + // Theoretically speaking, ante handler unit tests should only test + // ante handlers, but here we sometimes also test the tx creation + // process. + tx, txErr := s.CreateTestTx(args.privs, args.accNums, args.accSeqs, args.chainID) + newCtx, anteErr := s.anteHandler(s.ctx, tx, tc.simulate) + + if tc.expPass { + require.NoError(t, txErr) + require.NoError(t, anteErr) + require.NotNil(t, newCtx) + + s.ctx = newCtx + } else { + switch { + case txErr != nil: + require.Error(t, txErr) + require.ErrorIs(t, txErr, tc.expErr) + + case anteErr != nil: + require.Error(t, anteErr) + require.ErrorIs(t, anteErr, tc.expErr) + + default: + t.Fatal("expected one of txErr, anteErr to be an error") + } + } +} + +// CreateTestTx is a helper function to create a tx given multiple inputs. +func (s *AnteTestSuite) CreateTestTx(privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (authsigning.Tx, error) { + // First round: we gather all the signer infos. We use the "set empty + // signature" hack to do that. + var sigsV2 []signing.SignatureV2 + for i, priv := range privs { + sigV2 := signing.SignatureV2{ + PubKey: priv.PubKey(), + Data: &signing.SingleSignatureData{ + SignMode: s.clientCtx.TxConfig.SignModeHandler().DefaultMode(), + Signature: nil, + }, + Sequence: accSeqs[i], + } + + sigsV2 = append(sigsV2, sigV2) + } + err := s.txBuilder.SetSignatures(sigsV2...) + if err != nil { + return nil, err + } + + // Second round: all signer infos are set, so each signer can sign. + sigsV2 = []signing.SignatureV2{} + for i, priv := range privs { + signerData := authsigning.SignerData{ + ChainID: chainID, + AccountNumber: accNums[i], + Sequence: accSeqs[i], + } + sigV2, err := tx.SignWithPrivKey( + s.clientCtx.TxConfig.SignModeHandler().DefaultMode(), signerData, + s.txBuilder, priv, s.clientCtx.TxConfig, accSeqs[i]) + if err != nil { + return nil, err + } + + sigsV2 = append(sigsV2, sigV2) + } + err = s.txBuilder.SetSignatures(sigsV2...) + if err != nil { + return nil, err + } + + return s.txBuilder.GetTx(), nil +} + +// NewTestFeeAmount is a test fee amount. +func NewTestFeeAmount() sdk.Coins { + return sdk.NewCoins(sdk.NewInt64Coin("stake", 150)) +} + +// NewTestGasLimit is a test fee gas limit. +func NewTestGasLimit() uint64 { + return 200000 +} diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go index c09aa0b..fab1ab4 100644 --- a/x/feemarket/ante/fee_test.go +++ b/x/feemarket/ante/fee_test.go @@ -1,296 +1,74 @@ package ante_test import ( - "github.com/cosmos/cosmos-sdk/client/tx" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - "github.com/cosmos/cosmos-sdk/types/tx/signing" - "github.com/stretchr/testify/require" + "fmt" "testing" - "github.com/cosmos/cosmos-sdk/client" - cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" - storetypes "github.com/cosmos/cosmos-sdk/store/types" - "github.com/cosmos/cosmos-sdk/testutil" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" _ "github.com/cosmos/cosmos-sdk/x/auth" - authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - "github.com/stretchr/testify/suite" + "github.com/stretchr/testify/mock" - "github.com/skip-mev/feemarket/testutils" "github.com/skip-mev/feemarket/x/feemarket/ante" - "github.com/skip-mev/feemarket/x/feemarket/ante/mocks" - "github.com/skip-mev/feemarket/x/feemarket/keeper" "github.com/skip-mev/feemarket/x/feemarket/types" ) -type AnteTestSuite struct { - suite.Suite - ctx sdk.Context - anteHandler sdk.AnteHandler - clientCtx client.Context - txBuilder client.TxBuilder - - accountKeeper authkeeper.AccountKeeper - bankKeeper *mocks.BankKeeper - feeGrantKeeper *mocks.FeeGrantKeeper - feemarketKeeper *keeper.Keeper - encCfg testutils.EncodingConfig - key *storetypes.KVStoreKey - authorityAccount sdk.AccAddress - - // Message server variables - msgServer types.MsgServer - - // Query server variables - queryServer types.QueryServer -} - -func TestAnteTestSuite(t *testing.T) { - suite.Run(t, new(AnteTestSuite)) -} - -// TestAccount represents an account used in the tests in x/auth/ante. -type TestAccount struct { - acc authtypes.AccountI - priv cryptotypes.PrivKey -} - -func (s *AnteTestSuite) CreateTestAccounts(numAccs int) []TestAccount { - var accounts []TestAccount - - for i := 0; i < numAccs; i++ { - priv, _, addr := testdata.KeyTestPubAddr() - acc := s.accountKeeper.NewAccountWithAddress(s.ctx, addr) - err := acc.SetAccountNumber(uint64(i + 1000)) - if err != nil { - panic(err) - } - s.accountKeeper.SetAccount(s.ctx, acc) - accounts = append(accounts, TestAccount{acc, priv}) - } - - return accounts -} - -func (s *AnteTestSuite) SetupTest() { - s.encCfg = testutils.CreateTestEncodingConfig() - s.key = storetypes.NewKVStoreKey(types.StoreKey) - tkey := storetypes.NewTransientStoreKey("transient_test_feemarket") - testCtx := testutil.DefaultContextWithDB(s.T(), s.key, tkey) - s.ctx = testCtx.Ctx.WithIsCheckTx(true).WithBlockHeight(1) - cms, db := testCtx.CMS, testCtx.DB - - authKey := storetypes.NewKVStoreKey(authtypes.StoreKey) - tkey = storetypes.NewTransientStoreKey("transient_test_auth") - cms.MountStoreWithDB(authKey, storetypes.StoreTypeIAVL, db) - cms.MountStoreWithDB(tkey, storetypes.StoreTypeTransient, db) - err := cms.LoadLatestVersion() - s.Require().NoError(err) - - maccPerms := map[string][]string{ - "fee_collector": nil, - types.ModuleName: nil, - types.FeeCollectorName: {"burner"}, - "mint": {"minter"}, - "bonded_tokens_pool": {"burner", "staking"}, - "not_bonded_tokens_pool": {"burner", "staking"}, - "multiPerm": {"burner", "minter", "staking"}, - "random": {"random"}, - } - - s.authorityAccount = authtypes.NewModuleAddress("gov") - s.accountKeeper = authkeeper.NewAccountKeeper( - s.encCfg.Codec, authKey, authtypes.ProtoBaseAccount, maccPerms, sdk.Bech32MainPrefix, s.authorityAccount.String(), - ) - - s.feemarketKeeper = keeper.NewKeeper( - s.encCfg.Codec, - s.key, - s.accountKeeper, - s.authorityAccount.String(), - ) - - err = s.feemarketKeeper.SetParams(s.ctx, types.DefaultParams()) - s.Require().NoError(err) - - s.bankKeeper = mocks.NewBankKeeper(s.T()) - s.feeGrantKeeper = mocks.NewFeeGrantKeeper(s.T()) - - s.msgServer = keeper.NewMsgServer(*s.feemarketKeeper) - s.queryServer = keeper.NewQueryServer(*s.feemarketKeeper) -} - -// TestCase represents a test case used in test tables. -type TestCase struct { - desc string - malleate func(*AnteTestSuite) TestCaseArgs - simulate bool - expPass bool - expErr error -} - -type TestCaseArgs struct { - chainID string - accNums []uint64 - accSeqs []uint64 - feeAmount sdk.Coins - gasLimit uint64 - msgs []sdk.Msg - privs []cryptotypes.PrivKey -} - -// DeliverMsgs constructs a tx and runs it through the ante handler. This is used to set the context for a test case, for -// example to test for replay protection. -func (s *AnteTestSuite) DeliverMsgs(t *testing.T, privs []cryptotypes.PrivKey, msgs []sdk.Msg, feeAmount sdk.Coins, gasLimit uint64, accNums, accSeqs []uint64, chainID string, simulate bool) (sdk.Context, error) { - require.NoError(t, s.txBuilder.SetMsgs(msgs...)) - s.txBuilder.SetFeeAmount(feeAmount) - s.txBuilder.SetGasLimit(gasLimit) - - tx, txErr := s.CreateTestTx(privs, accNums, accSeqs, chainID) - require.NoError(t, txErr) - return s.anteHandler(s.ctx, tx, simulate) -} - -func (s *AnteTestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { - require.NoError(t, s.txBuilder.SetMsgs(args.msgs...)) - s.txBuilder.SetFeeAmount(args.feeAmount) - s.txBuilder.SetGasLimit(args.gasLimit) - - // Theoretically speaking, ante handler unit tests should only test - // ante handlers, but here we sometimes also test the tx creation - // process. - tx, txErr := s.CreateTestTx(args.privs, args.accNums, args.accSeqs, args.chainID) - newCtx, anteErr := s.anteHandler(s.ctx, tx, tc.simulate) - - if tc.expPass { - require.NoError(t, txErr) - require.NoError(t, anteErr) - require.NotNil(t, newCtx) - - s.ctx = newCtx - } else { - switch { - case txErr != nil: - require.Error(t, txErr) - require.ErrorIs(t, txErr, tc.expErr) - - case anteErr != nil: - require.Error(t, anteErr) - require.ErrorIs(t, anteErr, tc.expErr) - - default: - t.Fatal("expected one of txErr, anteErr to be an error") - } - } -} - -// CreateTestTx is a helper function to create a tx given multiple inputs. -func (s *AnteTestSuite) CreateTestTx(privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (xauthsigning.Tx, error) { - // First round: we gather all the signer infos. We use the "set empty - // signature" hack to do that. - var sigsV2 []signing.SignatureV2 - for i, priv := range privs { - sigV2 := signing.SignatureV2{ - PubKey: priv.PubKey(), - Data: &signing.SingleSignatureData{ - SignMode: s.clientCtx.TxConfig.SignModeHandler().DefaultMode(), - Signature: nil, - }, - Sequence: accSeqs[i], - } - - sigsV2 = append(sigsV2, sigV2) - } - err := s.txBuilder.SetSignatures(sigsV2...) - if err != nil { - return nil, err - } - - // Second round: all signer infos are set, so each signer can sign. - sigsV2 = []signing.SignatureV2{} - for i, priv := range privs { - signerData := xauthsigning.SignerData{ - ChainID: chainID, - AccountNumber: accNums[i], - Sequence: accSeqs[i], - } - sigV2, err := tx.SignWithPrivKey( - s.clientCtx.TxConfig.SignModeHandler().DefaultMode(), signerData, - s.txBuilder, priv, s.clientCtx.TxConfig, accSeqs[i]) - if err != nil { - return nil, err - } - - sigsV2 = append(sigsV2, sigV2) - } - err = s.txBuilder.SetSignatures(sigsV2...) - if err != nil { - return nil, err - } - - return s.txBuilder.GetTx(), nil -} - -func (s *AnteTestSuite) TestDeductCoins() { - accs := s.CreateTestAccounts(1) - +func TestDeductCoins(t *testing.T) { tests := []struct { name string - acc TestAccount coins sdk.Coins wantErr bool invalidCoin bool }{ { name: "valid", - acc: accs[0], coins: sdk.NewCoins(sdk.NewCoin("test", sdk.NewInt(10))), wantErr: false, }, { name: "valid no coins", - acc: accs[0], coins: sdk.NewCoins(), wantErr: false, }, { name: "invalid coins", - acc: accs[0], coins: sdk.Coins{sdk.Coin{Amount: sdk.NewInt(-1)}}, wantErr: true, invalidCoin: true, }, } for _, tc := range tests { - s.Run(tc.name, func() { + t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { + s := SetupTestSuite(t, false) + acc := s.CreateTestAccounts(1)[0] if !tc.invalidCoin { - s.bankKeeper.On("SendCoinsFromAccountToModule", s.ctx, tc.acc.acc.GetAddress(), types.FeeCollectorName, tc.coins).Return(nil).Once() + s.bankKeeper.On("SendCoinsFromAccountToModule", s.ctx, acc.acc.GetAddress(), types.FeeCollectorName, tc.coins).Return(nil).Once() } - if err := ante.DeductCoins(s.bankKeeper, s.ctx, tc.acc.acc, tc.coins); (err != nil) != tc.wantErr { + if err := ante.DeductCoins(s.bankKeeper, s.ctx, acc.acc, tc.coins); (err != nil) != tc.wantErr { s.Errorf(err, "DeductCoins() error = %v, wantErr %v", err, tc.wantErr) } }) } } -func (s *AnteTestSuite) TestAnteHandle() { +func TestAnteHandle(t *testing.T) { // Same data for every test cases - feeAmount := testdata.NewTestFeeAmount() - gasLimit := testdata.NewTestGasLimit() + gasLimit := NewTestGasLimit() + validFeeAmount := types.DefaultMinBaseFee.MulRaw(int64(gasLimit)) + validFee := sdk.NewCoins(sdk.NewCoin("stake", validFeeAmount)) testCases := []TestCase{ { "signer has no funds", func(suite *AnteTestSuite) TestCaseArgs { accs := suite.CreateTestAccounts(1) - suite.bankKeeper.On("SendCoinsFromAccountToModule", s.ctx, accs[0].acc.GetAddress(), types.FeeCollectorName, feeAmount).Return(sdkerrors.ErrInsufficientFunds) + suite.bankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].acc.GetAddress(), types.FeeCollectorName, mock.Anything).Return(sdkerrors.ErrInsufficientFunds) return TestCaseArgs{ msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].acc.GetAddress())}, - }.WithAccountsInfo(accs) + } }, false, false, @@ -300,11 +78,11 @@ func (s *AnteTestSuite) TestAnteHandle() { "signer has enough funds, should pass", func(suite *AnteTestSuite) TestCaseArgs { accs := suite.CreateTestAccounts(1) - suite.bankKeeper.On("SendCoinsFromAccountToModule", s.ctx, accs[0].acc.GetAddress(), types.FeeCollectorName, feeAmount).Return(nil) + suite.bankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].acc.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil) return TestCaseArgs{ msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].acc.GetAddress())}, - }.WithAccountsInfo(accs) + } }, false, true, @@ -313,14 +91,14 @@ func (s *AnteTestSuite) TestAnteHandle() { } for _, tc := range testCases { - t.Run(fmt.Sprintf("Case %s", tc.desc), func(t *testing.T) { - suite := SetupTestSuite(t, false) - suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() - args := tc.malleate(suite) - args.feeAmount = feeAmount + t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { + s := SetupTestSuite(t, false) + s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() + args := tc.malleate(s) + args.feeAmount = validFee args.gasLimit = gasLimit - suite.RunTestCase(t, tc, args) + s.RunTestCase(t, tc, args) }) } } diff --git a/x/feemarket/ante/mocks/mock_account_keeper.go b/x/feemarket/ante/mocks/mock_account_keeper.go index 207e219..fb833d3 100644 --- a/x/feemarket/ante/mocks/mock_account_keeper.go +++ b/x/feemarket/ante/mocks/mock_account_keeper.go @@ -102,7 +102,8 @@ func (_m *AccountKeeper) SetAccount(ctx types.Context, acc authtypes.AccountI) { func NewAccountKeeper(t interface { mock.TestingT Cleanup(func()) -}) *AccountKeeper { +}, +) *AccountKeeper { mock := &AccountKeeper{} mock.Mock.Test(t) diff --git a/x/feemarket/ante/mocks/mock_bank_keeper.go b/x/feemarket/ante/mocks/mock_bank_keeper.go index 351bafa..1cbcf2a 100644 --- a/x/feemarket/ante/mocks/mock_bank_keeper.go +++ b/x/feemarket/ante/mocks/mock_bank_keeper.go @@ -66,7 +66,8 @@ func (_m *BankKeeper) SendCoinsFromAccountToModule(ctx types.Context, senderAddr func NewBankKeeper(t interface { mock.TestingT Cleanup(func()) -}) *BankKeeper { +}, +) *BankKeeper { mock := &BankKeeper{} mock.Mock.Test(t) diff --git a/x/feemarket/ante/mocks/mock_feegrant_keeper.go b/x/feemarket/ante/mocks/mock_feegrant_keeper.go index 355ecd5..2a65489 100644 --- a/x/feemarket/ante/mocks/mock_feegrant_keeper.go +++ b/x/feemarket/ante/mocks/mock_feegrant_keeper.go @@ -31,7 +31,8 @@ func (_m *FeeGrantKeeper) UseGrantedFees(ctx types.Context, granter types.AccAdd func NewFeeGrantKeeper(t interface { mock.TestingT Cleanup(func()) -}) *FeeGrantKeeper { +}, +) *FeeGrantKeeper { mock := &FeeGrantKeeper{} mock.Mock.Test(t) diff --git a/x/feemarket/ante/mocks/mock_feemarket_keeper.go b/x/feemarket/ante/mocks/mock_feemarket_keeper.go index 3b1cd67..aa59895 100644 --- a/x/feemarket/ante/mocks/mock_feemarket_keeper.go +++ b/x/feemarket/ante/mocks/mock_feemarket_keeper.go @@ -3,9 +3,10 @@ package mocks import ( - feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" mock "github.com/stretchr/testify/mock" + feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" + types "github.com/cosmos/cosmos-sdk/types" ) @@ -69,7 +70,8 @@ func (_m *FeeMarketKeeper) GetState(ctx types.Context) (feemarkettypes.State, er func NewFeeMarketKeeper(t interface { mock.TestingT Cleanup(func()) -}) *FeeMarketKeeper { +}, +) *FeeMarketKeeper { mock := &FeeMarketKeeper{} mock.Mock.Test(t) diff --git a/x/feemarket/types/eip1559.go b/x/feemarket/types/eip1559.go index b2a2360..2d21af3 100644 --- a/x/feemarket/types/eip1559.go +++ b/x/feemarket/types/eip1559.go @@ -38,7 +38,7 @@ var ( // DefaultMinBaseFee is the default minimum base fee. This is the default // on Ethereum. Note that Ethereum is denominated in 1e18 wei. Cosmos chains will // likely want to change this to 1e6. - DefaultMinBaseFee = math.NewInt(1_000_000_000) + DefaultMinBaseFee = math.NewInt(1_000_000) // DefaultMinLearningRate is not used in the base EIP-1559 implementation. DefaultMinLearningRate = math.LegacyMustNewDecFromStr("0.125") From 96433e4d5af49102a33b3bbe42c173201c2bbbe6 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Tue, 21 Nov 2023 10:14:23 -0500 Subject: [PATCH 32/55] clean --- x/feemarket/ante/ante_test.go | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/x/feemarket/ante/ante_test.go b/x/feemarket/ante/ante_test.go index a6bb4bf..c853aa4 100644 --- a/x/feemarket/ante/ante_test.go +++ b/x/feemarket/ante/ante_test.go @@ -33,18 +33,12 @@ type AnteTestSuite struct { txBuilder client.TxBuilder accountKeeper authkeeper.AccountKeeper + feemarketKeeper *keeper.Keeper bankKeeper *mocks.BankKeeper feeGrantKeeper *mocks.FeeGrantKeeper - feemarketKeeper *keeper.Keeper encCfg testutils.EncodingConfig key *storetypes.KVStoreKey authorityAccount sdk.AccAddress - - // Message server variables - msgServer types.MsgServer - - // Query server variables - queryServer types.QueryServer } // TestAccount represents an account used in the tests in x/auth/ante. @@ -89,14 +83,8 @@ func SetupTestSuite(t *testing.T, isCheckTx bool) *AnteTestSuite { require.NoError(t, err) maccPerms := map[string][]string{ - "fee_collector": nil, - types.ModuleName: nil, - types.FeeCollectorName: {"burner"}, - "mint": {"minter"}, - "bonded_tokens_pool": {"burner", "staking"}, - "not_bonded_tokens_pool": {"burner", "staking"}, - "multiPerm": {"burner", "minter", "staking"}, - "random": {"random"}, + types.ModuleName: nil, + types.FeeCollectorName: {"burner"}, } s.authorityAccount = authtypes.NewModuleAddress("gov") @@ -120,34 +108,22 @@ func SetupTestSuite(t *testing.T, isCheckTx bool) *AnteTestSuite { s.bankKeeper = mocks.NewBankKeeper(t) s.feeGrantKeeper = mocks.NewFeeGrantKeeper(t) - s.msgServer = keeper.NewMsgServer(*s.feemarketKeeper) - s.queryServer = keeper.NewQueryServer(*s.feemarketKeeper) - s.clientCtx = client.Context{}.WithTxConfig(s.encCfg.TxConfig) s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() + // create basic antehandler with the feemarket decorator anteDecorators := []sdk.AnteDecorator{ authante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first - // authante.NewExtensionOptionsDecorator(nil), - // authante.NewValidateBasicDecorator(), - // authante.NewTxTimeoutHeightDecorator(), - // authante.NewValidateMemoDecorator(s.accountKeeper), - // authante.NewConsumeGasForTxSizeDecorator(s.accountKeeper), feemarketante.NewFeeMarketDecorator( // fee market replaces fee deduct decorator s.accountKeeper, s.bankKeeper, s.feeGrantKeeper, s.feemarketKeeper, ), - // authante.NewSetPubKeyDecorator(s.accountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators - // authante.NewValidateSigCountDecorator(s.accountKeeper), authante.NewSigGasConsumeDecorator(s.accountKeeper, authante.DefaultSigVerificationGasConsumer), - // authante.NewSigVerificationDecorator(s.accountKeeper, s.clientCtx.TxConfig.SignModeHandler()), - // authante.NewIncrementSequenceDecorator(s.accountKeeper), } s.anteHandler = sdk.ChainAnteDecorators(anteDecorators...) - return s } From ef715a08ff7e9c4ae3438c7cdd241129b9188e40 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Tue, 21 Nov 2023 11:18:49 -0500 Subject: [PATCH 33/55] add --- x/feemarket/ante/fee.go | 2 +- x/feemarket/ante/fee_test.go | 31 ++++++++++++++++++++++++------- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 2e22865..718dbfe 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -184,7 +184,7 @@ func checkTxFees(_ sdk.Context, fees sdk.DecCoins, tx sdk.Tx) (feeCoins sdk.Coin } tip = feeCoins.Sub(requiredFees...) // tip is whatever is left - feeCoins = requiredFees // set free coins to be ONLY the required amount + feeCoins = requiredFees // set fee coins to be ONLY the required amount } priority = getTxPriority(feeCoins, int64(gas)) diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go index fab1ab4..17b2783 100644 --- a/x/feemarket/ante/fee_test.go +++ b/x/feemarket/ante/fee_test.go @@ -54,7 +54,7 @@ func TestDeductCoins(t *testing.T) { } func TestAnteHandle(t *testing.T) { - // Same data for every test cases + // Same data for every test case gasLimit := NewTestGasLimit() validFeeAmount := types.DefaultMinBaseFee.MulRaw(int64(gasLimit)) validFee := sdk.NewCoins(sdk.NewCoin("stake", validFeeAmount)) @@ -64,24 +64,43 @@ func TestAnteHandle(t *testing.T) { "signer has no funds", func(suite *AnteTestSuite) TestCaseArgs { accs := suite.CreateTestAccounts(1) - suite.bankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].acc.GetAddress(), types.FeeCollectorName, mock.Anything).Return(sdkerrors.ErrInsufficientFunds) + suite.bankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].acc.GetAddress(), types.FeeCollectorName, mock.Anything).Return(sdkerrors.ErrInsufficientFunds).Once() return TestCaseArgs{ - msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].acc.GetAddress())}, + msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].acc.GetAddress())}, + gasLimit: gasLimit, + feeAmount: validFee, } }, false, false, sdkerrors.ErrInsufficientFunds, }, + { + "0 gas given should fail", + func(suite *AnteTestSuite) TestCaseArgs { + accs := suite.CreateTestAccounts(1) + + return TestCaseArgs{ + msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].acc.GetAddress())}, + gasLimit: 0, + feeAmount: validFee, + } + }, + false, + false, + sdkerrors.ErrInvalidGasLimit, + }, { "signer has enough funds, should pass", func(suite *AnteTestSuite) TestCaseArgs { accs := suite.CreateTestAccounts(1) - suite.bankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].acc.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil) + suite.bankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].acc.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil).Once() return TestCaseArgs{ - msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].acc.GetAddress())}, + msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].acc.GetAddress())}, + gasLimit: gasLimit, + feeAmount: validFee, } }, false, @@ -95,8 +114,6 @@ func TestAnteHandle(t *testing.T) { s := SetupTestSuite(t, false) s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() args := tc.malleate(s) - args.feeAmount = validFee - args.gasLimit = gasLimit s.RunTestCase(t, tc, args) }) From 6eb04d68435cd55648b32ebb6b4571d96521efe1 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Tue, 21 Nov 2023 11:33:23 -0500 Subject: [PATCH 34/55] fix --- go.mod | 2 +- x/feemarket/ante/ante_test.go | 4 ++-- x/feemarket/ante/fee.go | 7 ++++--- x/feemarket/ante/fee_test.go | 4 ++-- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index d2a35c1..8fa1f93 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( cosmossdk.io/api v0.7.2 cosmossdk.io/core v0.11.0 cosmossdk.io/depinject v1.0.0-alpha.4 + cosmossdk.io/errors v1.0.0 cosmossdk.io/log v1.2.1 cosmossdk.io/math v1.1.3-rc.1 github.com/client9/misspell v0.3.4 @@ -35,7 +36,6 @@ require ( cloud.google.com/go/compute/metadata v0.2.3 // indirect cloud.google.com/go/iam v1.1.4 // indirect cloud.google.com/go/storage v1.30.1 // indirect - cosmossdk.io/errors v1.0.0 // indirect cosmossdk.io/tools/rosetta v0.2.1 // indirect filippo.io/edwards25519 v1.0.0 // indirect github.com/4meepo/tagalign v1.3.3 // indirect diff --git a/x/feemarket/ante/ante_test.go b/x/feemarket/ante/ante_test.go index c853aa4..0197ca9 100644 --- a/x/feemarket/ante/ante_test.go +++ b/x/feemarket/ante/ante_test.go @@ -65,14 +65,14 @@ func (s *AnteTestSuite) CreateTestAccounts(numAccs int) []TestAccount { } // SetupTest setups a new test, with new app, context, and anteHandler. -func SetupTestSuite(t *testing.T, isCheckTx bool) *AnteTestSuite { +func SetupTestSuite(t *testing.T) *AnteTestSuite { s := &AnteTestSuite{} s.encCfg = testutils.CreateTestEncodingConfig() s.key = storetypes.NewKVStoreKey(types.StoreKey) tkey := storetypes.NewTransientStoreKey("transient_test_feemarket") testCtx := testutil.DefaultContextWithDB(t, s.key, tkey) - s.ctx = testCtx.Ctx.WithIsCheckTx(isCheckTx).WithBlockHeight(1) + s.ctx = testCtx.Ctx.WithIsCheckTx(false).WithBlockHeight(1) cms, db := testCtx.CMS, testCtx.DB authKey := storetypes.NewKVStoreKey(authtypes.StoreKey) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 718dbfe..b8e2bdd 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -21,6 +21,7 @@ type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) // FeeMarketDecorator deducts fees from the fee payer based off of the current state of the feemarket. // The fee payer is the fee granter (if specified) or first signer of the tx. // If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. +// If there is an excess between the given fee and the on-chain base fee is given as a tip. // Call next AnteHandler if fees successfully deducted. // CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator type FeeMarketDecorator struct { @@ -63,7 +64,7 @@ func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo fee := feeTx.GetFee() if !simulate { - fee, tip, priority, err = checkTxFees(ctx, minGasPricesDecCoins, tx) + fee, tip, priority, err = checkTxFees(minGasPricesDecCoins, tx) if err != nil { return ctx, err } @@ -157,7 +158,7 @@ func DeductCoins(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, // checkTxFees implements the logic for the fee market to check if a Tx has provided suffucient // fees given the current state of the fee market. Returns an error if insufficient fees. -func checkTxFees(_ sdk.Context, fees sdk.DecCoins, tx sdk.Tx) (feeCoins sdk.Coins, tip sdk.Coins, priority int64, err error) { +func checkTxFees(fees sdk.DecCoins, tx sdk.Tx) (feeCoins sdk.Coins, tip sdk.Coins, priority int64, err error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { return nil, nil, 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") @@ -194,7 +195,7 @@ func checkTxFees(_ sdk.Context, fees sdk.DecCoins, tx sdk.Tx) (feeCoins sdk.Coin // getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the gas price // provided in a transaction. // NOTE: This implementation should be used with a great consideration as it opens potential attack vectors -// where txs with multiple coins could not be prioritize as expected. +// where txs with multiple coins could not be prioritized as expected. func getTxPriority(fee sdk.Coins, gas int64) int64 { var priority int64 for _, c := range fee { diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go index 17b2783..110f0c8 100644 --- a/x/feemarket/ante/fee_test.go +++ b/x/feemarket/ante/fee_test.go @@ -40,7 +40,7 @@ func TestDeductCoins(t *testing.T) { } for _, tc := range tests { t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { - s := SetupTestSuite(t, false) + s := SetupTestSuite(t) acc := s.CreateTestAccounts(1)[0] if !tc.invalidCoin { s.bankKeeper.On("SendCoinsFromAccountToModule", s.ctx, acc.acc.GetAddress(), types.FeeCollectorName, tc.coins).Return(nil).Once() @@ -111,7 +111,7 @@ func TestAnteHandle(t *testing.T) { for _, tc := range testCases { t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { - s := SetupTestSuite(t, false) + s := SetupTestSuite(t) s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() args := tc.malleate(s) From 4e131c9d23c2e6fe6b4c34feae79b5839050cfd0 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 27 Nov 2023 11:12:48 -0500 Subject: [PATCH 35/55] rename --- tests/simapp/ante.go | 6 +++--- tests/simapp/app.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/simapp/ante.go b/tests/simapp/ante.go index 24ae0a4..1b53d61 100644 --- a/tests/simapp/ante.go +++ b/tests/simapp/ante.go @@ -10,8 +10,8 @@ import ( feemarketante "github.com/skip-mev/feemarket/x/feemarket/ante" ) -// HandlerOptions are the options required for constructing an SDK AnteHandler with the fee market injected. -type HandlerOptions struct { +// AnteHandlerOptions are the options required for constructing an SDK AnteHandler with the fee market injected. +type AnteHandlerOptions struct { BaseOptions authante.HandlerOptions AccountKeeper feemarketante.AccountKeeper FeeMarketKeeper feemarketante.FeeMarketKeeper @@ -20,7 +20,7 @@ type HandlerOptions struct { // NewAnteHandler returns an AnteHandler that checks and increments sequence // numbers, checks signatures & account numbers, and deducts fees from the first // signer. -func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { +func NewAnteHandler(options AnteHandlerOptions) (sdk.AnteHandler, error) { if options.AccountKeeper == nil { return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "account keeper is required for ante builder") } diff --git a/tests/simapp/app.go b/tests/simapp/app.go index 607ce2d..c74ae69 100644 --- a/tests/simapp/app.go +++ b/tests/simapp/app.go @@ -263,19 +263,19 @@ func NewSimApp( // ------------------------- Begin Custom Code -------------------------------- // // ---------------------------------------------------------------------------- // - handlerOptions := ante.HandlerOptions{ + anteHandlerOptions := ante.HandlerOptions{ BankKeeper: app.BankKeeper, FeegrantKeeper: app.FeeGrantKeeper, SigGasConsumer: ante.DefaultSigVerificationGasConsumer, SignModeHandler: app.txConfig.SignModeHandler(), } - options := HandlerOptions{ - BaseOptions: handlerOptions, + anteOptions := AnteHandlerOptions{ + BaseOptions: anteHandlerOptions, AccountKeeper: app.AccountKeeper, FeeMarketKeeper: app.FeeMarketKeeper, } - anteHandler, err := NewAnteHandler(options) + anteHandler, err := NewAnteHandler(anteOptions) if err != nil { panic(err) } From a05dac8bd3d382f8cd674528817aaf9940ad78d6 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 27 Nov 2023 11:15:25 -0500 Subject: [PATCH 36/55] set posthandler --- tests/simapp/app.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/simapp/app.go b/tests/simapp/app.go index c74ae69..213ee06 100644 --- a/tests/simapp/app.go +++ b/tests/simapp/app.go @@ -4,6 +4,7 @@ package simapp import ( "encoding/json" + "github.com/cosmos/cosmos-sdk/x/auth/posthandler" "io" "os" "path/filepath" @@ -281,6 +282,14 @@ func NewSimApp( } app.App.SetAnteHandler(anteHandler) + postHandlerOptions := posthandler.HandlerOptions{} + postHandler, err := posthandler.NewPostHandler(postHandlerOptions) + if err != nil { + panic(err) + } + + app.App.SetPostHandler(postHandler) + /**** Module Options ****/ app.ModuleManager.RegisterInvariants(app.CrisisKeeper) From 12282aa7ceb687eb14e1ad739258187730ab161c Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 27 Nov 2023 11:17:13 -0500 Subject: [PATCH 37/55] set posthandler --- tests/simapp/app.go | 5 ++--- tests/simapp/post.go | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 tests/simapp/post.go diff --git a/tests/simapp/app.go b/tests/simapp/app.go index 213ee06..ca4e8cf 100644 --- a/tests/simapp/app.go +++ b/tests/simapp/app.go @@ -4,7 +4,6 @@ package simapp import ( "encoding/json" - "github.com/cosmos/cosmos-sdk/x/auth/posthandler" "io" "os" "path/filepath" @@ -282,8 +281,8 @@ func NewSimApp( } app.App.SetAnteHandler(anteHandler) - postHandlerOptions := posthandler.HandlerOptions{} - postHandler, err := posthandler.NewPostHandler(postHandlerOptions) + postHandlerOptions := PostHandlerOptions{} + postHandler, err := NewPostHandler(postHandlerOptions) if err != nil { panic(err) } diff --git a/tests/simapp/post.go b/tests/simapp/post.go new file mode 100644 index 0000000..1e94e9f --- /dev/null +++ b/tests/simapp/post.go @@ -0,0 +1,15 @@ +package simapp + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// PostHandlerOptions are the options required for constructing a default SDK PostHandler. +type PostHandlerOptions struct{} + +// NewPostHandler returns an empty PostHandler chain. +func NewPostHandler(_ PostHandlerOptions) (sdk.PostHandler, error) { + var postDecorators []sdk.PostDecorator + + return sdk.ChainPostDecorators(postDecorators...), nil +} From 5e7c47ec3758d179b7e96b2a434b5270b4a8dc14 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 27 Nov 2023 11:21:45 -0500 Subject: [PATCH 38/55] setup post --- tests/simapp/ante.go | 4 + tests/simapp/post.go | 24 +++- x/feemarket/post/expected_keeper.go | 45 +++++++ x/feemarket/post/mocks/mock_account_keeper.go | 112 ++++++++++++++++++ x/feemarket/post/mocks/mock_bank_keeper.go | 77 ++++++++++++ .../post/mocks/mock_feegrant_keeper.go | 42 +++++++ .../post/mocks/mock_feemarket_keeper.go | 79 ++++++++++++ 7 files changed, 381 insertions(+), 2 deletions(-) create mode 100644 x/feemarket/post/expected_keeper.go create mode 100644 x/feemarket/post/mocks/mock_account_keeper.go create mode 100644 x/feemarket/post/mocks/mock_bank_keeper.go create mode 100644 x/feemarket/post/mocks/mock_feegrant_keeper.go create mode 100644 x/feemarket/post/mocks/mock_feemarket_keeper.go diff --git a/tests/simapp/ante.go b/tests/simapp/ante.go index 1b53d61..cf5c330 100644 --- a/tests/simapp/ante.go +++ b/tests/simapp/ante.go @@ -33,6 +33,10 @@ func NewAnteHandler(options AnteHandlerOptions) (sdk.AnteHandler, error) { return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for ante builder") } + if options.FeeMarketKeeper == nil { + return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "feemarket keeper is required for ante builder") + } + anteDecorators := []sdk.AnteDecorator{ authante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first authante.NewExtensionOptionsDecorator(options.BaseOptions.ExtensionOptionChecker), diff --git a/tests/simapp/post.go b/tests/simapp/post.go index 1e94e9f..cdb2ed2 100644 --- a/tests/simapp/post.go +++ b/tests/simapp/post.go @@ -1,14 +1,34 @@ package simapp import ( + errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + + feemarketante "github.com/skip-mev/feemarket/x/feemarket/ante" ) // PostHandlerOptions are the options required for constructing a default SDK PostHandler. -type PostHandlerOptions struct{} +type PostHandlerOptions struct { + AccountKeeper feemarketante.AccountKeeper + BankKeeper feemarketante.BankKeeper + FeeMarketKeeper feemarketante.FeeMarketKeeper +} // NewPostHandler returns an empty PostHandler chain. -func NewPostHandler(_ PostHandlerOptions) (sdk.PostHandler, error) { +func NewPostHandler(options PostHandlerOptions) (sdk.PostHandler, error) { + if options.AccountKeeper == nil { + return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "account keeper is required for post builder") + } + + if options.BankKeeper == nil { + return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "bank keeper is required for post builder") + } + + if options.FeeMarketKeeper == nil { + return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "feemarket keeper is required for post builder") + } + var postDecorators []sdk.PostDecorator return sdk.ChainPostDecorators(postDecorators...), nil diff --git a/x/feemarket/post/expected_keeper.go b/x/feemarket/post/expected_keeper.go new file mode 100644 index 0000000..0e8d6b7 --- /dev/null +++ b/x/feemarket/post/expected_keeper.go @@ -0,0 +1,45 @@ +package post + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + + feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" +) + +// AccountKeeper defines the contract needed for AccountKeeper related APIs. +// Interface provides support to use non-sdk AccountKeeper for AnteHandler's decorators. +// +//go:generate mockery --name AccountKeeper --filename mock_account_keeper.go +type AccountKeeper interface { + GetParams(ctx sdk.Context) (params authtypes.Params) + GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI + SetAccount(ctx sdk.Context, acc authtypes.AccountI) + GetModuleAddress(moduleName string) sdk.AccAddress + GetModuleAccount(ctx sdk.Context, name string) authtypes.ModuleAccountI + NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI +} + +// FeeGrantKeeper defines the expected feegrant keeper. +// +//go:generate mockery --name FeeGrantKeeper --filename mock_feegrant_keeper.go +type FeeGrantKeeper interface { + UseGrantedFees(ctx sdk.Context, granter, grantee sdk.AccAddress, fee sdk.Coins, msgs []sdk.Msg) error +} + +// BankKeeper defines the contract needed for supply related APIs. +// +//go:generate mockery --name BankKeeper --filename mock_bank_keeper.go +type BankKeeper interface { + IsSendEnabledCoins(ctx sdk.Context, coins ...sdk.Coin) error + SendCoins(ctx sdk.Context, from, to sdk.AccAddress, amt sdk.Coins) error + SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error +} + +// FeeMarketKeeper defines the expected feemarket keeper. +// +//go:generate mockery --name FeeMarketKeeper --filename mock_feemarket_keeper.go +type FeeMarketKeeper interface { + GetState(ctx sdk.Context) (feemarkettypes.State, error) + GetMinGasPrices(ctx sdk.Context) (sdk.Coins, error) +} diff --git a/x/feemarket/post/mocks/mock_account_keeper.go b/x/feemarket/post/mocks/mock_account_keeper.go new file mode 100644 index 0000000..207e219 --- /dev/null +++ b/x/feemarket/post/mocks/mock_account_keeper.go @@ -0,0 +1,112 @@ +// Code generated by mockery v0.0.0-dev. DO NOT EDIT. + +package mocks + +import ( + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + mock "github.com/stretchr/testify/mock" + + types "github.com/cosmos/cosmos-sdk/types" +) + +// AccountKeeper is an autogenerated mock type for the AccountKeeper type +type AccountKeeper struct { + mock.Mock +} + +// GetAccount provides a mock function with given fields: ctx, addr +func (_m *AccountKeeper) GetAccount(ctx types.Context, addr types.AccAddress) authtypes.AccountI { + ret := _m.Called(ctx, addr) + + var r0 authtypes.AccountI + if rf, ok := ret.Get(0).(func(types.Context, types.AccAddress) authtypes.AccountI); ok { + r0 = rf(ctx, addr) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(authtypes.AccountI) + } + } + + return r0 +} + +// GetModuleAccount provides a mock function with given fields: ctx, name +func (_m *AccountKeeper) GetModuleAccount(ctx types.Context, name string) authtypes.ModuleAccountI { + ret := _m.Called(ctx, name) + + var r0 authtypes.ModuleAccountI + if rf, ok := ret.Get(0).(func(types.Context, string) authtypes.ModuleAccountI); ok { + r0 = rf(ctx, name) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(authtypes.ModuleAccountI) + } + } + + return r0 +} + +// GetModuleAddress provides a mock function with given fields: moduleName +func (_m *AccountKeeper) GetModuleAddress(moduleName string) types.AccAddress { + ret := _m.Called(moduleName) + + var r0 types.AccAddress + if rf, ok := ret.Get(0).(func(string) types.AccAddress); ok { + r0 = rf(moduleName) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(types.AccAddress) + } + } + + return r0 +} + +// GetParams provides a mock function with given fields: ctx +func (_m *AccountKeeper) GetParams(ctx types.Context) authtypes.Params { + ret := _m.Called(ctx) + + var r0 authtypes.Params + if rf, ok := ret.Get(0).(func(types.Context) authtypes.Params); ok { + r0 = rf(ctx) + } else { + r0 = ret.Get(0).(authtypes.Params) + } + + return r0 +} + +// NewAccountWithAddress provides a mock function with given fields: ctx, addr +func (_m *AccountKeeper) NewAccountWithAddress(ctx types.Context, addr types.AccAddress) authtypes.AccountI { + ret := _m.Called(ctx, addr) + + var r0 authtypes.AccountI + if rf, ok := ret.Get(0).(func(types.Context, types.AccAddress) authtypes.AccountI); ok { + r0 = rf(ctx, addr) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(authtypes.AccountI) + } + } + + return r0 +} + +// SetAccount provides a mock function with given fields: ctx, acc +func (_m *AccountKeeper) SetAccount(ctx types.Context, acc authtypes.AccountI) { + _m.Called(ctx, acc) +} + +// NewAccountKeeper creates a new instance of AccountKeeper. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewAccountKeeper(t interface { + mock.TestingT + Cleanup(func()) +}) *AccountKeeper { + mock := &AccountKeeper{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/x/feemarket/post/mocks/mock_bank_keeper.go b/x/feemarket/post/mocks/mock_bank_keeper.go new file mode 100644 index 0000000..9cae7d7 --- /dev/null +++ b/x/feemarket/post/mocks/mock_bank_keeper.go @@ -0,0 +1,77 @@ +// Code generated by mockery v0.0.0-dev. DO NOT EDIT. + +package mocks + +import ( + mock "github.com/stretchr/testify/mock" + + types "github.com/cosmos/cosmos-sdk/types" +) + +// BankKeeper is an autogenerated mock type for the BankKeeper type +type BankKeeper struct { + mock.Mock +} + +// IsSendEnabledCoins provides a mock function with given fields: ctx, coins +func (_m *BankKeeper) IsSendEnabledCoins(ctx types.Context, coins ...types.Coin) error { + _va := make([]interface{}, len(coins)) + for _i := range coins { + _va[_i] = coins[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + var r0 error + if rf, ok := ret.Get(0).(func(types.Context, ...types.Coin) error); ok { + r0 = rf(ctx, coins...) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// SendCoins provides a mock function with given fields: ctx, from, to, amt +func (_m *BankKeeper) SendCoins(ctx types.Context, from types.AccAddress, to types.AccAddress, amt types.Coins) error { + ret := _m.Called(ctx, from, to, amt) + + var r0 error + if rf, ok := ret.Get(0).(func(types.Context, types.AccAddress, types.AccAddress, types.Coins) error); ok { + r0 = rf(ctx, from, to, amt) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// SendCoinsFromAccountToModule provides a mock function with given fields: ctx, senderAddr, recipientModule, amt +func (_m *BankKeeper) SendCoinsFromAccountToModule(ctx types.Context, senderAddr types.AccAddress, recipientModule string, amt types.Coins) error { + ret := _m.Called(ctx, senderAddr, recipientModule, amt) + + var r0 error + if rf, ok := ret.Get(0).(func(types.Context, types.AccAddress, string, types.Coins) error); ok { + r0 = rf(ctx, senderAddr, recipientModule, amt) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// NewBankKeeper creates a new instance of BankKeeper. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewBankKeeper(t interface { + mock.TestingT + Cleanup(func()) +}) *BankKeeper { + mock := &BankKeeper{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/x/feemarket/post/mocks/mock_feegrant_keeper.go b/x/feemarket/post/mocks/mock_feegrant_keeper.go new file mode 100644 index 0000000..ddff891 --- /dev/null +++ b/x/feemarket/post/mocks/mock_feegrant_keeper.go @@ -0,0 +1,42 @@ +// Code generated by mockery v0.0.0-dev. DO NOT EDIT. + +package mocks + +import ( + mock "github.com/stretchr/testify/mock" + + types "github.com/cosmos/cosmos-sdk/types" +) + +// FeeGrantKeeper is an autogenerated mock type for the FeeGrantKeeper type +type FeeGrantKeeper struct { + mock.Mock +} + +// UseGrantedFees provides a mock function with given fields: ctx, granter, grantee, fee, msgs +func (_m *FeeGrantKeeper) UseGrantedFees(ctx types.Context, granter types.AccAddress, grantee types.AccAddress, fee types.Coins, msgs []types.Msg) error { + ret := _m.Called(ctx, granter, grantee, fee, msgs) + + var r0 error + if rf, ok := ret.Get(0).(func(types.Context, types.AccAddress, types.AccAddress, types.Coins, []types.Msg) error); ok { + r0 = rf(ctx, granter, grantee, fee, msgs) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// NewFeeGrantKeeper creates a new instance of FeeGrantKeeper. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewFeeGrantKeeper(t interface { + mock.TestingT + Cleanup(func()) +}) *FeeGrantKeeper { + mock := &FeeGrantKeeper{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/x/feemarket/post/mocks/mock_feemarket_keeper.go b/x/feemarket/post/mocks/mock_feemarket_keeper.go new file mode 100644 index 0000000..3b1cd67 --- /dev/null +++ b/x/feemarket/post/mocks/mock_feemarket_keeper.go @@ -0,0 +1,79 @@ +// Code generated by mockery v0.0.0-dev. DO NOT EDIT. + +package mocks + +import ( + feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" + mock "github.com/stretchr/testify/mock" + + types "github.com/cosmos/cosmos-sdk/types" +) + +// FeeMarketKeeper is an autogenerated mock type for the FeeMarketKeeper type +type FeeMarketKeeper struct { + mock.Mock +} + +// GetMinGasPrices provides a mock function with given fields: ctx +func (_m *FeeMarketKeeper) GetMinGasPrices(ctx types.Context) (types.Coins, error) { + ret := _m.Called(ctx) + + var r0 types.Coins + var r1 error + if rf, ok := ret.Get(0).(func(types.Context) (types.Coins, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(types.Context) types.Coins); ok { + r0 = rf(ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(types.Coins) + } + } + + if rf, ok := ret.Get(1).(func(types.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetState provides a mock function with given fields: ctx +func (_m *FeeMarketKeeper) GetState(ctx types.Context) (feemarkettypes.State, error) { + ret := _m.Called(ctx) + + var r0 feemarkettypes.State + var r1 error + if rf, ok := ret.Get(0).(func(types.Context) (feemarkettypes.State, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(types.Context) feemarkettypes.State); ok { + r0 = rf(ctx) + } else { + r0 = ret.Get(0).(feemarkettypes.State) + } + + if rf, ok := ret.Get(1).(func(types.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewFeeMarketKeeper creates a new instance of FeeMarketKeeper. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewFeeMarketKeeper(t interface { + mock.TestingT + Cleanup(func()) +}) *FeeMarketKeeper { + mock := &FeeMarketKeeper{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} From d5a1c91469acbec7ab5566a9e421e0a202d3c702 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 27 Nov 2023 13:01:26 -0500 Subject: [PATCH 39/55] finalize --- tests/simapp/ante.go | 7 +- tests/simapp/post.go | 26 +- x/feemarket/ante/ante_test.go | 249 ---------------- x/feemarket/ante/fee.go | 142 ++------- x/feemarket/ante/fee_test.go | 96 ++---- x/feemarket/ante/suite/suite.go | 275 ++++++++++++++++++ x/feemarket/post/fee.go | 148 ++++++++++ x/feemarket/post/fee_test.go | 128 ++++++++ x/feemarket/post/mocks/mock_account_keeper.go | 3 +- x/feemarket/post/mocks/mock_bank_keeper.go | 3 +- .../post/mocks/mock_feegrant_keeper.go | 3 +- .../post/mocks/mock_feemarket_keeper.go | 6 +- 12 files changed, 624 insertions(+), 462 deletions(-) delete mode 100644 x/feemarket/ante/ante_test.go create mode 100644 x/feemarket/ante/suite/suite.go create mode 100644 x/feemarket/post/fee.go create mode 100644 x/feemarket/post/fee_test.go diff --git a/tests/simapp/ante.go b/tests/simapp/ante.go index cf5c330..db85957 100644 --- a/tests/simapp/ante.go +++ b/tests/simapp/ante.go @@ -44,12 +44,9 @@ func NewAnteHandler(options AnteHandlerOptions) (sdk.AnteHandler, error) { authante.NewTxTimeoutHeightDecorator(), authante.NewValidateMemoDecorator(options.AccountKeeper), authante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper), - feemarketante.NewFeeMarketDecorator( // fee market replaces fee deduct decorator - options.AccountKeeper, - options.BaseOptions.BankKeeper, - options.BaseOptions.FeegrantKeeper, + feemarketante.NewFeeMarketCheckDecorator( // fee market check replaces fee deduct decorator options.FeeMarketKeeper, - ), + ), // fees are deducted in the fee market deduct post handler authante.NewSetPubKeyDecorator(options.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators authante.NewValidateSigCountDecorator(options.AccountKeeper), authante.NewSigGasConsumeDecorator(options.AccountKeeper, options.BaseOptions.SigGasConsumer), diff --git a/tests/simapp/post.go b/tests/simapp/post.go index cdb2ed2..460eb68 100644 --- a/tests/simapp/post.go +++ b/tests/simapp/post.go @@ -5,17 +5,18 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - feemarketante "github.com/skip-mev/feemarket/x/feemarket/ante" + feemarketpost "github.com/skip-mev/feemarket/x/feemarket/post" ) -// PostHandlerOptions are the options required for constructing a default SDK PostHandler. +// PostHandlerOptions are the options required for constructing a FeeMarket PostHandler. type PostHandlerOptions struct { - AccountKeeper feemarketante.AccountKeeper - BankKeeper feemarketante.BankKeeper - FeeMarketKeeper feemarketante.FeeMarketKeeper + AccountKeeper feemarketpost.AccountKeeper + BankKeeper feemarketpost.BankKeeper + FeeMarketKeeper feemarketpost.FeeMarketKeeper + FeeGrantKeeper feemarketpost.FeeGrantKeeper } -// NewPostHandler returns an empty PostHandler chain. +// NewPostHandler returns a PostHandler chain with the fee deduct decorator. func NewPostHandler(options PostHandlerOptions) (sdk.PostHandler, error) { if options.AccountKeeper == nil { return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "account keeper is required for post builder") @@ -29,7 +30,18 @@ func NewPostHandler(options PostHandlerOptions) (sdk.PostHandler, error) { return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "feemarket keeper is required for post builder") } - var postDecorators []sdk.PostDecorator + if options.FeeGrantKeeper == nil { + return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "feegrant keeper is required for post builder") + } + + postDecorators := []sdk.PostDecorator{ + feemarketpost.NewFeeMarketDeductDecorator( + options.AccountKeeper, + options.BankKeeper, + options.FeeGrantKeeper, + options.FeeMarketKeeper, + ), + } return sdk.ChainPostDecorators(postDecorators...), nil } diff --git a/x/feemarket/ante/ante_test.go b/x/feemarket/ante/ante_test.go deleted file mode 100644 index 0197ca9..0000000 --- a/x/feemarket/ante/ante_test.go +++ /dev/null @@ -1,249 +0,0 @@ -package ante_test - -import ( - "testing" - - "github.com/cosmos/cosmos-sdk/client" - "github.com/cosmos/cosmos-sdk/client/tx" - cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" - storetypes "github.com/cosmos/cosmos-sdk/store/types" - "github.com/cosmos/cosmos-sdk/testutil" - "github.com/cosmos/cosmos-sdk/testutil/testdata" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/tx/signing" - authante "github.com/cosmos/cosmos-sdk/x/auth/ante" - authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" - authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" - - "github.com/skip-mev/feemarket/testutils" - feemarketante "github.com/skip-mev/feemarket/x/feemarket/ante" - "github.com/skip-mev/feemarket/x/feemarket/ante/mocks" - "github.com/skip-mev/feemarket/x/feemarket/keeper" - "github.com/skip-mev/feemarket/x/feemarket/types" -) - -type AnteTestSuite struct { - suite.Suite - ctx sdk.Context - anteHandler sdk.AnteHandler - clientCtx client.Context - txBuilder client.TxBuilder - - accountKeeper authkeeper.AccountKeeper - feemarketKeeper *keeper.Keeper - bankKeeper *mocks.BankKeeper - feeGrantKeeper *mocks.FeeGrantKeeper - encCfg testutils.EncodingConfig - key *storetypes.KVStoreKey - authorityAccount sdk.AccAddress -} - -// TestAccount represents an account used in the tests in x/auth/ante. -type TestAccount struct { - acc authtypes.AccountI - priv cryptotypes.PrivKey -} - -func (s *AnteTestSuite) CreateTestAccounts(numAccs int) []TestAccount { - var accounts []TestAccount - - for i := 0; i < numAccs; i++ { - priv, _, addr := testdata.KeyTestPubAddr() - acc := s.accountKeeper.NewAccountWithAddress(s.ctx, addr) - err := acc.SetAccountNumber(uint64(i + 1000)) - if err != nil { - panic(err) - } - s.accountKeeper.SetAccount(s.ctx, acc) - accounts = append(accounts, TestAccount{acc, priv}) - } - - return accounts -} - -// SetupTest setups a new test, with new app, context, and anteHandler. -func SetupTestSuite(t *testing.T) *AnteTestSuite { - s := &AnteTestSuite{} - - s.encCfg = testutils.CreateTestEncodingConfig() - s.key = storetypes.NewKVStoreKey(types.StoreKey) - tkey := storetypes.NewTransientStoreKey("transient_test_feemarket") - testCtx := testutil.DefaultContextWithDB(t, s.key, tkey) - s.ctx = testCtx.Ctx.WithIsCheckTx(false).WithBlockHeight(1) - cms, db := testCtx.CMS, testCtx.DB - - authKey := storetypes.NewKVStoreKey(authtypes.StoreKey) - tkey = storetypes.NewTransientStoreKey("transient_test_auth") - cms.MountStoreWithDB(authKey, storetypes.StoreTypeIAVL, db) - cms.MountStoreWithDB(tkey, storetypes.StoreTypeTransient, db) - err := cms.LoadLatestVersion() - require.NoError(t, err) - - maccPerms := map[string][]string{ - types.ModuleName: nil, - types.FeeCollectorName: {"burner"}, - } - - s.authorityAccount = authtypes.NewModuleAddress("gov") - s.accountKeeper = authkeeper.NewAccountKeeper( - s.encCfg.Codec, authKey, authtypes.ProtoBaseAccount, maccPerms, sdk.Bech32MainPrefix, s.authorityAccount.String(), - ) - - s.feemarketKeeper = keeper.NewKeeper( - s.encCfg.Codec, - s.key, - s.accountKeeper, - s.authorityAccount.String(), - ) - - err = s.feemarketKeeper.SetParams(s.ctx, types.DefaultParams()) - require.NoError(t, err) - - err = s.feemarketKeeper.SetState(s.ctx, types.DefaultState()) - require.NoError(t, err) - - s.bankKeeper = mocks.NewBankKeeper(t) - s.feeGrantKeeper = mocks.NewFeeGrantKeeper(t) - - s.clientCtx = client.Context{}.WithTxConfig(s.encCfg.TxConfig) - s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() - - // create basic antehandler with the feemarket decorator - anteDecorators := []sdk.AnteDecorator{ - authante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first - feemarketante.NewFeeMarketDecorator( // fee market replaces fee deduct decorator - s.accountKeeper, - s.bankKeeper, - s.feeGrantKeeper, - s.feemarketKeeper, - ), - authante.NewSigGasConsumeDecorator(s.accountKeeper, authante.DefaultSigVerificationGasConsumer), - } - - s.anteHandler = sdk.ChainAnteDecorators(anteDecorators...) - return s -} - -// TestCase represents a test case used in test tables. -type TestCase struct { - name string - malleate func(*AnteTestSuite) TestCaseArgs - simulate bool - expPass bool - expErr error -} - -type TestCaseArgs struct { - chainID string - accNums []uint64 - accSeqs []uint64 - feeAmount sdk.Coins - gasLimit uint64 - msgs []sdk.Msg - privs []cryptotypes.PrivKey -} - -// DeliverMsgs constructs a tx and runs it through the ante handler. This is used to set the context for a test case, for -// example to test for replay protection. -func (s *AnteTestSuite) DeliverMsgs(t *testing.T, privs []cryptotypes.PrivKey, msgs []sdk.Msg, feeAmount sdk.Coins, gasLimit uint64, accNums, accSeqs []uint64, chainID string, simulate bool) (sdk.Context, error) { - require.NoError(t, s.txBuilder.SetMsgs(msgs...)) - s.txBuilder.SetFeeAmount(feeAmount) - s.txBuilder.SetGasLimit(gasLimit) - - tx, txErr := s.CreateTestTx(privs, accNums, accSeqs, chainID) - require.NoError(t, txErr) - return s.anteHandler(s.ctx, tx, simulate) -} - -func (s *AnteTestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { - require.NoError(t, s.txBuilder.SetMsgs(args.msgs...)) - s.txBuilder.SetFeeAmount(args.feeAmount) - s.txBuilder.SetGasLimit(args.gasLimit) - - // Theoretically speaking, ante handler unit tests should only test - // ante handlers, but here we sometimes also test the tx creation - // process. - tx, txErr := s.CreateTestTx(args.privs, args.accNums, args.accSeqs, args.chainID) - newCtx, anteErr := s.anteHandler(s.ctx, tx, tc.simulate) - - if tc.expPass { - require.NoError(t, txErr) - require.NoError(t, anteErr) - require.NotNil(t, newCtx) - - s.ctx = newCtx - } else { - switch { - case txErr != nil: - require.Error(t, txErr) - require.ErrorIs(t, txErr, tc.expErr) - - case anteErr != nil: - require.Error(t, anteErr) - require.ErrorIs(t, anteErr, tc.expErr) - - default: - t.Fatal("expected one of txErr, anteErr to be an error") - } - } -} - -// CreateTestTx is a helper function to create a tx given multiple inputs. -func (s *AnteTestSuite) CreateTestTx(privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (authsigning.Tx, error) { - // First round: we gather all the signer infos. We use the "set empty - // signature" hack to do that. - var sigsV2 []signing.SignatureV2 - for i, priv := range privs { - sigV2 := signing.SignatureV2{ - PubKey: priv.PubKey(), - Data: &signing.SingleSignatureData{ - SignMode: s.clientCtx.TxConfig.SignModeHandler().DefaultMode(), - Signature: nil, - }, - Sequence: accSeqs[i], - } - - sigsV2 = append(sigsV2, sigV2) - } - err := s.txBuilder.SetSignatures(sigsV2...) - if err != nil { - return nil, err - } - - // Second round: all signer infos are set, so each signer can sign. - sigsV2 = []signing.SignatureV2{} - for i, priv := range privs { - signerData := authsigning.SignerData{ - ChainID: chainID, - AccountNumber: accNums[i], - Sequence: accSeqs[i], - } - sigV2, err := tx.SignWithPrivKey( - s.clientCtx.TxConfig.SignModeHandler().DefaultMode(), signerData, - s.txBuilder, priv, s.clientCtx.TxConfig, accSeqs[i]) - if err != nil { - return nil, err - } - - sigsV2 = append(sigsV2, sigV2) - } - err = s.txBuilder.SetSignatures(sigsV2...) - if err != nil { - return nil, err - } - - return s.txBuilder.GetTx(), nil -} - -// NewTestFeeAmount is a test fee amount. -func NewTestFeeAmount() sdk.Coins { - return sdk.NewCoins(sdk.NewInt64Coin("stake", 150)) -} - -// NewTestGasLimit is a test fee gas limit. -func NewTestGasLimit() uint64 { - return 200000 -} diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index b8e2bdd..661d9fe 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -1,7 +1,6 @@ package ante import ( - "fmt" "math" errorsmod "cosmossdk.io/errors" @@ -9,38 +8,25 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - - feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" ) -// TxFeeChecker check if the provided fee is enough and returns the effective fee and tx priority, -// the effective fee should be deducted later, and the priority should be returned in abci response. -type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) - -// FeeMarketDecorator deducts fees from the fee payer based off of the current state of the feemarket. +// FeeMarketCheckDecorator deducts fees from the fee payer based off of the current state of the feemarket. // The fee payer is the fee granter (if specified) or first signer of the tx. // If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. // If there is an excess between the given fee and the on-chain base fee is given as a tip. // Call next AnteHandler if fees successfully deducted. // CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator -type FeeMarketDecorator struct { - accountKeeper AccountKeeper - bankKeeper BankKeeper - feegrantKeeper FeeGrantKeeper +type FeeMarketCheckDecorator struct { feemarketKeeper FeeMarketKeeper } -func NewFeeMarketDecorator(ak AccountKeeper, bk BankKeeper, fk FeeGrantKeeper, fmk FeeMarketKeeper) FeeMarketDecorator { - return FeeMarketDecorator{ - accountKeeper: ak, - bankKeeper: bk, - feegrantKeeper: fk, +func NewFeeMarketCheckDecorator(fmk FeeMarketKeeper) FeeMarketCheckDecorator { + return FeeMarketCheckDecorator{ feemarketKeeper: fmk, } } -func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { +func (dfd FeeMarketCheckDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") @@ -50,130 +36,45 @@ func (dfd FeeMarketDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } - var ( - priority int64 - tip sdk.Coins - ) - minGasPrices, err := dfd.feemarketKeeper.GetMinGasPrices(ctx) if err != nil { return ctx, errorsmod.Wrapf(err, "unable to get fee market state") } - minGasPricesDecCoins := sdk.NewDecCoinsFromCoins(minGasPrices...) - fee := feeTx.GetFee() + gas := feeTx.GetGas() // use provided gas limit + if !simulate { - fee, tip, priority, err = checkTxFees(minGasPricesDecCoins, tx) + fee, _, err = CheckTxFees(minGasPrices, tx, gas) if err != nil { return ctx, err } } - if err := dfd.checkDeductFeeAndTip(ctx, tx, fee, tip); err != nil { - return ctx, err - } - - newCtx := ctx.WithPriority(priority).WithMinGasPrices(minGasPricesDecCoins) + minGasPricesDecCoins := sdk.NewDecCoinsFromCoins(minGasPrices...) + newCtx := ctx.WithPriority(getTxPriority(fee, int64(gas))).WithMinGasPrices(minGasPricesDecCoins) return next(newCtx, tx, simulate) } -func (dfd FeeMarketDecorator) checkDeductFeeAndTip(ctx sdk.Context, sdkTx sdk.Tx, fee, tip sdk.Coins) error { - feeTx, ok := sdkTx.(sdk.FeeTx) - if !ok { - return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") - } - - if addr := dfd.accountKeeper.GetModuleAddress(feemarkettypes.FeeCollectorName); addr == nil { - return fmt.Errorf("fee collector module account (%s) has not been set", feemarkettypes.FeeCollectorName) - } - - feePayer := feeTx.FeePayer() - feeGranter := feeTx.FeeGranter() - deductFeesFrom := feePayer - - // if feegranter set deduct fee from feegranter account. - // this works with only when feegrant enabled. - if feeGranter != nil { - if dfd.feegrantKeeper == nil { - return sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled") - } else if !feeGranter.Equals(feePayer) { - err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, sdkTx.GetMsgs()) - if err != nil { - return errorsmod.Wrapf(err, "%s does not allow to pay fees for %s", feeGranter, feePayer) - } - } - - deductFeesFrom = feeGranter - } - - deductFeesFromAcc := dfd.accountKeeper.GetAccount(ctx, deductFeesFrom) - if deductFeesFromAcc == nil { - return sdkerrors.ErrUnknownAddress.Wrapf("fee payer address: %s does not exist", deductFeesFrom) - } - - // deduct the fees - if !fee.IsZero() { - err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, fee) - if err != nil { - return err - } - } - - // deduct the tip - if !tip.IsZero() { - err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, tip) - if err != nil { - return err - } - } - - events := sdk.Events{ - sdk.NewEvent( - sdk.EventTypeTx, - sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()), - sdk.NewAttribute(sdk.AttributeKeyFeePayer, deductFeesFrom.String()), - sdk.NewAttribute(feemarkettypes.AttributeKeyTip, tip.String()), - sdk.NewAttribute(feemarkettypes.AttributeKeyTipPayer, sdk.AccAddress(ctx.BlockHeader().ProposerAddress).String()), - ), - } - ctx.EventManager().EmitEvents(events) - - return nil -} - -// DeductCoins deducts coins from the given account. Coins are sent to the feemarket module account. -func DeductCoins(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, coins sdk.Coins) error { - if !coins.IsValid() { - return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid coin amount: %s", coins) - } - - err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), feemarkettypes.FeeCollectorName, coins) - if err != nil { - return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) - } - - return nil -} - -// checkTxFees implements the logic for the fee market to check if a Tx has provided suffucient +// CheckTxFees implements the logic for the fee market to check if a Tx has provided suffucient // fees given the current state of the fee market. Returns an error if insufficient fees. -func checkTxFees(fees sdk.DecCoins, tx sdk.Tx) (feeCoins sdk.Coins, tip sdk.Coins, priority int64, err error) { +func CheckTxFees(minFees sdk.Coins, tx sdk.Tx, gas uint64) (feeCoins sdk.Coins, tip sdk.Coins, err error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { - return nil, nil, 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + return nil, nil, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } + feesDec := sdk.NewDecCoinsFromCoins(minFees...) + feeCoins = feeTx.GetFee() - gas := feeTx.GetGas() // Ensure that the provided fees meet the minimum - minGasPrices := fees + minGasPrices := feesDec if !minGasPrices.IsZero() { requiredFees := make(sdk.Coins, len(minGasPrices)) // Determine the required fees by multiplying each required minimum gas - // price by the gas limit, where fee = ceil(minGasPrice * gasLimit). + // price by the gas, where fee = ceil(minGasPrice * gas). glDec := sdkmath.LegacyNewDec(int64(gas)) for i, gp := range minGasPrices { fee := gp.Amount.Mul(glDec) @@ -181,15 +82,14 @@ func checkTxFees(fees sdk.DecCoins, tx sdk.Tx) (feeCoins sdk.Coins, tip sdk.Coin } if !feeCoins.IsAnyGTE(requiredFees) { - return nil, nil, 0, errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) + return nil, nil, errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) } - tip = feeCoins.Sub(requiredFees...) // tip is whatever is left - feeCoins = requiredFees // set fee coins to be ONLY the required amount + tip = feeCoins.Sub(minFees...) // tip is the difference between feeCoins and the min fees + feeCoins = requiredFees // set fee coins to be ONLY the required amount } - priority = getTxPriority(feeCoins, int64(gas)) - return feeCoins, tip, priority, nil + return feeCoins, tip, nil } // getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the gas price diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go index 110f0c8..5aec90c 100644 --- a/x/feemarket/ante/fee_test.go +++ b/x/feemarket/ante/fee_test.go @@ -8,101 +8,47 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" _ "github.com/cosmos/cosmos-sdk/x/auth" - "github.com/stretchr/testify/mock" - "github.com/skip-mev/feemarket/x/feemarket/ante" + antesuite "github.com/skip-mev/feemarket/x/feemarket/ante/suite" "github.com/skip-mev/feemarket/x/feemarket/types" ) -func TestDeductCoins(t *testing.T) { - tests := []struct { - name string - coins sdk.Coins - wantErr bool - invalidCoin bool - }{ - { - name: "valid", - coins: sdk.NewCoins(sdk.NewCoin("test", sdk.NewInt(10))), - wantErr: false, - }, - { - name: "valid no coins", - coins: sdk.NewCoins(), - wantErr: false, - }, - { - name: "invalid coins", - coins: sdk.Coins{sdk.Coin{Amount: sdk.NewInt(-1)}}, - wantErr: true, - invalidCoin: true, - }, - } - for _, tc := range tests { - t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { - s := SetupTestSuite(t) - acc := s.CreateTestAccounts(1)[0] - if !tc.invalidCoin { - s.bankKeeper.On("SendCoinsFromAccountToModule", s.ctx, acc.acc.GetAddress(), types.FeeCollectorName, tc.coins).Return(nil).Once() - } - - if err := ante.DeductCoins(s.bankKeeper, s.ctx, acc.acc, tc.coins); (err != nil) != tc.wantErr { - s.Errorf(err, "DeductCoins() error = %v, wantErr %v", err, tc.wantErr) - } - }) - } -} - func TestAnteHandle(t *testing.T) { // Same data for every test case - gasLimit := NewTestGasLimit() + gasLimit := antesuite.NewTestGasLimit() validFeeAmount := types.DefaultMinBaseFee.MulRaw(int64(gasLimit)) validFee := sdk.NewCoins(sdk.NewCoin("stake", validFeeAmount)) - testCases := []TestCase{ - { - "signer has no funds", - func(suite *AnteTestSuite) TestCaseArgs { - accs := suite.CreateTestAccounts(1) - suite.bankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].acc.GetAddress(), types.FeeCollectorName, mock.Anything).Return(sdkerrors.ErrInsufficientFunds).Once() - - return TestCaseArgs{ - msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].acc.GetAddress())}, - gasLimit: gasLimit, - feeAmount: validFee, - } - }, - false, - false, - sdkerrors.ErrInsufficientFunds, - }, + testCases := []antesuite.TestCase{ { "0 gas given should fail", - func(suite *AnteTestSuite) TestCaseArgs { + func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { accs := suite.CreateTestAccounts(1) - return TestCaseArgs{ - msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].acc.GetAddress())}, - gasLimit: 0, - feeAmount: validFee, + return antesuite.TestCaseArgs{ + Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, + GasLimit: 0, + FeeAmount: validFee, } }, + true, + false, false, false, sdkerrors.ErrInvalidGasLimit, }, { "signer has enough funds, should pass", - func(suite *AnteTestSuite) TestCaseArgs { + func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { accs := suite.CreateTestAccounts(1) - suite.bankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].acc.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil).Once() - - return TestCaseArgs{ - msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].acc.GetAddress())}, - gasLimit: gasLimit, - feeAmount: validFee, + return antesuite.TestCaseArgs{ + Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, + GasLimit: gasLimit, + FeeAmount: validFee, } }, + true, + false, false, true, nil, @@ -110,10 +56,10 @@ func TestAnteHandle(t *testing.T) { } for _, tc := range testCases { - t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { - s := SetupTestSuite(t) - s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() - args := tc.malleate(s) + t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) { + s := antesuite.SetupTestSuite(t) + s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder() + args := tc.Malleate(s) s.RunTestCase(t, tc, args) }) diff --git a/x/feemarket/ante/suite/suite.go b/x/feemarket/ante/suite/suite.go new file mode 100644 index 0000000..4151ee3 --- /dev/null +++ b/x/feemarket/ante/suite/suite.go @@ -0,0 +1,275 @@ +package suite + +import ( + "testing" + + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/client/tx" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + storetypes "github.com/cosmos/cosmos-sdk/store/types" + "github.com/cosmos/cosmos-sdk/testutil" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + authante "github.com/cosmos/cosmos-sdk/x/auth/ante" + authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/skip-mev/feemarket/testutils" + feemarketante "github.com/skip-mev/feemarket/x/feemarket/ante" + "github.com/skip-mev/feemarket/x/feemarket/ante/mocks" + "github.com/skip-mev/feemarket/x/feemarket/keeper" + feemarketpost "github.com/skip-mev/feemarket/x/feemarket/post" + "github.com/skip-mev/feemarket/x/feemarket/types" +) + +type TestSuite struct { + suite.Suite + + Ctx sdk.Context + AnteHandler sdk.AnteHandler + PostHandler sdk.PostHandler + ClientCtx client.Context + TxBuilder client.TxBuilder + + AccountKeeper authkeeper.AccountKeeper + FeemarketKeeper *keeper.Keeper + BankKeeper *mocks.BankKeeper + FeeGrantKeeper *mocks.FeeGrantKeeper + EncCfg testutils.EncodingConfig + Key *storetypes.KVStoreKey + AuthorityAccount sdk.AccAddress +} + +// TestAccount represents an account used in the tests in x/auth/ante. +type TestAccount struct { + Account authtypes.AccountI + Priv cryptotypes.PrivKey +} + +func (s *TestSuite) CreateTestAccounts(numAccs int) []TestAccount { + var accounts []TestAccount + + for i := 0; i < numAccs; i++ { + priv, _, addr := testdata.KeyTestPubAddr() + acc := s.AccountKeeper.NewAccountWithAddress(s.Ctx, addr) + err := acc.SetAccountNumber(uint64(i + 1000)) + if err != nil { + panic(err) + } + s.AccountKeeper.SetAccount(s.Ctx, acc) + accounts = append(accounts, TestAccount{acc, priv}) + } + + return accounts +} + +// SetupTest setups a new test, with new app, context, and anteHandler. +func SetupTestSuite(t *testing.T) *TestSuite { + s := &TestSuite{} + + s.EncCfg = testutils.CreateTestEncodingConfig() + s.Key = storetypes.NewKVStoreKey(types.StoreKey) + tkey := storetypes.NewTransientStoreKey("transient_test_feemarket") + testCtx := testutil.DefaultContextWithDB(t, s.Key, tkey) + s.Ctx = testCtx.Ctx.WithIsCheckTx(false).WithBlockHeight(1) + cms, db := testCtx.CMS, testCtx.DB + + authKey := storetypes.NewKVStoreKey(authtypes.StoreKey) + tkey = storetypes.NewTransientStoreKey("transient_test_auth") + cms.MountStoreWithDB(authKey, storetypes.StoreTypeIAVL, db) + cms.MountStoreWithDB(tkey, storetypes.StoreTypeTransient, db) + err := cms.LoadLatestVersion() + require.NoError(t, err) + + maccPerms := map[string][]string{ + types.ModuleName: nil, + types.FeeCollectorName: {"burner"}, + } + + s.AuthorityAccount = authtypes.NewModuleAddress("gov") + s.AccountKeeper = authkeeper.NewAccountKeeper( + s.EncCfg.Codec, authKey, authtypes.ProtoBaseAccount, maccPerms, sdk.Bech32MainPrefix, s.AuthorityAccount.String(), + ) + + s.FeemarketKeeper = keeper.NewKeeper( + s.EncCfg.Codec, + s.Key, + s.AccountKeeper, + s.AuthorityAccount.String(), + ) + + err = s.FeemarketKeeper.SetParams(s.Ctx, types.DefaultParams()) + require.NoError(t, err) + + err = s.FeemarketKeeper.SetState(s.Ctx, types.DefaultState()) + require.NoError(t, err) + + s.BankKeeper = mocks.NewBankKeeper(t) + s.FeeGrantKeeper = mocks.NewFeeGrantKeeper(t) + + s.ClientCtx = client.Context{}.WithTxConfig(s.EncCfg.TxConfig) + s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder() + + // create basic antehandler with the feemarket decorator + anteDecorators := []sdk.AnteDecorator{ + authante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first + feemarketante.NewFeeMarketCheckDecorator( // fee market replaces fee deduct decorator + s.FeemarketKeeper, + ), + authante.NewSigGasConsumeDecorator(s.AccountKeeper, authante.DefaultSigVerificationGasConsumer), + } + + s.AnteHandler = sdk.ChainAnteDecorators(anteDecorators...) + + // create basic postHandler with the feemarket decorator + postDecorators := []sdk.PostDecorator{ + feemarketpost.NewFeeMarketDeductDecorator( + s.AccountKeeper, + s.BankKeeper, + s.FeeGrantKeeper, + s.FeemarketKeeper, + ), + } + + s.PostHandler = sdk.ChainPostDecorators(postDecorators...) + return s +} + +// TestCase represents a test case used in test tables. +type TestCase struct { + Name string + Malleate func(*TestSuite) TestCaseArgs + RunAnte bool + RunPost bool + Simulate bool + ExpPass bool + ExpErr error +} + +type TestCaseArgs struct { + ChainID string + AccNums []uint64 + AccSeqs []uint64 + FeeAmount sdk.Coins + GasLimit uint64 + Msgs []sdk.Msg + Privs []cryptotypes.PrivKey +} + +// DeliverMsgs constructs a tx and runs it through the ante handler. This is used to set the context for a test case, for +// example to test for replay protection. +func (s *TestSuite) DeliverMsgs(t *testing.T, privs []cryptotypes.PrivKey, msgs []sdk.Msg, feeAmount sdk.Coins, gasLimit uint64, accNums, accSeqs []uint64, chainID string, simulate bool) (sdk.Context, error) { + require.NoError(t, s.TxBuilder.SetMsgs(msgs...)) + s.TxBuilder.SetFeeAmount(feeAmount) + s.TxBuilder.SetGasLimit(gasLimit) + + tx, txErr := s.CreateTestTx(privs, accNums, accSeqs, chainID) + require.NoError(t, txErr) + return s.AnteHandler(s.Ctx, tx, simulate) +} + +func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { + require.NoError(t, s.TxBuilder.SetMsgs(args.Msgs...)) + s.TxBuilder.SetFeeAmount(args.FeeAmount) + s.TxBuilder.SetGasLimit(args.GasLimit) + + // Theoretically speaking, ante handler unit tests should only test + // ante handlers, but here we sometimes also test the tx creation + // process. + tx, txErr := s.CreateTestTx(args.Privs, args.AccNums, args.AccSeqs, args.ChainID) + + var ( + newCtx sdk.Context + handleErr error + ) + + if tc.RunAnte { + newCtx, handleErr = s.AnteHandler(s.Ctx, tx, tc.Simulate) + } + + if tc.RunPost { + newCtx, handleErr = s.PostHandler(s.Ctx, tx, tc.Simulate, true) + } + + if tc.ExpPass { + require.NoError(t, txErr) + require.NoError(t, handleErr) + require.NotNil(t, newCtx) + + s.Ctx = newCtx + } else { + switch { + case txErr != nil: + require.Error(t, txErr) + require.ErrorIs(t, txErr, tc.ExpErr) + + case handleErr != nil: + require.Error(t, handleErr) + require.ErrorIs(t, handleErr, tc.ExpErr) + + default: + t.Fatal("expected one of txErr, handleErr to be an error") + } + } +} + +// CreateTestTx is a helper function to create a tx given multiple inputs. +func (s *TestSuite) CreateTestTx(privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (authsigning.Tx, error) { + // First round: we gather all the signer infos. We use the "set empty + // signature" hack to do that. + var sigsV2 []signing.SignatureV2 + for i, priv := range privs { + sigV2 := signing.SignatureV2{ + PubKey: priv.PubKey(), + Data: &signing.SingleSignatureData{ + SignMode: s.ClientCtx.TxConfig.SignModeHandler().DefaultMode(), + Signature: nil, + }, + Sequence: accSeqs[i], + } + + sigsV2 = append(sigsV2, sigV2) + } + err := s.TxBuilder.SetSignatures(sigsV2...) + if err != nil { + return nil, err + } + + // Second round: all signer infos are set, so each signer can sign. + sigsV2 = []signing.SignatureV2{} + for i, priv := range privs { + signerData := authsigning.SignerData{ + ChainID: chainID, + AccountNumber: accNums[i], + Sequence: accSeqs[i], + } + sigV2, err := tx.SignWithPrivKey( + s.ClientCtx.TxConfig.SignModeHandler().DefaultMode(), signerData, + s.TxBuilder, priv, s.ClientCtx.TxConfig, accSeqs[i]) + if err != nil { + return nil, err + } + + sigsV2 = append(sigsV2, sigV2) + } + err = s.TxBuilder.SetSignatures(sigsV2...) + if err != nil { + return nil, err + } + + return s.TxBuilder.GetTx(), nil +} + +// NewTestFeeAmount is a test fee amount. +func NewTestFeeAmount() sdk.Coins { + return sdk.NewCoins(sdk.NewInt64Coin("stake", 150)) +} + +// NewTestGasLimit is a test fee gas limit. +func NewTestGasLimit() uint64 { + return 200000 +} diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go new file mode 100644 index 0000000..7fd7e0d --- /dev/null +++ b/x/feemarket/post/fee.go @@ -0,0 +1,148 @@ +package post + +import ( + "fmt" + + errorsmod "cosmossdk.io/errors" + + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + + "github.com/skip-mev/feemarket/x/feemarket/ante" + feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" +) + +// FeeMarketDeductDecorator deducts fees from the fee payer based off of the current state of the feemarket. +// The fee payer is the fee granter (if specified) or first signer of the tx. +// If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. +// If there is an excess between the given fee and the on-chain base fee is given as a tip. +// Call next AnteHandler if fees successfully deducted. +// CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator +type FeeMarketDeductDecorator struct { + accountKeeper AccountKeeper + bankKeeper BankKeeper + feegrantKeeper FeeGrantKeeper + feemarketKeeper FeeMarketKeeper +} + +func NewFeeMarketDeductDecorator(ak AccountKeeper, bk BankKeeper, fk FeeGrantKeeper, fmk FeeMarketKeeper) FeeMarketDeductDecorator { + return FeeMarketDeductDecorator{ + accountKeeper: ak, + bankKeeper: bk, + feegrantKeeper: fk, + feemarketKeeper: fmk, + } +} + +func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simulate, success bool, next sdk.PostHandler) (sdk.Context, error) { + feeTx, ok := tx.(sdk.FeeTx) + if !ok { + return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + } + + if !simulate && ctx.BlockHeight() > 0 && feeTx.GetGas() == 0 { + return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") + } + + var tip sdk.Coins + + minGasPrices, err := dfd.feemarketKeeper.GetMinGasPrices(ctx) + if err != nil { + return ctx, errorsmod.Wrapf(err, "unable to get fee market state") + } + + fee := feeTx.GetFee() + gas := ctx.GasMeter().GasConsumed() // use context gas consumed + + if !simulate { + fee, tip, err = ante.CheckTxFees(minGasPrices, tx, gas) + if err != nil { + return ctx, err + } + } + + if err := dfd.checkDeductFeeAndTip(ctx, tx, fee, tip); err != nil { + return ctx, err + } + + return next(ctx, tx, simulate, success) +} + +func (dfd FeeMarketDeductDecorator) checkDeductFeeAndTip(ctx sdk.Context, sdkTx sdk.Tx, fee, tip sdk.Coins) error { + feeTx, ok := sdkTx.(sdk.FeeTx) + if !ok { + return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + } + + if addr := dfd.accountKeeper.GetModuleAddress(feemarkettypes.FeeCollectorName); addr == nil { + return fmt.Errorf("fee collector module account (%s) has not been set", feemarkettypes.FeeCollectorName) + } + + feePayer := feeTx.FeePayer() + feeGranter := feeTx.FeeGranter() + deductFeesFrom := feePayer + + // if feegranter set deduct fee from feegranter account. + // this works with only when feegrant enabled. + if feeGranter != nil { + if dfd.feegrantKeeper == nil { + return sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled") + } else if !feeGranter.Equals(feePayer) { + err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, sdkTx.GetMsgs()) + if err != nil { + return errorsmod.Wrapf(err, "%s does not allow to pay fees for %s", feeGranter, feePayer) + } + } + + deductFeesFrom = feeGranter + } + + deductFeesFromAcc := dfd.accountKeeper.GetAccount(ctx, deductFeesFrom) + if deductFeesFromAcc == nil { + return sdkerrors.ErrUnknownAddress.Wrapf("fee payer address: %s does not exist", deductFeesFrom) + } + + // deduct the fees + if !fee.IsZero() { + err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, fee) + if err != nil { + return err + } + } + + // deduct the tip + if !tip.IsZero() { + err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, tip) + if err != nil { + return err + } + } + + events := sdk.Events{ + sdk.NewEvent( + sdk.EventTypeTx, + sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()), + sdk.NewAttribute(sdk.AttributeKeyFeePayer, deductFeesFrom.String()), + sdk.NewAttribute(feemarkettypes.AttributeKeyTip, tip.String()), + sdk.NewAttribute(feemarkettypes.AttributeKeyTipPayer, sdk.AccAddress(ctx.BlockHeader().ProposerAddress).String()), + ), + } + ctx.EventManager().EmitEvents(events) + + return nil +} + +// DeductCoins deducts coins from the given account. Coins are sent to the feemarket module account. +func DeductCoins(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, coins sdk.Coins) error { + if !coins.IsValid() { + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid coin amount: %s", coins) + } + + err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), feemarkettypes.FeeCollectorName, coins) + if err != nil { + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) + } + + return nil +} diff --git a/x/feemarket/post/fee_test.go b/x/feemarket/post/fee_test.go new file mode 100644 index 0000000..b64ff99 --- /dev/null +++ b/x/feemarket/post/fee_test.go @@ -0,0 +1,128 @@ +package post_test + +import ( + "fmt" + "testing" + + "github.com/cosmos/cosmos-sdk/testutil/testdata" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/stretchr/testify/mock" + + sdk "github.com/cosmos/cosmos-sdk/types" + + antesuite "github.com/skip-mev/feemarket/x/feemarket/ante/suite" + "github.com/skip-mev/feemarket/x/feemarket/post" + "github.com/skip-mev/feemarket/x/feemarket/types" +) + +func TestDeductCoins(t *testing.T) { + tests := []struct { + name string + coins sdk.Coins + wantErr bool + invalidCoin bool + }{ + { + name: "valid", + coins: sdk.NewCoins(sdk.NewCoin("test", sdk.NewInt(10))), + wantErr: false, + }, + { + name: "valid no coins", + coins: sdk.NewCoins(), + wantErr: false, + }, + { + name: "invalid coins", + coins: sdk.Coins{sdk.Coin{Amount: sdk.NewInt(-1)}}, + wantErr: true, + invalidCoin: true, + }, + } + for _, tc := range tests { + t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { + s := antesuite.SetupTestSuite(t) + acc := s.CreateTestAccounts(1)[0] + if !tc.invalidCoin { + s.BankKeeper.On("SendCoinsFromAccountToModule", s.Ctx, acc.Account.GetAddress(), types.FeeCollectorName, tc.coins).Return(nil).Once() + } + + if err := post.DeductCoins(s.BankKeeper, s.Ctx, acc.Account, tc.coins); (err != nil) != tc.wantErr { + s.Errorf(err, "DeductCoins() error = %v, wantErr %v", err, tc.wantErr) + } + }) + } +} + +func TestPostHandle(t *testing.T) { + // Same data for every test case + gasLimit := antesuite.NewTestGasLimit() + validFeeAmount := types.DefaultMinBaseFee.MulRaw(int64(gasLimit)) + validFee := sdk.NewCoins(sdk.NewCoin("stake", validFeeAmount)) + + testCases := []antesuite.TestCase{ + { + "signer has no funds", + func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { + accs := suite.CreateTestAccounts(1) + suite.BankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(sdkerrors.ErrInsufficientFunds) + + return antesuite.TestCaseArgs{ + Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, + GasLimit: gasLimit, + FeeAmount: validFee, + } + }, + true, + true, + false, + false, + sdkerrors.ErrInsufficientFunds, + }, + { + "0 gas given should fail", + func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { + accs := suite.CreateTestAccounts(1) + + return antesuite.TestCaseArgs{ + Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, + GasLimit: 0, + FeeAmount: validFee, + } + }, + true, + true, + false, + false, + sdkerrors.ErrInvalidGasLimit, + }, + { + "signer has enough funds, should pass", + func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { + accs := suite.CreateTestAccounts(1) + suite.BankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil) + + return antesuite.TestCaseArgs{ + Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, + GasLimit: gasLimit, + FeeAmount: validFee, + } + }, + true, + true, + false, + true, + nil, + }, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) { + s := antesuite.SetupTestSuite(t) + s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder() + args := tc.Malleate(s) + + s.RunTestCase(t, tc, args) + }) + } +} diff --git a/x/feemarket/post/mocks/mock_account_keeper.go b/x/feemarket/post/mocks/mock_account_keeper.go index 207e219..fb833d3 100644 --- a/x/feemarket/post/mocks/mock_account_keeper.go +++ b/x/feemarket/post/mocks/mock_account_keeper.go @@ -102,7 +102,8 @@ func (_m *AccountKeeper) SetAccount(ctx types.Context, acc authtypes.AccountI) { func NewAccountKeeper(t interface { mock.TestingT Cleanup(func()) -}) *AccountKeeper { +}, +) *AccountKeeper { mock := &AccountKeeper{} mock.Mock.Test(t) diff --git a/x/feemarket/post/mocks/mock_bank_keeper.go b/x/feemarket/post/mocks/mock_bank_keeper.go index 9cae7d7..26d9807 100644 --- a/x/feemarket/post/mocks/mock_bank_keeper.go +++ b/x/feemarket/post/mocks/mock_bank_keeper.go @@ -67,7 +67,8 @@ func (_m *BankKeeper) SendCoinsFromAccountToModule(ctx types.Context, senderAddr func NewBankKeeper(t interface { mock.TestingT Cleanup(func()) -}) *BankKeeper { +}, +) *BankKeeper { mock := &BankKeeper{} mock.Mock.Test(t) diff --git a/x/feemarket/post/mocks/mock_feegrant_keeper.go b/x/feemarket/post/mocks/mock_feegrant_keeper.go index ddff891..aefc832 100644 --- a/x/feemarket/post/mocks/mock_feegrant_keeper.go +++ b/x/feemarket/post/mocks/mock_feegrant_keeper.go @@ -32,7 +32,8 @@ func (_m *FeeGrantKeeper) UseGrantedFees(ctx types.Context, granter types.AccAdd func NewFeeGrantKeeper(t interface { mock.TestingT Cleanup(func()) -}) *FeeGrantKeeper { +}, +) *FeeGrantKeeper { mock := &FeeGrantKeeper{} mock.Mock.Test(t) diff --git a/x/feemarket/post/mocks/mock_feemarket_keeper.go b/x/feemarket/post/mocks/mock_feemarket_keeper.go index 3b1cd67..aa59895 100644 --- a/x/feemarket/post/mocks/mock_feemarket_keeper.go +++ b/x/feemarket/post/mocks/mock_feemarket_keeper.go @@ -3,9 +3,10 @@ package mocks import ( - feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" mock "github.com/stretchr/testify/mock" + feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types" + types "github.com/cosmos/cosmos-sdk/types" ) @@ -69,7 +70,8 @@ func (_m *FeeMarketKeeper) GetState(ctx types.Context) (feemarkettypes.State, er func NewFeeMarketKeeper(t interface { mock.TestingT Cleanup(func()) -}) *FeeMarketKeeper { +}, +) *FeeMarketKeeper { mock := &FeeMarketKeeper{} mock.Mock.Test(t) From 34b43b878e5a4b6d52aab8722d60e728101f72fc Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 27 Nov 2023 13:51:13 -0500 Subject: [PATCH 40/55] add options --- tests/simapp/app.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/simapp/app.go b/tests/simapp/app.go index ca4e8cf..17e57d5 100644 --- a/tests/simapp/app.go +++ b/tests/simapp/app.go @@ -281,7 +281,12 @@ func NewSimApp( } app.App.SetAnteHandler(anteHandler) - postHandlerOptions := PostHandlerOptions{} + postHandlerOptions := PostHandlerOptions{ + AccountKeeper: app.AccountKeeper, + BankKeeper: app.BankKeeper, + FeeGrantKeeper: app.FeeGrantKeeper, + FeeMarketKeeper: app.FeeMarketKeeper, + } postHandler, err := NewPostHandler(postHandlerOptions) if err != nil { panic(err) From d9ddd15e71f80344b3c4b4f544dd078f418922b9 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 27 Nov 2023 13:51:58 -0500 Subject: [PATCH 41/55] clean --- tests/simapp/app.go | 3 ++- x/feemarket/types/state_fuzz_test.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/simapp/app.go b/tests/simapp/app.go index 17e57d5..8767cf2 100644 --- a/tests/simapp/app.go +++ b/tests/simapp/app.go @@ -279,7 +279,6 @@ func NewSimApp( if err != nil { panic(err) } - app.App.SetAnteHandler(anteHandler) postHandlerOptions := PostHandlerOptions{ AccountKeeper: app.AccountKeeper, @@ -292,6 +291,8 @@ func NewSimApp( panic(err) } + // set ante and post handlers + app.App.SetAnteHandler(anteHandler) app.App.SetPostHandler(postHandler) /**** Module Options ****/ diff --git a/x/feemarket/types/state_fuzz_test.go b/x/feemarket/types/state_fuzz_test.go index 661eaca..66c6a2f 100644 --- a/x/feemarket/types/state_fuzz_test.go +++ b/x/feemarket/types/state_fuzz_test.go @@ -4,8 +4,9 @@ import ( "testing" "cosmossdk.io/math" - "github.com/skip-mev/feemarket/x/feemarket/types" "github.com/stretchr/testify/require" + + "github.com/skip-mev/feemarket/x/feemarket/types" ) func FuzzDefaultFeeMarket(f *testing.F) { From e62a188631e724452460cb00101b5a67d3036820 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 27 Nov 2023 13:55:20 -0500 Subject: [PATCH 42/55] comments --- x/feemarket/ante/fee.go | 9 ++++----- x/feemarket/post/fee.go | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 661d9fe..730d95c 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -10,12 +10,11 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) -// FeeMarketCheckDecorator deducts fees from the fee payer based off of the current state of the feemarket. -// The fee payer is the fee granter (if specified) or first signer of the tx. +// FeeMarketCheckDecorator checks sufficient fees from the fee payer based off of the current +// state of the feemarket. // If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. -// If there is an excess between the given fee and the on-chain base fee is given as a tip. -// Call next AnteHandler if fees successfully deducted. -// CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator +// Call next AnteHandler if fees successfully checked. +// CONTRACT: Tx must implement FeeTx interface type FeeMarketCheckDecorator struct { feemarketKeeper FeeMarketKeeper } diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index 7fd7e0d..6886ef8 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -16,9 +16,9 @@ import ( // FeeMarketDeductDecorator deducts fees from the fee payer based off of the current state of the feemarket. // The fee payer is the fee granter (if specified) or first signer of the tx. // If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. -// If there is an excess between the given fee and the on-chain base fee is given as a tip. +// If there is an excess between the given fee and the on-chain min base fee is given as a tip. // Call next AnteHandler if fees successfully deducted. -// CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator +// CONTRACT: Tx must implement FeeTx interface type FeeMarketDeductDecorator struct { accountKeeper AccountKeeper bankKeeper BankKeeper From e4b6d3eba34b11906d894fbc4f76ed296b0378da Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 27 Nov 2023 14:06:06 -0500 Subject: [PATCH 43/55] clean --- tests/simapp/post.go | 4 ---- x/feemarket/ante/fee.go | 1 + x/feemarket/post/fee.go | 27 ++++++++++++--------------- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/tests/simapp/post.go b/tests/simapp/post.go index 460eb68..280a525 100644 --- a/tests/simapp/post.go +++ b/tests/simapp/post.go @@ -30,10 +30,6 @@ func NewPostHandler(options PostHandlerOptions) (sdk.PostHandler, error) { return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "feemarket keeper is required for post builder") } - if options.FeeGrantKeeper == nil { - return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "feegrant keeper is required for post builder") - } - postDecorators := []sdk.PostDecorator{ feemarketpost.NewFeeMarketDeductDecorator( options.AccountKeeper, diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 730d95c..82800a8 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -25,6 +25,7 @@ func NewFeeMarketCheckDecorator(fmk FeeMarketKeeper) FeeMarketCheckDecorator { } } +// AnteHandle checks if the tx provides sufficient fee to cover the required fee from the fee market. func (dfd FeeMarketCheckDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index 6886ef8..df68e6d 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -35,6 +35,9 @@ func NewFeeMarketDeductDecorator(ak AccountKeeper, bk BankKeeper, fk FeeGrantKee } } +// PostHandle deducts the fee from the fee payer based on the min base fee and the gas consumed in the gasmeter. +// If there is a difference between the provided fee and the min-base fee, the difference is paid as a tip. +// Fees are sent to the x/feemarket fee-collector address. func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simulate, success bool, next sdk.PostHandler) (sdk.Context, error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { @@ -62,14 +65,16 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul } } - if err := dfd.checkDeductFeeAndTip(ctx, tx, fee, tip); err != nil { + if err := dfd.DeductFeeAndTip(ctx, tx, fee, tip); err != nil { return ctx, err } return next(ctx, tx, simulate, success) } -func (dfd FeeMarketDeductDecorator) checkDeductFeeAndTip(ctx sdk.Context, sdkTx sdk.Tx, fee, tip sdk.Coins) error { +// DeductFeeAndTip deducts the provided fee and tip from the fee payer. +// If the tx uses a feegranter, the fee granter address will pay the fee instead of the tx signer. +func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.Tx, fee, tip sdk.Coins) error { feeTx, ok := sdkTx.(sdk.FeeTx) if !ok { return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") @@ -103,17 +108,9 @@ func (dfd FeeMarketDeductDecorator) checkDeductFeeAndTip(ctx sdk.Context, sdkTx return sdkerrors.ErrUnknownAddress.Wrapf("fee payer address: %s does not exist", deductFeesFrom) } - // deduct the fees - if !fee.IsZero() { - err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, fee) - if err != nil { - return err - } - } - - // deduct the tip - if !tip.IsZero() { - err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, tip) + // deduct the fees and tip + if !fee.Add(tip...).IsZero() { + err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, fee.Add(tip...)) if err != nil { return err } @@ -125,7 +122,7 @@ func (dfd FeeMarketDeductDecorator) checkDeductFeeAndTip(ctx sdk.Context, sdkTx sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()), sdk.NewAttribute(sdk.AttributeKeyFeePayer, deductFeesFrom.String()), sdk.NewAttribute(feemarkettypes.AttributeKeyTip, tip.String()), - sdk.NewAttribute(feemarkettypes.AttributeKeyTipPayer, sdk.AccAddress(ctx.BlockHeader().ProposerAddress).String()), + sdk.NewAttribute(feemarkettypes.AttributeKeyTipPayer, deductFeesFrom.String()), ), } ctx.EventManager().EmitEvents(events) @@ -133,7 +130,7 @@ func (dfd FeeMarketDeductDecorator) checkDeductFeeAndTip(ctx sdk.Context, sdkTx return nil } -// DeductCoins deducts coins from the given account. Coins are sent to the feemarket module account. +// DeductCoins deducts coins from the given account. Coins are sent to the feemarket fee collector account. func DeductCoins(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, coins sdk.Coins) error { if !coins.IsValid() { return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid coin amount: %s", coins) From 51898580b9f44b69d1f13950e9c4d5f344185a35 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 27 Nov 2023 14:07:06 -0500 Subject: [PATCH 44/55] rename --- x/feemarket/post/fee.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index df68e6d..d6f468f 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -17,7 +17,7 @@ import ( // The fee payer is the fee granter (if specified) or first signer of the tx. // If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. // If there is an excess between the given fee and the on-chain min base fee is given as a tip. -// Call next AnteHandler if fees successfully deducted. +// Call next PostHandler if fees successfully deducted. // CONTRACT: Tx must implement FeeTx interface type FeeMarketDeductDecorator struct { accountKeeper AccountKeeper From 265fd57ec418d8738fa7fcb5c1d66a74fbf6627f Mon Sep 17 00:00:00 2001 From: aljo242 Date: Mon, 27 Nov 2023 14:17:03 -0500 Subject: [PATCH 45/55] use names --- x/feemarket/ante/fee_test.go | 28 ++++++++++++------------ x/feemarket/post/fee_test.go | 42 ++++++++++++++++++------------------ 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go index 5aec90c..c8a4b4a 100644 --- a/x/feemarket/ante/fee_test.go +++ b/x/feemarket/ante/fee_test.go @@ -21,8 +21,8 @@ func TestAnteHandle(t *testing.T) { testCases := []antesuite.TestCase{ { - "0 gas given should fail", - func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { + Name: "0 gas given should fail", + Malleate: func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { accs := suite.CreateTestAccounts(1) return antesuite.TestCaseArgs{ @@ -31,15 +31,15 @@ func TestAnteHandle(t *testing.T) { FeeAmount: validFee, } }, - true, - false, - false, - false, - sdkerrors.ErrInvalidGasLimit, + RunAnte: true, + RunPost: false, + Simulate: false, + ExpPass: false, + ExpErr: sdkerrors.ErrInvalidGasLimit, }, { - "signer has enough funds, should pass", - func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { + Name: "signer has enough funds, should pass", + Malleate: func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { accs := suite.CreateTestAccounts(1) return antesuite.TestCaseArgs{ Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, @@ -47,11 +47,11 @@ func TestAnteHandle(t *testing.T) { FeeAmount: validFee, } }, - true, - false, - false, - true, - nil, + RunAnte: true, + RunPost: false, + Simulate: false, + ExpPass: true, + ExpErr: nil, }, } diff --git a/x/feemarket/post/fee_test.go b/x/feemarket/post/fee_test.go index b64ff99..d4c069a 100644 --- a/x/feemarket/post/fee_test.go +++ b/x/feemarket/post/fee_test.go @@ -62,8 +62,8 @@ func TestPostHandle(t *testing.T) { testCases := []antesuite.TestCase{ { - "signer has no funds", - func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { + Name: "signer has no funds", + Malleate: func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { accs := suite.CreateTestAccounts(1) suite.BankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(sdkerrors.ErrInsufficientFunds) @@ -73,15 +73,15 @@ func TestPostHandle(t *testing.T) { FeeAmount: validFee, } }, - true, - true, - false, - false, - sdkerrors.ErrInsufficientFunds, + RunAnte: true, + RunPost: true, + Simulate: false, + ExpPass: false, + ExpErr: sdkerrors.ErrInsufficientFunds, }, { - "0 gas given should fail", - func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { + Name: "0 gas given should fail", + Malleate: func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { accs := suite.CreateTestAccounts(1) return antesuite.TestCaseArgs{ @@ -90,15 +90,15 @@ func TestPostHandle(t *testing.T) { FeeAmount: validFee, } }, - true, - true, - false, - false, - sdkerrors.ErrInvalidGasLimit, + RunAnte: true, + RunPost: true, + Simulate: false, + ExpPass: false, + ExpErr: sdkerrors.ErrInvalidGasLimit, }, { - "signer has enough funds, should pass", - func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { + Name: "signer has enough funds, should pass", + Malleate: func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { accs := suite.CreateTestAccounts(1) suite.BankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil) @@ -108,11 +108,11 @@ func TestPostHandle(t *testing.T) { FeeAmount: validFee, } }, - true, - true, - false, - true, - nil, + RunAnte: true, + RunPost: true, + Simulate: false, + ExpPass: true, + ExpErr: nil, }, } From bda29014966a2b4f67e9f182d96fdaa545f99dd9 Mon Sep 17 00:00:00 2001 From: Alex Johnson Date: Tue, 28 Nov 2023 10:47:22 -0500 Subject: [PATCH 46/55] Update x/feemarket/types/expected_keepers.go Co-authored-by: David Terpay <35130517+davidterpay@users.noreply.github.com> --- x/feemarket/types/expected_keepers.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/feemarket/types/expected_keepers.go b/x/feemarket/types/expected_keepers.go index ff5f6f6..b455992 100644 --- a/x/feemarket/types/expected_keepers.go +++ b/x/feemarket/types/expected_keepers.go @@ -10,7 +10,6 @@ import ( //go:generate mockery --name AccountKeeper --filename mock_account_keeper.go type AccountKeeper interface { GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI - GetModuleAddress(name string) sdk.AccAddress GetModuleAccount(ctx sdk.Context, name string) authtypes.ModuleAccountI } From 5d9c9eb734d762d10e11a7af6f6601dfa7a51b12 Mon Sep 17 00:00:00 2001 From: Alex Johnson Date: Tue, 28 Nov 2023 10:47:42 -0500 Subject: [PATCH 47/55] Update x/feemarket/post/fee.go Co-authored-by: David Terpay <35130517+davidterpay@users.noreply.github.com> --- x/feemarket/post/fee.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index d6f468f..ec3dd12 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -109,8 +109,8 @@ func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.T } // deduct the fees and tip - if !fee.Add(tip...).IsZero() { - err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, fee.Add(tip...)) + if totalFee := fee.Add(tip...); !total.IsZero() { + err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, totalFee) if err != nil { return err } From b09b25a8d3a7f1d74599c443ec6970912f347d15 Mon Sep 17 00:00:00 2001 From: Alex Johnson Date: Tue, 28 Nov 2023 10:48:33 -0500 Subject: [PATCH 48/55] Update x/feemarket/ante/fee.go Co-authored-by: David Terpay <35130517+davidterpay@users.noreply.github.com> --- x/feemarket/ante/fee.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 82800a8..40466bc 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -56,7 +56,7 @@ func (dfd FeeMarketCheckDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula return next(newCtx, tx, simulate) } -// CheckTxFees implements the logic for the fee market to check if a Tx has provided suffucient +// CheckTxFees implements the logic for the fee market to check if a Tx has provided sufficient // fees given the current state of the fee market. Returns an error if insufficient fees. func CheckTxFees(minFees sdk.Coins, tx sdk.Tx, gas uint64) (feeCoins sdk.Coins, tip sdk.Coins, err error) { feeTx, ok := tx.(sdk.FeeTx) From 6992e7d57a94db6a9c279e1a0beac62f17dd1f29 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Tue, 28 Nov 2023 10:56:33 -0500 Subject: [PATCH 49/55] fix --- x/feemarket/post/fee.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index ec3dd12..255b6f3 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -109,7 +109,7 @@ func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.T } // deduct the fees and tip - if totalFee := fee.Add(tip...); !total.IsZero() { + if totalFee := fee.Add(tip...); !totalFee.IsZero() { err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, totalFee) if err != nil { return err From ceec81fa830bcae8d0253801f28dc8653d56f4de Mon Sep 17 00:00:00 2001 From: aljo242 Date: Tue, 28 Nov 2023 11:02:19 -0500 Subject: [PATCH 50/55] use feeTx --- x/feemarket/ante/fee.go | 9 ++------- x/feemarket/post/fee.go | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 40466bc..bccfb48 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -45,7 +45,7 @@ func (dfd FeeMarketCheckDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula gas := feeTx.GetGas() // use provided gas limit if !simulate { - fee, _, err = CheckTxFees(minGasPrices, tx, gas) + fee, _, err = CheckTxFees(minGasPrices, feeTx, gas) if err != nil { return ctx, err } @@ -58,12 +58,7 @@ func (dfd FeeMarketCheckDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula // CheckTxFees implements the logic for the fee market to check if a Tx has provided sufficient // fees given the current state of the fee market. Returns an error if insufficient fees. -func CheckTxFees(minFees sdk.Coins, tx sdk.Tx, gas uint64) (feeCoins sdk.Coins, tip sdk.Coins, err error) { - feeTx, ok := tx.(sdk.FeeTx) - if !ok { - return nil, nil, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") - } - +func CheckTxFees(minFees sdk.Coins, feeTx sdk.FeeTx, gas uint64) (feeCoins sdk.Coins, tip sdk.Coins, err error) { feesDec := sdk.NewDecCoinsFromCoins(minFees...) feeCoins = feeTx.GetFee() diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index 255b6f3..4e2c6b0 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -59,7 +59,7 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul gas := ctx.GasMeter().GasConsumed() // use context gas consumed if !simulate { - fee, tip, err = ante.CheckTxFees(minGasPrices, tx, gas) + fee, tip, err = ante.CheckTxFees(minGasPrices, feeTx, gas) if err != nil { return ctx, err } From ff44b2b8ed8454c8f377fc3be17e82314d8e8c1a Mon Sep 17 00:00:00 2001 From: aljo242 Date: Tue, 28 Nov 2023 11:23:32 -0500 Subject: [PATCH 51/55] fix --- x/feemarket/post/expected_keeper.go | 1 + x/feemarket/post/fee.go | 12 ++++++++++++ x/feemarket/post/mocks/mock_feemarket_keeper.go | 14 ++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/x/feemarket/post/expected_keeper.go b/x/feemarket/post/expected_keeper.go index 0e8d6b7..246e07d 100644 --- a/x/feemarket/post/expected_keeper.go +++ b/x/feemarket/post/expected_keeper.go @@ -41,5 +41,6 @@ type BankKeeper interface { //go:generate mockery --name FeeMarketKeeper --filename mock_feemarket_keeper.go type FeeMarketKeeper interface { GetState(ctx sdk.Context) (feemarkettypes.State, error) + SetState(ctx sdk.Context, state feemarkettypes.State) error GetMinGasPrices(ctx sdk.Context) (sdk.Coins, error) } diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index 4e2c6b0..2a5f20a 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -69,6 +69,18 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul return ctx, err } + // update fee market state + state, err := dfd.feemarketKeeper.GetState(ctx) + if err != nil { + return ctx, errorsmod.Wrapf(err, "unable to get fee market state") + } + + state.Window[state.Index] += gas + err = dfd.feemarketKeeper.SetState(ctx, state) + if err != nil { + return ctx, errorsmod.Wrapf(err, "unable to set fee market state") + } + return next(ctx, tx, simulate, success) } diff --git a/x/feemarket/post/mocks/mock_feemarket_keeper.go b/x/feemarket/post/mocks/mock_feemarket_keeper.go index aa59895..ccdd104 100644 --- a/x/feemarket/post/mocks/mock_feemarket_keeper.go +++ b/x/feemarket/post/mocks/mock_feemarket_keeper.go @@ -65,6 +65,20 @@ func (_m *FeeMarketKeeper) GetState(ctx types.Context) (feemarkettypes.State, er return r0, r1 } +// SetState provides a mock function with given fields: ctx, state +func (_m *FeeMarketKeeper) SetState(ctx types.Context, state feemarkettypes.State) error { + ret := _m.Called(ctx, state) + + var r0 error + if rf, ok := ret.Get(0).(func(types.Context, feemarkettypes.State) error); ok { + r0 = rf(ctx, state) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // NewFeeMarketKeeper creates a new instance of FeeMarketKeeper. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewFeeMarketKeeper(t interface { From 134a4c0362b19e7acdaae763c155cf948e4f14ab Mon Sep 17 00:00:00 2001 From: aljo242 Date: Tue, 28 Nov 2023 11:27:47 -0500 Subject: [PATCH 52/55] separate --- x/feemarket/post/fee.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index 2a5f20a..055f0c6 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -121,8 +121,15 @@ func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.T } // deduct the fees and tip - if totalFee := fee.Add(tip...); !totalFee.IsZero() { - err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, totalFee) + if fee.IsZero() { + err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, fee) + if err != nil { + return err + } + } + + if tip.IsZero() { + err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, tip) if err != nil { return err } From e7357bfc7ce75bcb397c9098b447b5a76bf00f7d Mon Sep 17 00:00:00 2001 From: aljo242 Date: Tue, 28 Nov 2023 11:40:34 -0500 Subject: [PATCH 53/55] refactor tip --- x/feemarket/post/fee.go | 20 +++++++++++++++--- x/feemarket/post/fee_test.go | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index 055f0c6..9603423 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -121,15 +121,15 @@ func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.T } // deduct the fees and tip - if fee.IsZero() { + if !fee.IsZero() { err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, fee) if err != nil { return err } } - if tip.IsZero() { - err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, tip) + if !tip.IsZero() { + err := SendTip(dfd.bankKeeper, ctx, deductFeesFromAcc.GetAddress(), ctx.BlockHeader().ProposerAddress, tip) if err != nil { return err } @@ -162,3 +162,17 @@ func DeductCoins(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, return nil } + +// SendTip sends a tip to the current block proposer. +func SendTip(bankKeeper BankKeeper, ctx sdk.Context, acc, proposer sdk.AccAddress, coins sdk.Coins) error { + if !coins.IsValid() { + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid coin amount: %s", coins) + } + + err := bankKeeper.SendCoins(ctx, acc, proposer, coins) + if err != nil { + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) + } + + return nil +} diff --git a/x/feemarket/post/fee_test.go b/x/feemarket/post/fee_test.go index d4c069a..17b0e3a 100644 --- a/x/feemarket/post/fee_test.go +++ b/x/feemarket/post/fee_test.go @@ -54,6 +54,45 @@ func TestDeductCoins(t *testing.T) { } } +func TestSendTip(t *testing.T) { + tests := []struct { + name string + coins sdk.Coins + wantErr bool + invalidCoin bool + }{ + { + name: "valid", + coins: sdk.NewCoins(sdk.NewCoin("test", sdk.NewInt(10))), + wantErr: false, + }, + { + name: "valid no coins", + coins: sdk.NewCoins(), + wantErr: false, + }, + { + name: "invalid coins", + coins: sdk.Coins{sdk.Coin{Amount: sdk.NewInt(-1)}}, + wantErr: true, + invalidCoin: true, + }, + } + for _, tc := range tests { + t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { + s := antesuite.SetupTestSuite(t) + accs := s.CreateTestAccounts(2) + if !tc.invalidCoin { + s.BankKeeper.On("SendCoins", s.Ctx, mock.Anything, mock.Anything, tc.coins).Return(nil).Once() + } + + if err := post.SendTip(s.BankKeeper, s.Ctx, accs[0].Account.GetAddress(), accs[1].Account.GetAddress(), tc.coins); (err != nil) != tc.wantErr { + s.Errorf(err, "SendCoins() error = %v, wantErr %v", err, tc.wantErr) + } + }) + } +} + func TestPostHandle(t *testing.T) { // Same data for every test case gasLimit := antesuite.NewTestGasLimit() @@ -101,6 +140,7 @@ func TestPostHandle(t *testing.T) { Malleate: func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { accs := suite.CreateTestAccounts(1) suite.BankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil) + suite.BankKeeper.On("SendCoins", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() return antesuite.TestCaseArgs{ Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, From 8fa833b21452fd4ad203ad0f6a38ea1ed722a5ae Mon Sep 17 00:00:00 2001 From: aljo242 Date: Tue, 28 Nov 2023 12:47:25 -0500 Subject: [PATCH 54/55] fix --- tests/integration/integration_test.go | 8 +-- tests/integration/setup.go | 95 ++++++++++++++++++++++++++- tests/integration/suite.go | 11 +++- 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 71e5624..6acf54a 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -17,8 +17,8 @@ import ( var ( // config params - numValidators = 1 - numFullNodes = 0 + numValidators = 3 + numFullNodes = 1 denom = "stake" image = ibc.DockerImage{ @@ -28,7 +28,7 @@ var ( } encodingConfig = MakeEncodingConfig() noHostMount = false - gasAdjustment = 2.0 + gasAdjustment = 10.0 genesisKV = []cosmos.GenesisKV{ { @@ -62,7 +62,7 @@ var ( Bech32Prefix: "cosmos", CoinType: "118", GasAdjustment: gasAdjustment, - GasPrices: fmt.Sprintf("20%s", denom), + GasPrices: fmt.Sprintf("200%s", denom), TrustingPeriod: "48h", NoHostMount: noHostMount, ModifyGenesis: cosmos.ModifyGenesis(genesisKV), diff --git a/tests/integration/setup.go b/tests/integration/setup.go index a92d193..714c620 100644 --- a/tests/integration/setup.go +++ b/tests/integration/setup.go @@ -6,7 +6,9 @@ import ( "context" "encoding/hex" "encoding/json" + "fmt" "io" + "math/rand" "os" "path" "strings" @@ -46,8 +48,8 @@ type KeyringOverride struct { // ChainBuilderFromChainSpec creates an interchaintest chain builder factory given a ChainSpec // and returns the associated chain func ChainBuilderFromChainSpec(t *testing.T, spec *interchaintest.ChainSpec) ibc.Chain { - // require that NumFullNodes == NumValidators == 4 - require.Equal(t, *spec.NumValidators, 1) + // require that NumFullNodes == NumValidators == 3 + require.Equal(t, *spec.NumValidators, 3) cf := interchaintest.NewBuiltinChainFactory(zaptest.NewLogger(t), []*interchaintest.ChainSpec{spec}) @@ -447,3 +449,92 @@ func (s *TestSuite) keyringDirFromNode() string { return localDir } + +// GetAndFundTestUserWithMnemonic restores a user using the given mnemonic +// and funds it with the native chain denom. +// The caller should wait for some blocks to complete before the funds will be accessible. +func (s *TestSuite) GetAndFundTestUserWithMnemonic( + ctx context.Context, + keyNamePrefix, mnemonic string, + amount int64, + chain *cosmos.CosmosChain, +) (ibc.Wallet, error) { + chainCfg := chain.Config() + keyName := fmt.Sprintf("%s-%s-%s", keyNamePrefix, chainCfg.ChainID, RandLowerCaseLetterString(3)) + user, err := chain.BuildWallet(ctx, keyName, mnemonic) + if err != nil { + return nil, fmt.Errorf("failed to get source user wallet: %w", err) + } + + err = chain.SendFunds(ctx, interchaintest.FaucetAccountKeyName, ibc.WalletAmount{ + Address: user.FormattedAddress(), + Amount: math.NewInt(amount), + Denom: chainCfg.Denom, + }) + + _, err = s.ExecTx( + ctx, + chain, + interchaintest.FaucetAccountKeyName, + "bank", + "send", + interchaintest.FaucetAccountKeyName, + user.FormattedAddress(), + fmt.Sprintf("%d%s", amount, chainCfg.Denom), + "--fees", + fmt.Sprintf("%d%s", 200000000000, chainCfg.Denom), + ) + + if err != nil { + return nil, fmt.Errorf("failed to get funds from faucet: %w", err) + } + return user, nil +} + +// GetAndFundTestUsers generates and funds chain users with the native chain denom. +// The caller should wait for some blocks to complete before the funds will be accessible. +func (s *TestSuite) GetAndFundTestUsers( + ctx context.Context, + keyNamePrefix string, + amount int64, + chains ...*cosmos.CosmosChain, +) []ibc.Wallet { + users := make([]ibc.Wallet, len(chains)) + var eg errgroup.Group + for i, chain := range chains { + i := i + chain := chain + eg.Go(func() error { + user, err := s.GetAndFundTestUserWithMnemonic(ctx, keyNamePrefix, "", amount, chain) + if err != nil { + return err + } + users[i] = user + return nil + }) + } + s.Require().NoError(eg.Wait()) + + // TODO(nix 05-17-2022): Map with generics once using go 1.18 + chainHeights := make([]testutil.ChainHeighter, len(chains)) + for i := range chains { + chainHeights[i] = chains[i] + } + return users +} + +func (s *TestSuite) ExecTx(ctx context.Context, chain *cosmos.CosmosChain, keyName string, command ...string) (string, error) { + node := chain.FullNodes[0] + return node.ExecTx(ctx, keyName, command...) +} + +var chars = []byte("abcdefghijklmnopqrstuvwxyz") + +// RandLowerCaseLetterString returns a lowercase letter string of given length +func RandLowerCaseLetterString(length int) string { + b := make([]byte, length) + for i := range b { + b[i] = chars[rand.Intn(len(chars))] + } + return string(b) +} diff --git a/tests/integration/suite.go b/tests/integration/suite.go index ce0276e..b5c973e 100644 --- a/tests/integration/suite.go +++ b/tests/integration/suite.go @@ -75,10 +75,15 @@ func (s *TestSuite) SetupSuite() { ctx := context.Background() s.ic = BuildInterchain(s.T(), ctx, s.chain) + cc, ok := s.chain.(*cosmos.CosmosChain) + if !ok { + panic("unable to assert ibc.Chain as CosmosChain") + } + // get the users - s.user1 = interchaintest.GetAndFundTestUsers(s.T(), ctx, s.T().Name(), initBalance, s.chain)[0] - s.user2 = interchaintest.GetAndFundTestUsers(s.T(), ctx, s.T().Name(), initBalance, s.chain)[0] - s.user3 = interchaintest.GetAndFundTestUsers(s.T(), ctx, s.T().Name(), initBalance, s.chain)[0] + s.user1 = s.GetAndFundTestUsers(ctx, s.T().Name(), initBalance, cc)[0] + s.user2 = s.GetAndFundTestUsers(ctx, s.T().Name(), initBalance, cc)[0] + s.user3 = s.GetAndFundTestUsers(ctx, s.T().Name(), initBalance, cc)[0] // create the broadcaster s.T().Log("creating broadcaster") From c65be636b6b55969b3014bd1af21469fade0efee Mon Sep 17 00:00:00 2001 From: aljo242 Date: Tue, 28 Nov 2023 12:59:00 -0500 Subject: [PATCH 55/55] use update --- x/feemarket/post/fee.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index 0e4d544..425c607 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -80,7 +80,11 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul return ctx, errorsmod.Wrapf(err, "unable to get fee market state") } - state.Window[state.Index] += gas + err = state.Update(gas) + if err != nil { + return ctx, errorsmod.Wrapf(err, "unable to update fee market state") + } + err = dfd.feemarketKeeper.SetState(ctx, state) if err != nil { return ctx, errorsmod.Wrapf(err, "unable to set fee market state")