Merge PR #1858: Fix Cliff Validator Update Bugs
This commit is contained in:
parent
0600d7356d
commit
ac26d33547
|
@ -73,6 +73,7 @@ BUG FIXES
|
||||||
* \#1799 Fix `gaiad export`
|
* \#1799 Fix `gaiad export`
|
||||||
* \#1828 Force user to specify amount on create-validator command by removing default
|
* \#1828 Force user to specify amount on create-validator command by removing default
|
||||||
* \#1839 Fixed bug where intra-tx counter wasn't set correctly for genesis validators
|
* \#1839 Fixed bug where intra-tx counter wasn't set correctly for genesis validators
|
||||||
|
* [staking] [#1858](https://github.com/cosmos/cosmos-sdk/pull/1858) Fixed bug where the cliff validator was not be updated correctly
|
||||||
* [tests] \#1675 Fix non-deterministic `test_cover`
|
* [tests] \#1675 Fix non-deterministic `test_cover`
|
||||||
* [client] \#1551: Refactored `CoreContext`
|
* [client] \#1551: Refactored `CoreContext`
|
||||||
* Renamed `CoreContext` to `QueryContext`
|
* Renamed `CoreContext` to `QueryContext`
|
||||||
|
@ -80,9 +81,8 @@ BUG FIXES
|
||||||
structure `TxContext` in `x/auth/client/context`
|
structure `TxContext` in `x/auth/client/context`
|
||||||
* Cleaned up documentation and API of what used to be `CoreContext`
|
* Cleaned up documentation and API of what used to be `CoreContext`
|
||||||
* Implemented `KeyType` enum for key info
|
* Implemented `KeyType` enum for key info
|
||||||
|
|
||||||
BUG FIXES
|
|
||||||
* \#1666 Add intra-tx counter to the genesis validators
|
* \#1666 Add intra-tx counter to the genesis validators
|
||||||
* [tests] \#1551: Fixed invalid LCD test JSON payload in `doIBCTransfer`
|
* [tests] \#1551: Fixed invalid LCD test JSON payload in `doIBCTransfer`
|
||||||
* \#1787 Fixed bug where Tally fails due to revoked/unbonding validator
|
* \#1787 Fixed bug where Tally fails due to revoked/unbonding validator
|
||||||
|
* \#1787 Fixed bug where Tally fails due to revoked/unbonding validator
|
||||||
* [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)
|
* [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)
|
|
@ -134,6 +134,8 @@ func (k Keeper) GetValidatorsBonded(ctx sdk.Context) (validators []types.Validat
|
||||||
}
|
}
|
||||||
|
|
||||||
// get the group of bonded validators sorted by power-rank
|
// get the group of bonded validators sorted by power-rank
|
||||||
|
//
|
||||||
|
// TODO: Rename to GetBondedValidatorsByPower or GetValidatorsByPower(ctx, status)
|
||||||
func (k Keeper) GetValidatorsByPower(ctx sdk.Context) []types.Validator {
|
func (k Keeper) GetValidatorsByPower(ctx sdk.Context) []types.Validator {
|
||||||
store := ctx.KVStore(k.storeKey)
|
store := ctx.KVStore(k.storeKey)
|
||||||
maxValidators := k.GetParams(ctx).MaxValidators
|
maxValidators := k.GetParams(ctx).MaxValidators
|
||||||
|
@ -191,13 +193,13 @@ func (k Keeper) ClearTendermintUpdates(ctx sdk.Context) {
|
||||||
|
|
||||||
//___________________________________________________________________________
|
//___________________________________________________________________________
|
||||||
|
|
||||||
// Perfom all the necessary steps for when a validator changes its power. This
|
// Perform all the necessary steps for when a validator changes its power. This
|
||||||
// function updates all validator stores as well as tendermint update store.
|
// function updates all validator stores as well as tendermint update store.
|
||||||
// It may kick out validators if a new validator is entering the bonded validator
|
// It may kick out validators if a new validator is entering the bonded validator
|
||||||
// group.
|
// group.
|
||||||
//
|
//
|
||||||
// nolint: gocyclo
|
// nolint: gocyclo
|
||||||
// TODO: Remove above nolint, function needs to be simplified
|
// TODO: Remove above nolint, function needs to be simplified!
|
||||||
func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) types.Validator {
|
func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) types.Validator {
|
||||||
store := ctx.KVStore(k.storeKey)
|
store := ctx.KVStore(k.storeKey)
|
||||||
pool := k.GetPool(ctx)
|
pool := k.GetPool(ctx)
|
||||||
|
@ -210,13 +212,23 @@ func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) type
|
||||||
cliffPower := k.GetCliffValidatorPower(ctx)
|
cliffPower := k.GetCliffValidatorPower(ctx)
|
||||||
|
|
||||||
switch {
|
switch {
|
||||||
// if already bonded and power increasing only need to update tendermint
|
// if the validator is already bonded and the power is increasing, we need
|
||||||
|
// perform the following:
|
||||||
|
// a) update Tendermint
|
||||||
|
// b) check if the cliff validator needs to be updated
|
||||||
case powerIncreasing && !validator.Revoked &&
|
case powerIncreasing && !validator.Revoked &&
|
||||||
(oldFound && oldValidator.Status == sdk.Bonded):
|
(oldFound && oldValidator.Status == sdk.Bonded):
|
||||||
|
|
||||||
bz := k.cdc.MustMarshalBinary(validator.ABCIValidator())
|
bz := k.cdc.MustMarshalBinary(validator.ABCIValidator())
|
||||||
store.Set(GetTendermintUpdatesKey(validator.Owner), bz)
|
store.Set(GetTendermintUpdatesKey(validator.Owner), bz)
|
||||||
|
|
||||||
|
if cliffPower != nil {
|
||||||
|
cliffAddr := sdk.AccAddress(k.GetCliffValidator(ctx))
|
||||||
|
if bytes.Equal(cliffAddr, validator.Owner) {
|
||||||
|
k.updateCliffValidator(ctx, validator)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// if is a new validator and the new power is less than the cliff validator
|
// if is a new validator and the new power is less than the cliff validator
|
||||||
case cliffPower != nil && !oldFound &&
|
case cliffPower != nil && !oldFound &&
|
||||||
bytes.Compare(valPower, cliffPower) == -1: //(valPower < cliffPower
|
bytes.Compare(valPower, cliffPower) == -1: //(valPower < cliffPower
|
||||||
|
@ -232,11 +244,10 @@ func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) type
|
||||||
// a) not-bonded and now has power-rank greater than cliff validator
|
// a) not-bonded and now has power-rank greater than cliff validator
|
||||||
// b) bonded and now has decreased in power
|
// b) bonded and now has decreased in power
|
||||||
default:
|
default:
|
||||||
|
|
||||||
// update the validator set for this validator
|
// update the validator set for this validator
|
||||||
// if updated, the validator has changed bonding status
|
|
||||||
updatedVal, updated := k.UpdateBondedValidators(ctx, validator)
|
updatedVal, updated := k.UpdateBondedValidators(ctx, validator)
|
||||||
if updated { // updates to validator occurred to be updated
|
if updated {
|
||||||
|
// the validator has changed bonding status
|
||||||
validator = updatedVal
|
validator = updatedVal
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
@ -246,13 +257,69 @@ func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) type
|
||||||
bz := k.cdc.MustMarshalBinary(validator.ABCIValidator())
|
bz := k.cdc.MustMarshalBinary(validator.ABCIValidator())
|
||||||
store.Set(GetTendermintUpdatesKey(validator.Owner), bz)
|
store.Set(GetTendermintUpdatesKey(validator.Owner), bz)
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
k.SetValidator(ctx, validator)
|
k.SetValidator(ctx, validator)
|
||||||
return validator
|
return validator
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// updateCliffValidator determines if the current cliff validator needs to be
|
||||||
|
// updated or swapped. If the provided affected validator is the current cliff
|
||||||
|
// validator before it's power was increased, either the cliff power key will
|
||||||
|
// be updated or if it's power is greater than the next bonded validator by
|
||||||
|
// power, it'll be swapped.
|
||||||
|
func (k Keeper) updateCliffValidator(ctx sdk.Context, affectedVal types.Validator) {
|
||||||
|
var newCliffVal types.Validator
|
||||||
|
|
||||||
|
store := ctx.KVStore(k.storeKey)
|
||||||
|
pool := k.GetPool(ctx)
|
||||||
|
cliffAddr := sdk.AccAddress(k.GetCliffValidator(ctx))
|
||||||
|
|
||||||
|
oldCliffVal, found := k.GetValidator(ctx, cliffAddr)
|
||||||
|
if !found {
|
||||||
|
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)
|
||||||
|
end := sdk.PrefixEndBytes(ValidatorsByPowerIndexKey)
|
||||||
|
iterator := store.Iterator(start, end)
|
||||||
|
|
||||||
|
if iterator.Valid() {
|
||||||
|
ownerAddr := iterator.Value()
|
||||||
|
currVal, found := k.GetValidator(ctx, ownerAddr)
|
||||||
|
if !found {
|
||||||
|
panic(fmt.Sprintf("validator record not found for address: %v\n", ownerAddr))
|
||||||
|
}
|
||||||
|
|
||||||
|
if currVal.Status != sdk.Bonded || currVal.Revoked {
|
||||||
|
panic(fmt.Sprintf("unexpected revoked or unbonded validator for address: %s\n", ownerAddr))
|
||||||
|
}
|
||||||
|
|
||||||
|
newCliffVal = currVal
|
||||||
|
iterator.Close()
|
||||||
|
} else {
|
||||||
|
panic("failed to create valid validator power iterator")
|
||||||
|
}
|
||||||
|
|
||||||
|
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 affected validator no longer remains the cliff validator as it's
|
||||||
|
// power is greater than the new current cliff validator.
|
||||||
|
k.setCliffValidator(ctx, newCliffVal, pool)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func (k Keeper) updateForRevoking(ctx sdk.Context, oldFound bool, oldValidator, newValidator types.Validator) types.Validator {
|
func (k Keeper) updateForRevoking(ctx sdk.Context, oldFound bool, oldValidator, newValidator types.Validator) types.Validator {
|
||||||
if newValidator.Revoked && oldFound && oldValidator.Status == sdk.Bonded {
|
if newValidator.Revoked && oldFound && oldValidator.Status == sdk.Bonded {
|
||||||
newValidator = k.unbondValidator(ctx, newValidator)
|
newValidator = k.unbondValidator(ctx, newValidator)
|
||||||
|
@ -317,8 +384,9 @@ func (k Keeper) updateValidatorPower(ctx sdk.Context, oldFound bool, oldValidato
|
||||||
//
|
//
|
||||||
// nolint: gocyclo
|
// nolint: gocyclo
|
||||||
// TODO: Remove the above golint
|
// TODO: Remove the above golint
|
||||||
func (k Keeper) UpdateBondedValidators(ctx sdk.Context,
|
func (k Keeper) UpdateBondedValidators(
|
||||||
affectedValidator types.Validator) (updatedVal types.Validator, updated bool) {
|
ctx sdk.Context, affectedValidator types.Validator) (
|
||||||
|
updatedVal types.Validator, updated bool) {
|
||||||
|
|
||||||
store := ctx.KVStore(k.storeKey)
|
store := ctx.KVStore(k.storeKey)
|
||||||
|
|
||||||
|
@ -328,7 +396,8 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context,
|
||||||
var validator, validatorToBond types.Validator
|
var validator, validatorToBond types.Validator
|
||||||
newValidatorBonded := false
|
newValidatorBonded := false
|
||||||
|
|
||||||
iterator := sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey) // largest to smallest
|
// create a validator iterator ranging from largest to smallest by power
|
||||||
|
iterator := sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey)
|
||||||
for {
|
for {
|
||||||
if !iterator.Valid() || bondedValidatorsCount > int(maxValidators-1) {
|
if !iterator.Valid() || bondedValidatorsCount > int(maxValidators-1) {
|
||||||
break
|
break
|
||||||
|
@ -354,6 +423,7 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context,
|
||||||
validatorToBond = validator
|
validatorToBond = validator
|
||||||
newValidatorBonded = true
|
newValidatorBonded = true
|
||||||
}
|
}
|
||||||
|
|
||||||
bondedValidatorsCount++
|
bondedValidatorsCount++
|
||||||
|
|
||||||
// sanity check
|
// sanity check
|
||||||
|
@ -363,6 +433,7 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context,
|
||||||
|
|
||||||
iterator.Next()
|
iterator.Next()
|
||||||
}
|
}
|
||||||
|
|
||||||
iterator.Close()
|
iterator.Close()
|
||||||
|
|
||||||
// clear or set the cliff validator
|
// clear or set the cliff validator
|
||||||
|
@ -372,17 +443,17 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context,
|
||||||
k.clearCliffValidator(ctx)
|
k.clearCliffValidator(ctx)
|
||||||
}
|
}
|
||||||
|
|
||||||
// swap the cliff validator for a new validator if the affected validator was bonded
|
// swap the cliff validator for a new validator if the affected validator
|
||||||
|
// was bonded
|
||||||
if newValidatorBonded {
|
if newValidatorBonded {
|
||||||
|
|
||||||
// unbond the cliff validator
|
// unbond the cliff validator
|
||||||
if oldCliffValidatorAddr != nil {
|
if oldCliffValidatorAddr != nil {
|
||||||
cliffVal, found := k.GetValidator(ctx, oldCliffValidatorAddr)
|
cliffVal, found := k.GetValidator(ctx, oldCliffValidatorAddr)
|
||||||
if !found {
|
if !found {
|
||||||
panic(fmt.Sprintf("validator record not found for address: %v\n", oldCliffValidatorAddr))
|
panic(fmt.Sprintf("validator record not found for address: %v\n", oldCliffValidatorAddr))
|
||||||
}
|
}
|
||||||
k.unbondValidator(ctx, cliffVal)
|
|
||||||
|
|
||||||
|
k.unbondValidator(ctx, cliffVal)
|
||||||
}
|
}
|
||||||
|
|
||||||
// bond the new validator
|
// bond the new validator
|
||||||
|
@ -391,6 +462,7 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context,
|
||||||
return validator, true
|
return validator, true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return types.Validator{}, false
|
return types.Validator{}, false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -1,6 +1,7 @@
|
||||||
package keeper
|
package keeper
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"fmt"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
sdk "github.com/cosmos/cosmos-sdk/types"
|
sdk "github.com/cosmos/cosmos-sdk/types"
|
||||||
|
@ -88,6 +89,62 @@ func TestUpdateValidatorByPowerIndex(t *testing.T) {
|
||||||
require.True(t, keeper.validatorByPowerIndexExists(ctx, power))
|
require.True(t, keeper.validatorByPowerIndexExists(ctx, power))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestCliffValidatorChange(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
|
||||||
|
}
|
||||||
|
|
||||||
|
// add a large amount of tokens to current cliff validator
|
||||||
|
currCliffVal := validators[numVals-maxVals]
|
||||||
|
currCliffVal, pool, _ = currCliffVal.AddTokensFromDel(pool, 200)
|
||||||
|
keeper.SetPool(ctx, pool)
|
||||||
|
currCliffVal = keeper.UpdateValidator(ctx, currCliffVal)
|
||||||
|
|
||||||
|
// assert new cliff validator to be set to the second lowest bonded validator by power
|
||||||
|
newCliffVal := validators[numVals-maxVals+1]
|
||||||
|
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)
|
||||||
|
|
||||||
|
// add small amount of tokens to new current cliff validator
|
||||||
|
newCliffVal, pool, _ = newCliffVal.AddTokensFromDel(pool, 1)
|
||||||
|
keeper.SetPool(ctx, pool)
|
||||||
|
newCliffVal = keeper.UpdateValidator(ctx, newCliffVal)
|
||||||
|
|
||||||
|
// assert cliff validator has not change but increased in power
|
||||||
|
cliffPower = keeper.GetCliffValidatorPower(ctx)
|
||||||
|
require.Equal(t, newCliffVal.Owner, sdk.AccAddress(keeper.GetCliffValidator(ctx)))
|
||||||
|
require.Equal(t, GetValidatorsByPowerIndexKey(newCliffVal, pool), cliffPower)
|
||||||
|
}
|
||||||
|
|
||||||
func TestSlashToZeroPowerRemoved(t *testing.T) {
|
func TestSlashToZeroPowerRemoved(t *testing.T) {
|
||||||
// initialize setup
|
// initialize setup
|
||||||
ctx, _, keeper := CreateTestInput(t, false, 100)
|
ctx, _, keeper := CreateTestInput(t, false, 100)
|
||||||
|
|
Loading…
Reference in New Issue