From eefd41969f5bfb12f1a939d5b116cc18ae4fcb44 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Tue, 12 Mar 2024 16:44:33 +0100 Subject: [PATCH 1/3] fix: add app version in the context in InitChain --- baseapp/abci.go | 21 +++++++++++---------- baseapp/baseapp.go | 8 +++++--- baseapp/options.go | 4 +--- types/module/module_test.go | 1 + 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index bb9fb0566ab5..46c9ab14b51f 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -14,6 +14,7 @@ import ( "github.com/gogo/protobuf/proto" abci "github.com/tendermint/tendermint/abci/types" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + tmversion "github.com/tendermint/tendermint/proto/tendermint/version" "google.golang.org/grpc/codes" grpcstatus "google.golang.org/grpc/status" @@ -37,13 +38,17 @@ const ( func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitChain) { // On a new chain, we consider the init chain block height as 0, even though // req.InitialHeight is 1 by default. - initHeader := tmproto.Header{ChainID: req.ChainId, Time: req.Time} + initHeader := tmproto.Header{ + ChainID: req.ChainId, + Time: req.Time, + Version: tmversion.Consensus{App: req.ConsensusParams.Version.AppVersion}, + } // If req.InitialHeight is > 1, then we set the initial version in the // stores. if req.InitialHeight > 1 { app.initialHeight = req.InitialHeight - initHeader = tmproto.Header{ChainID: req.ChainId, Height: req.InitialHeight, Time: req.Time} + initHeader.Height = req.InitialHeight err := app.cms.SetInitialVersion(req.InitialHeight) if err != nil { panic(err) @@ -58,12 +63,8 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC // 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 { - app.StoreConsensusParams(app.deliverState.ctx, req.ConsensusParams) - if req.ConsensusParams.Version != nil { - app.appVersion = req.ConsensusParams.Version.AppVersion - } - } + app.StoreConsensusParams(app.deliverState.ctx, req.ConsensusParams) + app.appVersion = req.ConsensusParams.Version.AppVersion if app.initChainer == nil { return res @@ -131,8 +132,8 @@ func (app *BaseApp) Info(req abci.RequestInfo) abci.ResponseInfo { if err != nil { panic(err) } - // get and set the app version - _ = app.AppVersion(ctx) + // initialise the app version by checking if it is already in state + app.InitAppVersion(ctx) } return abci.ResponseInfo{ Data: app.name, diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index c9877ddb012a..04274f31ecf2 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -202,15 +202,17 @@ func (app *BaseApp) Name() string { return app.name } -// AppVersion returns the application's protocol version. -func (app *BaseApp) AppVersion(ctx sdk.Context) uint64 { +func (app *BaseApp) InitAppVersion(ctx sdk.Context) { 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 } +} + +// AppVersion returns the application's protocol version. +func (app *BaseApp) AppVersion() uint64 { return app.appVersion } diff --git a/baseapp/options.go b/baseapp/options.go index ba29aa977f36..953ede102e17 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -113,9 +113,7 @@ func (app *BaseApp) SetVersion(v string) { // SetAppVersion sets the application's protocol version func (app *BaseApp) SetAppVersion(ctx sdk.Context, version uint64) { - // TODO: could make this less hacky in the future since the SDK - // shouldn't have to know about the app versioning scheme - if version >= 2 { + if app.paramStore.Has(ctx, ParamStoreKeyVersionParams) { vp := &tmproto.VersionParams{AppVersion: version} app.paramStore.Set(ctx, ParamStoreKeyVersionParams, vp) } diff --git a/types/module/module_test.go b/types/module/module_test.go index 926a4a4ab3f8..ce9d14ba7aff 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -280,5 +280,6 @@ func TestManager_EndBlock(t *testing.T) { // test panic mockAppModule1.EXPECT().EndBlock(gomock.Any(), gomock.Eq(req)).Times(1).Return([]abci.ValidatorUpdate{{}}) mockAppModule2.EXPECT().EndBlock(gomock.Any(), gomock.Eq(req)).Times(1).Return([]abci.ValidatorUpdate{{}}) + mm.EndBlock(sdk.Context{}, req) require.Panics(t, func() { mm.EndBlock(sdk.Context{}, req) }) } From 250b66579d27f0cb256d441fe34648615733ed58 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Tue, 12 Mar 2024 18:01:55 +0100 Subject: [PATCH 2/3] make setting the app version optional --- baseapp/abci.go | 15 +++++++++------ x/upgrade/keeper/keeper_test.go | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 46c9ab14b51f..daad19ba165f 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -38,10 +38,9 @@ const ( func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitChain) { // On a new chain, we consider the init chain block height as 0, even though // req.InitialHeight is 1 by default. - initHeader := tmproto.Header{ - ChainID: req.ChainId, - Time: req.Time, - Version: tmversion.Consensus{App: req.ConsensusParams.Version.AppVersion}, + initHeader := tmproto.Header{ChainID: req.ChainId, Time: req.Time} + if req.ConsensusParams != nil && req.ConsensusParams.Version != nil { + initHeader.Version = tmversion.Consensus{App: req.ConsensusParams.Version.AppVersion} } // If req.InitialHeight is > 1, then we set the initial version in the @@ -63,8 +62,12 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC // 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. - app.StoreConsensusParams(app.deliverState.ctx, req.ConsensusParams) - app.appVersion = req.ConsensusParams.Version.AppVersion + 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 { return res diff --git a/x/upgrade/keeper/keeper_test.go b/x/upgrade/keeper/keeper_test.go index b37ebcb62339..19fee1054ec9 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(s.ctx) + oldProtocolVersion := s.app.BaseApp.AppVersion() 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(s.ctx) + upgradedProtocolVersion := s.app.BaseApp.AppVersion() s.Require().Equal(oldProtocolVersion+1, upgradedProtocolVersion) } From ba0911f474b292e7839510bdf5ccc6721ec41288 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Tue, 12 Mar 2024 18:11:08 +0100 Subject: [PATCH 3/3] remove endblock call --- types/module/module_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/types/module/module_test.go b/types/module/module_test.go index ce9d14ba7aff..926a4a4ab3f8 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -280,6 +280,5 @@ func TestManager_EndBlock(t *testing.T) { // test panic mockAppModule1.EXPECT().EndBlock(gomock.Any(), gomock.Eq(req)).Times(1).Return([]abci.ValidatorUpdate{{}}) mockAppModule2.EXPECT().EndBlock(gomock.Any(), gomock.Eq(req)).Times(1).Return([]abci.ValidatorUpdate{{}}) - mm.EndBlock(sdk.Context{}, req) require.Panics(t, func() { mm.EndBlock(sdk.Context{}, req) }) }