Skip to content

Commit

Permalink
took into account comments
Browse files Browse the repository at this point in the history
  • Loading branch information
insumity committed Jun 5, 2024
1 parent e6ce423 commit 54afcba
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 22 deletions.
4 changes: 2 additions & 2 deletions x/ccv/provider/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ func (k Keeper) GetAllConsumerChains(ctx sdk.Context) (chains []types.Chain) {

var minPowerInTopN int64
if found && topN > 0 {
res, err := k.ComputeMinPowerToOptIn(ctx, chainID, k.stakingKeeper.GetLastValidators(ctx), topN)
res, err := k.ComputeMinPowerToOptIn(ctx, k.stakingKeeper.GetLastValidators(ctx), topN)
if err != nil {
k.Logger(ctx).Error("failed to compute min power to opt in for chain", "chain", chainID, "error", err)
minPowerInTopN = -1
Expand Down Expand Up @@ -1298,7 +1298,7 @@ func (k Keeper) HasToValidate(
bondedValidators := k.stakingKeeper.GetLastValidators(ctx)
if topN, found := k.GetTopN(ctx, chainID); found && topN > 0 {
// in a Top-N chain, we automatically opt in all validators that belong to the top N
minPower, err := k.ComputeMinPowerToOptIn(ctx, chainID, bondedValidators, topN)
minPower, err := k.ComputeMinPowerToOptIn(ctx, bondedValidators, topN)
if err == nil {
k.OptInTopNValidators(ctx, chainID, bondedValidators, minPower)
} else {
Expand Down
13 changes: 8 additions & 5 deletions x/ccv/provider/keeper/partial_set_security.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,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, err := k.ComputeMinPowerToOptIn(ctx, chainID, k.stakingKeeper.GetLastValidators(ctx), topN)
minPowerToOptIn, err := k.ComputeMinPowerToOptIn(ctx, k.stakingKeeper.GetLastValidators(ctx), topN)

if err != nil {
k.Logger(ctx).Error("failed to compute min power to opt in for chain", "chain", chainID, "error", err)
Expand Down Expand Up @@ -107,10 +107,13 @@ 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 for a Top N chain.
func (k Keeper) ComputeMinPowerToOptIn(ctx sdk.Context, chainID string, bondedValidators []stakingtypes.Validator, topN uint32) (int64, error) {
if topN < 50 || topN > 100 {
return 0, fmt.Errorf("trying to compute minimum power for a Top N chain (%s) with an incorrect"+
" topN value (%d). topN has to be in [50, 100]", chainID, topN)
func (k Keeper) ComputeMinPowerToOptIn(ctx sdk.Context, bondedValidators []stakingtypes.Validator, topN uint32) (int64, error) {
if topN == 0 || topN > 100 {
// Note that Top N chains have a lower limit on `topN`, namely that topN cannot be less than 50.
// However, we can envision that this method could be used for other (future) reasons where this might not
// be the case. For this, this method operates for `topN`s in (0, 100].
return 0, fmt.Errorf("trying to compute minimum power with an incorrect"+
" topN value (%d). topN has to be in (0, 100]", topN)
}

totalPower := sdk.ZeroDec()
Expand Down
30 changes: 17 additions & 13 deletions x/ccv/provider/keeper/partial_set_security_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,46 +299,50 @@ func TestComputeMinPowerToOptIn(t *testing.T) {
createStakingValidator(ctx, mocks, 5, 6),
}

m, err := providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 100)
m, err := providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 100)
require.NoError(t, err)
require.Equal(t, int64(1), m)

m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 97)
m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 97)
require.NoError(t, err)
require.Equal(t, int64(1), m)

m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 96)
m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 96)
require.NoError(t, err)
require.Equal(t, int64(3), m)

m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 85)
m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 85)
require.NoError(t, err)
require.Equal(t, int64(3), m)

m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 84)
m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 84)
require.NoError(t, err)
require.Equal(t, int64(5), m)

m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 65)
m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 65)
require.NoError(t, err)
require.Equal(t, int64(5), m)

m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 64)
m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 64)
require.NoError(t, err)
require.Equal(t, int64(6), m)

m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 50)
m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 50)
require.NoError(t, err)
require.Equal(t, int64(6), m)

// exceptional cases
_, err = providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 0)
require.Error(t, err)
m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 40)
require.NoError(t, err)
require.Equal(t, int64(10), m)

m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 1)
require.NoError(t, err)
require.Equal(t, int64(10), m)

m, err = providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 49)
_, err = providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 0)
require.Error(t, err)

_, err = providerKeeper.ComputeMinPowerToOptIn(ctx, "chainID", bondedValidators, 101)
_, err = providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 101)
require.Error(t, err)
}

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 @@ -264,7 +264,7 @@ func (k Keeper) MakeConsumerGenesis(

if topN, found := k.GetTopN(ctx, chainID); found && topN > 0 {
// in a Top-N chain, we automatically opt in all validators that belong to the top N
minPower, err := k.ComputeMinPowerToOptIn(ctx, chainID, bondedValidators, prop.Top_N)
minPower, err := k.ComputeMinPowerToOptIn(ctx, bondedValidators, prop.Top_N)
if err == nil {
k.OptInTopNValidators(ctx, chainID, bondedValidators, minPower)
} else {
Expand Down
2 changes: 1 addition & 1 deletion x/ccv/provider/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) {

if chain.Top_N > 0 {
// in a Top-N chain, we automatically opt in all validators that belong to the top N
minPower, err := k.ComputeMinPowerToOptIn(ctx, chain.ChainId, bondedValidators, chain.Top_N)
minPower, err := k.ComputeMinPowerToOptIn(ctx, bondedValidators, chain.Top_N)
if err == nil {
k.OptInTopNValidators(ctx, chain.ChainId, bondedValidators, minPower)
} else {
Expand Down

0 comments on commit 54afcba

Please sign in to comment.