diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ec3d6658..f6b91607a 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index debb91729..8105e3747 100644 --- a/x/stake/keeper/validator.go +++ b/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 diff --git a/x/stake/keeper/validator_test.go b/x/stake/keeper/validator_test.go index 3446f0dc8..62f9823a9 100644 --- a/x/stake/keeper/validator_test.go +++ b/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)