From 28447d0db1f0f9bf4920752c498890a770327991 Mon Sep 17 00:00:00 2001 From: duonghb53 Date: Sat, 22 Jun 2024 13:37:30 +0700 Subject: [PATCH 1/4] impl check reject transfer if len(hops) > 0 and ics20-1 --- modules/apps/transfer/keeper/relay.go | 5 ++++ modules/apps/transfer/keeper/relay_test.go | 32 ++++++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/modules/apps/transfer/keeper/relay.go b/modules/apps/transfer/keeper/relay.go index ce65244452e..673a6f1ddb7 100644 --- a/modules/apps/transfer/keeper/relay.go +++ b/modules/apps/transfer/keeper/relay.go @@ -81,6 +81,11 @@ func (k Keeper) sendTransfer( return 0, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "cannot transfer multiple coins with ics20-1") } + // reject transfer forwarding hops is not empty with ics20-1 + if appVersion == types.V1 && len(forwarding.Hops) > 0 { + return 0, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "reject transfer forwarding hops is not empty with ics20-1") + } + destinationPort := channel.Counterparty.PortId destinationChannel := channel.Counterparty.ChannelId diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index 937815d9344..bc77e1ee136 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -36,7 +36,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() { timeoutHeight clienttypes.Height memo string expEscrowAmount sdkmath.Int // total amount in escrow for denom on receiving chain - + forwarding types.Forwarding ) testCases := []struct { @@ -80,6 +80,18 @@ func (suite *KeeperTestSuite) TestSendTransfer() { }, nil, }, + { + "successful transfer len hops is empty with ics20-1", + func() { + expEscrowAmount = sdkmath.NewInt(100) + + // Set version to isc20-1. + path.EndpointA.UpdateChannel(func(channel *channeltypes.Channel) { + channel.Version = types.V1 + }) + }, + nil, + }, { "successful transfer of IBC token with memo", func() { @@ -146,6 +158,21 @@ func (suite *KeeperTestSuite) TestSendTransfer() { }, channeltypes.ErrInvalidPacket, }, + { + "failure: transfer len hops is not empty with ics20-1", + func() { + // Set version to isc20-1. + path.EndpointA.UpdateChannel(func(channel *channeltypes.Channel) { + channel.Version = types.V1 + }) + + forwarding = types.NewForwarding(false, types.Hop{ + PortId: path.EndpointA.ChannelConfig.PortID, + ChannelId: path.EndpointA.ChannelID, + }) + }, + ibcerrors.ErrInvalidRequest, + }, } for _, tc := range testCases { @@ -162,6 +189,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() { memo = "" timeoutHeight = suite.chainB.GetTimeoutHeight() expEscrowAmount = sdkmath.ZeroInt() + forwarding = emptyForwarding // create IBC token on chainA transferMsg := types.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, sdk.NewCoins(coin), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainA.GetTimeoutHeight(), 0, "", emptyForwarding) @@ -184,7 +212,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() { suite.chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0, // only use timeout height memo, - emptyForwarding, + forwarding, ) res, err := suite.chainA.GetSimApp().TransferKeeper.Transfer(suite.chainA.GetContext(), msg) From 2a8f35e371d4268587e8be8cd04446b67b7984c9 Mon Sep 17 00:00:00 2001 From: duonghb53 Date: Sat, 22 Jun 2024 22:09:15 +0700 Subject: [PATCH 2/4] add test case hops is not empty with ics20-2 --- modules/apps/transfer/keeper/relay_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index bc77e1ee136..28e69887d5a 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -92,6 +92,23 @@ func (suite *KeeperTestSuite) TestSendTransfer() { }, nil, }, + { + "successful transfer len hops is not empty with ics20-2", + func() { + expEscrowAmount = sdkmath.NewInt(100) + + // Set version to isc20-2. + path.EndpointA.UpdateChannel(func(channel *channeltypes.Channel) { + channel.Version = types.V2 + }) + + forwarding = types.NewForwarding(false, types.Hop{ + PortId: path.EndpointA.ChannelConfig.PortID, + ChannelId: path.EndpointA.ChannelID, + }) + }, + nil, + }, { "successful transfer of IBC token with memo", func() { From e3821fa198e78b19147800f99d60d5f75ee09d43 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Mon, 24 Jun 2024 09:04:33 +0200 Subject: [PATCH 3/4] address review comments --- modules/apps/transfer/keeper/relay.go | 16 +++++++++------- modules/apps/transfer/keeper/relay_test.go | 12 +++--------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/modules/apps/transfer/keeper/relay.go b/modules/apps/transfer/keeper/relay.go index 673a6f1ddb7..186a3268a6d 100644 --- a/modules/apps/transfer/keeper/relay.go +++ b/modules/apps/transfer/keeper/relay.go @@ -76,14 +76,16 @@ func (k Keeper) sendTransfer( return 0, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "application version not found for source port: %s and source channel: %s", sourcePort, sourceChannel) } - // ics20-1 only supports a single coin, so if that is the current version, we must only process a single coin. - if appVersion == types.V1 && len(coins) > 1 { - return 0, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "cannot transfer multiple coins with ics20-1") - } + if appVersion == types.V1 { + // ics20-1 only supports a single coin, so if that is the current version, we must only process a single coin. + if len(coins) > 1 { + return 0, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "cannot transfer multiple coins with %s", types.V1) + } - // reject transfer forwarding hops is not empty with ics20-1 - if appVersion == types.V1 && len(forwarding.Hops) > 0 { - return 0, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "reject transfer forwarding hops is not empty with ics20-1") + // ics20-1 does not support forwarding, so if that is the current version, we must reject the transfer. + if len(forwarding.Hops) > 0 { + return 0, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "cannot forward coins with %s", types.V1) + } } destinationPort := channel.Counterparty.PortId diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index 28e69887d5a..d6de73f4166 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -81,7 +81,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() { nil, }, { - "successful transfer len hops is empty with ics20-1", + "successful transfer with empty forwarding hops and ics20-1", func() { expEscrowAmount = sdkmath.NewInt(100) @@ -93,15 +93,9 @@ func (suite *KeeperTestSuite) TestSendTransfer() { nil, }, { - "successful transfer len hops is not empty with ics20-2", + "successful transfer with non-empty forwarding hops and ics20-2", func() { expEscrowAmount = sdkmath.NewInt(100) - - // Set version to isc20-2. - path.EndpointA.UpdateChannel(func(channel *channeltypes.Channel) { - channel.Version = types.V2 - }) - forwarding = types.NewForwarding(false, types.Hop{ PortId: path.EndpointA.ChannelConfig.PortID, ChannelId: path.EndpointA.ChannelID, @@ -176,7 +170,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() { channeltypes.ErrInvalidPacket, }, { - "failure: transfer len hops is not empty with ics20-1", + "failure: forwarding hops is not empty with ics20-1", func() { // Set version to isc20-1. path.EndpointA.UpdateChannel(func(channel *channeltypes.Channel) { From 9fd88f3de356adb45568b9e8a5794d822b140aca Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Mon, 24 Jun 2024 09:07:27 +0200 Subject: [PATCH 4/4] reorder variable declaration --- modules/apps/transfer/keeper/relay_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index d6de73f4166..c739d21187c 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -35,8 +35,8 @@ func (suite *KeeperTestSuite) TestSendTransfer() { sender sdk.AccAddress timeoutHeight clienttypes.Height memo string - expEscrowAmount sdkmath.Int // total amount in escrow for denom on receiving chain forwarding types.Forwarding + expEscrowAmount sdkmath.Int // total amount in escrow for denom on receiving chain ) testCases := []struct {