From 2885ac586e96a48eccfd07bb6e7192d49912ee6b Mon Sep 17 00:00:00 2001 From: Rigel Date: Thu, 12 Jul 2018 19:38:35 -0400 Subject: [PATCH] Merge PR #1660: Redelegation doesn't subtract from liquid * fix redelegation subtracting source coins * changelog * Add testcases for balance subtraction * Move changelog entry --- CHANGELOG.md | 2 +- x/stake/handler.go | 4 ++-- x/stake/handler_test.go | 25 ++++++++++++++++++++++++- x/stake/keeper/delegation.go | 19 +++++++++++-------- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cadfc352..8ac7bd437 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,8 @@ BREAKING CHANGES FEATURES * [baseapp] NewBaseApp now takes option functions as parameters - BUG FIXES +* \#1630 - redelegation nolonger removes tokens from the delegator liquid account * [keys] \#1629 - updating password no longer asks for a new password when the first entered password was incorrect * [lcd] importing an account would create a random account diff --git a/x/stake/handler.go b/x/stake/handler.go index c355179cf..031edda43 100644 --- a/x/stake/handler.go +++ b/x/stake/handler.go @@ -81,7 +81,7 @@ func handleMsgCreateValidator(ctx sdk.Context, msg types.MsgCreateValidator, k k // move coins from the msg.Address account to a (self-delegation) delegator account // the validator account and global shares are updated within here - _, err := k.Delegate(ctx, msg.DelegatorAddr, msg.Delegation, validator) + _, err := k.Delegate(ctx, msg.DelegatorAddr, msg.Delegation, validator, true) if err != nil { return err.Result() } @@ -136,7 +136,7 @@ func handleMsgDelegate(ctx sdk.Context, msg types.MsgDelegate, k keeper.Keeper) if validator.Revoked == true { return ErrValidatorRevoked(k.Codespace()).Result() } - _, err := k.Delegate(ctx, msg.DelegatorAddr, msg.Delegation, validator) + _, err := k.Delegate(ctx, msg.DelegatorAddr, msg.Delegation, validator, true) if err != nil { return err.Result() } diff --git a/x/stake/handler_test.go b/x/stake/handler_test.go index 95a078802..dac938e6b 100644 --- a/x/stake/handler_test.go +++ b/x/stake/handler_test.go @@ -268,6 +268,7 @@ func TestIncrementsMsgUnbond(t *testing.T) { initBond := int64(1000) ctx, accMapper, keeper := keep.CreateTestInput(t, false, initBond) params := setInstantUnbondPeriod(keeper, ctx) + denom := params.BondDenom // create validator, delegate validatorAddr, delegatorAddr := keep.Addrs[0], keep.Addrs[1] @@ -276,10 +277,17 @@ func TestIncrementsMsgUnbond(t *testing.T) { got := handleMsgCreateValidator(ctx, msgCreateValidator, keeper) require.True(t, got.IsOK(), "expected create-validator to be ok, got %v", got) + // initial balance + amt1 := accMapper.GetAccount(ctx, delegatorAddr).GetCoins().AmountOf(denom) + msgDelegate := newTestMsgDelegate(delegatorAddr, validatorAddr, initBond) got = handleMsgDelegate(ctx, msgDelegate, keeper) require.True(t, got.IsOK(), "expected delegation to be ok, got %v", got) + // balance should have been subtracted after delegation + amt2 := accMapper.GetAccount(ctx, delegatorAddr).GetCoins().AmountOf(denom) + require.Equal(t, amt1.Sub(sdk.NewInt(initBond)).Int64(), amt2.Int64(), "expected coins to be subtracted") + validator, found := keeper.GetValidator(ctx, validatorAddr) require.True(t, found) require.Equal(t, initBond*2, validator.DelegatorShares.RoundInt64()) @@ -528,8 +536,9 @@ func TestUnbondingPeriod(t *testing.T) { } func TestRedelegationPeriod(t *testing.T) { - ctx, _, keeper := keep.CreateTestInput(t, false, 1000) + ctx, AccMapper, keeper := keep.CreateTestInput(t, false, 1000) validatorAddr, validatorAddr2 := keep.Addrs[0], keep.Addrs[1] + denom := keeper.GetParams(ctx).BondDenom // set the unbonding time params := keeper.GetParams(ctx) @@ -538,18 +547,32 @@ func TestRedelegationPeriod(t *testing.T) { // create the validators msgCreateValidator := newTestMsgCreateValidator(validatorAddr, keep.PKs[0], 10) + + // initial balance + amt1 := AccMapper.GetAccount(ctx, validatorAddr).GetCoins().AmountOf(denom) + got := handleMsgCreateValidator(ctx, msgCreateValidator, keeper) require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") + // balance should have been subtracted after creation + amt2 := AccMapper.GetAccount(ctx, validatorAddr).GetCoins().AmountOf(denom) + require.Equal(t, amt1.Sub(sdk.NewInt(10)).Int64(), amt2.Int64(), "expected coins to be subtracted") + msgCreateValidator = newTestMsgCreateValidator(validatorAddr2, keep.PKs[1], 10) got = handleMsgCreateValidator(ctx, msgCreateValidator, keeper) require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") + bal1 := AccMapper.GetAccount(ctx, validatorAddr).GetCoins() + // begin redelegate msgBeginRedelegate := NewMsgBeginRedelegate(validatorAddr, validatorAddr, validatorAddr2, sdk.NewRat(10)) got = handleMsgBeginRedelegate(ctx, msgBeginRedelegate, keeper) require.True(t, got.IsOK(), "expected no error, %v", got) + // origin account should not lose tokens as with a regular delegation + bal2 := AccMapper.GetAccount(ctx, validatorAddr).GetCoins() + require.Equal(t, bal1, bal2) + // cannot complete redelegation at same time msgCompleteRedelegate := NewMsgCompleteRedelegate(validatorAddr, validatorAddr, validatorAddr2) got = handleMsgCompleteRedelegate(ctx, msgCompleteRedelegate, keeper) diff --git a/x/stake/keeper/delegation.go b/x/stake/keeper/delegation.go index e2f96f3d9..6bf357e79 100644 --- a/x/stake/keeper/delegation.go +++ b/x/stake/keeper/delegation.go @@ -200,9 +200,9 @@ func (k Keeper) RemoveRedelegation(ctx sdk.Context, red types.Redelegation) { //_____________________________________________________________________________________ -// Perform a delegation, set/update everything necessary within the store +// Perform a delegation, set/update everything necessary within the store. func (k Keeper) Delegate(ctx sdk.Context, delegatorAddr sdk.AccAddress, bondAmt sdk.Coin, - validator types.Validator) (newShares sdk.Rat, err sdk.Error) { + validator types.Validator, subtractAccount bool) (newShares sdk.Rat, err sdk.Error) { // Get or create the delegator delegation delegation, found := k.GetDelegation(ctx, delegatorAddr, validator.Owner) @@ -214,12 +214,15 @@ func (k Keeper) Delegate(ctx sdk.Context, delegatorAddr sdk.AccAddress, bondAmt } } - // Account new shares, save - pool := k.GetPool(ctx) - _, _, err = k.coinKeeper.SubtractCoins(ctx, delegation.DelegatorAddr, sdk.Coins{bondAmt}) - if err != nil { - return + if subtractAccount { + // Account new shares, save + _, _, err = k.coinKeeper.SubtractCoins(ctx, delegation.DelegatorAddr, sdk.Coins{bondAmt}) + if err != nil { + return + } } + + pool := k.GetPool(ctx) validator, pool, newShares = validator.AddTokensFromDel(pool, bondAmt.Amount.Int64()) delegation.Shares = delegation.Shares.Add(newShares) @@ -358,7 +361,7 @@ func (k Keeper) BeginRedelegation(ctx sdk.Context, delegatorAddr, validatorSrcAd if !found { return types.ErrBadRedelegationDst(k.Codespace()) } - sharesCreated, err := k.Delegate(ctx, delegatorAddr, returnCoin, dstValidator) + sharesCreated, err := k.Delegate(ctx, delegatorAddr, returnCoin, dstValidator, false) if err != nil { return err }