Tendermint config handle optional values + fix net::Address Display

.changelog/unreleased/bug-fixes/908-config-opt-values.md

@@ -0,0 +1 @@
+- `[tendermint]` Better handling of optional values in TendermintConfig ([#908](https://github.com/informalsystems/tendermint-rs/issues/908))

tendermint/Cargo.toml

@@ -65,5 +65,6 @@ ripemd160 = { version = "0.9", optional = true }
 secp256k1 = ["k256", "ripemd160"]
 
 [dev-dependencies]
+pretty_assertions = "0.7.2"
 proptest = "0.10.1"
 tendermint-pbt-gen = { path = "../pbt-gen" }

tendermint/src/config.rs

@@ -26,7 +26,7 @@ use std::{
 };
 
 /// Tendermint `config.toml` file
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct TendermintConfig {
     /// TCP or UNIX socket address of the ABCI application,
     /// or the name of an ABCI application compiled in with the Tendermint binary.
@@ -64,7 +64,10 @@ pub struct TendermintConfig {
 
     /// TCP or UNIX socket address for Tendermint to listen on for
     /// connections from an external PrivValidator process
-    #[serde(deserialize_with = "deserialize_optional_value")]
+    #[serde(
+        deserialize_with = "deserialize_optional_value",
+        serialize_with = "serialize_optional_value"
+    )]
     pub priv_validator_laddr: Option<net::Address>,
 
     /// Path to the JSON file containing the private key to use for node authentication in the p2p
@@ -282,7 +285,7 @@ pub enum AbciMode {
 }
 
 /// Tendermint `config.toml` file's `[rpc]` section
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct RpcConfig {
     /// TCP or UNIX socket address for the RPC server to listen on
     pub laddr: net::Address,
@@ -300,7 +303,10 @@ pub struct RpcConfig {
 
     /// TCP or UNIX socket address for the gRPC server to listen on
     /// NOTE: This server only supports `/broadcast_tx_commit`
-    #[serde(deserialize_with = "deserialize_optional_value")]
+    #[serde(
+        deserialize_with = "deserialize_optional_value",
+        serialize_with = "serialize_optional_value"
+    )]
     pub grpc_laddr: Option<net::Address>,
 
     /// Maximum number of simultaneous GRPC connections.
@@ -331,21 +337,30 @@ pub struct RpcConfig {
     pub max_header_bytes: u64,
 
     /// The name of a file containing certificate that is used to create the HTTPS server.
-    #[serde(deserialize_with = "deserialize_optional_value")]
+    #[serde(
+        deserialize_with = "deserialize_optional_value",
+        serialize_with = "serialize_optional_value"
+    )]
     pub tls_cert_file: Option<PathBuf>,
 
     /// The name of a file containing matching private key that is used to create the HTTPS server.
-    #[serde(deserialize_with = "deserialize_optional_value")]
+    #[serde(
+        deserialize_with = "deserialize_optional_value",
+        serialize_with = "serialize_optional_value"
+    )]
     pub tls_key_file: Option<PathBuf>,
 
     /// pprof listen address <https://golang.org/pkg/net/http/pprof>
-    #[serde(deserialize_with = "deserialize_optional_value")]
+    #[serde(
+        deserialize_with = "deserialize_optional_value",
+        serialize_with = "serialize_optional_value"
+    )]
     pub pprof_laddr: Option<net::Address>,
 }
 
 /// Origin hosts allowed with CORS requests to the RPC API
 // TODO(tarcieri): parse and validate this string
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct CorsOrigin(String);
 
 impl AsRef<str> for CorsOrigin {
@@ -362,7 +377,7 @@ impl fmt::Display for CorsOrigin {
 
 /// HTTP methods allowed with CORS requests to the RPC API
 // TODO(tarcieri): parse and validate this string
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct CorsMethod(String);
 
 impl AsRef<str> for CorsMethod {
@@ -379,7 +394,7 @@ impl fmt::Display for CorsMethod {
 
 /// HTTP headers allowed to be sent via CORS to the RPC API
 // TODO(tarcieri): parse and validate this string
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct CorsHeader(String);
 
 impl AsRef<str> for CorsHeader {
@@ -395,7 +410,7 @@ impl fmt::Display for CorsHeader {
 }
 
 /// peer to peer configuration options
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct P2PConfig {
     /// Address to listen for incoming connections
     pub laddr: net::Address,
@@ -404,7 +419,10 @@ pub struct P2PConfig {
     /// If empty, will use the same port as the laddr,
     /// and will introspect on the listener or use UPnP
     /// to figure out the address.
-    #[serde(deserialize_with = "deserialize_optional_value")]
+    #[serde(
+        deserialize_with = "deserialize_optional_value",
+        serialize_with = "serialize_optional_value"
+    )]
     pub external_address: Option<net::Address>,
 
     /// Comma separated list of seed nodes to connect to
@@ -486,7 +504,7 @@ pub struct P2PConfig {
 }
 
 /// mempool configuration options
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct MempoolConfig {
     /// Recheck enabled
     pub recheck: bool,
@@ -495,7 +513,10 @@ pub struct MempoolConfig {
     pub broadcast: bool,
 
     /// WAL dir
-    #[serde(deserialize_with = "deserialize_optional_value")]
+    #[serde(
+        deserialize_with = "deserialize_optional_value",
+        serialize_with = "serialize_optional_value"
+    )]
     pub wal_dir: Option<PathBuf>,
 
     /// Maximum number of transactions in the mempool
@@ -526,7 +547,7 @@ pub struct MempoolConfig {
 }
 
 /// consensus configuration options
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct ConsensusConfig {
     /// Path to WAL file
     pub wal_file: PathBuf,
@@ -575,7 +596,7 @@ pub struct ConsensusConfig {
 }
 
 /// transactions indexer configuration options
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct TxIndexConfig {
     /// What indexer to use for transactions
     #[serde(default)]
@@ -603,7 +624,7 @@ impl Default for TxIndexer {
 }
 
 /// instrumentation configuration options
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct InstrumentationConfig {
     /// When `true`, Prometheus metrics are served under /metrics on
     /// PrometheusListenAddr.
@@ -621,7 +642,7 @@ pub struct InstrumentationConfig {
 }
 
 /// statesync configuration options
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct StatesyncConfig {
     /// State sync rapidly bootstraps a new node by discovering, fetching, and restoring a state machine
     /// snapshot from peers instead of fetching and replaying historical blocks. Requires some peers in
@@ -660,7 +681,7 @@ pub struct StatesyncConfig {
 }
 
 /// fastsync configuration options
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct FastsyncConfig {
     /// Fast Sync version to use:
     ///   1) "v0" (default) - the legacy fast sync implementation
@@ -669,7 +690,7 @@ pub struct FastsyncConfig {
 }
 
 /// Rate at which bytes can be sent/received
-#[derive(Copy, Clone, Debug, Deserialize, Serialize)]
+#[derive(Copy, Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct TransferRate(u64);
 
 impl TransferRate {
@@ -686,7 +707,7 @@ where
     T: FromStr<Err = E>,
     E: fmt::Display,
 {
-    let string = String::deserialize(deserializer)?;
+    let string = Option::<String>::deserialize(deserializer).map(|str| str.unwrap_or_default())?;
 
     if string.is_empty() {
         return Ok(None);
@@ -698,6 +719,17 @@ where
         .map_err(|e| D::Error::custom(format!("{}", e)))
 }
 
+fn serialize_optional_value<S, T>(value: &Option<T>, serializer: S) -> Result<S::Ok, S::Error>
+where
+    S: ser::Serializer,
+    T: Serialize,
+{
+    match value {
+        Some(value) => value.serialize(serializer),
+        None => "".serialize(serializer),
+    }
+}
+
 /// Deserialize a comma separated list of types that impl `FromStr` as a `Vec`
 fn deserialize_comma_separated_list<'de, D, T, E>(deserializer: D) -> Result<Vec<T>, D::Error>
 where

tendermint/src/net.rs

@@ -57,7 +57,16 @@ impl<'de> Deserialize<'de> for Address {
 impl Display for Address {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            Address::Tcp { host, port, .. } => write!(f, "{}{}:{}", TCP_PREFIX, host, port),
+            Address::Tcp {
+                peer_id: None,
+                host,
+                port,
+            } => write!(f, "{}{}:{}", TCP_PREFIX, host, port),
+            Address::Tcp {
+                peer_id: Some(peer_id),
+                host,
+                port,
+            } => write!(f, "{}{}@{}:{}", TCP_PREFIX, peer_id, host, port),
             Address::Unix { path } => write!(f, "{}{}", UNIX_PREFIX, path.display()),
         }
     }

tendermint/src/timeout.rs

@@ -5,7 +5,7 @@ use serde::{de, de::Error as _, ser, Deserialize, Serialize};
 use std::{fmt, ops::Deref, str::FromStr, time::Duration};
 
 /// Timeout durations
-#[derive(Copy, Clone, Debug)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
 pub struct Timeout(Duration);
 
 impl Deref for Timeout {

tendermint/tests/config.rs

@@ -3,6 +3,8 @@
 //! Test config files are located in the `tests/support/config` subdirectory.
 
 mod files {
+    #[cfg(test)]
+    use pretty_assertions::assert_eq;
     use std::{fs, path::PathBuf, time::Duration};
     use tendermint::{config::*, net, node};
 
@@ -216,4 +218,21 @@ mod files {
             "F26BF4B2A2E84CEB7A53C3F1AE77408779B20064782FBADBDF0E365959EE4534"
         );
     }
+
+    /// Parse an example `config.toml` file to a `TendermintConfig` struct, then
+    /// serialize it and parse again.
+    #[test]
+    fn parsing_roundtrip() {
+        let config_toml = read_fixture("config.toml");
+        let config = TendermintConfig::parse_toml(&config_toml).unwrap();
+
+        let written_config_toml = toml::to_string(&config).unwrap();
+        let written_config = TendermintConfig::parse_toml(&written_config_toml).unwrap();
+
+        assert_eq!(
+            config, written_config,
+            "written config {}",
+            written_config_toml
+        );
+    }
 }