Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: store the minimal power to be in the top N on EndBlock #1977

Merged
merged 1 commit into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 11 additions & 16 deletions x/ccv/provider/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,15 @@ func (k Keeper) GetConsumerChain(ctx sdk.Context, chainID string) (types.Chain,
}

topN, found := k.GetTopN(ctx, chainID)
if !found {
k.Logger(ctx).Error("failed to get top N, treating as 0", "chain", chainID)
topN = 0
}

// Get MinPowerInTop_N
var minPowerInTopN int64
if found && topN > 0 {
bondedValidators, err := k.GetLastBondedValidators(ctx)
if err != nil {
return types.Chain{}, err
}
res, err := k.ComputeMinPowerToOptIn(ctx, bondedValidators, topN)
if err != nil {
return types.Chain{}, fmt.Errorf("failed to compute min power to opt in for chain (%s): %w", chainID, err)
}
minPowerInTopN = res
} else {
// Get the minimal power in the top N for the consumer chain
minPowerInTopN, found := k.GetMinimumPowerInTopN(ctx, chainID)
if !found {
k.Logger(ctx).Error("failed to get minimum power in top N, treating as -1", "chain", chainID)
bermuell marked this conversation as resolved.
Show resolved Hide resolved
minPowerInTopN = -1
}

Expand Down Expand Up @@ -395,11 +390,11 @@ func (k Keeper) hasToValidate(
}
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, bondedValidators, topN)
if err == nil {
minPower, found := k.GetMinimumPowerInTopN(ctx, chainID)
if found {
k.OptInTopNValidators(ctx, chainID, bondedValidators, minPower)
} else {
k.Logger(ctx).Error("failed to compute min power to opt in for chain", "chain", chainID, "error", err)
k.Logger(ctx).Error("did not find min power in top N for chain", "chain", chainID)
}
}

Expand Down
1 change: 1 addition & 0 deletions x/ccv/provider/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ func TestGetConsumerChain(t *testing.T) {
pk.SetTopN(ctx, chainID, topN)
pk.SetValidatorSetCap(ctx, chainID, validatorSetCaps[i])
pk.SetValidatorsPowerCap(ctx, chainID, validatorPowerCaps[i])
pk.SetMinimumPowerInTopN(ctx, chainID, expectedMinPowerInTopNs[i])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that setting minimal power in top N is covered by assertions in tests.

Consider adding assertions to verify that SetMinimumPowerInTopN correctly sets the expected minimal power. This will ensure the function's reliability through automated tests.

for _, addr := range allowlists[i] {
pk.SetAllowlist(ctx, chainID, addr)
}
Expand Down
39 changes: 39 additions & 0 deletions x/ccv/provider/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -1543,3 +1543,42 @@ func (k Keeper) IsDenylistEmpty(ctx sdk.Context, chainID string) bool {

return !iterator.Valid()
}

// SetMinimumPowerInTopN sets the minimum power required for a validator to be in the top N
// for a given consumer chain.
func (k Keeper) SetMinimumPowerInTopN(
ctx sdk.Context,
chainID string,
power int64,
) {
store := ctx.KVStore(k.storeKey)

buf := make([]byte, 8)
binary.BigEndian.PutUint64(buf, uint64(power))

store.Set(types.MinimumPowerInTopNKey(chainID), buf)
}
sainoe marked this conversation as resolved.
Show resolved Hide resolved

// GetMinimumPowerInTopN returns the minimum power required for a validator to be in the top N
// for a given consumer chain.
func (k Keeper) GetMinimumPowerInTopN(
ctx sdk.Context,
chainID string,
) (int64, bool) {
store := ctx.KVStore(k.storeKey)
buf := store.Get(types.MinimumPowerInTopNKey(chainID))
if buf == nil {
return 0, false
}
return int64(binary.BigEndian.Uint64(buf)), true
}

// DeleteMinimumPowerInTopN removes the minimum power required for a validator to be in the top N
// for a given consumer chain.
func (k Keeper) DeleteMinimumPowerInTopN(
ctx sdk.Context,
chainID string,
) {
store := ctx.KVStore(k.storeKey)
store.Delete(types.MinimumPowerInTopNKey(chainID))
}
30 changes: 30 additions & 0 deletions x/ccv/provider/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -831,3 +831,33 @@ func TestDenylist(t *testing.T) {
require.False(t, providerKeeper.IsDenylisted(ctx, chainID, providerAddr2))
require.True(t, providerKeeper.IsDenylistEmpty(ctx, chainID))
}

func TestMinimumPowerInTopN(t *testing.T) {
k, ctx, _, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))

chainID := "testChain"
minPower := int64(1000)

// Set the minimum power in top N
k.SetMinimumPowerInTopN(ctx, chainID, minPower)

// Retrieve the minimum power in top N
actualMinPower, found := k.GetMinimumPowerInTopN(ctx, chainID)
require.True(t, found)
require.Equal(t, minPower, actualMinPower)

// Update the minimum power
newMinPower := int64(2000)
k.SetMinimumPowerInTopN(ctx, chainID, newMinPower)

// Retrieve the updated minimum power in top N
newActualMinPower, found := k.GetMinimumPowerInTopN(ctx, chainID)
require.True(t, found)
require.Equal(t, newMinPower, newActualMinPower)

// Test when the chain ID does not exist
nonExistentChainID := "nonExistentChain"
nonExistentMinPower, found := k.GetMinimumPowerInTopN(ctx, nonExistentChainID)
require.False(t, found)
require.Equal(t, int64(0), nonExistentMinPower)
}
25 changes: 25 additions & 0 deletions x/ccv/provider/keeper/legacy_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,5 +113,30 @@ func (k Keeper) HandleLegacyConsumerModificationProposal(ctx sdk.Context, p *typ
k.SetDenylist(ctx, p.ChainId, types.NewProviderConsAddress(consAddr))
}

oldTopN, found := k.GetTopN(ctx, p.ChainId)
if !found {
oldTopN = 0
k.Logger(ctx).Info("consumer chain top N not found, treating as 0", "chainID", p.ChainId)
}

// if the top N changes, we need to update the new minimum power in top N
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)
if err != nil {
return err
}
minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, p.Top_N)
if err != nil {
return err
}
k.SetMinimumPowerInTopN(ctx, p.ChainId, minPower)
} else {
// if the chain receives a zero top N value, we delete the min power
k.DeleteMinimumPowerInTopN(ctx, p.ChainId)
}
}

bermuell marked this conversation as resolved.
Show resolved Hide resolved
return nil
}
25 changes: 9 additions & 16 deletions x/ccv/provider/keeper/partial_set_security.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,25 +69,18 @@ func (k Keeper) HandleOptOut(ctx sdk.Context, chainID string, providerAddr types
if err != nil {
return err
}
bondedValidators, err := k.GetLastBondedValidators(ctx)
if err != nil {
return errorsmod.Wrapf(stakingtypes.ErrNoValidatorFound, "error getting last bonded validators: %s", err)
}
minPowerToOptIn, err := k.ComputeMinPowerToOptIn(ctx, bondedValidators, topN)
if err != nil {
k.Logger(ctx).Error("failed to compute min power to opt in for chain", "chain", chainID, "error", err)
minPowerInTopN, found := k.GetMinimumPowerInTopN(ctx, chainID)
if !found {
return errorsmod.Wrapf(
types.ErrCannotOptOutFromTopN,
"validator with power (%d) cannot opt out from Top N chain (%s) because the min power"+
" could not be computed: %s", power, chainID, err.Error())

types.ErrUnknownConsumerChainId,
"Could not find minimum power in top N for chain with id: %s", chainID)
}

if power >= minPowerToOptIn {
if power >= minPowerInTopN {
return errorsmod.Wrapf(
types.ErrCannotOptOutFromTopN,
"validator with power (%d) cannot opt out from Top N chain (%s) because all validators"+
" with at least %d power have to validate", power, chainID, minPowerToOptIn)
" with at least %d power have to validate", power, chainID, minPowerInTopN)
bermuell marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -124,9 +117,9 @@ 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, bondedValidators []stakingtypes.Validator, topN uint32) (int64, error) {
// ComputeMinPowerInTopN returns the minimum power needed for a validator (from the bonded validators)
// to belong to the `topN`% of validators for a Top N chain.
func (k Keeper) ComputeMinPowerInTopN(ctx sdk.Context, bondedValidators []stakingtypes.Validator, topN uint32) (int64, error) {
bermuell marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
31 changes: 18 additions & 13 deletions x/ccv/provider/keeper/partial_set_security_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ func TestHandleOptOutFromTopNChain(t *testing.T) {

testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 4, []stakingtypes.Validator{valA, valB, valC, valD}, []int64{1, 2, 3, 4}, -1) // -1 to allow mocks AnyTimes

// initialize the minPowerInTopN correctly
minPowerInTopN, err := providerKeeper.ComputeMinPowerInTopN(ctx, []stakingtypes.Validator{valA, valB, valC, valD}, 50)
require.NoError(t, err)
providerKeeper.SetMinimumPowerInTopN(ctx, chainID, minPowerInTopN)

// opt in all validators
providerKeeper.SetOptedIn(ctx, chainID, types.NewProviderConsAddress(valAConsAddr))
providerKeeper.SetOptedIn(ctx, chainID, types.NewProviderConsAddress(valBConsAddr))
Expand Down Expand Up @@ -282,7 +287,7 @@ func TestOptInTopNValidators(t *testing.T) {
require.Empty(t, providerKeeper.GetAllOptedIn(ctx, "chainID"))
}

func TestComputeMinPowerToOptIn(t *testing.T) {
func TestComputeMinPowerInTopN(t *testing.T) {
providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

Expand All @@ -303,50 +308,50 @@ func TestComputeMinPowerToOptIn(t *testing.T) {
createStakingValidator(ctx, mocks, 5, 6, 5),
}

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

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

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

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

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

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

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

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

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

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

_, err = providerKeeper.ComputeMinPowerToOptIn(ctx, bondedValidators, 0)
_, err = providerKeeper.ComputeMinPowerInTopN(ctx, bondedValidators, 0)
require.Error(t, err)

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

Expand Down
12 changes: 6 additions & 6 deletions x/ccv/provider/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ func (k Keeper) StopConsumerChain(ctx sdk.Context, chainID string, closeChan boo
k.DeleteInitTimeoutTimestamp(ctx, chainID)
// Note: this call panics if the key assignment state is invalid
k.DeleteKeyAssignments(ctx, chainID)
k.DeleteMinimumPowerInTopN(ctx, chainID)

// close channel and delete the mappings between chain ID and channel ID
if channelID, found := k.GetChainToChannel(ctx, chainID); found {
Expand Down Expand Up @@ -282,16 +283,15 @@ func (k Keeper) MakeConsumerGenesis(
return gen, nil, errorsmod.Wrapf(stakingtypes.ErrNoValidatorFound, "error getting last bonded validators: %s", err)
}

if topN, found := k.GetTopN(ctx, chainID); found && topN > 0 {
if prop.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, bondedValidators, prop.Top_N)
if err == nil {
k.OptInTopNValidators(ctx, chainID, bondedValidators, minPower)
} else {
minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, prop.Top_N)
if err != nil {
return gen, nil, err
}
k.OptInTopNValidators(ctx, chainID, bondedValidators, minPower)
k.SetMinimumPowerInTopN(ctx, chainID, minPower)
}

nextValidators := k.ComputeNextValidators(ctx, chainID, bondedValidators)

k.SetConsumerValSet(ctx, chainID, nextValidators)
Expand Down
5 changes: 4 additions & 1 deletion x/ccv/provider/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,11 @@ 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.ComputeMinPowerToOptIn(ctx, bondedValidators, topN)
minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, topN)
if err == nil {
// set the minimal power of validators in the top N in the store
k.SetMinimumPowerInTopN(ctx, chainID, minPower)

bermuell marked this conversation as resolved.
Show resolved Hide resolved
k.OptInTopNValidators(ctx, chainID, bondedValidators, minPower)
} else {
// we just log here and do not panic because panic-ing would halt the provider chain
Expand Down
5 changes: 4 additions & 1 deletion x/ccv/provider/migrations/migrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ func (m Migrator) Migrate4to5(ctx sdktypes.Context) error {
}

// Migrate5to6 migrates x/ccvprovider state from consensus version 5 to 6.
// The migration consists of initializing new provider chain params using params from the legacy store.
// The migration consists of
// - computing and storing the minimal power in the top N for all registered consumer chains.
// - initializing new provider chain params using params from the legacy store.
func (m Migrator) Migrate5to6(ctx sdktypes.Context) error {
v6.MigrateMinPowerInTopN(ctx, m.providerKeeper)
return v6.MigrateLegacyParams(ctx, m.providerKeeper, m.paramSpace)
}
30 changes: 30 additions & 0 deletions x/ccv/provider/migrations/v6/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,33 @@ func MigrateLegacyParams(ctx sdk.Context, keeper providerkeeper.Keeper, legacyPa
keeper.Logger(ctx).Info("successfully migrated legacy provider parameters")
return nil
}

func MigrateMinPowerInTopN(ctx sdk.Context, providerKeeper providerkeeper.Keeper) {
// we only get the registered consumer chains and not also the proposed consumer chains because
// the minimal power is first set when the consumer chain addition proposal passes
registeredConsumerChains := providerKeeper.GetAllRegisteredConsumerChainIDs(ctx)

for _, chain := range registeredConsumerChains {
// get the top N
topN, found := providerKeeper.GetTopN(ctx, chain)
if !found {
providerKeeper.Logger(ctx).Error("failed to get top N", "chain", chain)
continue
} else if topN == 0 {
providerKeeper.Logger(ctx).Info("top N is 0, not setting minimal power", "chain", chain)
} else {
// set the minimal power in the top N
bondedValidators, err := providerKeeper.GetLastBondedValidators(ctx)
if err != nil {
providerKeeper.Logger(ctx).Error("failed to get last bonded validators", "chain", chain, "error", err)
continue
}
minPower, err := providerKeeper.ComputeMinPowerInTopN(ctx, bondedValidators, topN)
if err != nil {
providerKeeper.Logger(ctx).Error("failed to compute min power in top N", "chain", chain, "topN", topN, "error", err)
continue
}
providerKeeper.SetMinimumPowerInTopN(ctx, chain, minPower)
}
}
}
bermuell marked this conversation as resolved.
Show resolved Hide resolved
8 changes: 8 additions & 0 deletions x/ccv/provider/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ const (
// per validator per consumer chain
ConsumerCommissionRatePrefix

// MinimumPowerInTopNBytePrefix is the byte prefix for storing the
// minimum power required to be in the top N per consumer chain.
MinimumPowerInTopNBytePrefix

// NOTE: DO NOT ADD NEW BYTE PREFIXES HERE WITHOUT ADDING THEM TO getAllKeyPrefixes() IN keys_test.go
)

Expand Down Expand Up @@ -613,6 +617,10 @@ func ConsumerCommissionRateKey(chainID string, providerAddr ProviderConsAddress)
)
}

func MinimumPowerInTopNKey(chainID string) []byte {
return ChainIdWithLenKey(MinimumPowerInTopNBytePrefix, chainID)
}

//
// End of generic helpers section
//
Loading
Loading