Skip to content

Commit

Permalink
feat: impl check reject transfer if len(hops) > 0 and ics20-1 (#6675)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
Co-authored-by: Gjermund Garaba <[email protected]>
  • Loading branch information
3 people authored Jun 24, 2024
1 parent aa860ac commit 2b4d24b
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 5 deletions.
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)
}
}

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

0 comments on commit 2b4d24b

Please sign in to comment.