Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: use KVStoreService in x/consensus #15517

Merged
merged 20 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* (x/consensus) [#15517](https://github.com/cosmos/cosmos-sdk/pull/15517) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey`.
* (x/bank) [#15477](https://github.com/cosmos/cosmos-sdk/pull/15477) `banktypes.NewMsgMultiSend` and `keeper.InputOutputCoins` only accept one input.
* (mempool) [#15328](https://github.com/cosmos/cosmos-sdk/pull/15328) The `PriorityNonceMempool` is now generic over type `C comparable` and takes a single `PriorityNonceMempoolConfig[C]` argument. See `DefaultPriorityNonceMempoolConfig` for how to construct the configuration and a `TxPriority` type.
* (server) [#15358](https://github.com/cosmos/cosmos-sdk/pull/15358) Remove `server.ErrorCode` that was not used anywhere.
Expand Down
9 changes: 9 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,15 @@ All the store imports are now renamed to use `cosmossdk.io/store` instead of `gi

Capability was moved to [IBC-GO](https://github.com/cosmos/ibc-go). IBC V8 will contain the necessary changes to incorporate the new module location

#### `x/consensus`

The `NewKeeper` method now takes a `KVStoreService` instead of a `StoreKey`. When not using depinject, the `runtime.NewKVStoreService` method can be used to create a `KVStoreService` from a `StoreKey`.

```diff
- app.ConsensusParamsKeeper = consensusparamkeeper.NewKeeper(appCodec, keys[consensusparamtypes.StoreKey], authtypes.NewModuleAddress(govtypes.ModuleName).String())
+ app.ConsensusParamsKeeper = consensusparamkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[consensusparamtypes.StoreKey]), authtypes.NewModuleAddress(govtypes.ModuleName).String())
```

#### `x/gov`

##### Expedited Proposals
Expand Down
5 changes: 4 additions & 1 deletion baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
// 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)
err := app.StoreConsensusParams(app.deliverState.ctx, req.ConsensusParams)
if err != nil {
panic(err)
}
}

if app.initChainer == nil {
Expand Down
6 changes: 3 additions & 3 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,16 +448,16 @@ func (app *BaseApp) GetConsensusParams(ctx sdk.Context) *cmtproto.ConsensusParam
}

// StoreConsensusParams sets the consensus parameters to the baseapp's param store.
func (app *BaseApp) StoreConsensusParams(ctx sdk.Context, cp *cmtproto.ConsensusParams) {
func (app *BaseApp) StoreConsensusParams(ctx sdk.Context, cp *cmtproto.ConsensusParams) error {
if app.paramStore == nil {
panic("cannot store consensus params with no params store set")
}

if cp == nil {
return
return nil
}

app.paramStore.Set(ctx, cp)
return app.paramStore.Set(ctx, cp)
// We're explicitly not storing the CometBFT app_version in the param store. It's
// stored instead in the x/upgrade store, with its own bump logic.
}
Expand Down
10 changes: 5 additions & 5 deletions baseapp/params.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package baseapp

import (
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
"context"

sdk "github.com/cosmos/cosmos-sdk/types"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
)

// ParamStore defines the interface the parameter store used by the BaseApp must
// fulfill.
type ParamStore interface {
Get(ctx sdk.Context) (*cmtproto.ConsensusParams, error)
Has(ctx sdk.Context) bool
Set(ctx sdk.Context, cp *cmtproto.ConsensusParams)
Get(ctx context.Context) (*cmtproto.ConsensusParams, error)
Has(ctx context.Context) (bool, error)
Set(ctx context.Context, cp *cmtproto.ConsensusParams) error
}
23 changes: 10 additions & 13 deletions baseapp/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,28 +239,25 @@ type paramStore struct {
db *dbm.MemDB
}

func (ps *paramStore) Set(_ sdk.Context, value *cmtproto.ConsensusParams) {
var _ baseapp.ParamStore = (*paramStore)(nil)

func (ps *paramStore) Set(_ context.Context, value *cmtproto.ConsensusParams) error {
bz, err := json.Marshal(value)
if err != nil {
panic(err)
return err
}

ps.db.Set(ParamStoreKey, bz)
return ps.db.Set(ParamStoreKey, bz)
}

func (ps *paramStore) Has(_ sdk.Context) bool {
ok, err := ps.db.Has(ParamStoreKey)
if err != nil {
panic(err)
}

return ok
func (ps *paramStore) Has(_ context.Context) (bool, error) {
return ps.db.Has(ParamStoreKey)
}

func (ps paramStore) Get(ctx sdk.Context) (*cmtproto.ConsensusParams, error) {
func (ps paramStore) Get(ctx context.Context) (*cmtproto.ConsensusParams, error) {
bz, err := ps.db.Get(ParamStoreKey)
if err != nil {
panic(err)
return nil, err
}

if len(bz) == 0 {
Expand All @@ -269,7 +266,7 @@ func (ps paramStore) Get(ctx sdk.Context) (*cmtproto.ConsensusParams, error) {

var params cmtproto.ConsensusParams
if err := json.Unmarshal(bz, &params); err != nil {
panic(err)
return nil, err
}

return &params, nil
Expand Down
2 changes: 1 addition & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ func NewSimApp(
app.ParamsKeeper = initParamsKeeper(appCodec, legacyAmino, keys[paramstypes.StoreKey], tkeys[paramstypes.TStoreKey])

// set the BaseApp's parameter store
app.ConsensusParamsKeeper = consensusparamkeeper.NewKeeper(appCodec, keys[consensusparamtypes.StoreKey], authtypes.NewModuleAddress(govtypes.ModuleName).String())
app.ConsensusParamsKeeper = consensusparamkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[consensusparamtypes.StoreKey]), authtypes.NewModuleAddress(govtypes.ModuleName).String())
bApp.SetParamStore(&app.ConsensusParamsKeeper)

// add keepers
Expand Down
8 changes: 5 additions & 3 deletions x/consensus/exported/exported.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package exported

import (
"context"

cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -18,8 +20,8 @@ type (
// ConsensusParamSetter defines the interface fulfilled by BaseApp's
// ParamStore which allows setting its appVersion field.
ConsensusParamSetter interface {
Get(ctx sdk.Context) (*cmtproto.ConsensusParams, error)
Has(ctx sdk.Context) bool
Set(ctx sdk.Context, cp *cmtproto.ConsensusParams)
Get(ctx context.Context) (*cmtproto.ConsensusParams, error)
Has(ctx context.Context) (bool, error)
Set(ctx context.Context, cp *cmtproto.ConsensusParams) error
}
)
39 changes: 21 additions & 18 deletions x/consensus/keeper/keeper.go
Original file line number Diff line number Diff line change
@@ -1,30 +1,31 @@
package keeper

import (
"context"

cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"

storetypes "cosmossdk.io/store/types"
storetypes "cosmossdk.io/core/store"

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/consensus/exported"
"github.com/cosmos/cosmos-sdk/x/consensus/types"
)

var _ exported.ConsensusParamSetter = (*Keeper)(nil)

type Keeper struct {
storeKey storetypes.StoreKey
cdc codec.BinaryCodec
storeService storetypes.KVStoreService
cdc codec.BinaryCodec

authority string
}

func NewKeeper(cdc codec.BinaryCodec, storeKey storetypes.StoreKey, authority string) Keeper {
func NewKeeper(cdc codec.BinaryCodec, storeService storetypes.KVStoreService, authority string) Keeper {
return Keeper{
storeKey: storeKey,
cdc: cdc,
authority: authority,
storeService: storeService,
cdc: cdc,
authority: authority,
}
}

Expand All @@ -33,27 +34,29 @@ func (k *Keeper) GetAuthority() string {
}

// Get gets the consensus parameters
func (k *Keeper) Get(ctx sdk.Context) (*cmtproto.ConsensusParams, error) {
store := ctx.KVStore(k.storeKey)
func (k *Keeper) Get(ctx context.Context) (*cmtproto.ConsensusParams, error) {
store := k.storeService.OpenKVStore(ctx)

cp := &cmtproto.ConsensusParams{}
bz := store.Get(types.ParamStoreKeyConsensusParams)
bz, err := store.Get(types.ParamStoreKeyConsensusParams)
if err != nil {
return nil, err
}

cp := &cmtproto.ConsensusParams{}
if err := k.cdc.Unmarshal(bz, cp); err != nil {
return nil, err
}

return cp, nil
}

func (k *Keeper) Has(ctx sdk.Context) bool {
store := ctx.KVStore(k.storeKey)

func (k *Keeper) Has(ctx context.Context) (bool, error) {
store := k.storeService.OpenKVStore(ctx)
return store.Has(types.ParamStoreKeyConsensusParams)
}

// Set sets the consensus parameters
func (k *Keeper) Set(ctx sdk.Context, cp *cmtproto.ConsensusParams) {
store := ctx.KVStore(k.storeKey)
store.Set(types.ParamStoreKeyConsensusParams, k.cdc.MustMarshal(cp))
func (k *Keeper) Set(ctx context.Context, cp *cmtproto.ConsensusParams) error {
store := k.storeService.OpenKVStore(ctx)
return store.Set(types.ParamStoreKeyConsensusParams, k.cdc.MustMarshal(cp))
}
4 changes: 3 additions & 1 deletion x/consensus/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
storetypes "cosmossdk.io/store/types"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil"
sdk "github.com/cosmos/cosmos-sdk/types"
moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
Expand All @@ -32,8 +33,9 @@ func (s *KeeperTestSuite) SetupTest() {
testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test"))
ctx := testCtx.Ctx.WithBlockHeader(cmtproto.Header{})
encCfg := moduletestutil.MakeTestEncodingConfig()
storeService := runtime.NewKVStoreService(key)

keeper := consensusparamkeeper.NewKeeper(encCfg.Codec, key, authtypes.NewModuleAddress(govtypes.ModuleName).String())
keeper := consensusparamkeeper.NewKeeper(encCfg.Codec, storeService, authtypes.NewModuleAddress(govtypes.ModuleName).String())

s.ctx = ctx
s.consensusParamsKeeper = &keeper
Expand Down
10 changes: 5 additions & 5 deletions x/consensus/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime"
"github.com/spf13/cobra"

store "cosmossdk.io/store/types"
storetypes "cosmossdk.io/core/store"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/client"
Expand Down Expand Up @@ -139,9 +139,9 @@ func init() {
type ConsensusInputs struct {
depinject.In

Config *modulev1.Module
Cdc codec.Codec
Key *store.KVStoreKey
Config *modulev1.Module
Cdc codec.Codec
StoreService storetypes.KVStoreService
}

//nolint:revive
Expand All @@ -160,7 +160,7 @@ func ProvideModule(in ConsensusInputs) ConsensusOutputs {
authority = authtypes.NewModuleAddressOrBech32Address(in.Config.Authority)
}

k := keeper.NewKeeper(in.Cdc, in.Key, authority.String())
k := keeper.NewKeeper(in.Cdc, in.StoreService, authority.String())
m := NewAppModule(in.Cdc, k)
baseappOpt := func(app *baseapp.BaseApp) {
app.SetParamStore(&k)
Expand Down