took into account comments

x/ccv/provider/ibc_middleware.go

@@ -206,6 +206,7 @@ func (im IBCMiddleware) OnRecvPacket(
 				"denom", coinDenom,
 				"error", err.Error(),
 			)
+			return ack
 		}
 
 		alloc.Rewards = alloc.Rewards.Add(
@@ -223,6 +224,7 @@ func (im IBCMiddleware) OnRecvPacket(
 				"denom", coinDenom,
 				"error", err.Error(),
 			)
+			return ack
 		}
 
 		logger.Info(

x/ccv/provider/keeper/distribution.go

@@ -81,38 +81,28 @@ func (k Keeper) GetAllowlistedRewardDenoms(ctx sdk.Context, consumerId string) (
 	return denoms.Denoms, nil
 }
 
-// SetAllowlistedRewardDenom sets the allowlisted reward denom for the given consumer id.
-func (k Keeper) SetAllowlistedRewardDenom(ctx sdk.Context, consumerId string, denom string) error {
-	currentDenoms, err := k.GetAllowlistedRewardDenoms(ctx, consumerId)
-	if err != nil {
-		return err
-	}
+// SetAllowlistedRewardDenoms sets the allowlisted reward denoms for the given consumer id.
+func (k Keeper) SetAllowlistedRewardDenoms(ctx sdk.Context, consumerId string, rewardDenoms []string) error {
 	store := ctx.KVStore(k.storeKey)
-	updatedDenoms := types.AllowlistedRewardDenoms{Denoms: append(currentDenoms, denom)}
-	bz, err := updatedDenoms.Marshal()
+	allowlistedUpdatedDenoms := types.AllowlistedRewardDenoms{Denoms: rewardDenoms}
+	bz, err := allowlistedUpdatedDenoms.Marshal()
 	if err != nil {
 		return err
 	}
 	store.Set(types.ConsumerIdToAllowlistedRewardDenomKey(consumerId), bz)
 	return nil
 }
 
-// DeleteAllowlistedRewardDenom deletes the allowlisted reward denom for the given consumer id.
-func (k Keeper) DeleteAllowlistedRewardDenom(ctx sdk.Context, consumerId string) {
+// DeleteAllowlistedRewardDenoms deletes the allowlisted reward denom for the given consumer id.
+func (k Keeper) DeleteAllowlistedRewardDenoms(ctx sdk.Context, consumerId string) {
 	store := ctx.KVStore(k.storeKey)
 	store.Delete(types.ConsumerIdToAllowlistedRewardDenomKey(consumerId))
 }
 
 // UpdateAllowlistedRewardDenoms updates the allowlisted reward denoms for this consumer chain with the provided `rewardDenoms`
 func (k Keeper) UpdateAllowlistedRewardDenoms(ctx sdk.Context, consumerId string, rewardDenoms []string) error {
-	k.DeleteAllowlistedRewardDenom(ctx, consumerId)
-	for _, denom := range rewardDenoms {
-		err := k.SetAllowlistedRewardDenom(ctx, consumerId, denom)
-		if err != nil {
-			return err
-		}
-	}
-	return nil
+	k.DeleteAllowlistedRewardDenoms(ctx, consumerId)
+	return k.SetAllowlistedRewardDenoms(ctx, consumerId, rewardDenoms)
 }
 
 // GetConsumerRewardsAllocationByDenom returns the consumer rewards allocation for the given consumer id and denom
@@ -251,7 +241,7 @@ func (k Keeper) AllocateConsumerRewards(ctx sdk.Context, consumerId string, allo
 		"consumerId", consumerId,
 		"chainId", chainId,
 		"total-rewards", consumerRewards.String(),
-		"sent-to-distribution", validatorsRewardsTrunc.String(),
+		"sent-to-validators", validatorsRewardsTrunc.String(),
 		"sent-to-CP", remainingRewards.String(),
 	)
 
@@ -289,6 +279,7 @@ func (k Keeper) AllocateTokens(ctx sdk.Context) {
 				"fail to retrieve the allowlisted reward denoms for consumer chain",
 				"consumer id", consumerId,
 				"error", err.Error())
+			continue
 		}
 
 		allAllowlistedDenoms := append(allConsumerRewardDenoms, consumerAllowlistedRewardDenoms...)
@@ -304,8 +295,9 @@ func (k Keeper) AllocateTokens(ctx sdk.Context) {
 					"denom", denom,
 					"error", err.Error(),
 				)
+				continue
 			}
-			allocatedRewards, err := k.AllocateConsumerRewards(cachedCtx, consumerId, consumerRewards)
+			remainingRewardDec, err := k.AllocateConsumerRewards(cachedCtx, consumerId, consumerRewards)
 			if err != nil {
 				k.Logger(ctx).Error(
 					"fail to allocate rewards for consumer chain",
@@ -314,7 +306,7 @@ func (k Keeper) AllocateTokens(ctx sdk.Context) {
 				)
 				continue
 			}
-			err = k.SetConsumerRewardsAllocationByDenom(cachedCtx, consumerId, denom, allocatedRewards)
+			err = k.SetConsumerRewardsAllocationByDenom(cachedCtx, consumerId, denom, remainingRewardDec)
 			if err != nil {
 				k.Logger(ctx).Error(
 					"fail to set rewards for consumer chain",

x/ccv/provider/keeper/distribution_test.go

@@ -379,7 +379,7 @@ func TestHandleSetConsumerCommissionRate(t *testing.T) {
 }
 
 // TestAllowlistedRewardDenoms tests the `GetAllowlistedRewardDenoms`, `SetAllowlistedRewardDenom`,
-// `UpdateAllowlistedRewardDenoms` and `DeleteAllowlistedRewardDenom` methods.
+// `UpdateAllowlistedRewardDenoms` and `DeleteAllowlistedRewardDenoms` methods.
 func TestAllowlistedRewardDenoms(t *testing.T) {
 	providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
 	defer ctrl.Finish()
@@ -390,9 +390,7 @@ func TestAllowlistedRewardDenoms(t *testing.T) {
 	require.NoError(t, err)
 
 	denomsToSet := []string{"denom1", "denom2", "denom3"}
-	providerKeeper.SetAllowlistedRewardDenom(ctx, consumerId, "denom1")
-	providerKeeper.SetAllowlistedRewardDenom(ctx, consumerId, "denom2")
-	providerKeeper.SetAllowlistedRewardDenom(ctx, consumerId, "denom3")
+	providerKeeper.SetAllowlistedRewardDenoms(ctx, consumerId, denomsToSet)
 
 	denoms, err = providerKeeper.GetAllowlistedRewardDenoms(ctx, consumerId)
 	require.Equal(t, denomsToSet, denoms)
@@ -405,7 +403,7 @@ func TestAllowlistedRewardDenoms(t *testing.T) {
 	require.Equal(t, updatedDenoms, denoms)
 	require.NoError(t, err)
 
-	providerKeeper.DeleteAllowlistedRewardDenom(ctx, consumerId)
+	providerKeeper.DeleteAllowlistedRewardDenoms(ctx, consumerId)
 	denoms, err = providerKeeper.GetAllowlistedRewardDenoms(ctx, consumerId)
 	require.Empty(t, denoms)
 	require.NoError(t, err)

x/ccv/provider/keeper/msg_server.go

@@ -19,8 +19,6 @@ import (
 	ccvtypes "github.com/cosmos/interchain-security/v6/x/ccv/types"
 )
 
-const MaxAllowlistedRewardDenomsPerChain = 3
-
 type msgServer struct {
 	*Keeper
 }
@@ -417,9 +415,9 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon
 	}
 
 	if msg.AllowlistedRewardDenoms != nil {
-		if len(msg.AllowlistedRewardDenoms.Denoms) > MaxAllowlistedRewardDenomsPerChain {
+		if len(msg.AllowlistedRewardDenoms.Denoms) > types.MaxAllowlistedRewardDenomsPerChain {
 			return &resp, errorsmod.Wrapf(types.ErrInvalidAllowlistedRewardDenoms,
-				fmt.Sprintf("a consumer chain cannot allowlist more than %d reward denoms", MaxAllowlistedRewardDenomsPerChain))
+				fmt.Sprintf("a consumer chain cannot allowlist more than %d reward denoms", types.MaxAllowlistedRewardDenomsPerChain))
 		}
 
 		err := k.UpdateAllowlistedRewardDenoms(ctx, consumerId, msg.AllowlistedRewardDenoms.Denoms)
@@ -605,9 +603,9 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
 	}
 
 	if msg.AllowlistedRewardDenoms != nil {
-		if len(msg.AllowlistedRewardDenoms.Denoms) > MaxAllowlistedRewardDenomsPerChain {
+		if len(msg.AllowlistedRewardDenoms.Denoms) > types.MaxAllowlistedRewardDenomsPerChain {
 			return &resp, errorsmod.Wrapf(types.ErrInvalidAllowlistedRewardDenoms,
-				fmt.Sprintf("a consumer chain cannot allowlist more than %d reward denoms", MaxAllowlistedRewardDenomsPerChain))
+				fmt.Sprintf("a consumer chain cannot allowlist more than %d reward denoms", types.MaxAllowlistedRewardDenomsPerChain))
 		}
 
 		if err := k.UpdateAllowlistedRewardDenoms(ctx, consumerId, msg.AllowlistedRewardDenoms.Denoms); err != nil {

x/ccv/provider/types/keys.go

@@ -33,6 +33,10 @@ const (
 	// This address receives rewards from consumer chains
 	ConsumerRewardsPool = "consumer_rewards_pool"
 
+	// MaxAllowlistedRewardDenomsPerChain corresponds to the maximum number of reward denoms
+	// a consumer chain can allowlist
+	MaxAllowlistedRewardDenomsPerChain = 3
+
 	// Names for the store keys.
 	// Used for storing the byte prefixes in the constant map.
 	// See getKeyPrefixes().