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 <marius.poke@posteo.de>

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

Co-authored-by: Marius Poke <marius.poke@posteo.de>

* took into account comments

.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

@@ -251,6 +251,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
@@ -368,6 +371,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

@@ -137,7 +137,16 @@ func (s *CCVTestSuite) TestRewardsDistribution() {
 	}
 	consuValsRewards := consumerValsOutstandingRewardsFunc(s.providerCtx())
 
+	// increase the block height so validators are eligible for consumer rewards (see `IsEligibleForConsumerRewards`)
+	numberOfBlocksToStartReceivingRewards :=
+		providerKeeper.GetNumberOfEpochsToStartReceivingRewards(s.providerCtx()) * providerKeeper.GetBlocksPerEpoch(s.providerCtx())
+
+	for s.providerCtx().BlockHeight() <= numberOfBlocksToStartReceivingRewards {
+		s.providerChain.NextBlock()
+	}
+
 	// Transfer rewards from consumer to provider
+	// and distribute rewards by calling BeginBlockRD,
 	relayAllCommittedPackets(
 		s,
 		s.consumerChain,
@@ -147,15 +156,20 @@ func (s *CCVTestSuite) TestRewardsDistribution() {
 		1,
 	)
 
-	// Check that the consumer rewards allocation are empty since relayAllCommittedPackets calls BeginBlockRD,
-	// which in turns calls AllocateTokens.
+	// Consumer allocations are distributed between the validators and the community pool.
+	// The decimals resulting from the distribution are expected to remain in the consumer allocations.
 	rewardsAlloc := providerKeeper.GetConsumerRewardsAllocation(s.providerCtx(), s.consumerChain.ChainID)
-	s.Require().Empty(rewardsAlloc.Rewards)
+	remainingAlloc := rewardsAlloc.Rewards.AmountOf(rewardsIBCdenom)
+	s.Require().True(remainingAlloc.LTE(math.LegacyOneDec()))
 
-	// Check that the reward pool still holds the coins from the first transfer,
+	// Check that the reward pool still holds the coins from the first transfer
 	// which were never allocated since they were not whitelisted
+	// plus the remaining decimals from the second transfer.
 	rewardCoins = providerBankKeeper.GetAllBalances(s.providerCtx(), rewardPool)
-	s.Require().Equal(rewardCoins.AmountOf(rewardsIBCdenom), providerExpRewardsAmount)
+	s.Require().Equal(
+		math.LegacyNewDecFromInt(rewardCoins.AmountOf(rewardsIBCdenom)),
+		math.LegacyNewDecFromInt(providerExpRewardsAmount).Add(remainingAlloc),
+	)
 
 	// Check that the distribution module account balance is equal to the consumer rewards
 	consuValsRewardsReceived := consumerValsOutstandingRewardsFunc(s.providerCtx()).Sub(consuValsRewards)
@@ -705,9 +719,14 @@ func (s *CCVTestSuite) TestAllocateTokens() {
 
 	totalRewards := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, math.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 +737,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...),
@@ -740,42 +759,42 @@ func (s *CCVTestSuite) TestAllocateTokens() {
 	s.Require().True(totalValsRewards.IsZero())
 
 	// store community pool balance
-	lastCommPool := getDistrAcctBalFn(s.providerCtx())
+	lastCommPool := getDistrAcctBalFn(providerCtx)
 
 	// execute BeginBlock to trigger the token allocation
-	providerKeeper.BeginBlockRD(s.providerCtx())
+	providerKeeper.BeginBlockRD(providerCtx)
 
 	valNum := len(s.providerChain.Vals.Validators)
-	consuNum := len(s.consumerBundles)
+	consNum := len(s.consumerBundles)
 
 	// compute the expected validators token allocation by subtracting the community tax
 	rewardsPerConsumerDec := sdk.NewDecCoinsFromCoins(rewardsPerConsumer...)
-	communityTax, err := distributionKeeper.GetCommunityTax(s.providerCtx())
+	communityTax, err := distributionKeeper.GetCommunityTax(providerCtx)
 	s.Require().NoError(err)
 
-	validatorsExpRewards := rewardsPerConsumerDec.
-		MulDecTruncate(math.LegacyOneDec().Sub(communityTax)).
-		// multiply by the number of consumers since all the validators opted in
-		MulDec(math.LegacyNewDec(int64(consuNum)))
-	perValExpReward := validatorsExpRewards.QuoDec(math.LegacyNewDec(int64(valNum)))
+	rewardsPerChain, _ := rewardsPerConsumerDec.
+		MulDecTruncate(math.LegacyOneDec().Sub(communityTax)).TruncateDecimal()
+	validatorsExpRewardsPerchain := sdk.NewDecCoinsFromCoins(rewardsPerChain...).QuoDec(math.LegacyNewDec(int64(valNum)))
+	// multiply by the number of consumers
+	validatorsExpRewards := validatorsExpRewardsPerchain.MulDec(math.LegacyNewDec(int64(consNum)))
 
 	// 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, err := distributionKeeper.GetValidatorOutstandingRewards(s.providerCtx(), sdk.ValAddress(val.Address))
+		valRewards, err := distributionKeeper.GetValidatorOutstandingRewards(providerCtx, sdk.ValAddress(val.Address))
 		s.Require().NoError(err)
 
 		s.Require().Equal(
 			valRewards.Rewards,
-			lastValOutRewards[sdk.ValAddress(val.Address).String()].Add(perValExpReward...),
+			lastValOutRewards[sdk.ValAddress(val.Address).String()].Add(validatorsExpRewards...),
 		)
-		totalValsRewards = totalValsRewards.Add(valRewards.Rewards...)
 	}
 
-	commPoolExpRewards := lastCommPool.Add(sdk.NewDecCoinsFromCoins(totalRewards...).Sub(validatorsExpRewards)...)
-	currCommPool := getDistrAcctBalFn(s.providerCtx()).Sub(totalValsRewards)
+	// check that the total expected rewards is transferred to the distribution module account
+	allocRemPerConsu := providerKeeper.GetConsumerRewardsAllocation(providerCtx, s.consumerChain.ChainID).Rewards
+	totalRewardsDistributed := sdk.NewDecCoinsFromCoins(totalRewards...).Sub(allocRemPerConsu.MulDec(math.LegacyNewDec(int64(consNum))))
 
-	s.Require().Equal(commPoolExpRewards, currCommPool)
+	s.Require().Equal(lastCommPool.Add(totalRewardsDistributed...), getDistrAcctBalFn(providerCtx))
 }
 
 // TestAllocateTokens is a unit-test for TransferConsumerRewardsToDistributionModule()
@@ -933,7 +952,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()
@@ -979,6 +998,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 +1084,116 @@ 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 := math.LegacyOneDec()
+	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(math.LegacyNewDec(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, err := s.providerApp.GetTestDistributionKeeper().GetValidatorOutstandingRewards(
+			ctx,
+			valAddr,
+		)
+		s.Require().NoError(err)
+		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, err := s.providerApp.GetTestDistributionKeeper().GetValidatorOutstandingRewards(
+			ctx,
+			valAddr,
+		)
+		s.Require().NoError(err)
+		s.Require().Zero(rewards.Rewards)
+	}
+}
+
 // TestMultiConsumerRewardsDistribution tests the rewards distribution of multiple consumers chains
 func (s *CCVTestSuite) TestMultiConsumerRewardsDistribution() {
 	s.SetupAllCCVChannels()