From 99020c2a8578858c15da5c7fd27e1d4a567c8a63 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Tue, 12 Dec 2023 20:51:42 +0100 Subject: [PATCH 1/3] feat: introduce convenience Status::verify_active method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../improvements/1005-status-verify-active.md | 2 ++ ibc-core/ics02-client/src/handler/update_client.rs | 9 +++------ ibc-core/ics02-client/src/handler/upgrade_client.rs | 9 +++------ ibc-core/ics02-client/types/src/status.rs | 12 +++++++++++- .../ics03-connection/src/handler/conn_open_ack.rs | 11 +++-------- .../src/handler/conn_open_confirm.rs | 11 +++-------- .../ics03-connection/src/handler/conn_open_init.rs | 11 +++-------- .../ics03-connection/src/handler/conn_open_try.rs | 11 +++-------- .../ics04-channel/src/handler/acknowledgement.rs | 11 +++-------- .../ics04-channel/src/handler/chan_close_confirm.rs | 11 +++-------- .../ics04-channel/src/handler/chan_close_init.rs | 11 +++-------- ibc-core/ics04-channel/src/handler/chan_open_ack.rs | 11 +++-------- .../ics04-channel/src/handler/chan_open_confirm.rs | 11 +++-------- ibc-core/ics04-channel/src/handler/chan_open_init.rs | 11 +++-------- ibc-core/ics04-channel/src/handler/chan_open_try.rs | 12 ++++-------- ibc-core/ics04-channel/src/handler/recv_packet.rs | 12 ++++-------- ibc-core/ics04-channel/src/handler/send_packet.rs | 11 +++-------- ibc-core/ics04-channel/src/handler/timeout.rs | 12 ++++-------- .../ics04-channel/src/handler/timeout_on_close.rs | 12 ++++-------- 19 files changed, 68 insertions(+), 133 deletions(-) create mode 100644 .changelog/unreleased/improvements/1005-status-verify-active.md diff --git a/.changelog/unreleased/improvements/1005-status-verify-active.md b/.changelog/unreleased/improvements/1005-status-verify-active.md new file mode 100644 index 000000000..8ab631976 --- /dev/null +++ b/.changelog/unreleased/improvements/1005-status-verify-active.md @@ -0,0 +1,2 @@ +- Add ibc::core::client::types::Status::verify_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 index a9d6862d5..98f3a57aa 100644 --- 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_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 index f15c4ce99..1c84d7586 100644 --- 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_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 index b15ed61e7..b76e00a5b 100644 --- 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)] @@ -14,7 +16,7 @@ pub enum UpdateKind { } /// Represents the status of a client -#[derive(Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum Status { /// The client is active and allowed to be used Active, @@ -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_active(&self) -> Result<(), ClientError> { + match self.clone() { + 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 index ca5fdb5fa..33e8c3bd1 100644 --- 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_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 index a6a53f6e9..511801978 100644 --- 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_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 index b7deac746..55ffbb70e 100644 --- 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_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 index 61bd339a2..94d346fd0 100644 --- 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_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 index 2ae3a39e6..caffe0c19 100644 --- 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_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 index 30024afd3..41e40f41a 100644 --- 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_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 index a7f03163e..57a674d21 100644 --- 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_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 index c40fb2561..6077adc6d 100644 --- 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_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 index 5ef8bd769..b64eb908e 100644 --- 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_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 index 31f74ce0c..296ed3cf1 100644 --- 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_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 index cbc9ec590..5eeb595a3 100644 --- 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_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 index cc59d9100..5de5f9130 100644 --- 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_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 index da97c51c2..7cecd8afe 100644 --- 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_active()?; let latest_height_on_a = client_state_of_b_on_a.latest_height(); diff --git a/ibc-core/ics04-channel/src/handler/timeout.rs b/ibc-core/ics04-channel/src/handler/timeout.rs index 0cfed7c03..a1d7ec061 100644 --- a/ibc-core/ics04-channel/src/handler/timeout.rs +++ b/ibc-core/ics04-channel/src/handler/timeout.rs @@ -5,7 +5,6 @@ use ibc_core_channel_types::events::{ChannelClosed, TimeoutPacket}; use ibc_core_channel_types::msgs::{MsgTimeout, MsgTimeoutOnClose}; 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_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; @@ -189,13 +188,10 @@ 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_active()?; + client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?; // check that timeout height or timeout timestamp has passed on the other end diff --git a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs index c08c0e8d3..8765d1614 100644 --- a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs +++ b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs @@ -4,7 +4,6 @@ use ibc_core_channel_types::error::{ChannelError, PacketError}; use ibc_core_channel_types::msgs::MsgTimeoutOnClose; 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_handler_types::error::ContextError; use ibc_core_host::types::path::{ @@ -68,13 +67,10 @@ 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_active()?; + client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?; let client_cons_state_path_on_a = ClientConsensusStatePath::new( From ad7f68b3f8fb43db26b20b3f32b49217d6bd3f5c Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Wed, 13 Dec 2023 01:25:41 +0100 Subject: [PATCH 2/3] Update .changelog/unreleased/improvements/1005-status-verify-active.md Co-authored-by: Farhad Shabani Signed-off-by: Michal Nazarewicz --- .changelog/unreleased/improvements/1005-status-verify-active.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/unreleased/improvements/1005-status-verify-active.md b/.changelog/unreleased/improvements/1005-status-verify-active.md index 8ab631976..8aef96bb5 100644 --- a/.changelog/unreleased/improvements/1005-status-verify-active.md +++ b/.changelog/unreleased/improvements/1005-status-verify-active.md @@ -1,2 +1,2 @@ -- Add ibc::core::client::types::Status::verify_active method. +- `[ibc-core-client-types]` Add a convenient `Status::verify_is_active` method. ([#1005](https://github.com/cosmos/ibc-rs/pull/1005)) From b1f7a1eae102973433ee260290d779e13a320b49 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Wed, 13 Dec 2023 01:28:04 +0100 Subject: [PATCH 3/3] review --- ibc-core/ics02-client/src/handler/update_client.rs | 2 +- ibc-core/ics02-client/src/handler/upgrade_client.rs | 2 +- ibc-core/ics02-client/types/src/status.rs | 6 +++--- ibc-core/ics03-connection/src/handler/conn_open_ack.rs | 2 +- ibc-core/ics03-connection/src/handler/conn_open_confirm.rs | 2 +- ibc-core/ics03-connection/src/handler/conn_open_init.rs | 2 +- ibc-core/ics03-connection/src/handler/conn_open_try.rs | 2 +- ibc-core/ics04-channel/src/handler/acknowledgement.rs | 2 +- ibc-core/ics04-channel/src/handler/chan_close_confirm.rs | 2 +- ibc-core/ics04-channel/src/handler/chan_close_init.rs | 2 +- ibc-core/ics04-channel/src/handler/chan_open_ack.rs | 2 +- ibc-core/ics04-channel/src/handler/chan_open_confirm.rs | 2 +- ibc-core/ics04-channel/src/handler/chan_open_init.rs | 2 +- ibc-core/ics04-channel/src/handler/chan_open_try.rs | 2 +- ibc-core/ics04-channel/src/handler/recv_packet.rs | 2 +- ibc-core/ics04-channel/src/handler/send_packet.rs | 2 +- ibc-core/ics04-channel/src/handler/timeout.rs | 2 +- ibc-core/ics04-channel/src/handler/timeout_on_close.rs | 2 +- 18 files changed, 20 insertions(+), 20 deletions(-) diff --git a/ibc-core/ics02-client/src/handler/update_client.rs b/ibc-core/ics02-client/src/handler/update_client.rs index 98f3a57aa..54dd9fd42 100644 --- a/ibc-core/ics02-client/src/handler/update_client.rs +++ b/ibc-core/ics02-client/src/handler/update_client.rs @@ -30,7 +30,7 @@ where client_state .status(ctx.get_client_validation_context(), &client_id)? - .verify_active()?; + .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 index 1c84d7586..5bf1386e4 100644 --- a/ibc-core/ics02-client/src/handler/upgrade_client.rs +++ b/ibc-core/ics02-client/src/handler/upgrade_client.rs @@ -29,7 +29,7 @@ where // Check if the client is active. old_client_state .status(ctx.get_client_validation_context(), &client_id)? - .verify_active()?; + .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 index b76e00a5b..9660758f4 100644 --- a/ibc-core/ics02-client/types/src/status.rs +++ b/ibc-core/ics02-client/types/src/status.rs @@ -16,7 +16,7 @@ pub enum UpdateKind { } /// Represents the status of a client -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq)] pub enum Status { /// The client is active and allowed to be used Active, @@ -42,8 +42,8 @@ impl Status { } /// Checks whether the status is active; returns `Err` if not. - pub fn verify_active(&self) -> Result<(), ClientError> { - match self.clone() { + pub fn verify_is_active(self) -> Result<(), ClientError> { + match self { Self::Active => Ok(()), status => Err(ClientError::ClientNotActive { 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 index 33e8c3bd1..865bd78ff 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs @@ -57,7 +57,7 @@ where client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), vars.client_id_on_a())? - .verify_active()?; + .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 index 511801978..f265c6029 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs @@ -46,7 +46,7 @@ where client_state_of_a_on_b .status(ctx_b.get_client_validation_context(), client_id_on_b)? - .verify_active()?; + .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 index 55ffbb70e..2f4a4506d 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_init.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_init.rs @@ -21,7 +21,7 @@ where client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), &msg.client_id_on_a)? - .verify_active()?; + .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 index 94d346fd0..8ddd3c274 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_try.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_try.rs @@ -56,7 +56,7 @@ where client_state_of_a_on_b .status(ctx_b.get_client_validation_context(), &msg.client_id_on_b)? - .verify_active()?; + .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 index caffe0c19..516d581e6 100644 --- a/ibc-core/ics04-channel/src/handler/acknowledgement.rs +++ b/ibc-core/ics04-channel/src/handler/acknowledgement.rs @@ -179,7 +179,7 @@ where client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), client_id_on_a)? - .verify_active()?; + .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 index 41e40f41a..dd3b51b06 100644 --- a/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs +++ b/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs @@ -116,7 +116,7 @@ where client_state_of_a_on_b .status(ctx_b.get_client_validation_context(), client_id_on_b)? - .verify_active()?; + .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 index 57a674d21..fe7588361 100644 --- a/ibc-core/ics04-channel/src/handler/chan_close_init.rs +++ b/ibc-core/ics04-channel/src/handler/chan_close_init.rs @@ -113,7 +113,7 @@ where let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?; client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), client_id_on_a)? - .verify_active()?; + .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 index 6077adc6d..5bf0e3211 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_ack.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_ack.rs @@ -113,7 +113,7 @@ where client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), client_id_on_a)? - .verify_active()?; + .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 index b64eb908e..bb9920d3b 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs @@ -118,7 +118,7 @@ where client_state_of_a_on_b .status(ctx_b.get_client_validation_context(), client_id_on_b)? - .verify_active()?; + .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 index 296ed3cf1..81f645204 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_init.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_init.rs @@ -124,7 +124,7 @@ where client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), client_id_on_a)? - .verify_active()?; + .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 index 5eeb595a3..80888b0b1 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_try.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_try.rs @@ -139,7 +139,7 @@ where client_state_of_a_on_b .status(ctx_b.get_client_validation_context(), client_id_on_b)? - .verify_active()?; + .verify_is_active()?; client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?; diff --git a/ibc-core/ics04-channel/src/handler/recv_packet.rs b/ibc-core/ics04-channel/src/handler/recv_packet.rs index 5de5f9130..bd32a3258 100644 --- a/ibc-core/ics04-channel/src/handler/recv_packet.rs +++ b/ibc-core/ics04-channel/src/handler/recv_packet.rs @@ -183,7 +183,7 @@ where client_state_of_a_on_b .status(ctx_b.get_client_validation_context(), client_id_on_b)? - .verify_active()?; + .verify_is_active()?; client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?; diff --git a/ibc-core/ics04-channel/src/handler/send_packet.rs b/ibc-core/ics04-channel/src/handler/send_packet.rs index 7cecd8afe..a0f3405c0 100644 --- a/ibc-core/ics04-channel/src/handler/send_packet.rs +++ b/ibc-core/ics04-channel/src/handler/send_packet.rs @@ -55,7 +55,7 @@ pub fn send_packet_validate( client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), client_id_on_a)? - .verify_active()?; + .verify_is_active()?; let latest_height_on_a = client_state_of_b_on_a.latest_height(); diff --git a/ibc-core/ics04-channel/src/handler/timeout.rs b/ibc-core/ics04-channel/src/handler/timeout.rs index a1d7ec061..adef59bfb 100644 --- a/ibc-core/ics04-channel/src/handler/timeout.rs +++ b/ibc-core/ics04-channel/src/handler/timeout.rs @@ -190,7 +190,7 @@ where client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), client_id_on_a)? - .verify_active()?; + .verify_is_active()?; client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?; diff --git a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs index 8765d1614..eb6224a4b 100644 --- a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs +++ b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs @@ -69,7 +69,7 @@ where client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), client_id_on_a)? - .verify_active()?; + .verify_is_active()?; client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?;