Merge PR #2083: Fix broken invariant of bonded validator power decrease

CHANGELOG.md

@@ -112,7 +112,7 @@ BUG FIXES
   - [tests] \#1551 Fixed invalid LCD test JSON payload in `doIBCTransfer`
   - [basecoin] Fixes coin transaction failure and account query [discussion](https://forum.cosmos.network/t/unmarshalbinarybare-expected-to-read-prefix-bytes-75fbfab8-since-it-is-registered-concrete-but-got-0a141dfa/664/6)
   - [x/gov] \#1757 Fix VoteOption conversion to String
-
+  * [x/stake] [#2083] Fix broken invariant of bonded validator power decrease
 
 ## 0.23.1
 

x/stake/keeper/validator.go

@@ -452,21 +452,29 @@ func (k Keeper) UpdateBondedValidators(
 	// swap the cliff validator for a new validator if the affected validator
 	// was bonded
 	if newValidatorBonded {
-		// unbond the cliff validator
 		if oldCliffValidatorAddr != nil {
-			cliffVal, found := k.GetValidator(ctx, oldCliffValidatorAddr)
+			oldCliffVal, found := k.GetValidator(ctx, oldCliffValidatorAddr)
 			if !found {
 				panic(fmt.Sprintf("validator record not found for address: %v\n", oldCliffValidatorAddr))
 			}
 
-			k.unbondValidator(ctx, cliffVal)
+			if bytes.Equal(validatorToBond.Owner, affectedValidator.Owner) {
+				// unbond the old cliff validator iff the affected validator was
+				// newly bonded and has greater power
+				k.unbondValidator(ctx, oldCliffVal)
+			} else {
+				// otherwise unbond the affected validator, which must have been
+				// kicked out
+				affectedValidator = k.unbondValidator(ctx, affectedValidator)
+			}
 		}
 
-		// bond the new validator
 		validator = k.bondValidator(ctx, validatorToBond)
 		if bytes.Equal(validator.Owner, affectedValidator.Owner) {
 			return validator, true
 		}
+
+		return affectedValidator, true
 	}
 
 	return types.Validator{}, false

x/stake/keeper/validator_test.go

@@ -88,6 +88,69 @@ func TestUpdateValidatorByPowerIndex(t *testing.T) {
 	require.True(t, keeper.validatorByPowerIndexExists(ctx, power))
 }
 
+func TestUpdateBondedValidatorsDecreaseCliff(t *testing.T) {
+	numVals := 10
+	maxVals := 5
+
+	// create context, keeper, and pool for tests
+	ctx, _, keeper := CreateTestInput(t, false, 0)
+	pool := keeper.GetPool(ctx)
+
+	// create keeper parameters
+	params := keeper.GetParams(ctx)
+	params.MaxValidators = uint16(maxVals)
+	keeper.SetParams(ctx, params)
+
+	// create a random pool
+	pool.LooseTokens = sdk.NewRat(10000)
+	pool.BondedTokens = sdk.NewRat(1234)
+	keeper.SetPool(ctx, pool)
+
+	validators := make([]types.Validator, numVals)
+	for i := 0; i < len(validators); i++ {
+		moniker := fmt.Sprintf("val#%d", int64(i))
+		val := types.NewValidator(Addrs[i], PKs[i], types.Description{Moniker: moniker})
+		val.BondHeight = int64(i)
+		val.BondIntraTxCounter = int16(i)
+		val, pool, _ = val.AddTokensFromDel(pool, int64((i+1)*10))
+
+		keeper.SetPool(ctx, pool)
+		val = keeper.UpdateValidator(ctx, val)
+		validators[i] = val
+	}
+
+	nextCliffVal := validators[numVals-maxVals+1]
+
+	// remove enough tokens to kick out the validator below the current cliff
+	// validator and next in line cliff validator
+	nextCliffVal, pool, _ = nextCliffVal.RemoveDelShares(pool, sdk.NewRat(21))
+	keeper.SetPool(ctx, pool)
+	nextCliffVal = keeper.UpdateValidator(ctx, nextCliffVal)
+
+	// require the cliff validator has changed
+	cliffVal := validators[numVals-maxVals-1]
+	require.Equal(t, cliffVal.Owner, sdk.AccAddress(keeper.GetCliffValidator(ctx)))
+
+	// require the cliff validator power has changed
+	cliffPower := keeper.GetCliffValidatorPower(ctx)
+	require.Equal(t, GetValidatorsByPowerIndexKey(cliffVal, pool), cliffPower)
+
+	expectedValStatus := map[int]sdk.BondStatus{
+		9: sdk.Bonded, 8: sdk.Bonded, 7: sdk.Bonded, 5: sdk.Bonded, 4: sdk.Bonded,
+		0: sdk.Unbonded, 1: sdk.Unbonded, 2: sdk.Unbonded, 3: sdk.Unbonded, 6: sdk.Unbonded,
+	}
+
+	// require all the validators have their respective statuses
+	for valIdx, status := range expectedValStatus {
+		valAddr := validators[valIdx].Owner
+		val, _ := keeper.GetValidator(ctx, valAddr)
+
+		require.Equal(
+			t, val.GetStatus(), status,
+			fmt.Sprintf("expected validator to have status: %s", sdk.BondStatusToString(status)))
+	}
+}
+
 func TestCliffValidatorChange(t *testing.T) {
 	numVals := 10
 	maxVals := 5
@@ -415,11 +478,13 @@ func TestGetValidatorsEdgeCases(t *testing.T) {
 	var validators [4]types.Validator
 	for i, amt := range amts {
 		pool := keeper.GetPool(ctx)
-		validators[i] = types.NewValidator(Addrs[i], PKs[i], types.Description{})
+		moniker := fmt.Sprintf("val#%d", int64(i))
+		validators[i] = types.NewValidator(Addrs[i], PKs[i], types.Description{Moniker: moniker})
 		validators[i], pool, _ = validators[i].AddTokensFromDel(pool, amt)
 		keeper.SetPool(ctx, pool)
 		validators[i] = keeper.UpdateValidator(ctx, validators[i])
 	}
+
 	for i := range amts {
 		validators[i], found = keeper.GetValidator(ctx, validators[i].Owner)
 		require.True(t, found)