From c0ed9a3a38329b752d4a40821a7107c2cc6eb19e Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Wed, 12 Jul 2023 03:58:03 -0500 Subject: [PATCH] `config auto` now generates a config file even when it encounters an error (#3466) * Stub out code flow * Stub out code flow * Change return type of `hermes_cofig` fn * Define ConfigAutoError type * Add some printlns * Change `get_configs` return type * Change AutoCmd::run * Get it to compile * Fix false reporting of missing chain configs * Change get_data_from_handles * Get it working * Remove some debugging code * Cargo fmt * Update `get_configs` doc comment * Update gas price warning in guide * Cargo fmt --- crates/relayer-cli/src/chain_registry.rs | 189 ++++++++++-------- .../relayer-cli/src/commands/config/auto.rs | 114 +++++++---- .../src/core/ics24_host/identifier.rs | 6 + guide/src/documentation/commands/config.md | 2 +- 4 files changed, 194 insertions(+), 117 deletions(-) diff --git a/crates/relayer-cli/src/chain_registry.rs b/crates/relayer-cli/src/chain_registry.rs index 886953ed6c..305a8d615a 100644 --- a/crates/relayer-cli/src/chain_registry.rs +++ b/crates/relayer-cli/src/chain_registry.rs @@ -196,6 +196,10 @@ where )) } +/// Fetches the specified resources from the Cosmos chain registry, using the specified commit hash +/// if it is provided. Fetching is done in a concurrent fashion by spawning a task for each resource. +/// Returns a vector of handles that need to be awaited in order to access the fetched data, or the +/// error that occurred while fetching. async fn get_handles( resources: &[String], commit: &Option, @@ -211,21 +215,23 @@ async fn get_handles( handles } +/// Given a vector of handles, awaits them and returns a vector of results. Any errors +/// that occurred are mapped to a `RegistryError`. async fn get_data_from_handles( handles: Vec>>, error_task: &str, -) -> Result, RegistryError> { - let data_array: Result, JoinError> = join_all(handles).await.into_iter().collect(); - let data_array: Result, RegistryError> = data_array - .map_err(|e| RegistryError::join_error(error_task.to_string(), e))? +) -> Result>, RegistryError> { + join_all(handles) + .await .into_iter() - .collect(); - - data_array + .collect::, JoinError>>() + .map_err(|e| RegistryError::join_error(error_task.to_string(), e)) } -/// Generates a `Vec` for a slice of chain names by fetching data from -/// . Gas settings are set to default values. +/// Fetches a list of ChainConfigs specified by the given slice of chain names. These +/// configs are fetched from . The `default_gas` +/// and `max_gas` parameters set to default values. The `gas_price` parameter is set to +/// the average gas price for the chain listed in the chain registry. /// /// # Arguments /// @@ -242,7 +248,7 @@ async fn get_data_from_handles( pub async fn get_configs( chains: &[String], commit: Option, -) -> Result, RegistryError> { +) -> Result>, RegistryError> { let n = chains.len(); if n == 0 { @@ -267,11 +273,20 @@ pub async fn get_configs( } // Collect data from the spawned tasks - let chain_data_array = + let chain_data_results = get_data_from_handles::(chain_data_handle, "chain_data_join").await?; - let asset_lists = + let asset_list_results = get_data_from_handles::(asset_lists_handle, "asset_handle_join").await?; + let chain_data_array: Vec = chain_data_results + .into_iter() + .filter_map(|chain_data| chain_data.ok()) + .collect(); + let asset_lists: Vec = asset_list_results + .into_iter() + .filter_map(|asset_list| asset_list.ok()) + .collect(); + let path_data: Result, JoinError> = join_all(path_handles).await.into_iter().collect(); let path_data: Vec = path_data .map_err(|e| RegistryError::join_error("path_handle_join".to_string(), e))? @@ -322,10 +337,16 @@ mod tests { // chain-registry repository: https://github.com/cosmos/chain-registry/tree/master/_IBC async fn should_have_no_filter(test_chains: &[String]) -> Result<(), RegistryError> { let configs = get_configs(test_chains, Some(TEST_COMMIT.to_owned())).await?; + for config in configs { - match config.packet_filter.channel_policy { - ChannelPolicy::AllowAll => {} - _ => panic!("PacketFilter not allowed"), + match config { + Ok(config) => { + assert_eq!(config.packet_filter.channel_policy, ChannelPolicy::AllowAll); + } + Err(e) => panic!( + "Encountered an unexpected error in chain registry test: {}", + e + ), } } @@ -345,73 +366,79 @@ mod tests { let configs = get_configs(test_chains, Some(TEST_COMMIT.to_owned())).await?; for config in configs { - match config.packet_filter.channel_policy { - ChannelPolicy::Allow(channel_filter) => { - if config.id.as_str().contains("cosmoshub") { - assert!(channel_filter.is_exact()); - - let cosmoshub_juno = ( - &PortId::from_str("transfer").unwrap(), - &ChannelId::from_str("channel-207").unwrap(), - ); - - let cosmoshub_osmosis = ( - &PortId::from_str("transfer").unwrap(), - &ChannelId::from_str("channel-141").unwrap(), - ); - - assert!(channel_filter.matches(cosmoshub_juno)); - assert!(channel_filter.matches(cosmoshub_osmosis)); - assert!(channel_filter.len() == 2); - } else if config.id.as_str().contains("juno") { - assert!(channel_filter.is_exact()); - - let juno_cosmoshub = ( - &PortId::from_str("transfer").unwrap(), - &ChannelId::from_str("channel-1").unwrap(), - ); - - let juno_osmosis_1 = ( - &PortId::from_str("transfer").unwrap(), - &ChannelId::from_str("channel-0").unwrap(), - ); - - let juno_osmosis_2 = ( - &PortId::from_str("wasm.juno1v4887y83d6g28puzvt8cl0f3cdhd3y6y9mpysnsp3k8krdm7l6jqgm0rkn").unwrap(), - &ChannelId::from_str("channel-47").unwrap() - ); - - assert!(channel_filter.matches(juno_cosmoshub)); - assert!(channel_filter.matches(juno_osmosis_1)); - assert!(channel_filter.matches(juno_osmosis_2)); - assert!(channel_filter.len() == 3); - } else if config.id.as_str().contains("osmosis") { - assert!(channel_filter.is_exact()); - - let osmosis_cosmoshub = ( - &PortId::from_str("transfer").unwrap(), - &ChannelId::from_str("channel-0").unwrap(), - ); - - let osmosis_juno_1 = ( - &PortId::from_str("transfer").unwrap(), - &ChannelId::from_str("channel-42").unwrap(), - ); - - let osmosis_juno_2 = ( - &PortId::from_str("transfer").unwrap(), - &ChannelId::from_str("channel-169").unwrap(), - ); - - assert!(channel_filter.matches(osmosis_cosmoshub)); - assert!(channel_filter.matches(osmosis_juno_1)); - assert!(channel_filter.matches(osmosis_juno_2)); - assert!(channel_filter.len() == 3); - } else { - panic!("Unknown chain"); + match config { + Ok(config) => match config.packet_filter.channel_policy { + ChannelPolicy::Allow(channel_filter) => { + if config.id.as_str().contains("cosmoshub") { + assert!(channel_filter.is_exact()); + + let cosmoshub_juno = ( + &PortId::from_str("transfer").unwrap(), + &ChannelId::from_str("channel-207").unwrap(), + ); + + let cosmoshub_osmosis = ( + &PortId::from_str("transfer").unwrap(), + &ChannelId::from_str("channel-141").unwrap(), + ); + + assert!(channel_filter.matches(cosmoshub_juno)); + assert!(channel_filter.matches(cosmoshub_osmosis)); + assert!(channel_filter.len() == 2); + } else if config.id.as_str().contains("juno") { + assert!(channel_filter.is_exact()); + + let juno_cosmoshub = ( + &PortId::from_str("transfer").unwrap(), + &ChannelId::from_str("channel-1").unwrap(), + ); + + let juno_osmosis_1 = ( + &PortId::from_str("transfer").unwrap(), + &ChannelId::from_str("channel-0").unwrap(), + ); + + let juno_osmosis_2 = ( + &PortId::from_str("wasm.juno1v4887y83d6g28puzvt8cl0f3cdhd3y6y9mpysnsp3k8krdm7l6jqgm0rkn").unwrap(), + &ChannelId::from_str("channel-47").unwrap() + ); + + assert!(channel_filter.matches(juno_cosmoshub)); + assert!(channel_filter.matches(juno_osmosis_1)); + assert!(channel_filter.matches(juno_osmosis_2)); + assert!(channel_filter.len() == 3); + } else if config.id.as_str().contains("osmosis") { + assert!(channel_filter.is_exact()); + + let osmosis_cosmoshub = ( + &PortId::from_str("transfer").unwrap(), + &ChannelId::from_str("channel-0").unwrap(), + ); + + let osmosis_juno_1 = ( + &PortId::from_str("transfer").unwrap(), + &ChannelId::from_str("channel-42").unwrap(), + ); + + let osmosis_juno_2 = ( + &PortId::from_str("transfer").unwrap(), + &ChannelId::from_str("channel-169").unwrap(), + ); + + assert!(channel_filter.matches(osmosis_cosmoshub)); + assert!(channel_filter.matches(osmosis_juno_1)); + assert!(channel_filter.matches(osmosis_juno_2)); + assert!(channel_filter.len() == 3); + } else { + panic!("Unknown chain"); + } } - } - _ => panic!("PacketFilter not allowed"), + _ => panic!("PacketFilter not allowed"), + }, + Err(e) => panic!( + "Encountered an unexpected error in chain registry test: {}", + e + ), } } diff --git a/crates/relayer-cli/src/commands/config/auto.rs b/crates/relayer-cli/src/commands/config/auto.rs index 229bba7fa1..6736f0dd76 100644 --- a/crates/relayer-cli/src/commands/config/auto.rs +++ b/crates/relayer-cli/src/commands/config/auto.rs @@ -7,6 +7,7 @@ use crate::conclude::Output; use ibc_relayer::config::{store, ChainConfig, Config}; use ibc_relayer::keyring::list_keys; +use std::collections::HashSet; use std::path::PathBuf; use tracing::{info, warn}; @@ -75,6 +76,7 @@ impl Runnable for AutoCmd { // Assert that for every chain, a key name is provided let runtime = tokio::runtime::Runtime::new().unwrap(); + // Extract keys and sort chains by name let names_and_keys = extract_chains_and_keys(&self.chain_names); let sorted_names = names_and_keys .iter() @@ -82,52 +84,94 @@ impl Runnable for AutoCmd { .cloned() .collect::>(); + let sorted_names_set: HashSet = HashSet::from_iter(sorted_names.iter().cloned()); + let commit = self.commit.clone(); - // Extract keys and sort chains by name // Fetch chain configs from the chain registry - info!("Fetching configuration for chains: {sorted_names:?}"); - - match runtime.block_on(get_configs(&sorted_names, commit)) { - Ok(mut chain_configs) => { - let configs_and_keys = chain_configs - .iter_mut() - .zip(names_and_keys.iter().map(|n| &n.1).cloned()); - - for (chain_config, key_option) in configs_and_keys { - // If a key is provided, use it - if let Some(key_name) = key_option { - info!("{}: uses key \"{}\"", &chain_config.id, &key_name); - chain_config.key_name = key_name; - } else { - // Otherwise, find the key in the keystore - let chain_id = &chain_config.id; - let key = find_key(chain_config); - if let Some(key) = key { - info!("{}: uses key '{}'", &chain_id, &key); - chain_config.key_name = key; - } else { - // If no key is found, warn the user and continue - warn!("No key found for chain: {}", chain_id); - } - } + let config_results = runtime.block_on(get_configs(&sorted_names, commit)); + + if let Err(e) = config_results { + let config = Config::default(); + + match store(&config, &self.path) { + Ok(_) => Output::error(format!( + "An error occurred while generating the chain config file: {} + A default config file has been written at '{}'", + e, + self.path.display(), + )) + .exit(), + Err(e) => Output::error(format!( + "An error occurred while attempting to write the config file: {}", + e + )) + .exit(), + } + }; + + let mut chain_configs: Vec = config_results + .unwrap() + .into_iter() + .filter_map(|r| r.ok()) + .collect(); + + // Determine which chains were not fetched + let fetched_chains_set = HashSet::from_iter(chain_configs.iter().map(|c| c.id.name())); + let missing_chains_set: HashSet<_> = + sorted_names_set.difference(&fetched_chains_set).collect(); + + let configs_and_keys = chain_configs + .iter_mut() + .zip(names_and_keys.iter().map(|n| &n.1).cloned()); + + for (chain_config, key_option) in configs_and_keys { + // If a key is provided, use it + if let Some(key_name) = key_option { + info!("{}: uses key \"{}\"", &chain_config.id, &key_name); + chain_config.key_name = key_name; + } else { + // Otherwise, find the key in the keystore + let chain_id = &chain_config.id; + let key = find_key(chain_config); + if let Some(key) = key { + info!("{}: uses key '{}'", &chain_id, &key); + chain_config.key_name = key; + } else { + // If no key is found, warn the user and continue + warn!("No key found for chain: {}", chain_id); } + } + } - let config = Config { - chains: chain_configs, - ..Config::default() - }; + let config = Config { + chains: chain_configs, + ..Config::default() + }; - match store(&config, &self.path) { - Ok(_) => Output::success_msg(format!( + match store(&config, &self.path) { + Ok(_) => { + if missing_chains_set.is_empty() { + Output::success_msg(format!( "Config file written successfully at '{}'", self.path.display() )) - .exit(), - Err(e) => Output::error(e).exit(), + .exit() + } else { + Output::success_msg(format!( + "Config file written successfully at '{}'. + However, configurations for the following chains were not able to be generated: {:?}", + self.path.display(), + missing_chains_set, + )) + .exit() } } - Err(e) => Output::error(e).exit(), + Err(e) => Output::error(format!( + "An error occurred while attempting to write the config file: {}", + e + )) + .exit(), } } } diff --git a/crates/relayer-types/src/core/ics24_host/identifier.rs b/crates/relayer-types/src/core/ics24_host/identifier.rs index a98696a982..15713ec044 100644 --- a/crates/relayer-types/src/core/ics24_host/identifier.rs +++ b/crates/relayer-types/src/core/ics24_host/identifier.rs @@ -64,6 +64,12 @@ impl ChainId { self.version } + /// Extract the chain name from this chain identifier. The chain name + /// consists of the first part of the identifier, before the dash. + pub fn name(&self) -> String { + self.id.split('-').take(1).collect::>().join("") + } + /// Extract the version from the given chain identifier. /// ``` /// use ibc_relayer_types::core::ics24_host::identifier::ChainId; diff --git a/guide/src/documentation/commands/config.md b/guide/src/documentation/commands/config.md index 2f0fed8204..793a1a523c 100644 --- a/guide/src/documentation/commands/config.md +++ b/guide/src/documentation/commands/config.md @@ -17,7 +17,7 @@ The available sub-commands are the following: ### Automatically generate configuration files for specified chains Use `config auto` to automatically generate a configuration file from the [chain-registry](https://github.com/cosmos/chain-registry). -> __WARNING__: Currently, gas parameters are set to default value and require to be set manually. +> __WARNING__: Currently, `default_gas` and `max_gas` parameters are set to default values; these should be manually reset. The `gas_price` parameter is set as the average gas price listed for the chain in the chain registry. ``` {{#include ../../templates/help_templates/config/auto.md}}