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.

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

feat: impl check reject transfer if len(hops) > 0 and ics20-1 #6675

13 changes: 10 additions & 3 deletions modules/apps/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
destinationPort := channel.Counterparty.PortId
Expand Down
43 changes: 41 additions & 2 deletions modules/apps/transfer/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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)
Expand Down
Loading