diff --git a/packages/rs-platform-wallet/Cargo.toml b/packages/rs-platform-wallet/Cargo.toml index 33f0539c2ad..522b9c7a4e4 100644 --- a/packages/rs-platform-wallet/Cargo.toml +++ b/packages/rs-platform-wallet/Cargo.toml @@ -84,6 +84,10 @@ required-features = ["shielded"] # `tests/shielded_decrypt_bench.rs` to assemble per-chunk wire # fixtures and decode the `ShieldedEncryptedNote` wire type. drive-proof-verifier = { path = "../rs-drive-proof-verifier" } +# `test-utils` exposes key-wallet's `TestWalletContext` + `Address::dummy` +# (via `dashcore/test-utils`) for building funded, signable test wallets. +# Dev-only, so the production build stays on the leaner default features. +key-wallet = { workspace = true, features = ["test-utils"] } rand = "0.8" # Drives the parallel decrypt benchmark in `shielded_decrypt_bench.rs`. rayon = "1.10" diff --git a/packages/rs-platform-wallet/src/broadcaster.rs b/packages/rs-platform-wallet/src/broadcaster.rs index eba802e0ee9..11946185759 100644 --- a/packages/rs-platform-wallet/src/broadcaster.rs +++ b/packages/rs-platform-wallet/src/broadcaster.rs @@ -20,6 +20,8 @@ use crate::spv::SpvRuntime; /// Implementations may use DAPI (gRPC), SPV (P2P peers), or Core RPC. #[async_trait] pub trait TransactionBroadcaster: Send + Sync { + /// Contract: `Err` must mean the transaction was **not** accepted by the + /// network, so callers may safely release its reserved inputs and retry. async fn broadcast(&self, transaction: &Transaction) -> Result; } diff --git a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs index 4609d1fb6d2..7f70b128e5c 100644 --- a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs +++ b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs @@ -1,11 +1,53 @@ use dashcore::{Address as DashAddress, Transaction}; use key_wallet::account::account_type::StandardAccountType; use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; +use key_wallet::managed_account::ManagedCoreFundsAccount; use key_wallet::signer::Signer; use crate::broadcaster::TransactionBroadcaster; +use crate::wallet::platform_wallet::PlatformWalletInfo; use crate::{CoreWallet, PlatformWalletError}; +/// Funds account for `account_type`/`account_index`, or `None` if it is not +/// present. +fn funds_account( + info: &PlatformWalletInfo, + account_type: StandardAccountType, + account_index: u32, +) -> Option<&ManagedCoreFundsAccount> { + match account_type { + StandardAccountType::BIP44Account => info + .core_wallet + .accounts + .standard_bip44_accounts + .get(&account_index), + StandardAccountType::BIP32Account => info + .core_wallet + .accounts + .standard_bip32_accounts + .get(&account_index), + } +} + +/// Whether a failed broadcast is unambiguously *pre-send* — the transaction was +/// rejected by the submission endpoint or the local transport and never +/// accepted by the network — so its reserved inputs are safe to release for an +/// immediate retry. +/// +/// Backed by the [`TransactionBroadcaster::broadcast`] contract that `Err` +/// means "not accepted by the network". Only the variants a broadcaster emits +/// for a rejected submission are listed; anything else — notably the +/// `*Unconfirmed` results that signal the transaction *may* already have reached +/// the network, and any unforeseen future variant — is treated conservatively +/// as *possibly accepted*, keeping the reservation for the TTL backstop to +/// reclaim rather than risking a double-spend on retry. +fn broadcast_failure_is_pre_send(error: &PlatformWalletError) -> bool { + matches!( + error, + PlatformWalletError::TransactionBroadcast(_) | PlatformWalletError::SpvError(_) + ) +} + impl CoreWallet { /// Broadcast a signed transaction to the network. /// @@ -142,7 +184,295 @@ impl CoreWallet { tx }; - self.broadcast_transaction(&tx).await?; + if let Err(e) = self.broadcast_transaction(&tx).await { + // Release the abandoned build's reserved inputs so an immediate retry + // can reselect them, but only when the failure is unambiguously + // pre-send (see `broadcast_failure_is_pre_send`). `release_reservation` + // takes `&self` and the manager map is untouched, so a read lock + // suffices — this cleanup does not serialize concurrent sends. + if broadcast_failure_is_pre_send(&e) { + let wm = self.wallet_manager.read().await; + match wm.get_wallet_and_info(&self.wallet_id) { + Some((_, info)) => match funds_account(info, account_type, account_index) { + Some(managed_account) => managed_account.release_reservation(&tx), + None => tracing::warn!( + wallet_id = %hex::encode(self.wallet_id), + ?account_type, + account_index, + "could not release UTXO reservation after failed broadcast: \ + funds account not found" + ), + }, + None => tracing::warn!( + wallet_id = %hex::encode(self.wallet_id), + "could not release UTXO reservation after failed broadcast: \ + wallet not found" + ), + } + } + return Err(e); + } Ok(tx) } } + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::Arc; + + use async_trait::async_trait; + use dashcore::secp256k1::{ecdsa, Message, PublicKey, Secp256k1}; + use dashcore::{Address as DashAddress, Network, Transaction, Txid}; + use key_wallet::account::account_type::StandardAccountType; + use key_wallet::signer::{Signer, SignerMethod}; + use key_wallet::test_utils::TestWalletContext; + use key_wallet::{DerivationPath, Wallet}; + use key_wallet_manager::WalletManager; + use tokio::sync::RwLock; + + use crate::broadcaster::TransactionBroadcaster; + use crate::wallet::core::{CoreWallet, WalletBalance}; + use crate::wallet::identity::IdentityManager; + use crate::wallet::platform_wallet::PlatformWalletInfo; + use crate::PlatformWalletError; + + /// Broadcaster that fails its first call and succeeds afterwards, to model + /// a transient broadcast error followed by a user retry. + struct FailFirstBroadcaster { + failed_once: AtomicBool, + } + + impl FailFirstBroadcaster { + fn new() -> Self { + Self { + failed_once: AtomicBool::new(false), + } + } + } + + #[async_trait] + impl TransactionBroadcaster for FailFirstBroadcaster { + async fn broadcast(&self, transaction: &Transaction) -> Result { + if self.failed_once.swap(true, Ordering::SeqCst) { + Ok(transaction.txid()) + } else { + Err(PlatformWalletError::TransactionBroadcast( + "simulated broadcast failure".to_string(), + )) + } + } + } + + /// Broadcaster that always fails with an *ambiguous* result — the network + /// may already have accepted the transaction — so its inputs must NOT be + /// released on failure. + struct AlwaysUnconfirmedBroadcaster; + + #[async_trait] + impl TransactionBroadcaster for AlwaysUnconfirmedBroadcaster { + async fn broadcast(&self, _transaction: &Transaction) -> Result { + Err(PlatformWalletError::ShieldedSpendUnconfirmed { + operation: "transfer", + reason: "simulated ambiguous broadcast".to_string(), + }) + } + } + + /// Soft signer that derives keys straight from a test wallet's seed. Stands + /// in for the FFI keychain-backed signer used in production. + struct WalletSigner { + wallet: Wallet, + } + + #[async_trait] + impl Signer for WalletSigner { + type Error = String; + + fn supported_methods(&self) -> &[SignerMethod] { + &[SignerMethod::Digest] + } + + async fn sign_ecdsa( + &self, + path: &DerivationPath, + sighash: [u8; 32], + ) -> Result<(ecdsa::Signature, PublicKey), Self::Error> { + let secp = Secp256k1::new(); + let key = self + .wallet + .derive_private_key(path) + .map_err(|e| e.to_string())?; + let message = Message::from_digest(sighash); + Ok(( + secp.sign_ecdsa(&message, &key), + PublicKey::from_secret_key(&secp, &key), + )) + } + + async fn public_key(&self, path: &DerivationPath) -> Result { + let secp = Secp256k1::new(); + let key = self + .wallet + .derive_private_key(path) + .map_err(|e| e.to_string())?; + Ok(PublicKey::from_secret_key(&secp, &key)) + } + } + + /// Builds a testnet `CoreWallet` whose `account_type`/index-0 account holds a + /// single spendable UTXO (10_000_000 duffs) — the whole balance rides on that + /// one input, so a leaked reservation strands it. Returns the wallet, a soft + /// signer over its seed, and a 1_000_000-duff payment to a dummy recipient. + async fn funded_core_wallet( + account_type: StandardAccountType, + broadcaster: Arc, + ) -> (CoreWallet, WalletSigner, Vec<(DashAddress, u64)>) { + use key_wallet::transaction_checking::{TransactionContext, WalletTransactionChecker}; + + let mut ctx = TestWalletContext::new_random(); + + let receive_address = match account_type { + StandardAccountType::BIP44Account => { + let xpub = ctx + .wallet + .accounts + .standard_bip44_accounts + .get(&0) + .expect("bip44 account") + .account_xpub; + ctx.managed_wallet + .first_bip44_managed_account_mut() + .expect("bip44 managed account") + .next_receive_address(Some(&xpub), true) + .expect("bip44 receive address") + } + StandardAccountType::BIP32Account => { + let xpub = ctx + .wallet + .accounts + .standard_bip32_accounts + .get(&0) + .expect("bip32 account") + .account_xpub; + ctx.managed_wallet + .first_bip32_managed_account_mut() + .expect("bip32 managed account") + .next_receive_address(Some(&xpub), true) + .expect("bip32 receive address") + } + }; + + let funding_tx = Transaction::dummy(&receive_address, 0..1, &[10_000_000]); + let result = ctx + .managed_wallet + .check_core_transaction( + &funding_tx, + TransactionContext::Mempool, + &mut ctx.wallet, + true, + true, + ) + .await; + assert!( + result.is_relevant, + "funding tx should be relevant to {account_type:?}" + ); + assert!(result.is_new_transaction); + + let signer = WalletSigner { + wallet: ctx.wallet.clone(), + }; + + let balance = Arc::new(WalletBalance::new()); + let info = PlatformWalletInfo { + core_wallet: ctx.managed_wallet, + balance: Arc::clone(&balance), + identity_manager: IdentityManager::new(), + tracked_asset_locks: BTreeMap::new(), + }; + + let mut wm = WalletManager::::new(Network::Testnet); + let wallet_id = wm.insert_wallet(ctx.wallet, info).expect("insert wallet"); + let wallet_manager = Arc::new(RwLock::new(wm)); + + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let core = CoreWallet::new(sdk, wallet_manager, wallet_id, broadcaster, balance); + + let recipient = DashAddress::dummy(Network::Testnet, 42); + let outputs = vec![(recipient, 1_000_000u64)]; + + (core, signer, outputs) + } + + /// A pre-send broadcast failure must release the UTXO reservation taken while + /// building the transaction, so an immediate retry can reselect those inputs + /// instead of failing with spurious insufficient funds until the TTL backstop. + /// Covers both funds-account arms of the release path. + #[tokio::test] + async fn send_to_addresses_releases_reservation_on_broadcast_failure() { + for account_type in [ + StandardAccountType::BIP44Account, + StandardAccountType::BIP32Account, + ] { + let broadcaster = Arc::new(FailFirstBroadcaster::new()); + let (core, signer, outputs) = funded_core_wallet(account_type, broadcaster).await; + + // First attempt: build + sign succeed, broadcast fails. + let first = core + .send_to_addresses(account_type, 0, outputs.clone(), &signer) + .await; + assert!( + matches!(first, Err(PlatformWalletError::TransactionBroadcast(_))), + "first send should surface the broadcast failure for {account_type:?}, got {first:?}" + ); + + // Immediate retry: only succeeds if the failed broadcast released the + // reservation. With the leak, coin selection sees no spendable UTXO and + // this fails with a build error instead. + let second = core + .send_to_addresses(account_type, 0, outputs, &signer) + .await; + assert!( + second.is_ok(), + "retry after a failed broadcast should succeed once the reservation \ + is released for {account_type:?}, got {second:?}" + ); + } + } + + /// An *ambiguous* broadcast failure — the network may already have accepted + /// the transaction — must NOT release the reservation: retrying would risk a + /// double-spend. The reservation is kept, so an immediate retry fails at the + /// build stage (no spendable UTXO) rather than reaching broadcast again. + #[tokio::test] + async fn send_to_addresses_keeps_reservation_on_ambiguous_broadcast_failure() { + for account_type in [ + StandardAccountType::BIP44Account, + StandardAccountType::BIP32Account, + ] { + let broadcaster = Arc::new(AlwaysUnconfirmedBroadcaster); + let (core, signer, outputs) = funded_core_wallet(account_type, broadcaster).await; + + let first = core + .send_to_addresses(account_type, 0, outputs.clone(), &signer) + .await; + assert!( + matches!(first, Err(PlatformWalletError::ShieldedSpendUnconfirmed { .. })), + "first send should surface the ambiguous failure for {account_type:?}, got {first:?}" + ); + + // Reservation kept: the retry cannot reselect the reserved input and + // fails while building, never reaching the broadcaster again. + let second = core + .send_to_addresses(account_type, 0, outputs, &signer) + .await; + assert!( + matches!(second, Err(PlatformWalletError::TransactionBuild(_))), + "retry after an ambiguous failure must fail at build with the reservation \ + kept for {account_type:?}, got {second:?}" + ); + } + } +}