consensus::State deserialization fixes (#680)

* consensus::State deserialization fixes

* Updated CHANGELOG

* Deserialize consensus::state from string or integer

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
 

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),

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")]

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;

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

proto/src/serializers/optional.rs

@@ -1,4 +1,6 @@
 //! Serialize/deserialize Option<T> 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<T>
@@ -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::<T>::deserialize(deserializer)?.filter(|t| t != &T::default()))
 }
 
 /// Serialize Option<T>

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<u32, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    deserializer.deserialize_any(PartSetHeaderTotalStringOrU32)
+}
+
+/// Serialize from u32(part_set_header.total) into u32
+pub fn serialize<S>(value: &u32, serializer: S) -> Result<S::Ok, S::Error>
+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<E>(self, v: u64) -> Result<Self::Value, E>
+    where
+        E: Error,
+    {
+        u32::try_from(v).map_err(|e| E::custom(format!("part_set_header.total {}", e)))
+    }
+
+    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+    where
+        E: Error,
+    {
+        v.parse::<u32>()
+            .map_err(|e| E::custom(format!("part_set_header.total {}", e)))
+    }
+}

tendermint/src/block/parts.rs

@@ -4,14 +4,18 @@ 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,
 };
 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

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);
+    }
 }