fix: close same-UTXO concurrent-selection race in send_to_addresses

[lklimek]

Summary

Stacks on top of #3585 to close the same-UTXO concurrent-selection race in CoreWallet::send_to_addresses.

#3585 (now reframed) closes broadcast-failure atomicity — UTXOs and the change-address index are not mutated when a broadcast errors. That is a real fix, but it does not prevent two concurrent callers from both selecting the same UTXO X, both broadcasting, and the loser receiving an opaque network rejection. This PR closes that race in-process.

The race, before this PR

send_to_addresses acquires the wallet write lock, snapshots the spendable UTXO set, builds the transaction, drops the lock, then awaits broadcast. Two callers can interleave like this:

1. Caller A acquires the write lock, snapshots spendable {X, Y, Z}, selects {X}, drops the lock.
2. Caller B acquires the write lock, snapshots spendable. X still appears unspent — A has not called check_core_transaction(... update_state=true) yet. B also selects {X}, drops the lock.
3. Both broadcast. The network resolves the conflict; the loser gets an opaque TransactionBroadcast error.

Local state on the loser may or may not be corrupt depending on broadcast-failure handling — that part is what #3585 fixed. The selection race itself is untouched by #3585.

Approach: per-wallet outpoint reservations

Every CoreWallet gets an OutpointReservations field — a Mutex<HashSet<OutPoint>> of in-flight outpoints. Coin selection consults the reservation set in addition to the spendable set, and excludes any outpoint that is already reserved.

The fix flow:

1. Acquire the write lock (existing).
2. Snapshot spendable, build the transaction (existing).
3. Add selected outpoints to reservations — atomically, under the same write lock.
4. Drop the write lock (existing).
5. Broadcast.
6. On success: re-acquire the lock, call check_core_transaction(... update_state=true) to mark UTXOs spent normally, remove outpoints from reservations.
7. On failure: remove outpoints from reservations. No rollback — local state was never mutated, so there is nothing to undo.

A small RAII guard (ReservationGuard) ensures reservations are always released, including on panic in steps 5–7.

After this PR, the same interleaving above plays out as:

1. A reserves {X} under the write lock.
2. B acquires the write lock, snapshots spendable. B sees only {Y, Z} — X is reserved.
3. Either B selects {Y, Z} (different UTXOs — both succeed cleanly), or B has insufficient spendable for the requested output and gets a typed NoSpendableInputs error before broadcasting.

B never broadcasts a transaction that double-spends X. That is the property #3585 does not provide.

Design rationale — why a reservation set, not lock-held mark-spent

The textbook alternative is to call check_core_transaction(... update_state=true) inside the write lock, before broadcast — marking UTXOs spent up front and rolling back on broadcast failure. We considered it and rejected it for one practical reason: check_core_transaction does not currently expose an "unspend" / rollback primitive, so adopting that approach would require building rollback support in the upstream key-wallet crate. The reservation-set approach sidesteps that work entirely — there is nothing to undo on failure, so no rollback primitive is needed.

A reservation set has one tradeoff worth flagging: it closes the race in-process only. Two separate processes operating the same wallet can still race at the network layer. Production wallets are single-process per wallet, so this is acceptable; multi-process scenarios were never closed by #3585 either.

Files changed

File	Net	Purpose
packages/rs-platform-wallet/src/wallet/core/reservations.rs	new	OutpointReservations + RAII ReservationGuard
packages/rs-platform-wallet/src/wallet/core/broadcast.rs	mod	reserve under lock; release on success/failure
packages/rs-platform-wallet/src/wallet/core/wallet.rs	mod	CoreWallet.reservations: OutpointReservations field
packages/rs-platform-wallet/src/wallet/core/mod.rs	mod	module declaration
packages/rs-platform-wallet/src/error.rs	mod	new NoSpendableInputs { context: String } variant

Public API surface change

• New error variant: PlatformWalletError::NoSpendableInputs { context: String } — the typed error a race-loser sees instead of a generic broadcast failure.
• send_to_addresses signature unchanged.
• OutpointReservations is pub(crate) — not exported beyond the crate.

Test plan

• cargo fmt --check green
• cargo build -p platform-wallet green
• cargo clippy -p platform-wallet --lib --tests -- -D warnings clean
• cargo test -p platform-wallet --lib — 128 passed, 0 failed (fix: case-insensitive .dash + atomic state on broadcast failure #3585 baseline 123 + 5 new):
  • wallet::core::reservations::tests::reserve_then_drop_releases
  • wallet::core::reservations::tests::second_reservation_is_disjoint
  • wallet::core::reservations::tests::poisoned_mutex_still_releases
  • wallet::core::broadcast::tests::concurrent_same_utxo_sends_resolve_via_reservation_set — headline race-property test: tokio::join! of two concurrent send_to_addresses against the same UTXO, with a gated broadcaster; loser short-circuits without reaching the network.
  • wallet::core::broadcast::tests::broadcast_failure_releases_reservation_for_retry — pins the Drop-release contract.
• Manual: trigger concurrent broadcasts on testnet, verify the loser gets NoSpendableInputs rather than a broadcast-layer rejection (deferred — needs running node)

Out of scope (filed for follow-up)

• wallet/identity/network/payments.rs::send_payment has the same race shape. The same OutpointReservations instance on CoreWallet could be threaded into IdentityWallet to close that path. Tracked but not addressed here.

Stacking note

This PR targets fix/dpns-case-and-utxo-race-v3.1-dev (#3585's branch). Merge order: #3585 first, then this. If #3585 retargets to v3.1-dev directly before merging, this PR auto-rebases.

🤖 Co-authored by Claudius the Magnificent AI Agent

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6348a4e0-3db9-4bc8-beff-04a1317d56f0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/utxo-race-close-on-3585

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[thepastaclaw]

Review Gate

Commit: 86d599e5

• ⏳ Debounce: 22m ago (need 30m)

• ✅ CI checks: builds passed, 0/0 tests passed

• ✅ CodeRabbit review: comment found

• ⏳ Off-peak hours: peak window (5am-11am PT) — currently 05:59 AM PT Friday

• Run review now (check to override)