Skip to content

Commit

Permalink
Use type with V2 suffix for package data (#6330)
Browse files Browse the repository at this point in the history
  • Loading branch information
@chatton
chatton authored May 21, 2024
1 parent 0478cb9 commit f39d173
Show file tree
Hide file tree
Showing 18 changed files with 1,269 additions and 1,468 deletions.
31 changes: 15 additions & 16 deletions modules/apps/callbacks/ibc_middleware_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ 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"
transferv3types "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"
Expand Down Expand Up @@ -94,7 +93,7 @@ func (s *CallbacksTestSuite) TestWithICS4Wrapper() {
}

func (s *CallbacksTestSuite) TestSendPacket() {
var packetData transferv3types.FungibleTokenPacketData
var packetData transfertypes.FungibleTokenPacketDataV2

testCases := []struct {
name string
Expand Down Expand Up @@ -165,8 +164,8 @@ func (s *CallbacksTestSuite) TestSendPacket() {

transferICS4Wrapper := GetSimApp(s.chainA).TransferKeeper.GetICS4Wrapper()

packetData = transferv3types.NewFungibleTokenPacketData(
[]*transferv3types.Token{
packetData = transfertypes.NewFungibleTokenPacketDataV2(
[]*transfertypes.Token{
{
Denom: ibctesting.TestCoin.GetDenom(),
Amount: ibctesting.TestCoin.Amount.String(),
Expand Down Expand Up @@ -231,7 +230,7 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() {
)

var (
packetData transferv3types.FungibleTokenPacketData
packetData transfertypes.FungibleTokenPacketDataV2
packet channeltypes.Packet
ack []byte
ctx sdk.Context
Expand Down Expand Up @@ -307,8 +306,8 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() {
s.SetupTransferTest()

userGasLimit = 600000
packetData = transferv3types.NewFungibleTokenPacketData(
[]*transferv3types.Token{
packetData = transfertypes.NewFungibleTokenPacketDataV2(
[]*transfertypes.Token{
{
Denom: ibctesting.TestCoin.GetDenom(),
Amount: ibctesting.TestCoin.Amount.String(),
Expand Down Expand Up @@ -401,7 +400,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() {
)

var (
packetData transferv3types.FungibleTokenPacketData
packetData transfertypes.FungibleTokenPacketDataV2
packet channeltypes.Packet
ctx sdk.Context
)
Expand Down Expand Up @@ -563,7 +562,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() {
)

var (
packetData transferv3types.FungibleTokenPacketData
packetData transfertypes.FungibleTokenPacketDataV2
packet channeltypes.Packet
ctx sdk.Context
userGasLimit uint64
Expand Down Expand Up @@ -640,8 +639,8 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() {

// set user gas limit above panic level in mock contract keeper
userGasLimit = 600_000
packetData = transferv3types.NewFungibleTokenPacketData(
[]*transferv3types.Token{
packetData = transfertypes.NewFungibleTokenPacketDataV2(
[]*transfertypes.Token{
{
Denom: ibctesting.TestCoin.GetDenom(),
Amount: ibctesting.TestCoin.Amount.String(),
Expand Down Expand Up @@ -725,7 +724,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() {

func (s *CallbacksTestSuite) TestWriteAcknowledgement() {
var (
packetData transferv3types.FungibleTokenPacketData
packetData transfertypes.FungibleTokenPacketDataV2
packet channeltypes.Packet
ctx sdk.Context
ack ibcexported.Acknowledgement
Expand Down Expand Up @@ -772,8 +771,8 @@ func (s *CallbacksTestSuite) TestWriteAcknowledgement() {
s.SetupTransferTest()

// set user gas limit above panic level in mock contract keeper
packetData = transferv3types.NewFungibleTokenPacketData(
[]*transferv3types.Token{
packetData = transfertypes.NewFungibleTokenPacketDataV2(
[]*transfertypes.Token{
{
Denom: ibctesting.TestCoin.GetDenom(),
Amount: ibctesting.TestCoin.Amount.String(),
Expand Down Expand Up @@ -991,8 +990,8 @@ func (s *CallbacksTestSuite) TestUnmarshalPacketData() {
Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}, "dest_callback": {"address":"%s"}}`, ibctesting.TestAccAddress, ibctesting.TestAccAddress),
}

expPacketDataICS20V2 := transferv3types.FungibleTokenPacketData{
Tokens: []*transferv3types.Token{
expPacketDataICS20V2 := transfertypes.FungibleTokenPacketDataV2{
Tokens: []*transfertypes.Token{
{
Denom: ibctesting.TestCoin.Denom,
Amount: ibctesting.TestCoin.Amount.String(),
Expand Down
9 changes: 4 additions & 5 deletions modules/apps/transfer/ibc_module.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
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"
v3types "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"
Expand Down Expand Up @@ -175,25 +174,25 @@ func (IBCModule) OnChanCloseConfirm(
return nil
}

func (IBCModule) unmarshalPacketDataBytesToICS20V2(bz []byte) (v3types.FungibleTokenPacketData, error) {
func (IBCModule) unmarshalPacketDataBytesToICS20V2(bz []byte) (types.FungibleTokenPacketDataV2, 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
return convertinternal.PacketDataV1ToV2(datav1), nil
}
}

var data v3types.FungibleTokenPacketData
var data types.FungibleTokenPacketDataV2
if err := json.Unmarshal(bz, &data); err == nil {
if len(data.Tokens) != 0 {
return data, nil
}
}

return v3types.FungibleTokenPacketData{}, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS-20 transfer packet data")
return types.FungibleTokenPacketDataV2{}, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS-20 transfer packet data")
}

// OnRecvPacket implements the IBCModule interface. A successful acknowledgement
Expand Down
25 changes: 12 additions & 13 deletions modules/apps/transfer/ibc_module_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ 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"
v3types "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"
Expand Down Expand Up @@ -544,8 +543,8 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() {
{
"success: valid packet data multidenom with memo",
func() {
initialPacketData = v3types.FungibleTokenPacketData{
Tokens: []*v3types.Token{
initialPacketData = types.FungibleTokenPacketDataV2{
Tokens: []*types.Token{
{
Denom: "atom",
Amount: ibctesting.TestCoin.Amount.String(),
Expand All @@ -557,15 +556,15 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() {
Memo: "some memo",
}

data = initialPacketData.(v3types.FungibleTokenPacketData).GetBytes()
data = initialPacketData.(types.FungibleTokenPacketDataV2).GetBytes()
},
true,
},
{
"success: valid packet data multidenom without memo",
func() {
initialPacketData = v3types.FungibleTokenPacketData{
Tokens: []*v3types.Token{
initialPacketData = types.FungibleTokenPacketDataV2{
Tokens: []*types.Token{
{
Denom: ibctesting.TestCoin.Denom,
Amount: ibctesting.TestCoin.Amount.String(),
Expand All @@ -577,7 +576,7 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() {
Memo: "",
}

data = initialPacketData.(v3types.FungibleTokenPacketData).GetBytes()
data = initialPacketData.(types.FungibleTokenPacketDataV2).GetBytes()
},
true,
},
Expand All @@ -600,17 +599,17 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() {
if tc.expPass {
suite.Require().NoError(err)

v3PacketData, ok := packetData.(v3types.FungibleTokenPacketData)
v2PacketData, ok := packetData.(types.FungibleTokenPacketDataV2)
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)
suite.Require().Equal(v1PacketData.Amount, v2PacketData.Tokens[0].Amount)
suite.Require().Equal(v1PacketData.Sender, v2PacketData.Sender)
suite.Require().Equal(v1PacketData.Receiver, v2PacketData.Receiver)
suite.Require().Equal(v1PacketData.Memo, v2PacketData.Memo)
} else {
suite.Require().Equal(initialPacketData.(v3types.FungibleTokenPacketData), v3PacketData)
suite.Require().Equal(initialPacketData.(types.FungibleTokenPacketDataV2), v2PacketData)
}
} else {
suite.Require().Error(err)
Expand Down
9 changes: 4 additions & 5 deletions modules/apps/transfer/internal/convert/convert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,17 @@ import (
"strings"

"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3"
)

// PacketDataV1ToV3 converts a v1 (ICS20-V1) packet data to a v3 (ICS20-V2) packet data.
func PacketDataV1ToV3(packetData types.FungibleTokenPacketData) v3types.FungibleTokenPacketData {
// PacketDataV1ToV2 converts a v1 packet data to a v2 packet data.
func PacketDataV1ToV2(packetData types.FungibleTokenPacketData) types.FungibleTokenPacketDataV2 {
if err := packetData.ValidateBasic(); err != nil {
panic(err)
}

v2Denom, trace := ExtractDenomAndTraceFromV1Denom(packetData.Denom)
return v3types.FungibleTokenPacketData{
Tokens: []*v3types.Token{
return types.FungibleTokenPacketDataV2{
Tokens: []*types.Token{
{
Denom: v2Denom,
Amount: packetData.Amount,
Expand Down
41 changes: 20 additions & 21 deletions modules/apps/transfer/internal/convert/convert_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ import (
errorsmod "cosmossdk.io/errors"

"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3"
)

func TestConvertPacketV1ToPacketV3(t *testing.T) {
func TestConvertPacketV1ToPacketV2(t *testing.T) {
const (
sender = "sender"
receiver = "receiver"
Expand All @@ -20,14 +19,14 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) {
testCases := []struct {
name string
v1Data types.FungibleTokenPacketData
v3Data v3types.FungibleTokenPacketData
v2Data types.FungibleTokenPacketDataV2
expPanic error
}{
{
"success",
types.NewFungibleTokenPacketData("transfer/channel-0/atom", "1000", sender, receiver, ""),
v3types.NewFungibleTokenPacketData(
[]*v3types.Token{
types.NewFungibleTokenPacketDataV2(
[]*types.Token{
{
Denom: "atom",
Amount: "1000",
Expand All @@ -39,8 +38,8 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) {
{
"success with empty trace",
types.NewFungibleTokenPacketData("atom", "1000", sender, receiver, ""),
v3types.NewFungibleTokenPacketData(
[]*v3types.Token{
types.NewFungibleTokenPacketDataV2(
[]*types.Token{
{
Denom: "atom",
Amount: "1000",
Expand All @@ -52,8 +51,8 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) {
{
"success: base denom with '/'",
types.NewFungibleTokenPacketData("transfer/channel-0/atom/withslash", "1000", sender, receiver, ""),
v3types.NewFungibleTokenPacketData(
[]*v3types.Token{
types.NewFungibleTokenPacketDataV2(
[]*types.Token{
{
Denom: "atom/withslash",
Amount: "1000",
Expand All @@ -65,8 +64,8 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) {
{
"success: base denom with '/' at the end",
types.NewFungibleTokenPacketData("transfer/channel-0/atom/", "1000", sender, receiver, ""),
v3types.NewFungibleTokenPacketData(
[]*v3types.Token{
types.NewFungibleTokenPacketDataV2(
[]*types.Token{
{
Denom: "atom/",
Amount: "1000",
Expand All @@ -78,8 +77,8 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) {
{
"success: longer trace base denom with '/'",
types.NewFungibleTokenPacketData("transfer/channel-0/transfer/channel-1/atom/pool", "1000", sender, receiver, ""),
v3types.NewFungibleTokenPacketData(
[]*v3types.Token{
types.NewFungibleTokenPacketDataV2(
[]*types.Token{
{
Denom: "atom/pool",
Amount: "1000",
Expand All @@ -91,8 +90,8 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) {
{
"success: longer trace with non transfer port",
types.NewFungibleTokenPacketData("transfer/channel-0/transfer/channel-1/transfer-custom/channel-2/atom", "1000", sender, receiver, ""),
v3types.NewFungibleTokenPacketData(
[]*v3types.Token{
types.NewFungibleTokenPacketDataV2(
[]*types.Token{
{
Denom: "atom",
Amount: "1000",
Expand All @@ -104,8 +103,8 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) {
{
"success: base denom with slash, trace with non transfer port",
types.NewFungibleTokenPacketData("transfer/channel-0/transfer/channel-1/transfer-custom/channel-2/atom/pool", "1000", sender, receiver, ""),
v3types.NewFungibleTokenPacketData(
[]*v3types.Token{
types.NewFungibleTokenPacketDataV2(
[]*types.Token{
{
Denom: "atom/pool",
Amount: "1000",
Expand All @@ -117,19 +116,19 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) {
{
"failure: panics with empty denom",
types.NewFungibleTokenPacketData("", "1000", sender, receiver, ""),
v3types.FungibleTokenPacketData{},
types.FungibleTokenPacketDataV2{},
errorsmod.Wrap(types.ErrInvalidDenomForTransfer, "base denomination cannot be blank"),
},
}

for _, tc := range testCases {
expPass := tc.expPanic == nil
if expPass {
v3Data := PacketDataV1ToV3(tc.v1Data)
require.Equal(t, tc.v3Data, v3Data, "test case: %s", tc.name)
actualV2Data := PacketDataV1ToV2(tc.v1Data)
require.Equal(t, tc.v2Data, actualV2Data, "test case: %s", tc.name)
} else {
require.PanicsWithError(t, tc.expPanic.Error(), func() {
PacketDataV1ToV3(tc.v1Data)
PacketDataV1ToV2(tc.v1Data)
}, "test case: %s", tc.name)
}
}
Expand Down
7 changes: 3 additions & 4 deletions modules/apps/transfer/keeper/mbt_relay_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

convertinternal "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/convert"
"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3"
clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
Expand Down Expand Up @@ -67,7 +66,7 @@ type FungibleTokenPacket struct {
SourcePort string
DestChannel string
DestPort string
Data v3types.FungibleTokenPacketData
Data types.FungibleTokenPacketDataV2
}

type OnRecvPacketTestCase = struct {
Expand Down Expand Up @@ -151,8 +150,8 @@ func FungibleTokenPacketFromTla(packet TlaFungibleTokenPacket) FungibleTokenPack
SourcePort: packet.SourcePort,
DestChannel: packet.DestChannel,
DestPort: packet.DestPort,
Data: v3types.NewFungibleTokenPacketData(
[]*v3types.Token{
Data: types.NewFungibleTokenPacketDataV2(
[]*types.Token{
{
Denom: denom,
Amount: packet.Data.Amount,
Expand Down
Loading

0 comments on commit f39d173

Please sign in to comment.