Skip to content
Draft
Show file tree
Hide file tree
Changes from 12 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
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 lifecycle transition like registration and
// removal: 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 `remove_wallet` of a deterministic id this batch is
// mid-way through can neither drop a `wallet_manager` entry out
// from under an unpublished wallet nor detach a generation it
// never retired. 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
127 changes: 127 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,70 @@ 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 **lifecycle transitions** — registration
/// ([`register_wallet`](Self::register_wallet)), hydration
/// ([`load_from_persistor`](Self::load_from_persistor)) and removal
/// ([`remove_wallet`](Self::remove_wallet)) — over the
/// `wallet_manager` + `wallets` pair.
///
/// Neither of those two locks can do this job. Each transition is a
/// *multi-step* rewrite that takes them one at a time and releases
/// each before taking the next: registration inserts into
/// `wallet_manager`, persists, builds the handle, and only then
/// publishes into `wallets`; removal retires the asset-lock manager,
/// drops the `wallet_manager` entry, and only then detaches from
/// `wallets`. Interleaved, the two produce a torn result even though
/// every individual lock was held correctly.
///
/// The concrete hazard is a same-mnemonic re-import racing a removal.
/// `wallet_id` is deterministic in (seed, network), so once removal
/// has dropped the `wallet_manager` entry the id is free and a
/// concurrent `register_wallet` legitimately succeeds — publishing a
/// *replacement generation* into `wallets` before the removal reaches
/// its own `wallets.remove(wallet_id)`. That removal then detaches
/// the live replacement (whose `AssetLockManager` was never retired,
/// since `deactivate` ran against the previous generation) and hands
/// it back to the caller as the thing it removed. The manager is left
/// with a wallet registered in `wallet_manager` but absent from
/// `wallets` — invisible to the balance handler and every sync
/// coordinator, and un-removable, because a later `remove_wallet`
/// takes the `WalletNotFound` arm.
///
/// [`AssetLockManager::deactivate`](crate::AssetLockManager::deactivate)
/// cannot close this: it is per-*instance* by construction, and the
/// replacement's manager is a different instance with its own
/// `status_persist_serial`. Retirement makes a stale handle harmless;
/// it says nothing about which generation owns the map entry.
///
/// Lock ordering: this is the OUTERMOST lock. Acquire it before
/// `status_persist_serial` (via `deactivate`), before
/// `wallet_manager`, and before `wallets` — never the reverse, and
/// never from code already holding any of them. Nothing reachable
/// from inside a lifecycle transition re-enters one, so no cycle
/// exists.
pub(super) wallet_lifecycle_serial: tokio::sync::Mutex<()>,
/// Test-only gauge of tasks currently BLOCKED on
/// [`wallet_lifecycle_serial`](Self::wallet_lifecycle_serial):
/// incremented before the `lock().await` and RAII-decremented the
/// moment it is acquired (see
/// [`lock_wallet_lifecycle_serial`](Self::lock_wallet_lifecycle_serial)).
///
/// The arrival signal the lifecycle-ordering test rendezvous on, for
/// the same reason
/// [`status_serial_waiters`](crate::AssetLockManager) exists: a sleep
/// cannot distinguish "the competing registration is queued at the
/// boundary" from "it has not been scheduled yet", so a test that
/// released its parked removal after a delay would grade an
/// unserialized implementation as passing whenever the scheduler
/// happened to run things in the non-regressing order.
#[cfg(test)]
pub(super) wallet_lifecycle_waiters: std::sync::atomic::AtomicUsize,
Comment thread
thepastaclaw marked this conversation as resolved.
Outdated
/// Test-only pause point inside
/// [`remove_wallet`](Self::remove_wallet), between dropping the
/// shared `WalletManager` entry and detaching the handle from
/// `wallets`. `None` (the default) makes the hook a no-op.
#[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 +605,54 @@ 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)]
wallet_lifecycle_waiters: std::sync::atomic::AtomicUsize::new(0),
#[cfg(test)]
remove_pre_detach_gate: std::sync::Mutex::new(None),
}
}

/// Acquire
/// [`wallet_lifecycle_serial`](Self::wallet_lifecycle_serial).
///
/// Every lifecycle transition goes through here rather than locking
/// the field directly, so the test-only
/// [`wallet_lifecycle_waiters`](Self::wallet_lifecycle_waiters) gauge
/// sees every arrival at the boundary. In non-test builds this
/// compiles to the bare `lock().await`.
pub(super) async fn lock_wallet_lifecycle_serial(&self) -> tokio::sync::MutexGuard<'_, ()> {
// RAII rather than a bare decrement after the await: if the
// caller's future is dropped while still queued, the count must
// come back down, or a cancelled registration would leave the
// gauge permanently non-zero and every later wait would return
// instantly on a phantom arrival.
#[cfg(test)]
struct WaiterGauge<'a>(&'a std::sync::atomic::AtomicUsize);
#[cfg(test)]
impl Drop for WaiterGauge<'_> {
fn drop(&mut self) {
self.0.fetch_sub(1, std::sync::atomic::Ordering::SeqCst);
}
}
#[cfg(test)]
let waiting = {
self.wallet_lifecycle_waiters
.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
WaiterGauge(&self.wallet_lifecycle_waiters)
};

let guard = self.wallet_lifecycle_serial.lock().await;

// Dropped on acquisition, not on release: the gauge answers "who
// is still queued at the boundary", so the holder must not count
// itself.
#[cfg(test)]
drop(waiting);

guard
}

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