feat!: only distribute rewards to validators that have been validating a consumer chain for some time

.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. ([\#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. ([\#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 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

@@ -935,7 +935,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()
@@ -1061,6 +1061,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].Height = 1
+	consuVals[1].Height = 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].Height = 2
+	consuVals[3].Height = 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,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(
@@ -147,6 +164,10 @@ func (k Keeper) AllocateTokensToConsumerValidators(
 
 	// Allocate tokens by iterating over the consumer validators
 	for _, consumerVal := range k.GetConsumerValSet(ctx, chainID) {
+		if !k.IsEligibleForConsumerRewards(ctx, consumerVal.Height) {
+			continue
+		}
+
 		consAddr := sdk.ConsAddress(consumerVal.ProviderConsAddr)
 
 		// get the validator tokens fraction using its voting power
@@ -240,6 +261,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.Height) {
+			continue
+		}
+
 		totalPower += v.Power
 	}
 

x/ccv/provider/keeper/distribution_test.go

@@ -23,6 +23,11 @@ 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)
+
 	createVal := func(power int64) tmtypes.Validator {
 		signer := tmtypes.NewMockPV()
 		val := tmtypes.NewValidator(signer.PrivKey.PubKey(), power)
@@ -270,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))
+}

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

x/ccv/provider/keeper/relay_test.go

@@ -87,6 +87,55 @@ func TestQueueVSCPackets(t *testing.T) {
 	}
 }
 
+// TestQueueVSCPacketsDoesNotResetConsumerValidatorsHeights checks that the heights of consumer validators are not
+// getting incorrectly updated
+func TestQueueVSCPacketsDoesNotResetConsumerValidatorsHeights(t *testing.T) {
+	providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
+	defer ctrl.Finish()
+
+	chainHeight := int64(987654321)
+	ctx = ctx.WithBlockHeight(chainHeight)
+	providerKeeper.SetParams(ctx, providertypes.DefaultParams())
+
+	// mock 2 bonded validators
+	valA := createStakingValidator(ctx, mocks, 1, 1)
+	valAConsAddr, _ := valA.GetConsAddr()
+	valAPubKey, _ := valA.TmConsPublicKey()
+	mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(ctx, valAConsAddr).Return(valA, true).AnyTimes()
+	valB := createStakingValidator(ctx, mocks, 2, 2)
+	valBConsAddr, _ := valB.GetConsAddr()
+	mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(ctx, valBConsAddr).Return(valB, true).AnyTimes()
+	mocks.MockStakingKeeper.EXPECT().GetLastValidators(ctx).Return([]stakingtypes.Validator{valA, valB}).AnyTimes()
+
+	// set a consumer client, so we have a consumer chain (i.e., `k.GetAllConsumerChains(ctx)` is non empty)
+	providerKeeper.SetConsumerClientId(ctx, "chainID", "clientID")
+
+	// opt in validator A and set as a consumer validator
+	providerKeeper.SetOptedIn(ctx, "chainID", providertypes.NewProviderConsAddress(valAConsAddr))
+	consumerValidatorA := types.ConsumerValidator{
+		ProviderConsAddr:  valAConsAddr,
+		Power:             1,
+		ConsumerPublicKey: &valAPubKey,
+		Height:            123456789,
+	}
+	providerKeeper.SetConsumerValidator(ctx, "chainID", consumerValidatorA)
+
+	// Opt in validator B. Note that validator B is not a consumer validator and hence would become a consumer
+	// validator for the first time after the `QueueVSCPackets` call.
+	providerKeeper.SetOptedIn(ctx, "chainID", providertypes.NewProviderConsAddress(valBConsAddr))
+
+	providerKeeper.QueueVSCPackets(ctx)
+
+	// the height of consumer validator A should not be modified because A was already a consumer validator
+	cv, _ := providerKeeper.GetConsumerValidator(ctx, "chainID", providertypes.NewProviderConsAddress(valAConsAddr))
+	require.Equal(t, consumerValidatorA.Height, cv.Height, "the consumer validator's height was erroneously modified")
+
+	// the height of consumer validator B is set to be the same as the one of the current chain height because this
+	// consumer validator becomes a consumer validator for the first time (i.e., was not a consumer validator in the previous epoch)
+	cv, _ = providerKeeper.GetConsumerValidator(ctx, "chainID", providertypes.NewProviderConsAddress(valBConsAddr))
+	require.Equal(t, chainHeight, cv.Height, "the consumer validator's height was not correctly set")
+}
+
 // TestOnRecvVSCMaturedPacket tests the OnRecvVSCMaturedPacket method of the keeper.
 //
 // Note: Handling logic itself is not tested here.

x/ccv/provider/keeper/validator_set_update.go

@@ -72,6 +72,23 @@
 	return store.Get(types.ConsumerValidatorKey(chainID, providerAddr.ToSdkConsAddr())) != nil
 }
 
+// GetConsumerValidator returns the consumer validator with `providerAddr` if it exists for chain `chainID`
+func (k Keeper) GetConsumerValidator(ctx sdk.Context, chainID string, providerAddr types.ProviderConsAddress) (types.ConsumerValidator, bool) {
+	store := ctx.KVStore(k.storeKey)
+	marshalledConsumerValidator := store.Get(types.ConsumerValidatorKey(chainID, providerAddr.ToSdkConsAddr()))
+
+	if marshalledConsumerValidator == nil {
+		return types.ConsumerValidator{}, false
+	}
+
+	var validator types.ConsumerValidator
+	if err := validator.Unmarshal(marshalledConsumerValidator); err != nil {
+		panic(fmt.Errorf("failed to unmarshal ConsumerValidator: %w", err))
+	}
+
+	return validator, true
+}
+
 // GetConsumerValSet returns all the consumer validators for chain `chainID`
 func (k Keeper) GetConsumerValSet(
 	ctx sdk.Context,
@@ -152,10 +169,18 @@
 		}
 	}
 
+	height := ctx.BlockHeight()
+	if v, found := k.GetConsumerValidator(ctx, chainID, types.ProviderConsAddress{Address: consAddr}); found {
+		// if validator was already a consumer validator, then do not update the height set the first time
+		// the validator became a consumer validator
+		height = v.Height
+	}
+
 	return types.ConsumerValidator{
 		ProviderConsAddr:  consAddr,
 		Power:             power,
 		ConsumerPublicKey: &consumerPublicKey,
+		Height:            height,
 	}, nil
 }