R4R: Power Store Invariant

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

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)

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)
 }
 

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

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(),

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(),

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())

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) {

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
 	}
 }