add comments

cosmos · Feb 16, 2024 · e7a18f5 · e7a18f5
1 parent 48071a5
commit e7a18f5
Show file tree

Hide file tree

Showing 5 changed files with 212 additions and 23 deletions.
diff --git a/tests/integration/distribution.go b/tests/integration/distribution.go
@@ -6,6 +6,7 @@ import (
 
 	"cosmossdk.io/math"
 	abci "github.com/cometbft/cometbft/abci/types"
+	"github.com/cometbft/cometbft/libs/bytes"
 	"github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
 	transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
 	clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
@@ -901,3 +902,90 @@ func (s *CCVTestSuite) prepareRewardDist() {
 	blocksToGo := bpdt - blocksSinceLastTrans
 	s.coordinator.CommitNBlocks(s.consumerChain, uint64(blocksToGo))
 }
+
+func (s *CCVTestSuite) TestAllocateTokensToValidator() {
+
+	providerkeepr := s.providerApp.GetProviderKeeper()
+
+	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}},
+	}
+
+	testCases := []struct {
+		name               string
+		votes              []abci.VoteInfo
+		tokens             sdk.DecCoins
+		expCoinTransferred sdk.DecCoins
+	}{
+		{
+			name: "reward tokens are empty",
+		},
+		{
+			name:   "total voting power is zero",
+			tokens: sdk.DecCoins{sdk.NewDecCoin("uatom", math.NewInt(100_000))},
+		},
+		{
+			name:               "expect all tokens to be allocated to a single validator",
+			votes:              []abci.VoteInfo{votes[0]},
+			tokens:             sdk.DecCoins{sdk.NewDecCoin("uatom", math.NewInt(100_000))},
+			expCoinTransferred: sdk.DecCoins{sdk.NewDecCoin("uatom", math.NewInt(100_000))},
+		},
+		{
+			name:               "expect tokens to be allocated evenly between validators",
+			votes:              []abci.VoteInfo{votes[0], votes[1]},
+			tokens:             sdk.DecCoins{sdk.NewDecCoin("uatom", math.NewInt(555_555))},
+			expCoinTransferred: sdk.DecCoins{sdk.NewDecCoin("uatom", math.NewInt(555_555))},
+		},
+	}
+
+	for _, tc := range testCases {
+		s.Run(tc.name, func() {
+
+			ctx, _ := s.providerCtx().CacheContext()
+
+			// opt validators in
+			// for _, v := range tc.optedIn {
+			// 	keeper.SetOptedIn(
+			// 		ctx,
+			// 		chainID,
+			// 		types.NewProviderConsAddress(sdk.ConsAddress(v)),
+			// 		0,
+			// 	)
+			// }
+
+			// allocate tokens
+			res := providerkeepr.AllocateTokensToConsumerValidators(
+				ctx,
+				chainID,
+				tc.votes,
+				tc.tokens,
+			)
+
+			// check that the expect result is returned
+			s.Require().True(tc.expCoinTransferred.IsEqual(res))
+
+			if !tc.expCoinTransferred.Empty() {
+				// rewards are expected to be allocated evenly between validators
+				rewardsPerVal := tc.expCoinTransferred.QuoDec(sdk.NewDec(int64(len(tc.votes))))
+
+				// check that the rewards are allocated to validators as expected
+				for _, v := range tc.votes {
+					rewards := s.providerApp.GetTestDistributionKeeper().GetValidatorOutstandingRewards(
+						ctx,
+						sdk.ValAddress(v.Validator.Address),
+					)
+					s.Require().True(rewardsPerVal.IsEqual(rewards.Rewards))
+				}
+			}
+
+		})
+	}
+}
diff --git a/testutil/integration/debug_test.go b/testutil/integration/debug_test.go
@@ -295,3 +295,7 @@ func TestAllocateTokens(t *testing.T) {
 func TestTransferConsumerRewardsToDistributionModule(t *testing.T) {
 	runCCVTestByName(t, "TransferConsumerRewardsToDistributionModule")
 }
+
+func TestAllocateTokensToValidator(t *testing.T) {
+	runCCVTestByName(t, "TestAllocateTokensToValidator")
+}
diff --git a/x/ccv/provider/ibc_middleware.go b/x/ccv/provider/ibc_middleware.go
@@ -131,7 +131,7 @@ func (im IBCMiddleware) OnRecvPacket(
 			return ack
 		}
 
-		coinDenom := getProviderDenom(data.Denom, packet)
+		coinDenom := GetProviderDenom(data.Denom, packet)
 		coinAmt, _ := math.NewIntFromString(data.Amount)
 
 		// Iterate over the whitelisted consumer denoms
@@ -205,11 +205,11 @@ func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string)
 	return "", false
 }
 
-// getProviderDenom returns the updated given denom according to the given IBC packet
+// GetProviderDenom returns the updated given denom according to the given IBC packet
 // It follows the same logic than the OnRecvPacket method of the IBC transfer module
 // see https://github.com/cosmos/ibc-go/blob/v7.3.2/modules/apps/transfer/keeper/relay.go#L162
-func getProviderDenom(denom string, packet channeltypes.Packet) (providerDenom string) {
-	// If the the prefix denom corresponds to the provider transfer channel info,
+func GetProviderDenom(denom string, packet channeltypes.Packet) (providerDenom string) {
+	// If the the prefix denom corresponds to the packet's source port and channel,
 	// returns the base denom
 	if ibctransfertypes.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), denom) {
 		voucherPrefix := ibctransfertypes.GetDenomPrefix(packet.GetSourcePort(), packet.GetSourceChannel())

diff --git a/x/ccv/provider/ibc_middleware_test.go b/x/ccv/provider/ibc_middleware_test.go
@@ -0,0 +1,86 @@
+package provider_test
+
+import (
+	"testing"
+
+	clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
+	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
+	"github.com/cosmos/interchain-security/v4/x/ccv/provider"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetProviderDenom(t *testing.T) {
+	testCases := []struct {
+		name             string
+		denom            string
+		packet           channeltypes.Packet
+		expProviderDenom string
+	}{
+		{
+			name:  "returns base denom with destination port and channel as prefix",
+			denom: "stake",
+			packet: channeltypes.NewPacket(
+				[]byte{},
+				0,
+				"srcPort",
+				"srcChannel",
+				"dstPort",
+				"dstChannel",
+				clienttypes.NewHeight(1, 1),
+				0,
+			),
+			expProviderDenom: "dstPort/dstChannel/stake",
+		},
+		{
+			name:  "returns base denom if the prefix denom corresponds to the packet's port and channel source",
+			denom: "srcPort/srcChannel/stake",
+			packet: channeltypes.NewPacket(
+				[]byte{},
+				0,
+				"srcPort",
+				"srcChannel",
+				"dstPort",
+				"dstChannel",
+				clienttypes.NewHeight(1, 1),
+				0,
+			),
+			expProviderDenom: "stake",
+		},
+		{
+			name:  "returns prefixed denom updated with destination port and channel as prefix",
+			denom: "dstPort/dstChannel/stake",
+			packet: channeltypes.NewPacket(
+				[]byte{},
+				0,
+				"srcPort",
+				"srcChannel",
+				"dstPort",
+				"dstChannel",
+				clienttypes.NewHeight(1, 1),
+				0,
+			),
+			expProviderDenom: "dstPort/dstChannel/dstPort/dstChannel/stake",
+		},
+	}
+
+	channeltypes.NewPacket(
+		[]byte{},
+		0,
+		"srcPort",
+		"srcChannel",
+		"dstPort",
+		"dstChannel",
+		clienttypes.NewHeight(1, 1),
+		0,
+	)
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			res := provider.GetProviderDenom(
+				tc.denom,
+				tc.packet,
+			)
+
+			require.Equal(t, tc.expProviderDenom, res)
+		})
+	}
+}
diff --git a/x/ccv/provider/keeper/distribution.go b/x/ccv/provider/keeper/distribution.go
@@ -83,7 +83,7 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, totalPreviousPower int64, bonded
 		// note that the rewards transferred are only consumer whitelisted denoms
 		rewardsCollected, err := k.TransferConsumerRewardsToDistributionModule(ctx, consumer.ChainId)
 		if err != nil {
-			panic(err)
+			k.Logger(ctx).Error("fail to transfer consumer rewards to distribution module: %s", err)
 		}
 
 		if rewardsCollected.IsZero() {
@@ -111,8 +111,6 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, totalPreviousPower int64, bonded
 		feeAllocated := k.AllocateTokensToConsumerValidators(
 			ctx,
 			consumer.ChainId,
-			// pass consumer opted-in vals total power
-			k.ComputeConsumerTotalVotingPower(ctx, consumer.ChainId, bondedVotes),
 			bondedVotes,
 			feeMultiplier,
 		)
@@ -127,24 +125,33 @@ 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 consumer rewards pool to validator
-// according to their voting power
+// AllocateTokensToConsumerValidators allocates the given tokens from the
+// from consumer rewards pool to validator according to their voting power
 func (k Keeper) AllocateTokensToConsumerValidators(
 	ctx sdk.Context,
 	chainID string,
-	totalPreviousPower int64,
 	bondedVotes []abci.VoteInfo,
 	tokens sdk.DecCoins,
-) sdk.DecCoins {
-	totalReward := sdk.DecCoins{}
+) (totalReward sdk.DecCoins) {
+	// return early if the tokens are empty
+	if tokens.Empty() {
+		return totalReward
+	}
+
+	// get the consumer total voting power from the votes
+	totalPower := k.ComputeConsumerTotalVotingPower(ctx, chainID, bondedVotes)
+	if totalPower == 0 {
+		return totalReward
+	}
+
 	for _, vote := range bondedVotes {
 		// TODO: should check if validator IsOptIn or continue here
 		consAddr := sdk.ConsAddress(vote.Validator.Address)
 
 		// TODO: Consider micro-slashing for missing votes.
 		//
 		// Ref: https://github.com/cosmos/cosmos-sdk/issues/2525#issuecomment-430838701
-		powerFraction := math.LegacyNewDec(vote.Validator.Power).QuoTruncate(math.LegacyNewDec(totalPreviousPower))
+		powerFraction := math.LegacyNewDec(vote.Validator.Power).QuoTruncate(math.LegacyNewDec(totalPower))
 		tokensFraction := tokens.MulDecTruncate(powerFraction)
 
 		k.distributionKeeper.AllocateTokensToValidator(
@@ -166,6 +173,9 @@ func (k Keeper) TransferConsumerRewardsToDistributionModule(
 ) (sdk.Coins, error) {
 	// Get coins of the consumer rewards allocation
 	allocation := k.GetConsumerRewardsAllocation(ctx, chainID)
+
+	// TODO: check if this condition doesn't break invariant
+	// CanWithdrawInvariant
 	if allocation.Rewards.IsZero() {
 		return sdk.Coins{}, nil
 	}
@@ -216,23 +226,24 @@ func (k Keeper) GetConsumerRewardsPool(ctx sdk.Context) sdk.Coins {
 	)
 }
 
-// ComputeConsumerTotalVotingPower returns the total voting power
-// for the given consumer chain opted-in validators
+// 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 {
-	optedIn := map[string]struct{}{}
+	// optedIn := map[string]struct{}{}
 
-	// create set with opted-in validators
-	for _, v := range k.GetOptedIn(ctx, chainID) {
-		optedIn[v.ProviderAddr.ToSdkConsAddr().String()] = struct{}{}
-	}
+	// // create set with opted-in validators
+	// for _, v := range k.GetOptedIn(ctx, chainID) {
+	// 	optedIn[v.ProviderAddr.ToSdkConsAddr().String()] = struct{}{}
+	// }
 
 	var totalPower int64
 
 	// sum the opted-in validators set voting powers
 	for _, vote := range votes {
-		if _, ok := optedIn[sdk.ConsAddress(vote.Validator.Address).String()]; ok {
-			totalPower += vote.Validator.Power
-		}
+		// if _, ok := optedIn[sdk.ConsAddress(vote.Validator.Address).String()]; ok {
+		// 	totalPower += vote.Validator.Power
+		// }
+		totalPower += vote.Validator.Power
 	}
 
 	return totalPower