ICS4: Simplify sendPacket interface

[mpoke]

Currently, sendPacket requires the caller to pass a Packet, defined as

interface Packet {
  sequence: uint64
  timeoutHeight: Height
  timeoutTimestamp: uint64
  sourcePort: Identifier
  sourceChannel: Identifier
  destPort: Identifier
  destChannel: Identifier
  data: bytes
} 

This means that the caller needs to set sequence, destPort and destChannel. However, all three of these fields can be looked up in SendPacket by using sourcePort and sourceChannel. This is what actually happens in the ibc-go implementation in order to verify that these fields are matching the expected ones in the channel state., i.e., here, here, and here.

This is possible since currently there is a 1-to-1 mapping between source port and channel to destination port and channel. Thus, it would be sufficient for the calling module to just provide the source and ICS4 to do the rest (i.e., add the destination). As for the sequence, the only reason to have the caller specify it is to avoid sending duplicates (e.g., somehow the calling module calls SendPacket twice with the same data). However, I don’t really see how that could happen given the sequential nature of the state machine.

I propose to change the interface for sending a packet to SendPacket(data, channel, timeout), where channel is the tuple (sourcePort, sourceChannel) and timeout is the tuple (timeoutHeight, timeoutTimestamp). I would argue that simplifying the SendPacket interface will result in a simpler application logic, e.g., SendTransfer from token transfer needs the channelKeeper just to fill in these fields from the channel state.

@AdityaSripal @colin-axner Would you agree?

[AdityaSripal]

I agree with this proposal.

From discussion on Slack:

colin-axner: What about with the context of ICS 30? It makes sense to pass in all the necessary fields in SendPacket to the application at the bottom of a middleware stack, but to pass all this information upwards makes less sense as presumably only the data should be changing, although I could see middleware developers rerouting packets (although probably ill-advised). I'm also wondering about async packet queuing, but because packets cannot be sent out of order, I don't see an issue

mpoke: In case it is necessary, the application (or a middleware layer) could always query the IBC module (ICS4) for this information

I agree with Marius on this. We already have the ability to directly query the ICS4 state for channel information that is not directly related to packet handling.

I think that while writing the middleware developer guide it is very important that we standardize on how/when to go through the middleware stack for querying (e.g. packet handling/getting app version) and when to just directly query channel keeper.