From 3366b4bcac0d497e3c3dbd0fc4312e17cc590d76 Mon Sep 17 00:00:00 2001 From: Greg Szabo Date: Mon, 9 Nov 2020 18:43:27 -0500 Subject: [PATCH 1/4] Implemented rfc3339 direct serialization/deserialization for protobuf Timestamp --- light-client/tests/model_based.rs | 3 +- proto-compiler/src/constants.rs | 9 ++--- proto/src/prost/tendermint.types.rs | 6 +-- proto/src/protobuf.rs | 1 + proto/src/serializers.rs | 2 +- proto/src/serializers/nullable.rs | 2 +- .../{option_timestamp.rs => timestamp.rs} | 37 +++++++++++++------ tendermint/src/serializers/time.rs | 5 +-- 8 files changed, 39 insertions(+), 26 deletions(-) rename proto/src/serializers/{option_timestamp.rs => timestamp.rs} (76%) diff --git a/light-client/tests/model_based.rs b/light-client/tests/model_based.rs index 93e1239db..be3675451 100644 --- a/light-client/tests/model_based.rs +++ b/light-client/tests/model_based.rs @@ -119,7 +119,8 @@ struct HeaderVersionFuzzer {} impl SingleStepTestFuzzer for HeaderVersionFuzzer { // TODO: rehash the header and re-compute commit with it // TODO: Unlike in tendermint-go, we don't assert for a particular version in rust - // TODO: Either add this check in verification or remove this test because otherwise there's no point of it + // TODO: Either add this check in verification or remove this test because otherwise there's no + // point of it fn fuzz_input(input: &mut BlockVerdict) -> (String, bool) { let mut rng = rand::thread_rng(); diff --git a/proto-compiler/src/constants.rs b/proto-compiler/src/constants.rs index fc068fee2..53929c51a 100644 --- a/proto-compiler/src/constants.rs +++ b/proto-compiler/src/constants.rs @@ -18,7 +18,7 @@ const QUOTED: &str = r#"#[serde(with = "crate::serializers::from_str")]"#; const QUOTED_WITH_DEFAULT: &str = r#"#[serde(with = "crate::serializers::from_str", default)]"#; const HEXSTRING: &str = r#"#[serde(with = "crate::serializers::bytes::hexstring")]"#; const BASE64STRING: &str = r#"#[serde(with = "crate::serializers::bytes::base64string")]"#; -const TIMESTAMP: &str = r#"#[serde(with = "crate::serializers::option_timestamp")]"#; +const OPTIONAL: &str = r#"#[serde(with = "crate::serializers::optional")]"#; const VEC_SKIP_IF_EMPTY: &str = r#"#[serde(skip_serializing_if = "Vec::is_empty", with = "serde_bytes")]"#; const NULLABLEVECARRAY: &str = r#"#[serde(with = "crate::serializers::txs")]"#; @@ -50,7 +50,6 @@ pub static CUSTOM_TYPE_ATTRIBUTES: &[(&str, &str)] = &[ (".tendermint.types.Vote", SERIALIZED), (".tendermint.types.BlockID", SERIALIZED), (".tendermint.types.PartSetHeader", SERIALIZED), - (".google.protobuf.Timestamp", SERIALIZED), (".tendermint.types.LightClientAttackEvidence", SERIALIZED), (".tendermint.types.LightBlock", SERIALIZED), (".tendermint.types.SignedHeader", SERIALIZED), @@ -100,7 +99,7 @@ pub static CUSTOM_FIELD_ATTRIBUTES: &[(&str, &str)] = &[ ), (".tendermint.types.PartSetHeader.hash", HEXSTRING), (".tendermint.types.Header.height", QUOTED), - (".tendermint.types.Header.time", TIMESTAMP), + (".tendermint.types.Header.time", OPTIONAL), (".tendermint.types.Header.last_commit_hash", HEXSTRING), (".tendermint.types.Header.data_hash", HEXSTRING), (".tendermint.types.Header.validators_hash", HEXSTRING), @@ -115,14 +114,14 @@ pub static CUSTOM_FIELD_ATTRIBUTES: &[(&str, &str)] = &[ (".tendermint.types.Commit.height", QUOTED), (".tendermint.types.Commit.signatures", NULLABLE), (".tendermint.types.CommitSig.validator_address", HEXSTRING), - (".tendermint.types.CommitSig.timestamp", TIMESTAMP), + (".tendermint.types.CommitSig.timestamp", OPTIONAL), (".tendermint.types.CommitSig.signature", BASE64STRING), (".tendermint.types.Vote.round", QUOTED), (".tendermint.types.Vote.height", QUOTED), (".tendermint.types.Vote.validator_index", QUOTED), (".tendermint.types.Vote.validator_address", HEXSTRING), (".tendermint.types.Vote.signature", BASE64STRING), - (".tendermint.types.Vote.timestamp", TIMESTAMP), + (".tendermint.types.Vote.timestamp", OPTIONAL), (".tendermint.types.Validator.address", HEXSTRING), ( ".tendermint.types.Validator.voting_power", diff --git a/proto/src/prost/tendermint.types.rs b/proto/src/prost/tendermint.types.rs index 125192acd..6beced76d 100644 --- a/proto/src/prost/tendermint.types.rs +++ b/proto/src/prost/tendermint.types.rs @@ -75,7 +75,7 @@ pub struct Header { #[serde(with = "crate::serializers::from_str")] pub height: i64, #[prost(message, optional, tag="4")] - #[serde(with = "crate::serializers::option_timestamp")] + #[serde(with = "crate::serializers::optional")] pub time: ::std::option::Option, /// prev block info #[prost(message, optional, tag="5")] @@ -155,7 +155,7 @@ pub struct Vote { #[prost(message, optional, tag="4")] pub block_id: ::std::option::Option, #[prost(message, optional, tag="5")] - #[serde(with = "crate::serializers::option_timestamp")] + #[serde(with = "crate::serializers::optional")] pub timestamp: ::std::option::Option, #[prost(bytes, tag="6")] #[serde(with = "crate::serializers::bytes::hexstring")] @@ -197,7 +197,7 @@ pub struct CommitSig { #[serde(with = "crate::serializers::bytes::hexstring")] pub validator_address: std::vec::Vec, #[prost(message, optional, tag="3")] - #[serde(with = "crate::serializers::option_timestamp")] + #[serde(with = "crate::serializers::optional")] pub timestamp: ::std::option::Option, #[prost(bytes, tag="4")] #[serde(with = "crate::serializers::bytes::base64string")] diff --git a/proto/src/protobuf.rs b/proto/src/protobuf.rs index 5a5ddebe0..8cececae9 100644 --- a/proto/src/protobuf.rs +++ b/proto/src/protobuf.rs @@ -17,6 +17,7 @@ /// restricting to that range, we ensure that we can convert to and from [RFC /// 3339](https://www.ietf.org/rfc/rfc3339.txt) date strings. #[derive(Clone, PartialEq, ::prost::Message, ::serde::Deserialize, ::serde::Serialize)] +#[serde(from = "crate::serializers::timestamp::Rfc3339", into = "crate::serializers::timestamp::Rfc3339")] pub struct Timestamp { /// Represents seconds of UTC time since Unix epoch /// 1970-01-01T00:00:00Z. Must be from 0001-01-01T00:00:00Z to diff --git a/proto/src/serializers.rs b/proto/src/serializers.rs index 3107d7b35..8ab6a0474 100644 --- a/proto/src/serializers.rs +++ b/proto/src/serializers.rs @@ -48,7 +48,7 @@ pub mod bytes; pub mod evidence; pub mod from_str; pub mod nullable; -pub mod option_timestamp; pub mod optional; pub mod time_duration; +pub mod timestamp; pub mod txs; diff --git a/proto/src/serializers/nullable.rs b/proto/src/serializers/nullable.rs index 86b4e3491..cf708482b 100644 --- a/proto/src/serializers/nullable.rs +++ b/proto/src/serializers/nullable.rs @@ -1,4 +1,4 @@ -//! Serialize/deserialize Option type. +//! Serialize/deserialize `nil`able type into T, where nil turns into the default impl. use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// Deserialize Option diff --git a/proto/src/serializers/option_timestamp.rs b/proto/src/serializers/timestamp.rs similarity index 76% rename from proto/src/serializers/option_timestamp.rs rename to proto/src/serializers/timestamp.rs index 8899e3a02..cc56c67e0 100644 --- a/proto/src/serializers/option_timestamp.rs +++ b/proto/src/serializers/timestamp.rs @@ -1,38 +1,52 @@ -//! Serialize/deserialize Option type from and into string: +//! Serialize/deserialize Timestamp type from and into string: use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer}; use crate::google::protobuf::Timestamp; use chrono::{DateTime, Datelike, LocalResult, TimeZone, Timelike, 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)] +#[serde(transparent)] +pub struct Rfc3339(#[serde(with = "crate::serializers::timestamp")] Timestamp); +impl From for Rfc3339 { + fn from(value: Timestamp) -> Self { + Rfc3339(value) + } +} +impl From for Timestamp { + fn from(value: Rfc3339) -> Self { + value.0 + } +} + /// Deserialize string into Timestamp -pub fn deserialize<'de, D>(deserializer: D) -> Result, D::Error> +pub fn deserialize<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, { let value_string = String::deserialize(deserializer)?; let value_datetime = DateTime::parse_from_rfc3339(value_string.as_str()) .map_err(|e| D::Error::custom(format!("{}", e)))?; - Ok(Some(Timestamp { + Ok(Timestamp { seconds: value_datetime.timestamp(), nanos: value_datetime.timestamp_subsec_nanos() as i32, - })) + }) } /// Serialize from Timestamp into string -pub fn serialize(value: &Option, serializer: S) -> Result +pub fn serialize(value: &Timestamp, serializer: S) -> Result where S: Serializer, { - let value = value - .as_ref() - .ok_or_else(|| S::Error::custom("no time found"))?; if value.nanos < 0 { return Err(S::Error::custom("invalid nanoseconds in time")); } match Utc.timestamp_opt(value.seconds, value.nanos as u32) { LocalResult::None => Err(S::Error::custom("invalid time")), - LocalResult::Single(t) => Ok(to_rfc3999(t)), + LocalResult::Single(t) => Ok(to_rfc3339_custom(t)), LocalResult::Ambiguous(_, _) => Err(S::Error::custom("ambiguous time")), }? .serialize(serializer) @@ -41,7 +55,7 @@ where // 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. -fn to_rfc3999(t: DateTime) -> String { +fn to_rfc3339_custom(t: DateTime) -> String { let nanos = format!(".{}", t.nanosecond()); format!( "{:04}-{:02}-{:02}T{:02}:{:02}:{:02}{}Z", @@ -95,8 +109,7 @@ mod test { fn json_timestamp_precision() { #[derive(Serialize, Deserialize)] struct TimestampWrapper { - #[serde(with = "crate::serializers::option_timestamp")] - timestamp: Option, + timestamp: Timestamp, } let test_timestamps = vec![ "2020-09-14T16:33:54.21191421Z", diff --git a/tendermint/src/serializers/time.rs b/tendermint/src/serializers/time.rs index 06b6276b5..daaa5d647 100644 --- a/tendermint/src/serializers/time.rs +++ b/tendermint/src/serializers/time.rs @@ -10,8 +10,7 @@ pub fn deserialize<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, { - let timestamp = tendermint_proto::serializers::option_timestamp::deserialize(deserializer)? - .ok_or_else(|| serde::de::Error::custom(&""))?; + let timestamp = tendermint_proto::serializers::timestamp::deserialize(deserializer)?; Time::try_from(timestamp).map_err(serde::de::Error::custom) } @@ -21,5 +20,5 @@ where S: Serializer, { let timestamp: Timestamp = value.clone().into(); - tendermint_proto::serializers::option_timestamp::serialize(&Some(timestamp), serializer) + tendermint_proto::serializers::timestamp::serialize(×tamp, serializer) } From 6d9a8eb873c86c370bb278c8d456ce833607bf9a Mon Sep 17 00:00:00 2001 From: Greg Szabo Date: Mon, 9 Nov 2020 18:48:06 -0500 Subject: [PATCH 2/4] CHANGELOG update --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8e6db8d9..eddbc03f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ deemed valid instead of invalid ([#650]) - `[light-client]` Revert a change introduced in [#652] that would enable DoS attacks, where full nodes could spam the light client with massive commits (eg. 10k validators). +- `[tendermint]` (Since RC1) Time serialization fix (part of [#665]) +- `[tendermint-proto]` (Since RC1) Timestamp serialization fix (part of [#665]) ### FEATURES: @@ -25,7 +27,7 @@ [#652]: https://github.com/informalsystems/tendermint-rs/pulls/652 [#639]: https://github.com/informalsystems/tendermint-rs/pull/639 [#660]: https://github.com/informalsystems/tendermint-rs/issues/660 - +[#665]: https://github.com/informalsystems/tendermint-rs/issues/665 ## v0.17.0-rc1 *Oct 15, 2020* From c348cc6633e963568833d938ec31fbdbac0f7fa8 Mon Sep 17 00:00:00 2001 From: Greg Szabo Date: Mon, 9 Nov 2020 19:12:51 -0500 Subject: [PATCH 3/4] Removed custom Time serializer --- light-client/src/predicates/errors.rs | 2 -- light-client/src/tests.rs | 2 -- light-client/tests/model_based.rs | 1 - rpc/src/endpoint/net_info.rs | 2 +- rpc/src/endpoint/status.rs | 1 - tendermint/src/genesis.rs | 1 - tendermint/src/serializers.rs | 1 - tendermint/src/serializers/time.rs | 24 ------------------------ 8 files changed, 1 insertion(+), 33 deletions(-) delete mode 100644 tendermint/src/serializers/time.rs diff --git a/light-client/src/predicates/errors.rs b/light-client/src/predicates/errors.rs index 6631c26f9..569f3d157 100644 --- a/light-client/src/predicates/errors.rs +++ b/light-client/src/predicates/errors.rs @@ -16,10 +16,8 @@ pub enum VerificationError { #[error("header from the future: header_time={header_time} now={now}")] HeaderFromTheFuture { /// Time in the header - #[serde(with = "tendermint::serializers::time")] header_time: Time, /// Current time - #[serde(with = "tendermint::serializers::time")] now: Time, }, diff --git a/light-client/src/tests.rs b/light-client/src/tests.rs index 86099cb4c..6f5b32f1d 100644 --- a/light-client/src/tests.rs +++ b/light-client/src/tests.rs @@ -38,7 +38,6 @@ pub struct Initial { pub signed_header: SignedHeader, pub next_validator_set: ValidatorSet, pub trusting_period: DurationStr, - #[serde(with = "tendermint::serializers::time")] pub now: Time, } @@ -49,7 +48,6 @@ pub struct TestBisection { pub primary: Provider, pub witnesses: Vec>, pub height_to_verify: HeightStr, - #[serde(with = "tendermint::serializers::time")] pub now: Time, pub expected_output: Option, pub expected_num_of_bisections: usize, diff --git a/light-client/tests/model_based.rs b/light-client/tests/model_based.rs index be3675451..90e40a19d 100644 --- a/light-client/tests/model_based.rs +++ b/light-client/tests/model_based.rs @@ -62,7 +62,6 @@ pub struct SingleStepTestCase { pub struct BlockVerdict { block: AnonLightBlock, testgen_block: TestgenLightBlock, - #[serde(with = "tendermint::serializers::time")] now: Time, verdict: LiteVerdict, } diff --git a/rpc/src/endpoint/net_info.rs b/rpc/src/endpoint/net_info.rs index dad3b7d56..5be6b08cd 100644 --- a/rpc/src/endpoint/net_info.rs +++ b/rpc/src/endpoint/net_info.rs @@ -92,7 +92,7 @@ pub struct Monitor { pub active: bool, /// When the monitor started - #[serde(rename = "Start", with = "serializers::time")] + #[serde(rename = "Start")] pub start: Time, /// Duration of this monitor diff --git a/rpc/src/endpoint/status.rs b/rpc/src/endpoint/status.rs index 119ee6514..59e4b93bd 100644 --- a/rpc/src/endpoint/status.rs +++ b/rpc/src/endpoint/status.rs @@ -46,7 +46,6 @@ pub struct SyncInfo { pub latest_block_height: block::Height, /// Latest block time - #[serde(with = "tendermint::serializers::time")] pub latest_block_time: Time, /// Are we catching up? diff --git a/tendermint/src/genesis.rs b/tendermint/src/genesis.rs index f9ba90cb3..9649d9c32 100644 --- a/tendermint/src/genesis.rs +++ b/tendermint/src/genesis.rs @@ -11,7 +11,6 @@ use tendermint_proto::google::protobuf::Timestamp; #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Genesis { /// Time of genesis - #[serde(with = "crate::serializers::time")] pub genesis_time: Time, /// Chain ID diff --git a/tendermint/src/serializers.rs b/tendermint/src/serializers.rs index 8a0efc8b9..7650afdf9 100644 --- a/tendermint/src/serializers.rs +++ b/tendermint/src/serializers.rs @@ -10,4 +10,3 @@ pub use tendermint_proto::serializers::*; pub mod apphash; pub mod hash; pub mod option_hash; -pub mod time; diff --git a/tendermint/src/serializers/time.rs b/tendermint/src/serializers/time.rs deleted file mode 100644 index daaa5d647..000000000 --- a/tendermint/src/serializers/time.rs +++ /dev/null @@ -1,24 +0,0 @@ -//! Time serialization with validation - -use crate::Time; -use serde::{Deserializer, Serializer}; -use std::convert::TryFrom; -use tendermint_proto::google::protobuf::Timestamp; - -/// Deserialize string into Time -pub fn deserialize<'de, D>(deserializer: D) -> Result -where - D: Deserializer<'de>, -{ - let timestamp = tendermint_proto::serializers::timestamp::deserialize(deserializer)?; - Time::try_from(timestamp).map_err(serde::de::Error::custom) -} - -/// Serialize from Time into string -pub fn serialize(value: &Time, serializer: S) -> Result -where - S: Serializer, -{ - let timestamp: Timestamp = value.clone().into(); - tendermint_proto::serializers::timestamp::serialize(×tamp, serializer) -} From 8cf859d7748b3341ddd8939b38c2fbe52c33d679 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 10 Nov 2020 10:55:09 +0100 Subject: [PATCH 4/4] Fix whitespace in changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index eddbc03f3..831f855e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ [#639]: https://github.com/informalsystems/tendermint-rs/pull/639 [#660]: https://github.com/informalsystems/tendermint-rs/issues/660 [#665]: https://github.com/informalsystems/tendermint-rs/issues/665 + ## v0.17.0-rc1 *Oct 15, 2020*