imp!: define helper trait for Timestamp conversions + update `Conse…

…nsusState::timestamp()` to return `Result` (#1353)

* feat: introduce utility traits for converting to/from Timestamp

* chore: make TimestampError variants consistent with convention

* imp: allow ConsensusState::timestamp() return error

* chore: update Cargo.lock + changelogs

* fix: apply comments

.changelog/unreleased/breaking-changes/1352-update-ConsensusState-timestamp-to-return-Result.md

@@ -0,0 +1,3 @@
+- [ibc-core-client] Update ICS-02 `ConsensusState::timestamp()` to return
+  `Result<Timestamp, ClientError>`
+  ([\#1352](https://github.com/cosmos/ibc-rs/issues/1352))

.changelog/unreleased/improvements/1323-Timestamp-converstions-to-from-host-time.md

@@ -0,0 +1,3 @@
+- [ibc-primitives] Define utility traits for converting between `Timestamp` and
+  host-specific time types.
+  ([#1323](https://github.com/cosmos/ibc-rs/pull/1323)).

ibc-clients/ics07-tendermint/src/client_state/common.rs

@@ -180,7 +180,7 @@ pub fn consensus_state_status<CS: ConsensusState>(
     // 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 +

ibc-clients/ics07-tendermint/src/client_state/execution.rs

@@ -8,7 +8,7 @@ use ibc_core_host::types::identifiers::ClientId;
 use ibc_core_host::types::path::{ClientConsensusStatePath, ClientStatePath};
 use ibc_primitives::prelude::*;
 use ibc_primitives::proto::Any;
-use ibc_primitives::TimestampError;
+use ibc_primitives::{IntoHostTime, TimestampError};
 
 use super::ClientState;
 
@@ -337,7 +337,7 @@ where
         let tm_consensus_state: ConsensusStateType =
             consensus_state.try_into().map_err(Into::into)?;
 
-        let host_timestamp = ctx.host_timestamp()?.into_tm_time();
+        let host_timestamp = ctx.host_timestamp()?.into_host_time()?;
 
         let tm_consensus_state_timestamp = tm_consensus_state.timestamp();
         let tm_consensus_state_expiry = (tm_consensus_state_timestamp

ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs

@@ -8,7 +8,7 @@ use ibc_core_host::types::error::IdentifierError;
 use ibc_core_host::types::identifiers::{ChainId, ClientId};
 use ibc_core_host::types::path::ClientConsensusStatePath;
 use ibc_primitives::prelude::*;
-use ibc_primitives::Timestamp;
+use ibc_primitives::{IntoHostTime, IntoTimestamp, Timestamp};
 use tendermint::crypto::Sha256;
 use tendermint::merkle::MerkleHash;
 use tendermint::{Hash, Time};
@@ -98,7 +98,7 @@ where
 
     // ensure trusted consensus state is within trusting period
     {
-        let trusted_timestamp = trusted_time.try_into().expect("time conversion failed");
+        let trusted_timestamp = trusted_time.into_timestamp()?;
 
         let duration_since_consensus_state =
             current_timestamp.duration_since(&trusted_timestamp).ok_or(
@@ -128,7 +128,7 @@ where
     let trusted_state =
         header.as_trusted_block_state(tm_chain_id, trusted_time, trusted_next_validator_hash)?;
 
-    let current_timestamp = current_timestamp.into_tm_time();
+    let current_timestamp = current_timestamp.into_host_time()?;
 
     verifier
         .verify_misbehaviour_header(untrusted_state, trusted_state, options, current_timestamp)

ibc-clients/ics07-tendermint/src/client_state/update_client.rs

@@ -7,6 +7,7 @@ use ibc_core_host::types::error::IdentifierError;
 use ibc_core_host::types::identifiers::{ChainId, ClientId};
 use ibc_core_host::types::path::ClientConsensusStatePath;
 use ibc_primitives::prelude::*;
+use ibc_primitives::IntoHostTime;
 use tendermint::crypto::Sha256;
 use tendermint::merkle::MerkleHash;
 use tendermint_light_client_verifier::options::Options;
@@ -83,7 +84,7 @@ where
             next_validators: None,
         };
 
-        let now = ctx.host_timestamp()?.into_tm_time();
+        let now = ctx.host_timestamp()?.into_host_time()?;
 
         // main header verification, delegated to the tendermint-light-client crate.
         verifier

ibc-clients/ics07-tendermint/src/consensus_state.rs

@@ -9,11 +9,12 @@
 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::*;
 use ibc_primitives::proto::{Any, Protobuf};
-use ibc_primitives::Timestamp;
+use ibc_primitives::{IntoTimestamp, Timestamp};
 use tendermint::{Hash, Time};
 
 /// Newtype wrapper around the `ConsensusState` type imported from the
@@ -91,10 +92,7 @@ impl ConsensusStateTrait for ConsensusState {
         &self.0.root
     }
 
-    fn timestamp(&self) -> Timestamp {
-        self.0
-            .timestamp
-            .try_into()
-            .expect("UNIX Timestamp can't be negative")
+    fn timestamp(&self) -> Result<Timestamp, ClientError> {
+        self.0.timestamp.into_timestamp().map_err(Into::into)
     }
 }

ibc-clients/ics07-tendermint/types/src/consensus_state.rs

@@ -3,6 +3,7 @@
 use ibc_core_commitment_types::commitment::CommitmentRoot;
 use ibc_core_host_types::error::DecodingError;
 use ibc_primitives::prelude::*;
+use ibc_primitives::{IntoHostTime, Timestamp};
 use ibc_proto::google::protobuf::Any;
 use ibc_proto::ibc::lightclients::tendermint::v1::ConsensusState as RawConsensusState;
 use ibc_proto::Protobuf;
@@ -34,6 +35,7 @@ impl ConsensusState {
         }
     }
 
+    /// Returns the timestamp of the consensus state as a `tendermint::Time`.
     pub fn timestamp(&self) -> Time {
         self.timestamp
     }
@@ -56,15 +58,15 @@ impl TryFrom<RawConsensusState> for ConsensusState {
             ))?
             .hash;
 
-        let ibc_proto::google::protobuf::Timestamp { seconds, nanos } = raw
+        let timestamp: Timestamp = raw
             .timestamp
-            .ok_or(DecodingError::missing_raw_data("consensus state timestamp"))?;
-        // FIXME: shunts like this are necessary due to
-        // https://github.com/informalsystems/tendermint-rs/issues/1053
-        let proto_timestamp = tpb::Timestamp { seconds, nanos };
-        let timestamp = proto_timestamp
+            .ok_or(DecodingError::missing_raw_data("consensus state timestamp"))?
             .try_into()
-            .map_err(|e| DecodingError::invalid_raw_data(format!("timestamp: {e}")))?;
+            .map_err(DecodingError::invalid_raw_data)?;
+
+        let timestamp = timestamp
+            .into_host_time()
+            .map_err(DecodingError::invalid_raw_data)?;
 
         let next_validators_hash = Hash::from_bytes(Algorithm::Sha256, &raw.next_validators_hash)
             .map_err(|e| {

ibc-clients/ics07-tendermint/types/src/header.rs

@@ -8,7 +8,7 @@ use ibc_core_client_types::Height;
 use ibc_core_host_types::error::DecodingError;
 use ibc_core_host_types::identifiers::ChainId;
 use ibc_primitives::prelude::*;
-use ibc_primitives::Timestamp;
+use ibc_primitives::{IntoTimestamp, Timestamp};
 use ibc_proto::google::protobuf::Any;
 use ibc_proto::ibc::lightclients::tendermint::v1::Header as RawHeader;
 use ibc_proto::Protobuf;
@@ -52,8 +52,8 @@ impl Header {
         self.signed_header
             .header
             .time
-            .try_into()
-            .map_err(TendermintClientError::InvalidTimestamp)
+            .into_timestamp()
+            .map_err(Into::into)
     }
 
     pub fn height(&self) -> Height {

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<Any> {
     fn root(&self) -> &CommitmentRoot;
 
     /// The timestamp of the consensus state
-    fn timestamp(&self) -> Timestamp;
+    fn timestamp(&self) -> Result<Timestamp, ClientError>;
 }

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);

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(&timestamp_of_b, msg.proof_height_on_b) {
             return Err(ChannelError::InsufficientPacketTimeout {

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),*
                 }

ibc-primitives/Cargo.toml

@@ -31,9 +31,6 @@ time        = { version = ">=0.3.0, <0.3.37", default-features = false }
 # ibc dependencies
 ibc-proto = { workspace = true }
 
-# cosmos dependencies
-tendermint = { workspace = true }
-
 # parity dependencies
 parity-scale-codec = { workspace = true, optional = true }
 scale-info         = { workspace = true, optional = true }
@@ -49,7 +46,6 @@ std = [
   "prost/std",
   "serde/std",
   "ibc-proto/std",
-  "tendermint/std",
   "time/std",
 ]
 serde = [

ibc-primitives/src/types/timestamp.rs

@@ -12,7 +12,6 @@ use ibc_proto::google::protobuf::Timestamp as RawTimestamp;
 use ibc_proto::Protobuf;
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
-use tendermint::Time;
 use time::macros::offset;
 use time::{OffsetDateTime, PrimitiveDateTime};
 
@@ -37,8 +36,8 @@ pub struct Timestamp {
 
 impl Timestamp {
     pub fn from_nanoseconds(nanoseconds: u64) -> Self {
-        // As the `u64` representation can only represent times up to about year
-        // 2554, there is no risk of overflowing `Time` or `OffsetDateTime`.
+        // As the `u64` can only represent times up to about year 2554, there is
+        // no risk of overflowing `Time` or `OffsetDateTime`.
         let odt = OffsetDateTime::from_unix_timestamp_nanos(nanoseconds.into())
             .expect("nanoseconds as u64 is in the range");
         Self::from_utc(odt).expect("nanoseconds as u64 is in the range")
@@ -104,11 +103,6 @@ impl Timestamp {
         s.try_into()
             .expect("Fails UNIX timestamp is negative, but we don't allow that to be constructed")
     }
-
-    pub fn into_tm_time(self) -> Time {
-        Time::try_from(self.time.assume_offset(offset!(UTC)))
-            .expect("Timestamp is in the range of 0..=9999 years")
-    }
 }
 
 impl Protobuf<RawTimestamp> for Timestamp {}
@@ -189,12 +183,35 @@ impl Sub<Duration> for Timestamp {
     }
 }
 
-impl TryFrom<Time> for Timestamp {
-    type Error = TimestampError;
+/// Utility trait for converting a `Timestamp` into a host-specific time format.
+pub trait IntoHostTime<T: Sized> {
+    /// Converts a `Timestamp` into another time representation of type `T`.
+    ///
+    /// This method adapts the `Timestamp` to a domain-specific format, which
+    /// could represent a custom timestamp used by a light client, or any
+    /// hosting environment that requires its own time format.
+    fn into_host_time(self) -> Result<T, TimestampError>;
+}
+
+/// Utility trait for converting an arbitrary host-specific time format into a
+/// `Timestamp`.
+pub trait IntoTimestamp {
+    /// Converts a time representation of type `T` back into a `Timestamp`.
+    ///
+    /// This can be used to convert from custom light client or host time
+    /// formats back into the standard `Timestamp` format.
+    fn into_timestamp(self) -> Result<Timestamp, TimestampError>;
+}
+
+impl<T: TryFrom<OffsetDateTime>> IntoHostTime<T> for Timestamp {
+    fn into_host_time(self) -> Result<T, TimestampError> {
+        T::try_from(self.into()).map_err(|_| TimestampError::InvalidDate)
+    }
+}
 
-    fn try_from(tm_time: Time) -> Result<Self, Self::Error> {
-        let odt: OffsetDateTime = tm_time.into();
-        odt.try_into()
+impl<T: Into<OffsetDateTime>> IntoTimestamp for T {
+    fn into_timestamp(self) -> Result<Timestamp, TimestampError> {
+        Timestamp::try_from(self.into())
     }
 }
 
@@ -268,12 +285,12 @@ impl scale_info::TypeInfo for Timestamp {
 
 #[derive(Debug, Display, derive_more::From)]
 pub enum TimestampError {
-    /// parse int error: {0}
-    ParseInt(ParseIntError),
-    /// try from int error: {0}
-    TryFromInt(TryFromIntError),
-    /// failed to convert timestamp: {0}
-    Conversion(time::error::ComponentRange),
+    /// failed to parse integer: {0}
+    FailedToParseInt(ParseIntError),
+    /// failed try_from on integer: {0}
+    FailedTryFromInt(TryFromIntError),
+    /// failed to convert offset date: {0}
+    FailedToConvert(time::error::ComponentRange),
     /// invalid date: out of range
     InvalidDate,
     /// overflowed timestamp
@@ -284,9 +301,9 @@ pub enum TimestampError {
 impl std::error::Error for TimestampError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match &self {
-            Self::ParseInt(e) => Some(e),
-            Self::TryFromInt(e) => Some(e),
-            Self::Conversion(e) => Some(e),
+            Self::FailedToParseInt(e) => Some(e),
+            Self::FailedTryFromInt(e) => Some(e),
+            Self::FailedToConvert(e) => Some(e),
             _ => None,
         }
     }

ibc-testkit/src/hosts/tendermint.rs

@@ -10,7 +10,7 @@ use ibc::core::host::types::identifiers::ChainId;
 use ibc::core::primitives::prelude::*;
 use ibc::core::primitives::Timestamp;
 use ibc::primitives::proto::Any;
-use ibc::primitives::ToVec;
+use ibc::primitives::{IntoHostTime, IntoTimestamp, ToVec};
 use tendermint::block::Header as TmHeader;
 use tendermint::validator::Set as ValidatorSet;
 use tendermint_testgen::light_block::TmLightBlock;
@@ -66,7 +66,7 @@ impl TestHost for TendermintHost {
                 .height(height)
                 .chain_id(self.chain_id.as_str())
                 .next_validators(&params.next_validators)
-                .time(timestamp.into_tm_time()),
+                .time(timestamp.into_host_time().expect("Never fails")),
         )
         .validators(&params.validators)
         .next_validators(&params.next_validators)
@@ -116,7 +116,7 @@ impl TestBlock for TmLightBlock {
         self.signed_header
             .header
             .time
-            .try_into()
+            .into_timestamp()
             .expect("Never fails")
     }
 
@@ -200,7 +200,7 @@ impl TestHeader for TendermintHeader {
             .signed_header
             .header
             .time
-            .try_into()
+            .into_timestamp()
             .expect("Never fails")
     }
 }