Skip to content

Commit

Permalink
feat!: store the minimal power to be in the top N on EndBlock, instea…
Browse files Browse the repository at this point in the history
…d of computing on-the-fly (#1952)

* Store the minimal power among the top N in EndBlock

* Finish merge

* Fix unit tests

* Fix store method for the min power

* Fix migration

* Revert migration changes

* Change comment to proper name for key

* Add staking keeper to migration

* Revert "Add staking keeper to migration"

This reverts commit 575cfd3.

* Rename migration

* Update x/ccv/provider/keeper/grpc_query.go

* Clean up minimal power in top N on StopConsumerChain

* Set min power in consumer modification proposal

* Address comments

* Use GetLastBondedValidators instead of GetLastValidators

* Add migration

* Add comment for migration

* Improve comment in migration

* Handle case where topN is not found

* Add test for updating minimum power in top N

* Merged tests

* Rename updatedMinPower->newUpdatedMinPower

* Address comments
  • Loading branch information
p-offtermatt authored and bermuell committed Jun 20, 2024
1 parent 134de6a commit 680b291
Show file tree
Hide file tree
Showing 13 changed files with 186 additions and 53 deletions.
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)
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])
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)
}

// 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)
}
}

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)
}
}

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) {
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)

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)
}
}
}
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

0 comments on commit 680b291

Please sign in to comment.