From 4b7e7ca9bbc829db067310ddbe30af5ce786a097 Mon Sep 17 00:00:00 2001 From: Mattie Conover Date: Wed, 8 Feb 2023 13:12:05 -0800 Subject: [PATCH] Improve config errors (#1776) * Remove path to error as it was not working * Better domain parsing errors * Locked min version of config lib * Improve error messages for config loading --- rust/Cargo.lock | 10 ---- rust/agents/relayer/Cargo.toml | 2 +- rust/agents/scraper/Cargo.toml | 2 +- rust/agents/validator/Cargo.toml | 2 +- rust/hyperlane-base/Cargo.toml | 3 +- rust/hyperlane-base/src/settings/chains.rs | 56 +++++++++++++++------- rust/hyperlane-base/src/settings/loader.rs | 11 +++-- rust/hyperlane-core/Cargo.toml | 2 +- rust/hyperlane-core/src/chain.rs | 4 +- 9 files changed, 54 insertions(+), 38 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 13591ff9f..552d21e9d 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -2718,7 +2718,6 @@ dependencies = [ "rusoto_s3", "serde", "serde_json", - "serde_path_to_error", "thiserror", "tokio", "tracing", @@ -5014,15 +5013,6 @@ dependencies = [ "serde", ] -[[package]] -name = "serde_path_to_error" -version = "0.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "26b04f22b563c91331a10074bda3dd5492e3cc39d56bd557e91c0af42b6c7341" -dependencies = [ - "serde", -] - [[package]] name = "serde_repr" version = "0.1.10" diff --git a/rust/agents/relayer/Cargo.toml b/rust/agents/relayer/Cargo.toml index 8c19598f6..35c7f0ed2 100644 --- a/rust/agents/relayer/Cargo.toml +++ b/rust/agents/relayer/Cargo.toml @@ -6,7 +6,7 @@ edition = "2021" [dependencies] tokio = { version = "1", features = ["rt", "macros"] } coingecko = { git = "https://github.com/hyperlane-xyz/coingecko-rs", tag = "2022-09-14-02" } -config = "0.13" +config = "~0.13.3" serde = {version = "1.0", features = ["derive"]} serde_json = { version = "1.0", default-features = false } ethers = { git = "https://github.com/hyperlane-xyz/ethers-rs", tag = "2023-01-10-04" } diff --git a/rust/agents/scraper/Cargo.toml b/rust/agents/scraper/Cargo.toml index 852bad863..2f63fc9c0 100644 --- a/rust/agents/scraper/Cargo.toml +++ b/rust/agents/scraper/Cargo.toml @@ -6,7 +6,7 @@ edition = "2021" [dependencies] async-trait = { version = "0.1", default-features = false } chrono = "0.4" -config = "0.13" +config = "~0.13.3" ethers = { git = "https://github.com/hyperlane-xyz/ethers-rs", tag = "2023-01-10-04" } eyre = "0.6" itertools = "0.10" diff --git a/rust/agents/validator/Cargo.toml b/rust/agents/validator/Cargo.toml index 4a6894112..d716eaf6a 100644 --- a/rust/agents/validator/Cargo.toml +++ b/rust/agents/validator/Cargo.toml @@ -5,7 +5,7 @@ edition = "2021" [dependencies] tokio = { version = "1", features = ["rt", "macros"] } -config = "0.13" +config = "~0.13.3" serde = "1.0" serde_json = { version = "1.0", default-features = false } ethers = { git = "https://github.com/hyperlane-xyz/ethers-rs", tag = "2023-01-10-04" } diff --git a/rust/hyperlane-base/Cargo.toml b/rust/hyperlane-base/Cargo.toml index ce79ad9ab..68ff3fad7 100644 --- a/rust/hyperlane-base/Cargo.toml +++ b/rust/hyperlane-base/Cargo.toml @@ -7,10 +7,9 @@ edition = "2021" [dependencies] # Main block tokio = { version = "1", features = ["rt", "macros"] } -config = "0.13" +config = "~0.13.3" serde = { version = "1.0", features = ["derive"] } serde_json = { version = "1.0", default-features = false } -serde_path_to_error = "0.1" ethers = { git = "https://github.com/hyperlane-xyz/ethers-rs", tag = "2023-01-10-04" } fuels = "0.33" thiserror = "1.0" diff --git a/rust/hyperlane-base/src/settings/chains.rs b/rust/hyperlane-base/src/settings/chains.rs index 89d65f371..275f05ea4 100644 --- a/rust/hyperlane-base/src/settings/chains.rs +++ b/rust/hyperlane-base/src/settings/chains.rs @@ -143,21 +143,25 @@ impl ChainSetup { &self, metrics: &CoreMetrics, ) -> Result> { + let ctx = "Building provider"; match &self.chain { ChainConf::Ethereum(conf) => { - let locator = self.locator("0x0000000000000000000000000000000000000000")?; + let locator = self + .locator("0x0000000000000000000000000000000000000000") + .context(ctx)?; self.build_ethereum(conf, &locator, metrics, h_eth::HyperlaneProviderBuilder {}) .await } ChainConf::Fuel(_) => todo!(), } - .context("Building provider") + .context(ctx) } /// Try to convert the chain setting into a Mailbox contract pub async fn build_mailbox(&self, metrics: &CoreMetrics) -> Result> { - let locator = self.locator(&self.addresses.mailbox)?; + let ctx = "Building provider"; + let locator = self.locator(&self.addresses.mailbox).context(ctx)?; match &self.chain { ChainConf::Ethereum(conf) => { @@ -166,13 +170,13 @@ impl ChainSetup { } ChainConf::Fuel(conf) => { - let wallet = self.fuel_signer().await?; + let wallet = self.fuel_signer().await.context(ctx)?; hyperlane_fuel::FuelMailbox::new(conf, locator, wallet) .map(|m| Box::new(m) as Box) .map_err(Into::into) } } - .context("Building mailbox") + .context(ctx) } /// Try to convert the chain settings into a mailbox indexer @@ -180,7 +184,8 @@ impl ChainSetup { &self, metrics: &CoreMetrics, ) -> Result> { - let locator = self.locator(&self.addresses.mailbox)?; + let ctx = "Building mailbox indexer"; + let locator = self.locator(&self.addresses.mailbox).context(ctx)?; match &self.chain { ChainConf::Ethereum(conf) => { @@ -197,7 +202,7 @@ impl ChainSetup { ChainConf::Fuel(_) => todo!(), } - .context("Building mailbox indexer") + .context(ctx) } /// Try to convert the chain setting into an interchain gas paymaster @@ -206,7 +211,10 @@ impl ChainSetup { &self, metrics: &CoreMetrics, ) -> Result> { - let locator = self.locator(&self.addresses.interchain_gas_paymaster)?; + let ctx = "Building IGP"; + let locator = self + .locator(&self.addresses.interchain_gas_paymaster) + .context(ctx)?; match &self.chain { ChainConf::Ethereum(conf) => { @@ -221,7 +229,7 @@ impl ChainSetup { ChainConf::Fuel(_) => todo!(), } - .context("Building IGP") + .context(ctx) } /// Try to convert the chain settings into a IGP indexer @@ -229,7 +237,10 @@ impl ChainSetup { &self, metrics: &CoreMetrics, ) -> Result> { - let locator = self.locator(&self.addresses.interchain_gas_paymaster)?; + let ctx = "Building IGP indexer"; + let locator = self + .locator(&self.addresses.interchain_gas_paymaster) + .context(ctx)?; match &self.chain { ChainConf::Ethereum(conf) => { @@ -238,7 +249,12 @@ impl ChainSetup { &locator, metrics, h_eth::InterchainGasPaymasterIndexerBuilder { - mailbox_address: self.addresses.mailbox.parse()?, + mailbox_address: self + .addresses + .mailbox + .parse() + .context("Parsing mailbox address") + .context(ctx)?, finality_blocks: self.finality_blocks(), }, ) @@ -247,7 +263,7 @@ impl ChainSetup { ChainConf::Fuel(_) => todo!(), } - .context("Building IGP indexer") + .context(ctx) } /// Try to convert the chain setting into a Multisig Ism contract @@ -256,8 +272,12 @@ impl ChainSetup { address: H256, metrics: &CoreMetrics, ) -> Result> { + let ctx = "Building multisig ISM"; let locator = ContractLocator { - domain: self.domain()?, + domain: self + .domain() + .context("Invalid domain for locating contract") + .context(ctx)?, address, }; @@ -269,7 +289,7 @@ impl ChainSetup { ChainConf::Fuel(_) => todo!(), } - .context("Building multisig ISM") + .context(ctx) } /// Get the domain for this chain setup @@ -348,15 +368,17 @@ impl ChainSetup { } fn locator(&self, address: &str) -> Result { - let domain = self.domain()?; + let domain = self + .domain() + .context("Invalid domain for locating contract")?; let address = match self.chain { ChainConf::Ethereum(_) => address .parse::() - .context("Invalid ethereum address")? + .context("Invalid ethereum address for locating contract")? .into(), ChainConf::Fuel(_) => address .parse::() - .map_err(|e| eyre!("Invalid fuel contract id: {e}"))? + .map_err(|e| eyre!("Invalid fuel contract id for locating contract: {e}"))? .into_h256(), }; diff --git a/rust/hyperlane-base/src/settings/loader.rs b/rust/hyperlane-base/src/settings/loader.rs index c29754a66..3ce84047c 100644 --- a/rust/hyperlane-base/src/settings/loader.rs +++ b/rust/hyperlane-base/src/settings/loader.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; use std::env; +use std::error::Error; use config::{Config, Environment, File}; use eyre::{Context, Result}; @@ -74,15 +75,19 @@ pub(crate) fn load_settings_object<'de, T: Deserialize<'de>, S: AsRef>( ) .build()?; let formatted_config = format!("{:#?}", config_deserializer).replace('\n', "\\n"); - match serde_path_to_error::deserialize(config_deserializer) { + match Config::try_deserialize(config_deserializer) { Ok(cfg) => Ok(cfg), Err(err) => { println!( "Error during deserialization, showing the config for debugging: {}", formatted_config ); - let ctx = format!("Invalid config at `{}` {:?}", err.path(), err); - Err(err).context(ctx) + if let Some(source_err) = err.source() { + let source = format!("Config error source: {source_err}"); + Err(err).context(source) + } else { + Err(err.into()) + } } } } diff --git a/rust/hyperlane-core/Cargo.toml b/rust/hyperlane-core/Cargo.toml index 37d68cb8f..30d46fd39 100644 --- a/rust/hyperlane-core/Cargo.toml +++ b/rust/hyperlane-core/Cargo.toml @@ -10,7 +10,7 @@ auto_impl = "1.0" ethers = { git = "https://github.com/hyperlane-xyz/ethers-rs", tag = "2023-01-10-04" } ethers-signers = { git = "https://github.com/hyperlane-xyz/ethers-rs", tag = "2023-01-10-04", features=["aws"] } ethers-providers = { git = "https://github.com/hyperlane-xyz/ethers-rs", tag = "2023-01-10-04", features=["ws", "rustls"] } -config = "0.13" +config = "~0.13.3" hex = "0.4.3" sha3 = "0.10" lazy_static = "*" diff --git a/rust/hyperlane-core/src/chain.rs b/rust/hyperlane-core/src/chain.rs index f2344f929..0feecec8d 100644 --- a/rust/hyperlane-core/src/chain.rs +++ b/rust/hyperlane-core/src/chain.rs @@ -259,10 +259,10 @@ impl HyperlaneDomain { if name == domain.as_str() { Ok(HyperlaneDomain::Known(domain)) } else { - Err("Chain name does not match the name of a known domain id; the config is probably wrong.") + Err("Chain name does not match the name of a known domain id; the chain name is probably misspelled.") } } else if name.as_str().parse::().is_ok() { - Err("Chain name is known the domain is incorrect; the config is probably wrong.") + Err("Chain name implies a different domain than the domain id provided; the domain id is probably wrong.") } else { Ok(HyperlaneDomain::Unknown { domain_id,