From bd27973e1d2bff8303da886e732cf02f010872e9 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Tue, 24 Sep 2024 16:22:09 -0700 Subject: [PATCH] imp: allow ConsensusState::timestamp() return error --- ibc-clients/ics07-tendermint/src/client_state/common.rs | 2 +- ibc-clients/ics07-tendermint/src/consensus_state.rs | 8 +++----- ibc-clients/ics07-tendermint/types/src/consensus_state.rs | 1 + ibc-core/ics02-client/context/src/consensus_state.rs | 3 ++- ibc-core/ics04-channel/src/handler/send_packet.rs | 2 +- ibc-core/ics04-channel/src/handler/timeout.rs | 2 +- ibc-derive/src/consensus_state.rs | 3 ++- .../src/testapp/ibc/clients/mock/consensus_state.rs | 5 +++-- ibc-testkit/src/testapp/ibc/core/core_ctx.rs | 7 ++++++- 9 files changed, 20 insertions(+), 13 deletions(-) diff --git a/ibc-clients/ics07-tendermint/src/client_state/common.rs b/ibc-clients/ics07-tendermint/src/client_state/common.rs index 4623df3acc..bc7725e5b8 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/common.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/common.rs @@ -180,7 +180,7 @@ pub fn consensus_state_status( // consensus state is in the future, then we don't consider the client // to be expired. if let Some(elapsed_since_latest_consensus_state) = - host_timestamp.duration_since(&consensus_state.timestamp()) + host_timestamp.duration_since(&consensus_state.timestamp()?) { // Note: The equality is considered as expired to stay consistent with // the check in tendermint-rs, where a header at `trusted_header_time + diff --git a/ibc-clients/ics07-tendermint/src/consensus_state.rs b/ibc-clients/ics07-tendermint/src/consensus_state.rs index 156c76edd2..b85546f160 100644 --- a/ibc-clients/ics07-tendermint/src/consensus_state.rs +++ b/ibc-clients/ics07-tendermint/src/consensus_state.rs @@ -9,6 +9,7 @@ use ibc_client_tendermint_types::proto::v1::ConsensusState as RawTmConsensusState; use ibc_client_tendermint_types::ConsensusState as ConsensusStateType; use ibc_core_client::context::consensus_state::ConsensusState as ConsensusStateTrait; +use ibc_core_client::types::error::ClientError; use ibc_core_commitment_types::commitment::CommitmentRoot; use ibc_core_host::types::error::DecodingError; use ibc_primitives::prelude::*; @@ -91,10 +92,7 @@ impl ConsensusStateTrait for ConsensusState { &self.0.root } - fn timestamp(&self) -> Timestamp { - self.0 - .timestamp - .into_timestamp() - .expect("UNIX Timestamp can't be negative") + fn timestamp(&self) -> Result { + self.0.timestamp.into_timestamp().map_err(Into::into) } } diff --git a/ibc-clients/ics07-tendermint/types/src/consensus_state.rs b/ibc-clients/ics07-tendermint/types/src/consensus_state.rs index f36fd1807f..dd64ce0fbe 100644 --- a/ibc-clients/ics07-tendermint/types/src/consensus_state.rs +++ b/ibc-clients/ics07-tendermint/types/src/consensus_state.rs @@ -35,6 +35,7 @@ impl ConsensusState { } } + /// Returns the timestamp of the consensus state as a `tendermint::Time`. pub fn timestamp(&self) -> Time { self.timestamp } diff --git a/ibc-core/ics02-client/context/src/consensus_state.rs b/ibc-core/ics02-client/context/src/consensus_state.rs index 556486c676..2149c76ff9 100644 --- a/ibc-core/ics02-client/context/src/consensus_state.rs +++ b/ibc-core/ics02-client/context/src/consensus_state.rs @@ -1,5 +1,6 @@ //! Defines the trait to be implemented by all concrete consensus state types +use ibc_core_client_types::error::ClientError; use ibc_core_commitment_types::commitment::CommitmentRoot; use ibc_primitives::prelude::*; use ibc_primitives::proto::Any; @@ -16,5 +17,5 @@ pub trait ConsensusState: Send + Sync + Convertible { fn root(&self) -> &CommitmentRoot; /// The timestamp of the consensus state - fn timestamp(&self) -> Timestamp; + fn timestamp(&self) -> Result; } diff --git a/ibc-core/ics04-channel/src/handler/send_packet.rs b/ibc-core/ics04-channel/src/handler/send_packet.rs index 1206d04600..b393c74add 100644 --- a/ibc-core/ics04-channel/src/handler/send_packet.rs +++ b/ibc-core/ics04-channel/src/handler/send_packet.rs @@ -76,7 +76,7 @@ pub fn send_packet_validate( ); let consensus_state_of_b_on_a = client_val_ctx_a.consensus_state(&client_cons_state_path_on_a)?; - let latest_timestamp = consensus_state_of_b_on_a.timestamp(); + let latest_timestamp = consensus_state_of_b_on_a.timestamp()?; let packet_timestamp = packet.timeout_timestamp_on_b; if packet_timestamp.has_expired(&latest_timestamp) { return Err(ChannelError::ExpiredPacketTimestamp); diff --git a/ibc-core/ics04-channel/src/handler/timeout.rs b/ibc-core/ics04-channel/src/handler/timeout.rs index 33604b958a..da4e42d373 100644 --- a/ibc-core/ics04-channel/src/handler/timeout.rs +++ b/ibc-core/ics04-channel/src/handler/timeout.rs @@ -193,7 +193,7 @@ where ); let consensus_state_of_b_on_a = client_val_ctx_a.consensus_state(&client_cons_state_path_on_a)?; - let timestamp_of_b = consensus_state_of_b_on_a.timestamp(); + let timestamp_of_b = consensus_state_of_b_on_a.timestamp()?; if !msg.packet.timed_out(×tamp_of_b, msg.proof_height_on_b) { return Err(ChannelError::InsufficientPacketTimeout { diff --git a/ibc-derive/src/consensus_state.rs b/ibc-derive/src/consensus_state.rs index 87fafcffec..b4e0067c62 100644 --- a/ibc-derive/src/consensus_state.rs +++ b/ibc-derive/src/consensus_state.rs @@ -24,6 +24,7 @@ pub fn consensus_state_derive_impl(ast: DeriveInput, imports: &Imports) -> Token let CommitmentRoot = imports.commitment_root(); let ConsensusState = imports.consensus_state(); let Timestamp = imports.timestamp(); + let ClientError = imports.client_error(); quote! { impl #ConsensusState for #enum_name { @@ -33,7 +34,7 @@ pub fn consensus_state_derive_impl(ast: DeriveInput, imports: &Imports) -> Token } } - fn timestamp(&self) -> #Timestamp { + fn timestamp(&self) -> core::result::Result<#Timestamp, #ClientError> { match self { #(#timestamp_impl),* } diff --git a/ibc-testkit/src/testapp/ibc/clients/mock/consensus_state.rs b/ibc-testkit/src/testapp/ibc/clients/mock/consensus_state.rs index 8149a5d62b..beedced78b 100644 --- a/ibc-testkit/src/testapp/ibc/clients/mock/consensus_state.rs +++ b/ibc-testkit/src/testapp/ibc/clients/mock/consensus_state.rs @@ -1,4 +1,5 @@ use ibc::core::client::context::consensus_state::ConsensusState; +use ibc::core::client::types::error::ClientError; use ibc::core::commitment_types::commitment::CommitmentRoot; use ibc::core::host::types::error::DecodingError; use ibc::core::primitives::prelude::*; @@ -91,7 +92,7 @@ impl ConsensusState for MockConsensusState { &self.root } - fn timestamp(&self) -> Timestamp { - self.header.timestamp + fn timestamp(&self) -> Result { + Ok(self.header.timestamp) } } diff --git a/ibc-testkit/src/testapp/ibc/core/core_ctx.rs b/ibc-testkit/src/testapp/ibc/core/core_ctx.rs index 9e18373083..2158400ef7 100644 --- a/ibc-testkit/src/testapp/ibc/core/core_ctx.rs +++ b/ibc-testkit/src/testapp/ibc/core/core_ctx.rs @@ -48,7 +48,12 @@ where fn host_timestamp(&self) -> Result { let host_height = self.host_height()?; let host_cons_state = self.host_consensus_state(&host_height)?; - Ok(host_cons_state.timestamp()) + let timestamp = host_cons_state + .timestamp() + .map_err(|e| HostError::InvalidState { + description: e.to_string(), + })?; + Ok(timestamp) } fn client_counter(&self) -> Result {