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)) 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 5d21dc381..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. @@ -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 @@ -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, /// 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, /// 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, } /// 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 { @@ -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 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 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, /// 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, /// 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, 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 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()), } } 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 3c86ec6cf..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}; @@ -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 + ); + } }