From e1766bd6668d64cb5d75135e09037d535ee6b16b Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Thu, 18 Jan 2024 17:56:37 +0100 Subject: [PATCH] moar tests --- profile/src/v100/factors/factor_source.rs | 15 ++- .../device_factor_source.rs | 5 +- .../device_factor_source_hint.rs | 5 +- .../ledger_hardware_wallet_factor_source.rs | 5 +- .../ledger_hardware_wallet_hint.rs | 5 +- .../ledger_hardware_wallet_model.rs | 13 +- profile/src/wallet/wallet.rs | 3 +- profile/src/wallet/wallet_accounts.rs | 115 +++++++++++++++++- 8 files changed, 156 insertions(+), 10 deletions(-) diff --git a/profile/src/v100/factors/factor_source.rs b/profile/src/v100/factors/factor_source.rs index 73f70a39..accb3ea5 100644 --- a/profile/src/v100/factors/factor_source.rs +++ b/profile/src/v100/factors/factor_source.rs @@ -1,15 +1,28 @@ use crate::prelude::*; -#[derive(Serialize, Deserialize, Clone, EnumAsInner, Debug, PartialEq, Eq, Hash, uniffi::Enum)] +#[derive( + Serialize, + Deserialize, + Clone, + EnumAsInner, + Debug, + PartialEq, + Eq, + Hash, + derive_more::Display, + uniffi::Enum, +)] #[serde(untagged, remote = "Self")] pub enum FactorSource { Device { #[serde(rename = "device")] + #[display("DeviceFS({value})")] value: DeviceFactorSource, }, Ledger { #[serde(rename = "ledgerHQHardwareWallet")] + #[display("LedgerHWFS({value})")] value: LedgerHardwareWalletFactorSource, }, } diff --git a/profile/src/v100/factors/factor_sources/device_factor_source/device_factor_source.rs b/profile/src/v100/factors/factor_sources/device_factor_source/device_factor_source.rs index 503773ca..0a097c65 100644 --- a/profile/src/v100/factors/factor_sources/device_factor_source/device_factor_source.rs +++ b/profile/src/v100/factors/factor_sources/device_factor_source/device_factor_source.rs @@ -5,8 +5,11 @@ use crate::prelude::*; /// all new Accounts and Personas an users authenticate signing by authorizing /// the client (Wallet App) to access a mnemonic stored in secure storage on /// the device. -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash, uniffi::Record)] +#[derive( + Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash, derive_more::Display, uniffi::Record, +)] #[serde(rename_all = "camelCase")] +#[display("{hint} {id}")] pub struct DeviceFactorSource { /// Unique and stable identifier of this factor source, stemming from the /// hash of a special child key of the HD root of the mnemonic. diff --git a/profile/src/v100/factors/factor_sources/device_factor_source/device_factor_source_hint.rs b/profile/src/v100/factors/factor_sources/device_factor_source/device_factor_source_hint.rs index e5e6e24b..b3e584c3 100644 --- a/profile/src/v100/factors/factor_sources/device_factor_source/device_factor_source_hint.rs +++ b/profile/src/v100/factors/factor_sources/device_factor_source/device_factor_source_hint.rs @@ -2,8 +2,11 @@ use crate::prelude::*; /// Properties describing a DeviceFactorSource to help user disambiguate between /// it and another one. -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash, uniffi::Record)] +#[derive( + Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash, derive_more::Display, uniffi::Record, +)] #[serde(rename_all = "camelCase")] +#[display("{name} {model}")] pub struct DeviceFactorSourceHint { /// "iPhone RED" pub name: String, diff --git a/profile/src/v100/factors/factor_sources/ledger_hardware_wallet_factor_source/ledger_hardware_wallet_factor_source.rs b/profile/src/v100/factors/factor_sources/ledger_hardware_wallet_factor_source/ledger_hardware_wallet_factor_source.rs index 1adccd14..e165298f 100644 --- a/profile/src/v100/factors/factor_sources/ledger_hardware_wallet_factor_source/ledger_hardware_wallet_factor_source.rs +++ b/profile/src/v100/factors/factor_sources/ledger_hardware_wallet_factor_source/ledger_hardware_wallet_factor_source.rs @@ -1,7 +1,10 @@ use crate::prelude::*; -#[derive(Serialize, Deserialize, Clone, PartialEq, Eq, Hash, Debug, uniffi::Record)] +#[derive( + Serialize, Deserialize, Clone, PartialEq, Eq, Hash, Debug, derive_more::Display, uniffi::Record, +)] #[serde(rename_all = "camelCase")] +#[display("{hint} : {id}")] pub struct LedgerHardwareWalletFactorSource { /// Unique and stable identifier of this factor source, stemming from the /// hash of a special child key of the HD root of the mnemonic, diff --git a/profile/src/v100/factors/factor_sources/ledger_hardware_wallet_factor_source/ledger_hardware_wallet_hint.rs b/profile/src/v100/factors/factor_sources/ledger_hardware_wallet_factor_source/ledger_hardware_wallet_hint.rs index fe11576d..66f1af1f 100644 --- a/profile/src/v100/factors/factor_sources/ledger_hardware_wallet_factor_source/ledger_hardware_wallet_hint.rs +++ b/profile/src/v100/factors/factor_sources/ledger_hardware_wallet_factor_source/ledger_hardware_wallet_hint.rs @@ -1,6 +1,9 @@ use crate::prelude::*; -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, Hash, uniffi::Record)] +#[derive( + Serialize, Deserialize, Clone, Debug, PartialEq, Eq, Hash, derive_more::Display, uniffi::Record, +)] +#[display("{name} {model}")] pub struct LedgerHardwareWalletHint { /// "Orange, scratched" pub name: String, diff --git a/profile/src/v100/factors/factor_sources/ledger_hardware_wallet_factor_source/ledger_hardware_wallet_model.rs b/profile/src/v100/factors/factor_sources/ledger_hardware_wallet_factor_source/ledger_hardware_wallet_model.rs index ab6b3d2e..cb57ff18 100644 --- a/profile/src/v100/factors/factor_sources/ledger_hardware_wallet_factor_source/ledger_hardware_wallet_model.rs +++ b/profile/src/v100/factors/factor_sources/ledger_hardware_wallet_factor_source/ledger_hardware_wallet_model.rs @@ -3,7 +3,18 @@ use crate::prelude::*; /// The model of a Ledger HQ hardware wallet NanoS, e.g. /// *Ledger Nano S+*. #[derive( - Serialize, Deserialize, Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, uniffi::Enum, + Serialize, + Deserialize, + Clone, + Copy, + Debug, + PartialEq, + Eq, + Hash, + PartialOrd, + Ord, + derive_more::Display, + uniffi::Enum, )] #[serde(rename_all = "camelCase")] pub enum LedgerHardwareWalletModel { diff --git a/profile/src/wallet/wallet.rs b/profile/src/wallet/wallet.rs index 66956417..7e3b4ca2 100644 --- a/profile/src/wallet/wallet.rs +++ b/profile/src/wallet/wallet.rs @@ -12,7 +12,8 @@ impl Identifiable for Header { #[derive(Debug, uniffi::Object)] pub struct Wallet { - profile: RwLock, + // This is pub(crate) for testing purposes only, i.e. causing the RwLock to be poisoned. + pub(crate) profile: RwLock, pub(crate) wallet_client_storage: WalletClientStorage, } diff --git a/profile/src/wallet/wallet_accounts.rs b/profile/src/wallet/wallet_accounts.rs index 182ee2fb..9077dfdb 100644 --- a/profile/src/wallet/wallet_accounts.rs +++ b/profile/src/wallet/wallet_accounts.rs @@ -34,7 +34,7 @@ impl Wallet { self.add_factor_source(private_device_factor_source.factor_source.into()) .map_err(|e| { error!( - "Failed to Private DeviceFactorSource to SecureStorage, factor source id: {}", + "Failed to add Private DeviceFactorSource to SecureStorage, factor source id: {}", id ); _ = self.wallet_client_storage.delete_mnemonic(&id); @@ -52,10 +52,20 @@ impl Wallet { /// edited. pub fn add_factor_source(&self, factor_source: FactorSource) -> Result<()> { self.try_write(|mut p| { + trace!( + "About to add FactorSource: {}, to list of factor sources: {}", + &factor_source, + &p.factor_sources + ); if p.factor_sources.append(factor_source.to_owned()).0 { - Err(CommonError::Unknown) - } else { + debug!("Added FactorSource: {}", &factor_source); Ok(()) + } else { + error!( + "FactorSource not added, already present: {}", + &factor_source + ); + Err(CommonError::Unknown) } }) .map_err( @@ -188,6 +198,14 @@ impl Wallet { #[cfg(test)] mod tests { + use std::{ + borrow::{Borrow, BorrowMut}, + ops::Deref, + sync::atomic::AtomicBool, + }; + + use __private__::{Mutex, RwLock}; + use crate::prelude::*; #[test] @@ -229,4 +247,95 @@ mod tests { MnemonicWithPassphrase::placeholder() ); } + + #[test] + pub fn add_private_device_factor_source_successful() { + let profile = Profile::placeholder(); + let new = + PrivateHierarchicalDeterministicFactorSource::generate_new(WalletClientModel::Unknown); + let (wallet, storage) = Wallet::ephemeral(profile.clone()); + assert_eq!( + profile + .factor_sources + .contains_id(&new.clone().factor_source.factor_source_id()), + false + ); + assert!(wallet.add_private_device_factor_source(new.clone()).is_ok()); + assert!(storage.storage.read().unwrap().contains_key( + &SecureStorageKey::DeviceFactorSourceMnemonic { + factor_source_id: new.clone().factor_source.id, + }, + )); + assert_eq!( + wallet + .profile() + .factor_sources + .contains_id(&new.clone().factor_source.factor_source_id()), + true + ); + } + + #[test] + pub fn add_private_device_factor_source_ok_storage_when_save_to_profile_fails_then_deleted_from_storage( + ) { + let profile = Profile::placeholder(); + let new = + PrivateHierarchicalDeterministicFactorSource::generate_new(WalletClientModel::Unknown); + + assert_eq!( + profile + .factor_sources + .contains_id(&new.clone().factor_source.factor_source_id()), + false + ); + let delete_data_was_called = Arc::new(RwLock::new(Option::::None)); + #[derive(Debug)] + struct TestStorage { + delete_data_was_called: Arc>>, + } + impl SecureStorage for TestStorage { + fn load_data(&self, _key: SecureStorageKey) -> Result>> { + todo!() + } + + fn save_data(&self, _key: SecureStorageKey, _data: Vec) -> Result<()> { + Ok(()) // mnemonic gets saved + } + + fn delete_data_for_key(&self, key: SecureStorageKey) -> Result<()> { + let mut delete_data_was_called = self.delete_data_was_called.write().unwrap(); + *delete_data_was_called = Some(key); + Ok(()) + } + } + let storage = Arc::new(TestStorage { + delete_data_was_called: delete_data_was_called.clone(), + }); + let wallet = Wallet::by_importing_profile(profile, storage.clone()); + + // Acquire write lock, in order to make `wallet.add_private_device_factor_source` fail (because cant have multiple writers). + let lock = wallet.profile.write().unwrap(); + + assert_eq!( + wallet.add_private_device_factor_source(new.clone()), + Err(CommonError::UnableToSaveFactorSourceToProfile( + new.factor_source.factor_source_id() + )) + ); + drop(lock); + + assert_eq!( + wallet + .profile() + .factor_sources + .contains_id(&new.clone().factor_source.factor_source_id()), + false // should not have been saved. + ); + assert_eq!( + delete_data_was_called.read().unwrap().clone().unwrap(), + SecureStorageKey::DeviceFactorSourceMnemonic { + factor_source_id: new.clone().factor_source.id + } + ); + } }