Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update x/banking and x/crisis InitChain re slow Gaia startup #7764

Merged
merged 13 commits into from
Nov 2, 2020
Merged

Update x/banking and x/crisis InitChain re slow Gaia startup #7764

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
4c5f1fe
add more logs during the initialization process
robert-zaremba Oct 29, 2020
d1c5d58
initializtion: move profiling to the top of the startProcess function
robert-zaremba Oct 29, 2020
8f4ed06
x/bank InitGenesis: remove k.ValidateBalance
robert-zaremba Oct 29, 2020
d2c2351
debug: add logs and telemetry to x/bank and x/crisis
robert-zaremba Oct 29, 2020
aaf8787
make x/crisis AssertInvariants optional during InitGenesis
robert-zaremba Oct 30, 2020
555e020
Add module init flags mechanism
robert-zaremba Oct 30, 2020
76a615d
update changelog
robert-zaremba Oct 30, 2020
0701a7a
remove debug fmt.Print
robert-zaremba Oct 30, 2020
65f2837
fix testutil/network/
robert-zaremba Oct 30, 2020
77151af
fix log message
robert-zaremba Oct 30, 2020
6e78517
update test NewApp calls
robert-zaremba Oct 30, 2020
153ba79
review changes
robert-zaremba Nov 2, 2020
7233567
Merge branch 'master' into robert/initgensis-profiling-master
aaronc Nov 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,18 @@ Ref: https://keepachangelog.com/en/1.0.0/
* `MsgCreateValidator.Pubkey` type changed from `string` to `codectypes.Any`.
* Deprecating and renaming `MakeEncodingConfig` to `MakeTestEncodingConfig` (both in `simapp` and `simapp/params` packages).
* (tx) [\#7688](https://github.com/cosmos/cosmos-sdk/pull/7688) The gRPC simulate service method has been moved from `cosmos.base.v1beta1.simulate` to `cosmos.tx.v1beta1`, as a method in the Tx service.
* [#7764](https://github.com/cosmos/cosmos-sdk/pull/7764) Added module initialization options:
* `server/types.AppExporter` requires extra argument: `AppOptions`.
* `server.AddCommands` requires extra argument: `addStartFlags types.ModuleInitFlags`
* `x/crisis.NewAppModule` has a new attribute: `skipGenesisInvariants`. [PR](https://github.com/cosmos/cosmos-sdk/pull/7764)

### Features

* (codec) [\#7519](https://github.com/cosmos/cosmos-sdk/pull/7519) `InterfaceRegistry` now inherits `jsonpb.AnyResolver`, and has a `RegisterCustomTypeURL` method to support ADR 031 packing of `Any`s. `AnyResolver` is now a required parameter to `RejectUnknownFields`.
* (baseapp) [\#7519](https://github.com/cosmos/cosmos-sdk/pull/7519) Add `ServiceMsgRouter` to BaseApp to handle routing of protobuf service `Msg`s. The two new types defined in ADR 031, `sdk.ServiceMsg` and `sdk.MsgRequest` are introduced with this router.
* (tx) [\#7688](https://github.com/cosmos/cosmos-sdk/pull/7688) Add a new Tx gRPC service with methods `Simulate` and `GetTx` (by hash).
* `x/crisis` has a new function: `AddModuleInitFlags`, which will register optional crisis module flags for the start command.


### Bug Fixes

Expand Down
2 changes: 1 addition & 1 deletion server/export.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func ExportCmd(appExporter types.AppExporter, defaultNodeHome string) *cobra.Com
forZeroHeight, _ := cmd.Flags().GetBool(FlagForZeroHeight)
jailAllowedAddrs, _ := cmd.Flags().GetStringSlice(FlagJailAllowedAddrs)

exported, err := appExporter(serverCtx.Logger, db, traceWriter, height, forZeroHeight, jailAllowedAddrs)
exported, err := appExporter(serverCtx.Logger, db, traceWriter, height, forZeroHeight, jailAllowedAddrs, serverCtx.Viper)
if err != nil {
return fmt.Errorf("error exporting state: %v", err)
}
Expand Down
8 changes: 4 additions & 4 deletions server/export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func setupApp(t *testing.T, tempDir string) (*simapp.SimApp, context.Context, *t
logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout))
db := dbm.NewMemDB()
encCfg := simapp.MakeTestEncodingConfig()
app := simapp.NewSimApp(logger, db, nil, true, map[int64]bool{}, tempDir, 0, encCfg)
app := simapp.NewSimApp(logger, db, nil, true, map[int64]bool{}, tempDir, 0, encCfg, simapp.EmptyAppOptions{})

serverCtx := server.NewDefaultContext()
serverCtx.Config.RootDir = tempDir
Expand All @@ -148,18 +148,18 @@ func setupApp(t *testing.T, tempDir string) (*simapp.SimApp, context.Context, *t
app.Commit()

cmd := server.ExportCmd(
func(_ log.Logger, _ dbm.DB, _ io.Writer, height int64, forZeroHeight bool, jailAllowedAddrs []string) (types.ExportedApp, error) {
func(_ log.Logger, _ dbm.DB, _ io.Writer, height int64, forZeroHeight bool, jailAllowedAddrs []string, appOptons types.AppOptions) (types.ExportedApp, error) {
encCfg := simapp.MakeTestEncodingConfig()

var simApp *simapp.SimApp
if height != -1 {
simApp = simapp.NewSimApp(logger, db, nil, false, map[int64]bool{}, "", 0, encCfg)
simApp = simapp.NewSimApp(logger, db, nil, false, map[int64]bool{}, "", 0, encCfg, appOptons)

if err := simApp.LoadHeight(height); err != nil {
return types.ExportedApp{}, err
}
} else {
simApp = simapp.NewSimApp(logger, db, nil, true, map[int64]bool{}, "", 0, encCfg)
simApp = simapp.NewSimApp(logger, db, nil, true, map[int64]bool{}, "", 0, encCfg, appOptons)
}

return simApp.ExportAppStateAndValidators(forZeroHeight, jailAllowedAddrs)
Expand Down
41 changes: 21 additions & 20 deletions server/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,25 @@ func startStandAlone(ctx *Context, appCreator types.AppCreator) error {
func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.AppCreator) error {
cfg := ctx.Config
home := cfg.RootDir
var cpuProfileCleanup func()

if cpuProfile := ctx.Viper.GetString(flagCPUProfile); cpuProfile != "" {
f, err := os.Create(cpuProfile)
if err != nil {
return err
}

ctx.Logger.Info("starting CPU profiler", "profile", cpuProfile)
if err := pprof.StartCPUProfile(f); err != nil {
return err
}

cpuProfileCleanup = func() {
ctx.Logger.Info("stopping CPU profiler", "profile", cpuProfile)
pprof.StopCPUProfile()
f.Close()
}
}
robert-zaremba marked this conversation as resolved.
Show resolved Hide resolved

traceWriterFile := ctx.Viper.GetString(flagTraceStore)
db, err := openDB(home)
Expand Down Expand Up @@ -226,10 +245,12 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App
if err != nil {
return err
}
ctx.Logger.Debug("Initialization: tmNode created")

if err := tmNode.Start(); err != nil {
return err
}
ctx.Logger.Debug("Initialization: tmNode started")

// Add the tx service to the gRPC router.
app.RegisterTxService(clientCtx)
Expand Down Expand Up @@ -273,26 +294,6 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App
}
}

var cpuProfileCleanup func()

if cpuProfile := ctx.Viper.GetString(flagCPUProfile); cpuProfile != "" {
f, err := os.Create(cpuProfile)
if err != nil {
return err
}

ctx.Logger.Info("starting CPU profiler", "profile", cpuProfile)
if err := pprof.StartCPUProfile(f); err != nil {
return err
}

cpuProfileCleanup = func() {
ctx.Logger.Info("stopping CPU profiler", "profile", cpuProfile)
pprof.StopCPUProfile()
f.Close()
}
}

defer func() {
if tmNode.IsRunning() {
_ = tmNode.Stop()
Expand Down
6 changes: 5 additions & 1 deletion server/types/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"io"

"github.com/gogo/protobuf/grpc"
"github.com/spf13/cobra"
abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/libs/log"
tmtypes "github.com/tendermint/tendermint/types"
Expand Down Expand Up @@ -48,6 +49,9 @@ type (
// application using various configurations.
AppCreator func(log.Logger, dbm.DB, io.Writer, AppOptions) Application

// ModuleInitFlags takes a start command and adds modules specific init flags.
ModuleInitFlags func(startCmd *cobra.Command)

// ExportedApp represents an exported app state, along with
// validators, consensus params and latest app height.
ExportedApp struct {
Expand All @@ -63,5 +67,5 @@ type (

// AppExporter is a function that dumps all app state to
// JSON-serializable structure and returns the current validator set.
AppExporter func(log.Logger, dbm.DB, io.Writer, int64, bool, []string) (ExportedApp, error)
AppExporter func(log.Logger, dbm.DB, io.Writer, int64, bool, []string, AppOptions) (ExportedApp, error)
)
6 changes: 4 additions & 2 deletions server/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ func interceptConfigs(rootViper *viper.Viper) (*tmcfg.Config, error) {
}

// add server commands
func AddCommands(rootCmd *cobra.Command, defaultNodeHome string, appCreator types.AppCreator, appExport types.AppExporter) {
func AddCommands(rootCmd *cobra.Command, defaultNodeHome string, appCreator types.AppCreator, appExport types.AppExporter, addStartFlags types.ModuleInitFlags) {
tendermintCmd := &cobra.Command{
Use: "tendermint",
Short: "Tendermint subcommands",
Expand All @@ -213,9 +213,11 @@ func AddCommands(rootCmd *cobra.Command, defaultNodeHome string, appCreator type
ShowAddressCmd(),
VersionCmd(),
)
startCmd := StartCmd(appCreator, defaultNodeHome)
addStartFlags(startCmd)

rootCmd.AddCommand(
StartCmd(appCreator, defaultNodeHome),
startCmd,
UnsafeResetAllCmd(),
flags.LineBreak,
tendermintCmd,
Expand Down
12 changes: 10 additions & 2 deletions simapp/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/gorilla/mux"
"github.com/rakyll/statik/fs"
"github.com/spf13/cast"
abci "github.com/tendermint/tendermint/abci/types"
tmjson "github.com/tendermint/tendermint/libs/json"
"github.com/tendermint/tendermint/libs/log"
Expand Down Expand Up @@ -197,7 +198,8 @@ func init() {
// NewSimApp returns a reference to an initialized SimApp.
func NewSimApp(
logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bool, skipUpgradeHeights map[int64]bool,
homePath string, invCheckPeriod uint, encodingConfig simappparams.EncodingConfig, baseAppOptions ...func(*baseapp.BaseApp),
homePath string, invCheckPeriod uint, encodingConfig simappparams.EncodingConfig,
appOpts servertypes.AppOptions, baseAppOptions ...func(*baseapp.BaseApp),
Comment on lines 199 to +202
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any explicit argument we can remove from this constructor that are found in appOptons?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure. I can handle it in a separate PR.

Copy link
Contributor

@alexanderbez alexanderbez Nov 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can omit homePath string, invCheckPeriod uint. Feel free to do it in this PR or another :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer in another and have this one merged. This one is about InitChain options.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added TODOs to #7178

) *SimApp {

// TODO: Remove cdc in favor of appCodec once all modules are migrated.
Expand Down Expand Up @@ -317,6 +319,12 @@ func NewSimApp(
// If evidence needs to be handled for the app, set routes in router here and seal
app.EvidenceKeeper = *evidenceKeeper

/**** Module Options ****/

// NOTE: we may consider parsing `appOpts` inside module constructors. For the moment
// we prefer to be more strict in what arguments the modules expect.
var skipGenesisInvariants = cast.ToBool(appOpts.Get(crisis.FlagSkipGenesisInvariants))

// NOTE: Any module instantiated in the module manager that is later modified
// must be passed by reference here.
app.mm = module.NewManager(
Expand All @@ -328,7 +336,7 @@ func NewSimApp(
vesting.NewAppModule(app.AccountKeeper, app.BankKeeper),
bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper),
capability.NewAppModule(appCodec, *app.CapabilityKeeper),
crisis.NewAppModule(&app.CrisisKeeper),
crisis.NewAppModule(&app.CrisisKeeper, skipGenesisInvariants),
gov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper),
mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper),
slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
Expand Down
6 changes: 3 additions & 3 deletions simapp/app_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (

func TestSimAppExport(t *testing.T) {
db := dbm.NewMemDB()
app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, MakeTestEncodingConfig())
app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, MakeTestEncodingConfig(), EmptyAppOptions{})

genesisState := NewDefaultGenesisState()
stateBytes, err := json.MarshalIndent(genesisState, "", " ")
Expand All @@ -30,15 +30,15 @@ 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())
app2 := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, MakeTestEncodingConfig(), 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())
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)))
Expand Down
4 changes: 2 additions & 2 deletions simapp/sim_bench_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func BenchmarkFullAppSimulation(b *testing.B) {
}
}()

app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), interBlockCacheOpt())
app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, interBlockCacheOpt())

// run randomized simulation
_, simParams, simErr := simulation.SimulateFromSeed(
Expand Down Expand Up @@ -72,7 +72,7 @@ func BenchmarkInvariants(b *testing.B) {
}
}()

app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), interBlockCacheOpt())
app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, interBlockCacheOpt())

// run randomized simulation
_, simParams, simErr := simulation.SimulateFromSeed(
Expand Down
12 changes: 6 additions & 6 deletions simapp/sim_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func TestFullAppSimulation(t *testing.T) {
require.NoError(t, os.RemoveAll(dir))
}()

app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), fauxMerkleModeOpt)
app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, fauxMerkleModeOpt)
require.Equal(t, "SimApp", app.Name())

// run randomized simulation
Expand Down Expand Up @@ -106,7 +106,7 @@ func TestAppImportExport(t *testing.T) {
require.NoError(t, os.RemoveAll(dir))
}()

app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), fauxMerkleModeOpt)
app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, fauxMerkleModeOpt)
require.Equal(t, "SimApp", app.Name())

// Run randomized simulation
Expand Down Expand Up @@ -146,7 +146,7 @@ func TestAppImportExport(t *testing.T) {
require.NoError(t, os.RemoveAll(newDir))
}()

newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), fauxMerkleModeOpt)
newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, fauxMerkleModeOpt)
require.Equal(t, "SimApp", newApp.Name())

var genesisState GenesisState
Expand Down Expand Up @@ -203,7 +203,7 @@ func TestAppSimulationAfterImport(t *testing.T) {
require.NoError(t, os.RemoveAll(dir))
}()

app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), fauxMerkleModeOpt)
app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, fauxMerkleModeOpt)
require.Equal(t, "SimApp", app.Name())

// Run randomized simulation
Expand Down Expand Up @@ -248,7 +248,7 @@ func TestAppSimulationAfterImport(t *testing.T) {
require.NoError(t, os.RemoveAll(newDir))
}()

newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), fauxMerkleModeOpt)
newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, fauxMerkleModeOpt)
require.Equal(t, "SimApp", newApp.Name())

newApp.InitChain(abci.RequestInitChain{
Expand Down Expand Up @@ -299,7 +299,7 @@ func TestAppStateDeterminism(t *testing.T) {
}

db := dbm.NewMemDB()
app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), interBlockCacheOpt())
app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, interBlockCacheOpt())

fmt.Printf(
"running non-determinism simulation; seed %d: %d/%d, attempt: %d/%d\n",
Expand Down
22 changes: 14 additions & 8 deletions simapp/simd/cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ import (
"os"
"path/filepath"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/simapp/params"
"github.com/cosmos/cosmos-sdk/snapshots"

"github.com/spf13/cast"
"github.com/spf13/cobra"
tmcli "github.com/tendermint/tendermint/libs/cli"
Expand All @@ -22,16 +18,20 @@ 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"
"github.com/cosmos/cosmos-sdk/simapp/params"
"github.com/cosmos/cosmos-sdk/snapshots"
"github.com/cosmos/cosmos-sdk/store"
sdk "github.com/cosmos/cosmos-sdk/types"
authclient "github.com/cosmos/cosmos-sdk/x/auth/client"
authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli"
"github.com/cosmos/cosmos-sdk/x/auth/types"
vestingcli "github.com/cosmos/cosmos-sdk/x/auth/vesting/client/cli"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/cosmos/cosmos-sdk/x/crisis"
genutilcli "github.com/cosmos/cosmos-sdk/x/genutil/client/cli"
)

Expand Down Expand Up @@ -97,7 +97,7 @@ func initRootCmd(rootCmd *cobra.Command, encodingConfig params.EncodingConfig) {
debug.Cmd(),
)

server.AddCommands(rootCmd, simapp.DefaultNodeHome, newApp, createSimappAndExport)
server.AddCommands(rootCmd, simapp.DefaultNodeHome, newApp, createSimappAndExport, addModuleInitFlags)

// add keybase, auxiliary RPC, query, and tx child commands
rootCmd.AddCommand(
Expand All @@ -108,6 +108,10 @@ func initRootCmd(rootCmd *cobra.Command, encodingConfig params.EncodingConfig) {
)
}

func addModuleInitFlags(startCmd *cobra.Command) {
crisis.AddModuleInitFlags(startCmd)
}

func queryCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "query",
Expand Down Expand Up @@ -192,6 +196,7 @@ func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, appOpts serverty
cast.ToString(appOpts.Get(flags.FlagHome)),
cast.ToUint(appOpts.Get(server.FlagInvCheckPeriod)),
simapp.MakeTestEncodingConfig(), // Ideally, we would reuse the one created by NewRootCmd.
appOpts,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove cast.ToString(appOpts.Get(flags.FlagHome)), cast.ToUint(appOpts.Get(server.FlagInvCheckPeriod)), and use appOpts directly to read the values inside NewSimApp? Let's minimize the number of arguments when possible. May be not relavant for this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was planning to group all this config options already long time ago.
Should I do it in this PR or in a new one? Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A separate PR works too, can you create an issue to track this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we should do it in a separate post, because this will be another breaking feature. Also, not sure if that will land in release, @clevinson, @aaronc , what do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the old issue I created it. I'm happy that we like to see a reduction of this arguments. It's an easy change.
#7178

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well we should still keep the variadic options (which is what I was trying to advocate for in #7178 -- I'm against options structs for ALL arguments in this case). However, for the sake of this PR, we can at least avoid the following calls/args:

cast.ToString(appOpts.Get(flags.FlagHome)),
cast.ToUint(appOpts.Get(server.FlagInvCheckPeriod)),

Just pass appOpts and let the app do the above two calls. This is what I meant.

baseapp.SetPruning(pruningOpts),
baseapp.SetMinGasPrices(cast.ToString(appOpts.Get(server.FlagMinGasPrices))),
baseapp.SetHaltHeight(cast.ToUint64(appOpts.Get(server.FlagHaltHeight))),
Expand All @@ -210,18 +215,19 @@ func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, appOpts serverty
// and exports state.
func createSimappAndExport(
logger log.Logger, db dbm.DB, traceStore io.Writer, height int64, forZeroHeight bool, jailAllowedAddrs []string,
) (servertypes.ExportedApp, error) {
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
if height != -1 {
simApp = simapp.NewSimApp(logger, db, traceStore, false, map[int64]bool{}, "", uint(1), encCfg)
simApp = simapp.NewSimApp(logger, db, traceStore, false, map[int64]bool{}, "", uint(1), encCfg, appOpts)

if err := simApp.LoadHeight(height); err != nil {
return servertypes.ExportedApp{}, err
}
} else {
simApp = simapp.NewSimApp(logger, db, traceStore, true, map[int64]bool{}, "", uint(1), encCfg)
simApp = simapp.NewSimApp(logger, db, traceStore, true, map[int64]bool{}, "", uint(1), encCfg, appOpts)
}

return simApp.ExportAppStateAndValidators(forZeroHeight, jailAllowedAddrs)
Expand Down
Loading