diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index 180ca3eb056..1b265db095c 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/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) diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go index 4cfd55ffdef..ed9e1526a53 100644 --- a/modules/apps/transfer/ibc_module.go +++ b/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 } diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go index ad4b0be3861..62282b5f9a1 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/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) diff --git a/modules/apps/transfer/internal/convert/convert.go b/modules/apps/transfer/internal/convert/convert.go index 34ba1da17e6..f68a9ed808e 100644 --- a/modules/apps/transfer/internal/convert/convert.go +++ b/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") diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go index f43731ad5dd..006bf3eeb32 100644 --- a/modules/apps/transfer/internal/convert/convert_test.go +++ b/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, ""),