refactor: Move TxDecoder into its own middleware (#10612)

* WIP: middleware refactor

* refactor `tx.Request`

* Add MsgResponses any in sdk.Result

* add helper functions in abci

* refactor tips

* review changes

* Fix mock tests

* Update baseapp/abci.go

* Update baseapp/abci.go

* Update types/tx/middleware.go

* Update types/tx/middleware.go

* tx.Response to abci conversion

* refactor makeABCIData

* Add comments

* Fix build

* fix build error

* fix tests

* fix test

* fix tests

* Fix TestSimulateTx

* fix tests

* fix test

* Fix build

* Simplify code

* fix test build

* Use repeated bytes in txMsgData

* Fix grpc-gateway test

* Make proto-gen

* Automagically register MsgResponse

* review changes

* Use froydi's trick

* Use Any in TxMsgData

* Finally remove API breaking change

* Revert unnecessary stuff

* refactor: Move TxDecoder into its own middleware

* Add test for txDecoderMiddleware

* Fix some baseapp tests

* Fix some more tests

* Fix mock tests

* Fix middleware tests

* Add cl

* Fix tests

* Update types/tx/middleware.go

Co-authored-by: atheeshp <59333759+atheeshp@users.noreply.github.com>

Co-authored-by: atheesh <atheesh@vitwit.com>
Co-authored-by: atheeshp <59333759+atheeshp@users.noreply.github.com>

x/auth/middleware/basic_test.go

@@ -16,7 +16,7 @@ func (s *MWTestSuite) TestValidateBasic() {
 	ctx := s.SetupTest(true) // setup
 	txBuilder := s.clientCtx.TxConfig.NewTxBuilder()
 
-	txHandler := middleware.ComposeMiddlewares(noopTxHandler{}, middleware.ValidateBasicMiddleware)
+	txHandler := middleware.ComposeMiddlewares(noopTxHandler, middleware.ValidateBasicMiddleware)
 
 	// keys and addresses
 	priv1, _, addr1 := testdata.KeyTestPubAddr()
@@ -54,7 +54,7 @@ func (s *MWTestSuite) TestValidateBasic() {
 func (s *MWTestSuite) TestValidateMemo() {
 	ctx := s.SetupTest(true) // setup
 	txBuilder := s.clientCtx.TxConfig.NewTxBuilder()
-	txHandler := middleware.ComposeMiddlewares(noopTxHandler{}, middleware.ValidateMemoMiddleware(s.app.AccountKeeper))
+	txHandler := middleware.ComposeMiddlewares(noopTxHandler, middleware.ValidateMemoMiddleware(s.app.AccountKeeper))
 
 	// keys and addresses
 	priv1, _, addr1 := testdata.KeyTestPubAddr()
@@ -90,7 +90,7 @@ func (s *MWTestSuite) TestConsumeGasForTxSize() {
 	ctx := s.SetupTest(true) // setup
 	txBuilder := s.clientCtx.TxConfig.NewTxBuilder()
 
-	txHandler := middleware.ComposeMiddlewares(noopTxHandler{}, middleware.ConsumeTxSizeGasMiddleware(s.app.AccountKeeper))
+	txHandler := middleware.ComposeMiddlewares(noopTxHandler, middleware.ConsumeTxSizeGasMiddleware(s.app.AccountKeeper))
 
 	// keys and addresses
 	priv1, _, addr1 := testdata.KeyTestPubAddr()
@@ -174,7 +174,7 @@ func (s *MWTestSuite) TestConsumeGasForTxSize() {
 func (s *MWTestSuite) TestTxHeightTimeoutMiddleware() {
 	ctx := s.SetupTest(true)
 
-	txHandler := middleware.ComposeMiddlewares(noopTxHandler{}, middleware.TxTimeoutHeightMiddleware)
+	txHandler := middleware.ComposeMiddlewares(noopTxHandler, middleware.TxTimeoutHeightMiddleware)
 
 	// keys and addresses
 	priv1, _, addr1 := testdata.KeyTestPubAddr()

x/auth/middleware/ext_test.go

@@ -13,7 +13,7 @@ func (s *MWTestSuite) TestRejectExtensionOptionsMiddleware() {
 	ctx := s.SetupTest(true) // setup
 	txBuilder := s.clientCtx.TxConfig.NewTxBuilder()
 
-	txHandler := middleware.ComposeMiddlewares(noopTxHandler{}, middleware.RejectExtensionOptionsMiddleware)
+	txHandler := middleware.ComposeMiddlewares(noopTxHandler, middleware.RejectExtensionOptionsMiddleware)
 
 	// no extension options should not trigger an error
 	theTx := txBuilder.GetTx()

x/auth/middleware/fee_test.go

@@ -13,7 +13,7 @@ func (s *MWTestSuite) TestEnsureMempoolFees() {
 	ctx := s.SetupTest(true) // setup
 	txBuilder := s.clientCtx.TxConfig.NewTxBuilder()
 
-	txHandler := middleware.ComposeMiddlewares(noopTxHandler{}, middleware.MempoolFeeMiddleware)
+	txHandler := middleware.ComposeMiddlewares(noopTxHandler, middleware.MempoolFeeMiddleware)
 
 	// keys and addresses
 	priv1, _, addr1 := testdata.KeyTestPubAddr()
@@ -55,7 +55,7 @@ func (s *MWTestSuite) TestDeductFees() {
 	ctx := s.SetupTest(false) // setup
 	txBuilder := s.clientCtx.TxConfig.NewTxBuilder()
 	txHandler := middleware.ComposeMiddlewares(
-		noopTxHandler{},
+		noopTxHandler,
 		middleware.DeductFeeMiddleware(
 			s.app.AccountKeeper,
 			s.app.BankKeeper,

x/auth/middleware/feegrant_test.go

@@ -31,7 +31,7 @@ func (s *MWTestSuite) TestDeductFeesNoDelegation() {
 	protoTxCfg := tx.NewTxConfig(codec.NewProtoCodec(app.InterfaceRegistry()), tx.DefaultSignModes)
 
 	txHandler := middleware.ComposeMiddlewares(
-		noopTxHandler{},
+		noopTxHandler,
 		middleware.DeductFeeMiddleware(
 			s.app.AccountKeeper,
 			s.app.BankKeeper,

x/auth/middleware/gas_test.go

@@ -50,7 +50,7 @@ func (s *MWTestSuite) setupGasTx() (signing.Tx, []byte, sdk.Context, uint64) {
 
 func (s *MWTestSuite) TestSetup() {
 	testTx, _, ctx, gasLimit := s.setupGasTx()
-	txHandler := middleware.ComposeMiddlewares(noopTxHandler{}, middleware.GasTxMiddleware)
+	txHandler := middleware.ComposeMiddlewares(noopTxHandler, middleware.GasTxMiddleware)
 
 	testcases := []struct {
 		name        string
@@ -77,62 +77,48 @@ func (s *MWTestSuite) TestSetup() {
 
 func (s *MWTestSuite) TestRecoverPanic() {
 	testTx, txBytes, ctx, gasLimit := s.setupGasTx()
-	txHandler := middleware.ComposeMiddlewares(outOfGasTxHandler{}, middleware.GasTxMiddleware, middleware.RecoveryTxMiddleware)
+	txHandler := middleware.ComposeMiddlewares(outOfGasTxHandler, middleware.GasTxMiddleware, middleware.RecoveryTxMiddleware)
 	res, _, err := txHandler.CheckTx(sdk.WrapSDKContext(ctx), tx.Request{Tx: testTx, TxBytes: txBytes}, tx.RequestCheckTx{})
 	s.Require().Error(err, "Did not return error on OutOfGas panic")
 	s.Require().True(errors.Is(sdkerrors.ErrOutOfGas, err), "Returned error is not an out of gas error")
 	s.Require().Equal(gasLimit, uint64(res.GasWanted))
 
-	txHandler = middleware.ComposeMiddlewares(outOfGasTxHandler{}, middleware.GasTxMiddleware)
+	txHandler = middleware.ComposeMiddlewares(outOfGasTxHandler, middleware.GasTxMiddleware)
 	s.Require().Panics(func() {
 		txHandler.CheckTx(sdk.WrapSDKContext(ctx), tx.Request{Tx: testTx, TxBytes: txBytes}, tx.RequestCheckTx{})
 	}, "Recovered from non-Out-of-Gas panic")
 }
 
-// outOfGasTxHandler is a test middleware that will throw OutOfGas panic.
-type outOfGasTxHandler struct{}
-
-var _ tx.Handler = outOfGasTxHandler{}
-
-func (txh outOfGasTxHandler) DeliverTx(ctx context.Context, _ tx.Request) (tx.Response, error) {
-	sdkCtx := sdk.UnwrapSDKContext(ctx)
-	overLimit := sdkCtx.GasMeter().Limit() + 1
-
-	// Should panic with outofgas error
-	sdkCtx.GasMeter().ConsumeGas(overLimit, "test panic")
-
-	panic("not reached")
-}
-func (txh outOfGasTxHandler) CheckTx(ctx context.Context, _ tx.Request, _ tx.RequestCheckTx) (tx.Response, tx.ResponseCheckTx, error) {
-	sdkCtx := sdk.UnwrapSDKContext(ctx)
-	overLimit := sdkCtx.GasMeter().Limit() + 1
-
-	// Should panic with outofgas error
-	sdkCtx.GasMeter().ConsumeGas(overLimit, "test panic")
-
-	panic("not reached")
-}
-func (txh outOfGasTxHandler) SimulateTx(ctx context.Context, _ tx.Request) (tx.Response, error) {
-	sdkCtx := sdk.UnwrapSDKContext(ctx)
-	overLimit := sdkCtx.GasMeter().Limit() + 1
-
-	// Should panic with outofgas error
-	sdkCtx.GasMeter().ConsumeGas(overLimit, "test panic")
-
-	panic("not reached")
+// customTxHandler is a test middleware that will run a custom function.
+type customTxHandler struct {
+	fn func(context.Context, tx.Request) (tx.Response, error)
 }
 
-// noopTxHandler is a test middleware that does nothing.
-type noopTxHandler struct{}
+var _ tx.Handler = customTxHandler{}
 
-var _ tx.Handler = noopTxHandler{}
-
-func (txh noopTxHandler) CheckTx(_ context.Context, _ tx.Request, _ tx.RequestCheckTx) (tx.Response, tx.ResponseCheckTx, error) {
-	return tx.Response{}, tx.ResponseCheckTx{}, nil
+func (h customTxHandler) DeliverTx(ctx context.Context, req tx.Request) (tx.Response, error) {
+	return h.fn(ctx, req)
 }
-func (txh noopTxHandler) SimulateTx(_ context.Context, _ tx.Request) (tx.Response, error) {
+func (h customTxHandler) CheckTx(ctx context.Context, req tx.Request, _ tx.RequestCheckTx) (tx.Response, tx.ResponseCheckTx, error) {
+	res, err := h.fn(ctx, req)
+	return res, tx.ResponseCheckTx{}, err
+}
+func (h customTxHandler) SimulateTx(ctx context.Context, req tx.Request) (tx.Response, error) {
+	return h.fn(ctx, req)
+}
+
+// noopTxHandler is a test middleware that returns an empty response.
+var noopTxHandler = customTxHandler{func(_ context.Context, _ tx.Request) (tx.Response, error) {
 	return tx.Response{}, nil
-}
-func (txh noopTxHandler) DeliverTx(ctx context.Context, _ tx.Request) (tx.Response, error) {
-	return tx.Response{}, nil
-}
+}}
+
+// outOfGasTxHandler is a test middleware that panics with an outOfGas error.
+var outOfGasTxHandler = customTxHandler{func(ctx context.Context, _ tx.Request) (tx.Response, error) {
+	sdkCtx := sdk.UnwrapSDKContext(ctx)
+	overLimit := sdkCtx.GasMeter().Limit() + 1
+
+	// Should panic with outofgas error
+	sdkCtx.GasMeter().ConsumeGas(overLimit, "test panic")
+
+	panic("not reached")
+}}

x/auth/middleware/middleware.go

@@ -32,6 +32,10 @@ func ComposeMiddlewares(txHandler tx.Handler, middlewares ...tx.Middleware) tx.H
 
 type TxHandlerOptions struct {
 	Debug bool
+
+	// TxDecoder is used to decode the raw tx bytes into a sdk.Tx.
+	TxDecoder sdk.TxDecoder
+
 	// IndexEvents defines the set of events in the form {eventType}.{attributeKey},
 	// which informs Tendermint what to index. If empty, all events will be indexed.
 	IndexEvents map[string]struct{}
@@ -49,6 +53,10 @@ type TxHandlerOptions struct {
 // NewDefaultTxHandler defines a TxHandler middleware stacks that should work
 // for most applications.
 func NewDefaultTxHandler(options TxHandlerOptions) (tx.Handler, error) {
+	if options.TxDecoder == nil {
+		return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "txDecoder is required for middlewares")
+	}
+
 	if options.AccountKeeper == nil {
 		return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "account keeper is required for middlewares")
 	}
@@ -68,6 +76,7 @@ func NewDefaultTxHandler(options TxHandlerOptions) (tx.Handler, error) {
 
 	return ComposeMiddlewares(
 		NewRunMsgsTxHandler(options.MsgServiceRouter, options.LegacyRouter),
+		NewTxDecoderMiddleware(options.TxDecoder),
 		// Set a new GasMeter on sdk.Context.
 		//
 		// Make sure the Gas middleware is outside of all other middlewares

x/auth/middleware/middleware_test.go

@@ -1025,6 +1025,7 @@ func (s *MWTestSuite) TestCustomSignatureVerificationGasConsumer() {
 					return sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey)
 				}
 			},
+			TxDecoder: s.clientCtx.TxConfig.TxDecoder(),
 		},
 	)
 	s.Require().NoError(err)

x/auth/middleware/priority_test.go

@@ -12,7 +12,7 @@ func (s *MWTestSuite) TestPriority() {
 	ctx := s.SetupTest(true) // setup
 	txBuilder := s.clientCtx.TxConfig.NewTxBuilder()
 
-	txHandler := middleware.ComposeMiddlewares(noopTxHandler{}, middleware.TxPriorityMiddleware)
+	txHandler := middleware.ComposeMiddlewares(noopTxHandler, middleware.TxPriorityMiddleware)
 
 	// keys and addresses
 	priv1, _, addr1 := testdata.KeyTestPubAddr()

x/auth/middleware/sigverify_test.go

@@ -27,7 +27,7 @@ func (s *MWTestSuite) TestSetPubKey() {
 	txBuilder := s.clientCtx.TxConfig.NewTxBuilder()
 	require := s.Require()
 	txHandler := middleware.ComposeMiddlewares(
-		noopTxHandler{},
+		noopTxHandler,
 		middleware.SetPubKeyMiddleware(s.app.AccountKeeper),
 	)
 
@@ -127,7 +127,7 @@ func (s *MWTestSuite) TestSigVerification() {
 	// make block height non-zero to ensure account numbers part of signBytes
 	ctx = ctx.WithBlockHeight(1)
 	txHandler := middleware.ComposeMiddlewares(
-		noopTxHandler{},
+		noopTxHandler,
 		middleware.SetPubKeyMiddleware(s.app.AccountKeeper),
 		middleware.SigVerificationMiddleware(
 			s.app.AccountKeeper,
@@ -239,7 +239,7 @@ func (s *MWTestSuite) TestSigVerification_ExplicitAmino() {
 	gasLimit := testdata.NewTestGasLimit()
 
 	txHandler := middleware.ComposeMiddlewares(
-		noopTxHandler{},
+		noopTxHandler,
 		middleware.SetPubKeyMiddleware(s.app.AccountKeeper),
 		middleware.SigVerificationMiddleware(
 			s.app.AccountKeeper,
@@ -342,7 +342,7 @@ func (s *MWTestSuite) runSigMiddlewares(params types.Params, _ bool, privs ...cr
 	s.Require().NoError(err)
 
 	txHandler := middleware.ComposeMiddlewares(
-		noopTxHandler{},
+		noopTxHandler,
 		middleware.SetPubKeyMiddleware(s.app.AccountKeeper),
 		middleware.SigGasConsumeMiddleware(s.app.AccountKeeper, middleware.DefaultSigVerificationGasConsumer),
 		middleware.SigVerificationMiddleware(
@@ -382,7 +382,7 @@ func (s *MWTestSuite) TestIncrementSequenceMiddleware() {
 	s.Require().NoError(err)
 
 	txHandler := middleware.ComposeMiddlewares(
-		noopTxHandler{},
+		noopTxHandler,
 		middleware.IncrementSequenceMiddleware(s.app.AccountKeeper),
 	)
 

x/auth/middleware/testutil_test.go

@@ -89,6 +89,7 @@ func (s *MWTestSuite) SetupTest(isCheckTx bool) sdk.Context {
 		FeegrantKeeper:   s.app.FeeGrantKeeper,
 		SignModeHandler:  encodingConfig.TxConfig.SignModeHandler(),
 		SigGasConsumer:   middleware.DefaultSigVerificationGasConsumer,
+		TxDecoder:        s.clientCtx.TxConfig.TxDecoder(),
 	})
 	s.Require().NoError(err)
 	s.txHandler = txHandler

x/auth/middleware/tx.go

@@ -0,0 +1,77 @@
+package middleware
+
+import (
+	"context"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/cosmos/cosmos-sdk/types/tx"
+)
+
+type txDecoderHandler struct {
+	next      tx.Handler
+	txDecoder sdk.TxDecoder
+}
+
+// NewTxDecoderMiddleware creates a new middleware that will decode tx bytes
+// into a sdk.Tx. As input request, at least one of Tx or TxBytes must be set.
+// If only TxBytes is set, then TxDecoderMiddleware will populate the Tx field.
+// If only Tx is set, then TxBytes will be left empty, but some middlewares
+// such as signature verification might fail.
+func NewTxDecoderMiddleware(txDecoder sdk.TxDecoder) tx.Middleware {
+	return func(txh tx.Handler) tx.Handler {
+		return txDecoderHandler{next: txh, txDecoder: txDecoder}
+	}
+}
+
+var _ tx.Handler = gasTxHandler{}
+
+// CheckTx implements tx.Handler.CheckTx.
+func (h txDecoderHandler) CheckTx(ctx context.Context, req tx.Request, checkReq tx.RequestCheckTx) (tx.Response, tx.ResponseCheckTx, error) {
+	newReq, err := h.populateReq(req)
+	if err != nil {
+		return tx.Response{}, tx.ResponseCheckTx{}, err
+	}
+
+	return h.next.CheckTx(ctx, newReq, checkReq)
+}
+
+// DeliverTx implements tx.Handler.DeliverTx.
+func (h txDecoderHandler) DeliverTx(ctx context.Context, req tx.Request) (tx.Response, error) {
+	newReq, err := h.populateReq(req)
+	if err != nil {
+		return tx.Response{}, err
+	}
+
+	return h.next.DeliverTx(ctx, newReq)
+}
+
+// SimulateTx implements tx.Handler.SimulateTx method.
+func (h txDecoderHandler) SimulateTx(ctx context.Context, req tx.Request) (tx.Response, error) {
+	newReq, err := h.populateReq(req)
+	if err != nil {
+		return tx.Response{}, err
+	}
+
+	return h.next.SimulateTx(ctx, newReq)
+}
+
+// populateReq takes a tx.Request, and if its Tx field is not set, then
+// decodes the TxBytes and populates the decoded Tx field. It leaves
+// req.TxBytes untouched.
+func (h txDecoderHandler) populateReq(req tx.Request) (tx.Request, error) {
+	if len(req.TxBytes) == 0 && req.Tx == nil {
+		return tx.Request{}, sdkerrors.ErrInvalidRequest.Wrap("got empty tx request")
+	}
+
+	sdkTx := req.Tx
+	var err error
+	if len(req.TxBytes) != 0 {
+		sdkTx, err = h.txDecoder(req.TxBytes)
+		if err != nil {
+			return tx.Request{}, err
+		}
+	}
+
+	return tx.Request{Tx: sdkTx, TxBytes: req.TxBytes}, nil
+}

x/auth/middleware/tx_test.go

@@ -0,0 +1,57 @@
+package middleware_test
+
+import (
+	"context"
+
+	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
+	"github.com/cosmos/cosmos-sdk/testutil/testdata"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/tx"
+	"github.com/cosmos/cosmos-sdk/x/auth/middleware"
+)
+
+func (s *MWTestSuite) TestTxDecoderMiddleware() {
+	ctx := s.SetupTest(true) // setup
+	require := s.Require()
+
+	// Create a tx.
+	priv1, _, addr1 := testdata.KeyTestPubAddr()
+	txBuilder := s.clientCtx.TxConfig.NewTxBuilder()
+	err := txBuilder.SetMsgs(testdata.NewTestMsg(addr1))
+	require.NoError(err)
+	sdkTx, txBz, err := s.createTestTx(txBuilder, []cryptotypes.PrivKey{priv1}, []uint64{1}, []uint64{0}, ctx.ChainID())
+	require.NoError(err)
+
+	// Create a custom tx.Handler that checks that the req.Tx field is
+	// correctly populated.
+	txReqChecker := customTxHandler{func(c context.Context, r tx.Request) (tx.Response, error) {
+		require.NotNil(r.Tx)
+		require.Equal(sdkTx.GetMsgs()[0], r.Tx.GetMsgs()[0])
+		return tx.Response{}, nil
+	}}
+
+	testcases := []struct {
+		name   string
+		req    tx.Request
+		expErr bool
+	}{
+		{"empty tx bz", tx.Request{}, true},
+		{"tx bz and tx both given as inputs", tx.Request{Tx: sdkTx, TxBytes: txBz}, false},
+		{"tx bz only given as input", tx.Request{TxBytes: txBz}, false},
+		{"tx only given as input", tx.Request{Tx: sdkTx}, false},
+	}
+	for _, tc := range testcases {
+		s.Run(tc.name, func() {
+			txHandler := middleware.ComposeMiddlewares(
+				txReqChecker,
+				middleware.NewTxDecoderMiddleware(s.clientCtx.TxConfig.TxDecoder()),
+			)
+			_, err := txHandler.DeliverTx(sdk.WrapSDKContext(ctx), tc.req)
+			if tc.expErr {
+				require.Error(err)
+			} else {
+				require.NoError(err)
+			}
+		})
+	}
+}