Implement ClientState::status() (cosmos#774)

* Status enum

* implement status API

* replace most confirm_not_frozen

* validate_self_client no longer uses confirm_not_frozen

* remove confirm_not_frozen

* clippy

* fix test

* remove expired()

* changelog

* Remove `Status::Unknown`

* don't consider the client expired if consensus state is in the future

* Status methods

* remove redundant check

* Remove error variant

* update changelog

.changelog/unreleased/breaking-changes/536-clientstate-status.md

@@ -0,0 +1,2 @@
+- [ibc-derive] Replace `ClientState::{confirm_not_frozen, expired}()` with `ClientState::status()`
+  ([#536](https://github.com/cosmos/ibc-rs/issues/536))

crates/ibc-derive/src/client_state/traits/client_state_common.rs

@@ -32,16 +32,6 @@ pub(crate) fn impl_ClientStateCommon(
         enum_variants.iter(),
         quote! {validate_proof_height(cs, proof_height)},
     );
-    let confirm_not_frozen_impl = delegate_call_in_match(
-        client_state_enum_name,
-        enum_variants.iter(),
-        quote! {confirm_not_frozen(cs)},
-    );
-    let expired_impl = delegate_call_in_match(
-        client_state_enum_name,
-        enum_variants.iter(),
-        quote! {expired(cs, elapsed)},
-    );
     let verify_upgrade_client_impl = delegate_call_in_match(
         client_state_enum_name,
         enum_variants.iter(),
@@ -95,18 +85,6 @@ pub(crate) fn impl_ClientStateCommon(
                 }
             }
 
-            fn confirm_not_frozen(&self) -> core::result::Result<(), #ClientError> {
-                match self {
-                    #(#confirm_not_frozen_impl),*
-                }
-            }
-
-            fn expired(&self, elapsed: core::time::Duration) -> bool {
-                match self {
-                    #(#expired_impl),*
-                }
-            }
-
             fn verify_upgrade_client(
                 &self,
                 upgraded_client_state: #Any,

crates/ibc-derive/src/client_state/traits/client_state_validation.rs

@@ -30,13 +30,21 @@ pub(crate) fn impl_ClientStateValidation(
         quote! { check_for_misbehaviour(cs, ctx, client_id, client_message, update_kind) },
     );
 
+    let status_impl = delegate_call_in_match(
+        client_state_enum_name,
+        enum_variants.iter(),
+        opts,
+        quote! { status(cs, ctx, client_id) },
+    );
+
     let HostClientState = client_state_enum_name;
     let ClientValidationContext = &opts.client_validation_context;
 
     let Any = Imports::Any();
     let ClientId = Imports::ClientId();
     let ClientError = Imports::ClientError();
     let ClientStateValidation = Imports::ClientStateValidation();
+    let Status = Imports::Status();
     let UpdateKind = Imports::UpdateKind();
 
     quote! {
@@ -64,6 +72,17 @@ pub(crate) fn impl_ClientStateValidation(
                     #(#check_for_misbehaviour_impl),*
                 }
             }
+
+            fn status(
+                &self,
+                ctx: &#ClientValidationContext,
+                client_id: &#ClientId,
+            ) -> core::result::Result<#Status, #ClientError> {
+                match self {
+                    #(#status_impl),*
+                }
+
+            }
         }
 
     }

crates/ibc-derive/src/utils.rs

@@ -68,6 +68,10 @@ impl Imports {
     pub fn UpdateKind() -> TokenStream {
         quote! {ibc::core::ics02_client::client_state::UpdateKind}
     }
+
+    pub fn Status() -> TokenStream {
+        quote! {ibc::core::ics02_client::client_state::Status}
+    }
 }
 
 /// Retrieves the field of a given enum variant. Outputs an error message if the enum variant

crates/ibc/Cargo.toml

@@ -75,7 +75,7 @@ scale-info = { version = "2.1.2", default-features = false, features = ["derive"
 borsh = {version = "0.9.0", default-features = false, optional = true }
 parking_lot = { version = "0.12.1", default-features = false, optional = true }
 
-ibc-derive = "0.2.0"
+ibc-derive = { version ="0.2.0", path = "../ibc-derive" }
 
 schemars = { version = "0.8.12", optional = true }
 

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

@@ -28,7 +28,7 @@ use crate::clients::ics07_tendermint::error::Error;
 use crate::clients::ics07_tendermint::header::Header as TmHeader;
 use crate::clients::ics07_tendermint::misbehaviour::Misbehaviour as TmMisbehaviour;
 use crate::core::ics02_client::client_state::{
-    ClientStateCommon, ClientStateExecution, ClientStateValidation, UpdateKind,
+    ClientStateCommon, ClientStateExecution, ClientStateValidation, Status, UpdateKind,
 };
 use crate::core::ics02_client::client_type::ClientType;
 use crate::core::ics02_client::consensus_state::ConsensusState;
@@ -248,6 +248,10 @@ impl ClientState {
         self.chain_id.clone()
     }
 
+    pub fn is_frozen(&self) -> bool {
+        self.frozen_height.is_some()
+    }
+
     // Resets custom fields to zero values (used in `update_client`)
     pub fn zero_custom_fields(&mut self) {
         self.trusting_period = ZERO_DURATION;
@@ -289,19 +293,6 @@ impl ClientStateCommon for ClientState {
         Ok(())
     }
 
-    fn confirm_not_frozen(&self) -> Result<(), ClientError> {
-        if let Some(frozen_height) = self.frozen_height {
-            return Err(ClientError::ClientFrozen {
-                description: format!("the client is frozen at height {frozen_height}"),
-            });
-        }
-        Ok(())
-    }
-
-    fn expired(&self, elapsed: Duration) -> bool {
-        elapsed > self.trusting_period
-    }
-
     /// Perform client-specific verifications and check all data in the new
     /// client state to be the same across all valid Tendermint clients for the
     /// new chain.
@@ -422,6 +413,9 @@ impl ClientStateCommon for ClientState {
 impl<ClientValidationContext> ClientStateValidation<ClientValidationContext> for ClientState
 where
     ClientValidationContext: TmValidationContext,
+    ClientValidationContext::AnyConsensusState: TryInto<TmConsensusState>,
+    ClientError:
+        From<<ClientValidationContext::AnyConsensusState as TryInto<TmConsensusState>>::Error>,
 {
     fn verify_client_message(
         &self,
@@ -460,6 +454,43 @@ where
             }
         }
     }
+
+    fn status(
+        &self,
+        ctx: &ClientValidationContext,
+        client_id: &ClientId,
+    ) -> Result<Status, ClientError> {
+        if self.is_frozen() {
+            return Ok(Status::Frozen);
+        }
+
+        let latest_consensus_state: TmConsensusState = {
+            let any_latest_consensus_state = match ctx.consensus_state(
+                &ClientConsensusStatePath::new(client_id, &self.latest_height),
+            ) {
+                Ok(cs) => cs,
+                // if the client state does not have an associated consensus state for its latest height
+                // then it must be expired
+                Err(_) => return Ok(Status::Expired),
+            };
+
+            any_latest_consensus_state.try_into()?
+        };
+
+        // Note: if the `duration_since()` is `None`, indicating that the latest
+        // consensus state is in the future, then we don't consider the client
+        // to be expired.
+        let now = ctx.host_timestamp()?;
+        if let Some(elapsed_since_latest_consensus_state) =
+            now.duration_since(&latest_consensus_state.timestamp())
+        {
+            if elapsed_since_latest_consensus_state > self.trusting_period {
+                return Ok(Status::Expired);
+            }
+        }
+
+        Ok(Status::Active)
+    }
 }
 
 impl<E> ClientStateExecution<E> for ClientState

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

@@ -1,8 +1,7 @@
 //! Defines `ClientState`, the core type to be implemented by light clients
 
-use core::fmt::Debug;
+use core::fmt::{Debug, Display, Formatter};
 use core::marker::{Send, Sync};
-use core::time::Duration;
 
 use ibc_proto::google::protobuf::Any;
 
@@ -30,6 +29,39 @@ pub enum UpdateKind {
     SubmitMisbehaviour,
 }
 
+/// Represents the status of a client
+#[derive(Debug, PartialEq, Eq)]
+pub enum Status {
+    /// The client is active and allowed to be used
+    Active,
+    /// The client is frozen and not allowed to be used
+    Frozen,
+    /// The client is expired and not allowed to be used
+    Expired,
+    /// Unauthorized indicates that the client type is not registered as an allowed client type.
+    Unauthorized,
+}
+
+impl Status {
+    pub fn is_active(&self) -> bool {
+        *self == Status::Active
+    }
+
+    pub fn is_frozen(&self) -> bool {
+        *self == Status::Frozen
+    }
+
+    pub fn is_expired(&self) -> bool {
+        *self == Status::Expired
+    }
+}
+
+impl Display for Status {
+    fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
+        write!(f, "{self:?}")
+    }
+}
+
 /// `ClientState` methods needed in both validation and execution.
 ///
 /// They do not require access to a client `ValidationContext` nor
@@ -50,13 +82,6 @@ pub trait ClientStateCommon {
     /// Validate that the client is at a sufficient height
     fn validate_proof_height(&self, proof_height: Height) -> Result<(), ClientError>;
 
-    /// Assert that the client is not frozen
-    fn confirm_not_frozen(&self) -> Result<(), ClientError>;
-
-    /// Check if the state is expired when `elapsed` time has passed since the latest consensus
-    /// state timestamp
-    fn expired(&self, elapsed: Duration) -> bool;
-
     /// Verify the upgraded client and consensus states and validate proofs
     /// against the given root.
     ///
@@ -140,6 +165,13 @@ pub trait ClientStateValidation<ClientValidationContext> {
         client_message: Any,
         update_kind: &UpdateKind,
     ) -> Result<bool, ClientError>;
+
+    /// Returns the status of the client. Only Active clients are allowed to process packets.
+    fn status(
+        &self,
+        ctx: &ClientValidationContext,
+        client_id: &ClientId,
+    ) -> Result<Status, ClientError>;
 }
 
 /// `ClientState` methods which require access to the client's

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

@@ -12,6 +12,8 @@ use crate::core::timestamp::Timestamp;
 use crate::core::ContextError;
 use crate::Height;
 
+use super::client_state::Status;
+
 /// Encodes all the possible client errors
 #[derive(Debug, Display)]
 pub enum ClientError {
@@ -25,6 +27,8 @@ pub enum ClientError {
     },
     /// client is frozen with description: `{description}`
     ClientFrozen { description: String },
+    /// client is not active. Status=`{status}`
+    ClientNotActive { status: Status },
     /// client state not found: `{client_id}`
     ClientStateNotFound { client_id: ClientId },
     /// client state already exists: `{client_id}`
@@ -91,11 +95,6 @@ pub enum ClientError {
     },
     /// timestamp is invalid or missing, timestamp=`{time1}`,  now=`{time2}`
     InvalidConsensusStateTimestamp { time1: Timestamp, time2: Timestamp },
-    /// header not within trusting period: expires_at=`{latest_time}` now=`{update_time}`
-    HeaderNotWithinTrustPeriod {
-        latest_time: Timestamp,
-        update_time: Timestamp,
-    },
     /// the local consensus state could not be retrieved for height `{height}`
     MissingLocalConsensusState { height: Height },
     /// invalid signer error: `{reason}`
@@ -121,6 +120,14 @@ impl From<ContextError> for ClientError {
     }
 }
 
+impl From<&'static str> for ClientError {
+    fn from(s: &'static str) -> Self {
+        Self::Other {
+            description: s.to_string(),
+        }
+    }
+}
+
 #[cfg(feature = "std")]
 impl std::error::Error for ClientError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {

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

@@ -28,7 +28,12 @@ where
     // Read client state from the host chain store. The client should already exist.
     let client_state = ctx.client_state(&client_id)?;
 
-    client_state.confirm_not_frozen()?;
+    {
+        let status = client_state.status(ctx.get_client_validation_context(), &client_id)?;
+        if !status.is_active() {
+            return Err(ClientError::ClientNotActive { status }.into());
+        }
+    }
 
     let client_message = msg.client_message();
 
@@ -245,7 +250,10 @@ mod tests {
         assert!(res.is_ok(), "result: {res:?}");
 
         let client_state = ctx.client_state(&msg.client_id).unwrap();
-        assert!(client_state.confirm_not_frozen().is_ok());
+        assert!(client_state
+            .status(&ctx, &msg.client_id)
+            .unwrap()
+            .is_active());
         assert_eq!(client_state.latest_height(), latest_header_height);
     }
 
@@ -292,7 +300,10 @@ mod tests {
         assert!(res.is_ok(), "result: {res:?}");
 
         let client_state = ctx.client_state(&msg.client_id).unwrap();
-        assert!(client_state.confirm_not_frozen().is_ok());
+        assert!(client_state
+            .status(&ctx, &msg.client_id)
+            .unwrap()
+            .is_active());
         assert_eq!(client_state.latest_height(), latest_header_height);
     }
 
@@ -413,7 +424,10 @@ mod tests {
         assert!(res.is_ok(), "result: {res:?}");
 
         let client_state = ctx_a.client_state(&msg.client_id).unwrap();
-        assert!(client_state.confirm_not_frozen().is_ok());
+        assert!(client_state
+            .status(&ctx_a, &msg.client_id)
+            .unwrap()
+            .is_active());
         assert_eq!(client_state.latest_height(), latest_header_height);
         assert_eq!(client_state, ctx_a.latest_client_states(&msg.client_id));
     }
@@ -496,7 +510,8 @@ mod tests {
     fn ensure_misbehaviour(ctx: &MockContext, client_id: &ClientId, client_type: &ClientType) {
         let client_state = ctx.client_state(client_id).unwrap();
 
-        assert!(client_state.confirm_not_frozen().is_err());
+        let status = client_state.status(ctx, client_id).unwrap();
+        assert!(status.is_frozen(), "client_state status: {status}");
 
         // check events
         assert_eq!(ctx.events.len(), 2);