chore: implement transfer authz ValidateBasic tests

modules/apps/transfer/types/transfer_authorization.go

@@ -70,16 +70,23 @@ func (a TransferAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.Accep
 
 // ValidateBasic implements Authorization.ValidateBasic.
 func (a TransferAuthorization) ValidateBasic() error {
+	if len(a.Allocations) == 0 {
+		return sdkerrors.Wrap(ErrInvalidAuthorization, "allocations cannot be empty")
+	}
+
 	for _, allocation := range a.Allocations {
 		if allocation.SpendLimit == nil {
 			return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, "spend limit cannot be nil")
 		}
+
 		if err := allocation.SpendLimit.Validate(); err != nil {
 			return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, err.Error())
 		}
+
 		if err := host.PortIdentifierValidator(allocation.SourcePort); err != nil {
 			return sdkerrors.Wrap(err, "invalid source port ID")
 		}
+
 		if err := host.ChannelIdentifierValidator(allocation.SourceChannel); err != nil {
 			return sdkerrors.Wrap(err, "invalid source channel ID")
 		}
@@ -92,12 +99,17 @@ func (a TransferAuthorization) ValidateBasic() error {
 			found[allocation.AllowList[i]] = true
 		}
 	}
+
 	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 {
+	if len(allowedAddrs) == 0 {
+		return true
+	}
+
 	for _, addr := range allowedAddrs {
 		ctx.GasMeter().ConsumeGas(gasCostPerIteration, "transfer authorization")
 		if addr == receiver {

modules/apps/transfer/types/transfer_authorization_test.go

@@ -1,4 +1,4 @@
-package types
+package types_test
 
 import (
 	"testing"
@@ -8,6 +8,10 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/simapp"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
+	clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
+	ibctesting "github.com/cosmos/ibc-go/v6/testing"
+	"github.com/cosmos/ibc-go/v6/testing/mock"
 )
 
 var (
@@ -21,23 +25,24 @@ var (
 	coin500        = sdk.NewCoin("stake", sdk.NewInt(500))
 	fromAddr       = sdk.AccAddress("_____from _____")
 	toAddr         = sdk.AccAddress("_______to________")
+	timeoutHeight  = clienttypes.NewHeight(0, 10)
 )
 
 func TestTransferAuthorization(t *testing.T) {
 	app := simapp.Setup(t, false)
 	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
-	allocation := Allocation{
+	allocation := types.Allocation{
 		SourcePort:    sourcePort,
 		SourceChannel: sourceChannel,
 		SpendLimit:    coins1000,
 		AllowList:     []string{toAddr.String()},
 	}
-	authorization := NewTransferAuthorization(allocation)
+	authorization := types.NewTransferAuthorization(allocation)
 
 	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 := types.NewMsgTransfer(sourcePort, sourceChannel, coin1000, fromAddr.String(), toAddr.String(), timeoutHeight, 0, "")
 	require.NoError(t, authorization.ValidateBasic())
 
 	t.Log("verify updated authorization returns nil")
@@ -47,23 +52,23 @@ func TestTransferAuthorization(t *testing.T) {
 	require.Nil(t, resp.Updated)
 
 	t.Log("verify updated authorization returns remaining spent limit")
-	authorization = NewTransferAuthorization(allocation)
+	authorization = types.NewTransferAuthorization(allocation)
 	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 = types.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)
 	require.False(t, resp.Delete)
 	require.NotNil(t, resp.Updated)
 
-	allocation = Allocation{
+	allocation = types.Allocation{
 		SourcePort:    sourcePort,
 		SourceChannel: sourceChannel,
 		SpendLimit:    coins500,
 		AllowList:     []string{toAddr.String()},
 	}
-	sendAuth := NewTransferAuthorization(allocation)
+	sendAuth := types.NewTransferAuthorization(allocation)
 	require.Equal(t, sendAuth.String(), resp.Updated.String())
 
 	t.Log("expect updated authorization nil after spending remaining amount")
@@ -73,20 +78,20 @@ func TestTransferAuthorization(t *testing.T) {
 	require.Nil(t, resp.Updated)
 
 	t.Log("expect error when spend limit for specific port and channel is not set")
-	allocation = Allocation{
+	allocation = types.Allocation{
 		SourcePort:    sourcePort,
 		SourceChannel: sourceChannel,
 		SpendLimit:    coins1000,
 		AllowList:     []string{toAddr.String()},
 	}
-	authorization = NewTransferAuthorization(allocation)
-	transfer = NewMsgTransfer(sourcePort2, sourceChannel2, coin500, fromAddr.String(), toAddr.String(), timeoutHeight, 0, "")
+	authorization = types.NewTransferAuthorization(allocation)
+	transfer = types.NewMsgTransfer(sourcePort2, sourceChannel2, coin500, fromAddr.String(), toAddr.String(), timeoutHeight, 0, "")
 	_, err = authorization.Accept(ctx, transfer)
 	require.Error(t, err)
 
 	t.Log("expect removing only 1 allocation if spend limit is finalized for the port")
 
-	allocations := []Allocation{
+	allocations := []types.Allocation{
 		{
 			SourcePort:    sourcePort,
 			SourceChannel: sourceChannel,
@@ -100,23 +105,134 @@ func TestTransferAuthorization(t *testing.T) {
 			AllowList:     []string{toAddr.String()},
 		},
 	}
-	authorization = NewTransferAuthorization(allocations...)
-	transfer = NewMsgTransfer(sourcePort, sourceChannel, coin1000, fromAddr.String(), toAddr.String(), timeoutHeight, 0, "")
+	authorization = types.NewTransferAuthorization(allocations...)
+	transfer = types.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)
-	require.Equal(t, resp.Updated, NewTransferAuthorization(allocations[1]))
+	require.Equal(t, resp.Updated, types.NewTransferAuthorization(allocations[1]))
 	require.False(t, resp.Delete)
 
 	t.Log("expect error when transferring to not allowed address")
-	allocation = Allocation{
+	allocation = types.Allocation{
 		SourcePort:    sourcePort,
 		SourceChannel: sourceChannel,
 		SpendLimit:    coins1000,
 		AllowList:     []string{fromAddr.String()},
 	}
-	authorization = NewTransferAuthorization(allocation)
-	transfer = NewMsgTransfer(sourcePort, sourceChannel, coin500, fromAddr.String(), toAddr.String(), timeoutHeight, 0, "")
+	authorization = types.NewTransferAuthorization(allocation)
+	transfer = types.NewMsgTransfer(sourcePort, sourceChannel, coin500, fromAddr.String(), toAddr.String(), timeoutHeight, 0, "")
 	_, err = authorization.Accept(ctx, transfer)
 	require.Error(t, err)
 }
+
+func TestTransferAuthorizationValidateBasic(t *testing.T) {
+	var transferAuthz types.TransferAuthorization
+
+	testCases := []struct {
+		name     string
+		malleate func()
+		expPass  bool
+	}{
+		{
+			"success",
+			func() {},
+			true,
+		},
+		{
+			"success: empty allow list",
+			func() {
+				transferAuthz.Allocations[0].AllowList = []string{}
+			},
+			true,
+		},
+		{
+			"success: with multiple allocations",
+			func() {
+				allocation := types.Allocation{
+					SourcePort:    types.PortID,
+					SourceChannel: ibctesting.FirstChannelID,
+					SpendLimit:    sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))),
+					AllowList:     []string{},
+				}
+
+				transferAuthz.Allocations = append(transferAuthz.Allocations, allocation)
+			},
+			true,
+		},
+		{
+			"empty allocations",
+			func() {
+				transferAuthz = types.TransferAuthorization{Allocations: []types.Allocation{}}
+			},
+			false,
+		},
+		{
+			"nil allocations",
+			func() {
+				transferAuthz = types.TransferAuthorization{}
+			},
+			false,
+		},
+		{
+			"nil spend limit coins",
+			func() {
+				transferAuthz.Allocations[0].SpendLimit = nil
+			},
+			false,
+		},
+		{
+			"invalid spend limit coins",
+			func() {
+				transferAuthz.Allocations[0].SpendLimit = sdk.Coins{sdk.Coin{Denom: ""}}
+			},
+			false,
+		},
+		{
+			"duplicate entry in allow list",
+			func() {
+				transferAuthz.Allocations[0].AllowList = []string{ibctesting.TestAccAddress, ibctesting.TestAccAddress}
+			},
+			false,
+		},
+		{
+			"invalid port identifier",
+			func() {
+				transferAuthz.Allocations[0].SourcePort = ""
+			},
+			false,
+		},
+		{
+			"invalid channel identifier",
+			func() {
+				transferAuthz.Allocations[0].SourceChannel = ""
+			},
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			transferAuthz = types.TransferAuthorization{
+				Allocations: []types.Allocation{
+					{
+						SourcePort:    mock.PortID,
+						SourceChannel: ibctesting.FirstChannelID,
+						SpendLimit:    sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))),
+						AllowList:     []string{ibctesting.TestAccAddress},
+					},
+				},
+			}
+
+			tc.malleate()
+
+			err := transferAuthz.ValidateBasic()
+
+			if tc.expPass {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+			}
+		})
+	}
+}