Use a Timestamp and Duration newtype throughout the stack (#4368)

no more confusion about whether something is nanos or secs

Author: benluelo

Cargo.lock

@@ -1,7 +1,7 @@
 use cosmwasm_schema::cw_serde;
 use ethabi::{ParamType, Token};
 use ibc_union_msg::msg::MsgSendPacket;
-use ibc_union_spec::{ChannelId, Timestamp};
+use ibc_union_spec::{ChannelId, Duration, Timestamp};
 
 use crate::{state::Config, ContractError};
 
@@ -48,7 +48,8 @@ impl UCS00PingPong {
             source_channel_id: source_channel,
             timeout_height: 0,
             timeout_timestamp: current_timestamp
-                + Timestamp::from_secs(config.seconds_before_timeout),
+                .plus_duration(Duration::from_secs(config.seconds_before_timeout))
+                .expect("timeout overflow"),
             data: counterparty_packet.encode().into(),
         })
     }

cosmwasm/ibc-union/app/ucs00-pingpong/src/msg.rs

@@ -11,7 +11,7 @@ use ibc_union_msg::lightclient::{
     MisbehaviourResponse, QueryMsg, Status, StorageWrites, UpdateStateResponse,
     VerifyCreationResponse, VerifyCreationResponseEvent,
 };
-use ibc_union_spec::ClientId;
+use ibc_union_spec::{ClientId, Timestamp};
 use unionlabs::{
     encoding::{Decode, DecodeAs, DecodeErrorOf, Encode, EncodeAs, Encoding, EthAbi},
     primitives::Bytes,
@@ -23,7 +23,8 @@ use crate::{msg::InitMsg, state::IbcHost};
 pub mod msg;
 pub mod state;
 
-// TODO: Add #[source] to all variants
+pub use ibc_union_spec as spec;
+
 #[derive(macros::Debug, thiserror::Error)]
 #[debug(bound())]
 pub enum DecodeError<T: IbcClient> {
@@ -275,7 +276,7 @@ pub trait IbcClient: Sized + 'static {
     ) -> Result<(), IbcClientError<Self>>;
 
     /// Get the timestamp
-    fn get_timestamp(consensus_state: &Self::ConsensusState) -> u64;
+    fn get_timestamp(consensus_state: &Self::ConsensusState) -> Timestamp;
 
     /// Get the height
     fn get_latest_height(client_state: &Self::ClientState) -> u64;

cosmwasm/ibc-union/core/light-client-interface/src/lib.rs

@@ -1,3 +1,5 @@
+// TODO: Move this into ibc_union_msg, and then reexport
+
 use cosmwasm_std::Addr;
 
 #[derive(

cosmwasm/ibc-union/core/light-client-interface/src/msg.rs

@@ -3,7 +3,7 @@ use cosmwasm_std::{Addr, Empty};
 use ethereum_light_client::client::EthereumLightClient;
 use ethereum_light_client_types::StorageProof;
 use ibc_union_light_client::{
-    ClientCreationResult, IbcClient, IbcClientCtx, IbcClientError, StateUpdate,
+    spec::Timestamp, ClientCreationResult, IbcClient, IbcClientCtx, IbcClientError, StateUpdate,
 };
 use ibc_union_msg::lightclient::Status;
 use unionlabs::encoding::Bincode;
@@ -86,7 +86,7 @@ impl IbcClient for ArbitrumLightClient {
             state_root: header.l2_header.state_root,
             ibc_storage_root: header.l2_ibc_account_proof.storage_root,
             // must be nanos
-            timestamp: 1_000_000_000 * header.l2_header.timestamp,
+            timestamp: Timestamp::from_secs(header.l2_header.timestamp),
         };
 
         let new_latest_height = header
@@ -136,7 +136,7 @@ impl IbcClient for ArbitrumLightClient {
         Ok(ClientCreationResult::new())
     }
 
-    fn get_timestamp(consensus_state: &Self::ConsensusState) -> u64 {
+    fn get_timestamp(consensus_state: &Self::ConsensusState) -> Timestamp {
         consensus_state.timestamp
     }
 

cosmwasm/ibc-union/lightclient/arbitrum/src/client.rs

@@ -3,7 +3,7 @@ use berachain_light_client_types::{ClientState, ConsensusState, Header};
 use cosmwasm_std::{Addr, Empty};
 use ethereum_light_client_types::StorageProof;
 use ibc_union_light_client::{
-    ClientCreationResult, IbcClient, IbcClientCtx, IbcClientError, StateUpdate,
+    spec::Timestamp, ClientCreationResult, IbcClient, IbcClientCtx, IbcClientError, StateUpdate,
 };
 use ibc_union_msg::lightclient::Status;
 use tendermint_light_client::client::TendermintLightClient;
@@ -68,7 +68,7 @@ impl IbcClient for BerachainLightClient {
         Ok(())
     }
 
-    fn get_timestamp(consensus_state: &Self::ConsensusState) -> u64 {
+    fn get_timestamp(consensus_state: &Self::ConsensusState) -> Timestamp {
         consensus_state.timestamp
     }
 
@@ -146,7 +146,7 @@ impl IbcClient for BerachainLightClient {
         let update_height = header.execution_header.block_number;
 
         let consensus_state = ConsensusState {
-            timestamp: header.execution_header.timestamp,
+            timestamp: Timestamp::from_secs(header.execution_header.timestamp),
             state_root: header.execution_header.state_root,
             storage_root: header.account_proof.storage_root,
         };

cosmwasm/ibc-union/lightclient/berachain/src/client.rs

@@ -4,7 +4,7 @@ use cosmwasm_std::{Addr, Empty};
 use ethereum_light_client::client::EthereumLightClient;
 use ethereum_light_client_types::StorageProof;
 use ibc_union_light_client::{
-    ClientCreationResult, IbcClient, IbcClientCtx, IbcClientError, StateUpdate,
+    spec::Timestamp, ClientCreationResult, IbcClient, IbcClientCtx, IbcClientError, StateUpdate,
 };
 use ibc_union_msg::lightclient::Status;
 use unionlabs::encoding::Bincode;
@@ -64,7 +64,7 @@ impl IbcClient for BobLightClient {
         Ok(())
     }
 
-    fn get_timestamp(consensus_state: &Self::ConsensusState) -> u64 {
+    fn get_timestamp(consensus_state: &Self::ConsensusState) -> Timestamp {
         consensus_state.timestamp
     }
 
@@ -121,7 +121,7 @@ impl IbcClient for BobLightClient {
         let update_height = header.l2_header.number.try_into().expect("impossible");
 
         let consensus_state = ConsensusState {
-            timestamp: 1_000_000_000 * header.l2_header.timestamp,
+            timestamp: Timestamp::from_secs(header.l2_header.timestamp),
             state_root: header.l2_header.state_root,
             ibc_storage_root: header.l2_ibc_account_proof.storage_root,
         };

cosmwasm/ibc-union/lightclient/bob/src/client.rs

@@ -9,7 +9,7 @@ use ibc_union_light_client::{
     ClientCreationResult, IbcClient, IbcClientCtx, IbcClientError, StateUpdate,
 };
 use ibc_union_msg::lightclient::Status;
-use ibc_union_spec::path::IBC_UNION_COSMWASM_COMMITMENT_PREFIX;
+use ibc_union_spec::{path::IBC_UNION_COSMWASM_COMMITMENT_PREFIX, Duration, Timestamp};
 use ics23::ibc_api::SDK_SPECS;
 use unionlabs::{
     encoding::Bincode,
@@ -98,7 +98,7 @@ impl<T: ZkpVerifier> IbcClient for CometblsLightClient<T> {
         .map_err(Into::<Error>::into)?)
     }
 
-    fn get_timestamp(consensus_state: &Self::ConsensusState) -> u64 {
+    fn get_timestamp(consensus_state: &Self::ConsensusState) -> Timestamp {
         consensus_state.timestamp
     }
 
@@ -123,7 +123,7 @@ impl<T: ZkpVerifier> IbcClient for CometblsLightClient<T> {
             if is_client_expired(
                 consensus_state.timestamp,
                 client_state.trusting_period,
-                ctx.env.block.time.nanos(),
+                Timestamp::from_nanos(ctx.env.block.time.nanos()),
             ) {
                 return Status::Expired;
             }
@@ -203,7 +203,7 @@ fn verify_header<T: ZkpVerifier>(
 
     let trusted_timestamp = consensus_state.timestamp;
     // Normalized to nanoseconds to follow tendermint convention
-    let untrusted_timestamp = header.signed_header.time.as_unix_nanos();
+    let untrusted_timestamp = Timestamp::from_nanos(header.signed_header.time.as_unix_nanos());
 
     if untrusted_timestamp <= trusted_timestamp {
         return Err(InvalidHeaderError::SignedHeaderTimestampMustBeMoreRecent {
@@ -216,23 +216,19 @@ fn verify_header<T: ZkpVerifier>(
     if is_client_expired(
         trusted_timestamp,
         client_state.trusting_period,
-        ctx.env.block.time.nanos(),
+        Timestamp::from_nanos(ctx.env.block.time.nanos()),
     ) {
         return Err(InvalidHeaderError::HeaderExpired(consensus_state.timestamp).into());
     }
 
-    let max_clock_drift = ctx
-        .env
-        .block
-        .time
-        .nanos()
-        .checked_add(client_state.max_clock_drift)
+    let max_clock_drift_timestamp = Timestamp::from_nanos(ctx.env.block.time.nanos())
+        .plus_duration(client_state.max_clock_drift)
         .ok_or(Error::MathOverflow)?;
 
-    if untrusted_timestamp >= max_clock_drift {
+    if untrusted_timestamp >= max_clock_drift_timestamp {
         return Err(InvalidHeaderError::SignedHeaderCannotExceedMaxClockDrift {
             signed_timestamp: untrusted_timestamp,
-            max_clock_drift,
+            max_clock_drift_timestamp,
         }
         .into());
     }
@@ -301,7 +297,7 @@ fn update_state<T: ZkpVerifier>(
 
     consensus_state.next_validators_hash = header.signed_header.next_validators_hash;
     // Normalized to nanoseconds to follow tendermint convention
-    consensus_state.timestamp = header.signed_header.time.as_unix_nanos();
+    consensus_state.timestamp = Timestamp::from_nanos(header.signed_header.time.as_unix_nanos());
 
     let state_update = StateUpdate::new(untrusted_height.height(), consensus_state);
 
@@ -314,11 +310,11 @@ fn update_state<T: ZkpVerifier>(
 }
 
 fn is_client_expired(
-    consensus_state_timestamp: u64,
-    trusting_period: u64,
-    current_block_time: u64,
+    consensus_state_timestamp: Timestamp,
+    trusting_period: Duration,
+    current_block_time: Timestamp,
 ) -> bool {
-    if let Some(sum) = consensus_state_timestamp.checked_add(trusting_period) {
+    if let Some(sum) = consensus_state_timestamp.plus_duration(trusting_period) {
         sum < current_block_time
     } else {
         true

cosmwasm/ibc-union/lightclient/cometbls/src/client.rs

@@ -1,5 +1,6 @@
 use cosmwasm_std::StdError;
 use ibc_union_light_client::IbcClientError;
+use ibc_union_spec::Timestamp;
 use unionlabs::{
     ibc::core::client::height::Height,
     primitives::{encoding::HexUnprefixed, H256},
@@ -9,24 +10,36 @@ use crate::{client::CometblsLightClient, zkp_verifier::ZkpVerifier};
 
 #[derive(thiserror::Error, Debug, Clone, PartialEq)]
 pub enum InvalidHeaderError {
-    #[error("signed header's height ({signed_height}) must be greater than trusted height ({trusted_height})")]
+    #[error(
+        "signed header's height ({signed_height}) must be \
+        greater than trusted height ({trusted_height})"
+    )]
     SignedHeaderHeightMustBeMoreRecent {
         signed_height: u64,
         trusted_height: u64,
     },
-    #[error("signed header's timestamp ({signed_timestamp}) must be greater than trusted timestamp ({trusted_timestamp})")]
+    #[error(
+        "signed header's timestamp ({signed_timestamp}) must be \
+        greater than trusted timestamp ({trusted_timestamp})"
+    )]
     SignedHeaderTimestampMustBeMoreRecent {
-        signed_timestamp: u64,
-        trusted_timestamp: u64,
+        signed_timestamp: Timestamp,
+        trusted_timestamp: Timestamp,
     },
     #[error("header with timestamp ({0}) is expired")]
-    HeaderExpired(u64),
-    #[error("signed header timestamp ({signed_timestamp}) cannot exceed the max clock drift ({max_clock_drift})")]
+    HeaderExpired(Timestamp),
+    #[error(
+        "signed header timestamp ({signed_timestamp}) cannot \
+        exceed the max clock drift ({max_clock_drift_timestamp})"
+    )]
     SignedHeaderCannotExceedMaxClockDrift {
-        signed_timestamp: u64,
-        max_clock_drift: u64,
+        signed_timestamp: Timestamp,
+        max_clock_drift_timestamp: Timestamp,
     },
-    #[error("the validators hash ({actual}) doesn't match the trusted validators hash ({expected}) for an adjacent block")]
+    #[error(
+        "the validators hash ({actual}) doesn't match the trusted \
+        validators hash ({expected}) for an adjacent block"
+    )]
     InvalidValidatorsHash {
         expected: H256<HexUnprefixed>,
         actual: H256<HexUnprefixed>,