diff --git a/tests/integration/distribution.go b/tests/integration/distribution.go index 9f85b29f28..d3111a39a5 100644 --- a/tests/integration/distribution.go +++ b/tests/integration/distribution.go @@ -14,7 +14,6 @@ import ( distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" abci "github.com/cometbft/cometbft/abci/types" - "github.com/cometbft/cometbft/libs/bytes" icstestingutils "github.com/cosmos/interchain-security/v4/testutil/integration" consumerkeeper "github.com/cosmos/interchain-security/v4/x/ccv/consumer/keeper" @@ -51,6 +50,8 @@ func (s *CCVTestSuite) TestRewardsDistribution() { providerAccountKeeper := s.providerApp.GetTestAccountKeeper() consumerBankKeeper := s.consumerApp.GetTestBankKeeper() providerBankKeeper := s.providerApp.GetTestBankKeeper() + providerKeeper := s.providerApp.GetProviderKeeper() + providerDistributionKeeper := s.providerApp.GetTestDistributionKeeper() // send coins to the fee pool which is used for reward distribution consumerFeePoolAddr := consumerAccountKeeper.GetModuleAccount(s.consumerCtx(), authtypes.FeeCollectorName).GetAddress() @@ -79,7 +80,6 @@ func (s *CCVTestSuite) TestRewardsDistribution() { s.Require().Equal(providerExpectedRewards.AmountOf(sdk.DefaultBondDenom), providerTokens.AmountOf(sdk.DefaultBondDenom)) // send the reward to provider chain after 2 blocks - s.consumerChain.NextBlock() providerTokens = consumerBankKeeper.GetAllBalances(s.consumerCtx(), providerRedistributeAddr) s.Require().Equal(0, len(providerTokens)) @@ -91,52 +91,85 @@ func (s *CCVTestSuite) TestRewardsDistribution() { rewardPool := providerAccountKeeper.GetModuleAccount(s.providerCtx(), providertypes.ConsumerRewardsPool).GetAddress() rewardCoins := providerBankKeeper.GetAllBalances(s.providerCtx(), rewardPool) - ibcCoinIndex := -1 - for i, coin := range rewardCoins { + // Check that the reward pool contains a coin with an IBC denom + rewardsIBCdenom := "" + for _, coin := range rewardCoins { if strings.HasPrefix(coin.Denom, "ibc") { - ibcCoinIndex = i + rewardsIBCdenom = coin.Denom } } - - // Check that we found an ibc denom in the reward pool - s.Require().Greater(ibcCoinIndex, -1) + s.Require().NotZero(rewardsIBCdenom) // Check that the coins got into the ConsumerRewardsPool - s.Require().Equal(rewardCoins[ibcCoinIndex].Amount, (providerExpectedRewards[0].Amount)) + providerExpRewardsAmount := providerExpectedRewards.AmountOf(sdk.DefaultBondDenom) + s.Require().Equal(rewardCoins.AmountOf(rewardsIBCdenom), providerExpRewardsAmount) // Advance a block and check that the coins are still in the ConsumerRewardsPool s.providerChain.NextBlock() rewardCoins = providerBankKeeper.GetAllBalances(s.providerCtx(), rewardPool) - s.Require().Equal(rewardCoins[ibcCoinIndex].Amount, (providerExpectedRewards[0].Amount)) + s.Require().Equal(rewardCoins.AmountOf(rewardsIBCdenom), providerExpRewardsAmount) - // Set the consumer reward denom. This would be done by a governance proposal in prod - s.providerApp.GetProviderKeeper().SetConsumerRewardDenom(s.providerCtx(), rewardCoins[ibcCoinIndex].Denom) + // Set the consumer reward denom. This would be done by a governance proposal in prod. + providerKeeper.SetConsumerRewardDenom(s.providerCtx(), rewardsIBCdenom) // Refill the consumer fee pool - err = consumerBankKeeper.SendCoinsFromAccountToModule(s.consumerCtx(), s.consumerChain.SenderAccount.GetAddress(), authtypes.FeeCollectorName, fees) + err = consumerBankKeeper.SendCoinsFromAccountToModule( + s.consumerCtx(), + s.consumerChain.SenderAccount.GetAddress(), + authtypes.FeeCollectorName, + fees, + ) s.Require().NoError(err) - // pass two blocks + // Pass two blocks s.consumerChain.NextBlock() s.consumerChain.NextBlock() - // transfer rewards from consumer to provider - relayAllCommittedPackets(s, s.consumerChain, s.transferPath, transfertypes.PortID, s.transferPath.EndpointA.ChannelID, 1) + // Save the consumer validators total outstanding rewards on the provider + consumerValsOutstandingRewardsFunc := func(ctx sdk.Context) sdk.DecCoins { + totalRewards := sdk.DecCoins{} + for _, v := range providerKeeper.GetConsumerValSet(ctx, s.consumerChain.ChainID) { + val, ok := s.providerApp.GetTestStakingKeeper().GetValidatorByConsAddr(ctx, sdk.ConsAddress(v.ProviderConsAddr)) + s.Require().True(ok) + valReward := providerDistributionKeeper.GetValidatorOutstandingRewards(ctx, val.GetOperator()) + totalRewards = totalRewards.Add(valReward.Rewards...) + } + return totalRewards + } + consuValsRewards := consumerValsOutstandingRewardsFunc(s.providerCtx()) + + // Save community pool balance + communityPool := providerDistributionKeeper.GetFeePoolCommunityCoins(s.providerCtx()) + + // Transfer rewards from consumer to provider + relayAllCommittedPackets( + s, + s.consumerChain, + s.transferPath, + transfertypes.PortID, + s.transferPath.EndpointA.ChannelID, + 1, + ) - // check that the consumer rewards allocation are empty since relayAllCommittedPackets call BeginBlock - rewardsAlloc := s.providerApp.GetProviderKeeper().GetConsumerRewardsAllocation(s.providerCtx(), s.consumerChain.ChainID) + // Check that the consumer rewards allocation are empty since relayAllCommittedPackets calls BeginBlockRD, + // which in turns calls AllocateTokens. + rewardsAlloc := providerKeeper.GetConsumerRewardsAllocation(s.providerCtx(), s.consumerChain.ChainID) s.Require().Empty(rewardsAlloc.Rewards) - // Check that the reward pool still has the first coins transferred that were never allocated + // Check that the reward pool still holds the coins from the first transfer, + // which were never allocated since they were not whitelisted rewardCoins = providerBankKeeper.GetAllBalances(s.providerCtx(), rewardPool) - s.Require().Equal(rewardCoins[ibcCoinIndex].Amount, (providerExpectedRewards[0].Amount)) - - // check that the fee pool has the expected amount of coins - // Note that all rewards are allocated to the community pool since - // BeginBlock is called without the validators' votes in ibctesting. - // See NextBlock() in https://github.com/cosmos/ibc-go/blob/release/v7.3.x/testing/chain.go#L281 - communityCoins := s.providerApp.GetTestDistributionKeeper().GetFeePoolCommunityCoins(s.providerCtx()) - s.Require().Equal(communityCoins[ibcCoinIndex].Amount, (sdk.NewDecCoinFromCoin(providerExpectedRewards[0]).Amount)) + s.Require().Equal(rewardCoins.AmountOf(rewardsIBCdenom), providerExpRewardsAmount) + + // Check that summing the rewards received by the consumer validators and the community pool + // is equal to the expected provider rewards + consuValsRewardsReceived := consumerValsOutstandingRewardsFunc(s.providerCtx()).Sub(consuValsRewards) + communityPoolDelta := providerDistributionKeeper.GetFeePoolCommunityCoins(s.providerCtx()).Sub(communityPool) + + s.Require().Equal( + sdk.NewDecFromInt(providerExpRewardsAmount), + consuValsRewardsReceived.AmountOf(rewardsIBCdenom).Add(communityPoolDelta.AmountOf(rewardsIBCdenom)), + ) } // TestSendRewardsRetries tests that failed reward transmissions are retried every BlocksPerDistributionTransmission blocks @@ -906,19 +939,11 @@ func (s *CCVTestSuite) TestAllocateTokensToValidator() { distributionKeeper := s.providerApp.GetTestDistributionKeeper() bankKeeper := s.providerApp.GetTestBankKeeper() - chainID := "consumer" - validators := []bytes.HexBytes{ - s.providerChain.Vals.Validators[0].Address, - s.providerChain.Vals.Validators[1].Address, - } - votes := []abci.VoteInfo{ - {Validator: abci.Validator{Address: validators[0], Power: 1}}, - {Validator: abci.Validator{Address: validators[1], Power: 1}}, - } + chainID := s.consumerChain.ChainID testCases := []struct { name string - votes []abci.VoteInfo + consuValLen int tokens sdk.DecCoins rate sdk.Dec expAllocated sdk.DecCoins @@ -930,21 +955,21 @@ func (s *CCVTestSuite) TestAllocateTokensToValidator() { expAllocated: nil, }, { - name: "total voting power is zero", + name: "consumer valset is empty - total voting power is zero", tokens: sdk.DecCoins{sdk.NewDecCoin(sdk.DefaultBondDenom, math.NewInt(100_000))}, rate: sdk.ZeroDec(), expAllocated: nil, }, { name: "expect all tokens to be allocated to a single validator", - votes: []abci.VoteInfo{votes[0]}, + consuValLen: 1, tokens: sdk.DecCoins{sdk.NewDecCoin(sdk.DefaultBondDenom, math.NewInt(999))}, rate: sdk.NewDecWithPrec(5, 1), expAllocated: sdk.DecCoins{sdk.NewDecCoin(sdk.DefaultBondDenom, math.NewInt(999))}, }, { name: "expect tokens to be allocated evenly between validators", - votes: []abci.VoteInfo{votes[0], votes[1]}, + consuValLen: 2, 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))}, @@ -953,27 +978,29 @@ func (s *CCVTestSuite) TestAllocateTokensToValidator() { for _, tc := range testCases { s.Run(tc.name, func() { - // set the same consumer commission rate for all validators - for _, v := range s.providerChain.Vals.Validators { - provAddr := providertypes.NewProviderConsAddress(sdk.ConsAddress(v.Address)) + ctx, _ := s.providerCtx().CacheContext() + // change the consumer valset + consuVals := providerKeeper.GetConsumerValSet(ctx, chainID) + providerKeeper.DeleteConsumerValSet(ctx, chainID) + providerKeeper.SetConsumerValSet(ctx, chainID, consuVals[0:tc.consuValLen]) + 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)) providerKeeper.SetConsumerCommissionRate( - s.providerCtx(), + ctx, chainID, provAddr, tc.rate, ) } - // TODO: opt validators in and verify - // that rewards are only allocated to them - ctx, _ := s.providerCtx().CacheContext() - // allocate tokens res := providerKeeper.AllocateTokensToConsumerValidators( ctx, chainID, - tc.votes, tc.tokens, ) @@ -982,11 +1009,11 @@ func (s *CCVTestSuite) TestAllocateTokensToValidator() { if !tc.expAllocated.Empty() { // rewards are expected to be allocated evenly between validators - rewardsPerVal := tc.expAllocated.QuoDec(sdk.NewDec(int64(len(tc.votes)))) + rewardsPerVal := tc.expAllocated.QuoDec(sdk.NewDec(int64(len(consuVals)))) // check that the rewards are allocated to validators - for _, v := range tc.votes { - valAddr := sdk.ValAddress(v.Validator.Address) + for _, v := range consuVals { + valAddr := sdk.ValAddress(v.ProviderConsAddr) rewards := s.providerApp.GetTestDistributionKeeper().GetValidatorOutstandingRewards( ctx, valAddr, @@ -1019,8 +1046,8 @@ func (s *CCVTestSuite) TestAllocateTokensToValidator() { s.Require().Equal(withdrawnCoins, bankKeeper.GetAllBalances(ctx, sdk.AccAddress(valAddr))) } } else { - for _, v := range tc.votes { - valAddr := sdk.ValAddress(v.Validator.Address) + for _, v := range consuVals { + valAddr := sdk.ValAddress(v.ProviderConsAddr) rewards := s.providerApp.GetTestDistributionKeeper().GetValidatorOutstandingRewards( ctx, valAddr, diff --git a/x/ccv/provider/keeper/distribution.go b/x/ccv/provider/keeper/distribution.go index aa7195cf4b..cd7755ac3e 100644 --- a/x/ccv/provider/keeper/distribution.go +++ b/x/ccv/provider/keeper/distribution.go @@ -17,16 +17,11 @@ import ( // BeginBlockRD executes BeginBlock logic for the Reward Distribution sub-protocol. func (k Keeper) BeginBlockRD(ctx sdk.Context, req abci.RequestBeginBlock) { - // determine the total power signing the block - var previousTotalPower int64 - for _, voteInfo := range req.LastCommitInfo.GetVotes() { - previousTotalPower += voteInfo.Validator.Power - } // TODO this is Tendermint-dependent // ref https://github.com/cosmos/cosmos-sdk/issues/3095 if ctx.BlockHeight() > 1 { - k.AllocateTokens(ctx, previousTotalPower, req.LastCommitInfo.GetVotes()) + k.AllocateTokens(ctx) } } @@ -75,7 +70,7 @@ func (k Keeper) GetAllConsumerRewardDenoms(ctx sdk.Context) (consumerRewardDenom // AllocateTokens performs rewards distribution to the community pool and validators // based on the Partial Set Security distribution specification. -func (k Keeper) AllocateTokens(ctx sdk.Context, totalPreviousPower int64, bondedVotes []abci.VoteInfo) { +func (k Keeper) AllocateTokens(ctx sdk.Context) { // return if there is no coins in the consumer rewards pool if k.GetConsumerRewardsPool(ctx).IsZero() { return @@ -95,6 +90,9 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, totalPreviousPower int64, bonded continue } + // note that it's possible that no rewards are collected even though the + // reward pool isn't empty. This can happen if the reward pool holds some tokens + // of non-whitelisted denominations. if rewardsCollected.IsZero() { continue } @@ -104,13 +102,13 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, totalPreviousPower int64, bonded // temporary workaround to keep CanWithdrawInvariant happy // general discussions here: https://github.com/cosmos/cosmos-sdk/issues/2906#issuecomment-441867634 feePool := k.distributionKeeper.GetFeePool(ctx) - if k.ComputeConsumerTotalVotingPower(ctx, consumer.ChainId, bondedVotes) == 0 { + if k.ComputeConsumerTotalVotingPower(ctx, consumer.ChainId) == 0 { feePool.CommunityPool = feePool.CommunityPool.Add(rewardsCollectedDec...) k.distributionKeeper.SetFeePool(ctx, feePool) return } - // Calculate the reward allocations + // calculate the reward allocations remaining := rewardsCollectedDec communityTax := k.distributionKeeper.GetCommunityTax(ctx) voteMultiplier := math.LegacyOneDec().Sub(communityTax) @@ -120,7 +118,6 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, totalPreviousPower int64, bonded feeAllocated := k.AllocateTokensToConsumerValidators( ctx, consumer.ChainId, - bondedVotes, feeMultiplier, ) remaining = remaining.Sub(feeAllocated) @@ -131,15 +128,11 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, totalPreviousPower int64, bonded } } -// TODO: allocate tokens to validators that opted-in and for long enough e.g. 1000 blocks -// once the opt-in logic is integrated QueueVSCPackets() -// -// AllocateTokensToConsumerValidators allocates the given tokens from the -// from consumer rewards pool to validator according to their voting power +// AllocateTokensToConsumerValidators allocates tokens +// to the given consumer chain's validator set func (k Keeper) AllocateTokensToConsumerValidators( ctx sdk.Context, chainID string, - bondedVotes []abci.VoteInfo, tokens sdk.DecCoins, ) (allocated sdk.DecCoins) { // return early if the tokens are empty @@ -147,17 +140,18 @@ func (k Keeper) AllocateTokensToConsumerValidators( return allocated } - // get the consumer total voting power from the votes - totalPower := k.ComputeConsumerTotalVotingPower(ctx, chainID, bondedVotes) + // get the total voting power of the consumer valset + totalPower := k.ComputeConsumerTotalVotingPower(ctx, chainID) if totalPower == 0 { return allocated } - for _, vote := range bondedVotes { - // TODO: should check if validator IsOptIn or continue here - consAddr := sdk.ConsAddress(vote.Validator.Address) + // Allocate tokens by iterating over the consumer validators + for _, consumerVal := range k.GetConsumerValSet(ctx, chainID) { + consAddr := sdk.ConsAddress(consumerVal.ProviderConsAddr) - powerFraction := math.LegacyNewDec(vote.Validator.Power).QuoTruncate(math.LegacyNewDec(totalPower)) + // get the validator tokens fraction using its voting power + powerFraction := math.LegacyNewDec(consumerVal.Power).QuoTruncate(math.LegacyNewDec(totalPower)) tokensFraction := tokens.MulDecTruncate(powerFraction) // get the validator type struct for the consensus address @@ -242,21 +236,15 @@ func (k Keeper) GetConsumerRewardsPool(ctx sdk.Context) sdk.Coins { ) } -// ComputeConsumerTotalVotingPower returns the total voting power for a given consumer chain -// by summing its opted-in validators votes -func (k Keeper) ComputeConsumerTotalVotingPower(ctx sdk.Context, chainID string, votes []abci.VoteInfo) int64 { - // TODO: create a optedIn set from the OptedIn validators - // and sum their validator power - var totalPower int64 - +// ComputeConsumerTotalVotingPower returns the validator set total voting power +// for the given consumer chain +func (k Keeper) ComputeConsumerTotalVotingPower(ctx sdk.Context, chainID string) (totalPower int64) { // sum the opted-in validators set voting powers - for _, vote := range votes { - // TODO: check that val is in the optedIn set - - totalPower += vote.Validator.Power + for _, v := range k.GetConsumerValSet(ctx, chainID) { + totalPower += v.Power } - return totalPower + return } // IdentifyConsumerChainIDFromIBCPacket checks if the packet destination matches a registered consumer chain. diff --git a/x/ccv/provider/keeper/distribution_test.go b/x/ccv/provider/keeper/distribution_test.go index d12b2c910a..1e37b1112e 100644 --- a/x/ccv/provider/keeper/distribution_test.go +++ b/x/ccv/provider/keeper/distribution_test.go @@ -12,7 +12,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" - abci "github.com/cometbft/cometbft/abci/types" tmtypes "github.com/cometbft/cometbft/types" testkeeper "github.com/cosmos/interchain-security/v4/testutil/keeper" @@ -30,44 +29,37 @@ func TestComputeConsumerTotalVotingPower(t *testing.T) { } chainID := "consumer" - validatorsVotes := make([]abci.VoteInfo, 5) - expTotalPower := int64(0) - // create validators, opt them in and use them - // to create block votes + // verify that the total power returned is equal to zero + // when the consumer doesn't exist or has no validators. + require.Zero(t, keeper.ComputeConsumerTotalVotingPower( + ctx, + chainID, + )) + + // set 5 validators to the consumer chain for i := 0; i < 5; i++ { val := createVal(int64(i)) - keeper.SetOptedIn( + keeper.SetConsumerValidator( ctx, chainID, - types.OptedInValidator{ - ProviderAddr: val.Address, - BlockHeight: ctx.BlockHeight(), - Power: val.VotingPower, - PublicKey: val.PubKey.Bytes(), - }, - ) - - validatorsVotes = append( - validatorsVotes, - abci.VoteInfo{ - Validator: abci.Validator{ - Address: val.Address, - Power: val.VotingPower, - }, + types.ConsumerValidator{ + ProviderConsAddr: val.Address, + Power: val.VotingPower, }, ) expTotalPower += val.VotingPower } + // compute the total power of opted-in validators res := keeper.ComputeConsumerTotalVotingPower( ctx, chainID, - validatorsVotes, ) + // check the total power returned require.Equal(t, expTotalPower, res) }