Use chrono::DateTime instead of std::time::SystemTime (#994)

* Use chrono::DateTime instead of std::time::SystemTime * Disable "chrono/clock" feature as it requires std support
informalsystems · Oct 6, 2021 · aee72bd · aee72bd
1 parent 7239848
commit aee72bd
Show file tree

Hide file tree

Showing 24 changed files with 149 additions and 94 deletions.
diff --git a/light-client/Cargo.toml b/light-client/Cargo.toml
@@ -44,6 +44,7 @@ mbt = []
 tendermint = { version = "0.22.0", path = "../tendermint" }
 tendermint-rpc = { version = "0.22.0", path = "../rpc", default-features = false }
 
+chrono = { version = "0.4", default-features = false, features = ["clock"] }
 contracts = "0.4.0"
 crossbeam-channel = "0.4.2"
 derive_more = "0.99.5"

diff --git a/light-client/src/components/clock.rs b/light-client/src/components/clock.rs
@@ -1,6 +1,7 @@
 //! Provides an interface and a default implementation of the `Clock` component
 
 use crate::types::Time;
+use chrono::Utc;
 
 /// Abstracts over the current time.
 pub trait Clock: Send + Sync {
@@ -13,6 +14,6 @@ pub trait Clock: Send + Sync {
 pub struct SystemClock;
 impl Clock for SystemClock {
     fn now(&self) -> Time {
-        Time::now()
+        Time(Utc::now())
     }
 }
diff --git a/light-client/src/components/verifier.rs b/light-client/src/components/verifier.rs
@@ -128,14 +128,12 @@ where
     ///
     /// - Ensure the latest trusted header hasn't expired
     /// - Ensure the header validator hashes match the given validators
-    /// - Ensure the header next validator hashes match the given next
-    ///   validators
+    /// - Ensure the header next validator hashes match the given next validators
     /// - Additional implementation specific validation via `commit_validator`
     /// - Check that the untrusted block is more recent than the trusted state
-    /// - If the untrusted block is the very next block after the trusted block,
-    ///   check that their (next) validator sets hashes match.
-    /// - Otherwise, ensure that the untrusted block has a greater height than
-    ///   the trusted block.
+    /// - If the untrusted block is the very next block after the trusted block, check that their
+    ///   (next) validator sets hashes match.
+    /// - Otherwise, ensure that the untrusted block has a greater height than the trusted block.
     ///
     /// **NOTE**: If the untrusted state's `next_validators` field is `None`,
     /// this will not (and will not be able to) check whether the untrusted

diff --git a/light-client/src/contracts.rs b/light-client/src/contracts.rs
@@ -25,7 +25,10 @@ pub fn is_within_trust_period(
     now: Time,
 ) -> bool {
     let header_time = light_block.signed_header.header.time;
-    header_time > now - trusting_period
+    match now - trusting_period {
+        Ok(start) => header_time > start,
+        Err(_) => false,
+    }
 }
 
 /// Whether or not the given light store contains a trusted block

diff --git a/light-client/src/predicates.rs b/light-client/src/predicates.rs
@@ -99,7 +99,8 @@ pub trait VerificationPredicates: Send + Sync {
         trusting_period: Duration,
         now: Time,
     ) -> Result<(), VerificationError> {
-        let expires_at = trusted_header_time + trusting_period;
+        let expires_at =
+            (trusted_header_time + trusting_period).map_err(VerificationError::tendermint)?;
 
         if expires_at > now {
             Ok(())
@@ -115,7 +116,9 @@ pub trait VerificationPredicates: Send + Sync {
         clock_drift: Duration,
         now: Time,
     ) -> Result<(), VerificationError> {
-        if untrusted_header_time < now + clock_drift {
+        let drifted = (now + clock_drift).map_err(VerificationError::tendermint)?;
+
+        if untrusted_header_time < drifted {
             Ok(())
         } else {
             Err(VerificationError::header_from_the_future(
@@ -202,6 +205,7 @@ pub trait VerificationPredicates: Send + Sync {
 
 #[cfg(test)]
 mod tests {
+    use chrono::Utc;
     use std::ops::Sub;
     use std::time::Duration;
     use tendermint::Time;
@@ -290,7 +294,7 @@ mod tests {
 
         // 1. ensure valid header verifies
         let mut trusting_period = Duration::new(1000, 0);
-        let now = Time::now();
+        let now = Time(Utc::now());
 
         let result_ok = vp.is_within_trust_period(header.time, trusting_period, now);
         assert!(result_ok.is_ok());
@@ -300,7 +304,7 @@ mod tests {
 
         let result_err = vp.is_within_trust_period(header.time, trusting_period, now);
 
-        let expires_at = header.time + trusting_period;
+        let expires_at = (header.time + trusting_period).unwrap();
         match result_err {
             Err(VerificationError(VerificationErrorDetail::NotWithinTrustPeriod(e), _)) => {
                 assert_eq!(e.expires_at, expires_at);
@@ -319,12 +323,12 @@ mod tests {
         let one_second = Duration::new(1, 0);
 
         // 1. ensure valid header verifies
-        let result_ok = vp.is_header_from_past(header.time, one_second, Time::now());
+        let result_ok = vp.is_header_from_past(header.time, one_second, Time(Utc::now()));
 
         assert!(result_ok.is_ok());
 
         // 2. ensure it fails if header is from a future time
-        let now = Time::now().sub(one_second * 15);
+        let now = Time(Utc::now()).sub(one_second * 15).unwrap();
         let result_err = vp.is_header_from_past(header.time, one_second, now);
 
         match result_err {

diff --git a/light-client/src/predicates/errors.rs b/light-client/src/predicates/errors.rs
@@ -8,10 +8,15 @@ use crate::errors::ErrorExt;
 use crate::operations::voting_power::VotingPowerTally;
 use crate::types::{Hash, Height, Time, Validator, ValidatorAddress};
 use tendermint::account::Id;
+use tendermint::Error as TendermintError;
 
 define_error! {
     #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
     VerificationError {
+        Tendermint
+            [ TendermintError ]
+            | _ | { "tendermint error" },
+
         HeaderFromTheFuture
             {
                 header_time: Time,

diff --git a/light-client/src/supervisor.rs b/light-client/src/supervisor.rs
@@ -592,7 +592,11 @@ mod tests {
 
         let witness = change_provider(primary.clone(), None);
 
-        let peer_list = make_peer_list(Some(primary.clone()), Some(vec![witness]), get_time(11));
+        let peer_list = make_peer_list(
+            Some(primary.clone()),
+            Some(vec![witness]),
+            get_time(11).unwrap(),
+        );
 
         let (result, _) = run_bisection_test(peer_list, 10);
 
@@ -611,7 +615,7 @@ mod tests {
             .map(|lb| lb.generate().unwrap().into())
             .collect::<Vec<LightBlock>>();
 
-        let peer_list = make_peer_list(Some(primary), None, get_time(11));
+        let peer_list = make_peer_list(Some(primary), None, get_time(11).unwrap());
 
         let (result, _) = run_bisection_test(peer_list, 10);
 
@@ -634,7 +638,7 @@ mod tests {
         light_blocks.truncate(9);
         let witness = change_provider(light_blocks, None);
 
-        let peer_list = make_peer_list(Some(primary), Some(vec![witness]), get_time(11));
+        let peer_list = make_peer_list(Some(primary), Some(vec![witness]), get_time(11).unwrap());
 
         let (result, _) = run_bisection_test(peer_list, 10);
 
@@ -663,7 +667,7 @@ mod tests {
 
         let witness = make_conflicting_witness(5, None, None, None);
 
-        let peer_list = make_peer_list(Some(primary), Some(vec![witness]), get_time(11));
+        let peer_list = make_peer_list(Some(primary), Some(vec![witness]), get_time(11).unwrap());
 
         let (result, _) = run_bisection_test(peer_list, 10);
 
@@ -714,7 +718,7 @@ mod tests {
             None,
         );
 
-        let peer_list = make_peer_list(Some(primary), Some(vec![witness]), get_time(11));
+        let peer_list = make_peer_list(Some(primary), Some(vec![witness]), get_time(11).unwrap());
 
         let (result, _) = run_bisection_test(peer_list, 5);
 
@@ -742,7 +746,7 @@ mod tests {
         let mut peer_list = make_peer_list(
             Some(primary.clone()),
             Some(vec![witness1.clone(), witness2]),
-            get_time(11),
+            get_time(11).unwrap(),
         );
         peer_list
             .get_mut(&primary[0].provider)
@@ -783,8 +787,11 @@ mod tests {
 
         let witness = change_provider(primary.clone(), None);
 
-        let peer_list =
-            make_peer_list(Some(primary.clone()), Some(vec![witness]), get_time(604801));
+        let peer_list = make_peer_list(
+            Some(primary.clone()),
+            Some(vec![witness]),
+            get_time(604801).unwrap(),
+        );
 
         let (_, latest_status) = run_bisection_test(peer_list, 2);
 
@@ -812,7 +819,11 @@ mod tests {
 
         let witness = change_provider(primary.clone(), None);
 
-        let peer_list = make_peer_list(Some(primary.clone()), Some(vec![witness]), get_time(11));
+        let peer_list = make_peer_list(
+            Some(primary.clone()),
+            Some(vec![witness]),
+            get_time(11).unwrap(),
+        );
 
         let (_, latest_status) = run_bisection_test(peer_list, 10);
 

diff --git a/light-client/tests/backward.rs b/light-client/tests/backward.rs
@@ -48,7 +48,7 @@ fn make(chain: LightChain, trusted_height: Height) -> (LightClient, State) {
 
     let clock = MockClock {
         /// Set the current time to be ahead of the latest block in the chain
-        now: tendermint_testgen::helpers::get_time(chain.light_blocks.len() as u64 + 1),
+        now: tendermint_testgen::helpers::get_time(chain.light_blocks.len() as u64 + 1).unwrap(),
     };
 
     let options = Options {

diff --git a/light-client/tests/model_based.rs b/light-client/tests/model_based.rs
@@ -1,5 +1,6 @@
 #[cfg(feature = "mbt")]
 mod mbt {
+    use chrono::Utc;
     use rand::Rng;
     use serde::de::DeserializeOwned;
     use serde::{Deserialize, Serialize};
@@ -125,8 +126,8 @@ mod mbt {
     impl SingleStepTestFuzzer for HeaderVersionFuzzer {
         // TODO: rehash the header and re-compute commit with it
         // TODO: Unlike in tendermint-go, we don't assert for a particular version in rust
-        // TODO: Either add this check in verification or remove this test because otherwise there's no
-        // point of it
+        // TODO: Either add this check in verification or remove this test because otherwise there's
+        // no point of it
         fn fuzz_input(input: &mut BlockVerdict) -> (String, LiteVerdict) {
             let mut rng = rand::thread_rng();
 
@@ -179,13 +180,14 @@ mod mbt {
     impl SingleStepTestFuzzer for HeaderTimeFuzzer {
         fn fuzz_input(input: &mut BlockVerdict) -> (String, LiteVerdict) {
             let mut rng = rand::thread_rng();
-            let secs = tendermint::Time::now()
+            let secs = tendermint::Time(Utc::now())
                 .duration_since(tendermint::Time::unix_epoch())
                 .unwrap()
                 .as_secs();
             let rand_secs = rng.gen_range(1, secs);
-            input.block.signed_header.header.time =
-                tendermint::Time::unix_epoch() + std::time::Duration::from_secs(rand_secs);
+            input.block.signed_header.header.time = (tendermint::Time::unix_epoch()
+                + std::time::Duration::from_secs(rand_secs))
+            .unwrap();
             // TODO: the fuzzing below fails with one of:
             //   - 'overflow when adding duration to instant', src/libstd/time.rs:549:31
             //   - 'No such local time',

diff --git a/proto/Cargo.toml b/proto/Cargo.toml
@@ -25,7 +25,7 @@ subtle-encoding = "0.5"
 serde_bytes = "0.11"
 num-traits = "0.2"
 num-derive = "0.3"
-chrono = { version = "0.4", features = ["serde"] }
+chrono = { version = "0.4", default-features = false, features = ["serde", "alloc"] }
 flex-error = { version = "0.4.1", default-features = false }
 
 [dev-dependencies]

diff --git a/tendermint/src/block/commit_sig.rs b/tendermint/src/block/commit_sig.rs
@@ -100,7 +100,10 @@ impl TryFrom<RawCommitSig> for CommitSig {
                 return Err(Error::invalid_validator_address());
             }
 
-            let timestamp = value.timestamp.ok_or_else(Error::missing_timestamp)?.into();
+            let timestamp = value
+                .timestamp
+                .ok_or_else(Error::missing_timestamp)?
+                .try_into()?;
 
             return Ok(CommitSig::BlockIdFlagCommit {
                 validator_address: value.validator_address.try_into()?,
@@ -119,7 +122,10 @@ impl TryFrom<RawCommitSig> for CommitSig {
             }
             return Ok(CommitSig::BlockIdFlagNil {
                 validator_address: value.validator_address.try_into()?,
-                timestamp: value.timestamp.ok_or_else(Error::missing_timestamp)?.into(),
+                timestamp: value
+                    .timestamp
+                    .ok_or_else(Error::missing_timestamp)?
+                    .try_into()?,
                 signature: Signature::new(value.signature)?,
             });
         }

diff --git a/tendermint/src/block/header.rs b/tendermint/src/block/header.rs
@@ -112,7 +112,10 @@ impl TryFrom<RawHeader> for Header {
             version: value.version.ok_or_else(Error::missing_version)?.into(),
             chain_id: value.chain_id.try_into()?,
             height,
-            time: value.time.ok_or_else(Error::missing_timestamp)?.into(),
+            time: value
+                .time
+                .ok_or_else(Error::missing_timestamp)?
+                .try_into()?,
             last_block_id,
             last_commit_hash,
             data_hash: if value.data_hash.is_empty() {

diff --git a/tendermint/src/error.rs b/tendermint/src/error.rs
@@ -5,11 +5,11 @@ use crate::vote;
 use alloc::string::String;
 use core::num::TryFromIntError;
 use flex_error::{define_error, DisplayOnly};
+use serde::{Deserialize, Serialize};
 use std::io::Error as IoError;
-use time::OutOfRangeError;
 
 define_error! {
-    #[derive(Debug, Clone, PartialEq, Eq)]
+    #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
     Error {
         Crypto
             |_| { format_args!("cryptographic error") },
@@ -47,9 +47,11 @@ define_error! {
             { detail: String }
             |e| { format_args!("protocol error: {}", e.detail) },
 
-        OutOfRange
-            [ DisplayOnly<OutOfRangeError> ]
-            |_| { format_args!("value out of range") },
+        // When the oldtime feature is disabled, the chrono::oldtime::OutOfRangeError
+        // type is private and cannot be referred:
+        // https://github.com/chronotope/chrono/pull/541
+        DurationOutOfRange
+            |_| { format_args!("duration value out of range") },
 
         EmptySignature
             |_| { format_args!("empty signature") },
@@ -92,6 +94,13 @@ define_error! {
             [ DisplayOnly<TryFromIntError> ]
             |_| { format_args!("integer overflow") },
 
+        TimestampOverflow
+            [ DisplayOnly<TryFromIntError> ]
+            |_| { format_args!("timestamp overflow") },
+
+        TimestampConversion
+            |_| { format_args!("timestamp conversion error") },
+
         NoVoteFound
             |_| { format_args!("no vote found") },
 

diff --git a/tendermint/src/evidence.rs b/tendermint/src/evidence.rs
@@ -84,7 +84,10 @@ impl TryFrom<RawDuplicateVoteEvidence> for DuplicateVoteEvidence {
                 .try_into()?,
             total_voting_power: value.total_voting_power.try_into()?,
             validator_power: value.validator_power.try_into()?,
-            timestamp: value.timestamp.ok_or_else(Error::missing_timestamp)?.into(),
+            timestamp: value
+                .timestamp
+                .ok_or_else(Error::missing_timestamp)?
+                .try_into()?,
         })
     }
 }
@@ -113,7 +116,7 @@ impl DuplicateVoteEvidence {
             vote_b,
             total_voting_power: Default::default(),
             validator_power: Default::default(),
-            timestamp: Time::now(),
+            timestamp: Time::unix_epoch(),
         })
     }
     /// Get votes

diff --git a/tendermint/src/proposal.rs b/tendermint/src/proposal.rs
@@ -57,7 +57,7 @@ impl TryFrom<RawProposal> for Proposal {
             round: value.round.try_into()?,
             pol_round,
             block_id: value.block_id.map(TryInto::try_into).transpose()?,
-            timestamp: value.timestamp.map(|t| t.into()),
+            timestamp: value.timestamp.map(|t| t.try_into()).transpose()?,
             signature: Signature::new(value.signature)?,
         })
     }

diff --git a/tendermint/src/proposal/canonical_proposal.rs b/tendermint/src/proposal/canonical_proposal.rs
@@ -53,7 +53,7 @@ impl TryFrom<RawCanonicalProposal> for CanonicalProposal {
             round,
             pol_round,
             block_id: block_id.map(TryInto::try_into).transpose()?,
-            timestamp: value.timestamp.map(|t| t.into()),
+            timestamp: value.timestamp.map(|t| t.try_into()).transpose()?,
             chain_id: ChainId::try_from(value.chain_id).unwrap(),
         })
     }