From 8a1016729459c101aa8d6955f09e2089db5f931b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Wed, 16 Jun 2021 11:30:09 +0200 Subject: [PATCH 1/5] [config] allow to omit optional values --- tendermint/src/config.rs | 48 +++++++++++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/tendermint/src/config.rs b/tendermint/src/config.rs index 5d21dc381..ebd7d010f 100644 --- a/tendermint/src/config.rs +++ b/tendermint/src/config.rs @@ -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, /// Path to the JSON file containing the private key to use for node authentication in the p2p @@ -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, /// Maximum number of simultaneous GRPC connections. @@ -331,15 +337,24 @@ 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, /// 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, /// pprof listen address - #[serde(deserialize_with = "deserialize_optional_value")] + #[serde( + deserialize_with = "deserialize_optional_value", + serialize_with = "serialize_optional_value" + )] pub pprof_laddr: Option, } @@ -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, /// Comma separated list of seed nodes to connect to @@ -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, /// Maximum number of transactions in the mempool @@ -686,7 +707,7 @@ where T: FromStr, E: fmt::Display, { - let string = String::deserialize(deserializer)?; + let string = Option::::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(value: &Option, serializer: S) -> Result +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, D::Error> where From 4c51bc75a03af72b7c286bc5681c9586030a423c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Wed, 16 Jun 2021 11:30:59 +0200 Subject: [PATCH 2/5] [test] add config parsing roundtrip test --- tendermint/tests/config.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tendermint/tests/config.rs b/tendermint/tests/config.rs index 3c86ec6cf..6a0106e79 100644 --- a/tendermint/tests/config.rs +++ b/tendermint/tests/config.rs @@ -216,4 +216,17 @@ 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); + } } From 622227f55f466f701587445596f64b65798e6ed0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Wed, 16 Jun 2021 13:19:37 +0200 Subject: [PATCH 3/5] fix address display to include peer_id, if any --- tendermint/src/net.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tendermint/src/net.rs b/tendermint/src/net.rs index f43cf458a..3e48f2322 100644 --- a/tendermint/src/net.rs +++ b/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()), } } From 0c3d4bbffa82ad563e86d9bc701f83f26ea83fc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Wed, 16 Jun 2021 13:20:11 +0200 Subject: [PATCH 4/5] test config parsing roundtrip equality --- tendermint/Cargo.toml | 1 + tendermint/src/config.rs | 26 +++++++++++++------------- tendermint/src/timeout.rs | 2 +- tendermint/tests/config.rs | 10 ++++++++-- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/tendermint/Cargo.toml b/tendermint/Cargo.toml index 518bc67c1..e3ae5882b 100644 --- a/tendermint/Cargo.toml +++ b/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" } diff --git a/tendermint/src/config.rs b/tendermint/src/config.rs index ebd7d010f..9b1682661 100644 --- a/tendermint/src/config.rs +++ b/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. @@ -285,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, @@ -360,7 +360,7 @@ pub struct RpcConfig { /// 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 for CorsOrigin { @@ -377,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 for CorsMethod { @@ -394,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 for CorsHeader { @@ -410,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, @@ -504,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, @@ -547,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, @@ -596,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)] @@ -624,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. @@ -642,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 @@ -681,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 @@ -690,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 { diff --git a/tendermint/src/timeout.rs b/tendermint/src/timeout.rs index f861a7fde..9a1bc63b3 100644 --- a/tendermint/src/timeout.rs +++ b/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 { diff --git a/tendermint/tests/config.rs b/tendermint/tests/config.rs index 6a0106e79..60d947a74 100644 --- a/tendermint/tests/config.rs +++ b/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}; @@ -225,8 +227,12 @@ mod files { 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(); + let written_config = TendermintConfig::parse_toml(&written_config_toml).unwrap(); - // assert_eq!(config, written_config); + assert_eq!( + config, written_config, + "written config {}", + written_config_toml + ); } } From eacdc2a0e4547f55af22870188a2ce0149feafa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Wed, 16 Jun 2021 13:30:55 +0200 Subject: [PATCH 5/5] add entry in .changelog --- .changelog/unreleased/bug-fixes/908-config-opt-values.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 .changelog/unreleased/bug-fixes/908-config-opt-values.md diff --git a/.changelog/unreleased/bug-fixes/908-config-opt-values.md b/.changelog/unreleased/bug-fixes/908-config-opt-values.md new file mode 100644 index 000000000..40f7f4498 --- /dev/null +++ b/.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))