Promote start-multi to start (#971)

* Promote `start-multi` command to `start`

* Cleanup ForeignClient::restore

* Remove unused `relayer::relay` module

* Remove unused `Link::relay` method

* Update e2e test suite

* Update changelog

CHANGELOG.md

@@ -1,5 +1,14 @@
 # Changelog
 
+## Unreleased
+
+### BREAKING CHANGES
+
+- [ibc-relayer-cli]
+  - Promote `start-multi` command to `start` ([#911])
+
+[#911]: https://github.com/informalsystems/ibc-rs/issues/911
+
 ## v0.3.2
 *May 21st, 2021*
 

e2e/e2e/relayer.py

@@ -6,6 +6,6 @@
 
 
 def start(c: Config) -> Popen:
-    full_cmd = f'{c.relayer_cmd} -c {c.config_file} -j start-multi'.split(' ')
+    full_cmd = f'{c.relayer_cmd} -c {c.config_file} -j start'.split(' ')
     l.debug(' '.join(full_cmd))
     return Popen(full_cmd)

relayer-cli/src/commands.rs

@@ -17,8 +17,7 @@ use crate::DEFAULT_CONFIG_PATH;
 
 use self::{
     create::CreateCmds, keys::KeysCmd, listen::ListenCmd, query::QueryCmd, start::StartCmd,
-    start_multi::StartMultiCmd, tx::TxCmd, update::UpdateCmds, upgrade::UpgradeCmds,
-    version::VersionCmd,
+    tx::TxCmd, update::UpdateCmds, upgrade::UpgradeCmds, version::VersionCmd,
 };
 use crate::commands::misbehaviour::MisbehaviourCmd;
 
@@ -29,7 +28,6 @@ mod listen;
 mod misbehaviour;
 mod query;
 mod start;
-mod start_multi;
 mod tx;
 mod update;
 mod upgrade;
@@ -70,13 +68,9 @@ pub enum CliCmd {
     Upgrade(UpgradeCmds),
 
     /// The `start` subcommand
-    #[options(help = "Start the relayer")]
-    Start(StartCmd),
-
-    /// The `start-multi` subcommand
     #[options(help = "Start the relayer in multi-chain mode. \
                       Handles packet relaying across all open channels between all chains in the config.")]
-    StartMulti(StartMultiCmd),
+    Start(StartCmd),
 
     /// The `query` subcommand
     #[options(help = "Query objects from the chain")]

relayer-cli/src/commands/misbehaviour.rs

@@ -117,7 +117,7 @@ fn misbehaviour_handling(
             )
         })?;
 
-    let client = ForeignClient::restore(&client_id, chain.clone(), counterparty_chain.clone());
+    let client = ForeignClient::restore(client_id, chain, counterparty_chain);
     let result = client.detect_misbehaviour_and_submit_evidence(update);
     if let MisbehaviourResults::EvidenceSubmitted(events) = result {
         info!("evidence submission result {:?}", events);

relayer-cli/src/commands/start.rs

@@ -1,78 +1,20 @@
 use abscissa_core::{Command, Options, Runnable};
 
-use ibc::ics24_host::identifier::{ChainId, ChannelId, PortId};
-use ibc_relayer::link::LinkParameters;
-use ibc_relayer::relay::{channel_relay, relay_on_new_link};
+use ibc_relayer::supervisor::Supervisor;
 
-use crate::cli_utils::ChainHandlePair;
 use crate::conclude::Output;
 use crate::prelude::*;
 
 #[derive(Clone, Command, Debug, Options)]
-pub struct StartCmd {
-    #[options(free, required, help = "identifier of the source chain")]
-    src_chain_id: ChainId,
-
-    #[options(free, required, help = "identifier of the destination chain")]
-    dst_chain_id: ChainId,
-
-    #[options(help = "identifier of the source port", short = "p")]
-    src_port_id: Option<PortId>,
-
-    #[options(help = "identifier of the source channel", short = "c")]
-    src_channel_id: Option<ChannelId>,
-}
+pub struct StartCmd {}
 
 impl Runnable for StartCmd {
     fn run(&self) {
         let config = app_config();
-
-        let chains = match ChainHandlePair::spawn(&config, &self.src_chain_id, &self.dst_chain_id) {
-            Ok(chains) => chains,
-            Err(e) => return Output::error(format!("{}", e)).exit(),
-        };
-
-        match (&self.src_port_id, &self.src_channel_id) {
-            (Some(src_port_id), Some(src_channel_id)) => {
-                match channel_relay(
-                    chains.src,
-                    chains.dst,
-                    LinkParameters {
-                        src_port_id: src_port_id.clone(),
-                        src_channel_id: src_channel_id.clone(),
-                    },
-                ) {
-                    Ok(()) => Output::success(()).exit(),
-                    Err(e) => Output::error(e.to_string()).exit(),
-                }
-            }
-            (None, None) => {
-                // Relay for a single channel, first on the first connection between the two chains
-                let relay_path = config.first_matching_path(&self.src_chain_id, &self.dst_chain_id);
-
-                match relay_path {
-                    Some((connection, path)) => {
-                        info!("Start relayer on {:?}", self);
-
-                        match relay_on_new_link(
-                            chains.src,
-                            chains.dst,
-                            connection.delay,
-                            path.ordering,
-                            path.clone(),
-                        ) {
-                            Ok(()) => Output::success(()).exit(),
-                            Err(e) => Output::error(e.to_string()).exit(),
-                        }
-                    }
-                    None => Output::error(format!("No paths configured for {:?}", self)).exit(),
-                }
-            }
-            _ => Output::error(format!(
-                "Invalid parameters, either both port and channel must be specified or none: {:?}",
-                self
-            ))
-            .exit(),
+        let supervisor = Supervisor::spawn(config.clone()).expect("failed to spawn supervisor");
+        match supervisor.run() {
+            Ok(()) => Output::success_msg("done").exit(),
+            Err(e) => Output::error(e).exit(),
         }
     }
 }

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

@@ -34,7 +34,7 @@ impl Runnable for TxCreateClientCmd {
             Err(e) => return Output::error(format!("{}", e)).exit(),
         };
 
-        let client = ForeignClient::restore(&ClientId::default(), chains.dst, chains.src);
+        let client = ForeignClient::restore(ClientId::default(), chains.dst, chains.src);
 
         // Trigger client creation via the "build" interface, so that we obtain the resulting event
         let res: Result<IbcEvent, Error> = client

relayer/src/channel.rs

@@ -1,3 +1,7 @@
+#![allow(clippy::borrowed_box)]
+
+use std::time::Duration;
+
 use prost_types::Any;
 use serde::Serialize;
 use thiserror::Error;
@@ -19,8 +23,8 @@ use crate::chain::handle::ChainHandle;
 use crate::connection::Connection;
 use crate::error::Error;
 use crate::foreign_client::{ForeignClient, ForeignClientError};
-use crate::relay::MAX_ITER;
-use std::time::Duration;
+
+const MAX_RETRIES: usize = 5;
 
 #[derive(Debug, Error)]
 pub enum ChannelError {
@@ -137,12 +141,12 @@ impl Channel {
         Ok(channel)
     }
 
-    pub fn src_chain(&self) -> Box<dyn ChainHandle> {
-        self.a_side.chain.clone()
+    pub fn src_chain(&self) -> &Box<dyn ChainHandle> {
+        &self.a_side.chain
     }
 
-    pub fn dst_chain(&self) -> Box<dyn ChainHandle> {
-        self.b_side.chain.clone()
+    pub fn dst_chain(&self) -> &Box<dyn ChainHandle> {
+        &self.b_side.chain
     }
 
     pub fn src_client_id(&self) -> &ClientId {
@@ -191,13 +195,13 @@ impl Channel {
     fn handshake(&mut self) -> Result<(), ChannelError> {
         let done = '🥳';
 
-        let a_chain = self.src_chain();
-        let b_chain = self.dst_chain();
+        let a_chain = self.src_chain().clone();
+        let b_chain = self.dst_chain().clone();
 
         // Try chanOpenInit on a_chain
         let mut counter = 0;
         let mut init_success = false;
-        while counter < MAX_ITER {
+        while counter < MAX_RETRIES {
             counter += 1;
             match self.flipped().build_chan_open_init_and_send() {
                 Err(e) => {
@@ -217,14 +221,14 @@ impl Channel {
         if !init_success {
             return Err(ChannelError::Failed(format!(
                 "Failed to finish channel open init in {} iterations for {:?}",
-                MAX_ITER, self
+                MAX_RETRIES, self
             )));
         };
 
         // Try chanOpenTry on b_chain
         counter = 0;
         let mut try_success = false;
-        while counter < MAX_ITER {
+        while counter < MAX_RETRIES {
             counter += 1;
             match self.build_chan_open_try_and_send() {
                 Err(e) => {
@@ -243,12 +247,12 @@ impl Channel {
         if !try_success {
             return Err(ChannelError::Failed(format!(
                 "Failed to finish channel open try in {} iterations for {:?}",
-                MAX_ITER, self
+                MAX_RETRIES, self
             )));
         };
 
         counter = 0;
-        while counter < MAX_ITER {
+        while counter < MAX_RETRIES {
             counter += 1;
 
             // Continue loop if query error
@@ -298,13 +302,16 @@ impl Channel {
 
         Err(ChannelError::Failed(format!(
             "Failed to finish channel handshake in {} iterations for {:?}",
-            MAX_ITER, self
+            MAX_RETRIES, self
         )))
     }
 
     pub fn build_update_client_on_dst(&self, height: Height) -> Result<Vec<Any>, ChannelError> {
-        let client =
-            ForeignClient::restore(self.dst_client_id(), self.dst_chain(), self.src_chain());
+        let client = ForeignClient::restore(
+            self.dst_client_id().clone(),
+            self.dst_chain().clone(),
+            self.src_chain().clone(),
+        );
 
         client.build_update_client(height).map_err(|e| {
             ChannelError::ClientOperation(self.dst_client_id().clone(), self.dst_chain().id(), e)

relayer/src/connection.rs

@@ -22,11 +22,12 @@ use ibc::Height as ICSHeight;
 use crate::chain::handle::ChainHandle;
 use crate::error::Error;
 use crate::foreign_client::{ForeignClient, ForeignClientError};
-use crate::relay::MAX_ITER;
 
 /// Maximum value allowed for packet delay on any new connection that the relayer establishes.
 pub const MAX_PACKET_DELAY: Duration = Duration::from_secs(120);
 
+const MAX_RETRIES: usize = 5;
+
 #[derive(Debug, Error)]
 pub enum ConnectionError {
     #[error("failed with underlying cause: {0}")]
@@ -252,7 +253,7 @@ impl Connection {
 
         // Try connOpenInit on a_chain
         let mut counter = 0;
-        while counter < MAX_ITER {
+        while counter < MAX_RETRIES {
             counter += 1;
             match self.flipped().build_conn_init_and_send() {
                 Err(e) => {
@@ -269,7 +270,7 @@ impl Connection {
 
         // Try connOpenTry on b_chain
         counter = 0;
-        while counter < MAX_ITER {
+        while counter < MAX_RETRIES {
             counter += 1;
             match self.build_conn_try_and_send() {
                 Err(e) => {
@@ -285,7 +286,7 @@ impl Connection {
         }
 
         counter = 0;
-        while counter < MAX_ITER {
+        while counter < MAX_RETRIES {
             counter += 1;
 
             // Continue loop if query error
@@ -342,7 +343,7 @@ impl Connection {
 
         Err(ConnectionError::Failed(format!(
             "Failed to finish connection handshake in {:?} iterations",
-            MAX_ITER
+            MAX_RETRIES
         )))
     }
 
@@ -411,18 +412,14 @@ impl Connection {
     }
 
     pub fn build_update_client_on_src(&self, height: Height) -> Result<Vec<Any>, ConnectionError> {
-        let client =
-            ForeignClient::restore(self.src_client_id(), self.src_chain(), self.dst_chain());
-
+        let client = self.restore_src_client();
         client.build_update_client(height).map_err(|e| {
             ConnectionError::ClientOperation(self.src_client_id().clone(), self.src_chain().id(), e)
         })
     }
 
     pub fn build_update_client_on_dst(&self, height: Height) -> Result<Vec<Any>, ConnectionError> {
-        let client =
-            ForeignClient::restore(self.dst_client_id(), self.dst_chain(), self.src_chain());
-
+        let client = self.restore_dst_client();
         client.build_update_client(height).map_err(|e| {
             ConnectionError::ClientOperation(self.dst_client_id().clone(), self.dst_chain().id(), e)
         })
@@ -800,6 +797,22 @@ impl Connection {
             _ => panic!("internal error"),
         }
     }
+
+    fn restore_src_client(&self) -> ForeignClient {
+        ForeignClient::restore(
+            self.src_client_id().clone(),
+            self.src_chain().clone(),
+            self.dst_chain().clone(),
+        )
+    }
+
+    fn restore_dst_client(&self) -> ForeignClient {
+        ForeignClient::restore(
+            self.dst_client_id().clone(),
+            self.dst_chain().clone(),
+            self.src_chain().clone(),
+        )
+    }
 }
 
 fn extract_connection_id(event: &IbcEvent) -> Result<&ConnectionId, ConnectionError> {