Replace ChannelConfig in Channel::new (#511)

* Unify Connection::new and Channel::new

* Replace RelayPath argument in Channel::new

* Update CHANGELOG

* Change Connection::set_connection_id to take a owned value as argument

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased Changes
 
+- Replace `ChannelConfig` in `Channel::new` ([#511])
+
 ### FEATURES
 
 - Add support for streamlining releases ([#507])
@@ -12,6 +14,7 @@
 
 
 [#507]: https://github.com/informalsystems/ibc-rs/issues/507
+[#511]: https://github.com/informalsystems/ibc-rs/pull/511
 [#517]: https://github.com/informalsystems/ibc-rs/issues/517
 
 ## v0.0.6

relayer-cli/src/commands/tx/channel.rs

@@ -67,18 +67,18 @@ macro_rules! chan_open_cmd {
                 let opts = ChannelConfig {
                     ordering: self.ordering,
                     a_config: ChannelConfigSide::new(
-                        &src_chain_config.id,
-                        &ConnectionId::default(),
-                        &ClientId::default(),
-                        &self.src_port_id,
-                        &self.src_channel_id,
+                        src_chain_config.id.clone(),
+                        ClientId::default(),
+                        ConnectionId::default(),
+                        self.src_port_id.clone(),
+                        self.src_channel_id.clone(),
                     ),
                     b_config: ChannelConfigSide::new(
-                        &dst_chain_config.id,
-                        &self.dst_connection_id,
-                        &ClientId::default(),
-                        &self.dst_port_id,
-                        &self.dst_channel_id,
+                        dst_chain_config.id.clone(),
+                        ClientId::default(),
+                        self.dst_connection_id.clone(),
+                        self.dst_port_id.clone(),
+                        self.dst_channel_id.clone(),
                     ),
                 };
 

relayer-cli/src/commands/tx/connection.rs

@@ -61,13 +61,13 @@ macro_rules! conn_open_cmd {
                 let opts = ConnectionConfig {
                     a_config: ConnectionSideConfig::new(
                         src_chain_config.id.clone(),
-                        self.src_connection_id.clone(),
                         self.src_client_id.clone(),
+                        self.src_connection_id.clone(),
                     ),
                     b_config: ConnectionSideConfig::new(
                         dst_chain_config.id.clone(),
-                        self.dst_connection_id.clone(),
                         self.dst_client_id.clone(),
+                        self.dst_connection_id.clone(),
                     ),
                 };
 

relayer-cli/src/commands/v0.rs

@@ -4,9 +4,9 @@ use abscissa_core::{
     application::fatal_error, error::BoxError, tracing::debug, Command, Options, Runnable,
 };
 
+use ibc::ics04_channel::channel::Order;
+
 use relayer::chain::runtime::ChainRuntime;
-use relayer::channel::ChannelConfig;
-use relayer::connection::ConnectionConfig;
 use relayer::relay::channel_relay;
 
 use crate::config::Config;
@@ -38,18 +38,16 @@ pub fn v0_task(config: &Config) -> Result<(), BoxError> {
         .clone()
         .ok_or("No connections configured")?[0];
 
-    let path = &conn.paths.clone().ok_or("No paths configured")?[0];
-
-    let connection_cfg = ConnectionConfig::new(&conn.clone())?;
-    let channel_cfg = ChannelConfig::new(&connection_cfg, &path)?;
+    let ordering = Order::default(); // TODO - add to config
+    let path = conn.paths.clone().ok_or("No paths configured")?[0].clone();
 
     let src_chain_config = config
-        .find_chain(connection_cfg.src().chain_id())
+        .find_chain(&conn.a_chain)
         .cloned()
         .ok_or("Configuration for source chain not found")?;
 
     let dst_chain_config = config
-        .find_chain(connection_cfg.dst().chain_id())
+        .find_chain(&conn.b_chain)
         .cloned()
         .ok_or("Configuration for source chain not found")?;
 
@@ -59,6 +57,7 @@ pub fn v0_task(config: &Config) -> Result<(), BoxError> {
     Ok(channel_relay(
         src_chain_handle,
         dst_chain_handle,
-        channel_cfg,
+        ordering,
+        path,
     )?)
 }

relayer/src/channel.rs

@@ -20,8 +20,7 @@ use ibc::tx_msg::Msg;
 use ibc::Height;
 
 use crate::chain::handle::ChainHandle;
-use crate::config::RelayPath;
-use crate::connection::{Connection, ConnectionConfig};
+use crate::connection::Connection;
 use crate::error::{Error, Kind};
 use crate::foreign_client::build_update_client;
 use crate::relay::MAX_ITER;
@@ -35,56 +34,48 @@ pub enum ChannelError {
 #[derive(Clone, Debug)]
 pub struct ChannelConfigSide {
     chain_id: ChainId,
-    connection_id: ConnectionId,
     client_id: ClientId,
+    connection_id: ConnectionId,
     port_id: PortId,
     channel_id: ChannelId,
 }
 
 impl ChannelConfigSide {
     pub fn new(
-        chain_id: &ChainId,
-        connection_id: &ConnectionId,
-        client_id: &ClientId,
-        port_id: &PortId,
-        channel_id: &ChannelId,
+        chain_id: ChainId,
+        client_id: ClientId,
+        connection_id: ConnectionId,
+        port_id: PortId,
+        channel_id: ChannelId,
     ) -> ChannelConfigSide {
         Self {
-            chain_id: chain_id.clone(),
-            connection_id: connection_id.clone(),
-            client_id: client_id.clone(),
-            port_id: port_id.clone(),
-            channel_id: channel_id.clone(),
+            chain_id,
+            client_id,
+            connection_id,
+            port_id,
+            channel_id,
         }
     }
 
     pub fn chain_id(&self) -> &ChainId {
         &self.chain_id
     }
 
-    pub fn connection_id(&self) -> &ConnectionId {
-        &self.connection_id
-    }
-
     pub fn client_id(&self) -> &ClientId {
         &self.client_id
     }
 
+    pub fn connection_id(&self) -> &ConnectionId {
+        &self.connection_id
+    }
+
     pub fn port_id(&self) -> &PortId {
         &self.port_id
     }
 
     pub fn channel_id(&self) -> &ChannelId {
         &self.channel_id
     }
-
-    pub fn set_client_id(&mut self, id: &ClientId) {
-        self.client_id = id.clone()
-    }
-
-    pub fn set_connection_id(&mut self, id: &ConnectionId) {
-        self.connection_id = id.clone()
-    }
 }
 
 #[derive(Clone, Debug)]
@@ -126,48 +117,32 @@ pub struct Channel {
     connection: Connection,
 }
 
-impl ChannelConfig {
-    pub fn new(conn: &ConnectionConfig, path: &RelayPath) -> Result<ChannelConfig, String> {
-        let a_config = ChannelConfigSide {
-            chain_id: conn.a_end().chain_id().clone(),
-            connection_id: ConnectionId::default(),
-            client_id: ClientId::default(),
-            port_id: path.a_port.clone(),
-            channel_id: ChannelId::default(),
-        };
-
-        let b_config = ChannelConfigSide {
-            chain_id: conn.b_end().chain_id().clone(),
-            connection_id: ConnectionId::default(),
-            client_id: ClientId::default(),
-            port_id: path.b_port.clone(),
-            channel_id: ChannelId::default(),
-        };
-
-        Ok(ChannelConfig {
-            ordering: Default::default(), // TODO - add to config
-            a_config,
-            b_config,
-        })
-    }
-}
-
 impl Channel {
     /// Creates a new channel on top of the existing connection. If the channel is not already
     /// set-up on both sides of the connection, this functions also fulfils the channel handshake.
-    pub fn new(connection: Connection, mut config: ChannelConfig) -> Result<Channel, ChannelError> {
-        config
-            .a_config
-            .set_client_id(connection.config.a_config.client_id());
-        config
-            .b_config
-            .set_client_id(connection.config.b_config.client_id());
-        config
-            .a_config
-            .set_connection_id(connection.config.a_config.connection_id());
-        config
-            .b_config
-            .set_connection_id(connection.config.b_config.connection_id());
+    pub fn new(
+        connection: Connection,
+        ordering: Order,
+        a_port: PortId,
+        b_port: PortId,
+    ) -> Result<Channel, ChannelError> {
+        let config = ChannelConfig {
+            ordering,
+            a_config: ChannelConfigSide::new(
+                connection.config.a_config.chain_id().clone(),
+                connection.config.a_config.client_id().clone(),
+                connection.config.a_config.connection_id().clone(),
+                a_port,
+                Default::default(),
+            ),
+            b_config: ChannelConfigSide::new(
+                connection.config.b_config.chain_id().clone(),
+                connection.config.b_config.client_id().clone(),
+                connection.config.b_config.connection_id().clone(),
+                b_port,
+                Default::default(),
+            ),
+        };
         let mut channel = Channel { config, connection };
         channel.handshake()?;
         Ok(channel)

relayer/src/connection.rs

@@ -20,7 +20,6 @@ use ibc::tx_msg::Msg;
 use ibc::Height as ICSHeight;
 
 use crate::chain::handle::ChainHandle;
-use crate::config;
 use crate::error::{Error, Kind};
 use crate::foreign_client::{build_update_client, ForeignClient};
 use crate::relay::MAX_ITER;
@@ -44,37 +43,37 @@ pub struct Connection {
 #[derive(Clone, Debug)]
 pub struct ConnectionSideConfig {
     chain_id: ChainId,
-    connection_id: ConnectionId,
     client_id: ClientId,
+    connection_id: ConnectionId,
 }
 
 impl ConnectionSideConfig {
     pub fn new(
         chain_id: ChainId,
-        connection_id: ConnectionId,
         client_id: ClientId,
+        connection_id: ConnectionId,
     ) -> ConnectionSideConfig {
         Self {
             chain_id,
-            connection_id,
             client_id,
+            connection_id,
         }
     }
 
     pub fn chain_id(&self) -> &ChainId {
         &self.chain_id
     }
 
-    pub fn connection_id(&self) -> &ConnectionId {
-        &self.connection_id
-    }
-
     pub fn client_id(&self) -> &ClientId {
         &self.client_id
     }
 
-    pub fn set_connection_id(&mut self, id: &ConnectionId) {
-        self.connection_id = id.clone()
+    pub fn connection_id(&self) -> &ConnectionId {
+        &self.connection_id
+    }
+
+    pub(crate) fn set_connection_id(&mut self, id: ConnectionId) {
+        self.connection_id = id;
     }
 }
 
@@ -109,24 +108,6 @@ impl ConnectionConfig {
     }
 }
 
-impl ConnectionConfig {
-    pub fn new(conn: &config::Connection) -> Result<ConnectionConfig, String> {
-        let a_config = ConnectionSideConfig {
-            chain_id: conn.a_chain.clone(),
-            connection_id: ConnectionId::default(),
-            client_id: ClientId::default(),
-        };
-
-        let b_config = ConnectionSideConfig {
-            chain_id: conn.b_chain.clone(),
-            connection_id: ConnectionId::default(),
-            client_id: ClientId::default(),
-        };
-
-        Ok(ConnectionConfig { a_config, b_config })
-    }
-}
-
 impl Connection {
     /// Create a new connection, ensuring that the handshake has succeeded and the two connection
     /// ends exist on each side.
@@ -154,13 +135,13 @@ impl Connection {
             config: ConnectionConfig {
                 a_config: ConnectionSideConfig::new(
                     a_client.dst_chain().id(),
-                    Default::default(),
                     a_client.id().clone(),
+                    Default::default(),
                 ),
                 b_config: ConnectionSideConfig::new(
                     b_client.dst_chain().id(),
-                    Default::default(),
                     b_client.id().clone(),
+                    Default::default(),
                 ),
             },
             a_client,
@@ -201,9 +182,8 @@ impl Connection {
                     continue;
                 }
                 Ok(result) => {
-                    self.config
-                        .a_config
-                        .set_connection_id(extract_connection_id(&result)?);
+                    let connection_id = extract_connection_id(&result)?.clone();
+                    self.config.a_config.set_connection_id(connection_id);
                     info!("{}  {} => {:?}\n", done, a_chain.id(), result);
                     break;
                 }
@@ -222,9 +202,8 @@ impl Connection {
                     continue;
                 }
                 Ok(result) => {
-                    self.config
-                        .b_config
-                        .set_connection_id(extract_connection_id(&result)?);
+                    let connection_id = extract_connection_id(&result)?.clone();
+                    self.config.b_config.set_connection_id(connection_id);
                     info!("{}  {} => {:?}\n", done, b_chain.id(), result);
                     break;
                 }