Skip to content

refactor(key-wallet): wire ManagedCoreKeysAccount into the collection #716

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

Draft
QuantumExplorer wants to merge 11 commits into
base: v0.42-dev
Choose a base branch
from
Draft

refactor(key-wallet): wire ManagedCoreKeysAccount into the collection #716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7896c7f
feat(key-wallet): add keep_txs_in_memory toggle on ManagedCoreAccount
QuantumExplorer May 3, 2026
0282498
refactor(key-wallet): make keep_txs_in_memory a Cargo feature
QuantumExplorer May 3, 2026
0e6da68
chore(key-wallet): drop keep_txs_in_memory from default features
QuantumExplorer May 3, 2026
8d4cfa7
fix(key-wallet-manager): require keep_txs_in_memory; cargo fmt
QuantumExplorer May 3, 2026
2d38624
fix(key-wallet): keep dedup working when keep_txs_in_memory is off
QuantumExplorer May 3, 2026
b662295
refactor(key-wallet): split ManagedCoreAccount into funds + keys vari…
QuantumExplorer May 3, 2026
2f1ea9e
fix(key-wallet): rustfmt + qualify intra-doc link to ManagedCoreFunds…
QuantumExplorer May 3, 2026
2b9fbad
docs(key-wallet): fix broken receive-address example in IMPLEMENTATIO…
QuantumExplorer May 3, 2026
d427400
Merge branch 'claude/adoring-booth-151626' into v0.42-platform-nightly
QuantumExplorer May 3, 2026
4431a67
Merge branch 'claude/gracious-dirac-44a53b' into v0.42-platform-nightly
QuantumExplorer May 3, 2026
92d7568
refactor(key-wallet): wire ManagedCoreKeysAccount into the collection
QuantumExplorer May 3, 2026
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
2 changes: 1 addition & 1 deletion dash-spv/src/sync/mempool/sync_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl<W: WalletInterface + 'static> SyncManager for MempoolManager<W> {
// Rebuild bloom filter if the wallet's monitored set has changed.
//
// We poll the revision counter rather than using push-based wallet events
// for simplicity: the revision lives on `ManagedCoreAccount` and auto-bumps
// for simplicity: the revision lives on `ManagedCoreFundsAccount` and auto-bumps
// on address generation and UTXO mutations, giving us a single source of
// truth without needing event emission after every wallet operation.
// Adding a push-based approach would require a new `select!` branch in the
Expand Down
8 changes: 6 additions & 2 deletions dash-spv/tests/dashd_sync/helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,12 @@ pub(super) async fn count_wallet_transactions(
) -> usize {
let wallet_read = wallet.read().await;
let wallet_info = wallet_read.get_wallet_info(wallet_id).expect("Wallet info not found");
let txids: HashSet<_> =
wallet_info.accounts().all_accounts().iter().flat_map(|a| a.transactions.keys()).collect();
let txids: HashSet<_> = wallet_info
.accounts()
.all_accounts()
.into_iter()
.flat_map(|a| a.transactions_iter().map(|(txid, _)| txid))
.collect();
txids.len()
}

Expand Down
20 changes: 14 additions & 6 deletions dash-spv/tests/dashd_sync/setup.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,12 @@ impl TestContext {
let wallet_read = self.wallet.read().await;
let wallet_info =
wallet_read.get_wallet_info(&self.wallet_id).expect("Wallet info not found");
wallet_info.accounts().all_accounts().iter().map(|a| a.transactions.len()).sum()
wallet_info
.accounts()
.all_accounts()
.into_iter()
.map(|a| a.transactions_iter().count())
.sum()
Comment on lines +124 to +129
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

transaction_count overcounts if a txid appears in multiple accounts.

transaction_count sums per-account transactions_iter().count() while the conceptually equivalent count_wallet_transactions in helpers.rs (lines 65–70) builds a HashSet to count unique txids, and assert_wallet_synced also uses a HashSet. If any transaction is recorded under more than one account, transaction_count will return a higher value, silently diverging from the other two helpers and potentially causing assertion failures.

🔧 Proposed fix 
-        wallet_info
-            .accounts()
-            .all_accounts()
-            .into_iter()
-            .map(|a| a.transactions_iter().count())
-            .sum()
+        let txids: std::collections::HashSet<_> = wallet_info
+            .accounts()
+            .all_accounts()
+            .into_iter()
+            .flat_map(|a| a.transactions_iter().map(|(txid, _)| txid))
+            .collect();
+        txids.len()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
wallet_info
.accounts()
.all_accounts()
.into_iter()
.map(|a| a.transactions_iter().count())
.sum()
let txids: std::collections::HashSet<_> = wallet_info
.accounts()
.all_accounts()
.into_iter()
.flat_map(|a| a.transactions_iter().map(|(txid, _)| txid))
.collect();
txids.len()
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@dash-spv/tests/dashd_sync/setup.rs` around lines 124 - 129, The current
computation using wallet_info.accounts().all_accounts().into_iter().map(|a|
a.transactions_iter().count()).sum() overcounts when the same txid appears in
multiple accounts; change the logic in the transaction_count helper to collect
txids from each account via a transactions_iter(), insert them into a HashSet to
deduplicate, and return the set.len() instead of summing per-account counts (see
count_wallet_transactions in helpers.rs for the intended pattern and use of
HashSet to count unique txids).

}
/// Retrieves the spendable balance of the wallet.
pub(super) async fn spendable_balance(&self) -> u64 {
Expand Down Expand Up @@ -150,6 +155,9 @@ impl TestContext {
else {
panic!("Account 0 is not a Standard account type");
};
// `account` is a `&ManagedCoreFundsAccount` from the funds-typed
// `standard_bip44_accounts` map; the destructure above reads through
// its public `managed_account_type` field.

external_addresses
.unused_addresses()
Expand All @@ -166,8 +174,8 @@ impl TestContext {
wallet_info
.accounts()
.all_accounts()
.iter()
.any(|account| account.transactions.contains_key(txid))
.into_iter()
.any(|account| account.transactions_iter().any(|(stored, _)| stored == txid))
|| wallet_info.immature_transactions().iter().any(|tx| &tx.txid() == txid)
}

Expand Down Expand Up @@ -196,7 +204,7 @@ impl TestContext {

let mut spv_txids = HashSet::new();
for managed_account in wallet_info.accounts().all_accounts() {
for txid in managed_account.transactions.keys() {
for (txid, _) in managed_account.transactions_iter() {
spv_txids.insert(txid.to_string());
}
}
Expand Down Expand Up @@ -303,8 +311,8 @@ pub(super) async fn client_has_transaction(
wallet_info
.accounts()
.all_accounts()
.iter()
.any(|account| account.transactions.contains_key(txid))
.into_iter()
.any(|account| account.transactions_iter().any(|(stored, _)| stored == txid))
|| wallet_info.immature_transactions().iter().any(|tx| &tx.txid() == txid)
}

Expand Down
4 changes: 2 additions & 2 deletions key-wallet-ffi/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ eddsa = ["dashcore/eddsa", "key-wallet/eddsa"]
bls = ["dashcore/bls", "key-wallet/bls"]

[dependencies]
key-wallet = { path = "../key-wallet" }
key-wallet = { path = "../key-wallet", features = ["keep_txs_in_memory"] }
key-wallet-manager = { path = "../key-wallet-manager" }
dashcore = { path = "../dash" }
dash-network = { path = "../dash-network", features = ["ffi"] }
Expand All @@ -33,6 +33,6 @@ hex = "0.4"
cbindgen = "0.29"

[dev-dependencies]
key-wallet = { path = "../key-wallet", features = ["test-utils"] }
key-wallet = { path = "../key-wallet", features = ["test-utils", "keep_txs_in_memory"] }
key-wallet-manager = { path = "../key-wallet-manager", features = ["test-utils"] }
hex = "0.4"
Loading
Loading