Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 30 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
d4f51a2
Refactor tx middleware to support pluggable feemarket module
yihuang Mar 18, 2022
a6271d9
changelog
yihuang Mar 18, 2022
8a06c23
add overflow protection
yihuang Mar 18, 2022
3c0b971
reject non-empty auth extension options in legacy amino mode
yihuang Mar 21, 2022
62f4789
fix review suggestions
yihuang Mar 22, 2022
aabd04b
update changelog
yihuang Mar 22, 2022
5ef3bd4
Update x/auth/middleware/static_feemarket.go
yihuang Mar 22, 2022
6cd75ab
review suggestions
yihuang Mar 22, 2022
88df9fa
Merge remote-tracking branch 'fork/feemarket-step-1' into feemarket-s…
yihuang Mar 22, 2022
6e2468f
update changelog
yihuang Mar 22, 2022
a74ca39
make FeeMarket in TxHandlerOptions optional
yihuang Mar 22, 2022
a1f5911
Apply suggestions from code review
yihuang Mar 22, 2022
93f1396
remove extension options in AuthInfo
yihuang Mar 22, 2022
12b95d2
update changelog
yihuang Mar 22, 2022
e6e754c
Merge branch 'feemarket-step-1' of https://github.com/yihuang/cosmos-…
yihuang Mar 22, 2022
75ae1ee
fix extension option middleware unit tests
yihuang Mar 22, 2022
1c66c62
update comments
yihuang Mar 23, 2022
2585d45
Update x/auth/middleware/ext.go
yihuang Mar 25, 2022
21deb29
Merge branch 'master' into feemarket-step-1
yihuang Mar 25, 2022
78817e2
Update CHANGELOG.md
yihuang Mar 29, 2022
5b79cc8
remove FeeMarket interface
yihuang Mar 29, 2022
eb5dc95
Merge remote-tracking branch 'fork/feemarket-step-1' into feemarket-s…
yihuang Mar 29, 2022
ff1baa6
Merge remote-tracking branch 'origin/master' into feemarket-step-1
yihuang Mar 29, 2022
9a2bbed
Update CHANGELOG.md
yihuang Mar 29, 2022
14ce986
remove FeeMarket interface
yihuang Mar 29, 2022
a579692
Merge branch 'feemarket-step-1' of https://github.com/yihuang/cosmos-…
yihuang Mar 29, 2022
0b93914
unpack extension options Any's
yihuang Mar 29, 2022
1b36270
fix unit test
yihuang Mar 29, 2022
3fa9280
Merge branch 'master' into feemarket-step-1
yihuang Mar 29, 2022
3b76647
Merge branch 'master' into feemarket-step-1
amaury1093 Mar 29, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 5 additions & 0 deletions testutil/testdata/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -31,6 +32,10 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
(*HasHasAnimalI)(nil),
&HasHasAnimal{},
)
registry.RegisterImplementations(
(*tx.TxExtensionOptionI)(nil),
&Cat{},
)

msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}
Expand Down
21 changes: 21 additions & 0 deletions types/tx/ext.go
Original file line number Diff line number Diff line change
@@ -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{}
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved

// 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
}
16 changes: 15 additions & 1 deletion types/tx/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
}
2 changes: 1 addition & 1 deletion x/auth/middleware/branch_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
47 changes: 32 additions & 15 deletions x/auth/middleware/ext.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
yihuang marked this conversation as resolved.
Show resolved Hide resolved
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
}
}
}

Expand All @@ -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
}

Expand All @@ -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
}

Expand All @@ -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
}

Expand Down
58 changes: 38 additions & 20 deletions x/auth/middleware/ext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
})
}
}