Fix host_height, host_timestamp return value (#243)

* Change host_height, host_timestamp return Value

* fmt code

* Create 242-change-return-value.md

* fix clippy

* update .changelog

* Change host_height, host_timestamp return value in ValidationContext

* update Ics18context to RelayerContext in mock/context.rs

Signed-off-by: Davirain <[email protected]>

Author: DaviRain-Su

.changelog/unreleased/breaking-changes/242-change-return-value.md

@@ -0,0 +1,2 @@
+- Change `host_height`, `host_timestamp` return value to a `Result` in `ClientReader`, `ConnectionReader`, `ChannelReader` and `ValidationContext`
+  ([#242](https://github.com/cosmos/ibc-rs/issues/242))

crates/ibc/src/clients/ics07_tendermint/client_state.rs

@@ -403,7 +403,7 @@ impl Ics2ClientState for ClientState {
             untrusted_state,
             trusted_state,
             &options,
-            ctx.host_timestamp().into_tm_time().unwrap(),
+            ctx.host_timestamp()?.into_tm_time().unwrap(),
         );
 
         match verdict {
@@ -574,7 +574,7 @@ impl Ics2ClientState for ClientState {
             untrusted_state,
             trusted_state,
             &options,
-            ctx.host_timestamp().into_tm_time().unwrap(),
+            ctx.host_timestamp()?.into_tm_time().unwrap(),
         );
 
         match verdict {
@@ -915,8 +915,12 @@ fn verify_delay_passed(
     height: Height,
     connection_end: &ConnectionEnd,
 ) -> Result<(), Ics02Error> {
-    let current_timestamp = ctx.host_timestamp();
-    let current_height = ctx.host_height();
+    let current_timestamp = ctx
+        .host_timestamp()
+        .map_err(|e| Ics02Error::other(e.to_string()))?;
+    let current_height = ctx
+        .host_height()
+        .map_err(|e| Ics02Error::other(e.to_string()))?;
 
     let client_id = connection_end.client_id();
     let processed_time = ctx

crates/ibc/src/core/context.rs

@@ -90,14 +90,14 @@ pub trait ValidationContext {
     ) -> Result<Option<Box<dyn ConsensusState>>, ClientError>;
 
     /// Returns the current height of the local chain.
-    fn host_height(&self) -> Height;
+    fn host_height(&self) -> Result<Height, ClientError>;
 
     /// Returns the current timestamp of the local chain.
-    fn host_timestamp(&self) -> Timestamp {
+    fn host_timestamp(&self) -> Result<Timestamp, ClientError> {
         let pending_consensus_state = self
             .pending_host_consensus_state()
             .expect("host must have pending consensus state");
-        pending_consensus_state.timestamp()
+        Ok(pending_consensus_state.timestamp())
     }
 
     /// Returns the pending `ConsensusState` of the host (local) chain.

crates/ibc/src/core/ics02_client/context.rs

@@ -51,14 +51,14 @@ pub trait ClientReader {
     ) -> Result<Option<Box<dyn ConsensusState>>, Error>;
 
     /// Returns the current height of the local chain.
-    fn host_height(&self) -> Height;
+    fn host_height(&self) -> Result<Height, Error>;
 
     /// Returns the current timestamp of the local chain.
-    fn host_timestamp(&self) -> Timestamp {
+    fn host_timestamp(&self) -> Result<Timestamp, Error> {
         let pending_consensus_state = self
             .pending_host_consensus_state()
             .expect("host must have pending consensus state");
-        pending_consensus_state.timestamp()
+        Ok(pending_consensus_state.timestamp())
     }
 
     /// Returns the `ConsensusState` of the host (local) chain at a specific height.

crates/ibc/src/core/ics02_client/handler/create_client.rs

@@ -90,12 +90,12 @@ where
     ctx.store_update_time(
         client_id.clone(),
         client_state.latest_height(),
-        ctx.host_timestamp(),
+        ctx.host_timestamp()?,
     )?;
     ctx.store_update_height(
         client_id.clone(),
         client_state.latest_height(),
-        ctx.host_height(),
+        ctx.host_height()?,
     )?;
 
     ctx.emit_ibc_event(IbcEvent::CreateClient(CreateClient::new(
@@ -141,8 +141,8 @@ pub fn process(ctx: &dyn ClientReader, msg: MsgCreateClient) -> HandlerResult<Cl
         client_type: client_type.clone(),
         client_state,
         consensus_state,
-        processed_time: ctx.host_timestamp(),
-        processed_height: ctx.host_height(),
+        processed_time: ctx.host_timestamp()?,
+        processed_height: ctx.host_height()?,
     });
 
     output.emit(IbcEvent::CreateClient(CreateClient::new(

crates/ibc/src/core/ics02_client/handler/update_client.rs

@@ -56,7 +56,7 @@ where
 
     debug!("latest consensus state: {:?}", latest_consensus_state);
 
-    let now = ctx.host_timestamp();
+    let now = ctx.host_timestamp()?;
     let duration = now
         .duration_since(&latest_consensus_state.timestamp())
         .ok_or_else(|| {
@@ -109,12 +109,12 @@ where
     ctx.store_update_time(
         client_id.clone(),
         client_state.latest_height(),
-        ctx.host_timestamp(),
+        ctx.host_timestamp()?,
     )?;
     ctx.store_update_height(
         client_id.clone(),
         client_state.latest_height(),
-        ctx.host_height(),
+        ctx.host_height()?,
     )?;
 
     {
@@ -160,7 +160,7 @@ pub fn process<Ctx: ClientReader>(
 
     debug!("latest consensus state: {:?}", latest_consensus_state);
 
-    let now = ClientReader::host_timestamp(ctx);
+    let now = ClientReader::host_timestamp(ctx)?;
     let duration = now
         .duration_since(&latest_consensus_state.timestamp())
         .ok_or_else(|| {
@@ -191,8 +191,8 @@ pub fn process<Ctx: ClientReader>(
         client_id: client_id.clone(),
         client_state,
         consensus_state,
-        processed_time: ClientReader::host_timestamp(ctx),
-        processed_height: ctx.host_height(),
+        processed_time: ClientReader::host_timestamp(ctx)?,
+        processed_height: ctx.host_height()?,
     });
 
     output.emit(IbcEvent::UpdateClient(UpdateClient::new(

crates/ibc/src/core/ics03_connection/context.rs

@@ -28,11 +28,11 @@ pub trait ConnectionReader {
     fn decode_client_state(&self, client_state: Any) -> Result<Box<dyn ClientState>, Error>;
 
     /// Returns the current height of the local chain.
-    fn host_current_height(&self) -> Height;
+    fn host_current_height(&self) -> Result<Height, Error>;
 
     #[deprecated(since = "0.20.0")]
     /// Returns the oldest height available on the local chain.
-    fn host_oldest_height(&self) -> Height;
+    fn host_oldest_height(&self) -> Result<Height, Error>;
 
     /// Returns the prefix that the local chain uses in the KV store.
     fn commitment_prefix(&self) -> CommitmentPrefix;

crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs

@@ -19,10 +19,10 @@ pub(crate) fn process(
 ) -> HandlerResult<ConnectionResult, Error> {
     let mut output = HandlerOutput::builder();
 
-    if msg.consensus_height_of_a_on_b > ctx_a.host_current_height() {
+    if msg.consensus_height_of_a_on_b > ctx_a.host_current_height()? {
         return Err(Error::invalid_consensus_height(
             msg.consensus_height_of_a_on_b,
-            ctx_a.host_current_height(),
+            ctx_a.host_current_height()?,
         ));
     }
 

crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs

@@ -24,11 +24,11 @@ pub(crate) fn process(
 
     ctx_b.validate_self_client(msg.client_state_of_b_on_a.clone())?;
 
-    if msg.consensus_height_of_b_on_a > ctx_b.host_current_height() {
+    if msg.consensus_height_of_b_on_a > ctx_b.host_current_height()? {
         // Fail if the consensus height is too advanced.
         return Err(Error::invalid_consensus_height(
             msg.consensus_height_of_b_on_a,
-            ctx_b.host_current_height(),
+            ctx_b.host_current_height()?,
         ));
     }
 

crates/ibc/src/core/ics04_channel/context.rs

@@ -113,14 +113,14 @@ pub trait ChannelReader {
     fn hash(&self, value: Vec<u8>) -> Vec<u8>;
 
     /// Returns the current height of the local chain.
-    fn host_height(&self) -> Height;
+    fn host_height(&self) -> Result<Height, Error>;
 
     /// Returns the current timestamp of the local chain.
-    fn host_timestamp(&self) -> Timestamp {
+    fn host_timestamp(&self) -> Result<Timestamp, Error> {
         let pending_consensus_state = self
             .pending_host_consensus_state()
             .expect("host must have pending consensus state");
-        pending_consensus_state.timestamp()
+        Ok(pending_consensus_state.timestamp())
     }
 
     /// Returns the `ConsensusState` of the host (local) chain at a specific height.

crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs

@@ -133,7 +133,7 @@ mod tests {
         let client_id = ClientId::new(mock_client_type(), 24).unwrap();
         let conn_id = ConnectionId::new(2);
         let default_context = MockContext::default();
-        let client_consensus_state_height = default_context.host_height();
+        let client_consensus_state_height = default_context.host_height().unwrap();
 
         let conn_end = ConnectionEnd::new(
             ConnectionState::Open,

crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs

@@ -111,7 +111,7 @@ mod tests {
 
         let context = {
             let default_context = MockContext::default();
-            let client_consensus_state_height = default_context.host_height();
+            let client_consensus_state_height = default_context.host_height().unwrap();
 
             default_context
                 .with_client(&client_id, client_consensus_state_height)

crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs

@@ -140,7 +140,8 @@ mod tests {
         let client_id = ClientId::new(mock_client_type(), 24).unwrap();
         let conn_id = ConnectionId::new(2);
         let context = MockContext::default();
-        let client_consensus_state_height = context.host_current_height().revision_height();
+        let client_consensus_state_height =
+            context.host_current_height().unwrap().revision_height();
 
         // The connection underlying the channel we're trying to open.
         let conn_end = ConnectionEnd::new(

crates/ibc/src/core/ics04_channel/handler/recv_packet.rs

@@ -66,15 +66,15 @@ pub fn process<Ctx: ChannelReader>(
         ));
     }
 
-    let latest_height = ChannelReader::host_height(ctx);
+    let latest_height = ChannelReader::host_height(ctx)?;
     if packet.timeout_height.has_expired(latest_height) {
         return Err(Error::low_packet_height(
             latest_height,
             packet.timeout_height,
         ));
     }
 
-    let latest_timestamp = ChannelReader::host_timestamp(ctx);
+    let latest_timestamp = ChannelReader::host_timestamp(ctx)?;
     if let Expiry::Expired = latest_timestamp.check_expiry(&packet.timeout_timestamp) {
         return Err(Error::low_packet_timestamp());
     }
@@ -186,7 +186,7 @@ mod tests {
 
         let context = MockContext::default();
 
-        let host_height = context.query_latest_height().increment();
+        let host_height = context.query_latest_height().unwrap().increment();
 
         let client_height = host_height.increment();
 

crates/ibc/src/mock/context.rs

@@ -845,12 +845,12 @@ impl ChannelReader for MockContext {
         sha2::Sha256::digest(value).to_vec()
     }
 
-    fn host_height(&self) -> Height {
-        self.latest_height()
+    fn host_height(&self) -> Result<Height, Ics04Error> {
+        Ok(self.latest_height())
     }
 
-    fn host_timestamp(&self) -> Timestamp {
-        ClientReader::host_timestamp(self)
+    fn host_timestamp(&self) -> Result<Timestamp, Ics04Error> {
+        ClientReader::host_timestamp(self).map_err(|e| Ics04Error::other(e.to_string()))
     }
 
     fn host_consensus_state(&self, height: Height) -> Result<Box<dyn ConsensusState>, Ics04Error> {
@@ -1103,13 +1103,13 @@ impl ConnectionReader for MockContext {
         ClientReader::decode_client_state(self, client_state).map_err(Ics03Error::ics02_client)
     }
 
-    fn host_current_height(&self) -> Height {
-        self.latest_height()
+    fn host_current_height(&self) -> Result<Height, Ics03Error> {
+        Ok(self.latest_height())
     }
 
-    fn host_oldest_height(&self) -> Height {
+    fn host_oldest_height(&self) -> Result<Height, Ics03Error> {
         // history must be non-empty, so `self.history[0]` is valid
-        self.history[0].height()
+        Ok(self.history[0].height())
     }
 
     fn commitment_prefix(&self) -> CommitmentPrefix {
@@ -1275,17 +1275,18 @@ impl ClientReader for MockContext {
         Ok(None)
     }
 
-    fn host_height(&self) -> Height {
-        self.latest_height()
+    fn host_height(&self) -> Result<Height, Ics02Error> {
+        Ok(self.latest_height())
     }
 
-    fn host_timestamp(&self) -> Timestamp {
-        self.history
+    fn host_timestamp(&self) -> Result<Timestamp, Ics02Error> {
+        Ok(self
+            .history
             .last()
             .expect("history cannot be empty")
             .timestamp()
             .add(self.block_time)
-            .unwrap()
+            .unwrap())
     }
 
     fn host_consensus_state(&self, height: Height) -> Result<Box<dyn ConsensusState>, Ics02Error> {
@@ -1401,8 +1402,8 @@ impl ClientKeeper for MockContext {
 }
 
 impl RelayerContext for MockContext {
-    fn query_latest_height(&self) -> Height {
-        self.host_current_height()
+    fn query_latest_height(&self) -> Result<Height, Ics18Error> {
+        self.host_current_height().map_err(Ics18Error::ics03)
     }
 
     fn query_client_full_state(&self, client_id: &ClientId) -> Option<Box<dyn ClientState>> {
@@ -1411,7 +1412,7 @@ impl RelayerContext for MockContext {
     }
 
     fn query_latest_header(&self) -> Option<Box<dyn Header>> {
-        let block_ref = self.host_block(self.host_current_height());
+        let block_ref = self.host_block(self.host_current_height().unwrap());
         block_ref.cloned().map(Header::into_box)
     }
 

crates/ibc/src/relayer/ics18_relayer/context.rs

@@ -17,7 +17,7 @@ use crate::Height;
 /// types, light client, RPC client, etc.)
 pub trait RelayerContext {
     /// Returns the latest height of the chain.
-    fn query_latest_height(&self) -> Height;
+    fn query_latest_height(&self) -> Result<Height, Error>;
 
     /// Returns this client state for the given `client_id` on this chain.
     /// Wrapper over the `/abci_query?path=..` endpoint.

crates/ibc/src/relayer/ics18_relayer/error.rs

@@ -1,3 +1,4 @@
+use crate::core::ics03_connection;
 use crate::core::ics24_host::identifier::ClientId;
 use crate::core::ics26_routing::error::Error as RoutingError;
 use crate::Height;
@@ -34,5 +35,9 @@ define_error! {
         TransactionFailed
             [ RoutingError ]
             | _ | { "transaction processing by modules failed" },
+
+        Ics03
+            [ ics03_connection::error::Error ]
+            | _ | { "ics03 connection error" }
     }
 }

crates/ibc/src/relayer/ics18_relayer/utils.rs

@@ -149,7 +149,7 @@ mod tests {
                 .query_client_full_state(&client_on_b_for_a)
                 .unwrap()
                 .latest_height();
-            assert_eq!(client_height_b, ctx_a.query_latest_height());
+            assert_eq!(client_height_b, ctx_a.query_latest_height().unwrap());
 
             // Update client on chain B to latest height of B.
             // - create the client update message with the latest header from B
@@ -206,7 +206,7 @@ mod tests {
                 .query_client_full_state(&client_on_a_for_b)
                 .unwrap()
                 .latest_height();
-            assert_eq!(client_height_a, ctx_b.query_latest_height());
+            assert_eq!(client_height_a, ctx_b.query_latest_height().unwrap());
         }
     }
 }