Skip to content

Commit

Permalink
feat(statemachine)!: add list of allowed packet data keys to `Allocat…
Browse files Browse the repository at this point in the history
…ion` of `TransferAuthorization` (#5280)

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
Co-authored-by: Long <77162856+mgl2150@users.noreply.github.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: Du Nguyen <61083705+lichdu29@users.noreply.github.com>
Co-authored-by: Đỗ Việt Hoàng <hoangdv2429@gmail.com>
  • Loading branch information
7 people committed Dec 14, 2023
1 parent 549fd90 commit db11b9d
Show file tree
Hide file tree
Showing 6 changed files with 223 additions and 31 deletions.
6 changes: 6 additions & 0 deletions docs/docs/02-apps/01-transfer/08-authorizations.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ It takes:

- an `AllowList` list that specifies the list of addresses that are allowed to receive funds. If this list is empty, then all addresses are allowed to receive funds from the `TransferAuthorization`.

- an `AllowedPacketData` list that specifies the list of memo packet data keys that are allowed to send the packet. If this list is empty, then only an empty memo is allowed (a `memo` field with non-empty content will be denied). If this list includes a single element equal to `"*"`, then any content in `memo` field will be allowed.

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:

Expand All @@ -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 prohibits all packet data keys;
// a list only with "*" permits any packet data key
AllowedPacketData []string
}

```
113 changes: 86 additions & 27 deletions modules/apps/transfer/types/authz.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions modules/apps/transfer/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
61 changes: 57 additions & 4 deletions modules/apps/transfer/types/transfer_authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package types

import (
"context"
"encoding/json"
"math/big"
"strings"

"github.com/cosmos/gogoproto/proto"

Expand Down Expand Up @@ -50,6 +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")
}

err := validateMemo(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowedPacketData)
if err != nil {
return authz.AcceptResponse{}, err
}

// 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
Expand All @@ -70,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{
Expand Down Expand Up @@ -145,6 +153,51 @@ func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) b
return false
}

// 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 {
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 packet data keys
if len(allowedPacketDataList) == 1 && allowedPacketDataList[0] == AllowAllPacketDataKeys {
return nil
}

jsonObject := make(map[string]interface{})
err := json.Unmarshal([]byte(memo), &jsonObject)
if err != nil {
return err
}

if len(jsonObject) > len(allowedPacketDataList) {
return errorsmod.Wrapf(ErrInvalidAuthorization, "packet contains more packet data keys than packet allow list has")
}

gasCostPerIteration := ctx.KVGasConfig().IterNextCostFlat

for _, key := range allowedPacketDataList {
ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization")

_, ok := jsonObject[key]
if ok {
delete(jsonObject, key)
}
}

if len(jsonObject) != 0 {
return errorsmod.Wrapf(ErrInvalidAuthorization, "packet data not allowed")
}

return 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
Expand Down
68 changes: 68 additions & 0 deletions modules/apps/transfer/types/transfer_authorization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ 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
Expand Down Expand Up @@ -101,6 +103,72 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() {
suite.Require().Nil(res.Updated)
},
},
{
"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 = testMemo
},
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].AllowedPacketData = allowedList
msgTransfer.Memo = testMemo
},
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 = testMemo
},
func(res authz.AcceptResponse, err error) {
suite.Require().Error(err)
},
},
{
"memo not allowed",
func() {
allowedList := []string{"forward"}
transferAuthz.Allocations[0].AllowedPacketData = allowedList
msgTransfer.Memo = testMemo
},
func(res authz.AcceptResponse, err error) {
suite.Require().Error(err)
},
},
{
"test multiple coins does not overspend",
func() {
Expand Down
3 changes: 3 additions & 0 deletions proto/ibc/applications/transfer/v1/authz.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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 prohibits all packet data keys;
// a list only with "*" permits any packet data key
repeated string allowed_packet_data = 5;
}

// TransferAuthorization allows the grantee to spend up to spend_limit coins from
Expand Down

0 comments on commit db11b9d

Please sign in to comment.