Rework ibc::ics04_channel::events module

[romac]

Crate

ibc

Summary

The various channel events defined in the ibc::ics04_channel::events module are all defined as newtypes over the same Attributes which has some optional fields which are expected to always be defined for specific events. As such, this forces us to unwrap these values in some places. We should refactor this module to perform this check when parsing the source event, and perhaps refactor the parsing code a little bit while we're at it.

---

For Admin Use

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

[Wizdave97]

@romac @adizere I'll like to work on this, I'll need a bit more detail on your idea of what th solution should look like.

[romac]

The main idea is to go from this:

https://github.com/informalsystems/ibc-rs/blob/e16d5f64fdf7bbd6e0fa9449a567ad9ee1b3c180/modules/src/core/ics04_channel/events.rs#L510-L541

to a bare struct with only the necessary fields made public:

#[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq)]
pub struct CloseInit {
    pub height: Height,
    pub port_id: PortId,
    pub channel_id: ChannelId,
    pub counterparty_port_id: PortId,
    pub counterparty_channel_id: Option<ChannelId>,
}

Note that now we statically ensure that CloseInit has a channel_id whereas we were previously forced to unwrap the Option<ChannelId> contained in the Attributes.

[Wizdave97]

@romac @adizere I've made a draft implementation here https://github.com/Wizdave97/ibc-rs/blob/david/rework-channel-events/modules/src/core/ics04_channel/events.rs, I want to know if that's on track, especially the use of TryFrom<Attributes> instead of From<Attributes> since the other events still use optional channel_id values, or do I enforce existence of channel_id on all other events?