From c7c66dd0e73d4d4e1427252dcd14a09eb619477c Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Tue, 25 Apr 2023 13:11:06 +0200 Subject: [PATCH 1/5] Add configuration for keyring store directory --- config.toml | 4 +++ crates/relayer-cli/src/chain_registry.rs | 1 + crates/relayer-cli/src/commands/keys/add.rs | 16 +++++++--- .../relayer-cli/src/commands/keys/delete.rs | 16 +++++++--- crates/relayer/src/chain/cosmos.rs | 10 ++++-- crates/relayer/src/chain/cosmos/batch.rs | 8 ++++- crates/relayer/src/config.rs | 1 + crates/relayer/src/keyring.rs | 32 +++++++++++++++---- .../src/tests/client_refresh.rs | 19 +++++++---- .../src/bootstrap/binary/chain.rs | 13 +++++--- .../src/bootstrap/nary/chain.rs | 2 +- tools/test-framework/src/types/single/node.rs | 16 ++++++---- 12 files changed, 101 insertions(+), 37 deletions(-) diff --git a/config.toml b/config.toml index 9b5498ad1f..a04433df6d 100644 --- a/config.toml +++ b/config.toml @@ -158,6 +158,10 @@ address_type = { derivation = 'cosmos' } # Recommended value for Cosmos SDK: 'ibc' store_prefix = 'ibc' +# Specify the folder used to store the keys. Optional +# If this is not specified then the hermes home folder is used. +# key_store_folder = '/home/.hermes_keys' + # Gas Parameters # # The term 'gas' is used to denote the amount of computation needed to execute diff --git a/crates/relayer-cli/src/chain_registry.rs b/crates/relayer-cli/src/chain_registry.rs index ef0165cfa4..922d48aa9c 100644 --- a/crates/relayer-cli/src/chain_registry.rs +++ b/crates/relayer-cli/src/chain_registry.rs @@ -116,6 +116,7 @@ where account_prefix: chain_data.bech32_prefix, key_name: String::new(), key_store_type: Store::default(), + key_store_folder: None, store_prefix: "ibc".to_string(), default_gas: Some(100000), max_gas: Some(400000), diff --git a/crates/relayer-cli/src/commands/keys/add.rs b/crates/relayer-cli/src/commands/keys/add.rs index 5b876ece26..fb3e063c35 100644 --- a/crates/relayer-cli/src/commands/keys/add.rs +++ b/crates/relayer-cli/src/commands/keys/add.rs @@ -200,8 +200,12 @@ pub fn add_key( ) -> eyre::Result { let key_pair = match config.r#type { ChainType::CosmosSdk => { - let mut keyring = - KeyRing::new_secp256k1(Store::Test, &config.account_prefix, &config.id)?; + let mut keyring = KeyRing::new_secp256k1( + Store::Test, + &config.account_prefix, + &config.id, + &config.key_store_folder, + )?; check_key_exists(&keyring, key_name, overwrite); @@ -229,8 +233,12 @@ pub fn restore_key( let key_pair = match config.r#type { ChainType::CosmosSdk => { - let mut keyring = - KeyRing::new_secp256k1(Store::Test, &config.account_prefix, &config.id)?; + let mut keyring = KeyRing::new_secp256k1( + Store::Test, + &config.account_prefix, + &config.id, + &config.key_store_folder, + )?; check_key_exists(&keyring, key_name, overwrite); diff --git a/crates/relayer-cli/src/commands/keys/delete.rs b/crates/relayer-cli/src/commands/keys/delete.rs index b27e5ee19f..fdc892afa9 100644 --- a/crates/relayer-cli/src/commands/keys/delete.rs +++ b/crates/relayer-cli/src/commands/keys/delete.rs @@ -115,8 +115,12 @@ impl Runnable for KeysDeleteCmd { pub fn delete_key(config: &ChainConfig, key_name: &str) -> eyre::Result<()> { match config.r#type { ChainType::CosmosSdk => { - let mut keyring = - KeyRing::new_secp256k1(Store::Test, &config.account_prefix, &config.id)?; + let mut keyring = KeyRing::new_secp256k1( + Store::Test, + &config.account_prefix, + &config.id, + &config.key_store_folder, + )?; keyring.remove_key(key_name)?; } } @@ -126,8 +130,12 @@ pub fn delete_key(config: &ChainConfig, key_name: &str) -> eyre::Result<()> { pub fn delete_all_keys(config: &ChainConfig) -> eyre::Result<()> { match config.r#type { ChainType::CosmosSdk => { - let mut keyring = - KeyRing::new_secp256k1(Store::Test, &config.account_prefix, &config.id)?; + let mut keyring = KeyRing::new_secp256k1( + Store::Test, + &config.account_prefix, + &config.id, + &config.key_store_folder, + )?; let keys = keyring.keys()?; for (key_name, _) in keys { keyring.remove_key(&key_name)?; diff --git a/crates/relayer/src/chain/cosmos.rs b/crates/relayer/src/chain/cosmos.rs index 710dd55cb6..13b9fb9282 100644 --- a/crates/relayer/src/chain/cosmos.rs +++ b/crates/relayer/src/chain/cosmos.rs @@ -776,9 +776,13 @@ impl ChainEndpoint for CosmosSdkChain { let light_client = TmLightClient::from_config(&config, node_info.id)?; // Initialize key store and load key - let keybase = - KeyRing::new_secp256k1(config.key_store_type, &config.account_prefix, &config.id) - .map_err(Error::key_base)?; + let keybase = KeyRing::new_secp256k1( + config.key_store_type, + &config.account_prefix, + &config.id, + &config.key_store_folder, + ) + .map_err(Error::key_base)?; let grpc_addr = Uri::from_str(&config.grpc_addr.to_string()) .map_err(|e| Error::invalid_uri(config.grpc_addr.to_string(), e))?; diff --git a/crates/relayer/src/chain/cosmos/batch.rs b/crates/relayer/src/chain/cosmos/batch.rs index f1c60c1f30..f4eb7ad01f 100644 --- a/crates/relayer/src/chain/cosmos/batch.rs +++ b/crates/relayer/src/chain/cosmos/batch.rs @@ -339,7 +339,13 @@ mod tests { "/tests/config/fixtures/relayer-seed.json" ); let seed_file_content = fs::read_to_string(path).unwrap(); - let _keyring = KeyRing::new_secp256k1(keyring::Store::Memory, "cosmos", &chain_id).unwrap(); + let _keyring = KeyRing::new_secp256k1( + keyring::Store::Memory, + "cosmos", + &chain_id, + &chain_config.key_store_folder, + ) + .unwrap(); let hd_path = COSMOS_HD_PATH.parse().unwrap(); let key_pair = Secp256k1KeyPair::from_seed_file(&seed_file_content, &hd_path).unwrap(); diff --git a/crates/relayer/src/config.rs b/crates/relayer/src/config.rs index 851651dc6f..6922c46ae4 100644 --- a/crates/relayer/src/config.rs +++ b/crates/relayer/src/config.rs @@ -448,6 +448,7 @@ pub struct ChainConfig { pub key_name: String, #[serde(default)] pub key_store_type: Store, + pub key_store_folder: Option, pub store_prefix: String, pub default_gas: Option, pub max_gas: Option, diff --git a/crates/relayer/src/keyring.rs b/crates/relayer/src/keyring.rs index 4949f20d03..960c90c8f8 100644 --- a/crates/relayer/src/keyring.rs +++ b/crates/relayer/src/keyring.rs @@ -200,12 +200,17 @@ pub enum KeyRing { } impl KeyRing { - pub fn new(store: Store, account_prefix: &str, chain_id: &ChainId) -> Result { + pub fn new( + store: Store, + account_prefix: &str, + chain_id: &ChainId, + ks_folder: &Option, + ) -> Result { match store { Store::Memory => Ok(Self::Memory(Memory::new(account_prefix.to_string()))), Store::Test => { - let keys_folder = disk_store_path(chain_id.as_str())?; + let keys_folder = disk_store_path(chain_id.as_str(), ks_folder)?; // Create keys folder if it does not exist fs::create_dir_all(&keys_folder).map_err(|e| { @@ -265,8 +270,9 @@ impl KeyRing { store: Store, account_prefix: &str, chain_id: &ChainId, + ks_folder: &Option, ) -> Result { - Self::new(store, account_prefix, chain_id) + Self::new(store, account_prefix, chain_id, ks_folder) } } @@ -275,15 +281,21 @@ impl KeyRing { store: Store, account_prefix: &str, chain_id: &ChainId, + ks_folder: &Option, ) -> Result { - Self::new(store, account_prefix, chain_id) + Self::new(store, account_prefix, chain_id, ks_folder) } } pub fn list_keys(config: &ChainConfig) -> Result, Error> { let keys = match config.r#type { ChainType::CosmosSdk => { - let keyring = KeyRing::new_secp256k1(Store::Test, &config.account_prefix, &config.id)?; + let keyring = KeyRing::new_secp256k1( + Store::Test, + &config.account_prefix, + &config.id, + &config.key_store_folder, + )?; keyring .keys()? .into_iter() @@ -294,11 +306,17 @@ pub fn list_keys(config: &ChainConfig) -> Result Result { +fn disk_store_path(folder_name: &str, keystore_folder: &Option) -> Result { let home = dirs_next::home_dir().ok_or_else(Error::home_location_unavailable)?; + let ks_folder = if let Some(ks) = keystore_folder { + ks + } else { + KEYSTORE_DEFAULT_FOLDER + }; + let folder = Path::new(home.as_path()) - .join(KEYSTORE_DEFAULT_FOLDER) + .join(ks_folder) .join(folder_name) .join(KEYSTORE_DISK_BACKEND); diff --git a/tools/integration-test/src/tests/client_refresh.rs b/tools/integration-test/src/tests/client_refresh.rs index bd7204aedf..c5e79c2e71 100644 --- a/tools/integration-test/src/tests/client_refresh.rs +++ b/tools/integration-test/src/tests/client_refresh.rs @@ -119,15 +119,19 @@ impl TestOverrides for ClientFailsTest { impl BinaryChainTest for ClientFailsTest { fn run( &self, - _config: &TestConfig, + config: &TestConfig, _relayer: RelayerDriver, chains: ConnectedChains, ) -> Result<(), Error> { // Override the configuration in order to use a small `gas_multiplier` which will cause the update client to fail. - let chains2 = override_connected_chains(chains, |config| { - config.chains[0].gas_multiplier = Some(GasMultiplier::unsafe_new(0.8)); - config.chains[1].gas_multiplier = Some(GasMultiplier::unsafe_new(0.8)); - })?; + let chains2 = override_connected_chains( + chains, + |config| { + config.chains[0].gas_multiplier = Some(GasMultiplier::unsafe_new(0.8)); + config.chains[1].gas_multiplier = Some(GasMultiplier::unsafe_new(0.8)); + }, + config, + )?; // Use chains with misconfiguration in order to trigger a ChainError when submitting `MsgClientUpdate` // during the refresh call. @@ -156,6 +160,7 @@ impl BinaryChainTest for ClientFailsTest { fn override_connected_chains( chains: ConnectedChains, config_modifier: impl FnOnce(&mut Config), + test_config: &TestConfig, ) -> Result, Error> where ChainA: ChainHandle, @@ -163,8 +168,8 @@ where { let mut config = Config::default(); - add_chain_config(&mut config, chains.node_a.value())?; - add_chain_config(&mut config, chains.node_b.value())?; + add_chain_config(&mut config, chains.node_a.value(), test_config)?; + add_chain_config(&mut config, chains.node_b.value(), test_config)?; config_modifier(&mut config); diff --git a/tools/test-framework/src/bootstrap/binary/chain.rs b/tools/test-framework/src/bootstrap/binary/chain.rs index 5ed3029807..5ec10402d2 100644 --- a/tools/test-framework/src/bootstrap/binary/chain.rs +++ b/tools/test-framework/src/bootstrap/binary/chain.rs @@ -55,8 +55,8 @@ pub fn bootstrap_chains_with_full_nodes( > { let mut config = Config::default(); - add_chain_config(&mut config, &node_a)?; - add_chain_config(&mut config, &node_b)?; + add_chain_config(&mut config, &node_a, test_config)?; + add_chain_config(&mut config, &node_b, test_config)?; config_modifier(&mut config); @@ -249,8 +249,13 @@ pub fn new_registry(config: Config) -> SharedRegistry Result<(), Error> { - let chain_config = running_node.generate_chain_config(&running_node.chain_driver.chain_type)?; +pub fn add_chain_config( + config: &mut Config, + running_node: &FullNode, + test_config: &TestConfig, +) -> Result<(), Error> { + let chain_config = + running_node.generate_chain_config(&running_node.chain_driver.chain_type, test_config)?; config.chains.push(chain_config); Ok(()) diff --git a/tools/test-framework/src/bootstrap/nary/chain.rs b/tools/test-framework/src/bootstrap/nary/chain.rs index 30b2188266..b0233ecdf5 100644 --- a/tools/test-framework/src/bootstrap/nary/chain.rs +++ b/tools/test-framework/src/bootstrap/nary/chain.rs @@ -60,7 +60,7 @@ pub fn boostrap_chains_with_any_nodes( let mut config = Config::default(); for node in full_nodes.iter() { - add_chain_config(&mut config, node)?; + add_chain_config(&mut config, node, test_config)?; } config_modifier(&mut config); diff --git a/tools/test-framework/src/types/single/node.rs b/tools/test-framework/src/types/single/node.rs index 9facdb0b0a..1fc7feec0f 100644 --- a/tools/test-framework/src/types/single/node.rs +++ b/tools/test-framework/src/types/single/node.rs @@ -18,6 +18,7 @@ use tendermint_rpc::WebSocketClientUrl; use crate::chain::chain_type::ChainType as TestedChainType; use crate::chain::driver::ChainDriver; use crate::ibc::denom::Denom; +use crate::prelude::TestConfig; use crate::types::env::{prefix_writer, EnvWriter, ExportEnv}; use crate::types::process::ChildProcess; use crate::types::tagged::*; @@ -124,7 +125,14 @@ impl FullNode { pub fn generate_chain_config( &self, chain_type: &TestedChainType, + test_config: &TestConfig, ) -> Result { + let hermes_keystore_dir = test_config + .chain_store_dir + .join("hermes_keyring") + .as_path() + .display() + .to_string(); Ok(config::ChainConfig { id: self.chain_driver.chain_id.clone(), r#type: ChainType::CosmosSdk, @@ -135,12 +143,8 @@ impl FullNode { genesis_restart: None, account_prefix: self.chain_driver.account_prefix.clone(), key_name: self.wallets.relayer.id.0.clone(), - - // By default we use in-memory key store to avoid polluting - // ~/.hermes/keys. See - // https://github.com/informalsystems/hermes/issues/1541 - key_store_type: Store::Memory, - + key_store_type: Store::Test, + key_store_folder: Some(hermes_keystore_dir), store_prefix: "ibc".to_string(), default_gas: None, max_gas: Some(3000000), From 9f834228681e2b9af03ea8ecc1c8cddd28cec38f Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Tue, 25 Apr 2023 13:14:57 +0200 Subject: [PATCH 2/5] Add changelog entry --- .../ibc-relayer/1541-config-keyring-store-directory.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/ibc-relayer/1541-config-keyring-store-directory.md diff --git a/.changelog/unreleased/improvements/ibc-relayer/1541-config-keyring-store-directory.md b/.changelog/unreleased/improvements/ibc-relayer/1541-config-keyring-store-directory.md new file mode 100644 index 0000000000..74a58ed161 --- /dev/null +++ b/.changelog/unreleased/improvements/ibc-relayer/1541-config-keyring-store-directory.md @@ -0,0 +1,2 @@ +- Added a configuration to specify the directory used to the keyring store + ([#1541](https://github.com/informalsystems/hermes/issues/1541)) \ No newline at end of file From 9b9276a7bf4ee47a2d6ba638507b8d3b25c3551c Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Tue, 25 Apr 2023 13:59:32 +0200 Subject: [PATCH 3/5] Github suggested improvements --- config.toml | 8 ++++---- crates/relayer/src/config.rs | 9 +++++++-- crates/relayer/src/keyring.rs | 16 +++++++--------- tools/test-framework/src/types/single/node.rs | 2 +- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/config.toml b/config.toml index a04433df6d..a49329b414 100644 --- a/config.toml +++ b/config.toml @@ -140,6 +140,10 @@ account_prefix = 'cosmos' # https://hermes.informal.systems/commands/keys/index.html#adding-keys key_name = 'testkey' +# Specify the folder used to store the keys. Optional +# If this is not specified then the hermes home folder is used. +# key_store_folder = '/home/.hermes_keys' + # Specify the address type which determines: # 1) address derivation; # 2) how to retrieve and decode accounts and pubkeys; @@ -158,10 +162,6 @@ address_type = { derivation = 'cosmos' } # Recommended value for Cosmos SDK: 'ibc' store_prefix = 'ibc' -# Specify the folder used to store the keys. Optional -# If this is not specified then the hermes home folder is used. -# key_store_folder = '/home/.hermes_keys' - # Gas Parameters # # The term 'gas' is used to denote the amount of computation needed to execute diff --git a/crates/relayer/src/config.rs b/crates/relayer/src/config.rs index 6922c46ae4..54472f3df7 100644 --- a/crates/relayer/src/config.rs +++ b/crates/relayer/src/config.rs @@ -13,7 +13,12 @@ use core::{ str::FromStr, time::Duration, }; -use std::{fs, fs::File, io::Write, path::Path}; +use std::{ + fs, + fs::File, + io::Write, + path::{Path, PathBuf}, +}; use tendermint::block::Height as BlockHeight; use tendermint_rpc::{Url, WebSocketClientUrl}; @@ -448,7 +453,7 @@ pub struct ChainConfig { pub key_name: String, #[serde(default)] pub key_store_type: Store, - pub key_store_folder: Option, + pub key_store_folder: Option, pub store_prefix: String, pub default_gas: Option, pub max_gas: Option, diff --git a/crates/relayer/src/keyring.rs b/crates/relayer/src/keyring.rs index 960c90c8f8..06c2750df3 100644 --- a/crates/relayer/src/keyring.rs +++ b/crates/relayer/src/keyring.rs @@ -204,7 +204,7 @@ impl KeyRing { store: Store, account_prefix: &str, chain_id: &ChainId, - ks_folder: &Option, + ks_folder: &Option, ) -> Result { match store { Store::Memory => Ok(Self::Memory(Memory::new(account_prefix.to_string()))), @@ -270,7 +270,7 @@ impl KeyRing { store: Store, account_prefix: &str, chain_id: &ChainId, - ks_folder: &Option, + ks_folder: &Option, ) -> Result { Self::new(store, account_prefix, chain_id, ks_folder) } @@ -281,7 +281,7 @@ impl KeyRing { store: Store, account_prefix: &str, chain_id: &ChainId, - ks_folder: &Option, + ks_folder: &Option, ) -> Result { Self::new(store, account_prefix, chain_id, ks_folder) } @@ -306,14 +306,12 @@ pub fn list_keys(config: &ChainConfig) -> Result) -> Result { +fn disk_store_path(folder_name: &str, keystore_folder: &Option) -> Result { let home = dirs_next::home_dir().ok_or_else(Error::home_location_unavailable)?; - let ks_folder = if let Some(ks) = keystore_folder { - ks - } else { - KEYSTORE_DEFAULT_FOLDER - }; + let ks_folder = keystore_folder + .clone() + .unwrap_or(PathBuf::from(KEYSTORE_DEFAULT_FOLDER)); let folder = Path::new(home.as_path()) .join(ks_folder) diff --git a/tools/test-framework/src/types/single/node.rs b/tools/test-framework/src/types/single/node.rs index 1fc7feec0f..65a366c5d1 100644 --- a/tools/test-framework/src/types/single/node.rs +++ b/tools/test-framework/src/types/single/node.rs @@ -144,7 +144,7 @@ impl FullNode { account_prefix: self.chain_driver.account_prefix.clone(), key_name: self.wallets.relayer.id.0.clone(), key_store_type: Store::Test, - key_store_folder: Some(hermes_keystore_dir), + key_store_folder: Some(hermes_keystore_dir.into()), store_prefix: "ibc".to_string(), default_gas: None, max_gas: Some(3000000), From a16169657efe52a49d6c49ee02c86c7019f6a490 Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Tue, 25 Apr 2023 16:12:55 +0200 Subject: [PATCH 4/5] Updated example configuration for 'key_store_folder' --- config.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.toml b/config.toml index a49329b414..2d8f922c89 100644 --- a/config.toml +++ b/config.toml @@ -142,7 +142,7 @@ key_name = 'testkey' # Specify the folder used to store the keys. Optional # If this is not specified then the hermes home folder is used. -# key_store_folder = '/home/.hermes_keys' +# key_store_folder = '$HOME/.hermes/keys' # Specify the address type which determines: # 1) address derivation; From 0fc408739d715b29b4f881697f877c1a12b4bec2 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 25 Apr 2023 16:21:40 +0200 Subject: [PATCH 5/5] Use default of `$HOME/.hermes/keys` and otherwise treat the given path as absolute --- crates/relayer/src/keyring.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/crates/relayer/src/keyring.rs b/crates/relayer/src/keyring.rs index 06c2750df3..b5967b24a7 100644 --- a/crates/relayer/src/keyring.rs +++ b/crates/relayer/src/keyring.rs @@ -16,7 +16,7 @@ mod signing_key_pair; use alloc::collections::btree_map::BTreeMap as HashMap; use std::ffi::OsStr; use std::fs::{self, File}; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use ibc_relayer_types::core::ics24_host::identifier::ChainId; use serde::{Deserialize, Serialize}; @@ -307,16 +307,15 @@ pub fn list_keys(config: &ChainConfig) -> Result) -> Result { - let home = dirs_next::home_dir().ok_or_else(Error::home_location_unavailable)?; - - let ks_folder = keystore_folder - .clone() - .unwrap_or(PathBuf::from(KEYSTORE_DEFAULT_FOLDER)); + let ks_folder = match keystore_folder { + Some(folder) => folder.to_owned(), + None => { + let home = dirs_next::home_dir().ok_or_else(Error::home_location_unavailable)?; + home.join(KEYSTORE_DEFAULT_FOLDER) + } + }; - let folder = Path::new(home.as_path()) - .join(ks_folder) - .join(folder_name) - .join(KEYSTORE_DISK_BACKEND); + let folder = ks_folder.join(folder_name).join(KEYSTORE_DISK_BACKEND); Ok(folder) }