Skip to content

Commit

Permalink
refactor(x/crisis)!: use KVStoreService and context.Context (#16216)
Browse files Browse the repository at this point in the history
  • Loading branch information
facundomedica authored May 19, 2023
1 parent bda51f2 commit 30aa579
Show file tree
Hide file tree
Showing 16 changed files with 75 additions and 50 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,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)
}
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 := sdk.UnwrapSDKContext(ctx)
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

0 comments on commit 30aa579

Please sign in to comment.