From e77d200b4e78696f47118f4f25fd9506f8d85050 Mon Sep 17 00:00:00 2001 From: Dmitri Makarov Date: Tue, 22 Apr 2025 16:57:39 -0400 Subject: [PATCH 1/7] Use keyring to store exchange secrets in system store --- Cargo.lock | 62 ++++++++++++++++++++++++-- Cargo.toml | 1 + src/db.rs | 128 ++++++++++++++++++++++++++++++++++++++++------------- 3 files changed, 157 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6538896..d208f3d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2711,6 +2711,18 @@ dependencies = [ "cpufeatures", ] +[[package]] +name = "keyring" +version = "3.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1961983669d57bdfe6c0f3ef8e4c229b5ef751afcc7d87e4271d2f71f6ccfa8b" +dependencies = [ + "dbus-secret-service", + "log", + "security-framework 2.11.1", + "security-framework 3.2.0", +] + [[package]] name = "kraken_sdk_rest" version = "0.17.0" @@ -2739,6 +2751,15 @@ version = "0.2.169" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b5aba8db14291edd000dfcc4d620c7ebfb122c613afb886ca8803fa4e128a20a" +[[package]] +name = "libdbus-sys" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06085512b750d640299b79be4bad3d2fa90a9c00b1fd9e1b46364f66f0485c72" +dependencies = [ + "pkg-config", +] + [[package]] name = "libredox" version = "0.1.3" @@ -2962,10 +2983,24 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8536030f9fea7127f841b45bb6243b27255787fb4eb83958aa1ef9d2fdc0c36" dependencies = [ "num-bigint 0.2.6", - "num-complex", + "num-complex 0.2.4", + "num-integer", + "num-iter", + "num-rational 0.2.4", + "num-traits", +] + +[[package]] +name = "num" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35bd024e8b2ff75562e5f34e7f4905839deb4b22955ef5e73d2fea1b9813cb23" +dependencies = [ + "num-bigint 0.4.6", + "num-complex 0.4.6", "num-integer", "num-iter", - "num-rational", + "num-rational 0.4.2", "num-traits", ] @@ -3011,6 +3046,15 @@ dependencies = [ "num-traits", ] +[[package]] +name = "num-complex" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73f88a1307638156682bada9d7604135552957b7818057dcef22705b4d509495" +dependencies = [ + "num-traits", +] + [[package]] name = "num-conv" version = "0.1.0" @@ -3060,6 +3104,17 @@ dependencies = [ "num-traits", ] +[[package]] +name = "num-rational" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" +dependencies = [ + "num-bigint 0.4.6", + "num-integer", + "num-traits", +] + [[package]] name = "num-traits" version = "0.2.19" @@ -3260,7 +3315,7 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2fd23b938276f14057220b707937bcb42fa76dda7560e57a2da30cb52d557937" dependencies = [ - "num", + "num 0.2.1", ] [[package]] @@ -7416,6 +7471,7 @@ dependencies = [ "influxdb-client", "itertools 0.10.5", "jup-ag", + "keyring", "kraken_sdk_rest", "lazy_static", "log", diff --git a/Cargo.toml b/Cargo.toml index 339e3c5..6a1e706 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,7 @@ jup-ag = "0.9.1" #kraken_sdk_rest = { path = "../kraken_sdk_rust/kraken_sdk_rest" } kraken_sdk_rest = { git = "https://github.com/mvines/kraken_sdk_rust", rev = "80c634b3a8527f653db989689b298496dad30d4e" } #kraken_sdk_rest = "0.18.0" +keyring = { version = "3", features = ["apple-native", "sync-secret-service"] } lazy_static = "1.4.0" log = "0.4.17" num-derive = "0.4" diff --git a/src/db.rs b/src/db.rs index c17c97f..7685719 100644 --- a/src/db.rs +++ b/src/db.rs @@ -57,6 +57,9 @@ pub enum DbError { #[error("Import failed: {0}")] ImportFailed(String), + + #[error("Keyring failed: {0}")] + KeyringFailed(String), } pub type DbResult = std::result::Result; @@ -631,6 +634,7 @@ pub struct DbData { sweep_stake_account: Option, transitory_sweep_stake_accounts: Vec, tax_rate: Option, + exchanges: HashSet, } impl DbData { @@ -684,6 +688,7 @@ impl DbData { .get("transitory-sweep-stake-accounts") .unwrap_or_default(), tax_rate: None, + exchanges: HashSet::::new(), } } @@ -720,16 +725,28 @@ impl Db { exchange_account: &str, exchange_credentials: ExchangeCredentials, ) -> DbResult<()> { - self.clear_exchange_credentials(exchange, exchange_account)?; - - self.credentials_db - .set( - &format!("{exchange:?}{exchange_account}"), - &exchange_credentials, - ) - .unwrap(); - - Ok(self.credentials_db.dump()?) + for entry in [ + ("api_key", exchange_credentials.api_key), + ("secret", exchange_credentials.secret), + ("subaccount", exchange_credentials.subaccount.unwrap_or_default()), + ] { + let user = format!("{exchange_account}_{}", entry.0); + let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { + Ok(entry) => entry, + Err(e) => { + return Err(DbError::KeyringFailed(format!( + "Failed to create keyring entry {user}: {e}" + ))) + } + }; + if let Err(e) = keyring_entry.set_secret(entry.1.as_bytes()) { + return Err(DbError::KeyringFailed(format!( + "Failed to set {}: {e}", + entry.0 + ))); + } + } + self.add_exchange(exchange) } pub fn get_exchange_credentials( @@ -737,8 +754,35 @@ impl Db { exchange: Exchange, exchange_account: &str, ) -> Option { - self.credentials_db - .get(&format!("{exchange:?}{exchange_account}")) + let mut values = vec![]; + for entry in ["api_key", "secret", "subaccount"] { + let user = format!("{exchange_account}_{entry}"); + let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { + Ok(entry) => entry, + Err(e) => { + eprintln!("Failed to create keyring entry {user}: {e}"); + return None; + } + }; + let value = match keyring_entry.get_secret() { + Ok(secret) => String::from_utf8(secret).unwrap(), + Err(e) => { + eprintln!("Failed to get the {entry}: {e}"); + return None; + } + }; + values.push(value); + } + let subaccount = if values[2] != "" { + Some(values[2].clone()) + } else { + None + }; + Some(ExchangeCredentials { + api_key: values[0].clone(), + secret: values[1].clone(), + subaccount, + }) } pub fn clear_exchange_credentials( @@ -746,31 +790,33 @@ impl Db { exchange: Exchange, exchange_account: &str, ) -> DbResult<()> { - if self - .get_exchange_credentials(exchange, exchange_account) - .is_some() - { - self.credentials_db - .rem(&format!("{exchange:?}{exchange_account}")) - .ok(); - self.credentials_db.dump()?; + for entry in ["api_key", "secret", "subaccount"] { + let user = format!("{exchange_account}_{entry}"); + let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { + Ok(entry) => entry, + Err(e) => { + return Err(DbError::KeyringFailed(format!( + "Failed to create keyring entry {user}: {e}" + ))) + } + }; + if let Err(e) = keyring_entry.delete_credential() { + return Err(DbError::KeyringFailed(format!( + "Failed to delete {entry}: {e}" + ))); + } } - Ok(()) + self.remove_exchange(exchange) } pub fn get_default_accounts_from_configured_exchanges( &self, ) -> Vec<(Exchange, ExchangeCredentials, String)> { - self.credentials_db - .get_all() - .into_iter() - .filter_map(|key| { - if let Ok(exchange) = key.parse() { - self.get_exchange_credentials(exchange, "") - .map(|exchange_credentials| (exchange, exchange_credentials, "".into())) - } else { - None - } + self.get_exchanges() + .iter() + .filter_map(|exchange| { + self.get_exchange_credentials(*exchange, "") + .map(|exchange_credentials| (*exchange, exchange_credentials, "".into())) }) .collect() } @@ -1586,6 +1632,26 @@ impl Db { self.save() } + fn get_exchanges(&self) -> HashSet { + self.data.exchanges.clone() + } + + fn add_exchange(&mut self, exchange: Exchange) -> DbResult<()> { + if self.data.exchanges.insert(exchange) { + self.save() + } else { + Ok(()) + } + } + + fn remove_exchange(&mut self, exchange: Exchange) -> DbResult<()> { + if self.data.exchanges.remove(&exchange) { + self.save() + } else { + Ok(()) + } + } + #[allow(clippy::too_many_arguments)] pub fn record_transfer( &mut self, From 34120f8a6009837825e2f0c3b5dc729365f2c3a2 Mon Sep 17 00:00:00 2001 From: Dmitri Makarov Date: Wed, 23 Apr 2025 07:18:20 -0400 Subject: [PATCH 2/7] Serialize ExchangeCredentials and store as a single entry in keychain --- src/db.rs | 102 +++++++++++++++++++++++------------------------------- 1 file changed, 43 insertions(+), 59 deletions(-) diff --git a/src/db.rs b/src/db.rs index 7685719..1cd232c 100644 --- a/src/db.rs +++ b/src/db.rs @@ -725,26 +725,19 @@ impl Db { exchange_account: &str, exchange_credentials: ExchangeCredentials, ) -> DbResult<()> { - for entry in [ - ("api_key", exchange_credentials.api_key), - ("secret", exchange_credentials.secret), - ("subaccount", exchange_credentials.subaccount.unwrap_or_default()), - ] { - let user = format!("{exchange_account}_{}", entry.0); - let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { - Ok(entry) => entry, - Err(e) => { - return Err(DbError::KeyringFailed(format!( - "Failed to create keyring entry {user}: {e}" - ))) - } - }; - if let Err(e) = keyring_entry.set_secret(entry.1.as_bytes()) { + let ec = serde_json::to_string(&exchange_credentials).unwrap(); + // user can't be an empty string + let user = format!("{}_{exchange_account}", exchange.to_string()); + let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { + Ok(entry) => entry, + Err(e) => { return Err(DbError::KeyringFailed(format!( - "Failed to set {}: {e}", - entry.0 - ))); + "Failed to create keyring entry: {e}" + ))) } + }; + if let Err(e) = keyring_entry.set_secret(ec.as_bytes()) { + return Err(DbError::KeyringFailed(format!("Failed to set: {e}"))); } self.add_exchange(exchange) } @@ -754,35 +747,28 @@ impl Db { exchange: Exchange, exchange_account: &str, ) -> Option { - let mut values = vec![]; - for entry in ["api_key", "secret", "subaccount"] { - let user = format!("{exchange_account}_{entry}"); - let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { - Ok(entry) => entry, - Err(e) => { - eprintln!("Failed to create keyring entry {user}: {e}"); - return None; - } - }; - let value = match keyring_entry.get_secret() { - Ok(secret) => String::from_utf8(secret).unwrap(), - Err(e) => { - eprintln!("Failed to get the {entry}: {e}"); - return None; - } - }; - values.push(value); - } - let subaccount = if values[2] != "" { - Some(values[2].clone()) - } else { - None + let user = format!("{}_{exchange_account}", exchange.to_string()); + let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { + Ok(entry) => entry, + Err(e) => { + eprintln!("Failed to create keyring entry: {e}"); + return None; + } }; - Some(ExchangeCredentials { - api_key: values[0].clone(), - secret: values[1].clone(), - subaccount, - }) + let value = match keyring_entry.get_secret() { + Ok(secret) => String::from_utf8(secret).unwrap(), + Err(e) => { + eprintln!("Failed to get the secret: {e}"); + return None; + } + }; + match serde_json::from_str(&value) { + Ok(v) => Some(v), + Err(e) => { + eprintln!("Failed to deserialize credentials: {e}"); + None + }, + } } pub fn clear_exchange_credentials( @@ -790,21 +776,19 @@ impl Db { exchange: Exchange, exchange_account: &str, ) -> DbResult<()> { - for entry in ["api_key", "secret", "subaccount"] { - let user = format!("{exchange_account}_{entry}"); - let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { - Ok(entry) => entry, - Err(e) => { - return Err(DbError::KeyringFailed(format!( - "Failed to create keyring entry {user}: {e}" - ))) - } - }; - if let Err(e) = keyring_entry.delete_credential() { + let user = format!("{}_{exchange_account}", exchange.to_string()); + let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { + Ok(entry) => entry, + Err(e) => { return Err(DbError::KeyringFailed(format!( - "Failed to delete {entry}: {e}" - ))); + "Failed to create keyring entry: {e}" + ))) } + }; + if let Err(e) = keyring_entry.delete_credential() { + return Err(DbError::KeyringFailed(format!( + "Failed to delete credential: {e}" + ))); } self.remove_exchange(exchange) } From dbd4655677e4985d61a08f611f5429f6cb4b494d Mon Sep 17 00:00:00 2001 From: Dmitri Makarov Date: Sun, 27 Apr 2025 20:34:23 -0400 Subject: [PATCH 3/7] Report an error when request for accounts fails Also make sure the private key pem doesn't escape new lines --- src/coinbase_exchange.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coinbase_exchange.rs b/src/coinbase_exchange.rs index 330d518..34707ca 100644 --- a/src/coinbase_exchange.rs +++ b/src/coinbase_exchange.rs @@ -21,6 +21,9 @@ impl ExchangeClient for CoinbaseExchangeClient { pin_mut!(accounts); while let Some(account_result) = accounts.next().await { + if let Err(e) = account_result { + return Err(format!("Failed to get accounts: {e}").into()); + } for account in account_result.unwrap() { if let Ok(id) = coinbase_rs::Uuid::from_str(&account.id) { if token.name() == account.currency.code @@ -157,6 +160,7 @@ pub fn new( }: ExchangeCredentials, ) -> Result> { assert!(subaccount.is_none()); + let secret = secret.replace("\\n", "\n"); Ok(CoinbaseExchangeClient { client: coinbase_rs::Private::new(coinbase_rs::MAIN_URL, &api_key, &secret), }) From 943ecb0905233f0e00365e85fb4a043b4b9401e2 Mon Sep 17 00:00:00 2001 From: Dmitri Makarov Date: Sun, 3 Aug 2025 08:22:17 -0400 Subject: [PATCH 4/7] Rebase and formatting --- Cargo.lock | 24 ++++++++++++++++++++++++ src/db.rs | 2 +- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index d208f3d..056feef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1318,6 +1318,30 @@ version = "2.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0e60eed09d8c01d3cee5b7d30acb059b76614c918fa0f992e0dd6eeb10daad6f" +[[package]] +name = "dbus" +version = "0.9.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1bb21987b9fb1613058ba3843121dd18b163b254d8a6e797e144cbac14d96d1b" +dependencies = [ + "libc", + "libdbus-sys", + "winapi", +] + +[[package]] +name = "dbus-secret-service" +version = "4.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b42a16374481d92aed73ae45b1f120207d8e71d24fb89f357fadbd8f946fd84b" +dependencies = [ + "dbus", + "futures-util", + "num 0.4.3", + "once_cell", + "rand 0.8.5", +] + [[package]] name = "der-parser" version = "8.2.0" diff --git a/src/db.rs b/src/db.rs index 1cd232c..d3b0da0 100644 --- a/src/db.rs +++ b/src/db.rs @@ -767,7 +767,7 @@ impl Db { Err(e) => { eprintln!("Failed to deserialize credentials: {e}"); None - }, + } } } From 6ef1af6ebf9d740e2999637d933e72ebac0275bd Mon Sep 17 00:00:00 2001 From: Dmitri Makarov Date: Sun, 3 Aug 2025 08:40:20 -0400 Subject: [PATCH 5/7] Add workflow dependencies --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ece8fde..3e37d2f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -19,7 +19,7 @@ jobs: - if: runner.os == 'Linux' run: | sudo apt-get update - sudo apt-get install -y libudev-dev + sudo apt-get install -y libudev-dev libdbus-1-dev pkg-config - uses: actions-rs/toolchain@v1 with: From 235f5a86aebae1f6eb2bd225027b1baedc44cd11 Mon Sep 17 00:00:00 2001 From: Dmitri Makarov Date: Sun, 3 Aug 2025 13:00:34 -0400 Subject: [PATCH 6/7] Clippy --- src/db.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/db.rs b/src/db.rs index d3b0da0..6ee0a16 100644 --- a/src/db.rs +++ b/src/db.rs @@ -727,7 +727,7 @@ impl Db { ) -> DbResult<()> { let ec = serde_json::to_string(&exchange_credentials).unwrap(); // user can't be an empty string - let user = format!("{}_{exchange_account}", exchange.to_string()); + let user = format!("{exchange}_{exchange_account}"); let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { Ok(entry) => entry, Err(e) => { @@ -747,7 +747,7 @@ impl Db { exchange: Exchange, exchange_account: &str, ) -> Option { - let user = format!("{}_{exchange_account}", exchange.to_string()); + let user = format!("{exchange}_{exchange_account}"); let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { Ok(entry) => entry, Err(e) => { @@ -776,7 +776,7 @@ impl Db { exchange: Exchange, exchange_account: &str, ) -> DbResult<()> { - let user = format!("{}_{exchange_account}", exchange.to_string()); + let user = format!("{exchange}_{exchange_account}"); let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { Ok(entry) => entry, Err(e) => { From 4a96e10bf8239023be9176af97c56ebb9e91f94a Mon Sep 17 00:00:00 2001 From: Dmitri Makarov Date: Thu, 14 Aug 2025 12:51:51 -0400 Subject: [PATCH 7/7] Use the old exchange credentials database if available. The old exchange credentials database will be queried, but any new credentials will be added to the keyring. Probably better not to mix the two sets of credentials. The assumption is that the user will add existing credentials to keyring via the sys command line and remove the old database, otherwise the old database will be used as long as it exists. --- src/db.rs | 59 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/src/db.rs b/src/db.rs index 6ee0a16..e7e8ebc 100644 --- a/src/db.rs +++ b/src/db.rs @@ -634,7 +634,7 @@ pub struct DbData { sweep_stake_account: Option, transitory_sweep_stake_accounts: Vec, tax_rate: Option, - exchanges: HashSet, + exchanges: Option>, } impl DbData { @@ -688,7 +688,7 @@ impl DbData { .get("transitory-sweep-stake-accounts") .unwrap_or_default(), tax_rate: None, - exchanges: HashSet::::new(), + exchanges: None, } } @@ -747,6 +747,12 @@ impl Db { exchange: Exchange, exchange_account: &str, ) -> Option { + if let Some(cred) = self + .credentials_db + .get(&format!("{exchange:?}{exchange_account}")) + { + return Some(cred); + } let user = format!("{exchange}_{exchange_account}"); let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { Ok(entry) => entry, @@ -776,6 +782,16 @@ impl Db { exchange: Exchange, exchange_account: &str, ) -> DbResult<()> { + if self + .get_exchange_credentials(exchange, exchange_account) + .is_some() + { + self.credentials_db + .rem(&format!("{exchange:?}{exchange_account}")) + .ok(); + self.credentials_db.dump()?; + return Ok(()); + } let user = format!("{exchange}_{exchange_account}"); let keyring_entry = match keyring::Entry::new(&exchange.to_string(), &user) { Ok(entry) => entry, @@ -796,13 +812,28 @@ impl Db { pub fn get_default_accounts_from_configured_exchanges( &self, ) -> Vec<(Exchange, ExchangeCredentials, String)> { - self.get_exchanges() - .iter() - .filter_map(|exchange| { - self.get_exchange_credentials(*exchange, "") - .map(|exchange_credentials| (*exchange, exchange_credentials, "".into())) - }) - .collect() + if let Some(exchanges) = self.get_exchanges() { + exchanges + .iter() + .filter_map(|exchange| { + self.get_exchange_credentials(*exchange, "") + .map(|exchange_credentials| (*exchange, exchange_credentials, "".into())) + }) + .collect() + } else { + self.credentials_db + .get_all() + .into_iter() + .filter_map(|key| { + if let Ok(exchange) = key.parse() { + self.get_exchange_credentials(exchange, "") + .map(|exchange_credentials| (exchange, exchange_credentials, "".into())) + } else { + None + } + }) + .collect() + } } pub fn set_metrics_config(&mut self, metrics_config: MetricsConfig) -> DbResult<()> { @@ -1616,12 +1647,15 @@ impl Db { self.save() } - fn get_exchanges(&self) -> HashSet { + fn get_exchanges(&self) -> Option> { self.data.exchanges.clone() } fn add_exchange(&mut self, exchange: Exchange) -> DbResult<()> { - if self.data.exchanges.insert(exchange) { + if self.data.exchanges.is_none() { + self.data.exchanges = Some(HashSet::<_>::new()); + } + if self.data.exchanges.as_mut().unwrap().insert(exchange) { self.save() } else { Ok(()) @@ -1629,7 +1663,8 @@ impl Db { } fn remove_exchange(&mut self, exchange: Exchange) -> DbResult<()> { - if self.data.exchanges.remove(&exchange) { + if self.data.exchanges.is_some() && self.data.exchanges.as_mut().unwrap().remove(&exchange) + { self.save() } else { Ok(())