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(x/crisis)!: use KVStoreService and context.Context #16216

Merged
merged 14 commits into from
May 19, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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 @@ -129,6 +129,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* (x/crisis) [#16216](https://github.com/cosmos/cosmos-sdk/issues/16216) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey`, methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context` and return an `error` instead of panicking.
* (x/gov) [#15988](https://github.com/cosmos/cosmos-sdk/issues/15988) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey`, methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context` and return an `error` (instead of panicking or returning a `found bool`). Iterators callback functions now return an error instead of a `bool`.
* (x/auth) [#15985](https://github.com/cosmos/cosmos-sdk/pull/15985) The `AccountKeeper` does not expose the `QueryServer` and `MsgServer` APIs anymore.
* (x/authz) [#15962](https://github.com/cosmos/cosmos-sdk/issues/15962) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey`, methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context`. The `Authorization` interface's `Accept` method now takes a `context.Context` instead of a `sdk.Context`.
Expand Down
2 changes: 2 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ The following modules `NewKeeper` function now take a `KVStoreService` instead o
* `x/authz`
* `x/bank`
* `x/consensus`
* `x/crisis`
* `x/distribution`
* `x/evidence`
* `x/feegrant`
Expand All @@ -98,6 +99,7 @@ The following modules' `Keeper` methods now take in a `context.Context` instead

* `x/authz`
* `x/bank`
* `x/crisis`
* `x/distribution`
* `x/evidence`
* `x/gov`
Expand Down
3 changes: 2 additions & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"cosmossdk.io/x/circuit"
circuitkeeper "cosmossdk.io/x/circuit/keeper"
circuittypes "cosmossdk.io/x/circuit/types"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
Expand Down Expand Up @@ -289,7 +290,7 @@ func NewSimApp(
)

invCheckPeriod := cast.ToUint(appOpts.Get(server.FlagInvCheckPeriod))
app.CrisisKeeper = crisiskeeper.NewKeeper(appCodec, keys[crisistypes.StoreKey], invCheckPeriod,
app.CrisisKeeper = crisiskeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[crisistypes.StoreKey]), invCheckPeriod,
app.BankKeeper, authtypes.FeeCollectorName, authtypes.NewModuleAddress(govtypes.ModuleName).String(), app.AccountKeeper.GetAddressCodec())

app.FeeGrantKeeper = feegrantkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[feegrant.StoreKey]), app.AccountKeeper)
Expand Down
8 changes: 5 additions & 3 deletions x/crisis/abci.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package crisis

import (
"context"
"time"

"github.com/cosmos/cosmos-sdk/telemetry"
Expand All @@ -10,12 +11,13 @@ import (
)

// check all registered invariants
func EndBlocker(ctx sdk.Context, k keeper.Keeper) {
func EndBlocker(ctx context.Context, k keeper.Keeper) {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker)

if k.InvCheckPeriod() == 0 || ctx.BlockHeight()%int64(k.InvCheckPeriod()) != 0 {
sdkCtx := sdk.UnwrapSDKContext(ctx)
if k.InvCheckPeriod() == 0 || sdkCtx.BlockHeight()%int64(k.InvCheckPeriod()) != 0 {
// skip running the invariant check
return
}
k.AssertInvariants(ctx)
k.AssertInvariants(sdkCtx)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

path flow from Begin/EndBlock to a panic call
}
5 changes: 4 additions & 1 deletion x/crisis/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ func (k *Keeper) InitGenesis(ctx sdk.Context, data *types.GenesisState) {

// ExportGenesis returns a GenesisState for a given context and keeper.
func (k *Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
constantFee := k.GetConstantFee(ctx)
constantFee, err := k.GetConstantFee(ctx)
if err != nil {
panic(err)
}
return types.NewGenesisState(constantFee)
}
7 changes: 5 additions & 2 deletions x/crisis/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/cosmos/cosmos-sdk/codec"
addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
"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 @@ -34,6 +35,7 @@ func TestGenesisTestSuite(t *testing.T) {

func (s *GenesisTestSuite) SetupTest() {
key := storetypes.NewKVStoreKey(types.StoreKey)
storeService := runtime.NewKVStoreService(key)
testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test"))
encCfg := moduletestutil.MakeTestEncodingConfig(crisis.AppModuleBasic{})

Expand All @@ -44,7 +46,7 @@ func (s *GenesisTestSuite) SetupTest() {

supplyKeeper := crisistestutil.NewMockSupplyKeeper(ctrl)

s.keeper = *keeper.NewKeeper(s.cdc, key, 5, supplyKeeper, "", "", addresscodec.NewBech32Codec("cosmos"))
s.keeper = *keeper.NewKeeper(s.cdc, storeService, 5, supplyKeeper, "", "", addresscodec.NewBech32Codec("cosmos"))
}

func (s *GenesisTestSuite) TestImportExportGenesis() {
Expand All @@ -69,6 +71,7 @@ func (s *GenesisTestSuite) TestInitGenesis() {
genesisState.ConstantFee = sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(1000))
s.keeper.InitGenesis(s.sdkCtx, genesisState)

constantFee := s.keeper.GetConstantFee(s.sdkCtx)
constantFee, err := s.keeper.GetConstantFee(s.sdkCtx)
s.Require().NoError(err)
s.Require().Equal(genesisState.ConstantFee, constantFee)
}
16 changes: 9 additions & 7 deletions x/crisis/keeper/keeper.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package keeper

import (
"context"
"fmt"
"time"

"cosmossdk.io/core/address"
"cosmossdk.io/log"

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

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -18,7 +19,7 @@ import (
type Keeper struct {
routes []types.InvarRoute
invCheckPeriod uint
storeKey storetypes.StoreKey
storeService storetypes.KVStoreService
cdc codec.BinaryCodec

// the address capable of executing a MsgUpdateParams message. Typically, this
Expand All @@ -34,11 +35,11 @@ type Keeper struct {

// NewKeeper creates a new Keeper object
func NewKeeper(
cdc codec.BinaryCodec, storeKey storetypes.StoreKey, invCheckPeriod uint,
cdc codec.BinaryCodec, storeService storetypes.KVStoreService, invCheckPeriod uint,
supplyKeeper types.SupplyKeeper, feeCollectorName, authority string, ac address.Codec,
) *Keeper {
return &Keeper{
storeKey: storeKey,
storeService: storeService,
cdc: cdc,
routes: make([]types.InvarRoute, 0),
invCheckPeriod: invCheckPeriod,
Expand All @@ -55,8 +56,9 @@ func (k *Keeper) GetAuthority() string {
}

// Logger returns a module-specific logger.
func (k *Keeper) Logger(ctx sdk.Context) log.Logger {
return ctx.Logger().With("module", "x/"+types.ModuleName)
func (k *Keeper) Logger(ctx context.Context) log.Logger {
sdkCtx := ctx.(sdk.Context)
facundomedica marked this conversation as resolved.
Show resolved Hide resolved
return sdkCtx.Logger().With("module", "x/"+types.ModuleName)
}

// RegisterRoute register the routes for each of the invariants
Expand Down Expand Up @@ -108,6 +110,6 @@ func (k *Keeper) AssertInvariants(ctx sdk.Context) {
func (k *Keeper) InvCheckPeriod() uint { return k.invCheckPeriod }

// SendCoinsFromAccountToFeeCollector transfers amt to the fee collector account.
func (k *Keeper) SendCoinsFromAccountToFeeCollector(ctx sdk.Context, senderAddr sdk.AccAddress, amt sdk.Coins) error {
func (k *Keeper) SendCoinsFromAccountToFeeCollector(ctx context.Context, senderAddr sdk.AccAddress, amt sdk.Coins) error {
return k.supplyKeeper.SendCoinsFromAccountToModule(ctx, senderAddr, k.feeCollectorName, amt)
}
10 changes: 7 additions & 3 deletions x/crisis/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"

addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
"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 @@ -23,9 +24,10 @@ func TestLogger(t *testing.T) {
supplyKeeper := crisistestutil.NewMockSupplyKeeper(ctrl)

key := storetypes.NewKVStoreKey(types.StoreKey)
storeService := runtime.NewKVStoreService(key)
testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test"))
encCfg := moduletestutil.MakeTestEncodingConfig(crisis.AppModuleBasic{})
keeper := keeper.NewKeeper(encCfg.Codec, key, 5, supplyKeeper, "", "", addresscodec.NewBech32Codec("cosmos"))
keeper := keeper.NewKeeper(encCfg.Codec, storeService, 5, supplyKeeper, "", "", addresscodec.NewBech32Codec("cosmos"))

require.Equal(t,
testCtx.Ctx.Logger().With("module", "x/"+types.ModuleName),
Expand All @@ -37,8 +39,9 @@ func TestInvariants(t *testing.T) {
supplyKeeper := crisistestutil.NewMockSupplyKeeper(ctrl)

key := storetypes.NewKVStoreKey(types.StoreKey)
storeService := runtime.NewKVStoreService(key)
encCfg := moduletestutil.MakeTestEncodingConfig(crisis.AppModuleBasic{})
keeper := keeper.NewKeeper(encCfg.Codec, key, 5, supplyKeeper, "", "", addresscodec.NewBech32Codec("cosmos"))
keeper := keeper.NewKeeper(encCfg.Codec, storeService, 5, supplyKeeper, "", "", addresscodec.NewBech32Codec("cosmos"))
require.Equal(t, keeper.InvCheckPeriod(), uint(5))

orgInvRoutes := keeper.Routes()
Expand All @@ -52,9 +55,10 @@ func TestAssertInvariants(t *testing.T) {
supplyKeeper := crisistestutil.NewMockSupplyKeeper(ctrl)

key := storetypes.NewKVStoreKey(types.StoreKey)
storeService := runtime.NewKVStoreService(key)
testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test"))
encCfg := moduletestutil.MakeTestEncodingConfig(crisis.AppModuleBasic{})
keeper := keeper.NewKeeper(encCfg.Codec, key, 5, supplyKeeper, "", "", addresscodec.NewBech32Codec("cosmos"))
keeper := keeper.NewKeeper(encCfg.Codec, storeService, 5, supplyKeeper, "", "", addresscodec.NewBech32Codec("cosmos"))

keeper.RegisterRoute("testModule", "testRoute1", func(sdk.Context) (string, bool) { return "", false })
require.NotPanics(t, func() { keeper.AssertInvariants(testCtx.Ctx) })
Expand Down
2 changes: 1 addition & 1 deletion x/crisis/keeper/migrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ func NewMigrator(k *Keeper, ss exported.Subspace) Migrator {
// and managed by the x/params modules and stores them directly into the x/crisis
// module state.
func (m Migrator) Migrate1to2(ctx sdk.Context) error {
return v2.MigrateStore(ctx, m.keeper.storeKey, m.legacySubspace, m.keeper.cdc)
return v2.MigrateStore(ctx, m.keeper.storeService, m.legacySubspace, m.keeper.cdc)
}
6 changes: 5 additions & 1 deletion x/crisis/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ func (k *Keeper) VerifyInvariant(goCtx context.Context, msg *types.MsgVerifyInva
}

ctx := sdk.UnwrapSDKContext(goCtx)
constantFee := sdk.NewCoins(k.GetConstantFee(ctx))
params, err := k.GetConstantFee(ctx)
if err != nil {
return nil, err
}
constantFee := sdk.NewCoins(params)

if err := k.SendCoinsFromAccountToFeeCollector(ctx, sender, constantFee); err != nil {
return nil, err
Expand Down
4 changes: 3 additions & 1 deletion x/crisis/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
"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 @@ -34,9 +35,10 @@ func (s *KeeperTestSuite) SetupTest() {
supplyKeeper := crisistestutil.NewMockSupplyKeeper(ctrl)

key := storetypes.NewKVStoreKey(types.StoreKey)
storeService := runtime.NewKVStoreService(key)
testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test"))
encCfg := moduletestutil.MakeTestEncodingConfig(crisis.AppModuleBasic{})
keeper := keeper.NewKeeper(encCfg.Codec, key, 5, supplyKeeper, "", sdk.AccAddress([]byte("addr1_______________")).String(), addresscodec.NewBech32Codec("cosmos"))
keeper := keeper.NewKeeper(encCfg.Codec, storeService, 5, supplyKeeper, "", sdk.AccAddress([]byte("addr1_______________")).String(), addresscodec.NewBech32Codec("cosmos"))

s.ctx = testCtx.Ctx
s.keeper = keeper
Expand Down
23 changes: 12 additions & 11 deletions x/crisis/keeper/params.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package keeper

import (
"context"

errorsmod "cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -9,28 +11,27 @@ import (
)

// GetConstantFee get's the constant fee from the store
func (k *Keeper) GetConstantFee(ctx sdk.Context) (constantFee sdk.Coin) {
store := ctx.KVStore(k.storeKey)
bz := store.Get(types.ConstantFeeKey)
if bz == nil {
return constantFee
func (k *Keeper) GetConstantFee(ctx context.Context) (constantFee sdk.Coin, err error) {
store := k.storeService.OpenKVStore(ctx)
bz, err := store.Get(types.ConstantFeeKey)
if bz == nil || err != nil {
return constantFee, err
}
k.cdc.MustUnmarshal(bz, &constantFee)
return constantFee
err = k.cdc.Unmarshal(bz, &constantFee)
return constantFee, err
}

// GetConstantFee set's the constant fee in the store
func (k *Keeper) SetConstantFee(ctx sdk.Context, constantFee sdk.Coin) error {
func (k *Keeper) SetConstantFee(ctx context.Context, constantFee sdk.Coin) error {
if !constantFee.IsValid() || constantFee.IsNegative() {
return errorsmod.Wrapf(errors.ErrInvalidCoins, "negative or invalid constant fee: %s", constantFee)
}

store := ctx.KVStore(k.storeKey)
store := k.storeService.OpenKVStore(ctx)
bz, err := k.cdc.Marshal(&constantFee)
if err != nil {
return err
}

store.Set(types.ConstantFeeKey, bz)
return nil
return store.Set(types.ConstantFeeKey, bz)
}
8 changes: 5 additions & 3 deletions x/crisis/keeper/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ func (s *KeeperTestSuite) TestParams() {
for _, tc := range testCases {
tc := tc
s.Run(tc.name, func() {
expected := s.keeper.GetConstantFee(s.ctx)
err := s.keeper.SetConstantFee(s.ctx, tc.constantFee)
expected, err := s.keeper.GetConstantFee(s.ctx)
s.Require().NoError(err)
err = s.keeper.SetConstantFee(s.ctx, tc.constantFee)

if tc.expErr {
s.Require().Error(err)
Expand All @@ -47,7 +48,8 @@ func (s *KeeperTestSuite) TestParams() {
s.Require().NoError(err)
}

params := s.keeper.GetConstantFee(s.ctx)
params, err := s.keeper.GetConstantFee(s.ctx)
s.Require().NoError(err)
s.Require().Equal(expected, params)
})
}
Expand Down
10 changes: 4 additions & 6 deletions x/crisis/migrations/v2/migrate.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package v2

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

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -22,8 +22,8 @@ var (
// version 2. Specifically, it takes the `ConstantFee` parameter that is currently stored
// and managed by the x/params module and stores it directly into the x/crisis
// module state.
func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, legacySubspace exported.Subspace, cdc codec.BinaryCodec) error {
store := ctx.KVStore(storeKey)
func MigrateStore(ctx sdk.Context, storeService storetypes.KVStoreService, legacySubspace exported.Subspace, cdc codec.BinaryCodec) error {
store := storeService.OpenKVStore(ctx)
var currConstantFee sdk.Coin
legacySubspace.Get(ctx, ConstantFee, &currConstantFee)

Expand All @@ -36,7 +36,5 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, legacySubspace
return err
}

store.Set(ConstantFeeKey, bz)

return nil
return store.Set(ConstantFeeKey, bz)
}
4 changes: 3 additions & 1 deletion x/crisis/migrations/v2/migrate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

storetypes "cosmossdk.io/store/types"

"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 @@ -30,12 +31,13 @@ func (ms mockSubspace) Get(ctx sdk.Context, key []byte, ptr interface{}) {
func TestMigrate(t *testing.T) {
cdc := moduletestutil.MakeTestEncodingConfig(crisis.AppModuleBasic{}).Codec
storeKey := storetypes.NewKVStoreKey(v2.ModuleName)
storeService := runtime.NewKVStoreService(storeKey)
tKey := storetypes.NewTransientStoreKey("transient_test")
ctx := testutil.DefaultContext(storeKey, tKey)
store := ctx.KVStore(storeKey)

legacySubspace := newMockSubspace(types.DefaultGenesisState().ConstantFee)
require.NoError(t, v2.MigrateStore(ctx, storeKey, legacySubspace, cdc))
require.NoError(t, v2.MigrateStore(ctx, storeService, legacySubspace, cdc))

var res sdk.Coin
bz := store.Get(v2.ConstantFeeKey)
Expand Down
Loading