From 86f07d271db98b17f8f5a0aebba7195d2385cae7 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Fri, 24 Aug 2018 21:47:31 -0400 Subject: [PATCH 01/14] unbonding validators, delegtion effects --- x/stake/keeper/delegation.go | 47 ++++++++++++++++++++++++++- x/stake/keeper/slash.go | 37 ++++++++++++++++----- x/stake/keeper/validator.go | 62 +++++++++++++++++++++--------------- x/stake/types/validator.go | 45 ++++++++++++-------------- 4 files changed, 132 insertions(+), 59 deletions(-) diff --git a/x/stake/keeper/delegation.go b/x/stake/keeper/delegation.go index 2bad79c20f..107665a9b5 100644 --- a/x/stake/keeper/delegation.go +++ b/x/stake/keeper/delegation.go @@ -2,6 +2,7 @@ package keeper import ( "bytes" + "time" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/stake/types" @@ -311,6 +312,32 @@ func (k Keeper) unbond(ctx sdk.Context, delegatorAddr, validatorAddr sdk.AccAddr //______________________________________________________________________________________________________ +// get info for begin functions: MinTime and CreationHeight +func (k Keeper) getBeginInfo(ctx sdk.Context, params types.Params, validatorSrcAddr sdk.AccAddress) ( + minTime time.Time, height int64, completeNow bool) { + + validator, found := k.GetValidator(ctx, validatorSrcAddr) + switch { + case !found || validator.Status == sdk.Bonded: + + // longest wait - just unbonding period from now + minTime = ctx.BlockHeader().Time.Add(params.UnbondingTime) + height = ctx.BlockHeader().Height + return minTime, height, false + + case validator.Status == sdk.Unbonding: + minTime = validator.UnbondingMinTime + height = validator.UnbondingHeight + return minTime, height, false + + case validator.Status == sdk.Unbonded: + return minTime, height, true + + default: + panic("unknown validator status") + } +} + // complete unbonding an unbonding record func (k Keeper) BeginUnbonding(ctx sdk.Context, delegatorAddr, validatorAddr sdk.AccAddress, sharesAmount sdk.Dec) sdk.Error { @@ -327,12 +354,22 @@ func (k Keeper) BeginUnbonding(ctx sdk.Context, delegatorAddr, validatorAddr sdk // create the unbonding delegation params := k.GetParams(ctx) - minTime := ctx.BlockHeader().Time.Add(params.UnbondingTime) + minTime, height, completeNow := k.getBeginInfo(ctx, params, validatorAddr) balance := sdk.Coin{params.BondDenom, returnAmount.RoundInt()} + // no need to create the ubd object just complete now + if completeNow { + _, _, err := k.coinKeeper.AddCoins(ctx, delegatorAddr, sdk.Coins{balance}) + if err != nil { + return err + } + return nil + } + ubd := types.UnbondingDelegation{ DelegatorAddr: delegatorAddr, ValidatorAddr: validatorAddr, + CreationHeight: height, MinTime: minTime, Balance: balance, InitialBalance: balance, @@ -390,11 +427,19 @@ func (k Keeper) BeginRedelegation(ctx sdk.Context, delegatorAddr, validatorSrcAd // create the unbonding delegation minTime := ctx.BlockHeader().Time.Add(params.UnbondingTime) + height := ctx.BlockHeader().Height + + minTime, height, completeNow := k.getBeginInfo(ctx, params, validatorSrcAddr) + + if completeNow { // no need to create the redelegation object + return nil + } red := types.Redelegation{ DelegatorAddr: delegatorAddr, ValidatorSrcAddr: validatorSrcAddr, ValidatorDstAddr: validatorDstAddr, + CreationHeight: height, MinTime: minTime, SharesDst: sharesCreated, SharesSrc: sharesAmount, diff --git a/x/stake/keeper/slash.go b/x/stake/keeper/slash.go index aa6fe974d8..b936e585d0 100644 --- a/x/stake/keeper/slash.go +++ b/x/stake/keeper/slash.go @@ -43,6 +43,26 @@ func (k Keeper) Slash(ctx sdk.Context, pubkey crypto.PubKey, infractionHeight in pubkey.Address())) return } + + // do not slash if unbonded + unbonded := false + if validator.Status == sdk.Unbonded { + unbonded = true + } else if validator.Status == sdk.Unbonding { + ctxTime := ctx.BlockHeader().Time + if ctxTime.After(validator.UnbondingMinTime) { + + // TODO should we also just update the + // validator status to unbonded here? + unbonded = true + } + } + if unbonded { + logger.Info(fmt.Sprintf( + "failed attempt to slash an unbonded validator")) + return + } + operatorAddress := validator.GetOperator() // Track remaining slash amount for the validator @@ -91,17 +111,16 @@ func (k Keeper) Slash(ctx sdk.Context, pubkey crypto.PubKey, infractionHeight in // Cannot decrease balance below zero tokensToBurn := sdk.MinDec(remainingSlashAmount, validator.Tokens) - // Get the current pool + // burn validator's tokens pool := k.GetPool(ctx) - // remove tokens from the validator validator, pool = validator.RemoveTokens(pool, tokensToBurn) - // burn tokens pool.LooseTokens = pool.LooseTokens.Sub(tokensToBurn) - // update the pool k.SetPool(ctx, pool) + // update the validator, possibly kicking it out validator = k.UpdateValidator(ctx, validator) - // remove validator if it has been reduced to zero shares + + // remove validator if it has no more tokens if validator.Tokens.IsZero() { k.RemoveValidator(ctx, validator.Operator) } @@ -134,12 +153,12 @@ func (k Keeper) Unjail(ctx sdk.Context, pubkey crypto.PubKey) { } // set the jailed flag on a validator -func (k Keeper) setJailed(ctx sdk.Context, pubkey crypto.PubKey, jailed bool) { +func (k Keeper) setJailed(ctx sdk.Context, pubkey crypto.PubKey, isJailed bool) { validator, found := k.GetValidatorByPubKey(ctx, pubkey) if !found { - panic(fmt.Errorf("Validator with pubkey %s not found, cannot set jailed to %v", pubkey, jailed)) + panic(fmt.Errorf("Validator with pubkey %s not found, cannot set jailed to %v", pubkey, isJailed)) } - validator.Jailed = jailed + validator.Jailed = isJailed k.UpdateValidator(ctx, validator) // update validator, possibly unbonding or bonding it return } @@ -179,6 +198,7 @@ func (k Keeper) slashUnbondingDelegation(ctx sdk.Context, unbondingDelegation ty unbondingDelegation.Balance.Amount = unbondingDelegation.Balance.Amount.Sub(unbondingSlashAmount) k.SetUnbondingDelegation(ctx, unbondingDelegation) pool := k.GetPool(ctx) + // Burn loose tokens // Ref https://github.com/cosmos/cosmos-sdk/pull/1278#discussion_r198657760 pool.LooseTokens = pool.LooseTokens.Sub(slashAmount) @@ -239,6 +259,7 @@ func (k Keeper) slashRedelegation(ctx sdk.Context, validator types.Validator, re if err != nil { panic(fmt.Errorf("error unbonding delegator: %v", err)) } + // Burn loose tokens pool := k.GetPool(ctx) pool.LooseTokens = pool.LooseTokens.Sub(tokensToBurn) diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index e933a6d236..72567576be 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -212,6 +212,7 @@ func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) type cliffPower := k.GetCliffValidatorPower(ctx) switch { + // if the validator is already bonded and the power is increasing, we need // perform the following: // a) update Tendermint @@ -240,10 +241,11 @@ func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) type bytes.Compare(valPower, cliffPower) == -1: //(valPower < cliffPower // skip to completion - // default case - validator was either: + default: + // default case - validator was either: // a) not-bonded and now has power-rank greater than cliff validator // b) bonded and now has decreased in power - default: + // update the validator set for this validator updatedVal, updated := k.UpdateBondedValidators(ctx, validator) if updated { @@ -307,10 +309,13 @@ func (k Keeper) updateCliffValidator(ctx sdk.Context, affectedVal types.Validato newCliffValRank := GetValidatorsByPowerIndexKey(newCliffVal, pool) if bytes.Equal(affectedVal.Operator, newCliffVal.Operator) { + // The affected validator remains the cliff validator, however, since // the store does not contain the new power, update the new power rank. store.Set(ValidatorPowerCliffKey, affectedValRank) + } else if bytes.Compare(affectedValRank, newCliffValRank) > 0 { + // The affected validator no longer remains the cliff validator as it's // power is greater than the new cliff validator. k.setCliffValidator(ctx, newCliffVal, pool) @@ -321,7 +326,7 @@ func (k Keeper) updateCliffValidator(ctx sdk.Context, affectedVal types.Validato func (k Keeper) updateForJailing(ctx sdk.Context, oldFound bool, oldValidator, newValidator types.Validator) types.Validator { if newValidator.Jailed && oldFound && oldValidator.Status == sdk.Bonded { - newValidator = k.unbondValidator(ctx, newValidator) + newValidator = k.beginUnbondingValidator(ctx, newValidator) // need to also clear the cliff validator spot because the jail has // opened up a new spot which will be filled when @@ -416,20 +421,20 @@ func (k Keeper) UpdateBondedValidators( } } - // increment bondedValidatorsCount / get the validator to bond - if !validator.Jailed { - if validator.Status != sdk.Bonded { - validatorToBond = validator - if newValidatorBonded { - panic("already decided to bond a validator, can't bond another!") - } - newValidatorBonded = true - } - } else { - // TODO: document why we must break here. + // if we've reached jailed validators no further bonded validators exist + if validator.Jailed { break } + // increment bondedValidatorsCount / get the validator to bond + if validator.Status != sdk.Bonded { + validatorToBond = validator + if newValidatorBonded { + panic("already decided to bond a validator, can't bond another!") + } + newValidatorBonded = true + } + // increment the total number of bonded validators and potentially mark // the validator to bond if validator.Status != sdk.Bonded { @@ -464,13 +469,15 @@ func (k Keeper) UpdateBondedValidators( } if bytes.Equal(validatorToBond.Operator, affectedValidator.Operator) { - // unbond the old cliff validator iff the affected validator was - // newly bonded and has greater power - k.unbondValidator(ctx, oldCliffVal) + + // begin unbonding the old cliff validator iff the affected + // validator was newly bonded and has greater power + k.beginUnbondingValidator(ctx, oldCliffVal) } else { - // otherwise unbond the affected validator, which must have been - // kicked out - affectedValidator = k.unbondValidator(ctx, affectedValidator) + + // otherwise begin unbonding the affected validator, which must + // have been kicked out + affectedValidator = k.beginUnbondingValidator(ctx, affectedValidator) } } @@ -563,25 +570,30 @@ func kickOutValidators(k Keeper, ctx sdk.Context, toKickOut map[string]byte) { if !found { panic(fmt.Sprintf("validator record not found for address: %v\n", ownerAddr)) } - k.unbondValidator(ctx, validator) + k.beginUnbondingValidator(ctx, validator) } } // perform all the store operations for when a validator status becomes unbonded -func (k Keeper) unbondValidator(ctx sdk.Context, validator types.Validator) types.Validator { +func (k Keeper) beginUnbondingValidator(ctx sdk.Context, validator types.Validator) types.Validator { store := ctx.KVStore(k.storeKey) pool := k.GetPool(ctx) + params := k.GetParams(ctx) // sanity check - if validator.Status == sdk.Unbonded { - panic(fmt.Sprintf("should not already be unbonded, validator: %v\n", validator)) + if validator.Status == sdk.Unbonded || + validator.Status == sdk.Unbonding { + panic(fmt.Sprintf("should not already be unbonded or unbonding, validator: %v\n", validator)) } // set the status - validator, pool = validator.UpdateStatus(pool, sdk.Unbonded) + validator, pool = validator.UpdateStatus(pool, sdk.Unbonding) k.SetPool(ctx, pool) + validator.UnbondingMinTime = ctx.BlockHeader().Time.Add(params.UnbondingTime) + validator.UnbondingHeight = ctx.BlockHeader().Height + // save the now unbonded validator record k.SetValidator(ctx, validator) diff --git a/x/stake/types/validator.go b/x/stake/types/validator.go index f8404b5963..db9c8afd5d 100644 --- a/x/stake/types/validator.go +++ b/x/stake/types/validator.go @@ -3,6 +3,7 @@ package types import ( "bytes" "fmt" + "time" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto" @@ -31,15 +32,14 @@ type Validator struct { Description Description `json:"description"` // description terms for the validator BondHeight int64 `json:"bond_height"` // earliest height as a bonded validator BondIntraTxCounter int16 `json:"bond_intra_tx_counter"` // block-local tx index of validator change - ProposerRewardPool sdk.Coins `json:"proposer_reward_pool"` // XXX reward pool collected from being the proposer + + UnbondingHeight int64 `json:"unbonding_height"` // if unbonding, height at which this validator has begun unbonding + UnbondingMinTime time.Time `json:"unbonding_time"` // if unbonding, min time for the validator to complete unbonding Commission sdk.Dec `json:"commission"` // XXX the commission rate of fees charged to any delegators CommissionMax sdk.Dec `json:"commission_max"` // XXX maximum commission rate which this validator can ever charge CommissionChangeRate sdk.Dec `json:"commission_change_rate"` // XXX maximum daily increase of the validator commission CommissionChangeToday sdk.Dec `json:"commission_change_today"` // XXX commission rate change today, reset each day (UTC time) - - // fee related - LastBondedTokens sdk.Dec `json:"prev_bonded_tokens"` // Previous bonded tokens held } // NewValidator - initialize a new validator @@ -54,12 +54,12 @@ func NewValidator(operator sdk.AccAddress, pubKey crypto.PubKey, description Des Description: description, BondHeight: int64(0), BondIntraTxCounter: int16(0), - ProposerRewardPool: sdk.Coins{}, + UnbondingHeight: int64(0), + UnbondingMinTime: time.Unix(0, 0), Commission: sdk.ZeroDec(), CommissionMax: sdk.ZeroDec(), CommissionChangeRate: sdk.ZeroDec(), CommissionChangeToday: sdk.ZeroDec(), - LastBondedTokens: sdk.ZeroDec(), } } @@ -73,12 +73,12 @@ type validatorValue struct { Description Description BondHeight int64 BondIntraTxCounter int16 - ProposerRewardPool sdk.Coins + UnbondingHeight int64 + UnbondingMinTime time.Time Commission sdk.Dec CommissionMax sdk.Dec CommissionChangeRate sdk.Dec CommissionChangeToday sdk.Dec - LastBondedTokens sdk.Dec } // return the redelegation without fields contained within the key for the store @@ -92,12 +92,12 @@ func MustMarshalValidator(cdc *wire.Codec, validator Validator) []byte { Description: validator.Description, BondHeight: validator.BondHeight, BondIntraTxCounter: validator.BondIntraTxCounter, - ProposerRewardPool: validator.ProposerRewardPool, + UnbondingHeight: validator.UnbondingHeight, + UnbondingMinTime: validator.UnbondingMinTime, Commission: validator.Commission, CommissionMax: validator.CommissionMax, CommissionChangeRate: validator.CommissionChangeRate, CommissionChangeToday: validator.CommissionChangeToday, - LastBondedTokens: validator.LastBondedTokens, } return cdc.MustMarshalBinary(val) } @@ -108,7 +108,6 @@ func MustUnmarshalValidator(cdc *wire.Codec, operatorAddr, value []byte) Validat if err != nil { panic(err) } - return validator } @@ -134,12 +133,12 @@ func UnmarshalValidator(cdc *wire.Codec, operatorAddr, value []byte) (validator Description: storeValue.Description, BondHeight: storeValue.BondHeight, BondIntraTxCounter: storeValue.BondIntraTxCounter, - ProposerRewardPool: storeValue.ProposerRewardPool, + UnbondingHeight: storeValue.UnbondingHeight, + UnbondingMinTime: storeValue.UnbondingMinTime, Commission: storeValue.Commission, CommissionMax: storeValue.CommissionMax, CommissionChangeRate: storeValue.CommissionChangeRate, CommissionChangeToday: storeValue.CommissionChangeToday, - LastBondedTokens: storeValue.LastBondedTokens, }, nil } @@ -161,12 +160,12 @@ func (v Validator) HumanReadableString() (string, error) { resp += fmt.Sprintf("Delegator Shares: %s\n", v.DelegatorShares.String()) resp += fmt.Sprintf("Description: %s\n", v.Description) resp += fmt.Sprintf("Bond Height: %d\n", v.BondHeight) - resp += fmt.Sprintf("Proposer Reward Pool: %s\n", v.ProposerRewardPool.String()) + resp += fmt.Sprintf("Unbonding Height: %d\n", v.UnbondingHeight) + resp += fmt.Sprintf("Minimum Unbonding Time: %d\n", v.UnbondingMinTime) resp += fmt.Sprintf("Commission: %s\n", v.Commission.String()) resp += fmt.Sprintf("Max Commission Rate: %s\n", v.CommissionMax.String()) resp += fmt.Sprintf("Commission Change Rate: %s\n", v.CommissionChangeRate.String()) resp += fmt.Sprintf("Commission Change Today: %s\n", v.CommissionChangeToday.String()) - resp += fmt.Sprintf("Previous Bonded Tokens: %s\n", v.LastBondedTokens.String()) return resp, nil } @@ -186,15 +185,14 @@ type BechValidator struct { Description Description `json:"description"` // description terms for the validator BondHeight int64 `json:"bond_height"` // earliest height as a bonded validator BondIntraTxCounter int16 `json:"bond_intra_tx_counter"` // block-local tx index of validator change - ProposerRewardPool sdk.Coins `json:"proposer_reward_pool"` // XXX reward pool collected from being the proposer + + UnbondingHeight int64 `json:"unbonding_height"` // if unbonding, height at which this validator has begun unbonding + UnbondingMinTime time.Time `json:"unbonding_time"` // if unbonding, min time for the validator to complete unbonding Commission sdk.Dec `json:"commission"` // XXX the commission rate of fees charged to any delegators CommissionMax sdk.Dec `json:"commission_max"` // XXX maximum commission rate which this validator can ever charge CommissionChangeRate sdk.Dec `json:"commission_change_rate"` // XXX maximum daily increase of the validator commission CommissionChangeToday sdk.Dec `json:"commission_change_today"` // XXX commission rate change today, reset each day (UTC time) - - // fee related - LastBondedTokens sdk.Dec `json:"prev_bonded_shares"` // last bonded token amount } // get the bech validator from the the regular validator @@ -216,14 +214,13 @@ func (v Validator) Bech32Validator() (BechValidator, error) { Description: v.Description, BondHeight: v.BondHeight, BondIntraTxCounter: v.BondIntraTxCounter, - ProposerRewardPool: v.ProposerRewardPool, + UnbondingHeight: v.UnbondingHeight, + UnbondingMinTime: v.UnbondingMinTime, Commission: v.Commission, CommissionMax: v.CommissionMax, CommissionChangeRate: v.CommissionChangeRate, CommissionChangeToday: v.CommissionChangeToday, - - LastBondedTokens: v.LastBondedTokens, }, nil } @@ -238,12 +235,10 @@ func (v Validator) Equal(c2 Validator) bool { v.Tokens.Equal(c2.Tokens) && v.DelegatorShares.Equal(c2.DelegatorShares) && v.Description == c2.Description && - v.ProposerRewardPool.IsEqual(c2.ProposerRewardPool) && v.Commission.Equal(c2.Commission) && v.CommissionMax.Equal(c2.CommissionMax) && v.CommissionChangeRate.Equal(c2.CommissionChangeRate) && - v.CommissionChangeToday.Equal(c2.CommissionChangeToday) && - v.LastBondedTokens.Equal(c2.LastBondedTokens) + v.CommissionChangeToday.Equal(c2.CommissionChangeToday) } // constant used in flags to indicate that description field should not be updated From ec42e6650156b4f768e138231df5971a1d6b284e Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Mon, 27 Aug 2018 00:20:40 -0400 Subject: [PATCH 02/14] fix old tests --- x/slashing/keeper_test.go | 6 +++--- x/slashing/tick_test.go | 2 +- x/stake/handler_test.go | 4 ++-- x/stake/keeper/validator_test.go | 14 ++++++++------ x/stake/types/validator.go | 2 +- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/x/slashing/keeper_test.go b/x/slashing/keeper_test.go index 808c82014e..6e9765ab2b 100644 --- a/x/slashing/keeper_test.go +++ b/x/slashing/keeper_test.go @@ -114,7 +114,7 @@ func TestHandleAbsentValidator(t *testing.T) { // validator should have been jailed validator, _ = sk.GetValidatorByPubKey(ctx, val) - require.Equal(t, sdk.Unbonded, validator.GetStatus()) + require.Equal(t, sdk.Unbonding, validator.GetStatus()) // unrevocation should fail prior to jail expiration got = slh(ctx, NewMsgUnjail(addr)) @@ -161,7 +161,7 @@ func TestHandleAbsentValidator(t *testing.T) { keeper.handleValidatorSignature(ctx, val.Address(), amtInt, false) } validator, _ = sk.GetValidatorByPubKey(ctx, val) - require.Equal(t, sdk.Unbonded, validator.GetStatus()) + require.Equal(t, sdk.Unbonding, validator.GetStatus()) } // Test a new validator entering the validator set @@ -230,7 +230,7 @@ func TestHandleAlreadyJailed(t *testing.T) { // validator should have been jailed and slashed validator, _ := sk.GetValidatorByPubKey(ctx, val) - require.Equal(t, sdk.Unbonded, validator.GetStatus()) + require.Equal(t, sdk.Unbonding, validator.GetStatus()) // validator should have been slashed require.Equal(t, int64(amtInt-1), validator.GetTokens().RoundInt64()) diff --git a/x/slashing/tick_test.go b/x/slashing/tick_test.go index 78d2695494..9a8eaf4ffc 100644 --- a/x/slashing/tick_test.go +++ b/x/slashing/tick_test.go @@ -80,5 +80,5 @@ func TestBeginBlocker(t *testing.T) { // validator should be jailed validator, found := sk.GetValidatorByPubKey(ctx, pk) require.True(t, found) - require.Equal(t, sdk.Unbonded, validator.GetStatus()) + require.Equal(t, sdk.Unbonding, validator.GetStatus()) } diff --git a/x/stake/handler_test.go b/x/stake/handler_test.go index 6ab54e48c2..f0fce9dcb4 100644 --- a/x/stake/handler_test.go +++ b/x/stake/handler_test.go @@ -85,8 +85,8 @@ func TestValidatorByPowerIndex(t *testing.T) { keeper.Jail(ctx, keep.PKs[0]) validator, found = keeper.GetValidator(ctx, validatorAddr) require.True(t, found) - require.Equal(t, sdk.Unbonded, validator.Status) // ensure is unbonded - require.Equal(t, int64(500000), validator.Tokens.RoundInt64()) // ensure is unbonded + require.Equal(t, sdk.Unbonding, validator.Status) // ensure is unbonding + require.Equal(t, int64(500000), validator.Tokens.RoundInt64()) // ensure tokens slashed // the old power record should have been deleted as the power changed require.False(t, keep.ValidatorByPowerIndexExists(ctx, keeper, power)) diff --git a/x/stake/keeper/validator_test.go b/x/stake/keeper/validator_test.go index 54149707e8..dee8223660 100644 --- a/x/stake/keeper/validator_test.go +++ b/x/stake/keeper/validator_test.go @@ -137,7 +137,7 @@ func TestUpdateBondedValidatorsDecreaseCliff(t *testing.T) { expectedValStatus := map[int]sdk.BondStatus{ 9: sdk.Bonded, 8: sdk.Bonded, 7: sdk.Bonded, 5: sdk.Bonded, 4: sdk.Bonded, - 0: sdk.Unbonded, 1: sdk.Unbonded, 2: sdk.Unbonded, 3: sdk.Unbonded, 6: sdk.Unbonded, + 0: sdk.Unbonding, 1: sdk.Unbonding, 2: sdk.Unbonding, 3: sdk.Unbonding, 6: sdk.Unbonding, } // require all the validators have their respective statuses @@ -145,9 +145,11 @@ func TestUpdateBondedValidatorsDecreaseCliff(t *testing.T) { valAddr := validators[valIdx].Operator val, _ := keeper.GetValidator(ctx, valAddr) - require.Equal( - t, val.GetStatus(), status, - fmt.Sprintf("expected validator to have status: %s", sdk.BondStatusToString(status))) + assert.Equal( + t, status, val.GetStatus(), + fmt.Sprintf("expected validator at index %v to have status: %s", + valIdx, + sdk.BondStatusToString(status))) } } @@ -610,8 +612,8 @@ func TestFullValidatorSetPowerChange(t *testing.T) { validators[i], found = keeper.GetValidator(ctx, validators[i].Operator) require.True(t, found) } - assert.Equal(t, sdk.Unbonded, validators[0].Status) - assert.Equal(t, sdk.Unbonded, validators[1].Status) + assert.Equal(t, sdk.Unbonding, validators[0].Status) + assert.Equal(t, sdk.Unbonding, validators[1].Status) assert.Equal(t, sdk.Bonded, validators[2].Status) assert.Equal(t, sdk.Bonded, validators[3].Status) assert.Equal(t, sdk.Unbonded, validators[4].Status) diff --git a/x/stake/types/validator.go b/x/stake/types/validator.go index db9c8afd5d..c8868a56ca 100644 --- a/x/stake/types/validator.go +++ b/x/stake/types/validator.go @@ -161,7 +161,7 @@ func (v Validator) HumanReadableString() (string, error) { resp += fmt.Sprintf("Description: %s\n", v.Description) resp += fmt.Sprintf("Bond Height: %d\n", v.BondHeight) resp += fmt.Sprintf("Unbonding Height: %d\n", v.UnbondingHeight) - resp += fmt.Sprintf("Minimum Unbonding Time: %d\n", v.UnbondingMinTime) + resp += fmt.Sprintf("Minimum Unbonding Time: %v\n", v.UnbondingMinTime) resp += fmt.Sprintf("Commission: %s\n", v.Commission.String()) resp += fmt.Sprintf("Max Commission Rate: %s\n", v.CommissionMax.String()) resp += fmt.Sprintf("Commission Change Rate: %s\n", v.CommissionChangeRate.String()) From 527320c9fc2308aacd029b1e368d439de3a64804 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Mon, 27 Aug 2018 00:38:31 -0400 Subject: [PATCH 03/14] dummy tests --- x/stake/keeper/delegation_test.go | 16 ++++++++++++++ x/stake/keeper/slash_test.go | 26 +++++++++++++++++++---- x/stake/simulation/invariants.go | 1 + x/stake/simulation/msgs.go | 35 ++++++++++++++++++++++++------- 4 files changed, 67 insertions(+), 11 deletions(-) diff --git a/x/stake/keeper/delegation_test.go b/x/stake/keeper/delegation_test.go index 4333a74942..32594654af 100644 --- a/x/stake/keeper/delegation_test.go +++ b/x/stake/keeper/delegation_test.go @@ -180,6 +180,14 @@ func TestUnbondDelegation(t *testing.T) { require.Equal(t, int64(4), pool.BondedTokens.RoundInt64()) } +func TestUndelegateFromUnbondingValidator(t *testing.T) { + require.Fail(t) +} + +func TestUndelegateFromUnbondedValidator(t *testing.T) { + require.Fail(t) +} + // Make sure that that the retrieving the delegations doesn't affect the state func TestGetRedelegationsFromValidator(t *testing.T) { ctx, _, keeper := CreateTestInput(t, false, 0) @@ -259,3 +267,11 @@ func TestRedelegation(t *testing.T) { _, found = keeper.GetRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1]) require.False(t, found) } + +func TestRedelegationFromUnbondingValidator(t *testing.T) { + require.Fail(t) +} + +func TestRedelegationFromUnbondedValidator(t *testing.T) { + require.Fail(t) +} diff --git a/x/stake/keeper/slash_test.go b/x/stake/keeper/slash_test.go index 156f0602e9..5eb7c5e35e 100644 --- a/x/stake/keeper/slash_test.go +++ b/x/stake/keeper/slash_test.go @@ -11,8 +11,8 @@ import ( abci "github.com/tendermint/tendermint/abci/types" ) -// setup helper function -// creates two validators +// TODO integrate with test_common.go helper (CreateTestInput) +// setup helper function - creates two validators func setupHelper(t *testing.T, amt int64) (sdk.Context, Keeper, types.Params) { // setup ctx, _, keeper := CreateTestInput(t, false, amt) @@ -34,8 +34,11 @@ func setupHelper(t *testing.T, amt int64) (sdk.Context, Keeper, types.Params) { return ctx, keeper, params } +//_________________________________________________________________________________ + // tests Jail, Unjail func TestRevocation(t *testing.T) { + // setup ctx, keeper, _ := setupHelper(t, 10) addr := addrVals[0] @@ -57,7 +60,6 @@ func TestRevocation(t *testing.T) { val, found = keeper.GetValidator(ctx, addr) require.True(t, found) require.False(t, val.GetJailed()) - } // tests slashUnbondingDelegation @@ -95,8 +97,10 @@ func TestSlashUnbondingDelegation(t *testing.T) { require.Equal(t, int64(5), slashAmount.RoundInt64()) ubd, found := keeper.GetUnbondingDelegation(ctx, addrDels[0], addrVals[0]) require.True(t, found) + // initialbalance unchanged require.Equal(t, sdk.NewInt64Coin(params.BondDenom, 10), ubd.InitialBalance) + // balance decreased require.Equal(t, sdk.NewInt64Coin(params.BondDenom, 5), ubd.Balance) newPool := keeper.GetPool(ctx) @@ -155,14 +159,18 @@ func TestSlashRedelegation(t *testing.T) { require.Equal(t, int64(5), slashAmount.RoundInt64()) rd, found = keeper.GetRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1]) require.True(t, found) + // initialbalance unchanged require.Equal(t, sdk.NewInt64Coin(params.BondDenom, 10), rd.InitialBalance) + // balance decreased require.Equal(t, sdk.NewInt64Coin(params.BondDenom, 5), rd.Balance) + // shares decreased del, found = keeper.GetDelegation(ctx, addrDels[0], addrVals[1]) require.True(t, found) require.Equal(t, int64(5), del.Shares.RoundInt64()) + // pool bonded tokens decreased newPool := keeper.GetPool(ctx) require.Equal(t, int64(5), oldPool.BondedTokens.Sub(newPool.BondedTokens).RoundInt64()) @@ -177,7 +185,7 @@ func TestSlashAtFutureHeight(t *testing.T) { } // tests Slash at the current height -func TestSlashAtCurrentHeight(t *testing.T) { +func TestSlashValidatorAtCurrentHeight(t *testing.T) { ctx, keeper, _ := setupHelper(t, 10) pk := PKs[0] fraction := sdk.NewDecWithPrec(5, 1) @@ -198,6 +206,16 @@ func TestSlashAtCurrentHeight(t *testing.T) { require.Equal(t, sdk.NewDec(5).RoundInt64(), oldPool.BondedTokens.Sub(newPool.BondedTokens).RoundInt64()) } +// slash an unbonding validator +func TestSlashUnbondingValidator(t *testing.T) { + require.Fail(t) +} + +// fail an attept to slash an unbonded validator +func TestSlashUnbondedValidator(t *testing.T) { + require.Fail(t) +} + // tests Slash at a previous height with an unbonding delegation func TestSlashWithUnbondingDelegation(t *testing.T) { ctx, keeper, params := setupHelper(t, 10) diff --git a/x/stake/simulation/invariants.go b/x/stake/simulation/invariants.go index 1e961d05de..859bb05915 100644 --- a/x/stake/simulation/invariants.go +++ b/x/stake/simulation/invariants.go @@ -45,6 +45,7 @@ func SupplyInvariants(ck bank.Keeper, k stake.Keeper, am auth.AccountMapper) sim case sdk.Bonded: bonded = bonded.Add(validator.GetPower()) case sdk.Unbonding: + loose = loose.Add(validator.GetTokens().RoundInt()) case sdk.Unbonded: loose = loose.Add(validator.GetTokens().RoundInt()) } diff --git a/x/stake/simulation/msgs.go b/x/stake/simulation/msgs.go index 17f9808931..f2b37769a0 100644 --- a/x/stake/simulation/msgs.go +++ b/x/stake/simulation/msgs.go @@ -19,7 +19,10 @@ import ( // SimulateMsgCreateValidator func SimulateMsgCreateValidator(m auth.AccountMapper, k stake.Keeper) simulation.TestAndRunTx { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, + keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + denom := k.GetParams(ctx).BondDenom description := stake.Description{ Moniker: simulation.RandStringOfLength(r, 10), @@ -56,7 +59,10 @@ func SimulateMsgCreateValidator(m auth.AccountMapper, k stake.Keeper) simulation // SimulateMsgEditValidator func SimulateMsgEditValidator(k stake.Keeper) simulation.TestAndRunTx { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, + keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + description := stake.Description{ Moniker: simulation.RandStringOfLength(r, 10), Identity: simulation.RandStringOfLength(r, 10), @@ -84,7 +90,10 @@ func SimulateMsgEditValidator(k stake.Keeper) simulation.TestAndRunTx { // SimulateMsgDelegate func SimulateMsgDelegate(m auth.AccountMapper, k stake.Keeper) simulation.TestAndRunTx { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, + keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + denom := k.GetParams(ctx).BondDenom validatorKey := simulation.RandomKey(r, keys) validatorAddress := sdk.AccAddress(validatorKey.PubKey().Address()) @@ -116,7 +125,10 @@ func SimulateMsgDelegate(m auth.AccountMapper, k stake.Keeper) simulation.TestAn // SimulateMsgBeginUnbonding func SimulateMsgBeginUnbonding(m auth.AccountMapper, k stake.Keeper) simulation.TestAndRunTx { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, + keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + denom := k.GetParams(ctx).BondDenom validatorKey := simulation.RandomKey(r, keys) validatorAddress := sdk.AccAddress(validatorKey.PubKey().Address()) @@ -148,7 +160,10 @@ func SimulateMsgBeginUnbonding(m auth.AccountMapper, k stake.Keeper) simulation. // SimulateMsgCompleteUnbonding func SimulateMsgCompleteUnbonding(k stake.Keeper) simulation.TestAndRunTx { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, + keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + validatorKey := simulation.RandomKey(r, keys) validatorAddress := sdk.AccAddress(validatorKey.PubKey().Address()) delegatorKey := simulation.RandomKey(r, keys) @@ -171,7 +186,10 @@ func SimulateMsgCompleteUnbonding(k stake.Keeper) simulation.TestAndRunTx { // SimulateMsgBeginRedelegate func SimulateMsgBeginRedelegate(m auth.AccountMapper, k stake.Keeper) simulation.TestAndRunTx { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, + keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + denom := k.GetParams(ctx).BondDenom sourceValidatorKey := simulation.RandomKey(r, keys) sourceValidatorAddress := sdk.AccAddress(sourceValidatorKey.PubKey().Address()) @@ -207,7 +225,10 @@ func SimulateMsgBeginRedelegate(m auth.AccountMapper, k stake.Keeper) simulation // SimulateMsgCompleteRedelegate func SimulateMsgCompleteRedelegate(k stake.Keeper) simulation.TestAndRunTx { - return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + + return func(t *testing.T, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, + keys []crypto.PrivKey, log string, event func(string)) (action string, err sdk.Error) { + validatorSrcKey := simulation.RandomKey(r, keys) validatorSrcAddress := sdk.AccAddress(validatorSrcKey.PubKey().Address()) validatorDstKey := simulation.RandomKey(r, keys) From f2d47f9e9187e99b5e87d78145848205593add0b Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Mon, 27 Aug 2018 14:46:38 -0400 Subject: [PATCH 04/14] slash contract, cwgoes comment --- x/stake/keeper/slash.go | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/x/stake/keeper/slash.go b/x/stake/keeper/slash.go index b936e585d0..c3c79714ee 100644 --- a/x/stake/keeper/slash.go +++ b/x/stake/keeper/slash.go @@ -18,6 +18,8 @@ import ( // Infraction committed equal to or less than an unbonding period in the past, // so all unbonding delegations and redelegations from that height are stored // CONTRACT: +// Slash will not slash unbonded validators (for the above reason) +// CONTRACT: // Infraction committed at the current height or at a past height, // not at a height in the future func (k Keeper) Slash(ctx sdk.Context, pubkey crypto.PubKey, infractionHeight int64, power int64, slashFactor sdk.Dec) { @@ -44,24 +46,15 @@ func (k Keeper) Slash(ctx sdk.Context, pubkey crypto.PubKey, infractionHeight in return } - // do not slash if unbonded - unbonded := false + // slashing should not be slashing unbonded if validator.Status == sdk.Unbonded { - unbonded = true + panic(fmt.Sprintf("should not be slashing unbonded validator: %v", validator)) } else if validator.Status == sdk.Unbonding { ctxTime := ctx.BlockHeader().Time if ctxTime.After(validator.UnbondingMinTime) { - - // TODO should we also just update the - // validator status to unbonded here? - unbonded = true + panic(fmt.Sprintf("should not be slashing unbonded validator: %v", validator)) } } - if unbonded { - logger.Info(fmt.Sprintf( - "failed attempt to slash an unbonded validator")) - return - } operatorAddress := validator.GetOperator() From d1a2fed2bcec1f14829a6afb4e4ab6aff2d1d161 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Wed, 29 Aug 2018 02:28:27 -0400 Subject: [PATCH 05/14] tests working --- x/stake/keeper/delegation_test.go | 188 ++++++++++++++++++++++++++++-- x/stake/keeper/slash_test.go | 10 -- 2 files changed, 181 insertions(+), 17 deletions(-) diff --git a/x/stake/keeper/delegation_test.go b/x/stake/keeper/delegation_test.go index df324beab4..add16fb52c 100644 --- a/x/stake/keeper/delegation_test.go +++ b/x/stake/keeper/delegation_test.go @@ -7,6 +7,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/stake/types" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -162,9 +163,7 @@ func TestUnbondDelegation(t *testing.T) { } keeper.SetDelegation(ctx, delegation) - var err error - var amount sdk.Dec - amount, err = keeper.unbond(ctx, addrDels[0], addrVals[0], sdk.NewDec(6)) + amount, err := keeper.unbond(ctx, addrDels[0], addrVals[0], sdk.NewDec(6)) require.NoError(t, err) require.Equal(t, int64(6), amount.RoundInt64()) // shares to be added to an unbonding delegation / redelegation @@ -180,12 +179,187 @@ func TestUnbondDelegation(t *testing.T) { require.Equal(t, int64(4), pool.BondedTokens.RoundInt64()) } +// test removing all self delegation from a validator which should +// shift it from the bonded to unbonded state +func TestUndelegateSelfDelegation(t *testing.T) { + + ctx, _, keeper := CreateTestInput(t, false, 0) + pool := keeper.GetPool(ctx) + pool.LooseTokens = sdk.NewDec(20) + + //create a validator with a self-delegation + validator := types.NewValidator(addrVals[0], PKs[0], types.Description{}) + validator, pool, issuedShares := validator.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator = keeper.UpdateValidator(ctx, validator) + pool = keeper.GetPool(ctx) + selfDelegation := types.Delegation{ + DelegatorAddr: addrVals[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, selfDelegation) + + // create a second delegation to this validator + validator, pool, issuedShares = validator.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator = keeper.UpdateValidator(ctx, validator) + pool = keeper.GetPool(ctx) + delegation := types.Delegation{ + DelegatorAddr: addrDels[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, delegation) + + err := keeper.BeginUnbonding(ctx, addrVals[0], addrVals[0], sdk.NewDec(10)) + require.NoError(t, err) + + validator, found := keeper.GetValidator(ctx, addrVals[0]) + require.True(t, found) + pool = keeper.GetPool(ctx) + + require.Equal(t, int64(10), validator.Tokens.RoundInt64()) + require.Equal(t, sdk.Unbonding, validator.Status) +} + func TestUndelegateFromUnbondingValidator(t *testing.T) { - require.Fail(t) + ctx, _, keeper := CreateTestInput(t, false, 0) + pool := keeper.GetPool(ctx) + pool.LooseTokens = sdk.NewDec(20) + + //create a validator with a self-delegation + validator := types.NewValidator(addrVals[0], PKs[0], types.Description{}) + + validator, pool, issuedShares := validator.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator = keeper.UpdateValidator(ctx, validator) + pool = keeper.GetPool(ctx) + selfDelegation := types.Delegation{ + DelegatorAddr: addrVals[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, selfDelegation) + + // create a second delegation to this validator + validator, pool, issuedShares = validator.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator = keeper.UpdateValidator(ctx, validator) + pool = keeper.GetPool(ctx) + delegation := types.Delegation{ + DelegatorAddr: addrDels[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, delegation) + + header := ctx.BlockHeader() + blockHeight := int64(10) + header.Height = blockHeight + blockTime := time.Unix(333, 0) + header.Time = blockTime + ctx = ctx.WithBlockHeader(header) + + // unbond the all self-delegation to put validator in unbonding state + err := keeper.BeginUnbonding(ctx, addrVals[0], addrVals[0], sdk.NewDec(10)) + require.NoError(t, err) + + validator, found := keeper.GetValidator(ctx, addrVals[0]) + require.True(t, found) + require.Equal(t, blockHeight, validator.UnbondingHeight) + params := keeper.GetParams(ctx) + require.True(t, blockTime.Add(params.UnbondingTime).Equal(validator.UnbondingMinTime)) + + //change the context + header = ctx.BlockHeader() + blockHeight2 := int64(20) + header.Height = blockHeight2 + blockTime2 := time.Unix(444, 0) + header.Time = blockTime2 + ctx = ctx.WithBlockHeader(header) + + // unbond some of the other delegation's shares + err = keeper.BeginUnbonding(ctx, addrDels[0], addrVals[0], sdk.NewDec(6)) + require.NoError(t, err) + + // retrieve the unbonding delegation + ubd, found := keeper.GetUnbondingDelegation(ctx, addrDels[0], addrVals[0]) + require.True(t, found) + require.True(t, ubd.Balance.IsEqual(sdk.NewInt64Coin(params.BondDenom, 6))) + assert.Equal(t, blockHeight, ubd.CreationHeight) + assert.True(t, blockTime.Add(params.UnbondingTime).Equal(ubd.MinTime)) } func TestUndelegateFromUnbondedValidator(t *testing.T) { - require.Fail(t) + ctx, _, keeper := CreateTestInput(t, false, 0) + pool := keeper.GetPool(ctx) + pool.LooseTokens = sdk.NewDec(20) + + //create a validator with a self-delegation + validator := types.NewValidator(addrVals[0], PKs[0], types.Description{}) + + validator, pool, issuedShares := validator.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator = keeper.UpdateValidator(ctx, validator) + pool = keeper.GetPool(ctx) + selfDelegation := types.Delegation{ + DelegatorAddr: addrVals[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, selfDelegation) + + // create a second delegation to this validator + validator, pool, issuedShares = validator.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator = keeper.UpdateValidator(ctx, validator) + pool = keeper.GetPool(ctx) + delegation := types.Delegation{ + DelegatorAddr: addrDels[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, delegation) + + header := ctx.BlockHeader() + blockHeight := int64(10) + header.Height = blockHeight + blockTime := time.Unix(333, 0) + header.Time = blockTime + ctx = ctx.WithBlockHeader(header) + + // unbond the all self-delegation to put validator in unbonding state + err := keeper.BeginUnbonding(ctx, addrVals[0], addrVals[0], sdk.NewDec(10)) + require.NoError(t, err) + + validator, found := keeper.GetValidator(ctx, addrVals[0]) + require.True(t, found) + require.Equal(t, blockHeight, validator.UnbondingHeight) + params := keeper.GetParams(ctx) + require.True(t, blockTime.Add(params.UnbondingTime).Equal(validator.UnbondingMinTime)) + + // change the context to one which makes the validator considered unbonded + header = ctx.BlockHeader() + blockHeight2 := int64(20) + header.Height = blockHeight2 + blockTime2 := time.Unix(444, 0).Add(params.UnbondingTime) + header.Time = blockTime2 + ctx = ctx.WithBlockHeader(header) + + // unbond some of the other delegation's shares + err = keeper.BeginUnbonding(ctx, addrDels[0], addrVals[0], sdk.NewDec(6)) + require.NoError(t, err) + + // no ubd should have been found, coins should have been returned direcly to account + _, found = keeper.GetUnbondingDelegation(ctx, addrDels[0], addrVals[0]) + require.False(t, found) } // Make sure that that the retrieving the delegations doesn't affect the state @@ -269,9 +443,9 @@ func TestRedelegation(t *testing.T) { } func TestRedelegationFromUnbondingValidator(t *testing.T) { - require.Fail(t) + require.Fail(t, "") } func TestRedelegationFromUnbondedValidator(t *testing.T) { - require.Fail(t) + require.Fail(t, "") } diff --git a/x/stake/keeper/slash_test.go b/x/stake/keeper/slash_test.go index c53d4e1ec2..65bac2d808 100644 --- a/x/stake/keeper/slash_test.go +++ b/x/stake/keeper/slash_test.go @@ -206,16 +206,6 @@ func TestSlashValidatorAtCurrentHeight(t *testing.T) { require.Equal(t, sdk.NewDec(5).RoundInt64(), oldPool.BondedTokens.Sub(newPool.BondedTokens).RoundInt64()) } -// slash an unbonding validator -func TestSlashUnbondingValidator(t *testing.T) { - require.Fail(t) -} - -// fail an attept to slash an unbonded validator -func TestSlashUnbondedValidator(t *testing.T) { - require.Fail(t) -} - // tests Slash at a previous height with an unbonding delegation func TestSlashWithUnbondingDelegation(t *testing.T) { ctx, keeper, params := setupHelper(t, 10) From 52b249b0cdf6267541d6b7ccd7ab726336f8c363 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Wed, 29 Aug 2018 18:44:40 -0400 Subject: [PATCH 06/14] complete tests --- x/stake/keeper/delegation.go | 39 +++++- x/stake/keeper/delegation_test.go | 205 +++++++++++++++++++++++++++++- x/stake/keeper/slash.go | 9 +- x/stake/types/validator.go | 13 ++ 4 files changed, 248 insertions(+), 18 deletions(-) diff --git a/x/stake/keeper/delegation.go b/x/stake/keeper/delegation.go index 88260cb7d5..7f66cd5550 100644 --- a/x/stake/keeper/delegation.go +++ b/x/stake/keeper/delegation.go @@ -2,6 +2,7 @@ package keeper import ( "bytes" + "fmt" "time" sdk "github.com/cosmos/cosmos-sdk/types" @@ -256,44 +257,56 @@ func (k Keeper) Delegate(ctx sdk.Context, delegatorAddr sdk.AccAddress, bondAmt // unbond the the delegation return func (k Keeper) unbond(ctx sdk.Context, delegatorAddr, validatorAddr sdk.AccAddress, shares sdk.Dec) (amount sdk.Dec, err sdk.Error) { + fmt.Println("wackydebugoutput unbond 0") // check if delegation has any shares in it unbond delegation, found := k.GetDelegation(ctx, delegatorAddr, validatorAddr) if !found { + fmt.Println("wackydebugoutput unbond 1") err = types.ErrNoDelegatorForAddress(k.Codespace()) return } + fmt.Println("wackydebugoutput unbond 2") // retrieve the amount to remove if delegation.Shares.LT(shares) { + fmt.Println("wackydebugoutput unbond 3") err = types.ErrNotEnoughDelegationShares(k.Codespace(), delegation.Shares.String()) return } + fmt.Println("wackydebugoutput unbond 4") // get validator validator, found := k.GetValidator(ctx, validatorAddr) if !found { + fmt.Println("wackydebugoutput unbond 5") err = types.ErrNoValidatorFound(k.Codespace()) return } + fmt.Println("wackydebugoutput unbond 6") // subtract shares from delegator delegation.Shares = delegation.Shares.Sub(shares) // remove the delegation if delegation.Shares.IsZero() { + fmt.Println("wackydebugoutput unbond 7") // if the delegation is the operator of the validator then // trigger a jail validator if bytes.Equal(delegation.DelegatorAddr, validator.Operator) && validator.Jailed == false { + fmt.Println("wackydebugoutput unbond 8") validator.Jailed = true } + fmt.Println("wackydebugoutput unbond 9") k.RemoveDelegation(ctx, delegation) } else { + fmt.Println("wackydebugoutput unbond 10") // Update height delegation.Height = ctx.BlockHeight() k.SetDelegation(ctx, delegation) } + fmt.Println("wackydebugoutput unbond 11") // remove the coins from the validator pool := k.GetPool(ctx) @@ -303,9 +316,12 @@ func (k Keeper) unbond(ctx sdk.Context, delegatorAddr, validatorAddr sdk.AccAddr // update then remove validator if necessary validator = k.UpdateValidator(ctx, validator) + fmt.Printf("debug validator: %v\n", validator) if validator.DelegatorShares.IsZero() { + fmt.Println("wackydebugoutput unbond 12") k.RemoveValidator(ctx, validator.Operator) } + fmt.Println("wackydebugoutput unbond 13") return } @@ -315,24 +331,25 @@ func (k Keeper) unbond(ctx sdk.Context, delegatorAddr, validatorAddr sdk.AccAddr // get info for begin functions: MinTime and CreationHeight func (k Keeper) getBeginInfo(ctx sdk.Context, params types.Params, validatorSrcAddr sdk.AccAddress) ( minTime time.Time, height int64, completeNow bool) { + fmt.Println("wackydebugoutput getBeginInfo 0") validator, found := k.GetValidator(ctx, validatorSrcAddr) switch { case !found || validator.Status == sdk.Bonded: - // longest wait - just unbonding period from now + // the longest wait - just unbonding period from now minTime = ctx.BlockHeader().Time.Add(params.UnbondingTime) height = ctx.BlockHeader().Height return minTime, height, false + case validator.IsUnbonded(ctx): + return minTime, height, true + case validator.Status == sdk.Unbonding: minTime = validator.UnbondingMinTime height = validator.UnbondingHeight return minTime, height, false - case validator.Status == sdk.Unbonded: - return minTime, height, true - default: panic("unknown validator status") } @@ -403,27 +420,38 @@ func (k Keeper) CompleteUnbonding(ctx sdk.Context, delegatorAddr, validatorAddr // complete unbonding an unbonding record func (k Keeper) BeginRedelegation(ctx sdk.Context, delegatorAddr, validatorSrcAddr, validatorDstAddr sdk.AccAddress, sharesAmount sdk.Dec) sdk.Error { + fmt.Println("wackydebugoutput BeginRedelegation 0") // check if this is a transitive redelegation if k.HasReceivingRedelegation(ctx, delegatorAddr, validatorSrcAddr) { + fmt.Println("wackydebugoutput BeginRedelegation 1") return types.ErrTransitiveRedelegation(k.Codespace()) } + fmt.Println("wackydebugoutput BeginRedelegation 2") returnAmount, err := k.unbond(ctx, delegatorAddr, validatorSrcAddr, sharesAmount) if err != nil { + fmt.Println("wackydebugoutput BeginRedelegation 3") return err } + fmt.Println("wackydebugoutput BeginRedelegation 4") params := k.GetParams(ctx) returnCoin := sdk.Coin{params.BondDenom, returnAmount.RoundInt()} + fmt.Printf("debug returnCoin: %v\n", returnCoin) + fmt.Println("wackydebugoutput BeginRedelegation 5") dstValidator, found := k.GetValidator(ctx, validatorDstAddr) if !found { + fmt.Println("wackydebugoutput BeginRedelegation 6") return types.ErrBadRedelegationDst(k.Codespace()) } + fmt.Println("wackydebugoutput BeginRedelegation 7") sharesCreated, err := k.Delegate(ctx, delegatorAddr, returnCoin, dstValidator, false) if err != nil { + fmt.Println("wackydebugoutput BeginRedelegation 8") return err } + fmt.Println("wackydebugoutput BeginRedelegation 9") // create the unbonding delegation minTime := ctx.BlockHeader().Time.Add(params.UnbondingTime) @@ -432,8 +460,10 @@ func (k Keeper) BeginRedelegation(ctx sdk.Context, delegatorAddr, validatorSrcAd minTime, height, completeNow := k.getBeginInfo(ctx, params, validatorSrcAddr) if completeNow { // no need to create the redelegation object + fmt.Println("wackydebugoutput BeginRedelegation 10") return nil } + fmt.Println("wackydebugoutput BeginRedelegation 11") red := types.Redelegation{ DelegatorAddr: delegatorAddr, @@ -446,6 +476,7 @@ func (k Keeper) BeginRedelegation(ctx sdk.Context, delegatorAddr, validatorSrcAd Balance: returnCoin, InitialBalance: returnCoin, } + fmt.Println("wackydebugoutput BeginRedelegation 13") k.SetRedelegation(ctx, red) return nil } diff --git a/x/stake/keeper/delegation_test.go b/x/stake/keeper/delegation_test.go index add16fb52c..e64bfa38ba 100644 --- a/x/stake/keeper/delegation_test.go +++ b/x/stake/keeper/delegation_test.go @@ -219,8 +219,6 @@ func TestUndelegateSelfDelegation(t *testing.T) { validator, found := keeper.GetValidator(ctx, addrVals[0]) require.True(t, found) - pool = keeper.GetPool(ctx) - require.Equal(t, int64(10), validator.Tokens.RoundInt64()) require.Equal(t, sdk.Unbonding, validator.Status) } @@ -358,8 +356,8 @@ func TestUndelegateFromUnbondedValidator(t *testing.T) { require.NoError(t, err) // no ubd should have been found, coins should have been returned direcly to account - _, found = keeper.GetUnbondingDelegation(ctx, addrDels[0], addrVals[0]) - require.False(t, found) + ubd, found := keeper.GetUnbondingDelegation(ctx, addrDels[0], addrVals[0]) + require.False(t, found, "%v", ubd) } // Make sure that that the retrieving the delegations doesn't affect the state @@ -442,10 +440,203 @@ func TestRedelegation(t *testing.T) { require.False(t, found) } -func TestRedelegationFromUnbondingValidator(t *testing.T) { - require.Fail(t, "") +func TestRedelegateSelfDelegation(t *testing.T) { + + ctx, _, keeper := CreateTestInput(t, false, 0) + pool := keeper.GetPool(ctx) + pool.LooseTokens = sdk.NewDec(30) + + //create a validator with a self-delegation + validator := types.NewValidator(addrVals[0], PKs[0], types.Description{}) + validator, pool, issuedShares := validator.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator = keeper.UpdateValidator(ctx, validator) + pool = keeper.GetPool(ctx) + selfDelegation := types.Delegation{ + DelegatorAddr: addrVals[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, selfDelegation) + + // create a second validator + validator2 := types.NewValidator(addrVals[1], PKs[1], types.Description{}) + validator2, pool, issuedShares = validator2.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator2 = keeper.UpdateValidator(ctx, validator2) + + // create a second delegation to this validator + validator, pool, issuedShares = validator.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator = keeper.UpdateValidator(ctx, validator) + pool = keeper.GetPool(ctx) + delegation := types.Delegation{ + DelegatorAddr: addrDels[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, delegation) + + err := keeper.BeginRedelegation(ctx, addrVals[0], addrVals[0], addrVals[1], sdk.NewDec(10)) + require.NoError(t, err) + + validator, found := keeper.GetValidator(ctx, addrVals[0]) + require.True(t, found) + require.Equal(t, int64(10), validator.Tokens.RoundInt64()) + require.Equal(t, sdk.Unbonding, validator.Status) } -func TestRedelegationFromUnbondedValidator(t *testing.T) { +func TestRedelegateFromUnbondingValidator(t *testing.T) { + ctx, _, keeper := CreateTestInput(t, false, 0) + pool := keeper.GetPool(ctx) + pool.LooseTokens = sdk.NewDec(30) + + //create a validator with a self-delegation + validator := types.NewValidator(addrVals[0], PKs[0], types.Description{}) + + validator, pool, issuedShares := validator.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator = keeper.UpdateValidator(ctx, validator) + pool = keeper.GetPool(ctx) + selfDelegation := types.Delegation{ + DelegatorAddr: addrVals[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, selfDelegation) + + // create a second delegation to this validator + validator, pool, issuedShares = validator.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator = keeper.UpdateValidator(ctx, validator) + pool = keeper.GetPool(ctx) + delegation := types.Delegation{ + DelegatorAddr: addrDels[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, delegation) + + // create a second validator + validator2 := types.NewValidator(addrVals[1], PKs[1], types.Description{}) + validator2, pool, issuedShares = validator2.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator2 = keeper.UpdateValidator(ctx, validator2) + + header := ctx.BlockHeader() + blockHeight := int64(10) + header.Height = blockHeight + blockTime := time.Unix(333, 0) + header.Time = blockTime + ctx = ctx.WithBlockHeader(header) + + // unbond the all self-delegation to put validator in unbonding state + err := keeper.BeginUnbonding(ctx, addrVals[0], addrVals[0], sdk.NewDec(10)) + require.NoError(t, err) + + validator, found := keeper.GetValidator(ctx, addrVals[0]) + require.True(t, found) + require.Equal(t, blockHeight, validator.UnbondingHeight) + params := keeper.GetParams(ctx) + require.True(t, blockTime.Add(params.UnbondingTime).Equal(validator.UnbondingMinTime)) + + //change the context + header = ctx.BlockHeader() + blockHeight2 := int64(20) + header.Height = blockHeight2 + blockTime2 := time.Unix(444, 0) + header.Time = blockTime2 + ctx = ctx.WithBlockHeader(header) + + // unbond some of the other delegation's shares + err = keeper.BeginRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1], sdk.NewDec(6)) + require.NoError(t, err) + + // retrieve the unbonding delegation + ubd, found := keeper.GetRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1]) + require.True(t, found) + require.True(t, ubd.Balance.IsEqual(sdk.NewInt64Coin(params.BondDenom, 6))) + assert.Equal(t, blockHeight, ubd.CreationHeight) + assert.True(t, blockTime.Add(params.UnbondingTime).Equal(ubd.MinTime)) +} + +func TestRedelegateFromUnbondedValidator(t *testing.T) { + ctx, _, keeper := CreateTestInput(t, false, 0) + pool := keeper.GetPool(ctx) + pool.LooseTokens = sdk.NewDec(30) + + //create a validator with a self-delegation + validator := types.NewValidator(addrVals[0], PKs[0], types.Description{}) + + validator, pool, issuedShares := validator.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator = keeper.UpdateValidator(ctx, validator) + pool = keeper.GetPool(ctx) + selfDelegation := types.Delegation{ + DelegatorAddr: addrVals[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, selfDelegation) + + // create a second delegation to this validator + validator, pool, issuedShares = validator.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator = keeper.UpdateValidator(ctx, validator) + pool = keeper.GetPool(ctx) + delegation := types.Delegation{ + DelegatorAddr: addrDels[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, delegation) + + // create a second validator + validator2 := types.NewValidator(addrVals[1], PKs[1], types.Description{}) + validator2, pool, issuedShares = validator2.AddTokensFromDel(pool, sdk.NewInt(10)) + require.Equal(t, int64(10), issuedShares.RoundInt64()) + keeper.SetPool(ctx, pool) + validator2 = keeper.UpdateValidator(ctx, validator2) + + header := ctx.BlockHeader() + blockHeight := int64(10) + header.Height = blockHeight + blockTime := time.Unix(333, 0) + header.Time = blockTime + ctx = ctx.WithBlockHeader(header) + + // unbond the all self-delegation to put validator in unbonding state + err := keeper.BeginUnbonding(ctx, addrVals[0], addrVals[0], sdk.NewDec(10)) + require.NoError(t, err) + + validator, found := keeper.GetValidator(ctx, addrVals[0]) + require.True(t, found) + require.Equal(t, blockHeight, validator.UnbondingHeight) + params := keeper.GetParams(ctx) + require.True(t, blockTime.Add(params.UnbondingTime).Equal(validator.UnbondingMinTime)) + + // change the context to one which makes the validator considered unbonded + header = ctx.BlockHeader() + blockHeight2 := int64(20) + header.Height = blockHeight2 + blockTime2 := time.Unix(444, 0).Add(params.UnbondingTime) + header.Time = blockTime2 + ctx = ctx.WithBlockHeader(header) + + // unbond some of the other delegation's shares + err = keeper.BeginRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1], sdk.NewDec(6)) + require.NoError(t, err) + + // no ubd should have been found, coins should have been returned direcly to account + ubd, found := keeper.GetRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1]) + require.False(t, found, "%v", ubd) require.Fail(t, "") } diff --git a/x/stake/keeper/slash.go b/x/stake/keeper/slash.go index c3c79714ee..be3572d7c3 100644 --- a/x/stake/keeper/slash.go +++ b/x/stake/keeper/slash.go @@ -46,14 +46,9 @@ func (k Keeper) Slash(ctx sdk.Context, pubkey crypto.PubKey, infractionHeight in return } - // slashing should not be slashing unbonded - if validator.Status == sdk.Unbonded { + // should not be slashing unbonded + if validator.IsUnbonded(ctx) { panic(fmt.Sprintf("should not be slashing unbonded validator: %v", validator)) - } else if validator.Status == sdk.Unbonding { - ctxTime := ctx.BlockHeader().Time - if ctxTime.After(validator.UnbondingMinTime) { - panic(fmt.Sprintf("should not be slashing unbonded validator: %v", validator)) - } } operatorAddress := validator.GetOperator() diff --git a/x/stake/types/validator.go b/x/stake/types/validator.go index a912b26dc7..13b00f9094 100644 --- a/x/stake/types/validator.go +++ b/x/stake/types/validator.go @@ -418,6 +418,19 @@ func (v Validator) BondedTokens() sdk.Dec { return sdk.ZeroDec() } +// Returns if the validator should be considered unbonded +func (v Validator) IsUnbonded(ctx sdk.Context) bool { + if v.Status == sdk.Unbonded { + return true + } else if v.Status == sdk.Unbonding { + ctxTime := ctx.BlockHeader().Time + if ctxTime.After(v.UnbondingMinTime) { + return true + } + } + return false +} + //______________________________________________________________________ // ensure fulfills the sdk validator types From 2a1515f00a5fe06b74c4e8c005ccb4b5826ca1dc Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Wed, 29 Aug 2018 18:48:42 -0400 Subject: [PATCH 07/14] changelog --- PENDING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/PENDING.md b/PENDING.md index dd18be3c0b..5c7d04d75a 100644 --- a/PENDING.md +++ b/PENDING.md @@ -19,6 +19,8 @@ BREAKING CHANGES * [x/stake] \#1901 Validator type's Owner field renamed to Operator; Validator's GetOwner() renamed accordingly to comply with the SDK's Validator interface. * [docs] [#2001](https://github.com/cosmos/cosmos-sdk/pull/2001) Update slashing spec for slashing period * [x/stake, x/slashing] [#1305](https://github.com/cosmos/cosmos-sdk/issues/1305) - Rename "revoked" to "jailed" + * [x/stake] [#1676] Revoked and jailed validators put into the unbonding state + * [x/stake] [#1877] Redelegations/unbonding-delegation from unbonding validator have reduced time * SDK * [core] \#1807 Switch from use of rational to decimal From 6fd8299b1a000a63171a6de8cc828f05288840b3 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Wed, 29 Aug 2018 18:54:02 -0400 Subject: [PATCH 08/14] ... --- x/stake/keeper/delegation.go | 31 ------------------------------- x/stake/keeper/delegation_test.go | 1 - 2 files changed, 32 deletions(-) diff --git a/x/stake/keeper/delegation.go b/x/stake/keeper/delegation.go index 7f66cd5550..cfe21c03f7 100644 --- a/x/stake/keeper/delegation.go +++ b/x/stake/keeper/delegation.go @@ -2,7 +2,6 @@ package keeper import ( "bytes" - "fmt" "time" sdk "github.com/cosmos/cosmos-sdk/types" @@ -257,56 +256,44 @@ func (k Keeper) Delegate(ctx sdk.Context, delegatorAddr sdk.AccAddress, bondAmt // unbond the the delegation return func (k Keeper) unbond(ctx sdk.Context, delegatorAddr, validatorAddr sdk.AccAddress, shares sdk.Dec) (amount sdk.Dec, err sdk.Error) { - fmt.Println("wackydebugoutput unbond 0") // check if delegation has any shares in it unbond delegation, found := k.GetDelegation(ctx, delegatorAddr, validatorAddr) if !found { - fmt.Println("wackydebugoutput unbond 1") err = types.ErrNoDelegatorForAddress(k.Codespace()) return } - fmt.Println("wackydebugoutput unbond 2") // retrieve the amount to remove if delegation.Shares.LT(shares) { - fmt.Println("wackydebugoutput unbond 3") err = types.ErrNotEnoughDelegationShares(k.Codespace(), delegation.Shares.String()) return } - fmt.Println("wackydebugoutput unbond 4") // get validator validator, found := k.GetValidator(ctx, validatorAddr) if !found { - fmt.Println("wackydebugoutput unbond 5") err = types.ErrNoValidatorFound(k.Codespace()) return } - fmt.Println("wackydebugoutput unbond 6") // subtract shares from delegator delegation.Shares = delegation.Shares.Sub(shares) // remove the delegation if delegation.Shares.IsZero() { - fmt.Println("wackydebugoutput unbond 7") // if the delegation is the operator of the validator then // trigger a jail validator if bytes.Equal(delegation.DelegatorAddr, validator.Operator) && validator.Jailed == false { - fmt.Println("wackydebugoutput unbond 8") validator.Jailed = true } - fmt.Println("wackydebugoutput unbond 9") k.RemoveDelegation(ctx, delegation) } else { - fmt.Println("wackydebugoutput unbond 10") // Update height delegation.Height = ctx.BlockHeight() k.SetDelegation(ctx, delegation) } - fmt.Println("wackydebugoutput unbond 11") // remove the coins from the validator pool := k.GetPool(ctx) @@ -316,12 +303,9 @@ func (k Keeper) unbond(ctx sdk.Context, delegatorAddr, validatorAddr sdk.AccAddr // update then remove validator if necessary validator = k.UpdateValidator(ctx, validator) - fmt.Printf("debug validator: %v\n", validator) if validator.DelegatorShares.IsZero() { - fmt.Println("wackydebugoutput unbond 12") k.RemoveValidator(ctx, validator.Operator) } - fmt.Println("wackydebugoutput unbond 13") return } @@ -331,7 +315,6 @@ func (k Keeper) unbond(ctx sdk.Context, delegatorAddr, validatorAddr sdk.AccAddr // get info for begin functions: MinTime and CreationHeight func (k Keeper) getBeginInfo(ctx sdk.Context, params types.Params, validatorSrcAddr sdk.AccAddress) ( minTime time.Time, height int64, completeNow bool) { - fmt.Println("wackydebugoutput getBeginInfo 0") validator, found := k.GetValidator(ctx, validatorSrcAddr) switch { @@ -420,38 +403,27 @@ func (k Keeper) CompleteUnbonding(ctx sdk.Context, delegatorAddr, validatorAddr // complete unbonding an unbonding record func (k Keeper) BeginRedelegation(ctx sdk.Context, delegatorAddr, validatorSrcAddr, validatorDstAddr sdk.AccAddress, sharesAmount sdk.Dec) sdk.Error { - fmt.Println("wackydebugoutput BeginRedelegation 0") // check if this is a transitive redelegation if k.HasReceivingRedelegation(ctx, delegatorAddr, validatorSrcAddr) { - fmt.Println("wackydebugoutput BeginRedelegation 1") return types.ErrTransitiveRedelegation(k.Codespace()) } - fmt.Println("wackydebugoutput BeginRedelegation 2") returnAmount, err := k.unbond(ctx, delegatorAddr, validatorSrcAddr, sharesAmount) if err != nil { - fmt.Println("wackydebugoutput BeginRedelegation 3") return err } - fmt.Println("wackydebugoutput BeginRedelegation 4") params := k.GetParams(ctx) returnCoin := sdk.Coin{params.BondDenom, returnAmount.RoundInt()} - fmt.Printf("debug returnCoin: %v\n", returnCoin) - fmt.Println("wackydebugoutput BeginRedelegation 5") dstValidator, found := k.GetValidator(ctx, validatorDstAddr) if !found { - fmt.Println("wackydebugoutput BeginRedelegation 6") return types.ErrBadRedelegationDst(k.Codespace()) } - fmt.Println("wackydebugoutput BeginRedelegation 7") sharesCreated, err := k.Delegate(ctx, delegatorAddr, returnCoin, dstValidator, false) if err != nil { - fmt.Println("wackydebugoutput BeginRedelegation 8") return err } - fmt.Println("wackydebugoutput BeginRedelegation 9") // create the unbonding delegation minTime := ctx.BlockHeader().Time.Add(params.UnbondingTime) @@ -460,10 +432,8 @@ func (k Keeper) BeginRedelegation(ctx sdk.Context, delegatorAddr, validatorSrcAd minTime, height, completeNow := k.getBeginInfo(ctx, params, validatorSrcAddr) if completeNow { // no need to create the redelegation object - fmt.Println("wackydebugoutput BeginRedelegation 10") return nil } - fmt.Println("wackydebugoutput BeginRedelegation 11") red := types.Redelegation{ DelegatorAddr: delegatorAddr, @@ -476,7 +446,6 @@ func (k Keeper) BeginRedelegation(ctx sdk.Context, delegatorAddr, validatorSrcAd Balance: returnCoin, InitialBalance: returnCoin, } - fmt.Println("wackydebugoutput BeginRedelegation 13") k.SetRedelegation(ctx, red) return nil } diff --git a/x/stake/keeper/delegation_test.go b/x/stake/keeper/delegation_test.go index e64bfa38ba..d0fd736d93 100644 --- a/x/stake/keeper/delegation_test.go +++ b/x/stake/keeper/delegation_test.go @@ -638,5 +638,4 @@ func TestRedelegateFromUnbondedValidator(t *testing.T) { // no ubd should have been found, coins should have been returned direcly to account ubd, found := keeper.GetRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1]) require.False(t, found, "%v", ubd) - require.Fail(t, "") } From 8d02c3eba09cce9c88a5c710cfda059189708c4b Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Wed, 29 Aug 2018 20:24:35 -0400 Subject: [PATCH 09/14] fix golint --- x/stake/keeper/delegation.go | 3 --- x/stake/keeper/slash.go | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/x/stake/keeper/delegation.go b/x/stake/keeper/delegation.go index cfe21c03f7..d19c9bf82e 100644 --- a/x/stake/keeper/delegation.go +++ b/x/stake/keeper/delegation.go @@ -426,9 +426,6 @@ func (k Keeper) BeginRedelegation(ctx sdk.Context, delegatorAddr, validatorSrcAd } // create the unbonding delegation - minTime := ctx.BlockHeader().Time.Add(params.UnbondingTime) - height := ctx.BlockHeader().Height - minTime, height, completeNow := k.getBeginInfo(ctx, params, validatorSrcAddr) if completeNow { // no need to create the redelegation object diff --git a/x/stake/keeper/slash.go b/x/stake/keeper/slash.go index be3572d7c3..0658afb862 100644 --- a/x/stake/keeper/slash.go +++ b/x/stake/keeper/slash.go @@ -22,6 +22,8 @@ import ( // CONTRACT: // Infraction committed at the current height or at a past height, // not at a height in the future +// +// nolint: gocyclo func (k Keeper) Slash(ctx sdk.Context, pubkey crypto.PubKey, infractionHeight int64, power int64, slashFactor sdk.Dec) { logger := ctx.Logger().With("module", "x/stake") From d900c583ffa3f35cd881d71270309f0b8943711d Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 30 Aug 2018 21:16:23 -0400 Subject: [PATCH 10/14] melekes comment --- x/stake/types/validator.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x/stake/types/validator.go b/x/stake/types/validator.go index 13b00f9094..5a325ec73c 100644 --- a/x/stake/types/validator.go +++ b/x/stake/types/validator.go @@ -420,9 +420,10 @@ func (v Validator) BondedTokens() sdk.Dec { // Returns if the validator should be considered unbonded func (v Validator) IsUnbonded(ctx sdk.Context) bool { - if v.Status == sdk.Unbonded { + switch v.Status { + case sdk.Unbonded: return true - } else if v.Status == sdk.Unbonding { + case sdk.Unbonding: ctxTime := ctx.BlockHeader().Time if ctxTime.After(v.UnbondingMinTime) { return true From 07ba719c752fb3a746470fe342044861ee52c1e3 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Fri, 31 Aug 2018 20:50:12 -0400 Subject: [PATCH 11/14] minor corrections --- x/slashing/handler_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index 2ba8dcc4db..748164ebbe 100644 --- a/x/slashing/handler_test.go +++ b/x/slashing/handler_test.go @@ -50,10 +50,11 @@ func TestJailedValidatorDelegations(t *testing.T) { JailedUntil: time.Unix(0, 0), SignedBlocksCounter: int64(0), } - slashingKeeper.setValidatorSigningInfo(ctx, valAddr, newInfo) + consAddr := sdk.ConsAddress(valAddr.Bytes()) + slashingKeeper.setValidatorSigningInfo(ctx, consAddr, newInfo) // delegate tokens to the validator - delAddr := addrs[1] + delAddr := sdk.AccAddress(addrs[1]) msgDelegate := newTestMsgDelegate(delAddr, valAddr, bondAmount) got = stake.NewHandler(stakeKeeper)(ctx, msgDelegate) require.True(t, got.IsOK(), "expected delegation to be ok, got %v", got) From 9593a2641ae15e93a12df4a70a3500201219a98a Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Fri, 31 Aug 2018 20:52:45 -0400 Subject: [PATCH 12/14] ... --- x/slashing/handler_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index 748164ebbe..67139a06ec 100644 --- a/x/slashing/handler_test.go +++ b/x/slashing/handler_test.go @@ -38,7 +38,9 @@ func TestJailedValidatorDelegations(t *testing.T) { // create a validator amount := int64(10) - valAddr, valPubKey, bondAmount := sdk.ValAddress(addrs[0]), pks[0], sdk.NewInt(amount) + valPubKey, bondAmount := pks[0], sdk.NewInt(amount) + consAddr, valAddr := sdk.ConsAddress(addrs[0]), sdk.ValAddress(addrs[1]) + msgCreateVal := newTestMsgCreateValidator(valAddr, valPubKey, bondAmount) got := stake.NewHandler(stakeKeeper)(ctx, msgCreateVal) require.True(t, got.IsOK(), "expected create validator msg to be ok, got: %v", got) @@ -50,11 +52,10 @@ func TestJailedValidatorDelegations(t *testing.T) { JailedUntil: time.Unix(0, 0), SignedBlocksCounter: int64(0), } - consAddr := sdk.ConsAddress(valAddr.Bytes()) slashingKeeper.setValidatorSigningInfo(ctx, consAddr, newInfo) // delegate tokens to the validator - delAddr := sdk.AccAddress(addrs[1]) + delAddr := sdk.AccAddress(addrs[2]) msgDelegate := newTestMsgDelegate(delAddr, valAddr, bondAmount) got = stake.NewHandler(stakeKeeper)(ctx, msgDelegate) require.True(t, got.IsOK(), "expected delegation to be ok, got %v", got) From 6e295085c191a92ed8ac87421bc7071c8701e205 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Fri, 31 Aug 2018 20:56:35 -0400 Subject: [PATCH 13/14] ... --- x/slashing/handler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index 67139a06ec..c5afb87389 100644 --- a/x/slashing/handler_test.go +++ b/x/slashing/handler_test.go @@ -39,7 +39,7 @@ func TestJailedValidatorDelegations(t *testing.T) { // create a validator amount := int64(10) valPubKey, bondAmount := pks[0], sdk.NewInt(amount) - consAddr, valAddr := sdk.ConsAddress(addrs[0]), sdk.ValAddress(addrs[1]) + valAddr, consAddr := sdk.ValAddress(addrs[1]), sdk.ConsAddress(addrs[0]) msgCreateVal := newTestMsgCreateValidator(valAddr, valPubKey, bondAmount) got := stake.NewHandler(stakeKeeper)(ctx, msgCreateVal) From f0c13bbfbd835d4f05177f84dd1137f3c0d3d510 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Fri, 31 Aug 2018 21:38:50 -0400 Subject: [PATCH 14/14] resolve merge errors --- x/stake/keeper/delegation.go | 10 +++++----- x/stake/keeper/delegation_test.go | 30 ++++++++++++++++++------------ 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/x/stake/keeper/delegation.go b/x/stake/keeper/delegation.go index f6a99f529b..9a596ffbce 100644 --- a/x/stake/keeper/delegation.go +++ b/x/stake/keeper/delegation.go @@ -315,10 +315,10 @@ func (k Keeper) unbond(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValA //______________________________________________________________________________________________________ // get info for begin functions: MinTime and CreationHeight -func (k Keeper) getBeginInfo(ctx sdk.Context, params types.Params, validatorSrcAddr sdk.AccAddress) ( +func (k Keeper) getBeginInfo(ctx sdk.Context, params types.Params, valSrcAddr sdk.ValAddress) ( minTime time.Time, height int64, completeNow bool) { - validator, found := k.GetValidator(ctx, validatorSrcAddr) + validator, found := k.GetValidator(ctx, valSrcAddr) switch { case !found || validator.Status == sdk.Bonded: @@ -357,12 +357,12 @@ func (k Keeper) BeginUnbonding(ctx sdk.Context, // create the unbonding delegation params := k.GetParams(ctx) - minTime, height, completeNow := k.getBeginInfo(ctx, params, validatorAddr) + minTime, height, completeNow := k.getBeginInfo(ctx, params, valAddr) balance := sdk.Coin{params.BondDenom, returnAmount.RoundInt()} // no need to create the ubd object just complete now if completeNow { - _, _, err := k.coinKeeper.AddCoins(ctx, delegatorAddr, sdk.Coins{balance}) + _, _, err := k.coinKeeper.AddCoins(ctx, delAddr, sdk.Coins{balance}) if err != nil { return err } @@ -429,7 +429,7 @@ func (k Keeper) BeginRedelegation(ctx sdk.Context, delAddr sdk.AccAddress, } // create the unbonding delegation - minTime, height, completeNow := k.getBeginInfo(ctx, params, validatorSrcAddr) + minTime, height, completeNow := k.getBeginInfo(ctx, params, valSrcAddr) if completeNow { // no need to create the redelegation object return nil diff --git a/x/stake/keeper/delegation_test.go b/x/stake/keeper/delegation_test.go index d0fd736d93..023642bb22 100644 --- a/x/stake/keeper/delegation_test.go +++ b/x/stake/keeper/delegation_test.go @@ -195,7 +195,7 @@ func TestUndelegateSelfDelegation(t *testing.T) { validator = keeper.UpdateValidator(ctx, validator) pool = keeper.GetPool(ctx) selfDelegation := types.Delegation{ - DelegatorAddr: addrVals[0], + DelegatorAddr: sdk.AccAddress(addrVals[0].Bytes()), ValidatorAddr: addrVals[0], Shares: issuedShares, } @@ -214,7 +214,8 @@ func TestUndelegateSelfDelegation(t *testing.T) { } keeper.SetDelegation(ctx, delegation) - err := keeper.BeginUnbonding(ctx, addrVals[0], addrVals[0], sdk.NewDec(10)) + val0AccAddr := sdk.AccAddress(addrVals[0].Bytes()) + err := keeper.BeginUnbonding(ctx, val0AccAddr, addrVals[0], sdk.NewDec(10)) require.NoError(t, err) validator, found := keeper.GetValidator(ctx, addrVals[0]) @@ -237,7 +238,7 @@ func TestUndelegateFromUnbondingValidator(t *testing.T) { validator = keeper.UpdateValidator(ctx, validator) pool = keeper.GetPool(ctx) selfDelegation := types.Delegation{ - DelegatorAddr: addrVals[0], + DelegatorAddr: sdk.AccAddress(addrVals[0].Bytes()), ValidatorAddr: addrVals[0], Shares: issuedShares, } @@ -264,7 +265,8 @@ func TestUndelegateFromUnbondingValidator(t *testing.T) { ctx = ctx.WithBlockHeader(header) // unbond the all self-delegation to put validator in unbonding state - err := keeper.BeginUnbonding(ctx, addrVals[0], addrVals[0], sdk.NewDec(10)) + val0AccAddr := sdk.AccAddress(addrVals[0].Bytes()) + err := keeper.BeginUnbonding(ctx, val0AccAddr, addrVals[0], sdk.NewDec(10)) require.NoError(t, err) validator, found := keeper.GetValidator(ctx, addrVals[0]) @@ -306,8 +308,9 @@ func TestUndelegateFromUnbondedValidator(t *testing.T) { keeper.SetPool(ctx, pool) validator = keeper.UpdateValidator(ctx, validator) pool = keeper.GetPool(ctx) + val0AccAddr := sdk.AccAddress(addrVals[0].Bytes()) selfDelegation := types.Delegation{ - DelegatorAddr: addrVals[0], + DelegatorAddr: val0AccAddr, ValidatorAddr: addrVals[0], Shares: issuedShares, } @@ -334,7 +337,7 @@ func TestUndelegateFromUnbondedValidator(t *testing.T) { ctx = ctx.WithBlockHeader(header) // unbond the all self-delegation to put validator in unbonding state - err := keeper.BeginUnbonding(ctx, addrVals[0], addrVals[0], sdk.NewDec(10)) + err := keeper.BeginUnbonding(ctx, val0AccAddr, addrVals[0], sdk.NewDec(10)) require.NoError(t, err) validator, found := keeper.GetValidator(ctx, addrVals[0]) @@ -453,8 +456,9 @@ func TestRedelegateSelfDelegation(t *testing.T) { keeper.SetPool(ctx, pool) validator = keeper.UpdateValidator(ctx, validator) pool = keeper.GetPool(ctx) + val0AccAddr := sdk.AccAddress(addrVals[0].Bytes()) selfDelegation := types.Delegation{ - DelegatorAddr: addrVals[0], + DelegatorAddr: val0AccAddr, ValidatorAddr: addrVals[0], Shares: issuedShares, } @@ -480,7 +484,7 @@ func TestRedelegateSelfDelegation(t *testing.T) { } keeper.SetDelegation(ctx, delegation) - err := keeper.BeginRedelegation(ctx, addrVals[0], addrVals[0], addrVals[1], sdk.NewDec(10)) + err := keeper.BeginRedelegation(ctx, val0AccAddr, addrVals[0], addrVals[1], sdk.NewDec(10)) require.NoError(t, err) validator, found := keeper.GetValidator(ctx, addrVals[0]) @@ -502,8 +506,9 @@ func TestRedelegateFromUnbondingValidator(t *testing.T) { keeper.SetPool(ctx, pool) validator = keeper.UpdateValidator(ctx, validator) pool = keeper.GetPool(ctx) + val0AccAddr := sdk.AccAddress(addrVals[0].Bytes()) selfDelegation := types.Delegation{ - DelegatorAddr: addrVals[0], + DelegatorAddr: val0AccAddr, ValidatorAddr: addrVals[0], Shares: issuedShares, } @@ -537,7 +542,7 @@ func TestRedelegateFromUnbondingValidator(t *testing.T) { ctx = ctx.WithBlockHeader(header) // unbond the all self-delegation to put validator in unbonding state - err := keeper.BeginUnbonding(ctx, addrVals[0], addrVals[0], sdk.NewDec(10)) + err := keeper.BeginUnbonding(ctx, val0AccAddr, addrVals[0], sdk.NewDec(10)) require.NoError(t, err) validator, found := keeper.GetValidator(ctx, addrVals[0]) @@ -579,8 +584,9 @@ func TestRedelegateFromUnbondedValidator(t *testing.T) { keeper.SetPool(ctx, pool) validator = keeper.UpdateValidator(ctx, validator) pool = keeper.GetPool(ctx) + val0AccAddr := sdk.AccAddress(addrVals[0].Bytes()) selfDelegation := types.Delegation{ - DelegatorAddr: addrVals[0], + DelegatorAddr: val0AccAddr, ValidatorAddr: addrVals[0], Shares: issuedShares, } @@ -614,7 +620,7 @@ func TestRedelegateFromUnbondedValidator(t *testing.T) { ctx = ctx.WithBlockHeader(header) // unbond the all self-delegation to put validator in unbonding state - err := keeper.BeginUnbonding(ctx, addrVals[0], addrVals[0], sdk.NewDec(10)) + err := keeper.BeginUnbonding(ctx, val0AccAddr, addrVals[0], sdk.NewDec(10)) require.NoError(t, err) validator, found := keeper.GetValidator(ctx, addrVals[0])