feat(proto)!: serde feature to enable serde for all messages (#98)

* refactor(proto): move FromMillis/ToMillis to separate mod * feat(proto)!: serde feature to enable serde for all messages * chore: rename deprecated * chore: remove serde from defaults * chore: typos * refactor(proto): add error handling to FromMillis and ToMillis * chore: rabbit's review
dashpay · Oct 11, 2024 · a6da843 · a6da843
1 parent aad72f4
commit a6da843
Show file tree

Hide file tree

Showing 12 changed files with 225 additions and 187 deletions.
diff --git a/abci/Cargo.toml b/abci/Cargo.toml
@@ -40,6 +40,7 @@ crypto = ["dep:lhash"]
 tcp = ["server"]
 unix = ["server"]
 tracing-span = ["dep:uuid"]
+serde = ["tenderdash-proto/serde", "dep:serde_json"]
 
 [[example]]
 name = "echo_socket"
@@ -54,7 +55,7 @@ tracing-subscriber = { version = "0.3.18", optional = true, default-features = f
     "ansi",
     "env-filter",
 ] }
-serde_json = "1.0.115"
+serde_json = { version = "1.0.128", optional = true }
 thiserror = { version = "1.0.58" }
 url = { version = "2.5.0" }
 semver = { version = "1.0.22" }

diff --git a/abci/src/application.rs b/abci/src/application.rs
@@ -193,6 +193,27 @@ impl<A: Application> RequestDispatcher for A {
     }
 }
 
+/// Serialize message for logging.
+///
+/// This macro is used to serialize the message for logging.
+/// When `serde` feature is enabled, it uses `serde_json`, otherwise, it uses
+/// `format!` macro.
+macro_rules! serialize {
+    ($($key:expr => $value:expr),* $(,)?) => {
+        {
+            #[cfg(feature = "serde")]
+            {
+                serde_json::json!({ $($key: $value),* }).to_string()
+            }
+
+            #[cfg(not(feature = "serde"))]
+            {
+                format!(stringify!($($key " {:?}",)*), $($value,)*)
+            }
+        }
+    };
+}
+
 fn serialize_response_for_logging(response: &response::Value) -> String {
     match response {
         response::Value::PrepareProposal(response) => {
@@ -202,10 +223,10 @@ fn serialize_response_for_logging(response: &response::Value) -> String {
                 .map(|tx_record| {
                     // Convert each byte array in tx_record to hex string
                     let tx_hex = hex::encode(&tx_record.tx);
-                    serde_json::json!({
-                        "action": tx_record.action, // Adjust according to actual fields
-                        "tx": tx_hex,
-                    })
+                    serialize!(
+                        "action" => tx_record.action, // Adjust according to actual fields
+                        "tx" => tx_hex,
+                    )
                     .to_string()
                 })
                 .collect();
@@ -219,14 +240,14 @@ fn serialize_response_for_logging(response: &response::Value) -> String {
             let validator_set_update =
                 validator_set_update_to_string(response.validator_set_update.as_ref());
 
-            serde_json::json!({
-                "tx_records": tx_records_hex,
-                "app_hash": app_hash_hex,
-                "tx_results": tx_results_hex,
-                "consensus_param_updates": consensus_params,
-                "core_chain_lock_update": response.core_chain_lock_update,
-                "validator_set_update": validator_set_update,
-            })
+            serialize!(
+                "tx_records" => tx_records_hex,
+                "app_hash" => app_hash_hex,
+                "tx_results" => tx_results_hex,
+                "consensus_param_updates" => consensus_params,
+                "core_chain_lock_update" => response.core_chain_lock_update,
+                "validator_set_update" => validator_set_update,
+            )
             .to_string()
         },
         response::Value::ProcessProposal(response) => {
@@ -246,15 +267,16 @@ fn serialize_response_for_logging(response: &response::Value) -> String {
             let validator_set_update =
                 validator_set_update_to_string(response.validator_set_update.as_ref());
 
-            serde_json::json!({
-                "status": status_string,
-                "app_hash": app_hash_hex,
-                "tx_results": tx_results_hex,
-                "consensus_param_updates": consensus_params,
-                "validator_set_update": validator_set_update,
-            })
+            serialize!(
+                "status" => status_string,
+                "app_hash" => app_hash_hex,
+                "tx_results" => tx_results_hex,
+                "consensus_param_updates" => consensus_params,
+                "validator_set_update" => validator_set_update,
+            )
             .to_string()
         },
+
         value => format!("{:?}", value),
     }
 }
@@ -270,20 +292,21 @@ fn exec_tx_results_to_string(tx_results: &[ExecTxResult]) -> Vec<String> {
             // replace this with the actual serialization of `Event`.
             let events_serialized = format!("{:?}", tx_result.events);
 
-            serde_json::json!({
-                "code": tx_result.code,
-                "data": data_hex,
-                "log": tx_result.log,
-                "info": tx_result.info,
-                "gas_used": tx_result.gas_used,
-                "events": events_serialized,
-                "codespace": tx_result.codespace,
-            })
+            serialize!(
+                "code" => tx_result.code,
+                "data" =>data_hex,
+                "log" => tx_result.log,
+                "info" => tx_result.info,
+                "gas_used" => tx_result.gas_used,
+                "events" => events_serialized,
+                "codespace" => tx_result.codespace,
+            )
             .to_string()
         })
         .collect()
 }
 
+/// Serialize `ValidatorSetUpdate` to string for logging.
 fn validator_set_update_to_string(validator_set_update: Option<&ValidatorSetUpdate>) -> String {
     validator_set_update
         .as_ref()
@@ -295,20 +318,20 @@ fn validator_set_update_to_string(validator_set_update: Option<&ValidatorSetUpda
                 .iter()
                 .map(|validator_update| {
                     let pro_tx_hash_hex = hex::encode(&validator_update.pro_tx_hash);
-                    serde_json::json!({
-                    "pub_key" : validator_update.pub_key,
-                                        "power" :validator_update.power,
-                    "pro_tx_hash" : pro_tx_hash_hex,
-                    "node_address" : validator_update.node_address,
-                                })
+                    serialize!(
+                        "pub_key" => validator_update.pub_key,
+                        "power" => validator_update.power,
+                        "pro_tx_hash" => pro_tx_hash_hex,
+                        "node_address" => validator_update.node_address,
+                    )
                     .to_string()
                 })
                 .collect();
-            serde_json::json!({
-            "validator_updates": validator_updates_string,
-            "threshold_public_key": validator_set_update.threshold_public_key,
-            "quorum_hash": quorum_hash_hex,
-                        })
+            serialize!(
+                "validator_updates" => validator_updates_string,
+                "threshold_public_key" => validator_set_update.threshold_public_key,
+                "quorum_hash" => quorum_hash_hex,
+            )
             .to_string()
         })
         .unwrap_or("None".to_string())

diff --git a/proto-compiler/src/constants.rs b/proto-compiler/src/constants.rs
@@ -43,40 +43,46 @@ pub(crate) const DEFAULT_TENDERDASH_COMMITISH: &str = "v0.10-dev";
 
 /// Predefined custom attributes for message annotations
 const PRIMITIVE_ENUM: &str = r#"#[derive(::num_derive::FromPrimitive, ::num_derive::ToPrimitive)]"#;
-const SERIALIZED: &str = r#"#[derive(::serde::Deserialize, ::serde::Serialize)]"#;
-const TYPE_TAG: &str = r#"#[serde(tag = "type", content = "value")]"#;
+pub(crate) const SERIALIZED: &str =
+    r#"#[cfg_attr(feature = "serde", derive(::serde::Deserialize, ::serde::Serialize))]"#;
+const TYPE_TAG: &str = r#"#[cfg_attr(feature = "serde", serde(tag = "type", content = "value"))]"#;
 /// Predefined custom attributes for field annotations
-const QUOTED: &str = r#"#[serde(with = "crate::serializers::from_str")]"#;
-const QUOTED_WITH_DEFAULT: &str = r#"#[serde(with = "crate::serializers::from_str", default)]"#;
-const DEFAULT: &str = r#"#[serde(default)]"#;
-const HEXSTRING: &str = r#"#[serde(with = "crate::serializers::bytes::hexstring")]"#;
-const BASE64STRING: &str = r#"#[serde(with = "crate::serializers::bytes::base64string")]"#;
-const VEC_BASE64STRING: &str = r#"#[serde(with = "crate::serializers::bytes::vec_base64string")]"#;
-const OPTIONAL: &str = r#"#[serde(with = "crate::serializers::optional")]"#;
+const QUOTED: &str =
+    r#"#[cfg_attr(feature = "serde", serde(with = "crate::serializers::from_str"))]"#;
+const QUOTED_WITH_DEFAULT: &str =
+    r#"#[cfg_attr(feature = "serde", serde(with = "crate::serializers::from_str", default))]"#;
+const DEFAULT: &str = r#"#[cfg_attr(feature = "serde", serde(default))]"#;
+const HEXSTRING: &str =
+    r#"#[cfg_attr(feature = "serde", serde(with = "crate::serializers::bytes::hexstring"))]"#;
+const BASE64STRING: &str =
+    r#"#[cfg_attr(feature = "serde", serde(with = "crate::serializers::bytes::base64string"))]"#;
+const VEC_BASE64STRING: &str = r#"#[cfg_attr(feature = "serde", serde(with = "crate::serializers::bytes::vec_base64string"))]"#;
+const OPTIONAL: &str =
+    r#"#[cfg_attr(feature = "serde", serde(with = "crate::serializers::optional"))]"#;
 // const BYTES_SKIP_IF_EMPTY: &str = r#"#[serde(skip_serializing_if =
 // "bytes::Bytes::is_empty")]"#;
 const DERIVE_FROM_FORWARD: &str = r#"#[from(forward)]"#;
-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 NULLABLEVECARRAY: &str =
+    r#"#[cfg_attr(feature = "serde", serde(with = "crate::serializers::txs"))]"#;
+const NULLABLE: &str =
+    r#"#[cfg_attr(feature = "serde", serde(with = "crate::serializers::nullable"))]"#;
+const ALIAS_POWER_QUOTED: &str = r#"#[cfg_attr(feature = "serde", 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_EDPUBKEY: &str = r#"#[serde(rename = "tenderdash/PubKeyEd25519", with = "crate::serializers::bytes::base64string")]"#;
-const RENAME_SECPPUBKEY: &str = r#"#[serde(rename = "tenderdash/PubKeySecp256k1", with = "crate::serializers::bytes::base64string")]"#;
-const RENAME_SRPUBKEY: &str = r#"#[serde(rename = "tenderdash/PubKeySr25519", with = "crate::serializers::bytes::base64string")]"#;
-const RENAME_DUPLICATEVOTE: &str = r#"#[serde(rename = "tenderdash/DuplicateVoteEvidence")]"#;
+    r#"#[cfg_attr(feature = "serde", serde(with = "crate::serializers::part_set_header_total"))]"#;
+const RENAME_EDPUBKEY: &str = r#"#[cfg_attr(feature = "serde", serde(rename = "tenderdash/PubKeyEd25519", with = "crate::serializers::bytes::base64string"))]"#;
+const RENAME_SECPPUBKEY: &str = r#"#[cfg_attr(feature = "serde", serde(rename = "tenderdash/PubKeySecp256k1", with = "crate::serializers::bytes::base64string"))]"#;
+const RENAME_SRPUBKEY: &str = r#"#[cfg_attr(feature = "serde", serde(rename = "tenderdash/PubKeySr25519", with = "crate::serializers::bytes::base64string"))]"#;
+const RENAME_DUPLICATEVOTE: &str =
+    r#"#[cfg_attr(feature = "serde", serde(rename = "tenderdash/DuplicateVoteEvidence"))]"#;
 const RENAME_LIGHTCLIENTATTACK: &str =
-    r#"#[serde(rename = "tenderdash/LightClientAttackEvidence")]"#;
+    r#"#[cfg_attr(feature = "serde", serde(rename = "tenderdash/LightClientAttackEvidence"))]"#;
 // const EVIDENCE_VARIANT: &str = r#"#[serde(from =
 // "crate::serializers::evidence::EvidenceVariant",
 // into = "crate::serializers::evidence::EvidenceVariant")]"#;
-const ALIAS_VALIDATOR_POWER_QUOTED: &str =
-    r#"#[serde(alias = "ValidatorPower", with = "crate::serializers::from_str")]"#;
-const ALIAS_TOTAL_VOTING_POWER_QUOTED: &str =
-    r#"#[serde(alias = "TotalVotingPower", with = "crate::serializers::from_str")]"#;
-const ALIAS_TIMESTAMP: &str = r#"#[serde(alias = "Timestamp")]"#;
-const ALIAS_PARTS: &str = r#"#[serde(alias = "parts")]"#;
+const ALIAS_VALIDATOR_POWER_QUOTED: &str = r#"#[cfg_attr(feature = "serde", serde(alias = "ValidatorPower", with = "crate::serializers::from_str"))]"#;
+const ALIAS_TOTAL_VOTING_POWER_QUOTED: &str = r#"#[cfg_attr(feature = "serde", serde(alias = "TotalVotingPower", with = "crate::serializers::from_str"))]"#;
+const ALIAS_TIMESTAMP: &str = r#"#[cfg_attr(feature = "serde", serde(alias = "Timestamp"))]"#;
+const ALIAS_PARTS: &str = r#"#[cfg_attr(feature = "serde", serde(alias = "parts"))]"#;
 const DERIVE_FROM: &str = r#"#[derive(derive_more::From)]"#;
 const DERIVE_FROM_STR: &str = r#"#[derive(derive_more::FromStr)]"#;
 /// Custom type attributes applied on top of protobuf structs
@@ -85,54 +91,13 @@ const DERIVE_FROM_STR: &str = r#"#[derive(derive_more::FromStr)]"#;
 /// The first item is a path as defined in the prost_build::Config::btree_map
 /// here: <https://docs.rs/prost-build/0.6.1/prost_build/struct.Config.html#method.btree_map>
 pub static CUSTOM_TYPE_ATTRIBUTES: &[(&str, &str)] = &[
-    (".tendermint.abci.Event", SERIALIZED),
-    (".tendermint.abci.EventAttribute", SERIALIZED),
-    (".tendermint.libs.bits.BitArray", SERIALIZED),
-    (".tendermint.types.BlockIDFlag", PRIMITIVE_ENUM),
-    (".tendermint.types.Block", SERIALIZED),
-    (".tendermint.types.Data", SERIALIZED),
-    (".tendermint.types.EvidenceList", SERIALIZED),
-    (".tendermint.types.Evidence", SERIALIZED),
-    (".tendermint.types.EvidenceVariant", SERIALIZED),
-    (".tendermint.types.DuplicateVoteEvidence", SERIALIZED),
-    (".tendermint.types.Vote", SERIALIZED),
-    (".tendermint.types.BlockID", SERIALIZED),
-    (".tendermint.types.PartSetHeader", SERIALIZED),
-    (".tendermint.types.LightClientAttackEvidence", SERIALIZED),
-    (".tendermint.types.LightBlock", SERIALIZED),
-    (".tendermint.types.SignedHeader", SERIALIZED),
-    (".tendermint.types.Header", SERIALIZED),
-    (".tendermint.version.Consensus", SERIALIZED),
-    (".tendermint.types.Commit", SERIALIZED),
-    (".tendermint.types.CommitSig", SERIALIZED),
-    (".tendermint.types.CoreChainLock", SERIALIZED),
-    (".tendermint.types.ValidatorSet", SERIALIZED),
-    (".tendermint.crypto.PublicKey", SERIALIZED),
     (".tendermint.crypto.PublicKey.sum", TYPE_TAG),
+    (".tendermint.types.BlockIDFlag", PRIMITIVE_ENUM),
     (".tendermint.types.Evidence.sum", TYPE_TAG),
-    (".tendermint.abci.ResponseInfo", SERIALIZED),
+    (".tendermint.abci.Request.value", DERIVE_FROM),
+    (".tendermint.abci.Response.value", DERIVE_FROM),
     (".tendermint.abci.ResponseException", DERIVE_FROM),
     (".tendermint.abci.ResponseException", DERIVE_FROM_STR),
-    (".tendermint.types.CanonicalBlockID", SERIALIZED),
-    (".tendermint.types.CanonicalPartSetHeader", SERIALIZED),
-    (".tendermint.types.Validator", SERIALIZED),
-    (".tendermint.types.CanonicalVote", SERIALIZED),
-    (".tendermint.types.VoteExtension", SERIALIZED),
-    (".tendermint.types.BlockMeta", SERIALIZED),
-    // (".tendermint.types.Evidence", EVIDENCE_VARIANT),
-    (".tendermint.types.TxProof", SERIALIZED),
-    (".tendermint.crypto.Proof", SERIALIZED),
-    (".tendermint.abci.Response.value", DERIVE_FROM),
-    (".tendermint.abci.Request.value", DERIVE_FROM),
-    // Consensus params
-    (".tendermint.types.ConsensusParams", SERIALIZED),
-    (".tendermint.types.ABCIParams", SERIALIZED),
-    (".tendermint.types.BlockParams", SERIALIZED),
-    (".tendermint.types.EvidenceParams", SERIALIZED),
-    (".tendermint.types.ValidatorParams", SERIALIZED),
-    (".tendermint.types.VersionParams", SERIALIZED),
-    (".tendermint.types.SynchronyParams", SERIALIZED),
-    (".tendermint.types.TimeoutParams", SERIALIZED),
 ];
 
 /// Custom field attributes applied on top of protobuf fields in (a) struct(s)

diff --git a/proto-compiler/src/lib.rs b/proto-compiler/src/lib.rs
@@ -95,10 +95,14 @@ pub fn proto_compile(mode: GenerationMode) {
 
     // Compile proto files with added annotations, exchange prost_types to our own
     pb.out_dir(&out_dir);
+    pb.type_attribute(".", constants::SERIALIZED);
     for type_attribute in CUSTOM_TYPE_ATTRIBUTES {
+        println!("[info] => Adding type attribute: {:?}", type_attribute);
         pb.type_attribute(type_attribute.0, type_attribute.1);
     }
+
     for field_attribute in CUSTOM_FIELD_ATTRIBUTES {
+        println!("[info] => Adding field attribute: {:?}", field_attribute);
         pb.field_attribute(field_attribute.0, field_attribute.1);
     }
     // The below in-place path redirection replaces references to the Duration
@@ -124,7 +128,7 @@ pub fn proto_compile(mode: GenerationMode) {
             #[cfg(feature = "grpc")]
             tonic_build::configure()
                 .generate_default_stubs(true)
-                .compile_with_config(pb, &protos, &proto_includes_paths)
+                .compile_protos_with_config(pb, &protos, &proto_includes_paths)
                 .unwrap();
             #[cfg(not(feature = "grpc"))]
             panic!("grpc feature is required to compile {}", mode.to_string());

diff --git a/proto/Cargo.toml b/proto/Cargo.toml
@@ -46,13 +46,17 @@ grpc = [
     "dep:tonic",
 ]
 
+serde = ["dep:serde", "bytes/serde"]
+
 [dependencies]
+bytes = { version = "1.7", default-features = false }
 prost = { version = "0.13", default-features = false, features = [
     "prost-derive",
 ] }
 tonic = { version = "0.12", optional = true }
-bytes = { version = "1.7", default-features = false, features = ["serde"] }
-serde = { version = "1.0.208", default-features = false, features = ["derive"] }
+serde = { version = "1.0.208", default-features = false, features = [
+    "derive",
+], optional = true }
 subtle-encoding = { version = "0.5.1", default-features = false, features = [
     "hex",
     "base64",

diff --git a/proto/src/error.rs b/proto/src/error.rs
@@ -12,6 +12,11 @@ use crate::prelude::*;
 
 define_error! {
     Error {
+        TimeConversion
+            { reason: String }
+            | e | {
+            format!("error converting time: {}", e.reason)
+        },
         TryFromProtobuf
             { reason: String }
             | e | {

diff --git a/proto/src/lib.rs b/proto/src/lib.rs
@@ -44,13 +44,12 @@ pub use tenderdash_nostd::*;
 pub mod tenderdash_grpc;
 #[cfg(feature = "grpc")]
 pub use tenderdash_grpc::*;
-
+#[cfg(feature = "serde")]
 pub mod serializers;
+mod time;
 
 pub use meta::ABCI_VERSION;
-use prelude::*;
-#[cfg(feature = "grpc")]
-pub use tonic;
+pub use prelude::*;
 
 /// Allows for easy Google Protocol Buffers encoding and decoding of domain
 /// types with validation.

diff --git a/proto/src/prelude.rs b/proto/src/prelude.rs
@@ -1,6 +1,6 @@
 // Re-export according to alloc::prelude::v1 because it is not yet stabilized
 // https://doc.rust-lang.org/src/alloc/prelude/v1.rs.html
-#[allow(unused_imports)]
+#![allow(unused_imports)]
 pub use alloc::{
     borrow::ToOwned,
     boxed::Box,
@@ -10,3 +10,8 @@ pub use alloc::{
     vec::Vec,
 };
 pub use core::prelude::v1::*;
+
+#[cfg(feature = "grpc")]
+pub use tonic;
+
+pub use crate::time::{FromMillis, ToMillis};