From a5fc7d6ba67a14ebc792cdb7326469f67741b751 Mon Sep 17 00:00:00 2001 From: Anil Kumar Kammari Date: Fri, 3 Jan 2020 20:07:29 +0530 Subject: [PATCH] Merge PR #5367: Regen network/skip upgrade --- server/start.go | 23 +- simapp/app.go | 4 +- simapp/app_test.go | 6 +- simapp/sim_bench_test.go | 4 +- simapp/sim_test.go | 12 +- simapp/test_helpers.go | 4 +- x/crisis/handler_test.go | 2 +- x/crisis/internal/keeper/integration_test.go | 2 +- x/upgrade/abci.go | 18 +- x/upgrade/abci_test.go | 364 +++++++++++++++---- x/upgrade/internal/keeper/keeper.go | 29 +- x/upgrade/internal/keeper/querier.go | 2 +- 12 files changed, 350 insertions(+), 120 deletions(-) diff --git a/server/start.go b/server/start.go index 560d1614ac..8d1388791a 100644 --- a/server/start.go +++ b/server/start.go @@ -20,15 +20,16 @@ import ( // Tendermint full-node start flags const ( - flagWithTendermint = "with-tendermint" - flagAddress = "address" - flagTraceStore = "trace-store" - flagPruning = "pruning" - flagCPUProfile = "cpu-profile" - FlagMinGasPrices = "minimum-gas-prices" - FlagHaltHeight = "halt-height" - FlagHaltTime = "halt-time" - FlagInterBlockCache = "inter-block-cache" + flagWithTendermint = "with-tendermint" + flagAddress = "address" + flagTraceStore = "trace-store" + flagPruning = "pruning" + flagCPUProfile = "cpu-profile" + FlagMinGasPrices = "minimum-gas-prices" + FlagHaltHeight = "halt-height" + FlagHaltTime = "halt-time" + FlagInterBlockCache = "inter-block-cache" + FlagUnsafeSkipUpgrades = "unsafe-skip-upgrades" ) // StartCmd runs the service passed in, either stand-alone or in-process with @@ -77,6 +78,7 @@ which accepts a path for the resulting pprof file. FlagMinGasPrices, "", "Minimum gas prices to accept for transactions; Any fee in a tx must meet this minimum (e.g. 0.01photino;0.0001stake)", ) + cmd.Flags().IntSlice(FlagUnsafeSkipUpgrades, []int{}, "Skip a set of upgrade heights to continue the old binary") cmd.Flags().Uint64(FlagHaltHeight, 0, "Block height at which to gracefully halt the chain and shutdown the node") cmd.Flags().Uint64(FlagHaltTime, 0, "Minimum block time (in Unix seconds) at which to gracefully halt the chain and shutdown the node") cmd.Flags().Bool(FlagInterBlockCache, true, "Enable inter-block caching") @@ -130,12 +132,13 @@ func startStandAlone(ctx *Context, appCreator AppCreator) error { func startInProcess(ctx *Context, appCreator AppCreator) (*node.Node, error) { cfg := ctx.Config home := cfg.RootDir - traceWriterFile := viper.GetString(flagTraceStore) + traceWriterFile := viper.GetString(flagTraceStore) db, err := openDB(home) if err != nil { return nil, err } + traceWriter, err := openTraceWriter(traceWriterFile) if err != nil { return nil, err diff --git a/simapp/app.go b/simapp/app.go index e4e4ede4e9..2b5d240c3e 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -131,7 +131,7 @@ type SimApp struct { // NewSimApp returns a reference to an initialized SimApp. func NewSimApp( - logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bool, + logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bool, skipUpgradeHeights map[int64]bool, invCheckPeriod uint, baseAppOptions ...func(*bam.BaseApp), ) *SimApp { @@ -196,7 +196,7 @@ func NewSimApp( app.CrisisKeeper = crisis.NewKeeper( app.subspaces[crisis.ModuleName], invCheckPeriod, app.SupplyKeeper, auth.FeeCollectorName, ) - app.UpgradeKeeper = upgrade.NewKeeper(keys[upgrade.StoreKey], app.cdc) + app.UpgradeKeeper = upgrade.NewKeeper(skipUpgradeHeights, keys[upgrade.StoreKey], app.cdc) // create evidence keeper with router evidenceKeeper := evidence.NewKeeper( diff --git a/simapp/app_test.go b/simapp/app_test.go index 1f152976f1..9cf211a941 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -15,7 +15,7 @@ import ( func TestSimAppExport(t *testing.T) { db := dbm.NewMemDB() - app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, 0) + app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, 0) genesisState := NewDefaultGenesisState() stateBytes, err := codec.MarshalJSONIndent(app.Codec(), genesisState) @@ -31,7 +31,7 @@ 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, 0) + app2 := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, 0) _, _, err = app2.ExportAppStateAndValidators(false, []string{}) require.NoError(t, err, "ExportAppStateAndValidators should not have an error") } @@ -39,7 +39,7 @@ func TestSimAppExport(t *testing.T) { // ensure that black listed addresses are properly set in bank keeper func TestBlackListedAddrs(t *testing.T) { db := dbm.NewMemDB() - app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, 0) + app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, 0) for acc := range maccPerms { require.Equal(t, !allowedReceivingModAcc[acc], app.BankKeeper.BlacklistedAddr(app.SupplyKeeper.GetModuleAddress(acc))) diff --git a/simapp/sim_bench_test.go b/simapp/sim_bench_test.go index 2b6c486bf9..8e10ccd465 100644 --- a/simapp/sim_bench_test.go +++ b/simapp/sim_bench_test.go @@ -25,7 +25,7 @@ func BenchmarkFullAppSimulation(b *testing.B) { } }() - app := NewSimApp(logger, db, nil, true, FlagPeriodValue, interBlockCacheOpt()) + app := NewSimApp(logger, db, nil, true, map[int64]bool{}, FlagPeriodValue, interBlockCacheOpt()) // run randomized simulation _, simParams, simErr := simulation.SimulateFromSeed( @@ -64,7 +64,7 @@ func BenchmarkInvariants(b *testing.B) { } }() - app := NewSimApp(logger, db, nil, true, FlagPeriodValue, interBlockCacheOpt()) + app := NewSimApp(logger, db, nil, true, map[int64]bool{}, FlagPeriodValue, interBlockCacheOpt()) // run randomized simulation _, simParams, simErr := simulation.SimulateFromSeed( diff --git a/simapp/sim_test.go b/simapp/sim_test.go index 7738b29a42..3d41fa3b84 100644 --- a/simapp/sim_test.go +++ b/simapp/sim_test.go @@ -62,7 +62,7 @@ func TestFullAppSimulation(t *testing.T) { require.NoError(t, os.RemoveAll(dir)) }() - app := NewSimApp(logger, db, nil, true, FlagPeriodValue, fauxMerkleModeOpt) + app := NewSimApp(logger, db, nil, true, map[int64]bool{}, FlagPeriodValue, fauxMerkleModeOpt) require.Equal(t, "SimApp", app.Name()) // run randomized simulation @@ -94,7 +94,7 @@ func TestAppImportExport(t *testing.T) { require.NoError(t, os.RemoveAll(dir)) }() - app := NewSimApp(logger, db, nil, true, FlagPeriodValue, fauxMerkleModeOpt) + app := NewSimApp(logger, db, nil, true, map[int64]bool{}, FlagPeriodValue, fauxMerkleModeOpt) require.Equal(t, "SimApp", app.Name()) // Run randomized simulation @@ -128,7 +128,7 @@ func TestAppImportExport(t *testing.T) { require.NoError(t, os.RemoveAll(newDir)) }() - newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, FlagPeriodValue, fauxMerkleModeOpt) + newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, map[int64]bool{}, FlagPeriodValue, fauxMerkleModeOpt) require.Equal(t, "SimApp", newApp.Name()) var genesisState GenesisState @@ -180,7 +180,7 @@ func TestAppSimulationAfterImport(t *testing.T) { require.NoError(t, os.RemoveAll(dir)) }() - app := NewSimApp(logger, db, nil, true, FlagPeriodValue, fauxMerkleModeOpt) + app := NewSimApp(logger, db, nil, true, map[int64]bool{}, FlagPeriodValue, fauxMerkleModeOpt) require.Equal(t, "SimApp", app.Name()) // Run randomized simulation @@ -219,7 +219,7 @@ func TestAppSimulationAfterImport(t *testing.T) { require.NoError(t, os.RemoveAll(newDir)) }() - newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, FlagPeriodValue, fauxMerkleModeOpt) + newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, map[int64]bool{}, FlagPeriodValue, fauxMerkleModeOpt) require.Equal(t, "SimApp", newApp.Name()) newApp.InitChain(abci.RequestInitChain{ @@ -265,7 +265,7 @@ func TestAppStateDeterminism(t *testing.T) { db := dbm.NewMemDB() - app := NewSimApp(logger, db, nil, true, FlagPeriodValue, interBlockCacheOpt()) + app := NewSimApp(logger, db, nil, true, map[int64]bool{}, FlagPeriodValue, interBlockCacheOpt()) fmt.Printf( "running non-determinism simulation; seed %d: %d/%d, attempt: %d/%d\n", diff --git a/simapp/test_helpers.go b/simapp/test_helpers.go index d132394c9b..f06af61948 100644 --- a/simapp/test_helpers.go +++ b/simapp/test_helpers.go @@ -23,7 +23,7 @@ import ( // 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, 0) + app := NewSimApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, 0) if !isCheckTx { // init chain must be called to stop deliverState from being nil genesisState := NewDefaultGenesisState() @@ -48,7 +48,7 @@ func Setup(isCheckTx bool) *SimApp { // genesis accounts. func SetupWithGenesisAccounts(genAccs []authexported.GenesisAccount) *SimApp { db := dbm.NewMemDB() - app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, 0) + app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, 0) // initialize the chain with the passed in genesis accounts genesisState := NewDefaultGenesisState() diff --git a/x/crisis/handler_test.go b/x/crisis/handler_test.go index f4c2f8182d..290b0c373c 100644 --- a/x/crisis/handler_test.go +++ b/x/crisis/handler_test.go @@ -25,7 +25,7 @@ var ( func createTestApp() (*simapp.SimApp, sdk.Context, []sdk.AccAddress) { db := dbm.NewMemDB() - app := simapp.NewSimApp(log.NewNopLogger(), db, nil, true, 1) + app := simapp.NewSimApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, 1) ctx := app.NewContext(true, abci.Header{}) constantFee := sdk.NewInt64Coin(sdk.DefaultBondDenom, 10) diff --git a/x/crisis/internal/keeper/integration_test.go b/x/crisis/internal/keeper/integration_test.go index 339244325e..a98a0b4453 100644 --- a/x/crisis/internal/keeper/integration_test.go +++ b/x/crisis/internal/keeper/integration_test.go @@ -11,7 +11,7 @@ import ( func createTestApp() *simapp.SimApp { db := dbm.NewMemDB() - app := simapp.NewSimApp(log.NewNopLogger(), db, nil, true, 5) + app := simapp.NewSimApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, 5) // init chain must be called to stop deliverState from being nil genesisState := simapp.NewDefaultGenesisState() stateBytes, err := codec.MarshalJSONIndent(app.Codec(), genesisState) diff --git a/x/upgrade/abci.go b/x/upgrade/abci.go index 01b0aae6a0..7a19e0f866 100644 --- a/x/upgrade/abci.go +++ b/x/upgrade/abci.go @@ -2,37 +2,47 @@ package upgrade import ( "fmt" - abci "github.com/tendermint/tendermint/abci/types" sdk "github.com/cosmos/cosmos-sdk/types" ) // BeginBlock will check if there is a scheduled plan and if it is ready to be executed. +// If the current height is in the provided set of heights to skip, it will skip and clear the upgrade plan. // If it is ready, it will execute it if the handler is installed, and panic/abort otherwise. // If the plan is not ready, it will ensure the handler is not registered too early (and abort otherwise). // -// The purpose is to ensure the binary is switch EXACTLY at the desired block, and to allow +// The purpose is to ensure the binary is switched EXACTLY at the desired block, and to allow // a migration to be executed if needed upon this switch (migration defined in the new binary) +// skipUpgradeHeightArray is a set of block heights for which the upgrade must be skipped func BeginBlocker(k Keeper, ctx sdk.Context, _ abci.RequestBeginBlock) { plan, found := k.GetUpgradePlan(ctx) if !found { return } + // To make sure clear upgrade is executed at the same block if plan.ShouldExecute(ctx) { + // If skip upgrade has been set for current height, we clear the upgrade plan + if k.IsSkipHeight(ctx.BlockHeight()) { + skipUpgradeMsg := fmt.Sprintf("UPGRADE \"%s\" SKIPPED at %d: %s", plan.Name, plan.Height, plan.Info) + ctx.Logger().Info(skipUpgradeMsg) + + // Clear the upgrade plan at current height + k.ClearUpgradePlan(ctx) + return + } + if !k.HasHandler(plan.Name) { upgradeMsg := fmt.Sprintf("UPGRADE \"%s\" NEEDED at %s: %s", plan.Name, plan.DueAt(), plan.Info) // We don't have an upgrade handler for this upgrade name, meaning this software is out of date so shutdown ctx.Logger().Error(upgradeMsg) panic(upgradeMsg) } - // We have an upgrade handler for this upgrade name, so apply the upgrade ctx.Logger().Info(fmt.Sprintf("applying upgrade \"%s\" at %s", plan.Name, plan.DueAt())) ctx = ctx.WithBlockGasMeter(sdk.NewInfiniteGasMeter()) k.ApplyUpgrade(ctx, plan) - return } diff --git a/x/upgrade/abci_test.go b/x/upgrade/abci_test.go index 34c8944e00..c82fe757a5 100644 --- a/x/upgrade/abci_test.go +++ b/x/upgrade/abci_test.go @@ -5,19 +5,21 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/libs/log" + dbm "github.com/tendermint/tm-db" + + "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/simapp" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/cosmos-sdk/x/gov" "github.com/cosmos/cosmos-sdk/x/upgrade" - "github.com/stretchr/testify/suite" - abci "github.com/tendermint/tendermint/abci/types" ) type TestSuite struct { - suite.Suite - module module.AppModule keeper upgrade.Keeper querier sdk.Querier @@ -25,163 +27,369 @@ type TestSuite struct { ctx sdk.Context } -func (s *TestSuite) SetupTest() { - checkTx := false - app := simapp.Setup(checkTx) +var s TestSuite + +func setupTest(height int64, skip map[int64]bool) TestSuite { + db := dbm.NewMemDB() + app := simapp.NewSimApp(log.NewNopLogger(), db, nil, true, skip, 0) + genesisState := simapp.NewDefaultGenesisState() + stateBytes, err := codec.MarshalJSONIndent(app.Codec(), genesisState) + if err != nil { + panic(err) + } + app.InitChain( + abci.RequestInitChain{ + Validators: []abci.ValidatorUpdate{}, + AppStateBytes: stateBytes, + }, + ) s.keeper = app.UpgradeKeeper - s.handler = upgrade.NewSoftwareUpgradeProposalHandler(s.keeper) - s.querier = upgrade.NewQuerier(s.keeper) - s.ctx = app.BaseApp.NewContext(checkTx, abci.Header{Height: 10, Time: time.Now()}) + s.ctx = app.BaseApp.NewContext(false, abci.Header{Height: height, Time: time.Now()}) + s.module = upgrade.NewAppModule(s.keeper) + s.querier = s.module.NewQuerierHandler() + s.handler = upgrade.NewSoftwareUpgradeProposalHandler(s.keeper) + return s } -func (s *TestSuite) TestRequireName() { +func TestRequireName(t *testing.T) { + s := setupTest(10, map[int64]bool{}) + err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{}}) - s.Require().NotNil(err) - s.Require().True(errors.Is(sdkerrors.ErrInvalidRequest, err), err) + require.NotNil(t, err) + require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err) } -func (s *TestSuite) TestRequireFutureTime() { +func TestRequireFutureTime(t *testing.T) { + s := setupTest(10, map[int64]bool{}) err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Time: s.ctx.BlockHeader().Time}}) - s.Require().NotNil(err) - s.Require().True(errors.Is(sdkerrors.ErrInvalidRequest, err), err) + require.NotNil(t, err) + require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err) } -func (s *TestSuite) TestRequireFutureBlock() { +func TestRequireFutureBlock(t *testing.T) { + s := setupTest(10, map[int64]bool{}) err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: s.ctx.BlockHeight()}}) - s.Require().NotNil(err) - s.Require().True(errors.Is(sdkerrors.ErrInvalidRequest, err), err) + require.NotNil(t, err) + require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err) } -func (s *TestSuite) TestCantSetBothTimeAndHeight() { +func TestCantSetBothTimeAndHeight(t *testing.T) { + s := setupTest(10, map[int64]bool{}) err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Time: time.Now(), Height: s.ctx.BlockHeight() + 1}}) - s.Require().NotNil(err) - s.Require().True(errors.Is(sdkerrors.ErrInvalidRequest, err), err) + require.NotNil(t, err) + require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err) } -func (s *TestSuite) TestDoTimeUpgrade() { - s.T().Log("Verify can schedule an upgrade") +func TestDoTimeUpgrade(t *testing.T) { + s := setupTest(10, map[int64]bool{}) + t.Log("Verify can schedule an upgrade") err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Time: time.Now()}}) - s.Require().Nil(err) + require.Nil(t, err) - s.VerifyDoUpgrade() + VerifyDoUpgrade(t) } -func (s *TestSuite) TestDoHeightUpgrade() { - s.T().Log("Verify can schedule an upgrade") +func TestDoHeightUpgrade(t *testing.T) { + s := setupTest(10, map[int64]bool{}) + t.Log("Verify can schedule an upgrade") err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}}) - s.Require().Nil(err) + require.Nil(t, err) - s.VerifyDoUpgrade() + VerifyDoUpgrade(t) } -func (s *TestSuite) TestCanOverwriteScheduleUpgrade() { - s.T().Log("Can overwrite plan") +func TestCanOverwriteScheduleUpgrade(t *testing.T) { + s := setupTest(10, map[int64]bool{}) + t.Log("Can overwrite plan") err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "bad_test", Height: s.ctx.BlockHeight() + 10}}) - s.Require().Nil(err) + require.Nil(t, err) err = s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}}) - s.Require().Nil(err) + require.Nil(t, err) - s.VerifyDoUpgrade() + VerifyDoUpgrade(t) } -func (s *TestSuite) VerifyDoUpgrade() { - s.T().Log("Verify that a panic happens at the upgrade time/height") +func VerifyDoUpgrade(t *testing.T) { + t.Log("Verify that a panic happens at the upgrade time/height") newCtx := s.ctx.WithBlockHeight(s.ctx.BlockHeight() + 1).WithBlockTime(time.Now()) req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()} - s.Require().Panics(func() { + require.Panics(t, func() { s.module.BeginBlock(newCtx, req) }) - s.T().Log("Verify that the upgrade can be successfully applied with a handler") + t.Log("Verify that the upgrade can be successfully applied with a handler") s.keeper.SetUpgradeHandler("test", func(ctx sdk.Context, plan upgrade.Plan) {}) - s.Require().NotPanics(func() { + require.NotPanics(t, func() { s.module.BeginBlock(newCtx, req) }) - s.VerifyCleared(newCtx) + VerifyCleared(t, newCtx) } -func (s *TestSuite) TestHaltIfTooNew() { - s.T().Log("Verify that we don't panic with registered plan not in database at all") +func VerifyDoUpgradeWithCtx(t *testing.T, newCtx sdk.Context, proposalName string) { + t.Log("Verify that a panic happens at the upgrade time/height") + req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()} + require.Panics(t, func() { + s.module.BeginBlock(newCtx, req) + }) + + t.Log("Verify that the upgrade can be successfully applied with a handler") + s.keeper.SetUpgradeHandler(proposalName, func(ctx sdk.Context, plan upgrade.Plan) {}) + require.NotPanics(t, func() { + s.module.BeginBlock(newCtx, req) + }) + + VerifyCleared(t, newCtx) +} + +func TestHaltIfTooNew(t *testing.T) { + s := setupTest(10, map[int64]bool{}) + t.Log("Verify that we don't panic with registered plan not in database at all") var called int s.keeper.SetUpgradeHandler("future", func(ctx sdk.Context, plan upgrade.Plan) { called++ }) newCtx := s.ctx.WithBlockHeight(s.ctx.BlockHeight() + 1).WithBlockTime(time.Now()) req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()} - s.Require().NotPanics(func() { + require.NotPanics(t, func() { s.module.BeginBlock(newCtx, req) }) - s.Require().Equal(0, called) + require.Equal(t, 0, called) - s.T().Log("Verify we panic if we have a registered handler ahead of time") + t.Log("Verify we panic if we have a registered handler ahead of time") err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "future", Height: s.ctx.BlockHeight() + 3}}) - s.Require().NoError(err) - s.Require().Panics(func() { + require.NoError(t, err) + require.Panics(t, func() { s.module.BeginBlock(newCtx, req) }) - s.Require().Equal(0, called) + require.Equal(t, 0, called) - s.T().Log("Verify we no longer panic if the plan is on time") + t.Log("Verify we no longer panic if the plan is on time") futCtx := s.ctx.WithBlockHeight(s.ctx.BlockHeight() + 3).WithBlockTime(time.Now()) req = abci.RequestBeginBlock{Header: futCtx.BlockHeader()} - s.Require().NotPanics(func() { + require.NotPanics(t, func() { s.module.BeginBlock(futCtx, req) }) - s.Require().Equal(1, called) + require.Equal(t, 1, called) - s.VerifyCleared(futCtx) + VerifyCleared(t, futCtx) } -func (s *TestSuite) VerifyCleared(newCtx sdk.Context) { - s.T().Log("Verify that the upgrade plan has been cleared") +func VerifyCleared(t *testing.T, newCtx sdk.Context) { + t.Log("Verify that the upgrade plan has been cleared") bz, err := s.querier(newCtx, []string{upgrade.QueryCurrent}, abci.RequestQuery{}) - s.Require().NoError(err) - s.Require().Nil(bz) + require.NoError(t, err) + require.Nil(t, bz) } -func (s *TestSuite) TestCanClear() { - s.T().Log("Verify upgrade is scheduled") +func TestCanClear(t *testing.T) { + s := setupTest(10, map[int64]bool{}) + t.Log("Verify upgrade is scheduled") err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Time: time.Now()}}) - s.Require().Nil(err) + require.Nil(t, err) - s.handler(s.ctx, upgrade.CancelSoftwareUpgradeProposal{Title: "cancel"}) + err = s.handler(s.ctx, upgrade.CancelSoftwareUpgradeProposal{Title: "cancel"}) + require.Nil(t, err) - s.VerifyCleared(s.ctx) + VerifyCleared(t, s.ctx) } -func (s *TestSuite) TestCantApplySameUpgradeTwice() { - s.TestDoTimeUpgrade() - s.T().Log("Verify an upgrade named \"test\" can't be scheduled twice") +func TestCantApplySameUpgradeTwice(t *testing.T) { + s := setupTest(10, map[int64]bool{}) err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Time: time.Now()}}) - s.Require().NotNil(err) - s.Require().True(errors.Is(sdkerrors.ErrInvalidRequest, err), err) + require.Nil(t, err) + VerifyDoUpgrade(t) + t.Log("Verify an executed upgrade \"test\" can't be rescheduled") + err = s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Time: time.Now()}}) + require.NotNil(t, err) + require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err) } -func (s *TestSuite) TestNoSpuriousUpgrades() { - s.T().Log("Verify that no upgrade panic is triggered in the BeginBlocker when we haven't scheduled an upgrade") +func TestNoSpuriousUpgrades(t *testing.T) { + s := setupTest(10, map[int64]bool{}) + t.Log("Verify that no upgrade panic is triggered in the BeginBlocker when we haven't scheduled an upgrade") req := abci.RequestBeginBlock{Header: s.ctx.BlockHeader()} - s.Require().NotPanics(func() { + require.NotPanics(t, func() { s.module.BeginBlock(s.ctx, req) }) } -func (s *TestSuite) TestPlanStringer() { - t, err := time.Parse(time.RFC3339, "2020-01-01T00:00:00Z") - s.Require().Nil(err) - s.Require().Equal(`Upgrade Plan +func TestPlanStringer(t *testing.T) { + ti, err := time.Parse(time.RFC3339, "2020-01-01T00:00:00Z") + require.Nil(t, err) + require.Equal(t, `Upgrade Plan Name: test Time: 2020-01-01T00:00:00Z - Info: `, upgrade.Plan{Name: "test", Time: t}.String()) - s.Require().Equal(`Upgrade Plan + Info: `, upgrade.Plan{Name: "test", Time: ti}.String()) + require.Equal(t, `Upgrade Plan Name: test Height: 100 Info: `, upgrade.Plan{Name: "test", Height: 100}.String()) } -func TestTestSuite(t *testing.T) { - suite.Run(t, new(TestSuite)) +func VerifyNotDone(t *testing.T, newCtx sdk.Context, name string) { + t.Log("Verify that upgrade was not done") + height := s.keeper.GetDoneHeight(newCtx, name) + require.Zero(t, height) +} + +func VerifyDone(t *testing.T, newCtx sdk.Context, name string) { + t.Log("Verify that the upgrade plan has been executed") + height := s.keeper.GetDoneHeight(newCtx, name) + require.NotZero(t, height) +} + +func VerifySet(t *testing.T, skipUpgradeHeights map[int64]bool) { + t.Log("Verify if the skip upgrade has been set") + + for k := range skipUpgradeHeights { + require.True(t, s.keeper.IsSkipHeight(k)) + } +} + +func TestContains(t *testing.T) { + var ( + skipOne int64 = 11 + ) + s := setupTest(10, map[int64]bool{skipOne: true}) + + VerifySet(t, map[int64]bool{skipOne: true}) + t.Log("case where array contains the element") + require.True(t, s.keeper.IsSkipHeight(11)) + + t.Log("case where array doesn't contain the element") + require.False(t, s.keeper.IsSkipHeight(4)) +} + +func TestSkipUpgradeSkippingAll(t *testing.T) { + var ( + skipOne int64 = 11 + skipTwo int64 = 20 + ) + s := setupTest(10, map[int64]bool{skipOne: true, skipTwo: true}) + + newCtx := s.ctx + + req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()} + err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: skipOne}}) + require.NoError(t, err) + + t.Log("Verify if skip upgrade flag clears upgrade plan in both cases") + VerifySet(t, map[int64]bool{skipOne: true, skipTwo: true}) + + newCtx = newCtx.WithBlockHeight(skipOne) + require.NotPanics(t, func() { + s.module.BeginBlock(newCtx, req) + }) + + t.Log("Verify a second proposal also is being cleared") + err = s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop2", Plan: upgrade.Plan{Name: "test2", Height: skipTwo}}) + require.NoError(t, err) + + newCtx = newCtx.WithBlockHeight(skipTwo) + require.NotPanics(t, func() { + s.module.BeginBlock(newCtx, req) + }) + + // To ensure verification is being done only after both upgrades are cleared + t.Log("Verify if both proposals are cleared") + VerifyCleared(t, s.ctx) + VerifyNotDone(t, s.ctx, "test") + VerifyNotDone(t, s.ctx, "test2") +} + +func TestUpgradeSkippingOne(t *testing.T) { + var ( + skipOne int64 = 11 + skipTwo int64 = 20 + ) + s := setupTest(10, map[int64]bool{skipOne: true}) + + newCtx := s.ctx + + req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()} + err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: skipOne}}) + require.Nil(t, err) + + t.Log("Verify if skip upgrade flag clears upgrade plan in one case and does upgrade on another") + VerifySet(t, map[int64]bool{skipOne: true}) + + // Setting block height of proposal test + newCtx = newCtx.WithBlockHeight(skipOne) + require.NotPanics(t, func() { + s.module.BeginBlock(newCtx, req) + }) + + t.Log("Verify the second proposal is not skipped") + err = s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop2", Plan: upgrade.Plan{Name: "test2", Height: skipTwo}}) + require.Nil(t, err) + // Setting block height of proposal test2 + newCtx = newCtx.WithBlockHeight(skipTwo) + VerifyDoUpgradeWithCtx(t, newCtx, "test2") + + t.Log("Verify first proposal is cleared and second is done") + VerifyNotDone(t, s.ctx, "test") + VerifyDone(t, s.ctx, "test2") +} + +func TestUpgradeSkippingOnlyTwo(t *testing.T) { + var ( + skipOne int64 = 11 + skipTwo int64 = 20 + skipThree int64 = 25 + ) + s := setupTest(10, map[int64]bool{skipOne: true, skipTwo: true}) + + newCtx := s.ctx + + req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()} + err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: skipOne}}) + require.Nil(t, err) + + t.Log("Verify if skip upgrade flag clears upgrade plan in both cases and does third upgrade") + VerifySet(t, map[int64]bool{skipOne: true, skipTwo: true}) + + // Setting block height of proposal test + newCtx = newCtx.WithBlockHeight(skipOne) + require.NotPanics(t, func() { + s.module.BeginBlock(newCtx, req) + }) + + // A new proposal with height in skipUpgradeHeights + err = s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop2", Plan: upgrade.Plan{Name: "test2", Height: skipTwo}}) + require.Nil(t, err) + // Setting block height of proposal test2 + newCtx = newCtx.WithBlockHeight(skipTwo) + require.NotPanics(t, func() { + s.module.BeginBlock(newCtx, req) + }) + + t.Log("Verify a new proposal is not skipped") + err = s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop3", Plan: upgrade.Plan{Name: "test3", Height: skipThree}}) + require.Nil(t, err) + newCtx = newCtx.WithBlockHeight(skipThree) + VerifyDoUpgradeWithCtx(t, newCtx, "test3") + + t.Log("Verify two proposals are cleared and third is done") + VerifyNotDone(t, s.ctx, "test") + VerifyNotDone(t, s.ctx, "test2") + VerifyDone(t, s.ctx, "test3") +} + +func TestUpgradeWithoutSkip(t *testing.T) { + s := setupTest(10, map[int64]bool{}) + newCtx := s.ctx.WithBlockHeight(s.ctx.BlockHeight() + 1).WithBlockTime(time.Now()) + req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()} + err := s.handler(s.ctx, upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}}) + require.Nil(t, err) + t.Log("Verify if upgrade happens without skip upgrade") + require.Panics(t, func() { + s.module.BeginBlock(newCtx, req) + }) + + VerifyDoUpgrade(t) + VerifyDone(t, s.ctx, "test") } diff --git a/x/upgrade/internal/keeper/keeper.go b/x/upgrade/internal/keeper/keeper.go index e0c0823b08..7a33d7e4b6 100644 --- a/x/upgrade/internal/keeper/keeper.go +++ b/x/upgrade/internal/keeper/keeper.go @@ -4,26 +4,29 @@ import ( "encoding/binary" "fmt" + "github.com/tendermint/tendermint/libs/log" + "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/upgrade/internal/types" - "github.com/tendermint/tendermint/libs/log" ) type Keeper struct { - storeKey sdk.StoreKey - cdc *codec.Codec - upgradeHandlers map[string]types.UpgradeHandler + skipUpgradeHeights map[int64]bool + storeKey sdk.StoreKey + cdc *codec.Codec + upgradeHandlers map[string]types.UpgradeHandler } // NewKeeper constructs an upgrade Keeper -func NewKeeper(storeKey sdk.StoreKey, cdc *codec.Codec) Keeper { +func NewKeeper(skipUpgradeHeights map[int64]bool, storeKey sdk.StoreKey, cdc *codec.Codec) Keeper { return Keeper{ - storeKey: storeKey, - cdc: cdc, - upgradeHandlers: map[string]types.UpgradeHandler{}, + skipUpgradeHeights: skipUpgradeHeights, + storeKey: storeKey, + cdc: cdc, + upgradeHandlers: map[string]types.UpgradeHandler{}, } } @@ -50,7 +53,7 @@ func (k Keeper) ScheduleUpgrade(ctx sdk.Context, plan types.Plan) error { return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "upgrade cannot be scheduled in the past") } - if k.getDoneHeight(ctx, plan.Name) != 0 { + if k.GetDoneHeight(ctx, plan.Name) != 0 { return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "upgrade with name %s has already been completed", plan.Name) } @@ -61,7 +64,8 @@ func (k Keeper) ScheduleUpgrade(ctx sdk.Context, plan types.Plan) error { return nil } -func (k Keeper) getDoneHeight(ctx sdk.Context, name string) int64 { +// GetDoneHeight returns the height at which the given upgrade was executed +func (k Keeper) GetDoneHeight(ctx sdk.Context, name string) int64 { store := prefix.NewStore(ctx.KVStore(k.storeKey), []byte{types.DoneByte}) bz := store.Get([]byte(name)) if len(bz) == 0 { @@ -121,3 +125,8 @@ func (k Keeper) ApplyUpgrade(ctx sdk.Context, plan types.Plan) { k.ClearUpgradePlan(ctx) k.setDone(ctx, plan.Name) } + +// IsSkipHeight checks if the given height is part of skipUpgradeHeights +func (k Keeper) IsSkipHeight(height int64) bool { + return k.skipUpgradeHeights[height] +} diff --git a/x/upgrade/internal/keeper/querier.go b/x/upgrade/internal/keeper/querier.go index 164a2340c0..811acad230 100644 --- a/x/upgrade/internal/keeper/querier.go +++ b/x/upgrade/internal/keeper/querier.go @@ -48,7 +48,7 @@ func queryApplied(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]byte, err return nil, sdkerrors.Wrap(sdkerrors.ErrJSONUnmarshal, err.Error()) } - applied := k.getDoneHeight(ctx, params.Name) + applied := k.GetDoneHeight(ctx, params.Name) if applied == 0 { return nil, nil }