fix: vss client recovery and reset race

[ovitrif]

Fixes #1256
Fixes #1257

This PR:

1. Makes the VSS backup client recover after a failed setup instead of staying broken until the app restarts
2. Stops the node before the keychain is wiped on Reset, so a wipe no longer races an in-flight node start

Description

• Resets the setup state when setup fails, so a later successful setup actually makes the client usable. Previously the first failure was latched: every following setup reported success without doing anything, and every VSS call threw the stale error. Setup also captures its own gate, so a reset that swaps the gate mid-setup can no longer be completed by the old wallet's client.
• Restructures the wallet wipe into three phases. First it resets the backup clients and stops the node; this is the only step that can fail the reset, and it fails before anything is destroyed, so an in-flight node start is awaited under the lifecycle mutex instead of raced. Then it runs the remote Paykit and Pubky cleanup, which needs the keychain session, as best-effort. Finally it wipes local state, with the LDK storage wipe as the last abort point: if the node directory cannot be removed, nothing else is destroyed and the reset fails with the wallet intact. Every later local step is logged on failure and never aborts the rest; if the keychain wipe itself fails, the remaining steps still run, the reset reports a failure, and the wallet-exists state is re-read from the keychain so the user stays in the wallet where Reset can be retried. A reset cannot leave a wallet that still looks alive with its node data gone, and the LDK directory is never wiped after a new mnemonic was saved.
• Skips starting the backup observers while a wipe is in progress, so they never call VSS setup without a mnemonic, and restarts them after a failed node stop when the node is still running.
• Rejects a second Reset while one is in flight, and shows the Reset button in a loading state with back navigation and the Backup button disabled until the wipe completes.
• Makes LightningRepo.stop() really stop a node object that a failed start left alive instead of short-circuiting on the Stopped state, so Reset during the start retry window tears the node down before any cleanup rather than failing later at the storage wipe. The storage wipe now holds the lifecycle lock across the stop and the directory removal, and any node start arriving during the wipe is refused under that lock instead of queued, so nothing can rebuild the old seed's node before the keychain is gone.
• Applies the same setup recovery to the LDK VSS client used by Reset network graph and Reset pathfinding scores.
• Motivation: one failed setup (network or auth hiccup at first node start, or the wipe race from #1254) used to silently kill all VSS backups for the session and could send a restore to the RN backup.

Out of Scope

• WalletViewModel.start(): a restore-triggered start dropped by a stale isStarting flag (#1257, remaining part).
• BackupRepo restore picker: falling back to the RN backup when the VSS lookup returns null (#1254).
• Surfacing setup failures in the UI or adding retries beyond the existing setup-with-retry path.
• Cancelling an in-flight node start from Reset; the wipe waits for it as before, now visibly.

Design

ResetAndRestoreScreen gains a loading state on the Reset button while wiping. N/A — no design available.

Preview

N/A

QA Notes

Journeys

N/A — no backup/restore journey exists yet.

Manual Tests

• 1. Onboarding → New Wallet: node starts and all backup categories upload (Backup succeeded in logs).
• 2. Settings → Security → Reset and Restore → Reset → New Wallet: logs show backup reset, node stopped, remote cleanup, LDK storage wiped, then keychain wiped; the new wallet's node starts and backups upload.
• 3. Reset and Restore → Reset → Yes, Reset: Reset button shows a spinner, Backup button and back are disabled until onboarding shows.

The failed-setup path itself could not be reproduced on-device: setup does no network I/O and the app skips node start while offline, so the only real trigger is the wipe race from #1254. That path is proven by the unit tests only.

Automated Checks

• Unit tests added: cover a successful setup and setup-with-retry after a failed attempt leaving the client usable in VssBackupClientTest.kt, the same recovery in VssBackupClientLdkTest.kt, and observers being skipped while wiping in BackupRepoTest.kt.
• Unit tests added: cover the stop of a node left alive by a failed start, a start being excluded while the storage wipe holds the lifecycle lock, and a start being refused while wiping, in LightningRepoTest.kt; wallet existence re-read after an incomplete wipe in WalletRepoTest.kt.
• Unit tests added and modified: assert the phase order, the re-entrancy guard, completion despite later local step failures, the reported failure when the keychain wipe fails, the no-wipe path when the LDK storage wipe fails, and the no-wipe plus observer restart path when the node stop fails in WipeWalletUseCaseTest.kt.
• Local: just compile, just test, and just lint pass.
• On-device (Pixel 10 emulator, dev build): Reset → new wallet logs the phase order (backup reset, node stopped, remote cleanup, LDK storage wiped, then keychain wiped), the new wallet's node starts, VSS setup succeeds on attempt 1 and every backup category uploads. Reset shows the loading state on the Reset button with Backup and back disabled until onboarding appears.

[greptile-apps]

Confidence Score: 4/5

The PR should not merge until a failure after successful Lightning storage deletion can no longer leave the old wallet active with its node data erased.

Findings

1.  Wipe Can Leave Broken Wallet ▶

Summary

This PR makes VSS setup recoverable after transient failures, prevents backup observers from starting during wallet erasure, and moves Lightning shutdown and storage deletion ahead of keychain cleanup.

• Failed VSS setup now publishes the failure and installs a fresh setup gate for retries.
• Wallet reset stops and deletes Lightning state before clearing protected wallet data.
• Backup observation is suppressed while the wipe flag is active.
• Tests cover VSS recovery, wipe-time observer suppression, and the revised wipe order.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Reset requested] --> B[Mark backups as wiping]
    B --> C[Reset backup clients and observers]
    C --> D[Stop node and delete LDK storage]
    D --> E[Clean Paykit and Pubky state]
    E --> F[Wipe keychain]
    F --> G[Clear databases and app stores]
    G --> H[Reset wallet state]
    H --> I[Switch to onboarding]
    E -->|Failure| J[Return failure while old wallet remains represented]
    D -. LDK data already deleted .-> J

Loading

_{Reviews (2) · Last reviewed commit: "fix: stop node before wiping keychain on..."}

[github-actions]

Regtest APK

Built from bf677b3 (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

[piotr-iohk]

QA review from the diff and thread only — not run on device.

1. [gap] VssBackupClientLdk.setup() still does isSetup.completeExceptionally(it) without replacing the deferred — the same latch fixed here in VssBackupClient. Callers are clearNetworkGraph and resetPathfindingScores (LightningRepo.kt:665, :1904): one failed LDK-client setup makes every later "Reset network graph" / "Reset pathfinding scores" fail with the stale error and restart the node until the app is restarted. Same two-line fix; arguably in #1256's scope.

2. [+1 greptile] The partial-wipe window (LDK deleted, keychain kept) is real but narrow — I checked the steps between wipeStorage and keychain.wipe() and only privatePaykitAddressReservationRepo.clear() and keychain.wipe() itself can throw. Still, if it happens the app presents a wallet whose channel state is gone. Consider calling resetWalletState() regardless of failure once wipeStorage has succeeded, or treating the remaining local wipes as non-fatal.

3. [gap] Reset is now a long blocking operation with no UI feedback. ResetAndRestoreScreen never clears showDialog on confirm, so the dialog stays up until resetWalletState(), which now runs after stop() — the full node build + initial sync, or minutes in the #1257 stop-hang case. rememberDebouncedClick only debounces the tap; nothing stops a second wipeWallet() mid-flight, and WipeWalletUseCase has no re-entrancy guard, so the second run's finally { setWiping(false) } can clear the flag while the first is still wiping. Before this PR the onboarding switch was near-immediate so the window was tiny. A wiping flag on the dialog/button or a mutex in the use case would close it.

4. [nit] If stop() fails, wipeStorage().getOrThrow() aborts after backupRepo.reset() already stopped the observers and reset the VSS clients. Wallet survives, but backups are silently off until the next Running transition (app restart). Pre-existing ordering, new abort point — worth restarting observers on that failure path.

[ovitrif]

Original review

VssBackupClientLdk.setup() still does isSetup.completeExceptionally(it) without replacing the deferred

The partial-wipe window (LDK deleted, keychain kept)

no UI feedback

backups are silently off

@piotr-iohk Thanks, all four addressed in 2b5431e:

1. VssBackupClientLdk.setup() now replaces the deferred on failure, same as VssBackupClient; covered by VssBackupClientLdkTest.kt.
2. keychain.wipe() moved right after wipeStorage(), so a later cleanup failure leaves the pre-PR state (no keychain, no node data) rather than a wallet without its node data.
3. WipeWalletUseCase takes a tryLock mutex and returns WipeAlreadyInProgress for a second call. The screen dismisses the dialog on confirm and, while BackupRepo.isWiping, shows the Reset button loading, disables Backup, and blocks back navigation.
4. On wipe failure the use case restarts the backup observers when the node is still Running.

[ovitrif]

An agent is running and will check the Test 3 checkbox if it succeeds.

[piotr-iohk]

Re-reviewed 2b5431e from the diff — not run on device. Items 1, 3 and the Greptile P1 look good; two new issues from the fix-up:

1. [blocker] keychain.wipe() now runs before removePublishedEndpointsForCleanup, removeBitkitPaymentEndpoints and closeAndClear. All three go through PaykitSdkService, whose PaykitSdkSessionProvider.loadSessionAccess() reads PAYKIT_SESSION from the keychain on every call and returns null once it is gone (liveSessionAccess is only returned when its secret matches the keychain value). So syncPublicEndpoints(emptyList()) / removePaykitReceiverMarker() run without a session, fail, and are swallowed — and the contactSharingCleanupPending flag is then erased by settingsStore.reset(). Result: the wiped wallet's public endpoints and receiver marker stay published after Reset; on master they were removed. Every step between wipeStorage and the old keychain.wipe() position returns Result or swallows (only privatePaykitAddressReservationRepo.clear() can throw), so moving keychain.wipe() back after pubkyRepo.wipeLocalState() keeps Greptile's window closed in practice; wrap clear() if you want it airtight.

2. [gap] The observer restart in onFailure is dead code: backupRepo.startObservingBackups() runs while _isWiping is still true (cleared in finally), so the guard added in this PR returns early with "Skipped observing backups while wiping". The unit test only passes because BackupRepo is a mock. Call setWiping(false) before the restart, or move the restart after the finally.

[ovitrif]

Original review

keychain.wipe() now runs before removePublishedEndpointsForCleanup, removeBitkitPaymentEndpoints and closeAndClear.

The observer restart in onFailure is dead code

@piotr-iohk Both addressed in 47187b3:

1. keychain.wipe() is back after pubkyRepo.wipeLocalState(), so the Paykit and Pubky cleanup still see the session. privatePaykitAddressReservationRepo.clear() is wrapped and logged, so nothing between wipeStorage() and the keychain wipe can abort anymore; covered by a new case in WipeWalletUseCaseTest.kt.
2. The observer restart now runs after the finally that clears _isWiping, so the guard no longer short-circuits it. The test asserts setWiping(false) precedes startObservingBackups().

[ovitrif]

Handed off to pr-babysit skill which watches over reviews and CI. Warning: first run of this skill.

[ovitrif]

Original review

two new issues from the fix-up:

@piotr-iohk Both addressed in 43449af, which replaces the previous fix-up rather than moving lines around again. The wipe is now phased:

1. Reset backup clients and lightningRepo.stop(). This is the only step that can fail the reset, and it fails before anything is destroyed. On failure the observers restart after the wiping flag is cleared, so the guard no longer short-circuits it (your point 2).
2. Remote Paykit and Pubky cleanup, best-effort, while the keychain session still exists (your point 1).
3. Local wipe: LDK storage, reservations, Pubky local state, keychain, FCM token, Core, Room, stores, repo state. Every step is wrapped and logged and never aborts the rest, so the partial-wipe window is closed by construction rather than by ordering.

WipeWalletUseCaseTest.kt covers the phase order, completion despite local step failures, and the no-wipe plus observer restart path when the stop fails. Verified on the emulator: the log shows reset, node stopped, remote cleanup, LDK wiped, keychain wiped, then onboarding.

[jvsena42]

No blockers. Two LOW observations below, both pre-existing on v2.4.1 and neither introduced by this PR — I'm raising them only because they sit inside the window this PR's title claims to close, and in one case the PR body's wording is stronger than the code.

Neither is a reversal: greptile's guard, piotr's items 1-4 and the phased-wipe design in 43449af all stay as landed.

Checked and clean.

Exits from ResetAndRestoreScreen: system Back while wiping is swallowed by BackHandler(enabled = isWiping) {} (:76); top-bar back is hidden via onBackClick = null (:81); dialog dismiss only flips showDialog; Confirm launches on the activity-scoped WalletViewModel, so navigating away via the still-enabled drawer icon doesn't cancel the wipe; backgrounding pauses collection but the wipe continues on viewModelScope, and ON_STOP's stopDebounced() is a no-op against an already-stopped node; on failure isWiping clears, a toast fires, and the wallet is intact.

Ordering and lifecycle: remote cleanup doesn't depend on the LDK node, so moving it after stop() is safe, and it runs before keychain.wipe() so the Paykit session is still present — the ordering piotr asked for. stop() can't fail merely because the node never started (LightningService.stop() returns early on node == null), so Forgot-PIN before node start and ErrorStarting still reset. Observer restart on stop() failure runs after setWiping(false), so the new _isWiping guard in startObservingBackups doesn't short-circuit it.

Concurrency: wipeMutex.tryLock + finally unlock is correct, and a second concurrent wipe gets WipeAlreadyInProgress. step() uses runSuspendCatching, so a genuine CancellationException propagates out of the wipe. Cancelling during stopNode/cleanupRemote leaves the wallet intact — stop() is NonCancellable inside its lock and nothing destructive has run. An ON_START node restart mid-wipe is self-healing: wipeStorage's own stop() serialises on lifecycleMutex.

Key material: nothing in the touched code logs a mnemonic or passphrase; VssBackupClient logs only the VSS/LNURL URLs. Keychain.wipe() clears the DataStore and resets the keystore key, and the PR adds no seed-derived artifact that survives it — except the LDK-directory case below. Remote VSS state is intentionally kept for restore; unchanged.

Upgrade: no persisted format changed, and a v2.4.1 VSS backup is read by unchanged BackupRepo code.

I also checked the claim that setup() does no network I/O and it holds — VssClient::new_with_lnurl_auth only derives xprivs and builds the header provider; the JWT is fetched lazily on first request. That's what makes finding 1 below a millisecond-wide window rather than a 30-second one.

[jvsena42]

ee283d43f closes both LOW observations from my last pass. No blockers.

I verified the wipe fix by reverting the three main-code files to 43449afd6 while keeping the new tests: exactly one failure, invoke should fail before wiping local state when LDK storage wipe fails. So the test is load-bearing rather than decorative, and it asserts verify(keychain, never()).wipe() and verify(db, never()).clearAllTables() rather than just a Result. At head, WipeWalletUseCaseTest 11/11, VssBackupClientTest 6/6, VssBackupClientLdkTest 1/1 all pass. Run in a throwaway worktree outside the repo; nothing committed, working tree untouched.

One consequence of the change I asked for is worth your attention before merge — the abort now lands after cleanupRemote(), so a recoverable "wallet alive, Paykit endpoints unpublished" state is newly reachable. Details and the recovery path are in my reply on the WipeWalletUseCase.kt:88 thread. I'm not asking you to change it; it's a better trade than the leftover-LDK-directory outcome it replaces.

Also checked on the new control flow:

• runSuspendCatching at :59 replacing stopNode().map {} keeps cancellation propagating, and finally still clears isWiping and unlocks. A side benefit: the non-step calls in wipeLocal (the resetState()s, resetWalletState(), onSuccess()) now surface as a Result failure and a toast instead of an uncaught exception in viewModelScope.
• keychain.wipe() is unreachable on the abort path by construction.
• A second concurrent wipe still returns WipeAlreadyInProgress.
• The new abort path leaves the user on ResetAndRestore with Back and Reset re-enabled and a toast — it doesn't wedge isWiping.
• Re-walked all seven exits from ResetAndRestoreScreen (the file didn't change, but the wipe's control flow did): Back while wiping, top-bar back, dialog dismiss, Confirm, drawer navigation mid-wipe, backgrounding, process death before and after the keychain step. All still correct.
• No seed-derived material reaches a log or a toast on any new path — the new lines add no Logger calls, and NodeStillRunning's message is a fixed string.

On the VSS gate: the capture is right in both clients, and isSetup === gate stops a stale setup clobbering a fresh deferred. I traced the setupWithRetry path specifically for a poisoned-gate regression and there isn't one — an exceptionally-completed deferred reports isCancelled, so the guard doesn't short-circuit and the next setup() gets a fresh gate. Neither client has a test covering that race in either direction; noting it, not asking for one.

[jvsena42]

ed82850af is correct. No blockers, nothing new to raise as its own thread — one LOW refinement is on the WipeWalletUseCase.kt:88 thread, and it's a consequence of the change I asked for rather than a defect you introduced.

This commit reaches into LightningRepo, which the PR had deliberately left alone, so I reviewed it as a change to shipped lifecycle code rather than as a one-line follow-up. The question that mattered was whether making the early return conditional regresses stop() on the hot path. It doesn't:

• "Stopped with a live node" is not a routine state. The only writer of a non-null node is setup() under start()'s mutex, and it's reachable only when initialLifecycleState was Stopped — bounded to the 2s retry window, plus a recovery-mode edge. Initializing and ErrorStarting already took the real path before this commit, so a real stop against a set-up-but-unstarted node was already exercised (restartWithElectrumServer failure → restartWithPreviousConfig).
• stopDebounced's 5s delay outlives the 2s retry, so a normal background cycle sees no new behaviour. In recovery mode the change is strictly better: start() returns early without touching state, so Stopped + live node used to persist and make every Recovery-screen wipe abort.
• No caller inherits a new failure. LightningService.stop() can't realistically throw — it's NonCancellable, node.stop() is inside runSuspendCatching, and releaseHandle catches destroy() failures. I walked all ten stop() call sites anyway; the ones that ignore the result (LightningNodeService ×2, WakeNodeWorker, onProceedWithoutRestore) get the intended outcome, and the ones that handle failure already did.
• The node read is safely published: @Volatile at LightningService.kt:171, and both the write at :209 and the read at :618 are under lifecycleMutex regardless.
• The fall-through emits Stopping → Stopped and replaces the whole LightningState, which the early return didn't. Every real stop already does this and observers tolerate it; the retry re-derives isGeoBlocked. No probe-cache leak, since a node that never ran has emitted no events.

Tests are load-bearing: reverting line 618 alone fails exactly stop tears down a node object left alive by a failed start with WantedButNotInvoked: lightningService.stop(). The second case passes in both states, which is right — it pins the no-op branch. 115/115 at head. Throwaway worktree outside the repo, nothing committed, main checkout untouched.

Also confirmed the previous commit's work is byte-identical — the captured VSS setup gate and the fatal LDK-wipe step are untouched, and git diff --stat ee283d43f ed82850af is just these two files.

[piotr-iohk]

Re-reviewed 43449af..effa097 from the diff — not run on device. Both items from my last pass are fixed, and the phased design plus the stop()/lock hardening from the jvsena42 threads look right. One consequence of the phases, fine to take as a follow-up:

1. [gap] keychain.wipe() is now a best-effort step(). If it throws, wipeLocal() still returns success, resetWalletState() and onSuccess() run, and setWalletExistsState() sees the mnemonic still there → walletExists = true. The user lands back in a wallet whose LDK dir, Core data, Room and settings are gone but seed and PIN remain, with only a warn log; before the rewrite this aborted with a toast. Rare (DataStore/Keystore write failure) and recoverable (LDK state rebuilds from VSS, a second Reset retries), so not a merge blocker — but either make keychain.wipe() a second abort point (fail the Result so the toast fires) or toast when any local step fails. Past wipeStorage, "success" should mean onboarding.

Blockers fixed

[ovitrif]

Original review

either make keychain.wipe() a second abort point (fail the Result so the toast fires) or toast when any local step fails.

@piotr-iohk Taken in 38c9df8, your first option: keychain.wipe() is now the second abort point. The remaining local steps still run so the stores stay consistent, then wipeLocal() returns WipeIncomplete ("please reset again"), onSuccess() is skipped and the toast fires. Past wipeStorage, success now always means onboarding. Covered by a new case in WipeWalletUseCaseTest.kt.

[piotr-iohk]

QA from the diff plus emulator (emulator-5554, CI APK 38c9df8). Happy-path Reset and New Wallet backups pass; Pubky cleanup ran with a session before the keychain wipe.

1. [nit] If keychain.wipe() throws, resetWalletState() still sends the user to onboarding while the mnemonic is on the keychain, and the toast says “please reset again” from a screen that no longer has Reset. Rare, recoverable. Not a merge blocker.

On device:

• Reset while the node was Running landed on TOS with the expected order (observers stopped → VSS reset → node stopped → remote cleanup → LDK wiped → keychain wiped). New Wallet then set up VSS on attempt 1 and uploaded METADATA / WIDGETS / ACTIVITY. No MnemonicNotAvailableException.
• Re-tested on a wallet with Pubky + a contact: private Paykit cleanup warned PrivateUnavailable (best-effort); public cleanup then issued delete_payment_endpoint (btc-regtest-p2wpkh) and delete_paykit_receiver_marker before keychain wipe. No identity_error / no Pubky session. Did not confirm from a second device that the old identity is unresolvable.
• Reset-during-Building node… and the double-tap guard were not reproduced (wipe ~0.5s without Paykit; ~6s with).

[jvsena42]

Reviewed effa097b7 and 38c9df844. No HIGH/MEDIUM. Two LOWs: one inline below, and one as a reply on the WipeWalletUseCase.kt:88 thread, because it refines the lock I asked for there.

Checked and clean

• stop() / stopLocked() split. The body is equivalent: return Result.success(Unit) replaces return@withLock, and runCatching/NonCancellable/onFailure are unchanged. wipeStorage is still withContext(bgDispatcher), so stopLocked runs on the same dispatcher.
• No re-entrant lock. Nothing inside wipeStorage's hold acquires lifecycleMutex again. LightningService.stop() → listenerJob?.cancelAndJoin() cancels any handler waiting on the mutex, since Mutex.lock() is cancellable. releaseHandle/awaitNodeRelease join an ioDispatcher job that only calls destroy(). clearProbeOutcomes and setRecoveryMode are map and StateFlow writes. All six stop() call sites enter through the public stop().
• Hold duration. The lock is now also held across awaitNodeRelease() (≤90s) and the delete. The callers that can block are start(), stopDebounced, restartWithPreviousConfig and WakeNodeWorker. The worker's 2-minute withTimeout covers the deliver signal, not start, so a long hold delays it without failing it. Every entry point is on bgDispatcher, so there is no main-thread wait.
• Second Reset after WipeIncomplete. deleteRecursively() on a missing directory returns true, awaitNodeRelease is a no-op on a null job, and cleanupRemote without a session is swallowed by step(). A retry from inside the wallet works.
• Cancellation. step() still uses runSuspendCatching. A cancellation that stopLocked().mapCatching wraps inside wipeStorage is rethrown by getOrThrow() inside the outer runSuspendCatching. Nothing local is destroyed before deleteRecursively.
• Keychain.wipe() partial failure. edit { clear() } is atomic. If it succeeds and resetEncryptionKey() throws, the entries are gone and the toast is misleading but harmless.
• Callers of the new failure. WalletRepo.wipeWallet is the only caller of the use case. WalletViewModel, RecoveryViewModel and ForgotPin show the failure as a toast; DevSettingsViewModel ignores it. Only the use case calls lightningRepo.wipeStorage.
• Tests. wipeStorage holds the lifecycle lock… fails without the lock. invoke should fail after wiping the rest when keychain wipe fails pins the WipeIncomplete result. See the inline comment for the landing it also pins.
• Logs. No new log line touches seed material, and the WipeIncomplete message is a fixed string.

[jvsena42]

Reviewed 8acaa6759. No HIGH/MEDIUM, one LOW inline. Both fixes from my last pass hold.

Checked and clean

• Wipe flag. It is checked inside lifecycleMutex (LightningRepo.kt:347) before initialLifecycleState is read and before Starting is emitted. A refused start leaves the lifecycle state alone and schedules no retry, and a start that queued during wipeStorage's hold is now refused when it takes the lock. The flag is set right after wipeMutex.tryLock() and cleared in finally, so a failed or cancelled wipe cannot leave it stuck. A start after finally either finds an empty keychain (MnemonicNotFound) or, on WipeIncomplete, a wallet the user is intentionally kept in.
• Callers of the refused start. WakeNodeWorker.kt:85 ignores the Result and waits on the deliver signal as before. LightningNodeService.setupService only acts on success. AppViewModel.completeRNRemoteBackupRestore logs the error. In recovery mode, RecoveryModeError still wins because it is checked first.
• WipeIncomplete landing. WalletRepo.wipeWallet reads keychain.exists(BIP39_MNEMONIC) again after resetState(). A failed clear() keeps the user in the wallet with Reset available; a failed resetEncryptionKey() alone still lands on onboarding, where Create works. WalletRepoTest.kt pins the first case.
• Tests. start is refused while a wipe is in progress checks both the failure type and that lightningService.start is never called. The lock test keeping times(2) is correct, because it never sets the flag.

[jvsena42]

Reviewed bf677b369. No findings, and the toast LOW from my last pass is fixed.

Checked and clean

• WalletViewModel.startNode (:344-351) returns early for both RecoveryModeError and WipeInProgressError, logging at debug with no toast. Every other failure still logs at error and shows a toast, same as before.
• The only change in behaviour for RecoveryModeError is that it is now logged at debug instead of error. Recovery mode is a deliberate user state, so that fits.
• Logger.debug has no Throwable parameter (Logger.kt:81), so writing the message into the log text is the only way to keep the reason.
• No other start caller surfaces WipeInProgressError to the user: WakeNodeWorker ignores the Result, LightningNodeService.setupService only acts on success, and AppViewModel.completeRNRemoteBackupRestore only logs.
• No existing test asserts on the RecoveryModeError toast branch, so nothing depends on the old logging.

At bf677b369 I have nothing further on this PR. CI was still pending when I checked.

[jvsena42]

tAck

[piotr-iohk]

QA from the diff plus emulator (emulator-5554, CI APK bf677b3). No new findings. The keychain-incomplete and start-during-wipe follow-ups look right in the diff.

On device:

• Reset while the node was Running landed on TOS with the expected order (observers stopped → VSS reset → node stopped → remote cleanup → LDK wiped → keychain wiped). New Wallet then set up VSS on attempt 1 and uploaded METADATA / ACTIVITY. No MnemonicNotAvailableException.
• Background + resume during Reset did not hit WipeInProgress — wipe was ~0.6s on this empty wallet. That path is unit-covered only.
• Did not re-run the Pubky endpoint delete (last pass on 38c9df8).