Skip to content

Commit

Permalink
Fixes after testing with GM (informalsystems#974)
Browse files Browse the repository at this point in the history
* Fix panic when all chains are unreachable

* Fix for reverse params in client worker.

* FMT fix

* Removed useseless format

* Better comment. Changelog
  • Loading branch information
adizere authored May 25, 2021
1 parent d462445 commit 8f6ad19
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 8 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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*
Expand Down
7 changes: 7 additions & 0 deletions relayer/src/foreign_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ pub struct ForeignClient {
pub src_chain: Box<dyn ChainHandle>,
}

/// 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!(
Expand Down
14 changes: 8 additions & 6 deletions relayer/src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down Expand Up @@ -148,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<Self, BoxError> {
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())
Expand Down
5 changes: 5 additions & 0 deletions relayer/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion relayer/src/supervisor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -273,6 +273,12 @@ impl Supervisor {
}
}

// 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());
}

self.spawn_workers();

loop {
Expand Down
2 changes: 1 addition & 1 deletion relayer/src/worker/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ impl ClientWorker {
pub fn run(self) -> Result<(), BoxError> {
let mut client = ForeignClient::restore(
self.client.dst_client_id.clone(),
self.chains.a.clone(),
self.chains.b.clone(),
self.chains.a.clone(),
);

info!(
Expand Down

0 comments on commit 8f6ad19

Please sign in to comment.