Replace chrono with time 0.3 (informalsystems#1030)

* pbt-gen: Converted from chrono to time 0.3

chrono has soundness issues (see RUSTSEC-2020-0159) and does not
seem to be maintained.

* Replace dependency on chrono with time 0.3

Change Time implementation to crate time: chrono has soundness issues
(see RUSTSEC-2020-0159) and does not seem to be actively maintained.

* Add Time methods checked_add and checked_sub

These should be used instead of the overloaded operators that broke
the operator convention by returning a Result.

* proto: Don't use formatting methods of time

Drop the "formatting" feature of time, as this brings in std.

* pbt-gen: Add arb_datetime_for_rfc3339

With crate time, the range of supported dates stretches to the ancient
past beyond the common era, which is not representable in RFC 3339.
Add a generator to produce `OffsetDateTime` values that are
within the RFC 3339 spec.

* Ensure Time can only have values good for RFC 3339

As the time values are meant for human-readable representation in
the RFC 3339 format, make it not possible to construct Time with values
that fall outside this standard representation.
Conversion from time::OffsetDateTime is made fallible with
a TryFrom impl replacing the From impl.
Conversion from Unix timestamps is also affected.

* rpc: Replaced chrono with time 0.3

* testgen: Replaced chrono with time 0.3

* Less allocatey ways of formatting date-times

Provide and use another helper in proto mod serializers::timestamp,
one that formats into a provided fmt::Write object rather than
allocating a string.

* light-client: port from chrono to time

* Revert to Add/Sub returning Result

* light-client: changed the MBTs from chrono to time

* tendermint: Remove a comment referencing chrono

We use ErrorDetail::DurationOutOfRange without the source error
from the time library because that is uninformative in the context.
Also move the DateOutOfRange close with DurationOutOfRange
since they can occur in the same operations and are generally similar.

* light-client: add std feature for time

The clock needs OffsetDateTime::now_utc().

* tendermint: Simplify Time::duration_since

* testgen: minor code cleanup

* Restrict valid Time years to 1-9999

Exclude year 0 because the spec on Google protobuf Timestamp forbids it.

Add more helpers in pbt-gen to produce protobuf-safe values in both
OffsetDateTime and RFC 3339 strings.

Restrict the property tests to only use the protobuf-safe values
where expected.

* Test Time::checked_add and Time::checked_sub

* Changelog entries for informalsystems#1030

* Improve documentation of tendermint::Time

* proto: remove the chrono type conversions

The From impls are panicky and do not enforce compliance with
protobuf message value restrictions.

* tendermint: remove direct uses of chrono types

Replace with tendermint::Time.

* Harden Timestamp conversions and serde

Require the timestamp to be in the validity range (years 1-9999 in UTC)
and the nanosecond member value to not exceed  999_999_999.

Rename ErrorDetail::TimestampOverflow to TimestampNanosOutOfRange,
because the old variant was not very informative (the chained
TryFromIntError did not help) and we also use it for the above-range
case now which is not an overflow.

* Changelog entry about changed error variants

* Restore nanosecond range check in Time::from_unix_timestamp

Add a unit test to exercise the check.

* proto: Improve timestamp::fmt_as_rfc3339_nanos

- More ergonomic signature
- A non-allocating implementation

* Fix component name in changelog for 1030-remove-chrono

Co-authored-by: Thane Thomson <[email protected]>

* time: Use Self instead of the type name in methods

Co-authored-by: Thane Thomson <[email protected]>

* Comment on the inner representation of `Time`

* Don't alias crate time in testgen

* Document the Time::from_utc helper

Co-authored-by: Thane Thomson <[email protected]>

.changelog/unreleased/breaking-changes/1030-remove-chrono.md

@@ -0,0 +1,11 @@
+- `[tendermint]` Reform `tendermint::Time`
+  ([#1030](https://github.com/informalsystems/tendermint-rs/issues/1030)):
+  * The struct content is made private.
+  * The range of acceptable values is restricted to years 1-9999
+    (as reckoned in UTC).
+  * Removed conversions from/to `chrono::DateTime<chrono::Utc>`.
+  * Changes in error variants: removed `TimestampOverflow`, replaced with
+    `TimestampNanosOutOfRange`; removed `ChronoParse`, replaced with `TimeParse`.
+- `[tendermint-rpc]` Use `OffsetDateTime` and `Date` types provided by the `time` crate
+  in query operands instead of their `chrono` counterparts.
+  ([#1030](https://github.com/informalsystems/tendermint-rs/issues/1030))

.changelog/unreleased/improvements/1030-new-time-api.md

@@ -0,0 +1,11 @@
+- Remove dependencies on the `chrono` crate.
+  ([#1030](https://github.com/informalsystems/tendermint-rs/issues/1030))
+- `[tendermint]` Improve `tendermint::Time`
+  ([#1030](https://github.com/informalsystems/tendermint-rs/issues/1030)):
+  * Restrict the validity range of `Time` to dates with years in the range
+    1-9999, to match the specification of protobuf message `Timestamp`.
+    Add an `ErrorDetail` variant `DateOutOfRange` to report when this
+    restriction is not met.
+  * Added a conversion to, and a fallible conversion from,
+    `OffsetDateTime` of the `time` crate.
+  * Added `Time` methods `checked_add` and `checked_sub`.

light-client/Cargo.toml

@@ -41,7 +41,6 @@ tendermint = { version = "0.23.0", path = "../tendermint", default-features = fa
 tendermint-rpc = { version = "0.23.0", path = "../rpc", default-features = false }
 
 contracts = { version = "0.4.0", default-features = false }
-chrono = { version = "0.4", default-features = false, features = ["clock"] }
 crossbeam-channel = { version = "0.4.2", default-features = false }
 derive_more = { version = "0.99.5", default-features = false, features = ["display"] }
 futures = { version = "0.3.4", default-features = false }
@@ -50,6 +49,7 @@ serde_cbor = { version = "0.11.1", default-features = false, features = ["alloc"
 serde_derive = { version = "1.0.106", default-features = false }
 sled = { version = "0.34.3", optional = true, default-features = false }
 static_assertions = { version = "1.1.0", default-features = false }
+time = { version = "0.3", default-features = false, features = ["std"] }
 tokio = { version = "1.0", default-features = false, features = ["rt"], optional = true }
 flex-error = { version = "0.4.4", default-features = false }
 

light-client/src/components/clock.rs

@@ -1,7 +1,8 @@
 //! Provides an interface and a default implementation of the `Clock` component
 
 use crate::types::Time;
-use chrono::Utc;
+use std::convert::TryInto;
+use time::OffsetDateTime;
 
 /// Abstracts over the current time.
 pub trait Clock: Send + Sync {
@@ -14,6 +15,8 @@ pub trait Clock: Send + Sync {
 pub struct SystemClock;
 impl Clock for SystemClock {
     fn now(&self) -> Time {
-        Time(Utc::now())
+        OffsetDateTime::now_utc()
+            .try_into()
+            .expect("system clock produces invalid time")
     }
 }

light-client/src/predicates.rs

@@ -205,10 +205,11 @@ pub trait VerificationPredicates: Send + Sync {
 
 #[cfg(test)]
 mod tests {
-    use chrono::Utc;
-    use std::ops::Sub;
+    use std::convert::TryInto;
     use std::time::Duration;
-    use tendermint::Time;
+    use tendermint::block::CommitSig;
+    use tendermint::validator::Set;
+    use time::OffsetDateTime;
 
     use tendermint_testgen::{
         light_block::{LightBlock as TestgenLightBlock, TmLightBlock},
@@ -224,8 +225,6 @@ mod tests {
         Hasher, ProdCommitValidator, ProdHasher, ProdVotingPowerCalculator, VotingPowerTally,
     };
     use crate::types::{LightBlock, TrustThreshold};
-    use tendermint::block::CommitSig;
-    use tendermint::validator::Set;
 
     impl From<TmLightBlock> for LightBlock {
         fn from(lb: TmLightBlock) -> Self {
@@ -294,7 +293,7 @@ mod tests {
 
         // 1. ensure valid header verifies
         let mut trusting_period = Duration::new(1000, 0);
-        let now = Time(Utc::now());
+        let now = OffsetDateTime::now_utc().try_into().unwrap();
 
         let result_ok = vp.is_within_trust_period(header.time, trusting_period, now);
         assert!(result_ok.is_ok());
@@ -322,13 +321,15 @@ mod tests {
         let vp = ProdPredicates::default();
         let one_second = Duration::new(1, 0);
 
+        let now = OffsetDateTime::now_utc().try_into().unwrap();
+
         // 1. ensure valid header verifies
-        let result_ok = vp.is_header_from_past(header.time, one_second, Time(Utc::now()));
+        let result_ok = vp.is_header_from_past(header.time, one_second, now);
 
         assert!(result_ok.is_ok());
 
         // 2. ensure it fails if header is from a future time
-        let now = Time(Utc::now()).sub(one_second * 15).unwrap();
+        let now = now.checked_sub(one_second * 15).unwrap();
         let result_err = vp.is_header_from_past(header.time, one_second, now);
 
         match result_err {

light-client/tests/model_based.rs

@@ -1,11 +1,10 @@
 #[cfg(feature = "mbt")]
 mod mbt {
-    use chrono::Utc;
     use rand::Rng;
     use serde::de::DeserializeOwned;
     use serde::{Deserialize, Serialize};
     use serde_json::Error;
-    use std::convert::TryFrom;
+    use std::convert::{TryFrom, TryInto};
     use std::str::FromStr;
     use std::time::Duration;
     use tendermint::validator::Set;
@@ -20,6 +19,7 @@ mod mbt {
         apalache::*, jsonatr::*, light_block::TmLightBlock, validator::generate_validators,
         Command, Generator, LightBlock as TestgenLightBlock, TestEnv, Tester, Validator, Vote,
     };
+    use time::OffsetDateTime;
 
     fn testgen_to_lb(tm_lb: TmLightBlock) -> LightBlock {
         LightBlock {
@@ -180,7 +180,8 @@ mod mbt {
     impl SingleStepTestFuzzer for HeaderTimeFuzzer {
         fn fuzz_input(input: &mut BlockVerdict) -> (String, LiteVerdict) {
             let mut rng = rand::thread_rng();
-            let secs = tendermint::Time(Utc::now())
+            let now: Time = OffsetDateTime::now_utc().try_into().unwrap();
+            let secs = now
                 .duration_since(tendermint::Time::unix_epoch())
                 .unwrap()
                 .as_secs();

pbt-gen/Cargo.toml

@@ -17,8 +17,10 @@ description = """
 
 [features]
 default = ["time"]
-time = ["chrono"]
 
 [dependencies]
-chrono = { version = "0.4", default-features = false, features = ["serde"], optional = true}
+time = { version = "0.3.5", default-features = false, optional = true }
 proptest = { version = "0.10.1", default-features = false, features = ["std"] }
+
+[dev-dependencies]
+time = { version = "0.3.5", features = ["macros"] }