feat: introduce convenience Status::verify_is_active method (#1005)

* feat: introduce convenience Status::verify_active method Similarly to ChannelEnd::verify_not_closed, introduce a convenience method Status::verify_active which checks whether client’s status is active and returns an error if it isn’t. This simplifies whole bunch of code locations making them more DRY. * Update .changelog/unreleased/improvements/1005-status-verify-active.md Co-authored-by: Farhad Shabani <[email protected]> Signed-off-by: Michal Nazarewicz <[email protected]> * review --------- Signed-off-by: Michal Nazarewicz <[email protected]> Co-authored-by: Farhad Shabani <[email protected]>
cosmos · Dec 13, 2023 · 9a81b2b · 9a81b2b
1 parent 4b5004c
commit 9a81b2b
Show file tree

Hide file tree

Showing 19 changed files with 67 additions and 132 deletions.
diff --git a/.changelog/unreleased/improvements/1005-status-verify-active.md b/.changelog/unreleased/improvements/1005-status-verify-active.md
@@ -0,0 +1,2 @@
+- `[ibc-core-client-types]` Add a convenient `Status::verify_is_active` method.
+  ([#1005](https://github.com/cosmos/ibc-rs/pull/1005))
diff --git a/ibc-core/ics02-client/src/handler/update_client.rs b/ibc-core/ics02-client/src/handler/update_client.rs
@@ -28,12 +28,9 @@ where
     // Read client state from the host chain store. The client should already exist.
     let client_state = ctx.client_state(&client_id)?;
 
-    {
-        let status = client_state.status(ctx.get_client_validation_context(), &client_id)?;
-        if !status.is_active() {
-            return Err(ClientError::ClientNotActive { status }.into());
-        }
-    }
+    client_state
+        .status(ctx.get_client_validation_context(), &client_id)?
+        .verify_is_active()?;
 
     let client_message = msg.client_message();
 

diff --git a/ibc-core/ics02-client/src/handler/upgrade_client.rs b/ibc-core/ics02-client/src/handler/upgrade_client.rs
@@ -27,12 +27,9 @@ where
     let old_client_state = ctx.client_state(&client_id)?;
 
     // Check if the client is active.
-    {
-        let status = old_client_state.status(ctx.get_client_validation_context(), &client_id)?;
-        if !status.is_active() {
-            return Err(ClientError::ClientNotActive { status }.into());
-        }
-    }
+    old_client_state
+        .status(ctx.get_client_validation_context(), &client_id)?
+        .verify_is_active()?;
 
     // Read the latest consensus state from the host chain store.
     let old_client_cons_state_path = ClientConsensusStatePath::new(

diff --git a/ibc-core/ics02-client/types/src/status.rs b/ibc-core/ics02-client/types/src/status.rs
@@ -1,5 +1,7 @@
 use core::fmt::{Debug, Display, Formatter};
 
+use crate::error::ClientError;
+
 /// `UpdateKind` represents the 2 ways that a client can be updated
 /// in IBC: either through a `MsgUpdateClient`, or a `MsgSubmitMisbehaviour`.
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -38,6 +40,14 @@ impl Status {
     pub fn is_expired(&self) -> bool {
         *self == Status::Expired
     }
+
+    /// Checks whether the status is active; returns `Err` if not.
+    pub fn verify_is_active(self) -> Result<(), ClientError> {
+        match self {
+            Self::Active => Ok(()),
+            status => Err(ClientError::ClientNotActive { status }),
+        }
+    }
 }
 
 impl Display for Status {

diff --git a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs
@@ -2,7 +2,6 @@
 
 use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation};
 use ibc_core_client::context::consensus_state::ConsensusState;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_connection_types::error::ConnectionError;
 use ibc_core_connection_types::events::OpenAck;
 use ibc_core_connection_types::msgs::MsgConnectionOpenAck;
@@ -56,13 +55,9 @@ where
     {
         let client_state_of_b_on_a = ctx_a.client_state(vars.client_id_on_a())?;
 
-        {
-            let status = client_state_of_b_on_a
-                .status(ctx_a.get_client_validation_context(), vars.client_id_on_a())?;
-            if !status.is_active() {
-                return Err(ClientError::ClientNotActive { status }.into());
-            }
-        }
+        client_state_of_b_on_a
+            .status(ctx_a.get_client_validation_context(), vars.client_id_on_a())?
+            .verify_is_active()?;
         client_state_of_b_on_a.validate_proof_height(msg.proofs_height_on_b)?;
 
         let client_cons_state_path_on_a = ClientConsensusStatePath::new(

diff --git a/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs b/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs
@@ -2,7 +2,6 @@
 
 use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation};
 use ibc_core_client::context::consensus_state::ConsensusState;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_connection_types::error::ConnectionError;
 use ibc_core_connection_types::events::OpenConfirm;
 use ibc_core_connection_types::msgs::MsgConnectionOpenConfirm;
@@ -45,13 +44,9 @@ where
     {
         let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?;
 
-        {
-            let status = client_state_of_a_on_b
-                .status(ctx_b.get_client_validation_context(), client_id_on_b)?;
-            if !status.is_active() {
-                return Err(ClientError::ClientNotActive { status }.into());
-            }
-        }
+        client_state_of_a_on_b
+            .status(ctx_b.get_client_validation_context(), client_id_on_b)?
+            .verify_is_active()?;
         client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?;
 
         let client_cons_state_path_on_b = ClientConsensusStatePath::new(

diff --git a/ibc-core/ics03-connection/src/handler/conn_open_init.rs b/ibc-core/ics03-connection/src/handler/conn_open_init.rs
@@ -1,6 +1,5 @@
 //! Protocol logic specific to ICS3 messages of type `MsgConnectionOpenInit`.
 use ibc_core_client::context::client_state::ClientStateValidation;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_connection_types::events::OpenInit;
 use ibc_core_connection_types::msgs::MsgConnectionOpenInit;
 use ibc_core_connection_types::{ConnectionEnd, Counterparty, State};
@@ -20,13 +19,9 @@ where
     // An IBC client running on the local (host) chain should exist.
     let client_state_of_b_on_a = ctx_a.client_state(&msg.client_id_on_a)?;
 
-    {
-        let status = client_state_of_b_on_a
-            .status(ctx_a.get_client_validation_context(), &msg.client_id_on_a)?;
-        if !status.is_active() {
-            return Err(ClientError::ClientNotActive { status }.into());
-        }
-    }
+    client_state_of_b_on_a
+        .status(ctx_a.get_client_validation_context(), &msg.client_id_on_a)?
+        .verify_is_active()?;
 
     if let Some(version) = msg.version {
         version.verify_is_supported(&ctx_a.get_compatible_versions())?;

diff --git a/ibc-core/ics03-connection/src/handler/conn_open_try.rs b/ibc-core/ics03-connection/src/handler/conn_open_try.rs
@@ -1,7 +1,6 @@
 //! Protocol logic specific to processing ICS3 messages of type `MsgConnectionOpenTry`.;
 use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation};
 use ibc_core_client::context::consensus_state::ConsensusState;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_connection_types::error::ConnectionError;
 use ibc_core_connection_types::events::OpenTry;
 use ibc_core_connection_types::msgs::MsgConnectionOpenTry;
@@ -55,13 +54,9 @@ where
     {
         let client_state_of_a_on_b = ctx_b.client_state(vars.conn_end_on_b.client_id())?;
 
-        {
-            let status = client_state_of_a_on_b
-                .status(ctx_b.get_client_validation_context(), &msg.client_id_on_b)?;
-            if !status.is_active() {
-                return Err(ClientError::ClientNotActive { status }.into());
-            }
-        }
+        client_state_of_a_on_b
+            .status(ctx_b.get_client_validation_context(), &msg.client_id_on_b)?
+            .verify_is_active()?;
         client_state_of_a_on_b.validate_proof_height(msg.proofs_height_on_a)?;
 
         let client_cons_state_path_on_b = ClientConsensusStatePath::new(

diff --git a/ibc-core/ics04-channel/src/handler/acknowledgement.rs b/ibc-core/ics04-channel/src/handler/acknowledgement.rs
@@ -5,7 +5,6 @@ use ibc_core_channel_types::events::AcknowledgePacket;
 use ibc_core_channel_types::msgs::MsgAcknowledgement;
 use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation};
 use ibc_core_client::context::consensus_state::ConsensusState;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_connection::delay::verify_conn_delay_passed;
 use ibc_core_connection::types::State as ConnectionState;
 use ibc_core_handler_types::error::ContextError;
@@ -178,13 +177,9 @@ where
         let client_id_on_a = conn_end_on_a.client_id();
         let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?;
 
-        {
-            let status = client_state_of_b_on_a
-                .status(ctx_a.get_client_validation_context(), client_id_on_a)?;
-            if !status.is_active() {
-                return Err(ClientError::ClientNotActive { status }.into());
-            }
-        }
+        client_state_of_b_on_a
+            .status(ctx_a.get_client_validation_context(), client_id_on_a)?
+            .verify_is_active()?;
         client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?;
 
         let client_cons_state_path_on_a = ClientConsensusStatePath::new(

diff --git a/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs b/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs
@@ -6,7 +6,6 @@ use ibc_core_channel_types::events::CloseConfirm;
 use ibc_core_channel_types::msgs::MsgChannelCloseConfirm;
 use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation};
 use ibc_core_client::context::consensus_state::ConsensusState;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_connection::types::State as ConnectionState;
 use ibc_core_handler_types::error::ContextError;
 use ibc_core_handler_types::events::{IbcEvent, MessageEvent};
@@ -115,13 +114,9 @@ where
         let client_id_on_b = conn_end_on_b.client_id();
         let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?;
 
-        {
-            let status = client_state_of_a_on_b
-                .status(ctx_b.get_client_validation_context(), client_id_on_b)?;
-            if !status.is_active() {
-                return Err(ClientError::ClientNotActive { status }.into());
-            }
-        }
+        client_state_of_a_on_b
+            .status(ctx_b.get_client_validation_context(), client_id_on_b)?
+            .verify_is_active()?;
         client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?;
 
         let client_cons_state_path_on_b = ClientConsensusStatePath::new(

diff --git a/ibc-core/ics04-channel/src/handler/chan_close_init.rs b/ibc-core/ics04-channel/src/handler/chan_close_init.rs
@@ -4,7 +4,6 @@ use ibc_core_channel_types::error::ChannelError;
 use ibc_core_channel_types::events::CloseInit;
 use ibc_core_channel_types::msgs::MsgChannelCloseInit;
 use ibc_core_client::context::client_state::ClientStateValidation;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_connection::types::State as ConnectionState;
 use ibc_core_handler_types::error::ContextError;
 use ibc_core_handler_types::events::{IbcEvent, MessageEvent};
@@ -112,13 +111,9 @@ where
 
     let client_id_on_a = conn_end_on_a.client_id();
     let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?;
-    {
-        let status =
-            client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a)?;
-        if !status.is_active() {
-            return Err(ClientError::ClientNotActive { status }.into());
-        }
-    }
+    client_state_of_b_on_a
+        .status(ctx_a.get_client_validation_context(), client_id_on_a)?
+        .verify_is_active()?;
 
     Ok(())
 }
diff --git a/ibc-core/ics04-channel/src/handler/chan_open_ack.rs b/ibc-core/ics04-channel/src/handler/chan_open_ack.rs
@@ -5,7 +5,6 @@ use ibc_core_channel_types::events::OpenAck;
 use ibc_core_channel_types::msgs::MsgChannelOpenAck;
 use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation};
 use ibc_core_client::context::consensus_state::ConsensusState;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_connection::types::State as ConnectionState;
 use ibc_core_handler_types::error::ContextError;
 use ibc_core_handler_types::events::{IbcEvent, MessageEvent};
@@ -112,13 +111,9 @@ where
         let client_id_on_a = conn_end_on_a.client_id();
         let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?;
 
-        {
-            let status = client_state_of_b_on_a
-                .status(ctx_a.get_client_validation_context(), client_id_on_a)?;
-            if !status.is_active() {
-                return Err(ClientError::ClientNotActive { status }.into());
-            }
-        }
+        client_state_of_b_on_a
+            .status(ctx_a.get_client_validation_context(), client_id_on_a)?
+            .verify_is_active()?;
         client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?;
 
         let client_cons_state_path_on_a = ClientConsensusStatePath::new(

diff --git a/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs b/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs
@@ -6,7 +6,6 @@ use ibc_core_channel_types::events::OpenConfirm;
 use ibc_core_channel_types::msgs::MsgChannelOpenConfirm;
 use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation};
 use ibc_core_client::context::consensus_state::ConsensusState;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_connection::types::State as ConnectionState;
 use ibc_core_handler_types::error::ContextError;
 use ibc_core_handler_types::events::{IbcEvent, MessageEvent};
@@ -117,13 +116,9 @@ where
         let client_id_on_b = conn_end_on_b.client_id();
         let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?;
 
-        {
-            let status = client_state_of_a_on_b
-                .status(ctx_b.get_client_validation_context(), client_id_on_b)?;
-            if !status.is_active() {
-                return Err(ClientError::ClientNotActive { status }.into());
-            }
-        }
+        client_state_of_a_on_b
+            .status(ctx_b.get_client_validation_context(), client_id_on_b)?
+            .verify_is_active()?;
         client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?;
 
         let client_cons_state_path_on_b = ClientConsensusStatePath::new(

diff --git a/ibc-core/ics04-channel/src/handler/chan_open_init.rs b/ibc-core/ics04-channel/src/handler/chan_open_init.rs
@@ -4,7 +4,6 @@ use ibc_core_channel_types::channel::{ChannelEnd, Counterparty, State};
 use ibc_core_channel_types::events::OpenInit;
 use ibc_core_channel_types::msgs::MsgChannelOpenInit;
 use ibc_core_client::context::client_state::ClientStateValidation;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_handler_types::error::ContextError;
 use ibc_core_handler_types::events::{IbcEvent, MessageEvent};
 use ibc_core_host::types::identifiers::ChannelId;
@@ -123,13 +122,9 @@ where
     let client_id_on_a = conn_end_on_a.client_id();
     let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?;
 
-    {
-        let status =
-            client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a)?;
-        if !status.is_active() {
-            return Err(ClientError::ClientNotActive { status }.into());
-        }
-    }
+    client_state_of_b_on_a
+        .status(ctx_a.get_client_validation_context(), client_id_on_a)?
+        .verify_is_active()?;
 
     let conn_version = conn_end_on_a.versions();
 

diff --git a/ibc-core/ics04-channel/src/handler/chan_open_try.rs b/ibc-core/ics04-channel/src/handler/chan_open_try.rs
@@ -6,7 +6,6 @@ use ibc_core_channel_types::events::OpenTry;
 use ibc_core_channel_types::msgs::MsgChannelOpenTry;
 use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation};
 use ibc_core_client::context::consensus_state::ConsensusState;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_connection::types::State as ConnectionState;
 use ibc_core_handler_types::error::ContextError;
 use ibc_core_handler_types::events::{IbcEvent, MessageEvent};
@@ -138,13 +137,10 @@ where
         let client_id_on_b = conn_end_on_b.client_id();
         let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?;
 
-        {
-            let status = client_state_of_a_on_b
-                .status(ctx_b.get_client_validation_context(), client_id_on_b)?;
-            if !status.is_active() {
-                return Err(ClientError::ClientNotActive { status }.into());
-            }
-        }
+        client_state_of_a_on_b
+            .status(ctx_b.get_client_validation_context(), client_id_on_b)?
+            .verify_is_active()?;
+
         client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?;
 
         let client_cons_state_path_on_b = ClientConsensusStatePath::new(

diff --git a/ibc-core/ics04-channel/src/handler/recv_packet.rs b/ibc-core/ics04-channel/src/handler/recv_packet.rs
@@ -6,7 +6,6 @@ use ibc_core_channel_types::msgs::MsgRecvPacket;
 use ibc_core_channel_types::packet::Receipt;
 use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation};
 use ibc_core_client::context::consensus_state::ConsensusState;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_connection::delay::verify_conn_delay_passed;
 use ibc_core_connection::types::State as ConnectionState;
 use ibc_core_handler_types::error::ContextError;
@@ -182,13 +181,10 @@ where
         let client_id_on_b = conn_end_on_b.client_id();
         let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?;
 
-        {
-            let status = client_state_of_a_on_b
-                .status(ctx_b.get_client_validation_context(), client_id_on_b)?;
-            if !status.is_active() {
-                return Err(ClientError::ClientNotActive { status }.into());
-            }
-        }
+        client_state_of_a_on_b
+            .status(ctx_b.get_client_validation_context(), client_id_on_b)?
+            .verify_is_active()?;
+
         client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?;
 
         let client_cons_state_path_on_b = ClientConsensusStatePath::new(

diff --git a/ibc-core/ics04-channel/src/handler/send_packet.rs b/ibc-core/ics04-channel/src/handler/send_packet.rs
@@ -5,7 +5,6 @@ use ibc_core_channel_types::events::SendPacket;
 use ibc_core_channel_types::packet::Packet;
 use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation};
 use ibc_core_client::context::consensus_state::ConsensusState;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_handler_types::error::ContextError;
 use ibc_core_handler_types::events::{IbcEvent, MessageEvent};
 use ibc_core_host::types::path::{
@@ -54,13 +53,9 @@ pub fn send_packet_validate(
 
     let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?;
 
-    {
-        let status =
-            client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a)?;
-        if !status.is_active() {
-            return Err(ClientError::ClientNotActive { status }.into());
-        }
-    }
+    client_state_of_b_on_a
+        .status(ctx_a.get_client_validation_context(), client_id_on_a)?
+        .verify_is_active()?;
 
     let latest_height_on_a = client_state_of_b_on_a.latest_height();
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		- `[ibc-core-client-types]` Add a convenient `Status::verify_is_active` method.
		([#1005](https://github.com/cosmos/ibc-rs/pull/1005))