refactor(x/auth/middleware)!: tx middleware to support pluggable feem…

…arket module (#11413) Closes: #11415 ## Description - Create `FeeMarket` interface, and move exiting static fee logic into `StaticFeeMarket` implementation. - Merged `MempoolFeeMiddleware` and `TxPriorityMiddleware` into `DeductFeeMiddleware`, so we can deduct fee based on the check result. - ~~Support extension options in `Tx.AuthInfo`, so feemarket module can extend the tx fields.~~ Keep in TxBody - No Ledger support in v0.46 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
cosmos · Mar 29, 2022 · e75c734 · e75c734
1 parent 4e92b62
commit e75c734
Show file tree

Hide file tree

Showing 14 changed files with 251 additions and 237 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -148,7 +148,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [\#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).
 * (x/distribution)[\#11457](https://github.com/cosmos/cosmos-sdk/pull/11457) Add amount field to `distr.MsgWithdrawDelegatorRewardResponse` and `distr.MsgWithdrawValidatorCommissionResponse`.
-
+* (x/auth/middleware) [#11413](https://github.com/cosmos/cosmos-sdk/pull/11413) Refactor tx middleware to be extensible on tx fee logic. Merged `MempoolFeeMiddleware` and `TxPriorityMiddleware` functionalities into `DeductFeeMiddleware`, make the logic extensible using the `TxFeeChecker` option, the current fee logic is preserved by the default `checkTxFeeWithValidatorMinGasPrices` implementation. Change `RejectExtensionOptionsMiddleware` to `NewExtensionOptionsMiddleware` which is extensible with the `ExtensionOptionChecker` option. Unpack the tx extension options `Any`s to interface `TxExtensionOptionI`.
 
 ### Client Breaking Changes
 

diff --git a/testutil/testdata/codec.go b/testutil/testdata/codec.go
@@ -6,6 +6,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/msgservice"
+	tx "github.com/cosmos/cosmos-sdk/types/tx"
 )
 
 func NewTestInterfaceRegistry() types.InterfaceRegistry {
@@ -31,6 +32,10 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
 		(*HasHasAnimalI)(nil),
 		&HasHasAnimal{},
 	)
+	registry.RegisterImplementations(
+		(*tx.TxExtensionOptionI)(nil),
+		&Cat{},
+	)
 
 	msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
 }

diff --git a/types/tx/ext.go b/types/tx/ext.go
@@ -0,0 +1,21 @@
+package tx
+
+import (
+	"github.com/cosmos/cosmos-sdk/codec/types"
+)
+
+// TxExtensionOptionI defines the interface for tx extension options
+type TxExtensionOptionI interface{}
+
+// unpackTxExtensionOptionsI unpacks Any's to TxExtensionOptionI's.
+func unpackTxExtensionOptionsI(unpacker types.AnyUnpacker, anys []*types.Any) error {
+	for _, any := range anys {
+		var opt TxExtensionOptionI
+		err := unpacker.UnpackAny(any, &opt)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
diff --git a/types/tx/types.go b/types/tx/types.go
@@ -173,7 +173,19 @@ func (t *Tx) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error {
 
 // UnpackInterfaces implements the UnpackInterfaceMessages.UnpackInterfaces method
 func (m *TxBody) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error {
-	return UnpackInterfaces(unpacker, m.Messages)
+	if err := UnpackInterfaces(unpacker, m.Messages); err != nil {
+		return err
+	}
+
+	if err := unpackTxExtensionOptionsI(unpacker, m.ExtensionOptions); err != nil {
+		return err
+	}
+
+	if err := unpackTxExtensionOptionsI(unpacker, m.NonCriticalExtensionOptions); err != nil {
+		return err
+	}
+
+	return nil
 }
 
 // UnpackInterfaces implements the UnpackInterfaceMessages.UnpackInterfaces method
@@ -200,4 +212,6 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
 
 	registry.RegisterInterface("cosmos.tx.v1beta1.Tx", (*sdk.Tx)(nil))
 	registry.RegisterImplementations((*sdk.Tx)(nil), &Tx{})
+
+	registry.RegisterInterface("cosmos.tx.v1beta1.TxExtensionOptionI", (*TxExtensionOptionI)(nil))
 }
diff --git a/x/auth/middleware/branch_store_test.go b/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, nil),
 				middleware.IncrementSequenceMiddleware(s.app.AccountKeeper),
 				middleware.WithBranchedStore,
 				middleware.ConsumeBlockGasMiddleware,

diff --git a/x/auth/middleware/ext.go b/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
 	}
 

diff --git a/x/auth/middleware/ext_test.go b/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")
+			}
+		})
+	}
 }