fix: state-sync - app version stored in multi-store, state sync'd (co…

…smos#241)

Cherry-pick of:
a68d28e
c3bf09f

osmosis-labs/osmosis#1805

baseapp/abci.go

@@ -30,6 +30,16 @@ const (
 	QueryPathP2P    = "p2p"
 	QueryPathStore  = "store"
 )
+const initialAppVersion = 0
+
+type AppVersionError struct {
+	Actual  uint64
+	Initial uint64
+}
+
+func (e *AppVersionError) Error() string {
+	return fmt.Sprintf("app version (%d) is not initial (%d)", e.Actual, e.Initial)
+}
 
 // InitChain implements the ABCI interface. It runs the initialization logic
 // directly on the CommitMultiStore.
@@ -53,10 +63,20 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
 	app.setDeliverState(initHeader)
 	app.setCheckState(initHeader)
 
+	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 InitChain 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(AppVersionError{Actual: req.ConsensusParams.Version.AppVersion, Initial: initialAppVersion})
+		}
+
 		app.StoreConsensusParams(app.deliverState.ctx, req.ConsensusParams)
 	}
 
@@ -115,10 +135,15 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
 func (app *BaseApp) Info(req abci.RequestInfo) abci.ResponseInfo {
 	lastCommitID := app.store.LastCommitID()
 
+	appVersion, err := app.GetAppVersion()
+	if err != nil {
+		app.logger.Error("failed to get app version", err)
+	}
+
 	return abci.ResponseInfo{
 		Data:             app.name,
 		Version:          app.version,
-		AppVersion:       app.appVersion,
+		AppVersion:       appVersion,
 		LastBlockHeight:  lastCommitID.Version,
 		LastBlockAppHash: lastCommitID.Hash,
 	}
@@ -738,6 +763,7 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) abci.Res
 			}
 
 		case "version":
+
 			return abci.ResponseQuery{
 				Codespace: sdkerrors.RootCodespace,
 				Height:    req.Height,

baseapp/baseapp.go

@@ -19,6 +19,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
+	upgrade "github.com/cosmos/cosmos-sdk/x/upgrade/exported"
 )
 
 const (
@@ -32,6 +33,7 @@ const (
 )
 
 var _ abci.Application = (*BaseApp)(nil)
+var _ upgrade.AppVersionManager = (*BaseApp)(nil)
 
 type (
 	// Enum mode for app.runTx
@@ -133,13 +135,9 @@ type BaseApp struct { // nolint: maligned
 	// ResponseCommit.RetainHeight.
 	minRetainBlocks uint64
 
-	// application's version string
+	// version represents the application software semantic version
 	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
 
@@ -211,11 +209,6 @@ func (app *BaseApp) Name() string {
 	return app.name
 }
 
-// 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
@@ -292,6 +285,15 @@ func (app *BaseApp) Init() error {
 
 	// needed for the export command which inits from store but never calls initchain
 	app.setCheckState(tmproto.Header{})
+
+	appVersion, err := app.GetAppVersion()
+	if err != nil {
+		return err
+	}
+
+	if err := app.SetAppVersion(appVersion); err != nil {
+		return err
+	}
 	app.Seal()
 	return app.store.GetPruning().Validate()
 }
@@ -398,6 +400,13 @@ func (app *BaseApp) GetConsensusParams(ctx sdk.Context) *tmproto.ConsensusParams
 		cp.Validator = &vp
 	}
 
+	appVersion, err := app.store.GetAppVersion()
+	if err != nil {
+		panic(err)
+	}
+	cp.Version = &tmproto.VersionParams{
+		AppVersion: appVersion,
+	}
 	return cp
 }
 
@@ -421,8 +430,11 @@ func (app *BaseApp) StoreConsensusParams(ctx sdk.Context, cp *tmproto.ConsensusP
 	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.
+	// We do not store version param here because it is
+	// persisted in the multi-store which is used as the single source of truth.
+	// The reason for storing the version in multi-store is to be able
+	// to serialize it for state-sync snapshots. The app version is then
+	// deserialized by the state-synching node to be validated in Tendermint.
 }
 
 // getMaximumBlockGas gets the maximum gas from the consensus params. It panics

baseapp/baseapp_test.go

@@ -29,6 +29,7 @@ import (
 	stypes "github.com/cosmos/cosmos-sdk/store/v2alpha1"
 	"github.com/cosmos/cosmos-sdk/store/v2alpha1/multi"
 	"github.com/cosmos/cosmos-sdk/testutil"
+	"github.com/cosmos/cosmos-sdk/testutil/mock"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
@@ -44,13 +45,8 @@ var (
 	testTxPriority = int64(42)
 )
 
-type paramStore struct {
-	db *memdb.MemDB
-	rw dbm.DBReadWriter
-}
-
-func newParamStore(db *memdb.MemDB) *paramStore {
-	return &paramStore{db: db, rw: db.ReadWriter()}
+func newParamStore(db *memdb.MemDB) ParamStore {
+	return &mock.ParamStore{Db: db.ReadWriter()}
 }
 
 type setupConfig struct {
@@ -61,46 +57,16 @@ type setupConfig struct {
 	pruningOpts        pruningtypes.PruningOptions
 }
 
-func (ps *paramStore) Set(_ sdk.Context, key []byte, value interface{}) {
-	bz, err := json.Marshal(value)
-	if err != nil {
-		panic(err)
-	}
-
-	ps.rw.Set(key, bz)
-}
-
-func (ps *paramStore) Has(_ sdk.Context, key []byte) bool {
-	ok, err := ps.rw.Has(key)
-	if err != nil {
-		panic(err)
-	}
-
-	return ok
-}
-
-func (ps *paramStore) Get(_ sdk.Context, key []byte, ptr interface{}) {
-	bz, err := ps.rw.Get(key)
-	if err != nil {
-		panic(err)
-	}
-
-	if len(bz) == 0 {
-		return
-	}
-
-	if err := json.Unmarshal(bz, ptr); err != nil {
-		panic(err)
-	}
-}
-
 func defaultLogger() log.Logger {
 	return log.MustNewDefaultLogger("plain", "info", false).With("module", "sdk/app")
 }
 
 func newBaseApp(name string, options ...AppOption) *BaseApp {
+	return newBaseAppWithDB(name, memdb.NewDB(), options...)
+}
+
+func newBaseAppWithDB(name string, db dbm.DBConnection, options ...AppOption) *BaseApp {
 	logger := defaultLogger()
-	db := memdb.NewDB()
 	codec := codec.NewLegacyAmino()
 	registerTestCodec(codec)
 	return NewBaseApp(name, logger, db, testTxDecoder(codec), options...)
@@ -397,6 +363,7 @@ func TestTxDecoder(t *testing.T) {
 // Test that Info returns the latest committed state.
 func TestInfo(t *testing.T) {
 	app := newBaseApp(t.Name())
+	app.InitChain(abci.RequestInitChain{})
 
 	// ----- test an empty response -------
 	reqInfo := abci.RequestInfo{}
@@ -407,9 +374,11 @@ 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
+
+	appVersion, err := app.GetAppVersion()
+	require.NoError(t, err)
+
+	assert.Equal(t, appVersion, res.AppVersion)
 }
 
 func TestBaseAppOptionSeal(t *testing.T) {
@@ -536,6 +505,50 @@ func TestInitChainer(t *testing.T) {
 	require.Equal(t, value, res.Value)
 }
 
+func TestInitChain_AppVersionSetToZero(t *testing.T) {
+	const expectedAppVersion = uint64(0)
+
+	name := t.Name()
+	logger := defaultLogger()
+	app := NewBaseApp(name, logger, memdb.NewDB(), nil)
+	app.SetParamStore(newParamStore(memdb.NewDB()))
+
+	app.InitChain(
+		abci.RequestInitChain{
+			InitialHeight: 3,
+		},
+	)
+
+	protocolVersion, err := app.GetAppVersion()
+	require.NoError(t, err)
+	require.Equal(t, expectedAppVersion, protocolVersion)
+
+	consensusParams := app.GetConsensusParams(app.checkState.ctx)
+
+	require.NotNil(t, consensusParams)
+	require.Equal(t, expectedAppVersion, consensusParams.Version.AppVersion)
+}
+
+func TestInitChain_NonZeroAppVersionInRequestPanic(t *testing.T) {
+	name := t.Name()
+	logger := defaultLogger()
+	app := NewBaseApp(name, logger, memdb.NewDB(), nil)
+
+	sut := func() {
+		app.InitChain(
+			abci.RequestInitChain{
+				InitialHeight: 3,
+				ConsensusParams: &tmproto.ConsensusParams{
+					Version: &tmproto.VersionParams{
+						AppVersion: 10,
+					},
+				},
+			},
+		)
+	}
+	require.Panics(t, sut)
+}
+
 func TestInitChain_WithInitialHeight(t *testing.T) {
 	name := t.Name()
 	db := memdb.NewDB()
@@ -1654,7 +1667,7 @@ func TestSnapshotWithPruning(t *testing.T) {
 				pruningOpts:        pruningtypes.NewPruningOptions(pruningtypes.PruningNothing),
 			},
 			expectedSnapshots: []*abci.Snapshot{
-				{Height: 20, Format: 2, Chunks: 5},
+				{Height: 20, Format: snapshottypes.CurrentFormat, Chunks: 5},
 			},
 		},
 		"prune everything with snapshot": {
@@ -1666,7 +1679,7 @@ func TestSnapshotWithPruning(t *testing.T) {
 				pruningOpts:        pruningtypes.NewPruningOptions(pruningtypes.PruningEverything),
 			},
 			expectedSnapshots: []*abci.Snapshot{
-				{Height: 20, Format: 2, Chunks: 5},
+				{Height: 20, Format: snapshottypes.CurrentFormat, Chunks: 5},
 			},
 		},
 		"default pruning with snapshot": {
@@ -1678,7 +1691,7 @@ func TestSnapshotWithPruning(t *testing.T) {
 				pruningOpts:        pruningtypes.NewPruningOptions(pruningtypes.PruningDefault),
 			},
 			expectedSnapshots: []*abci.Snapshot{
-				{Height: 20, Format: 2, Chunks: 5},
+				{Height: 20, Format: snapshottypes.CurrentFormat, Chunks: 5},
 			},
 		},
 		"custom": {
@@ -1690,8 +1703,8 @@ func TestSnapshotWithPruning(t *testing.T) {
 				pruningOpts:        pruningtypes.NewCustomPruningOptions(12, 12),
 			},
 			expectedSnapshots: []*abci.Snapshot{
-				{Height: 25, Format: 2, Chunks: 6},
-				{Height: 20, Format: 2, Chunks: 5},
+				{Height: 25, Format: snapshottypes.CurrentFormat, Chunks: 6},
+				{Height: 20, Format: snapshottypes.CurrentFormat, Chunks: 5},
 			},
 		},
 		"no snapshots": {
@@ -1712,9 +1725,9 @@ func TestSnapshotWithPruning(t *testing.T) {
 				pruningOpts:        pruningtypes.NewPruningOptions(pruningtypes.PruningNothing),
 			},
 			expectedSnapshots: []*abci.Snapshot{
-				{Height: 9, Format: 2, Chunks: 2},
-				{Height: 6, Format: 2, Chunks: 2},
-				{Height: 3, Format: 2, Chunks: 1},
+				{Height: 9, Format: snapshottypes.CurrentFormat, Chunks: 2},
+				{Height: 6, Format: snapshottypes.CurrentFormat, Chunks: 2},
+				{Height: 3, Format: snapshottypes.CurrentFormat, Chunks: 1},
 			},
 		},
 	}
@@ -2219,3 +2232,43 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 		require.Equal(t, tc.expectedSnapshot.KeepRecent, tc.bapp.snapshotManager.GetKeepRecent(), k)
 	}
 }
+
+func TestBaseApp_Init_AppVersion(t *testing.T) {
+	const versionNotSet = 0
+
+	testcases := []struct {
+		name            string
+		protocolVersion uint64
+	}{
+		{
+			name:            "no app version was set - set to 0",
+			protocolVersion: versionNotSet,
+		},
+		{
+			name:            "app version was set to 10 - 10 kept",
+			protocolVersion: 10,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			db := memdb.NewDB()
+			app := newBaseAppWithDB(t.Name(), db)
+
+			if tc.protocolVersion != versionNotSet {
+				err := app.store.SetAppVersion(tc.protocolVersion)
+				require.NoError(t, err)
+			}
+			require.NoError(t, app.CloseStore())
+
+			// recreate app
+			app = newBaseAppWithDB(t.Name(), db)
+			require.NoError(t, app.Init())
+
+			actualProtocolVersion, err := app.GetAppVersion()
+			require.NoError(t, err)
+
+			require.Equal(t, tc.protocolVersion, actualProtocolVersion)
+		})
+	}
+}