Exclude ChannelEnd from MsgChannelOpenInit and MsgChannelOpenTry

.changelog/unreleased/breaking-changes/20-refactor-channel-msgs.md

@@ -0,0 +1,2 @@
+- Exclude `ChannelEnd` from `MsgChannelOpenInit` and `MsgChannelOpenTry` and refactor their fields to match the spec
+  ([#20](https://github.com/cosmos/ibc-rs/issues/20))

crates/ibc/src/core/ics04_channel/handler.rs

@@ -116,24 +116,24 @@ where
     match msg {
         ChannelMsg::ChannelOpenInit(msg) => {
             let (extras, version) = cb.on_chan_open_init(
-                msg.chan_end_on_a.ordering,
-                &msg.chan_end_on_a.connection_hops,
+                msg.ordering_on_a,
+                &msg.connection_hops_on_a,
                 &msg.port_id_on_a,
                 &result.channel_id,
-                msg.chan_end_on_a.counterparty(),
-                &msg.chan_end_on_a.version,
+                &Counterparty::new(msg.port_id_on_b.clone(), msg.chan_id_on_b.clone()),
+                &msg.version_on_a,
             )?;
             result.channel_end.version = version;
 
             Ok(extras)
         }
         ChannelMsg::ChannelOpenTry(msg) => {
             let (extras, version) = cb.on_chan_open_try(
-                msg.chan_end_on_b.ordering,
-                &msg.chan_end_on_b.connection_hops,
+                msg.ordering_on_b,
+                &msg.connection_hops_on_b,
                 &msg.port_id_on_b,
                 &result.channel_id,
-                msg.chan_end_on_b.counterparty(),
+                &Counterparty::new(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone()),
                 &msg.version_on_a,
             )?;
             result.channel_end.version = version;

crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs

@@ -198,24 +198,24 @@ mod tests {
 
         let chan_end = ChannelEnd::new(
             State::Init,
-            *msg_chan_try.chan_end_on_b.ordering(),
+            msg_chan_try.ordering_on_b,
             Counterparty::new(
                 msg_chan_ack.port_id_on_a.clone(),
                 Some(msg_chan_ack.chan_id_on_a.clone()),
             ),
             connection_vec0.clone(),
-            msg_chan_try.chan_end_on_b.version().clone(),
+            msg_chan_try.version_on_a,
         );
 
         let failed_chan_end = ChannelEnd::new(
             State::Open,
-            *msg_chan_try.chan_end_on_b.ordering(),
+            msg_chan_try.ordering_on_b,
             Counterparty::new(
                 msg_chan_ack.port_id_on_a.clone(),
                 Some(msg_chan_ack.chan_id_on_a.clone()),
             ),
             connection_vec0,
-            msg_chan_try.chan_end_on_b.version().clone(),
+            msg_chan_try.version_on_b,
         );
 
         let tests: Vec<Test> = vec![

crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs

@@ -1,6 +1,6 @@
 //! Protocol logic specific to ICS4 messages of type `MsgChannelOpenInit`.
 
-use crate::core::ics04_channel::channel::{ChannelEnd, State};
+use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, State};
 use crate::core::ics04_channel::context::ChannelReader;
 use crate::core::ics04_channel::error::ChannelError;
 use crate::core::ics04_channel::handler::{ChannelIdState, ChannelResult};
@@ -16,32 +16,32 @@ pub(crate) fn process<Ctx: ChannelReader>(
 ) -> HandlerResult<ChannelResult, ChannelError> {
     let mut output = HandlerOutput::builder();
 
-    if msg.chan_end_on_a.connection_hops().len() != 1 {
+    if msg.connection_hops_on_a.len() != 1 {
         return Err(ChannelError::InvalidConnectionHopsLength {
             expected: 1,
-            actual: msg.chan_end_on_a.connection_hops().len(),
+            actual: msg.connection_hops_on_a.len(),
         });
     }
 
     // An IBC connection running on the local (host) chain should exist.
-    let conn_end_on_a = ctx_a.connection_end(&msg.chan_end_on_a.connection_hops()[0])?;
+    let conn_end_on_a = ctx_a.connection_end(&msg.connection_hops_on_a[0])?;
 
     let conn_version = match conn_end_on_a.versions() {
         [version] => version,
         _ => return Err(ChannelError::InvalidVersionLengthConnection),
     };
 
-    let channel_feature = msg.chan_end_on_a.ordering().to_string();
+    let channel_feature = msg.ordering_on_a.to_string();
     if !conn_version.is_supported_feature(channel_feature) {
         return Err(ChannelError::ChannelFeatureNotSuportedByConnection);
     }
 
     let chan_end_on_a = ChannelEnd::new(
         State::Init,
-        *msg.chan_end_on_a.ordering(),
-        msg.chan_end_on_a.counterparty().clone(),
-        msg.chan_end_on_a.connection_hops().clone(),
-        msg.chan_end_on_a.version().clone(),
+        msg.ordering_on_a,
+        Counterparty::new(msg.port_id_on_b.clone(), msg.chan_id_on_b.clone()),
+        msg.connection_hops_on_a.clone(),
+        msg.version_on_a.clone(),
     );
 
     let chan_id_on_a = ChannelId::new(ctx_a.channel_counter()?);

crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs

@@ -19,17 +19,17 @@ pub(crate) fn process<Ctx: ChannelReader>(
     let mut output = HandlerOutput::builder();
 
     // An IBC connection running on the local (host) chain should exist.
-    if msg.chan_end_on_b.connection_hops().len() != 1 {
+    if msg.connection_hops_on_b.len() != 1 {
         return Err(ChannelError::InvalidConnectionHopsLength {
             expected: 1,
-            actual: msg.chan_end_on_b.connection_hops().len(),
+            actual: msg.connection_hops_on_b.len(),
         });
     }
 
-    let conn_end_on_b = ctx_b.connection_end(&msg.chan_end_on_b.connection_hops()[0])?;
+    let conn_end_on_b = ctx_b.connection_end(&msg.connection_hops_on_b[0])?;
     if !conn_end_on_b.state_matches(&ConnectionState::Open) {
         return Err(ChannelError::ConnectionNotOpen {
-            connection_id: msg.chan_end_on_b.connection_hops()[0].clone(),
+            connection_id: msg.connection_hops_on_b[0].clone(),
         });
     }
 
@@ -38,7 +38,7 @@ pub(crate) fn process<Ctx: ChannelReader>(
         _ => return Err(ChannelError::InvalidVersionLengthConnection),
     };
 
-    let channel_feature = msg.chan_end_on_b.ordering().to_string();
+    let channel_feature = msg.ordering_on_b.to_string();
     if !conn_version.is_supported_feature(channel_feature) {
         return Err(ChannelError::ChannelFeatureNotSuportedByConnection);
     }
@@ -50,15 +50,14 @@ pub(crate) fn process<Ctx: ChannelReader>(
         let consensus_state_of_a_on_b =
             ctx_b.client_consensus_state(&client_id_on_b, &msg.proof_height_on_a)?;
         let prefix_on_a = conn_end_on_b.counterparty().prefix();
-        let port_id_on_a = &msg.chan_end_on_b.counterparty().port_id;
+        let port_id_on_a = &&msg.port_id_on_a;
         let chan_id_on_a = msg
-            .chan_end_on_b
-            .counterparty()
-            .channel_id()
+            .chan_id_on_a
+            .clone()
             .ok_or(ChannelError::InvalidCounterpartyChannelId)?;
         let conn_id_on_a = conn_end_on_b.counterparty().connection_id().ok_or(
             ChannelError::UndefinedConnectionCounterparty {
-                connection_id: msg.chan_end_on_b.connection_hops()[0].clone(),
+                connection_id: msg.connection_hops_on_b[0].clone(),
             },
         )?;
 
@@ -71,7 +70,7 @@ pub(crate) fn process<Ctx: ChannelReader>(
 
         let expected_chan_end_on_a = ChannelEnd::new(
             State::Init,
-            *msg.chan_end_on_b.ordering(),
+            msg.ordering_on_b,
             Counterparty::new(msg.port_id_on_b.clone(), None),
             vec![conn_id_on_a.clone()],
             msg.version_on_a.clone(),
@@ -86,17 +85,17 @@ pub(crate) fn process<Ctx: ChannelReader>(
                 &msg.proof_chan_end_on_a,
                 consensus_state_of_a_on_b.root(),
                 port_id_on_a,
-                chan_id_on_a,
+                &chan_id_on_a,
                 &expected_chan_end_on_a,
             )
             .map_err(ChannelError::VerifyChannelFailed)?;
     }
 
     let chan_end_on_b = ChannelEnd::new(
         State::TryOpen,
-        *msg.chan_end_on_b.ordering(),
-        msg.chan_end_on_b.counterparty().clone(),
-        msg.chan_end_on_b.connection_hops().clone(),
+        msg.ordering_on_b,
+        Counterparty::new(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone()),
+        msg.connection_hops_on_b.clone(),
         // Note: This will be rewritten by the module callback
         Version::empty(),
     );
@@ -132,7 +131,7 @@ mod tests {
     use crate::core::ics03_connection::error as ics03_error;
     use crate::core::ics03_connection::msgs::test_util::get_dummy_raw_counterparty;
     use crate::core::ics03_connection::version::get_compatible_versions;
-    use crate::core::ics04_channel::channel::{ChannelEnd, State};
+    use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, State};
     use crate::core::ics04_channel::error;
     use crate::core::ics04_channel::msgs::chan_open_try::test_util::get_dummy_raw_msg_chan_open_try;
     use crate::core::ics04_channel::msgs::chan_open_try::MsgChannelOpenTry;
@@ -176,16 +175,16 @@ mod tests {
 
         let chan_id = ChannelId::new(24);
         let hops = vec![conn_id.clone()];
-        msg.chan_end_on_b.connection_hops = hops;
+        msg.connection_hops_on_b = hops;
 
         // A preloaded channel end that resides in the context. This is constructed so as to be
         // consistent with the incoming ChanOpenTry message `msg`.
         let correct_chan_end = ChannelEnd::new(
             State::Init,
-            *msg.chan_end_on_b.ordering(),
-            msg.chan_end_on_b.counterparty().clone(),
-            msg.chan_end_on_b.connection_hops().clone(),
-            msg.chan_end_on_b.version().clone(),
+            msg.ordering_on_b,
+            Counterparty::new(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone()),
+            msg.connection_hops_on_b.clone(),
+            msg.version_on_b.clone(),
         );
 
         let tests: Vec<Test> = vec![
@@ -195,7 +194,7 @@ mod tests {
                 msg: ChannelMsg::ChannelOpenTry(msg.clone()),
                 want_pass: false,
                 match_error: {
-                    let connection_id = msg.chan_end_on_b.connection_hops()[0].clone();
+                    let connection_id = msg.connection_hops_on_b[0].clone();
                     Box::new(move |e| match e {
                         error::ChannelError::Connection(e) => {
                             assert_eq!(
@@ -320,18 +319,18 @@ mod tests {
         // Note: we make the counterparty's channel_id `None`.
         let mut msg =
             MsgChannelOpenTry::try_from(get_dummy_raw_msg_chan_open_try(proof_height)).unwrap();
-        msg.chan_end_on_b.remote.channel_id = None;
+        msg.chan_id_on_a = None;
 
         let chan_id = ChannelId::new(24);
         let hops = vec![conn_id.clone()];
-        msg.chan_end_on_b.connection_hops = hops;
+        msg.connection_hops_on_b = hops;
 
         let chan_end = ChannelEnd::new(
             State::Init,
-            *msg.chan_end_on_b.ordering(),
-            msg.chan_end_on_b.counterparty().clone(),
-            msg.chan_end_on_b.connection_hops().clone(),
-            msg.chan_end_on_b.version().clone(),
+            msg.ordering_on_b,
+            Counterparty::new(msg.port_id_on_b.clone(), msg.chan_id_on_a.clone()),
+            msg.connection_hops_on_b.clone(),
+            msg.version_on_b.clone(),
         );
         let context = MockContext::default()
             .with_client(&client_id, Height::new(0, proof_height).unwrap())

crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs

@@ -1,6 +1,9 @@
 use crate::core::ics04_channel::channel::ChannelEnd;
+use crate::core::ics04_channel::channel::Counterparty;
+use crate::core::ics04_channel::channel::{Order, State};
 use crate::core::ics04_channel::error::ChannelError;
-use crate::core::ics24_host::identifier::PortId;
+use crate::core::ics04_channel::Version;
+use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId};
 use crate::prelude::*;
 use crate::signer::Signer;
 use crate::tx_msg::Msg;
@@ -17,15 +20,31 @@ pub const TYPE_URL: &str = "/ibc.core.channel.v1.MsgChannelOpenInit";
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct MsgChannelOpenInit {
     pub port_id_on_a: PortId,
-    pub chan_end_on_a: ChannelEnd,
+    pub ordering_on_a: Order,
+    pub version_on_a: Version,
+    pub connection_hops_on_a: Vec<ConnectionId>,
+    pub port_id_on_b: PortId,
+    pub chan_id_on_b: Option<ChannelId>,
     pub signer: Signer,
 }
 
 impl MsgChannelOpenInit {
-    pub fn new(port_id_on_a: PortId, chan_end_on_a: ChannelEnd, signer: Signer) -> Self {
+    pub fn new(
+        port_id_on_a: PortId,
+        ordering_on_a: Order,
+        connection_hops_on_a: Vec<ConnectionId>,
+        port_id_on_b: PortId,
+        version_on_a: Version,
+        chan_id_on_b: Option<ChannelId>,
+        signer: Signer,
+    ) -> Self {
         Self {
             port_id_on_a,
-            chan_end_on_a,
+            ordering_on_a,
+            version_on_a,
+            connection_hops_on_a,
+            port_id_on_b,
+            chan_id_on_b,
             signer,
         }
     }
@@ -45,22 +64,33 @@ impl TryFrom<RawMsgChannelOpenInit> for MsgChannelOpenInit {
     type Error = ChannelError;
 
     fn try_from(raw_msg: RawMsgChannelOpenInit) -> Result<Self, Self::Error> {
+        let chan_end_on_a: ChannelEnd = raw_msg
+            .channel
+            .ok_or(ChannelError::MissingChannel)?
+            .try_into()?;
         Ok(MsgChannelOpenInit {
             port_id_on_a: raw_msg.port_id.parse().map_err(ChannelError::Identifier)?,
-            chan_end_on_a: raw_msg
-                .channel
-                .ok_or(ChannelError::MissingChannel)?
-                .try_into()?,
+            ordering_on_a: chan_end_on_a.ordering,
+            connection_hops_on_a: chan_end_on_a.connection_hops,
+            port_id_on_b: chan_end_on_a.remote.port_id,
+            chan_id_on_b: chan_end_on_a.remote.channel_id,
+            version_on_a: chan_end_on_a.version,
             signer: raw_msg.signer.parse().map_err(ChannelError::Signer)?,
         })
     }
 }
-
 impl From<MsgChannelOpenInit> for RawMsgChannelOpenInit {
     fn from(domain_msg: MsgChannelOpenInit) -> Self {
+        let chan_end_on_a = ChannelEnd::new(
+            State::Init,
+            domain_msg.ordering_on_a,
+            Counterparty::new(domain_msg.port_id_on_b, domain_msg.chan_id_on_b),
+            domain_msg.connection_hops_on_a,
+            domain_msg.version_on_a,
+        );
         RawMsgChannelOpenInit {
             port_id: domain_msg.port_id_on_a.to_string(),
-            channel: Some(domain_msg.chan_end_on_a.into()),
+            channel: Some(chan_end_on_a.into()),
             signer: domain_msg.signer.to_string(),
         }
     }