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 May 31, 2024
1 parent c5b3aee commit e606ab7
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 13 deletions.
4 changes: 2 additions & 2 deletions tests/integration/distribution.go
Original file line number Diff line number Diff line change
Expand Up @@ -1077,8 +1077,8 @@ func (s *CCVTestSuite) TestAllocateTokensToConsumerValidatorsWithDifferentValida

ctx, _ := s.providerCtx().CacheContext()
// If the provider chain has not yet reached `GetNumberOfEpochsToStartReceivingRewards * GetBlocksPerEpoch` block height,
// then all validators receive rewards. In this test, we want to check whether validators receive rewards or not based on how
// long they have been consumer validators. Because of this, we increase the block height.
// then all validators receive rewards (see `IsEligibleForConsumerRewards`). In this test, we want to check whether
// validators receive rewards or not based on how long they have been consumer validators. Because of this, we increase the block height.
ctx = ctx.WithBlockHeight(providerKeeper.GetNumberOfEpochsToStartReceivingRewards(ctx)*providerKeeper.GetBlocksPerEpoch(ctx) + 1)

// update the consumer validators
Expand Down
25 changes: 19 additions & 6 deletions x/ccv/provider/keeper/distribution.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,23 @@ func (k Keeper) AllocateTokens(ctx sdk.Context) {
}
}

// IsEligibleForConsumerRewards returns `true` if the validator with `consumerValidatorHeight` has been a consumer
// validator for a long period of time and hence is eligible to receive rewards, and false otherwise
func (k Keeper) IsEligibleForConsumerRewards(ctx sdk.Context, consumerValidatorHeight int64) bool {
numberOfBlocksToStartReceivingRewards := k.GetNumberOfEpochsToStartReceivingRewards(ctx) * k.GetBlocksPerEpoch(ctx)

// Note that at the beginning of the chain's life (e.g., when the block height of the chain is still small), no validator
// could have been a consumer validator for more than `NumberOfEpochsToStartReceivingRewards` epochs and hence in this
// case all validators are eligible for rewards, even though they might have not been a consumer validator on the
// consumer chain for more than a single epoch.
isChainStillNew := ctx.BlockHeight() < numberOfBlocksToStartReceivingRewards

// a validator is eligible for rewards if it has been a consumer validator for `NumberOfEpochsToStartReceivingRewards` epochs
hasBeenConsumerValidatorForLongEnough := (ctx.BlockHeight() - consumerValidatorHeight) >= numberOfBlocksToStartReceivingRewards

return isChainStillNew || hasBeenConsumerValidatorForLongEnough
}

// AllocateTokensToConsumerValidators allocates tokens
// to the given consumer chain's validator set
func (k Keeper) AllocateTokensToConsumerValidators(
Expand All @@ -147,10 +164,7 @@ func (k Keeper) AllocateTokensToConsumerValidators(

// Allocate tokens by iterating over the consumer validators
for _, consumerVal := range k.GetConsumerValSet(ctx, chainID) {

// do not distribute rewards to a validator that has not been validating long enough
numberOfBlocksToStartReceivingRewards := k.GetNumberOfEpochsToStartReceivingRewards(ctx) * k.GetBlocksPerEpoch(ctx)
if ctx.BlockHeight() >= numberOfBlocksToStartReceivingRewards && ctx.BlockHeight()-consumerVal.Height < numberOfBlocksToStartReceivingRewards {
if !k.IsEligibleForConsumerRewards(ctx, consumerVal.Height) {
continue
}

Expand Down Expand Up @@ -249,8 +263,7 @@ func (k Keeper) ComputeConsumerTotalVotingPower(ctx sdk.Context, chainID string)
for _, v := range k.GetConsumerValSet(ctx, chainID) {

// only consider the voting power of a validator that would receive rewards (i.e., validator has been validating for a number of blocks)
numberOfBlocksToStartReceivingRewards := k.GetNumberOfEpochsToStartReceivingRewards(ctx) * k.GetBlocksPerEpoch(ctx)
if ctx.BlockHeight() >= numberOfBlocksToStartReceivingRewards && ctx.BlockHeight()-v.Height < numberOfBlocksToStartReceivingRewards {
if !k.IsEligibleForConsumerRewards(ctx, v.Height) {
continue
}

Expand Down
21 changes: 21 additions & 0 deletions x/ccv/provider/keeper/distribution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,24 @@ func TestGetConsumerRewardsAllocationNil(t *testing.T) {
}
require.Equal(t, expectedRewardAllocation, alloc)
}

func TestIsEligibleForConsumerRewards(t *testing.T) {
keeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

params := providertypes.DefaultParams()
params.NumberOfEpochsToStartReceivingRewards = 10
params.BlocksPerEpoch = 5
keeper.SetParams(ctx, params)

numberOfBlocks := params.NumberOfEpochsToStartReceivingRewards * params.BlocksPerEpoch

// if the provider's chain block height is less than `numberOfBlocks`, then irrespectively of the height of a consumer
// validator, the validator is eligible for rewards
require.True(t, keeper.IsEligibleForConsumerRewards(ctx.WithBlockHeight(numberOfBlocks-1), numberOfBlocks-1))

require.True(t, keeper.IsEligibleForConsumerRewards(ctx.WithBlockHeight(numberOfBlocks), 0))
require.True(t, keeper.IsEligibleForConsumerRewards(ctx.WithBlockHeight(numberOfBlocks+1), 0))
require.True(t, keeper.IsEligibleForConsumerRewards(ctx.WithBlockHeight(numberOfBlocks+1), 1))
require.False(t, keeper.IsEligibleForConsumerRewards(ctx.WithBlockHeight(numberOfBlocks+1), 2))
}
2 changes: 1 addition & 1 deletion x/ccv/provider/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestQueueVSCPackets(t *testing.T) {
}

// TestQueueVSCPacketsDoesNotResetConsumerValidatorsHeights checks that the heights of consumer validators are not
// getting correctly updated
// getting incorrectly updated
func TestQueueVSCPacketsDoesNotResetConsumerValidatorsHeights(t *testing.T) {
providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()
Expand Down
6 changes: 2 additions & 4 deletions x/ccv/provider/migrations/v6/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ import (

// MigrateParams adds missing provider chain params to the param store.
func MigrateParams(ctx sdk.Context, paramsSubspace paramtypes.Subspace) {
if paramsSubspace.HasKeyTable() {
paramsSubspace.Set(ctx, providertypes.KeyNumberOfEpochsToStartReceivingRewards, providertypes.DefaultNumberOfEpochsToStartReceivingRewards)
} else {
if !paramsSubspace.HasKeyTable() {
paramsSubspace.WithKeyTable(providertypes.ParamKeyTable())
paramsSubspace.Set(ctx, providertypes.KeyNumberOfEpochsToStartReceivingRewards, providertypes.DefaultNumberOfEpochsToStartReceivingRewards)
}
paramsSubspace.Set(ctx, providertypes.KeyNumberOfEpochsToStartReceivingRewards, providertypes.DefaultNumberOfEpochsToStartReceivingRewards)
}

0 comments on commit e606ab7

Please sign in to comment.