From d204afbdae16c325aa0392e25f863343d162e07c Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Fri, 31 May 2019 11:14:40 -0400 Subject: [PATCH] Merge PR #4461: Port advisory patch to master --- x/staking/handler_test.go | 3 +- x/staking/keeper/delegation.go | 54 ++++++++++++----------------- x/staking/keeper/delegation_test.go | 19 ++++------ 3 files changed, 29 insertions(+), 47 deletions(-) diff --git a/x/staking/handler_test.go b/x/staking/handler_test.go index 4a6fa4581064..b0cc6d183bc2 100644 --- a/x/staking/handler_test.go +++ b/x/staking/handler_test.go @@ -797,8 +797,7 @@ func TestUnbondingFromUnbondingValidator(t *testing.T) { got = handleMsgUndelegate(ctx, msgUndelegateDelegator, keeper) require.True(t, got.IsOK(), "expected no error") - // move the Block time forward by one second - ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(time.Second * 1)) + ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(keeper.UnbondingTime(ctx))) // Run the EndBlocker EndBlocker(ctx, keeper) diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index 621d566b9e73..92cb5a8082dd 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -2,6 +2,7 @@ package keeper import ( "bytes" + "fmt" "time" sdk "github.com/cosmos/cosmos-sdk/types" @@ -544,16 +545,18 @@ func (k Keeper) unbond(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValA return amount, nil } -// get info for begin functions: completionTime and CreationHeight -func (k Keeper) getBeginInfo(ctx sdk.Context, valSrcAddr sdk.ValAddress) ( - completionTime time.Time, height int64, completeNow bool) { +// getBeginInfo returns the completion time and height of a redelegation, along +// with a boolean signaling if the redelegation is complete based on the source +// validator. +func (k Keeper) getBeginInfo( + ctx sdk.Context, valSrcAddr sdk.ValAddress, +) (completionTime time.Time, height int64, completeNow bool) { validator, found := k.GetValidator(ctx, valSrcAddr) + // TODO: When would the validator not be found? switch { - // TODO: when would the validator not be found? case !found || validator.Status == sdk.Bonded: - // the longest wait - just unbonding period from now completionTime = ctx.BlockHeader().Time.Add(k.UnbondingTime(ctx)) height = ctx.BlockHeight() @@ -563,48 +566,35 @@ func (k Keeper) getBeginInfo(ctx sdk.Context, valSrcAddr sdk.ValAddress) ( return completionTime, height, true case validator.Status == sdk.Unbonding: - completionTime = validator.UnbondingCompletionTime - height = validator.UnbondingHeight - return completionTime, height, false + return validator.UnbondingCompletionTime, validator.UnbondingHeight, false default: - panic("unknown validator status") + panic(fmt.Sprintf("unknown validator status: %s", validator.Status)) } } -// begin unbonding part or all of a delegation -func (k Keeper) Undelegate(ctx sdk.Context, delAddr sdk.AccAddress, - valAddr sdk.ValAddress, sharesAmount sdk.Dec) (completionTime time.Time, sdkErr sdk.Error) { - - // create the unbonding delegation - completionTime, height, completeNow := k.getBeginInfo(ctx, valAddr) +// Undelegate unbonds an amount of delegator shares from a given validator. It +// will verify that the unbonding entries between the delegator and validator +// are not exceeded and unbond the staked tokens (based on shares) by creating +// an unbonding object and inserting it into the unbonding queue which will be +// processed during the staking EndBlocker. +func (k Keeper) Undelegate( + ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress, sharesAmount sdk.Dec, +) (time.Time, sdk.Error) { returnAmount, err := k.unbond(ctx, delAddr, valAddr, sharesAmount) if err != nil { - return completionTime, err - } - balance := sdk.NewCoin(k.BondDenom(ctx), returnAmount) - - // no need to create the ubd object just complete now - if completeNow { - // track undelegation only when remaining or truncated shares are non-zero - if !balance.IsZero() { - if _, err := k.bankKeeper.UndelegateCoins(ctx, delAddr, sdk.Coins{balance}); err != nil { - return completionTime, err - } - } - - return completionTime, nil + return time.Time{}, err } if k.HasMaxUnbondingDelegationEntries(ctx, delAddr, valAddr) { return time.Time{}, types.ErrMaxUnbondingDelegationEntries(k.Codespace()) } - ubd := k.SetUnbondingDelegationEntry(ctx, delAddr, - valAddr, height, completionTime, returnAmount) - + completionTime := ctx.BlockHeader().Time.Add(k.UnbondingTime(ctx)) + ubd := k.SetUnbondingDelegationEntry(ctx, delAddr, valAddr, ctx.BlockHeight(), completionTime, returnAmount) k.InsertUBDQueue(ctx, ubd, completionTime) + return completionTime, nil } diff --git a/x/staking/keeper/delegation_test.go b/x/staking/keeper/delegation_test.go index bc9d8e72bfae..0ab38591b552 100644 --- a/x/staking/keeper/delegation_test.go +++ b/x/staking/keeper/delegation_test.go @@ -350,13 +350,10 @@ func TestUndelegateFromUnbondingValidator(t *testing.T) { params := keeper.GetParams(ctx) require.True(t, blockTime.Add(params.UnbondingTime).Equal(validator.UnbondingCompletionTime)) - //change the context - header = ctx.BlockHeader() blockHeight2 := int64(20) - header.Height = blockHeight2 - blockTime2 := time.Unix(444, 0) - header.Time = blockTime2 - ctx = ctx.WithBlockHeader(header) + blockTime2 := time.Unix(444, 0).UTC() + ctx = ctx.WithBlockHeight(blockHeight2) + ctx = ctx.WithBlockTime(blockTime2) // unbond some of the other delegation's shares _, err = keeper.Undelegate(ctx, addrDels[0], addrVals[0], sdk.NewDec(6)) @@ -367,8 +364,8 @@ func TestUndelegateFromUnbondingValidator(t *testing.T) { require.True(t, found) require.Len(t, ubd.Entries, 1) require.True(t, ubd.Entries[0].Balance.Equal(sdk.NewInt(6))) - assert.Equal(t, blockHeight, ubd.Entries[0].CreationHeight) - assert.True(t, blockTime.Add(params.UnbondingTime).Equal(ubd.Entries[0].CompletionTime)) + assert.Equal(t, blockHeight2, ubd.Entries[0].CreationHeight) + assert.True(t, blockTime2.Add(params.UnbondingTime).Equal(ubd.Entries[0].CompletionTime)) } func TestUndelegateFromUnbondedValidator(t *testing.T) { @@ -377,7 +374,7 @@ func TestUndelegateFromUnbondedValidator(t *testing.T) { startTokens := sdk.TokensFromTendermintPower(20) pool.NotBondedTokens = startTokens - //create a validator with a self-delegation + // create a validator with a self-delegation validator := types.NewValidator(addrVals[0], PKs[0], types.Description{}) valTokens := sdk.TokensFromTendermintPower(10) @@ -432,10 +429,6 @@ func TestUndelegateFromUnbondedValidator(t *testing.T) { _, err = keeper.Undelegate(ctx, addrDels[0], addrVals[0], unbondTokens.ToDec()) require.NoError(t, err) - // no ubd should have been found, coins should have been returned direcly to account - ubd, found := keeper.GetUnbondingDelegation(ctx, addrDels[0], addrVals[0]) - require.False(t, found, "%v", ubd) - // unbond rest of the other delegation's shares remainingTokens := delTokens.Sub(unbondTokens) _, err = keeper.Undelegate(ctx, addrDels[0], addrVals[0], remainingTokens.ToDec())