diff --git a/.changelog/unreleased/breaking-changes/1352-update-ConsensusState-timestamp-to-return-Result.md b/.changelog/unreleased/breaking-changes/1352-update-ConsensusState-timestamp-to-return-Result.md new file mode 100644 index 000000000..f21ac0b1f --- /dev/null +++ b/.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` + ([\#1352](https://github.com/cosmos/ibc-rs/issues/1352)) diff --git a/.changelog/unreleased/improvements/1323-Timestamp-converstions-to-from-host-time.md b/.changelog/unreleased/improvements/1323-Timestamp-converstions-to-from-host-time.md new file mode 100644 index 000000000..26ad71bf9 --- /dev/null +++ b/.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)). diff --git a/ci/cw-check/Cargo.lock b/ci/cw-check/Cargo.lock index 128dbb7e6..1a6d8ae1a 100644 --- a/ci/cw-check/Cargo.lock +++ b/ci/cw-check/Cargo.lock @@ -1092,7 +1092,6 @@ dependencies = [ "scale-info", "schemars", "serde", - "tendermint", "time", ] diff --git a/ci/no-std-check/Cargo.lock b/ci/no-std-check/Cargo.lock index bbc64e565..a870e3c71 100644 --- a/ci/no-std-check/Cargo.lock +++ b/ci/no-std-check/Cargo.lock @@ -1521,7 +1521,6 @@ dependencies = [ "ibc-proto", "prost", "serde", - "tendermint", "time", ] diff --git a/ibc-clients/ics07-tendermint/src/client_state/common.rs b/ibc-clients/ics07-tendermint/src/client_state/common.rs index 4623df3ac..bc7725e5b 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/client_state/execution.rs b/ibc-clients/ics07-tendermint/src/client_state/execution.rs index d4aafbf1b..636e0cc3f 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/execution.rs +++ b/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 diff --git a/ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs b/ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs index 0d8ce51ce..eefa2eaa1 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs +++ b/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) diff --git a/ibc-clients/ics07-tendermint/src/client_state/update_client.rs b/ibc-clients/ics07-tendermint/src/client_state/update_client.rs index d0f706b30..695fe72d0 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/update_client.rs +++ b/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 diff --git a/ibc-clients/ics07-tendermint/src/consensus_state.rs b/ibc-clients/ics07-tendermint/src/consensus_state.rs index 52f0173ce..b85546f16 100644 --- a/ibc-clients/ics07-tendermint/src/consensus_state.rs +++ b/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 { + 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 d38fda228..dd64ce0fb 100644 --- a/ibc-clients/ics07-tendermint/types/src/consensus_state.rs +++ b/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 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| { diff --git a/ibc-clients/ics07-tendermint/types/src/header.rs b/ibc-clients/ics07-tendermint/types/src/header.rs index 376e16c78..80a1966a1 100644 --- a/ibc-clients/ics07-tendermint/types/src/header.rs +++ b/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 { diff --git a/ibc-core/ics02-client/context/src/consensus_state.rs b/ibc-core/ics02-client/context/src/consensus_state.rs index 556486c67..2149c76ff 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 1206d0460..b393c74ad 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 33604b958..da4e42d37 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 87fafcffe..b4e0067c6 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-primitives/Cargo.toml b/ibc-primitives/Cargo.toml index 2c56cd6de..7f8432e7d 100644 --- a/ibc-primitives/Cargo.toml +++ b/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 = [ diff --git a/ibc-primitives/src/types/timestamp.rs b/ibc-primitives/src/types/timestamp.rs index fe2b043dd..33fca85d6 100644 --- a/ibc-primitives/src/types/timestamp.rs +++ b/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 for Timestamp {} @@ -189,12 +183,35 @@ impl Sub for Timestamp { } } -impl TryFrom