token gets lost when using unsupported destination address

[akure]

In case of giving an unsupported destination address for the destination chain, the packet forwarder is not able to revert the tx. And the transferred amount is getting lost. This should be handled. I did not find any other failure cases so far.
Note - Though if you give the wrong channel-id for the middle chain, it is successfully reverting the tx, and giving tokens back to the sender address in the first chain.

• Example - sending 10 ibc atom from quasar to osmosis via cosmos-hub.

The actual destination address is osmo1t8eh66t2w5k67kwurmn5gqhtq6d2ja0vp7jmmq, but if you give xyz1t8eh66t2w5k67kwurmn5gqhtq6d2ja0vp7jmmq - tokens will be lost.

quasarnoded --home run/quasar/home/ --node=http://localhost:26659/ --chain-id quasar tx --from alice ibc-transfer transfer transfer channel-1 "cosmos1ppkxa0hxak05tcqq3338k76xqxy2qse96uelcu|transfer/channel-1:xyz1t8eh66t2w5k67kwurmn5gqhtq6d2ja0vp7jmmq" 10ibc/C4CFF46FD6DE35CA4CF4CE031E643C8FDC9BA4B99AE598E9B0ED98FE3A2319F9

In this case - 10ibc/C4CFF46FD6DE35CA4CF4CE031E643C8FDC9BA4B99AE598E9B0ED98FE3A2319F9 will get lost.

The detailed demo can be found for the successful case - https://github.com/quasar-finance/quasar/tree/main/demos/packet-forwarder

[jtieri]

Oh interesting catch!

Sounds like we need to do some address validation before attempting to do any packet forwarding.
Perhaps just marshaling to bytes and validating the address before doing anything https://github.com/cosmos/cosmos-sdk/blob/42f8d45b686be791c6b18cbbe02b84c91da524bc/types/address.go#L133-L174

[jtieri]

I threw together a test case inside ibctest to verify the described issue.
https://github.com/strangelove-ventures/ibctest/blob/justin/packet-forward/examples/packet_forward_test.go

It look like the middleware works exactly as intended. When initially designing the middleware we had to decide what would happen in the case of a failure during the second hop and we settled on leaving the funds in the intermediary account. This can definitely be confusing as a user since you would initially expect the funds to be returned to the original account. iirc we decided to go this route because there was always the possibility that after a transfer fails the second hop that we could try to revert the funds back to the source chain and have that tx fail as well. This could create even more fees that need to be paid and more confusion.

Billy had opened an issue to improve this design choice #1

[jtieri]

Closing this issue as this was confirmed to be expected behavior.