From e506aa91d443befc902ebdf027197dca784e7b3a Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Fri, 19 Jul 2024 12:40:03 +0200 Subject: [PATCH] Fix bonded/active validator distinction --- tests/e2e/steps_inactive_vals.go | 24 ++----------------- x/ccv/consumer/keeper/keeper.go | 6 ++++- x/ccv/provider/keeper/legacy_proposal.go | 4 ++-- x/ccv/provider/keeper/proposal.go | 10 ++++++-- x/ccv/provider/keeper/relay.go | 10 ++++++-- x/ccv/provider/keeper/validator_set_update.go | 11 ++++++++- x/ccv/types/utils.go | 11 ++++----- 7 files changed, 39 insertions(+), 37 deletions(-) diff --git a/tests/e2e/steps_inactive_vals.go b/tests/e2e/steps_inactive_vals.go index fd7bd912ee..7f3c75b800 100644 --- a/tests/e2e/steps_inactive_vals.go +++ b/tests/e2e/steps_inactive_vals.go @@ -606,32 +606,12 @@ func stepsInactiveValsWithTopN() []Step { State: State{ ChainID("consu"): ChainState{ ValPowers: &map[ValidatorID]uint{ - ValidatorID("alice"): 100, - ValidatorID("bob"): 200, + ValidatorID("alice"): 0, // alice and bob are not in the top N, so aren't in the validator set + ValidatorID("bob"): 0, ValidatorID("carol"): 300, }, }, }, }, - { - Action: AddIbcConnectionAction{ - ChainA: ChainID("consu"), - ChainB: ChainID("provi"), - ClientA: 0, - ClientB: 0, - }, - State: State{}, - }, - { - Action: AddIbcChannelAction{ - ChainA: ChainID("consu"), - ChainB: ChainID("provi"), - ConnectionA: 0, - PortA: "consumer", - PortB: "provider", - Order: "ordered", - }, - State: State{}, - }, } } diff --git a/x/ccv/consumer/keeper/keeper.go b/x/ccv/consumer/keeper/keeper.go index 644cfc33a7..a7f644ced8 100644 --- a/x/ccv/consumer/keeper/keeper.go +++ b/x/ccv/consumer/keeper/keeper.go @@ -716,5 +716,9 @@ func (k Keeper) IsPrevStandaloneChain(ctx sdk.Context) bool { // GetLastBondedValidators iterates the last validator powers in the staking module // and returns the first MaxValidators many validators with the largest powers. func (k Keeper) GetLastBondedValidators(ctx sdk.Context) ([]stakingtypes.Validator, error) { - return ccv.GetLastBondedValidatorsUtil(ctx, k.standaloneStakingKeeper, k.Logger(ctx)) + maxVals, err := k.standaloneStakingKeeper.MaxValidators(ctx) + if err != nil { + return nil, err + } + return ccv.GetLastBondedValidatorsUtil(ctx, k.standaloneStakingKeeper, k.Logger(ctx), maxVals) } diff --git a/x/ccv/provider/keeper/legacy_proposal.go b/x/ccv/provider/keeper/legacy_proposal.go index c35f9fd73f..83fe0e3204 100644 --- a/x/ccv/provider/keeper/legacy_proposal.go +++ b/x/ccv/provider/keeper/legacy_proposal.go @@ -125,11 +125,11 @@ func (k Keeper) HandleLegacyConsumerModificationProposal(ctx sdk.Context, p *typ if p.Top_N != oldTopN { if p.Top_N > 0 { // if the chain receives a non-zero top N value, store the minimum power in the top N - bondedValidators, err := k.GetLastBondedValidators(ctx) + activeValidators, err := k.GetLastActiveBondedValidators(ctx) if err != nil { return err } - minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, p.Top_N) + minPower, err := k.ComputeMinPowerInTopN(ctx, activeValidators, p.Top_N) if err != nil { return err } diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index df600e0a5c..af48ea3597 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -286,14 +286,20 @@ func (k Keeper) MakeConsumerGenesis( } if prop.Top_N > 0 { + // get the consensus active validators + activeValidators, err := k.GetLastActiveBondedValidators(ctx) + if err != nil { + return gen, nil, errorsmod.Wrapf(stakingtypes.ErrNoValidatorFound, "error getting last active bonded validators: %s", err) + } // in a Top-N chain, we automatically opt in all validators that belong to the top N - minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, prop.Top_N) + minPower, err := k.ComputeMinPowerInTopN(ctx, activeValidators, prop.Top_N) if err != nil { return gen, nil, err } - k.OptInTopNValidators(ctx, chainID, bondedValidators, minPower) + k.OptInTopNValidators(ctx, chainID, activeValidators, minPower) k.SetMinimumPowerInTopN(ctx, chainID, minPower) } + // need to use the bondedValidators, not activeValidators, here since the chain might be opt-in and allow inactive vals nextValidators := k.ComputeNextValidators(ctx, chainID, bondedValidators) k.SetConsumerValSet(ctx, chainID, nextValidators) diff --git a/x/ccv/provider/keeper/relay.go b/x/ccv/provider/keeper/relay.go index 9254347c3d..a0544fb5e2 100644 --- a/x/ccv/provider/keeper/relay.go +++ b/x/ccv/provider/keeper/relay.go @@ -282,12 +282,18 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) { if topN > 0 { // in a Top-N chain, we automatically opt in all validators that belong to the top N - minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, topN) + // of the active validators + activeValidators, err := k.GetLastActiveBondedValidators(ctx) + if err != nil { + // we just log here and do not panic because panic-ing would halt the provider chain + k.Logger(ctx).Error("failed to get active validators", "error", err) + } + minPower, err := k.ComputeMinPowerInTopN(ctx, activeValidators, topN) if err == nil { // set the minimal power of validators in the top N in the store k.SetMinimumPowerInTopN(ctx, chainID, minPower) - k.OptInTopNValidators(ctx, chainID, bondedValidators, minPower) + k.OptInTopNValidators(ctx, chainID, activeValidators, minPower) } else { // we just log here and do not panic because panic-ing would halt the provider chain k.Logger(ctx).Error("failed to compute min power to opt in for chain", "chain", chainID, "error", err) diff --git a/x/ccv/provider/keeper/validator_set_update.go b/x/ccv/provider/keeper/validator_set_update.go index 091d11e7b0..0a0a4444d8 100644 --- a/x/ccv/provider/keeper/validator_set_update.go +++ b/x/ccv/provider/keeper/validator_set_update.go @@ -167,5 +167,14 @@ func (k Keeper) FilterValidators( // GetLastBondedValidators iterates the last validator powers in the staking module // and returns the first MaxValidators many validators with the largest powers. func (k Keeper) GetLastBondedValidators(ctx sdk.Context) ([]stakingtypes.Validator, error) { - return ccv.GetLastBondedValidatorsUtil(ctx, k.stakingKeeper, k.Logger(ctx)) + maxVals, err := k.stakingKeeper.MaxValidators(ctx) + if err != nil { + return nil, err + } + return ccv.GetLastBondedValidatorsUtil(ctx, k.stakingKeeper, k.Logger(ctx), maxVals) +} + +func (k Keeper) GetLastActiveBondedValidators(ctx sdk.Context) ([]stakingtypes.Validator, error) { + maxVals := k.GetMaxProviderConsensusValidators(ctx) + return ccv.GetLastBondedValidatorsUtil(ctx, k.stakingKeeper, k.Logger(ctx), uint32(maxVals)) } diff --git a/x/ccv/types/utils.go b/x/ccv/types/utils.go index 3f69de50ad..bc2e052dcb 100644 --- a/x/ccv/types/utils.go +++ b/x/ccv/types/utils.go @@ -125,16 +125,13 @@ func GetConsAddrFromBech32(bech32str string) (sdk.ConsAddress, error) { return sdk.ConsAddress(addr), nil } -func GetLastBondedValidatorsUtil(ctx sdk.Context, stakingKeeper StakingKeeper, logger log.Logger) ([]stakingtypes.Validator, error) { - maxVals, err := stakingKeeper.MaxValidators(ctx) - if err != nil { - return nil, err - } - +// GetLastBondedValidatorsUtil iterates the last validator powers in the staking module +// and returns the first MaxValidators many validators with the largest powers. +func GetLastBondedValidatorsUtil(ctx sdk.Context, stakingKeeper StakingKeeper, logger log.Logger, maxVals uint32) ([]stakingtypes.Validator, error) { lastPowers := make([]stakingtypes.LastValidatorPower, maxVals) i := 0 - err = stakingKeeper.IterateLastValidatorPowers(ctx, func(addr sdk.ValAddress, power int64) (stop bool) { + err := stakingKeeper.IterateLastValidatorPowers(ctx, func(addr sdk.ValAddress, power int64) (stop bool) { lastPowers[i] = stakingtypes.LastValidatorPower{Address: addr.String(), Power: power} i++ return i >= int(maxVals) // stop iteration if true