fix: prevent duplicate loaded contracts

Author: thepastaclaw

src/context/contract_token_db.rs

@@ -5,13 +5,71 @@ use crate::model::wallet::WalletSeedHash;
 use crate::ui::tokens::tokens_screen::{IdentityTokenBalance, IdentityTokenIdentifier};
 use bincode::config;
 use dash_sdk::dpp::data_contract::TokenConfiguration;
+use dash_sdk::dpp::data_contract::accessors::v0::DataContractV0Getters;
 use dash_sdk::platform::{DataContract, Identifier};
 use dash_sdk::query_types::IndexMap;
 use rusqlite::Result;
 use std::sync::Arc;
 use std::sync::atomic::Ordering;
 
 impl AppContext {
+    /// Returns borrowed references to the cached system-contract `Arc`s alongside their aliases.
+    ///
+    /// Use this for read-only lookups (id checks, membership tests, `Arc` cloning) to avoid
+    /// the deep `DataContract` clones that `system_contracts()` performs.
+    pub(crate) fn system_contract_arcs(&self) -> [(&Arc<DataContract>, &'static str); 5] {
+        [
+            (&self.dpns_contract, "dpns"),
+            (&self.token_history_contract, "token_history"),
+            (&self.withdraws_contract, "withdrawals"),
+            (&self.keyword_search_contract, "keyword_search"),
+            (&self.dashpay_contract, "dashpay"),
+        ]
+    }
+
+    pub(crate) fn system_contracts(&self) -> [QualifiedContract; 5] {
+        self.system_contract_arcs()
+            .map(|(contract, alias)| QualifiedContract {
+                contract: contract.as_ref().clone(),
+                alias: Some(alias.to_string()),
+            })
+    }
+
+    pub(crate) fn system_contract_ids(&self) -> [Identifier; 5] {
+        self.system_contract_arcs()
+            .map(|(contract, _alias)| contract.id())
+    }
+
+    pub(crate) fn system_contract_by_id(
+        &self,
+        contract_id: &Identifier,
+    ) -> Option<QualifiedContract> {
+        self.system_contract_arcs()
+            .into_iter()
+            .find(|(contract, _alias)| contract.id() == *contract_id)
+            .map(|(contract, alias)| QualifiedContract {
+                contract: contract.as_ref().clone(),
+                alias: Some(alias.to_string()),
+            })
+    }
+
+    /// Returns an `Arc::clone` of a cached system contract by id, without deep cloning.
+    pub(crate) fn system_contract_arc_by_id(
+        &self,
+        contract_id: &Identifier,
+    ) -> Option<Arc<DataContract>> {
+        self.system_contract_arcs()
+            .into_iter()
+            .find(|(contract, _alias)| contract.id() == *contract_id)
+            .map(|(contract, _alias)| Arc::clone(contract))
+    }
+
+    pub(crate) fn is_system_contract_id(&self, contract_id: &Identifier) -> bool {
+        self.system_contract_arcs()
+            .iter()
+            .any(|(contract, _alias)| contract.id() == *contract_id)
+    }
+
     /// Retrieves all contracts from the database plus the system contracts from app context.
     pub fn get_contracts(
         &self,
@@ -21,50 +79,9 @@ impl AppContext {
         // Get contracts from the database
         let mut contracts = self.db.get_contracts(self, limit, offset)?;
 
-        // Add the DPNS contract to the list
-        let dpns_contract = QualifiedContract {
-            contract: Arc::clone(&self.dpns_contract).as_ref().clone(),
-            alias: Some("dpns".to_string()),
-        };
-
-        // Insert the DPNS contract at 0
-        contracts.insert(0, dpns_contract);
-
-        // Add the token history contract to the list
-        let token_history_contract = QualifiedContract {
-            contract: Arc::clone(&self.token_history_contract).as_ref().clone(),
-            alias: Some("token_history".to_string()),
-        };
-
-        // Insert the token history contract at 1
-        contracts.insert(1, token_history_contract);
-
-        // Add the withdrawal contract to the list
-        let withdraws_contract = QualifiedContract {
-            contract: Arc::clone(&self.withdraws_contract).as_ref().clone(),
-            alias: Some("withdrawals".to_string()),
-        };
-
-        // Insert the withdrawal contract at 2
-        contracts.insert(2, withdraws_contract);
-
-        // Add the keyword search contract to the list
-        let keyword_search_contract = QualifiedContract {
-            contract: Arc::clone(&self.keyword_search_contract).as_ref().clone(),
-            alias: Some("keyword_search".to_string()),
-        };
-
-        // Insert the keyword search contract at 3
-        contracts.insert(3, keyword_search_contract);
-
-        // Add the DashPay contract to the list
-        let dashpay_contract = QualifiedContract {
-            contract: Arc::clone(&self.dashpay_contract).as_ref().clone(),
-            alias: Some("dashpay".to_string()),
-        };
-
-        // Insert the DashPay contract at 4
-        contracts.insert(4, dashpay_contract);
+        for (index, system_contract) in self.system_contracts().into_iter().enumerate() {
+            contracts.insert(index, system_contract);
+        }
 
         Ok(contracts)
     }
@@ -73,6 +90,10 @@ impl AppContext {
         &self,
         contract_id: &Identifier,
     ) -> Result<Option<QualifiedContract>> {
+        if let Some(system_contract) = self.system_contract_by_id(contract_id) {
+            return Ok(Some(system_contract));
+        }
+
         // Get the contract from the database
         self.db.get_contract_by_id(*contract_id, self)
     }
@@ -81,12 +102,20 @@ impl AppContext {
         &self,
         contract_id: &Identifier,
     ) -> Result<Option<DataContract>> {
+        if let Some(system_contract) = self.system_contract_by_id(contract_id) {
+            return Ok(Some(system_contract.contract));
+        }
+
         // Get the contract from the database
         self.db.get_unqualified_contract_by_id(*contract_id, self)
     }
 
     // Remove contract from the database by ID
     pub fn remove_contract(&self, contract_id: &Identifier) -> Result<()> {
+        if self.is_system_contract_id(contract_id) {
+            return Ok(());
+        }
+
         self.db.remove_contract(contract_id.as_bytes(), self)
     }
 
@@ -95,6 +124,10 @@ impl AppContext {
         contract_id: Identifier,
         new_contract: &DataContract,
     ) -> Result<()> {
+        if self.is_system_contract_id(&contract_id) {
+            return Ok(());
+        }
+
         self.db.replace_contract(contract_id, new_contract, self)
     }
 
@@ -183,6 +216,6 @@ impl AppContext {
         let Some(contract_id) = self.db.get_contract_id_by_token_id(token_id, self)? else {
             return Ok(None);
         };
-        self.db.get_contract_by_id(contract_id, self)
+        self.get_contract_by_id(&contract_id)
     }
 }

src/context_provider.rs

@@ -4,7 +4,6 @@ use crate::context::AppContext;
 use crate::database::Database;
 use dash_sdk::core::LowLevelDashCoreClient as CoreClient;
 use dash_sdk::dpp::dashcore::Network;
-use dash_sdk::dpp::data_contract::accessors::v0::DataContractV0Getters;
 use dash_sdk::dpp::version::PlatformVersion;
 use dash_sdk::error::ContextProviderError;
 use dash_sdk::platform::{ContextProvider, DataContract, Identifier};
@@ -15,36 +14,14 @@ use std::sync::{Arc, Mutex};
 // Shared contract/token resolution used by both RPC and SPV providers.
 // ---------------------------------------------------------------------------
 
-/// Number of system contracts cached on [`AppContext`].
-/// Update this when adding a new system contract field.
-///
-/// The typed array size in [`resolve_data_contract`] must match — the compiler
-/// will reject a mismatch, catching forgotten additions at build time.
-pub(crate) const SYSTEM_CONTRACT_COUNT: usize = 5;
-
 /// Resolve a data contract by ID: check cached system contracts first, then DB.
-///
-/// All system contracts are listed in `cached` — adding a new one is a single
-/// array edit, which prevents the two providers from drifting out of sync.
-/// The array size is tied to [`SYSTEM_CONTRACT_COUNT`] so the compiler enforces
-/// completeness.
 pub(crate) fn resolve_data_contract(
     app_ctx: &AppContext,
     db: &Database,
     data_contract_id: &Identifier,
 ) -> Result<Option<Arc<DataContract>>, ContextProviderError> {
-    let cached: [&Arc<DataContract>; SYSTEM_CONTRACT_COUNT] = [
-        &app_ctx.dpns_contract,
-        &app_ctx.dashpay_contract,
-        &app_ctx.token_history_contract,
-        &app_ctx.withdraws_contract,
-        &app_ctx.keyword_search_contract,
-    ];
-
-    for contract in &cached {
-        if data_contract_id == &contract.id() {
-            return Ok(Some(Arc::clone(contract)));
-        }
+    if let Some(system_contract) = app_ctx.system_contract_arc_by_id(data_contract_id) {
+        return Ok(Some(system_contract));
     }
 
     // DB fallback for user-added / non-system contracts

src/database/contracts.rs

@@ -31,6 +31,10 @@ impl Database {
         insert_tokens_too: InsertTokensToo,
         app_context: &AppContext,
     ) -> Result<()> {
+        if app_context.is_system_contract_id(&data_contract.id()) {
+            return Ok(());
+        }
+
         // Serialize the contract
         let contract_bytes = data_contract
             .serialize_to_bytes_with_platform_version(app_context.platform_version())
@@ -260,8 +264,15 @@ impl Database {
     ) -> Result<Vec<QualifiedContract>> {
         let network = app_context.network.to_string();
 
+        let excluded_contract_ids = app_context
+            .system_contract_ids()
+            .map(|contract_id| contract_id.to_vec());
+        let excluded_contract_placeholders = vec!["?"; excluded_contract_ids.len()].join(", ");
+
         // Build the SQL query with optional limit and offset
-        let mut query = String::from("SELECT contract, alias FROM contract WHERE network = ?");
+        let mut query = format!(
+            "SELECT contract, alias FROM contract WHERE network = ? AND contract_id NOT IN ({excluded_contract_placeholders})"
+        );
         if limit.is_some() {
             query.push_str(" LIMIT ?");
         }
@@ -278,6 +289,9 @@ impl Database {
 
         // Collect parameters for query execution
         let mut params: Vec<&dyn rusqlite::ToSql> = vec![&network];
+        for contract_id in &excluded_contract_ids {
+            params.push(contract_id);
+        }
         if let Some(l) = limit {
             limit_value = l;
             params.push(&limit_value); // Now `limit_value` lives long enough

src/ui/components/contract_chooser_panel.rs

@@ -516,10 +516,7 @@ pub fn add_contract_chooser_panel(
                                         // Right‐aligned Remove button
                                         ui.horizontal(|ui| {
                                             ui.add_space(8.0);
-                                            if contract.alias != Some("dpns".to_string())
-                                                && contract.alias != Some("token_history".to_string())
-                                                && contract.alias != Some("withdrawals".to_string())
-                                                && contract.alias != Some("keyword_search".to_string())
+                                            if !app_context.is_system_contract_id(&contract.contract.id())
                                                 && ui.add(
                                                     egui::Button::new("Remove")
                                                         .min_size(egui::Vec2::new(60.0, 20.0))

src/ui/contracts_documents/add_contracts_screen.rs

@@ -80,6 +80,63 @@ impl AddContractsScreen {
     fn add_contracts_clicked(&mut self) -> AppAction {
         match self.parse_identifiers() {
             Ok(identifiers) => {
+                let duplicate_input =
+                    identifiers
+                        .iter()
+                        .enumerate()
+                        .find_map(|(index, identifier)| {
+                            identifiers[..index]
+                                .contains(identifier)
+                                .then_some(identifier)
+                        });
+                if let Some(identifier) = duplicate_input {
+                    let message = format!(
+                        "Contract {} was entered more than once. Remove the duplicate entry before adding contracts.",
+                        identifier.to_string(Encoding::Base58)
+                    );
+                    self.add_contracts_status = AddContractsStatus::Error;
+                    MessageBanner::set_global(
+                        self.app_context.egui_ctx(),
+                        &message,
+                        MessageType::Error,
+                    );
+                    return AppAction::None;
+                }
+
+                let existing_contract_ids = match self.app_context.get_contracts(None, None) {
+                    Ok(contracts) => contracts
+                        .into_iter()
+                        .map(|contract| contract.contract.id())
+                        .collect::<Vec<_>>(),
+                    Err(e) => {
+                        tracing::warn!("Failed to check loaded contracts before adding: {e}");
+                        self.add_contracts_status = AddContractsStatus::Error;
+                        MessageBanner::set_global(
+                            self.app_context.egui_ctx(),
+                            "Unable to check whether those contracts are already loaded. Please try again.",
+                            MessageType::Error,
+                        );
+                        return AppAction::None;
+                    }
+                };
+
+                let already_loaded = identifiers
+                    .iter()
+                    .find(|identifier| existing_contract_ids.contains(identifier));
+                if let Some(identifier) = already_loaded {
+                    let message = format!(
+                        "Contract {} is already loaded. Select it from the existing contracts list or enter a different contract ID.",
+                        identifier.to_string(Encoding::Base58)
+                    );
+                    self.add_contracts_status = AddContractsStatus::Error;
+                    MessageBanner::set_global(
+                        self.app_context.egui_ctx(),
+                        &message,
+                        MessageType::Error,
+                    );
+                    return AppAction::None;
+                }
+
                 self.add_banner.take_and_clear();
                 let handle = MessageBanner::set_global(
                     self.app_context.egui_ctx(),

src/ui/contracts_documents/update_contract_screen.rs

@@ -77,15 +77,11 @@ impl UpdateDataContractScreen {
             None
         };
 
-        let excluded_aliases = ["dpns", "keyword_search", "token_history", "withdrawals"];
         let known_contracts = app_context
             .get_contracts(None, None)
             .expect("Failed to load contracts")
             .into_iter()
-            .filter(|c| match &c.alias {
-                Some(alias) => !excluded_aliases.contains(&alias.as_str()),
-                None => true,
-            })
+            .filter(|contract| !app_context.is_system_contract_id(&contract.contract.id()))
             .collect::<Vec<_>>();
 
         let selected_key = selected_qualified_identity.as_ref().and_then(|identity| {