Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lklimek wants to merge 3 commits into
base: feat/platform-wallet
Choose a base branch
from
+44 −8
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c9c81b9
fix: case-insensitive .dash suffix and UTXO double-spend prevention
lklimek Apr 9, 2026
fe8f263
fix(deps): convert rust-dashcore path deps to git branch deps
lklimek Apr 9, 2026
d971326
Revert "fix(deps): convert rust-dashcore path deps to git branch deps"
lklimek Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 43 additions & 7 deletions packages/rs-platform-wallet/src/wallet/core/wallet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use std::sync::Arc;

use super::balance::WalletBalance;

use dashcore::Address as DashAddress;
use dashcore::secp256k1::{Message, Secp256k1};
use dashcore::sighash::SighashCache;
use dashcore::Address as DashAddress;
use dashcore::{OutPoint, ScriptBuf, Transaction, TxIn, TxOut};
use key_wallet::Utxo;
use tokio::sync::RwLock;
Expand Down Expand Up @@ -316,6 +316,38 @@ impl CoreWallet {
self.sign_transaction_inputs(&secp, &mut tx, &selected_utxos)
.await?;

// 5b. Validate-and-mark under write lock: verify that the UTXOs we
// selected (under a read lock earlier) haven't been claimed by a
// concurrent caller. If they have, fail fast with a clear error
// instead of broadcasting a transaction the network will reject.
{
use crate::wallet::platform_wallet_traits::WalletTransactionChecker;
use key_wallet::transaction_checking::TransactionContext;
let mut state = self.state.write().await;

// Re-check that our selected outpoints are still spendable.
let current_spendable: std::collections::BTreeSet<OutPoint> = state
.managed_state
.wallet_info()
.get_spendable_utxos()
.iter()
.map(|u| u.outpoint)
.collect();

for (outpoint, _, _) in &selected_utxos {
if !current_spendable.contains(outpoint) {
return Err(PlatformWalletError::TransactionBuild(
"Selected UTXOs are no longer available (concurrent transaction). Please retry.".to_string(),
));
}
}

// 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?;
Comment on lines +345 to 352
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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.

Comment on lines +345 to 352
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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.


Expand Down Expand Up @@ -479,12 +511,16 @@ impl CoreWallet {
// Derive private keys and sign.
for (i, (input, sighash)) in tx.input.iter_mut().zip(sighashes).enumerate() {
let path = &derivation_paths[i];
let extended_key = info.managed_state.wallet().derive_extended_private_key(path).map_err(|e| {
PlatformWalletError::TransactionBuild(format!(
"Failed to derive key for input {}: {}",
i, e
))
})?;
let extended_key = info
.managed_state
.wallet()
.derive_extended_private_key(path)
.map_err(|e| {
PlatformWalletError::TransactionBuild(format!(
"Failed to derive key for input {}: {}",
i, e
))
})?;
let input_private_key = extended_key.to_priv();

let message = Message::from_digest(sighash.into());
Expand Down
2 changes: 1 addition & 1 deletion packages/rs-sdk/src/platform/dpns_usernames/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ impl Sdk {
let label = if let Some(dot_pos) = name.rfind('.') {
let (label_part, suffix) = name.split_at(dot_pos);
// Only strip the suffix if it's exactly ".dash"
if suffix == ".dash" {
if suffix.eq_ignore_ascii_case(".dash") {
label_part
} else {
// If it's not ".dash", treat the whole thing as the label
Expand Down
Loading