fix: state-sync - app version stored in params (#241) (#243)

Closes: #XXX

## What is the purpose of the change

Backporting and expanding on: cosmos#11182

This is an attempt to fix app version bug related to state-sync and tendermit. More context is in the linked-above PR.

Upon backporting all relevant commits from the linked-above PR, it was discovered that some integration and simulation tests were failing. With the original design, it was difficult to find the source of the problem due to storing the protocol version in 2 database locations. Therefore, the original work was significantly redesigned to only keep the protocol version in param store.

Additionally, the protocol version was assumed to have already been set. Since we are creating a new entry in the database, it is now possible that the protocol version is non-existent at the start. As a result, mechanisms for detecting a missing protocol version were added:
- `baseapp.init(...)`
   * This is called on every restart. Here, we check if the protocol version is already in db. If not, set it to 0
- `baseapp.InitChain(...)`
   * This is called once at genesis. Here, we always set protocol version to 0

Then, on every upgrade the version gets incremented by 1

## Brief Changelog

- cherry-picked all relevant commits from cosmos#11182
- fixed some issues
- addressed some style comments from code review in cosmos#11182
- redesigned to only store protocol version in params store and removed logic for storing it in the upgrade keeper
- added logic for detecting missing protocol version and, if missing, setting it to 0
- unit and integration tested
- fixed all tests

## Testing and Verifying

This change is a trivial rework / code cleanup without any test coverage.

This change is already covered by existing tests, such as *(please describe tests)*.

TODO: This change needs to be manually tested

## Documentation and Release Note

  - Does this pull request introduce a new feature or user-facing behavior changes? yes
  - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? no
  - How is the feature or change documented? not applicable

(cherry picked from commit a68d28e)

Co-authored-by: Roman <[email protected]>

baseapp/abci.go

@@ -45,6 +45,10 @@ 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 {
+		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.
@@ -107,10 +111,15 @@ 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)
+	}
+
 	return abci.ResponseInfo{
 		Data:             app.name,
 		Version:          app.version,
-		AppVersion:       app.appVersion,
+		AppVersion:       appVersion,
 		LastBlockHeight:  lastCommitID.Version,
 		LastBlockAppHash: lastCommitID.Hash,
 	}
@@ -742,6 +751,7 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) abci.Res
 			}
 
 		case "version":
+
 			return abci.ResponseQuery{
 				Codespace: sdkerrors.RootCodespace,
 				Height:    req.Height,

baseapp/abci_test.go

@@ -12,6 +12,7 @@ import (
 	pruningtypes "github.com/cosmos/cosmos-sdk/pruning/types"
 	"github.com/cosmos/cosmos-sdk/snapshots"
 	snapshottypes "github.com/cosmos/cosmos-sdk/snapshots/types"
+	"github.com/cosmos/cosmos-sdk/testutil/mock"
 	snaphotstestutil "github.com/cosmos/cosmos-sdk/testutil/snapshots"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
@@ -110,7 +111,7 @@ func TestGetBlockRentionHeight(t *testing.T) {
 	for name, tc := range testCases {
 		tc := tc
 
-		tc.bapp.SetParamStore(&paramStore{db: dbm.NewMemDB()})
+		tc.bapp.SetParamStore(&mock.ParamStore{Db: dbm.NewMemDB()})
 		tc.bapp.InitChain(abci.RequestInitChain{
 			ConsensusParams: &abci.ConsensusParams{
 				Evidence: &tmprototypes.EvidenceParams{

baseapp/baseapp.go

@@ -1,6 +1,7 @@
 package baseapp
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"strings"
@@ -19,6 +20,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx"
+	upgrade "github.com/cosmos/cosmos-sdk/x/upgrade/exported"
 )
 
 const (
@@ -114,13 +116,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
 
@@ -132,6 +130,8 @@ type BaseApp struct { // nolint: maligned
 	indexEvents map[string]struct{}
 }
 
+var _ upgrade.ProtocolVersionManager = (*BaseApp)(nil)
+
 // NewBaseApp returns a reference to an initialized BaseApp. It accepts a
 // variadic number of option functions, which act on the BaseApp to set
 // configuration choices.
@@ -172,11 +172,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
@@ -314,6 +309,18 @@ 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) {
+		return err
+	}
+
+	if err := app.SetAppVersion(app.checkState.ctx, appVersion); err != nil {
+		return err
+	}
 	app.Seal()
 
 	rms, ok := app.cms.(*rootmulti.Store)
@@ -429,6 +436,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
+	}
+
 	return cp
 }
 
@@ -452,6 +466,9 @@ 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.
 }
 
 // getMaximumBlockGas gets the maximum gas from the consensus params. It panics