diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index b380fc2c5169..05c0a5ce1638 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -2,6 +2,7 @@ package baseapp import ( "context" + "cosmossdk.io/core/server" "errors" "fmt" "maps" @@ -89,6 +90,7 @@ type BaseApp struct { verifyVoteExt sdk.VerifyVoteExtensionHandler // ABCI VerifyVoteExtension handler prepareCheckStater sdk.PrepareCheckStater // logic to run during commit using the checkState precommiter sdk.Precommiter // logic to run during commit using the deliverState + versionModifier server.VersionModifier // interface to get and set the app version addrPeerFilter sdk.PeerFilter // filter peers by address and port idPeerFilter sdk.PeerFilter // filter peers by node ID @@ -254,18 +256,11 @@ func (app *BaseApp) Name() string { // AppVersion returns the application's protocol version. func (app *BaseApp) AppVersion(ctx context.Context) (uint64, error) { - if app.paramStore == nil { - return 0, errors.New("app.paramStore is nil") + if app.versionModifier == nil { + return 0, errors.New("app.versionModifier is nil") } - cp, err := app.paramStore.Get(ctx) - if err != nil { - return 0, fmt.Errorf("failed to get consensus params: %w", err) - } - if cp.Version == nil { - return 0, nil - } - return cp.Version.App, nil + return app.versionModifier.AppVersion(ctx) } // Version returns the application's version string. diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index ecaf6894b400..22b046cf8a63 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -81,6 +81,7 @@ func NewBaseAppSuite(t *testing.T, opts ...func(*baseapp.BaseApp)) *BaseAppSuite app.SetParamStore(paramStore{db: dbm.NewMemDB()}) app.SetTxDecoder(txConfig.TxDecoder()) app.SetTxEncoder(txConfig.TxEncoder()) + app.SetVersionModifier(newMockedVersionModifier(0)) // mount stores and seal require.Nil(t, app.LoadLatestVersion()) diff --git a/baseapp/options.go b/baseapp/options.go index 9b657f4136cf..05c4467a0ef4 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -2,6 +2,7 @@ package baseapp import ( "context" + "cosmossdk.io/core/server" "errors" "fmt" "io" @@ -159,22 +160,11 @@ func (app *BaseApp) SetVersion(v string) { // SetAppVersion sets the application's version this is used as part of the // header in blocks and is returned to the consensus engine in EndBlock. func (app *BaseApp) SetAppVersion(ctx context.Context, v uint64) error { - if app.paramStore == nil { - return errors.New("param store must be set to set app version") + if app.versionModifier == nil { + return errors.New("version modifier must be set to set app version") } - cp, err := app.paramStore.Get(ctx) - if err != nil { - return fmt.Errorf("failed to get consensus params: %w", err) - } - if cp.Version == nil { - return errors.New("version is not set in param store") - } - cp.Version.App = v - if err := app.paramStore.Set(ctx, cp); err != nil { - return err - } - return nil + return app.versionModifier.SetAppVersion(ctx, v) } func (app *BaseApp) SetDB(db corestore.KVStoreWithBatch) { @@ -336,6 +326,15 @@ func (app *BaseApp) SetTxEncoder(txEncoder sdk.TxEncoder) { app.txEncoder = txEncoder } +// SetVersionModifier sets the version modifier for the BaseApp that allows to set the app version. +func (app *BaseApp) SetVersionModifier(versionModifier server.VersionModifier) { + if app.sealed { + panic("SetVersionModifier() on sealed BaseApp") + } + + app.versionModifier = versionModifier +} + // SetQueryMultiStore set a alternative MultiStore implementation to support grpc query service. // // Ref: https://github.com/cosmos/cosmos-sdk/issues/13317 diff --git a/baseapp/utils_test.go b/baseapp/utils_test.go index 970aa0cedce5..3d1a867100f1 100644 --- a/baseapp/utils_test.go +++ b/baseapp/utils_test.go @@ -3,6 +3,7 @@ package baseapp_test import ( "bytes" "context" + "cosmossdk.io/core/server" "encoding/binary" "encoding/json" "errors" @@ -445,3 +446,20 @@ func (n NestedMessgesServerImpl) Check(ctx context.Context, message *baseapptest sdkCtx.GasMeter().ConsumeGas(gas, "nested messages test") return nil, nil } + +func newMockedVersionModifier(startingVersion uint64) server.VersionModifier { + return &mockedVersionModifier{version: startingVersion} +} + +type mockedVersionModifier struct { + version uint64 +} + +func (m *mockedVersionModifier) SetAppVersion(ctx context.Context, u uint64) error { + m.version = u + return nil +} + +func (m *mockedVersionModifier) AppVersion(ctx context.Context) (uint64, error) { + return m.version, nil +} diff --git a/core/server/app.go b/core/server/app.go index a785de5011a7..9576fa8825f8 100644 --- a/core/server/app.go +++ b/core/server/app.go @@ -50,7 +50,7 @@ type TxResult struct { } // VersionModifier defines the interface fulfilled by BaseApp -// which allows getting and setting it's appVersion field. This +// which allows getting and setting its appVersion field. This // in turn updates the consensus params that are sent to the // consensus engine in EndBlock type VersionModifier interface { diff --git a/runtime/module.go b/runtime/module.go index 2b2a9bb1b9df..95595fc209f8 100644 --- a/runtime/module.go +++ b/runtime/module.go @@ -15,7 +15,6 @@ import ( "cosmossdk.io/core/appmodule" "cosmossdk.io/core/comet" "cosmossdk.io/core/registry" - "cosmossdk.io/core/server" "cosmossdk.io/core/store" "cosmossdk.io/depinject" "cosmossdk.io/depinject/appconfig" @@ -103,7 +102,6 @@ func init() { ProvideEnvironment, ProvideTransientStoreService, ProvideModuleManager, - ProvideAppVersionModifier, ProvideCometService, ), appconfig.Invoke(SetupAppBuilder), @@ -293,10 +291,6 @@ func ProvideTransientStoreService( return transientStoreService{key: storeKey} } -func ProvideAppVersionModifier(app *AppBuilder) server.VersionModifier { - return app.app -} - func ProvideCometService() comet.Service { return NewContextAwareCometInfoService() } diff --git a/runtime/v2/module.go b/runtime/v2/module.go index d67c31b7a9e5..1cda0a577fd4 100644 --- a/runtime/v2/module.go +++ b/runtime/v2/module.go @@ -18,7 +18,6 @@ import ( appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/comet" "cosmossdk.io/core/registry" - "cosmossdk.io/core/server" "cosmossdk.io/core/store" "cosmossdk.io/core/transaction" "cosmossdk.io/depinject" @@ -98,7 +97,6 @@ func init() { ProvideEnvironment[transaction.Tx], ProvideModuleManager[transaction.Tx], ProvideCometService, - ProvideAppVersionModifier[transaction.Tx], ), appconfig.Invoke(SetupAppBuilder), ) @@ -238,9 +236,3 @@ func storeKeyOverride(config *runtimev2.Module, moduleName string) *runtimev2.St func ProvideCometService() comet.Service { return &services.ContextAwareCometInfoService{} } - -// ProvideAppVersionModifier returns nil, `app.VersionModifier` is a feature of BaseApp and neither used nor required for runtime/v2. -// nil is acceptable, see: https://github.com/cosmos/cosmos-sdk/blob/0a6ee406a02477ae8ccbfcbe1b51fc3930087f4c/x/upgrade/keeper/keeper.go#L438 -func ProvideAppVersionModifier[T transaction.Tx](app *AppBuilder[T]) server.VersionModifier { - return nil -} diff --git a/server/v2/stf/core_branch_service.go b/server/v2/stf/core_branch_service.go index 431730b2334a..5a83dff6be7c 100644 --- a/server/v2/stf/core_branch_service.go +++ b/server/v2/stf/core_branch_service.go @@ -41,7 +41,7 @@ func (bs BranchService) ExecuteWithGasLimit( // restore original context gasUsed = exCtx.meter.Limit() - exCtx.meter.Remaining() _ = originalGasMeter.Consume(gasUsed, "execute-with-gas-limit") - exCtx.setGasLimit(originalGasMeter.Limit() - originalGasMeter.Remaining()) + exCtx.setGasLimit(originalGasMeter.Remaining()) return gasUsed, err } @@ -62,6 +62,8 @@ func (bs BranchService) execute(ctx *executionContext, f func(ctx context.Contex branchFn: ctx.branchFn, makeGasMeter: ctx.makeGasMeter, makeGasMeteredStore: ctx.makeGasMeteredStore, + msgRouter: ctx.msgRouter, + queryRouter: ctx.queryRouter, } err := f(branchedCtx) diff --git a/server/v2/stf/core_branch_service_test.go b/server/v2/stf/core_branch_service_test.go index a54783b605e7..0394ce0f3b1a 100644 --- a/server/v2/stf/core_branch_service_test.go +++ b/server/v2/stf/core_branch_service_test.go @@ -5,12 +5,11 @@ import ( "errors" "testing" - gogotypes "github.com/cosmos/gogoproto/types" - appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/server/v2/stf/branch" "cosmossdk.io/server/v2/stf/gas" "cosmossdk.io/server/v2/stf/mock" + gogotypes "github.com/cosmos/gogoproto/types" ) func TestBranchService(t *testing.T) { @@ -71,6 +70,7 @@ func TestBranchService(t *testing.T) { t.Run("fail - reverts state", func(t *testing.T) { stfCtx := makeContext() + originalGas := stfCtx.meter.Remaining() gasUsed, err := branchService.ExecuteWithGasLimit(stfCtx, 10000, func(ctx context.Context) error { kvSet(t, ctx, "cookies") return errors.New("fail") @@ -81,6 +81,10 @@ func TestBranchService(t *testing.T) { if gasUsed == 0 { t.Error("expected non-zero gasUsed") } + if stfCtx.meter.Remaining() != originalGas-gasUsed { + t.Error("expected gas to be reverted") + } + stateNotHas(t, stfCtx.state, "cookies") }) diff --git a/simapp/CHANGELOG.md b/simapp/CHANGELOG.md index 5e6dae65ec2b..678870d8b74a 100644 --- a/simapp/CHANGELOG.md +++ b/simapp/CHANGELOG.md @@ -46,6 +46,8 @@ Always refer to the [UPGRADING.md](https://github.com/cosmos/cosmos-sdk/blob/mai * [#20490](https://github.com/cosmos/cosmos-sdk/pull/20490) Refactor simulations to make use of `testutil/sims` instead of `runsims`. * [#19726](https://github.com/cosmos/cosmos-sdk/pull/19726) Update APIs to match CometBFT v1. * [#21466](https://github.com/cosmos/cosmos-sdk/pull/21466) Allow chains to plug in their own public key types in `base.Account` +* [#21508](https://github.com/cosmos/cosmos-sdk/pull/21508) Abstract the way we update the version of the app state in `app.go` using the interface `VersionModifier`. + ## v0.47 to v0.50 diff --git a/simapp/app.go b/simapp/app.go index e15c14c0d9e5..6cf93e4046d0 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -296,6 +296,9 @@ func NewSimApp( app.ConsensusParamsKeeper = consensusparamkeeper.NewKeeper(appCodec, runtime.NewEnvironment(runtime.NewKVStoreService(keys[consensustypes.StoreKey]), logger.With(log.ModuleKey, "x/consensus")), govModuleAddr) bApp.SetParamStore(app.ConsensusParamsKeeper.ParamsStore) + // set the version modifier + bApp.SetVersionModifier(consensus.ProvideAppVersionModifier(app.ConsensusParamsKeeper)) + // add keepers accountsKeeper, err := accounts.NewKeeper( appCodec, diff --git a/x/consensus/depinject.go b/x/consensus/depinject.go index 127144d91a51..07048b4c8684 100644 --- a/x/consensus/depinject.go +++ b/x/consensus/depinject.go @@ -1,13 +1,14 @@ package consensus import ( + "context" modulev1 "cosmossdk.io/api/cosmos/consensus/module/v1" "cosmossdk.io/core/address" "cosmossdk.io/core/appmodule" + "cosmossdk.io/core/server" "cosmossdk.io/depinject" "cosmossdk.io/depinject/appconfig" "cosmossdk.io/x/consensus/keeper" - "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/runtime" @@ -23,6 +24,7 @@ func init() { appconfig.RegisterModule( &modulev1.Module{}, appconfig.Provide(ProvideModule), + appconfig.Provide(ProvideAppVersionModifier), ) } @@ -64,6 +66,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { m := NewAppModule(in.Cdc, k) baseappOpt := func(app *baseapp.BaseApp) { app.SetParamStore(k.ParamsStore) + app.SetVersionModifier(versionModifier{Keeper: k}) } return ModuleOutputs{ @@ -72,3 +75,37 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { BaseAppOption: baseappOpt, } } + +type versionModifier struct { + Keeper keeper.Keeper +} + +func (v versionModifier) SetAppVersion(ctx context.Context, version uint64) error { + params, err := v.Keeper.Params(ctx, nil) + if err != nil { + return err + } + + updatedParams := params.Params + updatedParams.Version.App = version + + err = v.Keeper.ParamsStore.Set(ctx, *updatedParams) + if err != nil { + return err + } + + return nil +} + +func (v versionModifier) AppVersion(ctx context.Context) (uint64, error) { + params, err := v.Keeper.Params(ctx, nil) + if err != nil { + return 0, err + } + + return params.Params.Version.GetApp(), nil +} + +func ProvideAppVersionModifier(k keeper.Keeper) server.VersionModifier { + return versionModifier{Keeper: k} +}