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,16 @@ 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
+}
+
 /// Check if a username is valid according to DPNS rules
 ///
 /// A username is valid if:
@@ -422,21 +432,7 @@ 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 label = extract_dpns_label(name);
 
         // Validate the label before proceeding
         if label.is_empty() {
@@ -499,6 +495,17 @@ mod tests {
         assert_eq!(convert_to_homograph_safe_chars("test123"), "test123");
     }
 
+    #[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