diff --git a/PENDING.md b/PENDING.md index 3207cce49..d93d0adb3 100644 --- a/PENDING.md +++ b/PENDING.md @@ -96,6 +96,12 @@ CLI flag. where validator is unexpectedly slashed throwing off test calculations * [\#3411] Include the `RequestInitChain.Time` in the block header init during `InitChain`. +* [\#3717] Update the vesting specification and implementation to cap deduction from +`DelegatedVesting` by at most `DelegatedVesting`. This accounts for the case where +the undelegation amount may exceed the original delegation amount due to +truncation of undelegation tokens. +* [\#3717] Ignore unknown proposers in allocating rewards for proposers, in case + unbonding period was just 1 block and proposer was already deleted. * [\#3726] Cap(clip) reward to remaining coins in AllocateTokens. ### Tendermint diff --git a/cmd/gaia/app/sim_test.go b/cmd/gaia/app/sim_test.go index fd3ef9804..04a7aae8d 100644 --- a/cmd/gaia/app/sim_test.go +++ b/cmd/gaia/app/sim_test.go @@ -171,7 +171,7 @@ func appStateRandomizedFn(r *rand.Rand, accs []simulation.Account, genesisTimest Pool: staking.InitialPool(), Params: staking.Params{ UnbondingTime: time.Duration(randIntBetween(r, 60, 60*60*24*3*2)) * time.Second, - MaxValidators: uint16(r.Intn(250)), + MaxValidators: uint16(r.Intn(250) + 1), BondDenom: sdk.DefaultBondDenom, }, } diff --git a/docs/spec/auth/05_vesting.md b/docs/spec/auth/05_vesting.md index a0c64fdd5..c6a16f5f3 100644 --- a/docs/spec/auth/05_vesting.md +++ b/docs/spec/auth/05_vesting.md @@ -251,10 +251,12 @@ func DelegateCoins(t Time, from Account, amount Coins) { ### Undelegating For a vesting account attempting to undelegate `D` coins, the following is performed: +NOTE: `DV < D` and `(DV + DF) < D` may be possible due to quirks in the rounding of +delegation/undelegation logic. -1. Verify `(DV + DF) >= D > 0` (this is simply a sanity check) +1. Verify `D > 0` 2. Compute `X := min(DF, D)` (portion of `D` that should become free, prioritizing free coins) -3. Compute `Y := D - X` (portion of `D` that should remain vesting) +3. Compute `Y := min(DV, D - X)` (portion of `D` that should remain vesting) 4. Set `DF -= X` 5. Set `DV -= Y` 6. Set `BC += D` @@ -274,6 +276,11 @@ func (cva ContinuousVestingAccount) TrackUndelegation(amount Coins) { with an excess `DV` amount, even after all its coins have vested. This is because undelegating free coins are prioritized. +**Note**: The undelegation (bond refund) amount may exceed the delegated +vesting (bond) amount due to the way undelegation truncates the bond refund, +which can increase the validator's exchange rate (tokens/shares) slightly if the +undelegated tokens are non-integral. + #### Keepers/Handlers ```go diff --git a/types/coin.go b/types/coin.go index 1122c859b..9fa1db0a6 100644 --- a/types/coin.go +++ b/types/coin.go @@ -389,8 +389,6 @@ func (coins Coins) AmountOf(denom string) Int { // IsAllPositive returns true if there is at least one coin and all currencies // have a positive value. -// -// TODO: Remove once unsigned integers are used. func (coins Coins) IsAllPositive() bool { if len(coins) == 0 { return false diff --git a/x/auth/account.go b/x/auth/account.go index 0983565ec..0c0e71a48 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -287,6 +287,11 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { // by which amount the base coins need to increase. The resulting base coins are // returned. // +// NOTE: The undelegation (bond refund) amount may exceed the delegated +// vesting (bond) amount due to the way undelegation truncates the bond refund, +// which can increase the validator's exchange rate (tokens/shares) slightly if +// the undelegated tokens are non-integral. +// // CONTRACT: The account's coins and undelegation coins must be sorted. func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { for _, coin := range amount { @@ -295,12 +300,13 @@ func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { panic("undelegation attempt with zero coins") } delegatedFree := bva.DelegatedFree.AmountOf(coin.Denom) + delegatedVesting := bva.DelegatedVesting.AmountOf(coin.Denom) // compute x and y per the specification, where: // X := min(DF, D) - // Y := D - X + // Y := min(DV, D - X) x := sdk.MinInt(delegatedFree, coin.Amount) - y := coin.Amount.Sub(x) + y := sdk.MinInt(delegatedVesting, coin.Amount.Sub(x)) if !x.IsZero() { xCoin := sdk.NewCoin(coin.Denom, x) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index de605f25a..81db06387 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -47,7 +47,13 @@ func NewBaseKeeper(ak auth.AccountKeeper, } // SetCoins sets the coins at the addr. -func (keeper BaseKeeper) SetCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) sdk.Error { +func (keeper BaseKeeper) SetCoins( + ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins, +) sdk.Error { + + if !amt.IsValid() { + return sdk.ErrInvalidCoins(amt.String()) + } return setCoins(ctx, keeper.ak, addr, amt) } @@ -56,6 +62,9 @@ func (keeper BaseKeeper) SubtractCoins( ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins, ) (sdk.Coins, sdk.Tags, sdk.Error) { + if !amt.IsValid() { + return nil, nil, sdk.ErrInvalidCoins(amt.String()) + } return subtractCoins(ctx, keeper.ak, addr, amt) } @@ -64,6 +73,9 @@ func (keeper BaseKeeper) AddCoins( ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins, ) (sdk.Coins, sdk.Tags, sdk.Error) { + if !amt.IsValid() { + return nil, nil, sdk.ErrInvalidCoins(amt.String()) + } return addCoins(ctx, keeper.ak, addr, amt) } @@ -78,14 +90,27 @@ func (keeper BaseKeeper) InputOutputCoins( // DelegateCoins performs delegation by deducting amt coins from an account with // address addr. For vesting accounts, delegations amounts are tracked for both // vesting and vested coins. -func (keeper BaseKeeper) DelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { +func (keeper BaseKeeper) DelegateCoins( + ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins, +) (sdk.Tags, sdk.Error) { + + if !amt.IsValid() { + return nil, sdk.ErrInvalidCoins(amt.String()) + } return delegateCoins(ctx, keeper.ak, addr, amt) } // UndelegateCoins performs undelegation by crediting amt coins to an account with // address addr. For vesting accounts, undelegation amounts are tracked for both // vesting and vested coins. -func (keeper BaseKeeper) UndelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { +// If any of the undelegation amounts are negative, an error is returned. +func (keeper BaseKeeper) UndelegateCoins( + ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins, +) (sdk.Tags, sdk.Error) { + + if !amt.IsValid() { + return nil, sdk.ErrInvalidCoins(amt.String()) + } return undelegateCoins(ctx, keeper.ak, addr, amt) } @@ -126,6 +151,10 @@ func NewBaseSendKeeper(ak auth.AccountKeeper, func (keeper BaseSendKeeper) SendCoins( ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins, ) (sdk.Tags, sdk.Error) { + + if !amt.IsValid() { + return nil, sdk.ErrInvalidCoins(amt.String()) + } return sendCoins(ctx, keeper.ak, fromAddr, toAddr, amt) } @@ -188,6 +217,9 @@ func getCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress) sdk.C } func setCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) sdk.Error { + if !amt.IsValid() { + return sdk.ErrInvalidCoins(amt.String()) + } acc := am.GetAccount(ctx, addr) if acc == nil { acc = am.NewAccountWithAddress(ctx, addr) @@ -218,6 +250,11 @@ func setAccount(ctx sdk.Context, ak auth.AccountKeeper, acc auth.Account) { // // CONTRACT: If the account is a vesting account, the amount has to be spendable. func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) { + + if !amt.IsValid() { + return nil, nil, sdk.ErrInvalidCoins(amt.String()) + } + oldCoins, spendableCoins := sdk.Coins{}, sdk.Coins{} acc := getAccount(ctx, ak, addr) @@ -244,6 +281,11 @@ func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, // AddCoins adds amt to the coins at the addr. func addCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) { + + if !amt.IsValid() { + return nil, nil, sdk.ErrInvalidCoins(amt.String()) + } + oldCoins := getCoins(ctx, am, addr) newCoins := oldCoins.Add(amt) @@ -260,9 +302,9 @@ func addCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt s } // SendCoins moves coins from one account to another +// Returns ErrInvalidCoins if amt is invalid. func sendCoins(ctx sdk.Context, am auth.AccountKeeper, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { // Safety check ensuring that when sending coins the keeper must maintain the - // supply invariant. if !amt.IsValid() { return nil, sdk.ErrInvalidCoins(amt.String()) } @@ -284,7 +326,7 @@ func sendCoins(ctx sdk.Context, am auth.AccountKeeper, fromAddr sdk.AccAddress, // NOTE: Make sure to revert state changes from tx on error func inputOutputCoins(ctx sdk.Context, am auth.AccountKeeper, inputs []Input, outputs []Output) (sdk.Tags, sdk.Error) { // Safety check ensuring that when sending coins the keeper must maintain the - // supply invariant. + // Check supply invariant and validity of Coins. if err := ValidateInputsOutputs(inputs, outputs); err != nil { return nil, err } @@ -314,6 +356,10 @@ func delegateCoins( ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, ) (sdk.Tags, sdk.Error) { + if !amt.IsValid() { + return nil, sdk.ErrInvalidCoins(amt.String()) + } + acc := getAccount(ctx, ak, addr) if acc == nil { return nil, sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", addr)) @@ -344,6 +390,10 @@ func undelegateCoins( ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, ) (sdk.Tags, sdk.Error) { + if !amt.IsValid() { + return nil, sdk.ErrInvalidCoins(amt.String()) + } + acc := getAccount(ctx, ak, addr) if acc == nil { return nil, sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", addr)) @@ -361,22 +411,24 @@ func undelegateCoins( ), nil } -func trackDelegation(acc auth.Account, blockTime time.Time, amount sdk.Coins) error { +// CONTRACT: assumes that amt is valid. +func trackDelegation(acc auth.Account, blockTime time.Time, amt sdk.Coins) error { vacc, ok := acc.(auth.VestingAccount) if ok { - vacc.TrackDelegation(blockTime, amount) + vacc.TrackDelegation(blockTime, amt) return nil } - return acc.SetCoins(acc.GetCoins().Sub(amount)) + return acc.SetCoins(acc.GetCoins().Sub(amt)) } -func trackUndelegation(acc auth.Account, amount sdk.Coins) error { +// CONTRACT: assumes that amt is valid. +func trackUndelegation(acc auth.Account, amt sdk.Coins) error { vacc, ok := acc.(auth.VestingAccount) if ok { - vacc.TrackUndelegation(amount) + vacc.TrackUndelegation(amt) return nil } - return acc.SetCoins(acc.GetCoins().Add(amount)) + return acc.SetCoins(acc.GetCoins().Add(amt)) } diff --git a/x/bank/msgs.go b/x/bank/msgs.go index eb047d09f..c9b4f5544 100644 --- a/x/bank/msgs.go +++ b/x/bank/msgs.go @@ -35,6 +35,9 @@ func (msg MsgSend) ValidateBasic() sdk.Error { if msg.ToAddress.Empty() { return sdk.ErrInvalidAddress("missing recipient address") } + if !msg.Amount.IsValid() { + return sdk.ErrInvalidCoins("send amount is invalid: " + msg.Amount.String()) + } if !msg.Amount.IsAllPositive() { return sdk.ErrInsufficientCoins("send amount must be positive") } diff --git a/x/distribution/keeper/allocation.go b/x/distribution/keeper/allocation.go index f32e47d02..e151da2fc 100644 --- a/x/distribution/keeper/allocation.go +++ b/x/distribution/keeper/allocation.go @@ -1,6 +1,8 @@ package keeper import ( + "fmt" + abci "github.com/tendermint/tendermint/abci/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -8,6 +10,7 @@ import ( // allocate fees handles distribution of the collected fees func (k Keeper) AllocateTokens(ctx sdk.Context, sumPrecommitPower, totalPower int64, proposer sdk.ConsAddress, votes []abci.VoteInfo) { + logger := ctx.Logger().With("module", "x/distribution") // fetch collected fees & fee pool feesCollectedInt := k.feeCollectionKeeper.GetCollectedFees(ctx) @@ -35,9 +38,20 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, sumPrecommitPower, totalPower in proposerReward := feesCollected.MulDecTruncate(proposerMultiplier) // pay proposer + remaining := feesCollected proposerValidator := k.stakingKeeper.ValidatorByConsAddr(ctx, proposer) - k.AllocateTokensToValidator(ctx, proposerValidator, proposerReward) - remaining := feesCollected.Sub(proposerReward) + if proposerValidator != nil { + k.AllocateTokensToValidator(ctx, proposerValidator, proposerReward) + remaining = feesCollected.Sub(proposerReward) + } else { + // proposer can be unknown if say, the unbonding period is 1 block, so + // e.g. a validator undelegates at block X, it's removed entirely by + // block X+1's endblock, then X+2 we need to refer to the previous + // proposer for X+1, but we've forgotten about them. + logger.Error(fmt.Sprintf( + "WARNING: Attempt to allocate proposer rewards to unknown proposer %s. This should happen only if the proposer unbonded completely within a single block, which generally should not happen except in exceptional circumstances (or fuzz testing). We recommend you investigate immediately.", + proposer.String())) + } // calculate fraction allocated to validators communityTax := k.GetCommunityTax(ctx) diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index af140474c..d63d8cd26 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -107,9 +107,11 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val sdk.Validator, de k.SetFeePool(ctx, feePool) // add coins to user account - withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, del.GetDelegatorAddr()) - if _, _, err := k.bankKeeper.AddCoins(ctx, withdrawAddr, coins); err != nil { - return err + if !coins.IsZero() { + withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, del.GetDelegatorAddr()) + if _, _, err := k.bankKeeper.AddCoins(ctx, withdrawAddr, coins); err != nil { + return err + } } // remove delegator starting info diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 0466e8f7e..a0534bf59 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -37,11 +37,13 @@ func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, _ sdk.ConsAddress, valAddr h.k.SetOutstandingRewards(ctx, outstanding.Sub(commission)) // add to validator account - accAddr := sdk.AccAddress(valAddr) - withdrawAddr := h.k.GetDelegatorWithdrawAddr(ctx, accAddr) + if !coins.IsZero() { + accAddr := sdk.AccAddress(valAddr) + withdrawAddr := h.k.GetDelegatorWithdrawAddr(ctx, accAddr) - if _, _, err := h.k.bankKeeper.AddCoins(ctx, withdrawAddr, coins); err != nil { - panic(err) + if _, _, err := h.k.bankKeeper.AddCoins(ctx, withdrawAddr, coins); err != nil { + panic(err) + } } } // remove commission record diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index 4a57c9bba..40fe13e8a 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -87,11 +87,13 @@ func (k Keeper) WithdrawValidatorCommission(ctx sdk.Context, valAddr sdk.ValAddr outstanding := k.GetOutstandingRewards(ctx) k.SetOutstandingRewards(ctx, outstanding.Sub(sdk.NewDecCoins(coins))) - accAddr := sdk.AccAddress(valAddr) - withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, accAddr) + if !coins.IsZero() { + accAddr := sdk.AccAddress(valAddr) + withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, accAddr) - if _, _, err := k.bankKeeper.AddCoins(ctx, withdrawAddr, coins); err != nil { - return err + if _, _, err := k.bankKeeper.AddCoins(ctx, withdrawAddr, coins); err != nil { + return err + } } return nil diff --git a/x/gov/simulation/msgs.go b/x/gov/simulation/msgs.go index d3707d525..551ed6538 100644 --- a/x/gov/simulation/msgs.go +++ b/x/gov/simulation/msgs.go @@ -183,7 +183,7 @@ func randomDeposit(r *rand.Rand) sdk.Coins { // Pick a random proposal ID func randomProposalID(r *rand.Rand, k gov.Keeper, ctx sdk.Context) (proposalID uint64, ok bool) { lastProposalID := k.GetLastProposalID(ctx) - if lastProposalID < 1 { + if lastProposalID < 1 || lastProposalID == (2<<63-1) { return 0, false } proposalID = uint64(r.Intn(1+int(lastProposalID)) - 1) diff --git a/x/ibc/handler.go b/x/ibc/handler.go index 9db876af2..7d7bda575 100644 --- a/x/ibc/handler.go +++ b/x/ibc/handler.go @@ -44,6 +44,7 @@ func handleIBCReceiveMsg(ctx sdk.Context, ibcm Mapper, ck BankKeeper, msg MsgIBC return ErrInvalidSequence(ibcm.codespace).Result() } + // XXX Check that packet.Coins is valid and positive (nonzero) _, _, err := ck.AddCoins(ctx, packet.DestAddr, packet.Coins) if err != nil { return err.Result() diff --git a/x/mock/simulation/mock_tendermint.go b/x/mock/simulation/mock_tendermint.go index 54e38d5c7..e241e9c67 100644 --- a/x/mock/simulation/mock_tendermint.go +++ b/x/mock/simulation/mock_tendermint.go @@ -17,6 +17,14 @@ type mockValidator struct { livenessState int } +func (mv mockValidator) String() string { + return fmt.Sprintf("mockValidator{%s:%X power:%v state:%v}", + mv.val.PubKey.Type, + mv.val.PubKey.Data, + mv.val.Power, + mv.livenessState) +} + type mockValidators map[string]mockValidator // get mockValidators from abci validators diff --git a/x/mock/simulation/rand_util.go b/x/mock/simulation/rand_util.go index a57775ecb..1a5e4566f 100644 --- a/x/mock/simulation/rand_util.go +++ b/x/mock/simulation/rand_util.go @@ -37,13 +37,32 @@ func RandStringOfLength(r *rand.Rand, n int) string { } // Generate a random amount +// Note: The range of RandomAmount includes max, and is, in fact, biased to return max as well as 0. func RandomAmount(r *rand.Rand, max sdk.Int) sdk.Int { - return sdk.NewInt(int64(r.Intn(int(max.Int64())))) + var randInt = big.NewInt(0) + switch r.Intn(10) { + case 0: + // randInt = big.NewInt(0) + case 1: + randInt = max.BigInt() + default: // NOTE: there are 10 total cases. + randInt = big.NewInt(0).Rand(r, max.BigInt()) // up to max - 1 + } + return sdk.NewIntFromBigInt(randInt) } // RandomDecAmount generates a random decimal amount +// Note: The range of RandomDecAmount includes max, and is, in fact, biased to return max as well as 0. func RandomDecAmount(r *rand.Rand, max sdk.Dec) sdk.Dec { - randInt := big.NewInt(0).Rand(r, max.Int) + var randInt = big.NewInt(0) + switch r.Intn(10) { + case 0: + // randInt = big.NewInt(0) + case 1: + randInt = max.Int // the underlying big int with all precision bits. + default: // NOTE: there are 10 total cases. + randInt = big.NewInt(0).Rand(r, max.Int) + } return sdk.NewDecFromBigIntWithPrec(randInt, sdk.Precision) } diff --git a/x/slashing/keeper.go b/x/slashing/keeper.go index de3751a15..8b8360492 100644 --- a/x/slashing/keeper.go +++ b/x/slashing/keeper.go @@ -47,7 +47,16 @@ func (k Keeper) handleDoubleSign(ctx sdk.Context, addr crypto.Address, infractio consAddr := sdk.ConsAddress(addr) pubkey, err := k.getPubkey(ctx, addr) if err != nil { - panic(fmt.Sprintf("Validator consensus-address %v not found", consAddr)) + // Ignore evidence that cannot be handled. + // NOTE: + // We used to panic with: + // `panic(fmt.Sprintf("Validator consensus-address %v not found", consAddr))`, + // but this couples the expectations of the app to both Tendermint and + // the simulator. Both are expected to provide the full range of + // allowable but none of the disallowed evidence types. Instead of + // getting this coordination right, it is easier to relax the + // constraints and ignore evidence that cannot be handled. + return } // Reject evidence if the double-sign is too old @@ -154,7 +163,7 @@ func (k Keeper) handleValidatorSignature(ctx sdk.Context, addr crypto.Address, p } if missed { - logger.Info(fmt.Sprintf("Absent validator %s at height %d, %d missed, threshold %d", addr, height, signInfo.MissedBlocksCounter, k.MinSignedPerWindow(ctx))) + logger.Info(fmt.Sprintf("Absent validator %s (%v) at height %d, %d missed, threshold %d", addr, pubkey, height, signInfo.MissedBlocksCounter, k.MinSignedPerWindow(ctx))) } minHeight := signInfo.StartHeight + k.SignedBlocksWindow(ctx) diff --git a/x/staking/keeper/delegation_test.go b/x/staking/keeper/delegation_test.go index 1c1d8dcb1..bc9d8e72b 100644 --- a/x/staking/keeper/delegation_test.go +++ b/x/staking/keeper/delegation_test.go @@ -207,7 +207,7 @@ func TestUnbondDelegation(t *testing.T) { } func TestUnbondingDelegationsMaxEntries(t *testing.T) { - ctx, _, keeper := CreateTestInput(t, false, 0) + ctx, _, keeper := CreateTestInput(t, false, 1) pool := keeper.GetPool(ctx) startTokens := sdk.TokensFromTendermintPower(10) pool.NotBondedTokens = startTokens @@ -372,7 +372,7 @@ func TestUndelegateFromUnbondingValidator(t *testing.T) { } func TestUndelegateFromUnbondedValidator(t *testing.T) { - ctx, _, keeper := CreateTestInput(t, false, 0) + ctx, _, keeper := CreateTestInput(t, false, 1) pool := keeper.GetPool(ctx) startTokens := sdk.TokensFromTendermintPower(20) pool.NotBondedTokens = startTokens diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index 89ffe13d1..30feb8b28 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -30,11 +30,11 @@ func TestPool(t *testing.T) { //check that the empty keeper loads the default resPool := keeper.GetPool(ctx) - require.True(t, expPool.Equal(resPool)) + require.Equal(t, expPool, resPool) //modify a params, save, and retrieve expPool.BondedTokens = sdk.NewInt(777) keeper.SetPool(ctx, expPool) resPool = keeper.GetPool(ctx) - require.True(t, expPool.Equal(resPool)) + require.Equal(t, expPool, resPool) } diff --git a/x/staking/keeper/test_common.go b/x/staking/keeper/test_common.go index fc367fc88..8edbe8e36 100644 --- a/x/staking/keeper/test_common.go +++ b/x/staking/keeper/test_common.go @@ -74,8 +74,9 @@ func MakeTestCodec() *codec.Codec { return cdc } -// hogpodge of all sorts of input required for testing -// init power is converted to an amount of tokens +// Hogpodge of all sorts of input required for testing. +// `initPower` is converted to an amount of tokens. +// If `initPower` is 0, no addrs get created. func CreateTestInput(t *testing.T, isCheckTx bool, initPower int64) (sdk.Context, auth.AccountKeeper, Keeper) { initCoins := sdk.TokensFromTendermintPower(initPower) @@ -128,9 +129,12 @@ func CreateTestInput(t *testing.T, isCheckTx bool, initPower int64) (sdk.Context // fill all the addresses with some coins, set the loose pool tokens simultaneously for _, addr := range Addrs { pool := keeper.GetPool(ctx) - _, _, err := ck.AddCoins(ctx, addr, sdk.Coins{ - {keeper.BondDenom(ctx), initCoins}, - }) + err := error(nil) + if !initCoins.IsZero() { + _, _, err = ck.AddCoins(ctx, addr, sdk.Coins{ + {keeper.BondDenom(ctx), initCoins}, + }) + } require.Nil(t, err) pool.NotBondedTokens = pool.NotBondedTokens.Add(initCoins) keeper.SetPool(ctx, pool) diff --git a/x/staking/simulation/msgs.go b/x/staking/simulation/msgs.go index 57df62be4..3caf1385e 100644 --- a/x/staking/simulation/msgs.go +++ b/x/staking/simulation/msgs.go @@ -82,6 +82,9 @@ func SimulateMsgEditValidator(k staking.Keeper) simulation.Operation { Details: simulation.RandStringOfLength(r, 10), } + if len(k.GetAllValidators(ctx)) == 0 { + return noOperation, nil, nil + } val := keeper.RandomValidator(r, k, ctx) address := val.GetOperator() newCommissionRate := simulation.RandomDecAmount(r, val.Commission.MaxRate) @@ -110,6 +113,9 @@ func SimulateMsgDelegate(m auth.AccountKeeper, k staking.Keeper) simulation.Oper action string, fOp []simulation.FutureOperation, err error) { denom := k.GetParams(ctx).BondDenom + if len(k.GetAllValidators(ctx)) == 0 { + return noOperation, nil, nil + } val := keeper.RandomValidator(r, k, ctx) validatorAddress := val.GetOperator() delegatorAcc := simulation.RandomAcc(r, accs) @@ -119,7 +125,7 @@ func SimulateMsgDelegate(m auth.AccountKeeper, k staking.Keeper) simulation.Oper amount = simulation.RandomAmount(r, amount) } if amount.Equal(sdk.ZeroInt()) { - return "no-operation", nil, nil + return noOperation, nil, nil } msg := staking.NewMsgDelegate( @@ -186,6 +192,9 @@ func SimulateMsgBeginRedelegate(m auth.AccountKeeper, k staking.Keeper) simulati action string, fOp []simulation.FutureOperation, err error) { denom := k.GetParams(ctx).BondDenom + if len(k.GetAllValidators(ctx)) == 0 { + return noOperation, nil, nil + } srcVal := keeper.RandomValidator(r, k, ctx) srcValidatorAddress := srcVal.GetOperator() destVal := keeper.RandomValidator(r, k, ctx)