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 <noreply@anthropic.com>
This commit is contained in:
@@ -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..."
|
||||
|
||||
@@ -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>(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>(ListWalletState.Init)
|
||||
val listWalletState = _listWalletState.asStateFlow()
|
||||
|
||||
private val _availableWallets = MutableStateFlow<Map<WalletId, UserWallet>>(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()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user