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: Redelegation doesn't subtract from liquid #1660

Merged
merged 5 commits into from
Jul 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions x/stake/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down Expand Up @@ -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()
}
Expand Down
25 changes: 24 additions & 1 deletion x/stake/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah, makes sense 👍


validator, found := keeper.GetValidator(ctx, validatorAddr)
require.True(t, found)
require.Equal(t, initBond*2, validator.DelegatorShares.RoundInt64())
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
19 changes: 11 additions & 8 deletions x/stake/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)

Expand Down Expand Up @@ -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
}
Expand Down