Introduce (PortId, ChannelId) domain type

[adizere]

Crate

ibc

Summary

Since we started working on ICS04 handlers(informalsystems/hermes#452) we noticed that there is a recurring need to encapsulate the tuple (PortId, ChannelId) as a type. This tuple is needed in error types:

https://github.com/informalsystems/ibc-rs/blob/d6250014e8d8fcddde651f9e3ceb6be5c93cbeae/modules/src/application/ics20_fungible_token_transfer/error.rs#L19-L20

or as parameters to structures:

https://github.com/informalsystems/ibc-rs/blob/71d9baa6a2eed5a9286f9fe88f1cfacbd51e19d8/modules/src/mock/context.rs#L86-L86

or as parameters to methods:

https://github.com/informalsystems/ibc-rs/blob/71d9baa6a2eed5a9286f9fe88f1cfacbd51e19d8/modules/src/mock/context.rs#L279-L282

So there's ample reasons to introduce a new type for capturing the two.

Proposal

The new type should probably reside in ICS04. Potential names:

pub struct PortChannel(PortId, ChannelId);
pub struct BoundChannel(PortId, ChannelId);
pub struct AuthorizedChannel(PortId, ChannelId);

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate milestone (priority) applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[romac]

How about IbcSocket? Much like a TCP socket consists of an IP address and a port number.

[adizere]

How about IbcSocket? Much like a TCP socket consists of an IP address and a port number.

I love this name.

I would propose the following plan:

1. prepare a PR and motivation for introducing the name IbcSocket (or just Socket) upstream in the cosmos/IBC spec repo, and then subsequently in the .proto files (SDK and IBC-go). The changes should be mostly aesthetic and primarily in ICS 004.
2. adopt the name in our own repo.

We can also consider moving faster and not waiting on the IBC spec team, but that would mean we'd need to do two sets of changes (once for the initial ibc-rs-specific changes, and a second time to adopt the .proto files once modifications flow from upstream into our repo).

[adizere]

Blocked on cosmos/ibc#583

[mzabaluev]

We can invent the type locally (already have as PortChannelId?) and use it in any internal or domain/idiomatic wrapper interfaces without the need for protobuf to be modified. In fact, a message type just to group the two fields consumes more space in protobuf wire encoding than the same two fields encoded inline.

Potential names:

Socket?

BoundChannel(PortId, ChannelId)

This looks too "stateful": there may not be a bound channel for this pair of IDs, as there isn't in your example of an error where this type could be used.

[romac]

I kinda like socket but i am afraid it's already too overloaded.

I agree that BoundChannel is too stateful.

[plafer]

I don't like Socket because IMO, in TCP/IP an (address, port) is more an "endpoint" than a "socket". For me, a socket represents a data structure over which I can perform operations; it is an interface provided by the kernel that lets me read/write the network. A socket is definitely stateful to me: it holds everything needed to properly interact with the network (e.g. the sequence number in the case of IBC). In other words, ChannelEnd is more a "socket" to me than (address, port) is.

I can't think of another analogy that I feel fits IBC's (channelId, portId), so I think I would vote for the simple PortChannelId.

[romac]

Good points! Let's go with PortChannelId then of nobody has a better suggestion.