fix: improve platform wallet UTXO checks and DPNS parsing

[thepastaclaw]

Summary

Addresses review feedback from #3585:

• documents the intentional post-build/pre-broadcast error behavior
• keeps post-broadcast wallet bookkeeping best-effort after a successful send
• logs unexpected post-broadcast registration outcomes instead of converting the
  send into an error
• uses the same height-aware spendable set for the in-lock UTXO sanity check
• extracts DPNS label parsing and adds mixed-case .dash suffix unit coverage

Validation

• git diff --check
• cargo test -p dash-sdk test_extract_dpns_label --lib
• cargo check -p platform-wallet
• cargo clippy -p dash-sdk --lib -- -D warnings
• cargo clippy -p platform-wallet --lib -- -D warnings was run and reached
  pre-existing clippy failures in
  packages/rs-platform-wallet/src/manager/accessors.rs unrelated to this diff
  (manual_unwrap_or_default, unnecessary_cast, redundant_closure).
• pre-commit hook also ran cargo fmt --all -- --check and workspace
  cargo check successfully during commit.

Summary by CodeRabbit

• Bug Fixes
  • Enhanced transaction validation during wallet broadcasting to ensure UTXOs are selected only from the confirmed spendable set.
  • Improved error handling for post-broadcast wallet state checks with better warning messages for edge cases.
  • Fixed DPNS username resolution to consistently handle naming variations with .dash suffix, covering different case variations.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c110c4ea-2d18-458e-b046-3e572af0f34d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Two independent changes across wallet and SDK packages: broadcast transaction validation now checks UTXOs against a pre-captured spendable set instead of re-deriving it, with updated error handling for missing wallets post-broadcast; DPNS label extraction is refactored into a private helper function with comprehensive unit tests.

Changes

Broadcast Transaction Validation & Registration

Layer / File(s)	Summary
Validation Logic packages/rs-platform-wallet/src/wallet/core/broadcast.rs (lines 137–150)	UTXO validation switches from re-deriving spendable set via info.get_spendable_utxos() to checking against pre-captured spendable_outpoints. Failure message updated from concurrent-transaction phrasing to generic "unavailable UTXO" retry message.
Post-Broadcast Registration packages/rs-platform-wallet/src/wallet/core/broadcast.rs (lines 155–190)	Registration logic now logs a warning and skips if wallet is missing, rather than erroring. Captures check_core_transaction result and warns when transaction is not relevant, instead of ignoring the result.

DPNS Label Extraction Helper

Layer / File(s)	Summary
Helper Function packages/rs-sdk/src/platform/dpns_usernames/mod.rs (lines 38–47)	New private extract_dpns_label(name: &str) -> &str returns the substring before the last dot only when the suffix is .dash (case-insensitive); otherwise returns the original input.
Usage Integration packages/rs-sdk/src/platform/dpns_usernames/mod.rs (lines 432–435)	Sdk::resolve_dpns_name replaces inline label-extraction conditional logic with a call to extract_dpns_label(name).
Tests packages/rs-sdk/src/platform/dpns_usernames/mod.rs (lines 498–508)	Unit tests cover .dash suffix extraction with mixed casing, plain labels, non-.dash dotted names, and edge case of ".dash" yielding empty label.

---

🎯 3 (Moderate) | ⏱️ ~25 minutes

---

🐰 Two little changes, both quite neat,
One checks UTXOs, validation sweet,
The other plucks labels with care—
DPNS names floating in air.
Refactored with tests, crisp and fair! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The pull request title accurately summarizes the two main changes: improving UTXO checks in the platform wallet and fixing DPNS parsing logic.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[thepastaclaw]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[thepastaclaw]

✅ Review complete (commit 1d944a8)

[thepastaclaw]

Code Review

Small, well-scoped fix-up PR. Most agent findings are either stale (referencing the previous comment text), nitpicky, or second-guessing intentional design choices that the PR explicitly documents. The one finding worth surfacing is the API inconsistency between resolve_dpns_name (now strips .DASH case-insensitively) and is_dpns_name_available (still doesn't), which is a reasonable extension of the same fix.

Reviewed commit: ecccf06

🟡 1 suggestion(s)

1 additional finding

🟡 suggestion: `is_dpns_name_available` does not share the case-insensitive `.dash` parsing now used by `resolve_dpns_name`

packages/rs-sdk/src/platform/dpns_usernames/mod.rs (lines 383-417)

The new extract_dpns_label helper is only consumed by resolve_dpns_name, so is_dpns_name_available("alice.DASH") still feeds "alice.DASH" into convert_to_homograph_safe_chars and queries normalizedLabel == "a11ce.dash", which never matches the indexed "a11ce" and reports the name as available even when resolve_dpns_name("alice.DASH") returns an existing identity. The function's docstring does specify that the argument is the label only (alice), so this is not strictly a bug, but it leaves two adjacent public DPNS APIs with divergent normalization rules for the same input. Routing is_dpns_name_available through extract_dpns_label (or documenting that it intentionally rejects full names) would close that gap and was the spirit of the original review feedback.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-sdk/src/platform/dpns_usernames/mod.rs`:
- [SUGGESTION] lines 383-417: `is_dpns_name_available` does not share the case-insensitive `.dash` parsing now used by `resolve_dpns_name`
The new `extract_dpns_label` helper is only consumed by `resolve_dpns_name`, so `is_dpns_name_available("alice.DASH")` still feeds `"alice.DASH"` into `convert_to_homograph_safe_chars` and queries `normalizedLabel == "a11ce.dash"`, which never matches the indexed `"a11ce"` and reports the name as available even when `resolve_dpns_name("alice.DASH")` returns an existing identity. The function's docstring does specify that the argument is the label only (`alice`), so this is not strictly a bug, but it leaves two adjacent public DPNS APIs with divergent normalization rules for the same input. Routing `is_dpns_name_available` through `extract_dpns_label` (or documenting that it intentionally rejects full names) would close that gap and was the spirit of the original review feedback.

[thepastaclaw]

Code Review

Verified at SHA 3dfeafc. The PR is small and focused, and the prior carry-forward concern about case-insensitive .dash handling is genuinely resolved by routing both DPNS APIs through normalize_dpns_label. One real residual inconsistency remains: is_dpns_name_available lacks the empty-normalized-label guard that resolve_dpns_name has, so suffix-only inputs like ".dash" are reported as available. The wallet best-effort registration after a successful broadcast is documented as intentional; the codex-general 'blocking' framing overstates the impact since the network rejects duplicate spends and the trade-off is explicitly noted in code comments.

Reviewed commit: 3dfeafc

🟡 2 suggestion(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-sdk/src/platform/dpns_usernames/mod.rs`:
- [SUGGESTION] lines 404-405: `is_dpns_name_available` should mirror `resolve_dpns_name`'s empty-normalized-label guard
After `let normalized_label = normalize_dpns_label(name);`, this function unconditionally issues a `fetch_many` against `normalizedLabel == ""` for inputs like `""`, `".dash"`, or `".DASH"` (covered by the new unit test at lines 527-530). Since no registered domain has an empty `normalizedLabel`, the function returns `Ok(true)` and reports an unregistrable label as available, which is the opposite of the safe answer. `resolve_dpns_name` at lines 451-454 short-circuits this exact case with `Ok(None)`, and the docstring at lines 390-392 explicitly promises that `is_dpns_name_available` mirrors `Sdk::resolve_dpns_name`. Add the same guard (returning `Ok(false)` for an empty normalized label, or surfacing a typed validation error) so the two adjacent APIs agree end-to-end and the network roundtrip is avoided for clearly malformed input.
In `packages/rs-platform-wallet/src/wallet/core/broadcast.rs`:
- [SUGGESTION] lines 171-190: Post-broadcast bookkeeping failures are silently swallowed; consider surfacing them or documenting expected recovery
After `broadcast_transaction(&tx)` succeeds, this block downgrades two failure modes to `tracing::warn!` and still returns `Ok(tx)`: (a) the wallet is missing from `wallet_manager`, and (b) `check_core_transaction` reports the just-built transaction as not relevant. In both cases the wallet never records the mempool spend or change output, so a subsequent `send_to_addresses` from the same handle can reselect the already-broadcast inputs; the network will then reject the duplicate spend at broadcast time, surfacing as a confusing `TransactionBuild`/broadcast error to the next caller until a later mempool/block sync repairs local state. The design comment at lines 155-170 documents broadcast-first semantics intentionally (returning `Err` after a successful broadcast would itself be misleading), so this is more of a robustness suggestion than a bug: consider either returning a typed warning to the caller, or expanding the comment to explicitly cover the wallet-missing / not-relevant cases and how the next sync is expected to repair state. The wallet-missing branch in particular does not look like a transient condition.

[thepastaclaw]

Code Review

Small PR with both prior carry-forward findings already addressed. One legitimate cross-language gap remains: the C/Swift FFI for DPNS availability still rejects full .dash names that the Rust SDK (widened in this PR) now accepts. The Rust-quality nitpick about the subset check duplicates a design decision the author already documented.

Reviewed commit: 1d944a8

🟡 1 suggestion(s)

1 additional finding

🟡 suggestion: C/Swift DPNS availability FFI still rejects full `.dash` names that the Rust SDK now accepts

packages/rs-sdk-ffi/src/dpns/queries/availability.rs (lines 59-83)

This PR widens Sdk::is_dpns_name_available to accept either a bare label or a full DPNS name, stripping a case-insensitive .dash suffix via normalize_dpns_label (packages/rs-sdk/src/platform/dpns_usernames/mod.rs:397-411). The C FFI wrapper, however, still validates the inbound string with is_valid_username(label_str) before calling into Rust. Because is_valid_username requires alphanumeric start/end and only [a-zA-Z0-9-] in between, inputs like "alice.dash" or "alice.DASH" short-circuit to valid: false, available: false instead of being normalized and queried like the Rust and WASM paths now do. Swift callers reach the SDK exclusively through this wrapper, so non-Rust consumers do not actually receive the new case-insensitive .dash behavior the PR title advertises. Consider either normalizing the suffix at the FFI boundary before the validity check, or invoking the SDK directly and letting it decide.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-sdk-ffi/src/dpns/queries/availability.rs`:
- [SUGGESTION] lines 59-83: C/Swift DPNS availability FFI still rejects full `.dash` names that the Rust SDK now accepts
This PR widens `Sdk::is_dpns_name_available` to accept either a bare label or a full DPNS name, stripping a case-insensitive `.dash` suffix via `normalize_dpns_label` (packages/rs-sdk/src/platform/dpns_usernames/mod.rs:397-411). The C FFI wrapper, however, still validates the inbound string with `is_valid_username(label_str)` before calling into Rust. Because `is_valid_username` requires alphanumeric start/end and only `[a-zA-Z0-9-]` in between, inputs like `"alice.dash"` or `"alice.DASH"` short-circuit to `valid: false, available: false` instead of being normalized and queried like the Rust and WASM paths now do. Swift callers reach the SDK exclusively through this wrapper, so non-Rust consumers do not actually receive the new case-insensitive `.dash` behavior the PR title advertises. Consider either normalizing the suffix at the FFI boundary before the validity check, or invoking the SDK directly and letting it decide.

Fresh dispatcher-requested review for this queue item. A same-SHA blank/placeholder review already existed, so this review is posted explicitly rather than skipped by same-SHA dedup.