diff --git a/key-wallet-ffi/FFI_API.md b/key-wallet-ffi/FFI_API.md index 566d3aa19..a9870ae9f 100644 --- a/key-wallet-ffi/FFI_API.md +++ b/key-wallet-ffi/FFI_API.md @@ -1305,14 +1305,14 @@ This function dereferences a raw pointer to FFIWallet. The caller must ensure th #### `wallet_build_and_sign_asset_lock_transaction` ```c -wallet_build_and_sign_asset_lock_transaction(manager: *const FFIWalletManager, wallet: *const FFIWallet, account_index: u32, funding_types: *const FFIAssetLockFundingType, identity_indices: *const u32, credit_output_scripts: *const *const u8, credit_output_script_lens: *const usize, credit_output_amounts: *const u64, credit_outputs_count: usize, fee_per_kb: u64, fee_out: *mut u64, tx_bytes_out: *mut *mut u8, tx_len_out: *mut usize, private_keys_out: *mut [u8; 32], error: *mut FFIError,) -> bool +wallet_build_and_sign_asset_lock_transaction(manager: *const FFIWalletManager, wallet: *const FFIWallet, funding_sources: *const FFIAccountTypePreference, funding_sources_count: usize, account_index: u32, funding_types: *const FFIAssetLockFundingType, identity_indices: *const u32, credit_output_scripts: *const *const u8, credit_output_script_lens: *const usize, credit_output_amounts: *const u64, credit_outputs_count: usize, fee_per_kb: u64, fee_out: *mut u64, tx_bytes_out: *mut *mut u8, tx_len_out: *mut usize, private_keys_out: *mut [u8; 32], error: *mut FFIError,) -> bool ``` **Description:** -Build and sign an asset lock transaction for Core to Platform transfers. Creates a special transaction (type 8) with `AssetLockPayload` that locks Dash for Platform credits. Derives one unique private key per credit output from the specified funding account types. # Parameters - `funding_types`: Array of `credit_outputs_count` funding account types, one per credit output (registration, top-up, invitation, etc.) - `identity_indices`: Array of `credit_outputs_count` identity indices. Only used for `IdentityTopUp` entries; ignored for other funding types. - `private_keys_out`: Caller-allocated array of `credit_outputs_count` × 32-byte buffers. On success, each `private_keys_out[i]` receives the one-time private key corresponding to `credit_output_scripts[i]`. # Safety - All pointer parameters must be valid and non-null - All parallel arrays must have at least `credit_outputs_count` elements - `private_keys_out` must point to an array of `credit_outputs_count` × `[u8; 32]` buffers - Caller must free `tx_bytes_out` with `transaction_bytes_free` +Build and sign an asset lock transaction for Core to Platform transfers. Creates a special transaction (type 8) with `AssetLockPayload` that locks Dash for Platform credits. Derives one unique private key per credit output from the specified funding account types. Funding is POOLED across the caller's `funding_sources`: coin selection draws from the union of those accounts' UTXOs, so a lock no longer needs the whole amount sitting in one account. Which accounts to pool is the caller's policy — this layer applies no default and never widens the list — so a client that wants only the primary transparent balance passes a single `BIP44` source and gets exactly the pre-pooling behavior. # Parameters - `funding_sources`: Array of `funding_sources_count` accounts to fund from, in priority order. The FIRST source supplies the change address, so pass the account that should receive change first. At least one is required. A single source is strict — it fails if the wallet has no such account — while a list of two or more skips the sources this wallet has nothing for and fails only if none of them funds anything. This entry point builds a non-drain lock, so `CoinJoin` is rejected here even as the sole source: mixed funds can only back a drain. - `funding_sources_count`: Number of entries in `funding_sources`. - `account_index`: Index addressing the standard families (BIP44, BIP32, CoinJoin). DashPay sources span their own indices and ignore it. - `funding_types`: Array of `credit_outputs_count` funding account types, one per credit output (registration, top-up, invitation, etc.) - `identity_indices`: Array of `credit_outputs_count` identity indices. Only used for `IdentityTopUp` entries; ignored for other funding types. - `private_keys_out`: Caller-allocated array of `credit_outputs_count` × 32-byte buffers. On success, each `private_keys_out[i]` receives the one-time private key corresponding to `credit_output_scripts[i]`. # Safety - All pointer parameters must be valid and non-null - `funding_sources` must have at least `funding_sources_count` elements - Every `funding_sources[i].kind` must be a declared [`FFIAccountTypePreferenceKind`] discriminant (`0..=5`). Reading any other value as that enum is undefined behavior, so it cannot be rejected here — pass the generated C enum rather than a cast integer. - All other parallel arrays must have at least `credit_outputs_count` elements - `private_keys_out` must point to an array of `credit_outputs_count` × `[u8; 32]` buffers - Caller must free `tx_bytes_out` with `transaction_bytes_free` **Safety:** -- All pointer parameters must be valid and non-null - All parallel arrays must have at least `credit_outputs_count` elements - `private_keys_out` must point to an array of `credit_outputs_count` × `[u8; 32]` buffers - Caller must free `tx_bytes_out` with `transaction_bytes_free` +- All pointer parameters must be valid and non-null - `funding_sources` must have at least `funding_sources_count` elements - Every `funding_sources[i].kind` must be a declared [`FFIAccountTypePreferenceKind`] discriminant (`0..=5`). Reading any other value as that enum is undefined behavior, so it cannot be rejected here — pass the generated C enum rather than a cast integer. - All other parallel arrays must have at least `credit_outputs_count` elements - `private_keys_out` must point to an array of `credit_outputs_count` × `[u8; 32]` buffers - Caller must free `tx_bytes_out` with `transaction_bytes_free` **Module:** `transaction` diff --git a/key-wallet-ffi/src/transaction.rs b/key-wallet-ffi/src/transaction.rs index eb09a0201..b74a81479 100644 --- a/key-wallet-ffi/src/transaction.rs +++ b/key-wallet-ffi/src/transaction.rs @@ -744,14 +744,101 @@ impl From for AssetLockFundingType { } } +/// Which family of accounts a funding source names. +/// +/// The discriminant of [`FFIAccountTypePreference`]; the two `Dashpay…` kinds +/// are the ones that read the identity IDs alongside it. +#[repr(C)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FFIAccountTypePreferenceKind { + /// The standard transparent account, `m/44'/coinType'/index'`. + BIP44 = 0, + /// The legacy standard account, `m/index'`. + BIP32 = 1, + /// Mixed CoinJoin funds. Asset locks accept these only as the *sole* + /// source, and only in drain mode — pooling mixed outputs with transparent + /// ones in one transaction links them and undoes the mixing. + /// + /// `wallet_build_and_sign_asset_lock_transaction` never drains, so this + /// kind always fails there; it exists for the drain entry points. + CoinJoin = 2, + /// One contact's receiving account, named by both identity IDs. + DashpayFriendshipReceivingFunds = 3, + /// Every receiving account of one identity; reads `user_identity_id` only. + DashpayIdentityReceivingFunds = 4, + /// Every DashPay receiving account this wallet can sign for. Ignores both + /// identity ID fields. + AllDashpayReceivingFunds = 5, +} + +/// A funding source offered to an asset lock's coin selection. +/// +/// `kind` selects the family; the identity IDs are read only by the kinds that +/// name one (see [`FFIAccountTypePreferenceKind`]) and may be left zeroed +/// otherwise. +#[repr(C)] +#[derive(Debug, Clone, Copy)] +pub struct FFIAccountTypePreference { + /// Which family of accounts this source names. + pub kind: FFIAccountTypePreferenceKind, + /// The local identity whose accounts to draw from. Read by + /// `DashpayFriendshipReceivingFunds` and `DashpayIdentityReceivingFunds`. + pub user_identity_id: [u8; 32], + /// The contact on the other side of the friendship. Read by + /// `DashpayFriendshipReceivingFunds` only. + pub friend_identity_id: [u8; 32], +} + +impl From for AccountTypePreference { + fn from(ffi: FFIAccountTypePreference) -> Self { + match ffi.kind { + FFIAccountTypePreferenceKind::BIP44 => Self::BIP44, + FFIAccountTypePreferenceKind::BIP32 => Self::BIP32, + FFIAccountTypePreferenceKind::CoinJoin => Self::CoinJoin, + FFIAccountTypePreferenceKind::DashpayFriendshipReceivingFunds => { + Self::DashpayFriendshipReceivingFunds { + user_identity_id: ffi.user_identity_id, + friend_identity_id: ffi.friend_identity_id, + } + } + FFIAccountTypePreferenceKind::DashpayIdentityReceivingFunds => { + Self::DashpayIdentityReceivingFunds { + user_identity_id: ffi.user_identity_id, + } + } + FFIAccountTypePreferenceKind::AllDashpayReceivingFunds => { + Self::AllDashpayReceivingFunds + } + } + } +} + /// Build and sign an asset lock transaction for Core to Platform transfers. /// /// Creates a special transaction (type 8) with `AssetLockPayload` that locks /// Dash for Platform credits. Derives one unique private key per credit output /// from the specified funding account types. /// +/// Funding is POOLED across the caller's `funding_sources`: coin selection +/// draws from the union of those accounts' UTXOs, so a lock no longer needs the +/// whole amount sitting in one account. Which accounts to pool is the caller's +/// policy — this layer applies no default and never widens the list — so a +/// client that wants only the primary transparent balance passes a single +/// `BIP44` source and gets exactly the pre-pooling behavior. +/// /// # Parameters /// +/// - `funding_sources`: Array of `funding_sources_count` accounts to fund from, +/// in priority order. The FIRST source supplies the change address, so pass +/// the account that should receive change first. At least one is required. +/// A single source is strict — it fails if the wallet has no such account — +/// while a list of two or more skips the sources this wallet has nothing for +/// and fails only if none of them funds anything. This entry point builds a +/// non-drain lock, so `CoinJoin` is rejected here even as the sole source: +/// mixed funds can only back a drain. +/// - `funding_sources_count`: Number of entries in `funding_sources`. +/// - `account_index`: Index addressing the standard families (BIP44, BIP32, +/// CoinJoin). DashPay sources span their own indices and ignore it. /// - `funding_types`: Array of `credit_outputs_count` funding account types, /// one per credit output (registration, top-up, invitation, etc.) /// - `identity_indices`: Array of `credit_outputs_count` identity indices. @@ -763,13 +850,20 @@ impl From for AssetLockFundingType { /// # Safety /// /// - All pointer parameters must be valid and non-null -/// - All parallel arrays must have at least `credit_outputs_count` elements +/// - `funding_sources` must have at least `funding_sources_count` elements +/// - Every `funding_sources[i].kind` must be a declared +/// [`FFIAccountTypePreferenceKind`] discriminant (`0..=5`). Reading any other +/// value as that enum is undefined behavior, so it cannot be rejected here — +/// pass the generated C enum rather than a cast integer. +/// - All other parallel arrays must have at least `credit_outputs_count` elements /// - `private_keys_out` must point to an array of `credit_outputs_count` × `[u8; 32]` buffers /// - Caller must free `tx_bytes_out` with `transaction_bytes_free` #[no_mangle] pub unsafe extern "C" fn wallet_build_and_sign_asset_lock_transaction( manager: *const FFIWalletManager, wallet: *const FFIWallet, + funding_sources: *const FFIAccountTypePreference, + funding_sources_count: usize, account_index: u32, funding_types: *const FFIAssetLockFundingType, identity_indices: *const u32, @@ -786,6 +880,7 @@ pub unsafe extern "C" fn wallet_build_and_sign_asset_lock_transaction( ) -> bool { check_ptr!(manager, error); check_ptr!(wallet, error); + check_ptr!(funding_sources, error); check_ptr!(funding_types, error); check_ptr!(identity_indices, error); check_ptr!(credit_output_scripts, error); @@ -801,6 +896,13 @@ pub unsafe extern "C" fn wallet_build_and_sign_asset_lock_transaction( return false; } + // No implicit default: an empty list would mean `AccountTypePreference::DEFAULT` + // one layer down, which is this layer picking the caller's funding policy. + if funding_sources_count == 0 { + (*error).set(FFIErrorCode::InvalidInput, "At least one funding source required"); + return false; + } + unsafe { let manager_ref = &*manager; let wallet_ref = &*wallet; @@ -811,6 +913,12 @@ pub unsafe extern "C" fn wallet_build_and_sign_asset_lock_transaction( let funding_types_slice = slice::from_raw_parts(funding_types, credit_outputs_count); let identity_indices_slice = slice::from_raw_parts(identity_indices, credit_outputs_count); + let funding_sources: Vec = + slice::from_raw_parts(funding_sources, funding_sources_count) + .iter() + .map(|&source| source.into()) + .collect(); + // Convert FFI arrays to domain types let mut fundings = Vec::with_capacity(credit_outputs_count); for i in 0..credit_outputs_count { @@ -840,9 +948,8 @@ pub unsafe extern "C" fn wallet_build_and_sign_asset_lock_transaction( let result = unwrap_or_return!(managed_wallet.build_asset_lock( wallet_ref.inner(), - key_wallet::wallet::managed_wallet_info::asset_lock_builder::AssetLockFundingAccount::Bip44 { - account_index, - }, + &funding_sources, + account_index, fundings, fee_per_kb, false, @@ -878,3 +985,63 @@ pub unsafe extern "C" fn wallet_build_and_sign_asset_lock_transaction( }) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn preference(kind: FFIAccountTypePreferenceKind) -> FFIAccountTypePreference { + FFIAccountTypePreference { + kind, + user_identity_id: [7u8; 32], + friend_identity_id: [9u8; 32], + } + } + + /// Every kind must map to its own preference. A transposed arm here would + /// silently fund an asset lock from an account the caller did not name — + /// and the build would succeed, so nothing downstream would catch it. + #[test] + fn every_funding_source_kind_maps_to_its_own_preference() { + use FFIAccountTypePreferenceKind as Kind; + + assert_eq!( + AccountTypePreference::from(preference(Kind::BIP44)), + AccountTypePreference::BIP44 + ); + assert_eq!( + AccountTypePreference::from(preference(Kind::BIP32)), + AccountTypePreference::BIP32 + ); + assert_eq!( + AccountTypePreference::from(preference(Kind::CoinJoin)), + AccountTypePreference::CoinJoin + ); + assert_eq!( + AccountTypePreference::from(preference(Kind::AllDashpayReceivingFunds)), + AccountTypePreference::AllDashpayReceivingFunds + ); + } + + /// The identity IDs ride alongside the tag rather than inside it, so the + /// kinds that read them must read the right ones — and a friendship source + /// must not collapse to the whole-identity one. + #[test] + fn dashpay_sources_carry_the_identity_ids_they_name() { + use FFIAccountTypePreferenceKind as Kind; + + assert_eq!( + AccountTypePreference::from(preference(Kind::DashpayFriendshipReceivingFunds)), + AccountTypePreference::DashpayFriendshipReceivingFunds { + user_identity_id: [7u8; 32], + friend_identity_id: [9u8; 32], + } + ); + assert_eq!( + AccountTypePreference::from(preference(Kind::DashpayIdentityReceivingFunds)), + AccountTypePreference::DashpayIdentityReceivingFunds { + user_identity_id: [7u8; 32], + } + ); + } +} diff --git a/key-wallet-ffi/tests/test_asset_lock_funding_sources.rs b/key-wallet-ffi/tests/test_asset_lock_funding_sources.rs new file mode 100644 index 000000000..c4b4ae0f9 --- /dev/null +++ b/key-wallet-ffi/tests/test_asset_lock_funding_sources.rs @@ -0,0 +1,185 @@ +//! Tests for the caller-supplied funding sources on the asset-lock FFI entry +//! point. +//! +//! Which accounts an asset lock may spend from is the client library's policy, +//! so the boundary has to carry the choice rather than apply one. These cover +//! the guards that keep it that way: a caller must name at least one source, a +//! well-formed list reaches the builder untouched, and a source that can never +//! work on this entry point is rejected rather than quietly replaced. + +use dash_network::ffi::FFINetwork; +use key_wallet_ffi::error::{FFIError, FFIErrorCode}; +use key_wallet_ffi::transaction::{ + transaction_bytes_free, wallet_build_and_sign_asset_lock_transaction, FFIAccountTypePreference, + FFIAccountTypePreferenceKind, FFIAssetLockFundingType, +}; +use key_wallet_ffi::wallet::wallet_free_const; +use key_wallet_ffi::wallet_manager::{ + wallet_manager_add_wallet_from_mnemonic_with_options, wallet_manager_create, + wallet_manager_free, wallet_manager_free_wallet_ids, wallet_manager_get_wallet, + wallet_manager_get_wallet_ids, +}; +use std::ffi::{CStr, CString}; +use std::ptr; + +const TEST_MNEMONIC: &str = + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"; + +/// The error's message as a Rust string, or empty when none was set. +fn error_message(error: &FFIError) -> String { + if error.message.is_null() { + String::new() + } else { + unsafe { CStr::from_ptr(error.message) }.to_string_lossy().into_owned() + } +} + +fn source(kind: FFIAccountTypePreferenceKind) -> FFIAccountTypePreference { + FFIAccountTypePreference { + kind, + user_identity_id: [0u8; 32], + friend_identity_id: [0u8; 32], + } +} + +/// The networks every case below runs against. Account derivation is +/// coin-type-scoped, so a guard that only ever saw one network could hide a +/// network-conditional path. +const NETWORKS: [FFINetwork; 2] = [FFINetwork::Mainnet, FFINetwork::Testnet]; + +/// Drives the entry point against a freshly created (and therefore unfunded) +/// wallet, returning the error it produced. +/// +/// The wallet has no UTXOs, so a call that gets past the argument guards fails +/// in coin selection instead — which is exactly what distinguishes "rejected at +/// the boundary" from "accepted and attempted". +unsafe fn call_with_sources(network: FFINetwork, sources: &[FFIAccountTypePreference]) -> FFIError { + let mut error = FFIError::default(); + + let manager = wallet_manager_create(network, &mut error); + assert!(!manager.is_null()); + + let mnemonic = CString::new(TEST_MNEMONIC).unwrap(); + assert!(wallet_manager_add_wallet_from_mnemonic_with_options( + manager, + mnemonic.as_ptr(), + ptr::null(), + &mut error, + )); + + let mut wallet_ids: *mut u8 = ptr::null_mut(); + let mut wallet_count: usize = 0; + assert!(wallet_manager_get_wallet_ids(manager, &mut wallet_ids, &mut wallet_count, &mut error)); + assert_eq!(wallet_count, 1); + + let wallet = wallet_manager_get_wallet(manager, wallet_ids, &mut error); + assert!(!wallet.is_null()); + + // One credit output; the script content does not matter, since no call here + // is expected to reach a successful build. + let script: [u8; 25] = + [0x76, 0xa9, 0x14, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x88, 0xac]; + let script_ptr = script.as_ptr(); + let script_len = script.len(); + let amount: u64 = 100_000; + let funding_type = FFIAssetLockFundingType::IdentityRegistration; + let identity_index: u32 = 0; + + let mut fee_out: u64 = 0; + let mut tx_bytes: *mut u8 = ptr::null_mut(); + let mut tx_len: usize = 0; + let mut private_key = [0u8; 32]; + + let mut call_error = FFIError::default(); + let ok = wallet_build_and_sign_asset_lock_transaction( + manager, + wallet, + sources.as_ptr(), + sources.len(), + 0, + &funding_type, + &identity_index, + &script_ptr, + &script_len, + &amount, + 1, + 1000, + &mut fee_out, + &mut tx_bytes, + &mut tx_len, + &mut private_key, + &mut call_error, + ); + assert!(!ok, "an unfunded wallet cannot produce an asset lock"); + + // Nothing here is expected to produce a transaction, but a future case that + // does must not leak it past the sanitizer. + transaction_bytes_free(tx_bytes); + // `wallet_manager_get_wallet` hands back an independently boxed clone, so + // it has to be freed separately from the manager. + wallet_free_const(wallet); + wallet_manager_free_wallet_ids(wallet_ids, wallet_count); + wallet_manager_free(manager); + + call_error +} + +/// An empty list must be rejected at the boundary rather than forwarded, where +/// it would mean `AccountTypePreference::DEFAULT` and silently reinstate a +/// funding policy this layer is not entitled to pick. +#[test] +fn an_empty_source_list_is_rejected() { + for network in NETWORKS { + unsafe { + let error = call_with_sources(network, &[]); + assert_eq!(error.code, FFIErrorCode::InvalidInput, "on {network:?}"); + let message = error_message(&error); + assert!( + message.contains("funding source"), + "the error must name the missing argument on {network:?}, got: {message}" + ); + } + } +} + +/// A caller that names sources gets past the guards and into the build, so the +/// failure comes from the empty wallet rather than from argument validation. +#[test] +fn a_named_source_reaches_the_builder() { + for network in NETWORKS { + unsafe { + let error = call_with_sources( + network, + &[ + source(FFIAccountTypePreferenceKind::BIP44), + source(FFIAccountTypePreferenceKind::BIP32), + ], + ); + assert_ne!( + error.code, + FFIErrorCode::InvalidInput, + "a pooled list is well-formed on {network:?}; the build must fail on funds, \ + not on arguments" + ); + } + } +} + +/// CoinJoin can never fund through this entry point: it always builds a +/// non-drain lock, and mixed coins may only back a drain. A caller selecting it +/// must get told, not silently handed transparent funds instead. +#[test] +fn coinjoin_is_rejected_on_the_non_drain_entry_point() { + for network in NETWORKS { + unsafe { + let error = + call_with_sources(network, &[source(FFIAccountTypePreferenceKind::CoinJoin)]); + let message = error_message(&error); + assert!( + message.contains("drain"), + "the error must explain that CoinJoin is drain-only on {network:?}, \ + got: {message}" + ); + } + } +} diff --git a/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs b/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs index 6aca94b34..8034bdcf4 100644 --- a/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs @@ -9,12 +9,19 @@ use dashcore::{OutPoint, Transaction, TxOut}; use secp256k1::PublicKey; use std::fmt; +use crate::account::AccountType; use crate::managed_account::managed_account_trait::ManagedAccountTrait; +use crate::managed_account::reservation::ReservationSet; use crate::managed_account::{ManagedCoreKeysAccount, ReservationToken}; use crate::signer::Signer; use crate::wallet::managed_wallet_info::coin_selection::SelectionStrategy; use crate::wallet::managed_wallet_info::fee::FeeRate; -use crate::wallet::managed_wallet_info::transaction_builder::{BuilderError, TransactionBuilder}; +use crate::wallet::managed_wallet_info::transaction_builder::{ + BuilderError, TransactionBuilder, TransactionSigner, +}; +use crate::wallet::managed_wallet_info::transaction_building::{ + AccountTypePreference, PooledFunding, +}; use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use crate::wallet::managed_wallet_info::ManagedWalletInfo; use crate::wallet::Wallet; @@ -38,14 +45,21 @@ pub enum AssetLockFundingType { AssetLockShieldedAddressTopUp, } -/// Which wallet account supplies the funding UTXOs (and signs the inputs) -/// of an asset lock transaction. +/// A single wallet account that supplies the funding UTXOs (and signs the +/// inputs) of a whole-balance **drain** asset lock. /// -/// `Bip44` is the standard spendable balance — the historical behavior of -/// the builders below. `CoinJoin` lets mixed coins fund an asset lock -/// directly, without first sweeping them through a transparent BIP44 -/// address (which would link the mixed UTXOs to a reusable transparent -/// address for an extra hop). +/// The asset-lock builders take a *list* of [`AccountTypePreference`] sources +/// and pool them; this is the narrower vocabulary of the drain flows, which +/// name exactly one account by construction (a drain has no change output, so +/// "which account supplies change" — the thing a pooled list decides — does not +/// arise). `Bip44` is the standard spendable balance; `CoinJoin` lets mixed +/// coins fund an asset lock directly, without first sweeping them through a +/// transparent BIP44 address (which would link the mixed UTXOs to a reusable +/// transparent address for an extra hop). +/// +/// To hand one to a builder it takes BOTH halves: [`AccountTypePreference::from`] +/// for the family and [`Self::account_index`] for the builder's `source_index`. +/// The conversion carries the family only. #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum AssetLockFundingAccount { @@ -75,6 +89,24 @@ impl AssetLockFundingAccount { } } +/// The account *family* only — the index does not survive, because +/// [`AccountTypePreference`] names a family and leaves the index to the +/// builder's separate `source_index`. Pass +/// [`AssetLockFundingAccount::account_index`] there, or the build silently +/// funds index 0 instead of the account that was named. +impl From for AccountTypePreference { + fn from(account: AssetLockFundingAccount) -> Self { + match account { + AssetLockFundingAccount::Bip44 { + .. + } => Self::BIP44, + AssetLockFundingAccount::CoinJoin { + .. + } => Self::CoinJoin, + } + } +} + /// Per-credit-output funding specification. pub struct CreditOutputFunding { /// The credit output (script + amount). @@ -111,17 +143,29 @@ pub struct AssetLockResult { /// ordering and variant semantics. pub keys: AssetLockCreditKeys, /// Owner token for the reservation this build took on the funding inputs, - /// or `None` if the funding account carried no reservation set. + /// or `None` if no funding account carried a reservation set. /// /// The caller broadcasts `transaction` and, on a rejected broadcast, must /// release the reserved inputs with /// [`ManagedCoreFundsAccount::release_reservation_if_owner`] passing this - /// token — never the unconditional `release_reservation`. See + /// token — never the unconditional `release_reservation` — on **every** + /// account in [`Self::funding_accounts`]. See /// `ReservationSet::release_if_owner` for why owner-guarded release is /// required here (`dashpay/platform#4185`). /// /// [`ManagedCoreFundsAccount::release_reservation_if_owner`]: crate::managed_account::ManagedCoreFundsAccount::release_reservation_if_owner pub reservation_token: Option, + /// The accounts that actually contributed inputs to `transaction`, and so + /// the accounts holding a share of this build's reservation — a pooled + /// build reserves in each contributing account's own set, all under the one + /// [`Self::reservation_token`]. + /// + /// This is the *contributor* list, not everything the source list offered: + /// coin selection routinely takes nothing from most offered accounts, and a + /// list naming every DashPay contact would make the caller's release and + /// bookkeeping scale with the address book while claiming contributions + /// that never happened. + pub funding_accounts: Vec, } /// Errors specific to asset lock transaction building. @@ -143,8 +187,6 @@ pub enum AssetLockError { SigningFailed(String), /// The wallet does not have a private key (watch-only). WatchOnlyWallet, - /// The specified funding account (BIP44 or CoinJoin, by index) was not found. - AccountNotFound(u32), /// No change address available. NoChangeAddress, /// Underlying transaction builder error. @@ -164,7 +206,6 @@ impl fmt::Display for AssetLockError { Self::Signer(msg) => write!(f, "Signer error: {msg}"), Self::SigningFailed(msg) => write!(f, "Signing failed: {msg}"), Self::WatchOnlyWallet => write!(f, "Cannot sign with watch-only wallet"), - Self::AccountNotFound(idx) => write!(f, "funding account {} not found", idx), Self::NoChangeAddress => write!(f, "No change address available"), Self::Builder(e) => write!(f, "Transaction builder error: {e}"), } @@ -218,12 +259,17 @@ fn resolve_funding_account( } } -/// Shared guard for both asset-lock builders: a drain rewrites exactly one -/// credit output, and CoinJoin accounts have no change-address pool semantics -/// for asset locks (change would need re-denomination), so they only support -/// the whole-balance drain. -fn validate_drain_funding( - funding_account: AssetLockFundingAccount, +/// Shared guard for both asset-lock builders, run before any wallet state is +/// touched. +/// +/// * A drain rewrites exactly one credit output, so it requires exactly one. +/// * CoinJoin funding is drain-only and must be the *sole* source. CoinJoin +/// accounts have no change-address pool semantics for asset locks (change +/// would need re-denomination), and pooling mixed coins with transparent ones +/// in a single transaction links them and undoes the mixing — the same +/// reasoning that keeps CoinJoin out of [`AccountTypePreference::DEFAULT`]. +fn validate_funding_sources( + sources: &[AccountTypePreference], credit_output_count: usize, drain: bool, ) -> Result<(), AssetLockError> { @@ -232,15 +278,160 @@ fn validate_drain_funding( "drain asset lock requires exactly one credit output".into(), ))); } - if matches!(funding_account, AssetLockFundingAccount::CoinJoin { .. }) && !drain { + let has_coinjoin = sources.contains(&AccountTypePreference::CoinJoin); + if has_coinjoin && !drain { return Err(AssetLockError::Builder(BuilderError::InvalidData( "CoinJoin-funded asset locks support drain mode only".into(), ))); } + if has_coinjoin && sources.len() > 1 { + return Err(AssetLockError::Builder(BuilderError::InvalidData( + "CoinJoin funding cannot be pooled with other sources: spending mixed outputs \ + alongside transparent ones in one transaction links them and undoes the mixing" + .into(), + ))); + } Ok(()) } +/// The accounts among `offered` that contributed an input to `transaction`. +/// +/// Only these hold a share of the build's reservation, so this is what the +/// caller must reconcile on a rejected broadcast. An outpoint is attributed to +/// an account when that account still holds it as a UTXO — a build reserves its +/// inputs but does not remove them, so this is exact right after the build. +fn contributing_accounts( + accounts: &crate::account::ManagedAccountCollection, + offered: &[AccountType], + transaction: &Transaction, +) -> Vec { + // Driven by the inputs, of which a transaction has few, rather than by each + // account's UTXO map, of which an offered account can hold many. + offered + .iter() + .copied() + .filter(|account_type| { + accounts.funds_account(account_type).is_some_and(|account| { + transaction + .input + .iter() + .any(|input| account.utxos.contains_key(&input.previous_output)) + }) + }) + .collect() +} + +/// The reservation a pooled build took, and the means to give it back. +/// +/// A pooled build reserves in EACH contributing account's own set under the one +/// owner token, so releasing a single account's set would strand the remaining +/// inputs until the 24-block TTL sweep. Both builders reach their release paths +/// *after* the transaction is already signed, and the caller never received the +/// token on an error path — so this is the only thing that can free them. +struct BuildReservations { + /// One handle per offered account. Offered rather than contributing, + /// because these are captured before the build reports who contributed; + /// releasing against an account that reserved nothing is a no-op. + sets: Vec, + /// The outpoints this build reserved. + reserved: Vec, + /// The owner token, or `None` if no funding account carried a set. + token: Option, +} + +impl BuildReservations { + /// Give back this build's reservation in every funded account. + /// + /// Owner-guarded only — never the unconditional `release_reservation` (see + /// `ReservationSet::release_if_owner`, `dashpay/platform#4185`). + fn release(&self) { + if let Some(token) = self.token { + for set in &self.sets { + set.release_if_owner(&self.reserved, token); + } + } + } +} + +/// A built, funded and signed asset lock, before either builder does its own +/// credit-key bookkeeping. +struct SignedAssetLock { + transaction: Transaction, + fee: u64, + /// The accounts whose UTXOs were offered to coin selection, in funding + /// order; the first supplied the change address. + offered: Vec, + reservations: BuildReservations, +} + impl ManagedWalletInfo { + /// Everything both asset-lock builders do before they diverge: validate the + /// sources, assemble the payload, fund from the pooled accounts, capture + /// what a post-build failure must hand back, and sign. + /// + /// Shared deliberately rather than mirrored. The reservation capture is the + /// subtle half, and it has to happen between funding and signing: a fix + /// applied to one builder and not the other would silently reintroduce the + /// stranded-input bug on the path that was missed. + #[allow(clippy::too_many_arguments)] + async fn build_signed_asset_lock( + &mut self, + wallet: &Wallet, + funding_sources: &[AccountTypePreference], + source_index: u32, + credit_outputs: Vec, + fee_per_kb: u64, + drain: bool, + signer: &T, + ) -> Result { + validate_funding_sources(funding_sources, credit_outputs.len(), drain)?; + + // Build first, derive credit keys after — a build failure must not + // consume any funding-key indices. + let mut builder = TransactionBuilder::new() + .set_fee_rate(FeeRate::new(fee_per_kb)) + .set_current_height(self.last_processed_height()) + .set_special_payload(TransactionPayload::AssetLockPayloadType(AssetLockPayload::new( + credit_outputs, + ))) + .require_final_inputs(); + if drain { + builder = builder.set_selection_strategy(SelectionStrategy::All); + } + let PooledFunding { + builder, + paths, + accounts: offered, + } = self.fund(wallet, funding_sources, source_index, builder)?; + + // Clone each offered account's reservation handle (a shared `Arc` view + // of the same set) NOW, before either caller's bookkeeping re-borrows + // `self.accounts` — past that point a failure can no longer reach these + // accounts to release them. + let sets: Vec = offered + .iter() + .filter_map(|account_type| self.accounts.funds_account(account_type)) + .map(|account| account.reservations().clone()) + .collect(); + + let (transaction, fee, token) = + builder.build_signed_reserved(signer, move |addr| paths.get(&addr).cloned()).await?; + + let reserved: Vec = + transaction.input.iter().map(|input| input.previous_output).collect(); + + Ok(SignedAssetLock { + transaction, + fee, + offered, + reservations: BuildReservations { + sets, + reserved, + token, + }, + }) + } + /// Build and sign an asset lock transaction. /// /// Creates a special transaction (type 8) with `AssetLockPayload` that locks @@ -249,16 +440,27 @@ impl ManagedWalletInfo { /// The transaction is built first, and keys are only derived after a successful /// build — so no addresses are consumed if the build fails. /// - /// `funding_account` picks which account family supplies (and signs) the - /// funding UTXOs — see [`AssetLockFundingAccount`]. `drain` locks the - /// account's whole spendable balance: every final UTXO is consumed and - /// the single credit output's value is rewritten to `Σ inputs − fee` - /// (the caller's credit-output value is ignored; exactly one credit - /// output is required). + /// `funding_sources` picks which account families supply (and sign) the + /// funding UTXOs; coin selection draws from the union of their UTXOs and + /// the first source supplies the change address. A single source is an + /// explicit request for that one account and errors if it is absent; a + /// pooled list skips the sources this wallet has nothing for. `source_index` + /// addresses the standard families (BIP44/BIP32/CoinJoin); DashPay set + /// selectors span their own indices. See + /// [`ManagedWalletInfo::build_and_sign_transaction`] for the shared + /// source-list semantics. + /// + /// `drain` locks the sourced accounts' whole spendable balance: every final + /// UTXO is consumed and the single credit output's value is rewritten to + /// `Σ inputs − fee` (the caller's credit-output value is ignored; exactly + /// one credit output is required). CoinJoin funding is drain-only and must + /// be the sole source: pooling mixed outputs with transparent ones in one + /// transaction links them and undoes the mixing. pub async fn build_asset_lock( &mut self, wallet: &Wallet, - funding_account: AssetLockFundingAccount, + funding_sources: &[AccountTypePreference], + source_index: u32, credit_output_fundings: Vec, fee_per_kb: u64, drain: bool, @@ -269,73 +471,27 @@ impl ManagedWalletInfo { wallet.root_extended_priv_key().map_err(|_| AssetLockError::WatchOnlyWallet)?.clone(); let network = self.network; - let height = self.last_processed_height(); - - let account_index = funding_account.account_index(); - let acc = match funding_account { - AssetLockFundingAccount::Bip44 { - .. - } => wallet - .get_bip44_account(account_index) - .ok_or(AssetLockError::AccountNotFound(account_index))?, - AssetLockFundingAccount::CoinJoin { - .. - } => wallet - .get_coinjoin_account(account_index) - .ok_or(AssetLockError::AccountNotFound(account_index))?, - }; - - let funds_acc = match funding_account { - AssetLockFundingAccount::Bip44 { - .. - } => self - .accounts - .standard_bip44_accounts - .get_mut(&account_index) - .ok_or(AssetLockError::AccountNotFound(account_index))?, - AssetLockFundingAccount::CoinJoin { - .. - } => self - .accounts - .coinjoin_accounts - .get_mut(&account_index) - .ok_or(AssetLockError::AccountNotFound(account_index))?, - }; - - validate_drain_funding(funding_account, credit_output_fundings.len(), drain)?; let credit_outputs: Vec = credit_output_fundings.iter().map(|f| f.output.clone()).collect(); - // Build first, derive credit keys after — a build failure must not - // consume any funding-key indices. - let mut builder = TransactionBuilder::new() - .set_fee_rate(FeeRate::new(fee_per_kb)) - .set_current_height(height) - .set_special_payload(TransactionPayload::AssetLockPayloadType(AssetLockPayload::new( + let SignedAssetLock { + transaction, + fee, + offered, + reservations, + } = self + .build_signed_asset_lock( + wallet, + funding_sources, + source_index, credit_outputs, - ))); - if drain { - builder = builder.set_selection_strategy(SelectionStrategy::All); - } - let (transaction, fee, reservation_token) = builder - .add_funding(funds_acc, acc) - .require_final_inputs() - .build_signed_reserved(wallet, |addr| funds_acc.address_derivation_path(&addr)) + fee_per_kb, + drain, + wallet, + ) .await?; - // The build above reserved the funding inputs. Clone the reservation - // handle (a shared `Arc` view of the same set) now, before the loop - // below re-borrows `self.accounts` — a mid-loop failure can no longer - // reach `funds_acc` to release, and the caller never received the token - // to release it either, so a leaked reservation would strand the - // already-signed inputs until the 24-block TTL sweep. Owner-guarded - // release only (see `ReservationSet::release_if_owner`, - // `dashpay/platform#4185`). - let reservations = funds_acc.reservations().clone(); - let reserved: Vec = - transaction.input.iter().map(|input| input.previous_output).collect(); - // Derive one private key per credit output. On any failure, release // this build's own reservation before returning. let keys = match (|| -> Result, AssetLockError> { @@ -355,18 +511,18 @@ impl ManagedWalletInfo { })() { Ok(keys) => keys, Err(e) => { - if let Some(token) = reservation_token { - reservations.release_if_owner(&reserved, token); - } + reservations.release(); return Err(e); } }; + let funding_accounts = contributing_accounts(&self.accounts, &offered, &transaction); Ok(AssetLockResult { transaction, fee, keys: AssetLockCreditKeys::Private(keys), - reservation_token, + reservation_token: reservations.token, + funding_accounts, }) } @@ -386,82 +542,39 @@ impl ManagedWalletInfo { /// request signatures from the same signer when later consuming the /// credits on Platform. /// - /// `funding_account` / `drain` — see [`Self::build_asset_lock`]. + /// `funding_sources` / `source_index` / `drain` — see + /// [`Self::build_asset_lock`]. + #[allow(clippy::too_many_arguments)] pub async fn build_asset_lock_with_signer( &mut self, wallet: &Wallet, - funding_account: AssetLockFundingAccount, + funding_sources: &[AccountTypePreference], + source_index: u32, credit_output_fundings: Vec, fee_per_kb: u64, drain: bool, signer: &S, ) -> Result { - let height = self.last_processed_height(); - - let account_index = funding_account.account_index(); - let acc = match funding_account { - AssetLockFundingAccount::Bip44 { - .. - } => wallet - .get_bip44_account(account_index) - .ok_or(AssetLockError::AccountNotFound(account_index))? - .clone(), - AssetLockFundingAccount::CoinJoin { - .. - } => wallet - .get_coinjoin_account(account_index) - .ok_or(AssetLockError::AccountNotFound(account_index))? - .clone(), - }; - - let funds_acc = match funding_account { - AssetLockFundingAccount::Bip44 { - .. - } => self - .accounts - .standard_bip44_accounts - .get_mut(&account_index) - .ok_or(AssetLockError::AccountNotFound(account_index))?, - AssetLockFundingAccount::CoinJoin { - .. - } => self - .accounts - .coinjoin_accounts - .get_mut(&account_index) - .ok_or(AssetLockError::AccountNotFound(account_index))?, - }; - - validate_drain_funding(funding_account, credit_output_fundings.len(), drain)?; - let credit_outputs: Vec = credit_output_fundings.iter().map(|f| f.output.clone()).collect(); - let mut builder = TransactionBuilder::new() - .set_fee_rate(FeeRate::new(fee_per_kb)) - .set_current_height(height) - .set_special_payload(TransactionPayload::AssetLockPayloadType(AssetLockPayload::new( + let SignedAssetLock { + transaction, + fee, + offered, + reservations, + } = self + .build_signed_asset_lock( + wallet, + funding_sources, + source_index, credit_outputs, - ))); - if drain { - builder = builder.set_selection_strategy(SelectionStrategy::All); - } - let (transaction, fee, reservation_token) = builder - .add_funding(funds_acc, &acc) - .require_final_inputs() - .build_signed_reserved(signer, |addr| funds_acc.address_derivation_path(&addr)) + fee_per_kb, + drain, + signer, + ) .await?; - // The build above reserved the funding inputs. Clone the reservation - // handle (a shared `Arc` view of the same set) before the bookkeeping - // loop below re-borrows `self.accounts`, so a failure during Phase 1–3 - // — which runs after the transaction is already signed — can still - // release THIS build's reservation instead of stranding the signed - // inputs until the 24-block TTL sweep. Owner-guarded release only (see - // `ReservationSet::release_if_owner`, `dashpay/platform#4185`). - let reservations = funds_acc.reservations().clone(); - let reserved: Vec = - transaction.input.iter().map(|input| input.previous_output).collect(); - // Credit-output bookkeeping: for each funding, peek the next unused // path on its account, ask the signer for the matching pubkey, and // only mark the index used once the signer has succeeded. @@ -516,18 +629,18 @@ impl ManagedWalletInfo { { Ok(keys) => keys, Err(e) => { - if let Some(token) = reservation_token { - reservations.release_if_owner(&reserved, token); - } + reservations.release(); return Err(e); } }; + let funding_accounts = contributing_accounts(&self.accounts, &offered, &transaction); Ok(AssetLockResult { transaction, fee, keys: AssetLockCreditKeys::Public(credit_output_keys), - reservation_token, + reservation_token: reservations.token, + funding_accounts, }) } } @@ -540,6 +653,8 @@ mod tests { use crate::{Network, Utxo}; use dashcore::{OutPoint, ScriptBuf, Txid}; use dashcore_hashes::Hash; + use std::collections::HashSet; + use test_case::test_case; fn test_credit_outputs(amounts: &[u64]) -> Vec { amounts @@ -659,9 +774,8 @@ mod tests { let result = info .build_asset_lock( &wallet, - AssetLockFundingAccount::CoinJoin { - account_index: 0, - }, + &[AccountTypePreference::CoinJoin], + 0, test_credit_outputs(&[0]), 1000, true, @@ -701,9 +815,8 @@ mod tests { let result = info .build_asset_lock( &wallet, - AssetLockFundingAccount::CoinJoin { - account_index: 0, - }, + &[AccountTypePreference::CoinJoin], + 0, test_credit_outputs(&[0, 0]), 1000, true, @@ -728,9 +841,8 @@ mod tests { let result = info .build_asset_lock( &wallet, - AssetLockFundingAccount::CoinJoin { - account_index: 0, - }, + &[AccountTypePreference::CoinJoin], + 0, test_credit_outputs(&[200_000]), 1000, false, @@ -751,7 +863,6 @@ mod tests { AssetLockError::WatchOnlyWallet.to_string(), "Cannot sign with watch-only wallet" ); - assert_eq!(AssetLockError::AccountNotFound(5).to_string(), "funding account 5 not found"); assert_eq!(AssetLockError::NoChangeAddress.to_string(), "No change address available"); } @@ -768,15 +879,7 @@ mod tests { async fn test_empty_credit_outputs_rejected() { let (wallet, mut info) = test_wallet_and_info(); let result = info - .build_asset_lock( - &wallet, - AssetLockFundingAccount::Bip44 { - account_index: 0, - }, - vec![], - 1000, - false, - ) + .build_asset_lock(&wallet, &[AccountTypePreference::BIP44], 0, vec![], 1000, false) .await; assert!(matches!(result, Err(AssetLockError::Builder(BuilderError::NoOutputs)))); } @@ -787,15 +890,17 @@ mod tests { let result = info .build_asset_lock( &wallet, - AssetLockFundingAccount::Bip44 { - account_index: 99, - }, + &[AccountTypePreference::BIP44], + 99, test_credit_outputs(&[100_000]), 1000, false, ) .await; - assert!(matches!(result, Err(AssetLockError::AccountNotFound(99)))); + assert!( + matches!(result, Err(AssetLockError::Builder(BuilderError::AccountNotFound(_)))), + "a single named source is strict: the absent account must be an error" + ); } #[tokio::test] @@ -805,9 +910,8 @@ mod tests { let result = info .build_asset_lock( &wallet, - AssetLockFundingAccount::Bip44 { - account_index: 0, - }, + &[AccountTypePreference::BIP44], + 0, test_credit_outputs(&[500_000]), 1000, false, @@ -833,9 +937,8 @@ mod tests { let result = info .build_asset_lock( &wallet, - AssetLockFundingAccount::Bip44 { - account_index: 0, - }, + &[AccountTypePreference::BIP44], + 0, test_credit_outputs(&[200_000]), 1000, false, @@ -860,9 +963,8 @@ mod tests { let result = info .build_asset_lock( &wallet, - AssetLockFundingAccount::Bip44 { - account_index: 0, - }, + &[AccountTypePreference::BIP44], + 0, test_credit_outputs(&[200_000]), 1000, false, @@ -987,9 +1089,8 @@ mod tests { let result = info .build_asset_lock_with_signer( &wallet, - AssetLockFundingAccount::Bip44 { - account_index: 0, - }, + &[AccountTypePreference::BIP44], + 0, vec![], 1000, false, @@ -1016,16 +1117,18 @@ mod tests { let result = info .build_asset_lock_with_signer( &wallet, - AssetLockFundingAccount::Bip44 { - account_index: 99, - }, + &[AccountTypePreference::BIP44], + 99, test_credit_outputs(&[100_000]), 1000, false, &signer, ) .await; - assert!(matches!(result, Err(AssetLockError::AccountNotFound(99)))); + assert!( + matches!(result, Err(AssetLockError::Builder(BuilderError::AccountNotFound(_)))), + "a single named source is strict: the absent account must be an error" + ); } #[tokio::test] @@ -1056,9 +1159,8 @@ mod tests { let result = info .build_asset_lock_with_signer( &wallet, - AssetLockFundingAccount::Bip44 { - account_index: 0, - }, + &[AccountTypePreference::BIP44], + 0, test_credit_outputs(&[100_000]), 1000, false, @@ -1096,9 +1198,8 @@ mod tests { let result = info .build_asset_lock_with_signer( &wallet, - AssetLockFundingAccount::Bip44 { - account_index: 0, - }, + &[AccountTypePreference::BIP44], + 0, fundings, 1000, false, @@ -1158,9 +1259,8 @@ mod tests { let result = info .build_asset_lock_with_signer( &wallet, - AssetLockFundingAccount::Bip44 { - account_index: 0, - }, + &[AccountTypePreference::BIP44], + 0, test_credit_outputs(&[500_000]), 1000, false, @@ -1173,4 +1273,353 @@ mod tests { result.err() ); } + + // -- Pooled funding -------------------------------------------------- + // + // Asset locks fund from a LIST of sources. These pin the three things the + // pooling has to get right: it really spans accounts, it tolerates the + // sources a wallet does not have, and every failure path after the build + // gives back the reservations it took — in *each* contributing account, + // since a pooled build reserves per account under one owner token. + + /// The default pooled set, mirroring platform's `ASSET_LOCK_FUNDING_SOURCES`. + const POOLED: [AccountTypePreference; 3] = [ + AccountTypePreference::BIP44, + AccountTypePreference::BIP32, + AccountTypePreference::AllDashpayReceivingFunds, + ]; + + /// Fund the BIP32 account at index 0 with a confirmed UTXO. + fn insert_funded_bip32_utxo( + info: &mut ManagedWalletInfo, + wallet: &Wallet, + txid_byte: u8, + value: u64, + ) -> OutPoint { + let account_xpub = wallet + .accounts + .standard_bip32_accounts + .get(&0) + .expect("default wallet has BIP32 account 0") + .account_xpub; + let account = info.accounts.standard_bip32_accounts.get_mut(&0).unwrap(); + let funding_address = account.next_receive_address(Some(&account_xpub), true).unwrap(); + let outpoint = OutPoint { + txid: Txid::from_byte_array([txid_byte; 32]), + vout: 0, + }; + account.utxos.insert( + outpoint, + Utxo { + outpoint, + txout: TxOut { + value, + script_pubkey: funding_address.script_pubkey(), + }, + address: funding_address, + height: 1000, + is_coinbase: false, + is_confirmed: true, + is_instantlocked: false, + is_locked: false, + is_trusted: false, + }, + ); + outpoint + } + + fn bip44_0() -> AccountType { + AccountType::Standard { + index: 0, + standard_account_type: crate::account::StandardAccountType::BIP44Account, + } + } + + fn bip32_0() -> AccountType { + AccountType::Standard { + index: 0, + standard_account_type: crate::account::StandardAccountType::BIP32Account, + } + } + + /// A credit output whose one-time key comes from an identity top-up + /// account that does not exist, so credit-key derivation fails *after* the + /// transaction is built and signed — the window in which a pooled build + /// holds reservations it must give back. + fn credit_output_with_missing_key_account() -> Vec { + let mut fundings = test_credit_outputs(&[400_000]); + fundings[0].funding_type = AssetLockFundingType::IdentityTopUp; + fundings[0].identity_index = 7; + fundings + } + + /// Reserved outpoints across the two standard accounts at height 1100. + fn reserved_outpoints(info: &ManagedWalletInfo) -> HashSet { + [bip44_0(), bip32_0()] + .iter() + .filter_map(|at| info.accounts.funds_account(at)) + .flat_map(|account| account.reservations().reserved(1100)) + .collect() + } + + /// Neither standard account covers the lock on its own, so the build only + /// succeeds by pooling both — which also proves the derivation paths were + /// collected across accounts, since every input had to be signed. Change + /// goes back to BIP44 (the first source), each account reserves what it + /// contributed in its own set, and both are reported as funding accounts. + #[tokio::test] + async fn pooled_asset_lock_spans_the_standard_accounts() { + let (wallet, mut info) = test_wallet_and_info(); + let bip44 = insert_funded_utxo(&mut info, &wallet, 0x11, 300_000, true); + let bip32 = insert_funded_bip32_utxo(&mut info, &wallet, 0x22, 300_000); + info.update_last_processed_height(1100); + + let result = info + .build_asset_lock(&wallet, &POOLED, 0, test_credit_outputs(&[500_000]), 1000, false) + .await + .expect("a 500k lock funded by two 300k accounts"); + + let spent: HashSet = + result.transaction.input.iter().map(|txin| txin.previous_output).collect(); + assert_eq!(spent, HashSet::from([bip44, bip32]), "the lock must pool both accounts"); + for (i, txin) in result.transaction.input.iter().enumerate() { + assert!(!txin.script_sig.is_empty(), "input {i} not signed"); + } + + // Change returns to the FIRST source, not to whichever account happened + // to be selected from last. + let change = result + .transaction + .output + .iter() + .find(|out| !out.script_pubkey.is_op_return()) + .expect("600k in against a 500k lock leaves change"); + let bip44_account = info.accounts.standard_bip44_accounts.get(&0).unwrap(); + assert!( + bip44_account + .managed_account_type() + .all_script_pubkeys() + .contains(&change.script_pubkey), + "change must return to the BIP44 account" + ); + + // One token, but the reservation lives in each contributing account's + // own set — that set is the one its next coin selection consults. + assert!(result.reservation_token.is_some()); + assert_eq!(bip44_account.reservations().reserved(1100), HashSet::from([bip44])); + assert_eq!( + info.accounts.standard_bip32_accounts.get(&0).unwrap().reservations().reserved(1100), + HashSet::from([bip32]) + ); + assert_eq!( + result.funding_accounts.iter().copied().collect::>(), + HashSet::from([bip44_0(), bip32_0()]) + ); + } + + /// A pooled list names sources this wallet may have nothing for. Skipping + /// them is the point: a wallet with no DashPay contacts still funds an + /// asset lock, and only the accounts that contributed are reported. + #[tokio::test] + async fn pooled_sources_skip_what_the_wallet_does_not_have() { + let (wallet, mut info) = test_wallet_and_info(); + let bip44 = insert_funded_utxo(&mut info, &wallet, 0x11, 900_000, true); + info.update_last_processed_height(1100); + + let result = info + .build_asset_lock(&wallet, &POOLED, 0, test_credit_outputs(&[500_000]), 1000, false) + .await + .expect("no contacts and an empty BIP32 account must not block the build"); + + let spent: Vec = + result.transaction.input.iter().map(|txin| txin.previous_output).collect(); + assert_eq!(spent, vec![bip44]); + assert_eq!( + result.funding_accounts, + vec![bip44_0()], + "an account that contributed nothing is not a funding account" + ); + } + + /// A pooled list that funds nothing at all is still an error — leniency + /// skips absent sources, it does not invent funds. + #[tokio::test] + async fn pooled_sources_that_resolve_to_nothing_are_an_error() { + let (wallet, mut info) = test_wallet_and_info(); + info.update_last_processed_height(1100); + + let result = info + .build_asset_lock(&wallet, &POOLED, 99, test_credit_outputs(&[500_000]), 1000, false) + .await; + + assert!( + matches!(result, Err(AssetLockError::Builder(BuilderError::AccountNotFound(_)))), + "no account of any named source at index 99" + ); + } + + /// Mixed coins must never ride alongside transparent ones: pooling would + /// link them in a single transaction and undo the mixing. Rejected before + /// any wallet state is touched, drain or not. + #[test_case(true ; "drain")] + #[test_case(false ; "exact amount")] + #[tokio::test] + async fn coinjoin_cannot_be_pooled_with_transparent_sources(drain: bool) { + let (wallet, mut info) = test_wallet_and_info(); + insert_funded_coinjoin_utxo(&mut info, &wallet, 0x41, 900_000, true); + insert_funded_utxo(&mut info, &wallet, 0x11, 900_000, true); + info.update_last_processed_height(1100); + + let result = info + .build_asset_lock( + &wallet, + &[AccountTypePreference::CoinJoin, AccountTypePreference::BIP44], + 0, + test_credit_outputs(&[500_000]), + 1000, + drain, + ) + .await; + + assert!(matches!(result, Err(AssetLockError::Builder(BuilderError::InvalidData(_))))); + assert!( + reserved_outpoints(&info).is_empty() + && info + .accounts + .coinjoin_accounts + .get(&0) + .unwrap() + .reservations() + .reserved(1100) + .is_empty(), + "a rejected source list must not have reserved anything" + ); + } + + /// The failure window a pooled build opens: the transaction is already + /// built, signed and reserved when credit-key derivation fails. The caller + /// never receives the token, so nothing else can release those inputs — + /// they must be freed here, in EVERY contributing account, or the funds + /// stay stranded until the 24-block TTL sweep. + #[tokio::test] + async fn credit_key_failure_releases_the_reservation_in_every_pooled_account() { + let (wallet, mut info) = test_wallet_and_info(); + insert_funded_utxo(&mut info, &wallet, 0x11, 300_000, true); + insert_funded_bip32_utxo(&mut info, &wallet, 0x22, 300_000); + info.update_last_processed_height(1100); + + let result = info + .build_asset_lock( + &wallet, + &POOLED, + 0, + credit_output_with_missing_key_account(), + 1000, + false, + ) + .await; + + assert!( + matches!(result, Err(AssetLockError::FundingAccountNotFound(_))), + "the absent identity top-up account must fail credit-key derivation" + ); + assert!( + reserved_outpoints(&info).is_empty(), + "both pooled accounts must have released this build's reservation" + ); + } + + /// [`credit_key_failure_releases_the_reservation_in_every_pooled_account`] + /// for the signer-driven builder, whose bookkeeping loop runs after an + /// `.await` and so had the same stranding window. + #[tokio::test] + async fn signer_credit_key_failure_releases_the_reservation_in_every_pooled_account() { + let (wallet, mut info) = test_wallet_and_info(); + insert_funded_utxo(&mut info, &wallet, 0x11, 300_000, true); + insert_funded_bip32_utxo(&mut info, &wallet, 0x22, 300_000); + info.update_last_processed_height(1100); + let root = match &wallet.wallet_type { + crate::wallet::WalletType::Mnemonic { + root_extended_private_key, + .. + } => root_extended_private_key.clone(), + _ => unreachable!("test_wallet_and_info produces a mnemonic wallet"), + }; + let signer = InMemorySigner { + root, + network: Network::Testnet, + }; + + let result = info + .build_asset_lock_with_signer( + &wallet, + &POOLED, + 0, + credit_output_with_missing_key_account(), + 1000, + false, + &signer, + ) + .await; + + assert!( + matches!(result, Err(AssetLockError::FundingAccountNotFound(_))), + "the absent identity top-up account must fail credit-key bookkeeping" + ); + assert!( + reserved_outpoints(&info).is_empty(), + "both pooled accounts must have released this build's reservation" + ); + } + + /// A signing failure is handled one layer down, by the builder itself — + /// which must also reach every funding account, not just the first. + #[tokio::test] + async fn signing_failure_releases_the_reservation_in_every_pooled_account() { + struct FailingSigner; + + #[async_trait::async_trait] + impl Signer for FailingSigner { + type Error = String; + + fn supported_methods(&self) -> &[SignerMethod] { + IN_MEMORY_METHODS + } + + async fn sign_ecdsa( + &self, + _path: &DerivationPath, + _sighash: [u8; 32], + ) -> Result<(secp256k1::ecdsa::Signature, PublicKey), Self::Error> { + Err("signing device unavailable".to_string()) + } + + async fn public_key(&self, _path: &DerivationPath) -> Result { + Err("signing device unavailable".to_string()) + } + } + + let (wallet, mut info) = test_wallet_and_info(); + insert_funded_utxo(&mut info, &wallet, 0x11, 300_000, true); + insert_funded_bip32_utxo(&mut info, &wallet, 0x22, 300_000); + info.update_last_processed_height(1100); + + let result = info + .build_asset_lock_with_signer( + &wallet, + &POOLED, + 0, + test_credit_outputs(&[500_000]), + 1000, + false, + &FailingSigner, + ) + .await; + + assert!(result.is_err(), "a failing signer must not produce a transaction"); + assert!( + reserved_outpoints(&info).is_empty(), + "both pooled accounts must have released this build's reservation" + ); + } } diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_building.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_building.rs index 50aa36277..3d1ec4f44 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_building.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_building.rs @@ -90,15 +90,36 @@ impl fmt::Display for AccountTypePreference { } } +/// A builder seeded with the funding of a resolved source list, plus what it +/// took to seed it: the derivation path of every candidate input address +/// (inputs can come from different accounts, so one account's resolver is not +/// enough) and the accounts whose UTXOs were offered to selection, in funding +/// order. +/// +/// The offered accounts are not the accounts that end up *contributing* inputs +/// — selection routinely takes nothing from most of them — but they are the +/// accounts holding this build's reservations, so they are what a failure path +/// must reconcile. +pub(super) struct PooledFunding { + /// The builder, with one `add_funding` call per resolved account. + pub builder: TransactionBuilder, + /// Address → derivation path for every UTXO offered to selection. + pub paths: HashMap, + /// The accounts funded, in funding order; the first supplied the change + /// address. + pub accounts: Vec, +} + impl ManagedWalletInfo { /// Build and sign a transaction funded from the given account types at /// `source_index`, signing with the wallet's own keys. /// /// Coin selection draws from the union of those accounts' UTXOs, and the /// first of them supplies the change address. An empty `sources` means - /// [`AccountTypePreference::DEFAULT`], skipping the types absent at the - /// index; a non-empty one is taken literally and every account named must - /// exist. + /// [`AccountTypePreference::DEFAULT`]. A *single* source is an explicit + /// request for that one account and errors if it is absent; a *pooled* + /// (multi-source) list skips the sources this wallet has nothing for and + /// errors only when none of them funds anything. pub async fn build_and_sign_transaction( &mut self, wallet: &Wallet, @@ -193,7 +214,11 @@ impl ManagedWalletInfo { .set_selection_strategy(strategy) .set_current_height(height); - let (mut builder, paths) = self.fund(wallet, sources, source_index, builder)?; + let PooledFunding { + mut builder, + paths, + accounts: _, + } = self.fund(wallet, sources, source_index, builder)?; for (address, value) in outputs { builder = builder.add_output(&address, value); @@ -238,26 +263,35 @@ impl ManagedWalletInfo { /// Seed `builder` with the UTXOs of every funding account named by /// `sources`, returning it alongside the derivation path of each candidate /// input address, since the inputs can come from different accounts. - fn fund( + /// + /// A single-source list is *strict*: it names one account and a caller that + /// asked for exactly those funds must not silently be given others', so a + /// missing account is an error. A pooled list (two or more sources, or the + /// empty list standing for [`AccountTypePreference::DEFAULT`]) is *lenient*: + /// a wallet with no BIP32 account and no DashPay contacts still funds from + /// the sources it does have, and only a list that funds nothing at all is an + /// error. + pub(super) fn fund( &mut self, wallet: &Wallet, sources: &[AccountTypePreference], source_index: u32, mut builder: TransactionBuilder, - ) -> Result<(TransactionBuilder, HashMap), BuilderError> { - let named_explicitly = !sources.is_empty(); - let preferences = if named_explicitly { - sources + ) -> Result { + let preferences = if sources.is_empty() { + &AccountTypePreference::DEFAULT[..] } else { - &AccountTypePreference::DEFAULT + sources }; + let strict = preferences.len() == 1; let mut paths = HashMap::new(); + let mut accounts: Vec = Vec::new(); let mut funded: HashSet = HashSet::new(); for &preference in preferences { let account_types = self.account_types_for(preference, source_index); - if account_types.is_empty() && named_explicitly { + if account_types.is_empty() && strict { return Err(BuilderError::AccountNotFound(format!("account {preference}"))); } @@ -274,7 +308,7 @@ impl ManagedWalletInfo { let managed_account = self.accounts.funds_account_mut(&account_type); let (Some(account), Some(managed_account)) = (account, managed_account) else { - if named_explicitly { + if strict { return Err(BuilderError::AccountNotFound(format!( "account {account_type}" ))); @@ -289,16 +323,21 @@ impl ManagedWalletInfo { } builder = builder.add_funding(managed_account, account); funded.insert(account_type); + accounts.push(account_type); } } - if funded.is_empty() { + if accounts.is_empty() { return Err(BuilderError::AccountNotFound(format!( - "no funding account of any type at index {source_index}" + "no funding account of any named source at index {source_index}" ))); } - Ok((builder, paths)) + Ok(PooledFunding { + builder, + paths, + accounts, + }) } } #[cfg(test)] @@ -1022,6 +1061,54 @@ mod tests { ); } + /// A pooled list names sources a wallet may have nothing for — the default + /// send set names every DashPay contact, and most wallets have none. Those + /// are skipped, not fatal; a SINGLE named source stays strict, because a + /// caller asking for exactly one account's funds must not silently be given + /// another's. + #[tokio::test] + async fn a_pooled_list_skips_absent_sources_where_a_single_one_is_strict() { + let (wallet, mut info) = test_wallet_and_info(); + let bip44 = fund(&wallet, &mut info, AccountTypePreference::BIP44, 0, 0x11); + info.update_last_processed_height(1100); + + // Pooled: no contacts exist, so `AllDashpayReceivingFunds` resolves to + // nothing and the send still goes out of BIP44. + let (tx, _fee) = info + .build_and_sign_transaction( + &wallet, + &[ + AccountTypePreference::BIP44, + AccountTypePreference::BIP32, + AccountTypePreference::AllDashpayReceivingFunds, + ], + 0, + dest_outputs(200_000), + FeeRate::normal(), + SelectionStrategy::BranchAndBound, + ) + .await + .expect("a wallet with no contacts still sends from its standard accounts"); + let spent: Vec = tx.input.iter().map(|txin| txin.previous_output).collect(); + assert_eq!(spent, vec![bip44]); + + // Strict: that same absent source, named alone, is an error. + let result = info + .build_and_sign_transaction( + &wallet, + &[AccountTypePreference::AllDashpayReceivingFunds], + 0, + dest_outputs(200_000), + FeeRate::normal(), + SelectionStrategy::BranchAndBound, + ) + .await; + assert!( + matches!(result, Err(BuilderError::AccountNotFound(_))), + "a single named source must not fall back to other accounts" + ); + } + /// Neither account covers the 500k target on its own, so the build only /// succeeds by pooling both — and signing them proves the derivation paths /// were collected across both accounts.