diff --git a/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountRow.kt b/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountRow.kt index 9ce34d89de..f795d4ca3f 100644 --- a/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountRow.kt +++ b/apps/flipcash/features/login/src/main/kotlin/com/flipcash/app/login/internal/accounts/AccountRow.kt @@ -1,23 +1,24 @@ package com.flipcash.app.login.internal.accounts import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.Image import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.material.Text -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.Check -import androidx.compose.material3.Icon import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color +import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import com.flipcash.app.core.ui.shimmer import com.flipcash.features.login.R import com.getcode.theme.CodeTheme @@ -43,17 +44,16 @@ internal fun AccountRow( verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x3), ) { - // The tick is laid out for every row so the names line up; it is only drawn, and only - // announced, for the account the user is signed into. - Icon( - modifier = Modifier.size(CodeTheme.dimens.grid.x4), - imageVector = Icons.Default.Check, + // The same mark as the region, token and group-invite pickers. + Image( + painter = painterResource( + if (isCurrent) R.drawable.ic_checked else R.drawable.ic_unchecked + ), contentDescription = if (isCurrent) { stringResource(R.string.subtitle_currentAccount) } else { null }, - tint = if (isCurrent) CodeTheme.colors.textMain else Color.Transparent, ) Column( @@ -81,7 +81,7 @@ internal fun AccountRow( } // Order matters: a resolved balance wins, then the backend's own "no such account", then - // our inability to ask. An account whose balance we could not fetch must not be reported as + // our inability to ask, and only then is the fetch still in flight. An account whose balance we could not fetch must not be reported as // not found — the two say very different things to someone checking their own wallet. when { account.balance != null -> Text( @@ -101,6 +101,15 @@ internal fun AccountRow( style = CodeTheme.typography.textSmall, color = CodeTheme.colors.textSecondary, ) + + // The skeleton the discovery list uses, sized to a short balance on the textMedium + // line. It carries no semantics: TalkBack announces the balance when it lands. + else -> Box( + Modifier + .size(width = 60.dp, height = 14.dp) + .shimmer() + ) } } } + 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 966f2c71a9..e13749cb6c 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,10 @@ 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 import com.flipcash.features.login.R import com.flipcash.libs.coroutines.DispatcherProvider import com.getcode.crypt.DerivePath @@ -15,11 +18,8 @@ import com.getcode.opencode.managers.MnemonicManager import com.getcode.opencode.model.accounts.AccountCluster import com.getcode.opencode.model.financial.Fiat import com.getcode.opencode.model.financial.Token -import com.getcode.opencode.model.financial.TokenResult import com.getcode.opencode.model.financial.sum import com.getcode.opencode.model.financial.usdf -import com.getcode.opencode.providers.TokenMetadataProvider -import com.getcode.solana.keys.Mint import com.getcode.solana.keys.base58 import com.getcode.util.resources.ResourceHelper import com.getcode.utils.TraceType @@ -28,9 +28,10 @@ import com.getcode.view.BaseViewModel import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.FlowPreview +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.asFlow -import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.filterIsInstance import kotlinx.coroutines.flow.flatMapLatest @@ -39,8 +40,10 @@ 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 import java.util.concurrent.atomic.AtomicBoolean import javax.inject.Inject @@ -50,6 +53,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( @@ -58,28 +62,6 @@ class AccountSelectionViewModel @Inject constructor( defaultDispatcher = dispatchers.Default, ) { - /** - * TokenController does not implement [TokenMetadataProvider] itself (only TokenCoordinator - * does, scoped to the single signed-in user, which is wrong for this screen's other, dormant - * accounts), so this screen brings its own, delegating to TokenController's network-tier - * lookup with no cache — there is no warm cache here to benefit from anyway. - * - * One instance per account fetch, because it also records whether any lookup failed. - * `fetchTokenAccounts` resolves metadata through `mapNotNull`, so a failed lookup silently - * drops that token: without this flag an account whose metadata we could not read is - * indistinguishable from one the backend has never heard of, and the row would say - * "Not Found" about a wallet that exists. - */ - private inner class RecordingMetadataProvider : TokenMetadataProvider { - var failed: Boolean = false - private set - - override suspend fun getTokenMetadata(mint: Mint): Result = - tokenController.getTokenMetadata(mint).onFailure { failed = true } - - override fun observeTokenCache(): Flow> = emptyFlow() - } - /** * One row. All three result fields false/null is the in-flight state. * @@ -96,13 +78,18 @@ class AccountSelectionViewModel @Inject constructor( */ val id: String, val entropy: String, - val name: String, + /** The mnemonic name, the title of last resort. */ + val mnemonicName: String, + val username: String? = null, + val displayName: String? = null, val ownerAddress: String, val creationDate: Long, val balance: Fiat? = null, val notFound: Boolean = false, val balanceUnavailable: Boolean = false, ) { + val name: String get() = title(username, displayName, mnemonicName) + // The default data class toString() would put a seed into every log line and crash report // that touches this state. override fun toString(): String = "AccountUiModel(owner=$ownerAddress)" @@ -123,6 +110,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 @@ -143,16 +131,19 @@ class AccountSelectionViewModel @Inject constructor( .onStart { emit(Event.Load) } .map { derive(authManager.accounts.all()) } .onEach { derived -> + val profiles = authManager.accountProfiles.all() dispatchEvent( Event.OnAccountsLoaded( accounts = derived.map { (record, cluster) -> - record.toUiModel(cluster.getOrNull()) + record.toUiModel(cluster.getOrNull(), profiles) }, currentEntropy = authManager.currentEntropy, ) ) } - .flatMapLatest { derived -> derived.asFlow().flatMapMerge { balance(it) } } + .flatMapLatest { derived -> + derived.asFlow().flatMapMerge { merge(balance(it), profile(it)) } + } .onEach { dispatchEvent(it) } // Derivation is PBKDF2 plus a SLIP-10 chain, and the fetches are network calls; // neither belongs on the thread drawing the list. @@ -219,18 +210,35 @@ class AccountSelectionViewModel @Inject constructor( * stretch plus a SLIP-10 chain, and this list can hold 50 accounts. A record that will not * derive is kept rather than dropped — the user should still see it. */ - private fun derive(records: List): List>> = - records.map { record -> record to runCatching { clusterFor(record.entropy) } } + /** + * Each account's key costs a PBKDF2 run and a SLIP-10 chain, and nothing on screen can show + * until every one is done, so they run side by side on the CPU pool instead of one after + * another. + */ + private suspend fun derive( + records: List, + ): List>> = withContext(dispatchers.Default) { + records.map { record -> + async { record to runCatching { clusterFor(record.entropy) } } + }.awaitAll() + } - private fun AccountRecord.toUiModel(cluster: AccountCluster?): AccountUiModel { + private fun AccountRecord.toUiModel( + cluster: AccountCluster?, + profiles: Map, + ): AccountUiModel { val mnemonic = runCatching { mnemonicManager.fromEntropyBase64(entropy) }.getOrNull() val owner = cluster?.authorityPublicKey?.base58() + val profile = owner?.let { profiles[it] } 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, - name = mnemonic?.let { displayName(it) }.orEmpty(), + // 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, ownerAddress = owner?.let { truncateAddress(it) }.orEmpty(), creationDate = creationDate, // A record we cannot derive from is one we can never fetch a balance for. @@ -262,13 +270,12 @@ class AccountSelectionViewModel @Inject constructor( return@flow emit(Event.OnBalanceUnavailable(record.entropy)) } - val metadata = RecordingMetadataProvider() - tokenController.fetchTokenAccounts(owner, metadata) + // Batched: one account lookup and one metadata lookup, however many tokens the account + // holds. It fails outright rather than drop a token whose metadata did not come back, so a + // short total is never shown as the balance. + tokenController.fetchTokenBalances(owner) .onSuccess { tokens -> when { - // A dropped token makes the total wrong as surely as it makes an empty list - // meaningless, so a failed lookup rules out both other answers. - metadata.failed -> emit(Event.OnBalanceUnavailable(record.entropy)) tokens.isEmpty() -> emit(Event.OnBalanceNotFound(record.entropy)) else -> emit( Event.OnBalanceResolved( @@ -289,9 +296,38 @@ 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" + /** + * A row's title: the account's `@username`, else its profile display name, else the + * mnemonic name. iOS picks in the same order, so an account reads the same on both. + * Blank counts as absent, so a cleared name falls through rather than showing nothing. + */ + fun title(username: String?, displayName: String?, mnemonicName: String): String = + username?.takeIf { it.isNotBlank() }?.asHandle() + ?: displayName?.takeIf { it.isNotBlank() } + ?: mnemonicName + /** iOS's `MnemonicPhrase.name`: first word, ellipsis, last word, both capitalised. */ fun displayName(mnemonic: MnemonicPhrase): String = listOf(mnemonic.words.first(), mnemonic.words.last()) @@ -313,6 +349,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 045318f03d..2b753730f7 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 @@ -2,6 +2,9 @@ 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 @@ -17,6 +20,7 @@ import kotlinx.coroutines.test.runTest import org.junit.Rule import org.junit.Test import org.mockito.kotlin.any +import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -46,6 +50,10 @@ 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() + } /** Records only what was asked of it; the real store's semantics are covered by its own tests. */ private class RecordingAccountStore(private var records: List) : AccountStore { @@ -66,6 +74,7 @@ class AccountSelectionViewModelStateTest { private fun viewModel(store: AccountStore, current: String?, dispatchers: TestDispatchers): AccountSelectionViewModel { whenever(authManager.accounts).thenReturn(store) + whenever(authManager.accountProfiles).thenReturn(profiles) whenever(authManager.currentEntropy).thenReturn(current) whenever(mnemonicManager.fromEntropyBase64(any())) .thenThrow(IllegalArgumentException("not a real seed")) @@ -73,6 +82,7 @@ class AccountSelectionViewModelStateTest { authManager = authManager, mnemonicManager = mnemonicManager, tokenController = tokenController, + profileFetcher = profileFetcher, resources = resources, dispatchers = dispatchers, ) @@ -114,6 +124,7 @@ class AccountSelectionViewModelStateTest { fun `selecting an account switches to its base58 seed`() = runTest(mainCoroutineRule.dispatcher) { val phrase = MnemonicPhrase(MnemonicPhrase.Kind.L12, List(12) { "abandon" }) whenever(authManager.accounts).thenReturn(RecordingAccountStore(listOf(record("a", 1_000L)))) + whenever(authManager.accountProfiles).thenReturn(profiles) whenever(authManager.currentEntropy).thenReturn(null) whenever(mnemonicManager.fromEntropyBase64(any())).thenReturn(phrase) whenever(mnemonicManager.getEncodedBase58(phrase)).thenReturn("base58-of-a") @@ -121,6 +132,7 @@ class AccountSelectionViewModelStateTest { authManager = authManager, mnemonicManager = mnemonicManager, tokenController = tokenController, + profileFetcher = profileFetcher, resources = resources, dispatchers = TestDispatchers(testScheduler), ) @@ -157,4 +169,44 @@ class AccountSelectionViewModelStateTest { assertEquals(listOf("a"), store.deleted) 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. */ + @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"), + ) + ) + advanceUntilIdle() + + assertEquals(listOf("@sally_streamer", ""), viewModel.stateFlow.value.accounts.map { it.name }) + } + + private val mnemonicName = "Apple ... Elder" + + @Test + fun `title prefers the username, as a handle`() { + assertEquals( + "@sally_streamer", + AccountSelectionViewModel.title(username = "sally_streamer", displayName = "Sally", mnemonicName = mnemonicName), + ) + } + + @Test + fun `title falls back to the display name without a username`() { + assertEquals("Sally", AccountSelectionViewModel.title(username = null, displayName = "Sally", mnemonicName = mnemonicName)) + assertEquals("Sally", AccountSelectionViewModel.title(username = " ", displayName = "Sally", mnemonicName = mnemonicName)) + } + + @Test + fun `title falls back to the mnemonic name without either`() { + assertEquals(mnemonicName, AccountSelectionViewModel.title(username = null, displayName = null, mnemonicName = mnemonicName)) + assertEquals(mnemonicName, AccountSelectionViewModel.title(username = "", displayName = "", mnemonicName = mnemonicName)) + } } diff --git a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt index c1987223dc..9d2577164c 100644 --- a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt +++ b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt @@ -2,6 +2,7 @@ package com.flipcash.app.auth import androidx.core.app.NotificationManagerCompat import com.flipcash.app.appsettings.AppSettingsCoordinator +import com.flipcash.app.auth.internal.accounts.AccountProfileCache import com.flipcash.app.auth.internal.accounts.AccountStore import com.flipcash.app.auth.internal.credentials.LookupResult import com.flipcash.app.auth.internal.credentials.PassphraseCredentialManager @@ -43,6 +44,7 @@ import javax.inject.Singleton class AuthManager @Inject constructor( private val credentialManager: PassphraseCredentialManager, private val accountStore: AccountStore, + private val accountProfileCache: AccountProfileCache, private val userManager: UserManager, private val notificationManager: NotificationManagerCompat, private val accountController: AccountController, @@ -288,6 +290,12 @@ class AuthManager @Inject constructor( val accounts: AccountStore get() = accountStore + /** + * The last known username and display name of each account, by owner public key. Filled as + * each account signs in, so a row for an account that is not signed in can still be named. + */ + val accountProfiles: AccountProfileCache get() = accountProfileCache + /** * The entropy of the signed-in account, or null when nobody is. The selection screen needs it * to mark its own row and to refuse to remove the account underneath the user. 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 new file mode 100644 index 0000000000..c596e3597a --- /dev/null +++ b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCache.kt @@ -0,0 +1,150 @@ +package com.flipcash.app.auth.internal.accounts + +import android.content.Context +import androidx.datastore.core.handlers.ReplaceFileCorruptionHandler +import androidx.datastore.preferences.core.MutablePreferences +import androidx.datastore.preferences.core.PreferenceDataStoreFactory +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.emptyPreferences +import androidx.datastore.preferences.core.stringPreferencesKey +import androidx.datastore.preferences.preferencesDataStoreFile +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.solana.keys.base58 +import com.getcode.utils.TraceType +import com.getcode.utils.trace +import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.mapNotNull +import kotlinx.coroutines.launch +import javax.inject.Inject +import javax.inject.Singleton + +/** The part of an account's profile the account list names it by. */ +data class AccountProfileName( + val username: String?, + val displayName: String?, +) + +/** + * 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. + * + * 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 + * key is the public owner key rather than the entropy so this file never holds a seed. + */ +@Singleton +class AccountProfileCache @Inject constructor( + @param:ApplicationContext private val context: Context, + userManager: UserManager, + dispatchers: DispatcherProvider, +) { + private val scope = CoroutineScope(SupervisorJob() + dispatchers.IO) + + private val dataStore = PreferenceDataStoreFactory.create( + corruptionHandler = ReplaceFileCorruptionHandler { emptyPreferences() }, + scope = scope, + produceFile = { context.preferencesDataStoreFile("account-profiles") } + ) + + init { + scope.launch { + userManager.state + .mapNotNull { state -> + entryFor( + owner = state.cluster?.authorityPublicKey?.base58(), + accountId = state.accountId, + profile = state.userProfile, + ) + } + .distinctUntilChanged() + .collect { (owner, name) -> persist(owner, name) } + } + } + + /** Every cached name, by owner public key. Empty when the file cannot be read. */ + suspend fun all(): Map = + runCatching { dataStore.data.first().toNames() } + .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 persist(owner: String, name: AccountProfileName) { + runCatching { + dataStore.edit { prefs -> + prefs.putOrRemove(usernameKey(owner), name.username) + prefs.putOrRemove(displayNameKey(owner), name.displayName) + } + }.onFailure { error -> + trace(tag = TAG, message = "Could not cache an account profile", error = error, type = TraceType.Error) + } + } + + private fun MutablePreferences.putOrRemove( + key: Preferences.Key, + value: String?, + ) { + if (value.isNullOrBlank()) remove(key) else set(key, value) + } + + companion object { + private const val TAG = "AccountProfileCache" + private const val USERNAME = "username:" + private const val DISPLAY_NAME = "displayName:" + + private fun usernameKey(owner: String) = stringPreferencesKey(USERNAME + owner) + private fun displayNameKey(owner: String) = stringPreferencesKey(DISPLAY_NAME + owner) + + private fun Preferences.toNames(): 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) } + }.toSet() + return owners.associateWith { owner -> + AccountProfileName( + username = values[USERNAME + owner] as? String, + displayName = values[DISPLAY_NAME + owner] as? String, + ) + } + } + + /** + * The entry to write for this state, or null when there is nothing to attribute. + * + * A server-fetched profile carries its user id; one that disagrees with the signed-in + * account is a fetch that outlived an account switch, and writing it would name one + * account after another. A profile with no id (the local restore) belongs to whoever is + * signed in, because the restore slot is wiped at logout. + */ + internal fun entryFor( + owner: String?, + accountId: ID?, + profile: UserProfile?, + ): Pair? { + if (owner == null || profile == null) return null + val profileId = profile.userId + if (profileId != null && accountId != null && profileId != accountId) return null + return owner to AccountProfileName( + username = profile.username?.takeIf { it.isNotBlank() }, + displayName = profile.displayName.takeIf { it.isNotBlank() }, + ) + } + } +} 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..585e6b71a3 --- /dev/null +++ b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcher.kt @@ -0,0 +1,46 @@ +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/AuthManagerTest.kt b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt index de36baacee..845ef1bc8e 100644 --- a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt +++ b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt @@ -105,6 +105,7 @@ class AuthManagerTest { authManager = AuthManager( credentialManager = credentialManager, accountStore = accountStore, + accountProfileCache = mockk(relaxed = true), userManager = userManager, notificationManager = notificationManager, accountController = accountController, 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 new file mode 100644 index 0000000000..21e288cd62 --- /dev/null +++ b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileCacheTest.kt @@ -0,0 +1,51 @@ +package com.flipcash.app.auth.internal.accounts + +import com.flipcash.services.models.UserProfile +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +class AccountProfileCacheTest { + + private val signedIn = listOf(1, 2, 3) + + @Test + fun `attributes a profile to the signed-in owner`() { + val profile = UserProfile.Empty.copy(displayName = "Sally", username = "sally", userId = signedIn) + assertEquals( + "owner" to AccountProfileName(username = "sally", displayName = "Sally"), + AccountProfileCache.entryFor(owner = "owner", accountId = signedIn, profile = profile), + ) + } + + @Test + fun `attributes a restored profile with no user id`() { + val profile = UserProfile.Empty.copy(displayName = "Sally") + assertEquals( + "owner" to AccountProfileName(username = null, displayName = "Sally"), + AccountProfileCache.entryFor(owner = "owner", accountId = signedIn, profile = profile), + ) + } + + /** A fetch that outlived an account switch must not name the new account after the old one. */ + @Test + fun `skips a profile that belongs to another account`() { + val profile = UserProfile.Empty.copy(displayName = "Sally", userId = listOf(9)) + assertNull(AccountProfileCache.entryFor(owner = "owner", accountId = signedIn, profile = profile)) + } + + @Test + fun `skips a state with no owner or no profile`() { + assertNull(AccountProfileCache.entryFor(owner = null, accountId = signedIn, profile = UserProfile.Empty)) + assertNull(AccountProfileCache.entryFor(owner = "owner", accountId = signedIn, profile = null)) + } + + @Test + fun `stores blank names as absent`() { + val profile = UserProfile.Empty.copy(displayName = " ", username = "") + assertEquals( + "owner" to AccountProfileName(username = null, displayName = null), + AccountProfileCache.entryFor(owner = "owner", accountId = signedIn, profile = profile), + ) + } +} 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..ab9ef234d4 --- /dev/null +++ b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/internal/accounts/AccountProfileFetcherTest.kt @@ -0,0 +1,67 @@ +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()) } + } +} diff --git a/services/opencode/src/main/kotlin/com/getcode/opencode/controllers/TokenController.kt b/services/opencode/src/main/kotlin/com/getcode/opencode/controllers/TokenController.kt index 49cb5060f6..577bbf40ed 100644 --- a/services/opencode/src/main/kotlin/com/getcode/opencode/controllers/TokenController.kt +++ b/services/opencode/src/main/kotlin/com/getcode/opencode/controllers/TokenController.kt @@ -131,6 +131,44 @@ class TokenController @Inject constructor( } } + /** + * Like [fetchTokenAccounts], for an owner with no warm metadata cache: one account lookup and + * one batched metadata lookup, however many tokens the owner holds. [fetchTokenAccounts] + * resolves metadata one mint at a time, which is cheap behind a cache and a round trip per + * token without one. This is the shape iOS's account list uses. + * + * A mint the batch does not return fails the whole fetch, rather than dropping that token and + * reporting a total that is silently short. + */ + suspend fun fetchTokenBalances(cluster: AccountCluster): Result> { + val response = retryable(maxRetries = 3) { + accountController.getAccounts( + cluster, cluster, AccountFilter.AccountType(AccountType.Primary) + ) + } ?: return Result.failure(IllegalStateException("Failed to fetch accounts after retries")) + + return response.mapCatching { accountResponse -> + val accounts = accountResponse.accounts.values + if (accounts.isEmpty()) return@mapCatching emptyList() + + val metadata = getTokenMetadata(accounts.map { it.mint }.distinct()) + .getOrThrow() + .associateBy { it.address } + + accounts.map { account -> + val token = metadata[account.mint] + ?: throw IllegalStateException("No metadata returned for ${account.mint}") + val tokenBalance = Fiat.tokenBalance(account.balance, token) + TokenWithBalance( + token = token, + balance = tokenBalance, + appreciation = tokenBalance - Fiat(fiat = account.usdCostBasis), + tokenQuarks = account.balance, + ) + } + } + } + /** * Fetches a single token account for the given mint and builds a [TokenWithBalance]. * diff --git a/services/opencode/src/test/kotlin/com/getcode/opencode/controllers/TokenControllerFetchTokenBalancesTest.kt b/services/opencode/src/test/kotlin/com/getcode/opencode/controllers/TokenControllerFetchTokenBalancesTest.kt new file mode 100644 index 0000000000..2cb8d48b5b --- /dev/null +++ b/services/opencode/src/test/kotlin/com/getcode/opencode/controllers/TokenControllerFetchTokenBalancesTest.kt @@ -0,0 +1,105 @@ +package com.getcode.opencode.controllers + +import com.flipcash.libs.currency.math.CurveTestInitializer +import com.getcode.opencode.model.accounts.AccountCluster +import com.getcode.opencode.model.accounts.AccountInfo +import com.getcode.opencode.model.accounts.AccountResponse +import com.getcode.opencode.model.financial.HolderMetrics +import com.getcode.opencode.model.financial.MintMetadata +import com.getcode.opencode.model.financial.Token +import com.getcode.opencode.model.financial.VmMetadata +import com.getcode.solana.keys.Mint +import com.getcode.solana.keys.PublicKey +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.test.runTest +import org.junit.BeforeClass +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class TokenControllerFetchTokenBalancesTest { + + companion object { + @JvmStatic + @BeforeClass + fun initCurve() { + CurveTestInitializer.initialize() + } + } + + private val cluster: AccountCluster = mockk() + private val accounts: AccountController = mockk() + private val currency: CurrencyController = mockk() + private val controller = TokenController(accounts, currency) + + /** The regression: an owner with many tokens paid one metadata round trip per token. */ + @Test + fun `metadata for every mint comes from one batched call`() = runTest { + givenAccounts(account(Mint.usdf, 1_000_000), account(Mint.usdc, 2_000_000)) + coEvery { currency.getMintMetadata(any()) } returns + Result.success(listOf(token(Mint.usdf), token(Mint.usdc))) + + val balances = controller.fetchTokenBalances(cluster).getOrThrow() + + assertEquals(listOf(Mint.usdf, Mint.usdc), balances.map { it.token.address }) + coVerify(exactly = 1) { currency.getMintMetadata(any()) } + coVerify { currency.getMintMetadata(match { it.toSet() == setOf(Mint.usdf, Mint.usdc) }) } + } + + @Test + fun `an owner with no accounts has no balances, and costs no metadata call`() = runTest { + givenAccounts() + + assertEquals(emptyList(), controller.fetchTokenBalances(cluster).getOrThrow()) + coVerify(exactly = 0) { currency.getMintMetadata(any()) } + } + + /** A dropped token would make the total silently short, so a missing mint fails the fetch. */ + @Test + fun `a mint the batch does not return fails the fetch`() = runTest { + givenAccounts(account(Mint.usdf, 1_000_000), account(Mint.usdc, 2_000_000)) + coEvery { currency.getMintMetadata(any()) } returns Result.success(listOf(token(Mint.usdf))) + + assertTrue(controller.fetchTokenBalances(cluster).isFailure) + } + + @Test + fun `a failed metadata batch fails the fetch`() = runTest { + givenAccounts(account(Mint.usdf, 1_000_000)) + coEvery { currency.getMintMetadata(any()) } returns Result.failure(IllegalStateException()) + + assertTrue(controller.fetchTokenBalances(cluster).isFailure) + } + + private fun givenAccounts(vararg infos: AccountInfo) { + coEvery { accounts.getAccounts(cluster, cluster, any()) } returns Result.success( + AccountResponse(infos.associateBy { mockk() }) + ) + } + + private fun account(mint: Mint, quarks: Long): AccountInfo = mockk { + every { this@mockk.mint } returns mint + every { balance } returns quarks + every { usdCostBasis } returns 0.0 + } + + private val dummyKey = PublicKey.fromBase58("11111111111111111111111111111111") + + private fun token(mint: Mint): Token = MintMetadata( + address = mint, + decimals = 6, + name = "", + symbol = "", + createdAt = null, + description = "", + imageUrl = "", + vmMetadata = VmMetadata(vm = dummyKey, authority = dummyKey, lockDurationInDays = 21), + launchpadMetadata = null, + billCustomizations = null, + socialLinks = emptyList(), + holderMetrics = HolderMetrics.None, + ) +}