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

WIP: Refactor crisis using internal package #4597

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
5 changes: 3 additions & 2 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/cosmos/cosmos-sdk/x/auth/genaccounts"
"github.com/cosmos/cosmos-sdk/x/bank"
"github.com/cosmos/cosmos-sdk/x/crisis"
"github.com/cosmos/cosmos-sdk/x/crisis/internal/keeper"
distr "github.com/cosmos/cosmos-sdk/x/distribution"
distrclient "github.com/cosmos/cosmos-sdk/x/distribution/client"
"github.com/cosmos/cosmos-sdk/x/genutil"
Expand Down Expand Up @@ -95,7 +96,7 @@ type SimApp struct {
mintKeeper mint.Keeper
distrKeeper distr.Keeper
govKeeper gov.Keeper
crisisKeeper crisis.Keeper
crisisKeeper keeper.Keeper
paramsKeeper params.Keeper

// the module manager
Expand Down Expand Up @@ -152,7 +153,7 @@ func NewSimApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bo
app.feeCollectionKeeper, distr.DefaultCodespace)
app.slashingKeeper = slashing.NewKeeper(app.cdc, app.keySlashing, &stakingKeeper,
slashingSubspace, slashing.DefaultCodespace)
app.crisisKeeper = crisis.NewKeeper(crisisSubspace, invCheckPeriod, app.distrKeeper,
app.crisisKeeper = keeper.NewKeeper(crisisSubspace, invCheckPeriod, app.distrKeeper,
app.bankKeeper, app.feeCollectionKeeper)

// register the proposal types
Expand Down
5 changes: 3 additions & 2 deletions x/crisis/abci_app.go → x/crisis/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import (
"github.com/tendermint/tendermint/libs/log"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/crisis/internal/keeper"
)

// check all registered invariants
func EndBlocker(ctx sdk.Context, k Keeper, logger log.Logger) {
if k.invCheckPeriod == 0 || ctx.BlockHeight()%int64(k.invCheckPeriod) != 0 {
func EndBlocker(ctx sdk.Context, k keeper.Keeper, logger log.Logger) {
if k.InvCheckPeriod() == 0 || ctx.BlockHeight()%int64(k.InvCheckPeriod()) != 0 {
// skip running the invariant check
return
}
Expand Down
3 changes: 1 addition & 2 deletions x/crisis/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
package crisis

import (
"github.com/cosmos/cosmos-sdk/x/crisis/types"
"github.com/cosmos/cosmos-sdk/x/crisis/internal/types"
)

const (
Expand All @@ -24,7 +24,6 @@ var (
DefaultGenesisState = types.DefaultGenesisState
NewMsgVerifyInvariant = types.NewMsgVerifyInvariant
ParamKeyTable = types.ParamKeyTable
NewInvarRoute = types.NewInvarRoute

// variable aliases
ModuleCdc = types.ModuleCdc
Expand Down
2 changes: 1 addition & 1 deletion x/crisis/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions x/crisis/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
12 changes: 6 additions & 6 deletions x/crisis/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/crisis/tags"
"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

func NewHandler(k Keeper) sdk.Handler {
func NewHandler(k keeper.Keeper) sdk.Handler {
return func(ctx sdk.Context, msg sdk.Msg) sdk.Result {
switch msg := msg.(type) {
case types.MsgVerifyInvariant:
Expand All @@ -24,7 +24,7 @@ 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))
Expand Down Expand Up @@ -73,8 +73,8 @@ func handleMsgVerifyInvariant(ctx sdk.Context, msg types.MsgVerifyInvariant, k K
}

resTags := sdk.NewTags(
tags.Sender, msg.Sender.String(),
tags.Invariant, msg.InvariantRoute,
types.Sender, msg.Sender.String(),
types.Invariant, msg.InvariantRoute,
)
return sdk.Result{
Tags: resTags,
Expand Down
44 changes: 26 additions & 18 deletions x/crisis/handler_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package crisis
package crisis_test

import (
"errors"
Expand All @@ -10,24 +10,27 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth"
"github.com/cosmos/cosmos-sdk/x/crisis"
"github.com/cosmos/cosmos-sdk/x/crisis/internal/keeper"
"github.com/cosmos/cosmos-sdk/x/crisis/internal/types"
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 = types.NewInvarRoute(testModuleName, "which-passes", func(_ sdk.Context) error { return nil })
dummyRouteWhichFails = types.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, keeper.Keeper, auth.AccountKeeper, distr.Keeper) {

communityTax := sdk.NewDecWithPrec(2, 2)
ctx, accKeeper, bankKeeper, distrKeeper, _, feeCollectionKeeper, paramsKeeper :=
distr.CreateTestInputAdvanced(t, false, 10, communityTax)

paramSpace := paramsKeeper.Subspace(DefaultParamspace)
crisisKeeper := NewKeeper(paramSpace, 1, distrKeeper, bankKeeper, feeCollectionKeeper)
paramSpace := paramsKeeper.Subspace(crisis.DefaultParamspace)
crisisKeeper := keeper.NewKeeper(paramSpace, 1, distrKeeper, bankKeeper, feeCollectionKeeper)
constantFee := sdk.NewInt64Coin("stake", 10000000)
crisisKeeper.SetConstantFee(ctx, constantFee)

Expand All @@ -51,28 +54,31 @@ 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)
h := crisis.NewHandler(crisisKeeper)
msg := crisis.NewMsgVerifyInvariant(sender, testModuleName, dummyRouteWhichPasses.Route)
res := h(ctx, msg)
require.False(t, res.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)
msg := crisis.NewMsgVerifyInvariant(sender, testModuleName, "route-that-doesnt-exist")
h := crisis.NewHandler(crisisKeeper)
res := h(ctx, msg)
require.False(t, res.IsOK())
}

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))
}

Expand All @@ -85,25 +91,27 @@ 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))
}

func TestHandleMsgVerifyInvariantWithInvariantNotBroken(t *testing.T) {
ctx, crisisKeeper, _, _ := CreateTestInput(t)
sender := addrs[0]

msg := NewMsgVerifyInvariant(sender, testModuleName, dummyRouteWhichPasses.Route)
res := handleMsgVerifyInvariant(ctx, msg, crisisKeeper)
h := crisis.NewHandler(crisisKeeper)
msg := crisis.NewMsgVerifyInvariant(sender, testModuleName, dummyRouteWhichPasses.Route)
res := h(ctx, msg)
require.True(t, res.IsOK())
}

func TestInvalidMsg(t *testing.T) {
k := Keeper{}
h := NewHandler(k)
k := keeper.Keeper{}
h := crisis.NewHandler(k)

res := h(sdk.Context{}, sdk.NewTestMsg())
require.False(t, res.IsOK())
Expand Down
16 changes: 9 additions & 7 deletions x/crisis/keeper.go → x/crisis/internal/keeper/keeper.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package crisis
package keeper

import (
"fmt"
Expand All @@ -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"
)

Expand All @@ -17,15 +17,15 @@ type Keeper struct {
paramSpace params.Subspace
invCheckPeriod uint

distrKeeper DistrKeeper
bankKeeper BankKeeper
feeCollectionKeeper FeeCollectionKeeper
distrKeeper types.DistrKeeper
bankKeeper types.BankKeeper
feeCollectionKeeper types.FeeCollectionKeeper
}

// NewKeeper creates a new Keeper object
func NewKeeper(paramSpace params.Subspace, invCheckPeriod uint,
distrKeeper DistrKeeper, bankKeeper BankKeeper,
feeCollectionKeeper FeeCollectionKeeper) Keeper {
distrKeeper types.DistrKeeper, bankKeeper types.BankKeeper,
feeCollectionKeeper types.FeeCollectionKeeper) Keeper {

return Keeper{
routes: []types.InvarRoute{},
Expand Down Expand Up @@ -79,4 +79,6 @@ func (k Keeper) AssertInvariants(ctx sdk.Context, logger log.Logger) {
logger.With("module", "x/crisis").Info("asserted all invariants", "duration", diff, "height", ctx.BlockHeight())
}

func (k Keeper) InvCheckPeriod() uint { return k.invCheckPeriod }

// DONTCOVER
4 changes: 2 additions & 2 deletions x/crisis/params.go → x/crisis/internal/keeper/params.go
Original file line number Diff line number Diff line change
@@ -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
Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package crisis
package types

import (
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down
10 changes: 10 additions & 0 deletions x/crisis/types/genesis.go → x/crisis/internal/types/genesis.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package types

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
)

Expand All @@ -22,3 +24,11 @@ func DefaultGenesisState() GenesisState {
ConstantFee: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1000)),
}
}

// ValidateGenesis validates crisis genesis data
func ValidateGenesis(data GenesisState) error {
if !data.ConstantFee.IsPositive() {
return fmt.Errorf("constant fee must be positive: %s", data.ConstantFee)
}
return nil
}
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion x/crisis/tags/tags.go → x/crisis/internal/types/tags.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package tags
package types

import (
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down
5 changes: 3 additions & 2 deletions x/crisis/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ 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/internal/keeper"
)

var (
Expand Down Expand Up @@ -62,12 +63,12 @@ func (AppModuleBasic) GetQueryCmd(_ *codec.Codec) *cobra.Command { return nil }
// app module for bank
type AppModule struct {
AppModuleBasic
keeper Keeper
keeper keeper.Keeper
logger log.Logger
}

// NewAppModule creates a new AppModule object
func NewAppModule(keeper Keeper, logger log.Logger) AppModule {
func NewAppModule(keeper keeper.Keeper, logger log.Logger) AppModule {
return AppModule{
AppModuleBasic: AppModuleBasic{},
keeper: keeper,
Expand Down