feat!: only distribute rewards to validators that have been validatin…

…g a consumer chain for some time (#1929)

* init commit

* added a warning

* took into account comments

* init commit

* added a warning

* took into account comments

* added a comment

* Update .changelog/unreleased/improvements/provider/1929-distribute-rewards-to-long-term-validating-validators.md

Co-authored-by: Marius Poke <[email protected]>

* Update .changelog/unreleased/state-breaking/provider/1929-distribute-rewards-to-long-term-validating-validators.md

Co-authored-by: Marius Poke <[email protected]>

* took into account comments

---------

Co-authored-by: Marius Poke <[email protected]>

.changelog/unreleased/improvements/provider/1929-distribute-rewards-to-long-term-validating-validators.md

@@ -0,0 +1,3 @@
+- Only start distributing rewards to validators after they have been validating
+  for a fixed number of blocks. Introduces the `NumberOfEpochsToStartReceivingRewards` param. 
+  ([\#1929](https://github.com/cosmos/interchain-security/pull/1929))

.changelog/unreleased/state-breaking/provider/1929-distribute-rewards-to-long-term-validating-validators.md

@@ -0,0 +1,3 @@
+- Only start distributing rewards to validators after they have been validating
+  for a fixed number of blocks. Introduces the `NumberOfEpochsToStartReceivingRewards` param. 
+  ([\#1929](https://github.com/cosmos/interchain-security/pull/1929))

docs/docs/validators/withdraw_rewards.md

@@ -2,8 +2,17 @@
 sidebar_position: 3
 ---
 
-# Withdrawing consumer chain validator rewards
+# Consumer chain validator rewards
 
+:::warning
+A validator can only receive rewards from a consumer chain if the validator has been validating the consumer chain
+for some time. Specifically, the validator has to be a consumer validator of the consumer chain for at least
+`NumberOfEpochsToStartReceivingRewards * BlocksPerEpoch` blocks (run `interchain-security-pd query provider params` for
+the actual values of the `NumberOfEpochsToStartReceivingRewards` and `BlocksPerEpoch` params).
+:::
+
+
+## Withdrawing rewards
 Here are example steps for withdrawing rewards from consumer chains in the provider chain
 
 :::info

proto/interchain_security/ccv/provider/v1/provider.proto

@@ -243,6 +243,9 @@ message Params {
 
   // The number of blocks that comprise an epoch.
   int64 blocks_per_epoch = 10;
+
+  // The number of epochs a validator has to validate a consumer chain in order to start receiving rewards from that chain.
+  int64 number_of_epochs_to_start_receiving_rewards = 11;
 }
 
 // SlashAcks contains cons addresses of consumer chain validators
@@ -360,6 +363,11 @@ message ConsumerValidator {
   int64 power = 2;
   // public key the validator uses on the consumer chain during this epoch
   tendermint.crypto.PublicKey consumer_public_key = 3;
+  // height the validator had when it FIRST became a consumer validator
+  // If a validator becomes a consumer validator at height `H` and is continuously a consumer validator for all the upcoming
+  // epochs, then the height of the validator SHOULD remain `H`. This height only resets to a different height if a validator
+  // stops being a consumer validator during an epoch and later becomes again a consumer validator.
+  int64 join_height = 4;
 }
 // ConsumerRewardsAllocation stores the rewards allocated by a consumer chain
 // to the consumer rewards pool. It is used to allocate the tokens to the consumer 

tests/integration/distribution.go

@@ -705,9 +705,14 @@ func (s *CCVTestSuite) TestAllocateTokens() {
 
 	totalRewards := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))}
 
+	// increase the block height so validators are eligible for consumer rewards (see `IsEligibleForConsumerRewards`)
+	numberOfBlocksToStartReceivingRewards :=
+		providerKeeper.GetNumberOfEpochsToStartReceivingRewards(s.providerCtx()) * providerKeeper.GetBlocksPerEpoch(s.providerCtx())
+	providerCtx := s.providerCtx().WithBlockHeight(numberOfBlocksToStartReceivingRewards + s.providerCtx().BlockHeight())
+
 	// fund consumer rewards pool
 	bankKeeper.SendCoinsFromAccountToModule(
-		s.providerCtx(),
+		providerCtx,
 		s.providerChain.SenderAccount.GetAddress(),
 		providertypes.ConsumerRewardsPool,
 		totalRewards,
@@ -718,7 +723,7 @@ func (s *CCVTestSuite) TestAllocateTokens() {
 	for chainID := range s.consumerBundles {
 		// update consumer allocation
 		providerKeeper.SetConsumerRewardsAllocation(
-			s.providerCtx(),
+			providerCtx,
 			chainID,
 			providertypes.ConsumerRewardsAllocation{
 				Rewards: sdk.NewDecCoinsFromCoins(rewardsPerConsumer...),
@@ -738,16 +743,16 @@ func (s *CCVTestSuite) TestAllocateTokens() {
 			},
 		)
 
-		valRewards := distributionKeeper.GetValidatorOutstandingRewards(s.providerCtx(), sdk.ValAddress(val.Address))
+		valRewards := distributionKeeper.GetValidatorOutstandingRewards(providerCtx, sdk.ValAddress(val.Address))
 		lastValOutRewards[sdk.ValAddress(val.Address).String()] = valRewards.Rewards
 	}
 
 	// store community pool balance
-	lastCommPool := distributionKeeper.GetFeePoolCommunityCoins(s.providerCtx())
+	lastCommPool := distributionKeeper.GetFeePoolCommunityCoins(providerCtx)
 
 	// execute BeginBlock to trigger the token allocation
 	providerKeeper.BeginBlockRD(
-		s.providerCtx(),
+		providerCtx,
 		abci.RequestBeginBlock{
 			LastCommitInfo: abci.CommitInfo{
 				Votes: votes,
@@ -760,7 +765,7 @@ func (s *CCVTestSuite) TestAllocateTokens() {
 
 	// compute the expected validators token allocation by subtracting the community tax
 	rewardsPerConsumerDec := sdk.NewDecCoinsFromCoins(rewardsPerConsumer...)
-	communityTax := distributionKeeper.GetCommunityTax(s.providerCtx())
+	communityTax := distributionKeeper.GetCommunityTax(providerCtx)
 	validatorsExpRewards := rewardsPerConsumerDec.
 		MulDecTruncate(math.LegacyOneDec().Sub(communityTax)).
 		// multiply by the number of consumers since all the validators opted in
@@ -770,15 +775,15 @@ func (s *CCVTestSuite) TestAllocateTokens() {
 	// verify the validator tokens allocation
 	// note that the validators have the same voting power to keep things simple
 	for _, val := range s.providerChain.Vals.Validators {
-		valRewards := distributionKeeper.GetValidatorOutstandingRewards(s.providerCtx(), sdk.ValAddress(val.Address))
+		valRewards := distributionKeeper.GetValidatorOutstandingRewards(providerCtx, sdk.ValAddress(val.Address))
 		s.Require().Equal(
 			valRewards.Rewards,
 			lastValOutRewards[sdk.ValAddress(val.Address).String()].Add(perValExpReward...),
 		)
 	}
 
 	commPoolExpRewards := sdk.NewDecCoinsFromCoins(totalRewards...).Sub(validatorsExpRewards)
-	currCommPool := distributionKeeper.GetFeePoolCommunityCoins(s.providerCtx())
+	currCommPool := distributionKeeper.GetFeePoolCommunityCoins(providerCtx)
 
 	s.Require().Equal(currCommPool, (lastCommPool.Add(commPoolExpRewards...)))
 }
@@ -935,7 +940,7 @@ func (s *CCVTestSuite) prepareRewardDist() {
 	s.coordinator.CommitNBlocks(s.consumerChain, uint64(blocksToGo))
 }
 
-func (s *CCVTestSuite) TestAllocateTokensToValidator() {
+func (s *CCVTestSuite) TestAllocateTokensToConsumerValidators() {
 	providerKeeper := s.providerApp.GetProviderKeeper()
 	distributionKeeper := s.providerApp.GetTestDistributionKeeper()
 	bankKeeper := s.providerApp.GetTestBankKeeper()
@@ -981,6 +986,10 @@ func (s *CCVTestSuite) TestAllocateTokensToValidator() {
 		s.Run(tc.name, func() {
 			ctx, _ := s.providerCtx().CacheContext()
 
+			// increase the block height so validators are eligible for consumer rewards (see `IsEligibleForConsumerRewards`)
+			ctx = ctx.WithBlockHeight(providerKeeper.GetNumberOfEpochsToStartReceivingRewards(ctx)*providerKeeper.GetBlocksPerEpoch(ctx) +
+				ctx.BlockHeight())
+
 			// change the consumer valset
 			consuVals := providerKeeper.GetConsumerValSet(ctx, chainID)
 			providerKeeper.DeleteConsumerValSet(ctx, chainID)
@@ -1061,6 +1070,114 @@ func (s *CCVTestSuite) TestAllocateTokensToValidator() {
 	}
 }
 
+// TestAllocateTokensToConsumerValidatorsWithDifferentValidatorHeights tests `AllocateTokensToConsumerValidators` with
+// consumer validators that have different heights. Specifically, test that validators that have been consumer validators
+// for some time receive rewards, while validators that recently became consumer validators do not receive rewards.
+func (s *CCVTestSuite) TestAllocateTokensToConsumerValidatorsWithDifferentValidatorHeights() {
+	// Note this test is an adaptation of a `TestAllocateTokensToConsumerValidators` testcase.
+	providerKeeper := s.providerApp.GetProviderKeeper()
+	distributionKeeper := s.providerApp.GetTestDistributionKeeper()
+	bankKeeper := s.providerApp.GetTestBankKeeper()
+
+	chainID := s.consumerChain.ChainID
+
+	tokens := sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, math.LegacyNewDecFromIntWithPrec(math.NewInt(999), 2))}
+	rate := sdk.OneDec()
+	expAllocated := sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, math.LegacyNewDecFromIntWithPrec(math.NewInt(999), 2))}
+
+	ctx, _ := s.providerCtx().CacheContext()
+	// If the provider chain has not yet reached `GetNumberOfEpochsToStartReceivingRewards * GetBlocksPerEpoch` 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
+	consuVals := providerKeeper.GetConsumerValSet(ctx, chainID)
+	// first 2 validators were consumer validators since block height 1 and hence get rewards
+	consuVals[0].JoinHeight = 1
+	consuVals[1].JoinHeight = 1
+	// last 2 validators were consumer validators since block height 2 and hence do not get rewards because they
+	// have not been consumer validators for `GetNumberOfEpochsToStartReceivingRewards * GetBlocksPerEpoch` blocks
+	consuVals[2].JoinHeight = 2
+	consuVals[3].JoinHeight = 2
+	providerKeeper.SetConsumerValSet(ctx, chainID, consuVals)
+
+	providerKeeper.DeleteConsumerValSet(ctx, chainID)
+	providerKeeper.SetConsumerValSet(ctx, chainID, consuVals)
+	consuVals = providerKeeper.GetConsumerValSet(ctx, chainID)
+
+	// set the same consumer commission rate for all consumer validators
+	for _, v := range consuVals {
+		provAddr := providertypes.NewProviderConsAddress(sdk.ConsAddress(v.ProviderConsAddr))
+		err := providerKeeper.SetConsumerCommissionRate(
+			ctx,
+			chainID,
+			provAddr,
+			rate,
+		)
+		s.Require().NoError(err)
+	}
+
+	// allocate tokens
+	res := providerKeeper.AllocateTokensToConsumerValidators(
+		ctx,
+		chainID,
+		tokens,
+	)
+
+	// check that the expected result is returned
+	s.Require().Equal(expAllocated, res)
+
+	// rewards are expected to be allocated evenly between validators 3 and 4
+	rewardsPerVal := expAllocated.QuoDec(sdk.NewDec(int64(2)))
+
+	// assert that the rewards are allocated to the first 2 validators
+	for _, v := range consuVals[0:2] {
+		valAddr := sdk.ValAddress(v.ProviderConsAddr)
+		rewards := s.providerApp.GetTestDistributionKeeper().GetValidatorOutstandingRewards(
+			ctx,
+			valAddr,
+		)
+		s.Require().Equal(rewardsPerVal, rewards.Rewards)
+
+		// send rewards to the distribution module
+		valRewardsTrunc, _ := rewards.Rewards.TruncateDecimal()
+		err := bankKeeper.SendCoinsFromAccountToModule(
+			ctx,
+			s.providerChain.SenderAccount.GetAddress(),
+			distrtypes.ModuleName,
+			valRewardsTrunc)
+		s.Require().NoError(err)
+
+		// check that validators can withdraw their rewards
+		withdrawnCoins, err := distributionKeeper.WithdrawValidatorCommission(
+			ctx,
+			valAddr,
+		)
+		s.Require().NoError(err)
+
+		// check that the withdrawn coins is equal to the entire reward amount
+		// times the set consumer commission rate
+		commission := rewards.Rewards.MulDec(rate)
+		c, _ := commission.TruncateDecimal()
+		s.Require().Equal(withdrawnCoins, c)
+
+		// check that validators get rewards in their balance
+		s.Require().Equal(withdrawnCoins, bankKeeper.GetAllBalances(ctx, sdk.AccAddress(valAddr)))
+	}
+
+	// assert that no rewards are allocated to the last 2 validators because they have not been consumer validators
+	// for at least `GetNumberOfEpochsToStartReceivingRewards * GetBlocksPerEpoch` blocks
+	for _, v := range consuVals[2:4] {
+		valAddr := sdk.ValAddress(v.ProviderConsAddr)
+		rewards := s.providerApp.GetTestDistributionKeeper().GetValidatorOutstandingRewards(
+			ctx,
+			valAddr,
+		)
+		s.Require().Zero(rewards.Rewards)
+	}
+}
+
 // TestMultiConsumerRewardsDistribution tests the rewards distribution of multiple consumers chains
 func (s *CCVTestSuite) TestMultiConsumerRewardsDistribution() {
 	s.SetupAllCCVChannels()

x/ccv/provider/keeper/distribution.go

@@ -127,6 +127,15 @@ 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)
+
+	// a validator is eligible for rewards if it has been a consumer validator for `NumberOfEpochsToStartReceivingRewards` epochs
+	return (ctx.BlockHeight() - consumerValidatorHeight) >= numberOfBlocksToStartReceivingRewards
+}
+
 // AllocateTokensToConsumerValidators allocates tokens
 // to the given consumer chain's validator set
 func (k Keeper) AllocateTokensToConsumerValidators(
@@ -147,6 +156,11 @@ func (k Keeper) AllocateTokensToConsumerValidators(
 
 	// Allocate tokens by iterating over the consumer validators
 	for _, consumerVal := range k.GetConsumerValSet(ctx, chainID) {
+		// if a validator is not eligible, this means that the other eligible validators would get more rewards
+		if !k.IsEligibleForConsumerRewards(ctx, consumerVal.JoinHeight) {
+			continue
+		}
+
 		consAddr := sdk.ConsAddress(consumerVal.ProviderConsAddr)
 
 		// get the validator tokens fraction using its voting power
@@ -240,6 +254,12 @@ func (k Keeper) GetConsumerRewardsPool(ctx sdk.Context) sdk.Coins {
 func (k Keeper) ComputeConsumerTotalVotingPower(ctx sdk.Context, chainID string) (totalPower int64) {
 	// sum the consumer validators set voting powers
 	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)
+		if !k.IsEligibleForConsumerRewards(ctx, v.JoinHeight) {
+			continue
+		}
+
 		totalPower += v.Power
 	}
 

x/ccv/provider/keeper/distribution_test.go

@@ -23,6 +23,14 @@ func TestComputeConsumerTotalVotingPower(t *testing.T) {
 	keeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
 	defer ctrl.Finish()
 
+	// `ComputeConsumerTotalVotingPower` used in this test retrieves the blocks per epoch, so we need to set this param
+	params := providertypes.DefaultParams()
+	params.BlocksPerEpoch = 1
+	keeper.SetParams(ctx, params)
+
+	// increase the block height so validators are eligible for consumer rewards (see `IsEligibleForConsumerRewards`)
+	ctx = ctx.WithBlockHeight(params.NumberOfEpochsToStartReceivingRewards * params.BlocksPerEpoch)
+
 	createVal := func(power int64) tmtypes.Validator {
 		signer := tmtypes.NewMockPV()
 		val := tmtypes.NewValidator(signer.PrivKey.PubKey(), power)
@@ -270,3 +278,21 @@ 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
+
+	require.False(t, keeper.IsEligibleForConsumerRewards(ctx.WithBlockHeight(numberOfBlocks-1), 0))
+	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))
+}

x/ccv/provider/keeper/params.go

@@ -85,6 +85,14 @@ func (k Keeper) GetBlocksPerEpoch(ctx sdk.Context) int64 {
 	return b
 }
 
+// GetNumberOfEpochsToStartReceivingRewards returns the number of epochs needed by a validator to continuously validate
+// to start receiving rewards
+func (k Keeper) GetNumberOfEpochsToStartReceivingRewards(ctx sdk.Context) int64 {
+	var b int64
+	k.paramSpace.Get(ctx, types.KeyNumberOfEpochsToStartReceivingRewards, &b)
+	return b
+}
+
 // GetParams returns the paramset for the provider module
 func (k Keeper) GetParams(ctx sdk.Context) types.Params {
 	return types.NewParams(
@@ -97,6 +105,7 @@ func (k Keeper) GetParams(ctx sdk.Context) types.Params {
 		k.GetSlashMeterReplenishFraction(ctx),
 		k.GetConsumerRewardDenomRegistrationFee(ctx),
 		k.GetBlocksPerEpoch(ctx),
+		k.GetNumberOfEpochsToStartReceivingRewards(ctx),
 	)
 }
 

x/ccv/provider/keeper/params_test.go

@@ -49,6 +49,7 @@ func TestParams(t *testing.T) {
 			Amount: sdk.NewInt(10000000),
 		},
 		600,
+		24,
 	)
 	providerKeeper.SetParams(ctx, newParams)
 	params = providerKeeper.GetParams(ctx)

x/ccv/provider/keeper/proposal_test.go

@@ -839,7 +839,8 @@ func TestMakeConsumerGenesis(t *testing.T) {
 			Denom:  "stake",
 			Amount: sdk.NewInt(1000000),
 		},
-		BlocksPerEpoch: 600,
+		BlocksPerEpoch:                        600,
+		NumberOfEpochsToStartReceivingRewards: 24,
 	}
 	providerKeeper.SetParams(ctx, moduleParams)
 	defer ctrl.Finish()