From dc75eb4eb8465ec1b299a2dae783809550febfe7 Mon Sep 17 00:00:00 2001 From: frog power 4000 Date: Thu, 24 Jan 2019 17:44:31 -0500 Subject: [PATCH] Merge PR #3372: Limit unbonding delegation / redelegations --- PENDING.md | 1 + docs/spec/staking/messages.md | 4 ++ docs/spec/staking/state.md | 1 + x/staking/keeper/delegation.go | 31 +++++++++ x/staking/keeper/delegation_test.go | 102 +++++++++++++++++++++++++++- x/staking/keeper/params.go | 19 ++++-- x/staking/types/errors.go | 10 ++- x/staking/types/params.go | 23 +++++-- 8 files changed, 177 insertions(+), 14 deletions(-) diff --git a/PENDING.md b/PENDING.md index 4d87301f0d5b..0a01c3eb6500 100644 --- a/PENDING.md +++ b/PENDING.md @@ -22,6 +22,7 @@ FEATURES * Gaia * SDK + * \#3270 [x/staking] limit number of ongoing unbonding delegations /redelegations per pair/trio * Tendermint diff --git a/docs/spec/staking/messages.md b/docs/spec/staking/messages.md index 7af6e6cccb2f..39bf43c927b7 100644 --- a/docs/spec/staking/messages.md +++ b/docs/spec/staking/messages.md @@ -101,6 +101,8 @@ This message is expected to fail if: - the delegation doesn't exist - the validator doesn't exist - the delegation has less shares than `SharesAmount` + - existing `UnbondingDelegation` has maximum entries as defined by + params.MaxEntries When this message is processed the following actions occur: - validator's `DelegatorShares` and the delegation's `Shares` are both reduced @@ -143,6 +145,8 @@ This message is expected to fail if: - the delegation has less shares than `SharesAmount` - the source validator has a receiving redelegation which is not matured (aka. the redelegation may be transitive) + - existing `Redelegation` has maximum entries as defined by + params.MaxEntries When this message is processed the following actions occur: - the source validator's `DelegatorShares` and the delegations `Shares` are diff --git a/docs/spec/staking/state.md b/docs/spec/staking/state.md index 5e8f4d327769..cd49b7326393 100644 --- a/docs/spec/staking/state.md +++ b/docs/spec/staking/state.md @@ -28,6 +28,7 @@ and defines overall functioning of the staking module. type Params struct { UnbondingTime time.Duration // time duration of unbonding MaxValidators uint16 // maximum number of validators + MaxEntries uint16 // max entries for either unbonding delegation or redelegation (per pair/trio) BondDenom string // bondable coin denomination } ``` diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index 5a15b4d03597..162d87aa4d6c 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -153,6 +153,17 @@ func (k Keeper) IterateUnbondingDelegations(ctx sdk.Context, fn func(index int64 } } +// HasMaxUnbondingDelegationEntries - unbonding delegation has maximum number of entries +func (k Keeper) HasMaxUnbondingDelegationEntries(ctx sdk.Context, + delegatorAddr sdk.AccAddress, validatorAddr sdk.ValAddress) bool { + + ubd, found := k.GetUnbondingDelegation(ctx, delegatorAddr, validatorAddr) + if !found { + return false + } + return len(ubd.Entries) >= int(k.MaxEntries(ctx)) +} + // set the unbonding delegation and associated index func (k Keeper) SetUnbondingDelegation(ctx sdk.Context, ubd types.UnbondingDelegation) { store := ctx.KVStore(k.storeKey) @@ -313,6 +324,18 @@ func (k Keeper) HasReceivingRedelegation(ctx sdk.Context, return false } +// HasMaxRedelegationEntries - redelegation has maximum number of entries +func (k Keeper) HasMaxRedelegationEntries(ctx sdk.Context, + delegatorAddr sdk.AccAddress, validatorSrcAddr, + validatorDstAddr sdk.ValAddress) bool { + + red, found := k.GetRedelegation(ctx, delegatorAddr, validatorSrcAddr, validatorDstAddr) + if !found { + return false + } + return len(red.Entries) >= int(k.MaxEntries(ctx)) +} + // set a redelegation and associated index func (k Keeper) SetRedelegation(ctx sdk.Context, red types.Redelegation) { store := ctx.KVStore(k.storeKey) @@ -580,6 +603,10 @@ func (k Keeper) Undelegate(ctx sdk.Context, delAddr sdk.AccAddress, return completionTime, nil } + if k.HasMaxUnbondingDelegationEntries(ctx, delAddr, valAddr) { + return time.Time{}, types.ErrMaxUnbondingDelegationEntries(k.Codespace()) + } + ubd := k.SetUnbondingDelegationEntry(ctx, delAddr, valAddr, height, completionTime, balance) @@ -637,6 +664,10 @@ func (k Keeper) BeginRedelegation(ctx sdk.Context, delAddr sdk.AccAddress, return time.Time{}, types.ErrTransitiveRedelegation(k.Codespace()) } + if k.HasMaxRedelegationEntries(ctx, delAddr, valSrcAddr, valDstAddr) { + return time.Time{}, types.ErrMaxRedelegationEntries(k.Codespace()) + } + returnAmount, err := k.unbond(ctx, delAddr, valSrcAddr, sharesAmount) if err != nil { return time.Time{}, err diff --git a/x/staking/keeper/delegation_test.go b/x/staking/keeper/delegation_test.go index 3b4243319ada..590e728267e6 100644 --- a/x/staking/keeper/delegation_test.go +++ b/x/staking/keeper/delegation_test.go @@ -198,7 +198,7 @@ func TestUnbondDelegation(t *testing.T) { amount, err := keeper.unbond(ctx, addrDels[0], addrVals[0], sdk.NewDec(6)) require.NoError(t, err) - require.Equal(t, int64(6), amount.Int64()) // shares to be added to an unbonding delegation / redelegation + require.Equal(t, int64(6), amount.Int64()) // shares to be added to an unbonding delegation delegation, found := keeper.GetDelegation(ctx, addrDels[0], addrVals[0]) require.True(t, found) @@ -212,6 +212,53 @@ func TestUnbondDelegation(t *testing.T) { require.Equal(t, int64(4), pool.BondedTokens.Int64()) } +func TestUnbondingDelegationsMaxEntries(t *testing.T) { + ctx, _, keeper := CreateTestInput(t, false, 0) + pool := keeper.GetPool(ctx) + pool.NotBondedTokens = sdk.NewInt(10) + + // create a validator and a delegator to that validator + 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 = TestingUpdateValidator(keeper, ctx, validator, true) + + pool = keeper.GetPool(ctx) + require.Equal(t, int64(10), pool.BondedTokens.Int64()) + require.Equal(t, int64(10), validator.BondedTokens().Int64()) + + delegation := types.Delegation{ + DelegatorAddr: addrDels[0], + ValidatorAddr: addrVals[0], + Shares: issuedShares, + } + keeper.SetDelegation(ctx, delegation) + + maxEntries := keeper.MaxEntries(ctx) + + // should all pass + var completionTime time.Time + for i := uint16(0); i < maxEntries; i++ { + var err error + completionTime, err = keeper.Undelegate(ctx, addrDels[0], addrVals[0], sdk.NewDec(1)) + require.NoError(t, err) + } + + // an additional unbond should fail due to max entries + _, err := keeper.Undelegate(ctx, addrDels[0], addrVals[0], sdk.NewDec(1)) + require.Error(t, err) + + // mature unbonding delegations + ctx = ctx.WithBlockTime(completionTime) + err = keeper.CompleteUnbonding(ctx, addrDels[0], addrVals[0]) + require.NoError(t, err) + + // unbonding should work again + _, err = keeper.Undelegate(ctx, addrDels[0], addrVals[0], sdk.NewDec(1)) + require.NoError(t, err) +} + // test removing all self delegation from a validator which should // shift it from the bonded to unbonded state func TestUndelegateSelfDelegation(t *testing.T) { @@ -593,6 +640,59 @@ func TestRedelegateToSameValidator(t *testing.T) { } +func TestRedelegationMaxEntries(t *testing.T) { + ctx, _, keeper := CreateTestInput(t, false, 0) + pool := keeper.GetPool(ctx) + pool.NotBondedTokens = sdk.NewInt(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 = TestingUpdateValidator(keeper, ctx, validator, true) + pool = keeper.GetPool(ctx) + val0AccAddr := sdk.AccAddress(addrVals[0].Bytes()) + selfDelegation := types.Delegation{ + DelegatorAddr: val0AccAddr, + 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()) + pool.BondedTokens = pool.BondedTokens.Add(sdk.NewInt(10)) + keeper.SetPool(ctx, pool) + validator2 = TestingUpdateValidator(keeper, ctx, validator2, true) + require.Equal(t, sdk.Bonded, validator2.Status) + + maxEntries := keeper.MaxEntries(ctx) + + // redelegations should pass + var completionTime time.Time + for i := uint16(0); i < maxEntries; i++ { + var err error + completionTime, err = keeper.BeginRedelegation(ctx, val0AccAddr, addrVals[0], addrVals[1], sdk.NewDec(1)) + require.NoError(t, err) + } + + // an additional redelegation should fail due to max entries + _, err := keeper.BeginRedelegation(ctx, val0AccAddr, addrVals[0], addrVals[1], sdk.NewDec(1)) + require.Error(t, err) + + // mature redelegations + ctx = ctx.WithBlockTime(completionTime) + err = keeper.CompleteRedelegation(ctx, val0AccAddr, addrVals[0], addrVals[1]) + require.NoError(t, err) + + // redelegation should work again + _, err = keeper.BeginRedelegation(ctx, val0AccAddr, addrVals[0], addrVals[1], sdk.NewDec(1)) + require.NoError(t, err) +} + func TestRedelegateSelfDelegation(t *testing.T) { ctx, _, keeper := CreateTestInput(t, false, 0) pool := keeper.GetPool(ctx) diff --git a/x/staking/keeper/params.go b/x/staking/keeper/params.go index 2d3fdf296e10..ffc5d45afde0 100644 --- a/x/staking/keeper/params.go +++ b/x/staking/keeper/params.go @@ -30,6 +30,13 @@ func (k Keeper) MaxValidators(ctx sdk.Context) (res uint16) { return } +// MaxEntries - Maximum number of simultaneous unbonding +// delegations or redelegations (per pair/trio) +func (k Keeper) MaxEntries(ctx sdk.Context) (res uint16) { + k.paramstore.Get(ctx, types.KeyMaxEntries, &res) + return +} + // BondDenom - Bondable coin denomination func (k Keeper) BondDenom(ctx sdk.Context) (res string) { k.paramstore.Get(ctx, types.KeyBondDenom, &res) @@ -37,11 +44,13 @@ func (k Keeper) BondDenom(ctx sdk.Context) (res string) { } // Get all parameteras as types.Params -func (k Keeper) GetParams(ctx sdk.Context) (res types.Params) { - res.UnbondingTime = k.UnbondingTime(ctx) - res.MaxValidators = k.MaxValidators(ctx) - res.BondDenom = k.BondDenom(ctx) - return +func (k Keeper) GetParams(ctx sdk.Context) types.Params { + return types.NewParams( + k.UnbondingTime(ctx), + k.MaxValidators(ctx), + k.MaxEntries(ctx), + k.BondDenom(ctx), + ) } // set the params diff --git a/x/staking/types/errors.go b/x/staking/types/errors.go index 2ed4a2411f84..337c9d043452 100644 --- a/x/staking/types/errors.go +++ b/x/staking/types/errors.go @@ -149,8 +149,9 @@ func ErrNoUnbondingDelegation(codespace sdk.CodespaceType) sdk.Error { return sdk.NewError(codespace, CodeInvalidDelegation, "no unbonding delegation found") } -func ErrExistingUnbondingDelegation(codespace sdk.CodespaceType) sdk.Error { - return sdk.NewError(codespace, CodeInvalidDelegation, "existing unbonding delegation found") +func ErrMaxUnbondingDelegationEntries(codespace sdk.CodespaceType) sdk.Error { + return sdk.NewError(codespace, CodeInvalidDelegation, + "too many unbonding delegation entries in this delegator/validator duo, please wait for some entries to mature") } func ErrBadRedelegationAddr(codespace sdk.CodespaceType) sdk.Error { @@ -178,6 +179,11 @@ func ErrTransitiveRedelegation(codespace sdk.CodespaceType) sdk.Error { "redelegation to this validator already in progress, first redelegation to this validator must complete before next redelegation") } +func ErrMaxRedelegationEntries(codespace sdk.CodespaceType) sdk.Error { + return sdk.NewError(codespace, CodeInvalidDelegation, + "too many redelegation entries in this delegator/src-validator/dst-validator trio, please wait for some entries to mature") +} + func ErrDelegatorShareExRateInvalid(codespace sdk.CodespaceType) sdk.Error { return sdk.NewError(codespace, CodeInvalidDelegation, "cannot delegate to validators with invalid (zero) ex-rate") diff --git a/x/staking/types/params.go b/x/staking/types/params.go index 7120085e046e..1f95b160f15b 100644 --- a/x/staking/types/params.go +++ b/x/staking/types/params.go @@ -28,6 +28,7 @@ const ( var ( KeyUnbondingTime = []byte("UnbondingTime") KeyMaxValidators = []byte("MaxValidators") + KeyMaxEntries = []byte("KeyMaxEntries") KeyBondDenom = []byte("BondDenom") ) @@ -37,14 +38,27 @@ var _ params.ParamSet = (*Params)(nil) type Params struct { UnbondingTime time.Duration `json:"unbonding_time"` // time duration of unbonding MaxValidators uint16 `json:"max_validators"` // maximum number of validators + MaxEntries uint16 `json:"max_entries"` // max entries for either unbonding delegation or redelegation (per pair/trio) BondDenom string `json:"bond_denom"` // bondable coin denomination } +func NewParams(unbondingTime time.Duration, maxValidators, maxEntries uint16, + bondDenom string) Params { + + return Params{ + UnbondingTime: unbondingTime, + MaxValidators: maxValidators, + MaxEntries: maxEntries, + BondDenom: bondDenom, + } +} + // Implements params.ParamSet func (p *Params) KeyValuePairs() params.KeyValuePairs { return params.KeyValuePairs{ {KeyUnbondingTime, &p.UnbondingTime}, {KeyMaxValidators, &p.MaxValidators}, + {KeyMaxEntries, &p.MaxEntries}, {KeyBondDenom, &p.BondDenom}, } } @@ -58,11 +72,7 @@ func (p Params) Equal(p2 Params) bool { // DefaultParams returns a default set of parameters. func DefaultParams() Params { - return Params{ - UnbondingTime: defaultUnbondingTime, - MaxValidators: 100, - BondDenom: DefaultBondDenom, - } + return NewParams(defaultUnbondingTime, 100, 7, DefaultBondDenom) } // String returns a human readable string representation of the parameters. @@ -70,8 +80,9 @@ func (p Params) String() string { return fmt.Sprintf(`Params: Unbonding Time: %s) Max Validators: %d) + Max Entries: %d) Bonded Coin Denomination: %s`, p.UnbondingTime, - p.MaxValidators, p.BondDenom) + p.MaxValidators, p.MaxEntries, p.BondDenom) } // unmarshal the current staking params value from store key or panic