From e3a0148bf6b120237c6626d9470bfb121c7de3a8 Mon Sep 17 00:00:00 2001 From: technicallyty <48813565+technicallyty@users.noreply.github.com> Date: Fri, 2 Apr 2021 07:11:58 -0700 Subject: [PATCH] x/upgrade: protocol version tracking (#8897) * -added consensus version tracking to x/upgrade * -added interface to module manager -added e2e test for migrations using consensus version store in x/upgrade -cleaned up x/upgrade Keeper -handler in apply upgrade now handles errors and setting consensus versions -cleaned up migration map keys -removed init chainer method -simapp now implements GetConsensusVersions to assist with testing * protocol version p1 * add protocol version to baseapp * rebase against master * add test * added test case * cleanup * docs and change to bigendian * changelog * cleanup keeper * swap appVersion and version * cleanup * change interface id * update keeper field name to versionSetter * reorder keys and docs * -move interface into exported folder * typo * typo2 * docs on keeper fields * docs for upgrade NewKeeper * cleanup * NewKeeper docs Co-authored-by: technicallyty <48813565+tytech3@users.noreply.github.com> Co-authored-by: Alessio Treglia --- CHANGELOG.md | 2 ++ baseapp/abci.go | 4 ++- baseapp/baseapp.go | 15 +++++++-- baseapp/baseapp_test.go | 12 +++---- baseapp/options.go | 10 ++++-- simapp/app.go | 4 +-- x/upgrade/exported/exported.go | 7 ++++ x/upgrade/keeper/keeper.go | 57 +++++++++++++++++++++++++++------ x/upgrade/keeper/keeper_test.go | 18 ++++++++++- x/upgrade/spec/02_state.md | 7 ++-- x/upgrade/types/keys.go | 3 ++ 11 files changed, 110 insertions(+), 29 deletions(-) create mode 100644 x/upgrade/exported/exported.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 7de63a77f..cb310964a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/bank) [\#8517](https://github.com/cosmos/cosmos-sdk/pull/8517) `SupplyI` interface and `Supply` are removed and uses `sdk.Coins` for supply tracking * (x/upgrade) [\#8743](https://github.com/cosmos/cosmos-sdk/pull/8743) `UpgradeHandler` includes a new argument `VersionMap` which helps facilitate in-place migrations. * (x/auth) [\#8129](https://github.com/cosmos/cosmos-sdk/pull/8828) Updated `SigVerifiableTx.GetPubKeys` method signature to return error. +* (x/upgrade) [\7487](https://github.com/cosmos/cosmos-sdk/pull/8897) Upgrade `Keeper` takes new argument `ProtocolVersionSetter` which implements setting a protocol version on baseapp. +* (baseapp) [\7487](https://github.com/cosmos/cosmos-sdk/pull/8897) BaseApp's fields appVersion and version were swapped to match Tendermint's fields. * [\#8682](https://github.com/cosmos/cosmos-sdk/pull/8682) `ante.NewAnteHandler` updated to receive all positional params as `ante.HandlerOptions` struct. If required fields aren't set, throws error accordingly. * (x/staking/types) [\#7447](https://github.com/cosmos/cosmos-sdk/issues/7447) Remove bech32 PubKey support: * `ValidatorI` interface update: `GetConsPubKey` renamed to `TmConsPubKey` (this is to clarify the return type: consensus public key must be a tendermint key); `TmConsPubKey`, `GetConsAddr` methods return error. diff --git a/baseapp/abci.go b/baseapp/abci.go index 36c36d1e1..f7ecb16e6 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -109,6 +109,8 @@ func (app *BaseApp) Info(req abci.RequestInfo) abci.ResponseInfo { return abci.ResponseInfo{ Data: app.name, + Version: app.version, + AppVersion: app.appVersion, LastBlockHeight: lastCommitID.Version, LastBlockAppHash: lastCommitID.Hash, } @@ -755,7 +757,7 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) abci.Res return abci.ResponseQuery{ Codespace: sdkerrors.RootCodespace, Height: req.Height, - Value: []byte(app.appVersion), + Value: []byte(app.version), } default: diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index a79aed801..5ec243ba9 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -117,7 +117,11 @@ type BaseApp struct { // nolint: maligned minRetainBlocks uint64 // application's version string - appVersion string + 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 @@ -170,11 +174,16 @@ func (app *BaseApp) Name() string { return app.name } -// AppVersion returns the application's version string. -func (app *BaseApp) AppVersion() string { +// 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 +} + // Logger returns the logger of the BaseApp. func (app *BaseApp) Logger() log.Logger { return app.logger diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index b0eace4d3..add6b982a 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -340,21 +340,21 @@ func TestSetLoader(t *testing.T) { } } -func TestAppVersionSetterGetter(t *testing.T) { +func TestVersionSetterGetter(t *testing.T) { logger := defaultLogger() pruningOpt := SetPruning(store.PruneDefault) db := dbm.NewMemDB() name := t.Name() app := NewBaseApp(name, logger, db, nil, pruningOpt) - require.Equal(t, "", app.AppVersion()) + require.Equal(t, "", app.Version()) res := app.Query(abci.RequestQuery{Path: "app/version"}) require.True(t, res.IsOK()) require.Equal(t, "", string(res.Value)) versionString := "1.0.0" - app.SetAppVersion(versionString) - require.Equal(t, versionString, app.AppVersion()) + app.SetVersion(versionString) + require.Equal(t, versionString, app.Version()) res = app.Query(abci.RequestQuery{Path: "app/version"}) require.True(t, res.IsOK()) require.Equal(t, versionString, string(res.Value)) @@ -498,7 +498,7 @@ 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 } @@ -510,7 +510,7 @@ func TestBaseAppOptionSeal(t *testing.T) { app.SetName("") }) require.Panics(t, func() { - app.SetAppVersion("") + app.SetVersion("") }) require.Panics(t, func() { app.SetDB(nil) diff --git a/baseapp/options.go b/baseapp/options.go index 33de1a3aa..be9fbdc65 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -95,12 +95,16 @@ func (app *BaseApp) SetParamStore(ps ParamStore) { app.paramStore = ps } -// SetAppVersion sets the application's version string. -func (app *BaseApp) SetAppVersion(v string) { +// SetVersion sets the application's version string. +func (app *BaseApp) SetVersion(v string) { if app.sealed { - panic("SetAppVersion() on sealed BaseApp") + panic("SetVersion() on sealed BaseApp") } + app.version = v +} +// SetProtocolVersion sets the application's protocol version +func (app *BaseApp) SetProtocolVersion(v uint64) { app.appVersion = v } diff --git a/simapp/app.go b/simapp/app.go index e0c4316f3..7d62f2e84 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -200,7 +200,7 @@ func NewSimApp( bApp := baseapp.NewBaseApp(appName, logger, db, encodingConfig.TxConfig.TxDecoder(), baseAppOptions...) bApp.SetCommitMultiStoreTracer(traceStore) - bApp.SetAppVersion(version.Version) + bApp.SetVersion(version.Version) bApp.SetInterfaceRegistry(interfaceRegistry) keys := sdk.NewKVStoreKeys( @@ -257,7 +257,7 @@ func NewSimApp( ) app.FeeGrantKeeper = feegrantkeeper.NewKeeper(appCodec, keys[feegranttypes.StoreKey], app.AccountKeeper) - app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, keys[upgradetypes.StoreKey], appCodec, homePath) + app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, keys[upgradetypes.StoreKey], appCodec, homePath, app.BaseApp) // register the staking hooks // NOTE: stakingKeeper above is passed by reference, so that it will contain these hooks diff --git a/x/upgrade/exported/exported.go b/x/upgrade/exported/exported.go new file mode 100644 index 000000000..859f7fe1f --- /dev/null +++ b/x/upgrade/exported/exported.go @@ -0,0 +1,7 @@ +package exported + +// ProtocolVersionSetter defines the interface fulfilled by BaseApp +// which allows setting it's appVersion field. +type ProtocolVersionSetter interface { + SetProtocolVersion(uint64) +} diff --git a/x/upgrade/keeper/keeper.go b/x/upgrade/keeper/keeper.go index b171a219b..3342c26a8 100644 --- a/x/upgrade/keeper/keeper.go +++ b/x/upgrade/keeper/keeper.go @@ -8,37 +8,44 @@ import ( "path" "path/filepath" - "github.com/tendermint/tendermint/libs/log" - tmos "github.com/tendermint/tendermint/libs/os" - "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store/prefix" store "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/module" + xp "github.com/cosmos/cosmos-sdk/x/upgrade/exported" "github.com/cosmos/cosmos-sdk/x/upgrade/types" + "github.com/tendermint/tendermint/libs/log" + tmos "github.com/tendermint/tendermint/libs/os" ) // UpgradeInfoFileName file to store upgrade information const UpgradeInfoFileName string = "upgrade-info.json" type Keeper struct { - homePath string - skipUpgradeHeights map[int64]bool - storeKey sdk.StoreKey - cdc codec.BinaryMarshaler - upgradeHandlers map[string]types.UpgradeHandler + homePath string // root directory of app config + skipUpgradeHeights map[int64]bool // map of heights to skip for an upgrade + storeKey sdk.StoreKey // key to access x/upgrade store + cdc codec.BinaryMarshaler // 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 } -// NewKeeper constructs an upgrade Keeper -func NewKeeper(skipUpgradeHeights map[int64]bool, storeKey sdk.StoreKey, cdc codec.BinaryMarshaler, homePath string) Keeper { +// NewKeeper constructs an upgrade Keeper which requires the following arguments: +// skipUpgradeHeights - map of heights to skip an upgrade +// 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.BinaryMarshaler, homePath string, vs xp.ProtocolVersionSetter) Keeper { return Keeper{ homePath: homePath, skipUpgradeHeights: skipUpgradeHeights, storeKey: storeKey, cdc: cdc, upgradeHandlers: map[string]types.UpgradeHandler{}, + versionSetter: vs, } } @@ -49,6 +56,28 @@ 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 { @@ -228,6 +257,14 @@ 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) + } + // Must clear IBC state after upgrade is applied as it is stored separately from the upgrade plan. // This will prevent resubmission of upgrade msg after upgrade is already completed. k.ClearIBCState(ctx, plan.Height) diff --git a/x/upgrade/keeper/keeper_test.go b/x/upgrade/keeper/keeper_test.go index 9a5c38f64..50545e18e 100644 --- a/x/upgrade/keeper/keeper_test.go +++ b/x/upgrade/keeper/keeper_test.go @@ -28,7 +28,7 @@ func (s *KeeperTestSuite) SetupTest() { app := simapp.Setup(false) homeDir := filepath.Join(s.T().TempDir(), "x_upgrade_keeper_test") app.UpgradeKeeper = keeper.NewKeeper( // recreate keeper in order to use a custom home path - make(map[int64]bool), app.GetKey(types.StoreKey), app.AppCodec(), homeDir, + make(map[int64]bool), app.GetKey(types.StoreKey), app.AppCodec(), homeDir, app.BaseApp, ) s.T().Log("home dir:", homeDir) s.homeDir = homeDir @@ -194,6 +194,22 @@ func (s *KeeperTestSuite) TestSetUpgradedClient() { } +// Test that the protocol version successfully increments after an +// upgrade and is succesfully set on BaseApp's appVersion. +func (s *KeeperTestSuite) TestIncrementProtocolVersion() { + oldProtocolVersion := s.app.BaseApp.AppVersion() + s.app.UpgradeKeeper.SetUpgradeHandler("dummy", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) { return vm, nil }) + dummyPlan := types.Plan{ + Name: "dummy", + Info: "some text here", + Height: 100, + } + s.app.UpgradeKeeper.ApplyUpgrade(s.ctx, dummyPlan) + upgradedProtocolVersion := s.app.BaseApp.AppVersion() + + s.Require().Equal(oldProtocolVersion+1, upgradedProtocolVersion) +} + // Tests that the underlying state of x/upgrade is set correctly after // an upgrade. func (s *KeeperTestSuite) TestMigrations() { diff --git a/x/upgrade/spec/02_state.md b/x/upgrade/spec/02_state.md index 796b99686..76391fa71 100644 --- a/x/upgrade/spec/02_state.md +++ b/x/upgrade/spec/02_state.md @@ -6,14 +6,15 @@ order: 2 The internal state of the `x/upgrade` module is relatively minimal and simple. The state contains the currently active upgrade `Plan` (if one exists) by key -`0x0` and if a `Plan` is marked as "done" by key `0x1`. Additionally, the state +`0x0` and if a `Plan` is marked as "done" by key `0x1`. The state 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`. +by the corresponding module name of type `string`. The state maintains a +`Protocol 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)` The `x/upgrade` module contains no genesis state. diff --git a/x/upgrade/types/keys.go b/x/upgrade/types/keys.go index 2505bd5ce..45128fc70 100644 --- a/x/upgrade/types/keys.go +++ b/x/upgrade/types/keys.go @@ -25,6 +25,9 @@ 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 + // KeyUpgradedIBCState is the key under which upgraded ibc state is stored in the upgrade store KeyUpgradedIBCState = "upgradedIBCState"