From 18e768c968e05660e4ad0a7f4cc3ea8a33d5c993 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Mon, 20 Nov 2023 17:39:44 +0300 Subject: [PATCH] feat!: add app version to param store (#360) This PR adds app version to the ParamStore meaning that app version is no longer just in memory but is also persisted. We need this for example when a node restarts as Tendermint will call Info and it must remember the correct app version. It's also important for ensuring that state sync works beyond v1 --- baseapp/abci.go | 13 +++++++++- baseapp/baseapp.go | 39 +++++++++++++++++++----------- baseapp/baseapp_test.go | 2 +- baseapp/options.go | 11 +++++++-- baseapp/params.go | 16 ++++++++++++ x/params/types/consensus_params.go | 3 +++ x/upgrade/exported/exported.go | 4 ++- x/upgrade/keeper/keeper.go | 2 +- x/upgrade/keeper/keeper_test.go | 4 +-- 9 files changed, 72 insertions(+), 22 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 4c2a06564b28..eb5971a5bf53 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -60,6 +60,9 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC // to state. if req.ConsensusParams != nil { app.StoreConsensusParams(app.deliverState.ctx, req.ConsensusParams) + if req.ConsensusParams.Version != nil { + app.appVersion = req.ConsensusParams.Version.AppVersion + } } if app.initChainer == nil { @@ -122,7 +125,15 @@ func (app *BaseApp) SetOption(req abci.RequestSetOption) (res abci.ResponseSetOp // Info implements the ABCI interface. func (app *BaseApp) Info(req abci.RequestInfo) abci.ResponseInfo { lastCommitID := app.cms.LastCommitID() - + // load the app version for a non zero height + if lastCommitID.Version > 0 { + ctx, err := app.createQueryContext(lastCommitID.Version, false) + if err != nil { + panic(err) + } + // get and set the app version + _ = app.AppVersion(ctx) + } return abci.ResponseInfo{ Data: app.name, Version: app.version, diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 41cc39eb62ef..c9877ddb012a 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -53,7 +53,6 @@ type BaseApp struct { // nolint: maligned postHandler sdk.AnteHandler // post handler, optional, e.g. for tips appStore - baseappVersions peerFilters snapshotData abciData @@ -98,6 +97,13 @@ type BaseApp struct { // nolint: maligned // ResponseCommit.RetainHeight. minRetainBlocks uint64 + // 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 @@ -145,15 +151,6 @@ type abciData struct { voteInfos []abci.VoteInfo } -type baseappVersions 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 -} - // should really get handled in some db struct // which then has a sub-item, persistence fields type snapshotData struct { @@ -206,7 +203,14 @@ func (app *BaseApp) Name() string { } // AppVersion returns the application's protocol version. -func (app *BaseApp) AppVersion() uint64 { +func (app *BaseApp) AppVersion(ctx sdk.Context) uint64 { + if app.appVersion == 0 && app.paramStore.Has(ctx, ParamStoreKeyVersionParams) { + var vp tmproto.VersionParams + + app.paramStore.Get(ctx, ParamStoreKeyVersionParams, &vp) + // set the app version + app.appVersion = vp.AppVersion + } return app.appVersion } @@ -482,7 +486,12 @@ func (app *BaseApp) GetConsensusParams(ctx sdk.Context) *abci.ConsensusParams { cp.Validator = &vp } - cp.Version = &tmproto.VersionParams{AppVersion: app.appVersion} + if app.paramStore.Has(ctx, ParamStoreKeyVersionParams) { + var vp tmproto.VersionParams + + app.paramStore.Get(ctx, ParamStoreKeyVersionParams, &vp) + cp.Version = &vp + } return cp } @@ -507,8 +516,10 @@ 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) - // 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. + // NOTE: we only persist the app version from v2 onwards + if cp.Version != nil && cp.Version.AppVersion >= 2 { + app.paramStore.Set(ctx, ParamStoreKeyVersionParams, cp.Version) + } } // getMaximumBlockGas gets the maximum gas from the consensus params. It panics diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 99e4541b5ea3..2a1ae556bcce 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -460,7 +460,7 @@ 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) + require.EqualValues(t, 0, res.AppVersion) // ----- test a proper response ------- // TODO } diff --git a/baseapp/options.go b/baseapp/options.go index 84b57e2e92e5..b3d4c20fb6cf 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -4,6 +4,7 @@ import ( "fmt" "io" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" dbm "github.com/tendermint/tm-db" "github.com/cosmos/cosmos-sdk/codec/types" @@ -110,8 +111,14 @@ func (app *BaseApp) SetVersion(v string) { app.version = v } -// SetProtocolVersion sets the application's protocol version -func (app *BaseApp) SetProtocolVersion(v uint64) { +// SetAppVersion sets the application's protocol version +func (app *BaseApp) SetAppVersion(ctx sdk.Context, v uint64) { + // TODO: could make this less hacky in the future since the SDK + // shouldn't have to know about the app versioning scheme + if ctx.BlockHeader().Version.App >= 2 { + vp := &tmproto.VersionParams{AppVersion: v} + app.paramStore.Set(ctx, ParamStoreKeyVersionParams, vp) + } app.appVersion = v } diff --git a/baseapp/params.go b/baseapp/params.go index 14701d524798..140379046b7b 100644 --- a/baseapp/params.go +++ b/baseapp/params.go @@ -18,6 +18,7 @@ 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 @@ -84,3 +85,18 @@ func ValidateValidatorParams(i interface{}) error { return nil } + +// ValidateVersionParams defines a stateless validation on VersionParams. This +// function is called whenever the parameters are updated or stored. +func ValidateVersionParams(i interface{}) error { + v, ok := i.(tmproto.VersionParams) + if !ok { + return fmt.Errorf("invalid parameter type: %T", i) + } + + if v.AppVersion == 0 { + return errors.New("application version must not be zero") + } + + return nil +} diff --git a/x/params/types/consensus_params.go b/x/params/types/consensus_params.go index fe27f7e8d9aa..e7fc59e38898 100644 --- a/x/params/types/consensus_params.go +++ b/x/params/types/consensus_params.go @@ -23,5 +23,8 @@ func ConsensusParamsKeyTable() KeyTable { NewParamSetPair( baseapp.ParamStoreKeyValidatorParams, tmproto.ValidatorParams{}, baseapp.ValidateValidatorParams, ), + NewParamSetPair( + baseapp.ParamStoreKeyVersionParams, tmproto.VersionParams{}, baseapp.ValidateVersionParams, + ), ) } diff --git a/x/upgrade/exported/exported.go b/x/upgrade/exported/exported.go index 859f7fe1f011..26ce10506463 100644 --- a/x/upgrade/exported/exported.go +++ b/x/upgrade/exported/exported.go @@ -1,7 +1,9 @@ package exported +import sdk "github.com/cosmos/cosmos-sdk/types" + // ProtocolVersionSetter defines the interface fulfilled by BaseApp // which allows setting it's appVersion field. type ProtocolVersionSetter interface { - SetProtocolVersion(uint64) + SetAppVersion(sdk.Context, uint64) } diff --git a/x/upgrade/keeper/keeper.go b/x/upgrade/keeper/keeper.go index c8edaceeb6b1..3ab3450e8102 100644 --- a/x/upgrade/keeper/keeper.go +++ b/x/upgrade/keeper/keeper.go @@ -346,7 +346,7 @@ func (k Keeper) ApplyUpgrade(ctx sdk.Context, plan types.Plan) { k.setProtocolVersion(ctx, nextProtocolVersion) if k.versionSetter != nil { // set protocol version on BaseApp - k.versionSetter.SetProtocolVersion(nextProtocolVersion) + k.versionSetter.SetAppVersion(ctx, nextProtocolVersion) } // Must clear IBC state after upgrade is applied as it is stored separately from the upgrade plan. diff --git a/x/upgrade/keeper/keeper_test.go b/x/upgrade/keeper/keeper_test.go index 19fee1054ec9..b37ebcb62339 100644 --- a/x/upgrade/keeper/keeper_test.go +++ b/x/upgrade/keeper/keeper_test.go @@ -203,7 +203,7 @@ func (s *KeeperTestSuite) TestSetUpgradedClient() { // Test that the protocol version successfully increments after an // upgrade and is successfully set on BaseApp's appVersion. func (s *KeeperTestSuite) TestIncrementProtocolVersion() { - oldProtocolVersion := s.app.BaseApp.AppVersion() + oldProtocolVersion := s.app.BaseApp.AppVersion(s.ctx) s.app.UpgradeKeeper.SetUpgradeHandler("dummy", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) { return vm, nil }) dummyPlan := types.Plan{ Name: "dummy", @@ -211,7 +211,7 @@ func (s *KeeperTestSuite) TestIncrementProtocolVersion() { Height: 100, } s.app.UpgradeKeeper.ApplyUpgrade(s.ctx, dummyPlan) - upgradedProtocolVersion := s.app.BaseApp.AppVersion() + upgradedProtocolVersion := s.app.BaseApp.AppVersion(s.ctx) s.Require().Equal(oldProtocolVersion+1, upgradedProtocolVersion) }