Skip to content

key-wallet: coinbase and asset-unlock outputs paying CoinJoin/DashPay addresses are never discovered #900

Description

@xdustinface

Summary

TransactionRouter::get_relevant_account_types returns only StandardBIP44 and StandardBIP32 for the Coinbase and AssetUnlock classifications, while fund_bearing_account_types() is {StandardBIP44, StandardBIP32, CoinJoin, DashpayReceivingFunds, DashpayExternalAccount}. An output of a coinbase (masternode payout, mining reward) or an asset-unlock (Platform credit withdrawal) that pays a CoinJoin or DashPay address is therefore never matched, so the coin is never credited and the balance undercounts.

This is the credit-side mirror of the debit-side routing bug #867 fixed for AssetLock, and the same reasoning applies: discovery is membership-based like Dash Core's IsMine, so consulting the full fund-bearing set cannot yield a false positive.

Note the filter layer already does the right thing — the wallet queries all of its scripts, so the block is downloaded on a match. The output is then dropped purely by the account-type narrowing, which makes this a wasted download as well as a lost coin.

Also worth fixing while there: the Coinbase arm's comment reads "Check all account types for unknown special transactions" but the arm lists two. The comment already describes the intended behavior.

Honest caveat on reachability

I have not demonstrated this against a real wallet, and it may be unreachable in practice. CoinJoin pools are mixing-internal and are not offered as receive destinations, and DashPay receiving addresses are derived per contact, so neither is a natural choice for a masternode payout address or a Platform withdrawal destination. The payout address in a ProRegTx and the withdrawal destination in an asset-unlock are both user-chosen, so nothing prevents it, but I would not claim a user is likely to do it.

Filing it because the fix is trivial, mirrors an established precedent, cannot regress anything (membership-based matching), and the stale comment suggests the narrow list was an oversight rather than a decision. If the reachability argument does not convince, closing this is a reasonable outcome — I would rather it be judged on merit than assumed.

Suggested fix

Replace the hand-written type lists in the Coinbase and AssetUnlock arms with fund_bearing_account_types(), mirroring what #867 did for AssetLock. Roughly -8/+4 in key-wallet/src/transaction_checking/transaction_router/mod.rs, no signature changes, no touched callers.

Tests: a router unit test asserting both arms contain all five fund-bearing types, plus a behavioral test funding a CoinJoin address via a coinbase output and asserting the balance credits it, red-proven against current dev.

Related

"Fund-bearing account" is spelled out by hand in three places — fund_bearing_account_types(), ManagedAccountCollection::all_funding_accounts(), and update_utxos's match arm. This bug is what drift between those lists looks like. Deriving them from one source would prevent recurrence, but that is a refactor for its own commit.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions