Merge PR cosmos#1660: Redelegation doesn't subtract from liquid

* fix redelegation subtracting source coins
* changelog
* Add testcases for balance subtraction
* Move changelog entry

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
 

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()
 	}

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)

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
 	}