From 6fc3a28f339580fecc2ec2d8223b8a2bc54a24b4 Mon Sep 17 00:00:00 2001
From: Anca Zamfir <ancazamfir@users.noreply.github.com>
Date: Mon, 1 Aug 2022 11:17:00 +0200
Subject: [PATCH] Wait for destination app height to be higher than consensus
 proof's height (#2449)

* Wait for destination app height to be higher than consensus proof's
height.

* Formatting

* Add changelog entry

* Change interval to 500ms

* Remove duplicate sleep

* Move sleep into methods which actually send the messages, no point in waiting when building them

Co-authored-by: Romain Ruetschi <romain@informal.systems>
Co-authored-by: Adi Seredinschi <adi@informal.systems>
---
 .../ibc-relayer/2433-wait-consensus-height.md |  4 ++
 relayer/src/connection.rs                     | 54 ++++++++++++++++---
 2 files changed, 52 insertions(+), 6 deletions(-)
 create mode 100644 .changelog/unreleased/bug-fixes/ibc-relayer/2433-wait-consensus-height.md

diff --git a/.changelog/unreleased/bug-fixes/ibc-relayer/2433-wait-consensus-height.md b/.changelog/unreleased/bug-fixes/ibc-relayer/2433-wait-consensus-height.md
new file mode 100644
index 0000000000..338624c13d
--- /dev/null
+++ b/.changelog/unreleased/bug-fixes/ibc-relayer/2433-wait-consensus-height.md
@@ -0,0 +1,4 @@
+- For the `ConnOpenTry` and `ConnOpenAck` steps, wait for the destination
+  app height to be higher than the consensus height, otherwise we fail to
+  complete the handshake when the block times of the two chains involved differ
+  significantly ([#2433](https://github.com/informalsystems/ibc-rs/issues/2433))
\ No newline at end of file
diff --git a/relayer/src/connection.rs b/relayer/src/connection.rs
index 714a391e5b..e4c7948157 100644
--- a/relayer/src/connection.rs
+++ b/relayer/src/connection.rs
@@ -1,4 +1,5 @@
 use core::time::Duration;
+use std::thread;
 
 use ibc_proto::google::protobuf::Any;
 use serde::Serialize;
@@ -916,8 +917,35 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Connection<ChainA, ChainB> {
         }
     }
 
+    /// Wait for the application on destination chain to advance beyond `consensus_height`.
+    fn wait_for_dest_app_height_higher_than_consensus_proof_height(
+        &self,
+        consensus_height: Height,
+    ) -> Result<(), ConnectionError> {
+        let dst_application_latest_height = || {
+            self.dst_chain()
+                .query_latest_height()
+                .map_err(|e| ConnectionError::chain_query(self.src_chain().id(), e))
+        };
+
+        while consensus_height >= dst_application_latest_height()? {
+            warn!(
+                "client consensus proof height too high, \
+                 waiting for destination chain to advance beyond {}",
+                consensus_height
+            );
+
+            thread::sleep(Duration::from_millis(500));
+        }
+
+        Ok(())
+    }
+
     /// Attempts to build a MsgConnOpenTry.
-    pub fn build_conn_try(&self) -> Result<Vec<Any>, ConnectionError> {
+    ///
+    /// Return the messages and the app height the destination chain must reach
+    /// before we send the messages.
+    pub fn build_conn_try(&self) -> Result<(Vec<Any>, Height), ConnectionError> {
         let src_connection_id = self
             .src_connection_id()
             .ok_or_else(ConnectionError::missing_local_connection_id)?;
@@ -1024,11 +1052,16 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Connection<ChainA, ChainB> {
         };
 
         msgs.push(new_msg.to_any());
-        Ok(msgs)
+
+        Ok((msgs, src_client_target_height))
     }
 
     pub fn build_conn_try_and_send(&self) -> Result<IbcEvent, ConnectionError> {
-        let dst_msgs = self.build_conn_try()?;
+        let (dst_msgs, src_client_target_height) = self.build_conn_try()?;
+
+        // Wait for the height of the application on the destination chain to be higher than
+        // the height of the consensus state included in the proofs.
+        self.wait_for_dest_app_height_higher_than_consensus_proof_height(src_client_target_height)?;
 
         let tm = TrackedMsgs::new_static(dst_msgs, "ConnectionOpenTry");
 
@@ -1057,7 +1090,10 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Connection<ChainA, ChainB> {
     }
 
     /// Attempts to build a MsgConnOpenAck.
-    pub fn build_conn_ack(&self) -> Result<Vec<Any>, ConnectionError> {
+    ///
+    /// Return the messages and the app height the destination chain must reach
+    /// before we send the messages.
+    pub fn build_conn_ack(&self) -> Result<(Vec<Any>, Height), ConnectionError> {
         let src_connection_id = self
             .src_connection_id()
             .ok_or_else(ConnectionError::missing_local_connection_id)?;
@@ -1087,6 +1123,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Connection<ChainA, ChainB> {
             .dst_chain()
             .query_latest_height()
             .map_err(|e| ConnectionError::chain_query(self.dst_chain().id(), e))?;
+
         let client_msgs = self.build_update_client_on_src(src_client_target_height)?;
 
         let tm =
@@ -1130,11 +1167,16 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Connection<ChainA, ChainB> {
         };
 
         msgs.push(new_msg.to_any());
-        Ok(msgs)
+
+        Ok((msgs, src_client_target_height))
     }
 
     pub fn build_conn_ack_and_send(&self) -> Result<IbcEvent, ConnectionError> {
-        let dst_msgs = self.build_conn_ack()?;
+        let (dst_msgs, src_client_target_height) = self.build_conn_ack()?;
+
+        // Wait for the height of the application on the destination chain to be higher than
+        // the height of the consensus state included in the proofs.
+        self.wait_for_dest_app_height_higher_than_consensus_proof_height(src_client_target_height)?;
 
         let tm = TrackedMsgs::new_static(dst_msgs, "ConnectionOpenAck");