refactor(x/auth/middleware)!: tx middleware to support pluggable feemarket module

CHANGELOG.md

@@ -145,7 +145,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [\#10961](https://github.com/cosmos/cosmos-sdk/pull/10961) Support third-party modules to add extension snapshots to state-sync.
 * [\#11274](https://github.com/cosmos/cosmos-sdk/pull/11274) `types/errors.New` now is an alias for `types/errors.Register` and should only be used in initialization code.
 * (authz)[\#11060](https://github.com/cosmos/cosmos-sdk/pull/11060) `authz.NewMsgGrant` `expiration` is now a pointer. When `nil` is used then no expiration will be set (grant won't expire).
-
+* (ante) [#11413](https://github.com/cosmos/cosmos-sdk/pull/11413) Refactor tx middleware to support pluggable feemarket module. Merged `MempoolFeeMiddleware` and `TxPriorityMiddleware` functionalities into `DeductFeeMiddleware`, so the potential feemarket module can implement these logics in one pass. Added a `FeeMarket` interface to plugin custom feemarket implementation in tx middleware, the current fee logic is preserved by the default `ValidatorTxFee` implementation. Change `RejectExtensionOptionsMiddleware` to `NewExtensionOptionsMiddleware` to allow extension options needed by feemarket.
 
 ### Client Breaking Changes
 

x/auth/middleware/branch_store_test.go

@@ -72,7 +72,7 @@ func (s *MWTestSuite) TestBranchStore() {
 				middleware.NewTxDecoderMiddleware(s.clientCtx.TxConfig.TxDecoder()),
 				middleware.GasTxMiddleware,
 				middleware.RecoveryTxMiddleware,
-				middleware.DeductFeeMiddleware(s.app.AccountKeeper, s.app.BankKeeper, s.app.FeeGrantKeeper),
+				middleware.DeductFeeMiddleware(s.app.AccountKeeper, s.app.BankKeeper, s.app.FeeGrantKeeper, middleware.ValidatorTxFee{}),
 				middleware.IncrementSequenceMiddleware(s.app.AccountKeeper),
 				middleware.WithBranchedStore,
 				middleware.ConsumeBlockGasMiddleware,

x/auth/middleware/ext.go

@@ -14,27 +14,44 @@ type HasExtensionOptionsTx interface {
 	GetNonCriticalExtensionOptions() []*codectypes.Any
 }
 
+// ExtensionOptionChecker is a function that returns true if the extension option is accepted.
+type ExtensionOptionChecker func(*codectypes.Any) bool
+
+// rejectExtensionOption is the default extension check that reject all tx
+// extensions.
+func rejectExtensionOption(*codectypes.Any) bool {
+	return false
+}
+
 type rejectExtensionOptionsTxHandler struct {
-	next tx.Handler
+	next    tx.Handler
+	checker ExtensionOptionChecker
 }
 
-// RejectExtensionOptionsMiddleware creates a new rejectExtensionOptionsMiddleware.
-// rejectExtensionOptionsMiddleware is a middleware that rejects all extension
-// options which can optionally be included in protobuf transactions. Users that
-// need extension options should create a custom middleware chain that handles
-// needed extension options properly and rejects unknown ones.
-func RejectExtensionOptionsMiddleware(txh tx.Handler) tx.Handler {
-	return rejectExtensionOptionsTxHandler{
-		next: txh,
+// NewExtensionOptionsMiddleware creates a new middleware that rejects all extension
+// options which can optionally be included in protobuf transactions that don't pass the checker.
+// Users that need extension options should pass a custom checker that returns true for the
+// needed extension options.
+func NewExtensionOptionsMiddleware(checker ExtensionOptionChecker) tx.Middleware {
+	if checker == nil {
+		checker = rejectExtensionOption
+	}
+	return func(txh tx.Handler) tx.Handler {
+		return rejectExtensionOptionsTxHandler{
+			next:    txh,
+			checker: checker,
+		}
 	}
 }
 
 var _ tx.Handler = rejectExtensionOptionsTxHandler{}
 
-func checkExtOpts(tx sdk.Tx) error {
+func checkExtOpts(tx sdk.Tx, checker ExtensionOptionChecker) error {
 	if hasExtOptsTx, ok := tx.(HasExtensionOptionsTx); ok {
-		if len(hasExtOptsTx.GetExtensionOptions()) != 0 {
-			return sdkerrors.ErrUnknownExtensionOptions
+		for _, opt := range hasExtOptsTx.GetExtensionOptions() {
+			if !checker(opt) {
+				return sdkerrors.ErrUnknownExtensionOptions
+			}
 		}
 	}
 
@@ -43,7 +60,7 @@ func checkExtOpts(tx sdk.Tx) error {
 
 // CheckTx implements tx.Handler.CheckTx.
 func (txh rejectExtensionOptionsTxHandler) CheckTx(ctx context.Context, req tx.Request, checkReq tx.RequestCheckTx) (tx.Response, tx.ResponseCheckTx, error) {
-	if err := checkExtOpts(req.Tx); err != nil {
+	if err := checkExtOpts(req.Tx, txh.checker); err != nil {
 		return tx.Response{}, tx.ResponseCheckTx{}, err
 	}
 
@@ -52,7 +69,7 @@ func (txh rejectExtensionOptionsTxHandler) CheckTx(ctx context.Context, req tx.R
 
 // DeliverTx implements tx.Handler.DeliverTx.
 func (txh rejectExtensionOptionsTxHandler) DeliverTx(ctx context.Context, req tx.Request) (tx.Response, error) {
-	if err := checkExtOpts(req.Tx); err != nil {
+	if err := checkExtOpts(req.Tx, txh.checker); err != nil {
 		return tx.Response{}, err
 	}
 
@@ -61,7 +78,7 @@ func (txh rejectExtensionOptionsTxHandler) DeliverTx(ctx context.Context, req tx
 
 // SimulateTx implements tx.Handler.SimulateTx method.
 func (txh rejectExtensionOptionsTxHandler) SimulateTx(ctx context.Context, req tx.Request) (tx.Response, error) {
-	if err := checkExtOpts(req.Tx); err != nil {
+	if err := checkExtOpts(req.Tx, txh.checker); err != nil {
 		return tx.Response{}, err
 	}
 

x/auth/middleware/ext_test.go

@@ -2,35 +2,53 @@ package middleware_test
 
 import (
 	"github.com/cosmos/cosmos-sdk/codec/types"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	typestx "github.com/cosmos/cosmos-sdk/types/tx"
 	"github.com/cosmos/cosmos-sdk/x/auth/middleware"
 	"github.com/cosmos/cosmos-sdk/x/auth/tx"
 )
 
-func (s *MWTestSuite) TestRejectExtensionOptionsMiddleware() {
-	ctx := s.SetupTest(true) // setup
-	txBuilder := s.clientCtx.TxConfig.NewTxBuilder()
+func (s *MWTestSuite) TestExtensionOptionsMiddleware() {
+	testCases := []struct {
+		msg   string
+		allow bool
+	}{
+		{"allow extension", true},
+		{"reject extension", false},
+	}
+	for _, tc := range testCases {
+		s.Run(tc.msg, func() {
+			ctx := s.SetupTest(true) // setup
+			txBuilder := s.clientCtx.TxConfig.NewTxBuilder()
 
-	txHandler := middleware.ComposeMiddlewares(noopTxHandler, middleware.RejectExtensionOptionsMiddleware)
+			txHandler := middleware.ComposeMiddlewares(noopTxHandler, middleware.NewExtensionOptionsMiddleware(func(_ *codectypes.Any) bool {
+				return tc.allow
+			}))
 
-	// no extension options should not trigger an error
-	theTx := txBuilder.GetTx()
-	_, _, err := txHandler.CheckTx(sdk.WrapSDKContext(ctx), typestx.Request{Tx: theTx}, typestx.RequestCheckTx{})
-	s.Require().NoError(err)
+			// no extension options should not trigger an error
+			theTx := txBuilder.GetTx()
+			_, _, err := txHandler.CheckTx(sdk.WrapSDKContext(ctx), typestx.Request{Tx: theTx}, typestx.RequestCheckTx{})
+			s.Require().NoError(err)
 
-	extOptsTxBldr, ok := txBuilder.(tx.ExtensionOptionsTxBuilder)
-	if !ok {
-		// if we can't set extension options, this middleware doesn't apply and we're done
-		return
-	}
+			extOptsTxBldr, ok := txBuilder.(tx.ExtensionOptionsTxBuilder)
+			if !ok {
+				// if we can't set extension options, this middleware doesn't apply and we're done
+				return
+			}
 
-	// setting any extension option should cause an error
-	any, err := types.NewAnyWithValue(testdata.NewTestMsg())
-	s.Require().NoError(err)
-	extOptsTxBldr.SetExtensionOptions(any)
-	theTx = txBuilder.GetTx()
-	_, _, err = txHandler.CheckTx(sdk.WrapSDKContext(ctx), typestx.Request{Tx: theTx}, typestx.RequestCheckTx{})
-	s.Require().EqualError(err, "unknown extension options")
+			// set an extension option and check
+			any, err := types.NewAnyWithValue(testdata.NewTestMsg())
+			s.Require().NoError(err)
+			extOptsTxBldr.SetExtensionOptions(any)
+			theTx = txBuilder.GetTx()
+			_, _, err = txHandler.CheckTx(sdk.WrapSDKContext(ctx), typestx.Request{Tx: theTx}, typestx.RequestCheckTx{})
+			if tc.allow {
+				s.Require().NoError(err)
+			} else {
+				s.Require().EqualError(err, "unknown extension options")
+			}
+		})
+	}
 }

x/auth/middleware/fee.go

@@ -4,78 +4,21 @@ import (
 	"context"
 	"fmt"
 
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/types/tx"
 	"github.com/cosmos/cosmos-sdk/x/auth/types"
 )
 
-var _ tx.Handler = mempoolFeeTxHandler{}
-
-type mempoolFeeTxHandler struct {
-	next tx.Handler
-}
-
-// MempoolFeeMiddleware will check if the transaction's fee is at least as large
-// as the local validator's minimum gasFee (defined in validator config).
-// If fee is too low, middleware returns error and tx is rejected from mempool.
-// Note this only applies when ctx.CheckTx = true
-// If fee is high enough or not CheckTx, then call next middleware
-// CONTRACT: Tx must implement FeeTx to use MempoolFeeMiddleware
-func MempoolFeeMiddleware(txh tx.Handler) tx.Handler {
-	return mempoolFeeTxHandler{
-		next: txh,
-	}
-}
-
-// CheckTx implements tx.Handler.CheckTx. It is responsible for determining if a
-// transaction's fees meet the required minimum of the processing node. Note, a
-// node can have zero fees set as the minimum. If non-zero minimum fees are set
-// and the transaction does not meet the minimum, the transaction is rejected.
-//
-// Recall, a transaction's fee is determined by ceil(minGasPrice * gasLimit).
-func (txh mempoolFeeTxHandler) CheckTx(ctx context.Context, req tx.Request, checkReq tx.RequestCheckTx) (tx.Response, tx.ResponseCheckTx, error) {
-	sdkCtx := sdk.UnwrapSDKContext(ctx)
-
-	feeTx, ok := req.Tx.(sdk.FeeTx)
-	if !ok {
-		return tx.Response{}, tx.ResponseCheckTx{}, 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.
-	minGasPrices := sdkCtx.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 := sdk.NewDec(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 tx.Response{}, tx.ResponseCheckTx{}, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees)
-		}
-	}
-
-	return txh.next.CheckTx(ctx, req, checkReq)
-}
-
-// DeliverTx implements tx.Handler.DeliverTx.
-func (txh mempoolFeeTxHandler) DeliverTx(ctx context.Context, req tx.Request) (tx.Response, error) {
-	return txh.next.DeliverTx(ctx, req)
-}
-
-// SimulateTx implements tx.Handler.SimulateTx.
-func (txh mempoolFeeTxHandler) SimulateTx(ctx context.Context, req tx.Request) (tx.Response, error) {
-	return txh.next.SimulateTx(ctx, req)
+// FeeMarket defines the interface of feemarket system for tx middleware.
+type FeeMarket interface {
+	// AllowExtensionOption returns true for allowed extension option,
+	// Some feemarket modules need to extend the tx.
+	AllowExtensionOption(*codectypes.Any) bool
+	// CheckTxFee 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.
+	CheckTxFee(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error)
 }
 
 var _ tx.Handler = deductFeeTxHandler{}
@@ -84,26 +27,27 @@ type deductFeeTxHandler struct {
 	accountKeeper  AccountKeeper
 	bankKeeper     types.BankKeeper
 	feegrantKeeper FeegrantKeeper
+	feemarket      FeeMarket
 	next           tx.Handler
 }
 
 // DeductFeeMiddleware deducts fees from the first signer of the tx
 // If the first signer does not have the funds to pay for the fees, return with InsufficientFunds error
 // Call next middleware if fees successfully deducted
 // CONTRACT: Tx must implement FeeTx interface to use deductFeeTxHandler
-func DeductFeeMiddleware(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper) tx.Middleware {
+func DeductFeeMiddleware(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, fm FeeMarket) tx.Middleware {
 	return func(txh tx.Handler) tx.Handler {
 		return deductFeeTxHandler{
 			accountKeeper:  ak,
 			bankKeeper:     bk,
 			feegrantKeeper: fk,
+			feemarket:      fm,
 			next:           txh,
 		}
 	}
 }
 
-func (dfd deductFeeTxHandler) checkDeductFee(ctx context.Context, sdkTx sdk.Tx) error {
-	sdkCtx := sdk.UnwrapSDKContext(ctx)
+func (dfd deductFeeTxHandler) 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")
@@ -113,10 +57,8 @@ func (dfd deductFeeTxHandler) checkDeductFee(ctx context.Context, sdkTx sdk.Tx)
 		return fmt.Errorf("Fee collector module account (%s) has not been set", types.FeeCollectorName)
 	}
 
-	fee := feeTx.GetFee()
 	feePayer := feeTx.FeePayer()
 	feeGranter := feeTx.FeeGranter()
-
 	deductFeesFrom := feePayer
 
 	// if feegranter set deduct fee from feegranter account.
@@ -125,7 +67,7 @@ func (dfd deductFeeTxHandler) checkDeductFee(ctx context.Context, sdkTx sdk.Tx)
 		if dfd.feegrantKeeper == nil {
 			return sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled")
 		} else if !feeGranter.Equals(feePayer) {
-			err := dfd.feegrantKeeper.UseGrantedFees(sdkCtx, feeGranter, feePayer, fee, sdkTx.GetMsgs())
+			err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, sdkTx.GetMsgs())
 			if err != nil {
 				return sdkerrors.Wrapf(err, "%s does not not allow to pay fees for %s", feeGranter, feePayer)
 			}
@@ -134,47 +76,65 @@ func (dfd deductFeeTxHandler) checkDeductFee(ctx context.Context, sdkTx sdk.Tx)
 		deductFeesFrom = feeGranter
 	}
 
-	deductFeesFromAcc := dfd.accountKeeper.GetAccount(sdkCtx, deductFeesFrom)
+	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 !feeTx.GetFee().IsZero() {
-		err := DeductFees(dfd.bankKeeper, sdkCtx, deductFeesFromAcc, feeTx.GetFee())
+	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, feeTx.GetFee().String()),
+		sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()),
 	)}
-	sdkCtx.EventManager().EmitEvents(events)
+	ctx.EventManager().EmitEvents(events)
 
 	return nil
 }
 
 // CheckTx implements tx.Handler.CheckTx.
 func (dfd deductFeeTxHandler) CheckTx(ctx context.Context, req tx.Request, checkReq tx.RequestCheckTx) (tx.Response, tx.ResponseCheckTx, error) {
-	if err := dfd.checkDeductFee(ctx, req.Tx); err != nil {
+	sdkCtx := sdk.UnwrapSDKContext(ctx)
+	fee, priority, err := dfd.feemarket.CheckTxFee(sdkCtx, req.Tx)
+	if err != nil {
+		return tx.Response{}, tx.ResponseCheckTx{}, err
+	}
+	if err := dfd.checkDeductFee(sdkCtx, req.Tx, fee); err != nil {
 		return tx.Response{}, tx.ResponseCheckTx{}, err
 	}
 
-	return dfd.next.CheckTx(ctx, req, checkReq)
+	res, checkRes, err := dfd.next.CheckTx(ctx, req, checkReq)
+	checkRes.Priority = priority
+
+	return res, checkRes, err
 }
 
 // DeliverTx implements tx.Handler.DeliverTx.
 func (dfd deductFeeTxHandler) DeliverTx(ctx context.Context, req tx.Request) (tx.Response, error) {
-	if err := dfd.checkDeductFee(ctx, req.Tx); err != nil {
+	sdkCtx := sdk.UnwrapSDKContext(ctx)
+	fee, _, err := dfd.feemarket.CheckTxFee(sdkCtx, req.Tx)
+	if err != nil {
+		return tx.Response{}, err
+	}
+	if err := dfd.checkDeductFee(sdkCtx, req.Tx, fee); err != nil {
 		return tx.Response{}, err
 	}
 
 	return dfd.next.DeliverTx(ctx, req)
 }
 
 func (dfd deductFeeTxHandler) SimulateTx(ctx context.Context, req tx.Request) (tx.Response, error) {
-	if err := dfd.checkDeductFee(ctx, req.Tx); err != nil {
+	sdkCtx := sdk.UnwrapSDKContext(ctx)
+	fee, _, err := dfd.feemarket.CheckTxFee(sdkCtx, req.Tx)
+	if err != nil {
+		return tx.Response{}, err
+	}
+	if err := dfd.checkDeductFee(sdkCtx, req.Tx, fee); err != nil {
 		return tx.Response{}, err
 	}