Skip to content

feat(key-wallet): pool BIP44 + BIP32 + DashPay receiving funds on the asset-lock path #935

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Closed
bfoss765 wants to merge 1 commit into from
Closed

feat(key-wallet): pool BIP44 + BIP32 + DashPay receiving funds on the asset-lock path #935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions key-wallet-ffi/src/transaction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,12 +744,33 @@ impl From<FFIAssetLockFundingType> for AssetLockFundingType {
}
}

/// The funding sources an asset lock pools, in order — the FIRST source
/// (BIP44) supplies the change address, so change from a pooled asset lock
/// always returns to the transparent primary account.
///
/// CoinJoin is deliberately absent (spending mixed outputs alongside
/// transparent ones links them and undoes the mixing — the same reasoning as
/// `AccountTypePreference::DEFAULT`), and so are a contact's watch-only
/// `DashpayExternalAccount` coins, which `AllDashpayReceivingFunds` excludes by
/// construction (it selects only the receiving side the local seed can sign).
const ASSET_LOCK_FUNDING_SOURCES: [AccountTypePreference; 3] = [
AccountTypePreference::BIP44,
AccountTypePreference::BIP32,
AccountTypePreference::AllDashpayReceivingFunds,
];
Comment on lines +747 to +760

@coderabbitai coderabbitai Bot Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

The C ABI is unchanged, but the funding behavior changes silently for every existing host.

wallet_build_and_sign_asset_lock_transaction keeps its signature, so a host cannot opt out of the new pooled set. Two consequences follow:

  1. An asset lock can now spend DashPay contact-receiving coins. That links funds received from a contact to the identity funding transaction. A host that previously relied on BIP44-only funding gets this new linkage without any code change.
  2. A host that already swept funds into BIP44 before locking sees no break, but a host that intentionally kept BIP32 or contact funds separate does.

Consider adding a funding-source parameter, or a second entry point that keeps the BIP44-only behavior, so hosts can choose. At minimum, record the behavior change in the crate's changelog and in the header documentation that host developers read.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@key-wallet-ffi/src/transaction.rs` around lines 747 - 760, Preserve host
control over asset-lock funding by adding an explicit funding-source option to
wallet_build_and_sign_asset_lock_transaction or providing a separate entry point
that retains BIP44-only behavior, while keeping the existing behavior available
to callers. Update the crate changelog and the public header documentation to
clearly describe the new pooled funding behavior and its linkage implications.


/// 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 [`ASSET_LOCK_FUNDING_SOURCES`]: coin selection
/// draws from the union of the BIP44, BIP32 and DashPay contact-receiving
/// accounts the wallet has, so a lock no longer needs the whole amount sitting
/// in one account (and change returns to BIP44). `account_index` addresses the
/// standard families; DashPay accounts span their own indices.
///
/// # Parameters
///
/// - `funding_types`: Array of `credit_outputs_count` funding account types,
Expand Down Expand Up @@ -840,9 +861,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,
},
&ASSET_LOCK_FUNDING_SOURCES,
account_index,
fundings,
fee_per_kb,
false,
Expand Down
Loading
Loading