Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

R4R: limit unbonding delegation / redelegations #3372

Merged
merged 4 commits into from
Jan 24, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ FEATURES
* [\#3179](https://github.com/cosmos/cosmos-sdk/pull/3179) New CodeNoSignatures error code.
* \#3319 [x/distribution] Queriers for all distribution state worth querying; distribution query commands
* [\#3356](https://github.com/cosmos/cosmos-sdk/issues/3356) [x/auth] bech32-ify accounts address in error message.
* \#3270 [x/staking] limit number of ongoing unbonding delegations /redelegations per pair/trio

* Tendermint

Expand Down
4 changes: 4 additions & 0 deletions docs/spec/staking/messages.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions docs/spec/staking/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
```
Expand Down
37 changes: 37 additions & 0 deletions x/staking/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,20 @@ 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
}
if len(ubd.Entries) >= int(k.MaxEntries(ctx)) {
rigelrozanski marked this conversation as resolved.
Show resolved Hide resolved
return true
}
return false
}

// set the unbonding delegation and associated index
func (k Keeper) SetUnbondingDelegation(ctx sdk.Context, ubd types.UnbondingDelegation) {
store := ctx.KVStore(k.storeKey)
Expand Down Expand Up @@ -314,6 +328,21 @@ 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
}
if len(red.Entries) >= int(k.MaxEntries(ctx)) {
rigelrozanski marked this conversation as resolved.
Show resolved Hide resolved
return true
}
return false
}

// set a redelegation and associated index
func (k Keeper) SetRedelegation(ctx sdk.Context, red types.Redelegation) {
store := ctx.KVStore(k.storeKey)
Expand Down Expand Up @@ -576,6 +605,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)

Expand Down Expand Up @@ -633,6 +666,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
Expand Down
102 changes: 101 additions & 1 deletion x/staking/keeper/delegation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
rigelrozanski marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand Down Expand Up @@ -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
rigelrozanski marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down
19 changes: 14 additions & 5 deletions x/staking/keeper/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,27 @@ 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)
return
}

// 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
Expand Down
10 changes: 8 additions & 2 deletions x/staking/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
"to many unbonding delegation entries in this delegator/validator duo, please wait for some entries to mature")
rigelrozanski marked this conversation as resolved.
Show resolved Hide resolved
}

func ErrBadRedelegationAddr(codespace sdk.CodespaceType) sdk.Error {
Expand Down Expand Up @@ -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,
"to many redelegation entries in this delegator/src-validator/dst-validator trio, please wait for some entries to mature")
rigelrozanski marked this conversation as resolved.
Show resolved Hide resolved
}

func ErrDelegatorShareExRateInvalid(codespace sdk.CodespaceType) sdk.Error {
return sdk.NewError(codespace, CodeInvalidDelegation,
"cannot delegate to validators with invalid (zero) ex-rate")
Expand Down
23 changes: 17 additions & 6 deletions x/staking/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const (
var (
KeyUnbondingTime = []byte("UnbondingTime")
KeyMaxValidators = []byte("MaxValidators")
KeyMaxEntries = []byte("KeyMaxEntries")
KeyBondDenom = []byte("BondDenom")
)

Expand All @@ -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},
}
}
Expand All @@ -58,20 +72,17 @@ 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.
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
Expand Down