Merge PR #2047: Fix Invalid Cliff Validator Power Comparison

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

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

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) {