From 85b5fa9b71a81aa2c2485d3b4394d3701681d8e8 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Wed, 6 Jan 2021 16:20:12 +0100 Subject: [PATCH] appcreator: Reuse encoding config instead of recreating it (#8250) * appcreator: Reuse encoding config instead of recreating it * Update tests * merge tests * fix export_test.go * fix genesis_test.go * fix withGenesis flag * fix setup parameters * Update server/README.md Co-authored-by: Alessio Treglia * Update server/README.md Co-authored-by: Alessio Treglia Co-authored-by: Alessio Treglia --- server/README.md | 32 +++++++++++++------------------- server/export_test.go | 7 ++++--- simapp/app_test.go | 24 ++++++++++-------------- simapp/genesis.go | 7 ++++--- simapp/simd/cmd/root.go | 23 ++++++++++++----------- simapp/state.go | 2 +- simapp/test_helpers.go | 26 +++++++++++++------------- x/gov/genesis_test.go | 2 +- x/upgrade/abci_test.go | 2 +- 9 files changed, 59 insertions(+), 66 deletions(-) diff --git a/server/README.md b/server/README.md index 3ee67d5c0ccd..79e46a3ae799 100644 --- a/server/README.md +++ b/server/README.md @@ -1,20 +1,21 @@ # Server The `server` package is responsible for providing the mechanisms necessary to -start an ABCI Tendermint application and providing the CLI framework necessary -to fully bootstrap an application. The package exposes two core commands, `StartCmd` -and `ExportCmd`. +start an ABCI Tendermint application and provides the CLI framework (based on [cobra](github.com/spf13/cobra)) +necessary to fully bootstrap an application. The package exposes two core functions: `StartCmd` +and `ExportCmd` which creates commands to start the application and export state respectively. ## Preliminary -The root command of an application typically is constructed with three core -sub-commands, query commands, tx commands, and auxiliary commands such as genesis -utilities, and starting an application binary. +The root command of an application typically is constructed with: ++ command to start an application binary ++ three meta commands: `query`, `tx`, and a few auxiliary commands such as `genesis`. +utilities. -It is vital that the root command of an application set the appropriate `PersistentPreRun(E)` -function so all child commands have access to the server and client contexts. +It is vital that the root command of an application uses `PersistentPreRun()` cobra command +property for executing the command, so all child commands have access to the server and client contexts. These contexts are set as their default values initially and maybe modified, -scoped to the command, in their respective `PersistentPreRun(E)` functions. Note, +scoped to the command, in their respective `PersistentPreRun()` functions. Note that the `client.Context` is typically pre-populated with "default" values that may be useful for all commands to inherit and override if necessary. @@ -22,6 +23,8 @@ Example: ```go var ( + initClientCtx = client.Context{...} + rootCmd = &cobra.Command{ Use: "simd", Short: "simulation app", @@ -33,16 +36,7 @@ var ( return server.InterceptConfigsPreRunHandler(cmd) }, } - - encodingConfig = simapp.MakeTestEncodingConfig() - initClientCtx = client.Context{}. - WithJSONMarshaler(encodingConfig.Marshaler). - WithTxConfig(encodingConfig.TxConfig). - WithCodec(encodingConfig.Amino). - WithInput(os.Stdin). - WithAccountRetriever(types.NewAccountRetriever(encodingConfig.Marshaler)). - WithBroadcastMode(flags.BroadcastBlock). - WithHomeDir(simapp.DefaultNodeHome) + // add root sub-commands ... ) ``` diff --git a/server/export_test.go b/server/export_test.go index 0971665fcfce..d4e41ef8e886 100644 --- a/server/export_test.go +++ b/server/export_test.go @@ -22,6 +22,7 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" + "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/server" "github.com/cosmos/cosmos-sdk/server/types" "github.com/cosmos/cosmos-sdk/simapp" @@ -135,7 +136,7 @@ func setupApp(t *testing.T, tempDir string) (*simapp.SimApp, context.Context, *t serverCtx.Config.RootDir = tempDir clientCtx := client.Context{}.WithJSONMarshaler(app.AppCodec()) - genDoc := newDefaultGenesisDoc() + genDoc := newDefaultGenesisDoc(encCfg.Marshaler) require.NoError(t, saveGenesisFile(genDoc, serverCtx.Config.GenesisFile())) app.InitChain( @@ -176,8 +177,8 @@ func createConfigFolder(dir string) error { return os.Mkdir(path.Join(dir, "config"), 0700) } -func newDefaultGenesisDoc() *tmtypes.GenesisDoc { - genesisState := simapp.NewDefaultGenesisState() +func newDefaultGenesisDoc(cdc codec.Marshaler) *tmtypes.GenesisDoc { + genesisState := simapp.NewDefaultGenesisState(cdc) stateBytes, err := json.MarshalIndent(genesisState, "", " ") if err != nil { diff --git a/simapp/app_test.go b/simapp/app_test.go index fef50310bff9..6543f94fd437 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -12,11 +12,17 @@ import ( abci "github.com/tendermint/tendermint/abci/types" ) -func TestSimAppExport(t *testing.T) { +func TestSimAppExportAndBlockedAddrs(t *testing.T) { + encCfg := MakeTestEncodingConfig() db := dbm.NewMemDB() - app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, MakeTestEncodingConfig(), EmptyAppOptions{}) + app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{}) - genesisState := NewDefaultGenesisState() + for acc := range maccPerms { + require.Equal(t, !allowedReceivingModAcc[acc], app.BankKeeper.BlockedAddr(app.AccountKeeper.GetModuleAddress(acc)), + "ensure that blocked addresses are properly set in bank keeper") + } + + genesisState := NewDefaultGenesisState(encCfg.Marshaler) stateBytes, err := json.MarshalIndent(genesisState, "", " ") require.NoError(t, err) @@ -30,21 +36,11 @@ func TestSimAppExport(t *testing.T) { app.Commit() // Making a new app object with the db, so that initchain hasn't been called - app2 := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, MakeTestEncodingConfig(), EmptyAppOptions{}) + app2 := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{}) _, err = app2.ExportAppStateAndValidators(false, []string{}) require.NoError(t, err, "ExportAppStateAndValidators should not have an error") } -// ensure that blocked addresses are properly set in bank keeper -func TestBlockedAddrs(t *testing.T) { - db := dbm.NewMemDB() - app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, MakeTestEncodingConfig(), EmptyAppOptions{}) - - for acc := range maccPerms { - require.Equal(t, !allowedReceivingModAcc[acc], app.BankKeeper.BlockedAddr(app.AccountKeeper.GetModuleAddress(acc))) - } -} - func TestGetMaccPerms(t *testing.T) { dup := GetMaccPerms() require.Equal(t, maccPerms, dup, "duplicated module account permissions differed from actual module account permissions") diff --git a/simapp/genesis.go b/simapp/genesis.go index 8fd7e83fd80f..dbb4e01c7690 100644 --- a/simapp/genesis.go +++ b/simapp/genesis.go @@ -2,6 +2,8 @@ package simapp import ( "encoding/json" + + "github.com/cosmos/cosmos-sdk/codec" ) // The genesis state of the blockchain is represented here as a map of raw json @@ -14,7 +16,6 @@ import ( type GenesisState map[string]json.RawMessage // NewDefaultGenesisState generates the default state for the application. -func NewDefaultGenesisState() GenesisState { - encCfg := MakeTestEncodingConfig() - return ModuleBasics.DefaultGenesis(encCfg.Marshaler) +func NewDefaultGenesisState(cdc codec.JSONMarshaler) GenesisState { + return ModuleBasics.DefaultGenesis(cdc) } diff --git a/simapp/simd/cmd/root.go b/simapp/simd/cmd/root.go index 534e22b0fb0d..e7713a284543 100644 --- a/simapp/simd/cmd/root.go +++ b/simapp/simd/cmd/root.go @@ -18,7 +18,6 @@ import ( "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/client/keys" "github.com/cosmos/cosmos-sdk/client/rpc" - "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/server" servertypes "github.com/cosmos/cosmos-sdk/server/types" "github.com/cosmos/cosmos-sdk/simapp" @@ -81,7 +80,8 @@ func initRootCmd(rootCmd *cobra.Command, encodingConfig params.EncodingConfig) { debug.Cmd(), ) - server.AddCommands(rootCmd, simapp.DefaultNodeHome, newApp, createSimappAndExport, addModuleInitFlags) + a := appCreator{encodingConfig} + server.AddCommands(rootCmd, simapp.DefaultNodeHome, a.newApp, a.appExport, addModuleInitFlags) // add keybase, auxiliary RPC, query, and tx child commands rootCmd.AddCommand( @@ -148,8 +148,12 @@ func txCommand() *cobra.Command { return cmd } +type appCreator struct { + encCfg params.EncodingConfig +} + // newApp is an AppCreator -func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, appOpts servertypes.AppOptions) servertypes.Application { +func (a appCreator) newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, appOpts servertypes.AppOptions) servertypes.Application { var cache sdk.MultiStorePersistentCache if cast.ToBool(appOpts.Get(server.FlagInterBlockCache)) { @@ -180,7 +184,7 @@ func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, appOpts serverty logger, db, traceStore, true, skipUpgradeHeights, cast.ToString(appOpts.Get(flags.FlagHome)), cast.ToUint(appOpts.Get(server.FlagInvCheckPeriod)), - simapp.MakeTestEncodingConfig(), // Ideally, we would reuse the one created by NewRootCmd. + a.encCfg, appOpts, baseapp.SetPruning(pruningOpts), baseapp.SetMinGasPrices(cast.ToString(appOpts.Get(server.FlagMinGasPrices))), @@ -196,29 +200,26 @@ func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, appOpts serverty ) } -// createSimappAndExport creates a new simapp (optionally at a given height) +// appExport creates a new simapp (optionally at a given height) // and exports state. -func createSimappAndExport( +func (a appCreator) appExport( logger log.Logger, db dbm.DB, traceStore io.Writer, height int64, forZeroHeight bool, jailAllowedAddrs []string, appOpts servertypes.AppOptions) (servertypes.ExportedApp, error) { - encCfg := simapp.MakeTestEncodingConfig() // Ideally, we would reuse the one created by NewRootCmd. - encCfg.Marshaler = codec.NewProtoCodec(encCfg.InterfaceRegistry) var simApp *simapp.SimApp - homePath, ok := appOpts.Get(flags.FlagHome).(string) if !ok || homePath == "" { return servertypes.ExportedApp{}, errors.New("application home not set") } if height != -1 { - simApp = simapp.NewSimApp(logger, db, traceStore, false, map[int64]bool{}, homePath, uint(1), encCfg, appOpts) + simApp = simapp.NewSimApp(logger, db, traceStore, false, map[int64]bool{}, homePath, uint(1), a.encCfg, appOpts) if err := simApp.LoadHeight(height); err != nil { return servertypes.ExportedApp{}, err } } else { - simApp = simapp.NewSimApp(logger, db, traceStore, true, map[int64]bool{}, homePath, uint(1), encCfg, appOpts) + simApp = simapp.NewSimApp(logger, db, traceStore, true, map[int64]bool{}, homePath, uint(1), a.encCfg, appOpts) } return simApp.ExportAppStateAndValidators(forZeroHeight, jailAllowedAddrs) diff --git a/simapp/state.go b/simapp/state.go index 3eb9afd375a6..4c3773813a04 100644 --- a/simapp/state.go +++ b/simapp/state.go @@ -79,7 +79,7 @@ func AppStateRandomizedFn( accs []simtypes.Account, genesisTimestamp time.Time, appParams simtypes.AppParams, ) (json.RawMessage, []simtypes.Account) { numAccs := int64(len(accs)) - genesisState := NewDefaultGenesisState() + genesisState := NewDefaultGenesisState(cdc) // generate a random amount of initial stake coins and a random initial // number of bonded accounts diff --git a/simapp/test_helpers.go b/simapp/test_helpers.go index 3d1f73e583bb..83f2ea45351f 100644 --- a/simapp/test_helpers.go +++ b/simapp/test_helpers.go @@ -49,13 +49,21 @@ var DefaultConsensusParams = &abci.ConsensusParams{ }, } +func setup(withGenesis bool, invCheckPeriod uint) (*SimApp, GenesisState) { + db := dbm.NewMemDB() + encCdc := MakeTestEncodingConfig() + app := NewSimApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, DefaultNodeHome, invCheckPeriod, encCdc, EmptyAppOptions{}) + if withGenesis { + return app, NewDefaultGenesisState(encCdc.Marshaler) + } + return app, GenesisState{} +} + // Setup initializes a new SimApp. A Nop logger is set in SimApp. func Setup(isCheckTx bool) *SimApp { - db := dbm.NewMemDB() - app := NewSimApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, DefaultNodeHome, 5, MakeTestEncodingConfig(), EmptyAppOptions{}) + app, genesisState := setup(!isCheckTx, 5) if !isCheckTx { // init chain must be called to stop deliverState from being nil - genesisState := NewDefaultGenesisState() stateBytes, err := json.MarshalIndent(genesisState, "", " ") if err != nil { panic(err) @@ -79,10 +87,7 @@ func Setup(isCheckTx bool) *SimApp { // of one consensus engine unit (10^6) in the default token of the simapp from first genesis // account. A Nop logger is set in SimApp. func SetupWithGenesisValSet(t *testing.T, valSet *tmtypes.ValidatorSet, genAccs []authtypes.GenesisAccount, balances ...banktypes.Balance) *SimApp { - db := dbm.NewMemDB() - app := NewSimApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, DefaultNodeHome, 5, MakeTestEncodingConfig(), EmptyAppOptions{}) - genesisState := NewDefaultGenesisState() - + app, genesisState := setup(true, 5) // set genesis accounts authGenesis := authtypes.NewGenesisState(authtypes.DefaultParams(), genAccs) genesisState[authtypes.ModuleName] = app.AppCodec().MustMarshalJSON(authGenesis) @@ -156,12 +161,7 @@ func SetupWithGenesisValSet(t *testing.T, valSet *tmtypes.ValidatorSet, genAccs // SetupWithGenesisAccounts initializes a new SimApp with the provided genesis // accounts and possible balances. func SetupWithGenesisAccounts(genAccs []authtypes.GenesisAccount, balances ...banktypes.Balance) *SimApp { - db := dbm.NewMemDB() - app := NewSimApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, MakeTestEncodingConfig(), EmptyAppOptions{}) - - // initialize the chain with the passed in genesis accounts - genesisState := NewDefaultGenesisState() - + app, genesisState := setup(true, 0) authGenesis := authtypes.NewGenesisState(authtypes.DefaultParams(), genAccs) genesisState[authtypes.ModuleName] = app.AppCodec().MustMarshalJSON(authGenesis) diff --git a/x/gov/genesis_test.go b/x/gov/genesis_test.go index 5ed4a11205e9..2fb3aea9467b 100644 --- a/x/gov/genesis_test.go +++ b/x/gov/genesis_test.go @@ -56,7 +56,7 @@ func TestImportExportQueues(t *testing.T) { // export the state and import it into a new app govGenState := gov.ExportGenesis(ctx, app.GovKeeper) - genesisState := simapp.NewDefaultGenesisState() + genesisState := simapp.NewDefaultGenesisState(app.AppCodec()) genesisState[authtypes.ModuleName] = app.AppCodec().MustMarshalJSON(authGenState) genesisState[banktypes.ModuleName] = app.AppCodec().MustMarshalJSON(bankGenState) diff --git a/x/upgrade/abci_test.go b/x/upgrade/abci_test.go index caa5e5a3c9c3..eb31857961bc 100644 --- a/x/upgrade/abci_test.go +++ b/x/upgrade/abci_test.go @@ -41,7 +41,7 @@ var s TestSuite func setupTest(height int64, skip map[int64]bool) TestSuite { db := dbm.NewMemDB() app := simapp.NewSimApp(log.NewNopLogger(), db, nil, true, skip, simapp.DefaultNodeHome, 0, simapp.MakeTestEncodingConfig(), simapp.EmptyAppOptions{}) - genesisState := simapp.NewDefaultGenesisState() + genesisState := simapp.NewDefaultGenesisState(app.AppCodec()) stateBytes, err := json.MarshalIndent(genesisState, "", " ") if err != nil { panic(err)