imp: update transfer authz implementation to account for multi denom transfers

modules/apps/transfer/types/transfer_authorization.go

@@ -22,6 +22,10 @@ import (
 
 var _ authz.Authorization = (*TransferAuthorization)(nil)
 
+const (
+	allocationNotFound = -1
+)
+
 // maxUint256 is the maximum value for a 256 bit unsigned integer.
 var maxUint256 = new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 256), big.NewInt(1))
 
@@ -38,62 +42,72 @@ func (TransferAuthorization) MsgTypeURL() string {
 }
 
 // Accept implements Authorization.Accept.
-func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (authz.AcceptResponse, error) {
+func (a TransferAuthorization) Accept(goCtx context.Context, msg proto.Message) (authz.AcceptResponse, error) {
 	msgTransfer, ok := msg.(*MsgTransfer)
 	if !ok {
 		return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidType, "type mismatch")
 	}
 
-	// TODO: replace with correct usage in https://github.com/cosmos/ibc-go/issues/5802
-	token := msgTransfer.GetTokens()[0]
+	index := getAllocationIndex(*msgTransfer, a.Allocations)
+	if index == allocationNotFound {
+		return authz.AcceptResponse{}, errorsmod.Wrapf(ibcerrors.ErrNotFound, "requested port and channel allocation does not exist")
+	}
 
-	for index, allocation := range a.Allocations {
-		if !(allocation.SourceChannel == msgTransfer.SourceChannel && allocation.SourcePort == msgTransfer.SourcePort) {
-			continue
-		}
+	allocation := a.Allocations[index]
+	ctx := sdk.UnwrapSDKContext(goCtx)
 
-		if !isAllowedAddress(sdk.UnwrapSDKContext(ctx), msgTransfer.Receiver, allocation.AllowList) {
-			return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "not allowed receiver address for transfer")
-		}
+	if !isAllowedAddress(ctx, msgTransfer.Receiver, allocation.AllowList) {
+		return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "not allowed receiver address for transfer")
+	}
 
-		err := validateMemo(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowedPacketData)
-		if err != nil {
-			return authz.AcceptResponse{}, err
-		}
+	err := validateMemo(ctx, msgTransfer.Memo, allocation.AllowedPacketData)
+	if err != nil {
+		return authz.AcceptResponse{}, err
+	}
 
+	// bool flag to see if we have updated any of the allocations
+	allocationModified := false
+
+	// update spend limit for each token in the MsgTransfer
+	for _, token := range msgTransfer.GetTokens() {
 		// If the spend limit is set to the MaxUint256 sentinel value, do not subtract the amount from the spend limit.
+		// if there is no unlimited spend, then we need to subtract the amount from the spend limit to get the limit left
 		if allocation.SpendLimit.AmountOf(token.Denom).Equal(UnboundedSpendLimit()) {
-			return authz.AcceptResponse{Accept: true, Delete: false, Updated: nil}, nil
+			continue
 		}
 
-		limitLeft, isNegative := allocation.SpendLimit.SafeSub(token)
+		limitLeft, isNegative := a.Allocations[index].SpendLimit.SafeSub(token)
 		if isNegative {
-			return authz.AcceptResponse{}, errorsmod.Wrapf(ibcerrors.ErrInsufficientFunds, "requested amount is more than spend limit")
+			return authz.AcceptResponse{}, errorsmod.Wrapf(ibcerrors.ErrInsufficientFunds, "requested amount of token %s is more than spend limit", token.Denom)
 		}
 
-		if limitLeft.IsZero() {
-			a.Allocations = append(a.Allocations[:index], a.Allocations[index+1:]...)
-			if len(a.Allocations) == 0 {
-				return authz.AcceptResponse{Accept: true, Delete: true}, nil
-			}
-			return authz.AcceptResponse{Accept: true, Delete: false, Updated: &TransferAuthorization{
-				Allocations: a.Allocations,
-			}}, nil
-		}
+		allocationModified = true
+
 		a.Allocations[index] = Allocation{
 			SourcePort:        allocation.SourcePort,
 			SourceChannel:     allocation.SourceChannel,
 			SpendLimit:        limitLeft,
 			AllowList:         allocation.AllowList,
 			AllowedPacketData: allocation.AllowedPacketData,
 		}
+	}
+
+	// if the spend limit is zero of the associated allocation then we delete it.
+	if a.Allocations[index].SpendLimit.IsZero() {
+		a.Allocations = append(a.Allocations[:index], a.Allocations[index+1:]...)
+	}
+
+	if len(a.Allocations) == 0 {
+		return authz.AcceptResponse{Accept: true, Delete: true}, nil
+	}
 
-		return authz.AcceptResponse{Accept: true, Delete: false, Updated: &TransferAuthorization{
-			Allocations: a.Allocations,
-		}}, nil
+	if !allocationModified {
+		return authz.AcceptResponse{Accept: true, Delete: false, Updated: nil}, nil
 	}
 
-	return authz.AcceptResponse{}, errorsmod.Wrapf(ibcerrors.ErrNotFound, "requested port and channel allocation does not exist")
+	return authz.AcceptResponse{Accept: true, Delete: false, Updated: &TransferAuthorization{
+		Allocations: a.Allocations,
+	}}, nil
 }
 
 // ValidateBasic implements Authorization.ValidateBasic.
@@ -212,3 +226,13 @@ func validateMemo(ctx sdk.Context, memo string, allowedPacketDataList []string)
 func UnboundedSpendLimit() sdkmath.Int {
 	return sdkmath.NewIntFromBigInt(maxUint256)
 }
+
+// getAllocationIndex ranges through a set of allocations, and returns the index of the allocation if found. If not, returns -1.
+func getAllocationIndex(msg MsgTransfer, allocations []Allocation) int {
+	for index, allocation := range allocations {
+		if allocation.SourceChannel == msg.SourceChannel && allocation.SourcePort == msg.SourcePort {
+			return index
+		}
+	}
+	return allocationNotFound
+}

modules/apps/transfer/types/transfer_authorization_test.go

@@ -67,15 +67,32 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
 			},
 		},
 		{
-			"success: with multiple allocations",
+			"success: with multiple allocations and multidenom transfer",
 			func() {
-				alloc := types.Allocation{
+				coins := sdk.NewCoins(
+					ibctesting.TestCoin,
+					sdk.NewCoin("atom", sdkmath.NewInt(100)),
+					sdk.NewCoin("osmo", sdkmath.NewInt(100)),
+				)
+
+				allocation := types.Allocation{
 					SourcePort:    ibctesting.MockPort,
 					SourceChannel: "channel-9",
-					SpendLimit:    ibctesting.TestCoins,
+					SpendLimit:    coins,
 				}
 
-				transferAuthz.Allocations = append(transferAuthz.Allocations, alloc)
+				transferAuthz.Allocations = append(transferAuthz.Allocations, allocation)
+
+				msgTransfer = types.NewMsgTransfer(
+					ibctesting.MockPort,
+					"channel-9",
+					coins,
+					suite.chainA.SenderAccount.GetAddress().String(),
+					ibctesting.TestAccAddress,
+					suite.chainB.GetTimeoutHeight(),
+					0,
+					"",
+				)
 			},
 			func(res authz.AcceptResponse, err error) {
 				suite.Require().NoError(err)
@@ -86,7 +103,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
 				updatedAuthz, ok := res.Updated.(*types.TransferAuthorization)
 				suite.Require().True(ok)
 
-				// assert spent spendlimit is removed from the list
+				// assert spent spendlimits are removed from the list
 				suite.Require().Len(updatedAuthz.Allocations, 1)
 			},
 		},