From b4d24b1bb860961f2edeb26e5b5968c1a7963b55 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 12 Jul 2018 18:51:57 -0400 Subject: [PATCH 1/4] fix redelegation subtracting source coins --- x/stake/handler.go | 4 ++-- x/stake/handler_test.go | 8 +++++++- x/stake/keeper/delegation.go | 19 +++++++++++-------- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/x/stake/handler.go b/x/stake/handler.go index c355179cf296..031edda43be3 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 95a07880224f..48f988e89061 100644 --- a/x/stake/handler_test.go +++ b/x/stake/handler_test.go @@ -528,7 +528,7 @@ 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] // set the unbonding time @@ -545,11 +545,17 @@ func TestRedelegationPeriod(t *testing.T) { 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 e2f96f3d9f57..6bf357e79ba7 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 } From 8c7c52e42685ca4286d21aaf4fa2a275037ef91d Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 12 Jul 2018 18:53:27 -0400 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4ad1c0d98d6..7f862f9704a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -143,6 +143,7 @@ BUG FIXES * \#1287 - prevent zero power validators at genesis * [x/stake] fix bug when unbonding/redelegating using `--shares-percent` * \#1010 - two validators can't bond with the same pubkey anymore +* \#1630 - redelegation nolonger removes tokens from the delegator liquid account ## 0.19.0 From b88fb7c3df8d448b33f3d11db16d5ad30e63e9ee Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Fri, 13 Jul 2018 01:14:12 +0200 Subject: [PATCH 3/4] Add testcases for balance subtraction --- x/stake/handler_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/x/stake/handler_test.go b/x/stake/handler_test.go index 48f988e89061..dac938e6be5f 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()) @@ -530,6 +538,7 @@ func TestUnbondingPeriod(t *testing.T) { func TestRedelegationPeriod(t *testing.T) { 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,9 +547,17 @@ 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") From 62ba32932d22bc9f4de0566d42213ce5c9b9e1c4 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Fri, 13 Jul 2018 01:23:09 +0200 Subject: [PATCH 4/4] Move changelog entry --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f862f9704a8..dd181f7f7985 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ BREAKING CHANGES FEATURES * [baseapp] NewBaseApp now takes option functions as parameters +BUG FIXES +* \#1630 - redelegation nolonger removes tokens from the delegator liquid account + ## 0.20.0 *July 10th, 2018* @@ -143,7 +146,6 @@ BUG FIXES * \#1287 - prevent zero power validators at genesis * [x/stake] fix bug when unbonding/redelegating using `--shares-percent` * \#1010 - two validators can't bond with the same pubkey anymore -* \#1630 - redelegation nolonger removes tokens from the delegator liquid account ## 0.19.0