From b25e92ce422bce66885431d71348146445a803d0 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 17 Jul 2023 15:17:52 -0400 Subject: [PATCH 01/15] Status enum --- crates/ibc/src/core/ics02_client/client_state.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index bc79de426..26eb450c2 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -30,6 +30,21 @@ 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, + /// Indicates there was an error in determining the status of a client. + Unknown, + /// Unauthorized indicates that the client type is not registered as an allowed client type. + Unauthorized, +} + /// `ClientState` methods needed in both validation and execution. /// /// They do not require access to a client `ValidationContext` nor From f07411feee930c400a85751b2cc6864ac8910a35 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 17 Jul 2023 16:08:09 -0400 Subject: [PATCH 02/15] implement status API --- .../traits/client_state_validation.rs | 19 ++++++ crates/ibc-derive/src/utils.rs | 4 ++ .../clients/ics07_tendermint/client_state.rs | 45 +++++++++++- .../ibc/src/core/ics02_client/client_state.rs | 3 + crates/ibc/src/mock/client_state.rs | 68 ++++++++++++++++++- crates/ibc/src/mock/context/clients.rs | 17 +++++ 6 files changed, 154 insertions(+), 2 deletions(-) diff --git a/crates/ibc-derive/src/client_state/traits/client_state_validation.rs b/crates/ibc-derive/src/client_state/traits/client_state_validation.rs index 3bc90958c..66067a8e4 100644 --- a/crates/ibc-derive/src/client_state/traits/client_state_validation.rs +++ b/crates/ibc-derive/src/client_state/traits/client_state_validation.rs @@ -30,6 +30,13 @@ 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; @@ -37,6 +44,7 @@ pub(crate) fn impl_ClientStateValidation( 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, + ) -> #Status { + match self { + #(#status_impl),* + } + + } } } diff --git a/crates/ibc-derive/src/utils.rs b/crates/ibc-derive/src/utils.rs index 4dc30ad29..fca155424 100644 --- a/crates/ibc-derive/src/utils.rs +++ b/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 diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index be1a1e7e0..0d2b73f95 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -27,7 +27,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; @@ -253,6 +253,10 @@ impl ClientState { self.chain_id.clone() } + 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; @@ -427,6 +431,7 @@ impl ClientStateCommon for ClientState { impl ClientStateValidation for ClientState where ClientValidationContext: TmValidationContext, + ClientValidationContext::AnyConsensusState: TryInto, { fn verify_client_message( &self, @@ -465,6 +470,44 @@ where } } } + + fn status(&self, ctx: &ClientValidationContext, client_id: &ClientId) -> Status { + if self.is_frozen() { + return 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 Status::Expired, + }; + + match any_latest_consensus_state.try_into() { + Ok(tm_cs) => tm_cs, + Err(_) => return Status::Unknown, + } + }; + + let now = match ctx.host_timestamp() { + Ok(ts) => ts, + Err(_) => return Status::Unknown, + }; + let elapsed_since_latest_consensus_state = + match now.duration_since(&latest_consensus_state.timestamp()) { + Some(ts) => ts, + None => return Status::Unknown, + }; + + if self.expired(elapsed_since_latest_consensus_state) { + return Status::Expired; + } + + Status::Active + } } impl ClientStateExecution for ClientState diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 26eb450c2..e408cbb97 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -155,6 +155,9 @@ pub trait ClientStateValidation { client_message: Any, update_kind: &UpdateKind, ) -> Result; + + /// Returns the status of the client. Only Active clients are allowed to process packets. + fn status(&self, ctx: &ClientValidationContext, client_id: &ClientId) -> Status; } /// `ClientState` methods which require access to the client's diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 24b2bd688..64672c0dc 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -1,3 +1,6 @@ +use crate::core::ics02_client::client_state::Status; +use crate::core::timestamp::Timestamp; +use crate::core::ContextError; use crate::prelude::*; use core::str::FromStr; @@ -66,6 +69,10 @@ impl MockClientState { ..self } } + + fn is_frozen(&self) -> bool { + self.frozen_height.is_some() + } } impl Protobuf for MockClientState {} @@ -125,6 +132,23 @@ impl From for Any { } } +pub trait MockClientContext { + type ConversionError: ToString; + type AnyConsensusState: TryInto; + + /// Retrieve the consensus state for the given client ID at the specified + /// height. + /// + /// Returns an error if no such state exists. + fn consensus_state( + &self, + client_cons_state_path: &ClientConsensusStatePath, + ) -> Result; + + /// Returns the current timestamp of the local chain. + fn host_timestamp(&self) -> Result; +} + impl ClientStateCommon for MockClientState { fn verify_consensus_state(&self, consensus_state: Any) -> Result<(), ClientError> { let _mock_consensus_state = MockConsensusState::try_from(consensus_state)?; @@ -204,7 +228,11 @@ impl ClientStateCommon for MockClientState { } } -impl ClientStateValidation for MockClientState { +impl ClientStateValidation for MockClientState +where + ClientValidationContext: MockClientContext, + ClientValidationContext::AnyConsensusState: TryInto, +{ fn verify_client_message( &self, _ctx: &ClientValidationContext, @@ -252,6 +280,44 @@ impl ClientStateValidation for } } } + + fn status(&self, ctx: &ClientValidationContext, client_id: &ClientId) -> Status { + if self.is_frozen() { + return Status::Frozen; + } + + let latest_consensus_state: MockConsensusState = { + 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 Status::Expired, + }; + + match any_latest_consensus_state.try_into() { + Ok(mock_cs) => mock_cs, + Err(_) => return Status::Unknown, + } + }; + + let now = match ctx.host_timestamp() { + Ok(ts) => ts, + Err(_) => return Status::Unknown, + }; + let elapsed_since_latest_consensus_state = + match now.duration_since(&latest_consensus_state.timestamp()) { + Some(ts) => ts, + None => return Status::Unknown, + }; + + if self.expired(elapsed_since_latest_consensus_state) { + return Status::Expired; + } + + Status::Active + } } impl ClientStateExecution for MockClientState diff --git a/crates/ibc/src/mock/context/clients.rs b/crates/ibc/src/mock/context/clients.rs index f9b13fd25..448195902 100644 --- a/crates/ibc/src/mock/context/clients.rs +++ b/crates/ibc/src/mock/context/clients.rs @@ -1,5 +1,6 @@ //! Client context implementations for `MockContext` +use crate::mock::client_state::MockClientContext; use crate::prelude::*; use super::{AnyClientState, AnyConsensusState, MockClientRecord, MockContext}; @@ -15,6 +16,22 @@ use crate::core::ContextError; use crate::core::ValidationContext; use crate::Height; +impl MockClientContext for MockContext { + type ConversionError = &'static str; + type AnyConsensusState = AnyConsensusState; + + fn consensus_state( + &self, + client_cons_state_path: &ClientConsensusStatePath, + ) -> Result { + ValidationContext::consensus_state(self, client_cons_state_path) + } + + fn host_timestamp(&self) -> Result { + ValidationContext::host_timestamp(self) + } +} + impl TmCommonContext for MockContext { type ConversionError = &'static str; type AnyConsensusState = AnyConsensusState; From 6d93e0c0f63b763dc92b6dac461d46139d3148b3 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 17 Jul 2023 16:48:45 -0400 Subject: [PATCH 03/15] replace most confirm_not_frozen --- crates/ibc/src/core/ics02_client/client_state.rs | 8 +++++++- crates/ibc/src/core/ics02_client/error.rs | 4 ++++ .../src/core/ics02_client/handler/update_client.rs | 12 ++++++++++-- .../src/core/ics02_client/handler/upgrade_client.rs | 9 +++++++-- .../core/ics03_connection/handler/conn_open_ack.rs | 11 +++++++++-- .../ics03_connection/handler/conn_open_confirm.rs | 11 +++++++++-- .../core/ics03_connection/handler/conn_open_init.rs | 12 ++++++++++-- .../core/ics03_connection/handler/conn_open_try.rs | 11 +++++++++-- crates/ibc/src/core/ics04_channel/context.rs | 7 +++++++ .../core/ics04_channel/handler/acknowledgement.rs | 11 +++++++++-- .../core/ics04_channel/handler/chan_close_confirm.rs | 11 +++++++++-- .../core/ics04_channel/handler/chan_close_init.rs | 11 +++++++++-- .../src/core/ics04_channel/handler/chan_open_ack.rs | 11 +++++++++-- .../core/ics04_channel/handler/chan_open_confirm.rs | 11 +++++++++-- .../src/core/ics04_channel/handler/chan_open_init.rs | 12 ++++++++++-- .../src/core/ics04_channel/handler/chan_open_try.rs | 11 +++++++++-- .../src/core/ics04_channel/handler/recv_packet.rs | 11 +++++++++-- .../src/core/ics04_channel/handler/send_packet.rs | 11 ++++++++++- crates/ibc/src/core/ics04_channel/handler/timeout.rs | 12 +++++++++++- .../core/ics04_channel/handler/timeout_on_close.rs | 11 +++++++++-- 20 files changed, 175 insertions(+), 33 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index e408cbb97..5aeec8a11 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -1,6 +1,6 @@ //! 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; @@ -45,6 +45,12 @@ pub enum Status { Unauthorized, } +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 diff --git a/crates/ibc/src/core/ics02_client/error.rs b/crates/ibc/src/core/ics02_client/error.rs index efda51a0f..2e8c841cb 100644 --- a/crates/ibc/src/core/ics02_client/error.rs +++ b/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}` diff --git a/crates/ibc/src/core/ics02_client/handler/update_client.rs b/crates/ibc/src/core/ics02_client/handler/update_client.rs index 3499dac35..20fc11f2f 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -4,10 +4,10 @@ use crate::prelude::*; use crate::core::context::ContextError; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::ClientStateCommon; use crate::core::ics02_client::client_state::ClientStateExecution; use crate::core::ics02_client::client_state::ClientStateValidation; use crate::core::ics02_client::client_state::UpdateKind; +use crate::core::ics02_client::client_state::{ClientStateCommon, Status}; use crate::core::ics02_client::error::ClientError; use crate::core::ics02_client::events::{ClientMisbehaviour, UpdateClient}; use crate::core::ics02_client::msgs::MsgUpdateOrMisbehaviour; @@ -28,7 +28,15 @@ 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 != Status::Active { + return Err(ClientError::ClientNotActive { + status, + } + .into()); + } + } let client_message = msg.client_message(); diff --git a/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs b/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs index 70035c5b3..45d122606 100644 --- a/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs @@ -4,8 +4,8 @@ use crate::prelude::*; use crate::core::context::ContextError; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::ClientStateCommon; use crate::core::ics02_client::client_state::ClientStateExecution; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics02_client::error::ClientError; use crate::core::ics02_client::events::UpgradeClient; @@ -27,7 +27,12 @@ where let old_client_state = ctx.client_state(&client_id)?; // Check if the client is frozen. - old_client_state.confirm_not_frozen()?; + { + let status = old_client_state.status(ctx.get_client_validation_context(), &client_id); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } // Read the latest consensus state from the host chain store. let old_client_cons_state_path = diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs index fffb0a747..dc0f06aac 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs @@ -4,8 +4,9 @@ use ibc_proto::protobuf::Protobuf; use prost::Message; use crate::core::context::ContextError; -use crate::core::ics02_client::client_state::ClientStateCommon; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; use crate::core::ics02_client::consensus_state::ConsensusState; +use crate::core::ics02_client::error::ClientError; use crate::core::ics03_connection::connection::{ConnectionEnd, Counterparty, State}; use crate::core::ics03_connection::error::ConnectionError; use crate::core::ics03_connection::events::OpenAck; @@ -58,7 +59,13 @@ where { let client_state_of_b_on_a = ctx_a.client_state(vars.client_id_on_a())?; - client_state_of_b_on_a.confirm_not_frozen()?; + { + let status = client_state_of_b_on_a + .status(ctx_a.get_client_validation_context(), vars.client_id_on_a()); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } client_state_of_b_on_a.validate_proof_height(msg.proofs_height_on_b)?; let client_cons_state_path_on_a = diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs index 3f7d88f23..fd1a24524 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs @@ -3,8 +3,9 @@ use ibc_proto::protobuf::Protobuf; use crate::core::context::ContextError; -use crate::core::ics02_client::client_state::ClientStateCommon; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; use crate::core::ics02_client::consensus_state::ConsensusState; +use crate::core::ics02_client::error::ClientError; use crate::core::ics03_connection::connection::{ConnectionEnd, Counterparty, State}; use crate::core::ics03_connection::error::ConnectionError; use crate::core::ics03_connection::events::OpenConfirm; @@ -47,7 +48,13 @@ where { let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?; - client_state_of_a_on_b.confirm_not_frozen()?; + { + let status = client_state_of_a_on_b + .status(ctx_b.get_client_validation_context(), client_id_on_b); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?; let client_cons_state_path_on_b = diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs index ec64be11b..78b506c19 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs @@ -1,9 +1,10 @@ //! Protocol logic specific to ICS3 messages of type `MsgConnectionOpenInit`. +use crate::core::ics02_client::client_state::{ClientStateValidation, Status}; +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use crate::core::context::ContextError; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::ClientStateCommon; use crate::core::ics03_connection::connection::{ConnectionEnd, Counterparty, State}; use crate::core::ics03_connection::events::OpenInit; use crate::core::ics03_connection::msgs::conn_open_init::MsgConnectionOpenInit; @@ -19,7 +20,14 @@ 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)?; - client_state_of_b_on_a.confirm_not_frozen()?; + + { + let status = client_state_of_b_on_a + .status(ctx_a.get_client_validation_context(), &msg.client_id_on_a); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } if let Some(version) = msg.version { version.verify_is_supported(&ctx_a.get_compatible_versions())?; diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs index 5608fe7c0..659237f4c 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs @@ -4,8 +4,9 @@ use ibc_proto::protobuf::Protobuf; use prost::Message; use crate::core::context::ContextError; -use crate::core::ics02_client::client_state::ClientStateCommon; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; use crate::core::ics02_client::consensus_state::ConsensusState; +use crate::core::ics02_client::error::ClientError; use crate::core::ics03_connection::connection::{ConnectionEnd, Counterparty, State}; use crate::core::ics03_connection::error::ConnectionError; use crate::core::ics03_connection::events::OpenTry; @@ -59,7 +60,13 @@ where { let client_state_of_a_on_b = ctx_b.client_state(vars.conn_end_on_b.client_id())?; - client_state_of_a_on_b.confirm_not_frozen()?; + { + let status = client_state_of_a_on_b + .status(ctx_b.get_client_validation_context(), &msg.client_id_on_b); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } client_state_of_a_on_b.validate_proof_height(msg.proofs_height_on_a)?; let client_cons_state_path_on_b = diff --git a/crates/ibc/src/core/ics04_channel/context.rs b/crates/ibc/src/core/ics04_channel/context.rs index 1f95c6521..3e24e1729 100644 --- a/crates/ibc/src/core/ics04_channel/context.rs +++ b/crates/ibc/src/core/ics04_channel/context.rs @@ -26,6 +26,9 @@ pub trait SendPacketValidationContext { type AnyConsensusState: ConsensusState; type AnyClientState: ClientState; + /// Retrieve the context that implements all clients' `ValidationContext`. + fn get_client_validation_context(&self) -> &Self::ClientValidationContext; + /// Returns the ChannelEnd for the given `port_id` and `chan_id`. fn channel_end(&self, channel_end_path: &ChannelEndPath) -> Result; @@ -54,6 +57,10 @@ where type AnyConsensusState = T::AnyConsensusState; type AnyClientState = T::AnyClientState; + fn get_client_validation_context(&self) -> &Self::ClientValidationContext { + self.get_client_validation_context() + } + fn channel_end(&self, channel_end_path: &ChannelEndPath) -> Result { self.channel_end(channel_end_path) } diff --git a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs index e65cbecaa..9af3ac6bc 100644 --- a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs @@ -1,7 +1,8 @@ +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use crate::core::events::MessageEvent; -use crate::core::ics02_client::client_state::ClientStateCommon; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics03_connection::delay::verify_conn_delay_passed; @@ -186,7 +187,13 @@ 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)?; - client_state_of_b_on_a.confirm_not_frozen()?; + { + let status = client_state_of_b_on_a + .status(ctx_a.get_client_validation_context(), client_id_on_a); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?; let client_cons_state_path_on_a = diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs index 85f5df02f..45dae0db7 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs @@ -1,10 +1,11 @@ //! Protocol logic specific to ICS4 messages of type `MsgChannelCloseConfirm`. +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use ibc_proto::protobuf::Protobuf; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::ClientStateCommon; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics04_channel::channel::State; @@ -122,7 +123,13 @@ 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)?; - client_state_of_a_on_b.confirm_not_frozen()?; + { + let status = client_state_of_a_on_b + .status(ctx_b.get_client_validation_context(), &client_id_on_b); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?; let client_cons_state_path_on_b = diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs index 448801a15..496c10403 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs @@ -1,8 +1,9 @@ //! Protocol logic specific to ICS4 messages of type `MsgChannelCloseInit`. +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::ClientStateCommon; +use crate::core::ics02_client::client_state::{ClientStateValidation, Status}; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics04_channel::channel::State; use crate::core::ics04_channel::error::ChannelError; @@ -117,7 +118,13 @@ 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)?; - client_state_of_b_on_a.confirm_not_frozen()?; + { + let status = + client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } Ok(()) } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs index b1d229cdb..e200acb47 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs @@ -1,10 +1,11 @@ //! Protocol logic specific to ICS4 messages of type `MsgChannelOpenAck`. +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use ibc_proto::protobuf::Protobuf; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::ClientStateCommon; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics04_channel::channel::State; @@ -120,7 +121,13 @@ 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)?; - client_state_of_b_on_a.confirm_not_frozen()?; + { + let status = client_state_of_b_on_a + .status(ctx_a.get_client_validation_context(), client_id_on_a); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?; let client_cons_state_path_on_a = diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs index dd8e52463..ac1335312 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs @@ -1,10 +1,11 @@ //! Protocol logic specific to ICS4 messages of type `MsgChannelOpenConfirm`. +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use ibc_proto::protobuf::Protobuf; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::ClientStateCommon; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics04_channel::channel::State; @@ -125,7 +126,13 @@ 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)?; - client_state_of_a_on_b.confirm_not_frozen()?; + { + let status = client_state_of_a_on_b + .status(ctx_b.get_client_validation_context(), &client_id_on_b); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?; let client_cons_state_path_on_b = diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs index 95e9ba3e9..6acfa1fde 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs @@ -1,9 +1,10 @@ //! Protocol logic specific to ICS4 messages of type `MsgChannelOpenInit`. +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::ClientStateCommon; +use crate::core::ics02_client::client_state::{ClientStateValidation, Status}; use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, State}; use crate::core::ics04_channel::error::ChannelError; use crate::core::ics04_channel::events::OpenInit; @@ -128,7 +129,14 @@ 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)?; - client_state_of_b_on_a.confirm_not_frozen()?; + + { + let status = + client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } let conn_version = conn_end_on_a.versions(); diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs index a9ca05e43..bef31a796 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs @@ -1,10 +1,11 @@ //! Protocol logic specific to ICS4 messages of type `MsgChannelOpenTry`. +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use ibc_proto::protobuf::Protobuf; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::ClientStateCommon; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics04_channel::channel::State; @@ -145,7 +146,13 @@ 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)?; - client_state_of_a_on_b.confirm_not_frozen()?; + { + let status = client_state_of_a_on_b + .status(ctx_b.get_client_validation_context(), &client_id_on_b); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?; let client_cons_state_path_on_b = diff --git a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs index fe0f0d131..39ab16329 100644 --- a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs @@ -1,7 +1,8 @@ +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::ClientStateCommon; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics03_connection::delay::verify_conn_delay_passed; @@ -189,7 +190,13 @@ 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)?; - client_state_of_a_on_b.confirm_not_frozen()?; + { + let status = client_state_of_a_on_b + .status(ctx_b.get_client_validation_context(), &client_id_on_b); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?; let client_cons_state_path_on_b = diff --git a/crates/ibc/src/core/ics04_channel/handler/send_packet.rs b/crates/ibc/src/core/ics04_channel/handler/send_packet.rs index 009f6ee99..5b2328532 100644 --- a/crates/ibc/src/core/ics04_channel/handler/send_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/send_packet.rs @@ -1,3 +1,6 @@ +use crate::core::ics02_client::client_state::ClientStateValidation; +use crate::core::ics02_client::client_state::Status; +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use crate::core::events::IbcEvent; @@ -56,7 +59,13 @@ pub fn send_packet_validate( let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?; - client_state_of_b_on_a.confirm_not_frozen()?; + { + let status = + client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } let latest_height_on_a = client_state_of_b_on_a.latest_height(); diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout.rs b/crates/ibc/src/core/ics04_channel/handler/timeout.rs index 8a142d9fc..a7bca71a7 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout.rs @@ -1,3 +1,6 @@ +use crate::core::ics02_client::client_state::ClientStateValidation; +use crate::core::ics02_client::client_state::Status; +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use prost::Message; @@ -200,7 +203,14 @@ 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)?; - client_state_of_b_on_a.confirm_not_frozen()?; + + { + let status = client_state_of_b_on_a + .status(ctx_a.get_client_validation_context(), client_id_on_a); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } 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/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs index 5630a1f2a..6ea0c391a 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs @@ -1,8 +1,9 @@ +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use ibc_proto::protobuf::Protobuf; use prost::Message; -use crate::core::ics02_client::client_state::ClientStateCommon; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::delay::verify_conn_delay_passed; use crate::core::ics04_channel::channel::State; @@ -70,7 +71,13 @@ 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)?; - client_state_of_b_on_a.confirm_not_frozen()?; + { + let status = client_state_of_b_on_a + .status(ctx_a.get_client_validation_context(), client_id_on_a); + if status != Status::Active { + return Err(ClientError::ClientNotActive { status }.into()); + } + } client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?; let client_cons_state_path_on_a = From c8a084b6404e1815c2ba8db22b1ee05c9d27e544 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 20 Jul 2023 13:33:20 -0400 Subject: [PATCH 04/15] validate_self_client no longer uses confirm_not_frozen --- crates/ibc/src/clients/ics07_tendermint/client_state.rs | 2 +- crates/ibc/src/hosts/tendermint/validate_self_client.rs | 7 ++++++- crates/ibc/src/mock/client_state.rs | 2 +- crates/ibc/src/mock/context.rs | 8 ++++++-- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 0d2b73f95..1f434b5e5 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -253,7 +253,7 @@ impl ClientState { self.chain_id.clone() } - fn is_frozen(&self) -> bool { + pub fn is_frozen(&self) -> bool { self.frozen_height.is_some() } diff --git a/crates/ibc/src/hosts/tendermint/validate_self_client.rs b/crates/ibc/src/hosts/tendermint/validate_self_client.rs index b305027d4..ccff5fcd4 100644 --- a/crates/ibc/src/hosts/tendermint/validate_self_client.rs +++ b/crates/ibc/src/hosts/tendermint/validate_self_client.rs @@ -30,7 +30,12 @@ pub trait ValidateSelfClientContext { tm_client_state.validate().map_err(ClientError::from)?; - tm_client_state.confirm_not_frozen()?; + if tm_client_state.is_frozen() { + return Err(ClientError::ClientFrozen { + description: String::new(), + } + .into()); + } let self_chain_id = self.chain_id(); if self_chain_id != &tm_client_state.chain_id { diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 64672c0dc..e77276ab5 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -70,7 +70,7 @@ impl MockClientState { } } - fn is_frozen(&self) -> bool { + pub fn is_frozen(&self) -> bool { self.frozen_height.is_some() } } diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index 78b5bc802..6d5d4c610 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -31,7 +31,6 @@ use crate::clients::ics07_tendermint::consensus_state::TENDERMINT_CONSENSUS_STAT use crate::core::dispatch; use crate::core::events::IbcEvent; use crate::core::ics02_client::client_state::ClientState; -use crate::core::ics02_client::client_state::ClientStateCommon; use crate::core::ics02_client::client_type::ClientType; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics02_client::error::ClientError; @@ -897,7 +896,12 @@ impl ValidationContext for MockContext { }) .map_err(ContextError::ConnectionError)?; - mock_client_state.confirm_not_frozen()?; + if mock_client_state.is_frozen() { + return Err(ClientError::ClientFrozen { + description: String::new(), + } + .into()); + } let self_chain_id = &self.host_chain_id; let self_revision_number = self_chain_id.version(); From d48b33e1f87a2a2ab408d043edd925ccdcfa6c72 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 20 Jul 2023 13:37:53 -0400 Subject: [PATCH 05/15] remove confirm_not_frozen --- .../src/client_state/traits/client_state_common.rs | 11 ----------- .../ibc/src/clients/ics07_tendermint/client_state.rs | 9 --------- crates/ibc/src/core/ics02_client/client_state.rs | 3 --- .../src/core/ics02_client/handler/update_client.rs | 8 ++++---- crates/ibc/src/mock/client_state.rs | 9 --------- 5 files changed, 4 insertions(+), 36 deletions(-) diff --git a/crates/ibc-derive/src/client_state/traits/client_state_common.rs b/crates/ibc-derive/src/client_state/traits/client_state_common.rs index 3c9f4f4b6..3fc3f0de5 100644 --- a/crates/ibc-derive/src/client_state/traits/client_state_common.rs +++ b/crates/ibc-derive/src/client_state/traits/client_state_common.rs @@ -32,11 +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(), @@ -95,12 +90,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),* diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 1f434b5e5..28d07db86 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -298,15 +298,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 } diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 5aeec8a11..0e181d215 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -71,9 +71,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; diff --git a/crates/ibc/src/core/ics02_client/handler/update_client.rs b/crates/ibc/src/core/ics02_client/handler/update_client.rs index 20fc11f2f..fad895133 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -253,7 +253,7 @@ 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) == Status::Active); assert_eq!(client_state.latest_height(), latest_header_height); } @@ -300,7 +300,7 @@ 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) == Status::Active); assert_eq!(client_state.latest_height(), latest_header_height); } @@ -419,7 +419,7 @@ 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) == Status::Active); assert_eq!(client_state.latest_height(), latest_header_height); assert_eq!(client_state, ctx_a.latest_client_states(&msg.client_id)); } @@ -502,7 +502,7 @@ 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()); + assert!(client_state.status(&ctx, &client_id) == Status::Active); // check events assert_eq!(ctx.events.len(), 2); diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index e77276ab5..0fdbf4ef7 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -174,15 +174,6 @@ impl ClientStateCommon for MockClientState { 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 { false } From 4219b33f9ed72e834fdf64c5eb31f5db9070ab21 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 20 Jul 2023 13:39:47 -0400 Subject: [PATCH 06/15] clippy --- crates/ibc/src/core/ics02_client/handler/update_client.rs | 7 ++----- .../src/core/ics04_channel/handler/chan_close_confirm.rs | 2 +- .../src/core/ics04_channel/handler/chan_open_confirm.rs | 2 +- crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs | 2 +- crates/ibc/src/core/ics04_channel/handler/recv_packet.rs | 2 +- 5 files changed, 6 insertions(+), 9 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/handler/update_client.rs b/crates/ibc/src/core/ics02_client/handler/update_client.rs index fad895133..eaf518e30 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -31,10 +31,7 @@ where { let status = client_state.status(ctx.get_client_validation_context(), &client_id); if status != Status::Active { - return Err(ClientError::ClientNotActive { - status, - } - .into()); + return Err(ClientError::ClientNotActive { status }.into()); } } @@ -502,7 +499,7 @@ 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.status(&ctx, &client_id) == Status::Active); + assert!(client_state.status(ctx, client_id) == Status::Active); // check events assert_eq!(ctx.events.len(), 2); diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs index 45dae0db7..0a814894b 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs @@ -125,7 +125,7 @@ where { let status = client_state_of_a_on_b - .status(ctx_b.get_client_validation_context(), &client_id_on_b); + .status(ctx_b.get_client_validation_context(), client_id_on_b); if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs index ac1335312..ada6d8f50 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs @@ -128,7 +128,7 @@ where { let status = client_state_of_a_on_b - .status(ctx_b.get_client_validation_context(), &client_id_on_b); + .status(ctx_b.get_client_validation_context(), client_id_on_b); if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs index bef31a796..bb866aadd 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs @@ -148,7 +148,7 @@ where { let status = client_state_of_a_on_b - .status(ctx_b.get_client_validation_context(), &client_id_on_b); + .status(ctx_b.get_client_validation_context(), client_id_on_b); if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs index 39ab16329..e51318d97 100644 --- a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs @@ -192,7 +192,7 @@ where { let status = client_state_of_a_on_b - .status(ctx_b.get_client_validation_context(), &client_id_on_b); + .status(ctx_b.get_client_validation_context(), client_id_on_b); if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } From 115ba50598d77c9c10e956d3caffbfc1042c6380 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 20 Jul 2023 13:44:17 -0400 Subject: [PATCH 07/15] fix test --- crates/ibc/src/core/ics02_client/handler/update_client.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ibc/src/core/ics02_client/handler/update_client.rs b/crates/ibc/src/core/ics02_client/handler/update_client.rs index eaf518e30..3284c83d4 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -499,7 +499,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.status(ctx, client_id) == Status::Active); + let status = client_state.status(ctx, client_id); + assert!(status == Status::Frozen, "client_state status: {status}"); // check events assert_eq!(ctx.events.len(), 2); From 79788882105554a1d8b32f10bd3a2588e86d255c Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 20 Jul 2023 13:53:42 -0400 Subject: [PATCH 08/15] remove expired() --- .../traits/client_state_common.rs | 11 -------- .../clients/ics07_tendermint/client_state.rs | 8 +++--- .../ibc/src/core/ics02_client/client_state.rs | 5 ---- .../ics02_client/handler/upgrade_client.rs | 27 ++++++++----------- crates/ibc/src/mock/client_state.rs | 8 +++--- 5 files changed, 19 insertions(+), 40 deletions(-) diff --git a/crates/ibc-derive/src/client_state/traits/client_state_common.rs b/crates/ibc-derive/src/client_state/traits/client_state_common.rs index 3fc3f0de5..b1dc37731 100644 --- a/crates/ibc-derive/src/client_state/traits/client_state_common.rs +++ b/crates/ibc-derive/src/client_state/traits/client_state_common.rs @@ -32,11 +32,6 @@ pub(crate) fn impl_ClientStateCommon( enum_variants.iter(), quote! {validate_proof_height(cs, proof_height)}, ); - 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(), @@ -90,12 +85,6 @@ pub(crate) fn impl_ClientStateCommon( } } - fn expired(&self, elapsed: core::time::Duration) -> bool { - match self { - #(#expired_impl),* - } - } - fn verify_upgrade_client( &self, upgraded_client_state: #Any, diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 28d07db86..81b2765f6 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -257,6 +257,10 @@ impl ClientState { self.frozen_height.is_some() } + fn expired(&self, elapsed: Duration) -> bool { + elapsed > self.trusting_period + } + // Resets custom fields to zero values (used in `update_client`) pub fn zero_custom_fields(&mut self) { self.trusting_period = ZERO_DURATION; @@ -298,10 +302,6 @@ impl ClientStateCommon for ClientState { 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. diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 0e181d215..21966dfa2 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -2,7 +2,6 @@ use core::fmt::{Debug, Display, Formatter}; use core::marker::{Send, Sync}; -use core::time::Duration; use ibc_proto::google::protobuf::Any; @@ -71,10 +70,6 @@ pub trait ClientStateCommon { /// Validate that the client is at a sufficient height fn validate_proof_height(&self, proof_height: Height) -> 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. /// diff --git a/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs b/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs index 45d122606..22f17532c 100644 --- a/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs @@ -44,23 +44,18 @@ where height: old_client_state.latest_height(), })?; - let now = ctx.host_timestamp()?; - let duration = now - .duration_since(&old_consensus_state.timestamp()) - .ok_or_else(|| ClientError::InvalidConsensusStateTimestamp { - time1: old_consensus_state.timestamp(), - time2: now, - })?; - // Check if the latest consensus state is within the trust period. - if old_client_state.expired(duration) { - return Err(ContextError::ClientError( - ClientError::HeaderNotWithinTrustPeriod { - latest_time: old_consensus_state.timestamp(), - update_time: now, - }, - )); - }; + { + let status = old_client_state.status(ctx.get_client_validation_context(), &client_id); + if status == Status::Expired { + return Err(ContextError::ClientError( + ClientError::HeaderNotWithinTrustPeriod { + latest_time: old_consensus_state.timestamp(), + update_time: ctx.host_timestamp()?, + }, + )); + } + } // Validate the upgraded client state and consensus state and verify proofs against the root old_client_state.verify_upgrade_client( diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 0fdbf4ef7..104b314c2 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -73,6 +73,10 @@ impl MockClientState { pub fn is_frozen(&self) -> bool { self.frozen_height.is_some() } + + fn expired(&self, _elapsed: Duration) -> bool { + false + } } impl Protobuf for MockClientState {} @@ -174,10 +178,6 @@ impl ClientStateCommon for MockClientState { Ok(()) } - fn expired(&self, _elapsed: Duration) -> bool { - false - } - fn verify_upgrade_client( &self, upgraded_client_state: Any, From 195c137b41bd896818b6ae6bf148606ee51a6eb3 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 20 Jul 2023 13:55:42 -0400 Subject: [PATCH 09/15] changelog --- .../unreleased/breaking-changes/536-clientstate-status.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/breaking-changes/536-clientstate-status.md diff --git a/.changelog/unreleased/breaking-changes/536-clientstate-status.md b/.changelog/unreleased/breaking-changes/536-clientstate-status.md new file mode 100644 index 000000000..a6d40b8d4 --- /dev/null +++ b/.changelog/unreleased/breaking-changes/536-clientstate-status.md @@ -0,0 +1,2 @@ +- Replace `ClientState::{confirm_not_frozen, expired}()` with `ClientState::status()` + ([#536](https://github.com/cosmos/ibc-rs/issues/536)) From 53cc396e60d3d94ab53f06a45b30cf43a0fc448e Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 24 Jul 2023 17:23:59 -0400 Subject: [PATCH 10/15] Remove `Status::Unknown` --- .../traits/client_state_validation.rs | 2 +- .../clients/ics07_tendermint/client_state.rs | 36 +++++++++---------- .../ibc/src/core/ics02_client/client_state.rs | 8 +++-- crates/ibc/src/core/ics02_client/error.rs | 8 +++++ .../ics02_client/handler/update_client.rs | 10 +++--- .../ics02_client/handler/upgrade_client.rs | 4 +-- .../ics03_connection/handler/conn_open_ack.rs | 2 +- .../handler/conn_open_confirm.rs | 2 +- .../handler/conn_open_init.rs | 2 +- .../ics03_connection/handler/conn_open_try.rs | 2 +- .../ics04_channel/handler/acknowledgement.rs | 2 +- .../handler/chan_close_confirm.rs | 2 +- .../ics04_channel/handler/chan_close_init.rs | 2 +- .../ics04_channel/handler/chan_open_ack.rs | 2 +- .../handler/chan_open_confirm.rs | 2 +- .../ics04_channel/handler/chan_open_init.rs | 2 +- .../ics04_channel/handler/chan_open_try.rs | 2 +- .../core/ics04_channel/handler/recv_packet.rs | 2 +- .../core/ics04_channel/handler/send_packet.rs | 2 +- .../src/core/ics04_channel/handler/timeout.rs | 2 +- .../ics04_channel/handler/timeout_on_close.rs | 2 +- crates/ibc/src/mock/client_state.rs | 36 +++++++++---------- 22 files changed, 72 insertions(+), 62 deletions(-) diff --git a/crates/ibc-derive/src/client_state/traits/client_state_validation.rs b/crates/ibc-derive/src/client_state/traits/client_state_validation.rs index 66067a8e4..38c8ba397 100644 --- a/crates/ibc-derive/src/client_state/traits/client_state_validation.rs +++ b/crates/ibc-derive/src/client_state/traits/client_state_validation.rs @@ -77,7 +77,7 @@ pub(crate) fn impl_ClientStateValidation( &self, ctx: &#ClientValidationContext, client_id: &#ClientId, - ) -> #Status { + ) -> core::result::Result<#Status, #ClientError> { match self { #(#status_impl),* } diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 81b2765f6..cdf69e8bb 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -423,6 +423,8 @@ impl ClientStateValidation for where ClientValidationContext: TmValidationContext, ClientValidationContext::AnyConsensusState: TryInto, + ClientError: + From<>::Error>, { fn verify_client_message( &self, @@ -462,9 +464,13 @@ where } } - fn status(&self, ctx: &ClientValidationContext, client_id: &ClientId) -> Status { + fn status( + &self, + ctx: &ClientValidationContext, + client_id: &ClientId, + ) -> Result { if self.is_frozen() { - return Status::Frozen; + return Ok(Status::Frozen); } let latest_consensus_state: TmConsensusState = { @@ -474,30 +480,24 @@ where 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 Status::Expired, + Err(_) => return Ok(Status::Expired), }; - match any_latest_consensus_state.try_into() { - Ok(tm_cs) => tm_cs, - Err(_) => return Status::Unknown, - } + any_latest_consensus_state.try_into()? }; - let now = match ctx.host_timestamp() { - Ok(ts) => ts, - Err(_) => return Status::Unknown, - }; - let elapsed_since_latest_consensus_state = - match now.duration_since(&latest_consensus_state.timestamp()) { - Some(ts) => ts, - None => return Status::Unknown, - }; + let now = ctx.host_timestamp()?; + let elapsed_since_latest_consensus_state = now + .duration_since(&latest_consensus_state.timestamp()) + .ok_or(ClientError::Other { + description: format!("latest consensus state is in the future. now: {now}, latest consensus state: {}", latest_consensus_state.timestamp()), + })?; if self.expired(elapsed_since_latest_consensus_state) { - return Status::Expired; + return Ok(Status::Expired); } - Status::Active + Ok(Status::Active) } } diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 21966dfa2..ec8e5e6c4 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -38,8 +38,6 @@ pub enum Status { Frozen, /// The client is expired and not allowed to be used Expired, - /// Indicates there was an error in determining the status of a client. - Unknown, /// Unauthorized indicates that the client type is not registered as an allowed client type. Unauthorized, } @@ -155,7 +153,11 @@ pub trait ClientStateValidation { ) -> Result; /// Returns the status of the client. Only Active clients are allowed to process packets. - fn status(&self, ctx: &ClientValidationContext, client_id: &ClientId) -> Status; + fn status( + &self, + ctx: &ClientValidationContext, + client_id: &ClientId, + ) -> Result; } /// `ClientState` methods which require access to the client's diff --git a/crates/ibc/src/core/ics02_client/error.rs b/crates/ibc/src/core/ics02_client/error.rs index 2e8c841cb..bde6f5900 100644 --- a/crates/ibc/src/core/ics02_client/error.rs +++ b/crates/ibc/src/core/ics02_client/error.rs @@ -125,6 +125,14 @@ impl From 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)> { diff --git a/crates/ibc/src/core/ics02_client/handler/update_client.rs b/crates/ibc/src/core/ics02_client/handler/update_client.rs index 3284c83d4..28abad52e 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -29,7 +29,7 @@ where let client_state = ctx.client_state(&client_id)?; { - let status = client_state.status(ctx.get_client_validation_context(), &client_id); + let status = client_state.status(ctx.get_client_validation_context(), &client_id)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } @@ -250,7 +250,7 @@ mod tests { assert!(res.is_ok(), "result: {res:?}"); let client_state = ctx.client_state(&msg.client_id).unwrap(); - assert!(client_state.status(&ctx, &msg.client_id) == Status::Active); + assert!(client_state.status(&ctx, &msg.client_id).unwrap() == Status::Active); assert_eq!(client_state.latest_height(), latest_header_height); } @@ -297,7 +297,7 @@ mod tests { assert!(res.is_ok(), "result: {res:?}"); let client_state = ctx.client_state(&msg.client_id).unwrap(); - assert!(client_state.status(&ctx, &msg.client_id) == Status::Active); + assert!(client_state.status(&ctx, &msg.client_id).unwrap() == Status::Active); assert_eq!(client_state.latest_height(), latest_header_height); } @@ -416,7 +416,7 @@ mod tests { assert!(res.is_ok(), "result: {res:?}"); let client_state = ctx_a.client_state(&msg.client_id).unwrap(); - assert!(client_state.status(&ctx_a, &msg.client_id) == Status::Active); + assert!(client_state.status(&ctx_a, &msg.client_id).unwrap() == Status::Active); assert_eq!(client_state.latest_height(), latest_header_height); assert_eq!(client_state, ctx_a.latest_client_states(&msg.client_id)); } @@ -499,7 +499,7 @@ mod tests { fn ensure_misbehaviour(ctx: &MockContext, client_id: &ClientId, client_type: &ClientType) { let client_state = ctx.client_state(client_id).unwrap(); - let status = client_state.status(ctx, client_id); + let status = client_state.status(ctx, client_id).unwrap(); assert!(status == Status::Frozen, "client_state status: {status}"); // check events diff --git a/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs b/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs index 22f17532c..a5d2de09b 100644 --- a/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs @@ -28,7 +28,7 @@ where // Check if the client is frozen. { - let status = old_client_state.status(ctx.get_client_validation_context(), &client_id); + let status = old_client_state.status(ctx.get_client_validation_context(), &client_id)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } @@ -46,7 +46,7 @@ where // Check if the latest consensus state is within the trust period. { - let status = old_client_state.status(ctx.get_client_validation_context(), &client_id); + let status = old_client_state.status(ctx.get_client_validation_context(), &client_id)?; if status == Status::Expired { return Err(ContextError::ClientError( ClientError::HeaderNotWithinTrustPeriod { diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs index dc0f06aac..c7067152f 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs @@ -61,7 +61,7 @@ where { let status = client_state_of_b_on_a - .status(ctx_a.get_client_validation_context(), vars.client_id_on_a()); + .status(ctx_a.get_client_validation_context(), vars.client_id_on_a())?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs index fd1a24524..41e9a7f2e 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs @@ -50,7 +50,7 @@ where { let status = client_state_of_a_on_b - .status(ctx_b.get_client_validation_context(), client_id_on_b); + .status(ctx_b.get_client_validation_context(), client_id_on_b)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs index 78b506c19..3fe82966d 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs @@ -23,7 +23,7 @@ where { let status = client_state_of_b_on_a - .status(ctx_a.get_client_validation_context(), &msg.client_id_on_a); + .status(ctx_a.get_client_validation_context(), &msg.client_id_on_a)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs index 659237f4c..0ba513bce 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs @@ -62,7 +62,7 @@ where { let status = client_state_of_a_on_b - .status(ctx_b.get_client_validation_context(), &msg.client_id_on_b); + .status(ctx_b.get_client_validation_context(), &msg.client_id_on_b)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs index 9af3ac6bc..a9a44d35e 100644 --- a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs @@ -189,7 +189,7 @@ where { let status = client_state_of_b_on_a - .status(ctx_a.get_client_validation_context(), client_id_on_a); + .status(ctx_a.get_client_validation_context(), client_id_on_a)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs index 0a814894b..8ba41c1b6 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs @@ -125,7 +125,7 @@ where { let status = client_state_of_a_on_b - .status(ctx_b.get_client_validation_context(), client_id_on_b); + .status(ctx_b.get_client_validation_context(), client_id_on_b)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs index 496c10403..d765c0b28 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs @@ -120,7 +120,7 @@ where 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); + client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs index e200acb47..883759c2e 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs @@ -123,7 +123,7 @@ where { let status = client_state_of_b_on_a - .status(ctx_a.get_client_validation_context(), client_id_on_a); + .status(ctx_a.get_client_validation_context(), client_id_on_a)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs index ada6d8f50..8d3007b31 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs @@ -128,7 +128,7 @@ where { let status = client_state_of_a_on_b - .status(ctx_b.get_client_validation_context(), client_id_on_b); + .status(ctx_b.get_client_validation_context(), client_id_on_b)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs index 6acfa1fde..a76f9fa6d 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs @@ -132,7 +132,7 @@ where { let status = - client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a); + client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs index bb866aadd..769394b9f 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs @@ -148,7 +148,7 @@ where { let status = client_state_of_a_on_b - .status(ctx_b.get_client_validation_context(), client_id_on_b); + .status(ctx_b.get_client_validation_context(), client_id_on_b)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs index e51318d97..dc4599e87 100644 --- a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs @@ -192,7 +192,7 @@ where { let status = client_state_of_a_on_b - .status(ctx_b.get_client_validation_context(), client_id_on_b); + .status(ctx_b.get_client_validation_context(), client_id_on_b)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/send_packet.rs b/crates/ibc/src/core/ics04_channel/handler/send_packet.rs index 5b2328532..fb59854eb 100644 --- a/crates/ibc/src/core/ics04_channel/handler/send_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/send_packet.rs @@ -61,7 +61,7 @@ pub fn send_packet_validate( { let status = - client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a); + client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout.rs b/crates/ibc/src/core/ics04_channel/handler/timeout.rs index a7bca71a7..09b29b5d6 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout.rs @@ -206,7 +206,7 @@ where { let status = client_state_of_b_on_a - .status(ctx_a.get_client_validation_context(), client_id_on_a); + .status(ctx_a.get_client_validation_context(), client_id_on_a)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs index 6ea0c391a..2bc672408 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs @@ -73,7 +73,7 @@ where { let status = client_state_of_b_on_a - .status(ctx_a.get_client_validation_context(), client_id_on_a); + .status(ctx_a.get_client_validation_context(), client_id_on_a)?; if status != Status::Active { return Err(ClientError::ClientNotActive { status }.into()); } diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 104b314c2..9f31e8d26 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -223,6 +223,8 @@ impl ClientStateValidation for where ClientValidationContext: MockClientContext, ClientValidationContext::AnyConsensusState: TryInto, + ClientError: + From<>::Error>, { fn verify_client_message( &self, @@ -272,9 +274,13 @@ where } } - fn status(&self, ctx: &ClientValidationContext, client_id: &ClientId) -> Status { + fn status( + &self, + ctx: &ClientValidationContext, + client_id: &ClientId, + ) -> Result { if self.is_frozen() { - return Status::Frozen; + return Ok(Status::Frozen); } let latest_consensus_state: MockConsensusState = { @@ -284,30 +290,24 @@ where 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 Status::Expired, + Err(_) => return Ok(Status::Expired), }; - match any_latest_consensus_state.try_into() { - Ok(mock_cs) => mock_cs, - Err(_) => return Status::Unknown, - } + any_latest_consensus_state.try_into()? }; - let now = match ctx.host_timestamp() { - Ok(ts) => ts, - Err(_) => return Status::Unknown, - }; - let elapsed_since_latest_consensus_state = - match now.duration_since(&latest_consensus_state.timestamp()) { - Some(ts) => ts, - None => return Status::Unknown, - }; + let now = ctx.host_timestamp()?; + let elapsed_since_latest_consensus_state = now + .duration_since(&latest_consensus_state.timestamp()) + .ok_or(ClientError::Other { + description: format!("latest consensus state is in the future. now: {now}, latest consensus state: {}", latest_consensus_state.timestamp()), + })?; if self.expired(elapsed_since_latest_consensus_state) { - return Status::Expired; + return Ok(Status::Expired); } - Status::Active + Ok(Status::Active) } } From b98a998dae45e00ba3e6e3e664d7b62b65af0237 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 26 Jul 2023 09:45:15 -0400 Subject: [PATCH 11/15] don't consider the client expired if consensus state is in the future --- .../clients/ics07_tendermint/client_state.rs | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index cdf69e8bb..4484f6220 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -257,10 +257,6 @@ impl ClientState { self.frozen_height.is_some() } - fn expired(&self, elapsed: Duration) -> bool { - elapsed > self.trusting_period - } - // Resets custom fields to zero values (used in `update_client`) pub fn zero_custom_fields(&mut self) { self.trusting_period = ZERO_DURATION; @@ -486,15 +482,16 @@ where 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()?; - let elapsed_since_latest_consensus_state = now - .duration_since(&latest_consensus_state.timestamp()) - .ok_or(ClientError::Other { - description: format!("latest consensus state is in the future. now: {now}, latest consensus state: {}", latest_consensus_state.timestamp()), - })?; - - if self.expired(elapsed_since_latest_consensus_state) { - return Ok(Status::Expired); + 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) From e7f9d6903d79077fbca240cfac83a0ff241ede5d Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 3 Aug 2023 16:12:22 -0400 Subject: [PATCH 12/15] Status methods --- .../ibc/src/core/ics02_client/client_state.rs | 14 +++++++++++++ .../ics02_client/handler/update_client.rs | 21 +++++++++++++------ .../ics02_client/handler/upgrade_client.rs | 2 +- .../ics03_connection/handler/conn_open_ack.rs | 4 ++-- .../handler/conn_open_confirm.rs | 4 ++-- .../handler/conn_open_init.rs | 4 ++-- .../ics03_connection/handler/conn_open_try.rs | 4 ++-- .../ics04_channel/handler/acknowledgement.rs | 4 ++-- .../handler/chan_close_confirm.rs | 4 ++-- .../ics04_channel/handler/chan_close_init.rs | 4 ++-- .../ics04_channel/handler/chan_open_ack.rs | 4 ++-- .../handler/chan_open_confirm.rs | 4 ++-- .../ics04_channel/handler/chan_open_init.rs | 4 ++-- .../ics04_channel/handler/chan_open_try.rs | 4 ++-- .../core/ics04_channel/handler/recv_packet.rs | 4 ++-- .../core/ics04_channel/handler/send_packet.rs | 3 +-- .../src/core/ics04_channel/handler/timeout.rs | 3 +-- .../ics04_channel/handler/timeout_on_close.rs | 4 ++-- 18 files changed, 58 insertions(+), 37 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index ec8e5e6c4..441e97b2b 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -42,6 +42,20 @@ pub enum Status { 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:?}") diff --git a/crates/ibc/src/core/ics02_client/handler/update_client.rs b/crates/ibc/src/core/ics02_client/handler/update_client.rs index 28abad52e..a95e10df4 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -4,10 +4,10 @@ use crate::prelude::*; use crate::core::context::ContextError; use crate::core::events::{IbcEvent, MessageEvent}; +use crate::core::ics02_client::client_state::ClientStateCommon; use crate::core::ics02_client::client_state::ClientStateExecution; use crate::core::ics02_client::client_state::ClientStateValidation; use crate::core::ics02_client::client_state::UpdateKind; -use crate::core::ics02_client::client_state::{ClientStateCommon, Status}; use crate::core::ics02_client::error::ClientError; use crate::core::ics02_client::events::{ClientMisbehaviour, UpdateClient}; use crate::core::ics02_client::msgs::MsgUpdateOrMisbehaviour; @@ -30,7 +30,7 @@ where { let status = client_state.status(ctx.get_client_validation_context(), &client_id)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } @@ -250,7 +250,10 @@ mod tests { assert!(res.is_ok(), "result: {res:?}"); let client_state = ctx.client_state(&msg.client_id).unwrap(); - assert!(client_state.status(&ctx, &msg.client_id).unwrap() == Status::Active); + assert!(client_state + .status(&ctx, &msg.client_id) + .unwrap() + .is_active()); assert_eq!(client_state.latest_height(), latest_header_height); } @@ -297,7 +300,10 @@ mod tests { assert!(res.is_ok(), "result: {res:?}"); let client_state = ctx.client_state(&msg.client_id).unwrap(); - assert!(client_state.status(&ctx, &msg.client_id).unwrap() == Status::Active); + assert!(client_state + .status(&ctx, &msg.client_id) + .unwrap() + .is_active()); assert_eq!(client_state.latest_height(), latest_header_height); } @@ -416,7 +422,10 @@ mod tests { assert!(res.is_ok(), "result: {res:?}"); let client_state = ctx_a.client_state(&msg.client_id).unwrap(); - assert!(client_state.status(&ctx_a, &msg.client_id).unwrap() == Status::Active); + 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)); } @@ -500,7 +509,7 @@ mod tests { let client_state = ctx.client_state(client_id).unwrap(); let status = client_state.status(ctx, client_id).unwrap(); - assert!(status == Status::Frozen, "client_state status: {status}"); + assert!(status.is_frozen(), "client_state status: {status}"); // check events assert_eq!(ctx.events.len(), 2); diff --git a/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs b/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs index a5d2de09b..d0ec57cf8 100644 --- a/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs @@ -29,7 +29,7 @@ where // Check if the client is frozen. { let status = old_client_state.status(ctx.get_client_validation_context(), &client_id)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs index c7067152f..de901eb8d 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs @@ -4,7 +4,7 @@ use ibc_proto::protobuf::Protobuf; use prost::Message; use crate::core::context::ContextError; -use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics02_client::error::ClientError; use crate::core::ics03_connection::connection::{ConnectionEnd, Counterparty, State}; @@ -62,7 +62,7 @@ where { let status = client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), vars.client_id_on_a())?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs index 41e9a7f2e..56a96b45b 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs @@ -3,7 +3,7 @@ use ibc_proto::protobuf::Protobuf; use crate::core::context::ContextError; -use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics02_client::error::ClientError; use crate::core::ics03_connection::connection::{ConnectionEnd, Counterparty, State}; @@ -51,7 +51,7 @@ where { let status = client_state_of_a_on_b .status(ctx_b.get_client_validation_context(), client_id_on_b)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs index 3fe82966d..1d7e11a5c 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs @@ -1,5 +1,5 @@ //! Protocol logic specific to ICS3 messages of type `MsgConnectionOpenInit`. -use crate::core::ics02_client::client_state::{ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::ClientStateValidation; use crate::core::ics02_client::error::ClientError; use crate::prelude::*; @@ -24,7 +24,7 @@ where { let status = client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), &msg.client_id_on_a)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs index 0ba513bce..acb17204d 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs @@ -4,7 +4,7 @@ use ibc_proto::protobuf::Protobuf; use prost::Message; use crate::core::context::ContextError; -use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics02_client::error::ClientError; use crate::core::ics03_connection::connection::{ConnectionEnd, Counterparty, State}; @@ -63,7 +63,7 @@ where { let status = client_state_of_a_on_b .status(ctx_b.get_client_validation_context(), &msg.client_id_on_b)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs index a9a44d35e..b6f55f03d 100644 --- a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs @@ -2,7 +2,7 @@ use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use crate::core::events::MessageEvent; -use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics03_connection::delay::verify_conn_delay_passed; @@ -190,7 +190,7 @@ where { let status = client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), client_id_on_a)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs index 8ba41c1b6..7934e655c 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs @@ -5,7 +5,7 @@ use crate::prelude::*; use ibc_proto::protobuf::Protobuf; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics04_channel::channel::State; @@ -126,7 +126,7 @@ where { let status = client_state_of_a_on_b .status(ctx_b.get_client_validation_context(), client_id_on_b)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs index d765c0b28..fe5beb180 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs @@ -3,7 +3,7 @@ use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::{ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::ClientStateValidation; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics04_channel::channel::State; use crate::core::ics04_channel::error::ChannelError; @@ -121,7 +121,7 @@ where { let status = client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs index 883759c2e..0769ff366 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs @@ -5,7 +5,7 @@ use crate::prelude::*; use ibc_proto::protobuf::Protobuf; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics04_channel::channel::State; @@ -124,7 +124,7 @@ where { let status = client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), client_id_on_a)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs index 8d3007b31..9915c6234 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs @@ -5,7 +5,7 @@ use crate::prelude::*; use ibc_proto::protobuf::Protobuf; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics04_channel::channel::State; @@ -129,7 +129,7 @@ where { let status = client_state_of_a_on_b .status(ctx_b.get_client_validation_context(), client_id_on_b)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs index a76f9fa6d..d8a53c8d1 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs @@ -4,7 +4,7 @@ use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::{ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::ClientStateValidation; use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, State}; use crate::core::ics04_channel::error::ChannelError; use crate::core::ics04_channel::events::OpenInit; @@ -133,7 +133,7 @@ where { let status = client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs index 769394b9f..a46f50d6f 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs @@ -5,7 +5,7 @@ use crate::prelude::*; use ibc_proto::protobuf::Protobuf; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics04_channel::channel::State; @@ -149,7 +149,7 @@ where { let status = client_state_of_a_on_b .status(ctx_b.get_client_validation_context(), client_id_on_b)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs index dc4599e87..dc8885023 100644 --- a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs @@ -2,7 +2,7 @@ use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use crate::core::events::{IbcEvent, MessageEvent}; -use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics03_connection::delay::verify_conn_delay_passed; @@ -193,7 +193,7 @@ where { let status = client_state_of_a_on_b .status(ctx_b.get_client_validation_context(), client_id_on_b)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics04_channel/handler/send_packet.rs b/crates/ibc/src/core/ics04_channel/handler/send_packet.rs index fb59854eb..59cb14c6d 100644 --- a/crates/ibc/src/core/ics04_channel/handler/send_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/send_packet.rs @@ -1,5 +1,4 @@ use crate::core::ics02_client::client_state::ClientStateValidation; -use crate::core::ics02_client::client_state::Status; use crate::core::ics02_client::error::ClientError; use crate::prelude::*; @@ -62,7 +61,7 @@ pub fn send_packet_validate( { let status = client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout.rs b/crates/ibc/src/core/ics04_channel/handler/timeout.rs index 09b29b5d6..2e8157263 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout.rs @@ -1,5 +1,4 @@ use crate::core::ics02_client::client_state::ClientStateValidation; -use crate::core::ics02_client::client_state::Status; use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use prost::Message; @@ -207,7 +206,7 @@ where { let status = client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), client_id_on_a)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs index 2bc672408..9215d588a 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs @@ -3,7 +3,7 @@ use crate::prelude::*; use ibc_proto::protobuf::Protobuf; use prost::Message; -use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics03_connection::delay::verify_conn_delay_passed; use crate::core::ics04_channel::channel::State; @@ -74,7 +74,7 @@ where { let status = client_state_of_b_on_a .status(ctx_a.get_client_validation_context(), client_id_on_a)?; - if status != Status::Active { + if !status.is_active() { return Err(ClientError::ClientNotActive { status }.into()); } } From d0dd1dea759e9f0f3dad934da66ab7526c9d3cef Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 3 Aug 2023 16:13:53 -0400 Subject: [PATCH 13/15] remove redundant check --- .../core/ics02_client/handler/upgrade_client.rs | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs b/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs index d0ec57cf8..3980d5b77 100644 --- a/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs @@ -5,7 +5,7 @@ use crate::prelude::*; use crate::core::context::ContextError; use crate::core::events::{IbcEvent, MessageEvent}; use crate::core::ics02_client::client_state::ClientStateExecution; -use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation, Status}; +use crate::core::ics02_client::client_state::{ClientStateCommon, ClientStateValidation}; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics02_client::error::ClientError; use crate::core::ics02_client::events::UpgradeClient; @@ -26,7 +26,7 @@ where // Read the current latest client state from the host chain store. let old_client_state = ctx.client_state(&client_id)?; - // Check if the client is frozen. + // Check if the client is active. { let status = old_client_state.status(ctx.get_client_validation_context(), &client_id)?; if !status.is_active() { @@ -44,19 +44,6 @@ where height: old_client_state.latest_height(), })?; - // Check if the latest consensus state is within the trust period. - { - let status = old_client_state.status(ctx.get_client_validation_context(), &client_id)?; - if status == Status::Expired { - return Err(ContextError::ClientError( - ClientError::HeaderNotWithinTrustPeriod { - latest_time: old_consensus_state.timestamp(), - update_time: ctx.host_timestamp()?, - }, - )); - } - } - // Validate the upgraded client state and consensus state and verify proofs against the root old_client_state.verify_upgrade_client( msg.upgraded_client_state.clone(), From 7dfa46bfc9ca2e25574b966603eca61c75018b54 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 3 Aug 2023 16:14:37 -0400 Subject: [PATCH 14/15] Remove error variant --- crates/ibc/src/core/ics02_client/error.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/error.rs b/crates/ibc/src/core/ics02_client/error.rs index bde6f5900..2ee0b4831 100644 --- a/crates/ibc/src/core/ics02_client/error.rs +++ b/crates/ibc/src/core/ics02_client/error.rs @@ -95,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}` From 6139215a61e7ae3310b4d594580e8dc9f65881c4 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 4 Aug 2023 11:45:45 -0400 Subject: [PATCH 15/15] update changelog --- .../unreleased/breaking-changes/536-clientstate-status.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/unreleased/breaking-changes/536-clientstate-status.md b/.changelog/unreleased/breaking-changes/536-clientstate-status.md index a6d40b8d4..dc414758f 100644 --- a/.changelog/unreleased/breaking-changes/536-clientstate-status.md +++ b/.changelog/unreleased/breaking-changes/536-clientstate-status.md @@ -1,2 +1,2 @@ -- Replace `ClientState::{confirm_not_frozen, expired}()` with `ClientState::status()` +- [ibc-derive] Replace `ClientState::{confirm_not_frozen, expired}()` with `ClientState::status()` ([#536](https://github.com/cosmos/ibc-rs/issues/536))