diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 6e11514bea..53ff14b303 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -71,6 +71,23 @@ pub enum PlatformWalletError { #[error("Asset lock transaction failed: {0}")] AssetLockTransaction(String), + /// Asset-lock coin selection could not raise the requested amount from + /// the wallet's spendable Core funds. Carries the exact `available` and + /// `required` duff amounts the coin selector reported so callers (and the + /// UI) can render a precise shortfall instead of a stringly-typed + /// "Insufficient funds" message. + /// + /// After dashpay/platform#4073 the `available` figure reflects the UNION + /// of every spendable funds account (BIP44 + BIP32 + CoinJoin + DashPay) + /// for shielded funding, not just the primary BIP44 slice — so a genuine + /// shortfall here means the whole wallet is short, not that funds are + /// stranded on an unreachable derivation account. + #[error( + "asset lock coin selection is short: available {available} duffs, \ + required {required} duffs" + )] + AssetLockInsufficientFunds { available: u64, required: u64 }, + #[error("Transaction broadcast failed: {0}")] TransactionBroadcast(String), diff --git a/packages/rs-platform-wallet/src/test_support.rs b/packages/rs-platform-wallet/src/test_support.rs index a4ecddbd2f..5f9c9fd261 100644 --- a/packages/rs-platform-wallet/src/test_support.rs +++ b/packages/rs-platform-wallet/src/test_support.rs @@ -12,13 +12,13 @@ use async_trait::async_trait; use dashcore::hashes::Hash; use dashcore::secp256k1::{ecdsa, Message, PublicKey, Secp256k1}; use dashcore::BlockHash; -use dashcore::{Network, Transaction, Txid}; +use dashcore::{Network, OutPoint, Transaction, TxOut, Txid}; use key_wallet::account::account_type::StandardAccountType; use key_wallet::bip32::ExtendedPubKey; use key_wallet::signer::{ExtendedPubKeySigner, Signer, SignerMethod}; use key_wallet::test_utils::TestWalletContext; use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; -use key_wallet::{DerivationPath, Wallet}; +use key_wallet::{DerivationPath, Utxo, Wallet}; use key_wallet_manager::WalletManager; use tokio::sync::RwLock; @@ -259,3 +259,400 @@ pub async fn funded_spv_core_wallet( signer, ) } + +/// Builds a testnet wallet manager whose balance is SPLIT across two +/// derivation accounts: BIP44 standard account 0 holds a single spendable +/// UTXO of `bip44_duffs`, and the DIP-9 CoinJoin account 0 holds a single +/// spendable UTXO of `coinjoin_duffs`. Mirrors the live S22/testnet wallet in +/// dashpay/platform#4073 whose ~1.44 DASH of previously-mixed coins sit on the +/// CoinJoin path while only ~0.09 DASH rides on BIP44 — the asset-lock coin +/// selector must span BOTH to shield the union. +/// +/// Returns the manager, the wallet id, and a soft signer over the wallet's +/// seed (which can derive keys for BOTH accounts, so per-account signing of a +/// mixed-input asset lock can be exercised end-to-end). +pub(crate) async fn split_funded_wallet_manager( + bip44_duffs: u64, + coinjoin_duffs: u64, +) -> ( + Arc>>, + WalletId, + WalletSigner, +) { + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + + let mut ctx = TestWalletContext::new_random(); + + // Fund BIP44 account 0 (the primary) at its pre-derived receive address. + let bip44_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[bip44_duffs]); + let bip44_result = ctx + .check_transaction( + &bip44_tx, + TransactionContext::InChainLockedBlock(BlockInfo::new( + 1, + BlockHash::all_zeros(), + 1_700_000_000, + )), + ) + .await; + assert!( + bip44_result.is_relevant && bip44_result.is_new_transaction, + "BIP44 funding tx should be recognized" + ); + + // Derive a fresh CoinJoin receive address (registering it in the CoinJoin + // pool so the checker recognizes the funding), then fund CoinJoin account 0. + let coinjoin_xpub = ctx + .wallet + .get_coinjoin_account(0) + .expect("default wallet has CoinJoin account 0") + .account_xpub; + // CoinJoin is a single-pool (non-standard) account, so it derives via + // `next_address` rather than `next_receive_address`. + let coinjoin_address = ctx + .managed_wallet + .first_coinjoin_managed_account_mut() + .expect("default wallet has a managed CoinJoin account 0") + .next_address(Some(&coinjoin_xpub), true) + .expect("CoinJoin receive address"); + let coinjoin_tx = Transaction::dummy(&coinjoin_address, 0..1, &[coinjoin_duffs]); + let coinjoin_result = ctx + .check_transaction( + &coinjoin_tx, + TransactionContext::InChainLockedBlock(BlockInfo::new( + 2, + BlockHash::all_zeros(), + 1_700_000_100, + )), + ) + .await; + assert!( + coinjoin_result.is_relevant && coinjoin_result.is_new_transaction, + "CoinJoin funding tx should be recognized" + ); + + let signer = WalletSigner { + wallet: ctx.wallet.clone(), + }; + + let balance = Arc::new(WalletBalance::new()); + let info = PlatformWalletInfo { + core_wallet: ctx.managed_wallet, + 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"); + + (Arc::new(RwLock::new(wm)), wallet_id, signer) +} + +/// Which DashPay funds account arm a [`split_funded_wallet_manager_dashpay`] +/// fixture provisions. The two arms differ only in which collection they land +/// in and the DIP-15 derivation order (user/friend vs friend/user), but both +/// are fund-bearing and both are covered by the vendored asset-lock router fix +/// (`get_relevant_account_types(AssetLock)` lists `DashpayReceivingFunds` AND +/// `DashpayExternalAccount` alongside `CoinJoin`). +#[derive(Clone, Copy, Debug)] +pub(crate) enum DashpayLeg { + /// Incoming DashPay funds account (`user_id/friend_id`). + ReceivingFunds, + /// DashPay external (watch-only-style) account (`friend_id/user_id`). + ExternalAccount, +} + +/// Builds a testnet wallet manager whose balance is SPLIT across BIP44 account +/// 0 (`bip44_duffs`) and a DashPay funds account (`dashpay_duffs`) — the +/// DashPay analogue of [`split_funded_wallet_manager`]'s BIP44 + CoinJoin split. +/// `leg` selects which DashPay account type carries the mixed slice. +/// +/// This exercises the DashPay legs of the vendored asset-lock router fix that +/// the CoinJoin fixture does not reach: `get_relevant_account_types(AssetLock)` +/// covers `CoinJoin`, `DashpayReceivingFunds`, AND `DashpayExternalAccount`, so +/// an asset lock funded from a DashPay UTXO must have that input debited by the +/// `check_core_transaction` scan (dashpay/platform#4073, dashpay/dash-wallet#1507). +/// +/// `WalletAccountCreationOptions::Default` does not create DashPay accounts, so +/// this provisions one — identity ids are arbitrary-but-distinct test vectors — +/// on BOTH the signing `Wallet` and the `ManagedWalletInfo`, derives a fresh +/// receive address from its single pool (registering it so the checker +/// recognizes the funding), and funds it, mirroring how +/// [`split_funded_wallet_manager`] funds the CoinJoin account. +/// +/// Signability of the DashPay input matches production per arm (see the inline +/// note in the body): the `ReceivingFunds` account is derived from our own +/// seed and is signable end-to-end; the `ExternalAccount` account is watch-only +/// (its xpub is a contact's, from a foreign seed), so the local signer CANNOT +/// sign its UTXOs — the asset-lock builder must exclude them. +/// An account-level xpub whose private keys the wallet under test does NOT +/// hold — derived from a SEPARATE random wallet. Models a DashPay contact's +/// decrypted xpub, from which production builds the watch-only +/// `DashpayExternalAccount` (`is_watch_only: true`, +/// `wallet/identity/network/contacts.rs`). Any well-formed testnet account +/// xpub serves as a single-pool account key; using a FOREIGN one makes the +/// account unsignable by the local seed exactly as it is in production, so an +/// asset-lock builder that (wrongly) selected its UTXOs would sign them with +/// the local mnemonic's key and produce an invalid input signature. +fn foreign_contact_account_xpub() -> ExtendedPubKey { + let foreign = TestWalletContext::new_random(); + foreign + .wallet + .accounts + .standard_bip44_accounts + .get(&0) + .expect("foreign wallet has BIP44 account 0") + .account_xpub +} + +pub(crate) async fn split_funded_wallet_manager_dashpay( + bip44_duffs: u64, + dashpay_duffs: u64, + leg: DashpayLeg, +) -> ( + Arc>>, + WalletId, + WalletSigner, +) { + use key_wallet::account::account_collection::DashpayAccountKey; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::wallet::managed_wallet_info::managed_account_operations::ManagedAccountOperations; + use key_wallet::AccountType; + + let mut ctx = TestWalletContext::new_random(); + + // Fund BIP44 account 0 (the primary) at its pre-derived receive address. + let bip44_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[bip44_duffs]); + let bip44_result = ctx + .check_transaction( + &bip44_tx, + TransactionContext::InChainLockedBlock(BlockInfo::new( + 1, + BlockHash::all_zeros(), + 1_700_000_000, + )), + ) + .await; + assert!( + bip44_result.is_relevant && bip44_result.is_new_transaction, + "BIP44 funding tx should be recognized" + ); + + // Provision a DashPay funds account of the requested arm on the wallet and + // mirror it into the managed side. The identity ids are arbitrary distinct + // test vectors; distinct ids keep the receiving (user/friend) and external + // (friend/user) derivations on different keys/paths. + let user_identity_id = [0x11u8; 32]; + let friend_identity_id = [0x22u8; 32]; + let account_type = match leg { + DashpayLeg::ReceivingFunds => AccountType::DashpayReceivingFunds { + index: 0, + user_identity_id, + friend_identity_id, + }, + DashpayLeg::ExternalAccount => AccountType::DashpayExternalAccount { + index: 0, + user_identity_id, + friend_identity_id, + }, + }; + // Provision the DashPay funds account of the requested arm, matching how + // production derives each so the local signer's capability is faithful: + // + // * `ReceivingFunds` is OURS. Production derives it from our own + // friendship xpub (`register_contact_account`, `is_watch_only: false`), + // so the local seed CAN sign it. `add_account(_, None)` models that by + // deriving the account from this wallet's own root xpriv. + // + // * `ExternalAccount` is the CONTACT's, WATCH-ONLY. Production builds it + // from the contact's decrypted xpub (`register_dashpay_external_account`, + // `is_watch_only: true`), whose private keys live under a DIFFERENT seed + // the wallet does not hold. Model that faithfully: derive the account + // xpub from a SEPARATE random wallet and insert it via + // `add_account(_, Some(xpub))`, which stores the account + // `is_watch_only: true`. The old shortcut — `add_account(_, None)` for + // BOTH arms — derived the external account from our OWN seed, making it + // locally signable and MASKING the union-funding bug (an asset lock + // would silently spend the contact's coins with a wrong-key, invalid + // signature). This arm now proves the builder excludes it. + match leg { + DashpayLeg::ReceivingFunds => { + ctx.wallet + .add_account(account_type, None) + .expect("add DashPay receiving account to wallet"); + } + DashpayLeg::ExternalAccount => { + let foreign_xpub = foreign_contact_account_xpub(); + ctx.wallet + .add_account(account_type, Some(foreign_xpub)) + .expect("add watch-only DashPay external account to wallet"); + } + } + ctx.managed_wallet + .add_managed_account(&ctx.wallet, account_type) + .expect("mirror DashPay account into managed wallet"); + + // Derive a fresh DashPay receive address from the single-pool managed + // account, then fund it. DashPay accounts are single-pool (like CoinJoin), + // so they derive via `next_address` rather than `next_receive_address`. + let key = DashpayAccountKey { + index: 0, + user_identity_id, + friend_identity_id, + }; + let dashpay_xpub = match leg { + DashpayLeg::ReceivingFunds => ctx.wallet.accounts.dashpay_receival_accounts.get(&key), + DashpayLeg::ExternalAccount => ctx.wallet.accounts.dashpay_external_accounts.get(&key), + } + .expect("DashPay account present in wallet") + .account_xpub; + let dashpay_address = { + let managed = match leg { + DashpayLeg::ReceivingFunds => ctx + .managed_wallet + .accounts + .dashpay_receival_accounts + .get_mut(&key), + DashpayLeg::ExternalAccount => ctx + .managed_wallet + .accounts + .dashpay_external_accounts + .get_mut(&key), + } + .expect("managed DashPay account present"); + managed + .next_address(Some(&dashpay_xpub), true) + .expect("DashPay receive address") + }; + let dashpay_tx = Transaction::dummy(&dashpay_address, 0..1, &[dashpay_duffs]); + let dashpay_result = ctx + .check_transaction( + &dashpay_tx, + TransactionContext::InChainLockedBlock(BlockInfo::new( + 2, + BlockHash::all_zeros(), + 1_700_000_100, + )), + ) + .await; + assert!( + dashpay_result.is_relevant && dashpay_result.is_new_transaction, + "DashPay funding tx should be recognized" + ); + + let signer = WalletSigner { + wallet: ctx.wallet.clone(), + }; + + let balance = Arc::new(WalletBalance::new()); + let info = PlatformWalletInfo { + core_wallet: ctx.managed_wallet, + 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"); + + (Arc::new(RwLock::new(wm)), wallet_id, signer) +} + +/// Like [`split_funded_wallet_manager`] but seeds CoinJoin account 0 with +/// `coinjoin_values.len()` separate spendable UTXOs, each at its own derived +/// CoinJoin address (so the cross-account signer resolver can find a +/// derivation path for every one). Models the many-small-denomination shape a +/// real DIP-9 CoinJoin account carries (0.001 / 0.01 / 0.1 DASH mixing +/// outputs) — the shape that made the asset-lock coin selector blow up +/// on-device when it defaulted to the exponential BranchAndBound subset-sum. +pub(crate) async fn split_funded_wallet_manager_many_coinjoin( + bip44_duffs: u64, + coinjoin_values: &[u64], +) -> ( + Arc>>, + WalletId, + WalletSigner, +) { + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + + let mut ctx = TestWalletContext::new_random(); + + // Fund BIP44 account 0 (the primary) at its pre-derived receive address. + let bip44_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[bip44_duffs]); + let bip44_result = ctx + .check_transaction( + &bip44_tx, + TransactionContext::InChainLockedBlock(BlockInfo::new( + 1, + BlockHash::all_zeros(), + 1_700_000_000, + )), + ) + .await; + assert!( + bip44_result.is_relevant && bip44_result.is_new_transaction, + "BIP44 funding tx should be recognized" + ); + + // Seed CoinJoin account 0 with one UTXO per requested value, each at a + // freshly-derived (and thus pool-registered) CoinJoin address so the + // cross-account resolver can derive its signing key. + let coinjoin_xpub = ctx + .wallet + .get_coinjoin_account(0) + .expect("default wallet has CoinJoin account 0") + .account_xpub; + for (i, &value) in coinjoin_values.iter().enumerate() { + let address = ctx + .managed_wallet + .first_coinjoin_managed_account_mut() + .expect("default wallet has a managed CoinJoin account 0") + .next_address(Some(&coinjoin_xpub), true) + .expect("CoinJoin address"); + let outpoint = OutPoint { + txid: Txid::from_byte_array([(i as u8).wrapping_add(1); 32]), + vout: 0, + }; + let utxo = Utxo { + outpoint, + txout: TxOut { + value, + script_pubkey: address.script_pubkey(), + }, + address, + height: 1, + is_coinbase: false, + is_confirmed: true, + is_instantlocked: false, + is_locked: false, + is_trusted: false, + }; + ctx.managed_wallet + .accounts + .coinjoin_accounts + .get_mut(&0) + .expect("managed CoinJoin account 0") + .utxos + .insert(outpoint, utxo); + } + ctx.managed_wallet.update_last_processed_height(1000); + + let signer = WalletSigner { + wallet: ctx.wallet.clone(), + }; + + let balance = Arc::new(WalletBalance::new()); + let info = PlatformWalletInfo { + core_wallet: ctx.managed_wallet, + 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"); + + (Arc::new(RwLock::new(wm)), wallet_id, signer) +} diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs index 3fbe004aa1..10b1b1446c 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs @@ -6,6 +6,8 @@ use crate::broadcaster::TransactionBroadcaster; use std::time::Duration; +use dashcore::blockdata::transaction::special_transaction::asset_lock::AssetLockPayload; +use dashcore::blockdata::transaction::special_transaction::TransactionPayload; use dashcore::Address as DashAddress; use dashcore::{OutPoint, Transaction, TxOut}; use key_wallet::account::AccountType; @@ -15,9 +17,17 @@ use key_wallet::signer::ExtendedPubKeySigner; use key_wallet::wallet::managed_wallet_info::asset_lock_builder::{ AssetLockFundingType, CreditOutputFunding, }; +use key_wallet::wallet::managed_wallet_info::coin_selection::{SelectionError, SelectionStrategy}; +use key_wallet::wallet::managed_wallet_info::fee::FeeRate; use key_wallet::wallet::managed_wallet_info::managed_account_operations::ManagedAccountOperations; +use key_wallet::wallet::managed_wallet_info::transaction_builder::{ + BuilderError, TransactionBuilder, +}; +use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use key_wallet::wallet::Wallet; +use key_wallet::ManagedAccountType; +use key_wallet::Utxo; use crate::changeset::{AccountRegistrationEntry, PlatformWalletChangeSet}; use crate::error::PlatformWalletError; @@ -106,7 +116,32 @@ impl AssetLockManager { identity_index, }; - // 3. Delegate to the key-wallet signer-driven builder. + // 3. Fund the asset lock. + // + // Shielded funding (`AssetLockShieldedAddressTopUp`) must be able to + // draw on previously-mixed CoinJoin coins, which live on the DIP-9 + // CoinJoin derivation account — the pinned key-wallet + // `build_asset_lock_with_signer` funds from a SINGLE BIP44 account + // only, so those coins counted in the balance but could not be + // shielded (dashpay/platform#4073). Route shielded funding through the + // union-of-accounts builder; every other funding type keeps the + // single-BIP44-account path (spending mixed CoinJoin coins into an + // identity registration would de-anonymize them — a deliberate + // privacy choice left out of scope here). + if funding_type == AssetLockFundingType::AssetLockShieldedAddressTopUp { + return self + .build_asset_lock_tx_from_all_funding_accounts( + wallet, + info, + account_index, + vec![funding], + DEFAULT_FEE_PER_KB, + signer, + ) + .await; + } + + // Delegate to the key-wallet signer-driven builder (single BIP44 account). let result = info .core_wallet .build_asset_lock_with_signer( @@ -150,6 +185,257 @@ impl AssetLockManager { Ok((result.transaction, path)) } + /// Build + sign an asset-lock transaction whose funding inputs are drawn + /// from the UNION of every spendable Core funds account (BIP44 + BIP32 + + /// CoinJoin + DashPay), not just the single BIP44 account at + /// `account_index`. + /// + /// ## Why this exists (dashpay/platform#4073) + /// + /// The pinned key-wallet `ManagedWalletInfo::build_asset_lock_with_signer` + /// funds an asset lock from exactly ONE BIP44 standard account: it calls + /// `TransactionBuilder::set_funding` on + /// `standard_bip44_accounts[account_index]` and signs with a + /// single-account path resolver (`funds_acc.address_derivation_path`). + /// Previously-mixed CoinJoin coins live on the DIP-9 CoinJoin derivation + /// account (`coinjoin_accounts`), so they are counted in the wallet + /// balance (which sums `all_funding_accounts`) yet were invisible to the + /// asset-lock coin selector — shielding failed with a coin-selection + /// "Insufficient funds" even though the wallet-wide balance covered the + /// amount. + /// + /// This method keeps `account_index` as the PRIMARY account (its + /// reservation ledger gates concurrent primary-account builds, and change + /// flows back to it via `set_funding`'s change address) but ADDS the + /// spendable UTXOs of every other funds account as explicit builder + /// inputs (`add_inputs`), and signs with a resolver that spans all funds + /// accounts. The credit-output key is still derived from the + /// shielded-topup account exactly as the single-account builder does + /// (peek path → signer pubkey → mark used), so the returned + /// `DerivationPath` lines up with the credit-output script the caller + /// already peeked. + /// + /// ## Interim caveat (superseded by the upstream fix) + /// + /// The clean long-term fix belongs upstream in key-wallet + /// (`build_asset_lock_with_signer` gathering inputs + reservations across + /// accounts). Until that pin bump lands, this workspace composition makes + /// CoinJoin funds shieldable today. `TransactionBuilder::set_funding` + /// captures only the PRIMARY account's `ReservationSet` (the type is + /// `pub(crate)` in key-wallet, so this crate cannot reserve per-account), + /// so inputs selected from non-primary accounts are recorded in the + /// primary account's reservation ledger rather than their own. They are + /// therefore not protected against a concurrent build on their own + /// account for the brief window before the broadcast tx is processed back + /// into the wallet (which releases the outpoints from every account's + /// ledger). Shielded funding is single-flighted under `shield_guard` and + /// the whole build runs under the wallet write lock, and the app issues no + /// concurrent non-shielded spend on the CoinJoin/BIP32 accounts, so the + /// race window is not reachable in practice. + async fn build_asset_lock_tx_from_all_funding_accounts( + &self, + wallet: &Wallet, + info: &mut PlatformWalletInfo, + account_index: u32, + credit_output_fundings: Vec, + fee_per_kb: u64, + signer: &S, + ) -> Result<(Transaction, DerivationPath), PlatformWalletError> { + use std::collections::{HashMap, HashSet}; + + let target_duffs: u64 = credit_output_fundings.iter().map(|f| f.output.value).sum(); + let height = info.core_wallet.last_processed_height(); + tracing::debug!( + target_duffs, + height, + primary_account_index = account_index, + funding_accounts = info.core_wallet.accounts.all_funding_accounts().len(), + "multi-account asset-lock funding: enumerating spendable funds accounts" + ); + + // Snapshot the primary account's spendable outpoints so the union + // sweep below does not add them twice: `set_funding` already seeds + // them, and `add_inputs` must contribute only the OTHER accounts. + let primary_outpoints: HashSet = info + .core_wallet + .accounts + .standard_bip44_accounts + .get(&account_index) + .map(|a| { + a.spendable_utxos(height) + .into_iter() + .map(|u| u.outpoint) + .collect() + }) + .unwrap_or_default(); + + // Build, from an immutable borrow of every funds account: + // (a) an owned `Address -> DerivationPath` resolver covering every + // spendable input across ALL accounts, so signing can resolve a + // key for an input selected from any account; and + // (b) the explicit extra inputs (all non-primary accounts). + let mut path_map: HashMap = HashMap::new(); + let mut extra_inputs: Vec = Vec::new(); + let mut union_value: u64 = 0; + let mut union_count: usize = 0; + for acc in info.core_wallet.accounts.all_funding_accounts() { + // Never fund an asset lock from coins the local wallet holds no + // signing key for. `all_funding_accounts()` (in the pinned + // key-wallet fork) includes `dashpay_external_accounts`, and + // `spendable_utxos()` filters on maturity but not ownership. The + // managed layer stores every account's PUBLIC key only (its + // `KeySource` is always `Public`), so a managed-level watch-only + // flag cannot discriminate here — the ownership signal is the + // account-type variant. `DashpayExternalAccount` is the sole + // watch-only funds account type: production builds it from a + // CONTACT's decrypted xpub with `is_watch_only: true` (see + // `wallet/identity/network/contacts.rs`), so its UTXOs are the + // contact's coins. `MnemonicResolverCoreSigner` would blindly + // derive `acc.address_derivation_path(&utxo.address)` from the + // LOCAL mnemonic, yielding a different key and therefore an invalid + // input signature. Skip such accounts entirely — both from the + // path resolver and from the explicit extra inputs. + if matches!( + acc.managed_account_type(), + ManagedAccountType::DashpayExternalAccount { .. } + ) { + continue; + } + for utxo in acc.spendable_utxos(height) { + union_value = union_value.saturating_add(utxo.value()); + union_count += 1; + if let Some(path) = acc.address_derivation_path(&utxo.address) { + path_map.insert(utxo.address.clone(), path); + } + if !primary_outpoints.contains(&utxo.outpoint) { + extra_inputs.push(utxo.clone()); + } + } + } + tracing::debug!( + union_count, + union_value, + primary_count = primary_outpoints.len(), + extra_count = extra_inputs.len(), + resolver_entries = path_map.len(), + "multi-account asset-lock funding: union UTXO set assembled" + ); + + let acc = wallet + .get_bip44_account(account_index) + .ok_or_else(|| { + PlatformWalletError::AssetLockTransaction(format!( + "BIP44 account {account_index} not found for asset-lock funding" + )) + })? + .clone(); + let credit_outputs: Vec = credit_output_fundings + .iter() + .map(|f| f.output.clone()) + .collect(); + + // Seed the primary account (inputs + change address + reservations), + // then append the union of the other accounts' spendable inputs. The + // `&mut` borrow of the primary account is scoped to this block; the + // returned builder owns cloned inputs / reservations / change address, + // so no account borrow is held across the signer await below. + let builder = { + let primary_funds = info + .core_wallet + .accounts + .standard_bip44_accounts + .get_mut(&account_index) + .ok_or_else(|| { + PlatformWalletError::AssetLockTransaction(format!( + "managed BIP44 account {account_index} not found for asset-lock funding" + )) + })?; + TransactionBuilder::new() + .set_fee_rate(FeeRate::new(fee_per_kb)) + .set_current_height(height) + // LargestFirst, NOT the `TransactionBuilder::new()` default + // `BranchAndBound`. This is load-bearing, not an optimization: + // BranchAndBound routes to a recursive exact-match subset-sum + // (`CoinSelector::find_exact_match`) whose search space is + // EXPONENTIAL in the number of sub-target UTXOs. The + // single-BIP44-account path tolerates it (a handful of UTXOs), + // but a CoinJoin account holds many small mixed denominations + // (0.001 / 0.01 / 0.1 DASH ...); feeding that whole set to + // BranchAndBound hangs the FFI call for minutes with no logs and + // no broadcast (observed on-device, dashpay/platform#4073 + // follow-up). LargestFirst uses the linear greedy accumulator + // (`accumulate_coins_with_size`), which also minimizes the input + // count — fewer signer round-trips (each input is one resolver + // upcall) and a smaller tx/fee. + .set_selection_strategy(SelectionStrategy::LargestFirst) + .set_special_payload(TransactionPayload::AssetLockPayloadType( + AssetLockPayload::new(credit_outputs), + )) + .set_funding(primary_funds, &acc) + .add_inputs(extra_inputs) + .require_final_inputs() + }; + + tracing::debug!( + target_duffs, + "multi-account asset-lock funding: selecting + signing (LargestFirst)" + ); + let (transaction, fee) = builder + .build_signed(signer, move |addr| path_map.get(&addr).cloned()) + .await + .map_err(|e| map_builder_error(e, target_duffs))?; + tracing::debug!( + selected_inputs = transaction.input.len(), + fee, + txid = %transaction.txid(), + "multi-account asset-lock funding: transaction built + signed" + ); + + // Derive the single credit-output key from the shielded-topup account, + // mirroring the pinned single-account builder's phase-1/2/3 sequence + // (peek without marking → signer round-trip → commit the index) so a + // signer failure never irreversibly consumes a pool index. + let (path, index) = { + let credit_account = info + .core_wallet + .accounts + .asset_lock_shielded_address_topup + .as_mut() + .ok_or_else(|| { + PlatformWalletError::AssetLockTransaction( + "Asset lock shielded address top-up account not found".to_string(), + ) + })?; + credit_account + .peek_next_path() + .map_err(|e| PlatformWalletError::AssetLockTransaction(e.to_string()))? + }; + signer.public_key(&path).await.map_err(|e| { + PlatformWalletError::AssetLockTransaction(format!("signer public_key failed: {e}")) + })?; + { + let credit_account = info + .core_wallet + .accounts + .asset_lock_shielded_address_topup + .as_mut() + .ok_or_else(|| { + PlatformWalletError::AssetLockTransaction( + "Asset lock shielded address top-up account not found".to_string(), + ) + })?; + credit_account + .mark_first_pool_index_used(index) + .map_err(|e| PlatformWalletError::AssetLockTransaction(e.to_string()))?; + } + + tracing::debug!( + selected_inputs = transaction.input.len(), + "multi-account asset-lock funding: credit-output key derived; returning built tx" + ); + Ok((transaction, path)) + } + /// Peek at the next unused address from a funding account without /// consuming it (i.e. without marking it as used). /// @@ -788,6 +1074,45 @@ impl AssetLockManager { } } +/// Map a key-wallet [`BuilderError`] to a [`PlatformWalletError`], promoting +/// every shortfall shape to the typed +/// [`PlatformWalletError::AssetLockInsufficientFunds`] so callers get one +/// structured shortfall contract (dashpay/platform#4073's typed-error ask) +/// instead of a string they must pattern-match: +/// - `BuilderError::InsufficientFunds` / `SelectionError::InsufficientFunds` +/// carry their own exact `available`/`required` duff amounts — preserved. +/// - `SelectionError::NoUtxosAvailable` — the zero-spendable-candidate case, +/// the MOST extreme shortfall — carries no amounts, so it previously fell +/// through to the generic string form while partial shortfalls stayed typed +/// (dashpay/platform#4074 prior-no-utxos-846). It now maps to `available: 0` +/// with the caller's `requested` target as `required`, keeping the empty +/// candidate set on the same structured path. +/// Every other builder error keeps the generic `AssetLockTransaction` string. +fn map_builder_error(e: BuilderError, requested: u64) -> PlatformWalletError { + match e { + BuilderError::InsufficientFunds { + available, + required, + } + | BuilderError::CoinSelection(SelectionError::InsufficientFunds { + available, + required, + }) => PlatformWalletError::AssetLockInsufficientFunds { + available, + required, + }, + BuilderError::CoinSelection(SelectionError::NoUtxosAvailable) => { + PlatformWalletError::AssetLockInsufficientFunds { + available: 0, + required: requested, + } + } + other => { + PlatformWalletError::AssetLockTransaction(format!("Asset lock builder failed: {other}")) + } + } +} + #[cfg(test)] mod tests { use std::sync::{Arc, Mutex}; @@ -807,7 +1132,7 @@ mod tests { }; use crate::test_support::{ funded_wallet_manager, AlwaysMaybeSentBroadcaster, AlwaysOkBroadcaster, - AlwaysRejectedBroadcaster, WalletSigner, + AlwaysRejectedBroadcaster, DashpayLeg, WalletSigner, }; use crate::wallet::asset_lock::manager::AssetLockManager; use crate::wallet::asset_lock::tracked::AssetLockStatus; @@ -816,6 +1141,52 @@ mod tests { use crate::wallet::platform_wallet::WalletId; use crate::{AssetLockFundingType, PlatformWalletError}; + /// prior-no-utxos-846 (dashpay/platform#4074): the zero-spendable-candidate + /// selection error must surface the SAME typed shortfall as a partial + /// shortfall (not the generic string), so hosts stay on one structured path; + /// and a partial shortfall must still carry its own exact amounts. + #[test] + fn no_utxos_available_maps_to_typed_insufficient_funds() { + use super::{map_builder_error, BuilderError, SelectionError}; + + // Zero spendable candidates -> typed, available: 0, required = requested. + match map_builder_error( + BuilderError::CoinSelection(SelectionError::NoUtxosAvailable), + 12_345, + ) { + PlatformWalletError::AssetLockInsufficientFunds { + available, + required, + } => { + assert_eq!(available, 0, "empty candidate set means nothing available"); + assert_eq!( + required, 12_345, + "requested target threaded through as required" + ); + } + other => panic!("expected typed AssetLockInsufficientFunds, got {other:?}"), + } + + // A partial shortfall keeps its own exact amounts; the requested arg is + // NOT substituted for the builder's carried values. + match map_builder_error( + BuilderError::CoinSelection(SelectionError::InsufficientFunds { + available: 100, + required: 500, + }), + 999, + ) { + PlatformWalletError::AssetLockInsufficientFunds { + available, + required, + } => { + assert_eq!(available, 100); + assert_eq!(required, 500, "carried amounts win over the requested arg"); + } + other => panic!("expected typed AssetLockInsufficientFunds, got {other:?}"), + } + } + /// Persistence stub that records every stored changeset so tests can /// assert what the asset-lock flow queued. `fail_flush` simulates a /// backend whose durability boundary fails; `flushes` counts `flush` @@ -1567,4 +1938,996 @@ mod tests { } } } + + // -- Multi-account asset-lock funding (dashpay/platform#4073) -- + + /// Wraps the split BIP44 + CoinJoin fixture in an `AssetLockManager`. + /// `build_asset_lock_transaction` never broadcasts, so the broadcaster is + /// irrelevant here. + async fn split_asset_lock_manager( + bip44_duffs: u64, + coinjoin_duffs: u64, + ) -> ( + Arc>, + WalletSigner, + ) { + let (wallet_manager, wallet_id, signer) = + crate::test_support::split_funded_wallet_manager(bip44_duffs, coinjoin_duffs).await; + let persistence = Arc::new(CapturingPersistence::default()); + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let manager = Arc::new(AssetLockManager::new( + sdk, + wallet_manager, + wallet_id, + Arc::new(Notify::new()), + Arc::new(AlwaysRejectedBroadcaster), + WalletPersister::new( + wallet_id, + Arc::clone(&persistence) as Arc, + ), + )); + (manager, signer) + } + + /// Wraps the many-CoinJoin-UTXO fixture in an `AssetLockManager`. + async fn split_asset_lock_manager_many_coinjoin( + bip44_duffs: u64, + coinjoin_values: &[u64], + ) -> ( + Arc>, + WalletSigner, + ) { + let (wallet_manager, wallet_id, signer) = + crate::test_support::split_funded_wallet_manager_many_coinjoin( + bip44_duffs, + coinjoin_values, + ) + .await; + let persistence = Arc::new(CapturingPersistence::default()); + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let manager = Arc::new(AssetLockManager::new( + sdk, + wallet_manager, + wallet_id, + Arc::new(Notify::new()), + Arc::new(AlwaysRejectedBroadcaster), + WalletPersister::new( + wallet_id, + Arc::clone(&persistence) as Arc, + ), + )); + (manager, signer) + } + + /// The `(BIP44 account 0, CoinJoin account 0)` UTXO outpoint sets, so a + /// test can prove a built transaction drew inputs from both accounts. + async fn account_outpoints( + manager: &AssetLockManager, + ) -> ( + std::collections::HashSet, + std::collections::HashSet, + ) { + let wm = manager.wallet_manager.read().await; + let (_, info) = wm + .get_wallet_and_info(&manager.wallet_id) + .expect("wallet present"); + let bip44 = info + .core_wallet + .accounts + .standard_bip44_accounts + .get(&0) + .map(|a| a.utxos.keys().copied().collect()) + .unwrap_or_default(); + let coinjoin = info + .core_wallet + .accounts + .coinjoin_accounts + .get(&0) + .map(|a| a.utxos.keys().copied().collect()) + .unwrap_or_default(); + (bip44, coinjoin) + } + + /// The bug: shielded asset-lock funding must be able to spend + /// previously-mixed CoinJoin coins, not just the BIP44 slice. Split the + /// balance so NEITHER account alone can fund the lock (0.09 DASH each) and + /// require 0.15 DASH — coin selection must reach across both the BIP44 and + /// the DIP-9 CoinJoin account, and the mixed inputs must each be signed + /// under their own account's derivation path. + #[tokio::test] + async fn shielded_asset_lock_funds_from_bip44_and_coinjoin_union() { + // 0.09 DASH on BIP44, 0.09 DASH on CoinJoin; require 0.15 DASH. + let (manager, signer) = split_asset_lock_manager(9_000_000, 9_000_000).await; + let (bip44_outpoints, coinjoin_outpoints) = account_outpoints(&manager).await; + + let (tx, _path) = manager + .build_asset_lock_transaction( + 15_000_000, + 0, + AssetLockFundingType::AssetLockShieldedAddressTopUp, + 0, + &signer, + ) + .await + .expect("shielded asset lock must fund from the BIP44 + CoinJoin union"); + + // Neither account alone covers 0.15 DASH, so both must be selected. + let spent: std::collections::HashSet = + tx.input.iter().map(|i| i.previous_output).collect(); + assert!( + spent.iter().any(|o| bip44_outpoints.contains(o)), + "expected at least one BIP44 input, tx spent {spent:?}" + ); + assert!( + spent.iter().any(|o| coinjoin_outpoints.contains(o)), + "expected at least one CoinJoin input (the #4073 fix), tx spent {spent:?}" + ); + + // Per-account signing: every selected input, regardless of which + // account's derivation path it needed, must carry a signature. + assert!(!tx.input.is_empty(), "asset lock must have selected inputs"); + for (i, txin) in tx.input.iter().enumerate() { + assert!( + !txin.script_sig.is_empty(), + "input {i} ({}) has an empty script_sig — the cross-account \ + resolver failed to derive/sign its key", + txin.previous_output + ); + } + } + + /// The widening is deliberately scoped to shielded funding: spending mixed + /// CoinJoin coins into an identity registration would de-anonymize them. + /// With the balance split 0.09/0.09, an identity-registration lock for + /// 0.15 DASH must still fail (BIP44 alone is short) while the shielded lock + /// for the same amount succeeds from the union. + #[tokio::test] + async fn non_shielded_asset_lock_stays_single_bip44_account() { + let (manager, signer) = split_asset_lock_manager(9_000_000, 9_000_000).await; + + let identity = manager + .build_asset_lock_transaction( + 15_000_000, + 0, + AssetLockFundingType::IdentityRegistration, + 0, + &signer, + ) + .await; + assert!( + identity.is_err(), + "identity registration must NOT reach CoinJoin coins — BIP44 alone \ + is short of 0.15 DASH, got {identity:?}" + ); + + let shielded = manager + .build_asset_lock_transaction( + 15_000_000, + 0, + AssetLockFundingType::AssetLockShieldedAddressTopUp, + 0, + &signer, + ) + .await; + assert!( + shielded.is_ok(), + "shielded funding must reach the union, got {shielded:?}" + ); + } + + /// A shielded lock exceeding even the UNION balance surfaces the typed + /// [`PlatformWalletError::AssetLockInsufficientFunds`], and its `available` + /// reflects the whole spendable balance (both accounts), not the BIP44 + /// slice — the pre-#4073 symptom was `available` reporting only the BIP44 + /// portion. + #[tokio::test] + async fn shielded_asset_lock_union_shortfall_is_typed() { + // Union spendable is 0.18 DASH; ask for 1.0 DASH. + let (manager, signer) = split_asset_lock_manager(9_000_000, 9_000_000).await; + + let result = manager + .build_asset_lock_transaction( + 100_000_000, + 0, + AssetLockFundingType::AssetLockShieldedAddressTopUp, + 0, + &signer, + ) + .await; + + match result { + Err(PlatformWalletError::AssetLockInsufficientFunds { + available, + required, + }) => { + // `available` must reflect the union (both 0.09 UTXOs), i.e. + // strictly more than the BIP44-only slice the old path saw. + assert!( + available > 9_000_000, + "available ({available}) should reflect the BIP44 + CoinJoin \ + union (> the 9_000_000 BIP44 slice)" + ); + assert!( + available <= 18_000_000, + "available ({available}) cannot exceed the 18_000_000 union" + ); + assert!( + required >= 100_000_000, + "required ({required}) should be at least the requested amount" + ); + } + other => panic!( + "expected typed AssetLockInsufficientFunds carrying the union \ + available/required, got {other:?}" + ), + } + } + + /// On-device regression: a real CoinJoin account holds many small mixed + /// denominations. The first version of the multi-account builder inherited + /// `TransactionBuilder`'s default `BranchAndBound`, whose recursive + /// exact-match subset-sum (`CoinSelector::find_exact_match`) is EXPONENTIAL + /// in the count of sub-target UTXOs — feeding it a large CoinJoin set hung + /// the whole FFI call for minutes with no logs and no broadcast. The builder + /// now pins `LargestFirst` (linear greedy). + /// + /// The blowup is SYNCHRONOUS CPU work with no `.await` points, so it cannot + /// be interrupted by `tokio::time::timeout` (that is exactly why on-device + /// it hangs RUNNABLE-in-native and the enclosing coroutine never yields). + /// The build therefore runs on a **detached OS thread**, and the test body + /// waits on a channel with a wall-clock deadline: a regression to an + /// exponential strategy makes `recv_timeout` fire and the test FAIL (rather + /// than hang the whole suite). The detached thread is reclaimed at process + /// exit; on the happy path (LargestFirst) it finishes in well under a + /// millisecond and the channel delivers immediately. + #[test] + fn shielded_asset_lock_over_many_coinjoin_utxos_does_not_hang() { + use std::sync::mpsc; + use std::time::Duration; + + // 40 x 0.02 DASH CoinJoin UTXOs (0.8 DASH), 0.09 DASH on BIP44; shield + // 0.2 DASH. BranchAndBound would explore ~sum_k C(40, k<=10) subsets — + // empirically minutes+; LargestFirst returns instantly. + let coinjoin: Vec = vec![2_000_000; 40]; + + // Fixture build is async; drive it on a throwaway current-thread runtime. + let setup_rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("setup runtime"); + let (manager, signer) = + setup_rt.block_on(split_asset_lock_manager_many_coinjoin(9_000_000, &coinjoin)); + + let (result_tx, result_rx) = mpsc::channel(); + // Detached: NOT joined anywhere, so a hung build can't wedge runtime + // teardown; libtest reclaims it at process exit. + std::thread::spawn(move || { + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("worker runtime"); + let outcome = rt + .block_on(manager.build_asset_lock_transaction( + 20_000_000, + 0, + AssetLockFundingType::AssetLockShieldedAddressTopUp, + 0, + &signer, + )) + .map(|(tx, _path)| tx); + let _ = result_tx.send(outcome); + }); + + // LargestFirst completes in ~25ms; a 30s deadline is a ~1000x margin + // against CI contention while still bounding a regression to an + // exponential strategy (which never returns) to a prompt failure. + match result_rx.recv_timeout(Duration::from_secs(30)) { + Ok(Ok(tx)) => { + // LargestFirst minimizes the input count; every selected input + // must be signed under its own account's derivation path. + assert!(!tx.input.is_empty(), "must select inputs"); + for txin in &tx.input { + assert!( + !txin.script_sig.is_empty(), + "input {} is unsigned", + txin.previous_output + ); + } + } + Ok(Err(e)) => panic!("funding must succeed from the CoinJoin union, got {e:?}"), + Err(_) => panic!( + "multi-account asset-lock funding did not return within 30s — \ + regression to an exponential coin-selection strategy over the \ + CoinJoin UTXO set (dashpay/platform#4073 on-device hang)" + ), + } + } + + /// Aggregate wallet balance across all funds accounts (recomputes from the + /// current UTXO maps). + async fn aggregate_total(manager: &AssetLockManager) -> u64 { + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + let mut wm = manager.wallet_manager.write().await; + let info = wm + .get_wallet_info_mut(&manager.wallet_id) + .expect("wallet present"); + info.core_wallet.update_balance(); + WalletInfoInterface::balance(&info.core_wallet).total() + } + + /// `true` iff CoinJoin account 0 still holds `outpoint` as an unspent UTXO. + async fn coinjoin_has_utxo( + manager: &AssetLockManager, + outpoint: &OutPoint, + ) -> bool { + let wm = manager.wallet_manager.read().await; + let (_, info) = wm + .get_wallet_and_info(&manager.wallet_id) + .expect("wallet present"); + info.core_wallet + .accounts + .coinjoin_accounts + .get(&0) + .is_some_and(|a| a.utxos.contains_key(outpoint)) + } + + /// Build a CoinJoin-funded shield over the split fixture and return the + /// manager, the pre-spend aggregate, the spent-input total, the wallet + /// change total, and the built tx. 0.09 DASH BIP44 + one 2.0 DASH CoinJoin + /// UTXO, shield 0.2 DASH — LargestFirst funds it entirely from the CoinJoin + /// UTXO, so the tx spends only a CoinJoin input and the change lands on BIP44. + async fn build_coinjoin_shield() -> ( + Arc>, + u64, + u64, + u64, + Transaction, + ) { + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + + let (manager, signer) = split_asset_lock_manager(9_000_000, 200_000_000).await; + + let (before_total, utxo_values) = { + let mut wm = manager.wallet_manager.write().await; + let info = wm + .get_wallet_info_mut(&manager.wallet_id) + .expect("wallet present"); + info.core_wallet.update_balance(); + let before = WalletInfoInterface::balance(&info.core_wallet).total(); + let mut values = std::collections::HashMap::new(); + for acc in info.core_wallet.accounts.all_funding_accounts() { + for (op, utxo) in &acc.utxos { + values.insert(*op, utxo.txout.value); + } + } + (before, values) + }; + + let (tx, _path) = manager + .build_asset_lock_transaction( + 20_000_000, + 0, + AssetLockFundingType::AssetLockShieldedAddressTopUp, + 0, + &signer, + ) + .await + .expect("build multi-account asset lock"); + + let sum_spent: u64 = tx + .input + .iter() + .map(|i| utxo_values.get(&i.previous_output).copied().unwrap_or(0)) + .sum(); + // Wallet-owned outputs = the change (the AssetLock burn is an OP_RETURN). + let sum_change: u64 = tx + .output + .iter() + .filter(|o| !o.script_pubkey.is_op_return()) + .map(|o| o.value) + .sum(); + assert!(sum_spent > 0, "tx must spend a wallet (CoinJoin) UTXO"); + assert!(sum_change > 0, "tx must return change to the wallet"); + + (manager, before_total, sum_spent, sum_change, tx) + } + + /// THE CASE THE DEVICE IS STUCK ON: after a hard reset the wallet is rebuilt + /// from scratch and the asset-lock tx is re-seen by a block/rescan. The + /// `check_core_transaction` scan — with NO broadcast-time mitigation in + /// play (a rescan never runs the broadcast path) — must debit the spent + /// CoinJoin input, so the balance settles to `previous − inputs + change` + /// rather than re-inflating by the spent amount and re-triggering the + /// reset→rescan→inflate loop. This passes ONLY because the vendored + /// rust-dashcore carries the router fix (CoinJoin in the AssetLock relevant + /// types); on the un-patched pin the CoinJoin input stays counted. + #[tokio::test] + async fn router_fix_debits_coinjoin_asset_lock_spend_on_rescan() { + use key_wallet::transaction_checking::{TransactionContext, WalletTransactionChecker}; + + let (manager, before_total, sum_spent, sum_change, tx) = build_coinjoin_shield().await; + let spent_outpoint = tx.input[0].previous_output; + assert!( + coinjoin_has_utxo(&manager, &spent_outpoint).await, + "the CoinJoin UTXO must be present before the scan (rescan re-added it)" + ); + + // Confirmation/rescan processing ONLY — no broadcast-time mitigation. + { + let mut wm = manager.wallet_manager.write().await; + let (wallet, info) = wm + .get_wallet_mut_and_info_mut(&manager.wallet_id) + .expect("wallet present"); + info.core_wallet + .check_core_transaction( + &tx, + TransactionContext::InChainLockedBlock( + key_wallet::transaction_checking::BlockInfo::new( + 10, + dashcore::BlockHash::from_raw_hash(dashcore::hashes::Hash::all_zeros()), + 1_700_001_000, + ), + ), + wallet, + true, + true, + ) + .await; + } + + assert!( + !coinjoin_has_utxo(&manager, &spent_outpoint).await, + "router fix must mark the spent CoinJoin UTXO spent on the scan" + ); + assert_eq!( + aggregate_total(&manager).await, + before_total - sum_spent + sum_change, + "post-rescan balance must be previous − inputs + change (no re-inflation)" + ); + } + + /// The persistence half of dashpay/dash-wallet#1507: proving the router- + /// fixed scan not only debits the CoinJoin input in memory but produces a + /// [`TransactionRecord`] whose `input_details` reference the spent CoinJoin + /// outpoint — the exact data `core_bridge::derive_spent_utxos` walks to tell + /// the persister to DELETE that UTXO row. This is what makes the debit + /// survive restart. The removed broadcast-time mitigation defeated this: + /// by `.remove()`ing the UTXO in-memory first, it made the later scan's + /// `ManagedCoreFundsAccount::check_transaction_for_match` find no UTXO, emit + /// no CoinJoin record, and thus leave `input_details` empty — so nothing was + /// ever persisted and the stale row reloaded on restart. With the mitigation + /// gone, the scan owns the debit and the record carries the deletion through. + #[tokio::test] + async fn router_fix_records_spent_coinjoin_input_for_persistence() { + use key_wallet::transaction_checking::{TransactionContext, WalletTransactionChecker}; + + let (manager, _before_total, _sum_spent, _sum_change, tx) = build_coinjoin_shield().await; + let spent_outpoint = tx.input[0].previous_output; + assert!( + coinjoin_has_utxo(&manager, &spent_outpoint).await, + "the CoinJoin UTXO must be present before the scan" + ); + + // Normal-pipeline scan (no mitigation), capturing the emitted records. + let result = { + let mut wm = manager.wallet_manager.write().await; + let (wallet, info) = wm + .get_wallet_mut_and_info_mut(&manager.wallet_id) + .expect("wallet present"); + info.core_wallet + .check_core_transaction(&tx, TransactionContext::Mempool, wallet, true, true) + .await + }; + + // A record must exist whose `input_details` resolve — through the same + // `record.transaction.input[detail.index].previous_output` lookup + // `derive_spent_utxos` uses — to the spent CoinJoin outpoint. Without + // that entry the persister is never told to delete the row (the #1507 + // gap). It must belong to the CoinJoin account, the account the pinned + // router omitted and the mitigation used to suppress. + let persisted_spend = result + .new_records + .iter() + .chain(result.updated_records.iter()) + .filter(|r| matches!(r.account_type, key_wallet::AccountType::CoinJoin { .. })) + .flat_map(|r| { + r.input_details.iter().filter_map(move |d| { + r.transaction + .input + .get(d.index as usize) + .map(|i| i.previous_output) + }) + }) + .any(|op| op == spent_outpoint); + + assert!( + persisted_spend, + "the router-fixed scan must emit a CoinJoin TransactionRecord whose \ + input_details cover the spent outpoint {spent_outpoint}, so \ + derive_spent_utxos persists the deletion (dashpay/dash-wallet#1507)" + ); + + // And the in-memory debit itself must have happened. + assert!( + !coinjoin_has_utxo(&manager, &spent_outpoint).await, + "the scan must mark the spent CoinJoin UTXO spent" + ); + } + + // -- DashPay-leg asset-lock persistence (dashpay/dash-wallet#1507) -- + // + // The vendored router fix and the removed broadcast-time mitigation's own + // doc comment (see the `no broadcast-time balance mitigation` note above) + // cover THREE previously-omitted fund-bearing account types for asset-lock + // spend detection: `CoinJoin`, `DashpayReceivingFunds`, and + // `DashpayExternalAccount`. The `build_coinjoin_shield` tests above exercise + // only the CoinJoin leg; the following mirror them for BOTH DashPay legs, so + // a latent gap in either DashPay arm of `get_relevant_account_types(AssetLock)` + // (e.g. a `DashpayReceivingFunds`-vs-`DashpayExternalAccount` routing edge + // case) cannot silently recur the exact persistence regression this PR closes. + + /// Wraps the split BIP44 + DashPay fixture (`leg` selects which DashPay + /// account type carries the mixed slice) in an `AssetLockManager`. + async fn split_asset_lock_manager_dashpay( + bip44_duffs: u64, + dashpay_duffs: u64, + leg: DashpayLeg, + ) -> ( + Arc>, + WalletSigner, + ) { + let (wallet_manager, wallet_id, signer) = + crate::test_support::split_funded_wallet_manager_dashpay( + bip44_duffs, + dashpay_duffs, + leg, + ) + .await; + let persistence = Arc::new(CapturingPersistence::default()); + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let manager = Arc::new(AssetLockManager::new( + sdk, + wallet_manager, + wallet_id, + Arc::new(Notify::new()), + Arc::new(AlwaysRejectedBroadcaster), + WalletPersister::new( + wallet_id, + Arc::clone(&persistence) as Arc, + ), + )); + (manager, signer) + } + + /// The set of outpoints held by the DashPay funds account of the given + /// `leg` on account key index 0. + async fn dashpay_account_outpoints( + manager: &AssetLockManager, + leg: DashpayLeg, + ) -> std::collections::HashSet { + let wm = manager.wallet_manager.read().await; + let (_, info) = wm + .get_wallet_and_info(&manager.wallet_id) + .expect("wallet present"); + let map = match leg { + DashpayLeg::ReceivingFunds => &info.core_wallet.accounts.dashpay_receival_accounts, + DashpayLeg::ExternalAccount => &info.core_wallet.accounts.dashpay_external_accounts, + }; + map.values().flat_map(|a| a.utxos.keys().copied()).collect() + } + + /// `true` iff the DashPay funds account of the given `leg` still holds + /// `outpoint` as an unspent UTXO. + async fn dashpay_has_utxo( + manager: &AssetLockManager, + leg: DashpayLeg, + outpoint: &OutPoint, + ) -> bool { + let wm = manager.wallet_manager.read().await; + let (_, info) = wm + .get_wallet_and_info(&manager.wallet_id) + .expect("wallet present"); + let map = match leg { + DashpayLeg::ReceivingFunds => &info.core_wallet.accounts.dashpay_receival_accounts, + DashpayLeg::ExternalAccount => &info.core_wallet.accounts.dashpay_external_accounts, + }; + map.values().any(|a| a.utxos.contains_key(outpoint)) + } + + /// `true` iff `account_type` is the DashPay funds variant matching `leg`. + fn record_matches_dashpay_leg(account_type: &key_wallet::AccountType, leg: DashpayLeg) -> bool { + match leg { + DashpayLeg::ReceivingFunds => matches!( + account_type, + key_wallet::AccountType::DashpayReceivingFunds { .. } + ), + DashpayLeg::ExternalAccount => matches!( + account_type, + key_wallet::AccountType::DashpayExternalAccount { .. } + ), + } + } + + /// DashPay analogue of [`build_coinjoin_shield`]: fund a shield entirely + /// from a single 2.0-DASH DashPay UTXO (the DashPay account of `leg`) over a + /// BIP44 slice too small to cover it, so the tx spends exactly the DashPay + /// input and change lands on BIP44. Returns the manager, the pre-spend + /// aggregate, the spent-input total, the change total, the built tx, and the + /// spent DashPay outpoint (resolved against the DashPay account's own UTXO + /// set rather than assumed positionally). + async fn build_dashpay_shield( + leg: DashpayLeg, + ) -> ( + Arc>, + u64, + u64, + u64, + Transaction, + OutPoint, + ) { + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + + let (manager, signer) = split_asset_lock_manager_dashpay(9_000_000, 200_000_000, leg).await; + let dashpay_outpoints = dashpay_account_outpoints(&manager, leg).await; + + let (before_total, utxo_values) = { + let mut wm = manager.wallet_manager.write().await; + let info = wm + .get_wallet_info_mut(&manager.wallet_id) + .expect("wallet present"); + info.core_wallet.update_balance(); + let before = WalletInfoInterface::balance(&info.core_wallet).total(); + let mut values = std::collections::HashMap::new(); + for acc in info.core_wallet.accounts.all_funding_accounts() { + for (op, utxo) in &acc.utxos { + values.insert(*op, utxo.txout.value); + } + } + (before, values) + }; + + let (tx, _path) = manager + .build_asset_lock_transaction( + 20_000_000, + 0, + AssetLockFundingType::AssetLockShieldedAddressTopUp, + 0, + &signer, + ) + .await + .expect("build DashPay-funded asset lock"); + + let sum_spent: u64 = tx + .input + .iter() + .map(|i| utxo_values.get(&i.previous_output).copied().unwrap_or(0)) + .sum(); + let sum_change: u64 = tx + .output + .iter() + .filter(|o| !o.script_pubkey.is_op_return()) + .map(|o| o.value) + .sum(); + assert!(sum_spent > 0, "tx must spend a wallet (DashPay) UTXO"); + assert!(sum_change > 0, "tx must return change to the wallet"); + + // The spent DashPay outpoint, resolved against the DashPay account's own + // UTXO set (not `input[0]`), so a future change in selection ordering + // can't quietly make this assert about the wrong input. + let dashpay_spent = tx + .input + .iter() + .map(|i| i.previous_output) + .find(|op| dashpay_outpoints.contains(op)) + .expect("shield must spend a DashPay UTXO (the multi-account #4073 fix)"); + + ( + manager, + before_total, + sum_spent, + sum_change, + tx, + dashpay_spent, + ) + } + + /// Rescan-debit body shared by both DashPay legs: mirrors + /// [`router_fix_debits_coinjoin_asset_lock_spend_on_rescan`] but over a + /// DashPay-funded shield. A confirmation/rescan `check_core_transaction` + /// (no broadcast-time mitigation) must debit the spent DashPay input so the + /// balance settles to `previous − inputs + change` rather than re-inflating. + async fn assert_router_fix_debits_dashpay_on_rescan(leg: DashpayLeg) { + use key_wallet::transaction_checking::{TransactionContext, WalletTransactionChecker}; + + let (manager, before_total, sum_spent, sum_change, tx, spent_outpoint) = + build_dashpay_shield(leg).await; + assert!( + dashpay_has_utxo(&manager, leg, &spent_outpoint).await, + "the DashPay UTXO must be present before the scan (rescan re-added it)" + ); + + { + let mut wm = manager.wallet_manager.write().await; + let (wallet, info) = wm + .get_wallet_mut_and_info_mut(&manager.wallet_id) + .expect("wallet present"); + info.core_wallet + .check_core_transaction( + &tx, + TransactionContext::InChainLockedBlock( + key_wallet::transaction_checking::BlockInfo::new( + 10, + dashcore::BlockHash::from_raw_hash(dashcore::hashes::Hash::all_zeros()), + 1_700_001_000, + ), + ), + wallet, + true, + true, + ) + .await; + } + + assert!( + !dashpay_has_utxo(&manager, leg, &spent_outpoint).await, + "router fix must mark the spent DashPay UTXO spent on the scan ({leg:?})" + ); + assert_eq!( + aggregate_total(&manager).await, + before_total - sum_spent + sum_change, + "post-rescan balance must be previous − inputs + change (no re-inflation, {leg:?})" + ); + } + + /// Persistence-record body shared by both DashPay legs: mirrors + /// [`router_fix_records_spent_coinjoin_input_for_persistence`]. The scan must + /// emit a `TransactionRecord` belonging to the DashPay account whose + /// `input_details` resolve — through the same + /// `record.transaction.input[detail.index].previous_output` lookup + /// `derive_spent_utxos` uses — to the spent DashPay outpoint, so the + /// persister is told to delete that UTXO row and the debit survives restart. + async fn assert_router_fix_records_spent_dashpay_input(leg: DashpayLeg) { + use key_wallet::transaction_checking::{TransactionContext, WalletTransactionChecker}; + + let (manager, _before_total, _sum_spent, _sum_change, tx, spent_outpoint) = + build_dashpay_shield(leg).await; + assert!( + dashpay_has_utxo(&manager, leg, &spent_outpoint).await, + "the DashPay UTXO must be present before the scan" + ); + + let result = { + let mut wm = manager.wallet_manager.write().await; + let (wallet, info) = wm + .get_wallet_mut_and_info_mut(&manager.wallet_id) + .expect("wallet present"); + info.core_wallet + .check_core_transaction(&tx, TransactionContext::Mempool, wallet, true, true) + .await + }; + + let persisted_spend = result + .new_records + .iter() + .chain(result.updated_records.iter()) + .filter(|r| record_matches_dashpay_leg(&r.account_type, leg)) + .flat_map(|r| { + r.input_details.iter().filter_map(move |d| { + r.transaction + .input + .get(d.index as usize) + .map(|i| i.previous_output) + }) + }) + .any(|op| op == spent_outpoint); + + assert!( + persisted_spend, + "the router-fixed scan must emit a {leg:?} TransactionRecord whose \ + input_details cover the spent outpoint {spent_outpoint}, so \ + derive_spent_utxos persists the deletion (dashpay/dash-wallet#1507)" + ); + + assert!( + !dashpay_has_utxo(&manager, leg, &spent_outpoint).await, + "the scan must mark the spent DashPay UTXO spent ({leg:?})" + ); + } + + /// DashpayReceivingFunds analogue of + /// `router_fix_debits_coinjoin_asset_lock_spend_on_rescan`. + #[tokio::test] + async fn router_fix_debits_dashpay_receiving_asset_lock_spend_on_rescan() { + assert_router_fix_debits_dashpay_on_rescan(DashpayLeg::ReceivingFunds).await; + } + + /// DashpayReceivingFunds analogue of + /// `router_fix_records_spent_coinjoin_input_for_persistence`. + #[tokio::test] + async fn router_fix_records_spent_dashpay_receiving_input_for_persistence() { + assert_router_fix_records_spent_dashpay_input(DashpayLeg::ReceivingFunds).await; + } + + // -- Watch-only DashpayExternalAccount exclusion (finding 5b52d9844055) -- + // + // A `DashpayExternalAccount` is created in production from a CONTACT's + // decrypted xpub with `is_watch_only: true` (wallet/identity/network/ + // contacts.rs): its UTXOs are the contact's coins, and the local mnemonic + // holds no key for them. `all_funding_accounts()` in the pinned key-wallet + // fork nonetheless includes it, so the multi-account asset-lock builder must + // filter it out — otherwise it would select those UTXOs, sign them with the + // wrong local key, and produce an invalid input signature. + // + // Unlike the DashPay RECEIVING-funds leg (which IS ours and signable, and is + // correctly INCLUDED — see the receiving tests above), these two tests assert + // the EXCLUSION. The fixture builds the external account watch-only from a + // FOREIGN seed's xpub, mirroring production; the earlier `add_account(_, None)` + // fixture derived it from the test wallet's own seed, making it locally + // signable and MASKING this defect. + + /// The union-of-accounts asset-lock builder must NOT select watch-only + /// `DashpayExternalAccount` UTXOs. The external account here holds the + /// LARGEST UTXO (0.5 DASH) while signable BIP44 (0.15 DASH) alone covers the + /// 0.1-DASH shield, so a naive `LargestFirst` over the union would grab the + /// external UTXO FIRST (the pre-fix bug). The build must instead succeed from + /// BIP44 alone and leave every external UTXO unspent. + #[tokio::test] + async fn asset_lock_funding_excludes_watch_only_dashpay_external_utxos() { + let (manager, signer) = + split_asset_lock_manager_dashpay(15_000_000, 50_000_000, DashpayLeg::ExternalAccount) + .await; + let external_outpoints = + dashpay_account_outpoints(&manager, DashpayLeg::ExternalAccount).await; + assert!( + !external_outpoints.is_empty(), + "fixture must seed at least one watch-only external UTXO" + ); + + let (tx, _path) = manager + .build_asset_lock_transaction( + 10_000_000, + 0, + AssetLockFundingType::AssetLockShieldedAddressTopUp, + 0, + &signer, + ) + .await + .expect("asset lock must build from signable BIP44 funds alone"); + + for op in &external_outpoints { + assert!( + !tx.input.iter().any(|i| i.previous_output == *op), + "watch-only external UTXO {op} must be excluded from asset-lock funding \ + (no invalid-signature input reachable)" + ); + assert!( + dashpay_has_utxo(&manager, DashpayLeg::ExternalAccount, op).await, + "excluded external UTXO {op} must remain unspent" + ); + } + } + + /// Watch-only external coins must not be *borrowed* to cover a shortfall. + /// Signable BIP44 (0.05 DASH) alone is too small for the 0.1-DASH shield; + /// the only way to cover it would be to (wrongly) spend the 0.5-DASH + /// watch-only external UTXO. The build must FAIL with the typed + /// insufficient-funds error rather than emit an invalid-signature input. + #[tokio::test] + async fn asset_lock_funding_cannot_borrow_watch_only_dashpay_external_utxos() { + let (manager, signer) = + split_asset_lock_manager_dashpay(5_000_000, 50_000_000, DashpayLeg::ExternalAccount) + .await; + + let result = manager + .build_asset_lock_transaction( + 10_000_000, + 0, + AssetLockFundingType::AssetLockShieldedAddressTopUp, + 0, + &signer, + ) + .await; + + assert!( + matches!( + result, + Err(PlatformWalletError::AssetLockInsufficientFunds { .. }) + ), + "must not fund an asset lock from watch-only external coins; got {result:?}" + ); + } + + /// Heavy-mixer CoinJoin discovery (dashpay/dash-wallet#1507): the CoinJoin + /// account's default gap limit must watch a discovery window wide enough to + /// bridge the address gaps a heavy mixer leaves — matched to dashj's + /// ~100-key lookahead. Index 50 is beyond the OLD 30-address window; a fresh + /// account must pre-generate it (so the BIP158 filter watches it) and + /// recognize a tx paying it. On the old gap of 30 the address was never + /// watched, so txs at far CoinJoin indices were skipped entirely — the + /// starvation that survived a clean re-creation + full rescan, missing both + /// the txs that created far-index UTXOs and the txs that spent nearer ones. + #[tokio::test] + async fn coinjoin_gap_limit_discovers_addresses_beyond_the_old_window() { + use key_wallet::managed_account::address_pool::AddressPoolType; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::transaction_checking::{ + BlockInfo, TransactionContext, WalletTransactionChecker, + }; + + // Fresh wallet (BIP44 funded, CoinJoin account provisioned but empty). + let (wallet_manager, wallet_id, _signer) = + crate::test_support::split_funded_wallet_manager_many_coinjoin(9_000_000, &[]).await; + + // `far_index` sits past the old 30-address gap but within dashj's window. + let far_index: u32 = 50; + let far_address = { + let wm = wallet_manager.read().await; + let (_, info) = wm.get_wallet_and_info(&wallet_id).expect("wallet present"); + let cj = info + .core_wallet + .accounts + .coinjoin_accounts + .get(&0) + .expect("coinjoin account 0"); + assert_eq!( + cj.gap_limit(), + Some(100), + "CoinJoin gap limit must match dashj's lookahead (100)" + ); + let external = cj + .managed_account_type() + .address_pools() + .into_iter() + .find(|p| p.pool_type == AddressPoolType::External) + .expect("external CoinJoin pool"); + // The load-bearing assertion: index 50 is pre-generated (and thus + // filter-watched) ONLY because the gap was widened. On the old gap + // of 30 this is `None` and the address below can't be fetched. + external + .address_at_index(far_index) + .expect("index 50 must be pre-generated with the widened CoinJoin gap") + }; + + // A tx paying the far-index CoinJoin address must be discovered and its + // UTXO tracked — proving the filter watched an address the old window + // would have missed. + let tx = Transaction::dummy(&far_address, 0..1, &[12_345_678]); + let result = { + let mut wm = wallet_manager.write().await; + let (wallet, info) = wm + .get_wallet_mut_and_info_mut(&wallet_id) + .expect("wallet present"); + info.core_wallet + .check_core_transaction( + &tx, + TransactionContext::InChainLockedBlock(BlockInfo::new( + 5, + dashcore::BlockHash::from_raw_hash(dashcore::hashes::Hash::all_zeros()), + 1_700_002_000, + )), + wallet, + true, + true, + ) + .await + }; + assert!( + result.is_relevant, + "a payment to the far-index CoinJoin address must be discovered" + ); + assert!(result.is_new_transaction); + + let wm = wallet_manager.read().await; + let (_, info) = wm.get_wallet_and_info(&wallet_id).expect("wallet present"); + let cj = info + .core_wallet + .accounts + .coinjoin_accounts + .get(&0) + .expect("coinjoin account 0"); + assert!( + cj.utxos.values().any(|u| u.txout.value == 12_345_678), + "the far-index CoinJoin UTXO must be tracked after discovery" + ); + } } diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rs index ea22396015..74ad492f03 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rs @@ -121,20 +121,27 @@ pub enum AssetLockFunding { /// - `AssetLockAddressTopUp` — for platform-address funding flows /// - others — see [`AssetLockFundingType`] /// - /// `account_index` selects which BIP44 *standard* account (by - /// BIP44 account index) supplies the UTXOs. Only BIP44 standard - /// accounts are supported today — CoinJoin / BIP32 funding for - /// any asset-lock-funded operation is out of scope and would - /// require additional plumbing in - /// [`AssetLockManager::create_funded_asset_lock_proof`]. + /// `account_index` selects the PRIMARY BIP44 *standard* account (by + /// BIP44 account index) — it supplies the change address and its + /// reservation ledger gates concurrent primary-account builds. + /// + /// Input SELECTION scope depends on the funding type: + /// + /// - `AssetLockShieldedAddressTopUp` — funds from the UNION of every + /// spendable funds account (BIP44 + BIP32 + CoinJoin + DashPay), so + /// previously-mixed CoinJoin coins can be shielded + /// (dashpay/platform#4073). See + /// [`AssetLockManager::build_asset_lock_tx_from_all_funding_accounts`]. + /// - every other funding type — funds from the single BIP44 account at + /// `account_index` only (spending mixed CoinJoin coins into an + /// identity registration would de-anonymize them). This is the pinned + /// key-wallet `build_asset_lock_with_signer` behavior. FromWalletBalance { /// Amount to lock (in duffs). amount_duffs: u64, - /// BIP44 standard-account index to draw the funding UTXOs from. - /// - /// Only BIP44 standard accounts (`AccountType::Standard` with - /// `StandardAccountTypeTag::Bip44`) are supported today; - /// CoinJoin / BIP32 are not. + /// Primary BIP44 standard-account index. Supplies the change + /// address and reservation ledger; for shielded funding the input + /// set is widened to every spendable funds account (see above). account_index: u32, },