From 39165c943e3a481b3172c04cb99ada0e7bd480e6 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Fri, 2 Nov 2018 14:35:17 -0700 Subject: [PATCH 1/6] fixed power store invariant --- types/stake.go | 4 ++++ x/stake/keeper/sdk_types.go | 24 +++++++++++++++++++++- x/stake/keeper/validator.go | 14 +++++++++++++ x/stake/simulation/invariants.go | 34 +++++++++++++++++++++----------- 4 files changed, 64 insertions(+), 12 deletions(-) diff --git a/types/stake.go b/types/stake.go index f5d3a4aae0ae..2a8f64445a61 100644 --- a/types/stake.go +++ b/types/stake.go @@ -67,6 +67,10 @@ type ValidatorSet interface { IterateValidatorsBonded(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 + IterateLastBlockConsValidators(Context, + func(index int64, validator Validator) (stop bool)) + Validator(Context, ValAddress) Validator // get a particular validator by operator address ValidatorByConsAddr(Context, ConsAddress) Validator // get a particular validator by consensus address TotalPower(Context) Dec // total power of the validator set diff --git a/x/stake/keeper/sdk_types.go b/x/stake/keeper/sdk_types.go index 4e859a42a887..26b6cac51641 100644 --- a/x/stake/keeper/sdk_types.go +++ b/x/stake/keeper/sdk_types.go @@ -30,7 +30,29 @@ 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)) { 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) IterateLastBlockConsValidators(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..5393343f2de6 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 } @@ -101,22 +103,32 @@ func SupplyInvariants(ck bank.Keeper, k stake.Keeper, } // PositivePowerInvariant checks that all stored validators have > 0 power -func PositivePowerInvariant(k stake.Keeper) simulation.Invariant { +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 } } -// ValidatorSetInvariant checks equivalence of Tendermint validator set and SDK validator set +// ValidatorSetInvariZant checks equivalence of Tendermint validator set and SDK validator set func ValidatorSetInvariant(k stake.Keeper) simulation.Invariant { return func(app *baseapp.BaseApp, _ abci.Header) error { // TODO From 3cf66871ef32e09413ad3f1f03442db5b10a1abd Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Fri, 2 Nov 2018 14:41:50 -0700 Subject: [PATCH 2/6] PENDING --- PENDING.md | 2 ++ 1 file changed, 2 insertions(+) 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 From 42eb4e208a30ad379c9689c28761a53d891ab4d3 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Fri, 2 Nov 2018 15:42:30 -0700 Subject: [PATCH 3/6] IterateValidatorsBonded -> IterateBondedValidatorsByPower --- examples/democoin/mock/validator.go | 4 ++-- types/stake.go | 4 ++-- x/gov/tally.go | 2 +- x/stake/genesis.go | 2 +- x/stake/keeper/sdk_types.go | 4 ++-- x/stake/simulation/invariants.go | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/examples/democoin/mock/validator.go b/examples/democoin/mock/validator.go index fc00b79be007..948199499fb8 100644 --- a/examples/democoin/mock/validator.go +++ b/examples/democoin/mock/validator.go @@ -82,8 +82,8 @@ 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) } diff --git a/types/stake.go b/types/stake.go index 2a8f64445a61..7b10b17f87d5 100644 --- a/types/stake.go +++ b/types/stake.go @@ -64,11 +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 - IterateLastBlockConsValidators(Context, + 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..47c14bd8fd7b 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.IterateBondedValidatorsByPower(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 26b6cac51641..1dea473f892b 100644 --- a/x/stake/keeper/sdk_types.go +++ b/x/stake/keeper/sdk_types.go @@ -28,7 +28,7 @@ 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) maxValidators := k.MaxValidators(ctx) @@ -51,7 +51,7 @@ func (k Keeper) IterateValidatorsBonded(ctx sdk.Context, fn func(index int64, va } // iterate through the active validator set and perform the provided function -func (k Keeper) IterateLastBlockConsValidators(ctx sdk.Context, fn func(index int64, validator sdk.Validator) (stop bool)) { +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() { diff --git a/x/stake/simulation/invariants.go b/x/stake/simulation/invariants.go index 5393343f2de6..321300132cec 100644 --- a/x/stake/simulation/invariants.go +++ b/x/stake/simulation/invariants.go @@ -128,7 +128,7 @@ func PowerStoreInvariant(k stake.Keeper) simulation.Invariant { } } -// ValidatorSetInvariZant checks equivalence of Tendermint validator set and SDK validator set +// ValidatorSetInvariant checks equivalence of Tendermint validator set and SDK validator set func ValidatorSetInvariant(k stake.Keeper) simulation.Invariant { return func(app *baseapp.BaseApp, _ abci.Header) error { // TODO From 888c061c5c604bffcf7e391c3e2d1dfa36fd3c33 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Fri, 2 Nov 2018 15:58:48 -0700 Subject: [PATCH 4/6] WriteValidators uses IterateLastValidators rather than IterateBondedValidatorsByPower --- baseapp/baseapp.go | 1 + x/stake/genesis.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) 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/x/stake/genesis.go b/x/stake/genesis.go index 47c14bd8fd7b..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.IterateBondedValidatorsByPower(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(), From 13d933f7f793faae85dc47d4fa8a6978ae1d1380 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Fri, 2 Nov 2018 16:55:49 -0700 Subject: [PATCH 5/6] fixed democoin interface --- examples/democoin/mock/validator.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/examples/democoin/mock/validator.go b/examples/democoin/mock/validator.go index 948199499fb8..1d10c48b2459 100644 --- a/examples/democoin/mock/validator.go +++ b/examples/democoin/mock/validator.go @@ -87,6 +87,11 @@ func (vs *ValidatorSet) IterateBondedValidatorsByPower(ctx sdk.Context, fn func( 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) +} + // Validator implements sdk.ValidatorSet func (vs *ValidatorSet) Validator(ctx sdk.Context, addr sdk.ValAddress) sdk.Validator { for _, val := range vs.Validators { From 191a732358d4519a2904950fd99ebfb11a108a07 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Fri, 2 Nov 2018 19:14:38 -0700 Subject: [PATCH 6/6] fixed comment --- x/stake/simulation/invariants.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/stake/simulation/invariants.go b/x/stake/simulation/invariants.go index 321300132cec..3b1130a167e4 100644 --- a/x/stake/simulation/invariants.go +++ b/x/stake/simulation/invariants.go @@ -102,7 +102,7 @@ func SupplyInvariants(ck bank.Keeper, k stake.Keeper, } } -// PositivePowerInvariant checks that all stored validators have > 0 power +// 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{})