Skip to content

Commit

Permalink
Replace chrono with time 0.3 (backport to 0.23.x) (#1036)
Browse files Browse the repository at this point in the history
* Replace dependency on chrono with time 0.3

Changes in tendermint:

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

Ensure Time can only have values that are valid for Google protobuf
Timestamp messages. Add ErrorDetail variant DateOutOfRange to report
when this restriction is not met.

Remove conversions between Time and chrono::DateTime, replacing them
with conversions from/to time::OffsetDateTime.
Conversion from time::OffsetDateTime is made fallible with
a TryFrom impl.

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.

Changes in tendermint-pbt-gen:

Change the time stragegies and helpers to produce
time::OffsetDateTime values.
Add strategies to generate date-time values and RFC 3339 strings
in the range valid for protobuf Timestamp messages.

Changes in tendermint-proto:

Provide another helper in the serializers::timestamp module,
one that formats into a provided fmt::Write object.

Changes in tendermint-rpc:

Use `OffsetDateTime` and `Date` types provided by the `time` crate
in query operands instead of their `chrono` counterparts.

* 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 <thane@informal.systems>

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

Co-authored-by: Thane Thomson <thane@informal.systems>

* 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 <thane@informal.systems>
  • Loading branch information
mzabaluev and thanethomson authored Dec 7, 2021
1 parent c64cbaa commit aef23ed
Show file tree
Hide file tree
Showing 23 changed files with 565 additions and 245 deletions.
11 changes: 11 additions & 0 deletions .changelog/unreleased/breaking-changes/1030-remove-chrono.md
Original file line number Diff line number Diff line change
@@ -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))
11 changes: 11 additions & 0 deletions .changelog/unreleased/improvements/1030-new-time-api.md
Original file line number Diff line number Diff line change
@@ -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`.
2 changes: 1 addition & 1 deletion light-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ tendermint = { version = "0.23.1", path = "../tendermint", default-features = fa
tendermint-rpc = { version = "0.23.1", 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 }
Expand All @@ -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 }

Expand Down
7 changes: 5 additions & 2 deletions light-client/src/components/clock.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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")
}
}
17 changes: 9 additions & 8 deletions light-client/src/predicates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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 {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 4 additions & 3 deletions light-client/tests/model_based.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down
6 changes: 4 additions & 2 deletions pbt-gen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Loading

0 comments on commit aef23ed

Please sign in to comment.