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.

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

Some options missing in App constructor #3600

Closed
max-hontar opened this issue Jul 27, 2023 · 1 comment · Fixed by #3614
Closed

Some options missing in App constructor #3600

max-hontar opened this issue Jul 27, 2023 · 1 comment · Fixed by #3614
Assignees

Comments

@max-hontar
Copy link
Contributor

Currently in app template application created with this call:

return app.New(
logger,
db,
traceStore,
true,
skipUpgradeHeights,
cast.ToString(appOpts.Get(flags.FlagHome)),
cast.ToUint(appOpts.Get(server.FlagInvCheckPeriod)),
a.encodingConfig,
appOpts,
baseapp.SetPruning(pruningOpts),
baseapp.SetMinGasPrices(cast.ToString(appOpts.Get(server.FlagMinGasPrices))),
baseapp.SetHaltHeight(cast.ToUint64(appOpts.Get(server.FlagHaltHeight))),
baseapp.SetHaltTime(cast.ToUint64(appOpts.Get(server.FlagHaltTime))),
baseapp.SetMinRetainBlocks(cast.ToUint64(appOpts.Get(server.FlagMinRetainBlocks))),
baseapp.SetInterBlockCache(cache),
baseapp.SetTrace(cast.ToBool(appOpts.Get(server.FlagTrace))),
baseapp.SetIndexEvents(cast.ToStringSlice(appOpts.Get(server.FlagIndexEvents))),
baseapp.SetSnapshot(snapshotStore, snapshotOptions),
baseapp.SetIAVLCacheSize(cast.ToInt(appOpts.Get(server.FlagIAVLCacheSize))),
baseapp.SetIAVLDisableFastNode(cast.ToBool(appOpts.Get(server.FlagDisableIAVLFastNode))),
baseapp.SetChainID(chainID),
)

But even in Cosmos SDK v0.46.x there is one more flag available that missing in app constructor:

baseapp.SetIAVLLazyLoading(cast.ToBool(appOpts.Get(FlagIAVLLazyLoading))),

In next Cosmos SDK v0.47.x was added another one option that also missing

baseapp.SetMempool(
	mempool.NewSenderNonceMempool(
		mempool.SenderNonceMaxTxOpt(cast.ToInt(appOpts.Get(FlagMempoolMaxTxs))),
	),
),

Not sure about effect of previous one, but if we don't have defined mempool we lose ABCI++ new functions (PrepareProposal + ProcessProposal) as they just skipped.

Also Cosmos SDK have helper function to build all default options, so why not use it, then constructor call look like this

	return app.New(
		logger,
		db,
		traceStore,
		true,
		skipUpgradeHeights,
		cast.ToString(appOpts.Get(flags.FlagHome)),
		cast.ToUint(appOpts.Get(server.FlagInvCheckPeriod)),
		a.encodingConfig,
		appOpts,
                server.DefaultBaseappOptions(appOpts)...,
	)

This allow remove huge part of default options initialization and make function much more compact, also all missed options already defined inside!

@Pantani
Copy link
Collaborator

Pantani commented Aug 5, 2023

@max-hontar, nice catch, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants