Skip to content

Commit

Permalink
fixed topN == 0 issue
Browse files Browse the repository at this point in the history
  • Loading branch information
insumity committed Mar 25, 2024
1 parent 629cfd1 commit daaeeab
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 18 deletions.
18 changes: 13 additions & 5 deletions x/ccv/provider/keeper/partial_set_security.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand All @@ -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(
Expand Down Expand Up @@ -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

Expand Down
24 changes: 14 additions & 10 deletions x/ccv/provider/keeper/partial_set_security_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper_test

import (
"bytes"
"math"
"sort"
"testing"

Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion x/ccv/provider/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
4 changes: 2 additions & 2 deletions x/ccv/provider/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down

0 comments on commit daaeeab

Please sign in to comment.