ICS4: question on send packet with channel not opened

[ancazamfir]

Summary

The send packet handler checks if a channel is not closed (*), however I believe it will only be successful if the channel is in open state. Trying to send a fungible token transfer packet over a channel in init state returns this error:

log=Log(\"failed to execute message; message index: 0: packet failed basic validation: invalid destination channel ID: identifier cannot be blank: invalid identifier\")"

With the latest specs/ code the destination channel identifier is only known once in open state.

(*) same in the ics02 pseudocode function sendPacket(packet: Packet) {.. but there is also this comment before "Checks that the channel & connection are open to send packets".

---

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

[colin-axner]

Thanks for bringing this up @ancazamfir. I think this area of design was overlooked in the last minute identifier changes. I believe it is safe for us to disable the identifier check since the destination port should only be used in receive at which point the channel must be open. We might need to add more packet data checks in receive since I think we rely on the checks in send.

At the very least this is not problematic right now, it just prevents optimistic sends (which seems a little unnecessary to be for ICS 20)

cc @cwgoes

[ancazamfir]

At the very least this is not problematic right now, it just prevents optimistic sends (which seems a little unnecessary to be for ICS 20)

Yes, was trying to test optimistic channel opening, optimistic send, channel closing. Optimistic sends handling in the relayer needs a bit of a special treatment and would be happy to leave it for later if we confirm this will not be supported in Stargate.

[cwgoes]

Thanks @ancazamfir - good catch - this should be changed, but it won't happen for Stargate, we aren't going to delay for it.

[cwgoes]

(this is technically breaking, so it can't be in 1.0.1)

[cwgoes]

I'm going to move this to the spec repo, it needs to be dealt with there first.

[ethanfrey]

We have used a partial version of this in wasmd. Basically, in the OnChannelAck or OnChannelConfirm handlers, you can now send a packet (this got fixed in Cosmos SDK v0.41.0). You cannot send a packet in OnChannelInit or OnChannelTry.

[cwgoes]

cc @AdityaSripal has this been fixed since?

[colin-axner]

I happened to revisit this issue. I think:

• checking that channel is not closed is correct behaviour in core IBC 04-channel sendPacket (otherwise optimistic sends are not possible)
• the spec comment is incorrect -> "Checks that the channel & connection are open to send packets" as pointed out in the original comment for this issue
• ibc-go does not allow optimistic ics20 sends during when the channel is in INIT because it gets the dest channel from the channel set state. Is this something we should change? We can bring the details of this change to our repo, but it'd be nice if the spec indicated what the expected behaviour was (optimistic or non optimistic send support)

[cwgoes]

The spec comment is clearly wrong and should change. If optimistic sends are desired, the ibc-go implementation will need to change as well. cc @dtribble is Agoric testing this?

[crodriguezvega]

I opened #865 to update the comment. If it gets merged I propose we close this issue and open a different one to discuss how to properly support optimistic sends.