From 33fa5b842585abda838fa3e601f04aafb2ab6861 Mon Sep 17 00:00:00 2001 From: bfoss765 Date: Sun, 9 Aug 2026 15:30:17 -0400 Subject: [PATCH 1/8] feat(key-wallet): fund an asset lock from a caller-chosen list of accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An asset lock could only ever be funded from ONE account. A wallet holding its balance across the standard families and its DashPay contact-receiving accounts had to sweep them into BIP44 first and lock out of that — an extra on-chain hop, an extra fee, and a transparent address reused for the privilege. The send path stopped needing that in dashpay/platform#4329; this is the same change for asset locks. Both builders now take a LIST of `AccountTypePreference` sources plus a `source_index` instead of a single `AssetLockFundingAccount`, and fold them through the same `transaction_building::fund` the send path uses: coin selection draws from the union, the first source supplies the change address, overlapping sources fund each account once (#931's dedup is what makes the repeated `add_funding` safe), and derivation paths are collected across every contributing account so the inputs can be signed. Which accounts to pool is the CALLER's decision, not this library's. The FFI entry point takes the source list as a parameter (`FFIAccountTypePreference`, a tag plus the two identity IDs the DashPay kinds read) rather than applying a default of its own: a client wanting today's behavior passes a single `BIP44` source, one wanting the whole spendable balance passes the wider list, and one funding out of a specific contact names that friendship. An empty list is rejected instead of falling through to `AccountTypePreference::DEFAULT`, since defaulting there would be this layer choosing a funding policy that only the client library knows. Reservation bookkeeping is the part that had to change shape. A pooled build reserves in EACH contributing account's own set under the one owner token, so the post-build failure paths — credit-key derivation on the soft-wallet builder, the peek/sign/commit loop on the signer builder, both running after the transaction is already signed — now release across every funded account instead of just the one. Releasing a single account's set would have stranded the rest of the inputs until the 24-block TTL sweep. `AssetLockResult` carries the contributing accounts so the caller's rejected-broadcast release can reach them all; it is the contributor list, not everything the sources offered, so a wallet's address book does not inflate the caller's bookkeeping. `fund`'s strictness rule now matches platform's: a SINGLE named source is strict (a caller asking for exactly one account's funds must not silently be given another's), while a pooled list skips the sources this wallet has nothing for — no BIP32 account, no contacts — and errors only when none of them funds anything. Without that, a pooled set would fail on the very wallets it is meant to serve. CoinJoin funding is unchanged and stays excluded from pooling: it remains drain-only, and it must now be the sole source, because spending mixed outputs alongside transparent ones in one transaction links them and undoes the mixing. The `AssetLockFundingAccount::CoinJoin` + `drain: true` flow that dashpay/platform#4327 ships on converts to a single-element source list and behaves exactly as before. `AssetLockError::AccountNotFound(u32)` is removed — account resolution is now the builder's, and it reports `BuilderError::AccountNotFound` with the source that failed. `AssetLockFundingAccount` remains as the drain flows' single-account vocabulary, with a `From` conversion into the source list. Co-authored-by: Claude Opus 5 --- key-wallet-ffi/FFI_API.md | 6 +- key-wallet-ffi/src/transaction.rs | 166 +++- .../managed_wallet_info/asset_lock_builder.rs | 742 +++++++++++++----- .../transaction_building.rs | 117 ++- 4 files changed, 833 insertions(+), 198 deletions(-) diff --git a/key-wallet-ffi/FFI_API.md b/key-wallet-ffi/FFI_API.md index 566d3aa19..206428197 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. - `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 - 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 - 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..4bce39761 100644 --- a/key-wallet-ffi/src/transaction.rs +++ b/key-wallet-ffi/src/transaction.rs @@ -744,14 +744,96 @@ 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. + 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. +/// - `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 +845,16 @@ 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 +/// - 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 +871,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 +887,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 +904,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 +939,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 +976,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/src/wallet/managed_wallet_info/asset_lock_builder.rs b/key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs index 6aca94b34..4c5f0dc60 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,16 +9,22 @@ 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_building::{ + AccountTypePreference, PooledFunding, +}; use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use crate::wallet::managed_wallet_info::ManagedWalletInfo; use crate::wallet::Wallet; use crate::DerivationPath; +use std::collections::HashSet; /// Which funding account to derive the one-time key from. #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -38,14 +44,19 @@ 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). +/// +/// Convert with [`AccountTypePreference::from`] to hand one to a builder. #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum AssetLockFundingAccount { @@ -75,6 +86,19 @@ impl AssetLockFundingAccount { } } +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 +135,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 +179,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 +198,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 +251,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,14 +270,46 @@ 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 { + let spent: HashSet = + transaction.input.iter().map(|input| input.previous_output).collect(); + offered + .iter() + .copied() + .filter(|account_type| { + accounts.funds_account(account_type).is_some_and(|account| { + account.utxos.keys().any(|outpoint| spent.contains(outpoint)) + }) + }) + .collect() +} + impl ManagedWalletInfo { /// Build and sign an asset lock transaction. /// @@ -249,16 +319,26 @@ 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 + /// cannot be pooled — see [`validate_funding_sources`]. 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, @@ -271,38 +351,7 @@ impl ManagedWalletInfo { 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)?; + validate_funding_sources(funding_sources, credit_output_fundings.len(), drain)?; let credit_outputs: Vec = credit_output_fundings.iter().map(|f| f.output.clone()).collect(); @@ -314,27 +363,46 @@ impl ManagedWalletInfo { .set_current_height(height) .set_special_payload(TransactionPayload::AssetLockPayloadType(AssetLockPayload::new( credit_outputs, - ))); + ))) + .require_final_inputs(); 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)) - .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 PooledFunding { + builder, + paths, + accounts: offered, + } = self.fund(wallet, funding_sources, source_index, builder)?; + + // The build below reserves the funding inputs in each contributing + // account's own set. Clone every offered account's reservation handle + // (a shared `Arc` view of the same set) now, before the loop further + // down re-borrows `self.accounts`: a mid-loop failure can no longer + // reach those accounts to release, and the caller never received the + // token to release with either, so a leaked reservation would strand + // the already-signed inputs until the 24-block TTL sweep. Offered + // rather than contributing, because the set is captured before the + // build tells us who contributed; releasing against an account that + // reserved nothing is a no-op. Owner-guarded release only (see + // `ReservationSet::release_if_owner`, `dashpay/platform#4185`). + let reservations: Vec = offered + .iter() + .filter_map(|account_type| self.accounts.funds_account(account_type)) + .map(|account| account.reservations().clone()) + .collect(); + + let (transaction, fee, reservation_token) = + builder.build_signed_reserved(wallet, move |addr| paths.get(&addr).cloned()).await?; + let reserved: Vec = transaction.input.iter().map(|input| input.previous_output).collect(); + let release_reservations = || { + if let Some(token) = reservation_token { + for set in &reservations { + set.release_if_owner(&reserved, token); + } + } + }; // Derive one private key per credit output. On any failure, release // this build's own reservation before returning. @@ -355,18 +423,18 @@ impl ManagedWalletInfo { })() { Ok(keys) => keys, Err(e) => { - if let Some(token) = reservation_token { - reservations.release_if_owner(&reserved, token); - } + release_reservations(); return Err(e); } }; + let funding_accounts = contributing_accounts(&self.accounts, &offered, &transaction); Ok(AssetLockResult { transaction, fee, keys: AssetLockCreditKeys::Private(keys), reservation_token, + funding_accounts, }) } @@ -386,11 +454,14 @@ 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, @@ -398,40 +469,7 @@ impl ManagedWalletInfo { ) -> 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)?; + validate_funding_sources(funding_sources, credit_output_fundings.len(), drain)?; let credit_outputs: Vec = credit_output_fundings.iter().map(|f| f.output.clone()).collect(); @@ -441,26 +479,43 @@ impl ManagedWalletInfo { .set_current_height(height) .set_special_payload(TransactionPayload::AssetLockPayloadType(AssetLockPayload::new( credit_outputs, - ))); + ))) + .require_final_inputs(); 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)) - .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 + let PooledFunding { + builder, + paths, + accounts: offered, + } = self.fund(wallet, funding_sources, source_index, builder)?; + + // The build below reserves the funding inputs in each contributing + // account's own set. Clone every offered account's reservation handle + // (a shared `Arc` view of the same set) before the bookkeeping loop + // further down 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 reservations: Vec = offered + .iter() + .filter_map(|account_type| self.accounts.funds_account(account_type)) + .map(|account| account.reservations().clone()) + .collect(); + + let (transaction, fee, reservation_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(); + let release_reservations = || { + if let Some(token) = reservation_token { + for set in &reservations { + set.release_if_owner(&reserved, token); + } + } + }; // Credit-output bookkeeping: for each funding, peek the next unused // path on its account, ask the signer for the matching pubkey, and @@ -516,18 +571,18 @@ impl ManagedWalletInfo { { Ok(keys) => keys, Err(e) => { - if let Some(token) = reservation_token { - reservations.release_if_owner(&reserved, token); - } + release_reservations(); return Err(e); } }; + let funding_accounts = contributing_accounts(&self.accounts, &offered, &transaction); Ok(AssetLockResult { transaction, fee, keys: AssetLockCreditKeys::Public(credit_output_keys), reservation_token, + funding_accounts, }) } } @@ -540,6 +595,7 @@ mod tests { use crate::{Network, Utxo}; use dashcore::{OutPoint, ScriptBuf, Txid}; use dashcore_hashes::Hash; + use test_case::test_case; fn test_credit_outputs(amounts: &[u64]) -> Vec { amounts @@ -659,9 +715,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 +756,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 +782,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 +804,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 +820,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 +831,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 +851,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 +878,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 +904,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 +1030,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 +1058,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 +1100,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 +1139,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 +1200,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 +1214,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. From 13fcb57be235672a5e572cf3fe36ab115dbdde82 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 11 Aug 2026 00:29:17 +0700 Subject: [PATCH 2/8] docs(key-wallet): drop the private intra-doc link from build_asset_lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `validate_funding_sources` is a private free function, so linking to it from a public doc comment fails `rustdoc::private_intra_doc_links` under the Documentation job's `-D warnings`. The link was also useless to a reader of the public docs, who cannot follow it — state the CoinJoin rule inline instead. Co-authored-by: Claude Opus 5 --- .../src/wallet/managed_wallet_info/asset_lock_builder.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 4c5f0dc60..a2f618c7d 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 @@ -332,8 +332,9 @@ impl ManagedWalletInfo { /// `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 - /// cannot be pooled — see [`validate_funding_sources`]. + /// 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, From f0d2cbf8cbfd52244e4798237c2dbb98444d2155 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 11 Aug 2026 00:41:07 +0700 Subject: [PATCH 3/8] docs(key-wallet-ffi): flag CoinJoin as unusable on the non-drain asset lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `wallet_build_and_sign_asset_lock_transaction` always builds with `drain: false`, and `validate_funding_sources` rejects a CoinJoin source outside drain mode — so a caller selecting that kind here always gets `InvalidData`, even passing it alone. The kind's own doc described the builder's rule ("sole source, drain only") without saying this entry point never drains, which reads as though the sole-source form would work. Raised by CodeRabbit on #944. Co-authored-by: Claude Opus 5 --- key-wallet-ffi/FFI_API.md | 2 +- key-wallet-ffi/src/transaction.rs | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/key-wallet-ffi/FFI_API.md b/key-wallet-ffi/FFI_API.md index 206428197..8d0538c09 100644 --- a/key-wallet-ffi/FFI_API.md +++ b/key-wallet-ffi/FFI_API.md @@ -1309,7 +1309,7 @@ wallet_build_and_sign_asset_lock_transaction(manager: *const FFIWalletManager, w ``` **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. 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. - `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 - 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` +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 - 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 - `funding_sources` must have at least `funding_sources_count` elements - 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` diff --git a/key-wallet-ffi/src/transaction.rs b/key-wallet-ffi/src/transaction.rs index 4bce39761..890d1fa40 100644 --- a/key-wallet-ffi/src/transaction.rs +++ b/key-wallet-ffi/src/transaction.rs @@ -758,6 +758,9 @@ pub enum FFIAccountTypePreferenceKind { /// 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, @@ -830,7 +833,9 @@ impl From for AccountTypePreference { /// 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. +/// 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. From 5a19149f4e14884d6a8d6e988900212adbf0ea01 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 11 Aug 2026 01:02:54 +0700 Subject: [PATCH 4/8] refactor(key-wallet): share the asset-lock funding and reservation prologue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `build_asset_lock` and `build_asset_lock_with_signer` had drifted into two near-identical copies of the same prologue: payload assembly, the drain strategy switch, `fund`, the per-account `ReservationSet` capture, and the release closure. They differed only in which signer reached `build_signed_reserved` and in comment wording. That duplication is a hazard on this specific logic. The reservation capture has to happen between funding and signing — a pooled build reserves in each contributing account's own set, and both builders reach their release paths after the transaction is already signed, with the caller holding no token. A fix applied to one copy and not the other silently reintroduces stranded inputs on the path that was missed. `build_signed_asset_lock` is now the single prologue, generic over `TransactionSigner` so the soft-wallet builder passes `wallet` and the signer builder passes `signer`. `BuildReservations` owns the sets, the reserved outpoints and the token, so releasing is one call that cannot reach only some of the funded accounts. Also from review: - `contributing_accounts` iterates the transaction's inputs rather than every UTXO of every offered account. A transaction has few inputs; an offered account can hold many UTXOs. - `From for AccountTypePreference` documents that the index does not survive. The type's own doc said "convert to hand one to a builder", and a caller doing only that funds source_index 0 rather than the account it named. - The FFI entry point documents that `funding_sources[i].kind` must be a declared discriminant, since reading any other value as the enum is UB and so cannot be rejected at the boundary. Raised by CodeRabbit on #944. Co-authored-by: Claude Opus 5 --- key-wallet-ffi/src/transaction.rs | 4 + .../managed_wallet_info/asset_lock_builder.rs | 266 +++++++++++------- 2 files changed, 166 insertions(+), 104 deletions(-) diff --git a/key-wallet-ffi/src/transaction.rs b/key-wallet-ffi/src/transaction.rs index 890d1fa40..b74a81479 100644 --- a/key-wallet-ffi/src/transaction.rs +++ b/key-wallet-ffi/src/transaction.rs @@ -851,6 +851,10 @@ impl From for AccountTypePreference { /// /// - 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` 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 a2f618c7d..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 @@ -16,7 +16,9 @@ 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, }; @@ -24,7 +26,6 @@ use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterfa use crate::wallet::managed_wallet_info::ManagedWalletInfo; use crate::wallet::Wallet; use crate::DerivationPath; -use std::collections::HashSet; /// Which funding account to derive the one-time key from. #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -56,7 +57,9 @@ pub enum AssetLockFundingType { /// transparent BIP44 address (which would link the mixed UTXOs to a reusable /// transparent address for an extra hop). /// -/// Convert with [`AccountTypePreference::from`] to hand one to a builder. +/// 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 { @@ -86,6 +89,11 @@ 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 { @@ -297,20 +305,133 @@ fn contributing_accounts( offered: &[AccountType], transaction: &Transaction, ) -> Vec { - let spent: HashSet = - transaction.input.iter().map(|input| input.previous_output).collect(); + // 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| { - account.utxos.keys().any(|outpoint| spent.contains(outpoint)) + 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 @@ -350,60 +471,26 @@ impl ManagedWalletInfo { wallet.root_extended_priv_key().map_err(|_| AssetLockError::WatchOnlyWallet)?.clone(); let network = self.network; - let height = self.last_processed_height(); - - validate_funding_sources(funding_sources, 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, - ))) - .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)?; - - // The build below reserves the funding inputs in each contributing - // account's own set. Clone every offered account's reservation handle - // (a shared `Arc` view of the same set) now, before the loop further - // down re-borrows `self.accounts`: a mid-loop failure can no longer - // reach those accounts to release, and the caller never received the - // token to release with either, so a leaked reservation would strand - // the already-signed inputs until the 24-block TTL sweep. Offered - // rather than contributing, because the set is captured before the - // build tells us who contributed; releasing against an account that - // reserved nothing is a no-op. Owner-guarded release only (see - // `ReservationSet::release_if_owner`, `dashpay/platform#4185`). - let reservations: Vec = offered - .iter() - .filter_map(|account_type| self.accounts.funds_account(account_type)) - .map(|account| account.reservations().clone()) - .collect(); - - let (transaction, fee, reservation_token) = - builder.build_signed_reserved(wallet, move |addr| paths.get(&addr).cloned()).await?; - - let reserved: Vec = - transaction.input.iter().map(|input| input.previous_output).collect(); - let release_reservations = || { - if let Some(token) = reservation_token { - for set in &reservations { - set.release_if_owner(&reserved, token); - } - } - }; + fee_per_kb, + drain, + wallet, + ) + .await?; // Derive one private key per credit output. On any failure, release // this build's own reservation before returning. @@ -424,7 +511,7 @@ impl ManagedWalletInfo { })() { Ok(keys) => keys, Err(e) => { - release_reservations(); + reservations.release(); return Err(e); } }; @@ -434,7 +521,7 @@ impl ManagedWalletInfo { transaction, fee, keys: AssetLockCreditKeys::Private(keys), - reservation_token, + reservation_token: reservations.token, funding_accounts, }) } @@ -468,55 +555,25 @@ impl ManagedWalletInfo { drain: bool, signer: &S, ) -> Result { - let height = self.last_processed_height(); - - validate_funding_sources(funding_sources, 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, - ))) - .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)?; - - // The build below reserves the funding inputs in each contributing - // account's own set. Clone every offered account's reservation handle - // (a shared `Arc` view of the same set) before the bookkeeping loop - // further down 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: Vec = offered - .iter() - .filter_map(|account_type| self.accounts.funds_account(account_type)) - .map(|account| account.reservations().clone()) - .collect(); - - let (transaction, fee, reservation_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(); - let release_reservations = || { - if let Some(token) = reservation_token { - for set in &reservations { - set.release_if_owner(&reserved, token); - } - } - }; + fee_per_kb, + drain, + signer, + ) + .await?; // Credit-output bookkeeping: for each funding, peek the next unused // path on its account, ask the signer for the matching pubkey, and @@ -572,7 +629,7 @@ impl ManagedWalletInfo { { Ok(keys) => keys, Err(e) => { - release_reservations(); + reservations.release(); return Err(e); } }; @@ -582,7 +639,7 @@ impl ManagedWalletInfo { transaction, fee, keys: AssetLockCreditKeys::Public(credit_output_keys), - reservation_token, + reservation_token: reservations.token, funding_accounts, }) } @@ -596,6 +653,7 @@ 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 { From c1b7edea7e67df56fed3da3a334a3d6a97d83829 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 11 Aug 2026 01:05:48 +0700 Subject: [PATCH 5/8] test(key-wallet-ffi): cover the caller-supplied asset-lock funding sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new boundary had no test of its own: the entry point's marshalling body is where the funding policy now lives, and nothing exercised it. Three cases, driven against a created-but-unfunded wallet so an accepted call fails in coin selection rather than at the guards — which is what tells the two apart: - an empty list is rejected with `InvalidInput` instead of being forwarded, where it would mean `AccountTypePreference::DEFAULT` and reinstate a policy this layer must not choose; - a well-formed pooled list gets past the guards into the build; - `CoinJoin` is rejected, pinning the behavior documented in f0d2cbf8 — this entry point always builds non-drain, so mixed funds can never back it. Prompted by the patch-coverage report on #944. Co-authored-by: Claude Opus 5 --- .../tests/test_asset_lock_funding_sources.rs | 161 ++++++++++++++++++ 1 file changed, 161 insertions(+) create mode 100644 key-wallet-ffi/tests/test_asset_lock_funding_sources.rs 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..00871ead7 --- /dev/null +++ b/key-wallet-ffi/tests/test_asset_lock_funding_sources.rs @@ -0,0 +1,161 @@ +//! 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::{ + wallet_build_and_sign_asset_lock_transaction, FFIAccountTypePreference, + FFIAccountTypePreferenceKind, FFIAssetLockFundingType, +}; +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], + } +} + +/// 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(sources: &[FFIAccountTypePreference]) -> FFIError { + let mut error = FFIError::default(); + + let manager = wallet_manager_create(FFINetwork::Testnet, &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"); + + 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() { + unsafe { + let error = call_with_sources(&[]); + assert_eq!(error.code, FFIErrorCode::InvalidInput); + let message = error_message(&error); + assert!( + message.contains("funding source"), + "the error must name the missing argument, 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() { + unsafe { + let error = call_with_sources(&[ + source(FFIAccountTypePreferenceKind::BIP44), + source(FFIAccountTypePreferenceKind::BIP32), + ]); + assert_ne!( + error.code, + FFIErrorCode::InvalidInput, + "a pooled list is well-formed; 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() { + unsafe { + let error = call_with_sources(&[source(FFIAccountTypePreferenceKind::CoinJoin)]); + let message = error_message(&error); + assert!( + message.contains("drain"), + "the error must explain that CoinJoin is drain-only here, got: {message}" + ); + } +} From f07f8d5a7b7caa427df01b088434ebb8fa5a813d Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 11 Aug 2026 01:18:25 +0700 Subject: [PATCH 6/8] docs(key-wallet-ffi): regenerate FFI_API.md for the discriminant safety note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `# Safety` requirement added in 5a19149f changed the doc comment that `scripts/generate_ffi_docs.py` extracts, and the generated file was not refreshed alongside it — which the verify-ffi pre-commit hook catches. Co-authored-by: Claude Opus 5 --- key-wallet-ffi/FFI_API.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/key-wallet-ffi/FFI_API.md b/key-wallet-ffi/FFI_API.md index 8d0538c09..a9870ae9f 100644 --- a/key-wallet-ffi/FFI_API.md +++ b/key-wallet-ffi/FFI_API.md @@ -1309,10 +1309,10 @@ wallet_build_and_sign_asset_lock_transaction(manager: *const FFIWalletManager, w ``` **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. 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 - 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` +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 - `funding_sources` must have at least `funding_sources_count` elements - 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` +- 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` From 6ab303ead80d4cdd5410abb0134e5f5224650e8d Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 11 Aug 2026 01:35:25 +0700 Subject: [PATCH 7/8] test(key-wallet-ffi): free the wallet handle the asset-lock test borrows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `wallet_manager_get_wallet` returns an independently boxed clone that the caller owns — its own docs say to free it with `wallet_free_const` — and the test dropped it, leaking a whole `Wallet` per case. The Address Sanitizer job caught it: 42144 bytes in 24 allocations, three tests' worth. Also frees `tx_bytes` defensively. Every case here fails before a transaction is produced, so it is always null today, but a future success case would leak it the same way. Verified by reproducing the exact CI figure locally under `-Zsanitizer=address` with `detect_leaks=1`, then confirming it goes away. Co-authored-by: Claude Opus 5 --- key-wallet-ffi/tests/test_asset_lock_funding_sources.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/key-wallet-ffi/tests/test_asset_lock_funding_sources.rs b/key-wallet-ffi/tests/test_asset_lock_funding_sources.rs index 00871ead7..67455b7f2 100644 --- a/key-wallet-ffi/tests/test_asset_lock_funding_sources.rs +++ b/key-wallet-ffi/tests/test_asset_lock_funding_sources.rs @@ -10,9 +10,10 @@ use dash_network::ffi::FFINetwork; use key_wallet_ffi::error::{FFIError, FFIErrorCode}; use key_wallet_ffi::transaction::{ - wallet_build_and_sign_asset_lock_transaction, FFIAccountTypePreference, + 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, @@ -106,6 +107,12 @@ unsafe fn call_with_sources(sources: &[FFIAccountTypePreference]) -> FFIError { ); 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); From c88f5d119b9d9eb22ba0e0f44f0a68f552e3b657 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 11 Aug 2026 01:48:06 +0700 Subject: [PATCH 8/8] test(key-wallet-ffi): run the asset-lock source cases on both networks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The helper hardcoded Testnet, so every FFI contract case here skipped Mainnet — against the repo's standing rule to test both configurations. Account derivation is coin-type-scoped, so a guard exercised on one network only could hide a network-conditional path. `call_with_sources` now takes the network and each case loops over both, naming the network in its assertion messages so a one-sided failure says which. Raised by CodeRabbit on #944. Co-authored-by: Claude Opus 5 --- .../tests/test_asset_lock_funding_sources.rs | 71 ++++++++++++------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/key-wallet-ffi/tests/test_asset_lock_funding_sources.rs b/key-wallet-ffi/tests/test_asset_lock_funding_sources.rs index 67455b7f2..c4b4ae0f9 100644 --- a/key-wallet-ffi/tests/test_asset_lock_funding_sources.rs +++ b/key-wallet-ffi/tests/test_asset_lock_funding_sources.rs @@ -42,16 +42,21 @@ fn source(kind: FFIAccountTypePreferenceKind) -> FFIAccountTypePreference { } } +/// 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(sources: &[FFIAccountTypePreference]) -> FFIError { +unsafe fn call_with_sources(network: FFINetwork, sources: &[FFIAccountTypePreference]) -> FFIError { let mut error = FFIError::default(); - let manager = wallet_manager_create(FFINetwork::Testnet, &mut error); + let manager = wallet_manager_create(network, &mut error); assert!(!manager.is_null()); let mnemonic = CString::new(TEST_MNEMONIC).unwrap(); @@ -124,14 +129,16 @@ unsafe fn call_with_sources(sources: &[FFIAccountTypePreference]) -> FFIError { /// funding policy this layer is not entitled to pick. #[test] fn an_empty_source_list_is_rejected() { - unsafe { - let error = call_with_sources(&[]); - assert_eq!(error.code, FFIErrorCode::InvalidInput); - let message = error_message(&error); - assert!( - message.contains("funding source"), - "the error must name the missing argument, got: {message}" - ); + 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}" + ); + } } } @@ -139,16 +146,22 @@ fn an_empty_source_list_is_rejected() { /// failure comes from the empty wallet rather than from argument validation. #[test] fn a_named_source_reaches_the_builder() { - unsafe { - let error = call_with_sources(&[ - source(FFIAccountTypePreferenceKind::BIP44), - source(FFIAccountTypePreferenceKind::BIP32), - ]); - assert_ne!( - error.code, - FFIErrorCode::InvalidInput, - "a pooled list is well-formed; the build must fail on funds, not on arguments" - ); + 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" + ); + } } } @@ -157,12 +170,16 @@ fn a_named_source_reaches_the_builder() { /// must get told, not silently handed transparent funds instead. #[test] fn coinjoin_is_rejected_on_the_non_drain_entry_point() { - unsafe { - let error = call_with_sources(&[source(FFIAccountTypePreferenceKind::CoinJoin)]); - let message = error_message(&error); - assert!( - message.contains("drain"), - "the error must explain that CoinJoin is drain-only here, got: {message}" - ); + 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}" + ); + } } }