From d06151d1f7928011d6b4c025bf4d344b8b6fed37 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi <106849+romac@users.noreply.github.com> Date: Mon, 22 Jan 2024 14:54:16 +0100 Subject: [PATCH] Improve error message when deserializing an invalid chain config Error messages when deserializing an invalid chain config have gotten a lot worse since https://github.com/informalsystems/hermes/pull/3787. For example, when mistyping the `gas_multiplier` field as `gas_multiplie` in the per-chain config: ``` data did not match any variant of untagged enum ChainConfig for key `chains` at line 424 column 1 ``` After this commit (same message as before #3787): ``` invalid CosmosSdk config: unknown field `gas_multiplie`, expected one of `id`, `rpc_addr`, `grpc_addr`, `event_source`, `rpc_timeout`, `trusted_node`, `account_prefix`, `key_name`, `key_store_type`, `key_store_folder`, `store_prefix`, `default_gas`, `max_gas`, `genesis_restart`, `gas_adjustment`, `gas_multiplier`, `fee_granter`, `max_msg_num`, `max_tx_size`, `max_grpc_decoding_size`, `query_packets_chunk_size`, `clock_drift`, `max_block_time`, `trusting_period`, `client_refresh_rate`, `ccv_consumer_chain`, `memo_prefix`, `sequential_batch_tx`, `proof_specs`, `trust_threshold`, `gas_price`, `packet_filter`, `address_type`, `extension_options`, `compat_mode`, `clear_interval` for key `chains` at line 424 column 1 ``` For this, we now use a custom deserializer for ChainConfig instead of relying on an untagged enum. --- crates/relayer/src/config.rs | 46 ++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/crates/relayer/src/config.rs b/crates/relayer/src/config.rs index 10721a7348..960f9fc45a 100644 --- a/crates/relayer/src/config.rs +++ b/crates/relayer/src/config.rs @@ -618,18 +618,16 @@ pub enum EventSourceMode { }, } -// NOTE: -// To work around a limitation of serde, which does not allow +// NOTE: To work around a limitation of serde, which does not allow // to specify a default variant if not tag is present, -// every underlying chain config MUST have a field `r#type` of -// type `monotstate::MustBe!("VariantName")`. +// every underlying chain config struct MUST have a field `r#type` with +// type `monotstate::MustBe!("VariantName")`, eg. the `CosmosSdkConfig` +// struct has a field `r#type: MustBe!("CosmosSdk")`. // -// For chains other than CosmosSdk, this field MUST NOT be annotated -// with `#[serde(default)]`. -// -// See https://github.com/serde-rs/serde/issues/2231 -#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] -#[serde(untagged)] +// IMPORTANT: Do not forget to update the `Deserializer` instance +// below when adding a new chain type. +#[derive(Clone, Debug, PartialEq, Serialize)] +#[serde(tag = "type")] pub enum ChainConfig { CosmosSdk(CosmosSdkConfig), } @@ -704,6 +702,34 @@ impl ChainConfig { } } +impl<'de> Deserialize<'de> for ChainConfig { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let value = toml::Value::deserialize(deserializer)?; + + let type_value = value + .get("type") + .cloned() + .unwrap_or_else(|| toml::Value::String("CosmosSdk".to_string())); + + let type_str = type_value + .as_str() + .ok_or_else(|| serde::de::Error::custom("invalid chain type, must be a string"))?; + + match type_str { + "CosmosSdk" => CosmosSdkConfig::deserialize(value) + .map(Self::CosmosSdk) + .map_err(|e| serde::de::Error::custom(format!("invalid CosmosSdk config: {e}"))), + + chain_type => Err(serde::de::Error::custom(format!( + "unknown chain type: {chain_type}", + ))), + } + } +} + /// Attempt to load and parse the TOML config file as a `Config`. pub fn load(path: impl AsRef) -> Result { let config_toml = std::fs::read_to_string(&path).map_err(Error::io)?;