Skip to content

Commit

Permalink
feat: unify version modifier for v2 (#21508)
Browse files Browse the repository at this point in the history
  • Loading branch information
randygrok authored Sep 6, 2024
1 parent e9d72f5 commit dce0365
Show file tree
Hide file tree
Showing 12 changed files with 90 additions and 43 deletions.
15 changes: 5 additions & 10 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package baseapp

import (
"context"
"cosmossdk.io/core/server"
"errors"
"fmt"
"maps"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
27 changes: 13 additions & 14 deletions baseapp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package baseapp

import (
"context"
"cosmossdk.io/core/server"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions baseapp/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package baseapp_test
import (
"bytes"
"context"
"cosmossdk.io/core/server"
"encoding/binary"
"encoding/json"
"errors"
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion core/server/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 0 additions & 6 deletions runtime/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -103,7 +102,6 @@ func init() {
ProvideEnvironment,
ProvideTransientStoreService,
ProvideModuleManager,
ProvideAppVersionModifier,
ProvideCometService,
),
appconfig.Invoke(SetupAppBuilder),
Expand Down Expand Up @@ -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()
}
8 changes: 0 additions & 8 deletions runtime/v2/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -98,7 +97,6 @@ func init() {
ProvideEnvironment[transaction.Tx],
ProvideModuleManager[transaction.Tx],
ProvideCometService,
ProvideAppVersionModifier[transaction.Tx],
),
appconfig.Invoke(SetupAppBuilder),
)
Expand Down Expand Up @@ -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
}
4 changes: 3 additions & 1 deletion server/v2/stf/core_branch_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions server/v2/stf/core_branch_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
Expand All @@ -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")
})

Expand Down
2 changes: 2 additions & 0 deletions simapp/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

<!-- TODO: move changelog.md elements to here -->

## v0.47 to v0.50
Expand Down
3 changes: 3 additions & 0 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
39 changes: 38 additions & 1 deletion x/consensus/depinject.go
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -23,6 +24,7 @@ func init() {
appconfig.RegisterModule(
&modulev1.Module{},
appconfig.Provide(ProvideModule),
appconfig.Provide(ProvideAppVersionModifier),
)
}

Expand Down Expand Up @@ -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{
Expand All @@ -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}
}

0 comments on commit dce0365

Please sign in to comment.