-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
appcreator: Reuse encoding config instead of recreating it #8250
Changes from all commits
cec3889
1c3f50c
2efc5c4
5d3f4a0
5fa1652
4467e95
4854a54
3e9e5e0
adf8025
9f60f7d
8c10ffa
2e7046d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Ouch, what a bad name for a variable... Mind renaming it to something more meaningful please? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you mean There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW: this is very cool guidance for naming in Go: https://talks.golang.org/2014/names.slide#1 |
||||||
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 | ||||||
alessio marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
|
||||||
// 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) | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I merged this test into
TestSimAppExportAndBlockedAddrs
- no need to recreate the DB and app. Other idea will be to do a subtest.