From 47a166c85f749eb40d8ab2db1d497981dff150e9 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 9 Jan 2023 12:47:25 +0000 Subject: [PATCH 1/2] chore: fixing unit tests and making isAllowedAddress private --- modules/apps/transfer/types/transfer_authz.go | 25 +++++++++++-------- .../transfer/types/transfer_authz_test.go | 10 ++++---- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/modules/apps/transfer/types/transfer_authz.go b/modules/apps/transfer/types/transfer_authz.go index bcf6e3ebb27..155971bb627 100644 --- a/modules/apps/transfer/types/transfer_authz.go +++ b/modules/apps/transfer/types/transfer_authz.go @@ -4,6 +4,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/authz" + host "github.com/cosmos/ibc-go/v6/modules/core/24-host" "golang.org/x/exp/slices" @@ -36,16 +37,6 @@ func (a TransferAuthorization) MsgTypeURL() string { return sdk.MsgTypeURL(&MsgTransfer{}) } -func IsAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) bool { - for _, addr := range allowedAddrs { - ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization") - if addr == receiver { - return true - } - } - return false -} - // Accept implements Authorization.Accept. func (a TransferAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptResponse, error) { msgTransfer, ok := msg.(*MsgTransfer) @@ -60,7 +51,7 @@ func (a TransferAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.Accep return authz.AcceptResponse{}, sdkerrors.ErrInsufficientFunds.Wrapf("requested amount is more than spend limit") } - if !IsAllowedAddress(ctx, msgTransfer.Receiver, allocation.AllowedAddresses) { + if !isAllowedAddress(ctx, msgTransfer.Receiver, allocation.AllowedAddresses) { return authz.AcceptResponse{}, sdkerrors.ErrInvalidAddress.Wrapf("not allowed address for transfer") } @@ -114,3 +105,15 @@ func (a TransferAuthorization) ValidateBasic() error { } return nil } + +// isAllowedAddress returns a boolean indicating if the receiver address is valid for transfer. +// gasCostPerIteration gas is consumed for each iteration. +func isAllowedAddress(ctx sdk.Context, receiver string, allowedAddrs []string) bool { + for _, addr := range allowedAddrs { + ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization") + if addr == receiver { + return true + } + } + return false +} diff --git a/modules/apps/transfer/types/transfer_authz_test.go b/modules/apps/transfer/types/transfer_authz_test.go index 022188e747c..873303d7e7a 100644 --- a/modules/apps/transfer/types/transfer_authz_test.go +++ b/modules/apps/transfer/types/transfer_authz_test.go @@ -31,7 +31,7 @@ func TestTransferAuthorization(t *testing.T) { t.Log("verify authorization returns valid method name") require.Equal(t, authorization.MsgTypeURL(), "/ibc.applications.transfer.v1.MsgTransfer") require.NoError(t, authorization.ValidateBasic()) - transfer := NewMsgTransfer(sourcePort, sourceChannel, coin1000, fromAddr.String(), toAddr.String(), timeoutHeight, 0) + transfer := NewMsgTransfer(sourcePort, sourceChannel, coin1000, fromAddr.String(), toAddr.String(), timeoutHeight, 0, "") require.NoError(t, authorization.ValidateBasic()) t.Log("verify updated authorization returns nil") @@ -44,7 +44,7 @@ func TestTransferAuthorization(t *testing.T) { authorization = NewTransferAuthorization([]string{sourcePort}, []string{sourceChannel}, []sdk.Coins{coins1000}, [][]string{{toAddr.String()}}) require.Equal(t, authorization.MsgTypeURL(), "/ibc.applications.transfer.v1.MsgTransfer") require.NoError(t, authorization.ValidateBasic()) - transfer = NewMsgTransfer(sourcePort, sourceChannel, coin500, fromAddr.String(), toAddr.String(), timeoutHeight, 0) + transfer = NewMsgTransfer(sourcePort, sourceChannel, coin500, fromAddr.String(), toAddr.String(), timeoutHeight, 0, "") require.NoError(t, authorization.ValidateBasic()) resp, err = authorization.Accept(ctx, transfer) require.NoError(t, err) @@ -61,7 +61,7 @@ func TestTransferAuthorization(t *testing.T) { t.Log("expect error when spend limit for specific port and channel is not set") authorization = NewTransferAuthorization([]string{sourcePort}, []string{sourceChannel}, []sdk.Coins{coins1000}, [][]string{{toAddr.String()}}) - transfer = NewMsgTransfer(sourcePort2, sourceChannel2, coin500, fromAddr.String(), toAddr.String(), timeoutHeight, 0) + transfer = NewMsgTransfer(sourcePort2, sourceChannel2, coin500, fromAddr.String(), toAddr.String(), timeoutHeight, 0, "") _, err = authorization.Accept(ctx, transfer) require.Error(t, err) @@ -71,7 +71,7 @@ func TestTransferAuthorization(t *testing.T) { []string{sourceChannel, sourceChannel2}, []sdk.Coins{coins1000, coins1000}, [][]string{{toAddr.String()}, {toAddr.String()}}) - transfer = NewMsgTransfer(sourcePort, sourceChannel, coin1000, fromAddr.String(), toAddr.String(), timeoutHeight, 0) + transfer = NewMsgTransfer(sourcePort, sourceChannel, coin1000, fromAddr.String(), toAddr.String(), timeoutHeight, 0, "") resp, err = authorization.Accept(ctx, transfer) require.NoError(t, err) require.NotNil(t, resp.Updated) @@ -80,7 +80,7 @@ func TestTransferAuthorization(t *testing.T) { t.Log("expect error when transferring to not allowed address") authorization = NewTransferAuthorization([]string{sourcePort}, []string{sourceChannel}, []sdk.Coins{coins1000}, [][]string{{fromAddr.String()}}) - transfer = NewMsgTransfer(sourcePort, sourceChannel, coin500, fromAddr.String(), toAddr.String(), timeoutHeight, 0) + transfer = NewMsgTransfer(sourcePort, sourceChannel, coin500, fromAddr.String(), toAddr.String(), timeoutHeight, 0, "") _, err = authorization.Accept(ctx, transfer) require.Error(t, err) } From ea645ded840018321c5996cd5c9e63ffca8b5c04 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 9 Jan 2023 12:53:30 +0000 Subject: [PATCH 2/2] chore: ran formatter --- modules/apps/transfer/types/transfer_authz.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/modules/apps/transfer/types/transfer_authz.go b/modules/apps/transfer/types/transfer_authz.go index 155971bb627..1baee282b3f 100644 --- a/modules/apps/transfer/types/transfer_authz.go +++ b/modules/apps/transfer/types/transfer_authz.go @@ -12,9 +12,7 @@ import ( const gasCostPerIteration = uint64(10) -var ( - _ authz.Authorization = &TransferAuthorization{} -) +var _ authz.Authorization = &TransferAuthorization{} // NewTransferAuthorization creates a new TransferAuthorization object. func NewTransferAuthorization(sourcePorts, sourceChannels []string, spendLimits []sdk.Coins, allowedAddrs [][]string) *TransferAuthorization {