init commit

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

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

@@ -212,6 +212,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
@@ -329,6 +332,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

@@ -934,7 +934,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()
@@ -1060,6 +1060,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. 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

@@ -147,6 +147,13 @@ 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 {
+			continue
+		}
+
 		consAddr := sdk.ConsAddress(consumerVal.ProviderConsAddr)
 
 		// get the validator tokens fraction using its voting power
@@ -240,6 +247,13 @@ func (k Keeper) GetConsumerRewardsPool(ctx sdk.Context) sdk.Coins {
 func (k Keeper) ComputeConsumerTotalVotingPower(ctx sdk.Context, chainID string) (totalPower int64) {
 	// sum the opted-in 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)
+		numberOfBlocksToStartReceivingRewards := k.GetNumberOfEpochsToStartReceivingRewards(ctx) * k.GetBlocksPerEpoch(ctx)
+		if ctx.BlockHeight() >= numberOfBlocksToStartReceivingRewards && ctx.BlockHeight()-v.Height < numberOfBlocksToStartReceivingRewards {
+			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)

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

@@ -789,7 +789,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 correctly 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

@@ -71,6 +71,23 @@ func (k Keeper) IsConsumerValidator(ctx sdk.Context, chainID string, providerAdd
 	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,
@@ -151,10 +168,18 @@ func (k Keeper) CreateConsumerValidator(ctx sdk.Context, chainID string, validat
 		}
 	}
 
+	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
 }
 

x/ccv/provider/migrations/migrator.go

@@ -8,6 +8,7 @@ import (
 	v3 "github.com/cosmos/interchain-security/v4/x/ccv/provider/migrations/v3"
 	v4 "github.com/cosmos/interchain-security/v4/x/ccv/provider/migrations/v4"
 	v5 "github.com/cosmos/interchain-security/v4/x/ccv/provider/migrations/v5"
+	v6 "github.com/cosmos/interchain-security/v4/x/ccv/provider/migrations/v6"
 )
 
 // Migrator is a struct for handling in-place store migrations.
@@ -47,3 +48,10 @@ func (m Migrator) Migrate4to5(ctx sdktypes.Context) error {
 	v5.MigrateTopNForRegisteredChains(ctx, m.providerKeeper)
 	return nil
 }
+
+// Migrate5to6 migrates x/ccvprovider state from consensus version 5 to 6.
+// The migration consists of a provider chain param addition.
+func (m Migrator) Migrate5to6(ctx sdktypes.Context) error {
+	v6.MigrateParams(ctx, m.paramSpace)
+	return nil
+}

x/ccv/provider/migrations/v6/migration_test.go

@@ -0,0 +1,27 @@
+package v4
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	testutil "github.com/cosmos/interchain-security/v4/testutil/keeper"
+	providertypes "github.com/cosmos/interchain-security/v4/x/ccv/provider/types"
+)
+
+func TestMigrateParams(t *testing.T) {
+	inMemParams := testutil.NewInMemKeeperParams(t)
+	_, ctx, ctrl, _ := testutil.GetProviderKeeperAndCtx(t, inMemParams)
+	defer ctrl.Finish()
+
+	// initially number of epochs param does not exist
+	require.False(t, inMemParams.ParamsSubspace.Has(ctx, providertypes.KeyNumberOfEpochsToStartReceivingRewards))
+
+	MigrateParams(ctx, *inMemParams.ParamsSubspace)
+
+	// after migration, number of epochs epoch param should exist and be equal to default
+	require.True(t, inMemParams.ParamsSubspace.Has(ctx, providertypes.KeyNumberOfEpochsToStartReceivingRewards))
+	var numberOfEpochsParam int64
+	inMemParams.ParamsSubspace.Get(ctx, providertypes.KeyNumberOfEpochsToStartReceivingRewards, &numberOfEpochsParam)
+	require.Equal(t, providertypes.DefaultNumberOfEpochsToStartReceivingRewards, numberOfEpochsParam)
+}