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

Small refactor of ForeignClient::build_update_client* methods #1059

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
18 changes: 9 additions & 9 deletions relayer-cli/src/commands/tx/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,15 @@ impl Runnable for TxUpdateClientCmd {
Err(e) => return Output::error(format!("{}", e)).exit(),
};

let height = match self.target_height {
Some(height) => ibc::Height::new(src_chain.id().version(), height),
None => ibc::Height::zero(),
};

let trusted_height = match self.trusted_height {
Some(height) => ibc::Height::new(src_chain.id().version(), height),
None => ibc::Height::zero(),
};
let height = self
.target_height
.filter(|&height| height > 0)
.map(|height| ibc::Height::new(src_chain.id().version(), height));

let trusted_height = self
.trusted_height
.filter(|&height| height > 0)
.map(|height| ibc::Height::new(src_chain.id().version(), height));

let client = ForeignClient::find(src_chain, dst_chain, &self.dst_client_id)
.unwrap_or_else(exit_with_unrecoverable_error);
Expand Down
49 changes: 26 additions & 23 deletions relayer/src/foreign_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,24 +400,27 @@ impl ForeignClient {
&self,
target_height: Height,
) -> Result<Vec<Any>, ForeignClientError> {
self.build_update_client_with_trusted(target_height, Height::zero())
self.build_update_client_with_trusted(target_height, None)
}

/// Returns a vector with a message for updating the client to height `target_height`.
/// If the client already stores consensus states for this height, returns an empty vector.
pub fn build_update_client_with_trusted(
&self,
target_height: Height,
trusted_height: Height,
trusted_height: Option<Height>,
) -> Result<Vec<Any>, ForeignClientError> {
let latest_height = || {
self.src_chain().query_latest_height().map_err(|e| {
ForeignClientError::ClientUpdate(format!(
"failed fetching src chain latest height with error: {}",
e
))
})
};

// Wait for source chain to reach `target_height`
while self.src_chain().query_latest_height().map_err(|e| {
ForeignClientError::ClientUpdate(format!(
"failed fetching src chain latest height with error: {}",
e
))
})? < target_height
{
while latest_height()? < target_height {
Copy link
Member Author

Choose a reason for hiding this comment

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

@ancazamfir Do we really want to wait here in a loop? Shouldn't this be handled via retries at the worker level?

thread::sleep(Duration::from_millis(100))
}

Expand All @@ -435,9 +438,9 @@ impl ForeignClient {
// If not specified, set trusted state to the highest height smaller than target height.
// Otherwise ensure that a consensus state at trusted height exists on-chain.
let cs_heights = self.consensus_state_heights()?;
let trusted_height = if trusted_height == Height::zero() {
let trusted_height = match trusted_height {
// Get highest height smaller than target height
cs_heights
None => cs_heights
.into_iter()
.find(|h| h < &target_height)
.ok_or_else(|| {
Expand All @@ -446,9 +449,10 @@ impl ForeignClient {
self.dst_chain().id(),
target_height
))
})?
} else {
cs_heights
})?,

// Ensure that a consensus stsate at trusted height exists on chain
Some(trusted_height) => cs_heights
.into_iter()
.find(|h| h == &trusted_height)
.ok_or_else(|| {
Expand All @@ -457,14 +461,15 @@ impl ForeignClient {
self.dst_chain().id(),
trusted_height
))
})?
})?,
};

if trusted_height >= target_height {
warn!(
"[{}] skipping update: trusted height ({}) >= chain target height ({})",
self, trusted_height, target_height
);

return Ok(vec![]);
}

Expand Down Expand Up @@ -502,25 +507,23 @@ impl ForeignClient {
}

pub fn build_latest_update_client_and_send(&self) -> Result<IbcEvent, ForeignClientError> {
self.build_update_client_and_send(Height::zero(), Height::zero())
self.build_update_client_and_send(None, None)
}

pub fn build_update_client_and_send(
&self,
height: Height,
trusted_height: Height,
height: Option<Height>,
trusted_height: Option<Height>,
) -> Result<IbcEvent, ForeignClientError> {
let h = if height == Height::zero() {
let h = height.map(Ok).unwrap_or_else(|| {
self.src_chain.query_latest_height().map_err(|e| {
ForeignClientError::ClientUpdate(format!(
"failed while querying src chain ({}) for latest height: {}",
self.src_chain.id(),
e
))
})?
} else {
height
};
})
})?;

let new_msgs = self.build_update_client_with_trusted(h, trusted_height)?;
if new_msgs.is_empty() {
Expand Down