refactor: Move TxDecoder into its own middleware (cosmos#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>
e-money · Dec 8, 2021 · bf9ba1b · bf9ba1b
1 parent eb1660c
commit bf9ba1b
Show file tree

Hide file tree

Showing 25 changed files with 295 additions and 201 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -112,6 +112,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (x/gov) [\#10373](https://github.com/cosmos/cosmos-sdk/pull/10373) Removed gov `keeper.{MustMarshal, MustUnmarshal}`.
 * [\#10348](https://github.com/cosmos/cosmos-sdk/pull/10348) StdSignBytes takes a new argument of type `*tx.Tip` for signing over tips using LEGACY_AMINO_JSON.
 * [\#10208](https://github.com/cosmos/cosmos-sdk/pull/10208) The `x/auth/signing.Tx` interface now also includes a new `GetTip() *tx.Tip` method for verifying tipped transactions. The `x/auth/types` expected BankKeeper interface now expects the `SendCoins` method too.
+* [\#10612](https://github.com/cosmos/cosmos-sdk/pull/10612) `baseapp.NewBaseApp` constructor function doesn't take the `sdk.TxDecoder` anymore. This logic has been moved into the TxDecoderMiddleware.
 
 ### Client Breaking Changes
 

diff --git a/baseapp/abci.go b/baseapp/abci.go
@@ -250,13 +250,8 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
 		panic(fmt.Sprintf("unknown RequestCheckTx type: %s", req.Type))
 	}
 
-	reqTx, err := app.txDecoder(req.Tx)
-	if err != nil {
-		return sdkerrors.ResponseCheckTx(err, 0, 0, app.trace)
-	}
-
 	ctx := app.getContextForTx(mode, req.Tx)
-	res, checkRes, err := app.txHandler.CheckTx(ctx, tx.Request{Tx: reqTx, TxBytes: req.Tx}, tx.RequestCheckTx{Type: req.Type})
+	res, checkRes, err := app.txHandler.CheckTx(ctx, tx.Request{TxBytes: req.Tx}, tx.RequestCheckTx{Type: req.Type})
 	if err != nil {
 		return sdkerrors.ResponseCheckTx(err, uint64(res.GasUsed), uint64(res.GasWanted), app.trace)
 	}
@@ -285,14 +280,9 @@ func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx
 			}
 		}
 	}()
-	reqTx, err := app.txDecoder(req.Tx)
-	if err != nil {
-		abciRes = sdkerrors.ResponseDeliverTx(err, 0, 0, app.trace)
-		return abciRes
-	}
 
 	ctx := app.getContextForTx(runTxModeDeliver, req.Tx)
-	res, err := app.txHandler.DeliverTx(ctx, tx.Request{Tx: reqTx, TxBytes: req.Tx})
+	res, err := app.txHandler.DeliverTx(ctx, tx.Request{TxBytes: req.Tx})
 	if err != nil {
 		abciRes = sdkerrors.ResponseDeliverTx(err, uint64(res.GasUsed), uint64(res.GasWanted), app.trace)
 		return abciRes

diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go
@@ -25,20 +25,20 @@ func TestGetBlockRentionHeight(t *testing.T) {
 		expected     int64
 	}{
 		"defaults": {
-			bapp:         baseapp.NewBaseApp(name, logger, db, nil),
+			bapp:         baseapp.NewBaseApp(name, logger, db),
 			maxAgeBlocks: 0,
 			commitHeight: 499000,
 			expected:     0,
 		},
 		"pruning unbonding time only": {
-			bapp:         baseapp.NewBaseApp(name, logger, db, nil, baseapp.SetMinRetainBlocks(1)),
+			bapp:         baseapp.NewBaseApp(name, logger, db, baseapp.SetMinRetainBlocks(1)),
 			maxAgeBlocks: 362880,
 			commitHeight: 499000,
 			expected:     136120,
 		},
 		"pruning iavl snapshot only": {
 			bapp: baseapp.NewBaseApp(
-				name, logger, db, nil,
+				name, logger, db,
 				baseapp.SetPruning(sdk.PruningOptions{KeepEvery: 10000}),
 				baseapp.SetMinRetainBlocks(1),
 			),
@@ -48,7 +48,7 @@ func TestGetBlockRentionHeight(t *testing.T) {
 		},
 		"pruning state sync snapshot only": {
 			bapp: baseapp.NewBaseApp(
-				name, logger, db, nil,
+				name, logger, db,
 				baseapp.SetSnapshotInterval(50000),
 				baseapp.SetSnapshotKeepRecent(3),
 				baseapp.SetMinRetainBlocks(1),
@@ -59,7 +59,7 @@ func TestGetBlockRentionHeight(t *testing.T) {
 		},
 		"pruning min retention only": {
 			bapp: baseapp.NewBaseApp(
-				name, logger, db, nil,
+				name, logger, db,
 				baseapp.SetMinRetainBlocks(400000),
 			),
 			maxAgeBlocks: 0,
@@ -68,7 +68,7 @@ func TestGetBlockRentionHeight(t *testing.T) {
 		},
 		"pruning all conditions": {
 			bapp: baseapp.NewBaseApp(
-				name, logger, db, nil,
+				name, logger, db,
 				baseapp.SetPruning(sdk.PruningOptions{KeepEvery: 10000}),
 				baseapp.SetMinRetainBlocks(400000),
 				baseapp.SetSnapshotInterval(50000), baseapp.SetSnapshotKeepRecent(3),
@@ -79,7 +79,7 @@ func TestGetBlockRentionHeight(t *testing.T) {
 		},
 		"no pruning due to no persisted state": {
 			bapp: baseapp.NewBaseApp(
-				name, logger, db, nil,
+				name, logger, db,
 				baseapp.SetPruning(sdk.PruningOptions{KeepEvery: 10000}),
 				baseapp.SetMinRetainBlocks(400000),
 				baseapp.SetSnapshotInterval(50000), baseapp.SetSnapshotKeepRecent(3),
@@ -90,7 +90,7 @@ func TestGetBlockRentionHeight(t *testing.T) {
 		},
 		"disable pruning": {
 			bapp: baseapp.NewBaseApp(
-				name, logger, db, nil,
+				name, logger, db,
 				baseapp.SetPruning(sdk.PruningOptions{KeepEvery: 10000}),
 				baseapp.SetMinRetainBlocks(0),
 				baseapp.SetSnapshotInterval(50000), baseapp.SetSnapshotKeepRecent(3),
@@ -127,7 +127,7 @@ func TestBaseAppCreateQueryContextRejectsNegativeHeights(t *testing.T) {
 	logger := defaultLogger()
 	db := dbm.NewMemDB()
 	name := t.Name()
-	app := baseapp.NewBaseApp(name, logger, db, nil)
+	app := baseapp.NewBaseApp(name, logger, db)
 
 	proves := []bool{
 		false, true,

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
@@ -52,7 +52,6 @@ type BaseApp struct { // nolint: maligned
 	queryRouter       sdk.QueryRouter      // router for redirecting query calls
 	grpcQueryRouter   *GRPCQueryRouter     // router for redirecting gRPC query calls
 	interfaceRegistry types.InterfaceRegistry
-	txDecoder         sdk.TxDecoder // unmarshal []byte into sdk.Tx
 
 	txHandler      tx.Handler       // txHandler for {Deliver,Check}Tx and simulations
 	initChainer    sdk.InitChainer  // initialize state with validators and state blob
@@ -137,7 +136,7 @@ type BaseApp struct { // nolint: maligned
 //
 // NOTE: The db is used to store the version number for now.
 func NewBaseApp(
-	name string, logger log.Logger, db dbm.DB, txDecoder sdk.TxDecoder, options ...func(*BaseApp),
+	name string, logger log.Logger, db dbm.DB, options ...func(*BaseApp),
 ) *BaseApp {
 	app := &BaseApp{
 		logger:          logger,
@@ -147,7 +146,6 @@ func NewBaseApp(
 		storeLoader:     DefaultStoreLoader,
 		queryRouter:     NewQueryRouter(),
 		grpcQueryRouter: NewGRPCQueryRouter(),
-		txDecoder:       txDecoder,
 		fauxMerkleMode:  false,
 	}