Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/backend_task/identity/load_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl AppContext {
QualifiedIdentityPublicKey::from_identity_public_key_with_wallets_check(
key,
self.network,
wallets.as_slice(),
&wallets.values().collect::<Vec<_>>(),
);
encrypted_private_keys.insert(
(PrivateKeyOnMainIdentity, key_id),
Expand All @@ -98,7 +98,7 @@ impl AppContext {
QualifiedIdentityPublicKey::from_identity_public_key_with_wallets_check(
key,
self.network,
wallets.as_slice(),
&wallets.values().collect::<Vec<_>>(),
);
encrypted_private_keys.insert(
(PrivateKeyOnMainIdentity, key_id),
Expand Down Expand Up @@ -138,7 +138,7 @@ impl AppContext {
QualifiedIdentityPublicKey::from_identity_public_key_with_wallets_check(
key.clone(),
self.network,
wallets.as_slice(),
&wallets.values().collect::<Vec<_>>(),
);
encrypted_private_keys.insert(
(PrivateKeyOnVoterIdentity, key.id()),
Expand Down Expand Up @@ -194,7 +194,7 @@ impl AppContext {
QualifiedIdentityPublicKey::from_identity_public_key_with_wallets_check(
public_key.clone(),
self.network,
wallets.as_slice(),
&wallets.values().collect::<Vec<_>>(),
);

if let Some(wallet_derivation_path) =
Expand Down Expand Up @@ -291,7 +291,7 @@ impl AppContext {
private_keys: encrypted_private_keys.into(),
dpns_names: maybe_owned_dpns_names,
associated_wallets: wallets
.iter()
.values()
.map(|wallet| (wallet.read().unwrap().seed_hash(), wallet.clone()))
.collect(),
wallet_index: None, //todo
Expand Down
12 changes: 6 additions & 6 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::model::contested_name::ContestedName;
use crate::model::password_info::PasswordInfo;
use crate::model::qualified_contract::QualifiedContract;
use crate::model::qualified_identity::{DPNSNameInfo, QualifiedIdentity};
use crate::model::wallet::Wallet;
use crate::model::wallet::{Wallet, WalletSeedHash};
use crate::sdk_wrapper::initialize_sdk;
use crate::ui::RootScreenType;
use crossbeam_channel::{Receiver, Sender};
Expand Down Expand Up @@ -44,7 +44,7 @@ pub struct AppContext {
pub(crate) withdraws_contract: Arc<DataContract>,
pub(crate) core_client: Client,
pub(crate) has_wallet: AtomicBool,
pub(crate) wallets: RwLock<Vec<Arc<RwLock<Wallet>>>>,
pub(crate) wallets: RwLock<BTreeMap<WalletSeedHash, Arc<RwLock<Wallet>>>>,
pub(crate) password_info: Option<PasswordInfo>,
pub(crate) transactions_waiting_for_finality: Mutex<BTreeMap<Txid, Option<AssetLockProof>>>,
pub(crate) platform_version: &'static PlatformVersion,
Expand Down Expand Up @@ -94,11 +94,11 @@ impl AppContext {
)
.ok()?;

let wallets: Vec<_> = db
let wallets: BTreeMap<_, _> = db
.get_wallets(&network)
.expect("expected to get wallets")
.into_iter()
.map(|w| Arc::new(RwLock::new(w)))
.map(|w| (w.seed_hash(), Arc::new(RwLock::new(w))))
Comment on lines +97 to +101

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider improving error handling in wallet initialization.

While the conversion to BTreeMap is implemented correctly, the use of expect could cause panic in production. Consider propagating the error instead.

-        let wallets: BTreeMap<_, _> = db
-            .get_wallets(&network)
-            .expect("expected to get wallets")
+        let wallets: BTreeMap<_, _> = db
+            .get_wallets(&network)
+            .map_err(|e| {
+                eprintln!("Failed to get wallets: {}", e);
+                return None;
+            })?
             .into_iter()
             .map(|w| (w.seed_hash(), Arc::new(RwLock::new(w))))
             .collect();

Committable suggestion skipped: line range outside the PR's diff.

.collect();

let app_context = AppContext {
Expand Down Expand Up @@ -266,7 +266,7 @@ impl AppContext {

// Identify the wallets associated with the transaction
let wallets = self.wallets.read().unwrap();
for wallet_arc in wallets.iter() {
for wallet_arc in wallets.values() {
let mut wallet = wallet_arc.write().unwrap();
for (vout, tx_out) in tx.output.iter().enumerate() {
let address = if let Ok(output_addr) =
Expand Down Expand Up @@ -356,7 +356,7 @@ impl AppContext {

// Identify the wallet associated with the transaction
let wallets = self.wallets.read().unwrap();
for wallet_arc in wallets.iter() {
for wallet_arc in wallets.values() {
let mut wallet = wallet_arc.write().unwrap();

// Check if any of the addresses in the transaction outputs match the wallet's known addresses
Expand Down
11 changes: 4 additions & 7 deletions src/database/identities.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::context::AppContext;
use crate::database::Database;
use crate::model::qualified_identity::QualifiedIdentity;
use crate::model::wallet::Wallet;
use crate::model::wallet::{Wallet, WalletSeedHash};
use dash_sdk::dpp::identity::accessors::IdentityGettersV0;
use dash_sdk::platform::Identifier;
use rusqlite::params;
use std::collections::BTreeMap;
use std::sync::{Arc, RwLock};
use std::sync::{Arc, RwLock, RwLockReadGuard};

impl Database {
/// Updates the alias of a specified identity.
Expand Down Expand Up @@ -163,7 +163,7 @@ impl Database {
pub fn get_local_qualified_identities(
&self,
app_context: &AppContext,
wallets: &[Arc<RwLock<Wallet>>],
wallets: &BTreeMap<WalletSeedHash, Arc<RwLock<Wallet>>>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider optimizing wallet association to avoid full map cloning

The current implementation clones the entire wallets map for each identity, which could be inefficient. The TODO comment correctly identifies this concern.

Consider these improvements:

-            identity.associated_wallets = wallets.clone(); //todo: use less wallets
+            // Only associate wallets that are relevant to this identity
+            identity.associated_wallets = if let Some(wallet_index) = identity.wallet_index {
+                wallets.iter()
+                    .filter(|(_, wallet)| {
+                        // Add your filtering logic here based on wallet_index
+                        // This is a placeholder - implement actual wallet matching logic
+                        true
+                    })
+                    .map(|(k, v)| (k.clone(), v.clone()))
+                    .collect()
+            } else {
+                BTreeMap::new()
+            };

This approach would:

  1. Only associate relevant wallets with each identity
  2. Reduce memory usage by avoiding unnecessary cloning
  3. Improve performance for operations involving identity wallet associations

Please implement the appropriate filtering logic based on your wallet matching requirements.

Also applies to: 192-192

) -> rusqlite::Result<Vec<QualifiedIdentity>> {
let network = app_context.network_string();

Expand All @@ -189,10 +189,7 @@ impl Database {
identity.wallet_index = wallet_index;

// Associate wallets
identity.associated_wallets = wallets
.iter()
.map(|wallet| (wallet.read().unwrap().seed_hash(), wallet.clone()))
.collect();
identity.associated_wallets = wallets.clone(); //todo: use less wallets

// Retrieve the identity_id as bytes
let identity_id = identity.identity.id().to_buffer();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ impl QualifiedIdentityPublicKey {
pub fn from_identity_public_key_with_wallets_check(
value: IdentityPublicKey,
network: Network,
wallets: &[Arc<RwLock<Wallet>>],
wallets: &[&Arc<RwLock<Wallet>>],
) -> Self {
// Initialize `in_wallet_at_derivation_path` as `None`
let mut in_wallet_at_derivation_path = None;
Expand Down
10 changes: 10 additions & 0 deletions src/model/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,16 @@ impl Wallet {
Ok(None)
}

pub fn private_key_at_derivation_path(
&self,
derivation_path: &DerivationPath,
) -> Result<PrivateKey, String> {
let extended_private_key = derivation_path
.derive_priv_ecdsa_for_master_seed(self.seed_bytes()?, Network::Dash)
.map_err(|e| e.to_string())?;
return Ok(extended_private_key.to_priv());
}

pub fn private_key_for_address(
&self,
address: &Address,
Expand Down
6 changes: 3 additions & 3 deletions src/ui/identities/add_existing_identity_screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ pub struct AddExistingIdentityScreen {

impl AddExistingIdentityScreen {
pub fn new(app_context: &Arc<AppContext>) -> Self {
let selected_wallet = app_context.wallets.read().unwrap().first().cloned();
let selected_wallet = app_context.wallets.read().unwrap().values().next().cloned();
let testnet_loaded_nodes = if app_context.network == Network::Testnet {
load_testnet_nodes_from_yml(".testnet_nodes.yml")
} else {
Expand Down Expand Up @@ -181,7 +181,7 @@ impl AddExistingIdentityScreen {
if self.app_context.has_wallet.load(Ordering::Relaxed) {
let wallets = &self.app_context.wallets.read().unwrap();
let wallet_aliases: Vec<String> = wallets
.iter()
.values()
.map(|wallet| {
wallet
.read()
Expand All @@ -202,7 +202,7 @@ impl AddExistingIdentityScreen {
ComboBox::from_label("")
.selected_text(selected_wallet_alias.clone())
.show_ui(ui, |ui| {
for (idx, wallet) in wallets.iter().enumerate() {
for (idx, wallet) in wallets.values().enumerate() {
let wallet_alias = wallet_aliases[idx].clone();

let is_selected = self
Expand Down
6 changes: 3 additions & 3 deletions src/ui/identities/add_new_identity_screen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl AddNewIdentityScreen {
let mut identity_keys = None;
if app_context.has_wallet.load(Ordering::Relaxed) {
let wallets = &app_context.wallets.read().unwrap();
if let Some(wallet) = wallets.first() {
if let Some(wallet) = wallets.values().next() {
// Automatically select the only available wallet
selected_wallet = Some(wallet.clone());
identity_id_number = wallet
Expand Down Expand Up @@ -402,7 +402,7 @@ impl AddNewIdentityScreen {
ComboBox::from_label("Select Wallet")
.selected_text(selected_wallet_alias)
.show_ui(ui, |ui| {
for wallet in wallets.iter() {
for wallet in wallets.values() {
let wallet_alias = wallet
.read()
.ok()
Expand All @@ -427,7 +427,7 @@ impl AddNewIdentityScreen {
});
ui.add_space(10.0);
true
} else if let Some(wallet) = wallets.first() {
} else if let Some(wallet) = wallets.values().next() {
if self.selected_wallet.is_none() {
// Automatically select the only available wallet
self.selected_wallet = Some(wallet.clone());
Expand Down
2 changes: 1 addition & 1 deletion src/ui/identities/identities_screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl IdentitiesScreen {
return Some(in_wallet_text.clone());
}
let wallets = self.app_context.wallets.read().unwrap();
for wallet in wallets.iter() {
for wallet in wallets.values() {
let wallet_guard = wallet.read().unwrap();
if &wallet_guard.seed_hash() == wallet_seed_hash {
let in_wallet_text = if let Some(alias) = wallet_guard.alias.as_ref() {
Expand Down
Loading