From 672276e15a6a48705727e0bf4f7bd43ddafc55fb Mon Sep 17 00:00:00 2001 From: Borja Castellano Date: Wed, 4 Nov 2026 07:20:35 -0800 Subject: [PATCH 1/5] fix(key-wallet): reject an account xpub another account already uses --- key-wallet/src/wallet/accounts.rs | 7 ++++ key-wallet/src/wallet/mod.rs | 54 +++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/key-wallet/src/wallet/accounts.rs b/key-wallet/src/wallet/accounts.rs index dd6f55b17..11a7fbe20 100644 --- a/key-wallet/src/wallet/accounts.rs +++ b/key-wallet/src/wallet/accounts.rs @@ -62,6 +62,13 @@ impl Wallet { ))); } + if self.accounts.all_accounts().iter().any(|a| a.account_xpub == account.account_xpub) { + return Err(Error::InvalidParameter(format!( + "Extended public key of {:?} is already used by another account", + account_type + ))); + } + // Insert into the collection self.accounts.insert(account).map_err(|e| Error::InvalidParameter(e.to_string())) } diff --git a/key-wallet/src/wallet/mod.rs b/key-wallet/src/wallet/mod.rs index ec961e222..201bfc28d 100644 --- a/key-wallet/src/wallet/mod.rs +++ b/key-wallet/src/wallet/mod.rs @@ -243,6 +243,7 @@ mod tests { use super::*; use crate::account::account_collection::AccountCollection; use crate::account::{AccountType, StandardAccountType}; + use crate::Error; use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; @@ -568,6 +569,59 @@ mod tests { assert!(wallet.accounts.count() >= 2); } + #[test] + fn test_add_account_rejects_imported_xpub_in_use() { + let mut wallet = Wallet::new_random( + Network::Testnet, + initialization::WalletAccountCreationOptions::Default, + ) + .unwrap(); + let xpub = wallet.get_bip44_account(0).unwrap().account_xpub; + let count = wallet.accounts.count(); + + let result = wallet.add_account( + AccountType::Standard { + index: 5, + standard_account_type: StandardAccountType::BIP32Account, + }, + Some(xpub), + ); + + assert!(matches!(result, Err(Error::InvalidParameter(_)))); + assert_eq!(wallet.accounts.count(), count); + } + + #[test] + fn test_add_account_rejects_derived_xpub_in_use() { + let mut wallet = Wallet::new_random( + Network::Testnet, + initialization::WalletAccountCreationOptions::Default, + ) + .unwrap(); + let bip44 = AccountType::Standard { + index: 5, + standard_account_type: StandardAccountType::BIP44Account, + }; + let xpub = wallet + .derive_extended_public_key(&bip44.derivation_path(Network::Testnet).unwrap()) + .unwrap(); + wallet + .add_account( + AccountType::Standard { + index: 5, + standard_account_type: StandardAccountType::BIP32Account, + }, + Some(xpub), + ) + .unwrap(); + let count = wallet.accounts.count(); + + let result = wallet.add_account(bip44, None); + + assert!(matches!(result, Err(Error::InvalidParameter(_)))); + assert_eq!(wallet.accounts.count(), count); + } + // ✓ Test wallet ID generation #[test] fn test_wallet_id_generation() { From c22d51177f7a2801323f056a7efcf40f454a05e6 Mon Sep 17 00:00:00 2001 From: Borja Castellano Date: Thu, 5 Nov 2026 03:47:49 -0800 Subject: [PATCH 2/5] fix(key-wallet): detect duplicate account xpubs by public key and chain code --- key-wallet/src/wallet/accounts.rs | 6 +++++- key-wallet/src/wallet/mod.rs | 12 +++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/key-wallet/src/wallet/accounts.rs b/key-wallet/src/wallet/accounts.rs index 11a7fbe20..b6ed51bf2 100644 --- a/key-wallet/src/wallet/accounts.rs +++ b/key-wallet/src/wallet/accounts.rs @@ -62,7 +62,11 @@ impl Wallet { ))); } - if self.accounts.all_accounts().iter().any(|a| a.account_xpub == account.account_xpub) { + let xpub = &account.account_xpub; + if self.accounts.all_accounts().iter().any(|a| { + a.account_xpub.public_key == xpub.public_key + && a.account_xpub.chain_code == xpub.chain_code + }) { return Err(Error::InvalidParameter(format!( "Extended public key of {:?} is already used by another account", account_type diff --git a/key-wallet/src/wallet/mod.rs b/key-wallet/src/wallet/mod.rs index 201bfc28d..8a19235b9 100644 --- a/key-wallet/src/wallet/mod.rs +++ b/key-wallet/src/wallet/mod.rs @@ -243,7 +243,9 @@ mod tests { use super::*; use crate::account::account_collection::AccountCollection; use crate::account::{AccountType, StandardAccountType}; + use crate::bip32::{ChildNumber, ExtendedPubKey}; use crate::Error; + use test_case::test_case; use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; @@ -569,14 +571,18 @@ mod tests { assert!(wallet.accounts.count() >= 2); } - #[test] - fn test_add_account_rejects_imported_xpub_in_use() { + #[test_case(|_| {} ; "same xpub")] + #[test_case(|x| x.depth += 1 ; "different depth")] + #[test_case(|x| x.parent_fingerprint = Default::default() ; "different parent fingerprint")] + #[test_case(|x| x.child_number = ChildNumber::from(7) ; "different child number")] + fn test_add_account_rejects_imported_xpub_in_use(alter: fn(&mut ExtendedPubKey)) { let mut wallet = Wallet::new_random( Network::Testnet, initialization::WalletAccountCreationOptions::Default, ) .unwrap(); - let xpub = wallet.get_bip44_account(0).unwrap().account_xpub; + let mut xpub = wallet.get_bip44_account(0).unwrap().account_xpub; + alter(&mut xpub); let count = wallet.accounts.count(); let result = wallet.add_account( From ee198da479cd43c00a34c41728b4cca3b11c4408 Mon Sep 17 00:00:00 2001 From: Borja Castellano Date: Thu, 5 Nov 2026 04:04:27 -0800 Subject: [PATCH 3/5] test(key-wallet): reject the reverse external account sharing the receiving xpub --- key-wallet/src/tests/restore_utxos_tests.rs | 100 -------------------- key-wallet/src/wallet/mod.rs | 33 +++++++ 2 files changed, 33 insertions(+), 100 deletions(-) diff --git a/key-wallet/src/tests/restore_utxos_tests.rs b/key-wallet/src/tests/restore_utxos_tests.rs index 9450f34e6..946602808 100644 --- a/key-wallet/src/tests/restore_utxos_tests.rs +++ b/key-wallet/src/tests/restore_utxos_tests.rs @@ -4,12 +4,10 @@ use dashcore::{Address, BlockHash, OutPoint, Transaction}; use test_case::test_case; -use crate::account::AccountType; use crate::managed_account::address_pool::AddressPool; use crate::managed_account::managed_account_trait::ManagedAccountTrait; use crate::test_utils::TestWalletContext; use crate::transaction_checking::{BlockInfo, TransactionContext}; -use crate::wallet::managed_wallet_info::managed_account_operations::ManagedAccountOperations; use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use crate::{KeySource, ManagedAccountType, Utxo}; @@ -42,45 +40,6 @@ fn funded(address: &Address, input: u8, value: u64) -> (Transaction, Utxo) { (tx, utxo) } -/// Add the DashPay receiving account of a friendship and the reverse -/// external account a wallet holding both identities also has. Both derive -/// the same addresses. Returns their account types. -fn befriend_both_ways(ctx: &mut TestWalletContext) -> (AccountType, AccountType) { - let (us, them) = ([0xaa; 32], [0xbb; 32]); - let receiving = AccountType::DashpayReceivingFunds { - index: 0, - user_identity_id: us, - friend_identity_id: them, - }; - let reverse_external = AccountType::DashpayExternalAccount { - index: 0, - user_identity_id: them, - friend_identity_id: us, - }; - for contact in [receiving, reverse_external] { - ctx.wallet.add_account(contact, None).expect("contact account is addable"); - ctx.managed_wallet - .add_managed_account(&ctx.wallet, contact) - .expect("contact account is manageable"); - } - (receiving, reverse_external) -} - -fn receiving_pool(ctx: &TestWalletContext) -> &AddressPool { - let account = ctx.managed_wallet.accounts.dashpay_receival_accounts.values().next(); - account.expect("receiving account").managed_account_type().address_pools()[0] -} - -fn held_by_receiving(ctx: &TestWalletContext, outpoint: &OutPoint) -> bool { - let accounts = &ctx.managed_wallet.accounts.dashpay_receival_accounts; - accounts.values().any(|account| account.utxos.contains_key(outpoint)) -} - -fn held_by_contact(ctx: &TestWalletContext, outpoint: &OutPoint) -> bool { - let accounts = &ctx.managed_wallet.accounts.dashpay_external_accounts; - accounts.values().any(|account| account.utxos.contains_key(outpoint)) -} - fn held_by_any_funds_account(ctx: &TestWalletContext, outpoint: &OutPoint) -> bool { ctx.managed_wallet .accounts @@ -143,65 +102,6 @@ fn a_utxo_matching_no_account_comes_back(ours_but_unreachable: bool) { assert!(!held_by_any_funds_account(&ctx, &utxo.outpoint)); } -/// A wallet holding both identities of a friendship derives the same -/// addresses for its receiving chain and for the reverse external chain. A -/// UTXO at such an address restores into the receiving account, whose coins -/// are the wallet's. -#[test] -fn a_utxo_both_a_receiving_and_a_contact_account_hold_restores_into_the_receiving_one() { - let mut ctx = TestWalletContext::new_random(); - befriend_both_ways(&mut ctx); - let address = receiving_pool(&ctx).address_at_index(0).expect("first address"); - let external = ctx.managed_wallet.accounts.dashpay_external_accounts.values().next(); - assert!(external.expect("external account").contains_script_pub_key(&address.script_pubkey())); - let (_, utxo) = funded(&address, 0, 29_000); - - let unmatched = - ctx.managed_wallet.restore_utxos(&ctx.wallet, [utxo.clone()]).expect("restores"); - - assert!(unmatched.is_empty()); - assert!(held_by_receiving(&ctx, &utxo.outpoint)); - assert!(!held_by_contact(&ctx, &utxo.outpoint)); -} - -/// The contact account's pool, derived further, holds a script from the -/// start, while the receiving account reaches it only once the UTXO at the -/// end of its window marks that address used. The UTXO still restores into -/// the receiving account. -#[test] -fn a_receiving_pool_that_reaches_a_script_in_a_later_pass_claims_it() { - let mut ctx = TestWalletContext::new_random(); - let (receiving, reverse_external) = befriend_both_ways(&mut ctx); - let pool = receiving_pool(&ctx); - let last = pool.highest_generated.expect("an initial window"); - let far_index = last + pool.gap_limit; - let mut far_pool = pool.clone(); - far_pool - .generate_up_to(far_index, &ctx.wallet.key_source_for_account(receiving)) - .expect("derives"); - let edge = pool.address_at_index(last).expect("derived"); - let far = far_pool.address_at_index(far_index).expect("derived"); - let key_source = ctx.wallet.key_source_for_account(reverse_external); - let external = ctx.managed_wallet.accounts.dashpay_external_accounts.values_mut().next(); - let external_pools = external.expect("external account").managed_account_type_mut(); - let external_pool = external_pools.address_pools_mut().into_iter().next().expect("a pool"); - external_pool.generate_up_to(far_index, &key_source).expect("derives"); - assert!(external_pool.contains_address(&far)); - let (_, at_edge) = funded(&edge, 0, 10_000); - let (_, beyond) = funded(&far, 1, 20_000); - - let unmatched = ctx - .managed_wallet - .restore_utxos(&ctx.wallet, [beyond.clone(), at_edge.clone()]) - .expect("restores"); - - assert!(unmatched.is_empty()); - for utxo in [&at_edge, &beyond] { - assert!(held_by_receiving(&ctx, &utxo.outpoint)); - assert!(!held_by_contact(&ctx, &utxo.outpoint)); - } -} - /// A pool already marked used up to the end of its window, as restored pool /// state can leave it, is extended before matching, so a UTXO one gap past /// that address restores without a UTXO at the used address. diff --git a/key-wallet/src/wallet/mod.rs b/key-wallet/src/wallet/mod.rs index 8a19235b9..36996c150 100644 --- a/key-wallet/src/wallet/mod.rs +++ b/key-wallet/src/wallet/mod.rs @@ -628,6 +628,39 @@ mod tests { assert_eq!(wallet.accounts.count(), count); } + #[test] + fn test_add_account_rejects_reverse_external_sharing_receiving_xpub() { + let mut wallet = Wallet::new_random( + Network::Testnet, + initialization::WalletAccountCreationOptions::Default, + ) + .unwrap(); + let (us, them) = ([0xaa; 32], [0xbb; 32]); + wallet + .add_account( + AccountType::DashpayReceivingFunds { + index: 0, + user_identity_id: us, + friend_identity_id: them, + }, + None, + ) + .unwrap(); + let count = wallet.accounts.count(); + + let result = wallet.add_account( + AccountType::DashpayExternalAccount { + index: 0, + user_identity_id: them, + friend_identity_id: us, + }, + None, + ); + + assert!(matches!(result, Err(Error::InvalidParameter(_)))); + assert_eq!(wallet.accounts.count(), count); + } + // ✓ Test wallet ID generation #[test] fn test_wallet_id_generation() { From 957be3b94e776dfc99ed7795e709d082a60327b3 Mon Sep 17 00:00:00 2001 From: Borja Castellano Date: Mon, 9 Nov 2026 09:39:40 -0800 Subject: [PATCH 4/5] test(key-wallet): restore the overlapping contact account tests The pair of DashPay accounts sharing an xpub is still reachable through `AccountCollection::insert`, so the `restore_utxos` tests that cover it insert the accounts directly instead of through `add_account`. --- key-wallet/src/tests/restore_utxos_tests.rs | 104 ++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/key-wallet/src/tests/restore_utxos_tests.rs b/key-wallet/src/tests/restore_utxos_tests.rs index 946602808..46133cab8 100644 --- a/key-wallet/src/tests/restore_utxos_tests.rs +++ b/key-wallet/src/tests/restore_utxos_tests.rs @@ -4,10 +4,12 @@ use dashcore::{Address, BlockHash, OutPoint, Transaction}; use test_case::test_case; +use crate::account::{Account, AccountType}; use crate::managed_account::address_pool::AddressPool; use crate::managed_account::managed_account_trait::ManagedAccountTrait; use crate::test_utils::TestWalletContext; use crate::transaction_checking::{BlockInfo, TransactionContext}; +use crate::wallet::managed_wallet_info::managed_account_operations::ManagedAccountOperations; use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use crate::{KeySource, ManagedAccountType, Utxo}; @@ -40,6 +42,49 @@ fn funded(address: &Address, input: u8, value: u64) -> (Transaction, Utxo) { (tx, utxo) } +/// Add the DashPay receiving account of a friendship and the reverse +/// external account a wallet holding both identities also has. Both derive +/// the same addresses. Returns their account types. +fn befriend_both_ways(ctx: &mut TestWalletContext) -> (AccountType, AccountType) { + let (us, them) = ([0xaa; 32], [0xbb; 32]); + let receiving = AccountType::DashpayReceivingFunds { + index: 0, + user_identity_id: us, + friend_identity_id: them, + }; + let reverse_external = AccountType::DashpayExternalAccount { + index: 0, + user_identity_id: them, + friend_identity_id: us, + }; + for contact in [receiving, reverse_external] { + let path = contact.derivation_path(ctx.wallet.network).expect("contact path"); + let xpub = ctx.wallet.derive_extended_public_key(&path).expect("contact xpub"); + let account = Account::new(Some(ctx.wallet.wallet_id), contact, xpub, ctx.wallet.network) + .expect("contact account"); + ctx.wallet.accounts.insert(account).expect("contact account is insertable"); + ctx.managed_wallet + .add_managed_account(&ctx.wallet, contact) + .expect("contact account is manageable"); + } + (receiving, reverse_external) +} + +fn receiving_pool(ctx: &TestWalletContext) -> &AddressPool { + let account = ctx.managed_wallet.accounts.dashpay_receival_accounts.values().next(); + account.expect("receiving account").managed_account_type().address_pools()[0] +} + +fn held_by_receiving(ctx: &TestWalletContext, outpoint: &OutPoint) -> bool { + let accounts = &ctx.managed_wallet.accounts.dashpay_receival_accounts; + accounts.values().any(|account| account.utxos.contains_key(outpoint)) +} + +fn held_by_contact(ctx: &TestWalletContext, outpoint: &OutPoint) -> bool { + let accounts = &ctx.managed_wallet.accounts.dashpay_external_accounts; + accounts.values().any(|account| account.utxos.contains_key(outpoint)) +} + fn held_by_any_funds_account(ctx: &TestWalletContext, outpoint: &OutPoint) -> bool { ctx.managed_wallet .accounts @@ -102,6 +147,65 @@ fn a_utxo_matching_no_account_comes_back(ours_but_unreachable: bool) { assert!(!held_by_any_funds_account(&ctx, &utxo.outpoint)); } +/// A wallet holding both identities of a friendship derives the same +/// addresses for its receiving chain and for the reverse external chain. A +/// UTXO at such an address restores into the receiving account, whose coins +/// are the wallet's. +#[test] +fn a_utxo_both_a_receiving_and_a_contact_account_hold_restores_into_the_receiving_one() { + let mut ctx = TestWalletContext::new_random(); + befriend_both_ways(&mut ctx); + let address = receiving_pool(&ctx).address_at_index(0).expect("first address"); + let external = ctx.managed_wallet.accounts.dashpay_external_accounts.values().next(); + assert!(external.expect("external account").contains_script_pub_key(&address.script_pubkey())); + let (_, utxo) = funded(&address, 0, 29_000); + + let unmatched = + ctx.managed_wallet.restore_utxos(&ctx.wallet, [utxo.clone()]).expect("restores"); + + assert!(unmatched.is_empty()); + assert!(held_by_receiving(&ctx, &utxo.outpoint)); + assert!(!held_by_contact(&ctx, &utxo.outpoint)); +} + +/// The contact account's pool, derived further, holds a script from the +/// start, while the receiving account reaches it only once the UTXO at the +/// end of its window marks that address used. The UTXO still restores into +/// the receiving account. +#[test] +fn a_receiving_pool_that_reaches_a_script_in_a_later_pass_claims_it() { + let mut ctx = TestWalletContext::new_random(); + let (receiving, reverse_external) = befriend_both_ways(&mut ctx); + let pool = receiving_pool(&ctx); + let last = pool.highest_generated.expect("an initial window"); + let far_index = last + pool.gap_limit; + let mut far_pool = pool.clone(); + far_pool + .generate_up_to(far_index, &ctx.wallet.key_source_for_account(receiving)) + .expect("derives"); + let edge = pool.address_at_index(last).expect("derived"); + let far = far_pool.address_at_index(far_index).expect("derived"); + let key_source = ctx.wallet.key_source_for_account(reverse_external); + let external = ctx.managed_wallet.accounts.dashpay_external_accounts.values_mut().next(); + let external_pools = external.expect("external account").managed_account_type_mut(); + let external_pool = external_pools.address_pools_mut().into_iter().next().expect("a pool"); + external_pool.generate_up_to(far_index, &key_source).expect("derives"); + assert!(external_pool.contains_address(&far)); + let (_, at_edge) = funded(&edge, 0, 10_000); + let (_, beyond) = funded(&far, 1, 20_000); + + let unmatched = ctx + .managed_wallet + .restore_utxos(&ctx.wallet, [beyond.clone(), at_edge.clone()]) + .expect("restores"); + + assert!(unmatched.is_empty()); + for utxo in [&at_edge, &beyond] { + assert!(held_by_receiving(&ctx, &utxo.outpoint)); + assert!(!held_by_contact(&ctx, &utxo.outpoint)); + } +} + /// A pool already marked used up to the end of its window, as restored pool /// state can leave it, is extended before matching, so a UTXO one gap past /// that address restores without a UTXO at the used address. From 517ae392221333e1abbcef15a452559efb10a277 Mon Sep 17 00:00:00 2001 From: Borja Castellano Date: Thu, 12 Nov 2026 05:59:15 -0800 Subject: [PATCH 5/5] fix(key-wallet): let contact accounts share an xpub A wallet holding both identities of a friendship derives the same xpub for the receiving account and the reverse external account. The external account holds no funds, so the pair never counts a coin twice. Skip contact-owned accounts in the duplicate xpub check of `add_account`. --- key-wallet/src/tests/restore_utxos_tests.rs | 8 ++------ key-wallet/src/wallet/accounts.rs | 11 +++++++---- key-wallet/src/wallet/mod.rs | 6 +++--- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/key-wallet/src/tests/restore_utxos_tests.rs b/key-wallet/src/tests/restore_utxos_tests.rs index 46133cab8..9450f34e6 100644 --- a/key-wallet/src/tests/restore_utxos_tests.rs +++ b/key-wallet/src/tests/restore_utxos_tests.rs @@ -4,7 +4,7 @@ use dashcore::{Address, BlockHash, OutPoint, Transaction}; use test_case::test_case; -use crate::account::{Account, AccountType}; +use crate::account::AccountType; use crate::managed_account::address_pool::AddressPool; use crate::managed_account::managed_account_trait::ManagedAccountTrait; use crate::test_utils::TestWalletContext; @@ -58,11 +58,7 @@ fn befriend_both_ways(ctx: &mut TestWalletContext) -> (AccountType, AccountType) friend_identity_id: us, }; for contact in [receiving, reverse_external] { - let path = contact.derivation_path(ctx.wallet.network).expect("contact path"); - let xpub = ctx.wallet.derive_extended_public_key(&path).expect("contact xpub"); - let account = Account::new(Some(ctx.wallet.wallet_id), contact, xpub, ctx.wallet.network) - .expect("contact account"); - ctx.wallet.accounts.insert(account).expect("contact account is insertable"); + ctx.wallet.add_account(contact, None).expect("contact account is addable"); ctx.managed_wallet .add_managed_account(&ctx.wallet, contact) .expect("contact account is manageable"); diff --git a/key-wallet/src/wallet/accounts.rs b/key-wallet/src/wallet/accounts.rs index b6ed51bf2..a10aa3f1a 100644 --- a/key-wallet/src/wallet/accounts.rs +++ b/key-wallet/src/wallet/accounts.rs @@ -63,10 +63,13 @@ impl Wallet { } let xpub = &account.account_xpub; - if self.accounts.all_accounts().iter().any(|a| { - a.account_xpub.public_key == xpub.public_key - && a.account_xpub.chain_code == xpub.chain_code - }) { + if !account_type.is_contact_owned() + && self.accounts.all_accounts().iter().any(|a| { + !a.account_type.is_contact_owned() + && a.account_xpub.public_key == xpub.public_key + && a.account_xpub.chain_code == xpub.chain_code + }) + { return Err(Error::InvalidParameter(format!( "Extended public key of {:?} is already used by another account", account_type diff --git a/key-wallet/src/wallet/mod.rs b/key-wallet/src/wallet/mod.rs index 36996c150..39e18fd8b 100644 --- a/key-wallet/src/wallet/mod.rs +++ b/key-wallet/src/wallet/mod.rs @@ -629,7 +629,7 @@ mod tests { } #[test] - fn test_add_account_rejects_reverse_external_sharing_receiving_xpub() { + fn test_add_account_accepts_reverse_external_sharing_receiving_xpub() { let mut wallet = Wallet::new_random( Network::Testnet, initialization::WalletAccountCreationOptions::Default, @@ -657,8 +657,8 @@ mod tests { None, ); - assert!(matches!(result, Err(Error::InvalidParameter(_)))); - assert_eq!(wallet.accounts.count(), count); + assert!(result.is_ok()); + assert_eq!(wallet.accounts.count(), count + 1); } // ✓ Test wallet ID generation