From bd9aefe8c6f0bed9fe7fb92d3131a178834b59c1 Mon Sep 17 00:00:00 2001 From: Adi Seredinschi Date: Fri, 21 May 2021 14:56:56 +0200 Subject: [PATCH 1/5] Fix panic when all chains are unreachable --- relayer/src/foreign_client.rs | 7 +++++++ relayer/src/object.rs | 4 +++- relayer/src/registry.rs | 5 +++++ relayer/src/supervisor.rs | 5 +++++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/relayer/src/foreign_client.rs b/relayer/src/foreign_client.rs index edd4fdf3e3..87356c6d42 100644 --- a/relayer/src/foreign_client.rs +++ b/relayer/src/foreign_client.rs @@ -76,6 +76,13 @@ pub struct ForeignClient { pub src_chain: Box, } +/// Used in Output messages. +/// Provides a concise description of a [`ForeignClient`], +/// using the format: +/// {CHAIN-ID} -> {CHAIN-ID}:{CLIENT} +/// where the first chain identifier is for the source +/// chain, and the second chain identifier is the +/// destination (which hosts the client) chain. impl fmt::Display for ForeignClient { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( diff --git a/relayer/src/object.rs b/relayer/src/object.rs index 7c460fca01..4185892ae1 100644 --- a/relayer/src/object.rs +++ b/relayer/src/object.rs @@ -18,12 +18,14 @@ use crate::chain::{ #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct Client { /// Destination chain identifier. + /// This is the chain hosting the client. pub dst_chain_id: ChainId, - /// Source channel identifier. + /// Client identifier (allocated on the destination chain `dst_chain_id`). pub dst_client_id: ClientId, /// Source chain identifier. + /// This is the chain whose headers the client worker is verifying. pub src_chain_id: ChainId, } diff --git a/relayer/src/registry.rs b/relayer/src/registry.rs index 86068f502f..005a037f56 100644 --- a/relayer/src/registry.rs +++ b/relayer/src/registry.rs @@ -32,6 +32,11 @@ impl Registry { } } + /// Return the size of the registry, i.e., the number of distinct chain runtimes. + pub fn size(&self) -> usize { + self.handles.len() + } + /// Get the [`ChainHandle`] associated with the given [`ChainId`]. /// /// If there is no handle yet, this will first spawn the runtime and then diff --git a/relayer/src/supervisor.rs b/relayer/src/supervisor.rs index 26fb5e1771..1e4cb5075e 100644 --- a/relayer/src/supervisor.rs +++ b/relayer/src/supervisor.rs @@ -273,6 +273,11 @@ impl Supervisor { } } + // At least one chain runtime should be alive and kicking, otherwise bail. + if self.registry.size() == 0 { + return Err(format!("supervisor was not able to connect to any chain").into()); + } + self.spawn_workers(); loop { From 2cba885da6dc257bf02e9154d2e2afc6e5a5a3da Mon Sep 17 00:00:00 2001 From: Adi Seredinschi Date: Fri, 21 May 2021 16:41:55 +0200 Subject: [PATCH 2/5] Fix for reverse params in client worker. --- relayer/src/object.rs | 10 +++++----- relayer/src/supervisor.rs | 2 +- relayer/src/worker/client.rs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/relayer/src/object.rs b/relayer/src/object.rs index 4185892ae1..0ae04f529f 100644 --- a/relayer/src/object.rs +++ b/relayer/src/object.rs @@ -150,27 +150,27 @@ impl Object { /// Build the client object associated with the given channel event attributes. pub fn for_chan_open_events( - e: &Attributes, - dst_chain: &dyn ChainHandle, + e: &Attributes, // The attributes of the emitted event + chain: &dyn ChainHandle, // The chain which emitted the event ) -> Result { let channel_id = e .channel_id() .as_ref() .ok_or_else(|| format!("channel_id missing in OpenAck event '{:?}'", e))?; - let client = channel_connection_client(dst_chain, e.port_id(), channel_id)?.client; + let client = channel_connection_client(chain, e.port_id(), channel_id)?.client; if client.client_state.refresh_period().is_none() { return Err(format!( "client '{}' on chain {} does not require refresh", client.client_id, - dst_chain.id() + chain.id() ) .into()); } Ok(Client { dst_client_id: client.client_id.clone(), - dst_chain_id: dst_chain.id(), + dst_chain_id: chain.id(), // The object's destination is the chain hosting the client src_chain_id: client.client_state.chain_id(), } .into()) diff --git a/relayer/src/supervisor.rs b/relayer/src/supervisor.rs index 1e4cb5075e..aca12fcaff 100644 --- a/relayer/src/supervisor.rs +++ b/relayer/src/supervisor.rs @@ -230,7 +230,7 @@ impl Supervisor { }); self.workers - .get_or_spawn(client_object, chain.clone(), counterparty_chain.clone()); + .get_or_spawn(client_object,counterparty_chain.clone(), chain.clone()); // TODO: Only start the Uni worker if there are outstanding packets or ACKs. // https://github.com/informalsystems/ibc-rs/issues/901 diff --git a/relayer/src/worker/client.rs b/relayer/src/worker/client.rs index 9e5c848ac6..979b0b21f5 100644 --- a/relayer/src/worker/client.rs +++ b/relayer/src/worker/client.rs @@ -33,8 +33,8 @@ impl ClientWorker { pub fn run(self) -> Result<(), BoxError> { let mut client = ForeignClient::restore( &self.client.dst_client_id, - self.chains.a.clone(), self.chains.b.clone(), + self.chains.a.clone(), ); info!( From a980bae702aabaee3cb8d1cd778fdd1c3046bad7 Mon Sep 17 00:00:00 2001 From: Adi Seredinschi Date: Fri, 21 May 2021 17:02:35 +0200 Subject: [PATCH 3/5] FMT fix --- relayer/src/object.rs | 6 +++--- relayer/src/supervisor.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/relayer/src/object.rs b/relayer/src/object.rs index 0ae04f529f..7462af1eb6 100644 --- a/relayer/src/object.rs +++ b/relayer/src/object.rs @@ -150,8 +150,8 @@ impl Object { /// Build the client object associated with the given channel event attributes. pub fn for_chan_open_events( - e: &Attributes, // The attributes of the emitted event - chain: &dyn ChainHandle, // The chain which emitted the event + e: &Attributes, // The attributes of the emitted event + chain: &dyn ChainHandle, // The chain which emitted the event ) -> Result { let channel_id = e .channel_id() @@ -170,7 +170,7 @@ impl Object { Ok(Client { dst_client_id: client.client_id.clone(), - dst_chain_id: chain.id(), // The object's destination is the chain hosting the client + dst_chain_id: chain.id(), // The object's destination is the chain hosting the client src_chain_id: client.client_state.chain_id(), } .into()) diff --git a/relayer/src/supervisor.rs b/relayer/src/supervisor.rs index aca12fcaff..5fe9fc1cbe 100644 --- a/relayer/src/supervisor.rs +++ b/relayer/src/supervisor.rs @@ -230,7 +230,7 @@ impl Supervisor { }); self.workers - .get_or_spawn(client_object,counterparty_chain.clone(), chain.clone()); + .get_or_spawn(client_object, counterparty_chain.clone(), chain.clone()); // TODO: Only start the Uni worker if there are outstanding packets or ACKs. // https://github.com/informalsystems/ibc-rs/issues/901 From 9306d52e337359a1994ea522460b2fddb5e6d410 Mon Sep 17 00:00:00 2001 From: Adi Seredinschi Date: Fri, 21 May 2021 18:35:21 +0200 Subject: [PATCH 4/5] Removed useseless format --- relayer/src/supervisor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relayer/src/supervisor.rs b/relayer/src/supervisor.rs index 5fe9fc1cbe..6462d30f42 100644 --- a/relayer/src/supervisor.rs +++ b/relayer/src/supervisor.rs @@ -275,7 +275,7 @@ impl Supervisor { // At least one chain runtime should be alive and kicking, otherwise bail. if self.registry.size() == 0 { - return Err(format!("supervisor was not able to connect to any chain").into()); + return Err("supervisor was not able to connect to any chain".into()); } self.spawn_workers(); From e4f69b28b7b1bfeb377bbdfe952d8e46d22f95f4 Mon Sep 17 00:00:00 2001 From: Adi Seredinschi Date: Tue, 25 May 2021 13:44:17 +0200 Subject: [PATCH 5/5] Better comment. Changelog --- CHANGELOG.md | 6 ++++++ relayer/src/supervisor.rs | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a9773383c..44ef01348e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,18 @@ ## Unreleased +### BUG FIXES + +- [ibc-relayer] + - Fix for a client worker bug; Hermes `start` returns error if no chain is reachable ([#972]) + ### BREAKING CHANGES - [ibc-relayer-cli] - Promote `start-multi` command to `start` ([#911]) [#911]: https://github.com/informalsystems/ibc-rs/issues/911 +[#972]: https://github.com/informalsystems/ibc-rs/issues/972 ## v0.3.2 *May 21st, 2021* diff --git a/relayer/src/supervisor.rs b/relayer/src/supervisor.rs index 6462d30f42..6910dec96d 100644 --- a/relayer/src/supervisor.rs +++ b/relayer/src/supervisor.rs @@ -273,7 +273,8 @@ impl Supervisor { } } - // At least one chain runtime should be alive and kicking, otherwise bail. + // At least one chain runtime should be available, otherwise the supervisor + // cannot do anything and will hang indefinitely. if self.registry.size() == 0 { return Err("supervisor was not able to connect to any chain".into()); }