From fa31165438bb3a333e77b3a2745a3a89566120c8 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 12 Jul 2018 18:32:48 -0700 Subject: [PATCH] Refactor BaseApp without breaking change --- baseapp/baseapp.go | 71 ++++++++++++++++++++++-------------- baseapp/baseapp_test.go | 48 +++++++++++++++--------- cmd/gaia/app/app.go | 2 +- docs/core/examples/app1.go | 10 +---- docs/core/examples/app2.go | 15 +++++++- docs/core/examples/app3.go | 2 +- docs/core/examples/app4.go | 2 +- examples/basecoin/app/app.go | 3 +- examples/democoin/app/app.go | 2 +- x/auth/stdtx.go | 20 ++++++++++ x/mock/app.go | 2 +- 11 files changed, 117 insertions(+), 60 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index b17e797774..3c96761e4b 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -4,6 +4,7 @@ import ( "fmt" "runtime/debug" "strings" + "encoding/json" "github.com/pkg/errors" @@ -15,8 +16,10 @@ import ( "github.com/cosmos/cosmos-sdk/store" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/version" - "github.com/cosmos/cosmos-sdk/wire" + + // TODO: Remove dependency on auth and wire "github.com/cosmos/cosmos-sdk/x/auth" + "github.com/cosmos/cosmos-sdk/wire" ) // Key to store the header in the DB itself. @@ -42,7 +45,7 @@ type BaseApp struct { // initialized on creation Logger log.Logger name string // application name from abci.Info - cdc *wire.Codec // Amino codec + cdc *wire.Codec // Amino codec (DEPRECATED) db dbm.DB // common DB backend cms sdk.CommitMultiStore // Main (uncached) state router Router // handle any kind of message @@ -74,6 +77,7 @@ var _ abci.Application = (*BaseApp)(nil) // Create and name new BaseApp // NOTE: The db is used to store the version number for now. // Accepts variable number of option functions, which act on the BaseApp to set configuration choices +// DEPRECATED func NewBaseApp(name string, cdc *wire.Codec, logger log.Logger, db dbm.DB, options ...func(*BaseApp)) *BaseApp { app := &BaseApp{ Logger: logger, @@ -83,7 +87,28 @@ func NewBaseApp(name string, cdc *wire.Codec, logger log.Logger, db dbm.DB, opti cms: store.NewCommitMultiStore(db), router: NewRouter(), codespacer: sdk.NewCodespacer(), - txDecoder: defaultTxDecoder(cdc), + txDecoder: auth.DefaultTxDecoder(cdc), + } + // Register the undefined & root codespaces, which should not be used by any modules + app.codespacer.RegisterOrPanic(sdk.CodespaceRoot) + for _, option := range options { + option(app) + } + return app +} + +// Create and name new BaseApp +// Does not set cdc and instead takes a user-defined txDecoder. If nil, BaseApp uses defaultTxDecoder +// TODO: Rename to NewBaseApp and remove above constructor once auth, wire dependencies removed +func NewBaseAppNoCodec(name string, logger log.Logger, db dbm.DB, txDecoder sdk.TxDecoder, options ...func(*BaseApp)) *BaseApp { + app := &BaseApp{ + Logger: logger, + name: name, + db: db, + cms: store.NewCommitMultiStore(db), + router: NewRouter(), + codespacer: sdk.NewCodespacer(), + txDecoder: txDecoder, } // Register the undefined & root codespaces, which should not be used by any modules app.codespacer.RegisterOrPanic(sdk.CodespaceRoot) @@ -121,34 +146,11 @@ func (app *BaseApp) MountStore(key sdk.StoreKey, typ sdk.StoreType) { } // Set the txDecoder function +// DEPRECATED func (app *BaseApp) SetTxDecoder(txDecoder sdk.TxDecoder) { app.txDecoder = txDecoder } -// default custom logic for transaction decoding -// TODO: remove auth and wire dependencies from baseapp -// - move this to auth.DefaultTxDecoder -// - set the default here to JSON decode like docs/examples/app1 (it will fail -// for multiple messages ;)) -// - pass a TxDecoder into NewBaseApp, instead of a codec. -func defaultTxDecoder(cdc *wire.Codec) sdk.TxDecoder { - return func(txBytes []byte) (sdk.Tx, sdk.Error) { - var tx = auth.StdTx{} - - if len(txBytes) == 0 { - return nil, sdk.ErrTxDecode("txBytes are empty") - } - - // StdTx.Msg is an interface. The concrete types - // are registered by MakeTxCodec - err := cdc.UnmarshalBinary(txBytes, &tx) - if err != nil { - return nil, sdk.ErrTxDecode("").TraceSDK(err.Error()) - } - return tx, nil - } -} - // nolint - Set functions func (app *BaseApp) SetInitChainer(initChainer sdk.InitChainer) { app.initChainer = initChainer @@ -349,7 +351,20 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abc default: result = sdk.ErrUnknownRequest(fmt.Sprintf("Unknown query: %s", path)).Result() } - value := app.cdc.MustMarshalBinary(result) + + // Encode with amino if defined, else use json + // TODO: Use JSON encoding only once app.cdc removed + var value []byte + if app.cdc != nil { + value = app.cdc.MustMarshalBinary(result) + } else { + var err error + value, err = json.Marshal(result) + if err != nil { + return sdk.ErrInternal("Encoding result failed").QueryResult() + } + } + return abci.ResponseQuery{ Code: uint32(sdk.ABCICodeOK), Value: value, diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 00897392ec..0da76e717a 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -3,6 +3,7 @@ package baseapp import ( "bytes" "encoding/binary" + "encoding/json" "fmt" "os" "testing" @@ -30,7 +31,7 @@ func newBaseApp(name string) *BaseApp { db := dbm.NewMemDB() codec := wire.NewCodec() registerTestCodec(codec) - return NewBaseApp(name, codec, logger, db) + return NewBaseAppNoCodec(name, logger, db, testTxDecoder(codec)) } func registerTestCodec(cdc *wire.Codec) { @@ -49,8 +50,6 @@ func setupBaseApp(t *testing.T) (*BaseApp, *sdk.KVStoreKey, *sdk.KVStoreKey) { app := newBaseApp(t.Name()) require.Equal(t, t.Name(), app.Name()) - app.SetTxDecoder(testTxDecoder(app.cdc)) - // make some cap keys capKey1 := sdk.NewKVStoreKey("key1") capKey2 := sdk.NewKVStoreKey("key2") @@ -85,7 +84,7 @@ func TestLoadVersion(t *testing.T) { logger := defaultLogger() db := dbm.NewMemDB() name := t.Name() - app := NewBaseApp(name, nil, logger, db) + app := NewBaseAppNoCodec(name, logger, db, nil) // make a cap key and mount the store capKey := sdk.NewKVStoreKey("main") @@ -114,7 +113,7 @@ func TestLoadVersion(t *testing.T) { commitID2 := sdk.CommitID{2, res.Data} // reload with LoadLatestVersion - app = NewBaseApp(name, nil, logger, db) + app = NewBaseAppNoCodec(name, logger, db, nil) app.MountStoresIAVL(capKey) err = app.LoadLatestVersion(capKey) require.Nil(t, err) @@ -122,7 +121,7 @@ func TestLoadVersion(t *testing.T) { // reload with LoadVersion, see if you can commit the same block and get // the same result - app = NewBaseApp(name, nil, logger, db) + app = NewBaseAppNoCodec(name, logger, db, nil) app.MountStoresIAVL(capKey) err = app.LoadVersion(1, capKey) require.Nil(t, err) @@ -142,9 +141,7 @@ func testLoadVersionHelper(t *testing.T, app *BaseApp, expectedHeight int64, exp func TestOptionFunction(t *testing.T) { logger := defaultLogger() db := dbm.NewMemDB() - codec := wire.NewCodec() - registerTestCodec(codec) - bap := NewBaseApp("starting name", codec, logger, db, testChangeNameHelper("new name")) + bap := NewBaseAppNoCodec("starting name", logger, db, nil, testChangeNameHelper("new name")) require.Equal(t, bap.name, "new name", "BaseApp should have had name changed via option function") } @@ -216,7 +213,7 @@ func TestInitChainer(t *testing.T) { // we can reload the same app later db := dbm.NewMemDB() logger := defaultLogger() - app := NewBaseApp(name, nil, logger, db) + app := NewBaseAppNoCodec(name, logger, db, nil) capKey := sdk.NewKVStoreKey("main") capKey2 := sdk.NewKVStoreKey("key2") app.MountStoresIAVL(capKey, capKey2) @@ -257,7 +254,7 @@ func TestInitChainer(t *testing.T) { require.Equal(t, value, res.Value) // reload app - app = NewBaseApp(name, nil, logger, db) + app = NewBaseAppNoCodec(name, logger, db, nil) app.MountStoresIAVL(capKey, capKey2) err = app.LoadLatestVersion(capKey) // needed to make stores non-nil require.Nil(t, err) @@ -444,9 +441,13 @@ func TestCheckTx(t *testing.T) { app.InitChain(abci.RequestInitChain{}) + // Create same codec used in txDecoder + codec := wire.NewCodec() + registerTestCodec(codec) + for i := int64(0); i < nTxs; i++ { tx := newTxCounter(i, 0) - txBytes, err := app.cdc.MarshalBinary(tx) + txBytes, err := codec.MarshalBinary(tx) require.NoError(t, err) r := app.CheckTx(txBytes) assert.True(t, r.IsOK(), fmt.Sprintf("%v", r)) @@ -481,6 +482,10 @@ func TestDeliverTx(t *testing.T) { deliverKey := []byte("deliver-key") app.Router().AddRoute(typeMsgCounter, handlerMsgCounter(t, capKey, deliverKey)) + // Create same codec used in txDecoder + codec := wire.NewCodec() + registerTestCodec(codec) + nBlocks := 3 txPerHeight := 5 for blockN := 0; blockN < nBlocks; blockN++ { @@ -488,7 +493,7 @@ func TestDeliverTx(t *testing.T) { for i := 0; i < txPerHeight; i++ { counter := int64(blockN*txPerHeight + i) tx := newTxCounter(counter, counter) - txBytes, err := app.cdc.MarshalBinary(tx) + txBytes, err := codec.MarshalBinary(tx) require.NoError(t, err) res := app.DeliverTx(txBytes) require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) @@ -518,12 +523,16 @@ func TestMultiMsgDeliverTx(t *testing.T) { app.Router().AddRoute(typeMsgCounter, handlerMsgCounter(t, capKey, deliverKey)) app.Router().AddRoute(typeMsgCounter2, handlerMsgCounter(t, capKey, deliverKey2)) + // Create same codec used in txDecoder + codec := wire.NewCodec() + registerTestCodec(codec) + // run a multi-msg tx // with all msgs the same type { app.BeginBlock(abci.RequestBeginBlock{}) tx := newTxCounter(0, 0, 1, 2) - txBytes, err := app.cdc.MarshalBinary(tx) + txBytes, err := codec.MarshalBinary(tx) require.NoError(t, err) res := app.DeliverTx(txBytes) require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) @@ -544,7 +553,7 @@ func TestMultiMsgDeliverTx(t *testing.T) { tx := newTxCounter(1, 3) tx.Msgs = append(tx.Msgs, msgCounter2{0}) tx.Msgs = append(tx.Msgs, msgCounter2{1}) - txBytes, err := app.cdc.MarshalBinary(tx) + txBytes, err := codec.MarshalBinary(tx) require.NoError(t, err) res := app.DeliverTx(txBytes) require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) @@ -589,6 +598,10 @@ func TestSimulateTx(t *testing.T) { }) app.InitChain(abci.RequestInitChain{}) + // Create same codec used in txDecoder + codec := wire.NewCodec() + registerTestCodec(codec) + nBlocks := 3 for blockN := 0; blockN < nBlocks; blockN++ { count := int64(blockN + 1) @@ -607,7 +620,7 @@ func TestSimulateTx(t *testing.T) { require.Equal(t, int64(gasConsumed), result.GasUsed) // simulate by calling Query with encoded tx - txBytes, err := app.cdc.MarshalBinary(tx) + txBytes, err := codec.MarshalBinary(tx) require.Nil(t, err) query := abci.RequestQuery{ Path: "/app/simulate", @@ -617,7 +630,8 @@ func TestSimulateTx(t *testing.T) { require.True(t, queryResult.IsOK(), queryResult.Log) var res sdk.Result - app.cdc.MustUnmarshalBinary(queryResult.Value, &res) + err = json.Unmarshal(queryResult.Value, &res) + require.Nil(t, err, "Result unmarshalling failed") require.True(t, res.IsOK(), res.Log) require.Equal(t, gasConsumed, res.GasUsed, res.Log) app.EndBlock(abci.RequestEndBlock{}) diff --git a/cmd/gaia/app/app.go b/cmd/gaia/app/app.go index 40e8686694..dd85fd7637 100644 --- a/cmd/gaia/app/app.go +++ b/cmd/gaia/app/app.go @@ -60,7 +60,7 @@ func NewGaiaApp(logger log.Logger, db dbm.DB) *GaiaApp { // create your application object var app = &GaiaApp{ - BaseApp: bam.NewBaseApp(appName, cdc, logger, db), + BaseApp: bam.NewBaseAppNoCodec(appName, logger, db, auth.DefaultTxDecoder(cdc)), cdc: cdc, keyMain: sdk.NewKVStoreKey("main"), keyAccount: sdk.NewKVStoreKey("acc"), diff --git a/docs/core/examples/app1.go b/docs/core/examples/app1.go index b208f75cf7..7be0d43e24 100644 --- a/docs/core/examples/app1.go +++ b/docs/core/examples/app1.go @@ -9,7 +9,6 @@ import ( bapp "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/wire" ) const ( @@ -18,17 +17,12 @@ const ( func NewApp1(logger log.Logger, db dbm.DB) *bapp.BaseApp { - cdc := wire.NewCodec() - // Create the base application object. - app := bapp.NewBaseApp(app1Name, cdc, logger, db) + app := bapp.NewBaseAppNoCodec(app1Name, logger, db, tx1Decoder) // Create a key for accessing the account store. keyAccount := sdk.NewKVStoreKey("acc") - // Determine how transactions are decoded. - app.SetTxDecoder(txDecoder) - // Register message routes. // Note the handler gets access to the account store. app.Router(). @@ -225,7 +219,7 @@ func (tx app1Tx) GetMsgs() []sdk.Msg { } // JSON decode MsgSend. -func txDecoder(txBytes []byte) (sdk.Tx, sdk.Error) { +func tx1Decoder(txBytes []byte) (sdk.Tx, sdk.Error) { var tx app1Tx err := json.Unmarshal(txBytes, &tx) if err != nil { diff --git a/docs/core/examples/app2.go b/docs/core/examples/app2.go index 4c20c17c3a..d5d6c3d518 100644 --- a/docs/core/examples/app2.go +++ b/docs/core/examples/app2.go @@ -37,7 +37,7 @@ func NewApp2(logger log.Logger, db dbm.DB) *bapp.BaseApp { cdc := NewCodec() // Create the base application object. - app := bapp.NewBaseApp(app2Name, cdc, logger, db) + app := bapp.NewBaseAppNoCodec(app2Name, logger, db, tx2Decoder(cdc)) // Create a key for accessing the account store. keyAccount := sdk.NewKVStoreKey("acc") @@ -191,6 +191,19 @@ func (tx app2Tx) GetSignatures() []auth.StdSignature { return tx.Signatures } +// Amino decode app2Tx. Capable of decoding both MsgSend and MsgIssue +func tx2Decoder(cdc *wire.Codec) sdk.TxDecoder { + return func(txBytes []byte) (sdk.Tx, sdk.Error) { + var tx app2Tx + err := cdc.UnmarshalBinary(txBytes, &tx) + if err != nil { + return nil, sdk.ErrTxDecode(err.Error()) + } + return tx, nil + } +} + + //------------------------------------------------------------------ // Simple anteHandler that ensures msg signers have signed. diff --git a/docs/core/examples/app3.go b/docs/core/examples/app3.go index 853ad687e1..5f9fe081c3 100644 --- a/docs/core/examples/app3.go +++ b/docs/core/examples/app3.go @@ -21,7 +21,7 @@ func NewApp3(logger log.Logger, db dbm.DB) *bapp.BaseApp { cdc := NewCodec() // Create the base application object. - app := bapp.NewBaseApp(app3Name, cdc, logger, db) + app := bapp.NewBaseAppNoCodec(app3Name, logger, db, auth.DefaultTxDecoder(cdc)) // Create a key for accessing the account store. keyAccount := sdk.NewKVStoreKey("acc") diff --git a/docs/core/examples/app4.go b/docs/core/examples/app4.go index a8ef37cee5..b6f9af2db7 100644 --- a/docs/core/examples/app4.go +++ b/docs/core/examples/app4.go @@ -22,7 +22,7 @@ func NewApp4(logger log.Logger, db dbm.DB) *bapp.BaseApp { cdc := NewCodec() // Create the base application object. - app := bapp.NewBaseApp(app3Name, cdc, logger, db) + app := bapp.NewBaseAppNoCodec(app4Name, logger, db, auth.DefaultTxDecoder(cdc)) // Create a key for accessing the account store. keyAccount := sdk.NewKVStoreKey("acc") diff --git a/examples/basecoin/app/app.go b/examples/basecoin/app/app.go index 7b2c7af3a3..a65e62592f 100644 --- a/examples/basecoin/app/app.go +++ b/examples/basecoin/app/app.go @@ -53,7 +53,7 @@ func NewBasecoinApp(logger log.Logger, db dbm.DB) *BasecoinApp { // create your application type var app = &BasecoinApp{ cdc: cdc, - BaseApp: bam.NewBaseApp(appName, cdc, logger, db), + BaseApp: bam.NewBaseAppNoCodec(appName, logger, db, auth.DefaultTxDecoder(cdc)), keyMain: sdk.NewKVStoreKey("main"), keyAccount: sdk.NewKVStoreKey("acc"), keyIBC: sdk.NewKVStoreKey("ibc"), @@ -78,6 +78,7 @@ func NewBasecoinApp(logger log.Logger, db dbm.DB) *BasecoinApp { app.SetBeginBlocker(app.BeginBlocker) app.SetEndBlocker(app.EndBlocker) app.SetAnteHandler(auth.NewAnteHandler(app.accountMapper, app.feeCollectionKeeper)) + app.SetTxDecoder(auth.DefaultTxDecoder(cdc)) // mount the multistore and load the latest state app.MountStoresIAVL(app.keyMain, app.keyAccount, app.keyIBC) diff --git a/examples/democoin/app/app.go b/examples/democoin/app/app.go index 51d10002a7..7038f8905f 100644 --- a/examples/democoin/app/app.go +++ b/examples/democoin/app/app.go @@ -58,7 +58,7 @@ func NewDemocoinApp(logger log.Logger, db dbm.DB) *DemocoinApp { // Create your application object. var app = &DemocoinApp{ - BaseApp: bam.NewBaseApp(appName, cdc, logger, db), + BaseApp: bam.NewBaseAppNoCodec(appName, logger, db, auth.DefaultTxDecoder(cdc)), cdc: cdc, capKeyMainStore: sdk.NewKVStoreKey("main"), capKeyAccountStore: sdk.NewKVStoreKey("acc"), diff --git a/x/auth/stdtx.go b/x/auth/stdtx.go index 316ff5e95a..3a37dce595 100644 --- a/x/auth/stdtx.go +++ b/x/auth/stdtx.go @@ -5,6 +5,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/tendermint/tendermint/crypto" + "github.com/cosmos/cosmos-sdk/wire" ) var _ sdk.Tx = (*StdTx)(nil) @@ -162,3 +163,22 @@ type StdSignature struct { AccountNumber int64 `json:"account_number"` Sequence int64 `json:"sequence"` } + +// default custom logic for transaction decoding +func DefaultTxDecoder(cdc *wire.Codec) sdk.TxDecoder { + return func(txBytes []byte) (sdk.Tx, sdk.Error) { + var tx = StdTx{} + + if len(txBytes) == 0 { + return nil, sdk.ErrTxDecode("txBytes are empty") + } + + // StdTx.Msg is an interface. The concrete types + // are registered by MakeTxCodec + err := cdc.UnmarshalBinary(txBytes, &tx) + if err != nil { + return nil, sdk.ErrTxDecode("").TraceSDK(err.Error()) + } + return tx, nil + } +} diff --git a/x/mock/app.go b/x/mock/app.go index 27f3b9d46e..9d43e649df 100644 --- a/x/mock/app.go +++ b/x/mock/app.go @@ -47,7 +47,7 @@ func NewApp() *App { // Create your application object app := &App{ - BaseApp: bam.NewBaseApp("mock", cdc, logger, db), + BaseApp: bam.NewBaseAppNoCodec("mock", logger, db, auth.DefaultTxDecoder(cdc)), Cdc: cdc, KeyMain: sdk.NewKVStoreKey("main"), KeyAccount: sdk.NewKVStoreKey("acc"),