diff --git a/x/ccv/provider/keeper/partial_set_security.go b/x/ccv/provider/keeper/partial_set_security.go index 3e8d1c1a8c..c34885e117 100644 --- a/x/ccv/provider/keeper/partial_set_security.go +++ b/x/ccv/provider/keeper/partial_set_security.go @@ -4,8 +4,8 @@ import ( errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - "github.com/cosmos/interchain-security/v4/x/ccv/provider/types" + "math" "sort" ) @@ -51,7 +51,7 @@ func (k Keeper) HandleOptOut(ctx sdk.Context, chainID string, providerAddr types "opting out of an unknown or not running consumer chain, with id: %s", chainID) } - if topN, found := k.GetTopN(ctx, chainID); found { + if topN, found := k.GetTopN(ctx, chainID); found && topN > 0 { // a validator cannot opt out from a Top N chain if the validator is in the Top N validators validator, validatorFound := k.stakingKeeper.GetValidatorByConsAddr(ctx, providerAddr.ToSdkConsAddr()) if !validatorFound { @@ -60,7 +60,7 @@ func (k Keeper) HandleOptOut(ctx sdk.Context, chainID string, providerAddr types "validator with consensus address %s could not be found", providerAddr.ToSdkConsAddr()) } power := k.stakingKeeper.GetLastValidatorPower(ctx, validator.GetOperator()) - minPowerToOptIn := k.ComputeMinPowerToOptIn(ctx, k.stakingKeeper.GetLastValidators(ctx), topN) + minPowerToOptIn := k.ComputeMinPowerToOptIn(ctx, chainID, k.stakingKeeper.GetLastValidators(ctx), topN) if power >= minPowerToOptIn { return errorsmod.Wrapf( @@ -94,8 +94,16 @@ func (k Keeper) OptInTopNValidators(ctx sdk.Context, chainID string, bondedValid } // ComputeMinPowerToOptIn returns the minimum power needed for a validator (from the bonded validators) -// to belong to the `topN` validators -func (k Keeper) ComputeMinPowerToOptIn(ctx sdk.Context, bondedValidators []stakingtypes.Validator, topN uint32) int64 { +// to belong to the `topN` validators. `chainID` is only used for logging purposes. +func (k Keeper) ComputeMinPowerToOptIn(ctx sdk.Context, chainID string, bondedValidators []stakingtypes.Validator, topN uint32) int64 { + if topN == 0 { + // This should never happen but because `ComputeMinPowerToOptIn` is called during an `EndBlock` we do want + // to `panic` here. Instead, we log an error and return the maximum possible `int64`. + k.Logger(ctx).Error("trying to compute minimum power to opt in for a non-Top-N chain", + "chainID", chainID) + return math.MaxInt64 + } + totalPower := sdk.ZeroDec() var powers []int64 diff --git a/x/ccv/provider/keeper/partial_set_security_test.go b/x/ccv/provider/keeper/partial_set_security_test.go index f17bd71a50..303bdd633f 100644 --- a/x/ccv/provider/keeper/partial_set_security_test.go +++ b/x/ccv/provider/keeper/partial_set_security_test.go @@ -2,6 +2,7 @@ package keeper_test import ( "bytes" + "math" "sort" "testing" @@ -271,16 +272,19 @@ func TestComputeMinPowerToOptIn(t *testing.T) { createStakingValidator(ctx, mocks, 5, 6), } - require.Equal(t, int64(1), providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 100)) - require.Equal(t, int64(1), providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 97)) - require.Equal(t, int64(3), providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 96)) - require.Equal(t, int64(3), providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 85)) - require.Equal(t, int64(5), providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 84)) - require.Equal(t, int64(5), providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 65)) - require.Equal(t, int64(6), providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 64)) - require.Equal(t, int64(6), providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 41)) - require.Equal(t, int64(10), providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 40)) - require.Equal(t, int64(10), providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 1)) + require.Equal(t, int64(1), providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 100)) + require.Equal(t, int64(1), providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 97)) + require.Equal(t, int64(3), providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 96)) + require.Equal(t, int64(3), providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 85)) + require.Equal(t, int64(5), providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 84)) + require.Equal(t, int64(5), providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 65)) + require.Equal(t, int64(6), providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 64)) + require.Equal(t, int64(6), providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 41)) + require.Equal(t, int64(10), providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 40)) + require.Equal(t, int64(10), providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 1)) + + // exceptional case when we erroneously call with `topN == 0` + require.Equal(t, int64(math.MaxInt64), providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 0)) } // TestShouldConsiderOnlyOptIn returns true if `validator` is opted in, in `chainID. diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index bab1690f8b..360ff8f9d2 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -291,7 +291,7 @@ func (k Keeper) MakeConsumerGenesis( if prop.Top_N > 0 { // in a Top-N chain, we automatically opt in all validators that belong to the top N - minPower := k.ComputeMinPowerToOptIn(ctx, bondedValidators, prop.Top_N) + minPower := k.ComputeMinPowerToOptIn(ctx, chainID, bondedValidators, prop.Top_N) k.OptInTopNValidators(ctx, chainID, bondedValidators, minPower) } diff --git a/x/ccv/provider/keeper/relay.go b/x/ccv/provider/keeper/relay.go index 9728d7f59f..32506297bb 100644 --- a/x/ccv/provider/keeper/relay.go +++ b/x/ccv/provider/keeper/relay.go @@ -223,9 +223,9 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) { for _, chain := range k.GetAllConsumerChains(ctx) { currentValidators := k.GetConsumerValSet(ctx, chain.ChainId) - if topN, found := k.GetTopN(ctx, chain.ChainId); found { + if topN, found := k.GetTopN(ctx, chain.ChainId); found && topN > 0 { // in a Top-N chain, we automatically opt in all validators that belong to the top N - minPower := k.ComputeMinPowerToOptIn(ctx, bondedValidators, topN) + minPower := k.ComputeMinPowerToOptIn(ctx, chain.ChainId, bondedValidators, topN) k.OptInTopNValidators(ctx, chain.ChainId, bondedValidators, minPower) }