From 134df145ef5330b9441ef0e79cfd55ec2d5dd118 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Thu, 30 Nov 2023 19:24:12 +0700 Subject: [PATCH 01/31] add keys allow packet --- modules/apps/transfer/types/keys.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/apps/transfer/types/keys.go b/modules/apps/transfer/types/keys.go index 3ee859e2239..025efda5c9d 100644 --- a/modules/apps/transfer/types/keys.go +++ b/modules/apps/transfer/types/keys.go @@ -30,6 +30,9 @@ 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 = "*" + KeyTotalEscrowPrefix = "totalEscrowForDenom" ParamsKey = "params" From 254b0b5b839ea20ee0b458de99117114cad1cdc3 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Thu, 30 Nov 2023 19:28:48 +0700 Subject: [PATCH 02/31] add allow packet data list --- modules/apps/transfer/types/authz.pb.go | 113 +++++++++++++----- .../ibc/applications/transfer/v1/authz.proto | 3 + 2 files changed, 89 insertions(+), 27 deletions(-) diff --git a/modules/apps/transfer/types/authz.pb.go b/modules/apps/transfer/types/authz.pb.go index e05cdc401e7..2520694fc88 100644 --- a/modules/apps/transfer/types/authz.pb.go +++ b/modules/apps/transfer/types/authz.pb.go @@ -36,6 +36,9 @@ type Allocation struct { SpendLimit github_com_cosmos_cosmos_sdk_types.Coins `protobuf:"bytes,3,rep,name=spend_limit,json=spendLimit,proto3,castrepeated=github.com/cosmos/cosmos-sdk/types.Coins" json:"spend_limit"` // allow list of receivers, an empty allow list permits any receiver address AllowList []string `protobuf:"bytes,4,rep,name=allow_list,json=allowList,proto3" json:"allow_list,omitempty"` + // allow list of packet data keys, an empty list. + // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key + AllowPacketDataList []string `protobuf:"bytes,5,rep,name=allow_packet_data_list,json=allowPacketDataList,proto3" json:"allow_packet_data_list,omitempty"` } func (m *Allocation) Reset() { *m = Allocation{} } @@ -99,6 +102,13 @@ func (m *Allocation) GetAllowList() []string { return nil } +func (m *Allocation) GetAllowPacketDataList() []string { + if m != nil { + return m.AllowPacketDataList + } + return nil +} + // TransferAuthorization allows the grantee to spend up to spend_limit coins from // the granter's account for ibc transfer on a specific channel type TransferAuthorization struct { @@ -156,33 +166,35 @@ func init() { } var fileDescriptor_b1a28b55d17325aa = []byte{ - // 408 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x52, 0x3d, 0x8f, 0xd4, 0x30, - 0x10, 0x5d, 0xb3, 0x27, 0xa4, 0x75, 0x04, 0x45, 0x04, 0x52, 0xee, 0x04, 0xd9, 0xd5, 0x4a, 0xa0, - 0x34, 0x6b, 0x13, 0x28, 0x40, 0x74, 0xb7, 0xd7, 0x5e, 0x71, 0x44, 0x54, 0x34, 0x91, 0xe3, 0x35, - 0x89, 0x85, 0x93, 0x89, 0x62, 0x27, 0x88, 0xfb, 0x15, 0xf0, 0x37, 0xa8, 0xf9, 0x11, 0x27, 0xaa, - 0x2b, 0xa9, 0xf8, 0xd8, 0xfd, 0x23, 0x28, 0xb6, 0x17, 0x16, 0x21, 0x5d, 0x95, 0xe4, 0xe5, 0xcd, - 0xcc, 0x7b, 0x6f, 0x06, 0x27, 0xb2, 0xe0, 0x94, 0xb5, 0xad, 0x92, 0x9c, 0x19, 0x09, 0x8d, 0xa6, - 0xa6, 0x63, 0x8d, 0x7e, 0x2b, 0x3a, 0x3a, 0xa4, 0x94, 0xf5, 0xa6, 0xba, 0x24, 0x6d, 0x07, 0x06, - 0xc2, 0x07, 0xb2, 0xe0, 0xe4, 0x90, 0x49, 0xf6, 0x4c, 0x32, 0xa4, 0x27, 0xc7, 0x1c, 0x74, 0x0d, - 0x3a, 0xb7, 0x5c, 0xea, 0x3e, 0x5c, 0xe1, 0xc9, 0xbd, 0x12, 0x4a, 0x70, 0xf8, 0xf8, 0xe6, 0xd1, - 0xd8, 0x71, 0x68, 0xc1, 0xb4, 0xa0, 0x43, 0x5a, 0x08, 0xc3, 0x52, 0xca, 0x41, 0x36, 0xee, 0xff, - 0xf2, 0x17, 0xc2, 0xf8, 0x54, 0x29, 0x70, 0xc3, 0xc2, 0x39, 0x0e, 0x34, 0xf4, 0x1d, 0x17, 0x79, - 0x0b, 0x9d, 0x89, 0xd0, 0x02, 0x25, 0xb3, 0x0c, 0x3b, 0xe8, 0x02, 0x3a, 0x13, 0x3e, 0xc2, 0x77, - 0x3d, 0x81, 0x57, 0xac, 0x69, 0x84, 0x8a, 0x6e, 0x59, 0xce, 0x1d, 0x87, 0x9e, 0x39, 0x30, 0x54, - 0x38, 0xd0, 0xad, 0x68, 0x36, 0xb9, 0x92, 0xb5, 0x34, 0xd1, 0x74, 0x31, 0x4d, 0x82, 0xa7, 0xc7, - 0xc4, 0x0b, 0x1e, 0xc5, 0x10, 0x2f, 0x86, 0x9c, 0x81, 0x6c, 0xd6, 0x4f, 0xae, 0xbe, 0xcf, 0x27, - 0x9f, 0x7f, 0xcc, 0x93, 0x52, 0x9a, 0xaa, 0x2f, 0x08, 0x87, 0xda, 0xbb, 0xf3, 0x8f, 0x95, 0xde, - 0xbc, 0xa3, 0xe6, 0x43, 0x2b, 0xb4, 0x2d, 0xd0, 0x19, 0xb6, 0xfd, 0xcf, 0xc7, 0xf6, 0xe1, 0x43, - 0x8c, 0x99, 0x52, 0xf0, 0x3e, 0x57, 0x52, 0x9b, 0xe8, 0x68, 0x31, 0x4d, 0x66, 0xd9, 0xcc, 0x22, - 0xe7, 0x52, 0x9b, 0xe5, 0x27, 0x84, 0xef, 0xbf, 0xf6, 0x21, 0x9e, 0xf6, 0xa6, 0x82, 0x4e, 0x5e, - 0x3a, 0xbb, 0x17, 0x38, 0x60, 0x7f, 0xcc, 0xeb, 0x08, 0x59, 0x99, 0x09, 0xb9, 0x69, 0x05, 0xe4, - 0x6f, 0x5a, 0xeb, 0xa3, 0x51, 0x75, 0x76, 0xd8, 0xe2, 0xe5, 0xe3, 0xaf, 0x5f, 0x56, 0x4b, 0x6f, - 0xd3, 0xad, 0x75, 0xef, 0xf3, 0x9f, 0xc9, 0xeb, 0x57, 0x57, 0xdb, 0x18, 0x5d, 0x6f, 0x63, 0xf4, - 0x73, 0x1b, 0xa3, 0x8f, 0xbb, 0x78, 0x72, 0xbd, 0x8b, 0x27, 0xdf, 0x76, 0xf1, 0xe4, 0xcd, 0xf3, - 0xff, 0x23, 0x90, 0x05, 0x5f, 0x95, 0x40, 0x87, 0x17, 0xb4, 0x86, 0x4d, 0xaf, 0x84, 0x1e, 0x4f, - 0xe9, 0xe0, 0x84, 0x6c, 0x2e, 0xc5, 0x6d, 0xbb, 0xd1, 0x67, 0xbf, 0x03, 0x00, 0x00, 0xff, 0xff, - 0xba, 0x7f, 0xf1, 0xfd, 0x6c, 0x02, 0x00, 0x00, + // 437 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x92, 0xbd, 0x8e, 0xd3, 0x40, + 0x10, 0xc7, 0xe3, 0xcb, 0x81, 0x94, 0x8d, 0xa0, 0x30, 0x1f, 0xf2, 0x9d, 0xc0, 0x89, 0x22, 0x81, + 0xdc, 0x64, 0x97, 0x70, 0x05, 0x88, 0xee, 0x72, 0x94, 0x57, 0x84, 0x88, 0x8a, 0xc6, 0x5a, 0x6f, + 0x96, 0x78, 0x75, 0x6b, 0x8f, 0xe5, 0x1d, 0x1b, 0x71, 0x4f, 0x01, 0x0d, 0x0f, 0x41, 0xcd, 0x43, + 0x9c, 0xa8, 0xae, 0xa4, 0x02, 0x94, 0xbc, 0x08, 0xf2, 0xee, 0x06, 0x82, 0x90, 0xa8, 0x6c, 0xff, + 0xe7, 0x3f, 0x5f, 0x3f, 0x0f, 0x49, 0x54, 0x26, 0x18, 0xaf, 0x2a, 0xad, 0x04, 0x47, 0x05, 0xa5, + 0x61, 0x58, 0xf3, 0xd2, 0xbc, 0x95, 0x35, 0x6b, 0x67, 0x8c, 0x37, 0x98, 0x5f, 0xd2, 0xaa, 0x06, + 0x84, 0xf0, 0x81, 0xca, 0x04, 0xdd, 0x77, 0xd2, 0x9d, 0x93, 0xb6, 0xb3, 0xe3, 0x23, 0x01, 0xa6, + 0x00, 0x93, 0x5a, 0x2f, 0x73, 0x1f, 0x2e, 0xf1, 0xf8, 0xee, 0x1a, 0xd6, 0xe0, 0xf4, 0xee, 0xcd, + 0xab, 0xb1, 0xf3, 0xb0, 0x8c, 0x1b, 0xc9, 0xda, 0x59, 0x26, 0x91, 0xcf, 0x98, 0x00, 0x55, 0xba, + 0xf8, 0xe4, 0xd3, 0x01, 0x21, 0xa7, 0x5a, 0x83, 0x6b, 0x16, 0x8e, 0xc8, 0xd0, 0x40, 0x53, 0x0b, + 0x99, 0x56, 0x50, 0x63, 0x14, 0x8c, 0x83, 0x64, 0xb0, 0x24, 0x4e, 0x5a, 0x40, 0x8d, 0xe1, 0x23, + 0x72, 0xdb, 0x1b, 0x44, 0xce, 0xcb, 0x52, 0xea, 0xe8, 0xc0, 0x7a, 0x6e, 0x39, 0xf5, 0xcc, 0x89, + 0xa1, 0x26, 0x43, 0x53, 0xc9, 0x72, 0x95, 0x6a, 0x55, 0x28, 0x8c, 0xfa, 0xe3, 0x7e, 0x32, 0x7c, + 0x7a, 0x44, 0xfd, 0xc0, 0xdd, 0x30, 0xd4, 0x0f, 0x43, 0xcf, 0x40, 0x95, 0xf3, 0x27, 0x57, 0xdf, + 0x47, 0xbd, 0xcf, 0x3f, 0x46, 0xc9, 0x5a, 0x61, 0xde, 0x64, 0x54, 0x40, 0xe1, 0xb7, 0xf3, 0x8f, + 0xa9, 0x59, 0x5d, 0x30, 0x7c, 0x5f, 0x49, 0x63, 0x13, 0xcc, 0x92, 0xd8, 0xfa, 0xe7, 0x5d, 0xf9, + 0xf0, 0x21, 0x21, 0x5c, 0x6b, 0x78, 0x97, 0x6a, 0x65, 0x30, 0x3a, 0x1c, 0xf7, 0x93, 0xc1, 0x72, + 0x60, 0x95, 0x73, 0x65, 0x30, 0x3c, 0x21, 0xf7, 0x5d, 0xb8, 0xe2, 0xe2, 0x42, 0x62, 0xba, 0xe2, + 0xc8, 0x9d, 0xf5, 0x86, 0xb5, 0xde, 0xb1, 0xd1, 0x85, 0x0d, 0xbe, 0xe4, 0xc8, 0xbb, 0xa4, 0xc9, + 0xc7, 0x80, 0xdc, 0x7b, 0xed, 0xc9, 0x9f, 0x36, 0x98, 0x43, 0xad, 0x2e, 0x1d, 0xa3, 0x05, 0x19, + 0xf2, 0xdf, 0xc4, 0x4c, 0x14, 0xd8, 0xdd, 0x12, 0xfa, 0xbf, 0xff, 0x46, 0xff, 0x20, 0x9e, 0x1f, + 0x76, 0xab, 0x2e, 0xf7, 0x4b, 0xbc, 0x78, 0xfc, 0xf5, 0xcb, 0x74, 0xe2, 0xd9, 0xb8, 0x5b, 0xd8, + 0xc1, 0xf9, 0xab, 0xf3, 0xfc, 0xd5, 0xd5, 0x26, 0x0e, 0xae, 0x37, 0x71, 0xf0, 0x73, 0x13, 0x07, + 0x1f, 0xb6, 0x71, 0xef, 0x7a, 0x1b, 0xf7, 0xbe, 0x6d, 0xe3, 0xde, 0x9b, 0x67, 0xff, 0x72, 0x53, + 0x99, 0x98, 0xae, 0x81, 0xb5, 0xcf, 0x59, 0x01, 0xab, 0x46, 0x4b, 0xd3, 0xdd, 0xdf, 0xde, 0xdd, + 0x59, 0x98, 0xd9, 0x4d, 0x7b, 0x06, 0x27, 0xbf, 0x02, 0x00, 0x00, 0xff, 0xff, 0x92, 0x8d, 0xa7, + 0x17, 0xa1, 0x02, 0x00, 0x00, } func (m *Allocation) Marshal() (dAtA []byte, err error) { @@ -205,6 +217,15 @@ func (m *Allocation) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l + if len(m.AllowPacketDataList) > 0 { + for iNdEx := len(m.AllowPacketDataList) - 1; iNdEx >= 0; iNdEx-- { + i -= len(m.AllowPacketDataList[iNdEx]) + copy(dAtA[i:], m.AllowPacketDataList[iNdEx]) + i = encodeVarintAuthz(dAtA, i, uint64(len(m.AllowPacketDataList[iNdEx]))) + i-- + dAtA[i] = 0x2a + } + } if len(m.AllowList) > 0 { for iNdEx := len(m.AllowList) - 1; iNdEx >= 0; iNdEx-- { i -= len(m.AllowList[iNdEx]) @@ -319,6 +340,12 @@ func (m *Allocation) Size() (n int) { n += 1 + l + sovAuthz(uint64(l)) } } + if len(m.AllowPacketDataList) > 0 { + for _, s := range m.AllowPacketDataList { + l = len(s) + n += 1 + l + sovAuthz(uint64(l)) + } + } return n } @@ -502,6 +529,38 @@ func (m *Allocation) Unmarshal(dAtA []byte) error { } m.AllowList = append(m.AllowList, string(dAtA[iNdEx:postIndex])) iNdEx = postIndex + case 5: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field AllowPacketDataList", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowAuthz + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthAuthz + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthAuthz + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.AllowPacketDataList = append(m.AllowPacketDataList, string(dAtA[iNdEx:postIndex])) + iNdEx = postIndex default: iNdEx = preIndex skippy, err := skipAuthz(dAtA[iNdEx:]) diff --git a/proto/ibc/applications/transfer/v1/authz.proto b/proto/ibc/applications/transfer/v1/authz.proto index 7b7a9f0846a..f525ec6d0f7 100644 --- a/proto/ibc/applications/transfer/v1/authz.proto +++ b/proto/ibc/applications/transfer/v1/authz.proto @@ -19,6 +19,9 @@ 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. + // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key + repeated string allow_packet_data_list = 5; } // TransferAuthorization allows the grantee to spend up to spend_limit coins from From 0543a9bcd9ed6ec960db070c10cbe62ef84bd3e9 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Thu, 30 Nov 2023 19:58:12 +0700 Subject: [PATCH 03/31] add func check allowed packet data --- .../transfer/types/transfer_authorization.go | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 82f40621efd..b7fbc505184 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -2,7 +2,11 @@ package types import ( "context" + "encoding/json" + fmt "fmt" "math/big" + "slices" + "strings" "github.com/cosmos/gogoproto/proto" @@ -145,6 +149,39 @@ func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) b return false } +// areAllowedPacketDataKeys returns a boolean indicating if the memo is valid for transfer. +func areAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList []string) (bool, error) { + // if the allow list is empty, then the memo must be an empty string + if len(allowedPacketDataList) == 0 { + return len(strings.TrimSpace(memo)) == 0, nil + } + + // if allowedPacketData have only 1 elements and it equal AllowAllPacketDataKeys + // then accept all the packet + if len(allowedPacketDataList) == 1 && allowedPacketDataList[0] == AllowAllPacketDataKeys { + return true, nil + } + + // unmarshal memo + jsonObject := make(map[string]interface{}) + err := json.Unmarshal([]byte(memo), &jsonObject) + if err != nil { + return false, err + } + + gasCostPerIteration := ctx.KVGasConfig().IterNextCostFlat + + for key := range jsonObject { + ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization") + + if !slices.Contains(allowedPacketDataList, key) { + return false, fmt.Errorf("not allowed packet data key: %s", key) + } + } + + return true, nil +} + // 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 From 26a55d68981ee3cfe49bfa029a742ef26e530468 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Thu, 30 Nov 2023 20:05:10 +0700 Subject: [PATCH 04/31] add check allow packet --- modules/apps/transfer/types/transfer_authorization.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index b7fbc505184..042a314c020 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -54,6 +54,15 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "not allowed receiver address for transfer") } + isAllowedPacket, err := areAllowedPacketDataKeys(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowPacketDataList) + if err != nil { + return authz.AcceptResponse{}, err + } + + if !isAllowedPacket { + return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidRequest, "not allowed packet data type for transfer") + } + // If the spend limit is set to the MaxUint256 sentinel value, do not subtract the amount from the spend limit. if allocation.SpendLimit.AmountOf(msgTransfer.Token.Denom).Equal(UnboundedSpendLimit()) { return authz.AcceptResponse{Accept: true, Delete: false, Updated: nil}, nil From cc6abf7fb1e0c5500112bc93d72bcd0a5b24812c Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Thu, 30 Nov 2023 20:49:57 +0700 Subject: [PATCH 05/31] add test --- .../types/transfer_authorization_test.go | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/modules/apps/transfer/types/transfer_authorization_test.go b/modules/apps/transfer/types/transfer_authorization_test.go index fdc877340e8..8f8ebecf68c 100644 --- a/modules/apps/transfer/types/transfer_authorization_test.go +++ b/modules/apps/transfer/types/transfer_authorization_test.go @@ -101,6 +101,72 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { suite.Require().Nil(res.Updated) }, }, + { + "success: empty allowedPacketDataList and empty memo", + func() { + allowedList := []string{} + transferAuthz.Allocations[0].AllowPacketDataList = 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: allowedPacketDataList allow any packet", + func() { + allowedList := []string{"*"} + transferAuthz.Allocations[0].AllowPacketDataList = allowedList + msgTransfer.Memo = `{"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"}}}}` + }, + 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{"wasm", "forward"} + transferAuthz.Allocations[0].AllowPacketDataList = allowedList + msgTransfer.Memo = `{"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"}}}}` + }, + 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) + }, + }, + { + "error: empty allowedPacketDataList but not empty memo", + func() { + allowedList := []string{} + transferAuthz.Allocations[0].AllowPacketDataList = allowedList + msgTransfer.Memo = `{"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"}}}}` + }, + func(res authz.AcceptResponse, err error) { + suite.Require().Error(err) + }, + }, + { + "error: memo not allowed", + func() { + allowedList := []string{"forward"} + transferAuthz.Allocations[0].AllowPacketDataList = allowedList + msgTransfer.Memo = `{"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"}}}}` + }, + func(res authz.AcceptResponse, err error) { + suite.Require().Error(err) + }, + }, { "test multiple coins does not overspend", func() { From 695152a9b4c16606d3334c8cda7586e550484e85 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Thu, 30 Nov 2023 21:00:23 +0700 Subject: [PATCH 06/31] update docs --- docs/docs/02-apps/01-transfer/08-authorizations.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/docs/02-apps/01-transfer/08-authorizations.md b/docs/docs/02-apps/01-transfer/08-authorizations.md index bd7a5b9269c..218af627f97 100644 --- a/docs/docs/02-apps/01-transfer/08-authorizations.md +++ b/docs/docs/02-apps/01-transfer/08-authorizations.md @@ -22,6 +22,8 @@ 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 `AllowPacketDataList` list that specifies the list of memo that are allowed to send the packet. If this list is empty, then only allow empty memo packet (any other type of `memo` will be denied). If this list is `"*"`, then allow any type of `memo` + Setting a `TransferAuthorization` is expected to fail if: - the spend limit is nil @@ -29,6 +31,7 @@ Setting a `TransferAuthorization` is expected to fail if: - the source port ID is invalid - the source channel ID is invalid - there are duplicate entries in the `AllowList` +- the packet `memo` not allowed by `AllowPacketDataList` Below is the `TransferAuthorization` message: @@ -48,6 +51,9 @@ 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. + // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key + AllowPacketDataList []string } ``` From ddec47c820707b2dc5fb2b8ddf89e2f703f05dd8 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Thu, 30 Nov 2023 21:11:31 +0700 Subject: [PATCH 07/31] const the memo --- .../apps/transfer/types/transfer_authorization_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/modules/apps/transfer/types/transfer_authorization_test.go b/modules/apps/transfer/types/transfer_authorization_test.go index 8f8ebecf68c..c5b046b1f57 100644 --- a/modules/apps/transfer/types/transfer_authorization_test.go +++ b/modules/apps/transfer/types/transfer_authorization_test.go @@ -17,6 +17,8 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { transferAuthz types.TransferAuthorization ) + 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"}}}}` + testCases := []struct { name string malleate func() @@ -120,7 +122,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { func() { allowedList := []string{"*"} transferAuthz.Allocations[0].AllowPacketDataList = allowedList - msgTransfer.Memo = `{"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"}}}}` + msgTransfer.Memo = testMemo }, func(res authz.AcceptResponse, err error) { suite.Require().NoError(err) @@ -135,7 +137,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { func() { allowedList := []string{"wasm", "forward"} transferAuthz.Allocations[0].AllowPacketDataList = allowedList - msgTransfer.Memo = `{"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"}}}}` + msgTransfer.Memo = testMemo }, func(res authz.AcceptResponse, err error) { suite.Require().NoError(err) @@ -150,7 +152,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { func() { allowedList := []string{} transferAuthz.Allocations[0].AllowPacketDataList = allowedList - msgTransfer.Memo = `{"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"}}}}` + msgTransfer.Memo = testMemo }, func(res authz.AcceptResponse, err error) { suite.Require().Error(err) @@ -161,7 +163,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { func() { allowedList := []string{"forward"} transferAuthz.Allocations[0].AllowPacketDataList = allowedList - msgTransfer.Memo = `{"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"}}}}` + msgTransfer.Memo = testMemo }, func(res authz.AcceptResponse, err error) { suite.Require().Error(err) From 180e419b099f6439e97b4ebb8a3c51f80d71295d Mon Sep 17 00:00:00 2001 From: GnaD <89174180+GNaD13@users.noreply.github.com> Date: Sat, 2 Dec 2023 02:20:31 +0700 Subject: [PATCH 08/31] Update modules/apps/transfer/types/transfer_authorization.go Co-authored-by: Carlos Rodriguez --- modules/apps/transfer/types/transfer_authorization.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 042a314c020..36f557e9805 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -60,7 +60,7 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a } if !isAllowedPacket { - return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidRequest, "not allowed packet data type for transfer") + return authz.AcceptResponse{}, ErrInvalidMemo } // If the spend limit is set to the MaxUint256 sentinel value, do not subtract the amount from the spend limit. From 92649be451abdf975f8e40bba9166af0feb79a56 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Sat, 2 Dec 2023 02:37:13 +0700 Subject: [PATCH 09/31] update nits --- docs/docs/02-apps/01-transfer/08-authorizations.md | 4 ++-- modules/apps/transfer/types/transfer_authorization.go | 8 ++++---- .../apps/transfer/types/transfer_authorization_test.go | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/docs/02-apps/01-transfer/08-authorizations.md b/docs/docs/02-apps/01-transfer/08-authorizations.md index 218af627f97..8fe81b7a9e1 100644 --- a/docs/docs/02-apps/01-transfer/08-authorizations.md +++ b/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 `AllowPacketDataList` list that specifies the list of memo that are allowed to send the packet. If this list is empty, then only allow empty memo packet (any other type of `memo` will be denied). If this list is `"*"`, then allow any type of `memo` +- an `AllowPacketDataList` 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. Setting a `TransferAuthorization` is expected to fail if: @@ -31,7 +31,7 @@ Setting a `TransferAuthorization` is expected to fail if: - the source port ID is invalid - the source channel ID is invalid - there are duplicate entries in the `AllowList` -- the packet `memo` not allowed by `AllowPacketDataList` +- the `memo` field is not allowed by `AllowPacketDataList` Below is the `TransferAuthorization` message: diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 36f557e9805..3a787ef3825 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -56,7 +56,7 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a isAllowedPacket, err := areAllowedPacketDataKeys(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowPacketDataList) if err != nil { - return authz.AcceptResponse{}, err + return authz.AcceptResponse{}, errorsmod.Wrapf(ErrInvalidMemo, "error: %v", err) } if !isAllowedPacket { @@ -158,15 +158,15 @@ func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) b return false } -// areAllowedPacketDataKeys returns a boolean indicating if the memo is valid for transfer. +// areAllowedPacketDataKeys returns a boolean indicating if the memo is valid for transfer. If it is not valid and non-nil error is also returned. func areAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList []string) (bool, error) { // if the allow list is empty, then the memo must be an empty string if len(allowedPacketDataList) == 0 { return len(strings.TrimSpace(memo)) == 0, nil } - // if allowedPacketData have only 1 elements and it equal AllowAllPacketDataKeys - // then accept all the packet + // if allowedPacketDataList has only 1 element and it equals AllowAllPacketDataKeys + // then accept all the packet data keys if len(allowedPacketDataList) == 1 && allowedPacketDataList[0] == AllowAllPacketDataKeys { return true, nil } diff --git a/modules/apps/transfer/types/transfer_authorization_test.go b/modules/apps/transfer/types/transfer_authorization_test.go index c5b046b1f57..a5f5d018a03 100644 --- a/modules/apps/transfer/types/transfer_authorization_test.go +++ b/modules/apps/transfer/types/transfer_authorization_test.go @@ -104,7 +104,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, }, { - "success: empty allowedPacketDataList and empty memo", + "success: empty allowPacketDataList and empty memo", func() { allowedList := []string{} transferAuthz.Allocations[0].AllowPacketDataList = allowedList @@ -118,7 +118,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, }, { - "success: allowedPacketDataList allow any packet", + "success: allowPacketDataList allows any packet", func() { allowedList := []string{"*"} transferAuthz.Allocations[0].AllowPacketDataList = allowedList @@ -148,7 +148,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, }, { - "error: empty allowedPacketDataList but not empty memo", + "empty allowPacketDataList but not empty memo", func() { allowedList := []string{} transferAuthz.Allocations[0].AllowPacketDataList = allowedList @@ -159,7 +159,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, }, { - "error: memo not allowed", + "memo not allowed", func() { allowedList := []string{"forward"} transferAuthz.Allocations[0].AllowPacketDataList = allowedList From 9a904552cfc5d5e7843dd6d4cca64d1cdb3f1bb7 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Tue, 5 Dec 2023 10:55:02 +0700 Subject: [PATCH 10/31] key --- modules/apps/transfer/types/transfer_authorization.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 3a787ef3825..942ac84ea27 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/cosmos/gogoproto/proto" + "golang.org/x/exp/maps" errorsmod "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" @@ -179,8 +180,9 @@ func areAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataLis } gasCostPerIteration := ctx.KVGasConfig().IterNextCostFlat + jsonObjectKeys := maps.Keys(jsonObject) - for key := range jsonObject { + for _, key := range jsonObjectKeys { ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization") if !slices.Contains(allowedPacketDataList, key) { From 7f28ad58e0ebac49ee6b4000b6750432485cf5c2 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Tue, 5 Dec 2023 10:56:12 +0700 Subject: [PATCH 11/31] remove keys because the order is indeterminate --- modules/apps/transfer/types/transfer_authorization.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 942ac84ea27..3a787ef3825 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -9,7 +9,6 @@ import ( "strings" "github.com/cosmos/gogoproto/proto" - "golang.org/x/exp/maps" errorsmod "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" @@ -180,9 +179,8 @@ func areAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataLis } gasCostPerIteration := ctx.KVGasConfig().IterNextCostFlat - jsonObjectKeys := maps.Keys(jsonObject) - for _, key := range jsonObjectKeys { + for key := range jsonObject { ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization") if !slices.Contains(allowedPacketDataList, key) { From 65cc3adf2732cdd1b13beba38cc27e29bad14a54 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Tue, 5 Dec 2023 11:03:35 +0700 Subject: [PATCH 12/31] iterator allowedPacketDataList --- .../apps/transfer/types/transfer_authorization.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 3a787ef3825..c9ee463be39 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -5,7 +5,6 @@ import ( "encoding/json" fmt "fmt" "math/big" - "slices" "strings" "github.com/cosmos/gogoproto/proto" @@ -180,14 +179,18 @@ func areAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataLis gasCostPerIteration := ctx.KVGasConfig().IterNextCostFlat - for key := range jsonObject { - ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization") - - if !slices.Contains(allowedPacketDataList, key) { - return false, fmt.Errorf("not allowed packet data key: %s", key) + for _, key := range allowedPacketDataList { + _, ok := jsonObject[key] + if ok { + ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization") + delete(jsonObject, key) } } + if len(jsonObject) != 0 { + return false, fmt.Errorf("not allowed packet data key") + } + return true, nil } From c42ba43e564b30a7e0c0da1f6bbb942f0fe8384b Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Wed, 6 Dec 2023 23:44:24 +0700 Subject: [PATCH 13/31] nit --- .../02-apps/01-transfer/08-authorizations.md | 4 +- modules/apps/transfer/types/authz.pb.go | 82 +++++++++---------- .../transfer/types/transfer_authorization.go | 4 +- .../types/transfer_authorization_test.go | 10 +-- .../ibc/applications/transfer/v1/authz.proto | 2 +- 5 files changed, 51 insertions(+), 51 deletions(-) diff --git a/docs/docs/02-apps/01-transfer/08-authorizations.md b/docs/docs/02-apps/01-transfer/08-authorizations.md index 8fe81b7a9e1..3d91692dd9c 100644 --- a/docs/docs/02-apps/01-transfer/08-authorizations.md +++ b/docs/docs/02-apps/01-transfer/08-authorizations.md @@ -52,8 +52,8 @@ type Allocation struct { // allow list of receivers, an empty allow list permits any receiver address AllowList []string // allow list of packet data keys, an empty list. - // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key - AllowPacketDataList []string + // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key + AllowPacketDataList []string } ``` diff --git a/modules/apps/transfer/types/authz.pb.go b/modules/apps/transfer/types/authz.pb.go index 2520694fc88..453ffc26d39 100644 --- a/modules/apps/transfer/types/authz.pb.go +++ b/modules/apps/transfer/types/authz.pb.go @@ -38,7 +38,7 @@ type Allocation struct { AllowList []string `protobuf:"bytes,4,rep,name=allow_list,json=allowList,proto3" json:"allow_list,omitempty"` // allow list of packet data keys, an empty list. // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key - AllowPacketDataList []string `protobuf:"bytes,5,rep,name=allow_packet_data_list,json=allowPacketDataList,proto3" json:"allow_packet_data_list,omitempty"` + AllowPacketData []string `protobuf:"bytes,5,rep,name=allow_packet_data,json=allowPacketData,proto3" json:"allow_packet_data,omitempty"` } func (m *Allocation) Reset() { *m = Allocation{} } @@ -102,9 +102,9 @@ func (m *Allocation) GetAllowList() []string { return nil } -func (m *Allocation) GetAllowPacketDataList() []string { +func (m *Allocation) GetAllowPacketData() []string { if m != nil { - return m.AllowPacketDataList + return m.AllowPacketData } return nil } @@ -166,35 +166,35 @@ func init() { } var fileDescriptor_b1a28b55d17325aa = []byte{ - // 437 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x92, 0xbd, 0x8e, 0xd3, 0x40, - 0x10, 0xc7, 0xe3, 0xcb, 0x81, 0x94, 0x8d, 0xa0, 0x30, 0x1f, 0xf2, 0x9d, 0xc0, 0x89, 0x22, 0x81, - 0xdc, 0x64, 0x97, 0x70, 0x05, 0x88, 0xee, 0x72, 0x94, 0x57, 0x84, 0x88, 0x8a, 0xc6, 0x5a, 0x6f, - 0x96, 0x78, 0x75, 0x6b, 0x8f, 0xe5, 0x1d, 0x1b, 0x71, 0x4f, 0x01, 0x0d, 0x0f, 0x41, 0xcd, 0x43, - 0x9c, 0xa8, 0xae, 0xa4, 0x02, 0x94, 0xbc, 0x08, 0xf2, 0xee, 0x06, 0x82, 0x90, 0xa8, 0x6c, 0xff, - 0xe7, 0x3f, 0x5f, 0x3f, 0x0f, 0x49, 0x54, 0x26, 0x18, 0xaf, 0x2a, 0xad, 0x04, 0x47, 0x05, 0xa5, - 0x61, 0x58, 0xf3, 0xd2, 0xbc, 0x95, 0x35, 0x6b, 0x67, 0x8c, 0x37, 0x98, 0x5f, 0xd2, 0xaa, 0x06, - 0x84, 0xf0, 0x81, 0xca, 0x04, 0xdd, 0x77, 0xd2, 0x9d, 0x93, 0xb6, 0xb3, 0xe3, 0x23, 0x01, 0xa6, - 0x00, 0x93, 0x5a, 0x2f, 0x73, 0x1f, 0x2e, 0xf1, 0xf8, 0xee, 0x1a, 0xd6, 0xe0, 0xf4, 0xee, 0xcd, - 0xab, 0xb1, 0xf3, 0xb0, 0x8c, 0x1b, 0xc9, 0xda, 0x59, 0x26, 0x91, 0xcf, 0x98, 0x00, 0x55, 0xba, - 0xf8, 0xe4, 0xd3, 0x01, 0x21, 0xa7, 0x5a, 0x83, 0x6b, 0x16, 0x8e, 0xc8, 0xd0, 0x40, 0x53, 0x0b, - 0x99, 0x56, 0x50, 0x63, 0x14, 0x8c, 0x83, 0x64, 0xb0, 0x24, 0x4e, 0x5a, 0x40, 0x8d, 0xe1, 0x23, - 0x72, 0xdb, 0x1b, 0x44, 0xce, 0xcb, 0x52, 0xea, 0xe8, 0xc0, 0x7a, 0x6e, 0x39, 0xf5, 0xcc, 0x89, - 0xa1, 0x26, 0x43, 0x53, 0xc9, 0x72, 0x95, 0x6a, 0x55, 0x28, 0x8c, 0xfa, 0xe3, 0x7e, 0x32, 0x7c, - 0x7a, 0x44, 0xfd, 0xc0, 0xdd, 0x30, 0xd4, 0x0f, 0x43, 0xcf, 0x40, 0x95, 0xf3, 0x27, 0x57, 0xdf, - 0x47, 0xbd, 0xcf, 0x3f, 0x46, 0xc9, 0x5a, 0x61, 0xde, 0x64, 0x54, 0x40, 0xe1, 0xb7, 0xf3, 0x8f, - 0xa9, 0x59, 0x5d, 0x30, 0x7c, 0x5f, 0x49, 0x63, 0x13, 0xcc, 0x92, 0xd8, 0xfa, 0xe7, 0x5d, 0xf9, - 0xf0, 0x21, 0x21, 0x5c, 0x6b, 0x78, 0x97, 0x6a, 0x65, 0x30, 0x3a, 0x1c, 0xf7, 0x93, 0xc1, 0x72, - 0x60, 0x95, 0x73, 0x65, 0x30, 0x3c, 0x21, 0xf7, 0x5d, 0xb8, 0xe2, 0xe2, 0x42, 0x62, 0xba, 0xe2, - 0xc8, 0x9d, 0xf5, 0x86, 0xb5, 0xde, 0xb1, 0xd1, 0x85, 0x0d, 0xbe, 0xe4, 0xc8, 0xbb, 0xa4, 0xc9, - 0xc7, 0x80, 0xdc, 0x7b, 0xed, 0xc9, 0x9f, 0x36, 0x98, 0x43, 0xad, 0x2e, 0x1d, 0xa3, 0x05, 0x19, - 0xf2, 0xdf, 0xc4, 0x4c, 0x14, 0xd8, 0xdd, 0x12, 0xfa, 0xbf, 0xff, 0x46, 0xff, 0x20, 0x9e, 0x1f, - 0x76, 0xab, 0x2e, 0xf7, 0x4b, 0xbc, 0x78, 0xfc, 0xf5, 0xcb, 0x74, 0xe2, 0xd9, 0xb8, 0x5b, 0xd8, - 0xc1, 0xf9, 0xab, 0xf3, 0xfc, 0xd5, 0xd5, 0x26, 0x0e, 0xae, 0x37, 0x71, 0xf0, 0x73, 0x13, 0x07, - 0x1f, 0xb6, 0x71, 0xef, 0x7a, 0x1b, 0xf7, 0xbe, 0x6d, 0xe3, 0xde, 0x9b, 0x67, 0xff, 0x72, 0x53, - 0x99, 0x98, 0xae, 0x81, 0xb5, 0xcf, 0x59, 0x01, 0xab, 0x46, 0x4b, 0xd3, 0xdd, 0xdf, 0xde, 0xdd, - 0x59, 0x98, 0xd9, 0x4d, 0x7b, 0x06, 0x27, 0xbf, 0x02, 0x00, 0x00, 0xff, 0xff, 0x92, 0x8d, 0xa7, - 0x17, 0xa1, 0x02, 0x00, 0x00, + // 435 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x92, 0xc1, 0x8e, 0xd3, 0x30, + 0x10, 0x86, 0x9b, 0xed, 0x82, 0x54, 0x57, 0x80, 0x88, 0x40, 0xca, 0xae, 0x20, 0xad, 0x2a, 0x81, + 0x22, 0xa4, 0xda, 0x14, 0x0e, 0x20, 0x6e, 0xdb, 0xe5, 0xb8, 0x87, 0x52, 0x71, 0xe2, 0x12, 0x39, + 0xae, 0x69, 0xac, 0x75, 0x32, 0x51, 0x3c, 0x09, 0x62, 0xdf, 0x01, 0x09, 0x5e, 0x83, 0x33, 0x0f, + 0xb1, 0xe2, 0xb4, 0x47, 0x4e, 0x80, 0xda, 0x17, 0x41, 0xb1, 0x5d, 0x28, 0x42, 0xe2, 0x94, 0xf8, + 0x9f, 0xdf, 0xe3, 0x99, 0x6f, 0x86, 0x24, 0x2a, 0x13, 0x8c, 0x57, 0x95, 0x56, 0x82, 0xa3, 0x82, + 0xd2, 0x30, 0xac, 0x79, 0x69, 0xde, 0xca, 0x9a, 0xb5, 0x33, 0xc6, 0x1b, 0xcc, 0x2f, 0x68, 0x55, + 0x03, 0x42, 0x78, 0x4f, 0x65, 0x82, 0xee, 0x3b, 0xe9, 0xce, 0x49, 0xdb, 0xd9, 0xf1, 0x91, 0x00, + 0x53, 0x80, 0x49, 0xad, 0x97, 0xb9, 0x83, 0xbb, 0x78, 0x7c, 0x67, 0x0d, 0x6b, 0x70, 0x7a, 0xf7, + 0xe7, 0xd5, 0xd8, 0x79, 0x58, 0xc6, 0x8d, 0x64, 0xed, 0x2c, 0x93, 0xc8, 0x67, 0x4c, 0x80, 0x2a, + 0x5d, 0x7c, 0xf2, 0xe1, 0x80, 0x90, 0x13, 0xad, 0xc1, 0x3d, 0x16, 0x8e, 0xc8, 0xd0, 0x40, 0x53, + 0x0b, 0x99, 0x56, 0x50, 0x63, 0x14, 0x8c, 0x83, 0x64, 0xb0, 0x24, 0x4e, 0x5a, 0x40, 0x8d, 0xe1, + 0x03, 0x72, 0xd3, 0x1b, 0x44, 0xce, 0xcb, 0x52, 0xea, 0xe8, 0xc0, 0x7a, 0x6e, 0x38, 0xf5, 0xd4, + 0x89, 0xa1, 0x26, 0x43, 0x53, 0xc9, 0x72, 0x95, 0x6a, 0x55, 0x28, 0x8c, 0xfa, 0xe3, 0x7e, 0x32, + 0x7c, 0x72, 0x44, 0x7d, 0xc1, 0x5d, 0x31, 0xd4, 0x17, 0x43, 0x4f, 0x41, 0x95, 0xf3, 0xc7, 0x97, + 0xdf, 0x47, 0xbd, 0xcf, 0x3f, 0x46, 0xc9, 0x5a, 0x61, 0xde, 0x64, 0x54, 0x40, 0xe1, 0xbb, 0xf3, + 0x9f, 0xa9, 0x59, 0x9d, 0x33, 0x7c, 0x5f, 0x49, 0x63, 0x2f, 0x98, 0x25, 0xb1, 0xf9, 0xcf, 0xba, + 0xf4, 0xe1, 0x7d, 0x42, 0xb8, 0xd6, 0xf0, 0x2e, 0xd5, 0xca, 0x60, 0x74, 0x38, 0xee, 0x27, 0x83, + 0xe5, 0xc0, 0x2a, 0x67, 0xca, 0x60, 0xf8, 0x88, 0xdc, 0x76, 0xe1, 0x8a, 0x8b, 0x73, 0x89, 0xe9, + 0x8a, 0x23, 0x8f, 0xae, 0x59, 0xd7, 0x2d, 0x1b, 0x58, 0x58, 0xfd, 0x25, 0x47, 0x3e, 0xf9, 0x14, + 0x90, 0xbb, 0xaf, 0x3d, 0xf0, 0x93, 0x06, 0x73, 0xa8, 0xd5, 0x85, 0x43, 0xb3, 0x20, 0x43, 0xfe, + 0x1b, 0x94, 0x89, 0x02, 0xdb, 0x52, 0x42, 0xff, 0x37, 0x2e, 0xfa, 0x87, 0xec, 0xfc, 0xb0, 0xeb, + 0x70, 0xb9, 0x9f, 0xe2, 0xc5, 0xc3, 0xaf, 0x5f, 0xa6, 0x13, 0x8f, 0xc4, 0xad, 0xc0, 0x8e, 0xc9, + 0x5f, 0x2f, 0xcf, 0x5f, 0x5d, 0x6e, 0xe2, 0xe0, 0x6a, 0x13, 0x07, 0x3f, 0x37, 0x71, 0xf0, 0x71, + 0x1b, 0xf7, 0xae, 0xb6, 0x71, 0xef, 0xdb, 0x36, 0xee, 0xbd, 0x79, 0xf6, 0x2f, 0x2e, 0x95, 0x89, + 0xe9, 0x1a, 0x58, 0xfb, 0x9c, 0x15, 0xb0, 0x6a, 0xb4, 0x34, 0xdd, 0xda, 0xed, 0xad, 0x9b, 0x65, + 0x98, 0x5d, 0xb7, 0xd3, 0x7f, 0xfa, 0x2b, 0x00, 0x00, 0xff, 0xff, 0x58, 0xa3, 0xbf, 0x1b, 0x98, + 0x02, 0x00, 0x00, } func (m *Allocation) Marshal() (dAtA []byte, err error) { @@ -217,11 +217,11 @@ func (m *Allocation) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l - if len(m.AllowPacketDataList) > 0 { - for iNdEx := len(m.AllowPacketDataList) - 1; iNdEx >= 0; iNdEx-- { - i -= len(m.AllowPacketDataList[iNdEx]) - copy(dAtA[i:], m.AllowPacketDataList[iNdEx]) - i = encodeVarintAuthz(dAtA, i, uint64(len(m.AllowPacketDataList[iNdEx]))) + if len(m.AllowPacketData) > 0 { + for iNdEx := len(m.AllowPacketData) - 1; iNdEx >= 0; iNdEx-- { + i -= len(m.AllowPacketData[iNdEx]) + copy(dAtA[i:], m.AllowPacketData[iNdEx]) + i = encodeVarintAuthz(dAtA, i, uint64(len(m.AllowPacketData[iNdEx]))) i-- dAtA[i] = 0x2a } @@ -340,8 +340,8 @@ func (m *Allocation) Size() (n int) { n += 1 + l + sovAuthz(uint64(l)) } } - if len(m.AllowPacketDataList) > 0 { - for _, s := range m.AllowPacketDataList { + if len(m.AllowPacketData) > 0 { + for _, s := range m.AllowPacketData { l = len(s) n += 1 + l + sovAuthz(uint64(l)) } @@ -531,7 +531,7 @@ func (m *Allocation) Unmarshal(dAtA []byte) error { iNdEx = postIndex case 5: if wireType != 2 { - return fmt.Errorf("proto: wrong wireType = %d for field AllowPacketDataList", wireType) + return fmt.Errorf("proto: wrong wireType = %d for field AllowPacketData", wireType) } var stringLen uint64 for shift := uint(0); ; shift += 7 { @@ -559,7 +559,7 @@ func (m *Allocation) Unmarshal(dAtA []byte) error { if postIndex > l { return io.ErrUnexpectedEOF } - m.AllowPacketDataList = append(m.AllowPacketDataList, string(dAtA[iNdEx:postIndex])) + m.AllowPacketData = append(m.AllowPacketData, string(dAtA[iNdEx:postIndex])) iNdEx = postIndex default: iNdEx = preIndex diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index c9ee463be39..9ec01813a50 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -53,7 +53,7 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "not allowed receiver address for transfer") } - isAllowedPacket, err := areAllowedPacketDataKeys(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowPacketDataList) + isAllowedPacket, err := isAllowedPacketDataKeys(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowPacketData) if err != nil { return authz.AcceptResponse{}, errorsmod.Wrapf(ErrInvalidMemo, "error: %v", err) } @@ -158,7 +158,7 @@ func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) b } // areAllowedPacketDataKeys returns a boolean indicating if the memo is valid for transfer. If it is not valid and non-nil error is also returned. -func areAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList []string) (bool, error) { +func isAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList []string) (bool, error) { // if the allow list is empty, then the memo must be an empty string if len(allowedPacketDataList) == 0 { return len(strings.TrimSpace(memo)) == 0, nil diff --git a/modules/apps/transfer/types/transfer_authorization_test.go b/modules/apps/transfer/types/transfer_authorization_test.go index a5f5d018a03..73b2a633585 100644 --- a/modules/apps/transfer/types/transfer_authorization_test.go +++ b/modules/apps/transfer/types/transfer_authorization_test.go @@ -107,7 +107,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { "success: empty allowPacketDataList and empty memo", func() { allowedList := []string{} - transferAuthz.Allocations[0].AllowPacketDataList = allowedList + transferAuthz.Allocations[0].AllowPacketData = allowedList }, func(res authz.AcceptResponse, err error) { suite.Require().NoError(err) @@ -121,7 +121,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { "success: allowPacketDataList allows any packet", func() { allowedList := []string{"*"} - transferAuthz.Allocations[0].AllowPacketDataList = allowedList + transferAuthz.Allocations[0].AllowPacketData = allowedList msgTransfer.Memo = testMemo }, func(res authz.AcceptResponse, err error) { @@ -136,7 +136,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { "success: transfer memo allowed", func() { allowedList := []string{"wasm", "forward"} - transferAuthz.Allocations[0].AllowPacketDataList = allowedList + transferAuthz.Allocations[0].AllowPacketData = allowedList msgTransfer.Memo = testMemo }, func(res authz.AcceptResponse, err error) { @@ -151,7 +151,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { "empty allowPacketDataList but not empty memo", func() { allowedList := []string{} - transferAuthz.Allocations[0].AllowPacketDataList = allowedList + transferAuthz.Allocations[0].AllowPacketData = allowedList msgTransfer.Memo = testMemo }, func(res authz.AcceptResponse, err error) { @@ -162,7 +162,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { "memo not allowed", func() { allowedList := []string{"forward"} - transferAuthz.Allocations[0].AllowPacketDataList = allowedList + transferAuthz.Allocations[0].AllowPacketData = allowedList msgTransfer.Memo = testMemo }, func(res authz.AcceptResponse, err error) { diff --git a/proto/ibc/applications/transfer/v1/authz.proto b/proto/ibc/applications/transfer/v1/authz.proto index f525ec6d0f7..1e1525ffc8f 100644 --- a/proto/ibc/applications/transfer/v1/authz.proto +++ b/proto/ibc/applications/transfer/v1/authz.proto @@ -21,7 +21,7 @@ message Allocation { repeated string allow_list = 4; // allow list of packet data keys, an empty list. // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key - repeated string allow_packet_data_list = 5; + repeated string allow_packet_data = 5; } // TransferAuthorization allows the grantee to spend up to spend_limit coins from From 075b9141fda13403d32e9bea7789d0b852ed57e5 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Wed, 6 Dec 2023 23:50:40 +0700 Subject: [PATCH 14/31] move consume gas out of if statement --- modules/apps/transfer/types/transfer_authorization.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 9ec01813a50..6386a2447d4 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -177,12 +177,17 @@ func isAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList return false, err } + if len(jsonObject) > len(allowedPacketDataList) { + return false, fmt.Errorf("packet type greater than packet allow list") + } + gasCostPerIteration := ctx.KVGasConfig().IterNextCostFlat for _, key := range allowedPacketDataList { + ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization") + _, ok := jsonObject[key] if ok { - ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization") delete(jsonObject, key) } } From 062f9eed18d4c15d6a55717c24a0ab724ea8c643 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Wed, 6 Dec 2023 23:53:30 +0700 Subject: [PATCH 15/31] add keys to error --- modules/apps/transfer/types/transfer_authorization.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 6386a2447d4..1b35e5a469a 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/cosmos/gogoproto/proto" + "golang.org/x/exp/maps" errorsmod "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" @@ -193,7 +194,7 @@ func isAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList } if len(jsonObject) != 0 { - return false, fmt.Errorf("not allowed packet data key") + return false, fmt.Errorf("not allowed packet data key %v", maps.Keys(jsonObject)) } return true, nil From a36979e9f6ee5e6734c1f820bd6d83108bb25917 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Thu, 7 Dec 2023 10:25:42 +0100 Subject: [PATCH 16/31] Update 08-authorizations.md --- docs/docs/02-apps/01-transfer/08-authorizations.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/docs/02-apps/01-transfer/08-authorizations.md b/docs/docs/02-apps/01-transfer/08-authorizations.md index 3d91692dd9c..c7cef03d787 100644 --- a/docs/docs/02-apps/01-transfer/08-authorizations.md +++ b/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 `AllowPacketDataList` 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 `AllowPacketData` 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. Setting a `TransferAuthorization` is expected to fail if: @@ -53,7 +53,7 @@ type Allocation struct { AllowList []string // allow list of packet data keys, an empty list. // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key - AllowPacketDataList []string + AllowPacketData []string } ``` From 08b49a630ed72f5d022c869c2979923212cbe227 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Thu, 7 Dec 2023 10:26:04 +0100 Subject: [PATCH 17/31] Update 08-authorizations.md --- docs/docs/02-apps/01-transfer/08-authorizations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/02-apps/01-transfer/08-authorizations.md b/docs/docs/02-apps/01-transfer/08-authorizations.md index c7cef03d787..309a725347c 100644 --- a/docs/docs/02-apps/01-transfer/08-authorizations.md +++ b/docs/docs/02-apps/01-transfer/08-authorizations.md @@ -31,7 +31,7 @@ Setting a `TransferAuthorization` is expected to fail if: - 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 `AllowPacketDataList` +- the `memo` field is not allowed by `AllowPacketData` Below is the `TransferAuthorization` message: From a04a68c636fab442f457ebd0a0a61503346b5cf3 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Thu, 7 Dec 2023 10:28:10 +0100 Subject: [PATCH 18/31] Update transfer_authorization_test.go --- modules/apps/transfer/types/transfer_authorization_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/apps/transfer/types/transfer_authorization_test.go b/modules/apps/transfer/types/transfer_authorization_test.go index 73b2a633585..2b7b0ae31f8 100644 --- a/modules/apps/transfer/types/transfer_authorization_test.go +++ b/modules/apps/transfer/types/transfer_authorization_test.go @@ -118,7 +118,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, }, { - "success: allowPacketDataList allows any packet", + "success: allowPacketData allows any packet", func() { allowedList := []string{"*"} transferAuthz.Allocations[0].AllowPacketData = allowedList @@ -148,7 +148,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, }, { - "empty allowPacketDataList but not empty memo", + "empty allowPacketData but not empty memo", func() { allowedList := []string{} transferAuthz.Allocations[0].AllowPacketData = allowedList From 15c674b2fb716938837bfb06aa400c3442dd6aed Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Thu, 7 Dec 2023 10:32:17 +0100 Subject: [PATCH 19/31] Update transfer_authorization.go --- modules/apps/transfer/types/transfer_authorization.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 1b35e5a469a..02d8b60a902 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -194,7 +194,7 @@ func isAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList } if len(jsonObject) != 0 { - return false, fmt.Errorf("not allowed packet data key %v", maps.Keys(jsonObject)) + return false, fmt.Errorf("not allowed packet data keys: %v", maps.Keys(jsonObject)) } return true, nil From 154a3e0c4e3be5a324ecb6207058858bc01b36f4 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Thu, 7 Dec 2023 10:32:55 +0100 Subject: [PATCH 20/31] Update transfer_authorization_test.go --- modules/apps/transfer/types/transfer_authorization_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/transfer_authorization_test.go b/modules/apps/transfer/types/transfer_authorization_test.go index 2b7b0ae31f8..e3319f61747 100644 --- a/modules/apps/transfer/types/transfer_authorization_test.go +++ b/modules/apps/transfer/types/transfer_authorization_test.go @@ -104,7 +104,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, }, { - "success: empty allowPacketDataList and empty memo", + "success: empty allowPacketData and empty memo", func() { allowedList := []string{} transferAuthz.Allocations[0].AllowPacketData = allowedList From 6cb2914243795424e48f2e3ea84d3b4f1c41f9a4 Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Thu, 7 Dec 2023 11:46:48 +0200 Subject: [PATCH 21/31] Move const outside of function --- modules/apps/transfer/types/transfer_authorization_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/apps/transfer/types/transfer_authorization_test.go b/modules/apps/transfer/types/transfer_authorization_test.go index e3319f61747..1526034c0c5 100644 --- a/modules/apps/transfer/types/transfer_authorization_test.go +++ b/modules/apps/transfer/types/transfer_authorization_test.go @@ -11,14 +11,14 @@ import ( "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"}}}}` + func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { var ( msgTransfer types.MsgTransfer transferAuthz types.TransferAuthorization ) - 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"}}}}` - testCases := []struct { name string malleate func() From 1a1c9e3bd488c1009727a4e7c5231b28ff9274f1 Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Thu, 7 Dec 2023 11:51:35 +0200 Subject: [PATCH 22/31] Update modules/apps/transfer/types/transfer_authorization.go --- modules/apps/transfer/types/transfer_authorization.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 02d8b60a902..ef57c6dd694 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -158,7 +158,7 @@ func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) b return false } -// areAllowedPacketDataKeys returns a boolean indicating if the memo is valid for transfer. If it is not valid and non-nil error is also returned. +// isAllowedPacketDataKeys returns a boolean indicating if the memo is valid for transfer. If it is not valid and non-nil error is also returned. func isAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList []string) (bool, error) { // if the allow list is empty, then the memo must be an empty string if len(allowedPacketDataList) == 0 { From 6f3e2061ff98511e4ba343a27674899b4ba1dfc7 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Fri, 8 Dec 2023 14:45:05 +0700 Subject: [PATCH 23/31] update --- modules/apps/transfer/types/errors.go | 21 +++++++------- .../transfer/types/transfer_authorization.go | 28 +++++++++---------- 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/modules/apps/transfer/types/errors.go b/modules/apps/transfer/types/errors.go index b3e45aa2ae2..74c5d895b07 100644 --- a/modules/apps/transfer/types/errors.go +++ b/modules/apps/transfer/types/errors.go @@ -6,14 +6,15 @@ import ( // IBC transfer sentinel errors var ( - ErrInvalidPacketTimeout = errorsmod.Register(ModuleName, 2, "invalid packet timeout") - ErrInvalidDenomForTransfer = errorsmod.Register(ModuleName, 3, "invalid denomination for cross-chain transfer") - ErrInvalidVersion = errorsmod.Register(ModuleName, 4, "invalid ICS20 version") - ErrInvalidAmount = errorsmod.Register(ModuleName, 5, "invalid token amount") - ErrTraceNotFound = errorsmod.Register(ModuleName, 6, "denomination trace not found") - ErrSendDisabled = errorsmod.Register(ModuleName, 7, "fungible token transfers from this chain are disabled") - ErrReceiveDisabled = errorsmod.Register(ModuleName, 8, "fungible token transfers to this chain are disabled") - ErrMaxTransferChannels = errorsmod.Register(ModuleName, 9, "max transfer channels") - ErrInvalidAuthorization = errorsmod.Register(ModuleName, 10, "invalid transfer authorization") - ErrInvalidMemo = errorsmod.Register(ModuleName, 11, "invalid memo") + ErrInvalidPacketTimeout = errorsmod.Register(ModuleName, 2, "invalid packet timeout") + ErrInvalidDenomForTransfer = errorsmod.Register(ModuleName, 3, "invalid denomination for cross-chain transfer") + ErrInvalidVersion = errorsmod.Register(ModuleName, 4, "invalid ICS20 version") + ErrInvalidAmount = errorsmod.Register(ModuleName, 5, "invalid token amount") + ErrTraceNotFound = errorsmod.Register(ModuleName, 6, "denomination trace not found") + ErrSendDisabled = errorsmod.Register(ModuleName, 7, "fungible token transfers from this chain are disabled") + ErrReceiveDisabled = errorsmod.Register(ModuleName, 8, "fungible token transfers to this chain are disabled") + ErrMaxTransferChannels = errorsmod.Register(ModuleName, 9, "max transfer channels") + ErrInvalidAuthorization = errorsmod.Register(ModuleName, 10, "invalid transfer authorization") + ErrInvalidMemo = errorsmod.Register(ModuleName, 11, "invalid memo") + ErrTransferAuthorizationPacket = errorsmod.Register(ModuleName, 12, "error TransferAuthorizationPacket not allowed") ) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 1b35e5a469a..43fcc54588f 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -3,12 +3,10 @@ package types import ( "context" "encoding/json" - fmt "fmt" "math/big" "strings" "github.com/cosmos/gogoproto/proto" - "golang.org/x/exp/maps" errorsmod "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" @@ -54,15 +52,11 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "not allowed receiver address for transfer") } - isAllowedPacket, err := isAllowedPacketDataKeys(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowPacketData) + err := isAllowedPacketDataKeys(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowPacketData) if err != nil { return authz.AcceptResponse{}, errorsmod.Wrapf(ErrInvalidMemo, "error: %v", err) } - if !isAllowedPacket { - return authz.AcceptResponse{}, ErrInvalidMemo - } - // If the spend limit is set to the MaxUint256 sentinel value, do not subtract the amount from the spend limit. if allocation.SpendLimit.AmountOf(msgTransfer.Token.Denom).Equal(UnboundedSpendLimit()) { return authz.AcceptResponse{Accept: true, Delete: false, Updated: nil}, nil @@ -158,28 +152,32 @@ func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) b return false } -// areAllowedPacketDataKeys returns a boolean indicating if the memo is valid for transfer. If it is not valid and non-nil error is also returned. -func isAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList []string) (bool, error) { +// areAllowedPacketDataKeys returns a nil error indicating if the memo is valid for transfer. +func isAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList []string) error { // if the allow list is empty, then the memo must be an empty string if len(allowedPacketDataList) == 0 { - return len(strings.TrimSpace(memo)) == 0, nil + if len(strings.TrimSpace(memo)) != 0 { + return errorsmod.Wrapf(ErrTransferAuthorizationPacket, "memo not allowed in empty allowedPacketDataList") + } + + return nil } // if allowedPacketDataList has only 1 element and it equals AllowAllPacketDataKeys // then accept all the packet data keys if len(allowedPacketDataList) == 1 && allowedPacketDataList[0] == AllowAllPacketDataKeys { - return true, nil + return nil } // unmarshal memo jsonObject := make(map[string]interface{}) err := json.Unmarshal([]byte(memo), &jsonObject) if err != nil { - return false, err + return err } if len(jsonObject) > len(allowedPacketDataList) { - return false, fmt.Errorf("packet type greater than packet allow list") + return errorsmod.Wrapf(ErrTransferAuthorizationPacket, "packet contains more packet data keys than packet allow list has") } gasCostPerIteration := ctx.KVGasConfig().IterNextCostFlat @@ -194,10 +192,10 @@ func isAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList } if len(jsonObject) != 0 { - return false, fmt.Errorf("not allowed packet data key %v", maps.Keys(jsonObject)) + return errorsmod.Wrapf(ErrTransferAuthorizationPacket, "packet data not allowed") } - return true, nil + return nil } // UnboundedSpendLimit returns the sentinel value that can be used From 4d5cd537dc8cb4086d991a4dbe445550a1da64c8 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Fri, 8 Dec 2023 14:49:45 +0700 Subject: [PATCH 24/31] proto --- docs/docs/02-apps/01-transfer/08-authorizations.md | 6 +++--- modules/apps/transfer/types/transfer_authorization.go | 1 - proto/ibc/applications/transfer/v1/authz.proto | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/docs/02-apps/01-transfer/08-authorizations.md b/docs/docs/02-apps/01-transfer/08-authorizations.md index 309a725347c..3654e5677a8 100644 --- a/docs/docs/02-apps/01-transfer/08-authorizations.md +++ b/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 `AllowPacketData` 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 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. Setting a `TransferAuthorization` is expected to fail if: @@ -31,7 +31,7 @@ Setting a `TransferAuthorization` is expected to fail if: - 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 `AllowPacketData` +- the `memo` field is not allowed by `AllowedPacketData` Below is the `TransferAuthorization` message: @@ -53,7 +53,7 @@ type Allocation struct { AllowList []string // allow list of packet data keys, an empty list. // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key - AllowPacketData []string + AllowedPacketData []string } ``` diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 43fcc54588f..7449c056df1 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -169,7 +169,6 @@ func isAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList return nil } - // unmarshal memo jsonObject := make(map[string]interface{}) err := json.Unmarshal([]byte(memo), &jsonObject) if err != nil { diff --git a/proto/ibc/applications/transfer/v1/authz.proto b/proto/ibc/applications/transfer/v1/authz.proto index 1e1525ffc8f..89621dea5e1 100644 --- a/proto/ibc/applications/transfer/v1/authz.proto +++ b/proto/ibc/applications/transfer/v1/authz.proto @@ -21,7 +21,7 @@ message Allocation { repeated string allow_list = 4; // allow list of packet data keys, an empty list. // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key - repeated string allow_packet_data = 5; + repeated string allowed_packet_data = 5; } // TransferAuthorization allows the grantee to spend up to spend_limit coins from From 6ef45f922472f60e5a476703e145c1a5e928237a Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Fri, 8 Dec 2023 15:19:41 +0700 Subject: [PATCH 25/31] AllowedPacketData --- modules/apps/transfer/types/authz.pb.go | 82 +++++++++---------- .../transfer/types/transfer_authorization.go | 2 +- .../types/transfer_authorization_test.go | 16 ++-- 3 files changed, 50 insertions(+), 50 deletions(-) diff --git a/modules/apps/transfer/types/authz.pb.go b/modules/apps/transfer/types/authz.pb.go index 453ffc26d39..22298d64b63 100644 --- a/modules/apps/transfer/types/authz.pb.go +++ b/modules/apps/transfer/types/authz.pb.go @@ -38,7 +38,7 @@ type Allocation struct { AllowList []string `protobuf:"bytes,4,rep,name=allow_list,json=allowList,proto3" json:"allow_list,omitempty"` // allow list of packet data keys, an empty list. // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key - AllowPacketData []string `protobuf:"bytes,5,rep,name=allow_packet_data,json=allowPacketData,proto3" json:"allow_packet_data,omitempty"` + AllowedPacketData []string `protobuf:"bytes,5,rep,name=allowed_packet_data,json=allowedPacketData,proto3" json:"allowed_packet_data,omitempty"` } func (m *Allocation) Reset() { *m = Allocation{} } @@ -102,9 +102,9 @@ func (m *Allocation) GetAllowList() []string { return nil } -func (m *Allocation) GetAllowPacketData() []string { +func (m *Allocation) GetAllowedPacketData() []string { if m != nil { - return m.AllowPacketData + return m.AllowedPacketData } return nil } @@ -166,35 +166,35 @@ func init() { } var fileDescriptor_b1a28b55d17325aa = []byte{ - // 435 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x92, 0xc1, 0x8e, 0xd3, 0x30, - 0x10, 0x86, 0x9b, 0xed, 0x82, 0x54, 0x57, 0x80, 0x88, 0x40, 0xca, 0xae, 0x20, 0xad, 0x2a, 0x81, - 0x22, 0xa4, 0xda, 0x14, 0x0e, 0x20, 0x6e, 0xdb, 0xe5, 0xb8, 0x87, 0x52, 0x71, 0xe2, 0x12, 0x39, - 0xae, 0x69, 0xac, 0x75, 0x32, 0x51, 0x3c, 0x09, 0x62, 0xdf, 0x01, 0x09, 0x5e, 0x83, 0x33, 0x0f, - 0xb1, 0xe2, 0xb4, 0x47, 0x4e, 0x80, 0xda, 0x17, 0x41, 0xb1, 0x5d, 0x28, 0x42, 0xe2, 0x94, 0xf8, - 0x9f, 0xdf, 0xe3, 0x99, 0x6f, 0x86, 0x24, 0x2a, 0x13, 0x8c, 0x57, 0x95, 0x56, 0x82, 0xa3, 0x82, - 0xd2, 0x30, 0xac, 0x79, 0x69, 0xde, 0xca, 0x9a, 0xb5, 0x33, 0xc6, 0x1b, 0xcc, 0x2f, 0x68, 0x55, - 0x03, 0x42, 0x78, 0x4f, 0x65, 0x82, 0xee, 0x3b, 0xe9, 0xce, 0x49, 0xdb, 0xd9, 0xf1, 0x91, 0x00, - 0x53, 0x80, 0x49, 0xad, 0x97, 0xb9, 0x83, 0xbb, 0x78, 0x7c, 0x67, 0x0d, 0x6b, 0x70, 0x7a, 0xf7, - 0xe7, 0xd5, 0xd8, 0x79, 0x58, 0xc6, 0x8d, 0x64, 0xed, 0x2c, 0x93, 0xc8, 0x67, 0x4c, 0x80, 0x2a, - 0x5d, 0x7c, 0xf2, 0xe1, 0x80, 0x90, 0x13, 0xad, 0xc1, 0x3d, 0x16, 0x8e, 0xc8, 0xd0, 0x40, 0x53, - 0x0b, 0x99, 0x56, 0x50, 0x63, 0x14, 0x8c, 0x83, 0x64, 0xb0, 0x24, 0x4e, 0x5a, 0x40, 0x8d, 0xe1, - 0x03, 0x72, 0xd3, 0x1b, 0x44, 0xce, 0xcb, 0x52, 0xea, 0xe8, 0xc0, 0x7a, 0x6e, 0x38, 0xf5, 0xd4, - 0x89, 0xa1, 0x26, 0x43, 0x53, 0xc9, 0x72, 0x95, 0x6a, 0x55, 0x28, 0x8c, 0xfa, 0xe3, 0x7e, 0x32, - 0x7c, 0x72, 0x44, 0x7d, 0xc1, 0x5d, 0x31, 0xd4, 0x17, 0x43, 0x4f, 0x41, 0x95, 0xf3, 0xc7, 0x97, - 0xdf, 0x47, 0xbd, 0xcf, 0x3f, 0x46, 0xc9, 0x5a, 0x61, 0xde, 0x64, 0x54, 0x40, 0xe1, 0xbb, 0xf3, - 0x9f, 0xa9, 0x59, 0x9d, 0x33, 0x7c, 0x5f, 0x49, 0x63, 0x2f, 0x98, 0x25, 0xb1, 0xf9, 0xcf, 0xba, - 0xf4, 0xe1, 0x7d, 0x42, 0xb8, 0xd6, 0xf0, 0x2e, 0xd5, 0xca, 0x60, 0x74, 0x38, 0xee, 0x27, 0x83, - 0xe5, 0xc0, 0x2a, 0x67, 0xca, 0x60, 0xf8, 0x88, 0xdc, 0x76, 0xe1, 0x8a, 0x8b, 0x73, 0x89, 0xe9, - 0x8a, 0x23, 0x8f, 0xae, 0x59, 0xd7, 0x2d, 0x1b, 0x58, 0x58, 0xfd, 0x25, 0x47, 0x3e, 0xf9, 0x14, - 0x90, 0xbb, 0xaf, 0x3d, 0xf0, 0x93, 0x06, 0x73, 0xa8, 0xd5, 0x85, 0x43, 0xb3, 0x20, 0x43, 0xfe, - 0x1b, 0x94, 0x89, 0x02, 0xdb, 0x52, 0x42, 0xff, 0x37, 0x2e, 0xfa, 0x87, 0xec, 0xfc, 0xb0, 0xeb, - 0x70, 0xb9, 0x9f, 0xe2, 0xc5, 0xc3, 0xaf, 0x5f, 0xa6, 0x13, 0x8f, 0xc4, 0xad, 0xc0, 0x8e, 0xc9, - 0x5f, 0x2f, 0xcf, 0x5f, 0x5d, 0x6e, 0xe2, 0xe0, 0x6a, 0x13, 0x07, 0x3f, 0x37, 0x71, 0xf0, 0x71, - 0x1b, 0xf7, 0xae, 0xb6, 0x71, 0xef, 0xdb, 0x36, 0xee, 0xbd, 0x79, 0xf6, 0x2f, 0x2e, 0x95, 0x89, - 0xe9, 0x1a, 0x58, 0xfb, 0x9c, 0x15, 0xb0, 0x6a, 0xb4, 0x34, 0xdd, 0xda, 0xed, 0xad, 0x9b, 0x65, - 0x98, 0x5d, 0xb7, 0xd3, 0x7f, 0xfa, 0x2b, 0x00, 0x00, 0xff, 0xff, 0x58, 0xa3, 0xbf, 0x1b, 0x98, - 0x02, 0x00, 0x00, + // 436 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x92, 0xcf, 0x8e, 0xd3, 0x30, + 0x10, 0xc6, 0x9b, 0xed, 0x82, 0x54, 0x57, 0x20, 0x11, 0x40, 0xca, 0xae, 0x20, 0xad, 0x2a, 0x81, + 0x72, 0xa9, 0x4d, 0xe1, 0x00, 0xe2, 0xb6, 0x5d, 0x8e, 0x7b, 0x28, 0x15, 0x27, 0x2e, 0x91, 0xe3, + 0x98, 0xd6, 0x5a, 0x27, 0x13, 0xc5, 0x93, 0x20, 0xf6, 0x29, 0xd8, 0xd7, 0xe0, 0xcc, 0x43, 0xac, + 0x38, 0xed, 0x91, 0x13, 0xa0, 0xf6, 0x45, 0x50, 0x6c, 0x17, 0x8a, 0x90, 0xf6, 0x94, 0xf8, 0x9b, + 0xcf, 0xf3, 0xe7, 0xe7, 0x21, 0x89, 0xca, 0x04, 0xe3, 0x55, 0xa5, 0x95, 0xe0, 0xa8, 0xa0, 0x34, + 0x0c, 0x6b, 0x5e, 0x9a, 0x0f, 0xb2, 0x66, 0xed, 0x8c, 0xf1, 0x06, 0xd7, 0x17, 0xb4, 0xaa, 0x01, + 0x21, 0x7c, 0xa4, 0x32, 0x41, 0xf7, 0x9d, 0x74, 0xe7, 0xa4, 0xed, 0xec, 0xf8, 0x48, 0x80, 0x29, + 0xc0, 0xa4, 0xd6, 0xcb, 0xdc, 0xc1, 0x5d, 0x3c, 0x7e, 0xb0, 0x82, 0x15, 0x38, 0xbd, 0xfb, 0xf3, + 0x6a, 0xec, 0x3c, 0x2c, 0xe3, 0x46, 0xb2, 0x76, 0x96, 0x49, 0xe4, 0x33, 0x26, 0x40, 0x95, 0x2e, + 0x3e, 0xb9, 0x3c, 0x20, 0xe4, 0x44, 0x6b, 0x70, 0xc5, 0xc2, 0x11, 0x19, 0x1a, 0x68, 0x6a, 0x21, + 0xd3, 0x0a, 0x6a, 0x8c, 0x82, 0x71, 0x90, 0x0c, 0x96, 0xc4, 0x49, 0x0b, 0xa8, 0x31, 0x7c, 0x42, + 0xee, 0x7a, 0x83, 0x58, 0xf3, 0xb2, 0x94, 0x3a, 0x3a, 0xb0, 0x9e, 0x3b, 0x4e, 0x3d, 0x75, 0x62, + 0xa8, 0xc9, 0xd0, 0x54, 0xb2, 0xcc, 0x53, 0xad, 0x0a, 0x85, 0x51, 0x7f, 0xdc, 0x4f, 0x86, 0xcf, + 0x8f, 0xa8, 0x6f, 0xb8, 0x6b, 0x86, 0xfa, 0x66, 0xe8, 0x29, 0xa8, 0x72, 0xfe, 0xec, 0xea, 0xc7, + 0xa8, 0xf7, 0xe5, 0xe7, 0x28, 0x59, 0x29, 0x5c, 0x37, 0x19, 0x15, 0x50, 0xf8, 0xe9, 0xfc, 0x67, + 0x6a, 0xf2, 0x73, 0x86, 0x9f, 0x2a, 0x69, 0xec, 0x05, 0xb3, 0x24, 0x36, 0xff, 0x59, 0x97, 0x3e, + 0x7c, 0x4c, 0x08, 0xd7, 0x1a, 0x3e, 0xa6, 0x5a, 0x19, 0x8c, 0x0e, 0xc7, 0xfd, 0x64, 0xb0, 0x1c, + 0x58, 0xe5, 0x4c, 0x19, 0x0c, 0x29, 0xb9, 0x6f, 0x0f, 0x32, 0x4f, 0x2b, 0x2e, 0xce, 0x25, 0xa6, + 0x39, 0x47, 0x1e, 0xdd, 0xb2, 0xbe, 0x7b, 0x3e, 0xb4, 0xb0, 0x91, 0x37, 0x1c, 0xf9, 0xe4, 0x32, + 0x20, 0x0f, 0xdf, 0x79, 0xe8, 0x27, 0x0d, 0xae, 0xa1, 0x56, 0x17, 0x0e, 0xcf, 0x82, 0x0c, 0xf9, + 0x1f, 0x58, 0x26, 0x0a, 0xec, 0x58, 0x09, 0xbd, 0xe9, 0xc9, 0xe8, 0x5f, 0xba, 0xf3, 0xc3, 0x6e, + 0xca, 0xe5, 0x7e, 0x8a, 0xd7, 0x4f, 0xbf, 0x7d, 0x9d, 0x4e, 0x3c, 0x16, 0xb7, 0x06, 0x3b, 0x2e, + 0xff, 0x54, 0x9e, 0xbf, 0xbd, 0xda, 0xc4, 0xc1, 0xf5, 0x26, 0x0e, 0x7e, 0x6d, 0xe2, 0xe0, 0xf3, + 0x36, 0xee, 0x5d, 0x6f, 0xe3, 0xde, 0xf7, 0x6d, 0xdc, 0x7b, 0xff, 0xf2, 0x7f, 0x64, 0x2a, 0x13, + 0xd3, 0x15, 0xb0, 0xf6, 0x15, 0x2b, 0x20, 0x6f, 0xb4, 0x34, 0xdd, 0xea, 0xed, 0xad, 0x9c, 0xe5, + 0x98, 0xdd, 0xb6, 0x1b, 0xf0, 0xe2, 0x77, 0x00, 0x00, 0x00, 0xff, 0xff, 0xcc, 0xb7, 0x0f, 0x97, + 0x9c, 0x02, 0x00, 0x00, } func (m *Allocation) Marshal() (dAtA []byte, err error) { @@ -217,11 +217,11 @@ func (m *Allocation) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l - if len(m.AllowPacketData) > 0 { - for iNdEx := len(m.AllowPacketData) - 1; iNdEx >= 0; iNdEx-- { - i -= len(m.AllowPacketData[iNdEx]) - copy(dAtA[i:], m.AllowPacketData[iNdEx]) - i = encodeVarintAuthz(dAtA, i, uint64(len(m.AllowPacketData[iNdEx]))) + if len(m.AllowedPacketData) > 0 { + for iNdEx := len(m.AllowedPacketData) - 1; iNdEx >= 0; iNdEx-- { + i -= len(m.AllowedPacketData[iNdEx]) + copy(dAtA[i:], m.AllowedPacketData[iNdEx]) + i = encodeVarintAuthz(dAtA, i, uint64(len(m.AllowedPacketData[iNdEx]))) i-- dAtA[i] = 0x2a } @@ -340,8 +340,8 @@ func (m *Allocation) Size() (n int) { n += 1 + l + sovAuthz(uint64(l)) } } - if len(m.AllowPacketData) > 0 { - for _, s := range m.AllowPacketData { + if len(m.AllowedPacketData) > 0 { + for _, s := range m.AllowedPacketData { l = len(s) n += 1 + l + sovAuthz(uint64(l)) } @@ -531,7 +531,7 @@ func (m *Allocation) Unmarshal(dAtA []byte) error { iNdEx = postIndex case 5: if wireType != 2 { - return fmt.Errorf("proto: wrong wireType = %d for field AllowPacketData", wireType) + return fmt.Errorf("proto: wrong wireType = %d for field AllowedPacketData", wireType) } var stringLen uint64 for shift := uint(0); ; shift += 7 { @@ -559,7 +559,7 @@ func (m *Allocation) Unmarshal(dAtA []byte) error { if postIndex > l { return io.ErrUnexpectedEOF } - m.AllowPacketData = append(m.AllowPacketData, string(dAtA[iNdEx:postIndex])) + m.AllowedPacketData = append(m.AllowedPacketData, string(dAtA[iNdEx:postIndex])) iNdEx = postIndex default: iNdEx = preIndex diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 7449c056df1..287ea5a1b09 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -52,7 +52,7 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "not allowed receiver address for transfer") } - err := isAllowedPacketDataKeys(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowPacketData) + err := isAllowedPacketDataKeys(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowedPacketData) if err != nil { return authz.AcceptResponse{}, errorsmod.Wrapf(ErrInvalidMemo, "error: %v", err) } diff --git a/modules/apps/transfer/types/transfer_authorization_test.go b/modules/apps/transfer/types/transfer_authorization_test.go index 1526034c0c5..1aa07438807 100644 --- a/modules/apps/transfer/types/transfer_authorization_test.go +++ b/modules/apps/transfer/types/transfer_authorization_test.go @@ -104,10 +104,10 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, }, { - "success: empty allowPacketData and empty memo", + "success: empty AllowedPacketData and empty memo", func() { allowedList := []string{} - transferAuthz.Allocations[0].AllowPacketData = allowedList + transferAuthz.Allocations[0].AllowedPacketData = allowedList }, func(res authz.AcceptResponse, err error) { suite.Require().NoError(err) @@ -118,10 +118,10 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, }, { - "success: allowPacketData allows any packet", + "success: AllowedPacketData allows any packet", func() { allowedList := []string{"*"} - transferAuthz.Allocations[0].AllowPacketData = allowedList + transferAuthz.Allocations[0].AllowedPacketData = allowedList msgTransfer.Memo = testMemo }, func(res authz.AcceptResponse, err error) { @@ -136,7 +136,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { "success: transfer memo allowed", func() { allowedList := []string{"wasm", "forward"} - transferAuthz.Allocations[0].AllowPacketData = allowedList + transferAuthz.Allocations[0].AllowedPacketData = allowedList msgTransfer.Memo = testMemo }, func(res authz.AcceptResponse, err error) { @@ -148,10 +148,10 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, }, { - "empty allowPacketData but not empty memo", + "empty AllowedPacketData but not empty memo", func() { allowedList := []string{} - transferAuthz.Allocations[0].AllowPacketData = allowedList + transferAuthz.Allocations[0].AllowedPacketData = allowedList msgTransfer.Memo = testMemo }, func(res authz.AcceptResponse, err error) { @@ -162,7 +162,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { "memo not allowed", func() { allowedList := []string{"forward"} - transferAuthz.Allocations[0].AllowPacketData = allowedList + transferAuthz.Allocations[0].AllowedPacketData = allowedList msgTransfer.Memo = testMemo }, func(res authz.AcceptResponse, err error) { From 4bc9d0013a322848c5d2b17ae312d013087d8043 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Fri, 8 Dec 2023 15:21:39 +0700 Subject: [PATCH 26/31] validate memo --- modules/apps/transfer/types/transfer_authorization.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 287ea5a1b09..00e82b11bd4 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -52,7 +52,7 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "not allowed receiver address for transfer") } - err := isAllowedPacketDataKeys(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowedPacketData) + err := validateMemo(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowedPacketData) if err != nil { return authz.AcceptResponse{}, errorsmod.Wrapf(ErrInvalidMemo, "error: %v", err) } @@ -153,7 +153,7 @@ func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) b } // areAllowedPacketDataKeys returns a nil error indicating if the memo is valid for transfer. -func isAllowedPacketDataKeys(ctx sdk.Context, memo string, allowedPacketDataList []string) error { +func validateMemo(ctx sdk.Context, memo string, allowedPacketDataList []string) error { // if the allow list is empty, then the memo must be an empty string if len(allowedPacketDataList) == 0 { if len(strings.TrimSpace(memo)) != 0 { From 481489ed4c730eba8ec7e4f135fa12c33b4cd3a9 Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Fri, 8 Dec 2023 22:35:40 +0700 Subject: [PATCH 27/31] nit --- modules/apps/transfer/types/transfer_authorization.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 00e82b11bd4..3bfdcdf62f6 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -152,7 +152,7 @@ func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) b return false } -// areAllowedPacketDataKeys returns a nil error indicating if the memo is valid for transfer. +// validateMemo returns a nil error indicating if the memo is valid for transfer. func validateMemo(ctx sdk.Context, memo string, allowedPacketDataList []string) error { // if the allow list is empty, then the memo must be an empty string if len(allowedPacketDataList) == 0 { From a74aaa2a0dbb5a028af2c8c0120b5b36183276bb Mon Sep 17 00:00:00 2001 From: GnaD13 Date: Mon, 11 Dec 2023 22:08:56 +0700 Subject: [PATCH 28/31] add allowed packet data to allocation --- modules/apps/transfer/types/errors.go | 21 +++++++++---------- .../transfer/types/transfer_authorization.go | 15 ++++++------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/modules/apps/transfer/types/errors.go b/modules/apps/transfer/types/errors.go index 74c5d895b07..b3e45aa2ae2 100644 --- a/modules/apps/transfer/types/errors.go +++ b/modules/apps/transfer/types/errors.go @@ -6,15 +6,14 @@ import ( // IBC transfer sentinel errors var ( - ErrInvalidPacketTimeout = errorsmod.Register(ModuleName, 2, "invalid packet timeout") - ErrInvalidDenomForTransfer = errorsmod.Register(ModuleName, 3, "invalid denomination for cross-chain transfer") - ErrInvalidVersion = errorsmod.Register(ModuleName, 4, "invalid ICS20 version") - ErrInvalidAmount = errorsmod.Register(ModuleName, 5, "invalid token amount") - ErrTraceNotFound = errorsmod.Register(ModuleName, 6, "denomination trace not found") - ErrSendDisabled = errorsmod.Register(ModuleName, 7, "fungible token transfers from this chain are disabled") - ErrReceiveDisabled = errorsmod.Register(ModuleName, 8, "fungible token transfers to this chain are disabled") - ErrMaxTransferChannels = errorsmod.Register(ModuleName, 9, "max transfer channels") - ErrInvalidAuthorization = errorsmod.Register(ModuleName, 10, "invalid transfer authorization") - ErrInvalidMemo = errorsmod.Register(ModuleName, 11, "invalid memo") - ErrTransferAuthorizationPacket = errorsmod.Register(ModuleName, 12, "error TransferAuthorizationPacket not allowed") + ErrInvalidPacketTimeout = errorsmod.Register(ModuleName, 2, "invalid packet timeout") + ErrInvalidDenomForTransfer = errorsmod.Register(ModuleName, 3, "invalid denomination for cross-chain transfer") + ErrInvalidVersion = errorsmod.Register(ModuleName, 4, "invalid ICS20 version") + ErrInvalidAmount = errorsmod.Register(ModuleName, 5, "invalid token amount") + ErrTraceNotFound = errorsmod.Register(ModuleName, 6, "denomination trace not found") + ErrSendDisabled = errorsmod.Register(ModuleName, 7, "fungible token transfers from this chain are disabled") + ErrReceiveDisabled = errorsmod.Register(ModuleName, 8, "fungible token transfers to this chain are disabled") + ErrMaxTransferChannels = errorsmod.Register(ModuleName, 9, "max transfer channels") + ErrInvalidAuthorization = errorsmod.Register(ModuleName, 10, "invalid transfer authorization") + ErrInvalidMemo = errorsmod.Register(ModuleName, 11, "invalid memo") ) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 3bfdcdf62f6..9ef0e9fa7e7 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -77,10 +77,11 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a }}, nil } a.Allocations[index] = Allocation{ - SourcePort: allocation.SourcePort, - SourceChannel: allocation.SourceChannel, - SpendLimit: limitLeft, - AllowList: allocation.AllowList, + SourcePort: allocation.SourcePort, + SourceChannel: allocation.SourceChannel, + SpendLimit: limitLeft, + AllowList: allocation.AllowList, + AllowedPacketData: allocation.AllowedPacketData, } return authz.AcceptResponse{Accept: true, Delete: false, Updated: &TransferAuthorization{ @@ -157,7 +158,7 @@ func validateMemo(ctx sdk.Context, memo string, allowedPacketDataList []string) // if the allow list is empty, then the memo must be an empty string if len(allowedPacketDataList) == 0 { if len(strings.TrimSpace(memo)) != 0 { - return errorsmod.Wrapf(ErrTransferAuthorizationPacket, "memo not allowed in empty allowedPacketDataList") + return errorsmod.Wrapf(ErrInvalidAuthorization, "memo not allowed in empty allowedPacketDataList") } return nil @@ -176,7 +177,7 @@ func validateMemo(ctx sdk.Context, memo string, allowedPacketDataList []string) } if len(jsonObject) > len(allowedPacketDataList) { - return errorsmod.Wrapf(ErrTransferAuthorizationPacket, "packet contains more packet data keys than packet allow list has") + return errorsmod.Wrapf(ErrInvalidAuthorization, "packet contains more packet data keys than packet allow list has") } gasCostPerIteration := ctx.KVGasConfig().IterNextCostFlat @@ -191,7 +192,7 @@ func validateMemo(ctx sdk.Context, memo string, allowedPacketDataList []string) } if len(jsonObject) != 0 { - return errorsmod.Wrapf(ErrTransferAuthorizationPacket, "packet data not allowed") + return errorsmod.Wrapf(ErrInvalidAuthorization, "packet data not allowed") } return nil From 7d00786ff809f6a66146235a3786432dcab6859d Mon Sep 17 00:00:00 2001 From: GnaD <89174180+GNaD13@users.noreply.github.com> Date: Tue, 12 Dec 2023 09:28:56 +0700 Subject: [PATCH 29/31] Update modules/apps/transfer/types/transfer_authorization.go Co-authored-by: Damian Nolan --- modules/apps/transfer/types/transfer_authorization.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 9ef0e9fa7e7..6e42028ffd5 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -54,7 +54,7 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a err := validateMemo(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowedPacketData) if err != nil { - return authz.AcceptResponse{}, errorsmod.Wrapf(ErrInvalidMemo, "error: %v", err) + return authz.AcceptResponse{}, err } // If the spend limit is set to the MaxUint256 sentinel value, do not subtract the amount from the spend limit. From 5673453f0e42309e138104a4f9dae411e2e9a7bb Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Wed, 13 Dec 2023 10:54:12 +0100 Subject: [PATCH 30/31] change error string --- modules/apps/transfer/types/transfer_authorization.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 6e42028ffd5..d2a10736fc7 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -158,7 +158,7 @@ func validateMemo(ctx sdk.Context, memo string, allowedPacketDataList []string) // if the allow list is empty, then the memo must be an empty string if len(allowedPacketDataList) == 0 { if len(strings.TrimSpace(memo)) != 0 { - return errorsmod.Wrapf(ErrInvalidAuthorization, "memo not allowed in empty allowedPacketDataList") + return errorsmod.Wrapf(ErrInvalidAuthorization, "memo must be empty because allowed packet data in allocation is empty") } return nil From b003428876f2b20e11c92eb65bbebb5ad84aa928 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Wed, 13 Dec 2023 11:05:44 +0100 Subject: [PATCH 31/31] Apply suggestions from code review Co-authored-by: Damian Nolan --- docs/docs/02-apps/01-transfer/08-authorizations.md | 4 ++-- proto/ibc/applications/transfer/v1/authz.proto | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/docs/02-apps/01-transfer/08-authorizations.md b/docs/docs/02-apps/01-transfer/08-authorizations.md index 3654e5677a8..9dde26c1867 100644 --- a/docs/docs/02-apps/01-transfer/08-authorizations.md +++ b/docs/docs/02-apps/01-transfer/08-authorizations.md @@ -51,8 +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. - // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key + // allow list of packet data keys, an empty list prohibits all packet data keys; + // a list only with "*" permits any packet data key AllowedPacketData []string } diff --git a/proto/ibc/applications/transfer/v1/authz.proto b/proto/ibc/applications/transfer/v1/authz.proto index 89621dea5e1..e7561b0708b 100644 --- a/proto/ibc/applications/transfer/v1/authz.proto +++ b/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. - // an empty list prohibits all packet data keys; a list only with "*" permits any packet data key + // allow list of packet data keys, an empty list prohibits all packet data keys; + // a list only with "*" permits any packet data key repeated string allowed_packet_data = 5; }