From cf03076a0a65f7989087579ce362b44439941875 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Fri, 17 Aug 2018 11:33:13 -0400 Subject: [PATCH] Merge PR #2047: Fix Invalid Cliff Validator Power Comparison --- CHANGELOG.md | 1 + x/stake/keeper/validator.go | 19 +++++++++---------- x/stake/keeper/validator_test.go | 14 +++++++++++++- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55104830b1..7ec3d66583 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -103,6 +103,7 @@ BUG FIXES - [x/stake] \#2021 Fixed repeated CLI commands in staking * Gaia + - [x/stake] [#2077](https://github.com/cosmos/cosmos-sdk/pull/2077) Fixed invalid cliff power comparison - \#1804 Fixes gen-tx genesis generation logic temporarily until upstream updates - \#1799 Fix `gaiad export` - \#1839 Fixed bug where intra-tx counter wasn't set correctly for genesis validators diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index 8b8cea73d1..debb917298 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -280,10 +280,6 @@ func (k Keeper) updateCliffValidator(ctx sdk.Context, affectedVal types.Validato panic(fmt.Sprintf("cliff validator record not found for address: %v\n", cliffAddr)) } - // NOTE: We get the power via affectedVal since the store (by power key) - // has yet to be updated. - affectedValPower := affectedVal.GetPower() - // Create a validator iterator ranging from smallest to largest by power // starting the current cliff validator's power. start := GetValidatorsByPowerIndexKey(oldCliffVal, pool) @@ -307,16 +303,19 @@ func (k Keeper) updateCliffValidator(ctx sdk.Context, affectedVal types.Validato panic("failed to create valid validator power iterator") } + affectedValRank := GetValidatorsByPowerIndexKey(affectedVal, pool) + newCliffValRank := GetValidatorsByPowerIndexKey(newCliffVal, pool) + if bytes.Equal(affectedVal.Owner, newCliffVal.Owner) { // The affected validator remains the cliff validator, however, since - // the store does not contain the new power, set the new cliff - // validator to the affected validator. - bz := GetValidatorsByPowerIndexKey(affectedVal, pool) - store.Set(ValidatorPowerCliffKey, bz) - } else if affectedValPower.GT(newCliffVal.GetPower()) { + // the store does not contain the new power, update the new power rank. + store.Set(ValidatorPowerCliffKey, affectedValRank) + } else if bytes.Compare(affectedValRank, newCliffValRank) > 0 { // The affected validator no longer remains the cliff validator as it's - // power is greater than the new current cliff validator. + // power is greater than the new cliff validator. k.setCliffValidator(ctx, newCliffVal, pool) + } else { + panic("invariant broken: the cliff validator should change or it should remain the same") } } diff --git a/x/stake/keeper/validator_test.go b/x/stake/keeper/validator_test.go index b9e61a1014..3446f0dc80 100644 --- a/x/stake/keeper/validator_test.go +++ b/x/stake/keeper/validator_test.go @@ -48,7 +48,6 @@ func TestSetValidator(t *testing.T) { updates := keeper.GetTendermintUpdates(ctx) require.Equal(t, 1, len(updates)) require.Equal(t, validator.ABCIValidator(), updates[0]) - } func TestUpdateValidatorByPowerIndex(t *testing.T) { @@ -143,6 +142,19 @@ func TestCliffValidatorChange(t *testing.T) { cliffPower = keeper.GetCliffValidatorPower(ctx) require.Equal(t, newCliffVal.Owner, sdk.AccAddress(keeper.GetCliffValidator(ctx))) require.Equal(t, GetValidatorsByPowerIndexKey(newCliffVal, pool), cliffPower) + + // add enough power to cliff validator to be equal in rank to next validator + newCliffVal, pool, _ = newCliffVal.AddTokensFromDel(pool, 9) + keeper.SetPool(ctx, pool) + newCliffVal = keeper.UpdateValidator(ctx, newCliffVal) + + // assert new cliff validator due to power rank construction + newCliffVal = validators[numVals-maxVals+2] + require.Equal(t, newCliffVal.Owner, sdk.AccAddress(keeper.GetCliffValidator(ctx))) + + // assert cliff validator power should have been updated + cliffPower = keeper.GetCliffValidatorPower(ctx) + require.Equal(t, GetValidatorsByPowerIndexKey(newCliffVal, pool), cliffPower) } func TestSlashToZeroPowerRemoved(t *testing.T) {