imp: allow memo strings instead of keys for transfer authorizations

CHANGELOG.md

@@ -59,6 +59,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * (apps/27-interchain-accounts) [\#5533](https://github.com/cosmos/ibc-go/pull/5533) ICA host sets the host connection ID on `OnChanOpenTry`, so that ICA controller implementations are not obliged to set the value on `OnChanOpenInit` if they are not able.
 * (apps/27-interchain-accounts, apps/tranfer, apps/29-fee) [\#6253](https://github.com/cosmos/ibc-go/pull/6253) Allow channel handshake to succeed if fee middleware is wired up on one side, but not the other.
+* (apps/transfer) [\#6268](https://github.com/cosmos/ibc-go/pull/6268) Use JSON-encoded memo strings instead of JSON keys in `AllowedPacketData` of transfer authorization.
 
 ### Features
 

docs/docs/02-apps/01-transfer/08-authorizations.md

@@ -22,7 +22,7 @@ It takes:
 
 - an `AllowList` list that specifies the list of addresses that are allowed to receive funds. If this list is empty, then all addresses are allowed to receive funds from the `TransferAuthorization`.
 
-- an `AllowedPacketData` list that specifies the list of memo packet data keys that are allowed to send the packet. If this list is empty, then only an empty memo is allowed (a `memo` field with non-empty content will be denied). If this list includes a single element equal to `"*"`, then any content in `memo` field will be allowed.
+- an `AllowedPacketData` list that specifies the list of (JSON-encoded) memo strings that are allowed to be included in the memo field of the packet. If this list is empty, then only an empty memo is allowed (a `memo` field with non-empty content will be denied). If this list includes a single element equal to `"*"`, then any content in `memo` field will be allowed.
 
 Setting a `TransferAuthorization` is expected to fail if:
 
@@ -51,9 +51,8 @@ type Allocation struct {
   SpendLimit sdk.Coins  
   // allow list of receivers, an empty allow list permits any receiver address
   AllowList []string 
-  // allow list of packet data keys, an empty list prohibits all packet data keys;
-  // a list only with "*" permits any packet data key
+  // allow list of (JSON-encoded) memo strings, an empty list prohibits all memo strings;
+  // a list only with "*" permits any memo string
   AllowedPacketData []string 
 }
-
 ```

modules/apps/transfer/types/keys.go

@@ -30,7 +30,7 @@ const (
 	// DenomPrefix is the prefix used for internal SDK coin representation.
 	DenomPrefix = "ibc"
 
-	// AllowAllPacketDataKeys holds the string key that allows all packet data keys in authz transfer messages
+	// AllowAllPacketDataKeys holds the string key that allows all memo strings in authz transfer messages
 	AllowAllPacketDataKeys = "*"
 
 	KeyTotalEscrowPrefix = "totalEscrowForDenom"

modules/apps/transfer/types/transfer_authorization.go

@@ -1,10 +1,10 @@
 package types
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"math/big"
-	"sort"
 	"strings"
 
 	"github.com/cosmos/gogoproto/proto"
@@ -131,6 +131,16 @@
 			}
 			found[allocation.AllowList[i]] = true
 		}
+
+		if len(allocation.AllowedPacketData) > 0 && allocation.AllowedPacketData[0] != AllowAllPacketDataKeys {
+			jsonObject := make(map[string]interface{})
+			for _, elem := range allocation.AllowedPacketData {
+				err := json.Unmarshal([]byte(elem), &jsonObject)
+				if err != nil {
+					return errorsmod.Wrapf(ErrInvalidAuthorization, "allowed packet data contains a non JSON-encoded string: %s", elem)
+				}
+			}
+		}
 	}
 
 	return nil
@@ -166,39 +176,30 @@
 	}
 
 	// if allowedPacketDataList has only 1 element and it equals AllowAllPacketDataKeys
-	// then accept all the packet data keys
+	// then accept all the memo strings
 	if len(allowedPacketDataList) == 1 && allowedPacketDataList[0] == AllowAllPacketDataKeys {
 		return nil
 	}
 
-	jsonObject := make(map[string]interface{})
-	err := json.Unmarshal([]byte(memo), &jsonObject)
-	if err != nil {
-		return err
-	}
-
 	gasCostPerIteration := ctx.KVGasConfig().IterNextCostFlat
 
-	for _, key := range allowedPacketDataList {
+	for _, allowedMemo := range allowedPacketDataList {
 		ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization")
 
-		_, ok := jsonObject[key]
-		if ok {
-			delete(jsonObject, key)
-		}
-	}
+		dst := &bytes.Buffer{}
+		json.Compact(dst, []byte(allowedMemo))
+		compactAllowedMemo := dst.String()
 
-	keys := make([]string, 0, len(jsonObject))
-	for k := range jsonObject {
-		keys = append(keys, k)
-	}
-	sort.Strings(keys)
+		dst = &bytes.Buffer{}
+		json.Compact(dst, []byte(memo))
+		compactMemo := dst.String()
 
-	if len(jsonObject) != 0 {
-		return errorsmod.Wrapf(ErrInvalidAuthorization, "not allowed packet data keys: %s", keys)
+		if compactMemo == compactAllowedMemo {
+			return nil
+		}
 	}
 
-	return nil
+	return errorsmod.Wrapf(ErrInvalidAuthorization, "not allowed memo: %s", memo)
 }
 
 // UnboundedSpendLimit returns the sentinel value that can be used

modules/apps/transfer/types/transfer_authorization_test.go

@@ -1,6 +1,9 @@
 package types_test
 
 import (
+	"bytes"
+	"encoding/json"
+
 	sdkmath "cosmossdk.io/math"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -11,9 +14,33 @@
 	"github.com/cosmos/ibc-go/v8/testing/mock"
 )
 
-const testMemo = `{"wasm":{"contract":"osmo1c3ljch9dfw5kf52nfwpxd2zmj2ese7agnx0p9tenkrryasrle5sqf3ftpg","msg":{"osmosis_swap":{"output_denom":"uosmo","slippage":{"twap":{"slippage_percentage":"20","window_seconds":10}},"receiver":"feeabs/feeabs1efd63aw40lxf3n4mhf7dzhjkr453axurwrhrrw","on_failed_delivery":"do_nothing"}}}}`
+const (
+	testMemo1 = `{
+"wasm": {
+	"contract": "osmo1c3ljch9dfw5kf52nfwpxd2zmj2ese7agnx0p9tenkrryasrle5sqf3ftpg",
+	"msg": {
+		"osmosis_swap": {
+			"output_denom": "uosmo",
+			"slippage": {
+				"twap": {
+					"slippage_percentage": "20",
+					"window_seconds": 10
+				}
+			},
+			"receiver": "feeabs/feeabs1efd63aw40lxf3n4mhf7dzhjkr453axurwrhrrw",
+			"on_failed_delivery": "do_nothing"
+		}
+	}
+}
+}`
+	testMemo2 = `{"forward":{"channel":"channel-11","port":"transfer","receiver":"stars1twfv52yxcyykx2lcvgl42svw46hsm5dd4ww6xy","retries":2,"timeout":1712146014542131200}}`
+)
 
 func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
+	dst := &bytes.Buffer{}
+	json.Compact(dst, []byte(testMemo1))
+	compactTestMemo1 := dst.String()
+
 	var (
 		msgTransfer   types.MsgTransfer
 		transferAuthz types.TransferAuthorization
@@ -122,7 +149,22 @@
 			func() {
 				allowedList := []string{"*"}
 				transferAuthz.Allocations[0].AllowedPacketData = allowedList
-				msgTransfer.Memo = testMemo
+				msgTransfer.Memo = testMemo1
+			},
+			func(res authz.AcceptResponse, err error) {
+				suite.Require().NoError(err)
+
+				suite.Require().True(res.Accept)
+				suite.Require().True(res.Delete)
+				suite.Require().Nil(res.Updated)
+			},
+		},
+		{
+			"success: non-compact transfer memo allowed",
+			func() {
+				allowedList := []string{compactTestMemo1, testMemo2}
+				transferAuthz.Allocations[0].AllowedPacketData = allowedList
+				msgTransfer.Memo = testMemo1
 			},
 			func(res authz.AcceptResponse, err error) {
 				suite.Require().NoError(err)
@@ -133,11 +175,11 @@
 			},
 		},
 		{
-			"success: transfer memo allowed",
+			"success: compact transfer memo allowed",
 			func() {
-				allowedList := []string{"wasm", "forward"}
+				allowedList := []string{testMemo1, testMemo2}
 				transferAuthz.Allocations[0].AllowedPacketData = allowedList
-				msgTransfer.Memo = testMemo
+				msgTransfer.Memo = compactTestMemo1
 			},
 			func(res authz.AcceptResponse, err error) {
 				suite.Require().NoError(err)
@@ -152,7 +194,7 @@
 			func() {
 				allowedList := []string{}
 				transferAuthz.Allocations[0].AllowedPacketData = allowedList
-				msgTransfer.Memo = testMemo
+				msgTransfer.Memo = testMemo1
 			},
 			func(res authz.AcceptResponse, err error) {
 				suite.Require().Error(err)
@@ -161,13 +203,13 @@
 		{
 			"memo not allowed",
 			func() {
-				allowedList := []string{"forward"}
+				allowedList := []string{testMemo1}
 				transferAuthz.Allocations[0].AllowedPacketData = allowedList
-				msgTransfer.Memo = testMemo
+				msgTransfer.Memo = testMemo2
 			},
 			func(res authz.AcceptResponse, err error) {
 				suite.Require().Error(err)
-				suite.Require().ErrorContains(err, "not allowed packet data keys: [wasm]")
+				suite.Require().ErrorContains(err, "not allowed memo: {\"forward\":{\"channel\":\"channel-11\",\"port\":\"transfer\",\"receiver\":\"stars1twfv52yxcyykx2lcvgl42svw46hsm5dd4ww6xy\",\"retries\":2,\"timeout\":1712146014542131200}}")
 			},
 		},
 		{
@@ -310,6 +352,20 @@
 			},
 			true,
 		},
+		{
+			"success: wildcard allowed packet data",
+			func() {
+				transferAuthz.Allocations[0].AllowedPacketData = []string{"*"}
+			},
+			true,
+		},
+		{
+			"success: JSON-encoded allowed packet data",
+			func() {
+				transferAuthz.Allocations[0].AllowedPacketData = []string{testMemo1}
+			},
+			true,
+		},
 		{
 			"empty allocations",
 			func() {
@@ -373,6 +429,21 @@
 			},
 			expPass: false,
 		},
+		{
+			name: "non JSON-encoded allowed packet data",
+			malleate: func() {
+				allocation := types.Allocation{
+					SourcePort:        mock.PortID,
+					SourceChannel:     transferAuthz.Allocations[0].SourceChannel,
+					SpendLimit:        sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100))),
+					AllowList:         []string{ibctesting.TestAccAddress},
+					AllowedPacketData: []string{"forward"},
+				}
+
+				transferAuthz.Allocations = append(transferAuthz.Allocations, allocation)
+			},
+			expPass: false,
+		},
 	}
 
 	for _, tc := range testCases {

proto/ibc/applications/transfer/v1/authz.proto

@@ -19,8 +19,8 @@ message Allocation {
       [(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];
   // allow list of receivers, an empty allow list permits any receiver address
   repeated string allow_list = 4;
-  // allow list of packet data keys, an empty list prohibits all packet data keys;
-  // a list only with "*" permits any packet data key
+  // allow list of (JSON-encoded) memo strings, an empty list prohibits all memo strings;
+  // a list only with "*" permits any memo string
   repeated string allowed_packet_data = 5;
 }