feat!: connect app version with consensus params in end block (#16244)

Co-authored-by: Marko <marko@baricevic.me>
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: marbar3778 <marbar3778@yahoo.com>

CHANGELOG.md

@@ -113,6 +113,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (x/slashing) [#16441](https://github.com/cosmos/cosmos-sdk/pull/16441) Params state is migrated to collections. `GetParams` has been removed.
 * (types) [#16918](https://github.com/cosmos/cosmos-sdk/pull/16918) Remove `IntProto` and `DecProto`. Instead, `math.Int` and `math.LegacyDec` should be used respectively. Both types support `Marshal` and `Unmarshal` which should be used for binary marshaling.
 * (client) [#17215](https://github.com/cosmos/cosmos-sdk/pull/17215) `server.StartCmd`,`server.ExportCmd`,`server.NewRollbackCmd`,`pruning.Cmd`,`genutilcli.InitCmd`,`genutilcli.GenTxCmd`,`genutilcli.CollectGenTxsCmd`,`genutilcli.AddGenesisAccountCmd`, do not take a home directory anymore. It is inferred from the root 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`.
 * (types) [#17348](https://github.com/cosmos/cosmos-sdk/pull/17348) Remove the `WrapServiceResult` function.
     * The `*sdk.Result` returned by the msg server router will not contain the `.Data` field.
 * (x/staking) [#17335](https://github.com/cosmos/cosmos-sdk/pull/17335) Remove usage of `"github.com/cosmos/cosmos-sdk/x/staking/types".Infraction_*` in favour of `"cosmossdk.io/api/cosmos/staking/v1beta1".Infraction_` in order to remove dependency between modules on staking
@@ -125,6 +126,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### State Machine Breaking
 
 * (x/distribution) [#17115](https://github.com/cosmos/cosmos-sdk/pull/17115) Migrate `PreviousProposer` to collections.
+* (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.
 
 ## [v0.50.0-rc.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.0-rc.0) - 2023-08-18
 

baseapp/abci.go

@@ -149,11 +149,22 @@ func (app *BaseApp) InitChain(req *abci.RequestInitChain) (*abci.ResponseInitCha
 
 func (app *BaseApp) Info(req *abci.RequestInfo) (*abci.ResponseInfo, error) {
 	lastCommitID := app.cms.LastCommitID()
+	appVersion := InitialAppVersion
+	if lastCommitID.Version > 0 {
+		ctx, err := app.CreateQueryContext(lastCommitID.Version, false)
+		if err != nil {
+			return nil, fmt.Errorf("failed creating query context: %w", err)
+		}
+		appVersion, err = app.AppVersion(ctx)
+		if err != nil {
+			return nil, fmt.Errorf("failed getting app version: %w", err)
+		}
+	}
 
 	return &abci.ResponseInfo{
 		Data:             app.name,
 		Version:          app.version,
-		AppVersion:       app.appVersion,
+		AppVersion:       appVersion,
 		LastBlockHeight:  lastCommitID.Version,
 		LastBlockAppHash: lastCommitID.Hash,
 	}, nil

baseapp/abci_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/cometbft/cometbft/crypto/secp256k1"
 	cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto"
 	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
+	cmttypes "github.com/cometbft/cometbft/types"
 	dbm "github.com/cosmos/cosmos-db"
 	protoio "github.com/cosmos/gogoproto/io"
 	"github.com/cosmos/gogoproto/jsonpb"
@@ -44,6 +45,9 @@ import (
 
 func TestABCI_Info(t *testing.T) {
 	suite := NewBaseAppSuite(t)
+	ctx := suite.baseApp.NewContext(true)
+	err := suite.baseApp.StoreConsensusParams(ctx, cmttypes.DefaultConsensusParams().ToProto())
+	require.NoError(t, err)
 
 	reqInfo := abci.RequestInfo{}
 	res, err := suite.baseApp.Info(&reqInfo)
@@ -53,7 +57,18 @@ 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)
+
+	_, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1})
+	require.NoError(t, err)
+	_, err = suite.baseApp.Commit()
+	require.NoError(t, err)
+	require.NoError(t, suite.baseApp.SetAppVersion(ctx, 1))
+	res, err = suite.baseApp.Info(&reqInfo)
+	require.NoError(t, err)
+	require.Equal(t, uint64(1), res.AppVersion)
 }
 
 func TestABCI_First_block_Height(t *testing.T) {

baseapp/baseapp.go

@@ -167,10 +167,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
 
@@ -249,8 +245,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("failed to get consensus params: %w", err)
+	}
+	if cp.Version == nil {
+		return 0, nil
+	}
+	return cp.Version.App, nil
 }
 
 // Version returns the application's version string.
@@ -522,9 +529,6 @@ func (app *BaseApp) GetConsensusParams(ctx sdk.Context) cmtproto.ConsensusParams
 
 // StoreConsensusParams sets the consensus parameters to the BaseApp's param
 // store.
-//
-// NOTE: We're explicitly not storing the CometBFT app_version in the param store.
-// It's stored instead in the x/upgrade store, with its own bump logic.
 func (app *BaseApp) StoreConsensusParams(ctx sdk.Context, cp cmtproto.ConsensusParams) error {
 	if app.paramStore == nil {
 		return errors.New("cannot store consensus params with no params store set")

baseapp/options.go

@@ -1,6 +1,8 @@
 package baseapp
 
 import (
+	"context"
+	"errors"
 	"fmt"
 	"io"
 	"math"
@@ -129,9 +131,25 @@ 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 fmt.Errorf("failed to get consensus params: %w", err)
+	}
+	if cp.Version == nil {
+		return errors.New("version is not set in param store")
+	}
+	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

@@ -75,6 +75,7 @@ func init() {
 			ProvideHeaderInfoService,
 			ProvideCometInfoService,
 			ProvideBasicManager,
+			ProvideAppVersionModifier,
 			ProvideAddressCodec,
 		),
 		appmodule.Invoke(SetupAppBuilder),
@@ -258,6 +259,10 @@ func ProvideBasicManager(app *AppBuilder) module.BasicManager {
 	return app.app.basicManager
 }
 
+func ProvideAppVersionModifier(app *AppBuilder) baseapp.AppVersionModifier {
+	return app.app
+}
+
 type (
 	// ValidatorAddressCodec is an alias for address.Codec for validator addresses.
 	ValidatorAddressCodec address.Codec

simapp/app_test.go

@@ -284,8 +284,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

simapp/go.mod

@@ -10,6 +10,7 @@ require (
 	cosmossdk.io/depinject v1.0.0-alpha.4
 	cosmossdk.io/log v1.2.1
 	cosmossdk.io/math v1.1.2
+	cosmossdk.io/collections v0.4.0
 	cosmossdk.io/store v1.0.0-rc.0
 	cosmossdk.io/tools/confix v0.0.0-20230613133644-0a778132a60f
 	cosmossdk.io/x/circuit v0.0.0-20230613133644-0a778132a60f

x/upgrade/abci_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
 	"github.com/stretchr/testify/require"
 
 	"cosmossdk.io/core/appmodule"
@@ -121,8 +122,9 @@ func setupTest(t *testing.T, height int64, skip map[int64]bool) *TestSuite {
 		s.encCfg.TxConfig.TxDecoder(),
 	)
 
-	s.keeper = keeper.NewKeeper(skip, storeService, s.encCfg.Codec, t.TempDir(), nil, authtypes.NewModuleAddress(govtypes.ModuleName).String())
-	s.keeper.SetVersionSetter(s.baseApp)
+	s.baseApp.SetParamStore(&paramStore{params: cmtproto.ConsensusParams{Version: &cmtproto.VersionParams{App: 1}}})
+
+	s.keeper = keeper.NewKeeper(skip, storeService, s.encCfg.Codec, t.TempDir(), s.baseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
 
 	s.ctx = testCtx.Ctx.WithHeaderInfo(header.Info{Time: time.Now(), Height: height})
 
@@ -527,3 +529,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/go.mod

@@ -178,6 +178,4 @@ require (
 	sigs.k8s.io/yaml v1.3.0 // indirect
 )
 
-// Fix upstream GHSA-h395-qcrw-5vmq and GHSA-3vp4-m3rf-835h vulnerabilities.
-// TODO Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409
-replace github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.9.1
+replace github.com/cosmos/cosmos-sdk => ../../.