Fix/secure store prompting

cmd/crates/soroban-test/src/lib.rs

@@ -27,6 +27,7 @@ use std::{
     ffi::OsString,
     fmt::Display,
     path::{Path, PathBuf},
+    sync::OnceLock,
 };
 
 use assert_cmd::{assert::Assert, Command};
@@ -277,6 +278,7 @@ impl TestEnv {
             locator: config::locator::Args {
                 global: false,
                 config_dir,
+                cached_keys: OnceLock::new(),
             },
             sign_with: config::sign_with::Args {
                 sign_with_key: None,

cmd/crates/soroban-test/tests/it/integration/hello_world.rs

@@ -1,3 +1,5 @@
+use super::util::{deploy_hello, extend};
+use crate::integration::util::extend_contract;
 use soroban_cli::{
     commands::{
         contract::{self, fetch},
@@ -6,9 +8,7 @@ use soroban_cli::{
     config::{locator, secret},
 };
 use soroban_test::{AssertExt, TestEnv, LOCAL_NETWORK_PASSPHRASE};
-
-use super::util::{deploy_hello, extend};
-use crate::integration::util::extend_contract;
+use std::sync::OnceLock;
 
 #[allow(clippy::too_many_lines)]
 #[tokio::test]
@@ -101,6 +101,7 @@ async fn invoke_contract() {
     let config_locator = locator::Args {
         global: false,
         config_dir: Some(dir.to_path_buf()),
+        cached_keys: OnceLock::new(),
     };
 
     config_locator

cmd/soroban-cli/src/commands/keys/add.rs

@@ -10,7 +10,7 @@ use crate::{
         secret::{self, Secret},
     },
     print::Print,
-    signer::secure_store,
+    signer::secure_store_entry::{self, SecureStoreEntry},
 };
 
 #[derive(thiserror::Error, Debug)]
@@ -23,7 +23,7 @@ pub enum Error {
     Config(#[from] locator::Error),
 
     #[error(transparent)]
-    SecureStore(#[from] secure_store::Error),
+    SecureStoreEntry(#[from] secure_store_entry::Error),
 
     #[error(transparent)]
     SeedPhrase(#[from] sep5::error::Error),
@@ -96,7 +96,7 @@ impl Cmd {
 
             let seed_phrase: SeedPhrase = secret_key.parse()?;
 
-            let secret = secure_store::save_secret(print, &self.name, &seed_phrase)?;
+            let secret = SecureStoreEntry::create_and_save(&self.name, &seed_phrase, print)?;
             Ok(secret.parse()?)
         } else {
             let prompt = "Type a secret key or 12/24 word seed phrase:";

cmd/soroban-cli/src/commands/keys/generate.rs

@@ -5,7 +5,12 @@ use super::super::config::{
     secret::{self, Secret},
 };
 
-use crate::{commands::global, config::address::KeyName, print::Print, signer::secure_store};
+use crate::{
+    commands::global,
+    config::address::KeyName,
+    print::Print,
+    signer::secure_store_entry::{self, SecureStoreEntry},
+};
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
@@ -22,7 +27,7 @@ pub enum Error {
     IdentityAlreadyExists(String),
 
     #[error(transparent)]
-    SecureStore(#[from] secure_store::Error),
+    SecureStoreEntry(#[from] secure_store_entry::Error),
 }
 
 #[derive(Debug, clap::Parser, Clone)]
@@ -115,7 +120,7 @@ impl Cmd {
     fn secret(&self, print: &Print) -> Result<Secret, Error> {
         let seed_phrase = self.seed_phrase()?;
         if self.secure_store {
-            let secret = secure_store::save_secret(print, &self.name, &seed_phrase)?;
+            let secret = SecureStoreEntry::create_and_save(&self.name, &seed_phrase, print)?;
             Ok(secret.parse()?)
         } else if self.as_secret {
             let secret: Secret = seed_phrase.into();
@@ -132,13 +137,16 @@ impl Cmd {
 
 #[cfg(test)]
 mod tests {
+    use std::sync::OnceLock;
+
     use crate::config::{address::KeyName, key::Key, secret::Secret};
 
     fn set_up_test() -> (super::locator::Args, super::Cmd) {
         let temp_dir = tempfile::tempdir().unwrap();
         let locator = super::locator::Args {
             global: false,
             config_dir: Some(temp_dir.path().to_path_buf()),
+            cached_keys: OnceLock::new(),
         };
 
         let cmd = super::Cmd {

cmd/soroban-cli/src/config/key.rs

@@ -17,7 +17,7 @@ pub enum Error {
     Parse,
 }
 
-#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
+#[derive(Debug, Serialize, Deserialize, Clone)]
 pub enum Key {
     #[serde(rename = "public_key")]
     PublicKey(Public),
@@ -85,7 +85,9 @@ impl From<&stellar_strkey::ed25519::PublicKey> for Key {
     }
 }
 
-#[derive(Debug, PartialEq, Eq, serde_with::SerializeDisplay, serde_with::DeserializeFromStr)]
+#[derive(
+    Debug, Clone, PartialEq, Eq, serde_with::SerializeDisplay, serde_with::DeserializeFromStr,
+)]
 pub struct Public(pub stellar_strkey::ed25519::PublicKey);
 
 impl FromStr for Public {
@@ -111,7 +113,9 @@ impl From<&Public> for stellar_strkey::ed25519::MuxedAccount {
     }
 }
 
-#[derive(Debug, PartialEq, Eq, serde_with::SerializeDisplay, serde_with::DeserializeFromStr)]
+#[derive(
+    Debug, Clone, PartialEq, Eq, serde_with::SerializeDisplay, serde_with::DeserializeFromStr,
+)]
 pub struct MuxedAccount(pub stellar_strkey::ed25519::MuxedAccount);
 
 impl FromStr for MuxedAccount {
@@ -143,13 +147,66 @@ impl TryFrom<Key> for Secret {
 
 #[cfg(test)]
 mod test {
+    use std::sync::Arc;
+
     use super::*;
 
     fn round_trip(key: &Key) {
         let serialized = toml::to_string(&key).unwrap();
-        println!("{serialized}");
         let deserialized: Key = toml::from_str(&serialized).unwrap();
-        assert_eq!(key, &deserialized);
+
+        assert_key_equality(key, &deserialized);
+    }
+
+    // using this fn instead of just doing assert_eq!(key, deserialized) because Secret::SecureStore keys contain a StellarEntry which contains a keyring::Entry
+    // keyring::Entry comes from the keyring crate which does not implement PartialEq
+    fn assert_key_equality(expected: &Key, actual: &Key) {
+        match (expected, actual) {
+            (Key::PublicKey(e), Key::PublicKey(a)) => {
+                assert_eq!(e, a);
+            }
+            (Key::MuxedAccount(e), Key::MuxedAccount(a)) => {
+                assert_eq!(e, a);
+            }
+            (Key::Secret(e), Key::Secret(a)) => match (e, a) {
+                (
+                    Secret::SecretKey {
+                        secret_key: e_secret_key,
+                    },
+                    Secret::SecretKey {
+                        secret_key: a_secret_key,
+                    },
+                ) => {
+                    assert_eq!(e_secret_key, a_secret_key);
+                }
+                (
+                    Secret::SeedPhrase {
+                        seed_phrase: e_seed_phrase,
+                    },
+                    Secret::SeedPhrase {
+                        seed_phrase: a_seed_phrase,
+                    },
+                ) => {
+                    assert_eq!(e_seed_phrase, a_seed_phrase);
+                }
+                (Secret::Ledger, Secret::Ledger) => todo!(),
+                (
+                    Secret::SecureStore {
+                        entry_name: e_entry_name,
+                        cached_entry: e_cached_entry,
+                    },
+                    Secret::SecureStore {
+                        entry_name: a_entry_name,
+                        cached_entry: a_cached_entry,
+                    },
+                ) => {
+                    assert_eq!(e_entry_name, a_entry_name);
+                    assert!(Arc::ptr_eq(e_cached_entry, a_cached_entry));
+                }
+                _ => panic!("keys are not equal {expected:?} != {actual:?}"),
+            },
+            _ => panic!("keys are not equal {expected:?} != {actual:?}"),
+        }
     }
 
     #[test]

cmd/soroban-cli/src/config/locator.rs

@@ -1,6 +1,8 @@
 use directories::UserDirs;
 use itertools::Itertools;
 use serde::de::DeserializeOwned;
+use std::collections::HashMap;
+use std::sync::{Arc, Mutex, OnceLock};
 use std::{
     ffi::OsStr,
     fmt::Display,
@@ -14,7 +16,7 @@ use stellar_strkey::{Contract, DecodeError};
 use crate::{
     commands::{global, HEADING_GLOBAL},
     print::Print,
-    signer::secure_store,
+    signer::secure_store_entry::{self, SecureStoreEntry},
     utils::find_config_dir,
     xdr, Pwd,
 };
@@ -96,7 +98,7 @@ pub enum Error {
     #[error("Key cannot {0} cannot overlap with contract alias")]
     KeyCannotOverlapWithContractAlias(String),
     #[error(transparent)]
-    SecureStore(#[from] secure_store::Error),
+    SecureStoreEntry(#[from] secure_store_entry::Error),
     #[error("Only private keys and seed phrases are supported for getting private keys {0}")]
     SecretKeyOnly(String),
     #[error(transparent)]
@@ -105,6 +107,8 @@ pub enum Error {
     ProjectDirsError(),
 }
 
+pub type CachedKeys = HashMap<String, Key>;
+
 #[derive(Debug, clap::Args, Default, Clone)]
 #[group(skip)]
 pub struct Args {
@@ -116,6 +120,10 @@ pub struct Args {
     /// Contains configuration files, aliases, and other persistent settings.
     #[arg(long, global = true, help_heading = HEADING_GLOBAL)]
     pub config_dir: Option<PathBuf>,
+
+    #[clap(skip)]
+    // This saves us from reading the same key from the file system more than once for one cmd
+    pub cached_keys: OnceLock<Arc<Mutex<CachedKeys>>>,
 }
 
 pub enum Location {
@@ -269,10 +277,30 @@ impl Args {
         KeyType::Identity.read_with_global(name, self)
     }
 
+    // read_key caches the Key after reading it from the config
     pub fn read_key(&self, key_or_name: &str) -> Result<Key, Error> {
-        key_or_name
+        // check cache for key & return it if its there
+        if let Some(arc) = self.cached_keys.get() {
+            let map = arc.lock().unwrap();
+            if let Some(k) = map.get(key_or_name) {
+                return Ok(k.clone());
+            }
+        }
+
+        // if its not in the cache, read it from config
+        let key = key_or_name
             .parse()
-            .or_else(|_| self.read_identity(key_or_name))
+            .or_else(|_| self.read_identity(key_or_name))?;
+
+        // get or initialize the cached keys
+        let arc = self
+            .cached_keys
+            .get_or_init(|| Arc::new(Mutex::new(HashMap::new())));
+        let mut map = arc.lock().unwrap();
+        // add the key to cached_keys
+        map.insert(key_or_name.to_string(), key.clone());
+
+        Ok(key)
     }
 
     pub fn get_secret_key(&self, key_or_name: &str) -> Result<Secret, Error> {
@@ -305,8 +333,9 @@ impl Args {
         let print = Print::new(global_args.quiet);
         let identity = self.read_identity(name)?;
 
-        if let Key::Secret(Secret::SecureStore { entry_name }) = identity {
-            secure_store::delete_secret(&print, &entry_name)?;
+        if let Key::Secret(Secret::SecureStore { entry_name, .. }) = identity {
+            let secure_store_entry = SecureStoreEntry::new(entry_name, None)?;
+            secure_store_entry.delete_secret(&print)?;
         }
 
         print.infoln("Removing the key's cli config file");