diff --git a/docs/apps/transfer/authorizations.md b/docs/apps/transfer/authorizations.md index 6dbbafab59b..83eec2ad734 100644 --- a/docs/apps/transfer/authorizations.md +++ b/docs/apps/transfer/authorizations.md @@ -1,6 +1,6 @@ # `TransferAuthorization` -`TransferAuthorization` implements the `Authorization` interface for `ibc.applications.transfer.v1.MsgTransfer`. It allows a granter to grant a grantee the privilege to submit MsgTransfer on its behalf. Please see the [Cosmos SDK docs](https://docs.cosmos.network/v0.47/modules/authz) for more details on granting privileges via the `x/authz` module. +`TransferAuthorization` implements the `Authorization` interface for `ibc.applications.transfer.v1.MsgTransfer`. It allows a granter to grant a grantee the privilege to submit `MsgTransfer` on its behalf. Please see the [Cosmos SDK docs](https://docs.cosmos.network/v0.47/modules/authz) for more details on granting privileges via the `x/authz` module. More specifically, the granter allows the grantee to transfer funds that belong to the granter over a specified channel. @@ -13,7 +13,7 @@ It takes: - a `SourcePort` and a `SourceChannel` which together comprise the unique transfer channel identifier over which authorized funds can be transferred. -- a `SpendLimit` that specifies the maximum amount of tokens the grantee can spend. The `SpendLimit` is updated as the tokens are spent. This `SpendLimit` may also be updated to increase or decrease the limit as the granter wishes. +- a `SpendLimit` that specifies the maximum amount of tokens the grantee can transfer. The `SpendLimit` is updated as the tokens are transfered, unless the sentinel value of the maximum value for a 256-bit unsigned integer (i.e. 2^256 - 1) is used for the amount, in which case the `SpendLimit` will not be updated (please be aware that using this sentinel value will grant the grantee the privilege to transfer **all** the tokens of a given denomination available at the granter's account). The helper function `UnboundedSpendLimit` in the `types` package of the `transfer` module provides the sentinel value that can be used. This `SpendLimit` may also be updated to increase or decrease the limit as the granter wishes. - an `AllowList` list that specifies the list of addresses that are allowed to receive funds. If this list is empty, then all addresses are allowed to receive funds from the `TransferAuthorization`. diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index b9f550d6f94..5e3ab9495f4 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -1,6 +1,11 @@ package types import ( + "math/big" + + errorsmod "cosmossdk.io/errors" + sdkmath "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/authz" @@ -9,10 +14,11 @@ import ( host "github.com/cosmos/ibc-go/v6/modules/core/24-host" ) -const gasCostPerIteration = uint64(10) - var _ authz.Authorization = &TransferAuthorization{} +// maxUint256 is the maximum value for a 256 bit unsigned integer. +var maxUint256 = new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 256), big.NewInt(1)) + // NewTransferAuthorization creates a new TransferAuthorization object. func NewTransferAuthorization(allocations ...Allocation) *TransferAuthorization { return &TransferAuthorization{ @@ -33,37 +39,45 @@ func (a TransferAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.Accep } for index, allocation := range a.Allocations { - if allocation.SourceChannel == msgTransfer.SourceChannel && allocation.SourcePort == msgTransfer.SourcePort { - limitLeft, isNegative := allocation.SpendLimit.SafeSub(msgTransfer.Token) - if isNegative { - return authz.AcceptResponse{}, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "requested amount is more than spend limit") - } + if !(allocation.SourceChannel == msgTransfer.SourceChannel && allocation.SourcePort == msgTransfer.SourcePort) { + continue + } - if !isAllowedAddress(ctx, msgTransfer.Receiver, allocation.AllowList) { - return authz.AcceptResponse{}, sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "not allowed address for transfer") - } + if !isAllowedAddress(ctx, msgTransfer.Receiver, allocation.AllowList) { + return authz.AcceptResponse{}, errorsmod.Wrap(sdkerrors.ErrInvalidAddress, "not allowed receiver address for transfer") + } - if limitLeft.IsZero() { - a.Allocations = append(a.Allocations[:index], a.Allocations[index+1:]...) - if len(a.Allocations) == 0 { - return authz.AcceptResponse{Accept: true, Delete: true}, nil - } - return authz.AcceptResponse{Accept: true, Delete: false, Updated: &TransferAuthorization{ - Allocations: a.Allocations, - }}, nil - } - a.Allocations[index] = Allocation{ - SourcePort: allocation.SourcePort, - SourceChannel: allocation.SourceChannel, - SpendLimit: limitLeft, - AllowList: allocation.AllowList, - } + // 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: &a}, nil + } + + limitLeft, isNegative := allocation.SpendLimit.SafeSub(msgTransfer.Token) + if isNegative { + return authz.AcceptResponse{}, errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, "requested amount is more than spend limit") + } + if limitLeft.IsZero() { + a.Allocations = append(a.Allocations[:index], a.Allocations[index+1:]...) + if len(a.Allocations) == 0 { + return authz.AcceptResponse{Accept: true, Delete: true}, nil + } return authz.AcceptResponse{Accept: true, Delete: false, Updated: &TransferAuthorization{ Allocations: a.Allocations, }}, nil } + a.Allocations[index] = Allocation{ + SourcePort: allocation.SourcePort, + SourceChannel: allocation.SourceChannel, + SpendLimit: limitLeft, + AllowList: allocation.AllowList, + } + + return authz.AcceptResponse{Accept: true, Delete: false, Updated: &TransferAuthorization{ + Allocations: a.Allocations, + }}, nil } + return authz.AcceptResponse{}, sdkerrors.Wrapf(sdkerrors.ErrNotFound, "requested port and channel allocation does not exist") } @@ -117,6 +131,8 @@ func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) b return true } + gasCostPerIteration := ctx.KVGasConfig().IterNextCostFlat + for _, addr := range allowedAddrs { ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization") if addr == receiver { @@ -125,3 +141,12 @@ func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) b } return false } + +// 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 +// will be granted the privilege to do ICS20 token transfers for the total amount +// of the denomination available at the granter's account. +func UnboundedSpendLimit() sdkmath.Int { + return sdk.NewIntFromBigInt(maxUint256) +} diff --git a/modules/apps/transfer/types/transfer_authorization_test.go b/modules/apps/transfer/types/transfer_authorization_test.go index 2e2f51bd81a..330260e9e25 100644 --- a/modules/apps/transfer/types/transfer_authorization_test.go +++ b/modules/apps/transfer/types/transfer_authorization_test.go @@ -86,6 +86,48 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { suite.Require().Len(updatedAuthz.Allocations, 1) }, }, + { + "success: with unlimited spend limit of max uint256", + func() { + transferAuthz.Allocations[0].SpendLimit = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, types.UnboundedSpendLimit())) + }, + func(res authz.AcceptResponse, err error) { + suite.Require().NoError(err) + + updatedTransferAuthz, ok := res.Updated.(*types.TransferAuthorization) + suite.Require().True(ok) + + remainder := updatedTransferAuthz.Allocations[0].SpendLimit.AmountOf(sdk.DefaultBondDenom) + suite.Require().True(types.UnboundedSpendLimit().Equal(remainder)) + }, + }, + { + "test multiple coins does not overspend", + func() { + transferAuthz.Allocations[0].SpendLimit = transferAuthz.Allocations[0].SpendLimit.Add( + sdk.NewCoins( + sdk.NewCoin("test-denom", sdk.NewInt(100)), + sdk.NewCoin("test-denom2", sdk.NewInt(100)), + )..., + ) + msgTransfer.Token = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(50)) + }, + func(res authz.AcceptResponse, err error) { + suite.Require().NoError(err) + + updatedTransferAuthz, ok := res.Updated.(*types.TransferAuthorization) + suite.Require().True(ok) + + remainder := updatedTransferAuthz.Allocations[0].SpendLimit.AmountOf(sdk.DefaultBondDenom) + suite.Require().True(sdk.NewInt(50).Equal(remainder)) + + remainder = updatedTransferAuthz.Allocations[0].SpendLimit.AmountOf("test-denom") + suite.Require().True(sdk.NewInt(100).Equal(remainder)) + + remainder = updatedTransferAuthz.Allocations[0].SpendLimit.AmountOf("test-denom2") + suite.Require().True(sdk.NewInt(100).Equal(remainder)) + }, + }, { "no spend limit set for MsgTransfer port/channel", func() { @@ -190,6 +232,13 @@ func (suite *TypesTestSuite) TestTransferAuthorizationValidateBasic() { }, true, }, + { + "success: with unlimited spend limit of max uint256", + func() { + transferAuthz.Allocations[0].SpendLimit = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, types.UnboundedSpendLimit())) + }, + true, + }, { "empty allocations", func() {