From 723e057d955e23852b4b3ea8a1cc5da3e72bb227 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Sat, 7 Jul 2018 00:00:00 +0200 Subject: [PATCH] Merge pull request #1566: Fix validator power decrease bug * Demonstrative testcase * Update when decreasing but still bonded * Only update when decreasing, not when equal * Cleanup conditional; changelog * Clarify comments * Simplify conditional --- CHANGELOG.md | 1 + x/stake/keeper/validator.go | 7 ++++++ x/stake/keeper/validator_test.go | 40 ++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7012336d94..4808b6db84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,6 +97,7 @@ IMPROVEMENTS BUG FIXES * [x/slashing] \#1510 Unrevoked validators cannot un-revoke themselves +* [x/stake] \#1567 Validators decreased in power but not unbonded are now updated in Tendermint * [gaia] Added self delegation for validators in the genesis creation * [lcd] tests now don't depend on raw json text * [stake] error strings lower case diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index 164c500492..c32b536cea 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -261,6 +261,13 @@ func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) type if updatedVal.Owner != nil { // updates to validator occurred to be updated validator = updatedVal } + // if decreased in power but still bonded, update Tendermint validator + // (if updatedVal is set, the validator has changed bonding status) + stillBonded := oldFound && oldValidator.Status() == sdk.Bonded && updatedVal.Owner == nil + if stillBonded && oldValidator.PoolShares.Bonded().GT(validator.PoolShares.Bonded()) { + bz := k.cdc.MustMarshalBinary(validator.ABCIValidator()) + store.Set(GetTendermintUpdatesKey(ownerAddr), bz) + } return validator } diff --git a/x/stake/keeper/validator_test.go b/x/stake/keeper/validator_test.go index c4d197a36b..9ad6e12bc0 100644 --- a/x/stake/keeper/validator_test.go +++ b/x/stake/keeper/validator_test.go @@ -674,3 +674,43 @@ func TestGetTendermintUpdatesNotValidatorCliff(t *testing.T) { require.Equal(t, validators[0].ABCIValidatorZero(), updates[0]) require.Equal(t, validators[2].ABCIValidator(), updates[1]) } + +func TestGetTendermintUpdatesPowerDecrease(t *testing.T) { + ctx, _, keeper := CreateTestInput(t, false, 1000) + + amts := []int64{100, 100} + var validators [2]types.Validator + for i, amt := range amts { + pool := keeper.GetPool(ctx) + validators[i] = types.NewValidator(Addrs[i], PKs[i], types.Description{}) + validators[i], pool, _ = validators[i].AddTokensFromDel(pool, amt) + keeper.SetPool(ctx, pool) + } + validators[0] = keeper.UpdateValidator(ctx, validators[0]) + validators[1] = keeper.UpdateValidator(ctx, validators[1]) + keeper.ClearTendermintUpdates(ctx) + require.Equal(t, 0, len(keeper.GetTendermintUpdates(ctx))) + + // check initial power + require.Equal(t, sdk.NewRat(100).RoundInt64(), validators[0].GetPower().RoundInt64()) + require.Equal(t, sdk.NewRat(100).RoundInt64(), validators[1].GetPower().RoundInt64()) + + // test multiple value change + // tendermintUpdate set: {c1, c3} -> {c1', c3'} + pool := keeper.GetPool(ctx) + validators[0], pool, _ = validators[0].RemoveDelShares(pool, sdk.NewRat(20)) + validators[1], pool, _ = validators[1].RemoveDelShares(pool, sdk.NewRat(30)) + keeper.SetPool(ctx, pool) + validators[0] = keeper.UpdateValidator(ctx, validators[0]) + validators[1] = keeper.UpdateValidator(ctx, validators[1]) + + // power has changed + require.Equal(t, sdk.NewRat(80).RoundInt64(), validators[0].GetPower().RoundInt64()) + require.Equal(t, sdk.NewRat(70).RoundInt64(), validators[1].GetPower().RoundInt64()) + + // Tendermint updates should reflect power change + updates := keeper.GetTendermintUpdates(ctx) + require.Equal(t, 2, len(updates)) + require.Equal(t, validators[0].ABCIValidator(), updates[0]) + require.Equal(t, validators[1].ABCIValidator(), updates[1]) +}