Repository navigation
Carry a sweep's wallet-owned held outpoints, symmetric to released_outpoints #968
Description
Summary
WalletEvent::TransactionsSwept names the outpoints a sweep releases (added in #962) but not the ones it leaves held. Persistence backends therefore cannot tell a loser's foreign input from a loser's wallet-owned-but-unclassified input, and must treat every absent non-released input as potentially ours. In dashpay/platform that means one permanent zero-value placeholder row per foreign input of a swept incoming payment.
Why storage cannot decide this locally
A transaction can be wallet-relevant purely because it pays a wallet output while every one of its inputs belongs to the sender. When a final replacement consumes that same input set, those inputs are (correctly) excluded from released_outpoints. Downstream sees "absent from the UTXO table, not released" — identical to the case the placeholder exists for.
Record metadata does not break the tie either. input_details is built from self.utxos.get(&input.previous_output) at record time (key-wallet/src/managed_account/managed_core_funds_account.rs:829-848) and direction derives from it. So a coin that is genuinely ours but spent before its funding output was classified — gap-limit address, or a second device on the same seed — produces a record whose input is absent from input_details and whose direction reads Incoming: byte-for-byte what an attacker's fan-in payment produces.
Why it cannot be decided at the sweep site either, today
At the sweep site the account holds the winner's inputs, each loser's inputs, self.utxos, and self.spent_outpoints. None of them carries ownership for this coin:
freedcollects every input of each removed loser with no ownership filter (managed_core_funds_account.rs:499).rebuild_spent_outpoints(managed_core_funds_account.rs:1293-1299) takes every input of every held record, also unfiltered — so membership there does not imply the wallet owns the funding output.- The unclassified-ours coin is in neither
utxosnorinput_details.
An intersection with the winner's inputs does not help: in the abuse case the winner consumes the same set the loser did.
Proposal
Carry per-wallet held outpoints on TransactionsSwept, symmetric to released_outpoints — the outpoints the sweep leaves claimed and which this wallet can attest are its own (loser_inputs ∩ (utxos ∪ ownership-filtered spent marks)). Persisters then create absent-input rows only for attested outpoints.
This is additive: WalletEvent crosses no ABI, and #[serde(default)] keeps old payloads decoding. Roughly ConflictSweep, the two construction sites in key-wallet-manager/src/process_block.rs, events.rs, and the per-wallet plumbing that already mirrors per_wallet_released_outpoints.
Known trade-off
Gating on attested ownership drops the hold for the unclassified-ours coin, which would read spendable from sweep until the winner confirms in a block (where #649's persisted observed_spent map covers it). That narrows behavior currently pinned on three backends, so the semantics need a decision before implementation — recording it here rather than assuming it.
Downstream reference
dashpay/platform#4406 documents the exposure at the placeholder INSERT (packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs, KNOWN EXPOSURE block) and ships the funds-safe status quo: the placeholders are zero-value spent = 1 rows excluded from every restore path, growable only by an attacker repeatedly losing intentional double-spend races against the victim.
Activity
romchornyi commentedon Aug 20, 2026
Correcting this issue after re-deriving the proposal at the sweep site. The diagnostic half stands; the proposal half does not survive contact with the code. All line references are at
5877d15f(current tip of the swept-descendant work; identical in these files to HEAD).What holds up. The diagnosis is correct and re-verified: storage cannot decide ownership locally (a transaction can be wallet-relevant purely because one output pays us while every input is the sender's), and record metadata cannot break the tie —
input_detailsis built fromself.utxos.get(&input.previous_output)at record time, so a genuinely-ours coin spent before its funding output classified produces a record byte-identical to an attacker's fan-in payment: input absent frominput_details, directionIncoming.What does not: the proposed attested set is empty by construction. The proposal asks the sweep site to emit
loser_inputs ∩ (utxos ∪ ownership-filtered spent marks). Both halves are vacuous:loser_inputs ∩ utxos ≡ ∅, always. Recording the loser already removed every one of its inputs fromutxosand marked them spent (key-wallet/src/managed_account/managed_core_funds_account.rs:407-409), and theis_outpoint_spentguard at:344blocks re-insertion while the record is held. By the time the sweep fires, nothing the loser spends can be inutxos.rebuild_spent_outpoints(:1298) takes every input of every held record with no ownership filter, and the unclassified-ours coin is in neitherutxosnor any record'sinput_details. There is no data to filter with.So gating the persister's absent-input placeholder on this set would not narrow the hold to attested-ours coins. It would make the placeholder unreachable and silently drop the one hold it exists to preserve.
The trade-off paragraph's safety net does not exist downstream. It accepted the dropped hold on the grounds that #649's persisted
observed_spentmap covers the coin once the winner confirms. Upstream persists that map only via whole-wallet serde;dashpay/platformrestores wallet state row-by-row, andobserved_spenthas zero occurrences acrossrs-platform-wallet,rs-platform-wallet-storage,rs-platform-wallet-ffi, the Swift SDK, and the Kotlin SDK. After any restart the map is empty. The exposure the paragraph was willing to accept is not "until block inclusion" — it is permanent.One more scoping error: "additive, crosses no ABI" is only true of the Rust event. The mobile persisters consume sweeps through
OnTransactionsSweptCallbackindash-spv-ffi, and #962 already documented that the struct has no safely-degrading insertion point — a new field there is a breaking C ABI change.Conclusion. The sound fix is storage-side in
dashpay/platformand needs no upstream change: keep creating the placeholder unconditionally — at this layer that is the only correct behavior — and bound its lifetime instead of its creation, mirroring the eviction doctrine key-wallet already applies to the same shape:prune_finalized_observed_spends(key-wallet/src/wallet/managed_wallet_info/mod.rs:372) evicts observed spends oncemin(chainlock_height, synced_height)passes them. That turns attacker-created rows from permanent into chainlock-transient and never touches the FFI surface.Recommend closing this as not planned; leaving that to a maintainer rather than closing my own issue on my own analysis. The storage-side fix is tracked on dashpay/platform#4406.
fix(platform-wallet-storage): bound the lifetime of unmaterialised sw…
fix(kotlin-sdk): bound the lifetime of swept pending-input tombstones
fix(swift-sdk): bound the lifetime of swept pending-input tombstones
fix(platform-wallet): preserve an unstamped tombstone for mempool-con…