Configurable keyring store directory (informalsystems#3273)

* Add configuration for keyring store directory * Add changelog entry * Github suggested improvements * Updated example configuration for 'key_store_folder' * Use default of `$HOME/.hermes/keys` and otherwise treat the given path as absolute --------- Co-authored-by: Romain Ruetschi <[email protected]>
octopus-network · Apr 25, 2023 · d5fa30d · d5fa30d
1 parent 12208fd
commit d5fa30d
Show file tree

Hide file tree

Showing 13 changed files with 111 additions and 43 deletions.
diff --git a/...elog/unreleased/improvements/ibc-relayer/1541-config-keyring-store-directory.md b/...elog/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))
diff --git 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;

diff --git 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
@@ -200,8 +200,12 @@ pub fn add_key(
 ) -> eyre::Result<AnySigningKeyPair> {
     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
@@ -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
@@ -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
@@ -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
@@ -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,6 +453,7 @@ pub struct ChainConfig {
     pub key_name: String,
     #[serde(default)]
     pub key_store_type: Store,
+    pub key_store_folder: Option<PathBuf>,
     pub store_prefix: String,
     pub default_gas: Option<u64>,
     pub max_gas: Option<u64>,

diff --git 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};
@@ -200,12 +200,17 @@ pub enum KeyRing<S> {
 }
 
 impl<S: SigningKeyPairSized> KeyRing<S> {
-    pub fn new(store: Store, account_prefix: &str, chain_id: &ChainId) -> Result<Self, Error> {
+    pub fn new(
+        store: Store,
+        account_prefix: &str,
+        chain_id: &ChainId,
+        ks_folder: &Option<PathBuf>,
+    ) -> Result<Self, Error> {
         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<Secp256k1KeyPair> {
         store: Store,
         account_prefix: &str,
         chain_id: &ChainId,
+        ks_folder: &Option<PathBuf>,
     ) -> Result<Self, Error> {
-        Self::new(store, account_prefix, chain_id)
+        Self::new(store, account_prefix, chain_id, ks_folder)
     }
 }
 
@@ -275,15 +281,21 @@ impl KeyRing<Ed25519KeyPair> {
         store: Store,
         account_prefix: &str,
         chain_id: &ChainId,
+        ks_folder: &Option<PathBuf>,
     ) -> Result<Self, Error> {
-        Self::new(store, account_prefix, chain_id)
+        Self::new(store, account_prefix, chain_id, ks_folder)
     }
 }
 
 pub fn list_keys(config: &ChainConfig) -> Result<Vec<(String, AnySigningKeyPair)>, 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,13 +306,16 @@ pub fn list_keys(config: &ChainConfig) -> Result<Vec<(String, AnySigningKeyPair)
     Ok(keys)
 }
 
-fn disk_store_path(folder_name: &str) -> Result<PathBuf, Error> {
-    let home = dirs_next::home_dir().ok_or_else(Error::home_location_unavailable)?;
+fn disk_store_path(folder_name: &str, keystore_folder: &Option<PathBuf>) -> Result<PathBuf, Error> {
+    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(KEYSTORE_DEFAULT_FOLDER)
-        .join(folder_name)
-        .join(KEYSTORE_DISK_BACKEND);
+    let folder = ks_folder.join(folder_name).join(KEYSTORE_DISK_BACKEND);
 
     Ok(folder)
 }
diff --git 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<ChainA: ChainHandle, ChainB: ChainHandle>(
         &self,
-        _config: &TestConfig,
+        config: &TestConfig,
         _relayer: RelayerDriver,
         chains: ConnectedChains<ChainA, ChainB>,
     ) -> 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,15 +160,16 @@ impl BinaryChainTest for ClientFailsTest {
 fn override_connected_chains<ChainA, ChainB>(
     chains: ConnectedChains<ChainA, ChainB>,
     config_modifier: impl FnOnce(&mut Config),
+    test_config: &TestConfig,
 ) -> Result<ConnectedChains<impl ChainHandle, impl ChainHandle>, Error>
 where
     ChainA: ChainHandle,
     ChainB: ChainHandle,
 {
     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
@@ -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<CountingAndCachingChainHan
    Generate [`ChainConfig`](ibc_relayer::config::ChainConfig) from a running
    [`FullNode`] and add it to the relayer's [`Config`].
 */
-pub fn add_chain_config(config: &mut Config, running_node: &FullNode) -> 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
@@ -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
@@ -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<config::ChainConfig, Error> {
+        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.into()),
             store_prefix: "ibc".to_string(),
             default_gas: None,
             max_gas: Some(3000000),
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		- Added a configuration to specify the directory used to the keyring store
		([#1541](https://github.com/informalsystems/hermes/issues/1541))