From 89c7e5afec6efcbddfea26f7a027bb33dae96f58 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 30 Sep 2026 11:43:00 -0400 Subject: [PATCH 1/4] fix(login): stop logging in as other accounts to name switcher rows The account switcher named each row by calling the Login RPC signed as that row's account, then GetProfile with the user id it returned (AccountProfileFetcher, from #1626). The app must not sign in as an account the user has not switched to, and iOS dropped the same call. Android stores no user id for accounts other than the signed-in one, so there is no GetProfile call left that avoids Login. Rows now take their username and display name only from AccountProfileCache, which is written while each account is signed in. An account that has never signed in on this device keeps its mnemonic name. The fallback order is unchanged: @username, then display name, then mnemonic name. The balance fetch per row is unchanged. It signs a read as the row's owner key but does not call Login or start a session. --- .../accounts/AccountSelectionViewModel.kt | 43 +------- .../AccountSelectionViewModelStateTest.kt | 97 +++++++++++++++---- .../internal/accounts/AccountProfileCache.kt | 13 ++- .../accounts/AccountProfileFetcher.kt | 46 --------- .../accounts/AccountProfileFetcherTest.kt | 67 ------------- 5 files changed, 91 insertions(+), 175 deletions(-) delete mode 100644 apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt delete mode 100644 apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt diff --git a/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModel.kt b/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModel.kt index e13749cb6c..e131c37d51 100644 --- a/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModel.kt +++ b/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModel.kt @@ -2,7 +2,6 @@ package com.flipcash.app.login.internal.accounts import androidx.lifecycle.viewModelScope import com.flipcash.app.auth.AuthManager -import com.flipcash.app.auth.internal.accounts.AccountProfileFetcher import com.flipcash.app.auth.internal.accounts.AccountProfileName import com.flipcash.app.auth.internal.accounts.AccountRecord import com.flipcash.services.models.asHandle @@ -40,7 +39,6 @@ import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map -import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.withContext @@ -53,7 +51,6 @@ class AccountSelectionViewModel @Inject constructor( private val authManager: AuthManager, private val mnemonicManager: MnemonicManager, private val tokenController: TokenController, - private val profileFetcher: AccountProfileFetcher, private val resources: ResourceHelper, private val dispatchers: DispatcherProvider, ) : BaseViewModel( @@ -110,7 +107,6 @@ class AccountSelectionViewModel @Inject constructor( val currentEntropy: String?, ) : Event - data class OnProfileResolved(val entropy: String, val name: AccountProfileName) : Event data class OnBalanceResolved(val entropy: String, val balance: Fiat) : Event data class OnBalanceNotFound(val entropy: String) : Event data class OnBalanceUnavailable(val entropy: String) : Event @@ -142,10 +138,10 @@ class AccountSelectionViewModel @Inject constructor( ) } .flatMapLatest { derived -> - derived.asFlow().flatMapMerge { merge(balance(it), profile(it)) } + derived.asFlow().flatMapMerge { balance(it) } } .onEach { dispatchEvent(it) } - // Derivation is PBKDF2 plus a SLIP-10 chain, and the fetches are network calls; + // Derivation is PBKDF2 plus a SLIP-10 chain, and the balance fetches are network calls; // neither belongs on the thread drawing the list. .flowOn(dispatchers.IO) .launchIn(viewModelScope) @@ -235,7 +231,9 @@ class AccountSelectionViewModel @Inject constructor( // timestamp is the only other thing distinguishing it. id = owner ?: "underived-$creationDate", entropy = entropy, - // The cached names show at once and offline; the fetch replaces them when it lands. + // Names come only from what this device cached while the account was signed in. There + // is no fetch for the other accounts: resolving their user id takes the Login RPC, and + // the app must not sign in as an account the user has not switched to. mnemonicName = mnemonic?.let { displayName(it) }.orEmpty(), username = profile?.username, displayName = profile?.displayName, @@ -296,25 +294,6 @@ class AccountSelectionViewModel @Inject constructor( } } - /** - * One account's username and display name, fetched signed as that account, so every row is - * named and not just the signed-in one. A failure keeps whatever the cache gave the row. - */ - private fun profile(entry: Pair>): Flow = flow { - val (record, cluster) = entry - val owner = cluster.getOrNull() ?: return@flow - profileFetcher.fetch(owner.authority.keyPair, owner.authorityPublicKey.base58()) - .onSuccess { emit(Event.OnProfileResolved(record.entropy, it)) } - .onFailure { error -> - trace( - tag = TAG, - message = "Profile fetch failed for a stored account", - error = error, - type = TraceType.Error, - ) - } - } - companion object { private const val TAG = "AccountSelection" @@ -349,18 +328,6 @@ class AccountSelectionViewModel @Inject constructor( ) } - is Event.OnProfileResolved -> { state -> - state.copy( - accounts = state.accounts.map { - if (it.entropy == event.entropy) { - it.copy(username = event.name.username, displayName = event.name.displayName) - } else { - it - } - } - ) - } - is Event.OnBalanceResolved -> { state -> state.copy( accounts = state.accounts.map { diff --git a/apps/flipcash/features/login/src/test/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModelStateTest.kt b/apps/flipcash/features/login/src/test/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModelStateTest.kt index 2b753730f7..12a4ad6744 100644 --- a/apps/flipcash/features/login/src/test/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModelStateTest.kt +++ b/apps/flipcash/features/login/src/test/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModelStateTest.kt @@ -3,20 +3,30 @@ package com.flipcash.app.login.internal.accounts import androidx.arch.core.executor.testing.InstantTaskExecutorRule import com.flipcash.app.auth.AuthManager import com.flipcash.app.auth.internal.accounts.AccountProfileCache -import com.flipcash.app.auth.internal.accounts.AccountProfileFetcher import com.flipcash.app.auth.internal.accounts.AccountProfileName import com.flipcash.app.auth.internal.accounts.AccountRecord import com.flipcash.app.auth.internal.accounts.AccountStore import com.flipcash.app.core.MainCoroutineRule import com.flipcash.app.core.dispatchers.TestDispatchers +import com.getcode.crypt.DerivedKey import com.getcode.crypt.MnemonicPhrase import com.getcode.opencode.controllers.TokenController import com.getcode.opencode.managers.MnemonicManager +import com.getcode.opencode.model.accounts.AccountCluster +import com.getcode.solana.keys.PublicKey +import com.getcode.solana.keys.base58 +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.unmockkAll import com.getcode.util.resources.FakeResourceHelper +import com.getcode.util.resources.ResourceHelper +import com.flipcash.libs.coroutines.DispatcherProvider import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest +import org.junit.After import org.junit.Rule import org.junit.Test import org.mockito.kotlin.any @@ -24,8 +34,10 @@ import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertTrue /** @@ -50,7 +62,6 @@ class AccountSelectionViewModelStateTest { private val mnemonicManager: MnemonicManager = mock() private val tokenController: TokenController = mock() private val resources = FakeResourceHelper() - private val profileFetcher: AccountProfileFetcher = mock() private val profiles: AccountProfileCache = mock { onBlocking { all() } doReturn emptyMap() } @@ -82,12 +93,14 @@ class AccountSelectionViewModelStateTest { authManager = authManager, mnemonicManager = mnemonicManager, tokenController = tokenController, - profileFetcher = profileFetcher, resources = resources, dispatchers = dispatchers, ) } + @After + fun tearDown() = unmockkAll() + @Test fun `load marks which account is signed in`() = runTest(mainCoroutineRule.dispatcher) { val store = RecordingAccountStore(listOf(record("a", 2_000L), record("b", 1_000L))) @@ -132,7 +145,6 @@ class AccountSelectionViewModelStateTest { authManager = authManager, mnemonicManager = mnemonicManager, tokenController = tokenController, - profileFetcher = profileFetcher, resources = resources, dispatchers = TestDispatchers(testScheduler), ) @@ -170,22 +182,73 @@ class AccountSelectionViewModelStateTest { assertEquals(listOf("b"), viewModel.stateFlow.value.accounts.map { it.entropy }) } - /** A fetched profile renames only its own row, and the username wins over the display name. */ + /** + * The product rule: the switcher never signs in as an account the user has not switched to. + * A row that is not the signed-in account takes its name from the local cache, and loading it + * makes one network call, the balance lookup — never the Login RPC, which is the only way from + * an owner key to the user id that GetProfile needs. + */ @Test - fun `a fetched profile retitles its row`() = runTest(mainCoroutineRule.dispatcher) { - val store = RecordingAccountStore(listOf(record("a", 2_000L), record("b", 1_000L))) - val viewModel = viewModel(store, current = "b", TestDispatchers(testScheduler)) - advanceUntilIdle() - - viewModel.dispatchEvent( - AccountSelectionViewModel.Event.OnProfileResolved( - entropy = "a", - name = AccountProfileName(username = "sally_streamer", displayName = "Sally"), + fun `a non-active row is titled from the cache without logging in`() = + runTest(mainCoroutineRule.dispatcher) { + // Real derivation reaches android.util.Base64, which is a stub on the JVM; the row only + // needs a cluster with an owner key to look up the cache and fetch a balance with. + val phrase = MnemonicPhrase(MnemonicPhrase.Kind.L12, List(12) { "abandon" }) + val ownerKey = PublicKey(List(32) { 7 }) + mockkObject(DerivedKey) + every { DerivedKey.derive(any(), phrase) } returns mockk(relaxed = true) + mockkObject(AccountCluster) + every { AccountCluster.newInstance(any(), any()) } returns + mockk(relaxed = true) { every { authorityPublicKey } returns ownerKey } + val owner = ownerKey.base58() + val cache: AccountProfileCache = mock { + onBlocking { all() } doReturn mapOf( + owner to AccountProfileName(username = "sally_streamer", displayName = "Sally"), + ) + } + whenever(authManager.accounts).thenReturn(RecordingAccountStore(listOf(record("a", 1_000L)))) + whenever(authManager.accountProfiles).thenReturn(cache) + whenever(authManager.currentEntropy).thenReturn("b") + whenever(mnemonicManager.fromEntropyBase64("a")).thenReturn(phrase) + val tokenController: TokenController = mock { + onBlocking { fetchTokenBalances(any()) } doReturn Result.failure(IllegalStateException("offline")) + } + val viewModel = AccountSelectionViewModel( + authManager = authManager, + mnemonicManager = mnemonicManager, + tokenController = tokenController, + resources = resources, + dispatchers = TestDispatchers(testScheduler), ) - ) - advanceUntilIdle() + advanceUntilIdle() + + val row = viewModel.stateFlow.value.accounts.single() + assertEquals("@sally_streamer", row.name) + assertFalse(row.notFound) + verify(authManager, never()).login(any(), any(), any(), any()) + verify(tokenController).fetchTokenBalances(any()) + verifyNoMoreInteractions(tokenController) + } - assertEquals(listOf("@sally_streamer", ""), viewModel.stateFlow.value.accounts.map { it.name }) + /** + * The row pipeline cannot call Login if it cannot reach it. The profile fetch this screen once + * had came in as one more constructor dependency and logged in as every listed account, so any + * new dependency fails here until someone confirms it cannot sign in as a non-active account. + * [AuthManager]'s own login is covered by the test above. + */ + @Test + fun `the view model depends on nothing that can log in as another account`() { + val allowed = listOf( + AuthManager::class.java, + MnemonicManager::class.java, + TokenController::class.java, + ResourceHelper::class.java, + DispatcherProvider::class.java, + ) + val injected = AccountSelectionViewModel::class.java.constructors + .single { it.parameterCount > 0 } + .parameterTypes.toList() + assertEquals(allowed, injected) } private val mnemonicName = "Apple ... Elder" diff --git a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt index c596e3597a..11944e5af4 100644 --- a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt +++ b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt @@ -36,10 +36,12 @@ data class AccountProfileName( * The last known username and display name of every account that has signed in on this device, * keyed by owner public key (base58). * - * Only the signed-in account has a session, so this is how the account list names the others - * before [AccountProfileFetcher] has answered, or when it cannot. It is written from two places: - * [UserManager]'s state, which every profile change of the signed-in account goes through — the - * restore at sign-in, the server refresh, and edits — and each fetch the account list makes. + * Only the signed-in account has a session, so this is the only source of names for the others: + * the account list does not fetch them, because resolving another account's user id takes the + * Login RPC, and the app must not sign in as an account the user has not switched to. It is written + * from [UserManager]'s state, which every profile change of the signed-in account goes through — + * the restore at sign-in, the server refresh, and edits. An account that has never signed in on + * this device has no entry, and its row keeps the mnemonic name. * * Kept out of the Block Store entry on purpose: that one has a 4KB budget sized for fixed-width * records, and a name costs nothing to lose — a missing entry falls back to the mnemonic name. The @@ -82,9 +84,6 @@ class AccountProfileCache @Inject constructor( } .getOrDefault(emptyMap()) - /** Records a name fetched for [owner], so the next offline visit still has it. */ - suspend fun put(owner: String, name: AccountProfileName) = persist(owner, name) - private suspend fun persist(owner: String, name: AccountProfileName) { runCatching { dataStore.edit { prefs -> diff --git a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt deleted file mode 100644 index 585e6b71a3..0000000000 --- a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt +++ /dev/null @@ -1,46 +0,0 @@ -package com.flipcash.app.auth.internal.accounts - -import com.flipcash.services.models.GetUserProfileError -import com.flipcash.services.models.ProfileIdentifier -import com.flipcash.services.repository.AccountRepository -import com.flipcash.services.repository.ProfileRepository -import com.getcode.ed25519.Ed25519.KeyPair -import javax.inject.Inject - -/** - * Fetches any stored account's username and display name, signed as that account. - * - * `GetProfile` is keyed by user id, and the only way from an owner key to its user id is `Login`, - * which the contract describes as a lookup for recovering an account — `Register` is the call that - * creates one. Both requests are signed with the account's own key, the same self-signed pattern the - * account list uses for balances, so this works for accounts that are not signed in. - * - * Every answer is written to [AccountProfileCache] so the list can still name the account offline. - */ -class AccountProfileFetcher @Inject constructor( - private val accountRepository: AccountRepository, - private val profileRepository: ProfileRepository, - private val cache: AccountProfileCache, -) { - /** - * The account's current names. An account the server has no profile for succeeds with both - * names null — it has none — rather than failing, so a stale cached name does not outlive it. - */ - suspend fun fetch(owner: KeyPair, ownerAddress: String): Result = - accountRepository.login(owner) - .mapCatching { userId -> - profileRepository.getProfile(ProfileIdentifier.UserId(userId), owner) - .map { profile -> - AccountProfileName( - username = profile.username?.takeIf { it.isNotBlank() }, - displayName = profile.displayName.takeIf { it.isNotBlank() }, - ) - } - .recover { error -> - if (error !is GetUserProfileError.NotFound) throw error - AccountProfileName(username = null, displayName = null) - } - .getOrThrow() - } - .onSuccess { name -> cache.put(ownerAddress, name) } -} diff --git a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt deleted file mode 100644 index ab9ef234d4..0000000000 --- a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt +++ /dev/null @@ -1,67 +0,0 @@ -package com.flipcash.app.auth.internal.accounts - -import com.flipcash.services.models.GetUserProfileError -import com.flipcash.services.models.ProfileIdentifier -import com.flipcash.services.models.UserProfile -import com.flipcash.services.repository.AccountRepository -import com.flipcash.services.repository.ProfileRepository -import com.getcode.ed25519.Ed25519.KeyPair -import io.mockk.coEvery -import io.mockk.coVerify -import io.mockk.mockk -import kotlinx.coroutines.test.runTest -import org.junit.Test -import kotlin.test.assertEquals -import kotlin.test.assertTrue - -class AccountProfileFetcherTest { - - private val owner: KeyPair = mockk() - private val userId = listOf(1, 2, 3) - private val accounts: AccountRepository = mockk() - private val profiles: ProfileRepository = mockk() - private val cache: AccountProfileCache = mockk(relaxed = true) - private val fetcher = AccountProfileFetcher(accounts, profiles, cache) - - @Test - fun `fetches the profile by the user id login returns, and caches it`() = runTest { - coEvery { accounts.login(owner) } returns Result.success(userId) - coEvery { profiles.getProfile(ProfileIdentifier.UserId(userId), owner) } returns - Result.success(UserProfile.Empty.copy(displayName = "Sally", username = "sally")) - - val name = fetcher.fetch(owner, "owner").getOrThrow() - - assertEquals(AccountProfileName(username = "sally", displayName = "Sally"), name) - coVerify { cache.put("owner", name) } - } - - /** No profile is an answer, not a failure: the account has no names, so a cached one must go. */ - @Test - fun `an account with no profile has no names`() = runTest { - coEvery { accounts.login(owner) } returns Result.success(userId) - coEvery { profiles.getProfile(any(), owner) } returns Result.failure(GetUserProfileError.NotFound()) - - val name = fetcher.fetch(owner, "owner").getOrThrow() - - assertEquals(AccountProfileName(username = null, displayName = null), name) - coVerify { cache.put("owner", name) } - } - - @Test - fun `a failed fetch leaves the cache alone`() = runTest { - coEvery { accounts.login(owner) } returns Result.success(userId) - coEvery { profiles.getProfile(any(), owner) } returns Result.failure(GetUserProfileError.Other()) - - assertTrue(fetcher.fetch(owner, "owner").isFailure) - coVerify(exactly = 0) { cache.put(any(), any()) } - } - - @Test - fun `a failed login skips the profile fetch`() = runTest { - coEvery { accounts.login(owner) } returns Result.failure(IllegalStateException("denied")) - - assertTrue(fetcher.fetch(owner, "owner").isFailure) - coVerify(exactly = 0) { profiles.getProfile(any(), any()) } - coVerify(exactly = 0) { cache.put(any(), any()) } - } -} From eede041d55226dc998513ef8939a90eb3c32e4c0 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 30 Sep 2026 11:54:40 -0400 Subject: [PATCH 2/4] feat(login): fetch switcher profiles by a user id cached at sign-in Rows for other accounts could only show the name cached while that account was signed in, because the user id GetProfile needs was never stored. AccountProfileCache now keeps each account's user id alongside its names, taken from UserManager's state while the account is signed in. NoId, the value clear() leaves at sign-out, is skipped. A row with a cached user id fetches its profile with GetProfile by that id, signed with the row's owner key the way the balance fetch signs. A row without one makes no profile call. AccountProfileFetcher now takes the user id and depends only on ProfileRepository and the cache, so it has no path to the Login RPC. Accounts that last signed in before this change have no cached user id until their next sign-in on this device, and keep their cached or mnemonic name until then. --- .../accounts/AccountSelectionViewModel.kt | 60 +++++++-- .../AccountSelectionViewModelStateTest.kt | 122 ++++++++++++------ .../internal/accounts/AccountProfileCache.kt | 86 +++++++++--- .../accounts/AccountProfileFetcher.kt | 42 ++++++ .../accounts/AccountProfileCacheTest.kt | 28 ++++ .../accounts/AccountProfileFetcherTest.kt | 64 +++++++++ 6 files changed, 339 insertions(+), 63 deletions(-) create mode 100644 apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt create mode 100644 apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt diff --git a/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModel.kt b/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModel.kt index e131c37d51..309703094b 100644 --- a/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModel.kt +++ b/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModel.kt @@ -2,7 +2,9 @@ package com.flipcash.app.login.internal.accounts import androidx.lifecycle.viewModelScope import com.flipcash.app.auth.AuthManager +import com.flipcash.app.auth.internal.accounts.AccountProfileFetcher import com.flipcash.app.auth.internal.accounts.AccountProfileName +import com.flipcash.app.auth.internal.accounts.CachedAccountProfile import com.flipcash.app.auth.internal.accounts.AccountRecord import com.flipcash.services.models.asHandle import com.flipcash.features.login.R @@ -39,6 +41,7 @@ import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.withContext @@ -51,6 +54,7 @@ class AccountSelectionViewModel @Inject constructor( private val authManager: AuthManager, private val mnemonicManager: MnemonicManager, private val tokenController: TokenController, + private val profileFetcher: AccountProfileFetcher, private val resources: ResourceHelper, private val dispatchers: DispatcherProvider, ) : BaseViewModel( @@ -107,6 +111,7 @@ class AccountSelectionViewModel @Inject constructor( val currentEntropy: String?, ) : Event + data class OnProfileResolved(val entropy: String, val name: AccountProfileName) : Event data class OnBalanceResolved(val entropy: String, val balance: Fiat) : Event data class OnBalanceNotFound(val entropy: String) : Event data class OnBalanceUnavailable(val entropy: String) : Event @@ -125,9 +130,8 @@ class AccountSelectionViewModel @Inject constructor( .filterIsInstance() // Nothing outside drives this screen, so the pipeline seeds its own first read. .onStart { emit(Event.Load) } - .map { derive(authManager.accounts.all()) } - .onEach { derived -> - val profiles = authManager.accountProfiles.all() + .map { derive(authManager.accounts.all()) to authManager.accountProfiles.all() } + .onEach { (derived, profiles) -> dispatchEvent( Event.OnAccountsLoaded( accounts = derived.map { (record, cluster) -> @@ -137,8 +141,8 @@ class AccountSelectionViewModel @Inject constructor( ) ) } - .flatMapLatest { derived -> - derived.asFlow().flatMapMerge { balance(it) } + .flatMapLatest { (derived, profiles) -> + derived.asFlow().flatMapMerge { merge(balance(it), profile(it, profiles)) } } .onEach { dispatchEvent(it) } // Derivation is PBKDF2 plus a SLIP-10 chain, and the balance fetches are network calls; @@ -221,19 +225,17 @@ class AccountSelectionViewModel @Inject constructor( private fun AccountRecord.toUiModel( cluster: AccountCluster?, - profiles: Map, + profiles: Map, ): AccountUiModel { val mnemonic = runCatching { mnemonicManager.fromEntropyBase64(entropy) }.getOrNull() val owner = cluster?.authorityPublicKey?.base58() - val profile = owner?.let { profiles[it] } + val profile = owner?.let { profiles[it]?.name } return AccountUiModel( // A record that will not derive has no owner key to be keyed on; its creation // timestamp is the only other thing distinguishing it. id = owner ?: "underived-$creationDate", entropy = entropy, - // Names come only from what this device cached while the account was signed in. There - // is no fetch for the other accounts: resolving their user id takes the Login RPC, and - // the app must not sign in as an account the user has not switched to. + // The cached names show at once and offline; the fetch replaces them when it lands. mnemonicName = mnemonic?.let { displayName(it) }.orEmpty(), username = profile?.username, displayName = profile?.displayName, @@ -294,6 +296,32 @@ class AccountSelectionViewModel @Inject constructor( } } + /** + * One account's username and display name, fetched by the user id cached from its last + * sign-in on this device. A row with no cached user id makes no call: the only other way to a + * user id is the Login RPC, and the app must not sign in as an account the user has not + * switched to. A failure keeps whatever the cache gave the row. + */ + private fun profile( + entry: Pair>, + profiles: Map, + ): Flow = flow { + val (record, cluster) = entry + val owner = cluster.getOrNull() ?: return@flow + val ownerAddress = owner.authorityPublicKey.base58() + val userId = profiles[ownerAddress]?.userId ?: return@flow + profileFetcher.fetch(owner.authority.keyPair, ownerAddress, userId) + .onSuccess { emit(Event.OnProfileResolved(record.entropy, it)) } + .onFailure { error -> + trace( + tag = TAG, + message = "Profile fetch failed for a stored account", + error = error, + type = TraceType.Error, + ) + } + } + companion object { private const val TAG = "AccountSelection" @@ -328,6 +356,18 @@ class AccountSelectionViewModel @Inject constructor( ) } + is Event.OnProfileResolved -> { state -> + state.copy( + accounts = state.accounts.map { + if (it.entropy == event.entropy) { + it.copy(username = event.name.username, displayName = event.name.displayName) + } else { + it + } + } + ) + } + is Event.OnBalanceResolved -> { state -> state.copy( accounts = state.accounts.map { diff --git a/apps/flipcash/features/login/src/test/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModelStateTest.kt b/apps/flipcash/features/login/src/test/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModelStateTest.kt index 12a4ad6744..f56f8ed520 100644 --- a/apps/flipcash/features/login/src/test/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModelStateTest.kt +++ b/apps/flipcash/features/login/src/test/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModelStateTest.kt @@ -3,6 +3,8 @@ package com.flipcash.app.login.internal.accounts import androidx.arch.core.executor.testing.InstantTaskExecutorRule import com.flipcash.app.auth.AuthManager import com.flipcash.app.auth.internal.accounts.AccountProfileCache +import com.flipcash.app.auth.internal.accounts.AccountProfileFetcher +import com.flipcash.app.auth.internal.accounts.CachedAccountProfile import com.flipcash.app.auth.internal.accounts.AccountProfileName import com.flipcash.app.auth.internal.accounts.AccountRecord import com.flipcash.app.auth.internal.accounts.AccountStore @@ -23,6 +25,7 @@ import com.getcode.util.resources.FakeResourceHelper import com.getcode.util.resources.ResourceHelper import com.flipcash.libs.coroutines.DispatcherProvider import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest @@ -31,9 +34,11 @@ import org.junit.Rule import org.junit.Test import org.mockito.kotlin.any import org.mockito.kotlin.doReturn +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever import kotlin.test.assertEquals @@ -62,6 +67,7 @@ class AccountSelectionViewModelStateTest { private val mnemonicManager: MnemonicManager = mock() private val tokenController: TokenController = mock() private val resources = FakeResourceHelper() + private val profileFetcher: AccountProfileFetcher = mock() private val profiles: AccountProfileCache = mock { onBlocking { all() } doReturn emptyMap() } @@ -93,6 +99,7 @@ class AccountSelectionViewModelStateTest { authManager = authManager, mnemonicManager = mnemonicManager, tokenController = tokenController, + profileFetcher = profileFetcher, resources = resources, dispatchers = dispatchers, ) @@ -145,6 +152,7 @@ class AccountSelectionViewModelStateTest { authManager = authManager, mnemonicManager = mnemonicManager, tokenController = tokenController, + profileFetcher = profileFetcher, resources = resources, dispatchers = TestDispatchers(testScheduler), ) @@ -182,43 +190,59 @@ class AccountSelectionViewModelStateTest { assertEquals(listOf("b"), viewModel.stateFlow.value.accounts.map { it.entropy }) } + /** + * A single non-active, derivable row with [cached] as its cache entry. Real derivation reaches + * android.util.Base64, a stub on the JVM, so the cluster is faked; the row only needs an owner + * key to look up the cache and fetch with. + */ + private fun TestScope.loadNonActiveRow( + cached: CachedAccountProfile, + fetcher: AccountProfileFetcher = profileFetcher, + tokens: TokenController = offlineTokens(), + ): AccountSelectionViewModel { + val phrase = MnemonicPhrase(MnemonicPhrase.Kind.L12, List(12) { "abandon" }) + val ownerKey = PublicKey(List(32) { 7 }) + mockkObject(DerivedKey) + every { DerivedKey.derive(any(), phrase) } returns mockk(relaxed = true) + mockkObject(AccountCluster) + every { AccountCluster.newInstance(any(), any()) } returns + mockk(relaxed = true) { every { authorityPublicKey } returns ownerKey } + val cache: AccountProfileCache = mock { + onBlocking { all() } doReturn mapOf(ownerKey.base58() to cached) + } + whenever(authManager.accounts).thenReturn(RecordingAccountStore(listOf(record("a", 1_000L)))) + whenever(authManager.accountProfiles).thenReturn(cache) + whenever(authManager.currentEntropy).thenReturn("b") + whenever(mnemonicManager.fromEntropyBase64("a")).thenReturn(phrase) + return AccountSelectionViewModel( + authManager = authManager, + mnemonicManager = mnemonicManager, + tokenController = tokens, + profileFetcher = fetcher, + resources = resources, + dispatchers = TestDispatchers(testScheduler), + ) + } + + private fun offlineTokens(): TokenController = mock { + onBlocking { fetchTokenBalances(any()) } doReturn Result.failure(IllegalStateException("offline")) + } + /** * The product rule: the switcher never signs in as an account the user has not switched to. - * A row that is not the signed-in account takes its name from the local cache, and loading it - * makes one network call, the balance lookup — never the Login RPC, which is the only way from - * an owner key to the user id that GetProfile needs. + * A row with no cached user id could only get one from the Login RPC, so it keeps its cached + * name and makes one network call, the balance lookup. */ @Test - fun `a non-active row is titled from the cache without logging in`() = + fun `a row with no stored user id is titled from the cache without logging in`() = runTest(mainCoroutineRule.dispatcher) { - // Real derivation reaches android.util.Base64, which is a stub on the JVM; the row only - // needs a cluster with an owner key to look up the cache and fetch a balance with. - val phrase = MnemonicPhrase(MnemonicPhrase.Kind.L12, List(12) { "abandon" }) - val ownerKey = PublicKey(List(32) { 7 }) - mockkObject(DerivedKey) - every { DerivedKey.derive(any(), phrase) } returns mockk(relaxed = true) - mockkObject(AccountCluster) - every { AccountCluster.newInstance(any(), any()) } returns - mockk(relaxed = true) { every { authorityPublicKey } returns ownerKey } - val owner = ownerKey.base58() - val cache: AccountProfileCache = mock { - onBlocking { all() } doReturn mapOf( - owner to AccountProfileName(username = "sally_streamer", displayName = "Sally"), - ) - } - whenever(authManager.accounts).thenReturn(RecordingAccountStore(listOf(record("a", 1_000L)))) - whenever(authManager.accountProfiles).thenReturn(cache) - whenever(authManager.currentEntropy).thenReturn("b") - whenever(mnemonicManager.fromEntropyBase64("a")).thenReturn(phrase) - val tokenController: TokenController = mock { - onBlocking { fetchTokenBalances(any()) } doReturn Result.failure(IllegalStateException("offline")) - } - val viewModel = AccountSelectionViewModel( - authManager = authManager, - mnemonicManager = mnemonicManager, - tokenController = tokenController, - resources = resources, - dispatchers = TestDispatchers(testScheduler), + val tokens = offlineTokens() + val viewModel = loadNonActiveRow( + cached = CachedAccountProfile( + userId = null, + name = AccountProfileName(username = "sally_streamer", displayName = "Sally"), + ), + tokens = tokens, ) advanceUntilIdle() @@ -226,15 +250,36 @@ class AccountSelectionViewModelStateTest { assertEquals("@sally_streamer", row.name) assertFalse(row.notFound) verify(authManager, never()).login(any(), any(), any(), any()) - verify(tokenController).fetchTokenBalances(any()) - verifyNoMoreInteractions(tokenController) + verifyNoInteractions(profileFetcher) + verify(tokens).fetchTokenBalances(any()) + verifyNoMoreInteractions(tokens) + } + + /** A stored user id is enough for GetProfile, a read; the fetched names replace the cached. */ + @Test + fun `a row with a stored user id is retitled by a profile fetch`() = + runTest(mainCoroutineRule.dispatcher) { + val userId = listOf(1, 2, 3) + val fetcher: AccountProfileFetcher = mock { + onBlocking { fetch(any(), any(), eq(userId)) } doReturn + Result.success(AccountProfileName(username = null, displayName = "Sally")) + } + val viewModel = loadNonActiveRow( + cached = CachedAccountProfile(userId = userId, name = null), + fetcher = fetcher, + ) + advanceUntilIdle() + + assertEquals("Sally", viewModel.stateFlow.value.accounts.single().name) + verify(authManager, never()).login(any(), any(), any(), any()) } /** - * The row pipeline cannot call Login if it cannot reach it. The profile fetch this screen once - * had came in as one more constructor dependency and logged in as every listed account, so any - * new dependency fails here until someone confirms it cannot sign in as a non-active account. - * [AuthManager]'s own login is covered by the test above. + * The row pipeline cannot call Login if it cannot reach it. An earlier profile fetch came in + * as one more constructor dependency and logged in as every listed account, so any new + * dependency fails here until someone confirms it cannot sign in as a non-active account. + * [AuthManager]'s own login is covered above, and [AccountProfileFetcher]'s dependencies by + * its own test. */ @Test fun `the view model depends on nothing that can log in as another account`() { @@ -242,6 +287,7 @@ class AccountSelectionViewModelStateTest { AuthManager::class.java, MnemonicManager::class.java, TokenController::class.java, + AccountProfileFetcher::class.java, ResourceHelper::class.java, DispatcherProvider::class.java, ) diff --git a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt index 11944e5af4..b0a19ed63b 100644 --- a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt +++ b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt @@ -13,8 +13,10 @@ import com.flipcash.libs.coroutines.DispatcherProvider import com.flipcash.services.models.UserProfile import com.flipcash.services.user.UserManager import com.getcode.opencode.model.core.ID +import com.getcode.opencode.model.core.NoId import com.getcode.solana.keys.base58 import com.getcode.utils.TraceType +import com.getcode.utils.hexEncodedString import com.getcode.utils.trace import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.CoroutineScope @@ -32,16 +34,26 @@ data class AccountProfileName( val displayName: String?, ) +/** What this device remembers about an account from the last time it was signed in. */ +data class CachedAccountProfile( + /** The account's user id, which `GetProfile` is keyed by. Null until it signs in here. */ + val userId: ID?, + val name: AccountProfileName?, +) + /** * The last known username and display name of every account that has signed in on this device, * keyed by owner public key (base58). * - * Only the signed-in account has a session, so this is the only source of names for the others: - * the account list does not fetch them, because resolving another account's user id takes the - * Login RPC, and the app must not sign in as an account the user has not switched to. It is written - * from [UserManager]'s state, which every profile change of the signed-in account goes through — - * the restore at sign-in, the server refresh, and edits. An account that has never signed in on - * this device has no entry, and its row keeps the mnemonic name. + * Only the signed-in account has a session. The user id cached here is what lets the account list + * fetch another account's profile with `GetProfile`, a read, instead of resolving the id with the + * Login RPC: the app must not sign in as an account the user has not switched to. An account with + * no cached user id has not signed in on this device since the id was first cached, so its row + * keeps whatever name is cached, or the mnemonic name. + * + * Written from [UserManager]'s state, which every profile change of the signed-in account goes + * through — the restore at sign-in, the server refresh, and edits — and from each profile the + * account list fetches. * * Kept out of the Block Store entry on purpose: that one has a 4KB budget sized for fixed-width * records, and a name costs nothing to lose — a missing entry falls back to the mnemonic name. The @@ -74,16 +86,38 @@ class AccountProfileCache @Inject constructor( .distinctUntilChanged() .collect { (owner, name) -> persist(owner, name) } } + scope.launch { + userManager.state + .mapNotNull { state -> + userIdFor( + owner = state.cluster?.authorityPublicKey?.base58(), + accountId = state.accountId, + ) + } + .distinctUntilChanged() + .collect { (owner, userId) -> persistUserId(owner, userId) } + } } - /** Every cached name, by owner public key. Empty when the file cannot be read. */ - suspend fun all(): Map = - runCatching { dataStore.data.first().toNames() } + /** Everything cached, by owner public key. Empty when the file cannot be read. */ + suspend fun all(): Map = + runCatching { dataStore.data.first().toEntries() } .onFailure { error -> trace(tag = TAG, message = "Could not read account profiles", error = error, type = TraceType.Error) } .getOrDefault(emptyMap()) + /** Records a name fetched for [owner], so the next offline visit still has it. */ + suspend fun put(owner: String, name: AccountProfileName) = persist(owner, name) + + private suspend fun persistUserId(owner: String, userId: ID) { + runCatching { + dataStore.edit { prefs -> prefs[userIdKey(owner)] = userId.hexEncodedString() } + }.onFailure { error -> + trace(tag = TAG, message = "Could not cache an account's user id", error = error, type = TraceType.Error) + } + } + private suspend fun persist(owner: String, name: AccountProfileName) { runCatching { dataStore.edit { prefs -> @@ -106,24 +140,46 @@ class AccountProfileCache @Inject constructor( private const val TAG = "AccountProfileCache" private const val USERNAME = "username:" private const val DISPLAY_NAME = "displayName:" + private const val USER_ID = "userId:" private fun usernameKey(owner: String) = stringPreferencesKey(USERNAME + owner) private fun displayNameKey(owner: String) = stringPreferencesKey(DISPLAY_NAME + owner) + private fun userIdKey(owner: String) = stringPreferencesKey(USER_ID + owner) + + private val PREFIXES = listOf(USERNAME, DISPLAY_NAME, USER_ID) - private fun Preferences.toNames(): Map { + private fun Preferences.toEntries(): Map { val values = asMap().mapKeys { it.key.name } val owners = values.keys.mapNotNull { key -> - key.removePrefix(USERNAME).takeIf { key.startsWith(USERNAME) } - ?: key.removePrefix(DISPLAY_NAME).takeIf { key.startsWith(DISPLAY_NAME) } + PREFIXES.firstOrNull { key.startsWith(it) }?.let { key.removePrefix(it) } }.toSet() return owners.associateWith { owner -> - AccountProfileName( - username = values[USERNAME + owner] as? String, - displayName = values[DISPLAY_NAME + owner] as? String, + val username = values[USERNAME + owner] as? String + val displayName = values[DISPLAY_NAME + owner] as? String + CachedAccountProfile( + userId = (values[USER_ID + owner] as? String)?.let(::decodeUserId), + name = if (username == null && displayName == null) null + else AccountProfileName(username = username, displayName = displayName), ) } } + /** Null for anything that is not an even run of hex digits, so a bad entry is just absent. */ + internal fun decodeUserId(hex: String): ID? { + if (hex.isEmpty() || hex.length % 2 != 0) return null + return hex.chunked(2).map { it.toIntOrNull(16)?.toByte() ?: return null } + } + + /** + * The user id to record for this state, or null when the state does not tie one to an + * owner key. [UserManager.clear] resets both at sign-out, so a pair seen here belongs to + * the same session; [NoId], the reset value, is empty and so is skipped. + */ + internal fun userIdFor(owner: String?, accountId: ID?): Pair? { + if (owner == null || accountId.isNullOrEmpty()) return null + return owner to accountId + } + /** * The entry to write for this state, or null when there is nothing to attribute. * diff --git a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt new file mode 100644 index 0000000000..83991151ae --- /dev/null +++ b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt @@ -0,0 +1,42 @@ +package com.flipcash.app.auth.internal.accounts + +import com.flipcash.services.models.GetUserProfileError +import com.flipcash.services.models.ProfileIdentifier +import com.flipcash.services.repository.ProfileRepository +import com.getcode.ed25519.Ed25519.KeyPair +import com.getcode.opencode.model.core.ID +import javax.inject.Inject + +/** + * Fetches a stored account's username and display name by the user id [AccountProfileCache] kept + * from when that account was signed in. + * + * `GetProfile` is a read, signed with the account's own key the same way the account list signs its + * balance lookups. There is deliberately no path from an owner key to a user id here: that takes the + * Login RPC, and the app must not sign in as an account the user has not switched to. A caller with + * no stored user id has no fetch to make. + * + * Every answer is written to [AccountProfileCache] so the list can still name the account offline. + */ +class AccountProfileFetcher @Inject constructor( + private val profileRepository: ProfileRepository, + private val cache: AccountProfileCache, +) { + /** + * The account's current names. An account the server has no profile for succeeds with both + * names null — it has none — rather than failing, so a stale cached name does not outlive it. + */ + suspend fun fetch(owner: KeyPair, ownerAddress: String, userId: ID): Result = + profileRepository.getProfile(ProfileIdentifier.UserId(userId), owner) + .map { profile -> + AccountProfileName( + username = profile.username?.takeIf { it.isNotBlank() }, + displayName = profile.displayName.takeIf { it.isNotBlank() }, + ) + } + .recoverCatching { error -> + if (error !is GetUserProfileError.NotFound) throw error + AccountProfileName(username = null, displayName = null) + } + .onSuccess { name -> cache.put(ownerAddress, name) } +} diff --git a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCacheTest.kt b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCacheTest.kt index 21e288cd62..1b1f4a10f2 100644 --- a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCacheTest.kt +++ b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCacheTest.kt @@ -1,6 +1,8 @@ package com.flipcash.app.auth.internal.accounts import com.flipcash.services.models.UserProfile +import com.getcode.opencode.model.core.NoId +import com.getcode.utils.hexEncodedString import org.junit.Test import kotlin.test.assertEquals import kotlin.test.assertNull @@ -48,4 +50,30 @@ class AccountProfileCacheTest { AccountProfileCache.entryFor(owner = "owner", accountId = signedIn, profile = profile), ) } + + @Test + fun `records the signed-in owner's user id`() { + assertEquals("owner" to signedIn, AccountProfileCache.userIdFor(owner = "owner", accountId = signedIn)) + } + + /** Sign-out resets the id to NoId, which is empty; it must not be stored as an account's id. */ + @Test + fun `skips a state with no owner or no user id`() { + assertNull(AccountProfileCache.userIdFor(owner = null, accountId = signedIn)) + assertNull(AccountProfileCache.userIdFor(owner = "owner", accountId = null)) + assertNull(AccountProfileCache.userIdFor(owner = "owner", accountId = NoId)) + } + + @Test + fun `decodes a stored user id`() { + val id = listOf(0, 15, -1, 127, -128) + assertEquals(id, AccountProfileCache.decodeUserId(id.hexEncodedString())) + } + + @Test + fun `treats a malformed user id as absent`() { + assertNull(AccountProfileCache.decodeUserId("")) + assertNull(AccountProfileCache.decodeUserId("abc")) + assertNull(AccountProfileCache.decodeUserId("zz")) + } } diff --git a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt new file mode 100644 index 0000000000..c88b4b4f95 --- /dev/null +++ b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt @@ -0,0 +1,64 @@ +package com.flipcash.app.auth.internal.accounts + +import com.flipcash.services.models.GetUserProfileError +import com.flipcash.services.models.ProfileIdentifier +import com.flipcash.services.models.UserProfile +import com.flipcash.services.repository.ProfileRepository +import com.getcode.ed25519.Ed25519.KeyPair +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.mockk +import kotlinx.coroutines.test.runTest +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class AccountProfileFetcherTest { + + private val owner: KeyPair = mockk() + private val userId = listOf(1, 2, 3) + private val profiles: ProfileRepository = mockk() + private val cache: AccountProfileCache = mockk(relaxed = true) + private val fetcher = AccountProfileFetcher(profiles, cache) + + @Test + fun `fetches the profile by the stored user id, and caches it`() = runTest { + coEvery { profiles.getProfile(ProfileIdentifier.UserId(userId), owner) } returns + Result.success(UserProfile.Empty.copy(displayName = "Sally", username = "sally")) + + val name = fetcher.fetch(owner, "owner", userId).getOrThrow() + + assertEquals(AccountProfileName(username = "sally", displayName = "Sally"), name) + coVerify { cache.put("owner", name) } + } + + /** No profile is an answer, not a failure: the account has no names, so a cached one must go. */ + @Test + fun `an account with no profile has no names`() = runTest { + coEvery { profiles.getProfile(any(), owner) } returns Result.failure(GetUserProfileError.NotFound()) + + val name = fetcher.fetch(owner, "owner", userId).getOrThrow() + + assertEquals(AccountProfileName(username = null, displayName = null), name) + coVerify { cache.put("owner", name) } + } + + @Test + fun `a failed fetch leaves the cache alone`() = runTest { + coEvery { profiles.getProfile(any(), owner) } returns Result.failure(GetUserProfileError.Other()) + + assertTrue(fetcher.fetch(owner, "owner", userId).isFailure) + coVerify(exactly = 0) { cache.put(any(), any()) } + } + + /** + * The fetcher runs for accounts the user has not switched to, so it must not be able to log in + * as them. An earlier version resolved the user id through the account service's Login RPC; + * any dependency beyond these two fails here until someone confirms it cannot do that. + */ + @Test + fun `depends on nothing that can log in`() { + val injected = AccountProfileFetcher::class.java.constructors.single().parameterTypes.toList() + assertEquals(listOf(ProfileRepository::class.java, AccountProfileCache::class.java), injected) + } +} From 1ee564087e78f5a00a645d2cba9cd7fb3d84c5d4 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 30 Sep 2026 12:17:09 -0400 Subject: [PATCH 3/4] fix(user-profile): stub setDisplayName with its String? result #1625 changed ProfileController.setDisplayName to return the server-assigned username as Result. NameEntryViewModelTest still stubbed Result, so the module's unit tests no longer compiled. --- .../app/userprofile/internal/name/NameEntryViewModelTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryViewModelTest.kt b/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryViewModelTest.kt index f397c5ae1a..0702286d28 100644 --- a/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryViewModelTest.kt +++ b/apps/flipcash/features/user-profile/src/test/kotlin/com/flipcash/app/userprofile/internal/name/NameEntryViewModelTest.kt @@ -68,7 +68,7 @@ class NameEntryViewModelTest { dispatchers = TestDispatchers(testScheduler) every { userManager.state } returns MutableStateFlow(UserManager.State()) every { userManager.profile } returns null - whenever(profileController.setDisplayName(any())).thenReturn(Result.success(Unit)) + whenever(profileController.setDisplayName(any())).thenReturn(Result.success(null)) val vm = createViewModel() vm.dispatchEvent(NameEntryViewModel.Event.CheckName(DisplayNameSource.Onboarding)) @@ -85,7 +85,7 @@ class NameEntryViewModelTest { dispatchers = TestDispatchers(testScheduler) every { userManager.state } returns MutableStateFlow(UserManager.State()) every { userManager.profile } returns profileNamed("Ada") - whenever(profileController.setDisplayName(any())).thenReturn(Result.success(Unit)) + whenever(profileController.setDisplayName(any())).thenReturn(Result.success(null)) val vm = createViewModel() vm.dispatchEvent(NameEntryViewModel.Event.CheckName(DisplayNameSource.MyAccount)) From ad2001203c4fa18d024a62e367992978e916b398 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 30 Sep 2026 12:17:10 -0400 Subject: [PATCH 4/4] fix(login): resolve a missing user id with Login as the last fallback A switcher row with a cached user id still calls GetProfile by that id. A row without one now calls Login signed with its own owner key, caches the user id it returns, then calls GetProfile. The cached id means each account goes through Login at most once on this device. This matches code-payments/code-ios-app#918, which calls login(owner:) only when a row has no stored user id and its database has none either. Android has no database step, so Login follows the cache directly. --- .../accounts/AccountSelectionViewModel.kt | 7 +- .../AccountSelectionViewModelStateTest.kt | 70 ++++++++----------- .../internal/accounts/AccountProfileCache.kt | 15 ++-- .../accounts/AccountProfileFetcher.kt | 47 ++++++++----- .../accounts/AccountProfileFetcherTest.kt | 36 +++++++--- 5 files changed, 99 insertions(+), 76 deletions(-) diff --git a/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModel.kt b/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModel.kt index 309703094b..da0153d4dd 100644 --- a/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModel.kt +++ b/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModel.kt @@ -298,9 +298,8 @@ class AccountSelectionViewModel @Inject constructor( /** * One account's username and display name, fetched by the user id cached from its last - * sign-in on this device. A row with no cached user id makes no call: the only other way to a - * user id is the Login RPC, and the app must not sign in as an account the user has not - * switched to. A failure keeps whatever the cache gave the row. + * sign-in on this device. A row with no cached user id resolves one through Login first, as a + * last resort. A failure keeps whatever the cache gave the row. */ private fun profile( entry: Pair>, @@ -309,7 +308,7 @@ class AccountSelectionViewModel @Inject constructor( val (record, cluster) = entry val owner = cluster.getOrNull() ?: return@flow val ownerAddress = owner.authorityPublicKey.base58() - val userId = profiles[ownerAddress]?.userId ?: return@flow + val userId = profiles[ownerAddress]?.userId profileFetcher.fetch(owner.authority.keyPair, ownerAddress, userId) .onSuccess { emit(Event.OnProfileResolved(record.entropy, it)) } .onFailure { error -> diff --git a/apps/flipcash/features/login/src/test/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModelStateTest.kt b/apps/flipcash/features/login/src/test/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModelStateTest.kt index f56f8ed520..4a726d634e 100644 --- a/apps/flipcash/features/login/src/test/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModelStateTest.kt +++ b/apps/flipcash/features/login/src/test/kotlin/com/flipcash/app/login/internal/accounts/AccountSelectionViewModelStateTest.kt @@ -22,8 +22,6 @@ import io.mockk.mockk import io.mockk.mockkObject import io.mockk.unmockkAll import com.getcode.util.resources.FakeResourceHelper -import com.getcode.util.resources.ResourceHelper -import com.flipcash.libs.coroutines.DispatcherProvider import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.UnconfinedTestDispatcher @@ -33,13 +31,13 @@ import org.junit.After import org.junit.Rule import org.junit.Test import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.doReturn import org.mockito.kotlin.eq +import org.mockito.kotlin.isNull import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify -import org.mockito.kotlin.verifyNoInteractions -import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -229,33 +227,50 @@ class AccountSelectionViewModelStateTest { } /** - * The product rule: the switcher never signs in as an account the user has not switched to. - * A row with no cached user id could only get one from the Login RPC, so it keeps its cached - * name and makes one network call, the balance lookup. + * A row with no stored user id asks the fetcher to resolve one (the fetcher's Login fallback). + * The switcher itself never switches the session: [AuthManager.login] is not called. */ @Test - fun `a row with no stored user id is titled from the cache without logging in`() = + fun `a row with no stored user id is fetched without a user id`() = runTest(mainCoroutineRule.dispatcher) { - val tokens = offlineTokens() + val fetcher: AccountProfileFetcher = mock { + onBlocking { fetch(any(), any(), isNull()) } doReturn + Result.success(AccountProfileName(username = "sally_streamer", displayName = "Sally")) + } + val viewModel = loadNonActiveRow( + cached = CachedAccountProfile(userId = null, name = null), + fetcher = fetcher, + ) + advanceUntilIdle() + + assertEquals("@sally_streamer", viewModel.stateFlow.value.accounts.single().name) + verify(fetcher).fetch(any(), any(), isNull()) + verify(authManager, never()).login(any(), any(), any(), any()) + } + + /** A failed fetch, Login included, keeps whatever name the cache gave the row. */ + @Test + fun `a failed fetch keeps the cached name`() = + runTest(mainCoroutineRule.dispatcher) { + val fetcher: AccountProfileFetcher = mock { + onBlocking { fetch(any(), any(), anyOrNull()) } doReturn + Result.failure(IllegalStateException("offline")) + } val viewModel = loadNonActiveRow( cached = CachedAccountProfile( userId = null, name = AccountProfileName(username = "sally_streamer", displayName = "Sally"), ), - tokens = tokens, + fetcher = fetcher, ) advanceUntilIdle() val row = viewModel.stateFlow.value.accounts.single() assertEquals("@sally_streamer", row.name) assertFalse(row.notFound) - verify(authManager, never()).login(any(), any(), any(), any()) - verifyNoInteractions(profileFetcher) - verify(tokens).fetchTokenBalances(any()) - verifyNoMoreInteractions(tokens) } - /** A stored user id is enough for GetProfile, a read; the fetched names replace the cached. */ + /** A stored user id goes straight to the fetch; the fetched names replace the cached. */ @Test fun `a row with a stored user id is retitled by a profile fetch`() = runTest(mainCoroutineRule.dispatcher) { @@ -271,32 +286,9 @@ class AccountSelectionViewModelStateTest { advanceUntilIdle() assertEquals("Sally", viewModel.stateFlow.value.accounts.single().name) - verify(authManager, never()).login(any(), any(), any(), any()) + verify(fetcher).fetch(any(), any(), eq(userId)) } - /** - * The row pipeline cannot call Login if it cannot reach it. An earlier profile fetch came in - * as one more constructor dependency and logged in as every listed account, so any new - * dependency fails here until someone confirms it cannot sign in as a non-active account. - * [AuthManager]'s own login is covered above, and [AccountProfileFetcher]'s dependencies by - * its own test. - */ - @Test - fun `the view model depends on nothing that can log in as another account`() { - val allowed = listOf( - AuthManager::class.java, - MnemonicManager::class.java, - TokenController::class.java, - AccountProfileFetcher::class.java, - ResourceHelper::class.java, - DispatcherProvider::class.java, - ) - val injected = AccountSelectionViewModel::class.java.constructors - .single { it.parameterCount > 0 } - .parameterTypes.toList() - assertEquals(allowed, injected) - } - private val mnemonicName = "Apple ... Elder" @Test diff --git a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt index b0a19ed63b..aedee8381b 100644 --- a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt +++ b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt @@ -45,15 +45,13 @@ data class CachedAccountProfile( * The last known username and display name of every account that has signed in on this device, * keyed by owner public key (base58). * - * Only the signed-in account has a session. The user id cached here is what lets the account list - * fetch another account's profile with `GetProfile`, a read, instead of resolving the id with the - * Login RPC: the app must not sign in as an account the user has not switched to. An account with - * no cached user id has not signed in on this device since the id was first cached, so its row - * keeps whatever name is cached, or the mnemonic name. + * The user id cached here is what lets the account list fetch another account's profile with + * `GetProfile` directly. Only an account with no cached user id goes through the Login RPC to + * resolve one, and that id is cached too, so each account needs Login at most once on this device. * * Written from [UserManager]'s state, which every profile change of the signed-in account goes - * through — the restore at sign-in, the server refresh, and edits — and from each profile the - * account list fetches. + * through — the restore at sign-in, the server refresh, and edits — and from each profile and user + * id the account list fetches. * * Kept out of the Block Store entry on purpose: that one has a 4KB budget sized for fixed-width * records, and a name costs nothing to lose — a missing entry falls back to the mnemonic name. The @@ -110,6 +108,9 @@ class AccountProfileCache @Inject constructor( /** Records a name fetched for [owner], so the next offline visit still has it. */ suspend fun put(owner: String, name: AccountProfileName) = persist(owner, name) + /** Records the user id Login resolved for [owner], so later fetches skip Login. */ + suspend fun putUserId(owner: String, userId: ID) = persistUserId(owner, userId) + private suspend fun persistUserId(owner: String, userId: ID) { runCatching { dataStore.edit { prefs -> prefs[userIdKey(owner)] = userId.hexEncodedString() } diff --git a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt index 83991151ae..cc2021b058 100644 --- a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt +++ b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt @@ -2,23 +2,25 @@ package com.flipcash.app.auth.internal.accounts import com.flipcash.services.models.GetUserProfileError import com.flipcash.services.models.ProfileIdentifier +import com.flipcash.services.repository.AccountRepository import com.flipcash.services.repository.ProfileRepository import com.getcode.ed25519.Ed25519.KeyPair import com.getcode.opencode.model.core.ID import javax.inject.Inject /** - * Fetches a stored account's username and display name by the user id [AccountProfileCache] kept - * from when that account was signed in. + * Fetches a stored account's username and display name, signed with that account's own key the + * same way the account list signs its balance lookups. * - * `GetProfile` is a read, signed with the account's own key the same way the account list signs its - * balance lookups. There is deliberately no path from an owner key to a user id here: that takes the - * Login RPC, and the app must not sign in as an account the user has not switched to. A caller with - * no stored user id has no fetch to make. + * `GetProfile` is keyed by user id. The fetch uses the user id [AccountProfileCache] kept from when + * the account was signed in. Only an account with no stored user id goes through the Login RPC, + * the one call that maps an owner key to its user id; the id it returns is cached so that account + * never needs Login again on this device. * * Every answer is written to [AccountProfileCache] so the list can still name the account offline. */ class AccountProfileFetcher @Inject constructor( + private val accountRepository: AccountRepository, private val profileRepository: ProfileRepository, private val cache: AccountProfileCache, ) { @@ -26,17 +28,28 @@ class AccountProfileFetcher @Inject constructor( * The account's current names. An account the server has no profile for succeeds with both * names null — it has none — rather than failing, so a stale cached name does not outlive it. */ - suspend fun fetch(owner: KeyPair, ownerAddress: String, userId: ID): Result = - profileRepository.getProfile(ProfileIdentifier.UserId(userId), owner) - .map { profile -> - AccountProfileName( - username = profile.username?.takeIf { it.isNotBlank() }, - displayName = profile.displayName.takeIf { it.isNotBlank() }, - ) - } - .recoverCatching { error -> - if (error !is GetUserProfileError.NotFound) throw error - AccountProfileName(username = null, displayName = null) + suspend fun fetch(owner: KeyPair, ownerAddress: String, userId: ID?): Result = + resolveUserId(owner, ownerAddress, userId) + .mapCatching { id -> + profileRepository.getProfile(ProfileIdentifier.UserId(id), owner) + .map { profile -> + AccountProfileName( + username = profile.username?.takeIf { it.isNotBlank() }, + displayName = profile.displayName.takeIf { it.isNotBlank() }, + ) + } + .recover { error -> + if (error !is GetUserProfileError.NotFound) throw error + AccountProfileName(username = null, displayName = null) + } + .getOrThrow() } .onSuccess { name -> cache.put(ownerAddress, name) } + + private suspend fun resolveUserId(owner: KeyPair, ownerAddress: String, userId: ID?): Result = + if (userId != null) { + Result.success(userId) + } else { + accountRepository.login(owner).onSuccess { id -> cache.putUserId(ownerAddress, id) } + } } diff --git a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt index c88b4b4f95..8900f70124 100644 --- a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt +++ b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt @@ -3,6 +3,7 @@ package com.flipcash.app.auth.internal.accounts import com.flipcash.services.models.GetUserProfileError import com.flipcash.services.models.ProfileIdentifier import com.flipcash.services.models.UserProfile +import com.flipcash.services.repository.AccountRepository import com.flipcash.services.repository.ProfileRepository import com.getcode.ed25519.Ed25519.KeyPair import io.mockk.coEvery @@ -17,9 +18,10 @@ class AccountProfileFetcherTest { private val owner: KeyPair = mockk() private val userId = listOf(1, 2, 3) + private val accounts: AccountRepository = mockk() private val profiles: ProfileRepository = mockk() private val cache: AccountProfileCache = mockk(relaxed = true) - private val fetcher = AccountProfileFetcher(profiles, cache) + private val fetcher = AccountProfileFetcher(accounts, profiles, cache) @Test fun `fetches the profile by the stored user id, and caches it`() = runTest { @@ -30,6 +32,7 @@ class AccountProfileFetcherTest { assertEquals(AccountProfileName(username = "sally", displayName = "Sally"), name) coVerify { cache.put("owner", name) } + coVerify(exactly = 0) { accounts.login(any()) } } /** No profile is an answer, not a failure: the account has no names, so a cached one must go. */ @@ -51,14 +54,29 @@ class AccountProfileFetcherTest { coVerify(exactly = 0) { cache.put(any(), any()) } } - /** - * The fetcher runs for accounts the user has not switched to, so it must not be able to log in - * as them. An earlier version resolved the user id through the account service's Login RPC; - * any dependency beyond these two fails here until someone confirms it cannot do that. - */ + /** Login is the last resort: only an account with no stored user id goes through it. */ @Test - fun `depends on nothing that can log in`() { - val injected = AccountProfileFetcher::class.java.constructors.single().parameterTypes.toList() - assertEquals(listOf(ProfileRepository::class.java, AccountProfileCache::class.java), injected) + fun `resolves a missing user id with Login, and caches the id`() = runTest { + val resolved = listOf(9, 9) + coEvery { accounts.login(owner) } returns Result.success(resolved) + coEvery { profiles.getProfile(ProfileIdentifier.UserId(resolved), owner) } returns + Result.success(UserProfile.Empty.copy(displayName = "Sally", username = null)) + + val name = fetcher.fetch(owner, "owner", userId = null).getOrThrow() + + assertEquals(AccountProfileName(username = null, displayName = "Sally"), name) + coVerify(exactly = 1) { accounts.login(owner) } + coVerify { cache.putUserId("owner", resolved) } + coVerify { cache.put("owner", name) } + } + + @Test + fun `a failed Login fetches nothing and leaves the cache alone`() = runTest { + coEvery { accounts.login(owner) } returns Result.failure(IllegalStateException()) + + assertTrue(fetcher.fetch(owner, "owner", userId = null).isFailure) + coVerify(exactly = 0) { profiles.getProfile(any(), any()) } + coVerify(exactly = 0) { cache.put(any(), any()) } + coVerify(exactly = 0) { cache.putUserId(any(), any()) } } }