refactor: Move TxDecoder into its own middleware

cosmos · Dec 1, 2021 · 963e5b6 · 963e5b6
1 parent ceb97a4
commit 963e5b6
Show file tree

Hide file tree

Showing 15 changed files with 118 additions and 102 deletions.
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,
 	}
 

diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go
@@ -90,7 +90,7 @@ func newBaseApp(name string, options ...func(*baseapp.BaseApp)) *baseapp.BaseApp
 	db := dbm.NewMemDB()
 	codec := codec.NewLegacyAmino()
 	registerTestCodec(codec)
-	return baseapp.NewBaseApp(name, logger, db, testTxDecoder(codec), options...)
+	return baseapp.NewBaseApp(name, logger, db, options...)
 }
 
 func registerTestCodec(cdc *codec.LegacyAmino) {
@@ -245,7 +245,7 @@ func TestLoadVersion(t *testing.T) {
 	pruningOpt := baseapp.SetPruning(storetypes.PruneNothing)
 	db := dbm.NewMemDB()
 	name := t.Name()
-	app := baseapp.NewBaseApp(name, logger, db, nil, pruningOpt)
+	app := baseapp.NewBaseApp(name, logger, db, pruningOpt)
 
 	// make a cap key and mount the store
 	err := app.LoadLatestVersion() // needed to make stores non-nil
@@ -272,15 +272,15 @@ func TestLoadVersion(t *testing.T) {
 	commitID2 := storetypes.CommitID{Version: 2, Hash: res.Data}
 
 	// reload with LoadLatestVersion
-	app = baseapp.NewBaseApp(name, logger, db, nil, pruningOpt)
+	app = baseapp.NewBaseApp(name, logger, db, pruningOpt)
 	app.MountStores()
 	err = app.LoadLatestVersion()
 	require.Nil(t, err)
 	testLoadVersionHelper(t, app, int64(2), commitID2)
 
 	// reload with LoadVersion, see if you can commit the same block and get
 	// the same result
-	app = baseapp.NewBaseApp(name, logger, db, nil, pruningOpt)
+	app = baseapp.NewBaseApp(name, logger, db, pruningOpt)
 	err = app.LoadVersion(1)
 	require.Nil(t, err)
 	testLoadVersionHelper(t, app, int64(1), commitID1)
@@ -359,7 +359,7 @@ func TestSetLoader(t *testing.T) {
 			if tc.setLoader != nil {
 				opts = append(opts, tc.setLoader)
 			}
-			app := baseapp.NewBaseApp(t.Name(), defaultLogger(), db, nil, opts...)
+			app := baseapp.NewBaseApp(t.Name(), defaultLogger(), db, opts...)
 			app.MountStores(sdk.NewKVStoreKey(tc.loadStoreKey))
 			err := app.LoadLatestVersion()
 			require.Nil(t, err)
@@ -381,7 +381,7 @@ func TestVersionSetterGetter(t *testing.T) {
 	pruningOpt := baseapp.SetPruning(storetypes.PruneDefault)
 	db := dbm.NewMemDB()
 	name := t.Name()
-	app := baseapp.NewBaseApp(name, logger, db, nil, pruningOpt)
+	app := baseapp.NewBaseApp(name, logger, db, pruningOpt)
 
 	require.Equal(t, "", app.Version())
 	res := app.Query(abci.RequestQuery{Path: "app/version"})
@@ -401,7 +401,7 @@ func TestLoadVersionInvalid(t *testing.T) {
 	pruningOpt := baseapp.SetPruning(storetypes.PruneNothing)
 	db := dbm.NewMemDB()
 	name := t.Name()
-	app := baseapp.NewBaseApp(name, logger, db, nil, pruningOpt)
+	app := baseapp.NewBaseApp(name, logger, db, pruningOpt)
 
 	err := app.LoadLatestVersion()
 	require.Nil(t, err)
@@ -416,7 +416,7 @@ func TestLoadVersionInvalid(t *testing.T) {
 	commitID1 := storetypes.CommitID{Version: 1, Hash: res.Data}
 
 	// create a new app with the stores mounted under the same cap key
-	app = baseapp.NewBaseApp(name, logger, db, nil, pruningOpt)
+	app = baseapp.NewBaseApp(name, logger, db, pruningOpt)
 
 	// require we can load the latest version
 	err = app.LoadVersion(1)
@@ -438,7 +438,7 @@ func TestLoadVersionPruning(t *testing.T) {
 	pruningOpt := baseapp.SetPruning(pruningOptions)
 	db := dbm.NewMemDB()
 	name := t.Name()
-	app := baseapp.NewBaseApp(name, logger, db, nil, pruningOpt)
+	app := baseapp.NewBaseApp(name, logger, db, pruningOpt)
 
 	// make a cap key and mount the store
 	capKey := sdk.NewKVStoreKey("key1")
@@ -476,7 +476,7 @@ func TestLoadVersionPruning(t *testing.T) {
 	}
 
 	// reload with LoadLatestVersion, check it loads last version
-	app = baseapp.NewBaseApp(name, logger, db, nil, pruningOpt)
+	app = baseapp.NewBaseApp(name, logger, db, pruningOpt)
 	app.MountStores(capKey)
 
 	err = app.LoadLatestVersion()
@@ -494,7 +494,7 @@ func testLoadVersionHelper(t *testing.T, app *baseapp.BaseApp, expectedHeight in
 func TestOptionFunction(t *testing.T) {
 	logger := defaultLogger()
 	db := dbm.NewMemDB()
-	bap := baseapp.NewBaseApp("starting name", logger, db, nil, testChangeNameHelper("new name"))
+	bap := baseapp.NewBaseApp("starting name", logger, db, testChangeNameHelper("new name"))
 	require.Equal(t, bap.GetName(), "new name", "BaseApp should have had name changed via option function")
 }
 
@@ -504,23 +504,6 @@ func testChangeNameHelper(name string) func(*baseapp.BaseApp) {
 	}
 }
 
-// Test that txs can be unmarshalled and read and that
-// correct error codes are returned when not
-func TestTxDecoder(t *testing.T) {
-	codec := codec.NewLegacyAmino()
-	registerTestCodec(codec)
-
-	app := newBaseApp(t.Name())
-	tx := newTxCounter(1, 0)
-	txBytes := codec.MustMarshal(tx)
-
-	dTx, err := app.TxDecoder(txBytes)
-	require.NoError(t, err)
-
-	cTx := dTx.(txTest)
-	require.Equal(t, tx.Counter, cTx.Counter)
-}
-
 // Test that Info returns the latest committed state.
 func TestInfo(t *testing.T) {
 	app := newBaseApp(t.Name())
@@ -589,7 +572,7 @@ func TestInitChainer(t *testing.T) {
 	// we can reload the same  app later
 	db := dbm.NewMemDB()
 	logger := defaultLogger()
-	app := baseapp.NewBaseApp(name, logger, db, nil)
+	app := baseapp.NewBaseApp(name, logger, db)
 	capKey := sdk.NewKVStoreKey("main")
 	capKey2 := sdk.NewKVStoreKey("key2")
 	app.MountStores(capKey, capKey2)
@@ -644,7 +627,7 @@ func TestInitChainer(t *testing.T) {
 	require.Equal(t, value, res.Value)
 
 	// reload app
-	app = baseapp.NewBaseApp(name, logger, db, nil)
+	app = baseapp.NewBaseApp(name, logger, db)
 	app.SetInitChainer(initChainer)
 	app.MountStores(capKey, capKey2)
 	err = app.LoadLatestVersion() // needed to make stores non-nil
@@ -668,7 +651,7 @@ func TestInitChain_WithInitialHeight(t *testing.T) {
 	name := t.Name()
 	db := dbm.NewMemDB()
 	logger := defaultLogger()
-	app := baseapp.NewBaseApp(name, logger, db, nil)
+	app := baseapp.NewBaseApp(name, logger, db)
 
 	app.InitChain(
 		abci.RequestInitChain{
@@ -684,7 +667,7 @@ func TestBeginBlock_WithInitialHeight(t *testing.T) {
 	name := t.Name()
 	db := dbm.NewMemDB()
 	logger := defaultLogger()
-	app := baseapp.NewBaseApp(name, logger, db, nil)
+	app := baseapp.NewBaseApp(name, logger, db)
 
 	app.InitChain(
 		abci.RequestInitChain{
@@ -2074,7 +2057,7 @@ func TestBaseApp_EndBlock(t *testing.T) {
 		},
 	}
 
-	app := baseapp.NewBaseApp(name, logger, db, nil)
+	app := baseapp.NewBaseApp(name, logger, db)
 	app.SetParamStore(&paramStore{db: dbm.NewMemDB()})
 	app.InitChain(abci.RequestInitChain{
 		ConsensusParams: cp,

diff --git a/baseapp/grpcrouter_test.go b/baseapp/grpcrouter_test.go
@@ -55,7 +55,7 @@ func TestRegisterQueryServiceTwice(t *testing.T) {
 	db := dbm.NewMemDB()
 	encCfg := simapp.MakeTestEncodingConfig()
 	logger, _ := log.NewDefaultLogger("plain", "info", false)
-	app := baseapp.NewBaseApp("test", logger, db, encCfg.TxConfig.TxDecoder())
+	app := baseapp.NewBaseApp("test", logger, db)
 	app.SetInterfaceRegistry(encCfg.InterfaceRegistry)
 	testdata.RegisterInterfaces(encCfg.InterfaceRegistry)
 

diff --git a/baseapp/test_helpers.go b/baseapp/test_helpers.go
@@ -36,13 +36,8 @@ func (app *BaseApp) SimCheck(txEncoder sdk.TxEncoder, sdkTx sdk.Tx) (sdk.GasInfo
 
 // Simulate executes a tx in simulate mode to get result and gas info.
 func (app *BaseApp) Simulate(txBytes []byte) (sdk.GasInfo, *sdk.Result, error) {
-	sdkTx, err := app.txDecoder(txBytes)
-	if err != nil {
-		return sdk.GasInfo{}, nil, err
-	}
-
 	ctx := app.getContextForTx(runTxModeSimulate, txBytes)
-	res, err := app.txHandler.SimulateTx(ctx, tx.Request{Tx: sdkTx, TxBytes: txBytes})
+	res, err := app.txHandler.SimulateTx(ctx, tx.Request{TxBytes: txBytes})
 	gasInfo := sdk.GasInfo{
 		GasWanted: res.GasWanted,
 		GasUsed:   res.GasUsed,

diff --git a/baseapp/util_test.go b/baseapp/util_test.go
@@ -45,13 +45,6 @@ func (app *BaseApp) GetName() string {
 	return app.name
 }
 
-// GetName return name.
-//
-// This method is only accessible in baseapp tests.
-func (app *BaseApp) TxDecoder(txBytes []byte) (sdk.Tx, error) {
-	return app.txDecoder(txBytes)
-}
-
 // CreateQueryContext calls app's createQueryContext.
 //
 // This method is only accessible in baseapp tests.

diff --git a/server/mock/app.go b/server/mock/app.go
@@ -42,7 +42,7 @@ func NewApp(rootDir string, logger log.Logger) (abci.Application, error) {
 	capKeyMainStore := sdk.NewKVStoreKey("main")
 
 	// Create BaseApp.
-	baseApp := bam.NewBaseApp("kvstore", logger, db, decodeTx)
+	baseApp := bam.NewBaseApp("kvstore", logger, db)
 
 	// Set mounts for BaseApp's MultiStore.
 	baseApp.MountStores(capKeyMainStore)

diff --git a/server/mock/tx.go b/server/mock/tx.go
@@ -2,12 +2,10 @@
 package mock
 
 import (
-	"bytes"
 	"fmt"
 	"math"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/x/auth/middleware"
 )
 
@@ -70,22 +68,3 @@ func (tx kvstoreTx) GetSigners() []sdk.AccAddress {
 func (tx kvstoreTx) GetGas() uint64 {
 	return math.MaxUint64
 }
-
-// takes raw transaction bytes and decodes them into an sdk.Tx. An sdk.Tx has
-// all the signatures and can be used to authenticate.
-func decodeTx(txBytes []byte) (sdk.Tx, error) {
-	var tx sdk.Tx
-
-	split := bytes.Split(txBytes, []byte("="))
-	if len(split) == 1 {
-		k := split[0]
-		tx = &kvstoreTx{k, k, txBytes}
-	} else if len(split) == 2 {
-		k, v := split[0], split[1]
-		tx = &kvstoreTx{k, v, txBytes}
-	} else {
-		return nil, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "too many '='")
-	}
-
-	return tx, nil
-}