imp: allow memo strings instead of keys for transfer authorizations (#…

…6268)

* imp: allow memo strings instead of keys for transfer authorizations

* add changelog

* handle error from compact

* return error

* improve test

* not enforce that memo strings of allowed packet data must be JSON-encoded strings

* use slices contains to check if memo is allowed

* Update modules/apps/transfer/types/transfer_authorization.go

Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com>

* Update modules/apps/transfer/types/transfer_authorization.go

Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com>

* lint

---------

Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com>
(cherry picked from commit 0a22b7a)

# Conflicts:
#	docs/docs/02-apps/01-transfer/08-authorizations.md
#	modules/apps/transfer/types/authz.pb.go
#	modules/apps/transfer/types/keys.go
#	modules/apps/transfer/types/transfer_authorization.go
#	modules/apps/transfer/types/transfer_authorization_test.go
#	proto/ibc/applications/transfer/v1/authz.proto

CHANGELOG.md

@@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (apps/27-interchain-accounts) [\#6147](https://github.com/cosmos/ibc-go/pull/6147) Emit an event signalling that the host submodule is disabled.
 * (testing) [\#6180](https://github.com/cosmos/ibc-go/pull/6180) Add version to tm abci headers in ibctesting.
 * (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 memo strings instead of JSON keys in `AllowedPacketData` of transfer authorization.
 
 ### Features
 

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

@@ -0,0 +1,58 @@
+---
+title: Authorizations
+sidebar_label: Authorizations
+sidebar_position: 8
+slug: /apps/transfer/authorizations
+---
+# `TransferAuthorization`
+
+`TransferAuthorization` implements the `Authorization` interface for `ibc.applications.transfer.v1.MsgTransfer`. It allows a granter to grant a grantee the privilege to submit `MsgTransfer` on its behalf. Please see the [Cosmos SDK docs](https://docs.cosmos.network/v0.47/modules/authz) for more details on granting privileges via the `x/authz` module.
+
+More specifically, the granter allows the grantee to transfer funds that belong to the granter over a specified channel.
+
+For the specified channel, the granter must be able to specify a spend limit of a specific denomination they wish to allow the grantee to be able to transfer.
+
+The granter may be able to specify the list of addresses that they allow to receive funds. If empty, then all addresses are allowed.
+
+It takes:
+
+- a `SourcePort` and a `SourceChannel` which together comprise the unique transfer channel identifier over which authorized funds can be transferred.
+
+- a `SpendLimit` that specifies the maximum amount of tokens the grantee can transfer. The `SpendLimit` is updated as the tokens are transferred, unless the sentinel value of the maximum value for a 256-bit unsigned integer (i.e. 2^256 - 1) is used for the amount, in which case the `SpendLimit` will not be updated (please be aware that using this sentinel value will grant the grantee the privilege to transfer **all** the tokens of a given denomination available at the granter's account). The helper function `UnboundedSpendLimit` in the `types` package of the `transfer` module provides the sentinel value that can be used. This `SpendLimit` may also be updated to increase or decrease the limit as the granter wishes.
+
+- 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 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:
+
+- the spend limit is nil
+- the denomination of the spend limit is an invalid coin type
+- the source port ID is invalid
+- the source channel ID is invalid
+- there are duplicate entries in the `AllowList`
+- the `memo` field is not allowed by `AllowedPacketData`
+
+Below is the `TransferAuthorization` message:
+
+```go
+func NewTransferAuthorization(allocations ...Allocation) *TransferAuthorization {
+  return &TransferAuthorization{
+    Allocations: allocations,
+  }
+}
+
+type Allocation struct {
+  // the port on which the packet will be sent
+  SourcePort string 
+  // the channel by which the packet will be sent
+  SourceChannel string 
+  // spend limitation on the channel
+  SpendLimit sdk.Coins  
+  // allow list of receivers, an empty allow list permits any receiver address
+  AllowList []string 
+  // allow list of 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,6 +30,12 @@ const (
 	// DenomPrefix is the prefix used for internal SDK coin representation.
 	DenomPrefix = "ibc"
 
+<<<<<<< HEAD
+=======
+	// AllowAllPacketDataKeys holds the string key that allows all memo strings in authz transfer messages
+	AllowAllPacketDataKeys = "*"
+
+>>>>>>> 0a22b7a2 (imp: allow memo strings instead of keys for transfer authorizations (#6268))
 	KeyTotalEscrowPrefix = "totalEscrowForDenom"
 )
 

modules/apps/transfer/types/transfer_authorization.go

@@ -1,7 +1,14 @@
 package types
 
 import (
+<<<<<<< HEAD
 	"math/big"
+=======
+	"context"
+	"math/big"
+	"slices"
+	"strings"
+>>>>>>> 0a22b7a2 (imp: allow memo strings instead of keys for transfer authorizations (#6268))
 
 	sdkmath "cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -140,6 +147,40 @@ func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) b
 	return false
 }
 
+<<<<<<< HEAD
+=======
+// validateMemo returns a nil error indicating if the memo is valid for transfer.
+func validateMemo(ctx sdk.Context, memo string, allowedMemos []string) error {
+	// if the allow list is empty, then the memo must be an empty string
+	if len(allowedMemos) == 0 {
+		if len(strings.TrimSpace(memo)) != 0 {
+			return errorsmod.Wrapf(ErrInvalidAuthorization, "memo must be empty because allowed packet data in allocation is empty")
+		}
+
+		return nil
+	}
+
+	// if allowedPacketDataList has only 1 element and it equals AllowAllPacketDataKeys
+	// then accept all the memo strings
+	if len(allowedMemos) == 1 && allowedMemos[0] == AllowAllPacketDataKeys {
+		return nil
+	}
+
+	gasCostPerIteration := ctx.KVGasConfig().IterNextCostFlat
+	isMemoAllowed := slices.ContainsFunc(allowedMemos, func(allowedMemo string) bool {
+		ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization")
+
+		return strings.TrimSpace(memo) == strings.TrimSpace(allowedMemo)
+	})
+
+	if !isMemoAllowed {
+		return errorsmod.Wrapf(ErrInvalidAuthorization, "not allowed memo: %s", memo)
+	}
+
+	return nil
+}
+
+>>>>>>> 0a22b7a2 (imp: allow memo strings instead of keys for transfer authorizations (#6268))
 // UnboundedSpendLimit returns the sentinel value that can be used
 // as the amount for a denomination's spend limit for which spend limit updating
 // should be disabled. Please note that using this sentinel value means that a grantee

modules/apps/transfer/types/transfer_authorization_test.go

@@ -1,13 +1,28 @@
 package types_test
 
 import (
+<<<<<<< HEAD
+=======
+	"fmt"
+
+	sdkmath "cosmossdk.io/math"
+
+>>>>>>> 0a22b7a2 (imp: allow memo strings instead of keys for transfer authorizations (#6268))
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/authz"
 	"github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
 	ibctesting "github.com/cosmos/ibc-go/v7/testing"
 	"github.com/cosmos/ibc-go/v7/testing/mock"
 )
 
+<<<<<<< HEAD
+=======
+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}}`
+)
+
+>>>>>>> 0a22b7a2 (imp: allow memo strings instead of keys for transfer authorizations (#6268))
 func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
 	var (
 		msgTransfer   types.MsgTransfer
@@ -99,6 +114,76 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
 			},
 		},
 		{
+<<<<<<< HEAD
+=======
+			"success: empty AllowedPacketData and empty memo",
+			func() {
+				allowedList := []string{}
+				transferAuthz.Allocations[0].AllowedPacketData = allowedList
+			},
+			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: AllowedPacketData allows any packet",
+			func() {
+				allowedList := []string{"*"}
+				transferAuthz.Allocations[0].AllowedPacketData = allowedList
+				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: transfer memo allowed",
+			func() {
+				allowedList := []string{testMemo1, testMemo2}
+				transferAuthz.Allocations[0].AllowedPacketData = allowedList
+				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)
+			},
+		},
+		{
+			"empty AllowedPacketData but not empty memo",
+			func() {
+				allowedList := []string{}
+				transferAuthz.Allocations[0].AllowedPacketData = allowedList
+				msgTransfer.Memo = testMemo1
+			},
+			func(res authz.AcceptResponse, err error) {
+				suite.Require().Error(err)
+			},
+		},
+		{
+			"memo not allowed",
+			func() {
+				allowedList := []string{testMemo1}
+				transferAuthz.Allocations[0].AllowedPacketData = allowedList
+				msgTransfer.Memo = testMemo2
+			},
+			func(res authz.AcceptResponse, err error) {
+				suite.Require().Error(err)
+				suite.Require().ErrorContains(err, fmt.Sprintf("not allowed memo: %s", testMemo2))
+			},
+		},
+		{
+>>>>>>> 0a22b7a2 (imp: allow memo strings instead of keys for transfer authorizations (#6268))
 			"test multiple coins does not overspend",
 			func() {
 				transferAuthz.Allocations[0].SpendLimit = transferAuthz.Allocations[0].SpendLimit.Add(
@@ -236,6 +321,13 @@ func (suite *TypesTestSuite) TestTransferAuthorizationValidateBasic() {
 			},
 			true,
 		},
+		{
+			"success: wildcard allowed packet data",
+			func() {
+				transferAuthz.Allocations[0].AllowedPacketData = []string{"*"}
+			},
+			true,
+		},
 		{
 			"empty allocations",
 			func() {

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

@@ -19,6 +19,12 @@ 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;
+<<<<<<< HEAD
+=======
+  // allow list of memo strings, an empty list prohibits all memo strings;
+  // a list only with "*" permits any memo string
+  repeated string allowed_packet_data = 5;
+>>>>>>> 0a22b7a2 (imp: allow memo strings instead of keys for transfer authorizations (#6268))
 }
 
 // TransferAuthorization allows the grantee to spend up to spend_limit coins from