fix: mnemonic not found crash #678

piotr-iohk · 2026-01-20T14:13:54Z

Description

This PR fixes a crash that occurs when VssBackupClient.setup() is called before the mnemonic is available in the keychain. This race condition can happen during wallet restoration when BackupRepo.startObservingBackups() is triggered (on NodeLifecycleState.Running) before the mnemonic has been saved.

Root cause:

FATAL EXCEPTION: DefaultDispatcher-worker-6
to.bitkit.utils.ServiceError$MnemonicNotFound: Mnemonic not found
    at to.bitkit.data.backup.VssStoreIdProvider.getVssStoreId(VssStoreIdProvider.kt:23)
    at to.bitkit.data.backup.VssBackupClient$setup$2$1$1.invokeSuspend(VssBackupClient.kt:38)

Changes:

VssBackupClient.setup() now returns Boolean (true = success, false = mnemonic not available yet)
Added early mnemonic availability check before attempting VSS initialization
Added idempotency check to return immediately if already set up
BackupRepo now retries setup() with linear backoff (1s, 2s, 3s... up to 10 attempts) when mnemonic is not yet available

Preview

N/A - crash fix, no UI changes

QA Notes

To reproduce the original crash: note: it is intermittent.
- crash with MnemonicNotFound exception after reseting app #553
- fix: sync exception causing infinite loop on start #657 (comment)
Unit tests added:
- setup returns false when mnemonic is not available
- setup does not call vssStoreIdProvider when mnemonic is not available
- setup checks mnemonic before proceeding with vss initialization
- setup can be called multiple times when mnemonic not available
Run with: ./gradlew testDevDebugUnitTest --tests "to.bitkit.data.backup.VssBackupClientTest"

ovitrif · 2026-01-20T14:44:06Z

app/src/test/java/to/bitkit/data/backup/VssBackupClientTest.kt

+import to.bitkit.data.keychain.Keychain
+import to.bitkit.test.BaseUnitTest
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue


@piotr-iohk
run in terminal ./gradlew detekt to self-check until no more issues.

If you can't fix an issue let us know, you can also just @Suppress("DetektCheckId") when applicable

ovitrif

utAck

PS. Still need to resolve the nit review comments to be able to merge AFAIU, so I let myself choose Approve even though I want the remarks addressed 🙏🏻

ovitrif · 2026-01-20T14:44:55Z

app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt

+    /**
+     * Sets up the VSS client. Returns true if setup succeeded, false if mnemonic not available.
+     * Throws on other errors.
+     */


Pls remove comments, Claude should do it based on CLAUDE.md but sometimes it forgets or you're not using claude 🙃

ovitrif · 2026-01-20T14:45:09Z

app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt

+     * Throws on other errors.
+     */
+    suspend fun setup(walletIndex: Int = 0): Boolean = withContext(ioDispatcher) {
+        // If already set up successfully, return immediately


nit: also inline comments, the less the better.

ovitrif · 2026-01-20T14:47:37Z

app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt

+            return@withContext false
+        }
+
        runCatching {


nit: why not keeping everything into the initial runCatching and relying on result type Result<Boolean>?!

ps. would help make nicer code here.

ovitrif · 2026-01-20T14:48:48Z

app/src/main/java/to/bitkit/repositories/BackupRepo.kt

+    private suspend fun setupVssClientWithRetry() {
+        var attempt = 0
+        val maxAttempts = 10
+        val baseDelayMs = 1000L
+
+        while (attempt < maxAttempts && isObserving) {
+            val success = runCatching { vssBackupClient.setup() }.getOrDefault(false)
+            if (success) {
+                Logger.debug("VSS client setup succeeded on attempt ${attempt + 1}", context = TAG)
+                return
+            }
+            attempt++
+            if (attempt < maxAttempts) {
+                val delayMs = baseDelayMs * attempt // Linear backoff: 1s, 2s, 3s, ...
+                Logger.debug(
+                    "VSS client setup deferred, retrying in ${delayMs}ms (attempt $attempt/$maxAttempts)",
+                    context = TAG,
+                )
+                delay(delayMs)
+            }
+        }
+
+        if (isObserving) {
+            Logger.warn("VSS client setup failed after $maxAttempts attempts", context = TAG)
+        }
+    }
+


@piotr-iohk any reason this isn't in VssBackupClient?!

nit: Optimised setup for it seems more like its own concern than the responsibility of BackupRepo; it even has a "dumber" setup already.

extra nit: if this is a logic that we have in multiple places (which I reckon we might) it might make sense to foresee an utility higher-order-function (accepting labmdas) for retryWithExponentialBackoff

claude · 2026-01-20T15:09:24Z

Code Review

I found 2 issues that need attention:

Issue 1: Logic Error in VssBackupClient.kt

File: app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
Line: 79

The setup() function returns true unconditionally at the end, even when the runCatching block fails. When onFailure is triggered (e.g., timeout, native VSS call failure), the code logs the error and calls isSetup.completeExceptionally(it), but then still falls through to return true.

Problem:

When setup fails with an exception, it returns true (indicating success)
This contradicts the documented behavior: "Returns true if setup succeeded, false if mnemonic not available. Throws on other errors."
The retry logic in BackupRepo.setupVssClientWithRetry() will think setup succeeded when it actually failed, preventing proper retries

Code reference:

bitkit-android/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt

Lines 72 to 79 in 1950109

    
                   isSetup.complete(Unit) 
        
                   Logger.info("VSS client setup with server: '$vssUrl'", context = TAG) 
        
               } 
        
           }.onFailure { 
        
               isSetup.completeExceptionally(it) 
        
               Logger.error("VSS client setup error", e = it, context = TAG) 
        
           } 
        
           true

Suggested fix:

}.onFailure {
    isSetup.completeExceptionally(it)
    Logger.error("VSS client setup error", e = it, context = TAG)
    return@withContext false  // Return false on failure
}
true

Or alternatively, re-throw the exception to match the documented behavior.

Issue 2: CLAUDE.md Violation in VssBackupClientTest.kt

File: app/src/test/java/to/bitkit/data/backup/VssBackupClientTest.kt
Line: 24

The setUp() method unnecessarily uses = runBlocking {}. According to CLAUDE.md line 203:

ALWAYS wrap unit tests setUp methods mocking suspending calls with runBlocking, e.g setUp() = runBlocking {}

The rule states to use runBlocking only when mocking suspending calls. In this setUp method:

The VssBackupClient constructor is not a suspend function
No suspending calls are being mocked
keychain.loadString() is a regular (non-suspending) function

Code reference:

bitkit-android/app/src/test/java/to/bitkit/data/backup/VssBackupClientTest.kt

Lines 23 to 29 in 1950109

    
           @Before 
        
           fun setUp() = runBlocking { 
        
               sut = VssBackupClient( 
        
                   ioDispatcher = testDispatcher, 
        
                   vssStoreIdProvider = vssStoreIdProvider, 
        
                   keychain = keychain, 
        
               )

Suggested fix:

@Before
fun setUp() {
    sut = VssBackupClient(
        ioDispatcher = testDispatcher,
        vssStoreIdProvider = vssStoreIdProvider,
        keychain = keychain,
    )
}

ovitrif · 2026-01-20T15:24:23Z

@piotr-iohk to streamline this ~~I am~~ claude is making a PR targeting the branch of this PR, where I address my own review comments 👍🏻

piotr-iohk added 2 commits January 20, 2026 15:05

fix: handle missing mnemonic during VSS backup setup

dde3f71

test: verify VssBackupClient handles missing mnemonic gracefully

6421346

piotr-iohk requested review from jvsena42 and ovitrif January 20, 2026 14:13

piotr-iohk self-assigned this Jan 20, 2026

github-advanced-security bot found potential problems Jan 20, 2026

View reviewed changes

jvsena42 changed the title ~~Fix/mnemonic not found crash~~ fix: mnemonic not found crash Jan 20, 2026

ovitrif approved these changes Jan 20, 2026

View reviewed changes

Merge branch 'master' into fix/MnemonicNotFound-crash

1950109

ovitrif mentioned this pull request Jan 20, 2026

fix: address PR review comments for mnemonic crash fix #684

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: mnemonic not found crash #678

fix: mnemonic not found crash #678

piotr-iohk commented Jan 20, 2026

Uh oh!

Check warning

ovitrif Jan 20, 2026

Uh oh!

ovitrif left a comment

Uh oh!

ovitrif Jan 20, 2026

Uh oh!

ovitrif Jan 20, 2026

Uh oh!

ovitrif Jan 20, 2026

Uh oh!

ovitrif Jan 20, 2026

Uh oh!

ovitrif Jan 20, 2026

Uh oh!

claude bot commented Jan 20, 2026

Uh oh!

ovitrif commented Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix: mnemonic not found crash #678

Are you sure you want to change the base?

fix: mnemonic not found crash #678

Conversation

piotr-iohk commented Jan 20, 2026

Description

Preview

QA Notes

Uh oh!

Check warning

ovitrif Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

ovitrif left a comment

Choose a reason for hiding this comment

Uh oh!

ovitrif Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

ovitrif Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

ovitrif Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

ovitrif Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

ovitrif Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot commented Jan 20, 2026

Code Review

Issue 1: Logic Error in VssBackupClient.kt

Issue 2: CLAUDE.md Violation in VssBackupClientTest.kt

Uh oh!

ovitrif commented Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants