Skip to content

Commit 1bd306a

Browse files
authored
fix: improve platform wallet UTXO checks and DPNS parsing (#3595)
Co-authored-by: PastaClaw <thepastaclaw@users.noreply.github.com>
1 parent 0d17a63 commit 1bd306a

2 files changed

Lines changed: 133 additions & 51 deletions

File tree

  • packages

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

Lines changed: 59 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -134,28 +134,18 @@ impl<B: TransactionBroadcaster + ?Sized> CoreWallet<B> {
134134
.build()
135135
.map_err(|e| PlatformWalletError::TransactionBuild(e.to_string()))?;
136136

137-
// Re-validate the selected outpoints are still spendable while
138-
// we still hold the write lock. The lock makes our build atomic
139-
// against other callers on this handle, but external mempool /
140-
// block events processed before we acquired the lock may have
141-
// invalidated UTXOs that were still in the spendable set when
142-
// `select_inputs` ran.
143-
//
144-
// We deliberately do NOT mark the inputs as spent here — that
145-
// happens after a successful broadcast (see #3466 review). A
146-
// failed broadcast must not leave UTXOs falsely marked spent.
137+
// Sanity-check that the builder only selected outpoints from
138+
// the same height-aware spendable set we handed to input
139+
// selection. We deliberately do NOT mark the inputs as spent here
140+
// — that happens after a successful broadcast (see #3466 review).
141+
// A failed broadcast must not leave UTXOs falsely marked spent.
147142
let selected: BTreeSet<OutPoint> =
148143
tx.input.iter().map(|txin| txin.previous_output).collect();
149-
let still_spendable: BTreeSet<OutPoint> = info
150-
.get_spendable_utxos()
151-
.into_iter()
152-
.map(|utxo| utxo.outpoint)
153-
.collect();
154-
if !selected.is_subset(&still_spendable) {
144+
let spendable_outpoints: BTreeSet<OutPoint> =
145+
spendable.iter().map(|utxo| utxo.outpoint).collect();
146+
if !selected.is_subset(&spendable_outpoints) {
155147
return Err(PlatformWalletError::TransactionBuild(
156-
"Selected UTXOs are no longer available (concurrent transaction). \
157-
Please retry."
158-
.to_string(),
148+
"Transaction builder selected an unavailable UTXO. Please retry.".to_string(),
159149
));
160150
}
161151

@@ -164,6 +154,11 @@ impl<B: TransactionBroadcaster + ?Sized> CoreWallet<B> {
164154

165155
// Broadcast first; if the network rejects we leave wallet state
166156
// untouched so the caller can retry without manual sync repair.
157+
// This is intentional even if the remote accepted the transaction
158+
// but the broadcast path returned an error: in that ambiguous case
159+
// later attempts may reuse the same inputs locally, but the network
160+
// rejects the duplicate spend instead of us marking UTXOs spent for
161+
// a transaction that might not have propagated.
167162
self.broadcast_transaction(&tx).await?;
168163

169164
// Now that the tx is in flight, register it as a mempool transaction
@@ -173,17 +168,53 @@ impl<B: TransactionBroadcaster + ?Sized> CoreWallet<B> {
173168
// network resolves that race exactly as it does on `v3.1-dev`
174169
// today, but neither caller corrupts local state on a transient
175170
// broadcast failure.
171+
//
172+
// Broadcast-first semantics: by the time we get here the network has
173+
// already accepted the transaction, so the two warning paths below
174+
// intentionally do NOT convert into a post-success `Err`. They
175+
// simply mean local wallet state did not get updated to reflect the
176+
// mempool spend / change output. Recovery in both cases:
177+
//
178+
// * The next `send_to_addresses` from the same handle may reselect
179+
// the same UTXOs because they still look spendable locally. That
180+
// follow-up transaction will be rejected by the network as a
181+
// duplicate spend (the broadcaster surfaces that as an error to
182+
// the caller), so funds are never double-spent on-chain.
183+
// * Once mempool/block sync catches up, the wallet will see the
184+
// original transaction and reconcile its UTXO set, after which
185+
// subsequent sends pick up the correct change outputs.
186+
//
187+
// The two cases differ in what they imply:
188+
//
189+
// * `!check_result.is_relevant` is the expected transient: the
190+
// wallet just hasn't ingested the tx yet (or some derivation
191+
// path/script is unrecognised), and a later sync will fix it.
192+
// * The `else` branch (wallet missing in the manager) is NOT a
193+
// normal transient — the broadcast succeeded against a
194+
// `CoreWallet` handle whose underlying wallet entry is gone
195+
// from the manager. That is a broken/inconsistent local handle
196+
// and the warning exists so operators can spot it; future
197+
// sends through the same handle will keep failing the lookup
198+
// above and surface a clean `WalletNotFound` error.
176199
{
177200
let mut wm = self.wallet_manager.write().await;
178-
let (wallet, info) =
179-
wm.get_wallet_mut_and_info_mut(&self.wallet_id)
180-
.ok_or_else(|| {
181-
crate::error::PlatformWalletError::WalletNotFound(
182-
"Wallet not found in wallet manager".to_string(),
183-
)
184-
})?;
185-
info.check_core_transaction(&tx, TransactionContext::Mempool, wallet, true, true)
186-
.await;
201+
if let Some((wallet, info)) = wm.get_wallet_mut_and_info_mut(&self.wallet_id) {
202+
let check_result = info
203+
.check_core_transaction(&tx, TransactionContext::Mempool, wallet, true, true)
204+
.await;
205+
if !check_result.is_relevant {
206+
tracing::warn!(
207+
txid = %tx.txid(),
208+
"broadcast transaction was not relevant during post-broadcast wallet registration"
209+
);
210+
}
211+
} else {
212+
tracing::warn!(
213+
wallet_id = %hex::encode(self.wallet_id),
214+
txid = %tx.txid(),
215+
"wallet missing during post-broadcast transaction registration"
216+
);
217+
}
187218
}
188219

189220
Ok(tx)

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

Lines changed: 74 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,27 @@ pub fn convert_to_homograph_safe_chars(input: &str) -> String {
3535
.collect()
3636
}
3737

38+
fn extract_dpns_label(name: &str) -> &str {
39+
if let Some(dot_pos) = name.rfind('.') {
40+
let (label_part, suffix) = name.split_at(dot_pos);
41+
if suffix.eq_ignore_ascii_case(".dash") {
42+
return label_part;
43+
}
44+
}
45+
name
46+
}
47+
48+
/// Strip an optional case-insensitive `.dash` suffix and apply DPNS
49+
/// homograph-safe normalization, producing a value suitable for matching
50+
/// against the `normalizedLabel` field of `domain` documents.
51+
///
52+
/// Accepts either a bare label (e.g. `"alice"`) or a full DPNS name
53+
/// (e.g. `"alice.dash"`, `"Alice.DASH"`) and returns the normalized label
54+
/// (e.g. `"a11ce"`).
55+
fn normalize_dpns_label(input: &str) -> String {
56+
convert_to_homograph_safe_chars(extract_dpns_label(input))
57+
}
58+
3859
/// Check if a username is valid according to DPNS rules
3960
///
4061
/// A username is valid if:
@@ -365,19 +386,31 @@ impl Sdk {
365386
///
366387
/// # Arguments
367388
///
368-
/// * `label` - The username label to check (e.g., "alice")
389+
/// * `name` - The username label (e.g., "alice") or full DPNS name
390+
/// (e.g., "alice.dash"). The `.dash` suffix is matched
391+
/// case-insensitively and stripped before normalization, mirroring
392+
/// [`Sdk::resolve_dpns_name`].
369393
///
370394
/// # Returns
371395
///
372396
/// Returns `true` if the name is available, `false` if it's taken
373-
pub async fn is_dpns_name_available(&self, label: &str) -> Result<bool, Error> {
397+
pub async fn is_dpns_name_available(&self, name: &str) -> Result<bool, Error> {
374398
use crate::platform::documents::document_query::DocumentQuery;
375399
use drive::query::WhereClause;
376400
use drive::query::WhereOperator;
377401

378-
let dpns_contract = self.fetch_dpns_contract().await?;
402+
let normalized_label = normalize_dpns_label(name);
403+
404+
// An empty normalized label (e.g. `""`, `".dash"`, `".DASH"`) is not
405+
// a registrable DPNS name, so report it as unavailable rather than
406+
// doing a network round-trip that would query for
407+
// `normalizedLabel == ""`. This mirrors the early-return guard in
408+
// `resolve_dpns_name` so the two APIs agree on malformed input.
409+
if normalized_label.is_empty() {
410+
return Ok(false);
411+
}
379412

380-
let normalized_label = convert_to_homograph_safe_chars(label);
413+
let dpns_contract = self.fetch_dpns_contract().await?;
381414

382415
// Query for existing domain with this label
383416
let query = DocumentQuery {
@@ -422,29 +455,13 @@ impl Sdk {
422455

423456
let dpns_contract = self.fetch_dpns_contract().await?;
424457

425-
// Extract label from full name if needed
426-
// Handle both "alice" and "alice.dash" formats
427-
let label = if let Some(dot_pos) = name.rfind('.') {
428-
let (label_part, suffix) = name.split_at(dot_pos);
429-
// Strip ".dash" / ".DASH" / mixed case — DPNS itself is case-insensitive.
430-
if suffix.eq_ignore_ascii_case(".dash") {
431-
label_part
432-
} else {
433-
// If it's not ".dash", treat the whole thing as the label
434-
name
435-
}
436-
} else {
437-
// No dot found, use the whole name as the label
438-
name
439-
};
458+
let normalized_label = normalize_dpns_label(name);
440459

441-
// Validate the label before proceeding
442-
if label.is_empty() {
460+
// Validate the normalized label before proceeding
461+
if normalized_label.is_empty() {
443462
return Ok(None);
444463
}
445464

446-
let normalized_label = convert_to_homograph_safe_chars(label);
447-
448465
// Query for domain with this label
449466
let query = DocumentQuery {
450467
data_contract: dpns_contract,
@@ -499,6 +516,40 @@ mod tests {
499516
assert_eq!(convert_to_homograph_safe_chars("test123"), "test123");
500517
}
501518

519+
#[test]
520+
fn test_normalize_dpns_label_strips_dash_suffix_case_insensitively() {
521+
// Bare label and full name normalize to the same value, regardless
522+
// of the case of the .dash suffix. This is the contract that
523+
// `is_dpns_name_available` and `resolve_dpns_name` share so that
524+
// queries against `normalizedLabel` agree.
525+
let expected = "a11ce";
526+
assert_eq!(normalize_dpns_label("alice"), expected);
527+
assert_eq!(normalize_dpns_label("alice.dash"), expected);
528+
assert_eq!(normalize_dpns_label("alice.DASH"), expected);
529+
assert_eq!(normalize_dpns_label("Alice.DaSh"), expected);
530+
assert_eq!(normalize_dpns_label("ALICE.DASH"), expected);
531+
532+
// Non-.dash suffixes are not stripped (they are treated as part of
533+
// the label and normalized whole).
534+
assert_eq!(normalize_dpns_label("alice.eth"), "a11ce.eth");
535+
536+
// Empty / suffix-only inputs normalize to an empty label.
537+
assert_eq!(normalize_dpns_label(""), "");
538+
assert_eq!(normalize_dpns_label(".dash"), "");
539+
assert_eq!(normalize_dpns_label(".DASH"), "");
540+
}
541+
542+
#[test]
543+
fn test_extract_dpns_label() {
544+
assert_eq!(extract_dpns_label("alice.dash"), "alice");
545+
assert_eq!(extract_dpns_label("alice.DASH"), "alice");
546+
assert_eq!(extract_dpns_label("alice.DaSh"), "alice");
547+
assert_eq!(extract_dpns_label("Alice.DASH"), "Alice");
548+
assert_eq!(extract_dpns_label("alice"), "alice");
549+
assert_eq!(extract_dpns_label("alice.eth"), "alice.eth");
550+
assert_eq!(extract_dpns_label(".dash"), "");
551+
}
552+
502553
#[test]
503554
fn test_is_valid_username() {
504555
// Valid usernames

0 commit comments

Comments
 (0)