feat!: connect app version with consensus params in end block

CHANGELOG.md

@@ -130,6 +130,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (x/staking) [#14590](https://github.com/cosmos/cosmos-sdk/pull/14590) `MsgUndelegateResponse` now includes undelegated amount. `x/staking` module's `keeper.Undelegate` now returns 3 values (completionTime,undelegateAmount,error)  instead of 2.
 * (x/staking) (#15731) (https://github.com/cosmos/cosmos-sdk/pull/15731) Introducing a new index to retrieve the delegations by validator efficiently.
 * (baseapp) [#15930](https://github.com/cosmos/cosmos-sdk/pull/15930) change vote info provided by prepare and process proposal to the one in the block 
+* (x/upgrade) [#16244](https://github.com/cosmos/cosmos-sdk/pull/16244) upgrade module no longer stores the app version but gets and sets the app version stored in the `ParamStore` of baseapp.
 
 ### API Breaking Changes
 
@@ -215,11 +216,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
     * Remove: errors unused errors
 
 * (sims) [#16155](https://github.com/cosmos/cosmos-sdk/pull/16155) 
-
-    * `simulation.NewOperationMsg` now marshals the operation msg as proto bytes instead of legacy amino JSON bytes.
-    * `simulation.NewOperationMsg` is now 2-arity instead of 3-arity with the obsolete argument `codec.ProtoCodec` removed.
-    * The field `OperationMsg.Msg` is now of type `[]byte` instead of `json.RawMessage`.
+  * `simulation.NewOperationMsg` now marshals the operation msg as proto bytes instead of legacy amino JSON bytes.
+  * `simulation.NewOperationMsg` is now 2-arity instead of 3-arity with the obsolete argument `codec.ProtoCodec` removed.
+  * The field `OperationMsg.Msg` is now of type `[]byte` instead of `json.RawMessage`.
 * (cli) [#16209](https://github.com/cosmos/cosmos-sdk/pull/16209) Add API `StartCmdWithOptions` to create customized start command.
+* (baseapp) [#16244](https://github.com/cosmos/cosmos-sdk/pull/16244) `SetProtocolVersion` has been renamed to `SetAppVersion`. It now updates the consensus params in baseapp's `ParamStore`.
 * (x/distribution) [#16211](https://github.com/cosmos/cosmos-sdk/pull/16211) Use collections for params state management.
 
 ### Client Breaking Changes

baseapp/abci.go

@@ -141,11 +141,26 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
 // Info implements the ABCI interface.
 func (app *BaseApp) Info(req abci.RequestInfo) abci.ResponseInfo {
 	lastCommitID := app.cms.LastCommitID()
+	qms := app.qms
+	if qms == nil {
+		qms = app.cms.(storetypes.MultiStore)
+	}
+	appVersion := InitialAppVersion
+	if lastCommitID.Version > 0 {
+		ctx, err := app.CreateQueryContext(lastCommitID.Version, false)
+		if err != nil {
+			panic(err)
+		}
+		appVersion, err = app.AppVersion(ctx)
+		if err != nil {
+			panic(err)
+		}
+	}
 
 	return abci.ResponseInfo{
 		Data:             app.name,
 		Version:          app.version,
-		AppVersion:       app.appVersion,
+		AppVersion:       appVersion,
 		LastBlockHeight:  lastCommitID.Version,
 		LastBlockAppHash: lastCommitID.Hash,
 	}

baseapp/abci_test.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 	"testing"
 
+
 	errorsmod "cosmossdk.io/errors"
 	"cosmossdk.io/log"
 	pruningtypes "cosmossdk.io/store/pruning/types"
@@ -15,6 +16,7 @@ import (
 	storetypes "cosmossdk.io/store/types"
 	abci "github.com/cometbft/cometbft/abci/types"
 	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
+	cmttypes "github.com/cometbft/cometbft/types"
 	dbm "github.com/cosmos/cosmos-db"
 	"github.com/cosmos/gogoproto/jsonpb"
 	"github.com/stretchr/testify/require"
@@ -31,6 +33,8 @@ import (
 
 func TestABCI_Info(t *testing.T) {
 	suite := NewBaseAppSuite(t)
+	ctx := suite.baseApp.NewContext(true, cmtproto.Header{})
+	suite.baseApp.StoreConsensusParams(ctx, cmttypes.DefaultConsensusParams().ToProto())
 
 	reqInfo := abci.RequestInfo{}
 	res := suite.baseApp.Info(reqInfo)
@@ -39,7 +43,16 @@ func TestABCI_Info(t *testing.T) {
 	require.Equal(t, t.Name(), res.GetData())
 	require.Equal(t, int64(0), res.LastBlockHeight)
 	require.Equal(t, []uint8(nil), res.LastBlockAppHash)
-	require.Equal(t, suite.baseApp.AppVersion(), res.AppVersion)
+	appVersion, err := suite.baseApp.AppVersion(ctx)
+	require.NoError(t, err)
+	require.Equal(t, appVersion, res.AppVersion)
+
+	header := cmtproto.Header{Height: 1}
+	suite.baseApp.BeginBlock(abci.RequestBeginBlock{Header: header})
+	suite.baseApp.Commit()
+	require.NoError(t, suite.baseApp.SetAppVersion(ctx, 1))
+	res = suite.baseApp.Info(reqInfo)
+	require.Equal(t, uint64(1), res.AppVersion)
 }
 
 func TestABCI_First_block_Height(t *testing.T) {

baseapp/baseapp.go

@@ -1,6 +1,7 @@
 package baseapp
 
 import (
+	"context"
 	"fmt"
 	"sort"
 	"strconv"
@@ -126,10 +127,6 @@ type BaseApp 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
-
 	// recovery handler for app.runTx method
 	runTxRecoveryMiddleware recoveryMiddleware
 
@@ -199,8 +196,19 @@ func (app *BaseApp) Name() string {
 }
 
 // AppVersion returns the application's protocol version.
-func (app *BaseApp) AppVersion() uint64 {
-	return app.appVersion
+func (app *BaseApp) AppVersion(ctx context.Context) (uint64, error) {
+	if app.paramStore == nil {
+		return 0, errors.New("app.paramStore is nil")
+	}
+
+	cp, err := app.paramStore.Get(ctx)
+	if err != nil {
+		return 0, fmt.Errorf("getting consensus params: %w", err)
+	}
+	if cp.Version == nil {
+		return 0, nil
+	}
+	return cp.Version.App, nil
 }
 
 // Version returns the application's version string.

baseapp/options.go

@@ -1,6 +1,8 @@
 package baseapp
 
 import (
+	"context"
+	"errors"
 	"fmt"
 	"io"
 
@@ -122,9 +124,22 @@ 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 version this is used as part of the
+// header in blocks and is returned to the consensus engine in EndBlock.
+func (app *BaseApp) SetAppVersion(ctx context.Context, v uint64) error {
+	if app.paramStore == nil {
+		return errors.New("param store must be set to set app version")
+	}
+
+	cp, err := app.paramStore.Get(ctx)
+	if err != nil {
+		return err
+	}
+	cp.Version.App = v
+	if err := app.paramStore.Set(ctx, cp); err != nil {
+		return err
+	}
+	return nil
 }
 
 func (app *BaseApp) SetDB(db dbm.DB) {

baseapp/params.go

@@ -6,10 +6,21 @@ import (
 	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
 )
 
+const InitialAppVersion uint64 = 0
+
 // ParamStore defines the interface the parameter store used by the BaseApp must
 // fulfill.
 type ParamStore interface {
 	Get(ctx context.Context) (cmtproto.ConsensusParams, error)
 	Has(ctx context.Context) (bool, error)
 	Set(ctx context.Context, cp cmtproto.ConsensusParams) error
 }
+
+// AppVersionModifier defines the interface fulfilled by BaseApp
+// which allows getting and setting it's appVersion field. This
+// in turn updates the consensus params that are sent to the
+// consensus engine in EndBlock
+type AppVersionModifier interface {
+	SetAppVersion(context.Context, uint64) error
+	AppVersion(context.Context) (uint64, error)
+}

runtime/module.go

@@ -69,6 +69,7 @@ func init() {
 			ProvideHeaderInfoService,
 			ProvideCometInfoService,
 			ProvideBasicManager,
+			ProvideAppVersionModifier,
 		),
 		appmodule.Invoke(SetupAppBuilder),
 	)
@@ -246,6 +247,10 @@ func ProvideBasicManager(app *AppBuilder) module.BasicManager {
 	return app.app.basicManager
 }
 
+func ProvideAppVersionModifier(app *AppBuilder) baseapp.AppVersionModifier {
+	return app.app
+}
+
 // globalAccAddressCodec is a temporary address codec that we will use until we
 // can populate it with the correct bech32 prefixes without depending on the global.
 type globalAccAddressCodec struct{}

simapp/app_test.go

@@ -269,8 +269,6 @@ func TestUpgradeStateOnGenesis(t *testing.T) {
 			require.Equal(t, vm[v], i.ConsensusVersion())
 		}
 	}
-
-	require.NotNil(t, app.UpgradeKeeper.GetVersionSetter())
 }
 
 // TestMergedRegistry tests that fetching the gogo/protov2 merged registry

x/upgrade/abci_test.go

@@ -1,6 +1,7 @@
 package upgrade_test
 
 import (
+	"context"
 	"errors"
 	"os"
 	"testing"
@@ -10,6 +11,7 @@ import (
 	"cosmossdk.io/log"
 	storetypes "cosmossdk.io/store/types"
 	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
+	cmttypes "github.com/cometbft/cometbft/types"
 	"github.com/cosmos/cosmos-sdk/baseapp"
 	addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
 	"github.com/cosmos/cosmos-sdk/testutil"
@@ -52,9 +54,9 @@ func setupTest(t *testing.T, height int64, skip map[int64]bool) *TestSuite {
 		testCtx.DB,
 		s.encCfg.TxConfig.TxDecoder(),
 	)
+	s.baseApp.SetParamStore(&paramStore{params: cmttypes.DefaultConsensusParams().ToProto()})
 
-	s.keeper = keeper.NewKeeper(skip, key, s.encCfg.Codec, t.TempDir(), nil, authtypes.NewModuleAddress(govtypes.ModuleName).String())
-	s.keeper.SetVersionSetter(s.baseApp)
+	s.keeper = keeper.NewKeeper(skip, key, s.encCfg.Codec, t.TempDir(), s.baseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
 
 	s.ctx = testCtx.Ctx.WithBlockHeader(cmtproto.Header{Time: time.Now(), Height: height})
 
@@ -550,3 +552,22 @@ func TestDowngradeVerification(t *testing.T) {
 		}
 	}
 }
+
+type paramStore struct {
+	params cmtproto.ConsensusParams
+}
+
+var _ baseapp.ParamStore = (*paramStore)(nil)
+
+func (ps paramStore) Set(_ context.Context, value cmtproto.ConsensusParams) error {
+	ps.params = value
+	return nil
+}
+
+func (ps paramStore) Has(_ context.Context) (bool, error) {
+	return true, nil
+}
+
+func (ps paramStore) Get(_ context.Context) (cmtproto.ConsensusParams, error) {
+	return ps.params, nil
+}

x/upgrade/exported/exported.go

@@ -1,7 +1,11 @@
 package exported
 
-// ProtocolVersionSetter defines the interface fulfilled by BaseApp
-// which allows setting it's appVersion field.
-type ProtocolVersionSetter interface {
-	SetProtocolVersion(uint64)
-}
+import (
+	"github.com/cosmos/cosmos-sdk/baseapp"
+)
+
+// AppVersionModifier defines the interface fulfilled by BaseApp
+// which allows getting and setting it's appVersion field. This
+// in turn updates the consensus params that are sent to the
+// consensus engine in EndBlock
+type AppVersionModifier baseapp.AppVersionModifier

x/upgrade/keeper/keeper.go

@@ -36,7 +36,7 @@ type Keeper struct {
 	storeKey           storetypes.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
+	versionModifier    xp.AppVersionModifier           // implements setting the protocol version field on BaseApp
 	downgradeVerified  bool                            // tells if we've already sanity checked that this binary version isn't being used against an old state.
 	authority          string                          // the address capable of executing and canceling an upgrade. Usually the gov module account
 	initVersionMap     module.VersionMap               // the module version map at init genesis
@@ -48,14 +48,14 @@ type Keeper struct {
 // 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 storetypes.StoreKey, cdc codec.BinaryCodec, homePath string, vs xp.ProtocolVersionSetter, authority string) *Keeper {
+func NewKeeper(skipUpgradeHeights map[int64]bool, storeKey storetypes.StoreKey, cdc codec.BinaryCodec, homePath string, vs xp.AppVersionModifier, authority string) *Keeper {
 	k := &Keeper{
 		homePath:           homePath,
 		skipUpgradeHeights: skipUpgradeHeights,
 		storeKey:           storeKey,
 		cdc:                cdc,
 		upgradeHandlers:    map[string]types.UpgradeHandler{},
-		versionSetter:      vs,
+		versionModifier:    vs,
 		authority:          authority,
 	}
 
@@ -66,16 +66,6 @@ func NewKeeper(skipUpgradeHeights map[int64]bool, storeKey storetypes.StoreKey,
 	return k
 }
 
-// SetVersionSetter sets the interface implemented by baseapp which allows setting baseapp's protocol version field
-func (k *Keeper) SetVersionSetter(vs xp.ProtocolVersionSetter) {
-	k.versionSetter = vs
-}
-
-// GetVersionSetter gets the protocol version field of baseapp
-func (k *Keeper) GetVersionSetter() xp.ProtocolVersionSetter {
-	return k.versionSetter
-}
-
 // SetInitVersionMap sets the initial version map.
 // This is only used in app wiring and should not be used in any other context.
 func (k *Keeper) SetInitVersionMap(vm module.VersionMap) {
@@ -95,28 +85,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 {
@@ -375,12 +343,16 @@ 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)
+	// incremement the app version and set it in state and baseapp
+	if k.versionModifier != nil {
+		currentAppVersion, err := k.versionModifier.AppVersion(ctx)
+		if err != nil {
+			panic(err)
+		}
+
+		if err := k.versionModifier.SetAppVersion(ctx, currentAppVersion + 1); err != nil {
+			panic(err)
+		}
 	}
 
 	// Must clear IBC state after upgrade is applied as it is stored separately from the upgrade plan.