fix: address platform wallet review feedback

Author: thepastaclaw

packages/rs-platform-wallet/src/wallet/core/broadcast.rs

@@ -134,28 +134,18 @@ impl<B: TransactionBroadcaster + ?Sized> CoreWallet<B> {
                 .build()
                 .map_err(|e| PlatformWalletError::TransactionBuild(e.to_string()))?;
 
-            // Re-validate the selected outpoints are still spendable while
-            // we still hold the write lock. The lock makes our build atomic
-            // against other callers on this handle, but external mempool /
-            // block events processed before we acquired the lock may have
-            // invalidated UTXOs that were still in the spendable set when
-            // `select_inputs` ran.
-            //
-            // We deliberately do NOT mark the inputs as spent here — that
-            // happens after a successful broadcast (see #3466 review). A
-            // failed broadcast must not leave UTXOs falsely marked spent.
+            // Sanity-check that the builder only selected outpoints from
+            // the same height-aware spendable set we handed to input
+            // selection. We deliberately do NOT mark the inputs as spent here
+            // — that happens after a successful broadcast (see #3466 review).
+            // A failed broadcast must not leave UTXOs falsely marked spent.
             let selected: BTreeSet<OutPoint> =
                 tx.input.iter().map(|txin| txin.previous_output).collect();
-            let still_spendable: BTreeSet<OutPoint> = info
-                .get_spendable_utxos()
-                .into_iter()
-                .map(|utxo| utxo.outpoint)
-                .collect();
-            if !selected.is_subset(&still_spendable) {
+            let spendable_outpoints: BTreeSet<OutPoint> =
+                spendable.iter().map(|utxo| utxo.outpoint).collect();
+            if !selected.is_subset(&spendable_outpoints) {
                 return Err(PlatformWalletError::TransactionBuild(
-                    "Selected UTXOs are no longer available (concurrent transaction). \
-                     Please retry."
-                        .to_string(),
+                    "Transaction builder selected an unavailable UTXO. Please retry.".to_string(),
                 ));
             }
 
@@ -164,6 +154,11 @@ impl<B: TransactionBroadcaster + ?Sized> CoreWallet<B> {
 
         // Broadcast first; if the network rejects we leave wallet state
         // untouched so the caller can retry without manual sync repair.
+        // This is intentional even if the remote accepted the transaction
+        // but the broadcast path returned an error: in that ambiguous case
+        // later attempts may reuse the same inputs locally, but the network
+        // rejects the duplicate spend instead of us marking UTXOs spent for
+        // a transaction that might not have propagated.
         self.broadcast_transaction(&tx).await?;
 
         // Now that the tx is in flight, register it as a mempool transaction
@@ -175,15 +170,23 @@ impl<B: TransactionBroadcaster + ?Sized> CoreWallet<B> {
         // broadcast failure.
         {
             let mut wm = self.wallet_manager.write().await;
-            let (wallet, info) =
-                wm.get_wallet_mut_and_info_mut(&self.wallet_id)
-                    .ok_or_else(|| {
-                        crate::error::PlatformWalletError::WalletNotFound(
-                            "Wallet not found in wallet manager".to_string(),
-                        )
-                    })?;
-            info.check_core_transaction(&tx, TransactionContext::Mempool, wallet, true, true)
-                .await;
+            if let Some((wallet, info)) = wm.get_wallet_mut_and_info_mut(&self.wallet_id) {
+                let check_result = info
+                    .check_core_transaction(&tx, TransactionContext::Mempool, wallet, true, true)
+                    .await;
+                if !check_result.is_relevant {
+                    tracing::warn!(
+                        txid = %tx.txid(),
+                        "broadcast transaction was not relevant during post-broadcast wallet registration"
+                    );
+                }
+            } else {
+                tracing::warn!(
+                    wallet_id = %hex::encode(self.wallet_id),
+                    txid = %tx.txid(),
+                    "wallet missing during post-broadcast transaction registration"
+                );
+            }
         }
 
         Ok(tx)

packages/rs-sdk/src/platform/dpns_usernames/mod.rs

@@ -35,6 +35,27 @@ pub fn convert_to_homograph_safe_chars(input: &str) -> String {
         .collect()
 }
 
+fn extract_dpns_label(name: &str) -> &str {
+    if let Some(dot_pos) = name.rfind('.') {
+        let (label_part, suffix) = name.split_at(dot_pos);
+        if suffix.eq_ignore_ascii_case(".dash") {
+            return label_part;
+        }
+    }
+    name
+}
+
+/// Strip an optional case-insensitive `.dash` suffix and apply DPNS
+/// homograph-safe normalization, producing a value suitable for matching
+/// against the `normalizedLabel` field of `domain` documents.
+///
+/// Accepts either a bare label (e.g. `"alice"`) or a full DPNS name
+/// (e.g. `"alice.dash"`, `"Alice.DASH"`) and returns the normalized label
+/// (e.g. `"a11ce"`).
+fn normalize_dpns_label(input: &str) -> String {
+    convert_to_homograph_safe_chars(extract_dpns_label(input))
+}
+
 /// Check if a username is valid according to DPNS rules
 ///
 /// A username is valid if:
@@ -365,19 +386,22 @@ impl Sdk {
     ///
     /// # Arguments
     ///
-    /// * `label` - The username label to check (e.g., "alice")
+    /// * `name` - The username label (e.g., "alice") or full DPNS name
+    ///   (e.g., "alice.dash"). The `.dash` suffix is matched
+    ///   case-insensitively and stripped before normalization, mirroring
+    ///   [`Sdk::resolve_dpns_name`].
     ///
     /// # Returns
     ///
     /// Returns `true` if the name is available, `false` if it's taken
-    pub async fn is_dpns_name_available(&self, label: &str) -> Result<bool, Error> {
+    pub async fn is_dpns_name_available(&self, name: &str) -> Result<bool, Error> {
         use crate::platform::documents::document_query::DocumentQuery;
         use drive::query::WhereClause;
         use drive::query::WhereOperator;
 
         let dpns_contract = self.fetch_dpns_contract().await?;
 
-        let normalized_label = convert_to_homograph_safe_chars(label);
+        let normalized_label = normalize_dpns_label(name);
 
         // Query for existing domain with this label
         let query = DocumentQuery {
@@ -422,29 +446,13 @@ impl Sdk {
 
         let dpns_contract = self.fetch_dpns_contract().await?;
 
-        // Extract label from full name if needed
-        // Handle both "alice" and "alice.dash" formats
-        let label = if let Some(dot_pos) = name.rfind('.') {
-            let (label_part, suffix) = name.split_at(dot_pos);
-            // Strip ".dash" / ".DASH" / mixed case — DPNS itself is case-insensitive.
-            if suffix.eq_ignore_ascii_case(".dash") {
-                label_part
-            } else {
-                // If it's not ".dash", treat the whole thing as the label
-                name
-            }
-        } else {
-            // No dot found, use the whole name as the label
-            name
-        };
+        let normalized_label = normalize_dpns_label(name);
 
-        // Validate the label before proceeding
-        if label.is_empty() {
+        // Validate the normalized label before proceeding
+        if normalized_label.is_empty() {
             return Ok(None);
         }
 
-        let normalized_label = convert_to_homograph_safe_chars(label);
-
         // Query for domain with this label
         let query = DocumentQuery {
             data_contract: dpns_contract,
@@ -499,6 +507,40 @@ mod tests {
         assert_eq!(convert_to_homograph_safe_chars("test123"), "test123");
     }
 
+    #[test]
+    fn test_normalize_dpns_label_strips_dash_suffix_case_insensitively() {
+        // Bare label and full name normalize to the same value, regardless
+        // of the case of the .dash suffix. This is the contract that
+        // `is_dpns_name_available` and `resolve_dpns_name` share so that
+        // queries against `normalizedLabel` agree.
+        let expected = "a11ce";
+        assert_eq!(normalize_dpns_label("alice"), expected);
+        assert_eq!(normalize_dpns_label("alice.dash"), expected);
+        assert_eq!(normalize_dpns_label("alice.DASH"), expected);
+        assert_eq!(normalize_dpns_label("Alice.DaSh"), expected);
+        assert_eq!(normalize_dpns_label("ALICE.DASH"), expected);
+
+        // Non-.dash suffixes are not stripped (they are treated as part of
+        // the label and normalized whole).
+        assert_eq!(normalize_dpns_label("alice.eth"), "a11ce.eth");
+
+        // Empty / suffix-only inputs normalize to an empty label.
+        assert_eq!(normalize_dpns_label(""), "");
+        assert_eq!(normalize_dpns_label(".dash"), "");
+        assert_eq!(normalize_dpns_label(".DASH"), "");
+    }
+
+    #[test]
+    fn test_extract_dpns_label() {
+        assert_eq!(extract_dpns_label("alice.dash"), "alice");
+        assert_eq!(extract_dpns_label("alice.DASH"), "alice");
+        assert_eq!(extract_dpns_label("alice.DaSh"), "alice");
+        assert_eq!(extract_dpns_label("Alice.DASH"), "Alice");
+        assert_eq!(extract_dpns_label("alice"), "alice");
+        assert_eq!(extract_dpns_label("alice.eth"), "alice.eth");
+        assert_eq!(extract_dpns_label(".dash"), "");
+    }
+
     #[test]
     fn test_is_valid_username() {
         // Valid usernames