From 97ea51a3357da5041948312b1149f62672af5df9 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Wed, 15 Aug 2018 14:51:19 -0400 Subject: [PATCH] Merge pull request #2023: Terminate Update Bonded Validators Iteration Properly --- Gopkg.lock | 23 +++++++------- PENDING.md | 1 + x/stake/keeper/validator.go | 60 ++++++++++++++++++++----------------- 3 files changed, 45 insertions(+), 39 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 6e45f42b0..de244c162 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -38,7 +38,7 @@ name = "github.com/btcsuite/btcd" packages = ["btcec"] pruneopts = "UT" - revision = "cf05f92c3f815bbd5091ed6c73eff51f7b1945e8" + revision = "f899737d7f2764dc13e4d01ff00108ec58f766a9" [[projects]] digest = "1:386de157f7d19259a7f9c81f26ce011223ce0f090353c1152ffdf730d7d10ac2" @@ -165,12 +165,13 @@ [[projects]] branch = "master" - digest = "1:12247a2e99a060cc692f6680e5272c8adf0b8f572e6bce0d7095e624c958a240" + digest = "1:a361611b8c8c75a1091f00027767f7779b29cb37c456a71b8f2604c88057ab40" name = "github.com/hashicorp/hcl" packages = [ ".", "hcl/ast", "hcl/parser", + "hcl/printer", "hcl/scanner", "hcl/strconv", "hcl/token", @@ -340,19 +341,19 @@ [[projects]] branch = "master" - digest = "1:080e5f630945ad754f4b920e60b4d3095ba0237ebf88dc462eb28002932e3805" + digest = "1:8a020f916b23ff574845789daee6818daf8d25a4852419aae3f0b12378ba432a" name = "github.com/spf13/jwalterweatherman" packages = ["."] pruneopts = "UT" - revision = "7c0cea34c8ece3fbeb2b27ab9b59511d360fb394" + revision = "14d3d4c518341bea657dd8a226f5121c0ff8c9f2" [[projects]] - digest = "1:9424f440bba8f7508b69414634aef3b2b3a877e522d8a4624692412805407bb7" + digest = "1:dab83a1bbc7ad3d7a6ba1a1cc1760f25ac38cdf7d96a5cdd55cd915a4f5ceaf9" name = "github.com/spf13/pflag" packages = ["."] pruneopts = "UT" - revision = "583c0c0531f06d5278b7d917446061adc344b5cd" - version = "v1.0.1" + revision = "9a97c102cda95a86cec2345a6f09f55a939babf5" + version = "v1.0.2" [[projects]] digest = "1:f8e1a678a2571e265f4bf91a3e5e32aa6b1474a55cb0ea849750cc177b664d96" @@ -517,7 +518,7 @@ "salsa20/salsa", ] pruneopts = "UT" - revision = "f027049dab0ad238e394a753dba2d14753473a04" + revision = "de0752318171da717af4ce24d0a2e8626afaeb11" [[projects]] digest = "1:d36f55a999540d29b6ea3c2ea29d71c76b1d9853fdcd3e5c5cb4836f2ba118f1" @@ -537,14 +538,14 @@ [[projects]] branch = "master" - digest = "1:4d64ef38a30b73db6e8e7a2824b7fd356d921e0ee3fdd3248658996821d3b47d" + digest = "1:4bd75b1a219bc590b05c976bbebf47f4e993314ebb5c7cbf2efe05a09a184d54" name = "golang.org/x/sys" packages = [ "cpu", "unix", ] pruneopts = "UT" - revision = "acbc56fc7007d2a01796d5bde54f39e3b3e95945" + revision = "4e1fef5609515ec7a2cee7b5de30ba6d9b438cbf" [[projects]] digest = "1:a2ab62866c75542dd18d2b069fec854577a20211d7c0ea6ae746072a1dccdd18" @@ -575,7 +576,7 @@ name = "google.golang.org/genproto" packages = ["googleapis/rpc/status"] pruneopts = "UT" - revision = "daca94659cb50e9f37c1b834680f2e46358f10b0" + revision = "383e8b2c3b9e36c4076b235b32537292176bae20" [[projects]] digest = "1:2dab32a43451e320e49608ff4542fdfc653c95dcc35d0065ec9c6c3dd540ed74" diff --git a/PENDING.md b/PENDING.md index 4fcc0c6a9..8cf301a01 100644 --- a/PENDING.md +++ b/PENDING.md @@ -75,6 +75,7 @@ IMPROVEMENTS * [x/stake] \#1815 Sped up the processing of `EditValidator` txs. * [server] \#1930 Transactions indexer indexes all tags by default. * [x/stake] \#2000 Added tests for new staking endpoints +* [x/stake] [#2023](https://github.com/cosmos/cosmos-sdk/pull/2023) Terminate iteration loop in `UpdateBondedValidators` and `UpdateBondedValidatorsFull` when the first revoked validator is encountered and perform a sanity check. * [tools] Make get_vendor_deps deletes `.vendor-new` directories, in case scratch files are present. BUG FIXES diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index eaaf3df9e..ce5a19061 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -417,20 +417,24 @@ func (k Keeper) UpdateBondedValidators( } } - // increment bondedValidatorsCount / get the validator to bond - if !validator.Revoked { - if validator.Status != sdk.Bonded { - validatorToBond = validator - newValidatorBonded = true + if validator.Revoked { + // we should no longer consider jailed validators as they are ranked + // lower than any non-jailed/bonded validators + if validator.Status == sdk.Bonded { + panic(fmt.Sprintf("revoked validator cannot be bonded for address: %s\n", ownerAddr)) } - bondedValidatorsCount++ - - // sanity check - } else if validator.Status == sdk.Bonded { - panic(fmt.Sprintf("revoked validator cannot be bonded, address: %v\n", ownerAddr)) + break } + // increment the total number of bonded validators and potentially mark + // the validator to bond + if validator.Status != sdk.Bonded { + validatorToBond = validator + newValidatorBonded = true + } + + bondedValidatorsCount++ iterator.Next() } @@ -468,7 +472,6 @@ func (k Keeper) UpdateBondedValidators( // full update of the bonded validator set, many can be added/kicked func (k Keeper) UpdateBondedValidatorsFull(ctx sdk.Context) { - store := ctx.KVStore(k.storeKey) // clear the current validators store, add to the ToKickOut temp store @@ -476,27 +479,26 @@ func (k Keeper) UpdateBondedValidatorsFull(ctx sdk.Context) { iterator := sdk.KVStorePrefixIterator(store, ValidatorsBondedIndexKey) for ; iterator.Valid(); iterator.Next() { ownerAddr := GetAddressFromValBondedIndexKey(iterator.Key()) - toKickOut[string(ownerAddr)] = 0 // set anything + toKickOut[string(ownerAddr)] = 0 } + iterator.Close() + var validator types.Validator + oldCliffValidatorAddr := k.GetCliffValidator(ctx) maxValidators := k.GetParams(ctx).MaxValidators bondedValidatorsCount := 0 - iterator = sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey) // largest to smallest - var validator types.Validator + iterator = sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey) for { if !iterator.Valid() || bondedValidatorsCount > int(maxValidators-1) { break } - // either retrieve the original validator from the store, - // or under the situation that this is the "new validator" just - // use the validator provided because it has not yet been updated - // in the main validator store - ownerAddr := iterator.Value() var found bool + + ownerAddr := iterator.Value() validator, found = k.GetValidator(ctx, ownerAddr) if !found { panic(fmt.Sprintf("validator record not found for address: %v\n", ownerAddr)) @@ -506,23 +508,26 @@ func (k Keeper) UpdateBondedValidatorsFull(ctx sdk.Context) { if found { delete(toKickOut, string(ownerAddr)) } else { - - // if it wasn't in the toKickOut group it means - // this wasn't a previously a validator, therefor - // update the validator to enter the validator group + // If the validator wasn't in the toKickOut group it means it wasn't + // previously a validator, therefor update the validator to enter + // the validator group. validator = k.bondValidator(ctx, validator) } - if !validator.Revoked { - bondedValidatorsCount++ - } else { + if validator.Revoked { + // we should no longer consider jailed validators as they are ranked + // lower than any non-jailed/bonded validators if validator.Status == sdk.Bonded { - panic(fmt.Sprintf("revoked validator cannot be bonded, address: %v\n", ownerAddr)) + panic(fmt.Sprintf("revoked validator cannot be bonded for address: %s\n", ownerAddr)) } + + break } + bondedValidatorsCount++ iterator.Next() } + iterator.Close() // clear or set the cliff validator @@ -532,7 +537,6 @@ func (k Keeper) UpdateBondedValidatorsFull(ctx sdk.Context) { k.clearCliffValidator(ctx) } - // perform the actual kicks kickOutValidators(k, ctx, toKickOut) return }