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 <alessio@tendermint.com>

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.

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:

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

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)

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
 }
 

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

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)
+}

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)

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() {

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.

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"