Skip to content

Commit

Permalink
refactor: remove weird casting for staking.Keeper in slashing and dis…
Browse files Browse the repository at this point in the history
…tribution (#12973)

## Description

Both slashing and distribution simulator is using ugly casting from the nice interface type to:

 ```go
stakeKeeper := sk.(*stakingkeeper.Keeper)
```

This creates additional dependency for `staking/keeper` package in other modules, and requires type of a specific type (here reference type, in 0.46 it's even worse - because it uses object type).

EXTRA:
 * removed `stakingkeepr.RandomValidator` function (which btw, was also weird, because it was in the keeper module and required Keeper as an argument) and created generic `testutil.RandSliceElem` function.


---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
robert-zaremba authored Aug 20, 2022
1 parent cdf27e9 commit 7cb85c5
Show file tree
Hide file tree
Showing 10 changed files with 57 additions and 44 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/authz) [#12648](https://github.com/cosmos/cosmos-sdk/pull/12648) Add an allow list, an optional list of addresses allowed to receive bank assests via authz MsgSend grant.
* (cli) [#12028](https://github.com/cosmos/cosmos-sdk/pull/12028) Add the `tendermint key-migrate` to perform Tendermint v0.35 DB key migration.
* (sdk.Coins) [#12627](https://github.com/cosmos/cosmos-sdk/pull/12627) Make a Denoms method on sdk.Coins.
* (testutil) [#12973](https://github.com/cosmos/cosmos-sdk/pull/12973) Add generic `testutil.RandSliceElem` function which selects a random element from the list.

### Improvements

Expand Down Expand Up @@ -111,6 +112,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/modules) Remove all LegacyQueries and related code from modules
* (store) [#11825](https://github.com/cosmos/cosmos-sdk/pull/11825) Make extension snapshotter interface safer to use, renamed the util function `WriteExtensionItem` to `WriteExtensionPayload`.
* (codec) [#12964](https://github.com/cosmos/cosmos-sdk/pull/12964) `ProtoCodec.MarshalInterface` now returns an error when serializing unregistered types and a subsequent `ProtoCodec.UnmarshalInterface` would fail.
* (x/staking) [#12973](https://github.com/cosmos/cosmos-sdk/pull/12973) Removed `stakingkeeper.RandomValidator`. Use `testutil.RandSliceElem(r, sk.GetAllValidators(ctx))` instead.

### CLI Breaking Changes

Expand Down
2 changes: 1 addition & 1 deletion go.work.sum
Original file line number Diff line number Diff line change
@@ -1 +1 @@
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 h1:Ovs26xHkKqVztRpIrF/92BcuyuQ/YW4NSIpoGtfXNho=
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 h1:Ovs26xHkKqVztRpIrF/92BcuyuQ/YW4NSIpoGtfXNho=
12 changes: 12 additions & 0 deletions testutil/list.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package testutil

import "math/rand"

func RandSliceElem[E any](r *rand.Rand, elems []E) (E, bool) {
if len(elems) == 0 {
var e E
return e, false
}

return elems[r.Intn(len(elems))], true
}
18 changes: 8 additions & 10 deletions x/distribution/simulation/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ import (
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/testutil"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
sdk "github.com/cosmos/cosmos-sdk/types"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
"github.com/cosmos/cosmos-sdk/x/auth/tx"
"github.com/cosmos/cosmos-sdk/x/distribution/keeper"
"github.com/cosmos/cosmos-sdk/x/distribution/types"
"github.com/cosmos/cosmos-sdk/x/simulation"
stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
)

// Simulation operation weights constants
Expand Down Expand Up @@ -58,8 +58,6 @@ func WeightedOperations(appParams simtypes.AppParams, cdc codec.JSONCodec, ak ty
},
)

stakeKeeper := sk.(*stakingkeeper.Keeper)

interfaceRegistry := codectypes.NewInterfaceRegistry()
txConfig := tx.NewTxConfig(codec.NewProtoCodec(interfaceRegistry), tx.DefaultSignModes)

Expand All @@ -70,15 +68,15 @@ func WeightedOperations(appParams simtypes.AppParams, cdc codec.JSONCodec, ak ty
),
simulation.NewWeightedOperation(
weightMsgWithdrawDelegationReward,
SimulateMsgWithdrawDelegatorReward(txConfig, ak, bk, k, stakeKeeper),
SimulateMsgWithdrawDelegatorReward(txConfig, ak, bk, k, sk),
),
simulation.NewWeightedOperation(
weightMsgWithdrawValidatorCommission,
SimulateMsgWithdrawValidatorCommission(txConfig, ak, bk, k, stakeKeeper),
SimulateMsgWithdrawValidatorCommission(txConfig, ak, bk, k, sk),
),
simulation.NewWeightedOperation(
weightMsgFundCommunityPool,
SimulateMsgFundCommunityPool(txConfig, ak, bk, k, stakeKeeper),
SimulateMsgFundCommunityPool(txConfig, ak, bk, k, sk),
),
}
}
Expand Down Expand Up @@ -120,7 +118,7 @@ func SimulateMsgSetWithdrawAddress(txConfig client.TxConfig, ak types.AccountKee
}

// SimulateMsgWithdrawDelegatorReward generates a MsgWithdrawDelegatorReward with random values.
func SimulateMsgWithdrawDelegatorReward(txConfig client.TxConfig, ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper, sk *stakingkeeper.Keeper) simtypes.Operation {
func SimulateMsgWithdrawDelegatorReward(txConfig client.TxConfig, ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper, sk types.StakingKeeper) simtypes.Operation {
return func(
r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simtypes.Account, chainID string,
) (simtypes.OperationMsg, []simtypes.FutureOperation, error) {
Expand Down Expand Up @@ -162,11 +160,11 @@ func SimulateMsgWithdrawDelegatorReward(txConfig client.TxConfig, ak types.Accou
}

// SimulateMsgWithdrawValidatorCommission generates a MsgWithdrawValidatorCommission with random values.
func SimulateMsgWithdrawValidatorCommission(txConfig client.TxConfig, ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper, sk *stakingkeeper.Keeper) simtypes.Operation {
func SimulateMsgWithdrawValidatorCommission(txConfig client.TxConfig, ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper, sk types.StakingKeeper) simtypes.Operation {
return func(
r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simtypes.Account, chainID string,
) (simtypes.OperationMsg, []simtypes.FutureOperation, error) {
validator, ok := stakingkeeper.RandomValidator(r, sk, ctx)
validator, ok := testutil.RandSliceElem(r, sk.GetAllValidators(ctx))
if !ok {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgWithdrawValidatorCommission, "random validator is not ok"), nil, nil
}
Expand Down Expand Up @@ -207,7 +205,7 @@ func SimulateMsgWithdrawValidatorCommission(txConfig client.TxConfig, ak types.A

// SimulateMsgFundCommunityPool simulates MsgFundCommunityPool execution where
// a random account sends a random amount of its funds to the community pool.
func SimulateMsgFundCommunityPool(txConfig client.TxConfig, ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper, sk *stakingkeeper.Keeper) simtypes.Operation {
func SimulateMsgFundCommunityPool(txConfig client.TxConfig, ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper, sk types.StakingKeeper) simtypes.Operation {
return func(
r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simtypes.Account, chainID string,
) (simtypes.OperationMsg, []simtypes.FutureOperation, error) {
Expand Down
2 changes: 2 additions & 0 deletions x/distribution/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ type StakingKeeper interface {
fn func(index int64, delegation stakingtypes.DelegationI) (stop bool))

GetAllSDKDelegations(ctx sdk.Context) []stakingtypes.Delegation
GetAllValidators(ctx sdk.Context) (validators []stakingtypes.Validator)
GetAllDelegatorDelegations(ctx sdk.Context, delegator sdk.AccAddress) []stakingtypes.Delegation
}

// StakingHooks event hooks for staking validator object (noalias)
Expand Down
8 changes: 4 additions & 4 deletions x/slashing/simulation/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ import (
"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/testutil"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
sdk "github.com/cosmos/cosmos-sdk/types"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
"github.com/cosmos/cosmos-sdk/x/auth/tx"
"github.com/cosmos/cosmos-sdk/x/simulation"
"github.com/cosmos/cosmos-sdk/x/slashing/keeper"
"github.com/cosmos/cosmos-sdk/x/slashing/types"
stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
)

// Simulation operation weights constants
Expand All @@ -40,18 +40,18 @@ func WeightedOperations(
return simulation.WeightedOperations{
simulation.NewWeightedOperation(
weightMsgUnjail,
SimulateMsgUnjail(codec.NewProtoCodec(interfaceRegistry), ak, bk, k, sk.(*stakingkeeper.Keeper)),
SimulateMsgUnjail(codec.NewProtoCodec(interfaceRegistry), ak, bk, k, sk),
),
}
}

// SimulateMsgUnjail generates a MsgUnjail with random values
func SimulateMsgUnjail(cdc *codec.ProtoCodec, ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper, sk *stakingkeeper.Keeper) simtypes.Operation {
func SimulateMsgUnjail(cdc *codec.ProtoCodec, ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper, sk types.StakingKeeper) simtypes.Operation {
return func(
r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context,
accs []simtypes.Account, chainID string,
) (simtypes.OperationMsg, []simtypes.FutureOperation, error) {
validator, ok := stakingkeeper.RandomValidator(r, sk, ctx)
validator, ok := testutil.RandSliceElem(r, sk.GetAllValidators(ctx))
if !ok {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgUnjail, "validator is not ok"), nil, nil // skip
}
Expand Down
14 changes: 14 additions & 0 deletions x/slashing/testutil/expected_keepers_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions x/slashing/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type StakingKeeper interface {
// Delegation allows for getting a particular delegation for a given validator
// and delegator outside the scope of the staking module.
Delegation(sdk.Context, sdk.AccAddress, sdk.ValAddress) stakingtypes.DelegationI
GetAllValidators(ctx sdk.Context) (validators []stakingtypes.Validator)

// MaxValidators returns the maximum amount of bonded validators
MaxValidators(sdk.Context) uint32
Expand Down
13 changes: 0 additions & 13 deletions x/staking/keeper/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package keeper // noalias

import (
"bytes"
"math/rand"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/staking/types"
Expand Down Expand Up @@ -55,15 +54,3 @@ func TestingUpdateValidator(keeper *Keeper, ctx sdk.Context, validator types.Val

return validator
}

// RandomValidator returns a random validator given access to the keeper and ctx
func RandomValidator(r *rand.Rand, keeper *Keeper, ctx sdk.Context) (val types.Validator, ok bool) {
vals := keeper.GetAllValidators(ctx)
if len(vals) == 0 {
return types.Validator{}, false
}

i := r.Intn(len(vals))

return vals[i], true
}
29 changes: 13 additions & 16 deletions x/staking/simulation/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
simappparams "github.com/cosmos/cosmos-sdk/simapp/params"
"github.com/cosmos/cosmos-sdk/testutil"
sdk "github.com/cosmos/cosmos-sdk/types"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
"github.com/cosmos/cosmos-sdk/x/simulation"
Expand Down Expand Up @@ -192,13 +193,12 @@ func SimulateMsgEditValidator(ak types.AccountKeeper, bk types.BankKeeper, k *ke
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgEditValidator, "number of validators equal zero"), nil, nil
}

val, ok := keeper.RandomValidator(r, k, ctx)
val, ok := testutil.RandSliceElem(r, k.GetAllValidators(ctx))
if !ok {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgEditValidator, "unable to pick a validator"), nil, nil
}

address := val.GetOperator()

newCommissionRate := simtypes.RandomDecAmount(r, val.Commission.MaxRate)

if err := val.Commission.ValidateNewRate(newCommissionRate, ctx.BlockHeader().Time); err != nil {
Expand Down Expand Up @@ -255,7 +255,7 @@ func SimulateMsgDelegate(ak types.AccountKeeper, bk types.BankKeeper, k *keeper.
}

simAccount, _ := simtypes.RandomAcc(r, accs)
val, ok := keeper.RandomValidator(r, k, ctx)
val, ok := testutil.RandSliceElem(r, k.GetAllValidators(ctx))
if !ok {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgDelegate, "unable to pick a validator"), nil, nil
}
Expand Down Expand Up @@ -313,14 +313,13 @@ func SimulateMsgUndelegate(ak types.AccountKeeper, bk types.BankKeeper, k *keepe
return func(
r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simtypes.Account, chainID string,
) (simtypes.OperationMsg, []simtypes.FutureOperation, error) {
// get random validator
validator, ok := keeper.RandomValidator(r, k, ctx)
val, ok := testutil.RandSliceElem(r, k.GetAllValidators(ctx))
if !ok {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgUndelegate, "validator is not ok"), nil, nil
}

valAddr := validator.GetOperator()
delegations := k.GetValidatorDelegations(ctx, validator.GetOperator())
valAddr := val.GetOperator()
delegations := k.GetValidatorDelegations(ctx, val.GetOperator())
if delegations == nil {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgUndelegate, "keeper does have any delegation entries"), nil, nil
}
Expand All @@ -333,7 +332,7 @@ func SimulateMsgUndelegate(ak types.AccountKeeper, bk types.BankKeeper, k *keepe
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgUndelegate, "keeper does have a max unbonding delegation entries"), nil, nil
}

totalBond := validator.TokensFromShares(delegation.GetShares()).TruncateInt()
totalBond := val.TokensFromShares(delegation.GetShares()).TruncateInt()
if !totalBond.IsPositive() {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgUndelegate, "total bond is negative"), nil, nil
}
Expand Down Expand Up @@ -395,19 +394,17 @@ func SimulateMsgCancelUnbondingDelegate(ak types.AccountKeeper, bk types.BankKee
if len(k.GetAllValidators(ctx)) == 0 {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgDelegate, "number of validators equal zero"), nil, nil
}
// get random account
simAccount, _ := simtypes.RandomAcc(r, accs)
// get random validator
validator, ok := keeper.RandomValidator(r, k, ctx)
val, ok := testutil.RandSliceElem(r, k.GetAllValidators(ctx))
if !ok {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgCancelUnbondingDelegation, "validator is not ok"), nil, nil
}

if validator.IsJailed() || validator.InvalidExRate() {
if val.IsJailed() || val.InvalidExRate() {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgCancelUnbondingDelegation, "validator is jailed"), nil, nil
}

valAddr := validator.GetOperator()
valAddr := val.GetOperator()
unbondingDelegation, found := k.GetUnbondingDelegation(ctx, simAccount.Address, valAddr)
if !found {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgCancelUnbondingDelegation, "account does have any unbonding delegation"), nil, nil
Expand Down Expand Up @@ -474,8 +471,8 @@ func SimulateMsgBeginRedelegate(ak types.AccountKeeper, bk types.BankKeeper, k *
return func(
r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simtypes.Account, chainID string,
) (simtypes.OperationMsg, []simtypes.FutureOperation, error) {
// get random source validator
srcVal, ok := keeper.RandomValidator(r, k, ctx)
allVals := k.GetAllValidators(ctx)
srcVal, ok := testutil.RandSliceElem(r, allVals)
if !ok {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgBeginRedelegate, "unable to pick validator"), nil, nil
}
Expand All @@ -495,7 +492,7 @@ func SimulateMsgBeginRedelegate(ak types.AccountKeeper, bk types.BankKeeper, k *
}

// get random destination validator
destVal, ok := keeper.RandomValidator(r, k, ctx)
destVal, ok := testutil.RandSliceElem(r, allVals)
if !ok {
return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgBeginRedelegate, "unable to pick validator"), nil, nil
}
Expand Down

0 comments on commit 7cb85c5

Please sign in to comment.