fix(key-wallet): stop counting a contact's watch-only coins as the wallet's - #926
Merged
Merged
Conversation
…llet's
`all_funding_accounts` included `dashpay_external_accounts`. A DashPay
external account is watch-only by construction — its addresses derive
from a *contact's* xpub, so this wallet can observe those outputs but can
never sign for them. They are the contact's coins, and this wallet only
ever pays into them.
Counting them as funding had two visible consequences.
The wallet balance did not move after paying a contact: the amount left
the standard account and reappeared under the external one, because the
payment output landed there. Observed on device — after sending 0.01
DASH, `account_balances=[Standard{idx:0,BIP44}=>0.99587131 DASH,
DashpayExternal{idx:0}=>0.01 DASH]`, with the external account climbing
across successive payments (0.111 → 0.461 → 0.511). To a user this reads
as "my balance is wrong", or worse, "the send did not happen".
And `utxos()` fed `get_spendable_utxos()`, so coin selection could pick
an output the wallet cannot sign for, failing at signing time with funds
apparently available.
Receival accounts stay: those addresses derive from our own xpub and a
contact pays into them, so the funds are genuinely ours.
This does not stop the wallet seeing those transactions. Detection runs
through `transaction_checking`, whose `AccountTypeToCheck` list still
includes `DashpayExternalAccount`, and the monitored-address /
compact-filter set is built from `all_accounts`, which also still
includes it. Only balance and UTXO aggregation change.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe funding-account iterators now exclude DashPay external accounts. They continue to include standard and DashPay receival accounts. Tests verify both iterator variants and confirm that external accounts remain stored. ChangesFunding account scope
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #926 +/- ##
==========================================
+ Coverage 74.95% 75.04% +0.09%
==========================================
Files 328 328
Lines 76884 76933 +49
==========================================
+ Hits 57626 57738 +112
+ Misses 19258 19195 -63
|
QuantumExplorer
approved these changes
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ManagedAccountCollection::all_funding_accountsincludesdashpay_external_accounts.A DashPay external account is watch-only by construction — the crate's own doc comment on the variant says so (
managed_account_type.rs: "DashPay external (watch-only) account"). Its addresses derive from a contact's xpub, so this wallet can observe those outputs but can never sign for them. They are the contact's coins; this wallet only ever pays into them.Treating them as funding has two visible consequences.
The balance does not move after paying a contact. The amount leaves the standard account and reappears under the external one, because the payment output landed there. Observed on an iOS device after sending 0.01 DASH to a contact:
The external account climbs across successive payments (0.111 → 0.461 → 0.511 in one session). To a user this reads as "my balance is wrong", or worse, "the send did not happen".
Unspendable outputs reach coin selection.
utxos()feedsget_spendable_utxos(), so selection can pick an output the wallet has no key for and fail at signing time with funds apparently available.What changed
dashpay_external_accountsis dropped fromall_funding_accountsandall_funding_accounts_mut. That covers every consumer at once —account_balances,balance/update_balance,utxos,get_spendable_utxos,immature_transactions,matured_coinbase_records.dashpay_receival_accountsstays: those addresses derive from our xpub and a contact pays into them, so the funds are genuinely ours.The doc comment now states the exclusion and the reason, so the line does not get "fixed" back.
What deliberately did not change
The wallet still sees these transactions.
transaction_checking, whoseAccountTypeToChecklist still includesDashpayExternalAccount(transaction_router/mod.rs,account_checker.rs), so a send to a contact is still routed and recorded.all_accounts, notall_funding_accounts, and still includes the external account.all_accounts/all_accounts_mutare untouched, so chain-lock promotion, transaction history and instant-lock marking keep working per-account.Only balance and UTXO aggregation change.
Alternatives considered
update_balance/get_spendable_utxosonly — strictly worse:account_balances()and rawutxos()would keep reporting the contact's coins as the wallet's, i.e. the same bug on a different surface.is_watch_onlyexists on the immutableAccountbut not onManagedCoreFundsAccount; the fact is already encoded in the account type, which is what the aggregation matches on anyway.Testing
cargo test -p key-wallet— 604 passed, 0 failed. No existing test encoded the old behaviour.New regression test
external_dashpay_accounts_are_not_funding_accountsbuilds a collection with a standard, a receival and an external account and asserts the external one is absent from the funding view while the receival one is present, that the mutable view visits the same set, and that the account itself still exists in the collection. Verified to fail without the fix ("the contact's watch-only account must not count as this wallet's funds").cargo clippy -p key-wallet --all-features --all-targets -- -D warningsandcargo fmt --checkare clean.Summary by CodeRabbit