From ad3b00d15292a70118b689a82499c91e04ed2b54 Mon Sep 17 00:00:00 2001 From: Greg Szabo <16846635+greg-szabo@users.noreply.github.com> Date: Wed, 18 Nov 2020 14:07:13 -0500 Subject: [PATCH] consensus::State deserialization fixes (#680) * consensus::State deserialization fixes * Updated CHANGELOG * Deserialize consensus::state from string or integer --- CHANGELOG.md | 2 + proto-compiler/src/constants.rs | 2 + proto/src/prost/tendermint.types.rs | 1 + proto/src/serializers.rs | 1 + proto/src/serializers/from_str.rs | 4 +- proto/src/serializers/optional.rs | 4 +- .../src/serializers/part_set_header_total.rs | 51 ++++++++++++++++ tendermint/src/block/parts.rs | 6 +- tendermint/src/consensus/state.rs | 60 +++++++++++++++++++ 9 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 proto/src/serializers/part_set_header_total.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d26fc719..dc83b5786 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - `[tendermint]` (Since v0.17.0-rc2) Fix abci::Data serialization to base64-encoded string. ([#667]) - `[tendermint]` (Since v0.17.0-rc2) Simplify abci::Log serialization ([#667]) +- `[tendermint]` (Since v0.17.0-rc2) consensus::State backwards compatibility for deserialization ([#679]) ### IMPROVEMENTS: @@ -20,6 +21,7 @@ [#653]: https://github.com/informalsystems/tendermint-rs/pull/653 [#667]: https://github.com/informalsystems/tendermint-rs/issues/667 [#672]: https://github.com/informalsystems/tendermint-rs/pull/672 +[#679]: https://github.com/informalsystems/tendermint-rs/issues/679 ## v0.17.0-rc2 diff --git a/proto-compiler/src/constants.rs b/proto-compiler/src/constants.rs index 53929c51a..34b1bfd00 100644 --- a/proto-compiler/src/constants.rs +++ b/proto-compiler/src/constants.rs @@ -25,6 +25,7 @@ const NULLABLEVECARRAY: &str = r#"#[serde(with = "crate::serializers::txs")]"#; const NULLABLE: &str = r#"#[serde(with = "crate::serializers::nullable")]"#; const ALIAS_POWER_QUOTED: &str = r#"#[serde(alias = "power", with = "crate::serializers::from_str")]"#; +const PART_SET_HEADER_TOTAL: &str = r#"#[serde(with = "crate::serializers::part_set_header_total")]"#; const RENAME_PUBKEY: &str = r#"#[serde(rename = "tendermint/PubKeyEd25519", with = "crate::serializers::bytes::base64string")]"#; const RENAME_DUPLICATEVOTE: &str = r#"#[serde(rename = "tendermint/DuplicateVoteEvidence")]"#; const RENAME_LIGHTCLIENTATTACK: &str = @@ -97,6 +98,7 @@ pub static CUSTOM_FIELD_ATTRIBUTES: &[(&str, &str)] = &[ ".tendermint.types.CanonicalBlockID.part_set_header", ALIAS_PARTS, ), + (".tendermint.types.PartSetHeader.total", PART_SET_HEADER_TOTAL), (".tendermint.types.PartSetHeader.hash", HEXSTRING), (".tendermint.types.Header.height", QUOTED), (".tendermint.types.Header.time", OPTIONAL), diff --git a/proto/src/prost/tendermint.types.rs b/proto/src/prost/tendermint.types.rs index 6beced76d..a7d70e073 100644 --- a/proto/src/prost/tendermint.types.rs +++ b/proto/src/prost/tendermint.types.rs @@ -35,6 +35,7 @@ pub struct SimpleValidator { #[derive(::serde::Deserialize, ::serde::Serialize)] pub struct PartSetHeader { #[prost(uint32, tag="1")] + #[serde(with = "crate::serializers::part_set_header_total")] pub total: u32, #[prost(bytes, tag="2")] #[serde(with = "crate::serializers::bytes::hexstring")] diff --git a/proto/src/serializers.rs b/proto/src/serializers.rs index 8ab6a0474..20202eb83 100644 --- a/proto/src/serializers.rs +++ b/proto/src/serializers.rs @@ -49,6 +49,7 @@ pub mod evidence; pub mod from_str; pub mod nullable; pub mod optional; +pub mod part_set_header_total; pub mod time_duration; pub mod timestamp; pub mod txs; diff --git a/proto/src/serializers/from_str.rs b/proto/src/serializers/from_str.rs index 098117b1c..9fe5800bb 100644 --- a/proto/src/serializers/from_str.rs +++ b/proto/src/serializers/from_str.rs @@ -1,6 +1,6 @@ //! Serialize and deserialize any `T` that implements [[std::str::FromStr]] -//! and [[std::fmt::Display]] from or into string. Note this be used for -//! all primitive data types (e.g. . +//! and [[std::fmt::Display]] from or into string. Note this can be used for +//! all primitive data types. use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer}; /// Deserialize string into T diff --git a/proto/src/serializers/optional.rs b/proto/src/serializers/optional.rs index e005b961d..cc14ea2f8 100644 --- a/proto/src/serializers/optional.rs +++ b/proto/src/serializers/optional.rs @@ -1,4 +1,6 @@ //! Serialize/deserialize Option type where T has a serializer/deserializer. +//! Use `null` if the received value equals the Default implementation. +// Todo: Rename this serializer to something like "default_eq_none" to mirror its purpose better. use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// Deserialize Option @@ -7,7 +9,7 @@ where D: Deserializer<'de>, T: Deserialize<'de> + Default + PartialEq, { - Ok(Some(T::deserialize(deserializer)?).filter(|t| t != &T::default())) + Ok(Option::::deserialize(deserializer)?.filter(|t| t != &T::default())) } /// Serialize Option diff --git a/proto/src/serializers/part_set_header_total.rs b/proto/src/serializers/part_set_header_total.rs new file mode 100644 index 000000000..54f6aa2a8 --- /dev/null +++ b/proto/src/serializers/part_set_header_total.rs @@ -0,0 +1,51 @@ +//! Serialize and deserialize part_set_header.total (from string or u32), (into u32 in +//! part_set_header.total). +//! +//! The deserializer is created for backwards compatibility: `total` was changed from a +//! string-quoted integer value into an integer value without quotes in Tendermint Core v0.34.0. +//! This deserializer allows backwards-compatibility by deserializing both ways. +//! See also: https://github.com/informalsystems/tendermint-rs/issues/679 +use serde::{de::Error, de::Visitor, Deserializer, Serialize, Serializer}; +use std::convert::TryFrom; +use std::fmt::Formatter; + +struct PartSetHeaderTotalStringOrU32; + +/// Deserialize (string or u32) into u32(part_set_header.total) +pub fn deserialize<'de, D>(deserializer: D) -> Result +where + D: Deserializer<'de>, +{ + deserializer.deserialize_any(PartSetHeaderTotalStringOrU32) +} + +/// Serialize from u32(part_set_header.total) into u32 +pub fn serialize(value: &u32, serializer: S) -> Result +where + S: Serializer, +{ + value.serialize(serializer) +} + +impl<'de> Visitor<'de> for PartSetHeaderTotalStringOrU32 { + type Value = u32; + + fn expecting(&self, formatter: &mut Formatter) -> std::fmt::Result { + formatter.write_str("an u32 integer or string between 0 and 2^32") + } + + fn visit_u64(self, v: u64) -> Result + where + E: Error, + { + u32::try_from(v).map_err(|e| E::custom(format!("part_set_header.total {}", e))) + } + + fn visit_str(self, v: &str) -> Result + where + E: Error, + { + v.parse::() + .map_err(|e| E::custom(format!("part_set_header.total {}", e))) + } +} diff --git a/tendermint/src/block/parts.rs b/tendermint/src/block/parts.rs index f5ed01a53..9aa0a7788 100644 --- a/tendermint/src/block/parts.rs +++ b/tendermint/src/block/parts.rs @@ -4,6 +4,7 @@ use crate::hash::Algorithm; use crate::hash::SHA256_HASH_SIZE; use crate::Hash; use crate::{Error, Kind}; +use serde::{Deserialize, Serialize}; use std::convert::TryFrom; use tendermint_proto::types::{ CanonicalPartSetHeader as RawCanonicalPartSetHeader, PartSetHeader as RawPartSetHeader, @@ -11,7 +12,10 @@ use tendermint_proto::types::{ use tendermint_proto::Protobuf; /// Block parts header -#[derive(Clone, Copy, Debug, Default, Hash, Eq, PartialEq, PartialOrd, Ord)] +#[derive( + Clone, Copy, Debug, Default, Hash, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize, +)] +#[serde(try_from = "RawPartSetHeader", into = "RawPartSetHeader")] // Used by KMS state file #[non_exhaustive] pub struct Header { /// Number of parts in this block diff --git a/tendermint/src/consensus/state.rs b/tendermint/src/consensus/state.rs index afa533ccd..5b9cf7d44 100644 --- a/tendermint/src/consensus/state.rs +++ b/tendermint/src/consensus/state.rs @@ -67,6 +67,8 @@ impl PartialOrd for State { mod tests { use super::State; use crate::block; + use crate::Hash; + use std::str::FromStr; #[test] fn state_ord_test() { @@ -103,4 +105,62 @@ mod tests { assert!(oldest < older); assert!(oldest < new); } + + #[test] + fn state_deser_update_null_test() { + // Testing that block_id == null is correctly deserialized. + let state_json_string = r#"{ + "height": "5", + "round": "1", + "step": 6, + "block_id": null + }"#; + let state: State = State { + height: block::Height::from(5_u32), + round: block::Round::from(1_u16), + step: 6, + block_id: None, + }; + let state_from_json: State = serde_json::from_str(state_json_string).unwrap(); + assert_eq!(state_from_json, state); + } + + #[test] + fn state_deser_update_total_test() { + // Testing, if total is correctly deserialized from string. + // Note that we use 'parts' to test backwards compatibility. + let state_json_string = r#"{ + "height": "5", + "round": "1", + "step": 6, + "block_id": { + "hash": "1234567890123456789012345678901234567890123456789012345678901234", + "parts": { + "total": "1", + "hash": "1234567890123456789012345678901234567890123456789012345678901234" + } + } + }"#; + let state: State = State { + height: block::Height::from(5_u32), + round: block::Round::from(1_u16), + step: 6, + block_id: Some(block::Id { + hash: Hash::from_str( + "1234567890123456789012345678901234567890123456789012345678901234", + ) + .unwrap(), + part_set_header: block::parts::Header::new( + 1, + Hash::from_str( + "1234567890123456789012345678901234567890123456789012345678901234", + ) + .unwrap(), + ) + .unwrap(), + }), + }; + let state_from_json: State = serde_json::from_str(state_json_string).unwrap(); + assert_eq!(state_from_json, state); + } }