Skip to content

Commit

Permalink
Improve error message when deserializing invalid chain config
Browse files Browse the repository at this point in the history
Error messages when deserializing an invalid chain config have gotten
a lot worse since #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 + `monostate::MustBe`.
  • Loading branch information
romac committed Jan 22, 2024
1 parent 90ebd6b commit 8f31834
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 46 deletions.
22 changes: 0 additions & 22 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion crates/relayer-cli/src/chain_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ where
};

Ok(ChainConfig::CosmosSdk(CosmosSdkConfig {
r#type: Default::default(),
id: chain_data.chain_id,
rpc_addr: rpc_data.rpc_address,
grpc_addr: grpc_address,
Expand Down
1 change: 0 additions & 1 deletion crates/relayer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ strum = { version = "0.25", features = ["derive"] }
tokio-stream = "0.1.14"
once_cell = "1.19.0"
tracing-subscriber = { version = "0.3.14", features = ["fmt", "env-filter", "json"] }
monostate = "0.1.11"

[dependencies.byte-unit]
version = "4.0.19"
Expand Down
6 changes: 0 additions & 6 deletions crates/relayer/src/chain/cosmos/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use core::time::Duration;
use std::path::PathBuf;

use byte_unit::Byte;
use monostate::MustBe;
use serde_derive::{Deserialize, Serialize};
use tendermint_rpc::Url;

Expand All @@ -24,11 +23,6 @@ pub mod error;
#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
#[serde(deny_unknown_fields)]
pub struct CosmosSdkConfig {
/// The type of this chain, must be "CosmosSdk"
/// This is the default if not specified.
#[serde(default)]
pub r#type: MustBe!("CosmosSdk"),

/// The chain's network identifier
pub id: ChainId,

Expand Down
47 changes: 32 additions & 15 deletions crates/relayer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,18 +618,8 @@ pub enum EventSourceMode {
},
}

// 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")`.
//
// 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)]
#[derive(Clone, Debug, PartialEq, Serialize)]
#[serde(tag = "type")]
pub enum ChainConfig {
CosmosSdk(CosmosSdkConfig),
}
Expand Down Expand Up @@ -704,6 +694,34 @@ impl ChainConfig {
}
}

impl<'de> Deserialize<'de> for ChainConfig {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
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<Path>) -> Result<Config, Error> {
let config_toml = std::fs::read_to_string(&path).map_err(Error::io)?;
Expand Down Expand Up @@ -779,7 +797,6 @@ mod tests {

use super::{load, parse_gas_prices, store_writer};
use crate::config::GasPrice;
use monostate::MustBe;
use test_log::test;

#[test]
Expand Down Expand Up @@ -850,8 +867,8 @@ mod tests {
let config = load(path).expect("could not parse config");

match config.chains[0] {
super::ChainConfig::CosmosSdk(ref config) => {
assert_eq!(config.r#type, MustBe!("CosmosSdk"));
super::ChainConfig::CosmosSdk(_) => {
// all good
}
}
}
Expand Down
1 change: 0 additions & 1 deletion tools/test-framework/src/types/single/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ impl FullNode {
};

Ok(config::ChainConfig::CosmosSdk(CosmosSdkConfig {
r#type: Default::default(),
id: self.chain_driver.chain_id.clone(),
rpc_addr: Url::from_str(&self.chain_driver.rpc_address())?,
grpc_addr: Url::from_str(&self.chain_driver.grpc_address())?,
Expand Down

0 comments on commit 8f31834

Please sign in to comment.