Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace chrono with time 0.3 (backport to 0.23.x) #1036

Merged
merged 10 commits into from
Dec 7, 2021

Conversation

mzabaluev
Copy link
Contributor

Backport of #1030 to 0.23.x.

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.
@mzabaluev mzabaluev added enhancement New feature or request dependencies Pull requests that update a dependency file serialization breaking code-quality Issues relating to linting configuration and general code quality labels Nov 29, 2021
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.
Add a unit test to exercise the check.
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (v0.23.x@1dedd72). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             v0.23.x   #1036   +/-   ##
=========================================
  Coverage           ?   66.3%           
=========================================
  Files              ?     209           
  Lines              ?   20967           
  Branches           ?       0           
=========================================
  Hits               ?   13916           
  Misses             ?    7051           
  Partials           ?       0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1dedd72...225c179. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking code-quality Issues relating to linting configuration and general code quality dependencies Pull requests that update a dependency file enhancement New feature or request serialization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants