From 6afaa2e508330c79f9009c77d1314dd578dd248d Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 23 May 2022 14:41:51 -0400 Subject: [PATCH] fix: state-sync - app version stored in params (#241) (#243) Closes: #XXX ## What is the purpose of the change Backporting and expanding on: https://github.com/cosmos/cosmos-sdk/pull/11182 This is an attempt to fix app version bug related to state-sync and tendermit. More context is in the linked-above PR. Upon backporting all relevant commits from the linked-above PR, it was discovered that some integration and simulation tests were failing. With the original design, it was difficult to find the source of the problem due to storing the protocol version in 2 database locations. Therefore, the original work was significantly redesigned to only keep the protocol version in param store. Additionally, the protocol version was assumed to have already been set. Since we are creating a new entry in the database, it is now possible that the protocol version is non-existent at the start. As a result, mechanisms for detecting a missing protocol version were added: - `baseapp.init(...)` * This is called on every restart. Here, we check if the protocol version is already in db. If not, set it to 0 - `baseapp.InitChain(...)` * This is called once at genesis. Here, we always set protocol version to 0 Then, on every upgrade the version gets incremented by 1 ## Brief Changelog - cherry-picked all relevant commits from #11182 - fixed some issues - addressed some style comments from code review in #11182 - redesigned to only store protocol version in params store and removed logic for storing it in the upgrade keeper - added logic for detecting missing protocol version and, if missing, setting it to 0 - unit and integration tested - fixed all tests ## Testing and Verifying This change is a trivial rework / code cleanup without any test coverage. This change is already covered by existing tests, such as *(please describe tests)*. TODO: This change needs to be manually tested ## Documentation and Release Note - Does this pull request introduce a new feature or user-facing behavior changes? yes - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? no - How is the feature or change documented? not applicable (cherry picked from commit a68d28e84de85ee65cd6432963fc29a76a56c543) Co-authored-by: Roman --- baseapp/abci.go | 12 ++- baseapp/abci_test.go | 3 +- baseapp/baseapp.go | 37 +++++--- baseapp/baseapp_test.go | 133 +++++++++++++++++++--------- baseapp/options.go | 37 +++++++- baseapp/params.go | 12 +++ server/mock/app.go | 3 + simapp/app_test.go | 2 + simapp/test_helpers.go | 3 + testutil/mock/param_store.go | 45 ++++++++++ x/params/keeper/consensus_params.go | 5 ++ x/simulation/params.go | 3 + x/upgrade/exported/exported.go | 10 ++- x/upgrade/keeper/keeper.go | 45 +++------- x/upgrade/keeper/keeper_test.go | 12 +-- x/upgrade/keeper/util_test.go | 8 ++ x/upgrade/spec/02_state.md | 4 +- x/upgrade/types/keys.go | 4 +- x/upgrade/types/storeloader_test.go | 3 + 19 files changed, 276 insertions(+), 105 deletions(-) create mode 100644 testutil/mock/param_store.go create mode 100644 x/upgrade/keeper/util_test.go diff --git a/baseapp/abci.go b/baseapp/abci.go index 1809bd70b5b..3ef208d5342 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -45,6 +45,10 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC app.setDeliverState(initHeader) app.setCheckState(initHeader) + if err := app.SetAppVersion(app.deliverState.ctx, 0); err != nil { + panic(err) + } + // Store the consensus params in the BaseApp's paramstore. Note, this must be // done after the deliver state and context have been set as it's persisted // to state. @@ -107,10 +111,15 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC func (app *BaseApp) Info(req abci.RequestInfo) abci.ResponseInfo { lastCommitID := app.cms.LastCommitID() + appVersion, err := app.GetAppVersion(app.checkState.ctx) + if err != nil { + app.logger.Error("failed to get app version", err) + } + return abci.ResponseInfo{ Data: app.name, Version: app.version, - AppVersion: app.appVersion, + AppVersion: appVersion, LastBlockHeight: lastCommitID.Version, LastBlockAppHash: lastCommitID.Hash, } @@ -742,6 +751,7 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) abci.Res } case "version": + return abci.ResponseQuery{ Codespace: sdkerrors.RootCodespace, Height: req.Height, diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 25e477217ac..ead6c5e68fb 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -12,6 +12,7 @@ import ( pruningtypes "github.com/cosmos/cosmos-sdk/pruning/types" "github.com/cosmos/cosmos-sdk/snapshots" snapshottypes "github.com/cosmos/cosmos-sdk/snapshots/types" + "github.com/cosmos/cosmos-sdk/testutil/mock" snaphotstestutil "github.com/cosmos/cosmos-sdk/testutil/snapshots" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -110,7 +111,7 @@ func TestGetBlockRentionHeight(t *testing.T) { for name, tc := range testCases { tc := tc - tc.bapp.SetParamStore(¶mStore{db: dbm.NewMemDB()}) + tc.bapp.SetParamStore(&mock.ParamStore{Db: dbm.NewMemDB()}) tc.bapp.InitChain(abci.RequestInitChain{ ConsensusParams: &abci.ConsensusParams{ Evidence: &tmprototypes.EvidenceParams{ diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 38a27ac8be7..177cb59801b 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -1,6 +1,7 @@ package baseapp import ( + "errors" "fmt" "reflect" "strings" @@ -19,6 +20,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx" + upgrade "github.com/cosmos/cosmos-sdk/x/upgrade/exported" ) const ( @@ -114,13 +116,9 @@ type BaseApp struct { // nolint: maligned // ResponseCommit.RetainHeight. minRetainBlocks uint64 - // application's version string + // version represents the application software semantic version version string - // application's protocol version that increments on every upgrade - // if BaseApp is passed to the upgrade keeper's NewKeeper method. - appVersion uint64 - // recovery handler for app.runTx method runTxRecoveryMiddleware recoveryMiddleware @@ -132,6 +130,8 @@ type BaseApp struct { // nolint: maligned indexEvents map[string]struct{} } +var _ upgrade.ProtocolVersionManager = (*BaseApp)(nil) + // NewBaseApp returns a reference to an initialized BaseApp. It accepts a // variadic number of option functions, which act on the BaseApp to set // configuration choices. @@ -172,11 +172,6 @@ func (app *BaseApp) Name() string { return app.name } -// AppVersion returns the application's protocol version. -func (app *BaseApp) AppVersion() uint64 { - return app.appVersion -} - // Version returns the application's version string. func (app *BaseApp) Version() string { return app.version @@ -314,6 +309,18 @@ func (app *BaseApp) init() error { // needed for the export command which inits from store but never calls initchain app.setCheckState(tmproto.Header{}) + + // If there is no app version set in the store, we should set it to 0. + // Panic on any other error. + // If errMsgNoProtocolVersionSet, we assume that appVersion is assigned to be 0. + appVersion, err := app.GetAppVersion(app.checkState.ctx) + if err != nil && !errors.Is(err, errMsgNoProtocolVersionSet) { + return err + } + + if err := app.SetAppVersion(app.checkState.ctx, appVersion); err != nil { + return err + } app.Seal() rms, ok := app.cms.(*rootmulti.Store) @@ -429,6 +436,13 @@ func (app *BaseApp) GetConsensusParams(ctx sdk.Context) *abci.ConsensusParams { cp.Validator = &vp } + if app.paramStore.Has(ctx, ParamStoreKeyVersionParams) { + var vp tmproto.VersionParams + + app.paramStore.Get(ctx, ParamStoreKeyVersionParams, &vp) + cp.Version = &vp + } + return cp } @@ -452,6 +466,9 @@ func (app *BaseApp) StoreConsensusParams(ctx sdk.Context, cp *abci.ConsensusPara app.paramStore.Set(ctx, ParamStoreKeyBlockParams, cp.Block) app.paramStore.Set(ctx, ParamStoreKeyEvidenceParams, cp.Evidence) app.paramStore.Set(ctx, ParamStoreKeyValidatorParams, cp.Validator) + app.paramStore.Set(ctx, ParamStoreKeyVersionParams, cp.Version) + // We're explicitly not storing the Tendermint app_version in the param store. It's + // stored instead in the x/upgrade store, with its own bump logic. } // getMaximumBlockGas gets the maximum gas from the consensus params. It panics diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 8693e62dc57..3bd9d0e846b 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -3,7 +3,6 @@ package baseapp import ( "bytes" "encoding/binary" - "encoding/json" "fmt" "io/ioutil" "math/rand" @@ -27,6 +26,7 @@ import ( snapshottypes "github.com/cosmos/cosmos-sdk/snapshots/types" "github.com/cosmos/cosmos-sdk/store/rootmulti" storetypes "github.com/cosmos/cosmos-sdk/store/types" + "github.com/cosmos/cosmos-sdk/testutil/mock" snaphotstestutil "github.com/cosmos/cosmos-sdk/testutil/snapshots" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" @@ -39,10 +39,6 @@ var ( capKey2 = sdk.NewKVStoreKey("key2") ) -type paramStore struct { - db *dbm.MemDB -} - type setupConfig struct { blocks uint64 blockTxs int @@ -51,39 +47,6 @@ type setupConfig struct { pruningOpts pruningtypes.PruningOptions } -func (ps *paramStore) Set(_ sdk.Context, key []byte, value interface{}) { - bz, err := json.Marshal(value) - if err != nil { - panic(err) - } - - ps.db.Set(key, bz) -} - -func (ps *paramStore) Has(_ sdk.Context, key []byte) bool { - ok, err := ps.db.Has(key) - if err != nil { - panic(err) - } - - return ok -} - -func (ps *paramStore) Get(_ sdk.Context, key []byte, ptr interface{}) { - bz, err := ps.db.Get(key) - if err != nil { - panic(err) - } - - if len(bz) == 0 { - return - } - - if err := json.Unmarshal(bz, ptr); err != nil { - panic(err) - } -} - func defaultLogger() log.Logger { return log.NewTMLogger(log.NewSyncWriter(os.Stdout)).With("module", "sdk/app") } @@ -122,7 +85,7 @@ func setupBaseApp(t *testing.T, options ...func(*BaseApp)) (*BaseApp, error) { require.Equal(t, t.Name(), app.Name()) app.MountStores(capKey1, capKey2) - app.SetParamStore(¶mStore{db: dbm.NewMemDB()}) + app.SetParamStore(&mock.ParamStore{Db: dbm.NewMemDB()}) // stores are mounted err := app.LoadLatestVersion() @@ -218,6 +181,7 @@ func TestLoadVersion(t *testing.T) { db := dbm.NewMemDB() name := t.Name() app := NewBaseApp(name, logger, db, nil, pruningOpt) + app.SetParamStore(&mock.ParamStore{Db: db}) // make a cap key and mount the store err := app.LoadLatestVersion() // needed to make stores non-nil @@ -245,6 +209,7 @@ func TestLoadVersion(t *testing.T) { // reload with LoadLatestVersion app = NewBaseApp(name, logger, db, nil, pruningOpt) + app.SetParamStore(&mock.ParamStore{Db: db}) app.MountStores() err = app.LoadLatestVersion() require.Nil(t, err) @@ -253,6 +218,7 @@ func TestLoadVersion(t *testing.T) { // reload with LoadVersion, see if you can commit the same block and get // the same result app = NewBaseApp(name, logger, db, nil, pruningOpt) + app.SetParamStore(&mock.ParamStore{Db: db}) err = app.LoadVersion(1) require.Nil(t, err) testLoadVersionHelper(t, app, int64(1), commitID1) @@ -332,6 +298,7 @@ func TestSetLoader(t *testing.T) { opts = append(opts, tc.setLoader) } app := NewBaseApp(t.Name(), defaultLogger(), db, nil, opts...) + app.SetParamStore(&mock.ParamStore{Db: db}) app.MountStores(sdk.NewKVStoreKey(tc.loadStoreKey)) err := app.LoadLatestVersion() require.Nil(t, err) @@ -374,6 +341,7 @@ func TestLoadVersionInvalid(t *testing.T) { db := dbm.NewMemDB() name := t.Name() app := NewBaseApp(name, logger, db, nil, pruningOpt) + app.SetParamStore(&mock.ParamStore{Db: db}) err := app.LoadLatestVersion() require.Nil(t, err) @@ -389,6 +357,7 @@ func TestLoadVersionInvalid(t *testing.T) { // create a new app with the stores mounted under the same cap key app = NewBaseApp(name, logger, db, nil, pruningOpt) + app.SetParamStore(&mock.ParamStore{Db: db}) // require we can load the latest version err = app.LoadVersion(1) @@ -412,6 +381,7 @@ func TestLoadVersionPruning(t *testing.T) { snapshotOpt := SetSnapshot(snapshotStore, snapshottypes.NewSnapshotOptions(3, 1)) app := NewBaseApp(name, logger, db, nil, pruningOpt, snapshotOpt) + app.SetParamStore(&mock.ParamStore{Db: db}) // make a cap key and mount the store capKey := sdk.NewKVStoreKey("key1") @@ -450,6 +420,7 @@ func TestLoadVersionPruning(t *testing.T) { // reload with LoadLatestVersion, check it loads last version app = NewBaseApp(name, logger, db, nil, pruningOpt, snapshotOpt) + app.SetParamStore(&mock.ParamStore{Db: db}) app.MountStores(capKey) err = app.LoadLatestVersion() @@ -497,6 +468,8 @@ func TestTxDecoder(t *testing.T) { // Test that Info returns the latest committed state. func TestInfo(t *testing.T) { app := newBaseApp(t.Name()) + app.SetParamStore(&mock.ParamStore{Db: dbm.NewMemDB()}) + app.InitChain(abci.RequestInitChain{}) // ----- test an empty response ------- reqInfo := abci.RequestInfo{} @@ -507,9 +480,11 @@ func TestInfo(t *testing.T) { assert.Equal(t, t.Name(), res.GetData()) assert.Equal(t, int64(0), res.LastBlockHeight) require.Equal(t, []uint8(nil), res.LastBlockAppHash) - require.Equal(t, app.AppVersion(), res.AppVersion) - // ----- test a proper response ------- - // TODO + + appVersion, err := app.GetAppVersion(app.deliverState.ctx) + require.NoError(t, err) + + assert.Equal(t, appVersion, res.AppVersion) } func TestBaseAppOptionSeal(t *testing.T) { @@ -567,6 +542,7 @@ func TestInitChainer(t *testing.T) { db := dbm.NewMemDB() logger := defaultLogger() app := NewBaseApp(name, logger, db, nil) + app.SetParamStore(&mock.ParamStore{Db: db}) capKey := sdk.NewKVStoreKey("main") capKey2 := sdk.NewKVStoreKey("key2") app.MountStores(capKey, capKey2) @@ -623,6 +599,7 @@ func TestInitChainer(t *testing.T) { // reload app app = NewBaseApp(name, logger, db, nil) app.SetInitChainer(initChainer) + app.SetParamStore(&mock.ParamStore{Db: db}) app.MountStores(capKey, capKey2) err = app.LoadLatestVersion() // needed to make stores non-nil require.Nil(t, err) @@ -641,11 +618,30 @@ func TestInitChainer(t *testing.T) { require.Equal(t, value, res.Value) } +func TestInitChain_ProtocolVersionSetToZero(t *testing.T) { + name := t.Name() + db := dbm.NewMemDB() + logger := defaultLogger() + app := NewBaseApp(name, logger, db, nil) + app.SetParamStore(&mock.ParamStore{Db: dbm.NewMemDB()}) + + app.InitChain( + abci.RequestInitChain{ + InitialHeight: 3, + }, + ) + + protocolVersion, err := app.GetAppVersion(app.deliverState.ctx) + require.NoError(t, err) + require.Equal(t, uint64(0), protocolVersion) +} + func TestInitChain_WithInitialHeight(t *testing.T) { name := t.Name() db := dbm.NewMemDB() logger := defaultLogger() app := NewBaseApp(name, logger, db, nil) + app.SetParamStore(&mock.ParamStore{Db: dbm.NewMemDB()}) app.InitChain( abci.RequestInitChain{ @@ -662,6 +658,7 @@ func TestBeginBlock_WithInitialHeight(t *testing.T) { db := dbm.NewMemDB() logger := defaultLogger() app := NewBaseApp(name, logger, db, nil) + app.SetParamStore(&mock.ParamStore{Db: dbm.NewMemDB()}) app.InitChain( abci.RequestInitChain{ @@ -2221,7 +2218,7 @@ func TestBaseApp_EndBlock(t *testing.T) { } app := NewBaseApp(name, logger, db, nil) - app.SetParamStore(¶mStore{db: dbm.NewMemDB()}) + app.SetParamStore(&mock.ParamStore{Db: dbm.NewMemDB()}) app.InitChain(abci.RequestInitChain{ ConsensusParams: cp, }) @@ -2241,7 +2238,7 @@ func TestBaseApp_EndBlock(t *testing.T) { require.Equal(t, cp.Block.MaxGas, res.ConsensusParamUpdates.Block.MaxGas) } -func TestBaseApp_Init(t *testing.T) { +func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) { db := dbm.NewMemDB() name := t.Name() logger := defaultLogger() @@ -2380,6 +2377,8 @@ func TestBaseApp_Init(t *testing.T) { } for _, tc := range testCases { + tc.bapp.SetParamStore(&mock.ParamStore{db}) + // Init and validate require.Equal(t, tc.expectedErr, tc.bapp.init()) if tc.expectedErr != nil { @@ -2399,3 +2398,49 @@ func TestBaseApp_Init(t *testing.T) { require.Equal(t, tc.expectedSnapshot.KeepRecent, tc.bapp.snapshotManager.GetKeepRecent()) } } + +func TestBaseApp_Init_ProtocolVersion(t *testing.T) { + const versionNotSet = -1 + + testcases := []struct { + name string + protocolVersion int64 + }{ + { + name: "no app version was set - set to 0", + protocolVersion: versionNotSet, + }, + { + name: "app version was set to 10 - 10 kept", + protocolVersion: 10, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + app := newBaseApp(t.Name()) + db := dbm.NewMemDB() + + app.SetParamStore(&mock.ParamStore{db}) + + var expectedProtocolVersion uint64 + if tc.protocolVersion != versionNotSet { + // Set version on another app with the same param store (db), + // pretending that the app version was set on the app in advance. + oldApp := newBaseApp(t.Name()) + oldApp.SetParamStore(&mock.ParamStore{db}) + require.NoError(t, oldApp.init()) + + expectedProtocolVersion = uint64(tc.protocolVersion) + require.NoError(t, oldApp.SetAppVersion(oldApp.checkState.ctx, expectedProtocolVersion)) + } + + require.NoError(t, app.init()) + + actualProtocolVersion, err := app.GetAppVersion(app.checkState.ctx) + require.NoError(t, err) + + require.Equal(t, expectedProtocolVersion, actualProtocolVersion) + }) + } +} diff --git a/baseapp/options.go b/baseapp/options.go index 92f1ca33826..3c241574b72 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -1,9 +1,11 @@ package baseapp import ( + "errors" "fmt" "io" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" dbm "github.com/tendermint/tm-db" "github.com/cosmos/cosmos-sdk/codec/types" @@ -14,6 +16,11 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) +var ( + errMsgNilParamStore = errors.New("paramStore was nil") + errMsgNoProtocolVersionSet = errors.New("param store did not have the app version set") +) + // File for storing in-package BaseApp optional functions, // for options that need access to non-exported fields of the BaseApp @@ -88,7 +95,6 @@ func (app *BaseApp) SetParamStore(ps ParamStore) { if app.sealed { panic("SetParamStore() on sealed BaseApp") } - app.paramStore = ps } @@ -100,9 +106,32 @@ func (app *BaseApp) SetVersion(v string) { app.version = v } -// SetProtocolVersion sets the application's protocol version -func (app *BaseApp) SetProtocolVersion(v uint64) { - app.appVersion = v +// SetAppVersion sets the application's app version +func (app *BaseApp) SetAppVersion(ctx sdk.Context, v uint64) error { + if app.paramStore == nil { + return errMsgNilParamStore + } + + av := &tmproto.VersionParams{AppVersion: v} + app.paramStore.Set(ctx, ParamStoreKeyVersionParams, av) + return nil +} + +// GetAppVersion gets the application's app version +// an error, if any. +func (app *BaseApp) GetAppVersion(ctx sdk.Context) (uint64, error) { + if app.paramStore == nil { + return 0, errMsgNilParamStore + } + + if !app.paramStore.Has(ctx, ParamStoreKeyVersionParams) { + return 0, errMsgNoProtocolVersionSet + } + + av := &tmproto.VersionParams{} + app.paramStore.Get(ctx, ParamStoreKeyVersionParams, av) + + return av.AppVersion, nil } func (app *BaseApp) SetDB(db dbm.DB) { diff --git a/baseapp/params.go b/baseapp/params.go index 14701d52479..dbfb44fb6aa 100644 --- a/baseapp/params.go +++ b/baseapp/params.go @@ -18,6 +18,7 @@ var ( ParamStoreKeyBlockParams = []byte("BlockParams") ParamStoreKeyEvidenceParams = []byte("EvidenceParams") ParamStoreKeyValidatorParams = []byte("ValidatorParams") + ParamStoreKeyVersionParams = []byte("VersionParams") ) // ParamStore defines the interface the parameter store used by the BaseApp must @@ -84,3 +85,14 @@ func ValidateValidatorParams(i interface{}) error { return nil } + +// ValidateVersionParams defines a stateless validation on VersionParams. This +// function is called whenever the parameters are updated or stored. +func ValidateVersionParams(i interface{}) error { + _, ok := i.(tmproto.VersionParams) + if !ok { + return fmt.Errorf("invalid parameter type: %T", i) + } + + return nil +} diff --git a/server/mock/app.go b/server/mock/app.go index e3fdd2c9147..e73329e107f 100644 --- a/server/mock/app.go +++ b/server/mock/app.go @@ -13,6 +13,7 @@ import ( bam "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/testutil/mock" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -31,6 +32,8 @@ func NewApp(rootDir string, logger log.Logger) (abci.Application, error) { // Create BaseApp. baseApp := bam.NewBaseApp("kvstore", logger, db, decodeTx) + baseApp.SetParamStore(&mock.ParamStore{Db: db}) + // Set mounts for BaseApp's MultiStore. baseApp.MountStores(capKeyMainStore) diff --git a/simapp/app_test.go b/simapp/app_test.go index 56c6dc1485a..435a7786cbe 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -14,6 +14,7 @@ import ( "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/tests/mocks" + "github.com/cosmos/cosmos-sdk/testutil/mock" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/cosmos-sdk/x/auth" @@ -80,6 +81,7 @@ func TestRunMigrations(t *testing.T) { // Create a new baseapp and configurator for the purpose of this test. bApp := baseapp.NewBaseApp(appName, logger, db, encCfg.TxConfig.TxDecoder()) + bApp.SetParamStore(&mock.ParamStore{Db: db}) bApp.SetCommitMultiStoreTracer(nil) bApp.SetInterfaceRegistry(encCfg.InterfaceRegistry) app.BaseApp = bApp diff --git a/simapp/test_helpers.go b/simapp/test_helpers.go index 4f45b23eacd..4a1189ee05e 100644 --- a/simapp/test_helpers.go +++ b/simapp/test_helpers.go @@ -49,6 +49,9 @@ var DefaultConsensusParams = &abci.ConsensusParams{ tmtypes.ABCIPubKeyTypeEd25519, }, }, + Version: &tmproto.VersionParams{ + AppVersion: 10, + }, } func setup(withGenesis bool, invCheckPeriod uint) (*SimApp, GenesisState) { diff --git a/testutil/mock/param_store.go b/testutil/mock/param_store.go new file mode 100644 index 00000000000..fa8a4e5da4e --- /dev/null +++ b/testutil/mock/param_store.go @@ -0,0 +1,45 @@ +package mock + +import ( + "encoding/json" + + sdk "github.com/cosmos/cosmos-sdk/types" + db "github.com/tendermint/tm-db" +) + +type ParamStore struct { + Db db.DB +} + +func (ps *ParamStore) Set(_ sdk.Context, key []byte, value interface{}) { + bz, err := json.Marshal(value) + if err != nil { + panic(err) + } + + ps.Db.Set(key, bz) +} + +func (ps *ParamStore) Has(_ sdk.Context, key []byte) bool { + ok, err := ps.Db.Has(key) + if err != nil { + panic(err) + } + + return ok +} + +func (ps *ParamStore) Get(_ sdk.Context, key []byte, ptr interface{}) { + bz, err := ps.Db.Get(key) + if err != nil { + panic(err) + } + + if len(bz) == 0 { + return + } + + if err := json.Unmarshal(bz, ptr); err != nil { + panic(err) + } +} diff --git a/x/params/keeper/consensus_params.go b/x/params/keeper/consensus_params.go index 5ce8d340d0d..30921ede639 100644 --- a/x/params/keeper/consensus_params.go +++ b/x/params/keeper/consensus_params.go @@ -24,5 +24,10 @@ func ConsensusParamsKeyTable() types.KeyTable { types.NewParamSetPair( baseapp.ParamStoreKeyValidatorParams, tmproto.ValidatorParams{}, baseapp.ValidateValidatorParams, ), + // This param is stored in the param store in order to send it to Tendermint after state sync. + // If this param is modified by governance, it will be reset by the upgrade module. + types.NewParamSetPair( + baseapp.ParamStoreKeyVersionParams, tmproto.VersionParams{}, baseapp.ValidateValidatorParams, + ), ) } diff --git a/x/simulation/params.go b/x/simulation/params.go index 51dfb6439f1..06d48b04f00 100644 --- a/x/simulation/params.go +++ b/x/simulation/params.go @@ -171,6 +171,9 @@ func randomConsensusParams(r *rand.Rand, appState json.RawMessage, cdc codec.JSO MaxAgeNumBlocks: int64(stakingGenesisState.Params.UnbondingTime / AverageBlockTime), MaxAgeDuration: stakingGenesisState.Params.UnbondingTime, }, + Version: &tmproto.VersionParams{ + AppVersion: uint64(simulation.RandIntBetween(r, 0, 10000)), + }, } bz, err := json.MarshalIndent(&consensusParams, "", " ") diff --git a/x/upgrade/exported/exported.go b/x/upgrade/exported/exported.go index 859f7fe1f01..36eb7a803f1 100644 --- a/x/upgrade/exported/exported.go +++ b/x/upgrade/exported/exported.go @@ -1,7 +1,9 @@ package exported -// ProtocolVersionSetter defines the interface fulfilled by BaseApp -// which allows setting it's appVersion field. -type ProtocolVersionSetter interface { - SetProtocolVersion(uint64) +import sdk "github.com/cosmos/cosmos-sdk/types" + +// ProtocolVersionManager defines the interface which allows managing the appVersion field. +type ProtocolVersionManager interface { + GetAppVersion(ctx sdk.Context) (uint64, error) + SetAppVersion(ctx sdk.Context, version uint64) error } diff --git a/x/upgrade/keeper/keeper.go b/x/upgrade/keeper/keeper.go index 7c13ce6f1d2..b67cb4ef16d 100644 --- a/x/upgrade/keeper/keeper.go +++ b/x/upgrade/keeper/keeper.go @@ -31,7 +31,7 @@ type Keeper struct { storeKey sdk.StoreKey // key to access x/upgrade store cdc codec.BinaryCodec // App-wide binary codec upgradeHandlers map[string]types.UpgradeHandler // map of plan name to upgrade handler - versionSetter xp.ProtocolVersionSetter // implements setting the protocol version field on BaseApp + versionManager xp.ProtocolVersionManager // implements setting the app version field on BaseApp } // NewKeeper constructs an upgrade Keeper which requires the following arguments: @@ -39,15 +39,15 @@ type Keeper struct { // storeKey - a store key with which to access upgrade's store // cdc - the app-wide binary codec // homePath - root directory of the application's config -// vs - the interface implemented by baseapp which allows setting baseapp's protocol version field -func NewKeeper(skipUpgradeHeights map[int64]bool, storeKey sdk.StoreKey, cdc codec.BinaryCodec, homePath string, vs xp.ProtocolVersionSetter) Keeper { +// vs - the interface implemented by baseapp which allows setting baseapp's app version field +func NewKeeper(skipUpgradeHeights map[int64]bool, storeKey sdk.StoreKey, cdc codec.BinaryCodec, homePath string, vs xp.ProtocolVersionManager) Keeper { return Keeper{ homePath: homePath, skipUpgradeHeights: skipUpgradeHeights, storeKey: storeKey, cdc: cdc, upgradeHandlers: map[string]types.UpgradeHandler{}, - versionSetter: vs, + versionManager: vs, } } @@ -58,28 +58,6 @@ func (k Keeper) SetUpgradeHandler(name string, upgradeHandler types.UpgradeHandl k.upgradeHandlers[name] = upgradeHandler } -// setProtocolVersion sets the protocol version to state -func (k Keeper) setProtocolVersion(ctx sdk.Context, v uint64) { - store := ctx.KVStore(k.storeKey) - versionBytes := make([]byte, 8) - binary.BigEndian.PutUint64(versionBytes, v) - store.Set([]byte{types.ProtocolVersionByte}, versionBytes) -} - -// getProtocolVersion gets the protocol version from state -func (k Keeper) getProtocolVersion(ctx sdk.Context) uint64 { - store := ctx.KVStore(k.storeKey) - ok := store.Has([]byte{types.ProtocolVersionByte}) - if ok { - pvBytes := store.Get([]byte{types.ProtocolVersionByte}) - protocolVersion := binary.BigEndian.Uint64(pvBytes) - - return protocolVersion - } - // default value - return 0 -} - // SetModuleVersionMap saves a given version map to state func (k Keeper) SetModuleVersionMap(ctx sdk.Context, vm module.VersionMap) { if len(vm) > 0 { @@ -305,12 +283,15 @@ func (k Keeper) ApplyUpgrade(ctx sdk.Context, plan types.Plan) { k.SetModuleVersionMap(ctx, updatedVM) - // incremement the protocol version and set it in state and baseapp - nextProtocolVersion := k.getProtocolVersion(ctx) + 1 - k.setProtocolVersion(ctx, nextProtocolVersion) - if k.versionSetter != nil { - // set protocol version on BaseApp - k.versionSetter.SetProtocolVersion(nextProtocolVersion) + if k.versionManager != nil { + // increment the app version and set it in state and baseapp + appVersion, err := k.versionManager.GetAppVersion(ctx) + if err != nil { + panic(err) + } + if err := k.versionManager.SetAppVersion(ctx, appVersion+1); err != nil { + panic(err) + } } // Must clear IBC state after upgrade is applied as it is stored separately from the upgrade plan. diff --git a/x/upgrade/keeper/keeper_test.go b/x/upgrade/keeper/keeper_test.go index 724848271b1..7dc1baad743 100644 --- a/x/upgrade/keeper/keeper_test.go +++ b/x/upgrade/keeper/keeper_test.go @@ -194,10 +194,11 @@ func (s *KeeperTestSuite) TestSetUpgradedClient() { } -// Test that the protocol version successfully increments after an +// Test that the app version successfully increments after an // upgrade and is successfully set on BaseApp's appVersion. -func (s *KeeperTestSuite) TestIncrementProtocolVersion() { - oldProtocolVersion := s.app.BaseApp.AppVersion() +func (s *KeeperTestSuite) TestIncrementAppVersion() { + oldAppVersion, err := s.app.BaseApp.GetAppVersion(s.ctx) + s.Require().NoError(err) s.app.UpgradeKeeper.SetUpgradeHandler("dummy", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) { return vm, nil }) dummyPlan := types.Plan{ Name: "dummy", @@ -205,9 +206,10 @@ func (s *KeeperTestSuite) TestIncrementProtocolVersion() { Height: 100, } s.app.UpgradeKeeper.ApplyUpgrade(s.ctx, dummyPlan) - upgradedProtocolVersion := s.app.BaseApp.AppVersion() + upgradedAppVersion, err := s.app.BaseApp.GetAppVersion(s.ctx) + s.Require().NoError(err) - s.Require().Equal(oldProtocolVersion+1, upgradedProtocolVersion) + s.Require().Equal(oldAppVersion+1, upgradedAppVersion) } // Tests that the underlying state of x/upgrade is set correctly after diff --git a/x/upgrade/keeper/util_test.go b/x/upgrade/keeper/util_test.go new file mode 100644 index 00000000000..354a0ea93b9 --- /dev/null +++ b/x/upgrade/keeper/util_test.go @@ -0,0 +1,8 @@ +package keeper + +import sdk "github.com/cosmos/cosmos-sdk/types" + +// SetAppVersion sets the app version to version. Used for testing the upgrade keeper. +func (k *Keeper) SetAppVersion(ctx sdk.Context, version uint64) error { + return k.versionManager.SetAppVersion(ctx, version) +} diff --git a/x/upgrade/spec/02_state.md b/x/upgrade/spec/02_state.md index 73c64ad2ddf..41126ea0e23 100644 --- a/x/upgrade/spec/02_state.md +++ b/x/upgrade/spec/02_state.md @@ -10,11 +10,11 @@ state contains the currently active upgrade `Plan` (if one exists) by key contains the consensus versions of all app modules in the application. The versions are stored as big endian `uint64`, and can be accessed with prefix `0x2` appended by the corresponding module name of type `string`. The state maintains a -`Protocol Version` which can be accessed by key `0x3`. +`App Version` which can be accessed by key `0x3`. - Plan: `0x0 -> Plan` - Done: `0x1 | byte(plan name) -> BigEndian(Block Height)` - ConsensusVersion: `0x2 | byte(module name) -> BigEndian(Module Consensus Version)` -- ProtocolVersion: `0x3 -> BigEndian(Protocol Version)` +- AppVersion: `0x3 -> BigEndian(App Version)` The `x/upgrade` module contains no genesis state. diff --git a/x/upgrade/types/keys.go b/x/upgrade/types/keys.go index 45128fc7049..4ed28689a45 100644 --- a/x/upgrade/types/keys.go +++ b/x/upgrade/types/keys.go @@ -25,8 +25,8 @@ const ( // VersionMapByte is a prefix to look up module names (key) and versions (value) VersionMapByte = 0x2 - // ProtocolVersionByte is a prefix to look up Protocol Version - ProtocolVersionByte = 0x3 + // AppVersionByte is a prefix to look up App Version + AppVersionByte = 0x3 // KeyUpgradedIBCState is the key under which upgraded ibc state is stored in the upgrade store KeyUpgradedIBCState = "upgradedIBCState" diff --git a/x/upgrade/types/storeloader_test.go b/x/upgrade/types/storeloader_test.go index 62a56acc5b1..78ce9cbb8c0 100644 --- a/x/upgrade/types/storeloader_test.go +++ b/x/upgrade/types/storeloader_test.go @@ -17,6 +17,7 @@ import ( pruningtypes "github.com/cosmos/cosmos-sdk/pruning/types" "github.com/cosmos/cosmos-sdk/store/rootmulti" store "github.com/cosmos/cosmos-sdk/store/types" + "github.com/cosmos/cosmos-sdk/testutil/mock" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -121,6 +122,7 @@ func TestSetLoader(t *testing.T) { opts := []func(*baseapp.BaseApp){baseapp.SetPruning(pruningtypes.NewPruningOptions(pruningtypes.PruningNothing))} origapp := baseapp.NewBaseApp(t.Name(), defaultLogger(), db, nil, opts...) + origapp.SetParamStore(&mock.ParamStore{Db: db}) origapp.MountStores(sdk.NewKVStoreKey(tc.origStoreKey)) err := origapp.LoadLatestVersion() require.Nil(t, err) @@ -137,6 +139,7 @@ func TestSetLoader(t *testing.T) { // load the new app with the original app db app := baseapp.NewBaseApp(t.Name(), defaultLogger(), db, nil, opts...) + app.SetParamStore(&mock.ParamStore{Db: db}) app.MountStores(sdk.NewKVStoreKey(tc.loadStoreKey)) err = app.LoadLatestVersion() require.Nil(t, err)