Skip to content

key-wallet: out-of-order delivery leaves the spending transaction absent from history #897

Description

@xdustinface

Summary

After the #649 fix (#851), a funding transaction delivered after its spend is recorded with its real received amount and the already-spent coin is never inserted, so balance and the UTXO set are correct. But the spending transaction is usually never recorded at all, so history shows the receive and not the spend. Field testing on #851 found 8 such transactions on a testnet CoinJoin wallet under #866 redelivery.

This is a history/display defect. No balance impact.

Why it happens, and why it is bigger than "retain the txid"

The obvious framing — remember the spending txid so a record can be backfilled — does not work, for two independent reasons.

1. The raw spending transaction is not retained. observed_spent_outpoints holds only OutPoint -> CoreBlockHeight. TransactionRecord.transaction is a mandatory owned Transaction and TransactionRecord::new derives the txid from it, so a txid alone cannot fill it. A stub transaction is not viable: the field is public and consumers render inputs, outputs, and fee from it.

2. Even holding the raw transaction, the spend is unattributable. Both attribution paths read the live UTXO set and nothing else — sent is computed via self.utxos.get(&input.previous_output) in account_checker.rs, and input_details is built the same way in managed_core_funds_account.rs. The #649 fix deliberately never inserts the funding coin (that is exactly what keeps balance correct), so the coin never exists in utxos, a redelivered spending transaction still computes sent = 0, matches no address, and dies at the relevance gate.

So any fix needs an attribution path for already-spent coins first. Retaining the transaction, parking it, or annotating the funding record are each only half of it.

Rejected options

  • Widen the map value to carry the spending txid. Dead on both blockers above.
  • Retain the full spending Transaction in the map. Recording happens for every transaction of every matched block, so this is roughly a 100x multiplier on a structure that is already the subject of key-wallet: observed_spent_outpoints grows unpruned for the whole initial sync #899 — tens of GB for a CoinJoin wallet. Disqualifying.
  • Park unattributed spends in a bounded side-buffer. A spend of an unknown coin is indistinguishable from a stranger's transaction, so "park the candidates" means parking the whole block; and bounding it silently evicts the very transactions it targets.
  • Annotate the funding record's outputs as spent-at-birth. Cheap and honest, but adds no spend to history: the user still never sees the outgoing transaction, its fee, or its destination. Viable as a small interim step, forecloses nothing.

Proposed design: spent-coin ledger + stored-block replay

Half 1, key-wallet — make an already-spent coin attributable. Where update_utxos currently skips inserting an observed-spent output, record the coin into a per-account ledger keyed by outpoint, carrying its TxOut, address, and spend height. Teach the two UTXO-keyed lookups (sent in account_checker.rs, input_details in managed_core_funds_account.rs) to fall back to it. Balance is unaffected because it derives only from utxos, and the coin still never enters utxos. A later redelivery of the spending transaction then records correctly as Outgoing with real input_details, net_amount, and fee.

There is precedent for the shape: ManagedCoreFundsAccount::spent_outpoints is already a #[serde(skip)] derived set rebuilt on load by rebuild_spent_outpoints. The ledger can be rebuilt the same way — from funding records' outputs intersected with the persisted observed_spent_outpoints — so it need not be persisted, which avoids any wallet-format change.

Half 2, dash-spv — get the spending transaction redelivered. No network fetch and no parking needed: dash-spv already stores every downloaded block, and the spend block is by definition one of them. Surface the skipped (OutPoint, spend_height) pairs on the check result, bubble them through the block-processing result, and have the block sync manager load_block those heights and re-queue them through the existing add_from_storage path, with a replay-once guard per height so a replay cannot re-trigger itself.

Rough shape: ~80 lines key-wallet, ~60 key-wallet-manager, ~60 dash-spv, plus tests. Call it 250-300 lines across three crates.

Open scope question: Half 2 is dash-spv-only. Consumers driving key-wallet-ffi directly get Half 1's correctness but no automatic redelivery, so their spends stay missing until a rescan.

Pruning interaction: the ledger must not outlive the corresponding observed_spent_outpoints entry, or a pruned map entry plus a live ledger entry could double-count. Tying it to the same finality boundary is natural since it is derived from the map. Pin it with a test rather than assuming it.

Sequencing

Wait for #851 to merge. This hard-requires the #649 map and the insert skip, which exist only on the #851 stack — on dev there is no observed_spent_outpoints at all. Stacking a 3-crate design change on two unmerged branches, one of which shed four subsystems in its most recent revision, is the wrong order for a display-only defect. Land #851, then open this against dev.

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