Use proper Timestamp type to track time (informalsystems#855)

* Use proper Timestamp type to track time

* Handle error when converting from u64 to Timestamp

* Use more precise result when comparing the expiry between two timestamps

* Move timestamp module to root crate

* Fix import errors

* Add basic tests for timestamp

* Address review feedback

* Update changelog

CHANGELOG.md

@@ -6,9 +6,10 @@
 
 - [ibc]
   - Reinstated `ics23` dependency ([#854])
+  - Use proper Timestamp type to track time ([#855])
 - [ibc-relayer]
   - Change the default for client creation to allow governance recovery in case of expiration or misbehaviour. ([#785])
-  
+
 ### BUG FIXES
 
 - [ibc-relayer]
@@ -24,6 +25,7 @@
 [#811]: https://github.com/informalsystems/ibc-rs/issues/811
 [#854]: https://github.com/informalsystems/ibc-rs/issues/854
 [#851]: https://github.com/informalsystems/ibc-rs/issues/851
+[#855]: https://github.com/informalsystems/ibc-rs/issues/855
 
 
 ## v0.2.0

modules/src/application/ics20_fungible_token_transfer/error.rs

@@ -13,16 +13,19 @@ pub enum Kind {
     #[error("error raised by message handler")]
     HandlerRaisedError,
 
-    #[error("Sending sequence number not found for port {0} and channel {1}")]
+    #[error("sending sequence number not found for port {0} and channel {1}")]
     SequenceSendNotFound(PortId, ChannelId),
 
-    #[error("Missing channel for port_id {0} and channel_id {1} ")]
+    #[error("missing channel for port_id {0} and channel_id {1} ")]
     ChannelNotFound(PortId, ChannelId),
 
     #[error(
-        "Destination channel not found in the counterparty of port_id {0} and channel_id {1} "
+        "destination channel not found in the counterparty of port_id {0} and channel_id {1} "
     )]
     DestinationChannelNotFound(PortId, ChannelId),
+
+    #[error("invalid packet timeout timestamp value")]
+    InvalidPacketTimestamp(u64),
 }
 
 impl Kind {

modules/src/application/ics20_fungible_token_transfer/relay_application_logic/send_transfer.rs

@@ -5,6 +5,7 @@ use crate::handler::HandlerOutput;
 use crate::ics04_channel::handler::send_packet::send_packet;
 use crate::ics04_channel::packet::Packet;
 use crate::ics04_channel::packet::PacketResult;
+use crate::timestamp::Timestamp;
 
 pub(crate) fn send_transfer<Ctx>(
     ctx: &Ctx,
@@ -34,6 +35,9 @@ where
             Kind::SequenceSendNotFound(msg.source_port.clone(), msg.source_channel.clone())
         })?;
 
+    let timeout_timestamp = Timestamp::from_nanoseconds(msg.timeout_timestamp)
+        .map_err(|_| Kind::InvalidPacketTimestamp(msg.timeout_timestamp))?;
+
     //TODO: Application LOGIC.
 
     let packet = Packet {
@@ -44,7 +48,7 @@ where
         destination_channel: destination_channel.clone(),
         data: vec![0],
         timeout_height: msg.timeout_height,
-        timeout_timestamp: msg.timeout_timestamp,
+        timeout_timestamp,
     };
 
     let handler_output =

modules/src/ics02_client/client_consensus.rs

@@ -17,6 +17,7 @@ use crate::ics23_commitment::commitment::CommitmentRoot;
 use crate::ics24_host::identifier::ClientId;
 #[cfg(any(test, feature = "mocks"))]
 use crate::mock::client_state::MockConsensusState;
+use crate::timestamp::Timestamp;
 
 pub const TENDERMINT_CONSENSUS_STATE_TYPE_URL: &str =
     "/ibc.lightclients.tendermint.v1.ConsensusState";
@@ -48,17 +49,15 @@ pub enum AnyConsensusState {
 }
 
 impl AnyConsensusState {
-    pub fn timestamp(&self) -> Result<u64, Kind> {
+    pub fn timestamp(&self) -> Timestamp {
         match self {
             Self::Tendermint(cs_state) => {
                 let date: DateTime<Utc> = cs_state.timestamp.into();
-                let value = date.timestamp();
-                u64::try_from(value)
-                    .map_err(|_| Kind::NegativeConsensusStateTimestamp(value.to_string()))
+                Timestamp::from_datetime(date)
             }
 
             #[cfg(any(test, feature = "mocks"))]
-            Self::Mock(mock_state) => Ok(mock_state.timestamp()),
+            Self::Mock(mock_state) => mock_state.timestamp(),
         }
     }
 

modules/src/ics02_client/error.rs

@@ -32,9 +32,6 @@ pub enum Kind {
     #[error("implementation specific")]
     ImplementationSpecific,
 
-    #[error("Negative timestamp in consensus state {0}; timestamp must be a positive value")]
-    NegativeConsensusStateTimestamp(String),
-
     #[error("header verification failed")]
     HeaderVerificationFailure,
 
@@ -83,6 +80,9 @@ pub enum Kind {
     #[error("invalid proof for the upgraded consensus state")]
     InvalidUpgradeConsensusStateProof(Ics23Error),
 
+    #[error("invalid packet timeout timestamp value")]
+    InvalidPacketTimestamp,
+
     #[error("mismatch between client and arguments types, expected: {0:?}")]
     ClientArgsTypeMismatch(ClientType),
 

modules/src/ics04_channel/context.rs

@@ -10,6 +10,7 @@ use crate::ics04_channel::handler::{ChannelIdState, ChannelResult};
 use crate::ics04_channel::{error::Error, packet::Receipt};
 use crate::ics05_port::capabilities::Capability;
 use crate::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId};
+use crate::timestamp::Timestamp;
 use crate::Height;
 
 use super::packet::{PacketResult, Sequence};
@@ -48,14 +49,14 @@ pub trait ChannelReader {
 
     fn get_packet_acknowledgement(&self, key: &(PortId, ChannelId, Sequence)) -> Option<String>;
 
-    /// A hashing function for packet commitments  
+    /// A hashing function for packet commitments
     fn hash(&self, value: String) -> String;
 
     /// Returns the current height of the local chain.
     fn host_height(&self) -> Height;
 
     /// Returns the current timestamp of the local chain.
-    fn host_timestamp(&self) -> u64;
+    fn host_timestamp(&self) -> Timestamp;
 
     /// Returns a counter on the number of channel ids have been created thus far.
     /// The value of this counter should increase only via method
@@ -172,7 +173,7 @@ pub trait ChannelKeeper {
     fn store_packet_commitment(
         &mut self,
         key: (PortId, ChannelId, Sequence),
-        timestamp: u64,
+        timestamp: Timestamp,
         heigh: Height,
         data: Vec<u8>,
     ) -> Result<(), Error>;

modules/src/ics04_channel/error.rs

@@ -6,6 +6,7 @@ pub type Error = anomaly::Error<Kind>;
 use super::packet::Sequence;
 use crate::ics04_channel::channel::State;
 use crate::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId};
+use crate::timestamp::Timestamp;
 use crate::{ics02_client, Height};
 
 #[derive(Clone, Debug, Error, Eq, PartialEq)]
@@ -126,11 +127,14 @@ pub enum Kind {
     PacketTimeoutHeightNotReached(Height, Height),
 
     #[error("Packet timeout timestamp {0} > chain timestamp {1}")]
-    PacketTimeoutTimestampNotReached(u64, u64),
+    PacketTimeoutTimestampNotReached(Timestamp, Timestamp),
 
     #[error("Receiving chain block timestamp >= packet timeout timestamp")]
     LowPacketTimestamp,
 
+    #[error("Invalid packet timeout timestamp value")]
+    InvalidPacketTimestamp,
+
     #[error("Invalid timestamp in consensus state; timestamp must be a positive value")]
     ErrorInvalidConsensusState(ics02_client::error::Kind),
 

modules/src/ics04_channel/handler/recv_packet.rs

@@ -10,6 +10,7 @@ use crate::ics04_channel::handler::verify::verify_packet_recv_proofs;
 use crate::ics04_channel::msgs::recv_packet::MsgRecvPacket;
 use crate::ics04_channel::packet::{PacketResult, Receipt, Sequence};
 use crate::ics24_host::identifier::{ChannelId, PortId};
+use crate::timestamp::Expiry;
 
 #[derive(Clone, Debug)]
 pub struct RecvPacketResult {
@@ -78,7 +79,7 @@ pub fn process(ctx: &dyn ChannelReader, msg: MsgRecvPacket) -> HandlerResult<Pac
 
     // Check if packet timestamp is newer than the local host chain timestamp
     let latest_timestamp = ctx.host_timestamp();
-    if (packet.timeout_timestamp != 0) && (packet.timeout_timestamp <= latest_timestamp) {
+    if let Expiry::Expired = latest_timestamp.check_expiry(&packet.timeout_timestamp) {
         return Err(Kind::LowPacketTimestamp.into());
     }
 
@@ -149,6 +150,7 @@ mod tests {
     use crate::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId};
     use crate::mock::context::MockContext;
     use crate::test_utils::get_dummy_account_id;
+    use crate::timestamp::Timestamp;
     use crate::{events::IbcEvent, ics04_channel::packet::Packet};
 
     #[test]
@@ -180,7 +182,7 @@ mod tests {
             destination_channel: ChannelId::default(),
             data: vec![],
             timeout_height: client_height,
-            timeout_timestamp: 1,
+            timeout_timestamp: Timestamp::from_nanoseconds(1).unwrap(),
         };
 
         let msg_packet_old =
@@ -245,7 +247,7 @@ mod tests {
                         1.into(),
                     )
                     .with_height(host_height)
-                    .with_timestamp(1)
+                    .with_timestamp(Timestamp::from_nanoseconds(1).unwrap())
                     // This `with_recv_sequence` is required for ordered channels
                     .with_recv_sequence(
                         packet.destination_port.clone(),
@@ -264,7 +266,7 @@ mod tests {
                     .with_channel(PortId::default(), ChannelId::default(), dest_channel_end)
                     .with_send_sequence(PortId::default(), ChannelId::default(), 1.into())
                     .with_height(host_height)
-                    .with_timestamp(3),
+                    .with_timestamp(Timestamp::from_nanoseconds(3).unwrap()),
                 msg: msg_packet_old,
                 want_pass: false,
             },

modules/src/ics04_channel/handler/send_packet.rs

@@ -7,6 +7,7 @@ use crate::ics04_channel::events::SendPacket;
 use crate::ics04_channel::packet::{PacketResult, Sequence};
 use crate::ics04_channel::{context::ChannelReader, error::Error, error::Kind, packet::Packet};
 use crate::ics24_host::identifier::{ChannelId, PortId};
+use crate::timestamp::{Expiry, Timestamp};
 use crate::Height;
 
 #[derive(Clone, Debug)]
@@ -16,7 +17,7 @@ pub struct SendPacketResult {
     pub seq: Sequence,
     pub seq_number: Sequence,
     pub timeout_height: Height,
-    pub timeout_timestamp: u64,
+    pub timeout_timestamp: Timestamp,
     pub data: Vec<u8>,
 }
 
@@ -77,12 +78,10 @@ pub fn send_packet(ctx: &dyn ChannelReader, packet: Packet) -> HandlerResult<Pac
         .client_consensus_state(&client_id, latest_height)
         .ok_or_else(|| Kind::MissingClientConsensusState(client_id.clone(), latest_height))?;
 
-    let latest_timestamp = consensus_state
-        .timestamp()
-        .map_err(Kind::ErrorInvalidConsensusState)?;
+    let latest_timestamp = consensus_state.timestamp();
 
     let packet_timestamp = packet.timeout_timestamp;
-    if packet.timeout_timestamp != 0 && packet_timestamp <= latest_timestamp {
+    if let Expiry::Expired = latest_timestamp.check_expiry(&packet_timestamp) {
         return Err(Kind::LowPacketTimestamp.into());
     }
 

modules/src/ics04_channel/handler/timeout.rs

@@ -10,6 +10,7 @@ use crate::ics04_channel::msgs::timeout::MsgTimeout;
 use crate::ics04_channel::packet::{PacketResult, Sequence};
 use crate::ics04_channel::{context::ChannelReader, error::Error, error::Kind};
 use crate::ics24_host::identifier::{ChannelId, PortId};
+use crate::timestamp::Expiry;
 
 #[derive(Clone, Debug)]
 pub struct TimeoutPacketResult {
@@ -70,12 +71,10 @@ pub fn process(ctx: &dyn ChannelReader, msg: MsgTimeout) -> HandlerResult<Packet
         .client_consensus_state(&client_id, proof_height)
         .ok_or_else(|| Kind::MissingClientConsensusState(client_id.clone(), proof_height))?;
 
-    let proof_timestamp = consensus_state
-        .timestamp()
-        .map_err(Kind::ErrorInvalidConsensusState)?;
+    let proof_timestamp = consensus_state.timestamp();
 
     let packet_timestamp = packet.timeout_timestamp;
-    if packet.timeout_timestamp != 0 && packet_timestamp > proof_timestamp {
+    if let Expiry::Expired = packet_timestamp.check_expiry(&proof_timestamp) {
         return Err(
             Kind::PacketTimeoutTimestampNotReached(packet_timestamp, proof_timestamp).into(),
         );

modules/src/ics04_channel/packet.rs

@@ -6,6 +6,7 @@ use ibc_proto::ibc::core::channel::v1::Packet as RawPacket;
 
 use crate::ics04_channel::error::Kind;
 use crate::ics24_host::identifier::{ChannelId, PortId};
+use crate::timestamp::Timestamp;
 use crate::Height;
 
 use super::handler::{
@@ -91,7 +92,7 @@ pub struct Packet {
     #[serde(serialize_with = "crate::serializers::ser_hex_upper")]
     pub data: Vec<u8>,
     pub timeout_height: Height,
-    pub timeout_timestamp: u64,
+    pub timeout_timestamp: Timestamp,
 }
 
 impl std::fmt::Debug for Packet {
@@ -131,7 +132,7 @@ impl Default for Packet {
             destination_channel: Default::default(),
             data: vec![],
             timeout_height: Default::default(),
-            timeout_timestamp: 0,
+            timeout_timestamp: Default::default(),
         }
     }
 }
@@ -156,6 +157,9 @@ impl TryFrom<RawPacket> for Packet {
             return Err(Kind::ZeroPacketData.into());
         }
 
+        let timeout_timestamp = Timestamp::from_nanoseconds(raw_pkt.timeout_timestamp)
+            .map_err(|_| Kind::InvalidPacketTimestamp)?;
+
         Ok(Packet {
             sequence: Sequence::from(raw_pkt.sequence),
             source_port: raw_pkt
@@ -176,7 +180,7 @@ impl TryFrom<RawPacket> for Packet {
                 .map_err(|e| Kind::IdentifierError.context(e))?,
             data: raw_pkt.data,
             timeout_height: packet_timeout_height,
-            timeout_timestamp: raw_pkt.timeout_timestamp,
+            timeout_timestamp,
         })
     }
 }
@@ -191,7 +195,7 @@ impl From<Packet> for RawPacket {
             destination_channel: packet.destination_channel.to_string(),
             data: packet.data,
             timeout_height: Some(packet.timeout_height.into()),
-            timeout_timestamp: packet.timeout_timestamp,
+            timeout_timestamp: packet.timeout_timestamp.as_nanoseconds(),
         }
     }
 }

modules/src/ics26_routing/handler.rs

@@ -274,6 +274,7 @@ mod tests {
     use crate::mock::context::MockContext;
     use crate::mock::header::MockHeader;
     use crate::test_utils::get_dummy_account_id;
+    use crate::timestamp::Timestamp;
     use crate::Height;
 
     #[test]
@@ -361,7 +362,8 @@ mod tests {
             MsgTimeoutOnClose::try_from(get_dummy_raw_msg_timeout_on_close(36, 5)).unwrap();
         msg_to_on_close.packet.sequence = 2.into();
         msg_to_on_close.packet.timeout_height = msg_transfer_two.timeout_height;
-        msg_to_on_close.packet.timeout_timestamp = msg_transfer_two.timeout_timestamp;
+        msg_to_on_close.packet.timeout_timestamp =
+            Timestamp::from_nanoseconds(msg_transfer_two.timeout_timestamp).unwrap();
 
         let msg_recv_packet = MsgRecvPacket::try_from(get_dummy_raw_msg_recv_packet(35)).unwrap();
 

modules/src/lib.rs

@@ -33,6 +33,7 @@ pub mod macros;
 pub mod proofs;
 pub mod query;
 pub mod signer;
+pub mod timestamp;
 pub mod tx_msg;
 
 pub mod ics02_client;

modules/src/mock/client_state.rs

@@ -15,6 +15,7 @@ use crate::ics02_client::error::Kind as ClientKind;
 use crate::ics23_commitment::commitment::CommitmentRoot;
 use crate::ics24_host::identifier::ChainId;
 use crate::mock::header::MockHeader;
+use crate::timestamp::Timestamp;
 use crate::Height;
 
 /// A mock of an IBC client record as it is stored in a mock context.
@@ -64,7 +65,7 @@ impl From<MockClientState> for RawMockClientState {
         RawMockClientState {
             header: Some(ibc_proto::ibc::mock::Header {
                 height: Some(value.0.height().into()),
-                timestamp: (value.0).timestamp,
+                timestamp: (value.0).timestamp.as_nanoseconds(),
             }),
         }
     }
@@ -103,7 +104,7 @@ impl From<MockConsensusState> for MockClientState {
 pub struct MockConsensusState(pub MockHeader);
 
 impl MockConsensusState {
-    pub fn timestamp(&self) -> u64 {
+    pub fn timestamp(&self) -> Timestamp {
         (self.0).timestamp
     }
 }
@@ -127,7 +128,7 @@ impl From<MockConsensusState> for RawMockConsensusState {
         RawMockConsensusState {
             header: Some(ibc_proto::ibc::mock::Header {
                 height: Some(value.0.height().into()),
-                timestamp: (value.0).timestamp,
+                timestamp: (value.0).timestamp.as_nanoseconds(),
             }),
         }
     }