fix: case-insensitive .dash suffix and UTXO double-spend prevention (backport from DET) by lklimek · Pull Request #3466 · dashpay/platform

lklimek · 2026-04-09T10:53:10Z

Summary

Two targeted fixes backported from dash-evo-tool into the platform wallet codebase. No upstream merge — just the fixes on top of feat/platform-wallet.

Context: what moved from dash-evo-tool to platform

The feat/platform-wallet refactor extracted wallet logic from dash-evo-tool into packages/rs-platform-wallet/:

From dash-evo-tool	Now in platform
`src/spv/manager.rs` — SPV lifecycle, broadcast	`rs-platform-wallet/src/spv/` — rewritten around `WalletManager`
`src/backend_task/dashpay/dip14_derivation.rs`	`rs-platform-wallet/src/wallet/dashpay/dip14.rs`
`src/model/wallet/asset_lock_transaction.rs`	`rs-platform-wallet/src/wallet/asset_lock/`
Wallet state, UTXO management (scattered)	`rs-platform-wallet/src/wallet/core/wallet.rs` — unified `CoreWallet`

These fixes address bugs discovered in dash-evo-tool (PRs #810 and #815) that also exist in the platform-side code after the split.

Fix 1: Case-insensitive `.dash` suffix (SDK)

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

resolve_dpns_name() stripped the .dash suffix using exact match (suffix == ".dash"). Inputs like "Alice.DASH" or "alice.Dash" would not match — the full string including .dash would be normalized as the label → DPNS lookup miss.

-            if suffix == ".dash" {
+            if suffix.eq_ignore_ascii_case(".dash") {

Backported from dash-evo-tool PR #810 which fixed the equivalent bug in DET's DPNS helpers.

Fix 2: UTXO double-spend prevention (platform-wallet)

File: packages/rs-platform-wallet/src/wallet/core/wallet.rs

CoreWallet::send_transaction() had a TOCTOU race between reading spendable UTXOs and broadcasting:

Read UTXOs (acquires read lock, drops it)
Select, build, sign (no lock held)
Broadcast (no lock held)

Concurrent callers could both select the same UTXOs between steps 1–3. The second broadcast would be rejected by the network (no IS lock issued for the conflicting tx), but the user sees a failed payment with no clear cause.

Fix — optimistic validation pattern:

After signing, acquire the write lock and re-validate that the selected outpoints are still in the spendable UTXO set. If a concurrent caller already claimed them via check_core_transaction, return a clear error instead of broadcasting a doomed transaction. If all UTXOs are still available, mark them atomically before dropping the lock:

1. Read UTXOs (read lock, drop)         ← optimistic read
2. Select, build, sign (no lock)        ← optimistic work
3. Acquire WRITE lock                   ← validation gate
4. Re-check selected outpoints          ← reject if stale
5. check_core_transaction (Mempool)     ← mark spent atomically
6. Drop write lock
7. Broadcast

This eliminates the race: the second concurrent caller gets "Selected UTXOs are no longer available (concurrent transaction). Please retry." instead of a network rejection.

Backported from dash-evo-tool v1.0-dev commit f4d3b3b3 which fixed the same race in the old SPV manager, adapted to the platform-wallet's RwLock<PlatformWalletInfo> architecture.

Companion PR

dash-evo-tool: chore: backport v1.0-dev into feat/platform-wallet (lazy contexts, DB migrations, DPNS fixes) dash-evo-tool#821 — backports v1.0-dev into feat/platform-wallet (lazy contexts, DB migrations, DPNS fixes)

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

coderabbitai · 2026-04-09T10:53:17Z

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: cdd8ebef-9606-4b81-9991-2194819f6f8a

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch backport/det-fixes-into-platform-wallet

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

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

thepastaclaw · 2026-04-09T11:03:04Z

✅ Review complete (commit d971326)

1. DPNS name normalization: strip the `.dash` suffix case-insensitively before querying Platform, so `.DASH`, `.Dash`, etc. all resolve. 2. UTXO double-spend prevention via optimistic validation: after signing the transaction (optimistically, without locks), re-acquire the write lock and verify that every selected UTXO is still in the spendable set before marking them spent. If a concurrent caller already claimed an outpoint, return an error instead of broadcasting a transaction the network would reject. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

thepastaclaw

Code Review

I verified the non-empty agent findings against HEAD c9c81b9. The DPNS suffix change is fine, but the wallet change introduces a real state-consistency bug: send_transaction() now mutates wallet spendability and balance before the broadcast succeeds, and there is no rollback on broadcast failure.

Reviewed commit: c9c81b9

🔴 1 blocking

🤖 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-platform-wallet/src/wallet/core/wallet.rs`:
- [BLOCKING] lines 345-352: Pre-broadcast state mutation leaves UTXOs locally spent when broadcast fails
  `send_transaction()` now calls `check_core_transaction(&tx, TransactionContext::Mempool, true, true)` before `broadcast_transaction()`. In this crate, that call delegates into `managed_state.check_core_transaction(...)` and immediately refreshes the wallet balance atomics when the transaction is relevant, so the wallet state is committed before any network side effect is known to have succeeded. If `broadcast_transaction()` then returns `TransactionBroadcast`, `SpvNotRunning`, or `SpvError`, the function exits with `Err` but never restores the consumed UTXOs or reverted balance, so a transient broadcast failure can make those outpoints disappear from future `get_spendable_utxos()` results until an external sync repairs state.

thepastaclaw · 2026-04-09T13:56:28Z

+            // All UTXOs still available — mark them as spent atomically.
+            state
+                .check_core_transaction(&tx, TransactionContext::Mempool, true, true)
+                .await;
+        }
+
        // 6. Broadcast.
        self.broadcast_transaction(&tx).await?;


🔴 Blocking: Pre-broadcast state mutation leaves UTXOs locally spent when broadcast fails

send_transaction() now calls check_core_transaction(&tx, TransactionContext::Mempool, true, true) before broadcast_transaction(). In this crate, that call delegates into managed_state.check_core_transaction(...) and immediately refreshes the wallet balance atomics when the transaction is relevant, so the wallet state is committed before any network side effect is known to have succeeded. If broadcast_transaction() then returns TransactionBroadcast, SpvNotRunning, or SpvError, the function exits with Err but never restores the consumed UTXOs or reverted balance, so a transient broadcast failure can make those outpoints disappear from future get_spendable_utxos() results until an external sync repairs state.

_{source: ['codex-general', 'codex-rust-quality']}

🤖 Fix this 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-platform-wallet/src/wallet/core/wallet.rs`: - [BLOCKING] lines 345-352: Pre-broadcast state mutation leaves UTXOs locally spent when broadcast fails `send_transaction()` now calls `check_core_transaction(&tx, TransactionContext::Mempool, true, true)` before `broadcast_transaction()`. In this crate, that call delegates into `managed_state.check_core_transaction(...)` and immediately refreshes the wallet balance atomics when the transaction is relevant, so the wallet state is committed before any network side effect is known to have succeeded. If `broadcast_transaction()` then returns `TransactionBroadcast`, `SpvNotRunning`, or `SpvError`, the function exits with `Err` but never restores the consumed UTXOs or reverted balance, so a transient broadcast failure can make those outpoints disappear from future `get_spendable_utxos()` results until an external sync repairs state.

Path dependencies on ../rust-dashcore break when this repo is used as a git dependency (e.g. from dash-evo-tool CI). Switch to git deps pointing to feat/blockchain-identities-account-type branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

This reverts commit fe8f263.

thepastaclaw

Code Review

The DPNS suffix change is fine. The wallet concurrency fix introduces a real state-consistency regression: send_transaction() now commits wallet-local spend state before the only fallible network step, so a transient broadcast failure can make the wallet report funds as spent even though the transaction was never accepted. That is a blocking correctness issue for wallet behavior on this head.

Reviewed commit: d971326

🔴 1 blocking

🤖 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-platform-wallet/src/wallet/core/wallet.rs`:
- [BLOCKING] lines 345-352: send_transaction mutates wallet spend state before a fallible broadcast
  This new write-locked path calls `check_core_transaction(&tx, TransactionContext::Mempool, true, true)` before broadcasting. In this crate that is not a dry run: `PlatformWalletInfo::check_core_transaction()` forwards `update_state=true` into `managed_state.check_core_transaction(...)` and then refreshes the shared balance cache when the transaction is relevant (`packages/rs-platform-wallet/src/wallet/platform_wallet_traits.rs:179`). The broadcaster call that follows can still fail with ordinary runtime errors such as `TransactionBroadcast`, `SpvNotRunning`, or `SpvError` (`packages/rs-platform-wallet/src/broadcaster.rs:41`, `packages/rs-platform-wallet/src/spv/runtime.rs:128`), and there is no compensating rollback on the error path. That leaves the live wallet in a self-contradictory state: `send_transaction()` returns `Err`, but the selected outpoints have already been removed from future spendable-UTXO selection and the cached balance may already be reduced until some later sync repairs it.

thepastaclaw · 2026-04-09T23:50:13Z

+            // All UTXOs still available — mark them as spent atomically.
+            state
+                .check_core_transaction(&tx, TransactionContext::Mempool, true, true)
+                .await;
+        }
+
        // 6. Broadcast.
        self.broadcast_transaction(&tx).await?;


🔴 Blocking: send_transaction mutates wallet spend state before a fallible broadcast

This new write-locked path calls check_core_transaction(&tx, TransactionContext::Mempool, true, true) before broadcasting. In this crate that is not a dry run: PlatformWalletInfo::check_core_transaction() forwards update_state=true into managed_state.check_core_transaction(...) and then refreshes the shared balance cache when the transaction is relevant (packages/rs-platform-wallet/src/wallet/platform_wallet_traits.rs:179). The broadcaster call that follows can still fail with ordinary runtime errors such as TransactionBroadcast, SpvNotRunning, or SpvError (packages/rs-platform-wallet/src/broadcaster.rs:41, packages/rs-platform-wallet/src/spv/runtime.rs:128), and there is no compensating rollback on the error path. That leaves the live wallet in a self-contradictory state: send_transaction() returns Err, but the selected outpoints have already been removed from future spendable-UTXO selection and the cached balance may already be reduced until some later sync repairs it.

_{source: ['codex']}

🤖 Fix this 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-platform-wallet/src/wallet/core/wallet.rs`: - [BLOCKING] lines 345-352: send_transaction mutates wallet spend state before a fallible broadcast This new write-locked path calls `check_core_transaction(&tx, TransactionContext::Mempool, true, true)` before broadcasting. In this crate that is not a dry run: `PlatformWalletInfo::check_core_transaction()` forwards `update_state=true` into `managed_state.check_core_transaction(...)` and then refreshes the shared balance cache when the transaction is relevant (`packages/rs-platform-wallet/src/wallet/platform_wallet_traits.rs:179`). The broadcaster call that follows can still fail with ordinary runtime errors such as `TransactionBroadcast`, `SpvNotRunning`, or `SpvError` (`packages/rs-platform-wallet/src/broadcaster.rs:41`, `packages/rs-platform-wallet/src/spv/runtime.rs:128`), and there is no compensating rollback on the error path. That leaves the live wallet in a self-contradictory state: `send_transaction()` returns `Err`, but the selected outpoints have already been removed from future spendable-UTXO selection and the cached balance may already be reduced until some later sync repairs it.

lklimek mentioned this pull request Apr 9, 2026

chore: backport v1.0-dev into feat/platform-wallet (lazy contexts, DB migrations, DPNS fixes) dashpay/dash-evo-tool#821

Merged

lklimek marked this pull request as ready for review April 9, 2026 11:01

lklimek requested review from QuantumExplorer and shumkov as code owners April 9, 2026 11:01

lklimek force-pushed the backport/det-fixes-into-platform-wallet branch from 1ef274a to c9c81b9 Compare April 9, 2026 11:09

thepastaclaw suggested changes Apr 9, 2026

View reviewed changes

lklimek and others added 2 commits April 9, 2026 16:26

Revert "fix(deps): convert rust-dashcore path deps to git branch deps"

d971326

This reverts commit fe8f263.

thepastaclaw suggested changes Apr 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: case-insensitive .dash suffix and UTXO double-spend prevention (backport from DET)#3466

fix: case-insensitive .dash suffix and UTXO double-spend prevention (backport from DET)#3466
lklimek wants to merge 3 commits intofeat/platform-walletfrom
backport/det-fixes-into-platform-wallet

lklimek commented Apr 9, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Apr 9, 2026 •

edited

Loading

Review skipped

Uh oh!

thepastaclaw commented Apr 9, 2026 •

edited

Loading

Uh oh!

thepastaclaw left a comment

Uh oh!

thepastaclaw Apr 9, 2026

Uh oh!

thepastaclaw left a comment

Uh oh!

thepastaclaw Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

lklimek commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Context: what moved from dash-evo-tool to platform

Fix 1: Case-insensitive .dash suffix (SDK)

Fix 2: UTXO double-spend prevention (platform-wallet)

Companion PR

Uh oh!

coderabbitai bot commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

thepastaclaw commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

thepastaclaw Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

thepastaclaw Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lklimek commented Apr 9, 2026 •

edited

Loading

Fix 1: Case-insensitive `.dash` suffix (SDK)

coderabbitai bot commented Apr 9, 2026 •

edited

Loading

thepastaclaw commented Apr 9, 2026 •

edited

Loading