Refactor panic to use return error (#6391)

* chore: refactor panic to use return error

* chore: addressing PR feedback

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

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 {

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,

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,22 +128,22 @@ 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"),
 		},
 	}
 
 	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)
 		}
 	}
 }