Skip to content

Found-012: wait_for_proof and recover_asset_lock_blocking hard-code BIP-44 — asset-lock proof flow misses CoinJoin / legacy BIP-32 funding #3642

@lklimek

Description

@lklimek

Summary

Five sites in rs-platform-wallet's asset-lock proof flow hard-code standard_bip44_accounts.get(...).transactions().get(&txid) when looking up the TransactionRecord for a tracked asset-lock. Any wallet that funds an asset-lock from a CoinJoin or legacy BIP-32 account silently fails on this lookup — the record is correctly inserted into the funding account's transactions map by the post-broadcast pipeline, but the lookup consults the wrong collection. Result: wait_for_proof returns None, and the asset-lock proof flow surfaces "asset lock not tracked" / FinalityTimeout errors despite chain-side correctness.

The fix is downstream-onlyManagedAccountCollection::all_funding_accounts() (rust-dashcore/key-wallet/src/managed_account/managed_account_collection.rs:1102) already iterates standard_bip44_accounts + standard_bip32_accounts + coinjoin_accounts + dashpay_*. The 5 sites just need to use it instead of hard-coding standard_bip44_accounts.

This is documented as Found-012 ("account-type tunnel vision in validate_or_upgrade_proof") and Found-023 ("ManagedAccountCollection lacks a find_transaction_record(&Txid) helper") in packages/rs-platform-wallet/tests/e2e/TEST_SPEC.md. After audit, the actionable layer is downstream — the upstream helper is an optional ergonomic improvement, not a prerequisite.

User Story

As a Dash Platform application developer using rs-platform-wallet to fund identity operations from a wallet that has CoinJoin or legacy BIP-32 funding sources, I need AssetLockManager::wait_for_proof and recover_asset_lock_blocking to find the tracked asset-lock transaction regardless of which funding account it came from, so that:

  1. CoinJoin-using wallets can fund identity operations. Privacy-conscious users who mix UTXOs via CoinJoin have their funding UTXOs in coinjoin_accounts, not standard_bip44_accounts. Today, asset-lock proof construction silently fails for these wallets: the post-broadcast pipeline correctly records the tx in coinjoin_accounts[i].transactions, but wait_for_proof's lookup at line 376 only inspects standard_bip44_accounts[account_index]. The lookup returns None; the proof flow times out.

  2. Legacy BIP-32 wallets can fund identity operations. Users importing older Dash-Core wallets see the same disease in standard_bip32_accounts. Same lookup, same wrong account collection, same silent failure. CR-004's test setup uses BIP-32 directly to expose related concerns.

  3. Wallet recovery flows work across funding types. recover_asset_lock_blocking at recovery.rs:63 has the same hard-code. A wallet restored from seed that has CoinJoin or BIP-32 funding cannot re-derive its in-flight asset-lock proofs.

The bug is silent in failure: the wallet broadcasts the asset-lock successfully, the IS-lock arrives, the block confirms, and the chainlock lands — but the proof can't be assembled because the lookup looks in the wrong drawer.

Reproduction

On branch feat/rs-platform-wallet-e2e (PR #3549, today's HEAD):

  1. Create a wallet with WalletAccountCreationOptions::Default — populates both standard_bip44_accounts[0] AND standard_bip32_accounts[0] (see key_wallet::wallet::initialization::WalletAccountCreationOptions).
  2. Fund standard_bip32_accounts[0] (or coinjoin_accounts[0]) with enough Core balance to construct an asset-lock.
  3. Build an asset-lock transaction whose funding inputs come from the BIP-32 / CoinJoin account.
  4. Call AssetLockManager::wait_for_proof(out_point, timeout).

Expected: the function returns Ok(AssetLockProof::Instant(...)) or Ok(AssetLockProof::Chain(...)) once the IS-lock / chainlock arrives.

Actual: the function loops over tracked_asset_locks lookups that consistently return None because they consult standard_bip44_accounts[account_index] instead of the funding account. The loop times out with FinalityTimeout(<txid>).

A direct unit-test repro (no live network) is feasible: construct a ManagedWalletInfo with a tx pre-inserted into standard_bip32_accounts[0].transactions, then call any of the 5 sites listed below — observe None.

Root Cause

Five hard-coded sites in packages/rs-platform-wallet/src/wallet/asset_lock/sync/:

File:line Surface
proof.rs:103 validate_or_upgrade_proof — initial tracked-lock lookup
proof.rs:204 proof status check
proof.rs:302 wait_for_chain_lock per-iteration record fetch
proof.rs:376 wait_for_proof per-iteration record fetch
recovery.rs:63 recover_asset_lock_blocking — wallet-restore lookup

All five do (roughly):

info.core_wallet.accounts.standard_bip44_accounts.get(&account_index)
    .and_then(|a| a.transactions().get(&out_point.txid).cloned())

The correct shape is to iterate all_funding_accounts():

info.core_wallet.accounts.all_funding_accounts()
    .iter()
    .find_map(|a| a.transactions().get(&out_point.txid).cloned())

ManagedAccountCollection::all_funding_accounts() at rust-dashcore/key-wallet/src/managed_account/managed_account_collection.rs:1102 already exists and iterates every funds-bearing account collection — no upstream change required.

Why the SPV / tracking side is fine (preempting a likely review question)

  • The wallet's bloom filter is built from WalletInterface::monitored_addresses() (rust-dashcore/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs:260), which calls self.accounts.all_accounts() — that helper iterates standard_bip44_accounts, standard_bip32_accounts, coinjoin_accounts, and all identity/provider/dashpay account types. Address registration is automatic across all account types; no user-level workaround is needed.
  • TransactionRouter::get_relevant_account_types(TransactionType::Standard) (rust-dashcore/key-wallet/src/transaction_checking/transaction_router/mod.rs:83) returns [StandardBIP44, StandardBIP32, DashpayReceivingFunds, DashpayExternalAccount] — the post-broadcast check_core_transaction correctly routes the tx into the matching funding account's transactions map.
  • The bug is purely in the consumer-side lookup that hard-codes standard_bip44_accounts.

Affected Versions

  • v3.1-dev HEAD today (feat/rs-platform-wallet-e2e carries the same hard-codes) — confirmed via direct file inspection
  • All prior versions since the asset-lock manager was introduced (search returned 0 issues / 0 PRs touching the asset-lock lookup pattern)

Cross-Repository Impact

rust-dashcore (optional, not blocking): the existing all_funding_accounts() helper is sufficient for the platform-side fix. Adding ManagedAccountCollection::find_transaction_record(&Txid) -> Option<(AccountType, &TransactionRecord)> would be a nicer ergonomic surface, but is not required. If desired, a separate (lower-priority) rust-dashcore issue can be filed for the helper. The platform-side fix here is independent and can ship first.

Severity

MEDIUM — silent failure on the asset-lock proof critical path for any wallet with non-BIP-44 funding. Empirical evidence: 5 consumer sites in one repo all got this wrong the same way (Found-012 in TEST_SPEC.md), which is the hallmark of a class-of-bugs problem rather than a one-off mistake. The chain-side flow is correct; the wallet just can't see the record because it looks in the wrong place.

Notes

No fix proposal in this body. Two refactor options visible at a glance (left to implementer):

  • Option A: replace each hard-coded standard_bip44_accounts.get(&account_index) with all_funding_accounts().iter().find_map(|a| a.transactions().get(&txid).cloned()). Simple, drops the account_index parameter from the lookup site (it's no longer meaningful when iterating all accounts).
  • Option B: introduce a wrapper helper inside rs-platform-wallet (e.g. fn find_tracked_record(info: &ManagedWalletInfo, txid: &Txid) -> Option<TransactionRecord>) so the 5 sites share one implementation. Closes the same class of bug for future consumers.

Both leave the account_index parameter as a hint-only / fallback fast path if desired.

TEST_SPEC cross-references

  • Matrix row: Found-012 (P2) at TEST_SPEC.md:262
  • Detail body: Found-012 at line 2204, Found-023 (the upstream ergonomic framing) at line 2440
  • Tracked-locks ↔ per-account-transactions consumer surface documented under AL-001 / Found-008 cross-refs

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions