From f1b08b85f2388bc0dae63d696fee74e2005c16fc Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Mon, 1 Jul 2019 17:50:04 +0100 Subject: [PATCH] Merge PR #4649: Refactor crisis package using internal --- .pending/breaking/sdk/4649-Refactor-x-cris | 1 + x/crisis/abci.go | 2 +- x/crisis/alias.go | 5 ++- x/crisis/client/cli/tx.go | 2 +- x/crisis/genesis.go | 7 +-- x/crisis/handler.go | 14 +++--- x/crisis/handler_test.go | 44 ++++++++++--------- x/crisis/{ => internal/keeper}/keeper.go | 12 ++++- x/crisis/{ => internal/keeper}/params.go | 4 +- x/crisis/{ => internal}/types/codec.go | 0 x/crisis/{ => internal}/types/errors.go | 0 x/crisis/{ => internal}/types/events.go | 0 .../{ => internal}/types/expected_keepers.go | 0 x/crisis/{ => internal}/types/genesis.go | 0 x/crisis/{ => internal}/types/keys.go | 0 x/crisis/{ => internal}/types/msgs.go | 0 x/crisis/{ => internal}/types/params.go | 0 x/crisis/{ => internal}/types/route.go | 0 x/crisis/module.go | 7 +-- 19 files changed, 58 insertions(+), 40 deletions(-) create mode 100644 .pending/breaking/sdk/4649-Refactor-x-cris rename x/crisis/{ => internal/keeper}/keeper.go (81%) rename x/crisis/{ => internal/keeper}/params.go (86%) rename x/crisis/{ => internal}/types/codec.go (100%) rename x/crisis/{ => internal}/types/errors.go (100%) rename x/crisis/{ => internal}/types/events.go (100%) rename x/crisis/{ => internal}/types/expected_keepers.go (100%) rename x/crisis/{ => internal}/types/genesis.go (100%) rename x/crisis/{ => internal}/types/keys.go (100%) rename x/crisis/{ => internal}/types/msgs.go (100%) rename x/crisis/{ => internal}/types/params.go (100%) rename x/crisis/{ => internal}/types/route.go (100%) diff --git a/.pending/breaking/sdk/4649-Refactor-x-cris b/.pending/breaking/sdk/4649-Refactor-x-cris new file mode 100644 index 0000000000..8ea877cf4b --- /dev/null +++ b/.pending/breaking/sdk/4649-Refactor-x-cris @@ -0,0 +1 @@ +#4649 Refactor x/crisis as per modules new specs. diff --git a/x/crisis/abci.go b/x/crisis/abci.go index 590c1fca8a..3e54859799 100644 --- a/x/crisis/abci.go +++ b/x/crisis/abci.go @@ -6,7 +6,7 @@ import ( // check all registered invariants func EndBlocker(ctx sdk.Context, k Keeper) { - if k.invCheckPeriod == 0 || ctx.BlockHeight()%int64(k.invCheckPeriod) != 0 { + if k.InvCheckPeriod() == 0 || ctx.BlockHeight()%int64(k.InvCheckPeriod()) != 0 { // skip running the invariant check return } diff --git a/x/crisis/alias.go b/x/crisis/alias.go index be9fd5c54a..ce6a3ed849 100644 --- a/x/crisis/alias.go +++ b/x/crisis/alias.go @@ -5,7 +5,8 @@ package crisis import ( - "github.com/cosmos/cosmos-sdk/x/crisis/types" + "github.com/cosmos/cosmos-sdk/x/crisis/internal/keeper" + "github.com/cosmos/cosmos-sdk/x/crisis/internal/types" ) const ( @@ -25,6 +26,7 @@ var ( NewMsgVerifyInvariant = types.NewMsgVerifyInvariant ParamKeyTable = types.ParamKeyTable NewInvarRoute = types.NewInvarRoute + NewKeeper = keeper.NewKeeper // variable aliases ModuleCdc = types.ModuleCdc @@ -35,4 +37,5 @@ type ( GenesisState = types.GenesisState MsgVerifyInvariant = types.MsgVerifyInvariant InvarRoute = types.InvarRoute + Keeper = keeper.Keeper ) diff --git a/x/crisis/client/cli/tx.go b/x/crisis/client/cli/tx.go index a698f1602d..1b79eac87f 100644 --- a/x/crisis/client/cli/tx.go +++ b/x/crisis/client/cli/tx.go @@ -10,7 +10,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" "github.com/cosmos/cosmos-sdk/x/auth/client/utils" - "github.com/cosmos/cosmos-sdk/x/crisis/types" + "github.com/cosmos/cosmos-sdk/x/crisis/internal/types" ) // command to replace a delegator's withdrawal address diff --git a/x/crisis/genesis.go b/x/crisis/genesis.go index 58d2689e32..dede2ab891 100644 --- a/x/crisis/genesis.go +++ b/x/crisis/genesis.go @@ -2,16 +2,17 @@ package crisis import ( sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/crisis/types" + "github.com/cosmos/cosmos-sdk/x/crisis/internal/keeper" + "github.com/cosmos/cosmos-sdk/x/crisis/internal/types" ) // new crisis genesis -func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) { +func InitGenesis(ctx sdk.Context, keeper keeper.Keeper, data types.GenesisState) { keeper.SetConstantFee(ctx, data.ConstantFee) } // ExportGenesis returns a GenesisState for a given context and keeper. -func ExportGenesis(ctx sdk.Context, keeper Keeper) types.GenesisState { +func ExportGenesis(ctx sdk.Context, keeper keeper.Keeper) types.GenesisState { constantFee := keeper.GetConstantFee(ctx) return types.NewGenesisState(constantFee) } diff --git a/x/crisis/handler.go b/x/crisis/handler.go index fa627a7cc5..b2b14d54e4 100644 --- a/x/crisis/handler.go +++ b/x/crisis/handler.go @@ -4,13 +4,14 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/crisis/types" + "github.com/cosmos/cosmos-sdk/x/crisis/internal/keeper" + "github.com/cosmos/cosmos-sdk/x/crisis/internal/types" ) // RouterKey -const RouterKey = ModuleName +const RouterKey = types.ModuleName -func NewHandler(k Keeper) sdk.Handler { +func NewHandler(k keeper.Keeper) sdk.Handler { return func(ctx sdk.Context, msg sdk.Msg) sdk.Result { ctx = ctx.WithEventManager(sdk.NewEventManager()) @@ -25,12 +26,11 @@ func NewHandler(k Keeper) sdk.Handler { } } -func handleMsgVerifyInvariant(ctx sdk.Context, msg types.MsgVerifyInvariant, k Keeper) sdk.Result { +func handleMsgVerifyInvariant(ctx sdk.Context, msg types.MsgVerifyInvariant, k keeper.Keeper) sdk.Result { // remove the constant fee constantFee := sdk.NewCoins(k.GetConstantFee(ctx)) - err := k.supplyKeeper.SendCoinsFromAccountToModule(ctx, msg.Sender, k.feeCollectorName, constantFee) - if err != nil { + if err := k.SendCoinsFromAccountToFeeCollector(ctx, msg.Sender, constantFee); err != nil { return err.Result() } @@ -41,7 +41,7 @@ func handleMsgVerifyInvariant(ctx sdk.Context, msg types.MsgVerifyInvariant, k K msgFullRoute := msg.FullInvariantRoute() var invarianceErr error - for _, invarRoute := range k.routes { + for _, invarRoute := range k.Routes() { if invarRoute.FullRoute() == msgFullRoute { invarianceErr = invarRoute.Invar(cacheCtx) found = true diff --git a/x/crisis/handler_test.go b/x/crisis/handler_test.go index 392903b718..7616f74e07 100644 --- a/x/crisis/handler_test.go +++ b/x/crisis/handler_test.go @@ -1,4 +1,4 @@ -package crisis +package crisis_test import ( "errors" @@ -11,24 +11,25 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" + "github.com/cosmos/cosmos-sdk/x/crisis" distr "github.com/cosmos/cosmos-sdk/x/distribution" ) var ( testModuleName = "dummy" - dummyRouteWhichPasses = NewInvarRoute(testModuleName, "which-passes", func(_ sdk.Context) error { return nil }) - dummyRouteWhichFails = NewInvarRoute(testModuleName, "which-fails", func(_ sdk.Context) error { return errors.New("whoops") }) + dummyRouteWhichPasses = crisis.NewInvarRoute(testModuleName, "which-passes", func(_ sdk.Context) error { return nil }) + dummyRouteWhichFails = crisis.NewInvarRoute(testModuleName, "which-fails", func(_ sdk.Context) error { return errors.New("whoops") }) addrs = distr.TestAddrs ) -func CreateTestInput(t *testing.T) (sdk.Context, Keeper, auth.AccountKeeper, distr.Keeper) { +func CreateTestInput(t *testing.T) (sdk.Context, crisis.Keeper, auth.AccountKeeper, distr.Keeper) { communityTax := sdk.NewDecWithPrec(2, 2) ctx, accKeeper, _, distrKeeper, _, paramsKeeper, supplyKeeper := distr.CreateTestInputAdvanced(t, false, 10, communityTax) - paramSpace := paramsKeeper.Subspace(DefaultParamspace) - crisisKeeper := NewKeeper(paramSpace, 1, supplyKeeper, auth.FeeCollectorName) + paramSpace := paramsKeeper.Subspace(crisis.DefaultParamspace) + crisisKeeper := crisis.NewKeeper(paramSpace, 1, supplyKeeper, auth.FeeCollectorName) constantFee := sdk.NewInt64Coin("stake", 10000000) crisisKeeper.SetConstantFee(ctx, constantFee) @@ -52,17 +53,18 @@ func TestHandleMsgVerifyInvariantWithNotEnoughSenderCoins(t *testing.T) { excessCoins := sdk.NewCoin(coin.Denom, coin.Amount.AddRaw(1)) crisisKeeper.SetConstantFee(ctx, excessCoins) - msg := NewMsgVerifyInvariant(sender, testModuleName, dummyRouteWhichPasses.Route) - res := handleMsgVerifyInvariant(ctx, msg, crisisKeeper) - require.False(t, res.IsOK()) + h := crisis.NewHandler(crisisKeeper) + msg := crisis.NewMsgVerifyInvariant(sender, testModuleName, dummyRouteWhichPasses.Route) + require.False(t, h(ctx, msg).IsOK()) } func TestHandleMsgVerifyInvariantWithBadInvariant(t *testing.T) { ctx, crisisKeeper, _, _ := CreateTestInput(t) sender := addrs[0] - msg := NewMsgVerifyInvariant(sender, testModuleName, "route-that-doesnt-exist") - res := handleMsgVerifyInvariant(ctx, msg, crisisKeeper) + h := crisis.NewHandler(crisisKeeper) + msg := crisis.NewMsgVerifyInvariant(sender, testModuleName, "route-that-doesnt-exist") + res := h(ctx, msg) require.False(t, res.IsOK()) } @@ -70,10 +72,11 @@ func TestHandleMsgVerifyInvariantWithInvariantBroken(t *testing.T) { ctx, crisisKeeper, _, _ := CreateTestInput(t) sender := addrs[0] - msg := NewMsgVerifyInvariant(sender, testModuleName, dummyRouteWhichFails.Route) + h := crisis.NewHandler(crisisKeeper) + msg := crisis.NewMsgVerifyInvariant(sender, testModuleName, dummyRouteWhichFails.Route) var res sdk.Result require.Panics(t, func() { - res = handleMsgVerifyInvariant(ctx, msg, crisisKeeper) + res = h(ctx, msg) }, fmt.Sprintf("%v", res)) } @@ -86,10 +89,11 @@ func TestHandleMsgVerifyInvariantWithInvariantBrokenAndNotEnoughPoolCoins(t *tes feePool.CommunityPool = sdk.DecCoins{} distrKeeper.SetFeePool(ctx, feePool) - msg := NewMsgVerifyInvariant(sender, testModuleName, dummyRouteWhichFails.Route) + h := crisis.NewHandler(crisisKeeper) + msg := crisis.NewMsgVerifyInvariant(sender, testModuleName, dummyRouteWhichFails.Route) var res sdk.Result require.Panics(t, func() { - res = handleMsgVerifyInvariant(ctx, msg, crisisKeeper) + res = h(ctx, msg) }, fmt.Sprintf("%v", res)) } @@ -97,14 +101,14 @@ func TestHandleMsgVerifyInvariantWithInvariantNotBroken(t *testing.T) { ctx, crisisKeeper, _, _ := CreateTestInput(t) sender := addrs[0] - msg := NewMsgVerifyInvariant(sender, testModuleName, dummyRouteWhichPasses.Route) - res := handleMsgVerifyInvariant(ctx, msg, crisisKeeper) - require.True(t, res.IsOK()) + h := crisis.NewHandler(crisisKeeper) + msg := crisis.NewMsgVerifyInvariant(sender, testModuleName, dummyRouteWhichPasses.Route) + require.True(t, h(ctx, msg).IsOK()) } func TestInvalidMsg(t *testing.T) { - k := Keeper{} - h := NewHandler(k) + k := crisis.Keeper{} + h := crisis.NewHandler(k) res := h(sdk.NewContext(nil, abci.Header{}, false, nil), sdk.NewTestMsg()) require.False(t, res.IsOK()) diff --git a/x/crisis/keeper.go b/x/crisis/internal/keeper/keeper.go similarity index 81% rename from x/crisis/keeper.go rename to x/crisis/internal/keeper/keeper.go index a6f331d314..07d50633fe 100644 --- a/x/crisis/keeper.go +++ b/x/crisis/internal/keeper/keeper.go @@ -1,4 +1,4 @@ -package crisis +package keeper import ( "fmt" @@ -7,7 +7,7 @@ import ( "github.com/tendermint/tendermint/libs/log" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/crisis/types" + "github.com/cosmos/cosmos-sdk/x/crisis/internal/types" "github.com/cosmos/cosmos-sdk/x/params" ) @@ -83,4 +83,12 @@ func (k Keeper) AssertInvariants(ctx sdk.Context) { logger.Info("asserted all invariants", "duration", diff, "height", ctx.BlockHeight()) } +// InvCheckPeriod returns the invariant checks period. +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) sdk.Error { + return k.supplyKeeper.SendCoinsFromAccountToModule(ctx, senderAddr, k.feeCollectorName, amt) +} + // DONTCOVER diff --git a/x/crisis/params.go b/x/crisis/internal/keeper/params.go similarity index 86% rename from x/crisis/params.go rename to x/crisis/internal/keeper/params.go index e8f40759dd..cdf7169af0 100644 --- a/x/crisis/params.go +++ b/x/crisis/internal/keeper/params.go @@ -1,8 +1,8 @@ -package crisis +package keeper import ( sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/crisis/types" + "github.com/cosmos/cosmos-sdk/x/crisis/internal/types" ) // GetConstantFee get's the constant fee from the paramSpace diff --git a/x/crisis/types/codec.go b/x/crisis/internal/types/codec.go similarity index 100% rename from x/crisis/types/codec.go rename to x/crisis/internal/types/codec.go diff --git a/x/crisis/types/errors.go b/x/crisis/internal/types/errors.go similarity index 100% rename from x/crisis/types/errors.go rename to x/crisis/internal/types/errors.go diff --git a/x/crisis/types/events.go b/x/crisis/internal/types/events.go similarity index 100% rename from x/crisis/types/events.go rename to x/crisis/internal/types/events.go diff --git a/x/crisis/types/expected_keepers.go b/x/crisis/internal/types/expected_keepers.go similarity index 100% rename from x/crisis/types/expected_keepers.go rename to x/crisis/internal/types/expected_keepers.go diff --git a/x/crisis/types/genesis.go b/x/crisis/internal/types/genesis.go similarity index 100% rename from x/crisis/types/genesis.go rename to x/crisis/internal/types/genesis.go diff --git a/x/crisis/types/keys.go b/x/crisis/internal/types/keys.go similarity index 100% rename from x/crisis/types/keys.go rename to x/crisis/internal/types/keys.go diff --git a/x/crisis/types/msgs.go b/x/crisis/internal/types/msgs.go similarity index 100% rename from x/crisis/types/msgs.go rename to x/crisis/internal/types/msgs.go diff --git a/x/crisis/types/params.go b/x/crisis/internal/types/params.go similarity index 100% rename from x/crisis/types/params.go rename to x/crisis/internal/types/params.go diff --git a/x/crisis/types/route.go b/x/crisis/internal/types/route.go similarity index 100% rename from x/crisis/types/route.go rename to x/crisis/internal/types/route.go diff --git a/x/crisis/module.go b/x/crisis/module.go index c36f8a72e2..bbcdaf0f22 100644 --- a/x/crisis/module.go +++ b/x/crisis/module.go @@ -13,7 +13,8 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/cosmos-sdk/x/crisis/client/cli" - "github.com/cosmos/cosmos-sdk/x/crisis/types" + "github.com/cosmos/cosmos-sdk/x/crisis/internal/keeper" + "github.com/cosmos/cosmos-sdk/x/crisis/internal/types" ) var ( @@ -65,11 +66,11 @@ func (AppModuleBasic) GetQueryCmd(_ *codec.Codec) *cobra.Command { return nil } // app module for bank type AppModule struct { AppModuleBasic - keeper Keeper + keeper keeper.Keeper } // NewAppModule creates a new AppModule object -func NewAppModule(keeper Keeper) AppModule { +func NewAppModule(keeper keeper.Keeper) AppModule { return AppModule{ AppModuleBasic: AppModuleBasic{}, keeper: keeper,