feat!: add app version to param store (#360)

This PR adds app version to the ParamStore meaning that app version is no longer just in memory but is also persisted. We need this for example when a node restarts as Tendermint will call Info and it must remember the correct app version. It's also important for ensuring that state sync works beyond v1

baseapp/abci.go

@@ -60,6 +60,9 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
 	// to state.
 	if req.ConsensusParams != nil {
 		app.StoreConsensusParams(app.deliverState.ctx, req.ConsensusParams)
+		if req.ConsensusParams.Version != nil {
+			app.appVersion = req.ConsensusParams.Version.AppVersion
+		}
 	}
 
 	if app.initChainer == nil {
@@ -122,7 +125,15 @@ func (app *BaseApp) SetOption(req abci.RequestSetOption) (res abci.ResponseSetOp
 // Info implements the ABCI interface.
 func (app *BaseApp) Info(req abci.RequestInfo) abci.ResponseInfo {
 	lastCommitID := app.cms.LastCommitID()
-
+	// load the app version for a non zero height
+	if lastCommitID.Version > 0 {
+		ctx, err := app.createQueryContext(lastCommitID.Version, false)
+		if err != nil {
+			panic(err)
+		}
+		// get and set the app version
+		_ = app.AppVersion(ctx)
+	}
 	return abci.ResponseInfo{
 		Data:             app.name,
 		Version:          app.version,

baseapp/baseapp.go

@@ -53,7 +53,6 @@ type BaseApp struct { // nolint: maligned
 	postHandler sdk.AnteHandler // post handler, optional, e.g. for tips
 
 	appStore
-	baseappVersions
 	peerFilters
 	snapshotData
 	abciData
@@ -98,6 +97,13 @@ type BaseApp struct { // nolint: maligned
 	// ResponseCommit.RetainHeight.
 	minRetainBlocks uint64
 
+	// application's version 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
 
@@ -145,15 +151,6 @@ type abciData struct {
 	voteInfos []abci.VoteInfo
 }
 
-type baseappVersions struct {
-	// application's version 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
-}
-
 // should really get handled in some db struct
 // which then has a sub-item, persistence fields
 type snapshotData struct {
@@ -206,7 +203,14 @@ func (app *BaseApp) Name() string {
 }
 
 // AppVersion returns the application's protocol version.
-func (app *BaseApp) AppVersion() uint64 {
+func (app *BaseApp) AppVersion(ctx sdk.Context) uint64 {
+	if app.appVersion == 0 && app.paramStore.Has(ctx, ParamStoreKeyVersionParams) {
+		var vp tmproto.VersionParams
+
+		app.paramStore.Get(ctx, ParamStoreKeyVersionParams, &vp)
+		// set the app version
+		app.appVersion = vp.AppVersion
+	}
 	return app.appVersion
 }
 
@@ -482,7 +486,12 @@ func (app *BaseApp) GetConsensusParams(ctx sdk.Context) *abci.ConsensusParams {
 		cp.Validator = &vp
 	}
 
-	cp.Version = &tmproto.VersionParams{AppVersion: app.appVersion}
+	if app.paramStore.Has(ctx, ParamStoreKeyVersionParams) {
+		var vp tmproto.VersionParams
+
+		app.paramStore.Get(ctx, ParamStoreKeyVersionParams, &vp)
+		cp.Version = &vp
+	}
 
 	return cp
 }
@@ -507,8 +516,10 @@ 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)
-	// 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.
+	// NOTE: we only persist the app version from v2 onwards
+	if cp.Version != nil && cp.Version.AppVersion >= 2 {
+		app.paramStore.Set(ctx, ParamStoreKeyVersionParams, cp.Version)
+	}
 }
 
 // getMaximumBlockGas gets the maximum gas from the consensus params. It panics

baseapp/baseapp_test.go

@@ -460,7 +460,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)
+	require.EqualValues(t, 0, res.AppVersion)
 	// ----- test a proper response -------
 	// TODO
 }

baseapp/options.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	dbm "github.com/tendermint/tm-db"
 
 	"github.com/cosmos/cosmos-sdk/codec/types"
@@ -110,8 +111,14 @@ func (app *BaseApp) SetVersion(v string) {
 	app.version = v
 }
 
-// SetProtocolVersion sets the application's protocol version
-func (app *BaseApp) SetProtocolVersion(v uint64) {
+// SetAppVersion sets the application's protocol version
+func (app *BaseApp) SetAppVersion(ctx sdk.Context, v uint64) {
+	// TODO: could make this less hacky in the future since the SDK
+	// shouldn't have to know about the app versioning scheme
+	if ctx.BlockHeader().Version.App >= 2 {
+		vp := &tmproto.VersionParams{AppVersion: v}
+		app.paramStore.Set(ctx, ParamStoreKeyVersionParams, vp)
+	}
 	app.appVersion = v
 }
 

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,18 @@ 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 {
+	v, ok := i.(tmproto.VersionParams)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.AppVersion == 0 {
+		return errors.New("application version must not be zero")
+	}
+
+	return nil
+}

x/params/types/consensus_params.go

@@ -23,5 +23,8 @@ func ConsensusParamsKeyTable() KeyTable {
 		NewParamSetPair(
 			baseapp.ParamStoreKeyValidatorParams, tmproto.ValidatorParams{}, baseapp.ValidateValidatorParams,
 		),
+		NewParamSetPair(
+			baseapp.ParamStoreKeyVersionParams, tmproto.VersionParams{}, baseapp.ValidateVersionParams,
+		),
 	)
 }

x/upgrade/exported/exported.go

@@ -1,7 +1,9 @@
 package exported
 
+import sdk "github.com/cosmos/cosmos-sdk/types"
+
 // ProtocolVersionSetter defines the interface fulfilled by BaseApp
 // which allows setting it's appVersion field.
 type ProtocolVersionSetter interface {
-	SetProtocolVersion(uint64)
+	SetAppVersion(sdk.Context, uint64)
 }

x/upgrade/keeper/keeper.go

@@ -346,7 +346,7 @@ func (k Keeper) ApplyUpgrade(ctx sdk.Context, plan types.Plan) {
 	k.setProtocolVersion(ctx, nextProtocolVersion)
 	if k.versionSetter != nil {
 		// set protocol version on BaseApp
-		k.versionSetter.SetProtocolVersion(nextProtocolVersion)
+		k.versionSetter.SetAppVersion(ctx, nextProtocolVersion)
 	}
 
 	// Must clear IBC state after upgrade is applied as it is stored separately from the upgrade plan.

x/upgrade/keeper/keeper_test.go

@@ -203,15 +203,15 @@ func (s *KeeperTestSuite) TestSetUpgradedClient() {
 // Test that the protocol version successfully increments after an
 // upgrade and is successfully set on BaseApp's appVersion.
 func (s *KeeperTestSuite) TestIncrementProtocolVersion() {
-	oldProtocolVersion := s.app.BaseApp.AppVersion()
+	oldProtocolVersion := s.app.BaseApp.AppVersion(s.ctx)
 	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()
+	upgradedProtocolVersion := s.app.BaseApp.AppVersion(s.ctx)
 
 	s.Require().Equal(oldProtocolVersion+1, upgradedProtocolVersion)
 }