Skip to content

Commit

Permalink
Fix formatting of tendermint::Time (#775)
Browse files Browse the repository at this point in the history 
The `Time` datatype needs to be formatted the same way as
Go's `time.RFC3339Nano` format, ie. a RFC3339 date-time
with left-padded nanoseconds digits without trailing zeros
and no trailing dot.

This is taken care of by the `to_rfc339_nanos` function in
the `tendermint_proto::serializers::timestamp` module.

Prior to this commit, this function would incorrectly
trim the leading zeros of the nanoseconds digits, which
would break the `Time` -> String -> `Time` roundtrip.

For example, the date "2021-01-07T17:52:04.034475Z" was serialized
as "2021-01-07T17:52:04.34475Z", which then decodes to
"2021-01-07T17:52:04.344750Z", shifting the leading zero to the end.

This commit fixes `to_rfc339_nanos` function by using `chrono`'s
formatting facilities to preserve the precision of the nanoseconds,
and then manually trim the trailing zeros and dot.
  • Loading branch information
@romac
romac committed Jan 8, 2021
1 parent e65337a commit c44ae5b
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 50 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@
[#758]: https://github.com/informalsystems/tendermint-rs/pull/758
[cargo make]: https://github.com/sagiegurari/cargo-make

### BUG FIXES:

- `[tendermint]` `Time` values were not always formatted properly,
causing the light client to sometimes return malformed light blocks. ([#774])

[#774]: https://github.com/informalsystems/tendermint-rs/issue/774

## v0.17.0

*Dec 17, 2020*
Expand Down
127 changes: 78 additions & 49 deletions proto/src/serializers/timestamp.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@
use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer};

use crate::google::protobuf::Timestamp;
use chrono::{DateTime, Datelike, LocalResult, TimeZone, Timelike, Utc};
use chrono::{DateTime, LocalResult, TimeZone, Utc};
use serde::ser::Error;

/// Helper struct to serialize and deserialize Timestamp into an RFC3339-compatible string
/// This is required because the serde `with` attribute is only available to fields of a struct but
/// not the whole struct.
#[derive(Serialize, Deserialize)]
#[derive(Debug, Serialize, Deserialize)]
#[serde(transparent)]
pub struct Rfc3339(#[serde(with = "crate::serializers::timestamp")] Timestamp);

impl From<Timestamp> for Rfc3339 {
fn from(value: Timestamp) -> Self {
Rfc3339(value)
Expand Down Expand Up @@ -46,85 +47,113 @@ where
}
match Utc.timestamp_opt(value.seconds, value.nanos as u32) {
LocalResult::None => Err(S::Error::custom("invalid time")),
LocalResult::Single(t) => Ok(to_rfc3339_custom(&t)),
LocalResult::Single(t) => Ok(to_rfc3339_nanos(&t)),
LocalResult::Ambiguous(_, _) => Err(S::Error::custom("ambiguous time")),
}?
.serialize(serializer)
}

/// Serialization helper for converting a `DateTime<Utc>` object to a string.
///
/// Due to incompatibilities between the way that `chrono` serializes timestamps
/// and the way that Go does for RFC3339, we unfortunately need to define our
/// own timestamp serialization mechanism.
pub fn to_rfc3339_custom(t: &DateTime<Utc>) -> String {
let nanos = format!(".{}", t.nanosecond());
format!(
"{:04}-{:02}-{:02}T{:02}:{:02}:{:02}{}Z",
t.year(),
t.month(),
t.day(),
t.hour(),
t.minute(),
t.second(),
nanos.trim_end_matches('0').trim_end_matches('.'),
)
/// This reproduces the behavior of Go's `time.RFC3339Nano` format,
/// ie. a RFC3339 date-time with left-padded subsecond digits without
/// trailing zeros and no trailing dot.
pub fn to_rfc3339_nanos(t: &DateTime<Utc>) -> String {
use chrono::format::{Fixed, Item, Numeric::*, Pad::Zero};

const PREFIX: &[Item<'_>] = &[
Item::Numeric(Year, Zero),
Item::Literal("-"),
Item::Numeric(Month, Zero),
Item::Literal("-"),
Item::Numeric(Day, Zero),
Item::Literal("T"),
Item::Numeric(Hour, Zero),
Item::Literal(":"),
Item::Numeric(Minute, Zero),
Item::Literal(":"),
Item::Numeric(Second, Zero),
];

const NANOS: &[Item<'_>] = &[Item::Fixed(Fixed::Nanosecond)];

// Format as RFC339 without nanoseconds nor timezone marker
let prefix = t.format_with_items(PREFIX.iter());

// Format nanoseconds with dot, leading zeros, and variable number of trailing zeros
let nanos = t.format_with_items(NANOS.iter()).to_string();

// Trim trailing zeros and remove leftover dot if any
let nanos_trimmed = nanos.trim_end_matches('0').trim_end_matches('.');

format!("{}{}Z", prefix, nanos_trimmed)
}

#[allow(warnings)]
#[cfg(test)]
mod test {
use super::*;
use crate::google::protobuf::Timestamp;
use serde::{Deserialize, Serialize};

// The Go code with which the following timestamps were tested is as
// follows:
// The Go code with which the following timestamps
// were tested is as follows:
//
// ```go
// package main
//
// import (
// "fmt"
// "time"
// "fmt"
// "time"
// )
//
// func main() {
// timestamps := []string{
// "2020-09-14T16:33:54.21191421Z",
// "2020-09-14T16:33:00Z",
// "2020-09-14T16:33:00.1Z",
// "2020-09-14T16:33:00.211914212Z",
// }
// for _, timestamp := range timestamps {
// ts, err := time.Parse(time.RFC3339Nano, timestamp)
// if err != nil {
// panic(err)
// }
// tss := ts.Format(time.RFC3339Nano)
// if timestamp != tss {
// panic(fmt.Sprintf("\nExpected : %s\nActual : %s", timestamp, tss))
// }
// }
// fmt.Println("All good!")
// timestamps := []string{
// "1970-01-01T00:00:00Z",
// "0001-01-01T00:00:00Z",
// "2020-09-14T16:33:00Z",
// "2020-09-14T16:33:00.1Z",
// "2020-09-14T16:33:00.211914212Z",
// "2020-09-14T16:33:54.21191421Z",
// "2021-01-07T20:25:56.045576Z",
// "2021-01-07T20:25:57.039219Z",
// "2021-01-07T20:26:05.00509Z",
// "2021-01-07T20:26:05.005096Z",
// "2021-01-07T20:26:05.0005096Z",
// }
// for _, timestamp := range timestamps {
// ts, err := time.Parse(time.RFC3339Nano, timestamp)
// if err != nil {
// panic(err)
// }
// tss := ts.Format(time.RFC3339Nano)
// if timestamp != tss {
// panic(fmt.Sprintf("\nExpected : %s\nActual : %s", timestamp, tss))
// }
// }
// fmt.Println("All good!")
// }
// ```
#[test]
fn json_timestamp_precision() {
#[derive(Serialize, Deserialize)]
struct TimestampWrapper {
timestamp: Timestamp,
}
let test_timestamps = vec![
"2020-09-14T16:33:54.21191421Z",
"1970-01-01T00:00:00Z",
"0001-01-01T00:00:00Z",
"2020-09-14T16:33:00Z",
"2020-09-14T16:33:00.1Z",
"2020-09-14T16:33:00.211914212Z",
"1970-01-01T00:00:00Z",
"0001-01-01T00:00:00Z",
"2020-09-14T16:33:54.21191421Z",
"2021-01-07T20:25:56.045576Z",
"2021-01-07T20:25:57.039219Z",
"2021-01-07T20:26:05.00509Z",
"2021-01-07T20:26:05.005096Z",
"2021-01-07T20:26:05.0005096Z",
];

for timestamp in test_timestamps {
let json = "{\"timestamp\":\"".to_owned() + timestamp + "\"}";
let wrapper = serde_json::from_str::<TimestampWrapper>(&json).unwrap();
assert_eq!(json, serde_json::to_string(&wrapper).unwrap());
let json = format!("\"{}\"", timestamp);
let rfc = serde_json::from_str::<Rfc3339>(&json).unwrap();
assert_eq!(json, serde_json::to_string(&rfc).unwrap());
}
}
}
38 changes: 37 additions & 1 deletion tendermint/src/time.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ impl TryFrom<Timestamp> for Time {
seconds: value.seconds,
nanos: value.nanos,
};

Ok(SystemTime::try_from(prost_value)
.map_err(|e| {
Kind::OutOfRange.context(format!("time before EPOCH by {} seconds", e.as_secs()))
Expand Down Expand Up @@ -79,7 +80,7 @@ impl Time {

/// Return an RFC 3339 and ISO 8601 date and time string with 6 subseconds digits and Z.
pub fn to_rfc3339(&self) -> String {
timestamp::to_rfc3339_custom(&self.0)
timestamp::to_rfc3339_nanos(&self.0)
}

/// Convert [`Time`] to [`SystemTime`]
Expand Down Expand Up @@ -150,3 +151,38 @@ pub trait ParseTimestamp {
/// Parse [`Time`], or return an [`Error`] if parsing failed
fn parse_timestamp(&self) -> Result<Time, Error>;
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn serde_roundtrip() {
const DATES: &[&str] = &[
"2020-09-14T16:33:54.21191421Z",
"2020-09-14T16:33:00Z",
"2020-09-14T16:33:00.1Z",
"2020-09-14T16:33:00.211914212Z",
"1970-01-01T00:00:00Z",
"2021-01-07T20:25:56.0455760Z",
"2021-01-07T20:25:57.039219Z",
"2021-01-07T20:25:58.03562100Z",
"2021-01-07T20:25:59.000955200Z",
"2021-01-07T20:26:04.0121030Z",
"2021-01-07T20:26:05.005096Z",
"2021-01-07T20:26:09.08488400Z",
"2021-01-07T20:26:11.0875340Z",
"2021-01-07T20:26:12.078268Z",
"2021-01-07T20:26:13.08074100Z",
"2021-01-07T20:26:15.079663000Z",
];

for input in DATES {
let initial_time: Time = input.parse().unwrap();
let encoded_time = serde_json::to_value(&initial_time).unwrap();
let decoded_time = serde_json::from_value(encoded_time.clone()).unwrap();

assert_eq!(initial_time, decoded_time);
}
}
}

0 comments on commit c44ae5b

Please sign in to comment.