-
Notifications
You must be signed in to change notification settings - Fork 224
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 #1030
Conversation
chrono has soundness issues (see RUSTSEC-2020-0159) and does not seem to be maintained.
Change Time implementation to crate time: chrono has soundness issues (see RUSTSEC-2020-0159) and does not seem to be actively maintained.
These should be used instead of the overloaded operators that broke the operator convention by returning a Result.
Drop the "formatting" feature of time, as this brings in std.
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.
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.
Provide and use another helper in proto mod serializers::timestamp, one that formats into a provided fmt::Write object rather than allocating a string.
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.
Codecov Report
@@ Coverage Diff @@
## master #1030 +/- ##
========================================
+ Coverage 62.0% 62.4% +0.3%
========================================
Files 237 236 -1
Lines 21169 21352 +183
========================================
+ Hits 13143 13339 +196
+ Misses 8026 8013 -13
Continue to review full report at Codecov.
|
The clock needs OffsetDateTime::now_utc().
I'd recommend targeting the I was planning on merging #1022 ASAP, then we'll need to backport these changes to |
No, we only really need it to fix the "broken window" of audit failures. Also, it's a breaking change, so it should properly be included starting from 0.24.x (assuming this project follows cargo's stricter semver rules). |
We're actually going to allow for breaking changes to the v0.23.x series from one "patch" version to the next. I should add a note that people need to pin a specific version of tendermint-rs in their dependencies. This is one of the side effects of the versioning strategy we're going with. Since we're also pre-1.0, I think there's still some leniency here. So I'd still recommend merging this into |
@@ -1,53 +0,0 @@ | |||
use core::convert::TryInto; | |||
|
|||
use chrono::{DateTime, Duration, TimeZone, Utc}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole module is removed, rather than being replaced with time
equivalents, because of the problems detailed below. Conversions from/to timestamp::Time
can serve as domain-validating, fallible alternatives, until more direct convenience methods are reintroduced here if needed.
fn from(dt: DateTime<Utc>) -> pb::Timestamp { | ||
pb::Timestamp { | ||
seconds: dt.timestamp(), | ||
// This can exceed 1_000_000_000 in the case of a leap second, but |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was against the wire protocol specification.
|
||
impl From<pb::Timestamp> for DateTime<Utc> { | ||
fn from(ts: pb::Timestamp) -> DateTime<Utc> { | ||
Utc.timestamp(ts.seconds, ts.nanos as u32) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This panicked if members of ts
have invalid values (subject to chrono
's interpretation which does not readily conform to the protobuf specification).
As the most likely origin of a pb::Timestamp
is by having been parsed from an incoming protobuf message by prost without any validation as to its contents, this is an easy DoS bomb under any application developers who would make use of this conversion. Please consider using fallible conversions when dealing with untrusted values originating from the network.
let seconds = d.num_seconds(); | ||
let nanos = (d - Duration::seconds(seconds)) | ||
.num_nanoseconds() | ||
.expect("we computed the fractional part, so there's no overflow") | ||
.try_into() | ||
.expect("the fractional part fits in i32"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could produce protobuf messages that violate the specification.
The whole approach of having infallible conversions that can only either panic or produce off-spec values for the wire protocol in case the destination's validity requirements are not met, needs to be reconsidered.
impl From<DateTime<Utc>> for pb::Timestamp { | ||
fn from(dt: DateTime<Utc>) -> pb::Timestamp { | ||
pb::Timestamp { | ||
seconds: dt.timestamp(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timestamp's range as represented by chrono::DateTime
can exceed the range valid for the protobuf Timestamp
messages.
fn from(d: pb::Duration) -> Duration { | ||
// there's no constructor that supplies both at once | ||
Duration::seconds(d.seconds) + Duration::nanoseconds(d.nanos as i64) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually possible to replace with an infallible time
equivalent (and time
does have a one-stop shop constructor for that!), but it felt odd to leave a lone conversion standing when others were sent back to the drawing board.
We should consider introducing a newtype for durations in the tendermint API, with similar range constraint enforcement as maintained in this PR for Time
.
This reimplementation does not do so; the inner value is a
This is clever, but the type parameter of An instance of unwanted complexity that bugs me more about chrono is its handling of leap seconds. It looks like a recipe for a wide variety of bugs, and it's not compatible with Google's definition of a protobuf timestamp, which specifically avoids dealing with leap second times. It's certainly not something I'd feel comfortable basing the protocol library's support for time representation on.
That requires actual malicious intent. By contrast, Are there really many other easy ways to violate soundness from safe Rust? Are we living in an illusion of safety?
There is already a number of dependencies managed with feature gates, so this seems like more of the same.
As long as we don't have any other support for:
I think it's necessary to have a newtype with fallible conversions here. |
I managed to get the desired behaviour out of it, but unfortunately that currently requires |
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.
- More ergonomic signature - A non-allocating implementation
Can we have a parallel branch in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor nits and some questions. Otherwise looks great! 🎉
All my questions/comments apply to #1036 as well, where relevant.
} | ||
} | ||
|
||
impl Time { | ||
fn from_utc(t: OffsetDateTime) -> Result<Self, Error> { | ||
debug_assert_eq!(t.offset(), offset!(UTC)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason to reserve this check only for debug builds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an internal method, all call sites should either produce a value with the UTC offset or convert to it.
The debug check is to catch if a programmer forgot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a comment on the function.
let mut secfrac = nanos; | ||
let mut secfrac_width = 9; | ||
while secfrac % 10 == 0 { | ||
secfrac /= 10; | ||
secfrac_width -= 1; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some back-of-the-envelope calculations so far indicate to me that this works for formatting. Neat little algorithm 🙂 Would it not perhaps be worthwhile to add some test cases to test this fmt_as_rfc3339_nanos
method out? Or is the best place to test this in the property-based tests for the tendermint::Time
struct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existing tests exercise to_rfc3339
which uses this method, and they caught me out when I did it wrong, so I assume it is covered.
Co-authored-by: Thane Thomson <[email protected]>
Co-authored-by: Thane Thomson <[email protected]>
01902bc
to
f5e6244
Compare
Clippy failures should be fixed by #1041. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, I think we're good to go here. Thanks @mzabaluev!
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]>
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]>
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]>
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]>
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]>
* split A.4.1, proto tooling changes * split A.2, changelog abci * split A.1, tendermint and abci changes * split A.3, abci-in-rpc * clean up imports in abci crate * Return to 0.34 ABCI encoding * Cherry-pick chrono changes Replace chrono with time 0.3 (#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 #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]> * fixup! split A.3, abci-in-rpc * split A.4.1.1, check_event_attrs * split A.4.1.2, some slice change? * split A.4.1.3, b.data.get(0).is_none() * split A.4.1.4, remove test attrs * split A.4.1.5, someting optiony * clippy fix * Remove unnessarily generated files * Fix deliver_tx.events KV tests * Remove Option wart in net_info::Response * rpc: Revert Event member events to a KV map The event list change did not make it into tendermint 0.37. * Revert tests to checking decoded KVs * rpc: Restore base64 serialization of event KV tags * Post-merge fixes for rpc tests * rpc: Use the re-exported serializers mod * Remove duplicate changelog entries * Fix kvstore-test build * Changelog entry for switching to Bytes * Remove a commented out line * abci: Restore Client::set_option as deprecated * Update abci/src/client.rs Co-authored-by: Henry de Valence <[email protected]> Co-authored-by: Thane Thomson <[email protected]>
Fixes: #1012, #1007, #1008
Important or breaking changes:
Removed
chrono
from dependencies and public API, replaced bytime
.The inner value of the
Time
tuple struct is made private.Renamed
Time::as_rfc3339
toto_rfc3339
to follow Rust naming conventions (somehow clippy got it wrong at the time of Fix latest clippy assertion failures #910, but it no longer does).Instead of
From<chrono::DateTime<Utc>>
,Time
now implementsTryFrom<time::OffsetDateTime>
. The reason is thatOffsetDateTime
supports a wider range of dates than is supported by RFC 3339, andtendermint::Time
needs its string conversion to be standard-compliant and infallible.Added
Time
methodschecked_add
/checked_sub
, to follow the pattern set bySystemTime
andtime::OffsetDateTime
. In a later change, these could replace use of theAdd
/Sub
operator impls, whoseResult
output types make for poor ergonomics in operator overloading.rpc
usestime
typesOffsetDateTime
andDate
in query operands instead of theirchrono
counterparts.Referenced an issue explaining the need for the change
Updated all relevant documentation in docs
Updated all code comments where relevant
Wrote tests
Added entry in
.changelog/