From 757f035ce41475dda7b12e470ac9a12782a262f5 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 7 May 2026 23:37:26 +0700 Subject: [PATCH 1/6] fix(platform-wallet): fall back to persister for chainlocked asset-lock tx records MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five asset-lock proof lookup sites in `wallet/asset_lock/sync/{proof,recovery}.rs` resolve a transaction record from the in-memory `transactions()` map keyed by `(account_index, txid)`. With upstream's `keep-finalized-transactions` Cargo feature OFF (the default), chainlocked records are evicted from that map and only their txids retained in `finalized_txids` for dedup. The chain-lock height / block hash that an asset-lock proof needs is no longer reachable through the wallet-info API. The narrow failure mode is "first lookup happens after the chainlock-eviction window" — `wait_for_proof` polls and normally observes `InstantSend` / `InBlock` state while the record is still present, but a tx that's already chain-locked at the moment the proof flow starts (e.g. crash recovery resuming an asset lock that chain-locked while we were down) hits the missing-record path. This adds a persister fallback. The persister received the full record on the chainlock-transition `store` call before eviction, so it can answer the lookup. Changes: * `PlatformWalletPersistence::get_core_tx_record(wallet_id, txid) -> Result, PersistenceError>` — new trait method with a default `Ok(None)` impl. Backward-compatible: persisters that don't index records by txid (the in-tree `NoPlatformPersistence`, no-op test stubs in the wider tree) need no change. Persisters whose backing store keys records by txid (`SqliteWalletPersister` in evo-tool, the SwiftData iOS persister) should override; until then those callers see the same behavior as before this PR (None response, caller's existing not-found path applies). * `WalletPersister::get_core_tx_record(txid)` — `pub(crate)` per-wallet passthrough. * `record_or_persister(in_memory, persister, txid) -> Option` — `pub(super)` helper in `proof.rs` that returns the in-memory record if present, otherwise queries the persister. Persister errors are logged at `debug` and surfaced as `None` so a transient backend failure doesn't turn into a hard error in the proof flow. * Five lookup sites switched to the helper: - `validate_or_upgrade_proof` (proof.rs): error message broadened to "in-memory or persister". - `upgrade_to_chain_lock_proof` (proof.rs): "tx not found" no longer errors; falls through to `wait_for_chain_lock` so SPV events can still complete the proof. - `wait_for_chain_lock` (proof.rs): poll loop now also checks persister so a chainlock that arrives between polls and is evicted is still observed on the next iteration. - `wait_for_proof` (proof.rs): same treatment. - `resolve_status_from_info` (recovery.rs): converted from associated `fn` → method (`&self`) to access the persister. * Tests: 5 new unit tests in `proof.rs::tests` covering the helper — in-memory hit short-circuits, persister-only hit returns the record, both-miss returns `None`, default trait impl returns `None`, backend errors swallowed as `None`. Fake `PlatformWalletPersistence` implementations (`FakeRecordStore`, `ErroringStore`) added inline. Total: 120/120 platform-wallet lib tests pass. Backlinks dashpay/platform#3617 (CodeRabbit's "Major" finding on the proof.rs eviction window). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/changeset/traits.rs | 31 ++ .../src/wallet/asset_lock/sync/proof.rs | 364 ++++++++++++++---- .../src/wallet/asset_lock/sync/recovery.rs | 14 +- .../src/wallet/persister.rs | 14 + 4 files changed, 348 insertions(+), 75 deletions(-) diff --git a/packages/rs-platform-wallet/src/changeset/traits.rs b/packages/rs-platform-wallet/src/changeset/traits.rs index 81329d0c385..a6438324abd 100644 --- a/packages/rs-platform-wallet/src/changeset/traits.rs +++ b/packages/rs-platform-wallet/src/changeset/traits.rs @@ -6,6 +6,8 @@ use crate::changeset::changeset::PlatformWalletChangeSet; use crate::changeset::client_start_state::ClientStartState; use crate::wallet::platform_wallet::WalletId; +use dashcore::Txid; +use key_wallet::managed_account::transaction_record::TransactionRecord; /// Errors returned by a [`PlatformWalletPersistence`] backend. /// @@ -148,4 +150,33 @@ pub trait PlatformWalletPersistence: Send + Sync { /// already keyed by wallet id and the sub-changesets carry their own /// wallet attribution where needed. fn load(&self) -> Result; + + /// Look up a single core transaction record by `txid` for `wallet_id`. + /// + /// Used by the asset-lock proof flow to recover records that the + /// in-memory `transactions()` map has evicted. Upstream's + /// `keep-finalized-transactions` Cargo feature is OFF by default — + /// chainlocked records are dropped from the in-memory map and only + /// their txids are retained in `finalized_txids` for dedup. The + /// chain-lock height / block hash that an asset-lock proof needs is + /// no longer reachable through the wallet-info API, but the + /// persister received the full record on the last `store` call + /// before eviction, so it can answer this lookup. + /// + /// The default implementation returns `Ok(None)` — backwards + /// compatible for persisters that don't index records by txid (e.g. + /// [`NoPlatformPersistence`]). The asset-lock proof flow's hot path + /// (mempool / `InBlock` window) hits the in-memory map first, so a + /// `None` response from this method only matters for the rare race + /// where the first lookup happens after the chainlock-eviction + /// window. Persisters whose backing store keys records by txid + /// (`SqliteWalletPersister`, the SwiftData iOS persister) should + /// override. + fn get_core_tx_record( + &self, + _wallet_id: WalletId, + _txid: &Txid, + ) -> Result, PersistenceError> { + Ok(None) + } } diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/proof.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/proof.rs index 36f69719b2d..ca7bc9a4a09 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/proof.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/proof.rs @@ -3,13 +3,52 @@ use crate::broadcaster::TransactionBroadcaster; use std::time::Duration; -use dashcore::OutPoint; +use dashcore::{OutPoint, Txid}; use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; +use key_wallet::managed_account::transaction_record::TransactionRecord; use crate::error::PlatformWalletError; use super::super::manager::AssetLockManager; +/// Fall back to the persister if the in-memory `transactions()` map +/// didn't have the record. +/// +/// With upstream's `keep-finalized-transactions` Cargo feature OFF +/// (the default), chainlocked records are evicted from the in-memory +/// map and only their txids retained in `finalized_txids`. The +/// persister received the full record on the chainlock-transition +/// `store` call before eviction, so it can answer the lookup. A +/// persister that doesn't index records by txid (the trait's default +/// impl, +/// [`NoPlatformPersistence`](crate::wallet::persister::NoPlatformPersistence)) +/// returns `None` here — callers must still handle the absence. +/// +/// Persister errors are logged at `debug` and surfaced as `None` so a +/// transient backend failure doesn't turn into a hard error in the +/// proof flow — the caller's existing `None` handling (timeout / poll +/// loop / not-found error) still applies. +pub(super) fn record_or_persister( + in_memory: Option, + persister: &crate::wallet::persister::WalletPersister, + txid: &Txid, +) -> Option { + if let Some(record) = in_memory { + return Some(record); + } + match persister.get_core_tx_record(txid) { + Ok(opt) => opt, + Err(e) => { + tracing::debug!( + txid = %txid, + error = %e, + "Persister fallback for core tx record failed" + ); + None + } + } +} + impl AssetLockManager { /// Validate an IS-lock proof and upgrade it to a ChainLock proof if the /// transaction is old enough that the IS-lock may have expired. @@ -36,20 +75,23 @@ impl AssetLockManager { return Ok(proof); } - let wm = self.wallet_manager.read().await; - let info = wm - .get_wallet_info(&self.wallet_id) - .ok_or_else(|| PlatformWalletError::WalletNotFound(hex::encode(self.wallet_id)))?; - - let record = info - .core_wallet - .accounts - .standard_bip44_accounts - .get(&account_index) - .and_then(|a| a.transactions().get(&out_point.txid)) - .ok_or_else(|| { + let in_memory = { + let wm = self.wallet_manager.read().await; + let info = wm + .get_wallet_info(&self.wallet_id) + .ok_or_else(|| PlatformWalletError::WalletNotFound(hex::encode(self.wallet_id)))?; + info.core_wallet + .accounts + .standard_bip44_accounts + .get(&account_index) + .and_then(|a| a.transactions().get(&out_point.txid).cloned()) + // wm dropped at end of block — release before persister + DAPI calls. + }; + + let record = + record_or_persister(in_memory, &self.persister, &out_point.txid).ok_or_else(|| { PlatformWalletError::AssetLockProofWait(format!( - "Transaction {} not found in account {}", + "Transaction {} not found in account {} (in-memory or persister)", out_point.txid, account_index )) })?; @@ -57,9 +99,6 @@ impl AssetLockManager { let is_chain_locked = matches!(record.context, TransactionContext::InChainLockedBlock(_)); let height = record.height().unwrap_or(0); - // Drop the read lock before making the DAPI call. - drop(wm); - if is_chain_locked && height > 0 { let platform_height = self.get_platform_core_chain_locked_height().await?; @@ -129,30 +168,26 @@ impl AssetLockManager { })? }; - // Check if already chain-locked. - let height = { + // Check if already chain-locked. Falls back to the persister if + // the in-memory map already evicted the record (default config). + let in_memory = { let wm = self.wallet_manager.read().await; let info = wm .get_wallet_info(&self.wallet_id) .ok_or_else(|| PlatformWalletError::WalletNotFound(hex::encode(self.wallet_id)))?; - let record = info - .core_wallet + info.core_wallet .accounts .standard_bip44_accounts .get(&account_index) - .and_then(|a| a.transactions().get(&txid)) - .ok_or_else(|| { - PlatformWalletError::AssetLockProofWait(format!( - "Transaction {} not found in account {}", - txid, account_index - )) - })?; + .and_then(|a| a.transactions().get(&txid).cloned()) + }; - if matches!(record.context, TransactionContext::InChainLockedBlock(_)) { + let height = match record_or_persister(in_memory, &self.persister, &txid) { + Some(record) if matches!(record.context, TransactionContext::InChainLockedBlock(_)) => { record.height() - } else { - None } + Some(_) => None, + None => None, }; let height = match height { @@ -215,22 +250,24 @@ impl AssetLockManager { let deadline = tokio::time::Instant::now() + timeout; loop { - // Check — might have been updated by SPV sync. - { + // Check — might have been updated by SPV sync. Falls back + // to the persister so a chainlock that arrived between + // polls (and was evicted from the in-memory map) is still + // observed. + let in_memory = { let wm = self.wallet_manager.read().await; - if let Some(info) = wm.get_wallet_info(&self.wallet_id) { - if let Some(record) = info - .core_wallet + wm.get_wallet_info(&self.wallet_id).and_then(|info| { + info.core_wallet .accounts .standard_bip44_accounts .get(&account_index) - .and_then(|a| a.transactions().get(&out_point.txid)) - { - if matches!(record.context, TransactionContext::InChainLockedBlock(_)) { - if let Some(h) = record.height() { - return Ok(h); - } - } + .and_then(|a| a.transactions().get(&out_point.txid).cloned()) + }) + }; + if let Some(record) = record_or_persister(in_memory, &self.persister, &out_point.txid) { + if matches!(record.context, TransactionContext::InChainLockedBlock(_)) { + if let Some(h) = record.height() { + return Ok(h); } } } @@ -286,40 +323,39 @@ impl AssetLockManager { }; loop { - // Check the transaction record context for finality. - { + // Check the transaction record context for finality. Falls + // back to the persister so a chainlocked record evicted + // from the in-memory map is still observed. + let in_memory = { let wm = self.wallet_manager.read().await; - if let Some(info) = wm.get_wallet_info(&self.wallet_id) { - if let Some(record) = info - .core_wallet + wm.get_wallet_info(&self.wallet_id).and_then(|info| { + info.core_wallet .accounts .standard_bip44_accounts .get(&account_index) - .and_then(|a| a.transactions().get(&out_point.txid)) - { - match &record.context { - TransactionContext::InstantSend(instant_lock) => { - return Ok(dpp::prelude::AssetLockProof::Instant( - InstantAssetLockProof::new( - instant_lock.clone(), - tracked_tx, - out_point.vout, - ), - )); - } - TransactionContext::InChainLockedBlock(_) => { - if let Some(height) = record.height() { - return Ok(dpp::prelude::AssetLockProof::Chain( - ChainAssetLockProof { - core_chain_locked_height: height, - out_point: *out_point, - }, - )); - } - } - _ => {} + .and_then(|a| a.transactions().get(&out_point.txid).cloned()) + }) + }; + if let Some(record) = record_or_persister(in_memory, &self.persister, &out_point.txid) { + match &record.context { + TransactionContext::InstantSend(instant_lock) => { + return Ok(dpp::prelude::AssetLockProof::Instant( + InstantAssetLockProof::new( + instant_lock.clone(), + tracked_tx, + out_point.vout, + ), + )); + } + TransactionContext::InChainLockedBlock(_) => { + if let Some(height) = record.height() { + return Ok(dpp::prelude::AssetLockProof::Chain(ChainAssetLockProof { + core_chain_locked_height: height, + out_point: *out_point, + })); } } + _ => {} } } @@ -338,3 +374,189 @@ impl AssetLockManager { } } } + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashMap; + use std::sync::{Arc, Mutex}; + + use dashcore::blockdata::transaction::Transaction; + use dashcore::TxIn; + use key_wallet::account::account_type::StandardAccountType; + use key_wallet::account::AccountType; + use key_wallet::managed_account::transaction_record::TransactionDirection; + use key_wallet::transaction_checking::{TransactionContext, TransactionType}; + + use crate::changeset::{ + ClientStartState, PersistenceError, PlatformWalletChangeSet, PlatformWalletPersistence, + }; + use crate::wallet::persister::{NoPlatformPersistence, WalletPersister}; + use crate::wallet::platform_wallet::WalletId; + + fn record_with_txid(seed: u8) -> TransactionRecord { + // A unique txid per `seed` falls out of the (different) input + // outpoint; the actual transaction body doesn't matter for the + // helper-under-test's purposes. + let tx = Transaction { + version: 1, + lock_time: 0, + input: vec![TxIn { + previous_output: dashcore::OutPoint::new(Txid::from([seed; 32]), 0), + ..Default::default() + }], + output: Vec::new(), + special_transaction_payload: None, + }; + TransactionRecord::new( + tx, + AccountType::Standard { + index: 0, + standard_account_type: StandardAccountType::BIP44Account, + }, + TransactionContext::Mempool, + TransactionType::Standard, + TransactionDirection::Incoming, + Vec::new(), + Vec::new(), + 0, + ) + } + + /// Test persister that answers `get_core_tx_record` from a + /// configurable in-memory map. `store` / `flush` are no-ops; `load` + /// returns the default state. + struct FakeRecordStore { + records: Mutex>, + } + + impl FakeRecordStore { + fn with_records>(records: I) -> Self { + let map = records.into_iter().map(|r| (r.txid, r)).collect(); + Self { + records: Mutex::new(map), + } + } + } + + impl PlatformWalletPersistence for FakeRecordStore { + fn store( + &self, + _wallet_id: WalletId, + _changeset: PlatformWalletChangeSet, + ) -> Result<(), PersistenceError> { + Ok(()) + } + fn flush(&self, _wallet_id: WalletId) -> Result<(), PersistenceError> { + Ok(()) + } + fn load(&self) -> Result { + Ok(ClientStartState::default()) + } + fn get_core_tx_record( + &self, + _wallet_id: WalletId, + txid: &Txid, + ) -> Result, PersistenceError> { + Ok(self.records.lock().unwrap().get(txid).cloned()) + } + } + + /// Test persister that always errors out on `get_core_tx_record`, + /// to exercise the error-swallowing branch in `record_or_persister`. + struct ErroringStore; + + impl PlatformWalletPersistence for ErroringStore { + fn store( + &self, + _wallet_id: WalletId, + _changeset: PlatformWalletChangeSet, + ) -> Result<(), PersistenceError> { + Ok(()) + } + fn flush(&self, _wallet_id: WalletId) -> Result<(), PersistenceError> { + Ok(()) + } + fn load(&self) -> Result { + Ok(ClientStartState::default()) + } + fn get_core_tx_record( + &self, + _wallet_id: WalletId, + _txid: &Txid, + ) -> Result, PersistenceError> { + Err(PersistenceError::Backend( + "simulated backend failure".into(), + )) + } + } + + fn wallet_persister(inner: Arc) -> WalletPersister { + WalletPersister::new([0u8; 32], inner) + } + + #[test] + fn record_or_persister_prefers_in_memory_when_present() { + // The in-memory record wins; the persister's record is never + // consulted (so even a record stored under a *different* txid + // would be ignored — this verifies the helper short-circuits on + // a hit). + let in_memory = record_with_txid(0xAA); + let in_memory_txid = in_memory.txid; + let other = record_with_txid(0xBB); + let persister = wallet_persister(Arc::new(FakeRecordStore::with_records([other]))); + + let resolved = record_or_persister(Some(in_memory.clone()), &persister, &in_memory_txid); + assert_eq!(resolved.map(|r| r.txid), Some(in_memory_txid)); + } + + #[test] + fn record_or_persister_falls_back_to_persister_on_miss() { + // The in-memory map evicted (None); the persister still has it. + // This is the chain-locked-eviction recovery path. + let stored = record_with_txid(0xCC); + let stored_txid = stored.txid; + let persister = wallet_persister(Arc::new(FakeRecordStore::with_records([stored]))); + + let resolved = record_or_persister(None, &persister, &stored_txid); + assert_eq!(resolved.map(|r| r.txid), Some(stored_txid)); + } + + #[test] + fn record_or_persister_returns_none_when_neither_has_it() { + // Both miss → None; callers handle this as "tx not found locally" + // (proof flow returns its own AssetLockProofWait error, poll + // loops continue waiting). + let unknown_txid = Txid::from([0xDD; 32]); + let persister = wallet_persister(Arc::new(FakeRecordStore::with_records([]))); + + let resolved = record_or_persister(None, &persister, &unknown_txid); + assert!(resolved.is_none()); + } + + #[test] + fn record_or_persister_default_persister_returns_none() { + // The trait default impl on `NoPlatformPersistence` returns + // `Ok(None)` — confirms backends that don't override the new + // method gracefully no-op (the proof flow still works for + // mempool/InBlock txs; only the chainlock-eviction recovery is + // unavailable). + let unknown_txid = Txid::from([0xEE; 32]); + let persister = wallet_persister(Arc::new(NoPlatformPersistence)); + + let resolved = record_or_persister(None, &persister, &unknown_txid); + assert!(resolved.is_none()); + } + + #[test] + fn record_or_persister_swallows_backend_errors_as_none() { + // A transient backend failure must not turn into a hard error + // in the proof flow — the caller's existing `None` handling + // (timeout / poll loop / not-found error) still applies. + let unknown_txid = Txid::from([0xFF; 32]); + let persister = wallet_persister(Arc::new(ErroringStore)); + + let resolved = record_or_persister(None, &persister, &unknown_txid); + assert!(resolved.is_none()); + } +} diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs index f7052041bea..32a000fee22 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs @@ -63,7 +63,7 @@ impl AssetLockManager { None => { // Need to resolve from wallet info - drop the write guard and use a read. // Actually we already have mutable access to info, so we can read from it. - Self::resolve_status_from_info(&info.core_wallet, account_index, &out_point) + self.resolve_status_from_info(&info.core_wallet, account_index, &out_point) } }; @@ -89,24 +89,30 @@ impl AssetLockManager { } /// Determine asset lock status by looking up the transaction in - /// `ManagedWalletInfo`. + /// `ManagedWalletInfo`, falling back to the persister if the + /// in-memory map evicted the record (default + /// `keep-finalized-transactions` off). /// /// If the TX is in a chain-locked block, returns `ChainLocked` with a /// constructed `ChainAssetLockProof`. If the TX has an InstantSend /// context, returns `InstantSendLocked` (without a proof, since we lack /// the IS-lock data). Otherwise defaults to `Broadcast`. fn resolve_status_from_info( + &self, wallet_info: &ManagedWalletInfo, account_index: u32, out_point: &OutPoint, ) -> (AssetLockStatus, Option) { + use super::proof::record_or_persister; use key_wallet::transaction_checking::TransactionContext; - let record = wallet_info + let in_memory = wallet_info .accounts .standard_bip44_accounts .get(&account_index) - .and_then(|a| a.transactions().get(&out_point.txid)); + .and_then(|a| a.transactions().get(&out_point.txid).cloned()); + + let record = record_or_persister(in_memory, &self.persister, &out_point.txid); match record { Some(record) => match &record.context { diff --git a/packages/rs-platform-wallet/src/wallet/persister.rs b/packages/rs-platform-wallet/src/wallet/persister.rs index 2bd787e2efa..dfd889a581e 100644 --- a/packages/rs-platform-wallet/src/wallet/persister.rs +++ b/packages/rs-platform-wallet/src/wallet/persister.rs @@ -6,6 +6,9 @@ use std::sync::Arc; +use dashcore::Txid; +use key_wallet::managed_account::transaction_record::TransactionRecord; + use crate::changeset::{ ClientStartState, PersistenceError, PlatformWalletChangeSet, PlatformWalletPersistence, }; @@ -38,6 +41,17 @@ impl WalletPersister { pub(crate) fn load(&self) -> Result { self.inner.load() } + + /// Look up a single core transaction record by `txid`. Used by the + /// asset-lock proof flow to recover chainlocked records that the + /// in-memory map evicted (see + /// [`PlatformWalletPersistence::get_core_tx_record`]). + pub(crate) fn get_core_tx_record( + &self, + txid: &Txid, + ) -> Result, PersistenceError> { + self.inner.get_core_tx_record(self.wallet_id, txid) + } } /// No-op platform persistence for standalone wallets. From d8de2c257e6d774220505fd7e5c0b3537b6fbc43 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 8 May 2026 00:04:11 +0700 Subject: [PATCH 2/6] feat(platform-wallet-ffi,swift-sdk): wire chainlocked-tx persister fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hooks the persister-fallback path added in the previous commit through to the iOS persister so it actually fires in production builds. The trait method `PlatformWalletPersistence::get_core_tx_record` is the seam; this commit adds the FFI plumbing + the Swift implementation that backs it via the existing `PersistentTransaction` SwiftData rows. ## platform-wallet (trait doc) Tightened the field contract on `get_core_tx_record`. Implementations are only required to populate `txid` + `context` (and the `BlockInfo` inside `InChainLockedBlock` / `InBlock` carrying real height + hash + timestamp). Other fields MAY be best-effort placeholders; callers (currently only the asset-lock proof flow) MUST NOT read them. This makes the FFI-backed implementation feasible without serializing the full `Transaction` body across the C ABI. ## platform-wallet-ffi * `PersistenceCallbacks.on_get_chainlocked_tx_record_fn` — new C callback. Takes a wallet_id + txid; on hit, populates `out_block_height`, `out_block_hash` (32 bytes), `out_block_timestamp` and sets `out_found = true`. Returns 0 on success regardless of hit / miss; non-zero is treated as a transient backend failure (surfaced as `None` per the helper contract). Scope is intentionally narrow: chainlocked records only — IS-locked / mempool / InBlock records aren't evicted from the in-memory map and so don't need a fallback. * `FFIPersister::get_core_tx_record` — overrides the trait default to call the new callback and synthesize a minimal `TransactionRecord` with `context = InChainLockedBlock(BlockInfo::new(height, hash, timestamp))`. Placeholder transaction body / account_type / direction / etc. — proof flow callers never read those, per the trait contract. ## swift-sdk * `PlatformWalletPersistenceHandler.chainlockedTxRecord(walletId:txid:)` — internal lookup. Walks `PersistentTransaction` rows by `txid` (raw 32 wire bytes — same orientation Rust hands across the FFI) filtered to `context == 3` (InChainLockedBlock). Returns nil on miss or when the row's `blockHash` is missing / wrong-length (treated as miss rather than fabricating a zero hash that would round-trip to Rust as a real block id). * `getChainlockedTxRecordCallback` — top-level C shim mirroring the pattern of the other persistence callbacks. Handles the Rust→Swift→Rust conversion + populates the output pointers. * `makeCallbacks()` registers the new shim. Existing `PlatformWalletManager()` construction picks it up automatically; the example app needed no changes. ## Verification - `cargo check --workspace --all-targets`: clean - `cargo test -p platform-wallet --lib`: 120/120 (the 5 new helper tests from the prior commit still pass since the helper itself is unchanged) - `cargo clippy -p platform-wallet-ffi --all-targets`: clean (the 2 warnings are pre-existing in unrelated test code) - `bash packages/swift-sdk/build_ios.sh --target sim --profile dev`: BUILD SUCCEEDED end-to-end including SwiftExampleApp link Co-Authored-By: Claude Opus 4.7 (1M context) --- .../rs-platform-wallet-ffi/src/persistence.rs | 145 ++++++++++++++++++ .../src/changeset/traits.rs | 15 ++ .../PlatformWalletPersistenceHandler.swift | 121 +++++++++++++++ 3 files changed, 281 insertions(+) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 82d29fb3b65..14cd6a7382c 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -287,6 +287,47 @@ pub struct PersistenceCallbacks { removed_incoming_count: usize, ) -> i32, >, + /// Look up a single chain-locked core transaction record by `txid` + /// for the asset-lock proof flow's persister fallback. + /// + /// With upstream's `keep-finalized-transactions` Cargo feature OFF + /// (the default), chain-locked records are evicted from the + /// in-memory `transactions()` map and only their txids retained in + /// `finalized_txids` for dedup. The asset-lock proof flow needs to + /// recover the chain-lock height to construct a + /// `ChainAssetLockProof`; the persister has the record (it + /// received it on the chain-lock-transition `store` call before + /// eviction) and answers this lookup. + /// + /// Scope: **chain-locked records only.** InstantSend records are + /// never evicted from the in-memory map, so a persister fallback + /// for them isn't needed; mempool / `InBlock` records similarly + /// stay in the in-memory map until they reach chain-lock. The + /// Rust side wraps the result in an `InChainLockedBlock` context + /// regardless of what the persister has stored — implementations + /// SHOULD only return found=true for records they know to be + /// chain-locked, and MUST populate `block_height`, `block_hash`, + /// `block_timestamp` accordingly. + /// + /// Output contract: + /// - Set `*out_found = true` and populate the three block fields + /// when a chain-locked record exists for `txid`. + /// - Set `*out_found = false` when no such record exists. + /// - Return `0` on a successful lookup (whether found or not). + /// Non-zero values are treated as a transient backend failure + /// by the Rust side and surfaced as `None` (no error + /// propagation through the proof flow). + pub on_get_chainlocked_tx_record_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + wallet_id: *const u8, + txid: *const u8, + out_block_height: *mut u32, + out_block_hash: *mut u8, + out_block_timestamp: *mut u32, + out_found: *mut bool, + ) -> i32, + >, } // SAFETY: The context pointer is managed by the FFI caller who must ensure @@ -902,6 +943,110 @@ impl PlatformWalletPersistence for FFIPersister { } Ok(out) } + + /// Look up a chain-locked transaction record by `txid` via the + /// `on_get_chainlocked_tx_record_fn` callback, and synthesize a + /// minimal [`TransactionRecord`] for the asset-lock proof flow. + /// + /// The callers in `platform-wallet/src/wallet/asset_lock/sync/` only + /// read `record.context` and `record.height()` (which is + /// `record.context.block_info().map(|b| b.height)`), so the + /// returned record only populates `txid` + `context` reliably. + /// All other fields are placeholders (empty / default). See the + /// trait method docs on [`PlatformWalletPersistence::get_core_tx_record`] + /// for the full contract. + /// + /// Returns `Ok(None)` when the callback is unset, when the callback + /// reports `out_found = false`, or when the callback returns a + /// non-zero result code (treated as a transient backend failure + /// per the trait contract — surfaced as `None` rather than + /// propagating an error through the proof flow). + fn get_core_tx_record( + &self, + wallet_id: WalletId, + txid: &dashcore::Txid, + ) -> Result< + Option, + PersistenceError, + > { + use dashcore::hashes::Hash; + use key_wallet::account::{AccountType, StandardAccountType}; + use key_wallet::managed_account::transaction_record::{ + TransactionDirection, TransactionRecord, + }; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext, TransactionType}; + + let Some(get_cb) = self.callbacks.on_get_chainlocked_tx_record_fn else { + return Ok(None); + }; + + let txid_bytes: [u8; 32] = *txid.as_byte_array(); + let mut block_height: u32 = 0; + let mut block_hash: [u8; 32] = [0u8; 32]; + let mut block_timestamp: u32 = 0; + let mut found: bool = false; + + // SAFETY: All output pointers reference Rust-owned stack + // locals that outlive the callback invocation. `wallet_id` and + // `txid` are fixed-size byte arrays. + let rc = unsafe { + get_cb( + self.callbacks.context, + wallet_id.as_ptr(), + txid_bytes.as_ptr(), + &mut block_height, + block_hash.as_mut_ptr(), + &mut block_timestamp, + &mut found, + ) + }; + + if rc != 0 { + tracing::debug!( + txid = %txid, + rc, + "on_get_chainlocked_tx_record_fn returned a non-zero \ + result; treating as miss" + ); + return Ok(None); + } + if !found { + return Ok(None); + } + + let context = TransactionContext::InChainLockedBlock(BlockInfo::new( + block_height, + dashcore::BlockHash::from_byte_array(block_hash), + block_timestamp, + )); + + // Synthetic record: only `txid` and `context` are reliable. + // Placeholder transaction body keeps `dashcore::Transaction` + // construction trivial — proof-flow callers never read it. + let placeholder_tx = dashcore::blockdata::transaction::Transaction { + version: 1, + lock_time: 0, + input: Vec::new(), + output: Vec::new(), + special_transaction_payload: None, + }; + Ok(Some(TransactionRecord { + transaction: placeholder_tx, + txid: *txid, + account_type: AccountType::Standard { + index: 0, + standard_account_type: StandardAccountType::BIP44Account, + }, + context, + transaction_type: TransactionType::Standard, + direction: TransactionDirection::Internal, + input_details: Vec::new(), + output_details: Vec::new(), + net_amount: 0, + fee: None, + label: String::new(), + })) + } } /// Flatten an `AccountType` + encoded xpub into the C-flat diff --git a/packages/rs-platform-wallet/src/changeset/traits.rs b/packages/rs-platform-wallet/src/changeset/traits.rs index a6438324abd..1e567e451ed 100644 --- a/packages/rs-platform-wallet/src/changeset/traits.rs +++ b/packages/rs-platform-wallet/src/changeset/traits.rs @@ -172,6 +172,21 @@ pub trait PlatformWalletPersistence: Send + Sync { /// window. Persisters whose backing store keys records by txid /// (`SqliteWalletPersister`, the SwiftData iOS persister) should /// override. + /// + /// **Field contract.** Implementations are only required to + /// populate `txid` and `context` (with the `BlockInfo` inside + /// `InChainLockedBlock` / `InBlock` carrying real height + block + /// hash + timestamp). Other fields (`transaction`, `input_details`, + /// `output_details`, `account_type`, `transaction_type`, + /// `direction`, `net_amount`, `fee`, `label`) MAY be returned as + /// best-effort placeholders and MUST NOT be relied upon by callers. + /// The current consumer — the asset-lock proof flow — only reads + /// `context` and `height()` (which is + /// `context.block_info().map(|b| b.height)`). FFI-backed + /// implementations (e.g. the SwiftData iOS persister) take + /// advantage of this contract by emitting a synthetic record with a + /// placeholder transaction body, since reconstructing the full + /// `Transaction` over the C ABI is not free and isn't needed. fn get_core_tx_record( &self, _wallet_id: WalletId, diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index 345ff652779..f376d4c84db 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -781,6 +781,17 @@ public class PlatformWalletPersistenceHandler { cb.on_persist_identity_keys_fn = persistIdentityKeysCallback cb.on_persist_token_balances_fn = persistTokenBalancesCallback cb.on_persist_contacts_fn = persistContactsCallback + // Persister fallback for the asset-lock proof flow's + // chainlocked-tx lookup (Rust trait method + // `PlatformWalletPersistence::get_core_tx_record`). Without + // this wired, the Rust side's `record_or_persister` helper + // gracefully falls back to "miss" — but then chainlocked + // asset-lock proofs can't recover the chain-lock height + // once upstream's `keep-finalized-transactions` Cargo + // feature evicts the in-memory record (the production + // default). See the Rust trait doc for the field contract + // and the helper for the call sites. + cb.on_get_chainlocked_tx_record_fn = getChainlockedTxRecordCallback return cb } @@ -2771,6 +2782,61 @@ public class PlatformWalletPersistenceHandler { return data } + /// Look up a chain-locked transaction record for the asset-lock + /// proof flow's persister fallback (Rust trait method + /// `PlatformWalletPersistence::get_core_tx_record`). + /// + /// The Rust-side asset-lock proof flow needs the chain-lock + /// height + block hash + timestamp to construct a + /// `ChainAssetLockProof`. With upstream's + /// `keep-finalized-transactions` Cargo feature OFF (the default), + /// chain-locked records are evicted from the in-memory + /// `transactions()` map and only their txids retained for dedup, + /// so the chain-lock metadata is no longer reachable through the + /// wallet-info API. The persister received the record on the + /// chain-lock-transition `store` call before eviction; this + /// lookup walks the corresponding `PersistentTransaction` row. + /// + /// Returns `nil` when: + /// - no `PersistentTransaction` row exists for `txid`, or + /// - the row exists but is not chain-locked (`context != 3`). + /// + /// The wallet-id is currently unused for the lookup (`txid` is + /// globally unique), but is accepted to match the Rust trait + /// signature and to leave room for a wallet-scoped variant. + func chainlockedTxRecord( + walletId: Data, + txid: Data + ) -> (blockHeight: UInt32, blockHash: Data, blockTimestamp: UInt32)? { + _ = walletId + return onQueue { + // PersistentTransaction.context: 3 = InChainLockedBlock. + // Predicate-side filter on context narrows the index scan + // to chain-locked rows so the lookup stays cheap when the + // store has many in-flight (mempool / IS / InBlock) rows. + let chainLocked: UInt32 = 3 + let descriptor = FetchDescriptor( + predicate: #Predicate { $0.txid == txid && $0.context == chainLocked } + ) + guard let row = try? backgroundContext.fetch(descriptor).first else { + return nil + } + // `blockHash` is optional in the row — chain-locked rows + // should always have it, but if a row got upserted in an + // older state without one, we treat that as "miss" rather + // than fabricate a zero hash that would round-trip back + // to Rust as a real block id. + guard let blockHash = row.blockHash, blockHash.count == 32 else { + return nil + } + return ( + blockHeight: row.blockHeight, + blockHash: blockHash, + blockTimestamp: row.blockTimestamp + ) + } + } + /// Look up the network for a wallet id by reading the owning /// `PersistentWallet` row. Returns `nil` if the wallet row /// doesn't exist or its network hasn't been resolved yet. @@ -3555,3 +3621,58 @@ private func persistWalletMetadataCallback( ) return 0 } + +/// C shim for `on_get_chainlocked_tx_record_fn`. Calls +/// `PlatformWalletPersistenceHandler.chainlockedTxRecord(...)` and +/// writes the block height / hash / timestamp to the Rust-owned +/// output pointers when a chain-locked row exists for `txid`. +/// +/// Output contract: +/// - Sets `*outFound = true` and populates the three block fields +/// on a hit; returns `0`. +/// - Sets `*outFound = false` and leaves the block fields untouched +/// on a miss; returns `0`. +/// - Returns `0` even on Swift-side errors (treated as miss); the +/// Rust side's `record_or_persister` helper logs and falls +/// through to the caller's existing not-found / poll path. +private func getChainlockedTxRecordCallback( + context: UnsafeMutableRawPointer?, + walletIdPtr: UnsafePointer?, + txidPtr: UnsafePointer?, + outBlockHeight: UnsafeMutablePointer?, + outBlockHash: UnsafeMutablePointer?, + outBlockTimestamp: UnsafeMutablePointer?, + outFound: UnsafeMutablePointer? +) -> Int32 { + guard let context = context, + let walletIdPtr = walletIdPtr, + let txidPtr = txidPtr, + let outFound = outFound else { + return 0 + } + outFound.pointee = false + + let handler = Unmanaged + .fromOpaque(context) + .takeUnretainedValue() + let walletId = Data(bytes: walletIdPtr, count: 32) + let txid = Data(bytes: txidPtr, count: 32) + + guard let row = handler.chainlockedTxRecord(walletId: walletId, txid: txid) else { + // Miss — outFound already set to false above. + return 0 + } + + outBlockHeight?.pointee = row.blockHeight + outBlockTimestamp?.pointee = row.blockTimestamp + if let outBlockHash = outBlockHash { + // `chainlockedTxRecord` validates `blockHash.count == 32` + // before returning, so this copy is bounded. + row.blockHash.copyBytes( + to: UnsafeMutableBufferPointer(start: outBlockHash, count: 32), + count: 32 + ) + } + outFound.pointee = true + return 0 +} From bdd28235a24057bc5c8baa5f4532fc2a9a85a2dd Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 8 May 2026 00:07:08 +0700 Subject: [PATCH 3/6] chore(swift-sdk): drop verbose registration comment in makeCallbacks Rest of `makeCallbacks()` is one-line-per-registration; the new line follows the same shape. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../PlatformWalletPersistenceHandler.swift | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index f376d4c84db..78e5afa5d8e 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -781,16 +781,6 @@ public class PlatformWalletPersistenceHandler { cb.on_persist_identity_keys_fn = persistIdentityKeysCallback cb.on_persist_token_balances_fn = persistTokenBalancesCallback cb.on_persist_contacts_fn = persistContactsCallback - // Persister fallback for the asset-lock proof flow's - // chainlocked-tx lookup (Rust trait method - // `PlatformWalletPersistence::get_core_tx_record`). Without - // this wired, the Rust side's `record_or_persister` helper - // gracefully falls back to "miss" — but then chainlocked - // asset-lock proofs can't recover the chain-lock height - // once upstream's `keep-finalized-transactions` Cargo - // feature evicts the in-memory record (the production - // default). See the Rust trait doc for the field contract - // and the helper for the call sites. cb.on_get_chainlocked_tx_record_fn = getChainlockedTxRecordCallback return cb } From 8e9b75d86636c673151a0e49f79150c2825a39d9 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 8 May 2026 00:29:19 +0700 Subject: [PATCH 4/6] fix(platform-wallet-ffi,swift-sdk): faithful context kind + real tx body in persister fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes on top of the previous commit: 1. **FFI now sends the actual context kind, Rust faithfully reconstructs.** Previously the FFI was scoped to "chainlocked only" and the Rust side wrapped every hit as `InChainLockedBlock(...)` regardless. The Swift side compensated by filtering rows to `context == 3` in the predicate. That was correct in steady state (the eviction invariant means persister-only-not-memory implies chainlocked) but wrong at startup: an in-memory map starts empty, so any persister hit on a non-chainlocked row would round-trip to Rust as a false- positive chainlock — `resolve_status_from_info` would emit `(ChainLocked, ChainAssetLockProof)` for a tx that isn't chainlocked yet. Now the FFI carries `out_context_kind` (0=Mempool, 1=IS, 2=InBlock, 3=InChainLockedBlock). Rust constructs the matching `TransactionContext` variant, the chainlock filter on Swift goes away, and false-positive chainlocks are impossible. IS hits surface as `None` to the proof flow (the persister doesn't store the IS-lock blob; SPV-event wait completes the proof from the live stream — same outcome as if the fallback weren't wired at all). 2. **Persister returns the real transaction bytes; Rust decodes a real `Transaction`.** Previously the synthesized `TransactionRecord` carried a placeholder `Transaction { version: 1, lock_time: 0, input: [], output: [], … }` because the FFI surface didn't carry the tx body. The proof flow doesn't read `record.transaction` today, so the placeholder was technically safe — but it was a lie that would silently corrupt any future code reading those fields. `PersistentTransaction.transactionData` is what gets stored on the FFI write path (consensus-encoded — `dashcore::consensus:: encode::serialize` upstream, NOT bincode). The new callback hands those bytes back over FFI; Rust decodes via `Transaction::consensus_decode`. Buffer ownership: Swift allocates with `UnsafeMutablePointer.allocate`, Rust takes the pointer, an RAII guard on the Rust side calls the paired `on_get_core_tx_record_free_fn` exactly once on every exit path (success, decode failure, IS-skip, unknown context kind). `PersistentTransaction.transactionData` is now non-optional — the FFI write path always populates it in practice (the `Data?` was a vestige of a defensive guard that never fires). Init gains a required `transactionData` arg; the upsert path lifts the bytes extraction to a single binding shared between the init and the unconditional post-init assignment. The one stub creation site (UTXO upsert path that pre-creates the parent transaction row before the real upsert arrives) passes empty `Data()` — the real upsert overwrites every field including `transactionData`; an orphaned stub correctly reads back as miss because the empty buffer won't decode. Touched the example app's `StorageRecordDetailViews` (one line) to drop the `?.count` optional chain on the de-optionalized field. Verification: - `cargo check --workspace --all-targets`: clean - `cargo test -p platform-wallet --lib`: 120/120 - `cargo clippy -p platform-wallet-ffi --all-targets`: clean - `bash packages/swift-sdk/build_ios.sh --target sim --profile dev`: BUILD SUCCEEDED end-to-end including SwiftExampleApp link Co-Authored-By: Claude Opus 4.7 (1M context) --- .../rs-platform-wallet-ffi/src/persistence.rs | 228 ++++++++++++++---- .../Models/PersistentTransaction.swift | 12 +- .../PlatformWalletPersistenceHandler.swift | 190 +++++++++++---- .../Views/StorageRecordDetailViews.swift | 4 +- 4 files changed, 328 insertions(+), 106 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 14cd6a7382c..4fc7ddba2df 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -287,8 +287,8 @@ pub struct PersistenceCallbacks { removed_incoming_count: usize, ) -> i32, >, - /// Look up a single chain-locked core transaction record by `txid` - /// for the asset-lock proof flow's persister fallback. + /// Look up a single core transaction record by `txid` for the + /// asset-lock proof flow's persister fallback. /// /// With upstream's `keep-finalized-transactions` Cargo feature OFF /// (the default), chain-locked records are evicted from the @@ -299,35 +299,62 @@ pub struct PersistenceCallbacks { /// received it on the chain-lock-transition `store` call before /// eviction) and answers this lookup. /// - /// Scope: **chain-locked records only.** InstantSend records are - /// never evicted from the in-memory map, so a persister fallback - /// for them isn't needed; mempool / `InBlock` records similarly - /// stay in the in-memory map until they reach chain-lock. The - /// Rust side wraps the result in an `InChainLockedBlock` context - /// regardless of what the persister has stored — implementations - /// SHOULD only return found=true for records they know to be - /// chain-locked, and MUST populate `block_height`, `block_hash`, - /// `block_timestamp` accordingly. - /// /// Output contract: - /// - Set `*out_found = true` and populate the three block fields - /// when a chain-locked record exists for `txid`. - /// - Set `*out_found = false` when no such record exists. + /// - Set `*out_found = true` when a row exists for `txid`. Set + /// `*out_context_kind` to the row's actual context (0=Mempool, + /// 1=InstantSend, 2=InBlock, 3=InChainLockedBlock). For + /// context kinds 2 and 3, populate `out_block_height`, + /// `out_block_hash` (32 bytes), and `out_block_timestamp` from + /// the row's block info; the Rust side ignores those fields + /// for kinds 0 and 1. + /// - Hand back the row's raw transaction bytes via + /// `*out_tx_bytes` + `*out_tx_bytes_len`. The buffer is + /// caller-allocated and must remain valid until the Rust side + /// invokes [`Self::on_get_core_tx_record_free_fn`]. Set + /// `*out_tx_bytes = null` + `*out_tx_bytes_len = 0` if the + /// row exists but the persister never stored the bytes (the + /// Rust side will surface `None` rather than synthesize a + /// placeholder). + /// - Set `*out_found = false` when no row exists for `txid`. /// - Return `0` on a successful lookup (whether found or not). /// Non-zero values are treated as a transient backend failure /// by the Rust side and surfaced as `None` (no error /// propagation through the proof flow). - pub on_get_chainlocked_tx_record_fn: Option< + /// + /// The Rust side faithfully reconstructs the + /// [`TransactionContext`](key_wallet::transaction_checking::TransactionContext) + /// from `*out_context_kind` and decodes the tx bytes into a real + /// [`dashcore::Transaction`]. InstantSend rows are reported back + /// with the kind tag but not currently consumed (the persister + /// doesn't store the IS-lock blob), so for an IS hit the Rust + /// side surfaces `None` to the proof flow — same outcome as a + /// miss. The proof flow then falls through to its existing + /// SPV-event-driven wait path, which is what would have happened + /// without the fallback at all. + pub on_get_core_tx_record_fn: Option< unsafe extern "C" fn( context: *mut c_void, wallet_id: *const u8, txid: *const u8, + out_context_kind: *mut u8, out_block_height: *mut u32, out_block_hash: *mut u8, out_block_timestamp: *mut u32, + out_tx_bytes: *mut *const u8, + out_tx_bytes_len: *mut usize, out_found: *mut bool, ) -> i32, >, + /// Paired free callback for the tx-bytes buffer returned by + /// [`Self::on_get_core_tx_record_fn`]. The Rust side invokes + /// this with the same `(tx_bytes, tx_bytes_len)` pair the lookup + /// callback wrote into the output pointers, exactly once per + /// hit. Implementations should release the buffer (e.g. + /// `UnsafeMutablePointer.deallocate()` on the Swift + /// side). + pub on_get_core_tx_record_free_fn: Option< + unsafe extern "C" fn(context: *mut c_void, tx_bytes: *const u8, tx_bytes_len: usize), + >, } // SAFETY: The context pointer is managed by the FFI caller who must ensure @@ -944,23 +971,36 @@ impl PlatformWalletPersistence for FFIPersister { Ok(out) } - /// Look up a chain-locked transaction record by `txid` via the - /// `on_get_chainlocked_tx_record_fn` callback, and synthesize a - /// minimal [`TransactionRecord`] for the asset-lock proof flow. + /// Look up a transaction record by `txid` via the + /// `on_get_core_tx_record_fn` callback and reconstruct the + /// [`TransactionRecord`] for the asset-lock proof flow. /// - /// The callers in `platform-wallet/src/wallet/asset_lock/sync/` only - /// read `record.context` and `record.height()` (which is - /// `record.context.block_info().map(|b| b.height)`), so the - /// returned record only populates `txid` + `context` reliably. - /// All other fields are placeholders (empty / default). See the - /// trait method docs on [`PlatformWalletPersistence::get_core_tx_record`] - /// for the full contract. + /// The proof-flow callers in + /// `platform-wallet/src/wallet/asset_lock/sync/` only read + /// `record.context`, `record.height()`, and (for site 4) + /// `record.transaction.txid`. The Swift side hands back the + /// row's actual context kind plus the raw transaction bytes, so + /// `txid` / `context` / `transaction` are all reliable. Other + /// fields (`account_type`, `transaction_type`, `direction`, + /// `input_details`, `output_details`, `net_amount`, `fee`, + /// `label`) are best-effort placeholders per the trait field + /// contract; see + /// [`PlatformWalletPersistence::get_core_tx_record`]. /// - /// Returns `Ok(None)` when the callback is unset, when the callback - /// reports `out_found = false`, or when the callback returns a - /// non-zero result code (treated as a transient backend failure - /// per the trait contract — surfaced as `None` rather than - /// propagating an error through the proof flow). + /// The InstantSend variant requires an + /// [`InstantLock`](dashcore::ephemerealdata::instant_lock::InstantLock) + /// blob that the persister doesn't currently store, so for an IS + /// hit we surface `None` (treat as miss) and let the proof + /// flow's existing SPV-event-driven wait path complete the + /// proof. + /// + /// Returns `Ok(None)` when the callback is unset, when the + /// callback reports `out_found = false`, when the callback + /// returns a non-zero result code (treated as a transient backend + /// failure per the trait contract — surfaced as `None` rather + /// than propagating), when the callback hands back a null / + /// empty tx-bytes buffer, when the bytes don't decode as a + /// `dashcore::Transaction`, or for an IS hit (see above). fn get_core_tx_record( &self, wallet_id: WalletId, @@ -969,6 +1009,7 @@ impl PlatformWalletPersistence for FFIPersister { Option, PersistenceError, > { + use dashcore::consensus::Decodable; use dashcore::hashes::Hash; use key_wallet::account::{AccountType, StandardAccountType}; use key_wallet::managed_account::transaction_record::{ @@ -976,37 +1017,78 @@ impl PlatformWalletPersistence for FFIPersister { }; use key_wallet::transaction_checking::{BlockInfo, TransactionContext, TransactionType}; - let Some(get_cb) = self.callbacks.on_get_chainlocked_tx_record_fn else { + let Some(get_cb) = self.callbacks.on_get_core_tx_record_fn else { return Ok(None); }; let txid_bytes: [u8; 32] = *txid.as_byte_array(); + let mut context_kind: u8 = 0; let mut block_height: u32 = 0; let mut block_hash: [u8; 32] = [0u8; 32]; let mut block_timestamp: u32 = 0; + let mut tx_bytes_ptr: *const u8 = std::ptr::null(); + let mut tx_bytes_len: usize = 0; let mut found: bool = false; // SAFETY: All output pointers reference Rust-owned stack - // locals that outlive the callback invocation. `wallet_id` and - // `txid` are fixed-size byte arrays. + // locals that outlive the callback invocation. `wallet_id` + // and `txid` are fixed-size byte arrays. let rc = unsafe { get_cb( self.callbacks.context, wallet_id.as_ptr(), txid_bytes.as_ptr(), + &mut context_kind, &mut block_height, block_hash.as_mut_ptr(), &mut block_timestamp, + &mut tx_bytes_ptr, + &mut tx_bytes_len, &mut found, ) }; + // RAII guard so the tx-bytes free callback fires on every + // exit path past this point — early returns for unknown + // context kinds, decode failures, and the IS-skip case all + // correctly hand the buffer back to Swift. + struct TxBytesGuard<'a> { + ptr: *const u8, + len: usize, + free_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + tx_bytes: *const u8, + tx_bytes_len: usize, + ), + >, + ctx: *mut c_void, + _marker: std::marker::PhantomData<&'a ()>, + } + impl<'a> Drop for TxBytesGuard<'a> { + fn drop(&mut self) { + if let (Some(free), false) = (self.free_fn, self.ptr.is_null()) { + // SAFETY: ptr+len match the values the lookup + // callback wrote; Swift owns the allocation + // until this free fires. + unsafe { free(self.ctx, self.ptr, self.len) }; + } + } + } + let _bytes_guard = TxBytesGuard { + ptr: tx_bytes_ptr, + len: tx_bytes_len, + free_fn: self.callbacks.on_get_core_tx_record_free_fn, + ctx: self.callbacks.context, + _marker: std::marker::PhantomData, + }; + if rc != 0 { tracing::debug!( txid = %txid, rc, - "on_get_chainlocked_tx_record_fn returned a non-zero \ - result; treating as miss" + "on_get_core_tx_record_fn returned a non-zero result; \ + treating as miss" ); return Ok(None); } @@ -1014,24 +1096,66 @@ impl PlatformWalletPersistence for FFIPersister { return Ok(None); } - let context = TransactionContext::InChainLockedBlock(BlockInfo::new( - block_height, - dashcore::BlockHash::from_byte_array(block_hash), - block_timestamp, - )); - - // Synthetic record: only `txid` and `context` are reliable. - // Placeholder transaction body keeps `dashcore::Transaction` - // construction trivial — proof-flow callers never read it. - let placeholder_tx = dashcore::blockdata::transaction::Transaction { - version: 1, - lock_time: 0, - input: Vec::new(), - output: Vec::new(), - special_transaction_payload: None, + let context = match context_kind { + 0 => TransactionContext::Mempool, + 1 => { + // InstantSend requires the IS-lock blob, which the + // persister doesn't currently store. Treat as miss + // so the proof flow's SPV wait path completes the + // proof from the live event stream. + return Ok(None); + } + 2 => TransactionContext::InBlock(BlockInfo::new( + block_height, + dashcore::BlockHash::from_byte_array(block_hash), + block_timestamp, + )), + 3 => TransactionContext::InChainLockedBlock(BlockInfo::new( + block_height, + dashcore::BlockHash::from_byte_array(block_hash), + block_timestamp, + )), + unknown => { + tracing::debug!( + txid = %txid, + unknown, + "on_get_core_tx_record_fn returned an unknown \ + context kind; treating as miss" + ); + return Ok(None); + } }; + + if tx_bytes_ptr.is_null() || tx_bytes_len == 0 { + tracing::debug!( + txid = %txid, + "on_get_core_tx_record_fn reported a hit but no tx \ + bytes; treating as miss" + ); + return Ok(None); + } + // SAFETY: Swift guarantees `tx_bytes_ptr` points to + // `tx_bytes_len` valid bytes for the duration of the + // callback window — `_bytes_guard` keeps that window open + // until this function returns. + let tx_slice = unsafe { slice::from_raw_parts(tx_bytes_ptr, tx_bytes_len) }; + let transaction = match dashcore::blockdata::transaction::Transaction::consensus_decode( + &mut &tx_slice[..], + ) { + Ok(tx) => tx, + Err(err) => { + tracing::debug!( + txid = %txid, + error = %err, + "on_get_core_tx_record_fn returned undecodable \ + tx bytes; treating as miss" + ); + return Ok(None); + } + }; + Ok(Some(TransactionRecord { - transaction: placeholder_tx, + transaction, txid: *txid, account_type: AccountType::Standard { index: 0, diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTransaction.swift b/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTransaction.swift index 8c39c89a48b..3e072af6555 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTransaction.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTransaction.swift @@ -29,8 +29,14 @@ public final class PersistentTransaction { /// instead of a 64-char hex string, and the persistence /// handler avoids a hex round-trip on every write. @Attribute(.unique) public var txid: Data - /// Raw transaction bytes. - public var transactionData: Data? + /// Raw transaction bytes (consensus-encoded — the same wire + /// format `dashcore::consensus::encode::serialize` produces and + /// `Transaction::consensus_decode` round-trips). The FFI write + /// path always populates this; the persister-fallback read path + /// (`PlatformWalletPersistence::get_core_tx_record`) hands it + /// back over FFI so Rust can decode a real `Transaction` + /// without a placeholder body. + public var transactionData: Data /// Context: 0=mempool, 1=instantSend, 2=inBlock, 3=inChainLockedBlock. public var context: UInt32 /// Block height (0 for mempool). @@ -90,6 +96,7 @@ public final class PersistentTransaction { public init( txid: Data, + transactionData: Data, context: UInt32 = 0, blockHeight: UInt32 = 0, direction: UInt32 = 0, @@ -98,6 +105,7 @@ public final class PersistentTransaction { firstSeen: UInt64 = 0 ) { self.txid = txid + self.transactionData = transactionData self.context = context self.blockHeight = blockHeight self.blockTimestamp = 0 diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index 78e5afa5d8e..965e236e55d 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -407,12 +407,25 @@ public class PlatformWalletPersistenceHandler { predicate: #Predicate { $0.txid == txidData } ) + // The FFI projection always serializes the transaction body + // (`dashcore::consensus::encode::serialize` upstream), so + // `tx.tx_data` is non-null and `tx.tx_data_len > 0` in + // practice. Fall back to empty `Data()` only as a defensive + // guard against a future projection change — the + // persister-fallback read path treats empty bytes as miss + // (the Rust side can't decode an empty consensus buffer). + let transactionData: Data = { + guard let dataPtr = tx.tx_data, tx.tx_data_len > 0 else { return Data() } + return Data(bytes: dataPtr, count: Int(tx.tx_data_len)) + }() + let record: PersistentTransaction if let existing = try? backgroundContext.fetch(descriptor).first { record = existing } else { record = PersistentTransaction( txid: txidData, + transactionData: transactionData, context: tx.context, blockHeight: tx.block_height, direction: tx.direction, @@ -438,9 +451,7 @@ public class PlatformWalletPersistenceHandler { record.label = String(cString: labelPtr) } record.firstSeen = tx.first_seen - if let dataPtr = tx.tx_data, tx.tx_data_len > 0 { - record.transactionData = Data(bytes: dataPtr, count: Int(tx.tx_data_len)) - } + record.transactionData = transactionData record.lastUpdated = Date() // Walk every input in this transaction and reconcile it @@ -593,7 +604,14 @@ public class PlatformWalletPersistenceHandler { if let existingTx = try? backgroundContext.fetch(txDescriptor).first { parentTx = existingTx } else { - parentTx = PersistentTransaction(txid: txidData) + // Stub row — `transactionData` is left as empty + // `Data()` on purpose. The real upsert (which has the + // tx bytes) overwrites every field including + // `transactionData` when it arrives. An orphaned + // stub (real upsert never lands) reads back as empty + // bytes, which the persister-fallback decode path + // treats as miss. + parentTx = PersistentTransaction(txid: txidData, transactionData: Data()) backgroundContext.insert(parentTx) } @@ -781,7 +799,8 @@ public class PlatformWalletPersistenceHandler { cb.on_persist_identity_keys_fn = persistIdentityKeysCallback cb.on_persist_token_balances_fn = persistTokenBalancesCallback cb.on_persist_contacts_fn = persistContactsCallback - cb.on_get_chainlocked_tx_record_fn = getChainlockedTxRecordCallback + cb.on_get_core_tx_record_fn = getCoreTxRecordCallback + cb.on_get_core_tx_record_free_fn = getCoreTxRecordFreeCallback return cb } @@ -2772,8 +2791,8 @@ public class PlatformWalletPersistenceHandler { return data } - /// Look up a chain-locked transaction record for the asset-lock - /// proof flow's persister fallback (Rust trait method + /// Look up a transaction record for the asset-lock proof flow's + /// persister fallback (Rust trait method /// `PlatformWalletPersistence::get_core_tx_record`). /// /// The Rust-side asset-lock proof flow needs the chain-lock @@ -2781,49 +2800,81 @@ public class PlatformWalletPersistenceHandler { /// `ChainAssetLockProof`. With upstream's /// `keep-finalized-transactions` Cargo feature OFF (the default), /// chain-locked records are evicted from the in-memory - /// `transactions()` map and only their txids retained for dedup, - /// so the chain-lock metadata is no longer reachable through the - /// wallet-info API. The persister received the record on the - /// chain-lock-transition `store` call before eviction; this - /// lookup walks the corresponding `PersistentTransaction` row. + /// `transactions()` map, so the chain-lock metadata is no longer + /// reachable through the wallet-info API. The persister received + /// the record on the chain-lock-transition `store` call before + /// eviction; this lookup walks the corresponding + /// `PersistentTransaction` row. /// - /// Returns `nil` when: - /// - no `PersistentTransaction` row exists for `txid`, or - /// - the row exists but is not chain-locked (`context != 3`). + /// Returns the row's actual `context` discriminant alongside the + /// block info (when applicable). The Rust side faithfully + /// reconstructs the matching `TransactionContext` variant — no + /// chain-lock filter here, so a row in any state may be + /// returned. `blockHash` / `blockHeight` / `blockTimestamp` are + /// only meaningful for `context` 2 (InBlock) and 3 + /// (InChainLockedBlock); the Rust side ignores those fields for + /// 0 (Mempool) and 1 (InstantSend). /// - /// The wallet-id is currently unused for the lookup (`txid` is - /// globally unique), but is accepted to match the Rust trait - /// signature and to leave room for a wallet-scoped variant. - func chainlockedTxRecord( + /// Returns `nil` when no `PersistentTransaction` row exists for + /// `txid`, when an in-block / chain-locked row is missing its + /// `blockHash` (treated as miss rather than fabricating a zero + /// hash that would round-trip back to Rust as a real block id), + /// or when the row has no `transactionData` (the FFI write path + /// always populates it, so a missing one signals a corrupt row + /// the Rust side can't decode anyway). + /// + /// The wallet-id is currently unused (`txid` is globally + /// unique), but is accepted to match the Rust trait signature + /// and to leave room for a wallet-scoped variant. + func coreTxRecord( walletId: Data, txid: Data - ) -> (blockHeight: UInt32, blockHash: Data, blockTimestamp: UInt32)? { + ) -> (context: UInt32, blockHeight: UInt32, blockHash: Data, blockTimestamp: UInt32, transactionData: Data)? { _ = walletId return onQueue { - // PersistentTransaction.context: 3 = InChainLockedBlock. - // Predicate-side filter on context narrows the index scan - // to chain-locked rows so the lookup stays cheap when the - // store has many in-flight (mempool / IS / InBlock) rows. - let chainLocked: UInt32 = 3 let descriptor = FetchDescriptor( - predicate: #Predicate { $0.txid == txid && $0.context == chainLocked } + predicate: #Predicate { $0.txid == txid } ) guard let row = try? backgroundContext.fetch(descriptor).first else { return nil } - // `blockHash` is optional in the row — chain-locked rows - // should always have it, but if a row got upserted in an - // older state without one, we treat that as "miss" rather - // than fabricate a zero hash that would round-trip back - // to Rust as a real block id. - guard let blockHash = row.blockHash, blockHash.count == 32 else { + // The Rust side decodes `transactionData` into a + // `dashcore::Transaction`; an empty buffer (left over + // from an orphaned stub row in the UTXO upsert path + // whose real upsert never arrived) won't decode, so + // treat it as miss. + guard !row.transactionData.isEmpty else { return nil } - return ( - blockHeight: row.blockHeight, - blockHash: blockHash, - blockTimestamp: row.blockTimestamp - ) + let transactionData = row.transactionData + switch row.context { + case 0, 1: + // Mempool / InstantSend — block fields not meaningful; + // the Rust side ignores them. Hand back zeroed + // placeholders so the caller's tuple shape stays + // uniform. + return ( + context: row.context, + blockHeight: 0, + blockHash: Data(count: 32), + blockTimestamp: 0, + transactionData: transactionData + ) + default: + // InBlock / InChainLockedBlock — `blockHash` MUST be + // present and 32 bytes for the row to round-trip + // correctly to Rust as a `BlockHash`. + guard let blockHash = row.blockHash, blockHash.count == 32 else { + return nil + } + return ( + context: row.context, + blockHeight: row.blockHeight, + blockHash: blockHash, + blockTimestamp: row.blockTimestamp, + transactionData: transactionData + ) + } } } @@ -3612,26 +3663,35 @@ private func persistWalletMetadataCallback( return 0 } -/// C shim for `on_get_chainlocked_tx_record_fn`. Calls -/// `PlatformWalletPersistenceHandler.chainlockedTxRecord(...)` and -/// writes the block height / hash / timestamp to the Rust-owned -/// output pointers when a chain-locked row exists for `txid`. +/// C shim for `on_get_core_tx_record_fn`. Calls +/// `PlatformWalletPersistenceHandler.coreTxRecord(...)` and writes +/// the row's actual context kind, block info (when applicable), and +/// raw transaction bytes to the Rust-owned output pointers. +/// +/// The transaction bytes are allocated here via +/// `UnsafeMutablePointer.allocate(capacity:)` and the +/// allocation is owned by the Rust side until it invokes +/// `getCoreTxRecordFreeCallback` below — Rust calls free exactly +/// once per hit. /// /// Output contract: -/// - Sets `*outFound = true` and populates the three block fields -/// on a hit; returns `0`. -/// - Sets `*outFound = false` and leaves the block fields untouched -/// on a miss; returns `0`. +/// - Sets `*outFound = true` and populates `outContextKind` (and +/// the three block fields when context is 2 or 3, plus the tx +/// bytes pointer + length) on a hit; returns `0`. +/// - Sets `*outFound = false` on a miss; returns `0`. /// - Returns `0` even on Swift-side errors (treated as miss); the /// Rust side's `record_or_persister` helper logs and falls /// through to the caller's existing not-found / poll path. -private func getChainlockedTxRecordCallback( +private func getCoreTxRecordCallback( context: UnsafeMutableRawPointer?, walletIdPtr: UnsafePointer?, txidPtr: UnsafePointer?, + outContextKind: UnsafeMutablePointer?, outBlockHeight: UnsafeMutablePointer?, outBlockHash: UnsafeMutablePointer?, outBlockTimestamp: UnsafeMutablePointer?, + outTxBytes: UnsafeMutablePointer?>?, + outTxBytesLen: UnsafeMutablePointer?, outFound: UnsafeMutablePointer? ) -> Int32 { guard let context = context, @@ -3641,6 +3701,8 @@ private func getChainlockedTxRecordCallback( return 0 } outFound.pointee = false + outTxBytes?.pointee = nil + outTxBytesLen?.pointee = 0 let handler = Unmanaged .fromOpaque(context) @@ -3648,21 +3710,51 @@ private func getChainlockedTxRecordCallback( let walletId = Data(bytes: walletIdPtr, count: 32) let txid = Data(bytes: txidPtr, count: 32) - guard let row = handler.chainlockedTxRecord(walletId: walletId, txid: txid) else { + guard let row = handler.coreTxRecord(walletId: walletId, txid: txid) else { // Miss — outFound already set to false above. return 0 } + outContextKind?.pointee = UInt8(row.context) outBlockHeight?.pointee = row.blockHeight outBlockTimestamp?.pointee = row.blockTimestamp if let outBlockHash = outBlockHash { - // `chainlockedTxRecord` validates `blockHash.count == 32` - // before returning, so this copy is bounded. + // `coreTxRecord` returns a 32-byte `blockHash` (real for + // in-block / chain-locked rows, zeroed placeholder for + // mempool / IS rows that the Rust side will ignore), so + // this copy is bounded. row.blockHash.copyBytes( to: UnsafeMutableBufferPointer(start: outBlockHash, count: 32), count: 32 ) } + + // Hand the tx bytes to Rust. The buffer outlives this callback + // — Rust calls `getCoreTxRecordFreeCallback` to release it. + let len = row.transactionData.count + let buffer = UnsafeMutablePointer.allocate(capacity: len) + row.transactionData.copyBytes( + to: UnsafeMutableBufferPointer(start: buffer, count: len), + count: len + ) + outTxBytes?.pointee = UnsafePointer(buffer) + outTxBytesLen?.pointee = UInt(len) + outFound.pointee = true return 0 } + +/// Paired free callback for `on_get_core_tx_record_free_fn`. +/// Releases the buffer `getCoreTxRecordCallback` allocated above. +/// `UInt8` is trivial so no `deinitialize(count:)` is required — +/// `deallocate()` alone matches the `allocate(capacity:)`. +private func getCoreTxRecordFreeCallback( + context: UnsafeMutableRawPointer?, + txBytes: UnsafePointer?, + _ txBytesLen: UInt +) { + guard let txBytes = txBytes else { return } + UnsafeMutablePointer(mutating: txBytes).deallocate() + _ = context + _ = txBytesLen +} diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift index 77b6136663d..5a5a947a89c 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift @@ -1477,9 +1477,7 @@ struct TransactionStorageDetailView: View { Section("Metadata") { FieldRow(label: "Label", value: record.label.isEmpty ? "None" : record.label) FieldRow(label: "First Seen", value: "\(record.firstSeen)") - if let size = record.transactionData?.count { - FieldRow(label: "TX Size", value: "\(size) bytes") - } + FieldRow(label: "TX Size", value: "\(record.transactionData.count) bytes") } // Per-output drill-downs. Each row navigates to the // owning `PersistentTxo` so the address / spent state / From 372697d04598df5fa72329c79e9f9d4b27f579ef Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 8 May 2026 00:36:37 +0700 Subject: [PATCH 5/6] fix(platform-wallet): preserve fast-fail + surface persister errors + drop lock before persister I/O MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses three findings from the @thepastaclaw review on commit 757f035c (the FFI/Swift wiring + tx-bytes plumbing in d8de2c2 and 8e9b75d already addressed the fourth — "FFI not wired" — and already reach the iOS build through the new `on_get_core_tx_record_fn` callback). ## #2 — fast-fail regression in `upgrade_to_chain_lock_proof` Pre-PR, a missing in-memory record errored fast with `AssetLockProofWait("Transaction X not found in account Y")`. After the helper landed, the missing case fell through to `wait_for_chain_lock(...)` and burned the full chainlock timeout before returning `FinalityTimeout` — a regression for genuine "tx never tracked" cases (wallet-state mismatch, post-wipe race). Restored the fast-fail: when both the in-memory map and the persister miss, the function returns the same `AssetLockProofWait` error pre-PR callers expected. The "persister returned a non-chainlocked record" case still falls through to `wait_for_chain_lock` (correct — the tx exists, just hasn't chainlocked yet). ## #3 — silent error downgrade on transient persister failure `record_or_persister` previously logged backend errors at `debug` and surfaced them as `None`. That's correct for the poll loops (`wait_for_chain_lock`, `wait_for_proof`) where the next tick retries — but wrong for `resolve_status_from_info`, which is a one-shot recovery call: a transient backend error silently classified the lock as `AssetLockStatus::Broadcast` even when the underlying tx was actually chain-locked, forcing `resume_asset_lock` down the wasteful `Broadcast → wait_for_proof` path with no operator-visible signal. Split the helper: * `record_or_persister` now returns `Result, PersistenceError>`. Call sites choose their own policy. * `record_or_persister_or_log` is the new poll-loop variant — same `Option` return, but logs persister errors at `warn` (up from `debug`) before downgrading to `None`. A persistent failure is now visible in operator logs. * The two poll-loop sites (`wait_for_chain_lock`, `wait_for_proof`) switched to `_or_log`. * The two fast-fail sites (`validate_or_upgrade_proof`, `upgrade_to_chain_lock_proof`) propagate `Err` as `AssetLockProofWait("Persister lookup for tx X failed: …")`, distinct from the "neither lookup found it" message. * The one-shot recovery site (renamed `resolve_status_with_in_memory`, see #4 below) logs at `error` and degrades to `None`, preserving recovery's "always make some progress" semantics while making the failure visible. Tests updated for the new signature. Added a new test `record_or_persister_or_log_swallows_backend_errors_as_none` covering the poll-loop variant; renamed the old "swallows" test to `propagates_backend_errors` and asserts `Err`. 121/121 tests pass. ## #4 — wallet-manager lock held across persister I/O `recover_asset_lock_blocking` was acquiring `self.wallet_manager.blocking_write()` and then dispatching to `resolve_status_from_info`, which after the helper's persister fallback could perform synchronous SQLite/SwiftData/FFI I/O behind the write lock — serializing other writers on a single disk lookup. Restructured into three phases: 1. **Lock held**: claim the `tracked_asset_locks` slot, snapshot the in-memory record (only when no proof was provided), drop the lock. 2. **No lock held**: resolve status, including the persister fallback. The renamed `resolve_status_with_in_memory(snapshot, ...)` takes the pre-snapshotted record so this phase no longer touches the wallet manager. 3. **Lock held**: re-acquire to commit the `TrackedAssetLock` entry. A re-check of `tracked_asset_locks.contains_key` handles the case where another caller raced in during phase 2 (first writer wins). Net effect: the persister I/O now runs without serializing other wallet-manager writers. The lock-hold time drops from "in-memory lookup + persister round-trip" to two short in-memory operations. Verification: - `cargo fmt --all`: clean - `cargo test -p platform-wallet --lib`: 121/121 (was 120, +1 from the new `_or_log` test) - `bash packages/swift-sdk/build_ios.sh --target sim --profile dev`: BUILD SUCCEEDED end-to-end including SwiftExampleApp link Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/wallet/asset_lock/sync/proof.rs | 123 ++++++++++++----- .../src/wallet/asset_lock/sync/recovery.rs | 130 ++++++++++++------ 2 files changed, 181 insertions(+), 72 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/proof.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/proof.rs index ca7bc9a4a09..8850f595b35 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/proof.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/proof.rs @@ -22,27 +22,45 @@ use super::super::manager::AssetLockManager; /// persister that doesn't index records by txid (the trait's default /// impl, /// [`NoPlatformPersistence`](crate::wallet::persister::NoPlatformPersistence)) -/// returns `None` here — callers must still handle the absence. +/// returns `Ok(None)` here — callers must still handle the absence. /// -/// Persister errors are logged at `debug` and surfaced as `None` so a -/// transient backend failure doesn't turn into a hard error in the -/// proof flow — the caller's existing `None` handling (timeout / poll -/// loop / not-found error) still applies. +/// Persister errors are surfaced as `Err(PersistenceError)` so call +/// sites can choose their own policy: +/// +/// - **Poll loops** (`wait_for_chain_lock`, `wait_for_proof`) typically +/// downgrade to `None` for the current iteration so the next tick +/// retries — see [`record_or_persister_or_log`] for that policy. +/// - **One-shot recovery / fast-fail call sites** want the error +/// visible so a transient backend failure isn't silently classified +/// as "tx not found" — they handle the `Err` arm explicitly. pub(super) fn record_or_persister( in_memory: Option, persister: &crate::wallet::persister::WalletPersister, txid: &Txid, -) -> Option { +) -> Result, crate::changeset::PersistenceError> { if let Some(record) = in_memory { - return Some(record); + return Ok(Some(record)); } - match persister.get_core_tx_record(txid) { + persister.get_core_tx_record(txid) +} + +/// Variant of [`record_or_persister`] that swallows persister errors +/// as `None` after a `warn`-level log. Use this from poll loops where +/// the next iteration retries — a hard error from a single tick would +/// abort the whole poll prematurely. +pub(super) fn record_or_persister_or_log( + in_memory: Option, + persister: &crate::wallet::persister::WalletPersister, + txid: &Txid, +) -> Option { + match record_or_persister(in_memory, persister, txid) { Ok(opt) => opt, Err(e) => { - tracing::debug!( + tracing::warn!( txid = %txid, error = %e, - "Persister fallback for core tx record failed" + "Persister fallback for core tx record failed; \ + treating as miss for this poll iteration" ); None } @@ -88,8 +106,14 @@ impl AssetLockManager { // wm dropped at end of block — release before persister + DAPI calls. }; - let record = - record_or_persister(in_memory, &self.persister, &out_point.txid).ok_or_else(|| { + let record = record_or_persister(in_memory, &self.persister, &out_point.txid) + .map_err(|e| { + PlatformWalletError::AssetLockProofWait(format!( + "Persister lookup for tx {} failed: {}", + out_point.txid, e + )) + })? + .ok_or_else(|| { PlatformWalletError::AssetLockProofWait(format!( "Transaction {} not found in account {} (in-memory or persister)", out_point.txid, account_index @@ -182,12 +206,28 @@ impl AssetLockManager { .and_then(|a| a.transactions().get(&txid).cloned()) }; - let height = match record_or_persister(in_memory, &self.persister, &txid) { - Some(record) if matches!(record.context, TransactionContext::InChainLockedBlock(_)) => { - record.height() - } - Some(_) => None, - None => None, + let record = record_or_persister(in_memory, &self.persister, &txid).map_err(|e| { + PlatformWalletError::AssetLockProofWait(format!( + "Persister lookup for tx {} failed: {}", + txid, e + )) + })?; + let record = record.ok_or_else(|| { + // Both lookups missed. The asset lock is known-tracked + // (we validated `tracked_asset_locks.get` above), so this + // is a wallet-state mismatch / post-wipe race rather than + // a "not chain-locked yet" case. Fast-fail rather than + // dispatching to `wait_for_chain_lock` and burning the + // full timeout. + PlatformWalletError::AssetLockProofWait(format!( + "Transaction {} not found in account {} (in-memory or persister)", + txid, account_index + )) + })?; + let height = if matches!(record.context, TransactionContext::InChainLockedBlock(_)) { + record.height() + } else { + None }; let height = match height { @@ -264,7 +304,9 @@ impl AssetLockManager { .and_then(|a| a.transactions().get(&out_point.txid).cloned()) }) }; - if let Some(record) = record_or_persister(in_memory, &self.persister, &out_point.txid) { + if let Some(record) = + record_or_persister_or_log(in_memory, &self.persister, &out_point.txid) + { if matches!(record.context, TransactionContext::InChainLockedBlock(_)) { if let Some(h) = record.height() { return Ok(h); @@ -336,7 +378,9 @@ impl AssetLockManager { .and_then(|a| a.transactions().get(&out_point.txid).cloned()) }) }; - if let Some(record) = record_or_persister(in_memory, &self.persister, &out_point.txid) { + if let Some(record) = + record_or_persister_or_log(in_memory, &self.persister, &out_point.txid) + { match &record.context { TransactionContext::InstantSend(instant_lock) => { return Ok(dpp::prelude::AssetLockProof::Instant( @@ -506,7 +550,8 @@ mod tests { let other = record_with_txid(0xBB); let persister = wallet_persister(Arc::new(FakeRecordStore::with_records([other]))); - let resolved = record_or_persister(Some(in_memory.clone()), &persister, &in_memory_txid); + let resolved = record_or_persister(Some(in_memory.clone()), &persister, &in_memory_txid) + .expect("in-memory hit cannot fail"); assert_eq!(resolved.map(|r| r.txid), Some(in_memory_txid)); } @@ -518,19 +563,21 @@ mod tests { let stored_txid = stored.txid; let persister = wallet_persister(Arc::new(FakeRecordStore::with_records([stored]))); - let resolved = record_or_persister(None, &persister, &stored_txid); + let resolved = + record_or_persister(None, &persister, &stored_txid).expect("persister succeeded"); assert_eq!(resolved.map(|r| r.txid), Some(stored_txid)); } #[test] fn record_or_persister_returns_none_when_neither_has_it() { - // Both miss → None; callers handle this as "tx not found locally" - // (proof flow returns its own AssetLockProofWait error, poll - // loops continue waiting). + // Both miss → Ok(None); callers handle this as "tx not found + // locally" (proof flow returns its own AssetLockProofWait + // error, poll loops continue waiting). let unknown_txid = Txid::from([0xDD; 32]); let persister = wallet_persister(Arc::new(FakeRecordStore::with_records([]))); - let resolved = record_or_persister(None, &persister, &unknown_txid); + let resolved = + record_or_persister(None, &persister, &unknown_txid).expect("persister succeeded"); assert!(resolved.is_none()); } @@ -544,19 +591,33 @@ mod tests { let unknown_txid = Txid::from([0xEE; 32]); let persister = wallet_persister(Arc::new(NoPlatformPersistence)); - let resolved = record_or_persister(None, &persister, &unknown_txid); + let resolved = + record_or_persister(None, &persister, &unknown_txid).expect("persister succeeded"); assert!(resolved.is_none()); } #[test] - fn record_or_persister_swallows_backend_errors_as_none() { - // A transient backend failure must not turn into a hard error - // in the proof flow — the caller's existing `None` handling - // (timeout / poll loop / not-found error) still applies. + fn record_or_persister_propagates_backend_errors() { + // Backend errors surface as `Err` so call sites can choose + // their own policy (one-shot recovery logs at error and + // degrades; poll loops downgrade to None for one tick via + // `record_or_persister_or_log`). let unknown_txid = Txid::from([0xFF; 32]); let persister = wallet_persister(Arc::new(ErroringStore)); let resolved = record_or_persister(None, &persister, &unknown_txid); + assert!(resolved.is_err()); + } + + #[test] + fn record_or_persister_or_log_swallows_backend_errors_as_none() { + // The poll-loop variant downgrades errors to `None` (after a + // `warn` log) so a transient backend failure on one tick + // doesn't abort the whole poll. + let unknown_txid = Txid::from([0xFF; 32]); + let persister = wallet_persister(Arc::new(ErroringStore)); + + let resolved = record_or_persister_or_log(None, &persister, &unknown_txid); assert!(resolved.is_none()); } } diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs index 32a000fee22..4a781af37cb 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs @@ -44,14 +44,36 @@ impl AssetLockManager { out_point: OutPoint, proof: Option, ) { - let mut wm = self.wallet_manager.blocking_write(); - let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) else { - return; + // Phase 1 (lock held): claim the tracked-asset-lock slot and + // pull the in-memory record out so the lookup work is + // bounded to a single hashmap fetch. + let in_memory_record = { + let mut wm = self.wallet_manager.blocking_write(); + let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) else { + return; + }; + if info.tracked_asset_locks.contains_key(&out_point) { + return; + } + // Only fetch the in-memory record when we actually need + // it (no proof was provided). Otherwise the proof we + // already have determines the status without a lookup. + if proof.is_none() { + info.core_wallet + .accounts + .standard_bip44_accounts + .get(&account_index) + .and_then(|a| a.transactions().get(&out_point.txid).cloned()) + } else { + None + } + // wm dropped here — release before persister I/O. }; - if info.tracked_asset_locks.contains_key(&out_point) { - return; - } + // Phase 2 (no lock held): resolve status. The persister + // fallback's I/O (synchronous lookup, possibly an FFI + // callback into a SwiftData query) is no longer serialized + // behind the wallet-manager write lock. let (status, proof) = match proof { Some(ref p) => { let status = match p { @@ -60,59 +82,85 @@ impl AssetLockManager { }; (status, proof) } - None => { - // Need to resolve from wallet info - drop the write guard and use a read. - // Actually we already have mutable access to info, so we can read from it. - self.resolve_status_from_info(&info.core_wallet, account_index, &out_point) - } + None => self.resolve_status_with_in_memory(in_memory_record, account_index, &out_point), }; - let lock = TrackedAssetLock { - out_point, - transaction: tx, - account_index, - funding_type, - identity_index, - amount, - status, - proof, + // Phase 3 (lock held): commit the tracked-asset-lock entry. + // We re-check `tracked_asset_locks.contains_key` because + // another caller could have raced in during phase 2 — first + // writer wins. + let cs = { + let mut wm = self.wallet_manager.blocking_write(); + let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) else { + return; + }; + if info.tracked_asset_locks.contains_key(&out_point) { + return; + } + let lock = TrackedAssetLock { + out_point, + transaction: tx, + account_index, + funding_type, + identity_index, + amount, + status, + proof, + }; + let mut cs = AssetLockChangeSet::default(); + cs.asset_locks.insert(out_point, (&lock).into()); + info.tracked_asset_locks.insert(out_point, lock); + cs + // wm dropped here — must release before queue_asset_lock_changeset + // since the persister flush may need the wallet-manager lock + // for other sub-changesets. }; - let mut cs = AssetLockChangeSet::default(); - cs.asset_locks.insert(out_point, (&lock).into()); - info.tracked_asset_locks.insert(out_point, lock); - - // Must drop the write guard before queuing — the persister's - // flush (if strategy is Immediate) may need the wallet manager - // lock for other sub-changesets. - drop(wm); self.queue_asset_lock_changeset(cs); } - /// Determine asset lock status by looking up the transaction in - /// `ManagedWalletInfo`, falling back to the persister if the - /// in-memory map evicted the record (default - /// `keep-finalized-transactions` off). + /// Determine asset lock status from a pre-snapshotted in-memory + /// record, falling back to the persister if the snapshot was + /// `None`. The caller is responsible for dropping the + /// wallet-manager lock between the snapshot and this call so the + /// persister fallback's I/O isn't serialized behind it. /// /// If the TX is in a chain-locked block, returns `ChainLocked` with a /// constructed `ChainAssetLockProof`. If the TX has an InstantSend /// context, returns `InstantSendLocked` (without a proof, since we lack /// the IS-lock data). Otherwise defaults to `Broadcast`. - fn resolve_status_from_info( + /// + /// Persister errors are logged at `error` and treated the same as + /// "not found" — we'd rather classify as `Broadcast` (and let + /// `resume_asset_lock` re-derive the proof via SPV) than abort + /// recovery entirely. The `error` log surfaces the failure to + /// operators since this is a one-shot path: a silently-degraded + /// `Broadcast` classification for a genuinely chain-locked tx + /// makes `resume_asset_lock` take the wasteful `wait_for_proof` + /// path instead of constructing a `ChainAssetLockProof` + /// directly, with no other signal that anything went wrong. + fn resolve_status_with_in_memory( &self, - wallet_info: &ManagedWalletInfo, + in_memory: Option, account_index: u32, out_point: &OutPoint, ) -> (AssetLockStatus, Option) { use super::proof::record_or_persister; use key_wallet::transaction_checking::TransactionContext; - let in_memory = wallet_info - .accounts - .standard_bip44_accounts - .get(&account_index) - .and_then(|a| a.transactions().get(&out_point.txid).cloned()); - - let record = record_or_persister(in_memory, &self.persister, &out_point.txid); + let record = match record_or_persister(in_memory, &self.persister, &out_point.txid) { + Ok(opt) => opt, + Err(e) => { + tracing::error!( + txid = %out_point.txid, + account_index, + error = %e, + "Persister fallback failed during asset-lock status \ + recovery; classifying as Broadcast (resume will \ + re-derive the proof via SPV)" + ); + None + } + }; match record { Some(record) => match &record.context { From 74f6b3f5b006f6cead7f75f63c3f6cba809aaa53 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 8 May 2026 00:52:29 +0700 Subject: [PATCH 6/6] chore(platform-wallet): drop unused ManagedWalletInfo import in recovery.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The import was referenced only in a doc comment after the `resolve_status_from_info` → `resolve_status_with_in_memory` rename dropped the `&ManagedWalletInfo` parameter. Spotted by @thepastaclaw on PR #3619 review. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs index 4a781af37cb..b26a140f3fd 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs @@ -11,7 +11,6 @@ use dashcore::Address as DashAddress; use dashcore::{OutPoint, PrivateKey}; use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; use key_wallet::wallet::managed_wallet_info::asset_lock_builder::AssetLockFundingType; -use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use crate::changeset::changeset::AssetLockChangeSet; use crate::error::PlatformWalletError;