Serialization improvements

CHANGES.md

@@ -1,3 +1,8 @@
+## Pending
+
+- Serialization refactor [#247]
+- CommitSig enum implemented [#246][#247]
+
 ## [0.13.0] (2020-04-20)
 
 Dependencies:

tendermint/Cargo.toml

@@ -45,6 +45,7 @@ prost-amino-derive = "0.5"
 serde = { version = "1", features = ["derive"] }
 serde_json = { version = "1" }
 serde_bytes = "0.11"
+serde_repr = "0.1"
 sha2 = { version = "0.8", default-features = false }
 signatory = { version = "0.19", features = ["ed25519", "ecdsa"] }
 signatory-dalek = "0.19"

tendermint/src/abci/proof.rs

@@ -19,18 +19,10 @@ pub struct ProofOp {
     #[serde(alias = "type")]
     pub field_type: String,
     /// Key of the ProofOp
-    #[serde(
-        default,
-        serialize_with = "serializers::serialize_base64",
-        deserialize_with = "serializers::parse_base64"
-    )]
+    #[serde(default, with = "serializers::bytes::base64string")]
     pub key: Vec<u8>,
     /// Actual data
-    #[serde(
-        default,
-        serialize_with = "serializers::serialize_base64",
-        deserialize_with = "serializers::parse_base64"
-    )]
+    #[serde(default, with = "serializers::bytes::base64string")]
     pub data: Vec<u8>,
 }
 

tendermint/src/block.rs

@@ -49,10 +49,7 @@ where
     #[derive(Deserialize)]
     struct TmpCommit {
         pub height: Height,
-        #[serde(
-            serialize_with = "serializers::serialize_u64",
-            deserialize_with = "serializers::parse_u64"
-        )]
+        #[serde(with = "serializers::primitives::string")]
         pub round: u64,
         #[serde(deserialize_with = "serializers::parse_non_empty_block_id")]
         pub block_id: Option<Id>,

tendermint/src/block/commit.rs

@@ -17,10 +17,7 @@ pub struct Commit {
     pub height: Height,
 
     /// Round
-    #[serde(
-        serialize_with = "serializers::serialize_u64",
-        deserialize_with = "serializers::parse_u64"
-    )]
+    #[serde(with = "serializers::primitives::string")]
     pub round: u64,
 
     /// Block ID

tendermint/src/block/commit_sig.rs

@@ -1,78 +1,100 @@
 //! CommitSig within Commit
 
-use crate::serializers;
+use crate::serializers::BlockIDFlag;
+use crate::serializers::RawCommitSig;
 use crate::{account, Signature, Time};
-use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer};
+use serde::{Deserialize, Serialize};
+use std::convert::TryFrom;
 
-/// BlockIDFlag is used to indicate the validator has voted either for nil, a particular BlockID or was absent.
-#[derive(Copy, Clone, Debug, PartialEq)]
-pub enum BlockIDFlag {
-    /// BlockIDFlagAbsent - no vote was received from a validator.
-    BlockIDFlagAbsent = 1,
-    /// BlockIDFlagCommit - voted for the Commit.BlockID.
-    BlockIDFlagCommit = 2,
-    /// BlockIDFlagNil - voted for nil.
-    BlockIDFlagNil = 3,
+/// CommitSig represents a signature of a validator.
+/// It's a part of the Commit and can be used to reconstruct the vote set given the validator set.
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+#[serde(try_from = "RawCommitSig")]
+pub enum CommitSig {
+    /// no vote was received from a validator.
+    BlockIDFlagAbsent {
+        /// Validator address
+        validator_address: account::Id,
+    },
+    /// voted for the Commit.BlockID.
+    BlockIDFlagCommit {
+        /// Validator address
+        validator_address: account::Id,
+        /// Timestamp of vote
+        timestamp: Time,
+        /// Signature of vote
+        signature: Signature,
+    },
+    /// voted for nil.
+    BlockIDFlagNil {
+        /// Validator address
+        validator_address: account::Id,
+        /// Timestamp of vote
+        timestamp: Time,
+        /// Signature of vote
+        signature: Signature,
+    },
 }
 
-impl BlockIDFlag {
-    /// Deserialize this type from a byte
-    pub fn from_u8(byte: u8) -> Option<BlockIDFlag> {
-        match byte {
-            1 => Some(BlockIDFlag::BlockIDFlagAbsent),
-            2 => Some(BlockIDFlag::BlockIDFlagCommit),
-            3 => Some(BlockIDFlag::BlockIDFlagNil),
-            _ => None,
+/// CommitSig implementation
+impl CommitSig {
+    /// Helper: Extract validator address, since it's always present (according to ADR-025)
+    pub fn validator_address(&self) -> &account::Id {
+        match &self {
+            CommitSig::BlockIDFlagAbsent { validator_address } => validator_address,
+            CommitSig::BlockIDFlagCommit {
+                validator_address, ..
+            } => validator_address,
+            CommitSig::BlockIDFlagNil {
+                validator_address, ..
+            } => validator_address,
         }
     }
-
-    /// Serialize this type as a byte
-    pub fn to_u8(self) -> u8 {
-        self as u8
-    }
-
-    /// Serialize this type as a 32-bit unsigned integer
-    pub fn to_u32(self) -> u32 {
-        self as u32
-    }
 }
 
-impl Serialize for BlockIDFlag {
-    fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
-        self.to_u8().serialize(serializer)
-    }
-}
+impl TryFrom<RawCommitSig> for CommitSig {
+    type Error = &'static str;
 
-impl<'de> Deserialize<'de> for BlockIDFlag {
-    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
-        let byte = u8::deserialize(deserializer)?;
-        BlockIDFlag::from_u8(byte)
-            .ok_or_else(|| D::Error::custom(format!("invalid block ID flag: {}", byte)))
-    }
-}
-
-/// CommitSig represents a signature of a validator.
-/// It's a part of the Commit and can be used to reconstruct the vote set given the validator set.
-#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)]
-pub struct CommitSig {
-    /// Block ID FLag
-    pub block_id_flag: BlockIDFlag,
-
-    /// Validator address
-    #[serde(deserialize_with = "serializers::parse_non_empty_id")]
-    pub validator_address: Option<account::Id>,
-
-    /// Timestamp
-    pub timestamp: Time,
-
-    /// Signature
-    #[serde(deserialize_with = "serializers::parse_non_empty_signature")]
-    pub signature: Option<Signature>,
-}
-
-impl CommitSig {
-    /// Checks if a validator's vote is absent
-    pub fn is_absent(&self) -> bool {
-        self.block_id_flag == BlockIDFlag::BlockIDFlagAbsent
+    fn try_from(value: RawCommitSig) -> Result<Self, Self::Error> {
+        // Validate CommitSig (strict)
+        match value.block_id_flag {
+            BlockIDFlag::BlockIDFlagAbsent => {
+                if value.timestamp.is_some() {
+                    return Err("timestamp is present for BlockIDFlagAbsent CommitSig");
+                }
+                if value.signature.is_some() {
+                    return Err("signature is present for BlockIDFlagAbsent CommitSig");
+                }
+                Ok(CommitSig::BlockIDFlagAbsent {
+                    validator_address: value.validator_address,
+                })
+            }
+            BlockIDFlag::BlockIDFlagCommit => {
+                if value.timestamp.is_none() {
+                    Err("timestamp is null for BlockIDFlagCommit CommitSig")
+                } else if value.signature.is_none() {
+                    Err("signature is null for BlockIDFlagCommit CommitSig")
+                } else {
+                    Ok(CommitSig::BlockIDFlagCommit {
+                        validator_address: value.validator_address,
+                        timestamp: value.timestamp.unwrap(),
+                        signature: value.signature.unwrap(),
+                    })
+                }
+            }
+            BlockIDFlag::BlockIDFlagNil => {
+                if value.timestamp.is_none() {
+                    Err("timestamp is null for BlockIDFlagNil CommitSig")
+                } else if value.signature.is_none() {
+                    Err("signature is null for BlockIDFlagNil CommitSig")
+                } else {
+                    Ok(CommitSig::BlockIDFlagNil {
+                        validator_address: value.validator_address,
+                        timestamp: value.timestamp.unwrap(),
+                        signature: value.signature.unwrap(),
+                    })
+                }
+            }
+        }
     }
 }

tendermint/src/block/header.rs

@@ -45,10 +45,7 @@ pub struct Header {
     pub consensus_hash: Hash,
 
     /// State after txs from the previous block
-    #[serde(
-        serialize_with = "serializers::serialize_hex",
-        deserialize_with = "serializers::parse_hex"
-    )]
+    #[serde(with = "serializers::bytes::hexstring")]
     pub app_hash: Vec<u8>,
 
     /// Root hash of all results from the txs from the previous block
@@ -70,17 +67,11 @@ pub struct Header {
 #[derive(Serialize, Deserialize, Clone, PartialEq, Debug)]
 pub struct Version {
     /// Block version
-    #[serde(
-        serialize_with = "serializers::serialize_u64",
-        deserialize_with = "serializers::parse_u64"
-    )]
+    #[serde(with = "serializers::primitives::string")]
     pub block: u64,
 
     /// App version
-    #[serde(
-        serialize_with = "serializers::serialize_u64",
-        deserialize_with = "serializers::parse_u64"
-    )]
+    #[serde(with = "serializers::primitives::string")]
     pub app: u64,
 }
 

tendermint/src/block/parts.rs

@@ -10,10 +10,7 @@ use {
 #[derive(Serialize, Deserialize, Clone, Debug, Hash, Eq, PartialEq, PartialOrd, Ord)]
 pub struct Header {
     /// Number of parts in this block
-    #[serde(
-        serialize_with = "serializers::serialize_u64",
-        deserialize_with = "serializers::parse_u64"
-    )]
+    #[serde(with = "serializers::primitives::string")]
     pub total: u64,
 
     /// Hash of the parts set header,

tendermint/src/block/size.rs

@@ -9,16 +9,10 @@ use {
 #[derive(Serialize, Deserialize, Clone, Debug, Eq, PartialEq)]
 pub struct Size {
     /// Maximum number of bytes in a block
-    #[serde(
-        serialize_with = "serializers::serialize_u64",
-        deserialize_with = "serializers::parse_u64"
-    )]
+    #[serde(with = "serializers::primitives::string")]
     pub max_bytes: u64,
 
     /// Maximum amount of gas which can be spent on a block
-    #[serde(
-        serialize_with = "serializers::serialize_i64",
-        deserialize_with = "serializers::parse_i64"
-    )]
+    #[serde(with = "serializers::primitives::string")]
     pub max_gas: i64,
 }

tendermint/src/channel.rs

@@ -15,35 +15,19 @@ pub struct Channel {
     pub id: Id,
 
     /// Capacity of the send queue
-    #[serde(
-        rename = "SendQueueCapacity",
-        serialize_with = "serializers::serialize_u64",
-        deserialize_with = "serializers::parse_u64"
-    )]
+    #[serde(rename = "SendQueueCapacity", with = "serializers::primitives::string")]
     pub send_queue_capacity: u64,
 
     /// Size of the send queue
-    #[serde(
-        rename = "SendQueueSize",
-        serialize_with = "serializers::serialize_u64",
-        deserialize_with = "serializers::parse_u64"
-    )]
+    #[serde(rename = "SendQueueSize", with = "serializers::primitives::string")]
     pub send_queue_size: u64,
 
     /// Priority value
-    #[serde(
-        rename = "Priority",
-        serialize_with = "serializers::serialize_u64",
-        deserialize_with = "serializers::parse_u64"
-    )]
+    #[serde(rename = "Priority", with = "serializers::primitives::string")]
     pub priority: u64,
 
     /// Amount of data recently sent
-    #[serde(
-        rename = "RecentlySent",
-        serialize_with = "serializers::serialize_u64",
-        deserialize_with = "serializers::parse_u64"
-    )]
+    #[serde(rename = "RecentlySent", with = "serializers::primitives::string")]
     pub recently_sent: u64,
 }
 

tendermint/src/consensus/state.rs

@@ -18,10 +18,7 @@ pub struct State {
     pub height: block::Height,
 
     /// Current consensus round
-    #[serde(
-        serialize_with = "serializers::serialize_i64",
-        deserialize_with = "serializers::parse_i64"
-    )]
+    #[serde(with = "serializers::primitives::string")]
     pub round: i64,
 
     /// Current consensus step

tendermint/src/evidence.rs

@@ -70,10 +70,7 @@ impl AsRef<[Evidence]> for Data {
 #[derive(Deserialize, Serialize, Clone, Debug, Eq, PartialEq)]
 pub struct Params {
     /// Maximum allowed age for evidence to be collected
-    #[serde(
-        serialize_with = "serializers::serialize_u64",
-        deserialize_with = "serializers::parse_u64"
-    )]
+    #[serde(with = "serializers::primitives::string")]
     pub max_age_num_blocks: u64,
 
     /// Max age duration
@@ -84,13 +81,7 @@ pub struct Params {
 /// essentially, to keep the usages look cleaner
 /// i.e. you can avoid using serde annotations everywhere
 #[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)]
-pub struct Duration(
-    #[serde(
-        serialize_with = "serializers::serialize_duration",
-        deserialize_with = "serializers::parse_duration"
-    )]
-    std::time::Duration,
-);
+pub struct Duration(#[serde(with = "serializers::timeduration::string")] std::time::Duration);
 
 impl From<Duration> for std::time::Duration {
     fn from(d: Duration) -> std::time::Duration {

tendermint/src/lib.rs

@@ -39,7 +39,7 @@ pub mod node;
 pub mod private_key;
 pub mod public_key;
 pub mod rpc;
-mod serializers;
+pub mod serializers;
 pub mod signature;
 pub mod time;
 mod timeout;

tendermint/src/lite/types.rs

@@ -90,15 +90,9 @@ pub trait TrustThreshold: Copy + Clone + Debug + Serialize + DeserializeOwned {
 /// [`TrustThreshold`] which can be passed into all relevant methods.
 #[derive(Copy, Clone, Debug, PartialEq, Serialize, Deserialize)]
 pub struct TrustThresholdFraction {
-    #[serde(
-        serialize_with = "serializers::serialize_u64",
-        deserialize_with = "serializers::parse_u64"
-    )]
+    #[serde(with = "serializers::primitives::string")]
     numerator: u64,
-    #[serde(
-        serialize_with = "serializers::serialize_u64",
-        deserialize_with = "serializers::parse_u64"
-    )]
+    #[serde(with = "serializers::primitives::string")]
     denominator: u64,
 }