diff --git a/PENDING.md b/PENDING.md index 56b49b822ad2..d102cd1a23c0 100644 --- a/PENDING.md +++ b/PENDING.md @@ -46,6 +46,7 @@ IMPROVEMENTS - #2617 [x/mock/simulation] Randomize all genesis parameters - \#1924 [simulation] Use a transition matrix for block size - #2610 [x/stake] Block redelegation to and from the same validator + - #2669 [x/stake] Added invarant check to make sure validator's power aligns with its spot in the power store. * Tendermint @@ -58,6 +59,7 @@ BUG FIXES * Gaia CLI (`gaiacli`) * Gaia + - #2670 [x/stake] fixed incorrent `IterateBondedValidators` and split into two functions: `IterateBondedValidators` and `IterateLastBlockConsValidators` * SDK - #2625 [x/gov] fix AppendTag function usage error diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 17942b976aa9..9b3fc365b3fc 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -394,6 +394,7 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res ctx := sdk.NewContext(app.cms.CacheMultiStore(), app.checkState.ctx.BlockHeader(), true, app.Logger). WithMinimumFees(app.minimumFees) + // Passes the rest of the path as an argument to the querier. // For example, in the path "custom/gov/proposal/test", the gov querier gets []string{"proposal", "test"} as the path resBytes, err := querier(ctx, path[2:], req) diff --git a/examples/democoin/mock/validator.go b/examples/democoin/mock/validator.go index fc00b79be007..1d10c48b2459 100644 --- a/examples/democoin/mock/validator.go +++ b/examples/democoin/mock/validator.go @@ -82,8 +82,13 @@ func (vs *ValidatorSet) IterateValidators(ctx sdk.Context, fn func(index int64, } } -// IterateValidatorsBonded implements sdk.ValidatorSet -func (vs *ValidatorSet) IterateValidatorsBonded(ctx sdk.Context, fn func(index int64, Validator sdk.Validator) bool) { +// IterateBondedValidatorsByPower implements sdk.ValidatorSet +func (vs *ValidatorSet) IterateBondedValidatorsByPower(ctx sdk.Context, fn func(index int64, Validator sdk.Validator) bool) { + vs.IterateValidators(ctx, fn) +} + +// IterateLastValidators implements sdk.ValidatorSet +func (vs *ValidatorSet) IterateLastValidators(ctx sdk.Context, fn func(index int64, Validator sdk.Validator) bool) { vs.IterateValidators(ctx, fn) } diff --git a/types/stake.go b/types/stake.go index f5d3a4aae0ae..7b10b17f87d5 100644 --- a/types/stake.go +++ b/types/stake.go @@ -64,7 +64,11 @@ type ValidatorSet interface { func(index int64, validator Validator) (stop bool)) // iterate through bonded validators by operator address, execute func for each validator - IterateValidatorsBonded(Context, + IterateBondedValidatorsByPower(Context, + func(index int64, validator Validator) (stop bool)) + + // iterate through the consensus validator set of the last block by operator address, execute func for each validator + IterateLastValidators(Context, func(index int64, validator Validator) (stop bool)) Validator(Context, ValAddress) Validator // get a particular validator by operator address diff --git a/x/gov/tally.go b/x/gov/tally.go index b6e42c4b5a02..4805b4b47222 100644 --- a/x/gov/tally.go +++ b/x/gov/tally.go @@ -23,7 +23,7 @@ func tally(ctx sdk.Context, keeper Keeper, proposal Proposal) (passes bool, tall totalVotingPower := sdk.ZeroDec() currValidators := make(map[string]validatorGovInfo) - keeper.vs.IterateValidatorsBonded(ctx, func(index int64, validator sdk.Validator) (stop bool) { + keeper.vs.IterateBondedValidatorsByPower(ctx, func(index int64, validator sdk.Validator) (stop bool) { currValidators[validator.GetOperator().String()] = validatorGovInfo{ Address: validator.GetOperator(), Power: validator.GetPower(), diff --git a/x/stake/genesis.go b/x/stake/genesis.go index 28c5973c741b..63b038613de5 100644 --- a/x/stake/genesis.go +++ b/x/stake/genesis.go @@ -72,7 +72,7 @@ func WriteGenesis(ctx sdk.Context, keeper Keeper) types.GenesisState { // WriteValidators returns a slice of bonded genesis validators. func WriteValidators(ctx sdk.Context, keeper Keeper) (vals []tmtypes.GenesisValidator) { - keeper.IterateValidatorsBonded(ctx, func(_ int64, validator sdk.Validator) (stop bool) { + keeper.IterateLastValidators(ctx, func(_ int64, validator sdk.Validator) (stop bool) { vals = append(vals, tmtypes.GenesisValidator{ PubKey: validator.GetConsPubKey(), Power: validator.GetPower().RoundInt64(), diff --git a/x/stake/keeper/sdk_types.go b/x/stake/keeper/sdk_types.go index 4e859a42a887..1dea473f892b 100644 --- a/x/stake/keeper/sdk_types.go +++ b/x/stake/keeper/sdk_types.go @@ -28,9 +28,31 @@ func (k Keeper) IterateValidators(ctx sdk.Context, fn func(index int64, validato } // iterate through the active validator set and perform the provided function -func (k Keeper) IterateValidatorsBonded(ctx sdk.Context, fn func(index int64, validator sdk.Validator) (stop bool)) { +func (k Keeper) IterateBondedValidatorsByPower(ctx sdk.Context, fn func(index int64, validator sdk.Validator) (stop bool)) { store := ctx.KVStore(k.storeKey) - iterator := sdk.KVStorePrefixIterator(store, LastValidatorPowerKey) + maxValidators := k.MaxValidators(ctx) + + iterator := sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey) + defer iterator.Close() + + i := int64(0) + for ; iterator.Valid() && i < int64(maxValidators); iterator.Next() { + address := iterator.Value() + validator := k.mustGetValidator(ctx, address) + + if validator.Status == sdk.Bonded { + stop := fn(i, validator) // XXX is this safe will the validator unexposed fields be able to get written to? + if stop { + break + } + i++ + } + } +} + +// iterate through the active validator set and perform the provided function +func (k Keeper) IterateLastValidators(ctx sdk.Context, fn func(index int64, validator sdk.Validator) (stop bool)) { + iterator := k.LastValidatorsIterator(ctx) i := int64(0) for ; iterator.Valid(); iterator.Next() { address := AddressFromLastValidatorPowerKey(iterator.Key()) diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index 9fd7434d313a..44a0ddba5327 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -261,6 +261,13 @@ func (k Keeper) GetLastValidators(ctx sdk.Context) (validators []types.Validator return validators[:i] // trim } +// returns an iterator for the consensus validators in the last block +func (k Keeper) LastValidatorsIterator(ctx sdk.Context) (iterator sdk.Iterator) { + store := ctx.KVStore(k.storeKey) + iterator = sdk.KVStorePrefixIterator(store, LastValidatorPowerKey) + return iterator +} + // get the current group of bonded validators sorted by power-rank func (k Keeper) GetBondedValidatorsByPower(ctx sdk.Context) []types.Validator { store := ctx.KVStore(k.storeKey) @@ -283,6 +290,13 @@ func (k Keeper) GetBondedValidatorsByPower(ctx sdk.Context) []types.Validator { return validators[:i] // trim } +// returns an iterator for the current validator power store +func (k Keeper) ValidatorsPowerStoreIterator(ctx sdk.Context) (iterator sdk.Iterator) { + store := ctx.KVStore(k.storeKey) + iterator = sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey) + return iterator +} + // gets a specific validator queue timeslice. A timeslice is a slice of ValAddresses corresponding to unbonding validators // that expire at a certain time. func (k Keeper) GetValidatorQueueTimeSlice(ctx sdk.Context, timestamp time.Time) (valAddrs []sdk.ValAddress) { diff --git a/x/stake/simulation/invariants.go b/x/stake/simulation/invariants.go index 3b97bdb72f0e..3b1130a167e4 100644 --- a/x/stake/simulation/invariants.go +++ b/x/stake/simulation/invariants.go @@ -1,6 +1,7 @@ package simulation import ( + "bytes" "fmt" "github.com/cosmos/cosmos-sdk/baseapp" @@ -10,6 +11,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/distribution" "github.com/cosmos/cosmos-sdk/x/mock/simulation" "github.com/cosmos/cosmos-sdk/x/stake" + "github.com/cosmos/cosmos-sdk/x/stake/keeper" abci "github.com/tendermint/tendermint/abci/types" ) @@ -24,7 +26,7 @@ func AllInvariants(ck bank.Keeper, k stake.Keeper, if err != nil { return err } - err = PositivePowerInvariant(k)(app, header) + err = PowerStoreInvariant(k)(app, header) if err != nil { return err } @@ -100,19 +102,29 @@ func SupplyInvariants(ck bank.Keeper, k stake.Keeper, } } -// PositivePowerInvariant checks that all stored validators have > 0 power -func PositivePowerInvariant(k stake.Keeper) simulation.Invariant { +// PowerStoreInvariant checks that a validator's power aligns with its key in the power store +func PowerStoreInvariant(k stake.Keeper) simulation.Invariant { return func(app *baseapp.BaseApp, _ abci.Header) error { ctx := app.NewContext(false, abci.Header{}) - var err error - k.IterateValidatorsBonded(ctx, func(_ int64, validator sdk.Validator) bool { - if !validator.GetPower().GT(sdk.ZeroDec()) { - err = fmt.Errorf("validator with non-positive power stored. (pubkey %v)", validator.GetConsPubKey()) - return true + + iterator := k.ValidatorsPowerStoreIterator(ctx) + pool := k.GetPool(ctx) + + for ; iterator.Valid(); iterator.Next() { + validator, found := k.GetValidator(ctx, iterator.Value()) + if !found { + panic(fmt.Sprintf("validator record not found for address: %X\n", iterator.Value())) } - return false - }) - return err + + powerKey := keeper.GetValidatorsByPowerIndexKey(validator, pool) + + if !bytes.Equal(iterator.Key(), powerKey) { + return fmt.Errorf("power store invariance:\n\tvalidator.Power: %v"+ + "\n\tkey should be: %v\n\tkey in store: %v", validator.GetPower(), powerKey, iterator.Key()) + } + } + iterator.Close() + return nil } }