diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go index af6db056856..cf9f6620ceb 100644 --- a/modules/apps/transfer/ibc_module.go +++ b/modules/apps/transfer/ibc_module.go @@ -185,11 +185,7 @@ func (IBCModule) unmarshalPacketDataBytesToICS20V2(bz []byte, ics20Version strin return types.FungibleTokenPacketDataV2{}, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS20-V1 transfer packet data: %s", err.Error()) } - if err := datav1.ValidateBasic(); err != nil { - return types.FungibleTokenPacketDataV2{}, err - } - - return convertinternal.PacketDataV1ToV2(datav1), nil + return convertinternal.PacketDataV1ToV2(datav1) case types.V2: var datav2 types.FungibleTokenPacketDataV2 if err := json.Unmarshal(bz, &datav2); err != nil { diff --git a/modules/apps/transfer/internal/convert/convert.go b/modules/apps/transfer/internal/convert/convert.go index 26a8dcaa4ad..5dd36afa4f6 100644 --- a/modules/apps/transfer/internal/convert/convert.go +++ b/modules/apps/transfer/internal/convert/convert.go @@ -1,15 +1,19 @@ package convert import ( + "errors" "strings" + errorsmod "cosmossdk.io/errors" + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" ) -// PacketDataV1ToV2 converts a v1 packet data to a v2 packet data. -func PacketDataV1ToV2(packetData types.FungibleTokenPacketData) types.FungibleTokenPacketDataV2 { +// PacketDataV1ToV2 converts a v1 packet data to a v2 packet data. The packet data is validated +// before conversion. +func PacketDataV1ToV2(packetData types.FungibleTokenPacketData) (types.FungibleTokenPacketDataV2, error) { if err := packetData.ValidateBasic(); err != nil { - panic(err) + return types.FungibleTokenPacketDataV2{}, errorsmod.Wrapf(err, "invalid packet data") } v2Denom, trace := ExtractDenomAndTraceFromV1Denom(packetData.Denom) @@ -26,7 +30,7 @@ func PacketDataV1ToV2(packetData types.FungibleTokenPacketData) types.FungibleTo Sender: packetData.Sender, Receiver: packetData.Receiver, Memo: packetData.Memo, - } + }, nil } // extractDenomAndTraceFromV1Denom extracts the base denom and remaining trace from a v1 IBC denom. @@ -42,7 +46,7 @@ func ExtractDenomAndTraceFromV1Denom(v1Denom string) (string, []string) { // this condition should never be reached. if len(splitPath)%2 != 0 { - panic("pathSlice length is not even") + panic(errors.New("path slice length is not even")) } // the path slices consists of entries of ports and channel ids separately, diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go index f08726fef0d..fa289c32d7f 100644 --- a/modules/apps/transfer/internal/convert/convert_test.go +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -20,7 +20,7 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { name string v1Data types.FungibleTokenPacketData v2Data types.FungibleTokenPacketDataV2 - expPanic error + expError error }{ { "success", @@ -128,7 +128,7 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { nil, }, { - "failure: panics with empty denom", + "failure: packet data fails validation with empty denom", types.NewFungibleTokenPacketData("", "1000", sender, receiver, ""), types.FungibleTokenPacketDataV2{}, errorsmod.Wrap(types.ErrInvalidDenomForTransfer, "base denomination cannot be blank"), @@ -136,14 +136,14 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { } for _, tc := range testCases { - expPass := tc.expPanic == nil + actualV2Data, err := PacketDataV1ToV2(tc.v1Data) + + expPass := tc.expError == nil if expPass { - actualV2Data := PacketDataV1ToV2(tc.v1Data) + require.NoError(t, err, "test case: %s", tc.name) require.Equal(t, tc.v2Data, actualV2Data, "test case: %s", tc.name) } else { - require.PanicsWithError(t, tc.expPanic.Error(), func() { - PacketDataV1ToV2(tc.v1Data) - }, "test case: %s", tc.name) + require.Error(t, err, "test case: %s", tc.name) } } }