From 2b4d24b7741a38ee63a89cd8b0cf4ae6b92d1817 Mon Sep 17 00:00:00 2001 From: Maintain Date: Mon, 24 Jun 2024 22:01:47 +0700 Subject: [PATCH] feat: impl check reject transfer if len(hops) > 0 and ics20-1 (#6675) * impl check reject transfer if len(hops) > 0 and ics20-1 * add test case hops is not empty with ics20-2 * address review comments * reorder variable declaration --------- Co-authored-by: Carlos Rodriguez Co-authored-by: Gjermund Garaba --- modules/apps/transfer/keeper/relay.go | 13 +++++-- modules/apps/transfer/keeper/relay_test.go | 43 +++++++++++++++++++++- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/modules/apps/transfer/keeper/relay.go b/modules/apps/transfer/keeper/relay.go index 6d2d8d1e5e5..7cff4bc333c 100644 --- a/modules/apps/transfer/keeper/relay.go +++ b/modules/apps/transfer/keeper/relay.go @@ -76,9 +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) + } + + // 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 937815d9344..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 + forwarding types.Forwarding expEscrowAmount sdkmath.Int // total amount in escrow for denom on receiving chain - ) testCases := []struct { @@ -80,6 +80,29 @@ func (suite *KeeperTestSuite) TestSendTransfer() { }, nil, }, + { + "successful transfer with empty forwarding hops and 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 with non-empty forwarding hops and ics20-2", + func() { + expEscrowAmount = sdkmath.NewInt(100) + forwarding = types.NewForwarding(false, types.Hop{ + PortId: path.EndpointA.ChannelConfig.PortID, + ChannelId: path.EndpointA.ChannelID, + }) + }, + nil, + }, { "successful transfer of IBC token with memo", func() { @@ -146,6 +169,21 @@ func (suite *KeeperTestSuite) TestSendTransfer() { }, channeltypes.ErrInvalidPacket, }, + { + "failure: forwarding 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 +200,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 +223,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)