Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use type with V2 suffix for package data #6330

Merged
merged 5 commits into from
May 21, 2024
Merged

Use type with V2 suffix for package data #6330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2703e83
chore: refactor packet data v2 to use type with suffix in v2 proto pa…
chatton May 20, 2024
9835815
chore: run lint fix
chatton May 20, 2024
a8ada64
chore: running linter again
chatton May 20, 2024
88962af
chore: correcting docstring
chatton May 20, 2024
327f71c
chore: addresing PR feedback
chatton May 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 15 additions & 16 deletions modules/apps/callbacks/ibc_middleware_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading