refactor: app version stored in multistore, state sync snapshots share app version (testing on v10)

baseapp/abci.go

@@ -24,6 +24,16 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
+const (
+	initialAppVersion = 0
+)
+
+var (
+	errAppVersionIsNotInitial = func(actualVersion uint64) error {
+		return fmt.Errorf("app version is not initial, was %d, expected %d", actualVersion, initialAppVersion)
+	}
+)
+
 // InitChain implements the ABCI interface. It runs the initialization logic
 // directly on the CommitMultiStore.
 func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitChain) {
@@ -46,14 +56,20 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
 	app.setDeliverState(initHeader)
 	app.setCheckState(initHeader)
 
-	if err := app.SetAppVersion(app.deliverState.ctx, 0); err != nil {
+	if err := app.SetAppVersion(initialAppVersion); err != nil {
 		panic(err)
 	}
 
 	// Store the consensus params in the BaseApp's paramstore. Note, this must be
 	// done after the deliver state and context have been set as it's persisted
 	// to state.
 	if req.ConsensusParams != nil {
+		// When init chain is called, the app version should either be absent and determined by the application
+		// or set to 0. Panic if it's not.
+		if req.ConsensusParams.Version != nil && req.ConsensusParams.Version.AppVersion != initialAppVersion {
+			panic(errAppVersionIsNotInitial(req.ConsensusParams.Version.AppVersion))
+		}
+
 		app.StoreConsensusParams(app.deliverState.ctx, req.ConsensusParams)
 	}
 
@@ -112,10 +128,8 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
 func (app *BaseApp) Info(req abci.RequestInfo) abci.ResponseInfo {
 	lastCommitID := app.cms.LastCommitID()
 
-	appVersion, err := app.GetAppVersion(app.checkState.ctx)
-	if err != nil {
-		app.logger.Error("failed to get app version", err)
-	}
+	appVersion, err := app.GetAppVersion()
+	app.logger.Info("get app version", "app version", appVersion, "error", err)
 
 	return abci.ResponseInfo{
 		Data:             app.name,

baseapp/baseapp.go

@@ -1,7 +1,6 @@
 package baseapp
 
 import (
-	"errors"
 	"fmt"
 	"reflect"
 	"strings"
@@ -310,15 +309,12 @@ func (app *BaseApp) init() error {
 	// needed for the export command which inits from store but never calls initchain
 	app.setCheckState(tmproto.Header{})
 
-	// If there is no app version set in the store, we should set it to 0.
-	// Panic on any other error.
-	// If errMsgNoProtocolVersionSet, we assume that appVersion is assigned to be 0.
-	appVersion, err := app.GetAppVersion(app.checkState.ctx)
-	if err != nil && !errors.Is(err, errMsgNoProtocolVersionSet) {
+	appVersion, err := app.GetAppVersion()
+	if err != nil {
 		return err
 	}
 
-	if err := app.SetAppVersion(app.checkState.ctx, appVersion); err != nil {
+	if err := app.SetAppVersion(appVersion); err != nil {
 		return err
 	}
 	app.Seal()
@@ -436,13 +432,13 @@ func (app *BaseApp) GetConsensusParams(ctx sdk.Context) *abci.ConsensusParams {
 		cp.Validator = &vp
 	}
 
-	if app.paramStore.Has(ctx, ParamStoreKeyVersionParams) {
-		var vp tmproto.VersionParams
-
-		app.paramStore.Get(ctx, ParamStoreKeyVersionParams, &vp)
-		cp.Version = &vp
+	appVersion, err := app.cms.GetAppVersion()
+	if err != nil {
+		panic(err)
+	}
+	cp.Version = &tmproto.VersionParams{
+		AppVersion: appVersion,
 	}
-
 	return cp
 }
 
@@ -466,9 +462,8 @@ 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)
-	app.paramStore.Set(ctx, ParamStoreKeyVersionParams, cp.Version)
-	// 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.
+	// We do not store the version params here because they are
+	// persisted in multi stored which is used as the single source of truth.
 }
 
 // getMaximumBlockGas gets the maximum gas from the consensus params. It panics

baseapp/baseapp_test.go

@@ -53,8 +53,12 @@ func defaultLogger() log.Logger {
 }
 
 func newBaseApp(name string, options ...func(*BaseApp)) *BaseApp {
-	logger := defaultLogger()
 	db := dbm.NewMemDB()
+	return newBaseAppWithDb(name, db, options...)
+}
+
+func newBaseAppWithDb(name string, db dbm.DB, options ...func(*BaseApp)) *BaseApp {
+	logger := defaultLogger()
 	codec := codec.NewLegacyAmino()
 	registerTestCodec(codec)
 	return NewBaseApp(name, logger, db, testTxDecoder(codec), options...)
@@ -482,7 +486,7 @@ func TestInfo(t *testing.T) {
 	assert.Equal(t, int64(0), res.LastBlockHeight)
 	require.Equal(t, []uint8(nil), res.LastBlockAppHash)
 
-	appVersion, err := app.GetAppVersion(app.deliverState.ctx)
+	appVersion, err := app.GetAppVersion()
 	require.NoError(t, err)
 
 	assert.Equal(t, appVersion, res.AppVersion)
@@ -632,7 +636,7 @@ func TestInitChain_ProtocolVersionSetToZero(t *testing.T) {
 		},
 	)
 
-	protocolVersion, err := app.GetAppVersion(app.deliverState.ctx)
+	protocolVersion, err := app.GetAppVersion()
 	require.NoError(t, err)
 	require.Equal(t, uint64(0), protocolVersion)
 }
@@ -1823,8 +1827,8 @@ func TestListSnapshots(t *testing.T) {
 	defer teardown()
 
 	expected := abci.ResponseListSnapshots{Snapshots: []*abci.Snapshot{
-		{Height: 4, Format: 1, Chunks: 2},
-		{Height: 2, Format: 1, Chunks: 1},
+		{Height: 4, Format: 2, Chunks: 2},
+		{Height: 2, Format: 2, Chunks: 1},
 	}}
 
 	resp := app.ListSnapshots(abci.RequestListSnapshots{})
@@ -1867,7 +1871,7 @@ func TestSnapshotWithPruning(t *testing.T) {
 				pruningOpts:       pruningtypes.NewPruningOptions(pruningtypes.PruningNothing),
 			},
 			expectedSnapshots: []*abci.Snapshot{
-				{Height: 20, Format: 1, Chunks: 5},
+				{Height: 20, Format: 2, Chunks: 5},
 			},
 		},
 		"prune everything with snapshot": {
@@ -1879,7 +1883,7 @@ func TestSnapshotWithPruning(t *testing.T) {
 				pruningOpts:       pruningtypes.NewPruningOptions(pruningtypes.PruningEverything),
 			},
 			expectedSnapshots: []*abci.Snapshot{
-				{Height: 20, Format: 1, Chunks: 5},
+				{Height: 20, Format: 2, Chunks: 5},
 			},
 		},
 		"default pruning with snapshot": {
@@ -1891,7 +1895,7 @@ func TestSnapshotWithPruning(t *testing.T) {
 				pruningOpts:       pruningtypes.NewPruningOptions(pruningtypes.PruningDefault),
 			},
 			expectedSnapshots: []*abci.Snapshot{
-				{Height: 20, Format: 1, Chunks: 5},
+				{Height: 20, Format: 2, Chunks: 5},
 			},
 		},
 		"custom": {
@@ -1903,8 +1907,8 @@ func TestSnapshotWithPruning(t *testing.T) {
 				pruningOpts:       pruningtypes.NewCustomPruningOptions(12, 12),
 			},
 			expectedSnapshots: []*abci.Snapshot{
-				{Height: 25, Format: 1, Chunks: 6},
-				{Height: 20, Format: 1, Chunks: 5},
+				{Height: 25, Format: 2, Chunks: 6},
+				{Height: 20, Format: 2, Chunks: 5},
 			},
 		},
 		"no snapshots": {
@@ -1925,9 +1929,9 @@ func TestSnapshotWithPruning(t *testing.T) {
 				pruningOpts:       pruningtypes.NewPruningOptions(pruningtypes.PruningNothing),
 			},
 			expectedSnapshots: []*abci.Snapshot{
-				{Height: 9, Format: 1, Chunks: 2},
-				{Height: 6, Format: 1, Chunks: 2},
-				{Height: 3, Format: 1, Chunks: 1},
+				{Height: 9, Format: 2, Chunks: 2},
+				{Height: 6, Format: 2, Chunks: 2},
+				{Height: 3, Format: 2, Chunks: 1},
 			},
 		},
 	}
@@ -2004,13 +2008,13 @@ func TestLoadSnapshotChunk(t *testing.T) {
 		chunk       uint32
 		expectEmpty bool
 	}{
-		"Existing snapshot": {2, 1, 1, false},
-		"Missing height":    {100, 1, 1, true},
-		"Missing format":    {2, 2, 1, true},
-		"Missing chunk":     {2, 1, 9, true},
-		"Zero height":       {0, 1, 1, true},
+		"Existing snapshot": {2, 2, 1, false},
+		"Missing height":    {100, 2, 1, true},
+		"Missing format":    {2, 1, 1, true},
+		"Missing chunk":     {2, 2, 9, true},
+		"Zero height":       {0, 2, 1, true},
 		"Zero format":       {2, 0, 1, true},
-		"Zero chunk":        {2, 1, 0, false},
+		"Zero chunk":        {2, 2, 0, false},
 	}
 	for name, tc := range testcases {
 		tc := tc
@@ -2056,13 +2060,13 @@ func TestOfferSnapshot_Errors(t *testing.T) {
 			Height: 1, Format: 9, Chunks: 3, Hash: hash, Metadata: metadata,
 		}, abci.ResponseOfferSnapshot_REJECT_FORMAT},
 		"incorrect chunk count": {&abci.Snapshot{
-			Height: 1, Format: 1, Chunks: 2, Hash: hash, Metadata: metadata,
+			Height: 1, Format: 2, Chunks: 2, Hash: hash, Metadata: metadata,
 		}, abci.ResponseOfferSnapshot_REJECT},
 		"no chunks": {&abci.Snapshot{
-			Height: 1, Format: 1, Chunks: 0, Hash: hash, Metadata: metadata,
+			Height: 1, Format: 2, Chunks: 0, Hash: hash, Metadata: metadata,
 		}, abci.ResponseOfferSnapshot_REJECT},
 		"invalid metadata serialization": {&abci.Snapshot{
-			Height: 1, Format: 1, Chunks: 0, Hash: hash, Metadata: []byte{3, 1, 4},
+			Height: 1, Format: 2, Chunks: 0, Hash: hash, Metadata: []byte{3, 1, 4},
 		}, abci.ResponseOfferSnapshot_REJECT},
 	}
 	for name, tc := range testcases {
@@ -2443,11 +2447,11 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 }
 
 func TestBaseApp_Init_ProtocolVersion(t *testing.T) {
-	const versionNotSet = -1
+	const versionNotSet = 0
 
 	testcases := []struct {
 		name            string
-		protocolVersion int64
+		protocolVersion uint64
 	}{
 		{
 			name:            "no app version was set - set to 0",
@@ -2461,29 +2465,23 @@ func TestBaseApp_Init_ProtocolVersion(t *testing.T) {
 
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			app := newBaseApp(t.Name())
 			db := dbm.NewMemDB()
+			app := newBaseAppWithDb(t.Name(), db)
 
-			app.SetParamStore(&mock.ParamStore{db})
-
-			var expectedProtocolVersion uint64
 			if tc.protocolVersion != versionNotSet {
-				// Set version on another app with the same param store (db),
-				// pretending that the app version was set on the app in advance.
-				oldApp := newBaseApp(t.Name())
-				oldApp.SetParamStore(&mock.ParamStore{db})
-				require.NoError(t, oldApp.init())
-
-				expectedProtocolVersion = uint64(tc.protocolVersion)
-				require.NoError(t, oldApp.SetAppVersion(oldApp.checkState.ctx, expectedProtocolVersion))
+				err := app.cms.SetAppVersion(tc.protocolVersion)
+				require.NoError(t, err)
 			}
 
+			// recreate app
+			app = newBaseAppWithDb(t.Name(), db)
+
 			require.NoError(t, app.init())
 
-			actualProtocolVersion, err := app.GetAppVersion(app.checkState.ctx)
+			actualProtocolVersion, err := app.GetAppVersion()
 			require.NoError(t, err)
 
-			require.Equal(t, expectedProtocolVersion, actualProtocolVersion)
+			require.Equal(t, tc.protocolVersion, actualProtocolVersion)
 		})
 	}
 }

baseapp/options.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"io"
 
-	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	dbm "github.com/tendermint/tm-db"
 
 	"github.com/cosmos/cosmos-sdk/codec/types"
@@ -17,8 +16,7 @@ import (
 )
 
 var (
-	errMsgNilParamStore        = errors.New("paramStore was nil")
-	errMsgNoProtocolVersionSet = errors.New("param store did not have the app version set")
+	errMsgNilParamStore = errors.New("paramStore was nil")
 )
 
 // File for storing in-package BaseApp optional functions,
@@ -107,31 +105,19 @@ func (app *BaseApp) SetVersion(v string) {
 }
 
 // SetAppVersion sets the application's app version
-func (app *BaseApp) SetAppVersion(ctx sdk.Context, v uint64) error {
-	if app.paramStore == nil {
-		return errMsgNilParamStore
-	}
-
-	av := &tmproto.VersionParams{AppVersion: v}
-	app.paramStore.Set(ctx, ParamStoreKeyVersionParams, av)
-	return nil
+func (app *BaseApp) SetAppVersion(v uint64) error {
+	return app.cms.SetAppVersion(v)
 }
 
 // GetAppVersion gets the application's app version
 // an error, if any.
-func (app *BaseApp) GetAppVersion(ctx sdk.Context) (uint64, error) {
-	if app.paramStore == nil {
-		return 0, errMsgNilParamStore
-	}
+func (app *BaseApp) GetAppVersion() (uint64, error) {
+	appVersion, err := app.cms.GetAppVersion()
+	if err != nil {
+		return 0, err
 
-	if !app.paramStore.Has(ctx, ParamStoreKeyVersionParams) {
-		return 0, errMsgNoProtocolVersionSet
 	}
-
-	av := &tmproto.VersionParams{}
-	app.paramStore.Get(ctx, ParamStoreKeyVersionParams, av)
-
-	return av.AppVersion, nil
+	return appVersion, nil
 }
 
 func (app *BaseApp) SetDB(db dbm.DB) {

baseapp/params.go

@@ -18,7 +18,6 @@ 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