From 39d4047ce2b766646cc2179857c5eefd677c2290 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 22 Jun 2022 13:05:40 +0200 Subject: [PATCH] feat!: migrate `x/upgrade` to use app wiring (#12312) --- app.go | 29 +++++++++++++---------------- app.yaml | 4 ++++ app_test.go | 35 +++++++++++++++++------------------ sim_bench_test.go | 5 +++-- sim_test.go | 12 ++++++------ simd/cmd/root.go | 10 ++++++---- test_helpers.go | 24 +++++++----------------- 7 files changed, 56 insertions(+), 63 deletions(-) diff --git a/app.go b/app.go index 802b99f6ada3..8beea90b13e8 100644 --- a/app.go +++ b/app.go @@ -99,8 +99,6 @@ import ( _ "github.com/cosmos/cosmos-sdk/client/docs/statik" ) -const appName = "SimApp" - var ( // DefaultNodeHome default home directories for the application daemon DefaultNodeHome string @@ -200,17 +198,18 @@ var AppConfig = appconfig.LoadYAML(appConfigYaml) // 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, + logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bool, invCheckPeriod uint, encodingConfig simappparams.EncodingConfig, appOpts servertypes.AppOptions, baseAppOptions ...func(*baseapp.BaseApp), ) *SimApp { - app := &SimApp{ - invCheckPeriod: invCheckPeriod, - } - var appBuilder *runtime.AppBuilder + var ( + appBuilder *runtime.AppBuilder + app = &SimApp{invCheckPeriod: invCheckPeriod} + // merge the app.yaml and the appOpts in one config + appConfig = depinject.Configs(AppConfig, depinject.Supply(appOpts)) + ) - if err := depinject.Inject(AppConfig, + if err := depinject.Inject(appConfig, &appBuilder, &app.ParamsKeeper, &app.CapabilityKeeper, @@ -228,16 +227,14 @@ func NewSimApp( &app.MintKeeper, &app.EvidenceKeeper, &app.DistrKeeper, + &app.UpgradeKeeper, ); err != nil { panic(err) } app.App = appBuilder.Build(logger, db, traceStore, baseAppOptions...) - app.keys = sdk.NewKVStoreKeys( - govtypes.StoreKey, - upgradetypes.StoreKey, - ) + app.keys = sdk.NewKVStoreKeys(govtypes.StoreKey) // configure state listening capabilities using AppOptions // we are doing nothing with the returned streamingServices and waitGroup in this case @@ -276,11 +273,12 @@ func NewSimApp( // register the governance hooks ), ) - // set the governance module account as the authority for conducting upgrades - app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, app.keys[upgradetypes.StoreKey], app.appCodec, homePath, app.BaseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String()) /**** Module Options ****/ + // Sets the version setter for the upgrade module + app.UpgradeKeeper.SetVersionSetter(app.BaseApp) + // NOTE: we may consider parsing `appOpts` inside module constructors. For the moment // we prefer to be more strict in what arguments the modules expect. skipGenesisInvariants := cast.ToBool(appOpts.Get(crisis.FlagSkipGenesisInvariants)) @@ -290,7 +288,6 @@ func NewSimApp( if err := app.RegisterModules( crisis.NewAppModule(&app.CrisisKeeper, skipGenesisInvariants), gov.NewAppModule(app.appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper), - upgrade.NewAppModule(app.UpgradeKeeper), ); err != nil { panic(err) } diff --git a/app.yaml b/app.yaml index 56b4a75a486f..277b236d006b 100644 --- a/app.yaml +++ b/app.yaml @@ -137,3 +137,7 @@ modules: - name: vesting config: "@type": cosmos.vesting.module.v1.Module + + - name: upgrade + config: + "@type": cosmos.upgrade.module.v1.Module diff --git a/app_test.go b/app_test.go index e8e27f1e2bf5..b3e3874a2dfd 100644 --- a/app_test.go +++ b/app_test.go @@ -13,6 +13,7 @@ import ( "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/tests/mocks" + simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/cosmos-sdk/x/auth" @@ -40,13 +41,11 @@ func TestSimAppExportAndBlockedAddrs(t *testing.T) { db := dbm.NewMemDB() logger, _ := log.NewDefaultLogger("plain", "info", false) app := NewSimappWithCustomOptions(t, false, SetupOptions{ - Logger: logger, - DB: db, - InvCheckPeriod: 0, - EncConfig: encCfg, - HomePath: DefaultNodeHome, - SkipUpgradeHeights: map[int64]bool{}, - AppOpts: EmptyAppOptions{}, + Logger: logger, + DB: db, + InvCheckPeriod: 0, + EncConfig: encCfg, + AppOpts: simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome), }) for acc := range maccPerms { @@ -61,7 +60,7 @@ func TestSimAppExportAndBlockedAddrs(t *testing.T) { logger2, _ := log.NewDefaultLogger("plain", "info", false) // Making a new app object with the db, so that initchain hasn't been called - app2 := NewSimApp(logger2, db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{}) + app2 := NewSimApp(logger2, db, nil, true, 0, encCfg, simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome)) _, err := app2.ExportAppStateAndValidators(false, []string{}) require.NoError(t, err, "ExportAppStateAndValidators should not have an error") } @@ -75,10 +74,10 @@ func TestRunMigrations(t *testing.T) { db := dbm.NewMemDB() encCfg := MakeTestEncodingConfig() logger, _ := log.NewDefaultLogger("plain", "info", false) - app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{}) + app := NewSimApp(logger, db, nil, true, 0, encCfg, simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome)) // Create a new baseapp and configurator for the purpose of this test. - bApp := baseapp.NewBaseApp(appName, logger, db, encCfg.TxConfig.TxDecoder()) + bApp := baseapp.NewBaseApp(app.Name(), logger, db, encCfg.TxConfig.TxDecoder()) bApp.SetCommitMultiStoreTracer(nil) bApp.SetInterfaceRegistry(encCfg.InterfaceRegistry) app.BaseApp = bApp @@ -208,7 +207,7 @@ func TestInitGenesisOnMigration(t *testing.T) { db := dbm.NewMemDB() encCfg := MakeTestEncodingConfig() logger, _ := log.NewDefaultLogger("plain", "info", false) - app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{}) + app := NewSimApp(logger, db, nil, true, 0, encCfg, simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome)) ctx := app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}) // Create a mock module. This module will serve as the new module we're @@ -253,13 +252,11 @@ func TestUpgradeStateOnGenesis(t *testing.T) { db := dbm.NewMemDB() logger, _ := log.NewDefaultLogger("plain", "info", false) app := NewSimappWithCustomOptions(t, false, SetupOptions{ - Logger: logger, - DB: db, - InvCheckPeriod: 0, - EncConfig: encCfg, - HomePath: DefaultNodeHome, - SkipUpgradeHeights: map[int64]bool{}, - AppOpts: EmptyAppOptions{}, + Logger: logger, + DB: db, + InvCheckPeriod: 0, + EncConfig: encCfg, + AppOpts: simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome), }) // make sure the upgrade keeper has version map in state @@ -268,4 +265,6 @@ func TestUpgradeStateOnGenesis(t *testing.T) { for v, i := range app.ModuleManager.Modules { require.Equal(t, vm[v], i.ConsensusVersion()) } + + require.NotNil(t, app.UpgradeKeeper.GetVersionSetter()) } diff --git a/sim_bench_test.go b/sim_bench_test.go index ae96661a9a16..9536a9699d17 100644 --- a/sim_bench_test.go +++ b/sim_bench_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" "github.com/cosmos/cosmos-sdk/x/simulation" ) @@ -30,7 +31,7 @@ func BenchmarkFullAppSimulation(b *testing.B) { require.NoError(b, os.RemoveAll(dir)) }() - app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, interBlockCacheOpt()) + app := NewSimApp(logger, db, nil, true, FlagPeriodValue, MakeTestEncodingConfig(), simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome), interBlockCacheOpt()) // run randomized simulation _, simParams, simErr := simulation.SimulateFromSeed( @@ -77,7 +78,7 @@ func BenchmarkInvariants(b *testing.B) { require.NoError(b, os.RemoveAll(dir)) }() - app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, interBlockCacheOpt()) + app := NewSimApp(logger, db, nil, true, FlagPeriodValue, MakeTestEncodingConfig(), simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome), interBlockCacheOpt()) // run randomized simulation _, simParams, simErr := simulation.SimulateFromSeed( diff --git a/sim_test.go b/sim_test.go index 189f2d119fe3..d50a6b0df4d3 100644 --- a/sim_test.go +++ b/sim_test.go @@ -71,7 +71,7 @@ func TestFullAppSimulation(t *testing.T) { require.NoError(t, os.RemoveAll(dir)) }() - app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, fauxMerkleModeOpt) + app := NewSimApp(logger, db, nil, true, FlagPeriodValue, MakeTestEncodingConfig(), simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome), fauxMerkleModeOpt) require.Equal(t, "SimApp", app.Name()) // run randomized simulation @@ -109,7 +109,7 @@ func TestAppImportExport(t *testing.T) { require.NoError(t, os.RemoveAll(dir)) }() - app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, fauxMerkleModeOpt) + app := NewSimApp(logger, db, nil, true, FlagPeriodValue, MakeTestEncodingConfig(), simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome), fauxMerkleModeOpt) require.Equal(t, "SimApp", app.Name()) // Run randomized simulation @@ -149,7 +149,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(), EmptyAppOptions{}, fauxMerkleModeOpt) + newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, FlagPeriodValue, MakeTestEncodingConfig(), simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome), fauxMerkleModeOpt) require.Equal(t, "SimApp", newApp.Name()) var genesisState GenesisState @@ -218,7 +218,7 @@ func TestAppSimulationAfterImport(t *testing.T) { require.NoError(t, os.RemoveAll(dir)) }() - app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, fauxMerkleModeOpt) + app := NewSimApp(logger, db, nil, true, FlagPeriodValue, MakeTestEncodingConfig(), simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome), fauxMerkleModeOpt) require.Equal(t, "SimApp", app.Name()) // Run randomized simulation @@ -263,7 +263,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(), EmptyAppOptions{}, fauxMerkleModeOpt) + newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, FlagPeriodValue, MakeTestEncodingConfig(), simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome), fauxMerkleModeOpt) require.Equal(t, "SimApp", newApp.Name()) newApp.InitChain(abci.RequestInitChain{ @@ -314,7 +314,7 @@ func TestAppStateDeterminism(t *testing.T) { } db := dbm.NewMemDB() - app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, FlagPeriodValue, MakeTestEncodingConfig(), EmptyAppOptions{}, interBlockCacheOpt()) + app := NewSimApp(logger, db, nil, true, FlagPeriodValue, MakeTestEncodingConfig(), simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome), interBlockCacheOpt()) fmt.Printf( "running non-determinism simulation; seed %d: %d/%d, attempt: %d/%d\n", diff --git a/simd/cmd/root.go b/simd/cmd/root.go index cb0d3f6e0c34..af91a0437778 100644 --- a/simd/cmd/root.go +++ b/simd/cmd/root.go @@ -277,8 +277,7 @@ func (a appCreator) newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, a ) return simapp.NewSimApp( - logger, db, traceStore, true, skipUpgradeHeights, - cast.ToString(appOpts.Get(flags.FlagHome)), + logger, db, traceStore, true, cast.ToUint(appOpts.Get(server.FlagInvCheckPeriod)), a.encCfg, appOpts, @@ -300,19 +299,22 @@ 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) { var simApp *simapp.SimApp + + // this check is necessary as we use the flag in x/upgrade. + // we can exit more gracefully by checking the flag here. 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), a.encCfg, appOpts) + simApp = simapp.NewSimApp(logger, db, traceStore, false, 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), a.encCfg, appOpts) + simApp = simapp.NewSimApp(logger, db, traceStore, true, uint(1), a.encCfg, appOpts) } return simApp.ExportAppStateAndValidators(forZeroHeight, jailAllowedAddrs) diff --git a/test_helpers.go b/test_helpers.go index aff0a84e3cb9..92849b80a88c 100644 --- a/test_helpers.go +++ b/test_helpers.go @@ -37,19 +37,17 @@ import ( // SetupOptions defines arguments that are passed into `Simapp` constructor. type SetupOptions struct { - Logger log.Logger - DB *dbm.MemDB - InvCheckPeriod uint - HomePath string - SkipUpgradeHeights map[int64]bool - EncConfig params.EncodingConfig - AppOpts types.AppOptions + Logger log.Logger + DB *dbm.MemDB + InvCheckPeriod uint + EncConfig params.EncodingConfig + AppOpts types.AppOptions } 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{}) + app := NewSimApp(log.NewNopLogger(), db, nil, true, invCheckPeriod, encCdc, simtestutil.NewAppOptionsWithFlagHome(DefaultNodeHome)) if withGenesis { return app, NewDefaultGenesisState(encCdc.Codec) } @@ -75,7 +73,7 @@ func NewSimappWithCustomOptions(t *testing.T, isCheckTx bool, options SetupOptio Coins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100000000000000))), } - app := NewSimApp(options.Logger, options.DB, nil, true, options.SkipUpgradeHeights, options.HomePath, options.InvCheckPeriod, options.EncConfig, options.AppOpts) + app := NewSimApp(options.Logger, options.DB, nil, true, options.InvCheckPeriod, options.EncConfig, options.AppOpts) genesisState := NewDefaultGenesisState(app.appCodec) genesisState, err = simtestutil.GenesisStateWithValSet(app.AppCodec(), genesisState, valSet, []authtypes.GenesisAccount{acc}, balance) require.NoError(t, err) @@ -378,14 +376,6 @@ func NewPubKeyFromHex(pk string) (res cryptotypes.PubKey) { return &ed25519.PubKey{Key: pkBytes} } -// EmptyAppOptions is a stub implementing AppOptions -type EmptyAppOptions struct{} - -// Get implements AppOptions -func (ao EmptyAppOptions) Get(o string) interface{} { - return nil -} - // ModuleAccountAddrs provides a list of blocked module accounts from configuration in app.yaml // // Ported from SimApp