Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions packages/rs-platform-wallet-ffi/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,14 @@ impl From<PlatformWalletError> for PlatformWalletFFIResult {
PlatformWalletError::AssetLockFundingMismatch { .. } => {
PlatformWalletFFIResultCode::ErrorAssetLockFundingMismatch
}
// A retained asset-lock manager whose wallet generation was removed
// (or replaced under the same deterministic id). The owning generation
// no longer exists, so the same NotFound semantic as a missing wallet
// handle applies — Swift/Kotlin re-acquire from the current wallet
// rather than parsing the English message.
PlatformWalletError::AssetLockManagerInactive(..) => {
PlatformWalletFFIResultCode::NotFound
}
// A quiesce/drain barrier that did not complete within budget
// (clear/reset paths). The host must fail closed: keep its
// callback context alive and skip any paired persistence wipe.
Expand Down Expand Up @@ -1042,6 +1050,32 @@ mod tests {
);
}

/// A retained asset-lock manager whose wallet was removed must surface
/// as NotFound (the generation no longer exists), not ErrorUnknown —
/// Swift/Kotlin re-acquire from the current wallet on that code.
#[test]
fn asset_lock_manager_inactive_maps_to_not_found() {
let err = PlatformWalletError::AssetLockManagerInactive("deadbeef".to_string());
let rendered = err.to_string();
let result: PlatformWalletFFIResult = err.into();
assert_eq!(
result.code,
PlatformWalletFFIResultCode::NotFound,
"AssetLockManagerInactive should map to NotFound (rendered: {rendered})"
);
let msg = unsafe { std::ffi::CStr::from_ptr(result.message) }
.to_string_lossy()
.into_owned();
assert_eq!(
msg, rendered,
"Display payload must survive the FFI boundary verbatim"
);
assert!(
msg.contains("no longer active"),
"typed Display must name the inactive condition: {msg}"
);
}

/// Other wallet-error variants without a dedicated FFI arm still
/// fall through to `ErrorUnknown` while carrying the typed
/// Display rendering as the message. Pin this so the catch-all
Expand Down
23 changes: 23 additions & 0 deletions packages/rs-platform-wallet/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,29 @@ pub enum PlatformWalletError {
actual_identity_index: u32,
},

/// The operation was issued through an `AssetLockManager` whose wallet
/// has since been removed from the `PlatformWalletManager`.
///
/// Wallet ids are deterministic in (seed, network), so re-importing the
/// same mnemonic re-creates the very same id against a *fresh*
/// `PlatformWalletInfo` and a *fresh* `AssetLockManager`. A handle
/// retained across the removal (an FFI `asset_lock_manager` handle the
/// host never destroyed, or an in-flight resume task) resolves through
/// the shared `WalletManager` by id alone, so without this guard it
/// would silently start mutating and persisting the replacement
/// wallet's rows under a different `status_persist_serial` than the
/// live manager — reintroducing the very stale-snapshot reversal the
/// ordering mutex closes within one instance.
///
/// Always a stale-handle bug on the caller's side; the fix is to
/// re-acquire the manager from the current `PlatformWallet`.
#[error(
"Asset lock manager for wallet {0} is no longer active — its wallet was \
removed from the manager; re-acquire the asset lock manager from the \
current wallet handle"
)]
AssetLockManagerInactive(String),
Comment thread
thepastaclaw marked this conversation as resolved.

#[error("SDK error: {0}")]
Sdk(#[from] dash_sdk::Error),

Expand Down
11 changes: 11 additions & 0 deletions packages/rs-platform-wallet/src/manager/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,17 @@ impl<P: PlatformWalletPersistence + 'static> PlatformWalletManager<P> {

let persister_dyn: Arc<dyn PlatformWalletPersistence> = Arc::clone(&self.persister) as _;

// Hydration is a multi-step registration-class rewrite: each wallet
// goes live in `wallet_manager` well before it is published into
// `self.wallets`, and the batch rollback at the bottom unwinds
// both maps. Held across the whole loop so a concurrent
// registration/hydration of a deterministic id this batch is
// mid-way through cannot interleave those steps. Removal is
// generation-gated separately and does not take this mutex —
// see
// [`wallet_lifecycle_serial`](PlatformWalletManager::wallet_lifecycle_serial).
let _lifecycle = self.lock_wallet_lifecycle_serial().await;

// Track every wallet successfully inserted into
// `wallet_manager` and `self.wallets` during this call so the
// batch is transactional: if any later iteration fails (id
Expand Down
64 changes: 64 additions & 0 deletions packages/rs-platform-wallet/src/manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,24 @@ pub(crate) fn coordinator_worker_config() -> WorkerConfig {
}
}

/// Test-only rendezvous for
/// [`PlatformWalletManager::remove_pre_detach_gate`]. `arrived` fires
/// once [`remove_wallet`](PlatformWalletManager::remove_wallet) has
/// retired the asset-lock manager and dropped the shared
/// `WalletManager` entry but has NOT yet detached the handle from
/// `wallets`; the removal then blocks on `release`.
///
/// That is precisely the window in which a same-mnemonic re-import can
/// succeed (the id it collides on is already free) and publish a
/// replacement generation the removal would then detach as if it were
/// the generation it retired.
#[cfg(test)]
#[derive(Clone)]
pub(super) struct RemovePreDetachGate {
pub(super) arrived: Arc<Notify>,
pub(super) release: Arc<Notify>,
}

/// Multi-wallet coordinator with SPV sync and event handling.
///
/// Events are dispatched through [`PlatformEventManager`] to all registered
Expand Down Expand Up @@ -400,6 +418,41 @@ pub struct PlatformWalletManager<P: PlatformWalletPersistence + 'static> {
/// failed / rescan pending" state rather than re-freezing silently on
/// the next launch.
pub(super) sync_fault: Arc<std::sync::atomic::AtomicBool>,
/// Serializes whole-wallet **registration and hydration** —
/// [`register_wallet`](Self::register_wallet) /
/// [`create_wallet_from_seed_bytes`](Self::create_wallet_from_seed_bytes)
/// and [`load_from_persistor`](Self::load_from_persistor) — over the
/// multi-step `wallet_manager` + `wallets` rewrite those paths perform.
///
/// Registration inserts into `wallet_manager`, persists, builds the
/// handle, and only then publishes into `wallets`. Hydration walks the
/// same maps in the opposite direction. Without an outer serial, two
/// concurrent creates (or a create racing a load) can interleave those
/// steps even though every individual map lock was held correctly.
///
/// Removal does **not** take this mutex. Same-id re-import is allowed
/// in the free-id window after a removal has dropped the inner
/// `WalletManager` entry; the replacement is protected by
/// generation-aware detach (`Arc::ptr_eq` on the retired generation)
/// and by each generation's own lifecycle gate
/// ([`WalletGeneration::teardown_guard`](crate::wallet::core::WalletGeneration::teardown_guard)),
/// not by serializing registration against removal. See
/// [`remove_wallet_with_teardown`](Self::remove_wallet_with_teardown).
///
/// Lock ordering: when held, this is the OUTERMOST lock of a
/// registration/hydration transition. Acquire it before
/// `wallet_manager` and before `wallets` — never the reverse, and
/// never from code already holding either. Nothing reachable from
/// inside a registration/hydration transition re-enters one, so no
/// cycle exists.
pub(super) wallet_lifecycle_serial: tokio::sync::Mutex<()>,
/// Test-only pause point inside
/// [`remove_wallet_with_teardown`](Self::remove_wallet_with_teardown),
/// between dropping the shared `WalletManager` entry and detaching
/// the handle from `wallets`. `None` (the default) makes the hook a
/// no-op. Used by the generation-aware detach regression test.
#[cfg(test)]
pub(super) remove_pre_detach_gate: std::sync::Mutex<Option<RemovePreDetachGate>>,
}

impl<P: PlatformWalletPersistence + 'static> PlatformWalletManager<P> {
Expand Down Expand Up @@ -523,9 +576,20 @@ impl<P: PlatformWalletPersistence + 'static> PlatformWalletManager<P> {
event_adapter_join: tokio::sync::Mutex::new(Some(event_adapter_join)),
registry,
sync_fault,
wallet_lifecycle_serial: tokio::sync::Mutex::new(()),
#[cfg(test)]
remove_pre_detach_gate: std::sync::Mutex::new(None),
}
}

/// Acquire
/// [`wallet_lifecycle_serial`](Self::wallet_lifecycle_serial) for a
/// registration or hydration transition. Removal does not take this
/// lock — see the field docs.
pub(super) async fn lock_wallet_lifecycle_serial(&self) -> tokio::sync::MutexGuard<'_, ()> {
self.wallet_lifecycle_serial.lock().await
}

/// Whether the wallet-event adapter has frozen a durable sync
/// watermark this manager's lifetime (dashpay/platform#4069).
///
Expand Down
Loading
Loading