From f388aab601ef823414334f59ef7c557ce9e25107 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Thu, 10 Sep 2026 17:12:22 +0300 Subject: [PATCH 1/8] fix(platform-wallet): give an unconfirmed outgoing send an owner across a restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A send whose broadcast saw no acceptance signal is left with nothing responsible for it once the app is closed. Two failures follow from that one gap, and they have to be closed together. The spend effect is never persisted. `isSpent` deliberately stays false on the input row until the spending transaction reaches a block, because a mempool-only sighting is reversible by eviction. A running app is still correct — it holds the effect in memory — and until now a restart recovered it only by re-observing the transaction on the network. A transaction that never reached the network cannot be re-observed, so its input came back spendable and the balance re-counted the coin, for good. Nothing resent it either. dash-spv's rebroadcast timer is the only retry, and its `broadcasts` map is filled at the broadcast call and never seeded from persisted rows, so the transaction had no owner in the new process. Restore both. `ClientWalletStartState` now carries the raw bytes of the sends the host still holds as unconfirmed, ordered by `first_seen` so a parent is applied before a child that spends its change. The replay runs at the async boundary in `load_from_persistor`, through the ordinary `check_core_transaction(.., Mempool, ..)` path so `update_utxos` fires — dropping the input from `utxos` and recording it in `spent_outpoints`, reproducing exactly what the live process held — and before `generation.set(..)`, so the balance the UI reads is the corrected one. A detached task in the same loop waits for the SPV transport and re-dispatches the same signed bytes, handing the transaction back to the 600 s timer. Deliberately not a raw `transactions_mut().insert` like the asset-lock record restore: that bypasses `update_utxos`, leaves `spent_outpoints` empty, and then makes every later re-dispatch a no-op because `has_transaction` reports the record as not new. Deliberately no `isSpent` write either, and no automatic release on a timeout — the pending-spend phase ends on evidence and nothing else, or either user intent can win the double-spend race. Re-dispatching is safe only because the accounting replay lands with it: without it the input would be selectable again and this wallet could sign a conflicting transaction. Verified against a wallet left in the broken state: `replayed=1` at launch, ownership restored (`tracked: 1`, previously 0 indefinitely), and the orphaned transaction reached the chain at the ordinary timer mark — InstantSend-locked and ChainLocked, store reconciled on its own. Refs: support ticket 32189 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HyZwoAc4kS8B7Jq6M5S2c1 --- .../rs-platform-wallet-ffi/src/persistence.rs | 58 +++++++- .../src/wallet_restore_types.rs | 27 ++++ .../changeset/client_wallet_start_state.rs | 29 +++- .../rs-platform-wallet/src/manager/load.rs | 126 +++++++++++++++++- .../rs-platform-wallet/src/manager/startup.rs | 1 + 5 files changed, 237 insertions(+), 4 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 52148306238..918fd151dcf 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -66,7 +66,8 @@ use crate::wallet_registration_persistence::AccountAddressPoolFFI; use crate::wallet_restore_types::{ AccountSpecFFI, AccountTypeTagFFI, ContactProfileRestoreEntryFFI, IdentityKeyRestoreFFI, IdentityRestoreEntryFFI, LoadWalletListFreeFn, PaymentRestoreEntryFFI, - ProviderSpecialTxRestoreEntryFFI, StandardAccountTypeTagFFI, UnresolvedAssetLockTxRecordFFI, + ProviderSpecialTxRestoreEntryFFI, StandardAccountTypeTagFFI, UnconfirmedOutgoingTxRecordFFI, + UnresolvedAssetLockTxRecordFFI, UtxoRestoreEntryFFI, WalletRestoreEntryFFI, }; use dpp::address_funds::PlatformAddress; @@ -5482,11 +5483,66 @@ fn build_wallet_start_state( // was interrupted by an app kill can resume from the latest // status without rebroadcasting. let unused_asset_locks = build_unused_asset_locks(entry)?; + + // Decode the sends the host still holds as unconfirmed. Decode + // only: applying the spend needs `check_core_transaction`, which is + // async and wants the `Wallet` and the `ManagedWalletInfo` + // together, so the replay happens at the async boundary in + // `manager::load::load_from_persistor`. See + // `ClientWalletStartState::unconfirmed_outgoing_txs`. + // + // Sorted by the host's `first_seen` so a parent send is replayed + // before a child that spends its change; a child applied first + // would find its input still absent and be dropped as irrelevant. + let unconfirmed_outgoing_txs = { + use dashcore::consensus::Decodable; + let recs: &[UnconfirmedOutgoingTxRecordFFI] = if entry + .unconfirmed_outgoing_tx_records + .is_null() + || entry.unconfirmed_outgoing_tx_records_count == 0 + { + &[] + } else { + unsafe { + slice::from_raw_parts( + entry.unconfirmed_outgoing_tx_records, + entry.unconfirmed_outgoing_tx_records_count, + ) + } + }; + let mut decoded: Vec<(u64, dashcore::blockdata::transaction::Transaction)> = + Vec::with_capacity(recs.len()); + let mut dropped_decode = 0usize; + for rec in recs { + if rec.tx_bytes.is_null() || rec.tx_bytes_len == 0 { + dropped_decode += 1; + continue; + } + let bytes = unsafe { slice::from_raw_parts(rec.tx_bytes, rec.tx_bytes_len) }; + match dashcore::blockdata::transaction::Transaction::consensus_decode( + &mut &bytes[..], + ) { + Ok(tx) => decoded.push((rec.first_seen, tx)), + Err(_) => dropped_decode += 1, + } + } + if dropped_decode > 0 { + tracing::warn!( + wallet_id = %hex::encode(entry.wallet_id), + dropped_decode, + "load: unconfirmed outgoing tx records failed to decode" + ); + } + decoded.sort_by_key(|(first_seen, _)| *first_seen); + decoded.into_iter().map(|(_, tx)| tx).collect::>() + }; + let wallet_state = ClientWalletStartState { wallet, wallet_info, identity_manager, unused_asset_locks, + unconfirmed_outgoing_txs, }; let platform_address_state = if per_account.is_empty() diff --git a/packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs b/packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs index c49be7de1b7..ffd91f761a7 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs @@ -573,6 +573,21 @@ pub struct ProviderSpecialTxRestoreEntryFFI { pub first_seen: u64, } +/// One outgoing transaction the host still holds as unconfirmed, +/// replayed at load so its spend effect survives a restart. +#[repr(C)] +pub struct UnconfirmedOutgoingTxRecordFFI { + /// Consensus-encoded transaction body, the same wire format + /// `dashcore::consensus::encode::serialize` produces. Swift-owned + /// for the callback window; freed by `LoadWalletListFreeFn`. + pub tx_bytes: *mut u8, + pub tx_bytes_len: usize, + /// Host's `firstSeen` for the row, in seconds. The load path + /// replays in ascending order so a parent send is applied before a + /// child that spends its change. + pub first_seen: u64, +} + /// Per-wallet entry returned by `on_load_wallet_list_fn`. /// /// `accounts` points to a contiguous array of length `accounts_count`. @@ -644,6 +659,16 @@ pub struct WalletRestoreEntryFFI { /// unresolved asset locks. pub unresolved_asset_lock_tx_records: *const UnresolvedAssetLockTxRecordFFI, pub unresolved_asset_lock_tx_records_count: usize, + /// Outgoing transactions the host still holds as unconfirmed + /// (mempool context, no block height), oldest `first_seen` first. + /// + /// Replayed at load through the ordinary mempool check so their + /// spend effect is restored — see + /// [`UnconfirmedOutgoingTxRecordFFI`]. `null` / `0` when the wallet + /// has none. Each entry's `tx_bytes` buffer is Swift-owned and + /// freed by `LoadWalletListFreeFn`. + pub unconfirmed_outgoing_tx_records: *const UnconfirmedOutgoingTxRecordFFI, + pub unconfirmed_outgoing_tx_records_count: usize, /// Persisted provider special transactions (ProRegTx / ProUpServTx / /// ProUpRegTx / ProUpRevTx) re-staged onto the wallet's provider-key /// accounts so rust-dashcore #876 retention keeps them resident and @@ -702,6 +727,8 @@ impl Default for WalletRestoreEntryFFI { tracked_asset_locks_count: 0, unresolved_asset_lock_tx_records: std::ptr::null(), unresolved_asset_lock_tx_records_count: 0, + unconfirmed_outgoing_tx_records: std::ptr::null(), + unconfirmed_outgoing_tx_records_count: 0, provider_special_txs: std::ptr::null(), provider_special_txs_count: 0, core_address_pools: std::ptr::null(), diff --git a/packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs b/packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs index 83b6d860742..6eab3d88780 100644 --- a/packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs +++ b/packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs @@ -9,7 +9,7 @@ use std::collections::BTreeMap; use crate::changeset::identity_manager_start_state::IdentityManagerStartState; use crate::wallet::asset_lock::tracked::TrackedAssetLock; -use dashcore::OutPoint; +use dashcore::{OutPoint, Transaction}; use key_wallet::wallet::ManagedWalletInfo; use key_wallet::Wallet; @@ -33,4 +33,31 @@ pub struct ClientWalletStartState { /// Asset locks that have not yet been consumed by an identity /// registration / top-up, keyed by account index → outpoint. pub unused_asset_locks: BTreeMap>, + /// Outgoing transactions the host still has as unconfirmed, in + /// `first_seen` order (a parent send precedes a child that spends + /// its change). + /// + /// These are decoded here but deliberately NOT applied here: the + /// spend has to travel through the normal + /// `check_core_transaction(.., Mempool, ..)` path so `update_utxos` + /// runs — dropping the input from `utxos` and recording it in + /// `spent_outpoints`. That call is async and needs the `Wallet` and + /// the `ManagedWalletInfo` together, so the replay happens at the + /// async boundary in + /// [`load_from_persistor`](crate::manager::load), not while this + /// snapshot is being built. + /// + /// # Why the replay exists + /// + /// The spend effect of an unconfirmed outgoing transaction is never + /// persisted: `isSpent` deliberately stays `false` on the input row + /// until the spending transaction reaches a block, because a + /// mempool-only sighting is reversible by eviction. A running app is + /// still correct — the effect lives in memory. Across a restart it + /// used to be recovered only by re-observing the transaction on the + /// network, which is impossible for one that never got there: the + /// input came back as spendable and the balance re-counted it, + /// permanently. Replaying the record at load restores exactly the + /// state the live process held. + pub unconfirmed_outgoing_txs: Vec, } diff --git a/packages/rs-platform-wallet/src/manager/load.rs b/packages/rs-platform-wallet/src/manager/load.rs index ae04b3b0633..fdedc21a5f4 100644 --- a/packages/rs-platform-wallet/src/manager/load.rs +++ b/packages/rs-platform-wallet/src/manager/load.rs @@ -10,8 +10,20 @@ use crate::wallet::identity::IdentityManager; use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; use crate::wallet::PlatformWallet; +use std::time::Duration; + +use crate::broadcaster::TransactionBroadcaster; +use key_wallet::transaction_checking::transaction_context::TransactionContext; +use key_wallet::transaction_checking::wallet_checker::WalletTransactionChecker; + use super::{run_blocking_load, PlatformWalletManager}; +/// How long the load-time re-dispatch waits for the SPV transport before +/// giving up for this launch. Zero connected peers turns a send into a +/// definitive rejection rather than a retry, and there is no urgency: the +/// next launch offers the same transactions again. +const RESEND_TRANSPORT_READY_WAIT: Duration = Duration::from_secs(30); + impl PlatformWalletManager

{ /// Load the full [`ClientStartState`] from the configured persister /// and rehydrate the manager's `wallet_manager` and `wallets` maps. @@ -97,12 +109,63 @@ impl PlatformWalletManager

{ 'load: for (expected_wallet_id, wallet_state) in wallets { let ClientWalletStartState { - wallet, - wallet_info, + mut wallet, + mut wallet_info, identity_manager, unused_asset_locks, + unconfirmed_outgoing_txs, } = wallet_state; + // Replay the sends the host still holds as unconfirmed, before + // anything reads the restored balance. + // + // Their spend effect is never persisted: `isSpent` stays `false` + // on the input row until the spending transaction reaches a + // block, because a mempool-only sighting is reversible by + // eviction. So the UTXO restore above has just handed those + // inputs back as spendable. A live process was still correct — + // it held the effect in memory — and until now a restart + // recovered it only by re-observing the transaction on the + // network. A transaction that never reached the network cannot + // be re-observed, so its input stayed spendable for good and the + // balance re-counted the coin. + // + // Routing each record through the ordinary mempool check (rather + // than inserting it into `transactions_mut()` raw, the way the + // asset-lock record restore does) is the whole point: it runs + // `update_utxos`, which drops the input from `utxos` and records + // it in `spent_outpoints`, reproducing exactly the state the + // live process held. A raw insert would leave `spent_outpoints` + // empty AND make every later re-dispatch a no-op, because + // `has_transaction` would then report the record as not new. + // + // `update_state` and `update_balance` are both on: the balance + // this produces is what `generation.set(..)` mirrors a few lines + // below, and the UI reads that. + if !unconfirmed_outgoing_txs.is_empty() { + let mut replayed = 0usize; + for tx in &unconfirmed_outgoing_txs { + let result = wallet_info + .check_core_transaction( + tx, + TransactionContext::Mempool, + &mut wallet, + true, + true, + ) + .await; + if result.is_relevant { + replayed += 1; + } + } + tracing::info!( + wallet_id = %hex::encode(expected_wallet_id), + offered = unconfirmed_outgoing_txs.len(), + replayed, + "load: replayed unconfirmed outgoing sends" + ); + } + // Flatten the (account → outpoint → lock) map into the flat // OutPoint → TrackedAssetLock map that `PlatformWalletInfo` // holds today. @@ -205,6 +268,63 @@ impl PlatformWalletManager

{ let broadcaster = Arc::new(crate::broadcaster::SpvBroadcaster::new(Arc::clone( &self.spv_manager, ))); + + // Give the replayed sends an owner again on the network side. + // + // dash-spv's rebroadcast timer is the only thing that retries a + // transaction whose broadcast saw no acceptance signal, and its + // `broadcasts` map is process-local: it is filled at the + // broadcast call and never seeded from persisted rows. So a send + // that did not reach the network before the app was closed had + // nobody left to resend it — measured, it never went out again. + // Re-dispatching here hands it back to that timer. + // + // Deliberately fire-and-forget on a detached task: this must not + // hold up the load, and no verdict is wanted. `broadcast_transaction` + // is used rather than the awaiting variant precisely because the + // timer, not this call, is meant to own the outcome — and an + // unrequested `Uncertain` 60 s later has no listener on the app + // side, so it cannot surface a stray dialog. + // + // Safe against double-spending: this re-sends the SAME signed + // bytes, which is idempotent for the network, and `start_broadcast` + // is idempotent per txid. The real hazard would be re-dispatching + // without the accounting replay above — the input would be + // selectable again and this wallet could sign a conflicting + // transaction. That is why the two halves ship together. + if !unconfirmed_outgoing_txs.is_empty() { + let broadcaster_for_resend = Arc::clone(&broadcaster); + let txs_to_resend = unconfirmed_outgoing_txs.clone(); + tokio::spawn(async move { + // Zero connected peers makes the send a definitive + // `Rejected` rather than a retry, so wait for the + // transport before offering anything. + if !broadcaster_for_resend + .wait_until_ready(RESEND_TRANSPORT_READY_WAIT) + .await + { + tracing::warn!( + pending = txs_to_resend.len(), + "load: broadcast transport not ready; leaving unconfirmed \ + sends for the next launch" + ); + return; + } + for tx in txs_to_resend { + let txid = tx.txid(); + // Goes through the acceptance wait, which is fine on + // a detached task: the verdict is only logged, and + // dash-spv has already taken ownership by then. + match broadcaster_for_resend.broadcast(&tx).await { + Ok(_) => tracing::info!(%txid, "load: re-dispatched unconfirmed send"), + Err(e) => { + tracing::warn!(%txid, error = ?e, "load: re-dispatch failed") + } + } + } + }); + } + let platform_wallet = PlatformWallet::new( Arc::clone(&self.sdk), wallet_id, @@ -435,6 +555,7 @@ mod idempotent_load_tests { wallet_info: self.managed.clone(), identity_manager: IdentityManagerStartState::default(), unused_asset_locks: BTreeMap::new(), + unconfirmed_outgoing_txs: Vec::new(), }, ); Ok(ClientStartState { @@ -473,6 +594,7 @@ mod idempotent_load_tests { wallet_info: self.managed.clone(), identity_manager: IdentityManagerStartState::default(), unused_asset_locks: BTreeMap::new(), + unconfirmed_outgoing_txs: Vec::new(), }; let mut wallets = BTreeMap::new(); wallets.insert(self.wallet.compute_wallet_id(), entry()); diff --git a/packages/rs-platform-wallet/src/manager/startup.rs b/packages/rs-platform-wallet/src/manager/startup.rs index 30b67703db3..04c07d50832 100644 --- a/packages/rs-platform-wallet/src/manager/startup.rs +++ b/packages/rs-platform-wallet/src/manager/startup.rs @@ -1841,6 +1841,7 @@ mod tests { wallet_info: self.managed.clone(), identity_manager: crate::changeset::IdentityManagerStartState::default(), unused_asset_locks: std::collections::BTreeMap::new(), + unconfirmed_outgoing_txs: Vec::new(), }, ); Ok(crate::changeset::ClientStartState { From a926bde1bf676f54f391a3285b5ef1567dcfbbf9 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Thu, 10 Sep 2026 17:12:34 +0300 Subject: [PATCH 2/8] fix(swift-sdk): hand the unconfirmed outgoing sends to the load path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fills the restore buffer the previous commit reads. Without this the Rust side sees an empty array and the replay is inert. The candidates are selected from the TXO side rather than the transaction side, which makes the liveness rule fall out for free: a row is offered only while one of our own outputs still points at it as its spender and is itself still unspent. A send that already lost a conflict has had its inputs flipped by the winning spender, so it drops out on its own — which matters, because the FFI restore does not rebuild `observed_spent` and Rust could not make that judgement for itself. The bytes are the ones already on disk (`transactionData`), so nothing new is persisted and the SwiftData schema is untouched — worth keeping, since one added property would force freezing every linked model. Asset-lock funding transactions are excluded: they ride `unresolved_asset_lock_tx_records` and `resume_asset_lock` already owns them. One owner per transaction. Refs: support ticket 32189 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HyZwoAc4kS8B7Jq6M5S2c1 --- .../PlatformWalletPersistenceHandler.swift | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index 74b7446411e..27f11f4ddb1 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -7014,6 +7014,22 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { entry.unresolved_asset_lock_tx_records = unresolvedBuf.map { UnsafePointer($0) } entry.unresolved_asset_lock_tx_records_count = UInt(unresolvedCount) + // Sends still unconfirmed on the host. Replayed Rust-side so + // their spend effect survives the restart; without it the + // input comes back spendable and the balance re-counts the + // coin — permanently, for a send that never reached the + // network. Asset-lock funding rows are excluded here because + // they already ride the array above and `resume_asset_lock` + // owns them. + let (unconfirmedBuf, unconfirmedCount) = + buildUnconfirmedOutgoingTxRecordBuffer( + walletId: w.walletId, + allocation: allocation, + excludingTxids: unresolvedAssetLockFundingTxids(walletId: w.walletId) + ) + entry.unconfirmed_outgoing_tx_records = unconfirmedBuf.map { UnsafePointer($0) } + entry.unconfirmed_outgoing_tx_records_count = UInt(unconfirmedCount) + // Provider special transactions (ProRegTx / ProUpServTx / // ProUpRegTx / ProUpRevTx) re-staged onto the provider-key // accounts so #876 retention keeps them and the masternode @@ -7515,6 +7531,98 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { /// transaction table) are skipped — the Rust side has no way to /// reconstruct a transaction without its consensus bytes, so /// projecting an empty row would just bloat the FFI surface. + /// Project the sends this wallet still holds as unconfirmed into the + /// FFI restore array, so the Rust load path can replay their spend + /// effect (see `ClientWalletStartState::unconfirmed_outgoing_txs`). + /// + /// Why this is needed at all: `spendIsInBlock` deliberately withholds + /// `isSpent` from an input whose spender is only in the mempool, + /// because that sighting is reversible by eviction. The UTXO restore + /// therefore hands the input back as spendable, and the balance + /// re-counts the coin. A running app never showed this — it held the + /// spend in memory — and a restart used to recover it only by + /// re-observing the transaction on the network, which never happens + /// for a send that did not reach the network in the first place. + /// + /// The selection is driven from the TXO side rather than the + /// transaction side, which makes the liveness rule fall out for free: + /// a row is offered only while one of *our* outputs still points at it + /// as its spender and is still unspent. A send that already lost a + /// conflict has had its inputs flipped by the winning spender, so it + /// drops out on its own — important, because the FFI restore does not + /// rebuild `observed_spent`, so Rust could not make that judgement. + /// + /// Asset-lock funding transactions are excluded: they ride + /// `unresolved_asset_lock_tx_records` and already have an owner in + /// `resume_asset_lock`. One owner per transaction. + /// Wire-order txids of the funding transactions already carried by + /// `unresolved_asset_lock_tx_records`. Read from the same source that + /// buffer selects from, rather than re-deriving a txid from bytes. + private func unresolvedAssetLockFundingTxids(walletId: Data) -> Set { + let descriptor = FetchDescriptor( + predicate: #Predicate { entry in + entry.walletId == walletId && entry.statusRaw < 2 + } + ) + guard let locks = try? backgroundContext.fetch(descriptor) else { return [] } + var txids = Set() + for lock in locks { + guard let outpoint = decodeOutPointHex(lock.outPointHex) else { continue } + txids.insert(Data(outpoint.prefix(32))) + } + return txids + } + + private func buildUnconfirmedOutgoingTxRecordBuffer( + walletId: Data, + allocation: LoadAllocation, + excludingTxids excluded: Set + ) -> (UnsafeMutablePointer?, Int) { + let descriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId && $0.isSpent == false } + ) + guard let txos = try? backgroundContext.fetch(descriptor), !txos.isEmpty else { + return (nil, 0) + } + + // Distinct spenders, still unconfirmed, still ours to replay. + var candidates: [Data: PersistentTransaction] = [:] + for txo in txos { + guard let spender = txo.spendingTransaction else { continue } + guard spender.context == 0, spender.blockHeight == 0 else { continue } + guard !spender.transactionData.isEmpty else { continue } + guard !excluded.contains(spender.txid) else { continue } + candidates[spender.txid] = spender + } + guard !candidates.isEmpty else { return (nil, 0) } + + // Ascending `firstSeen`: a parent send must be replayed before a + // child that spends its change, or the child finds no input and is + // discarded as irrelevant. + let ordered = candidates.values.sorted { $0.firstSeen < $1.firstSeen } + + var entries: [UnconfirmedOutgoingTxRecordFFI] = [] + entries.reserveCapacity(ordered.count) + for row in ordered { + let txBytes = row.transactionData + let txBuf = UnsafeMutablePointer.allocate(capacity: txBytes.count) + txBytes.copyBytes(to: txBuf, count: txBytes.count) + allocation.scalarBuffers.append((txBuf, txBytes.count)) + var entry = UnconfirmedOutgoingTxRecordFFI() + entry.tx_bytes = txBuf + entry.tx_bytes_len = UInt(txBytes.count) + entry.first_seen = row.firstSeen + entries.append(entry) + } + + let buf = UnsafeMutablePointer.allocate( + capacity: entries.count + ) + buf.initialize(from: entries, count: entries.count) + allocation.unconfirmedOutgoingTxRecordArrays.append((buf, entries.count)) + return (buf, entries.count) + } + private func buildUnresolvedAssetLockTxRecordBuffer( walletId: Data, allocation: LoadAllocation @@ -8654,6 +8762,10 @@ private final class LoadAllocation { /// so the next chain-lock event can cascade-promote them. The /// `tx_bytes` buffer each row references lives in `scalarBuffers`. var unresolvedAssetLockTxRecordArrays: [(UnsafeMutablePointer, Int)] = [] + /// `UnconfirmedOutgoingTxRecordFFI` arrays per wallet. The `tx_bytes` + /// each entry points at are staged on `scalarBuffers`, like the + /// asset-lock records above. + var unconfirmedOutgoingTxRecordArrays: [(UnsafeMutablePointer, Int)] = [] /// Per-wallet `ProviderSpecialTxRestoreEntryFFI` arrays — provider /// special txs re-staged so #876 retention keeps them resident after a /// restart. The `tx_bytes` buffer each row references lives in @@ -8734,6 +8846,10 @@ private final class LoadAllocation { ptr.deinitialize(count: count) ptr.deallocate() } + for (ptr, count) in unconfirmedOutgoingTxRecordArrays { + ptr.deinitialize(count: count) + ptr.deallocate() + } for (ptr, count) in providerSpecialTxRecordArrays { ptr.deinitialize(count: count) ptr.deallocate() From 4d15fbeb90194e38039f45c80630082230509002 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Fri, 11 Sep 2026 08:44:36 +0300 Subject: [PATCH 3/8] test(swift-sdk): pin the selection rule, and stop discarding legacy TXO rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writing the tests surfaced a real defect in the pass they cover. The buffer ran its own `walletId == walletId` fetch, which silently drops rows migrated from the schema that never backfilled that column — ownership there resolves through `account.wallet.walletId`, which is exactly what the caller's bucketing pass already does. On a wallet carrying that history the buffer would have come back empty, no send would have been replayed, and the balance would have stayed wrong with nothing to show for it. Take the bucketed rows instead; that also drops a redundant fetch and picks up the caller's `spendingTransaction` prefetch, which this pass reads for every row. Four tests hold the rule in place: - an unconfirmed send whose input is still ours and still unspent is offered — the case the fix exists for; - a send that already lost a conflict is not. Nothing else can catch this: the FFI restore never rebuilds `observed_spent`, so Rust cannot judge it, and replaying a dead send would re-spend a coin this wallet no longer owns while re-dispatching would put it back on the wire; - a settled send is not, since the chain already carries the spend; - a legacy row with no `walletId` still is — the defect above, pinned so it cannot come back. Refs: support ticket 32189 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HyZwoAc4kS8B7Jq6M5S2c1 --- .../PlatformWalletPersistenceHandler.swift | 17 +- .../UnconfirmedOutgoingSendRestoreTests.swift | 194 ++++++++++++++++++ 2 files changed, 203 insertions(+), 8 deletions(-) create mode 100644 packages/swift-sdk/SwiftTests/SwiftDashSDKTests/UnconfirmedOutgoingSendRestoreTests.swift diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index 27f11f4ddb1..dd79fbf58d7 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -7023,7 +7023,7 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { // owns them. let (unconfirmedBuf, unconfirmedCount) = buildUnconfirmedOutgoingTxRecordBuffer( - walletId: w.walletId, + rows: unspentBuckets[w.walletId] ?? [], allocation: allocation, excludingTxids: unresolvedAssetLockFundingTxids(walletId: w.walletId) ) @@ -7555,6 +7555,12 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { /// Asset-lock funding transactions are excluded: they ride /// `unresolved_asset_lock_tx_records` and already have an owner in /// `resume_asset_lock`. One owner per transaction. + /// + /// Takes the bucketed `isSpent == false` rows the caller already + /// fetched rather than querying by `walletId` again: that bucketing + /// routes a legacy row whose `walletId` was never backfilled through + /// `account.wallet.walletId`, and it prefetches `spendingTransaction`, + /// which this pass reads for every row. /// Wire-order txids of the funding transactions already carried by /// `unresolved_asset_lock_tx_records`. Read from the same source that /// buffer selects from, rather than re-deriving a txid from bytes. @@ -7574,16 +7580,11 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { } private func buildUnconfirmedOutgoingTxRecordBuffer( - walletId: Data, + rows txos: [PersistentTxo], allocation: LoadAllocation, excludingTxids excluded: Set ) -> (UnsafeMutablePointer?, Int) { - let descriptor = FetchDescriptor( - predicate: #Predicate { $0.walletId == walletId && $0.isSpent == false } - ) - guard let txos = try? backgroundContext.fetch(descriptor), !txos.isEmpty else { - return (nil, 0) - } + guard !txos.isEmpty else { return (nil, 0) } // Distinct spenders, still unconfirmed, still ours to replay. var candidates: [Data: PersistentTransaction] = [:] diff --git a/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/UnconfirmedOutgoingSendRestoreTests.swift b/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/UnconfirmedOutgoingSendRestoreTests.swift new file mode 100644 index 00000000000..60de389ef43 --- /dev/null +++ b/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/UnconfirmedOutgoingSendRestoreTests.swift @@ -0,0 +1,194 @@ +import XCTest +import SwiftData +import DashSDKFFI +@testable import SwiftDashSDK + +/// Coverage for the selection rule behind `unconfirmed_outgoing_tx_records`. +/// +/// The spend effect of an unconfirmed outgoing send is never persisted — +/// `spendIsInBlock` withholds `isSpent` from an input whose spender is only +/// in the mempool, because that sighting is reversible by eviction. The UTXO +/// restore therefore hands the input back as spendable, and unless the send +/// is replayed at load the balance re-counts the coin. For a send that never +/// reached the network there is nothing to re-observe, so it stays wrong. +/// +/// What these tests pin down is *which* rows may be offered for that replay. +/// The rule is driven from the TXO side on purpose: a send is offered only +/// while one of our own outputs still names it as its spender and is itself +/// still unspent. That makes liveness fall out for free — a send that lost a +/// conflict has had its input flipped by the winner and drops out on its own, +/// which matters because the FFI restore never rebuilds `observed_spent` and +/// Rust cannot make that judgement for itself. +@MainActor +final class UnconfirmedOutgoingSendRestoreTests: XCTestCase { + + private let walletId = Data(repeating: 0x07, count: 32) + private let fundingTxid = Data(repeating: 0x51, count: 32) + private let sendTxid = Data(repeating: 0x52, count: 32) + private let rivalTxid = Data(repeating: 0x53, count: 32) + private let fundingVout: UInt32 = 0 + + private func makeHandler() throws -> (PlatformWalletPersistenceHandler, ModelContainer) { + let container = try DashModelContainer.createInMemory() + let handler = PlatformWalletPersistenceHandler(modelContainer: container, network: .testnet) + return (handler, container) + } + + /// A plain version-2 transaction spending `input` — the shape + /// `TransactionDecoder` parses, so the fixture cannot drift from what the + /// load path actually reads. + private func serializedSpend(of input: (txid: Data, vout: UInt32)) -> Data { + var bytes = Data() + bytes.append(contentsOf: withUnsafeBytes(of: UInt32(2).littleEndian) { Data($0) }) + bytes.append(0x01) + bytes.append(input.txid) + bytes.append(contentsOf: withUnsafeBytes(of: input.vout.littleEndian) { Data($0) }) + bytes.append(0x00) + bytes.append(contentsOf: [0xff, 0xff, 0xff, 0xff]) + bytes.append(0x01) + bytes.append(contentsOf: withUnsafeBytes(of: UInt64(9_000).littleEndian) { Data($0) }) + bytes.append(0x00) + bytes.append(contentsOf: [0x00, 0x00, 0x00, 0x00]) + return bytes + } + + /// One funded coin, and one transaction recorded as spending it. + /// + /// - `sendContext`/`sendHeight`: the spender's settlement state. + /// - `inputStillOurs`: whether the coin still points at that spender and + /// is still unspent — `false` models a send that lost a conflict, where + /// the winning spender flipped the row. + /// - `legacyTxoWalletId`: a row migrated from the schema that never + /// backfilled `walletId`, whose ownership resolves through the account. + private func seed( + in container: ModelContainer, + sendContext: UInt32 = 0, + sendHeight: UInt32 = 0, + inputStillOurs: Bool = true, + legacyTxoWalletId: Bool = false + ) throws { + let context = ModelContext(container) + let wallet = PersistentWallet(walletId: walletId, network: .testnet) + context.insert(wallet) + let account = PersistentAccount( + wallet: wallet, + accountType: 0, + accountIndex: 0, + accountTypeName: "Standard" + ) + // A wallet only reaches the restore path with at least one account + // carrying an xpub — that is what Rust rebuilds the watch-only + // wallet from. + account.accountExtendedPubKeyBytes = Data(repeating: 0x30, count: 78) + context.insert(account) + + let funding = PersistentTransaction( + txid: fundingTxid, + transactionData: Data(repeating: 0x01, count: 10), + context: 2, + blockHeight: 100, + netAmount: 10_000 + ) + context.insert(funding) + + let send = PersistentTransaction( + txid: sendTxid, + transactionData: serializedSpend(of: (txid: fundingTxid, vout: fundingVout)), + context: sendContext, + blockHeight: sendHeight, + netAmount: -10_000 + ) + context.insert(send) + + let coin = PersistentTxo( + transaction: funding, + vout: fundingVout, + amount: 10_000, + address: "yFundAddr", + height: 100 + ) + coin.account = account + coin.walletId = legacyTxoWalletId ? Data() : walletId + + if inputStillOurs { + coin.isSpent = false + coin.spendingTransaction = send + } else { + // A different, settled transaction took the coin first. The + // winner's flip is what retires our send. + let rival = PersistentTransaction( + txid: rivalTxid, + transactionData: Data(repeating: 0x02, count: 10), + context: 2, + blockHeight: 101, + netAmount: -10_000 + ) + context.insert(rival) + coin.isSpent = true + coin.spendingTransaction = rival + } + context.insert(coin) + + try context.save() + } + + /// Drive the real load path and report how many sends were offered. + private func offeredCount(_ handler: PlatformWalletPersistenceHandler) -> Int { + let loaded = handler.loadWalletList() + XCTAssertFalse(loaded.errored, "the load must not fail") + guard let entries = loaded.entries, loaded.count > 0 else { return -1 } + defer { handler.loadWalletListFree(entries: UnsafeRawPointer(entries)) } + return Int(entries[0].unconfirmed_outgoing_tx_records_count) + } + + /// The case the fix exists for: an unconfirmed send whose input is still + /// ours and still unspent is offered for replay. + func testUnconfirmedSendIsOffered() throws { + let (handler, container) = try makeHandler() + try seed(in: container) + + XCTAssertEqual(offeredCount(handler), 1) + } + + /// A send that already lost a conflict must never be offered: replaying + /// it would re-spend a coin this wallet no longer owns, and + /// re-dispatching it would put a dead transaction back on the wire. + /// Nothing else can catch this — the FFI restore does not rebuild + /// `observed_spent`. + func testSendThatLostAConflictIsNotOffered() throws { + let (handler, container) = try makeHandler() + try seed(in: container, inputStillOurs: false) + + XCTAssertEqual( + offeredCount(handler), + 0, + "the winning spender flipped the input; our send is dead and must not be replayed" + ) + } + + /// A settled send needs no replay: the chain already carries the spend, + /// and the ordinary restore path reconstructs it. + func testConfirmedSendIsNotOffered() throws { + let (handler, container) = try makeHandler() + try seed(in: container, sendContext: 2, sendHeight: 101) + + XCTAssertEqual(offeredCount(handler), 0) + } + + /// A row migrated from the older schema, where `walletId` was never + /// backfilled. Comparing that column raw would discard exactly these + /// rows and silently leave the balance wrong for the wallets most likely + /// to be carrying history — ownership has to resolve through the account, + /// which is why this pass consumes the caller's bucketed rows rather than + /// running its own `walletId` query. + func testLegacyTxoWithNoWalletIdIsStillOffered() throws { + let (handler, container) = try makeHandler() + try seed(in: container, legacyTxoWalletId: true) + + XCTAssertEqual( + offeredCount(handler), + 1, + "a legacy TXO resolving to this wallet through its account must not be discarded" + ) + } +} From 0faf637276caf8c6ce18a8c606533e6dd8318fac Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Fri, 11 Sep 2026 10:55:27 +0300 Subject: [PATCH 4/8] fix(platform-wallet): stop logging the expected re-dispatch answer as a failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second review pass on the merged base. Three corrections, no behaviour change to the replay itself. The comment above the re-dispatch claimed `broadcast_transaction` was used "rather than the awaiting variant". There is no such variant on the platform trait: `TransactionBroadcaster::broadcast` is `broadcast_and_wait`. The code was right and the comment was wrong, but the log followed the comment — `MaybeSent` came out as `warn "re-dispatch failed"`, which is precisely the answer this path expects for the case it exists for. A send that never reached the network goes out, no peer echoes it inside the acceptance window, and the rebroadcast timer takes ownership. Logging the healthy path as a failure is how an investigation gets sent the wrong way, so `MaybeSent` is now `info` and says what actually happened; `warn` is kept for `Rejected`, where nothing carried the transaction at all. `RESEND_TRANSPORT_READY_WAIT` goes 30 s → 90 s. Readiness means the client started AND at least one peer is connected; a simulator gets there in seconds but a cold device on a slow network may not, and giving up early silently defers the send to the next launch — the delay this path exists to remove. Nothing is blocked on the wait. `unresolvedAssetLockFundingTxids` now uses the existing `assetLockFundingTxid(outPointHex:)` instead of decoding the outpoint a second time. Also adds the Rust half of the test coverage: `load_replays_an_unconfirmed_outgoing_send` funds a wallet, hands the loader a send spending its only coin, and requires the balance to be zero afterwards — without the replay the restore hands that input back and the assertion fails on the re-counted coin. Verified on the merged base (v4.2-dev +27, incl. #4582 pooled spendable balance, #4644 frozen SwiftData models): 4 Swift + 1 Rust green. #4582 computes its figure live from `utxos`, so the replay stays consistent with it; #4644 freezes namespaced copies this code does not touch. Refs: support ticket 32189 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HyZwoAc4kS8B7Jq6M5S2c1 --- .../rs-platform-wallet/src/manager/load.rs | 195 ++++++++++++++++-- .../PlatformWalletPersistenceHandler.swift | 12 +- 2 files changed, 182 insertions(+), 25 deletions(-) diff --git a/packages/rs-platform-wallet/src/manager/load.rs b/packages/rs-platform-wallet/src/manager/load.rs index fdedc21a5f4..54e2e11d7ce 100644 --- a/packages/rs-platform-wallet/src/manager/load.rs +++ b/packages/rs-platform-wallet/src/manager/load.rs @@ -12,17 +12,23 @@ use crate::wallet::PlatformWallet; use std::time::Duration; -use crate::broadcaster::TransactionBroadcaster; +use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; use key_wallet::transaction_checking::transaction_context::TransactionContext; use key_wallet::transaction_checking::wallet_checker::WalletTransactionChecker; use super::{run_blocking_load, PlatformWalletManager}; /// How long the load-time re-dispatch waits for the SPV transport before -/// giving up for this launch. Zero connected peers turns a send into a -/// definitive rejection rather than a retry, and there is no urgency: the -/// next launch offers the same transactions again. -const RESEND_TRANSPORT_READY_WAIT: Duration = Duration::from_secs(30); +/// giving up for this launch. Readiness means the client started AND at +/// least one peer is connected; zero peers turns a send into a definitive +/// rejection rather than a retry, so waiting is the cheaper mistake. +/// +/// Generous on purpose: a simulator reaches readiness in seconds, but a +/// cold device on a slow network can take far longer, and giving up early +/// silently defers the send to the next launch — the very delay this whole +/// path exists to remove. Nothing is blocked on the wait; it runs on a +/// detached task. +const RESEND_TRANSPORT_READY_WAIT: Duration = Duration::from_secs(90); impl PlatformWalletManager

{ /// Load the full [`ClientStartState`] from the configured persister @@ -280,11 +286,20 @@ impl PlatformWalletManager

{ // Re-dispatching here hands it back to that timer. // // Deliberately fire-and-forget on a detached task: this must not - // hold up the load, and no verdict is wanted. `broadcast_transaction` - // is used rather than the awaiting variant precisely because the - // timer, not this call, is meant to own the outcome — and an - // unrequested `Uncertain` 60 s later has no listener on the app - // side, so it cannot surface a stray dialog. + // hold up the load, and the verdict is only logged. The platform + // broadcaster has one entry point and it waits for acceptance + // (`TransactionBroadcaster::broadcast` → `broadcast_and_wait`), + // which is harmless here — nothing is blocked on this task, and + // dash-spv has already taken ownership by the time the wait ends. + // The app registers no listener for that event, so a late + // `Uncertain` cannot surface a stray dialog. + // + // For the case this exists for — a send that never reached the + // network — `MaybeSent` is the EXPECTED answer, not a failure: + // the transaction goes out, no peer echoes it back inside the + // acceptance window, and the rebroadcast timer takes it from + // there. Logging that at warn would make the healthy path look + // broken. // // Safe against double-spending: this re-sends the SAME signed // bytes, which is idempotent for the network, and `start_broadcast` @@ -312,14 +327,29 @@ impl PlatformWalletManager

{ } for tx in txs_to_resend { let txid = tx.txid(); - // Goes through the acceptance wait, which is fine on - // a detached task: the verdict is only logged, and - // dash-spv has already taken ownership by then. match broadcaster_for_resend.broadcast(&tx).await { - Ok(_) => tracing::info!(%txid, "load: re-dispatched unconfirmed send"), - Err(e) => { - tracing::warn!(%txid, error = ?e, "load: re-dispatch failed") - } + Ok(_) => tracing::info!( + %txid, + "load: re-dispatched unconfirmed send, accepted" + ), + // Expected for the orphaned case: sent, no + // acceptance signal, now owned by the timer. + Err(BroadcastError::MaybeSent { + reason, + }) => tracing::info!( + %txid, + %reason, + "load: re-dispatched unconfirmed send, no acceptance signal yet — \ + handed to the rebroadcast timer" + ), + // Provably never sent: worth a warning, since + // nothing carried it and the next launch is the + // only remaining chance. + Err(e) => tracing::warn!( + %txid, + error = ?e, + "load: re-dispatch was not sent" + ), } } }); @@ -522,6 +552,70 @@ mod idempotent_load_tests { use crate::wallet::platform_wallet::WalletId; use crate::PlatformWalletManager; + /// Persister that hands back one wallet plus the outgoing sends the + /// host still holds as unconfirmed — the shape `loadWalletList` + /// produces for a send whose broadcast got no acceptance signal. + struct PendingSendPersister { + wallet: Wallet, + managed: ManagedWalletInfo, + pending: Vec, + } + + impl PlatformWalletPersistence for PendingSendPersister { + fn store( + &self, + _wallet_id: WalletId, + _changeset: PlatformWalletChangeSet, + ) -> Result<(), PersistenceError> { + Ok(()) + } + + fn flush(&self, _wallet_id: WalletId) -> Result<(), PersistenceError> { + Ok(()) + } + + fn load(&self) -> Result { + let mut wallets = BTreeMap::new(); + wallets.insert( + self.wallet.compute_wallet_id(), + ClientWalletStartState { + wallet: self.wallet.clone(), + wallet_info: self.managed.clone(), + identity_manager: IdentityManagerStartState::default(), + unused_asset_locks: BTreeMap::new(), + unconfirmed_outgoing_txs: self.pending.clone(), + }, + ); + Ok(ClientStartState { + wallets, + ..Default::default() + }) + } + } + + /// A transaction spending `previous_output` to somewhere that is not + /// this wallet — enough for the mempool check to see the input leave. + fn spend_to(previous_output: dashcore::OutPoint, value: u64) -> dashcore::Transaction { + dashcore::Transaction { + version: 2, + lock_time: 0, + input: vec![dashcore::TxIn { + previous_output, + script_sig: dashcore::ScriptBuf::new(), + sequence: 0xffff_ffff, + witness: Default::default(), + }], + output: vec![dashcore::TxOut { + value, + script_pubkey: dashcore::ScriptBuf::from_hex( + "76a914000000000000000000000000000000000000000088ac", + ) + .expect("static foreign p2pkh script"), + }], + special_transaction_payload: None, + } + } + /// Persister whose `load()` returns a single-wallet snapshot rebuilt /// fresh on every call — `load_from_persistor` moves `wallets` out of /// the returned state, so each hydration needs its own copy. Mirrors a @@ -619,6 +713,73 @@ mod idempotent_load_tests { )) } + /// A send the host still holds as unconfirmed has to be replayed at + /// load, or the coin it spent comes back as spendable. + /// + /// The spend effect is never persisted — `isSpent` stays false on the + /// input row until the spending transaction reaches a block, because a + /// mempool-only sighting is reversible by eviction — so the restored + /// UTXO set hands that input straight back. A running app is still + /// correct, holding the effect in memory; across a restart it used to + /// be recovered only by re-observing the transaction on the network, + /// which never happens for a send that did not reach the network. The + /// balance then re-counted the coin, permanently (support ticket + /// 32189). + /// + /// Asserting on `balance()` rather than on the account internals is + /// deliberate: that is the number the UI reads, and it is mirrored + /// from the replayed state a few lines after the replay runs. + #[tokio::test] + async fn load_replays_an_unconfirmed_outgoing_send() { + let (ctx, funding) = TestWalletContext::new_random() + .with_mempool_funding(100_000) + .await; + let wallet_id = ctx.wallet.compute_wallet_id(); + let funded_outpoint = dashcore::OutPoint { + txid: funding.txid(), + vout: 0, + }; + + // Without a replay this is what the restore alone would leave + // standing, so it is also the failure the assertion below catches. + let spend = spend_to(funded_outpoint, 74_000); + let manager = make_pending_manager(PendingSendPersister { + wallet: ctx.wallet, + managed: ctx.managed_wallet, + pending: vec![spend], + }); + + manager + .load_from_persistor() + .await + .expect("the wallet must load"); + + let wallet = manager + .get_wallet(&wallet_id) + .await + .expect("the loaded wallet must be registered"); + let balance = wallet.balance(); + let total = balance.confirmed() + balance.unconfirmed(); + assert_eq!( + total, 0, + "the replayed send spends the only coin, so nothing may remain \ + spendable; {} duffs left means the input came back", + total + ); + } + + fn make_pending_manager( + persister: PendingSendPersister, + ) -> Arc> { + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let event_handler: Arc = Arc::new(NoopTestEventHandler); + Arc::new(PlatformWalletManager::new( + sdk, + Arc::new(persister), + event_handler, + )) + } + /// The app re-activates its per-network manager on every SDK emission, /// which re-runs `load_from_persistor` against a manager that already /// holds the persisted wallet. The second (and every later) call must diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index dd79fbf58d7..e182a249218 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -7562,8 +7562,9 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { /// `account.wallet.walletId`, and it prefetches `spendingTransaction`, /// which this pass reads for every row. /// Wire-order txids of the funding transactions already carried by - /// `unresolved_asset_lock_tx_records`. Read from the same source that - /// buffer selects from, rather than re-deriving a txid from bytes. + /// `unresolved_asset_lock_tx_records`. Read from the same rows that + /// buffer selects from, through the same decoder, rather than + /// re-deriving a txid from the serialized bytes. private func unresolvedAssetLockFundingTxids(walletId: Data) -> Set { let descriptor = FetchDescriptor( predicate: #Predicate { entry in @@ -7571,12 +7572,7 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { } ) guard let locks = try? backgroundContext.fetch(descriptor) else { return [] } - var txids = Set() - for lock in locks { - guard let outpoint = decodeOutPointHex(lock.outPointHex) else { continue } - txids.insert(Data(outpoint.prefix(32))) - } - return txids + return Set(locks.compactMap { Self.assetLockFundingTxid(outPointHex: $0.outPointHex) }) } private func buildUnconfirmedOutgoingTxRecordBuffer( From 66960932efc15dd5d4c2bc44c72ad876f0786a28 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:13:41 +0300 Subject: [PATCH 5/8] fix(platform-wallet): replay IS-locked sends too, and order the batch by dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on #4659, both real, plus the CI break the first push caused. `rs-unified-sdk-jni` builds `WalletRestoreEntryFFI` with a struct literal, so adding two fields broke the Kotlin build. That is the failure mode the struct's own comment asks for — every field is named explicitly precisely so a new one is a compile error rather than a silently widened `mem::zeroed()` — it just needed the Android side to pass null/0 as well. The replay stays iOS-only for now and is inert there. `spendIsInBlock` withholds `isSpent` for every context below `inBlock`, so an InstantSend-locked send (context 1) leaves its input unspent in the store exactly as a mempool send does. The buffer filtered on `context == 0`, covering only half of the rule it was meant to mirror, and IS-locked sends were left out of the replay. Ordering the batch by `first_seen` alone was unsound: the host records it in whole seconds, so a parent and the child spending its change can share one and their relative order was undefined. A child replayed first has no input to spend, is discarded as irrelevant, and that send's replay is lost with no trace. The sort now only sets a baseline, and `order_unconfirmed_outgoing` moves any send that spends another send in the same batch behind it — bounded, so a cycle degrades to `first_seen` order instead of spinning. Tests: `unconfirmed_outgoing_order` covers a same-second parent/child pair offered child-first, a fully reversed three-link chain, and independent sends keeping their baseline order; `testInstantSendLockedSendIsOffered` covers the context rule. 6 Swift + 4 Rust green. Refs: support ticket 32189 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HyZwoAc4kS8B7Jq6M5S2c1 --- .../rs-platform-wallet-ffi/src/persistence.rs | 157 +++++++++++++++++- .../rs-unified-sdk-jni/src/persistence.rs | 5 + .../PlatformWalletPersistenceHandler.swift | 8 +- .../UnconfirmedOutgoingSendRestoreTests.swift | 16 ++ 4 files changed, 180 insertions(+), 6 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 918fd151dcf..14dafbb6fe8 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -4853,6 +4853,45 @@ impl Drop for LoadGuard { } } +/// Put a batch of unconfirmed outgoing sends into replay order. +/// +/// `first_seen` establishes the baseline, but the host records it in whole +/// seconds, so two sends a moment apart share one and their relative order is +/// undefined. A dependency pass then moves any send that spends another send +/// in the same batch behind it — replaying a child first leaves it with no +/// input to spend, so it is discarded as irrelevant and that send's replay is +/// silently lost. +fn order_unconfirmed_outgoing( + mut decoded: Vec<(u64, dashcore::blockdata::transaction::Transaction)>, +) -> Vec { + decoded.sort_by_key(|(first_seen, _)| *first_seen); + + let in_batch: std::collections::HashSet<_> = + decoded.iter().map(|(_, tx)| tx.txid()).collect(); + let mut emitted: std::collections::HashSet<_> = std::collections::HashSet::new(); + let mut ordered = Vec::with_capacity(decoded.len()); + let mut queue: std::collections::VecDeque<_> = decoded.into_iter().collect(); + // Bounded: a full lap with nothing emitted means the remainder depends on + // itself, which valid transactions cannot do. Emit in `first_seen` order + // rather than spin. + let mut passed_over = 0usize; + while let Some((first_seen, tx)) = queue.pop_front() { + let waits_on_batch_peer = tx.input.iter().any(|input| { + let parent = input.previous_output.txid; + in_batch.contains(&parent) && !emitted.contains(&parent) + }); + if waits_on_batch_peer && passed_over <= queue.len() { + queue.push_back((first_seen, tx)); + passed_over += 1; + continue; + } + emitted.insert(tx.txid()); + ordered.push(tx); + passed_over = 0; + } + ordered +} + /// Reconstruct an external-signable [`Wallet`] + matching start-state /// bucket from a single `WalletRestoreEntryFFI`. The mnemonic / seed /// stays in the host's keychain; signing requests route back through @@ -5491,9 +5530,15 @@ fn build_wallet_start_state( // `manager::load::load_from_persistor`. See // `ClientWalletStartState::unconfirmed_outgoing_txs`. // - // Sorted by the host's `first_seen` so a parent send is replayed - // before a child that spends its change; a child applied first - // would find its input still absent and be dropped as irrelevant. + // Ordered so a parent send is replayed before a child that spends its + // change — a child applied first finds its input absent and is dropped + // as irrelevant, silently losing that send's replay. + // + // `first_seen` alone cannot express this: the host stores it in whole + // seconds, and two sends a moment apart share one. So the `first_seen` + // sort only establishes a stable starting order, and a dependency pass + // then moves any send that spends another send in the same batch behind + // it. let unconfirmed_outgoing_txs = { use dashcore::consensus::Decodable; let recs: &[UnconfirmedOutgoingTxRecordFFI] = if entry @@ -5533,8 +5578,7 @@ fn build_wallet_start_state( "load: unconfirmed outgoing tx records failed to decode" ); } - decoded.sort_by_key(|(first_seen, _)| *first_seen); - decoded.into_iter().map(|(_, tx)| tx).collect::>() + order_unconfirmed_outgoing(decoded) }; let wallet_state = ClientWalletStartState { @@ -6714,6 +6758,109 @@ mod tests { //! restoration loops that don't need the full FFI plumbing — //! exercising the in-memory mutation against synthetic input. + mod unconfirmed_outgoing_order { + use super::super::order_unconfirmed_outgoing; + use dashcore::blockdata::transaction::Transaction; + use dashcore::{OutPoint, ScriptBuf, TxIn, TxOut}; + + fn tx_spending(parents: &[(dashcore::Txid, u32)], value: u64) -> Transaction { + Transaction { + version: 2, + lock_time: 0, + input: parents + .iter() + .map(|(txid, vout)| TxIn { + previous_output: OutPoint { + txid: *txid, + vout: *vout, + }, + script_sig: ScriptBuf::new(), + sequence: 0xffff_ffff, + witness: Default::default(), + }) + .collect(), + output: vec![TxOut { + value, + script_pubkey: ScriptBuf::new(), + }], + special_transaction_payload: None, + } + } + + fn root(value: u64) -> Transaction { + tx_spending( + &[( + "0000000000000000000000000000000000000000000000000000000000000001" + .parse() + .expect("static txid"), + 0, + )], + value, + ) + } + + /// The host stores `first_seen` in whole seconds, so a parent and the + /// child spending its change can share one. Replaying the child first + /// leaves it with no input and it is dropped as irrelevant — that + /// send's replay is then silently lost, which is the whole failure + /// this ordering exists to prevent. + #[test] + fn a_child_sharing_its_parents_second_is_replayed_after_it() { + let parent = root(50_000); + let child = tx_spending(&[(parent.txid(), 0)], 40_000); + + // Child offered first, identical timestamps: nothing but the + // dependency pass can separate them. + let ordered = order_unconfirmed_outgoing(vec![ + (1_700_000_000, child.clone()), + (1_700_000_000, parent.clone()), + ]); + + assert_eq!( + ordered.iter().map(|tx| tx.txid()).collect::>(), + vec![parent.txid(), child.txid()], + "the parent must be replayed before the child that spends it" + ); + } + + /// A chain of three, offered fully reversed and all in one second. + #[test] + fn a_reversed_chain_is_restored_to_dependency_order() { + let a = root(90_000); + let b = tx_spending(&[(a.txid(), 0)], 80_000); + let c = tx_spending(&[(b.txid(), 0)], 70_000); + + let ordered = order_unconfirmed_outgoing(vec![ + (1_700_000_000, c.clone()), + (1_700_000_000, b.clone()), + (1_700_000_000, a.clone()), + ]); + + assert_eq!( + ordered.iter().map(|tx| tx.txid()).collect::>(), + vec![a.txid(), b.txid(), c.txid()] + ); + } + + /// Sends that do not depend on each other keep the order `first_seen` + /// gave them — the dependency pass must not reshuffle the baseline. + #[test] + fn independent_sends_keep_their_first_seen_order() { + let older = root(10_000); + let newer = root(20_000); + + let ordered = order_unconfirmed_outgoing(vec![ + (1_700_000_050, newer.clone()), + (1_700_000_000, older.clone()), + ]); + + assert_eq!( + ordered.iter().map(|tx| tx.txid()).collect::>(), + vec![older.txid(), newer.txid()] + ); + } + } + use super::*; // --- persists_durably: the fail-closed durability attestation --- diff --git a/packages/rs-unified-sdk-jni/src/persistence.rs b/packages/rs-unified-sdk-jni/src/persistence.rs index 732534d25f0..333a719d2ea 100644 --- a/packages/rs-unified-sdk-jni/src/persistence.rs +++ b/packages/rs-unified-sdk-jni/src/persistence.rs @@ -2582,6 +2582,11 @@ fn build_wallet_restore_entry( tracked_asset_locks_count: 0, unresolved_asset_lock_tx_records: ptr::null(), unresolved_asset_lock_tx_records_count: 0, + // Android does not stage unconfirmed outgoing sends yet: the replay + // that consumes them is wired on the iOS path only. Null/0 leaves it + // inert here, exactly as it was before the field existed. + unconfirmed_outgoing_tx_records: ptr::null(), + unconfirmed_outgoing_tx_records_count: 0, core_address_pools: ptr::null(), core_address_pools_count: 0, last_applied_chain_lock_bytes: ptr::null(), diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index e182a249218..28c2d152142 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -7586,7 +7586,13 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { var candidates: [Data: PersistentTransaction] = [:] for txo in txos { guard let spender = txo.spendingTransaction else { continue } - guard spender.context == 0, spender.blockHeight == 0 else { continue } + // Mirror `spendIsInBlock` exactly: it withholds `isSpent` for + // every context below `inBlock`, so an InstantSend-locked send + // (context 1) leaves its input unspent in the store too and needs + // the same replay. Filtering on `== 0` covered only half of that. + guard spender.context < TransactionContextType.inBlock.rawValue, + spender.blockHeight == 0 + else { continue } guard !spender.transactionData.isEmpty else { continue } guard !excluded.contains(spender.txid) else { continue } candidates[spender.txid] = spender diff --git a/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/UnconfirmedOutgoingSendRestoreTests.swift b/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/UnconfirmedOutgoingSendRestoreTests.swift index 60de389ef43..18be5e43ffd 100644 --- a/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/UnconfirmedOutgoingSendRestoreTests.swift +++ b/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/UnconfirmedOutgoingSendRestoreTests.swift @@ -166,6 +166,22 @@ final class UnconfirmedOutgoingSendRestoreTests: XCTestCase { ) } + /// An InstantSend-locked send is still unconfirmed as far as the store is + /// concerned — `spendIsInBlock` withholds `isSpent` for every context + /// below `inBlock`, so its input is handed back as spendable exactly like + /// a mempool send's. Filtering on `context == 0` covered only half of + /// that rule and left IS-locked sends out of the replay. + func testInstantSendLockedSendIsOffered() throws { + let (handler, container) = try makeHandler() + try seed(in: container, sendContext: 1) + + XCTAssertEqual( + offeredCount(handler), + 1, + "an IS-locked send has not reached a block, so its spend is not persisted either" + ) + } + /// A settled send needs no replay: the chain already carries the spend, /// and the ordinary restore path reconstructs it. func testConfirmedSendIsNotOffered() throws { From 02d8e0d8c451b6872ce589e1802644ac6fcf8e9f Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Fri, 11 Sep 2026 16:52:37 +0300 Subject: [PATCH 6/8] fix(platform-wallet): tie the re-dispatch to the wallet it was loaded for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on #4659. **The detached re-dispatch could outlive its wallet.** It was spawned inside the load loop, before platform-address initialization, so a later iteration's failure would roll the registration back while the task sat waiting on transport readiness — and it would then broadcast on behalf of a wallet the manager no longer had. The resends are now queued during the loop and spawned past the rollback point, so a failed load never leaves one behind, and each task re-checks that its wallet is still the live registration before putting anything on the wire. The check is `Arc::ptr_eq` against the generation it was created for, the same rule `rollback_targets` applies: an id can be freed and re-registered under a different generation, and that wallet is not ours to broadcast for. Cancelling the task at teardown was the other option offered. It would need a cancellation channel the manager does not have today; re-validating at the point of use closes the same hole without inventing one. **Records are now required to hash to their row.** The FFI record carries only `first_seen` and bytes, and the replay applies each transaction through the ordinary state-update path — so a stale or partially-written `transactionData` would not merely be ignored, it would move accounting for whatever inputs and outputs those bytes describe. `UnconfirmedOutgoingTxRecordFFI` now carries the expected txid and a record that does not decode to it is dropped with a warning. **A failed asset-lock lookup no longer reads as "no asset locks".** `unresolvedAssetLockFundingTxids` turned every fetch error into an empty exclusion set, which would let an asset-lock funding transaction into the ordinary replay even though `resume_asset_lock` owns it. It returns `nil` on failure now and the buffer offers nothing at all: one launch without a replay beats applying a transaction through the wrong path. Also fixes the `cargo fmt` break that failed CI on the previous push. Tests: `a_record_that_does_not_hash_to_its_row_is_dropped` offers two records carrying the same bytes under different txids — without the identity check both would replay. 5 Rust + 6 Swift green. Refs: support ticket 32189 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HyZwoAc4kS8B7Jq6M5S2c1 --- .../rs-platform-wallet-ffi/src/persistence.rs | 166 +++++++++++++----- .../src/wallet_restore_types.rs | 8 + .../rs-platform-wallet/src/manager/load.rs | 132 +++++++++----- .../PlatformWalletPersistenceHandler.swift | 32 +++- 4 files changed, 245 insertions(+), 93 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 14dafbb6fe8..afb2c55d80e 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -67,8 +67,7 @@ use crate::wallet_restore_types::{ AccountSpecFFI, AccountTypeTagFFI, ContactProfileRestoreEntryFFI, IdentityKeyRestoreFFI, IdentityRestoreEntryFFI, LoadWalletListFreeFn, PaymentRestoreEntryFFI, ProviderSpecialTxRestoreEntryFFI, StandardAccountTypeTagFFI, UnconfirmedOutgoingTxRecordFFI, - UnresolvedAssetLockTxRecordFFI, - UtxoRestoreEntryFFI, WalletRestoreEntryFFI, + UnresolvedAssetLockTxRecordFFI, UtxoRestoreEntryFFI, WalletRestoreEntryFFI, }; use dpp::address_funds::PlatformAddress; use dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; @@ -4853,6 +4852,70 @@ impl Drop for LoadGuard { } } +/// Decode the unconfirmed outgoing sends the host staged for replay. +/// +/// Fail-closed on identity: a record must decode AND hash to the txid of the +/// row it was selected from. The replay applies each transaction through the +/// ordinary state-update path, so bytes that do not belong to that row would +/// move accounting for inputs and outputs unrelated to the send — a stale or +/// partially-written `transactionData` must drop out rather than be applied. +fn decode_unconfirmed_outgoing( + entry: &WalletRestoreEntryFFI, +) -> Vec { + use dashcore::consensus::Decodable; + use dashcore::hashes::Hash; + let recs: &[UnconfirmedOutgoingTxRecordFFI] = if entry.unconfirmed_outgoing_tx_records.is_null() + || entry.unconfirmed_outgoing_tx_records_count == 0 + { + &[] + } else { + unsafe { + slice::from_raw_parts( + entry.unconfirmed_outgoing_tx_records, + entry.unconfirmed_outgoing_tx_records_count, + ) + } + }; + let mut decoded: Vec<(u64, dashcore::blockdata::transaction::Transaction)> = + Vec::with_capacity(recs.len()); + let mut dropped_decode = 0usize; + let mut dropped_identity = 0usize; + for rec in recs { + if rec.tx_bytes.is_null() || rec.tx_bytes_len == 0 { + dropped_decode += 1; + continue; + } + let bytes = unsafe { slice::from_raw_parts(rec.tx_bytes, rec.tx_bytes_len) }; + match dashcore::blockdata::transaction::Transaction::consensus_decode(&mut &bytes[..]) { + // The bytes must be the row they were selected from. The + // replay runs through the ordinary state-update path, so a + // stale or partially-written `transactionData` would apply a + // different transaction and move accounting for inputs and + // outputs unrelated to this send. + Ok(tx) if *tx.txid().as_byte_array() == rec.txid => decoded.push((rec.first_seen, tx)), + Ok(tx) => { + tracing::warn!( + wallet_id = %hex::encode(entry.wallet_id), + expected = %hex::encode(rec.txid), + decoded = %tx.txid(), + "load: unconfirmed outgoing record does not hash to its row; dropped" + ); + dropped_identity += 1; + } + Err(_) => dropped_decode += 1, + } + } + if dropped_decode > 0 || dropped_identity > 0 { + tracing::warn!( + wallet_id = %hex::encode(entry.wallet_id), + dropped_decode, + dropped_identity, + "load: unconfirmed outgoing tx records were dropped" + ); + } + order_unconfirmed_outgoing(decoded) +} + /// Put a batch of unconfirmed outgoing sends into replay order. /// /// `first_seen` establishes the baseline, but the host records it in whole @@ -4866,8 +4929,7 @@ fn order_unconfirmed_outgoing( ) -> Vec { decoded.sort_by_key(|(first_seen, _)| *first_seen); - let in_batch: std::collections::HashSet<_> = - decoded.iter().map(|(_, tx)| tx.txid()).collect(); + let in_batch: std::collections::HashSet<_> = decoded.iter().map(|(_, tx)| tx.txid()).collect(); let mut emitted: std::collections::HashSet<_> = std::collections::HashSet::new(); let mut ordered = Vec::with_capacity(decoded.len()); let mut queue: std::collections::VecDeque<_> = decoded.into_iter().collect(); @@ -5539,47 +5601,7 @@ fn build_wallet_start_state( // sort only establishes a stable starting order, and a dependency pass // then moves any send that spends another send in the same batch behind // it. - let unconfirmed_outgoing_txs = { - use dashcore::consensus::Decodable; - let recs: &[UnconfirmedOutgoingTxRecordFFI] = if entry - .unconfirmed_outgoing_tx_records - .is_null() - || entry.unconfirmed_outgoing_tx_records_count == 0 - { - &[] - } else { - unsafe { - slice::from_raw_parts( - entry.unconfirmed_outgoing_tx_records, - entry.unconfirmed_outgoing_tx_records_count, - ) - } - }; - let mut decoded: Vec<(u64, dashcore::blockdata::transaction::Transaction)> = - Vec::with_capacity(recs.len()); - let mut dropped_decode = 0usize; - for rec in recs { - if rec.tx_bytes.is_null() || rec.tx_bytes_len == 0 { - dropped_decode += 1; - continue; - } - let bytes = unsafe { slice::from_raw_parts(rec.tx_bytes, rec.tx_bytes_len) }; - match dashcore::blockdata::transaction::Transaction::consensus_decode( - &mut &bytes[..], - ) { - Ok(tx) => decoded.push((rec.first_seen, tx)), - Err(_) => dropped_decode += 1, - } - } - if dropped_decode > 0 { - tracing::warn!( - wallet_id = %hex::encode(entry.wallet_id), - dropped_decode, - "load: unconfirmed outgoing tx records failed to decode" - ); - } - order_unconfirmed_outgoing(decoded) - }; + let unconfirmed_outgoing_txs = decode_unconfirmed_outgoing(entry); let wallet_state = ClientWalletStartState { wallet, @@ -6842,6 +6864,60 @@ mod tests { ); } + /// A record whose bytes do not hash to the txid of the row it came + /// from is dropped, not replayed. + /// + /// The replay runs through the ordinary state-update path, so a stale + /// or partially-written `transactionData` would not merely be ignored + /// — it would move accounting for whatever inputs and outputs those + /// bytes happen to describe. + #[test] + fn a_record_that_does_not_hash_to_its_row_is_dropped() { + use crate::wallet_restore_types::{ + UnconfirmedOutgoingTxRecordFFI, WalletRestoreEntryFFI, + }; + use dashcore::consensus::encode::serialize; + + let honest = root(50_000); + let impostor = root(60_000); + + let mut honest_bytes = serialize(&honest); + let mut impostor_bytes = serialize(&impostor); + let honest_txid = *dashcore::hashes::Hash::as_byte_array(&honest.txid()); + let impostor_txid = *dashcore::hashes::Hash::as_byte_array(&impostor.txid()); + + let records = [ + UnconfirmedOutgoingTxRecordFFI { + txid: honest_txid, + tx_bytes: honest_bytes.as_mut_ptr(), + tx_bytes_len: honest_bytes.len(), + first_seen: 1_700_000_000, + }, + // Same shape, but the bytes belong to a different transaction. + UnconfirmedOutgoingTxRecordFFI { + txid: impostor_txid, + tx_bytes: honest_bytes.as_mut_ptr(), + tx_bytes_len: honest_bytes.len(), + first_seen: 1_700_000_001, + }, + ]; + + let entry = WalletRestoreEntryFFI { + unconfirmed_outgoing_tx_records: records.as_ptr(), + unconfirmed_outgoing_tx_records_count: records.len(), + ..Default::default() + }; + + let decoded = super::super::decode_unconfirmed_outgoing(&entry); + + assert_eq!( + decoded.iter().map(|tx| tx.txid()).collect::>(), + vec![honest.txid()], + "only the record whose bytes match its row may be replayed" + ); + let _ = impostor_bytes.as_mut_ptr(); + } + /// Sends that do not depend on each other keep the order `first_seen` /// gave them — the dependency pass must not reshuffle the baseline. #[test] diff --git a/packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs b/packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs index ffd91f761a7..14d620db7c4 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs @@ -577,6 +577,14 @@ pub struct ProviderSpecialTxRestoreEntryFFI { /// replayed at load so its spend effect survives a restart. #[repr(C)] pub struct UnconfirmedOutgoingTxRecordFFI { + /// Wire-order txid of the row this record came from. + /// + /// The load path decodes `tx_bytes` and requires the result to hash to + /// this, then drops the record if it does not. The replay applies the + /// transaction through the ordinary state-update path, so bytes that do + /// not belong to the row Swift selected would rewrite accounting for + /// inputs and outputs nobody asked about. Fail closed instead. + pub txid: [u8; 32], /// Consensus-encoded transaction body, the same wire format /// `dashcore::consensus::encode::serialize` produces. Swift-owned /// for the callback window; freed by `LoadWalletListFreeFn`. diff --git a/packages/rs-platform-wallet/src/manager/load.rs b/packages/rs-platform-wallet/src/manager/load.rs index 54e2e11d7ce..776b67eaeaf 100644 --- a/packages/rs-platform-wallet/src/manager/load.rs +++ b/packages/rs-platform-wallet/src/manager/load.rs @@ -107,6 +107,15 @@ impl PlatformWalletManager

{ // boundary with no Swift-side reset path, so transactional // semantics matter for this hydration API. let mut inserted_in_manager: Vec = Vec::new(); + // Re-dispatches owed by this load, held until the rollback point has + // passed. See the push site for why they cannot be spawned inline. + #[allow(clippy::type_complexity)] + let mut pending_resends: Vec<( + WalletId, + Arc, + Arc, + Vec, + )> = Vec::new(); // The generation travels with the id: a rollback may only remove the // registration THIS call published (see the rollback block below). let mut inserted_in_wallets: Vec<(WalletId, Arc)> = @@ -308,51 +317,19 @@ impl PlatformWalletManager

{ // selectable again and this wallet could sign a conflicting // transaction. That is why the two halves ship together. if !unconfirmed_outgoing_txs.is_empty() { - let broadcaster_for_resend = Arc::clone(&broadcaster); - let txs_to_resend = unconfirmed_outgoing_txs.clone(); - tokio::spawn(async move { - // Zero connected peers makes the send a definitive - // `Rejected` rather than a retry, so wait for the - // transport before offering anything. - if !broadcaster_for_resend - .wait_until_ready(RESEND_TRANSPORT_READY_WAIT) - .await - { - tracing::warn!( - pending = txs_to_resend.len(), - "load: broadcast transport not ready; leaving unconfirmed \ - sends for the next launch" - ); - return; - } - for tx in txs_to_resend { - let txid = tx.txid(); - match broadcaster_for_resend.broadcast(&tx).await { - Ok(_) => tracing::info!( - %txid, - "load: re-dispatched unconfirmed send, accepted" - ), - // Expected for the orphaned case: sent, no - // acceptance signal, now owned by the timer. - Err(BroadcastError::MaybeSent { - reason, - }) => tracing::info!( - %txid, - %reason, - "load: re-dispatched unconfirmed send, no acceptance signal yet — \ - handed to the rebroadcast timer" - ), - // Provably never sent: worth a warning, since - // nothing carried it and the next launch is the - // only remaining chance. - Err(e) => tracing::warn!( - %txid, - error = ?e, - "load: re-dispatch was not sent" - ), - } - } - }); + // Queued, not spawned: a later iteration can still fail and + // roll this registration back, and a task already waiting on + // transport readiness would outlive it and rebroadcast for a + // wallet that no longer exists. Spawned after the rollback + // point instead, with the generation carried along so the + // task can tell whether the registration it was created for + // is still the live one. + pending_resends.push(( + wallet_id, + Arc::clone(&generation), + Arc::clone(&broadcaster), + unconfirmed_outgoing_txs, + )); } let platform_wallet = PlatformWallet::new( @@ -500,6 +477,71 @@ impl PlatformWalletManager

{ return Err(err); } + // Past the rollback point: every registration here is one this load + // actually committed, so the transactions now have a wallet to belong + // to for as long as it stays registered. + // + // Detached on purpose — nothing may block the load on transport + // readiness — which is why each task re-checks that its wallet is + // still the live registration before putting anything on the wire. A + // wallet removed while the task waits leaves the generation pointer + // pointing at nothing the map holds any more, and the re-dispatch is + // abandoned rather than broadcasting on behalf of a wallet that is + // gone. + for (wallet_id, generation, broadcaster, txs) in pending_resends { + let wallets = Arc::clone(&self.wallets); + tokio::spawn(async move { + if !broadcaster + .wait_until_ready(RESEND_TRANSPORT_READY_WAIT) + .await + { + tracing::warn!( + pending = txs.len(), + "load: broadcast transport not ready; leaving unconfirmed \ + sends for the next launch" + ); + return; + } + let still_live = wallets + .load() + .get(&wallet_id) + .is_some_and(|wallet| Arc::ptr_eq(wallet.generation(), &generation)); + if !still_live { + tracing::info!( + wallet_id = %hex::encode(wallet_id), + pending = txs.len(), + "load: wallet no longer registered; abandoning the re-dispatch" + ); + return; + } + for tx in txs { + let txid = tx.txid(); + match broadcaster.broadcast(&tx).await { + Ok(_) => tracing::info!( + %txid, + "load: re-dispatched unconfirmed send, accepted" + ), + // Expected for the orphaned case: sent, no acceptance + // signal, now owned by the rebroadcast timer. + Err(BroadcastError::MaybeSent { reason }) => tracing::info!( + %txid, + %reason, + "load: re-dispatched unconfirmed send, no acceptance signal yet — \ + handed to the rebroadcast timer" + ), + // Provably never sent: worth a warning, since nothing + // carried it and the next launch is the only remaining + // chance. + Err(e) => tracing::warn!( + %txid, + error = ?e, + "load: re-dispatch was not sent" + ), + } + } + }); + } + Ok(()) } } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index 28c2d152142..9eff79d990d 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -7565,21 +7565,38 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { /// `unresolved_asset_lock_tx_records`. Read from the same rows that /// buffer selects from, through the same decoder, rather than /// re-deriving a txid from the serialized bytes. - private func unresolvedAssetLockFundingTxids(walletId: Data) -> Set { + private func unresolvedAssetLockFundingTxids(walletId: Data) -> Set? { let descriptor = FetchDescriptor( predicate: #Predicate { entry in entry.walletId == walletId && entry.statusRaw < 2 } ) - guard let locks = try? backgroundContext.fetch(descriptor) else { return [] } + // `nil`, not an empty set, when the fetch fails: an empty exclusion + // set reads as "this wallet has no unresolved asset locks", which + // would let a funding transaction into the ordinary replay even + // though `resume_asset_lock` owns it. The caller offers nothing at + // all instead — one launch without a replay, rather than a + // transaction applied through the wrong path. + guard let locks = try? backgroundContext.fetch(descriptor) else { return nil } return Set(locks.compactMap { Self.assetLockFundingTxid(outPointHex: $0.outPointHex) }) } private func buildUnconfirmedOutgoingTxRecordBuffer( rows txos: [PersistentTxo], allocation: LoadAllocation, - excludingTxids excluded: Set + excludingTxids excluded: Set? ) -> (UnsafeMutablePointer?, Int) { + // Fail closed: without a trustworthy exclusion set we cannot tell an + // asset-lock funding transaction from an ordinary send. + guard let excluded else { + SDKLogger.event( + "persistence_unconfirmed_outgoing_skipped", + category: .persistence, + severity: .error, + fields: ["reason": .publicText("asset_lock_exclusion_fetch_failed")] + ) + return (nil, 0) + } guard !txos.isEmpty else { return (nil, 0) } // Distinct spenders, still unconfirmed, still ours to replay. @@ -7608,10 +7625,19 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { entries.reserveCapacity(ordered.count) for row in ordered { let txBytes = row.transactionData + // Carry the row's identity so Rust can refuse bytes that do not + // hash to it. The replay applies the transaction through the + // ordinary state-update path, so a stale or partially-written + // `transactionData` would move accounting for inputs and outputs + // that have nothing to do with this send. + guard row.txid.count == 32 else { continue } let txBuf = UnsafeMutablePointer.allocate(capacity: txBytes.count) txBytes.copyBytes(to: txBuf, count: txBytes.count) allocation.scalarBuffers.append((txBuf, txBytes.count)) var entry = UnconfirmedOutgoingTxRecordFFI() + withUnsafeMutableBytes(of: &entry.txid) { raw in + raw.copyBytes(from: row.txid) + } entry.tx_bytes = txBuf entry.tx_bytes_len = UInt(txBytes.count) entry.first_seen = row.firstSeen From c383d0d107dcefb497ee9b77bc8a73ff33f57d5f Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Fri, 11 Sep 2026 17:44:10 +0300 Subject: [PATCH 7/8] fix(platform-wallet-storage): stage the replay field in the SQLite persister too `platform-wallet-storage` (the embeddable SQLite backend that landed in #3968, which arrived with the v4.2-dev merge on this branch) builds `ClientWalletStartState` as well, so the new field left it uncompilable and clippy failed on the workspace. Empty, like the JNI path: this backend does not stage unconfirmed outgoing sends for replay, and the FFI persister is the only producer today. Inert here, which is what this path did before the field existed. Refs: support ticket 32189 Co-Authored-By: Claude Opus 5 (1M context) --- packages/rs-platform-wallet-storage/src/sqlite/persister.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/rs-platform-wallet-storage/src/sqlite/persister.rs b/packages/rs-platform-wallet-storage/src/sqlite/persister.rs index 03560ac2b89..fd865405121 100644 --- a/packages/rs-platform-wallet-storage/src/sqlite/persister.rs +++ b/packages/rs-platform-wallet-storage/src/sqlite/persister.rs @@ -1854,6 +1854,11 @@ fn load_one_wallet( wallet_info, identity_manager, unused_asset_locks, + // This backend does not stage unconfirmed outgoing sends for replay + // yet; the FFI persister is the only producer today. Empty leaves the + // replay inert here, which is the behaviour this path had before the + // field existed. + unconfirmed_outgoing_txs: Vec::new(), }) } From 9ae3ec80e2a67895a97b2c46f621e774ce233d48 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Fri, 11 Sep 2026 17:58:29 +0300 Subject: [PATCH 8/8] fix(platform-wallet): hold the lifecycle gate across the liveness check and the broadcast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding on #4659. The re-dispatch checked `Arc::ptr_eq` against the generation it was loaded for and then broadcast — but that check is a point-in-time observation, and each broadcast waits for an acceptance signal. Teardown can take the exclusive side of the lifecycle gate in between, so a wallet removed while the task was waiting would still have its transaction put on the wire. `WalletGeneration::payment_guard` exists for exactly this pairing and its contract says so: hold it across the check *and* the publication step. It is now taken per transaction, with the registration re-read under it, in the lock order the gate documents — gate first, wallet-manager read lock second. Per transaction rather than once around the batch on purpose: the gate blocks teardown, and each broadcast waits out an acceptance window, so holding it for a whole batch would stall a removal for minutes. Refs: support ticket 32189 Co-Authored-By: Claude Opus 5 (1M context) --- .../rs-platform-wallet/src/manager/load.rs | 40 +++++++++++++------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/packages/rs-platform-wallet/src/manager/load.rs b/packages/rs-platform-wallet/src/manager/load.rs index 776b67eaeaf..050dd4ae320 100644 --- a/packages/rs-platform-wallet/src/manager/load.rs +++ b/packages/rs-platform-wallet/src/manager/load.rs @@ -489,7 +489,7 @@ impl PlatformWalletManager

{ // abandoned rather than broadcasting on behalf of a wallet that is // gone. for (wallet_id, generation, broadcaster, txs) in pending_resends { - let wallets = Arc::clone(&self.wallets); + let wallet_manager = Arc::clone(&self.wallet_manager); tokio::spawn(async move { if !broadcaster .wait_until_ready(RESEND_TRANSPORT_READY_WAIT) @@ -502,20 +502,34 @@ impl PlatformWalletManager

{ ); return; } - let still_live = wallets - .load() - .get(&wallet_id) - .is_some_and(|wallet| Arc::ptr_eq(wallet.generation(), &generation)); - if !still_live { - tracing::info!( - wallet_id = %hex::encode(wallet_id), - pending = txs.len(), - "load: wallet no longer registered; abandoning the re-dispatch" - ); - return; - } for tx in txs { let txid = tx.txid(); + // The lifecycle gate, held across the liveness check AND + // the network step — that pairing is the whole contract + // (`WalletGeneration::payment_guard`). A bare + // `Arc::ptr_eq` is a point-in-time observation, and + // teardown can take the exclusive side between it and the + // broadcast, so a removed wallet's transaction would still + // go out. Re-taken per transaction rather than once around + // the loop: each broadcast waits for an acceptance signal, + // and holding the gate across all of them would stall a + // removal for as long as the whole batch takes. + // + // Lock order is the one the gate documents: this first, + // the wallet-manager read lock second, never the reverse. + let _payment = generation.payment_guard().await; + let still_live = { + let wm = wallet_manager.read().await; + wm.get_wallet_info(&wallet_id) + .is_some_and(|info| Arc::ptr_eq(&info.generation, &generation)) + }; + if !still_live { + tracing::info!( + wallet_id = %hex::encode(wallet_id), + "load: wallet no longer registered; abandoning the re-dispatch" + ); + return; + } match broadcaster.broadcast(&tx).await { Ok(_) => tracing::info!( %txid,