Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes after testing with GM #974

Merged
merged 7 commits into from
May 25, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions relayer/src/foreign_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,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
7 changes: 6 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,11 @@ impl Supervisor {
}
}

// At least one chain runtime should be alive and kicking, otherwise bail.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

funny comment but what does it mean exactly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant to say that if the vector of chain runtimes (self.registry) is empty, that means no chain runtime could be instantiated, so the supervisor (i) cannot spawn any worker and (ii) has no subscription to any chain to select on. I'll use more accurate language here!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also informalsystems/ibc-rs#972 (comment)
I am not able to see that panic. What are the steps to see that? I tried to start hermes without any chains started and also tried to kill the chain procs while hermes is running.

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,
self.chains.a.clone(),
self.chains.b.clone(),
self.chains.a.clone(),
);

info!(
Expand Down