From 7c41992ea7d432a415041262980e94c439ddfb5c Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Mon, 7 Sep 2026 15:57:02 +0200 Subject: [PATCH] fix: hold the wallet state where a background thread can be heard, so a device with no seed boots Since 11ab889 removed the "Introducing... Torch" gate, a device with no account boots to "Decrypting..." and stays there. The gate was not the cause. It was the only thing standing between the user and a write that had already been getting thrown away, and taking it out is what made the app depend on that write. **What the gate was doing for a device with no account.** It navigated off SovereignWalletStartupScreen on every cold boot, and the handler it navigated through did two things: onNavigateToWalletIntroPage = { navController.navigate(route = LoadingRoute(text = "Introducing... Torch")) applicationIOScope.launch { navigationViewModel.loadNostrProfile(route) } } The second line is what routed a device with no account. `loadNostrProfile` found no local account and a non-null startupRoute, answered `NavigationUIState.Landing`, and the user got the create-a-profile screen. It never read `listWalletState`. With the gate gone the only remaining route to Landing is `availableWallets.isEmpty()` inside the startup screen, and every branch of that screen sits behind `ListWalletState.Success`. So a state that nothing used to depend on became the whole of first run. **The screen is only ever shown Init.** Instrumented on an emulator with no seed.dat, one process, one view model (`vm=52654198` throughout): 15:15:41.400 31861 31888 SeedFileNotFound branch done: state=Success stateObj=150621551 snap=GlobalSnapshot@151321212 15:15:43.527 31861 31861 composing: vm=52654198 stateObj=150621551 listWalletState=Init 15:15:44.554 31861 31861 poll #0: stateObj=150621551 dbgMarker=Success snap=GlobalSnapshot@151321212 before=Init afterSendApply=Init Thread 31888 is `Dispatchers.IO`; 31861 is main. The write happened two seconds before the read, on the same `MutableState` object, and the reader never saw it. Neither a later `Snapshot.sendApplyNotifications()` nor anything else recovered it -- the value stayed Init for the life of the process, which is the spinner the user was looking at. **Three probes, written on the same line, on the same thread, at the same instant.** They were needed because the two obvious explanations -- two view models, or a stale snapshot -- both predict the same log line, and both are wrong: - a plain `@Volatile` String field: the main thread read `Success`. So this is one object written once, not two objects that happen to share an identity hash. - a `mutableStateOf` held in a top-level `object`, i.e. created at class-load time and never inside a composition: the main thread read `Success`. So writing Compose state from `Dispatchers.IO` works fine in this app. - the view model's own `mutableStateOf`: `Init`. Same instant, same thread, same write site, opposite answer. And a `mutableStateOf` written from `Dispatchers.IO` *later*, from the poll, was seen immediately (`ioWrite/mainRead=io-0`). What separates the two is not the thread and not the state -- it is when the write happens relative to the composition that created the state. **Why.** `SovereignWalletViewModel` is built by `viewModel(factory = ...)` in MantraNavHost, so its constructor runs *inside* a composition, and a `mutableStateOf` created there is created inside that composition's snapshot. A write from another thread that lands before that composition is applied does not survive it. Delaying the write past the composition proves the boundary rather than describing it -- with `delay(1500)` inserted ahead of the decrypt and nothing else changed: 15:18:07.520 result=SeedFileNotFound -> state=Success 15:18:08.142 composing: listWalletState=Success 15:18:08.143 no wallets -> landing Same code, same threads, same write; two seconds later it is heard. **Why only a device without an account.** The window is a few milliseconds wide, and which side of it you land on is decided by how long reading the seed takes. With no seed file, `SeedManager.loadAndDecrypt` goes as far as `FileSystem.SYSTEM.exists(seedFile)`, says "seed file doesn't exist", and returns `SeedFileNotFound`: 15:09:59.060 SeedManager: loadAndDecrypt 15:09:59.063 SeedManager: seed file doesn't exist Three milliseconds after `init` fired it, which is inside the first composition. With a seed the same call decrypts through the keystore, runs `MnemonicCode.toSeed`, builds a `LocalKeyManager` and derives a node id, then `DecryptSeedResult.Success` waits on a DataStore read for the wallet metadata -- long enough, every time, to land after. So the account case booted normally and the empty case hung, and the difference had nothing to do with accounts. That also means this was never specific to the no-seed path: it is a race that path always loses. **The change.** `listWalletState` becomes a `MutableStateFlow` behind `asStateFlow()`, and the screen collects it with `collectAsState()`. A StateFlow has no snapshot to belong to, so the same write from the same thread at the same moment is seen. This is not a new pattern here -- it is what `_availableWallets`, `_desiredWalletId` and `_activeWalletInUI` already do on this same class, and `_availableWallets` is written on the line below the one that was being lost and was always read correctly. `listWalletState` was the odd one out. The comment on the declaration says why it has to stay a flow. Changing it back would restore this bug exactly, and would do so silently: nothing throws, nothing logs, the state simply keeps its initial value. **Not audited.** `press.mantra.compose.ui.view.model` has around a dozen more `by mutableStateOf` properties on view models built the same way -- HomeViewModel, ChatMessageListViewModel, InReplyToViewModel, SovereignWalletStartupViewModel and others. Every one of them is exposed to this, and every one of them is fine only for as long as nothing writes it off the main thread during the composition that creates it. Most load from Room, which is slow enough to be safe by accident, which is the same kind of safety this bug had until the gate came out. Converting them is a sweep, not this commit; it wants each call site read rather than a mechanical replace. **Not tested.** There is no Compose UI test infrastructure in this repo, and the defect lives in the interaction between a view model constructor, a composition snapshot and a background coroutine -- there is nothing to assert against without a running composition. A test that the property's type is a StateFlow would only restate the declaration the compiler already checks. What stands in for it is the comment and the fact that the three sibling properties on the class establish the pattern. **Verified on a device, twice over.** emulator-5554, no seed.dat: was "Decrypting..." for ever, now goes to the landing screen and on into Create Profile. emulator-5556, seed.dat present and a broadcast profile: still boots through `processLocalAccount` to its home screen, so the path that always worked still does. 906 tests pass, 576 jvm over 69 classes and 330 android over 41 classes, unchanged -- this commit adds none. Co-Authored-By: Claude Opus 5 --- .../SovereignWalletStartupScreen.kt | 4 ++- .../ui/view/model/SovereignWalletViewModel.kt | 27 ++++++++++++------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/SovereignWalletStartupScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/SovereignWalletStartupScreen.kt index 924f6002..98e8bb28 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/SovereignWalletStartupScreen.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/SovereignWalletStartupScreen.kt @@ -46,13 +46,15 @@ fun SovereignWalletStartupScreen( // to completion here: everything downstream reads the node's key manager. An intro screen has // to be a step inside this flow, not a detour around it. + val listWalletState by sovereignWalletViewModel.listWalletState.collectAsState() + Box( modifier = Modifier .fillMaxSize() .imePadding(), contentAlignment = Alignment.Center ) { - when (sovereignWalletViewModel.listWalletState.value) { + when (listWalletState) { is ListWalletState.Init -> { LoadingDataIndicator( text = "Decrypting..." diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SovereignWalletViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SovereignWalletViewModel.kt index 90fc6a6a..5229bd03 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SovereignWalletViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SovereignWalletViewModel.kt @@ -65,7 +65,16 @@ class SovereignWalletViewModel( // We might end up only using the machankuraWalletRepository in the future where we send the walletId in each request. ): ViewModel() { private val log = Logger.withTag("SovereignWalletViewModel") - val listWalletState = mutableStateOf(ListWalletState.Init) + + // A StateFlow, not a `mutableStateOf`, and it has to stay one. This view model is built by + // `viewModel()` during composition, so a Compose state created in its constructor is created + // inside that composition's snapshot -- and a write from another thread that lands before the + // composition is applied is discarded. `listAvailableWallets` runs from `init` and, on a device + // with no seed file, answers in about four milliseconds, which is squarely inside that window: + // the write to Success was lost and the app sat on "Decrypting..." for ever. A StateFlow has no + // snapshot to belong to, so the same write from the same thread at the same moment is seen. + private val _listWalletState = MutableStateFlow(ListWalletState.Init) + val listWalletState = _listWalletState.asStateFlow() private val _availableWallets = MutableStateFlow>(emptyMap()) val availableWallets = _availableWallets.asStateFlow() @@ -119,33 +128,33 @@ class SovereignWalletViewModel( fun listAvailableWallets(onDone: () -> Unit) { viewModelScope.launch(Dispatchers.IO + CoroutineExceptionHandler { _, e -> // log.error("error when initialising startup-view: ", e) - listWalletState.value = ListWalletState.Error.Generic(e) + _listWalletState.value = ListWalletState.Error.Generic(e) }) { when (val result = loadAndDecryptSeed(phoenixGlobal)) { is DecryptSeedResult.Failure.SerializationError -> { log.error {"cannot deserialize seed file: "} - listWalletState.value = ListWalletState.Error.Serialization + _listWalletState.value = ListWalletState.Error.Serialization } is DecryptSeedResult.Failure.DecryptionError -> { log.e("cannot decrypt seed file: ", throwable = result.cause) - listWalletState.value = ListWalletState.Error.DecryptionError.GeneralException(result.cause) + _listWalletState.value = ListWalletState.Error.DecryptionError.GeneralException(result.cause) } is DecryptSeedResult.Failure.KeyStoreFailure -> { log.e("key store failure: ", throwable = result.cause) - listWalletState.value = ListWalletState.Error.DecryptionError.KeystoreFailure(result.cause) + _listWalletState.value = ListWalletState.Error.DecryptionError.KeystoreFailure(result.cause) } is DecryptSeedResult.Failure.SeedFileUnreadable -> { log.e("aborting, unreadable seed file") - listWalletState.value = ListWalletState.Error.Generic(null) + _listWalletState.value = ListWalletState.Error.Generic(null) } is DecryptSeedResult.Failure.SeedInvalid -> { log.e("aborting, seed is invalid") - listWalletState.value = ListWalletState.Error.Generic(null) + _listWalletState.value = ListWalletState.Error.Generic(null) } is DecryptSeedResult.Failure.SeedFileNotFound -> { - listWalletState.value = ListWalletState.Success + _listWalletState.value = ListWalletState.Success _availableWallets.value = emptyMap() } @@ -164,7 +173,7 @@ class SovereignWalletViewModel( } } _availableWallets.value = result.userWalletsMap - listWalletState.value = ListWalletState.Success + _listWalletState.value = ListWalletState.Success viewModelScope.launch(Dispatchers.Main) { onDone() }