Refactor: MsgChannelOpenInit and MsgChannelOpenTry should not contain a ChannelEnd

[plafer]

Summary of bug

Both MsgChannelOpenInit and MsgChannelOpenTry contain a ChannelEnd field. Although convenient because ChannelEnd contains most of the fields we need, it's semantically incorrect and confusing to developers not used to the codebase. For example, in the case of MsgChannelOpenInit, how am I supposed to provide one end of the channel I am trying to create?

Note that MsgChannelOpenAck and MsgChannelOpenConfirm don't contain a ChannelEnd field.

Version

v0.13.0

Acceptance Criteria

MsgChannelOpenInit and MsgChannelOpenTry don't contain a ChannelEnd field.

---

For Admin Use

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

[ancazamfir]

... how am I supposed to provide one end of the channel I am trying to create?

Could you please clarify?
For MsgChannelOpenInit it's only the counterparty's channel-id that is not known. Everything else should be known, right? for MsgChannelOpenTry all might be known (if done after MsgChannelOpenInit on the same chain). Note also that our domain types follow closely the protobuf definitions.

[plafer]

My point is that the creation of a ChannelEnd comes as a result of processing a MsgChannelOpenInit. The message shouldn't know what a ChannelEnd is; that's an internal IBC handler concept. In other words, the current interface asks the caller to provide a ChannelEnd in the message, which makes no sense if we agree that the ChannelEnd is created and managed internally by the IBC handler. Notice that we never use the msg.channel.state field in the handler, because it is meaningless, and could be populated with any value without changing the result of processing the message.

The ibc-go implementation of chanOpenInit handler, and ICS-4, correctly (in my opinion) don't ask for a ChannelEnd.

As for the protobuf definitions, you are right... In my opinion, that should be changed too, for the same reasons.

A similar argument goes for MsgChannelOpenTry.

[ancazamfir]

In the first version of IBC, the user was managing the channel IDs. So you could specify fully the two ends of the channel in MsgChanOpenInit and MsgChanOpenTry. And we implemented this version.
I also remember a discussion with @adizere about using the ChannelEnd vs enumerating the full list of params. And even if the state was/is completely out of place there and ignored we decided for the re-use of the datastructure.

Then next IBC spec version changed with the chain being the one to manage the IDs. We haven't changed the handler signatures in our impls. Maybe by that time we got used to them the way they were and it was faster to just make the ID optional.

Anyway that's a bit of history and I personally don't mind changing the handlers.
But I think there will be resistence on protobuf changes.

[adizere]

In the first version of IBC, the user was managing the channel IDs.

Exactly. As Anca described, we also tried to simplify handler interfaces because they were very complex (having a long list of parameters) but we had varying success with that initiative.

Aligning with IBC-go would be a good idea. This refactor should be fixed upstream (in specs) first, to make our dependencies coherent, then we can proceed here. @plafer would you mind opening an issue in the specs repo and providing the rational there?

[romac]

I am wary of changing the protos, as this might create backward compatibility problems which which we've already struggled quite a bit with. I would vote for changing the Msg structs but leave the protos as is.