From f358214b43f8d20ead225d3031e7e7830ad46f1e Mon Sep 17 00:00:00 2001 From: testinginprod <98415576+testinginprod@users.noreply.github.com> Date: Mon, 29 May 2023 19:12:57 +0200 Subject: [PATCH] refactor(crisis)!: use collections for state management (#16328) Co-authored-by: unknown unknown --- CHANGELOG.md | 2 ++ x/crisis/keeper/genesis.go | 4 +-- x/crisis/keeper/genesis_test.go | 6 ++-- x/crisis/keeper/keeper.go | 15 +++++++- x/crisis/keeper/msg_server.go | 4 +-- x/crisis/keeper/msg_server_test.go | 2 +- x/crisis/keeper/params.go | 37 -------------------- x/crisis/keeper/params_test.go | 56 ------------------------------ x/crisis/types/genesis.go | 3 ++ x/crisis/types/keys.go | 4 ++- 10 files changed, 30 insertions(+), 103 deletions(-) delete mode 100644 x/crisis/keeper/params.go delete mode 100644 x/crisis/keeper/params_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 11528158ad..06fc1183c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -240,6 +240,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * this finalizes the gov collections migration * Removed: keeper `InsertActiveProposalsQueue`, `RemoveActiveProposalsQueue`, `InsertInactiveProposalsQueue`, `RemoveInactiveProposalsQueue`, `IterateInactiveProposalsQueue`, `IterateActiveProposalsQueue`, `ActiveProposalsQueueIterator`, `InactiveProposalsQueueIterator` * Remove: types all the key related functions +* (x/crisis) [#16328](https://github.com/cosmos/cosmos-sdk/pull/16328) Use collections for state management: + * Removed: keeper `GetConstantFee`, `SetConstantFee` ### Client Breaking Changes diff --git a/x/crisis/keeper/genesis.go b/x/crisis/keeper/genesis.go index f44657736b..e68dc05d58 100644 --- a/x/crisis/keeper/genesis.go +++ b/x/crisis/keeper/genesis.go @@ -7,14 +7,14 @@ import ( // new crisis genesis func (k *Keeper) InitGenesis(ctx sdk.Context, data *types.GenesisState) { - if err := k.SetConstantFee(ctx, data.ConstantFee); err != nil { + if err := k.ConstantFee.Set(ctx, data.ConstantFee); err != nil { panic(err) } } // ExportGenesis returns a GenesisState for a given context and keeper. func (k *Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState { - constantFee, err := k.GetConstantFee(ctx) + constantFee, err := k.ConstantFee.Get(ctx) if err != nil { panic(err) } diff --git a/x/crisis/keeper/genesis_test.go b/x/crisis/keeper/genesis_test.go index ba1e0ef6ba..f97ab21f1b 100644 --- a/x/crisis/keeper/genesis_test.go +++ b/x/crisis/keeper/genesis_test.go @@ -52,13 +52,13 @@ func (s *GenesisTestSuite) SetupTest() { func (s *GenesisTestSuite) TestImportExportGenesis() { // default params constantFee := sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(1000)) - err := s.keeper.SetConstantFee(s.sdkCtx, constantFee) + err := s.keeper.ConstantFee.Set(s.sdkCtx, constantFee) s.Require().NoError(err) genesis := s.keeper.ExportGenesis(s.sdkCtx) // set constant fee to zero constantFee = sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(0)) - err = s.keeper.SetConstantFee(s.sdkCtx, constantFee) + err = s.keeper.ConstantFee.Set(s.sdkCtx, constantFee) s.Require().NoError(err) s.keeper.InitGenesis(s.sdkCtx, genesis) @@ -71,7 +71,7 @@ func (s *GenesisTestSuite) TestInitGenesis() { genesisState.ConstantFee = sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(1000)) s.keeper.InitGenesis(s.sdkCtx, genesisState) - constantFee, err := s.keeper.GetConstantFee(s.sdkCtx) + constantFee, err := s.keeper.ConstantFee.Get(s.sdkCtx) s.Require().NoError(err) s.Require().Equal(genesisState.ConstantFee, constantFee) } diff --git a/x/crisis/keeper/keeper.go b/x/crisis/keeper/keeper.go index da9d9fa911..6c8e24ae99 100644 --- a/x/crisis/keeper/keeper.go +++ b/x/crisis/keeper/keeper.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + "cosmossdk.io/collections" "cosmossdk.io/core/address" "cosmossdk.io/log" @@ -31,6 +32,9 @@ type Keeper struct { feeCollectorName string // name of the FeeCollector ModuleAccount addressCodec address.Codec + + Schema collections.Schema + ConstantFee collections.Item[sdk.Coin] } // NewKeeper creates a new Keeper object @@ -38,7 +42,8 @@ func NewKeeper( cdc codec.BinaryCodec, storeService storetypes.KVStoreService, invCheckPeriod uint, supplyKeeper types.SupplyKeeper, feeCollectorName, authority string, ac address.Codec, ) *Keeper { - return &Keeper{ + sb := collections.NewSchemaBuilder(storeService) + k := &Keeper{ storeService: storeService, cdc: cdc, routes: make([]types.InvarRoute, 0), @@ -47,7 +52,15 @@ func NewKeeper( feeCollectorName: feeCollectorName, authority: authority, addressCodec: ac, + + ConstantFee: collections.NewItem(sb, types.ConstantFeeKey, "constant_fee", codec.CollValue[sdk.Coin](cdc)), } + schema, err := sb.Build() + if err != nil { + panic(err) + } + k.Schema = schema + return k } // GetAuthority returns the x/crisis module's authority. diff --git a/x/crisis/keeper/msg_server.go b/x/crisis/keeper/msg_server.go index 306d076563..f68c32771b 100644 --- a/x/crisis/keeper/msg_server.go +++ b/x/crisis/keeper/msg_server.go @@ -25,7 +25,7 @@ func (k *Keeper) VerifyInvariant(goCtx context.Context, msg *types.MsgVerifyInva } ctx := sdk.UnwrapSDKContext(goCtx) - params, err := k.GetConstantFee(ctx) + params, err := k.ConstantFee.Get(goCtx) if err != nil { return nil, err } @@ -90,7 +90,7 @@ func (k *Keeper) UpdateParams(ctx context.Context, msg *types.MsgUpdateParams) ( } sdkCtx := sdk.UnwrapSDKContext(ctx) - if err := k.SetConstantFee(sdkCtx, msg.ConstantFee); err != nil { + if err := k.ConstantFee.Set(sdkCtx, msg.ConstantFee); err != nil { return nil, err } diff --git a/x/crisis/keeper/msg_server_test.go b/x/crisis/keeper/msg_server_test.go index 1f8ab50bf5..e097939f6c 100644 --- a/x/crisis/keeper/msg_server_test.go +++ b/x/crisis/keeper/msg_server_test.go @@ -48,7 +48,7 @@ func (s *KeeperTestSuite) SetupTest() { func (s *KeeperTestSuite) TestMsgVerifyInvariant() { // default params constantFee := sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1000)) - err := s.keeper.SetConstantFee(s.ctx, constantFee) + err := s.keeper.ConstantFee.Set(s.ctx, constantFee) s.Require().NoError(err) encCfg := moduletestutil.MakeTestEncodingConfig(crisis.AppModuleBasic{}) diff --git a/x/crisis/keeper/params.go b/x/crisis/keeper/params.go deleted file mode 100644 index fb3c83ede7..0000000000 --- a/x/crisis/keeper/params.go +++ /dev/null @@ -1,37 +0,0 @@ -package keeper - -import ( - "context" - - errorsmod "cosmossdk.io/errors" - - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/errors" - "github.com/cosmos/cosmos-sdk/x/crisis/types" -) - -// GetConstantFee get's the constant fee from the store -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 - } - err = k.cdc.Unmarshal(bz, &constantFee) - return constantFee, err -} - -// GetConstantFee set's the constant fee in the store -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 := k.storeService.OpenKVStore(ctx) - bz, err := k.cdc.Marshal(&constantFee) - if err != nil { - return err - } - - return store.Set(types.ConstantFeeKey, bz) -} diff --git a/x/crisis/keeper/params_test.go b/x/crisis/keeper/params_test.go deleted file mode 100644 index 9b24be22ab..0000000000 --- a/x/crisis/keeper/params_test.go +++ /dev/null @@ -1,56 +0,0 @@ -package keeper_test - -import ( - sdkmath "cosmossdk.io/math" - - sdk "github.com/cosmos/cosmos-sdk/types" -) - -func (s *KeeperTestSuite) TestParams() { - // default params - constantFee := sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(1000)) - - testCases := []struct { - name string - constantFee sdk.Coin - expErr bool - expErrMsg string - }{ - { - name: "invalid constant fee", - constantFee: sdk.Coin{}, - expErr: true, - }, - { - name: "negative constant fee", - constantFee: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdkmath.NewInt(-1000)}, - expErr: true, - }, - { - name: "all good", - constantFee: constantFee, - expErr: false, - }, - } - - for _, tc := range testCases { - tc := tc - s.Run(tc.name, func() { - 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) - s.Require().Contains(err.Error(), tc.expErrMsg) - } else { - expected = tc.constantFee - s.Require().NoError(err) - } - - params, err := s.keeper.GetConstantFee(s.ctx) - s.Require().NoError(err) - s.Require().Equal(expected, params) - }) - } -} diff --git a/x/crisis/types/genesis.go b/x/crisis/types/genesis.go index 4a136bb3c5..6583ae175a 100644 --- a/x/crisis/types/genesis.go +++ b/x/crisis/types/genesis.go @@ -24,6 +24,9 @@ func DefaultGenesisState() *GenesisState { // ValidateGenesis - validate crisis genesis data func ValidateGenesis(data *GenesisState) error { + if !data.ConstantFee.IsValid() { + return fmt.Errorf("constant fee is invalid") + } if !data.ConstantFee.IsPositive() { return fmt.Errorf("constant fee must be positive: %s", data.ConstantFee) } diff --git a/x/crisis/types/keys.go b/x/crisis/types/keys.go index fb936e6665..26218e63fd 100644 --- a/x/crisis/types/keys.go +++ b/x/crisis/types/keys.go @@ -1,5 +1,7 @@ package types +import "cosmossdk.io/collections" + const ( // module name ModuleName = "crisis" @@ -7,4 +9,4 @@ const ( StoreKey = ModuleName ) -var ConstantFeeKey = []byte{0x01} +var ConstantFeeKey = collections.NewPrefix(1)