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

Fix formatting of tendermint::Time values #775

Merged
merged 1 commit into from
Jan 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
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
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
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);
}
}
}