imp: getMultiDenomFungibleTokenPacketDatato be used in packet unmarshalling/conversion (#6226)

* chore: adding proto files for ics20-v2

* chore: add newline

* chore: modify MsgTransfer to accept coins instead of coin

* chore: reverted unintentional comment changes

* chore: reverted unintentional comment changes

* chore: adding test for conversion fn

* chore: fix e2e linter

* chore: adding docs

* chore: addressing PR feedback

* chore: remove duplicate import

* chore: addressing PR feedback

* chore: moved extration logic into internal package

* chore: commented out failing test

* chore: adding link to issue

* chore: remove duplicate import

* chore: fix linting errors

* FungibleTokenPacketData interface methods + tests

* linter

* wip: token validation

* update trace identifier validation in Token + tests

* rm Printf

* update with pr review

* add CurrentVersion, EscrowVersion, update tests

* pr review

* fix e2e tests

* pr review

* update e2e test version

* linter

* update hardcoded transfer channel version from interchaintest

* update hardcoded transfer channel version from interchaintest

* wip packet unmarshaller

* wip

* wip testing

* update test

* linter

* rm unnecessary version changes

* rm unnecessary artifacts

* update callbacks test

* update comment

* pr review

* rename getMultiDenomFungibleTokenPacketData to unmarshalPacketDataBytesToICS20V2

---------

Co-authored-by: chatton <github.qpeyb@simplelogin.fr>
Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

Author: 3 people

modules/apps/callbacks/ibc_middleware_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/cosmos/ibc-go/modules/apps/callbacks/types"
 	icacontrollertypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/types"
 	transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
+	multidenom "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3"
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 	channelkeeper "github.com/cosmos/ibc-go/v8/modules/core/04-channel/keeper"
 	channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
@@ -950,14 +951,27 @@ func (s *CallbacksTestSuite) TestUnmarshalPacketData() {
 	unmarshalerStack, ok := transferStack.(types.CallbacksCompatibleModule)
 	s.Require().True(ok)
 
-	expPacketData := transfertypes.FungibleTokenPacketData{
+	initialPacketData := transfertypes.FungibleTokenPacketData{
 		Denom:    ibctesting.TestCoin.Denom,
 		Amount:   ibctesting.TestCoin.Amount.String(),
 		Sender:   ibctesting.TestAccAddress,
 		Receiver: ibctesting.TestAccAddress,
 		Memo:     fmt.Sprintf(`{"src_callback": {"address": "%s"}, "dest_callback": {"address":"%s"}}`, ibctesting.TestAccAddress, ibctesting.TestAccAddress),
 	}
-	data := expPacketData.GetBytes()
+	data := initialPacketData.GetBytes()
+
+	expPacketData := multidenom.FungibleTokenPacketData{
+		Tokens: []*multidenom.Token{
+			{
+				Denom:  initialPacketData.Denom,
+				Amount: initialPacketData.Amount,
+				Trace:  []string{""},
+			},
+		},
+		Sender:   initialPacketData.Sender,
+		Receiver: initialPacketData.Receiver,
+		Memo:     initialPacketData.Memo,
+	}
 
 	packetData, err := unmarshalerStack.UnmarshalPacketData(data)
 	s.Require().NoError(err)

modules/apps/transfer/ibc_module.go

@@ -11,8 +11,10 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
+	convertinternal "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/convert"
 	"github.com/cosmos/ibc-go/v8/modules/apps/transfer/keeper"
 	"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
+	multidenom "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3"
 	channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
 	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
 	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
@@ -170,6 +172,27 @@ func (IBCModule) OnChanCloseConfirm(
 	return nil
 }
 
+func (IBCModule) unmarshalPacketDataBytesToICS20V2(bz []byte) (multidenom.FungibleTokenPacketData, error) {
+	// TODO: remove support for this function parsing v1 packet data
+	// TODO: explicit check for packet data type against app version
+
+	var datav1 types.FungibleTokenPacketData
+	if err := json.Unmarshal(bz, &datav1); err == nil {
+		if len(datav1.Denom) != 0 {
+			return convertinternal.PacketDataV1ToV3(datav1), nil
+		}
+	}
+
+	var data multidenom.FungibleTokenPacketData
+	if err := json.Unmarshal(bz, &data); err == nil {
+		if len(data.Tokens) != 0 {
+			return data, nil
+		}
+	}
+
+	return multidenom.FungibleTokenPacketData{}, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS-20 transfer packet data")
+}
+
 // OnRecvPacket implements the IBCModule interface. A successful acknowledgement
 // is returned if the packet data is successfully decoded and the receive application
 // logic returns without error.
@@ -351,11 +374,11 @@ func (IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, pr
 // UnmarshalPacketData attempts to unmarshal the provided packet data bytes
 // into a FungibleTokenPacketData. This function implements the optional
 // PacketDataUnmarshaler interface required for ADR 008 support.
-func (IBCModule) UnmarshalPacketData(bz []byte) (interface{}, error) {
-	var packetData types.FungibleTokenPacketData
-	if err := json.Unmarshal(bz, &packetData); err != nil {
+func (im IBCModule) UnmarshalPacketData(bz []byte) (interface{}, error) {
+	ftpd, err := im.unmarshalPacketDataBytesToICS20V2(bz)
+	if err != nil {
 		return nil, err
 	}
 
-	return packetData, nil
+	return ftpd, nil
 }

modules/apps/transfer/ibc_module_test.go

@@ -9,6 +9,7 @@ import (
 	capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
 	"github.com/cosmos/ibc-go/v8/modules/apps/transfer"
 	"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
+	multidenom "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3"
 	connectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types"
 	channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
 	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
@@ -474,8 +475,8 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() {
 		sender   = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()
 		receiver = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()
 
-		data          []byte
-		expPacketData types.FungibleTokenPacketData
+		data              []byte
+		initialPacketData interface{}
 	)
 
 	testCases := []struct {
@@ -484,30 +485,87 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() {
 		expPass  bool
 	}{
 		{
-			"success: valid packet data with memo",
+			"success: valid packet data single denom -> multidenom conversion with memo",
 			func() {
-				expPacketData = types.FungibleTokenPacketData{
+				initialPacketData = types.FungibleTokenPacketData{
 					Denom:    ibctesting.TestCoin.Denom,
 					Amount:   ibctesting.TestCoin.Amount.String(),
 					Sender:   sender,
 					Receiver: receiver,
 					Memo:     "some memo",
 				}
-				data = expPacketData.GetBytes()
+
+				data = initialPacketData.(types.FungibleTokenPacketData).GetBytes()
 			},
 			true,
 		},
 		{
-			"success: valid packet data without memo",
+			"success: valid packet data single denom -> multidenom conversion without memo",
 			func() {
-				expPacketData = types.FungibleTokenPacketData{
+				initialPacketData = types.FungibleTokenPacketData{
 					Denom:    ibctesting.TestCoin.Denom,
 					Amount:   ibctesting.TestCoin.Amount.String(),
 					Sender:   sender,
 					Receiver: receiver,
 					Memo:     "",
 				}
-				data = expPacketData.GetBytes()
+
+				data = initialPacketData.(types.FungibleTokenPacketData).GetBytes()
+			},
+			true,
+		},
+		{
+			"success: valid packet data single denom with trace -> multidenom conversion with trace",
+			func() {
+				initialPacketData = types.FungibleTokenPacketData{
+					Denom:    "transfer/channel-0/atom",
+					Amount:   ibctesting.TestCoin.Amount.String(),
+					Sender:   sender,
+					Receiver: receiver,
+					Memo:     "",
+				}
+
+				data = initialPacketData.(types.FungibleTokenPacketData).GetBytes()
+			},
+			true,
+		},
+		{
+			"success: valid packet data multidenom with memo",
+			func() {
+				initialPacketData = multidenom.FungibleTokenPacketData{
+					Tokens: []*multidenom.Token{
+						{
+							Denom:  "atom",
+							Amount: ibctesting.TestCoin.Amount.String(),
+							Trace:  []string{"transfer/channel-0"},
+						},
+					},
+					Sender:   sender,
+					Receiver: receiver,
+					Memo:     "some memo",
+				}
+
+				data = initialPacketData.(multidenom.FungibleTokenPacketData).GetBytes()
+			},
+			true,
+		},
+		{
+			"success: valid packet data multidenom without memo",
+			func() {
+				initialPacketData = multidenom.FungibleTokenPacketData{
+					Tokens: []*multidenom.Token{
+						{
+							Denom:  ibctesting.TestCoin.Denom,
+							Amount: ibctesting.TestCoin.Amount.String(),
+							Trace:  []string{""},
+						},
+					},
+					Sender:   sender,
+					Receiver: receiver,
+					Memo:     "",
+				}
+
+				data = initialPacketData.(multidenom.FungibleTokenPacketData).GetBytes()
 			},
 			true,
 		},
@@ -529,7 +587,19 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() {
 
 			if tc.expPass {
 				suite.Require().NoError(err)
-				suite.Require().Equal(expPacketData, packetData)
+
+				v3PacketData, ok := packetData.(multidenom.FungibleTokenPacketData)
+				suite.Require().True(ok)
+
+				if v1PacketData, ok := initialPacketData.(types.FungibleTokenPacketData); ok {
+					// Note: testing of the denom trace parsing/conversion should be done as part of testing internal conversion functions
+					suite.Require().Equal(v1PacketData.Amount, v3PacketData.Tokens[0].Amount)
+					suite.Require().Equal(v1PacketData.Sender, v3PacketData.Sender)
+					suite.Require().Equal(v1PacketData.Receiver, v3PacketData.Receiver)
+					suite.Require().Equal(v1PacketData.Memo, v3PacketData.Memo)
+				} else {
+					suite.Require().Equal(initialPacketData.(multidenom.FungibleTokenPacketData), v3PacketData)
+				}
 			} else {
 				suite.Require().Error(err)
 				suite.Require().Nil(packetData)

modules/apps/transfer/internal/convert/convert.go

@@ -32,13 +32,13 @@ func PacketDataV1ToV3(packetData v1types.FungibleTokenPacketData) v3types.Fungib
 func extractDenomAndTraceFromV1Denom(v1Denom string) (string, []string) {
 	v1DenomTrace := v1types.ParseDenomTrace(v1Denom)
 
-	splitPath := strings.Split(v1DenomTrace.Path, "/")
-
-	// if the path slice is empty, then the base denom is the full native denom.
-	if len(splitPath) == 0 {
-		return v1DenomTrace.BaseDenom, nil
+	// if the path string is empty, then the base denom is the full native denom.
+	if v1DenomTrace.Path == "" {
+		return v1DenomTrace.BaseDenom, []string{""}
 	}
 
+	splitPath := strings.Split(v1DenomTrace.Path, "/")
+
 	// this condition should never be reached.
 	if len(splitPath)%2 != 0 {
 		panic("pathSlice length is not even")

modules/apps/transfer/internal/convert/convert_test.go

@@ -36,6 +36,19 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) {
 				}, sender, receiver, ""),
 			nil,
 		},
+		{
+			"success with empty trace",
+			v1types.NewFungibleTokenPacketData("atom", "1000", sender, receiver, ""),
+			v3types.NewFungibleTokenPacketData(
+				[]*v3types.Token{
+					{
+						Denom:  "atom",
+						Amount: "1000",
+						Trace:  []string{""},
+					},
+				}, sender, receiver, ""),
+			nil,
+		},
 		{
 			"success: base denom with '/'",
 			v1types.NewFungibleTokenPacketData("transfer/channel-0/atom/withslash", "1000", sender, receiver, ""),