feat(spark): UI/UX launch polish across settings and flows#3769
Conversation
esaugomez31
changed the title
esaugomez31
changed the title
|
Additional small changes:
|
|
When restoring, there is no loading state for balance, instead I see $0.00 which is quite bad UX since I expect to see the full balance. zero-balance-error.webm |
esaugomez31
force-pushed
the
feat--self-custodial-launch-polish
branch
from
18d14a7 to
17e5ae9
Compare
|
Reviewing on android emulator shows these still dont work:
|
esaugomez31
force-pushed
the
feat--self-custodial-launch-polish
branch
from
9c9bf69 to
f8fc5f1
Compare
|
@esaugomez31 
|
esaugomez31
force-pushed
the
feat--self-custodial-launch-polish
branch
from
b958e5f to
d93e65a
Compare
Already fixed! |
esaugomez31
force-pushed
the
feat--spark-rollout-and-hardening
branch
from
7bb7129 to
d464d96
Compare
esaugomez31
force-pushed
the
feat--self-custodial-launch-polish
branch
from
16e3919 to
5c7a7ea
Compare
grimen
left a comment
grimen
left a comment
There was a problem hiding this comment.
Review — bugs + test coverage
Important
The structural concerns flagged in the #3758 review continue to compound through this PR — same deferral applies. Findings layered on top of this PR:
- SOLID violations —
useEffective{DisplayCurrency,Language}return mode-dependent shapes (theloadingflag is meaningful for custodial, hardcodedfalsefor SC);useEffectiveAuthTokenreturns""instead of a typedToken | null;BackendFeatureGatenow branches onactiveAccount.typeso the gate logic mode-leaks;useNonCustodialConversionGuardanduseNonCustodialConversionboth subscribe touseConversionQuoteindependently — duplicated quote-running logic across two consumers;useAccountTypeOptionsmixes country detection, feature-flag reading, restore-vs-create mode, and UI-state into one hook. - Incomplete adapter pattern — the
useEffective*ports are well-shaped, but the rest of the codebase keeps bifurcating at call sites.selfCustodialDefaultWalletCurrencyhas nouseEffective*equivalent, souse-receive-asset-mode.tsreads persistent state directly. Probe vs. live-wallets logic is split by type at the call site (self-custodial-profile-row.tsx) instead of unified behind agetWalletsForAccount(accountId)port. Settings, home, conversion screens still branch onisSelfCustodialeverywhere — the adapter wins haven't propagated. - High cyclomatic complexity in the screen layer —
home-screen.tsxnow referencesisSelfCustodial/activeIsCustodialin 17 places (was 15 at the time of #3758's review);contacts-card.tsx13;send-bitcoin-destination-screen.tsx12;receive-screen.tsx11;transaction-history-screen.tsx10;conversion-details-screen.tsxandconversion-confirmation-screen.tsx9 each. 53 new mode-branching lines added in this PR alone. The bifurcation is expanding faster than the adapter pattern is absorbing it. - Code smells — error reporting is now four-way inconsistent:
reportError,crashlytics().recordError,crashlytics().log(breadcrumb only), and bareconsole.errorall appear in this PR's added code (see Critical #3, I6, I9). Empty.catch(() => {})blocks remain inwallet-provider.tsx,use-payment-request.ts, anduse-sdk-lifecycle.ts. TheAutoConvertStatusProviderrebuilds a freshMapper update (O(n) insert) and never prunes (I11).findContactByPaymentIdentifieris an O(n) linear scan against a hardcoded 100-item page (I13). String-matched error classification (if (err.message.includes(...))) flagged in #3758 still appears in deposit/onchain paths. - Naming smells — 49 files in this PR carry
selfCustodial/SelfCustodialin their path; the mode prefix is now baked into ~36 new exports (selfCustodialDefaultWalletCurrencyByAccountId,selfCustodialDisplayCurrencyByAccountId,selfCustodialLanguageByAccountId,getSelfCustodialDefaultCurrency,withSelfCustodialDefaultCurrency,probeSelfCustodialAccountWallets,SelfCustodialAccountFields, …). TheEffectiveprefix is imprecise —useEffectiveDisplayCurrencyis just the resolved active value; if the bifurcating versions were renamed/removed it would beuseDisplayCurrency.Sparkbrand-name still leaks into types (SparkSDK*,SparkViewBackupPhraseScreen). And the same concept is named four different ways across the codebase:Custodial/Non-custodial/SelfCustodial/SC.
Why this is not in scope for this PR: the cleanup ticket from #3758's review already covers the adapter-shape unification, mode-leakage cleanup, and rename passes. Doing them here would (a) require a multi-day restack across the launch-polish stack right before launch, (b) overlap with work that has to land against main post-stack anyway, and (c) duplicate effort already scoped on the cleanup ticket. #3768's production rollout was already gated on the cleanup ticket reaching the adapter-pattern-unification milestone — this PR doesn't change that gating.
Decision: this review covers correctness only. The structural / SOLID / complexity / naming items above are not raised as blockers here — they're recorded for the existing cleanup ticket so the deferred debt remains visible and counted. New items surfaced in this PR (mainly the 17-mode home-screen.tsx, the 53 added mode-branches, and the four-way error-reporting inconsistency) should be appended to that ticket rather than addressed in-stack.
Real strengths first: the useEffective{DisplayCurrency,Language} adapter pattern is the right shape — a port hiding the custodial-GraphQL vs SC-persistent-state divergence behind one interface, with a clean useEffectiveAuthToken test that pins the architectural guarantee (token dropped even when KeyStore still holds it). The v10→v11 migration is correctly identity-preserving (additive optional fields). The cache-and-network + denominator-guard fix in usePriceConversion solves a real bug. useAutoConvertListener.spec.ts is a comprehensive scenario matrix (multi-invoice, dedup-on-rerender, replay vs live-trigger, attempt cap). The probe-based delete flow design — checking balance via short-lived SDK without switching active accounts — is the right idea.
Severity:
- Critical — fund-loss, mis-attribution, or silent failure that misleads the user. Must fix before merge.
- Important — correctness/reliability that doesn't lose funds but degrades UX or hides incidents.
- Test — coverage gap or test that passes while a confirmed bug is alive.
Critical
1. Probe failure routes user to delete-confirm modal → fund loss
app/self-custodial/probe-account-wallets.ts:13-26 + app/screens/settings-screen/account/multi-account/self-custodial-profile-row.tsx:82-84
probeSelfCustodialAccountWallets throws when initSdk or getSelfCustodialWalletSnapshot throws (storage corrupt, network blip, mnemonic-decrypt error, concurrent SDK contention). The caller catches and opens the plain delete confirm:
} catch (err) {
reportError("self-custodial-profile-row probeBalance", err)
setConfirmVisible(true) // user sees "are you sure you want to delete?"
}User taps yes → mnemonic wiped, balance still on Spark. The has-funds warning was the only barrier and it was silently skipped.
Compounding: when the row is the active account, the probe's initSdk(mnemonic, storageDirFor(accountId)) opens a second SDK against the same SQLite storage dir while useSdkLifecycle still holds the primary one — SQLite "database is locked", stale balance reads, mnemonic-mismatch panics.
Fix: discriminated result { status: "ok", wallets } | { status: "probe-failed" }. On failure, surface a "couldn't verify balance" toast and do not open the confirm modal. Short-circuit when accountId === activeAccount?.id — read live wallets from useSelfCustodialWallet() instead of probing.
2. iOS Keychain backup: only the most-recently saved wallet is recoverable
app/hooks/use-credential-backup.ts:87-99
const credentials = await Keychain.getInternetCredentials(BLINK_DOMAIN)react-native-keychain's getInternetCredentials(server) returns one entry — iOS Keychain Internet Password keys on (server, account) but the SecItem query without kSecAttrAccount returns the first match the OS chooses. Multi-account password-manager backup on iOS therefore silently loses access to one wallet on restore.
Fix: disable Keychain backup as a multi-account method on iOS, OR enumerate via raw SecItemCopyMatching with kSecMatchLimitAll and surface a picker.
3. Migration failure silently resets every per-account preference
app/store/persistent-state/state-migrations.ts:204-209
} catch (err) {
console.error({ err }, "error migrating persistent state")
}
return defaultPersistentStateconsole.error only — no Sentry/Crashlytics. A real-device migration throw silently logs the user out, resets selfCustodialDisplayCurrencyByAccountId, selfCustodialLanguageByAccountId, activeAccountId. No toast, no telemetry, no recovery.
Fix: crashlytics().recordError(err). Quarantine the raw input under a backup key before returning defaults so support can recover the state.
4. AsyncStorage transient failure → all SC accounts vanish from the registry
app/self-custodial/storage/account-index.ts:26-47
readIndex does catch { return [] } — any AsyncStorage hiccup makes the registry think the user has no SC accounts. The active wallet disappears from the home screen even though the mnemonic is still in keychain. Combined with #5 below, the user lands on USD pricing for a wallet that's actually still on Spark.
Fix: crashlytics().recordError on the catch; distinguish "empty index" from "could not read index" so callers can retry rather than treat it as "user has no accounts."
5. addContact upsert silently discards user-supplied name on duplicate
app/self-custodial/bridge/contacts.ts:31-42
return sdk.updateContact({
id: existing.id,
name: existing.name, // <- always existing, ignores request.name
paymentIdentifier: existing.paymentIdentifier,
})The "preserve user-edited contact name" intent is right when the user has previously renamed the contact — but the same code drops request.name even when the user just typed a new one. The PR description implies the request name takes effect on first save and is preserved on subsequent payments; the code preserves the existing name unconditionally. The caller use-save-lnaddress-contact.ts:56 only catches exceptions, so the silent no-op is invisible.
Fix: decide explicitly. Either pass request.name through, or return { deduped: true, existingName } so the UI can surface "already saved as X — keep or rename?". Today's behavior is documentation/implementation skew.
6. Convert guard ignores quote-error and is disabled during SC SDK boot
app/screens/conversion-flow/hooks/use-non-custodial-conversion-guard.ts:24-35, 37-61 + app/screens/conversion-flow/hooks/use-conversion-quote.ts
useConversionQuote records errors to crashlytics and sets { status: QuoteStatus.Error, quote: null }. The guard destructures only { isQuoting, amountAdjustment } — when the quote fails, amountAdjustment === null → blockingReason === null → "no problem, allow review." User taps Review with no quote.
Adjacent: enabled: isSelfCustodial from useActiveWallet flips to false while the SC SDK is in Unavailable — during boot the guard is silently disabled. Same outcome, different trigger.
Fix: plumb hasQuoteError out of useConversionQuote into the guard; disable Review while either hasQuoteError || isQuoting. Gate enabled on !isReady rather than isSelfCustodial so the boot window is covered.
7. Default-currency fallback leaks legacy preference into new SC accounts
app/store/persistent-state/self-custodial-default-currency.ts:13-21
return fromMap ?? state.selfCustodialDefaultWalletCurrency ?? "BTC"A user with the legacy selfCustodialDefaultWalletCurrency = "USD" who creates a second SC wallet sees it inherit "USD". display-currency and language correctly fall back to constants — only default-currency carries the legacy field. Inconsistent with the rest of the multi-account design.
Fix: drop the legacy fallback (return fromMap ?? "BTC"), OR migrate the legacy value into the active account's slot during v10→v11 and clear it.
8. Drive API errors → user told "no backups exist"
app/utils/google-drive-client.ts (throws on !response.ok) + app/hooks/use-google-drive-backup.ts:67-89 (only console.errors) + app/screens/spark-onboarding/restore/hooks/use-cloud-restore.ts:97-105
if (files.length === 0) { setStep(CloudStep.NotFound); ... }
const result = await downloadById(files[0].id, token)
if (!result.success) { setStep(CloudStep.NotFound); ... }Auth-expired (401) and rate-limited (429) responses are indistinguishable from "no backups." Per-file failures (line 126) silently disappear from the picker. User thinks their backup is gone.
Fix: result type {success: false, reason: 'not-found' | 'transient' | 'auth'}. Only fall to NotFound for genuine 404s. Replace console.error with crashlytics().recordError.
9. Single-file cloud restore skips metadata validation
app/screens/spark-onboarding/restore/hooks/use-cloud-restore.ts:102-110
When files.length === 1, calls proceedWithBackup(content) immediately — no parseBackupMetadata check. Multi-file path correctly validates (line 119-126). A single corrupt or legacy-format file lands the user on step: Error via parseBackupPayload throwing inside proceedWithBackup, with no path forward.
Fix: apply the same parseBackupMetadata validation to the single-file path; fall to NotFound on parse failure.
10. Decrypt failure for any reason → "wrong password"
app/screens/spark-onboarding/restore/hooks/use-cloud-restore.ts:163-169
try {
const { mnemonic } = parseEncryptedBackupPayload(backupContent, password)
await restore(mnemonic)
} catch {
setPasswordError(LL.RestoreScreen.wrongPassword())
}Corrupt ciphertext, missing salt/iv, unsupported cipher, or restore failing for unrelated reasons (network, keystore) all collapse into "wrong password." User retypes the correct password ten times and gives up.
Adjacent: parseBackupPayload (app/utils/backup-payload.ts:129) doesn't validate parsed.mnemonic is a non-empty string. A misclassified-encrypted file (metadata claims encrypted but payload's encrypted field is missing) silently restores whatever (possibly empty) string is there.
Fix: catch only the AES-GCM auth-tag-mismatch path as "wrong password"; surface other errors distinctly. In parseBackupPayload, validate mnemonic is a non-empty string and throw a typed error.
11. SDK lifecycle and auto-convert provider have zero direct test coverage
app/self-custodial/providers/use-sdk-lifecycle.ts (305 LOC, 53 changed) + app/self-custodial/providers/auto-convert-status-provider.tsx (87 LOC, new)
The SDK init/reconnect/refresh state machine for the entire SC experience and the provider that drives the receive-screen converting spinner have no test files. wallet-provider.spec.tsx mocks the lifecycle entirely. Untested behaviors include reconnect backoff escalation, account-switch disconnect ordering (must disconnectSdk(prev) before initSdk(next) — payment events route to wrong account otherwise), pendingRefreshRef re-entrancy, multi-invoice isolation in the auto-convert provider, and identity preservation on no-op updates.
Fix: test files for both. Priority: account-switch disconnect ordering and multi-invoice converting-state isolation.
Important
| # | Item | Location |
|---|---|---|
| I1 | useEffectiveAuthToken ignores registry — orphaned activeAccountId strips Apollo auth, combined with BackendFeatureGate blocking SC, locks user out with no signal |
app/graphql/use-effective-auth-token.ts:7-14 |
| I2 | Apollo client rebuild on token swap silently drops in-flight queries; no log of the rebuild | app/graphql/client.tsx:93-318 + use-effective-auth-token.ts |
| I3 | Pull-to-refresh on Account screen — no try/finally around await work; rejection leaves spinner stuck forever |
app/screens/settings-screen/account/account-screen.tsx:36-48 |
| I4 | usePriceConversion denominator-guard sets convertMoneyAmount to undefined for the network-fetch window after a currency change → receive screen amountInSats undefined, conversion screen canExecute: false |
app/hooks/use-price-conversion.ts:32-52, 96-98 |
| I5 | Logged-out custodial users see USD instead of cached preference — useEffectiveDisplayCurrency skips the query when !isAuthed and falls back to DEFAULT_DISPLAY_CURRENCY |
app/hooks/use-effective-display-currency.ts:31-33, 62-67 |
| I6 | wallet-provider.tsx:107, 109-111, 129-131 — three opportunistic catches with no Crashlytics breadcrumb; stale Lightning address goes unobserved |
app/self-custodial/providers/wallet-provider.tsx |
| I7 | Invoice generation empty catch — opaque Error UI, no Sentry trace of why |
app/self-custodial/hooks/use-payment-request.ts:136-138 |
| I8 | use-create-wallet sets Error status with no toast — stuck Create button, no explanation |
app/screens/spark-onboarding/hooks/use-create-wallet.ts:46-49 |
| I9 | Post-connect sync failure logged as crashlytics().log (breadcrumb only); finally block forces Ready regardless → user sees stale balance |
app/self-custodial/providers/use-sdk-lifecycle.ts:188-195 |
| I10 | Generic restoreFailed toast — invalid-mnemonic vs network failure indistinguishable |
app/screens/spark-onboarding/restore/hooks/use-restore-wallet.ts:70-75 |
| I11 | AutoConvertStatusProvider.statusByInvoice is never pruned — entries leak per session, every update rebuilds a fresh Map (O(n) per insert) |
app/self-custodial/providers/auto-convert-status-provider.tsx:34-46 |
| I12 | useAutoConvertListener.processedPaymentIdsRef and inFlightInvoicesRef Sets leak unboundedly per session |
app/self-custodial/hooks/use-auto-convert-listener.ts:192-193, 248-249 |
| I13 | addContact paginates to 100 — a user with >100 contacts gets duplicates because matches past index 100 are invisible to findContactByPaymentIdentifier |
app/self-custodial/bridge/contacts.ts:11, 22-29 |
| I14 | disconnectSdk(sdk).catch(() => undefined) swallows failure with no log; repeated probes accumulate SDK instances holding open SQLite handles |
app/self-custodial/probe-account-wallets.ts:24 |
| I15 | cloud-backup overwrite confirmation doesn't show the existing backup's lightningAddress / createdAt — user can't spot a wrong-wallet overwrite |
app/screens/spark-onboarding/hooks/use-cloud-backup.ts:49-52 + google-drive-client.ts:91-102 |
Test coverage
Estimated behavioural coverage of the new SC code added in this PR: ~71%, dense on hooks + bridge + persistent-state, sparse on the two providers and the screen wiring of the converting indicator.
Tier 1 — production code with no test file
Verified against __tests__/:
| File | LOC | Notes |
|---|---|---|
app/self-custodial/providers/auto-convert-status-provider.tsx |
87 (new) | Drives the receive-screen converting spinner; multi-invoice isolation never exercised |
app/self-custodial/providers/use-sdk-lifecycle.ts |
305 (53 changed) | SDK init/reconnect/refresh state machine for the entire SC experience |
app/screens/conversion-flow/build-convert-params.ts |
25 (new) | Pure function deciding direction/from/to for the SDK call — only exercised via mocked guard tests |
app/screens/settings-screen/self-custodial/account-fields.tsx |
159 (new) | Loading/error states for the boxed Wallet identifier / Lightning address / Backup status fields |
Tier 2 — tests that pass while a Critical bug is alive
Each should be made to fail on the current code first, then fixed alongside the production fix.
| Test file | What it should catch but doesn't | Bug |
|---|---|---|
__tests__/screens/receive-self-custodial.spec.tsx:256-265 |
Mocks QRView and drops the converting prop entirely. The PR's headline "inline converting state" feature has zero verification. grep "converting" in the spec returns no matches. |
wires-up of #11 (auto-convert provider) |
__tests__/screens/settings-screen/delete-account-confirm-modal.spec.tsx + .../delete-account-has-funds-modal.spec.tsx |
Both verify modal rendering in isolation. Neither verifies the routing decision based on probe result — i.e. balance > 0 → has-funds modal, probe-failed → no modal. | #1 (fund loss) |
__tests__/screens/spark-onboarding/restore/hooks/use-cloud-restore.spec.ts:318-355 |
Multi-pick test covers plain-text picks only. Encrypted-multi-pick → password screen → wrong password → pick-another flow uncovered. | #10 (decrypt → wrong password) |
__tests__/store/persistent-state/state-migrations.spec.ts |
Doesn't test the catch-all defaultPersistentState return path — a regression that swaps recordError for console.error ships invisibly. |
#3 |
__tests__/hooks/use-effective-display-currency.spec.ts:85-89 |
Uses loading: false with data: undefined — conflates "loading-with-no-data" and "ready-with-no-data." A consumer flicker on account switch is invisible. |
I5 |
__tests__/hooks/use-price-conversion.spec.ts |
Doesn't test the cross-currency convertMoneyAmount recordError safeguard at use-price-conversion.ts:114-125 — regression dropping the guard ships invisibly. |
(safeguard removal) |
__tests__/screens/conversion-flow/use-non-custodial-conversion-guard.spec.ts |
convertMoneyAmount: undefined path uncovered — guard returns { blockingReason: null }, consumer screen behavior unverified. |
#6, I4 |
__tests__/self-custodial/bridge/contacts.spec.ts |
listContacts rejection uncovered. Pagination-cap behavior (>100 contacts) untested — bug is encoded by omission. |
I13 |
__tests__/self-custodial/hooks/use-auto-convert-listener.spec.ts |
recordAutoConvertAttempt.mockRejectedValue not exercised — if persistence rejects, markConverting skipped AND markSettled bypassed → spinner stuck indefinitely. |
(silent stall) |
__tests__/self-custodial/probe-account-wallets.spec.ts |
Covers the API in isolation. Doesn't exercise the consumer path that turns probe-failure into a delete-confirm modal. | #1 |
Tier 3 — Critical bugs with no test coverage anywhere
For each, no existing test exercises the behaviour. Add a regression test alongside the fix.
| Critical | Recommended test |
|---|---|
| #1 | Render self-custodial-profile-row, mock probeSelfCustodialAccountWallets rejecting → assert no confirm modal opens (toast/error state instead) |
| #2 | iOS-platform integration: save 2 wallets via Keychain → assert both retrievable via readIOS (current code returns only one) |
| #3 | Mock state read to throw → assert recordError called and quarantine key written, NOT silent fall-through to defaults |
| #4 | Mock AsyncStorage rejecting → assert recordError called and registry distinguishes "empty" from "failed-to-load" |
| #5 | addContact test for request.name !== existing.name → assert documented behavior (today's code silently keeps existing) |
| #6 | Mock useConversionQuote returning { status: Error } → assert blockingReason set or new quote-failed reason emitted |
| #7 | Persistent state with legacy selfCustodialDefaultWalletCurrency: "USD" and a fresh SC account id not in the per-account map → assert getSelfCustodialDefaultCurrency returns "BTC" not "USD" |
| #8 | Mock listAppDataFiles rejecting with 401 → assert step === CloudStep.Error not NotFound |
| #9 | Single corrupt file in Drive → assert step === NotFound (or distinct Error), not crash via parseBackupPayload throw |
| #10 | Mock decrypt throwing non-auth-tag error → assert distinct error message, not wrongPassword |
| #11 | auto-convert-status-provider: markConverting(A) then markConverting(B) → assert isolated state per key, markSettled(A) doesn't clear B. use-sdk-lifecycle: account switch from sc-1 to sc-2 → assert disconnectSdk(sdk1) called before initSdk(...sc-2-storage-dir) |
Suggested order of operations
- Land Criticals 1–10 in this PR with regression tests (Tier 3) co-located with each fix.
- Add the two missing spec files (Tier 1:
auto-convert-status-provider,use-sdk-lifecycle) — these are the riskiest gaps for the launch surface; ~250 LOC of test code total. - Make Tier 2 tests fail first on current code: pass
convertingthrough the QRView mock, add probe→modal-routing assertions, exercise the migration catch-all, exerciseconvertMoneyAmount: undefinedin the guard. - Important items I1–I15 — most are small; I1–I4 affect core flows and should fold into the Critical commits where adjacent.
- Tier 1
build-convert-params.tsandaccount-fields.tsxspecs in a follow-up before public rollout — both small (~30–50 LOC each) and worth the regression net.
esaugomez31
force-pushed
the
feat--spark-rollout-and-hardening
branch
from
d464d96 to
ff7941a
Compare
…ecks for accurate semantics
… and drop Spark prefix from ViewBackupPhrase
…storePhrase screens
…letCreation screens and rename i18n key
…and close emission gaps
… on SC boot-window test
…, refresh coalesce, and Drive errors
grimen
force-pushed
the
feat--self-custodial-launch-polish
branch
from
e463ddd to
2babc7d
Compare
grimen
force-pushed
the
graphite-base/3769
branch
from
50b91f1 to
f6f1d7b
Compare
grimen
force-pushed
the
feat--self-custodial-launch-polish
branch
from
2babc7d to
a8b65ce
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| </ContextForScreen>, | ||
| ) | ||
|
|
||
| expect(getByText(new RegExp(formattedDate.replace(/\//g, "\\/")))).toBeTruthy() |
| </ContextForScreen>, | ||
| ) | ||
|
|
||
| expect(queryByText(new RegExp(legacyFormatted.replace(/\//g, "\\/")))).toBeNull() |
4 participants
Summary
UI/UX launch polish for the self-custodial (Spark) experience: stable-balance gating, settings reorganization, account redesign, delete flow, default-account preference, receive flow QR by default, single-screen backup phrase, dynamic success messaging, stablecoins-aware Dollar modal, removal of the legacy "You Are in Control" trust-model modal, country-aware account-type gating, a comprehensive multi-account stack with auto-convert UX polish, and SDK amount-adjust block on the convert-details screen.
Primary ticket: blinkbitcoin/blink-wip#692 — feat(spark/ui): Review and fixes. Also addresses blinkbitcoin/blink-wip#664 (Stablesats/Dollar popup modal — non-custodial variant).
Ticket blinkbitcoin/blink-wip#692 — Tasks
stableBalanceEnabledremote flag (defaultfalse) gatesisStableBalanceActive. Stable-balance settings hidden when off.SettingsGroupno longer renders.Wallet identifier/Lightning address/Backup statusboxed read-only fields (shared with the AccountInformation sub-screen viaSelfCustodialAccountFields), then the danger zone with the delete flow (balance check + warning screen + SDK storage wipe +activeAccountIdclear). Custodial layout untouched.TrustModelModal,useTrustModelSeenhook,trust-model-screenre-export, and the fourBackupNudge.trustModel*i18n keys deleted across 28 locales. Home-screen integration removed; regression test asserts the modal no longer renders even for SC users with positive balance (the previous trigger condition).SparkViewBackupPhraseScreenrenders all 12 words in a 2-column grid. Success screen accepts a dynamicmessageroute param ("Your backup phrase is correct" for the test-backup flow). Onboarding 2-step flow untouched.useAccountTypeOptionshides the Custodial card when the detected country isn't inCUSTODIAL_ALLOWED_COUNTRIES, and the SC card whennonCustodialEnabledis off. Loading skeleton while detection runs; banner explains the state when SC is temporarily disabled.Ticket blinkbitcoin/blink-wip#664 — Stablesats/Dollar popup modal
StableSatsModalaccepts avariantprop. SC variant renders close-X, Dollar pill, centered title, centered body explaining stablecoins, primary "Back home", secondary "Learn more" →https://www.blink.sv/en/dollar-account. Custodial variant unchanged.Additional work (beyond ticket scope)
Account & default preferences
selfCustodialDefaultWalletCurrency(BTC | USD), scoped per account; settings picker; honored in send and receive flows; currency list always loaded so SC users can pick.dollarStablecoini18n key. Custodial keeps "Dollar (Stablesats)".Multi-account preferences (display currency + language)
selfCustodialDisplayCurrencyByAccountId,selfCustodialLanguageByAccountId); migration is identity.useEffective*adapter hooks (useEffectiveDisplayCurrency,useEffectiveLanguage) unify custodial (GraphQL-backed) and self-custodial (persistent-state-backed) sources behind a single interface. Screens,LanguageSync, andusePriceConversionconsume the adapter and never bifurcate by account type.usePriceConversionis fixed at the root:displayCurrencynow flows from the adapter rather than from Apollo's cacheddenominatorCurrency(which kept showing the previous account's currency on switch). A guard discards a cached price whose denominator disagrees with the active preference, andfetchPolicy: "cache-and-network"on both the authed and unauthed price queries forces an immediate refresh on account switches (custodial→custodial included).ConvertQuote.feeAmountis typed asUsdMoneyAmount(the SDK returns the USDB swap fee in cents, pegged 1:1 to USD) and the conversion-flow hook converts it viaconvertMoneyAmountbefore formatting. Previously the fee row rendered "Currency issue. Refresh needed" for EUR, JPY, and any other non-USD locale.Self-custodial contact dedup
bridge/addContactis now an upsert: when a contact with the samepaymentIdentifieralready exists (matched case-insensitive and trimmed via the existingnormalizeStringhelper), it callssdk.updateContactwith the existing record's data — preserving any user-edited name (so a contact renamed "Mom" isn't clobbered back to its LN address) and bumpingupdatedAtfor future "recent contacts" sorting. When no match exists, it falls through tosdk.addContact. Fixes the bug where every payment to the same LN address created another duplicate contact.Account-type labels i18n alignment
Custodial/Non-custodiallabels shown under each account's title in the multi-account switcher were inconsistent across locales: the English source saidCustodial/Non-custodial, but all 28 translation files saidBlink/Spark(a brand-name swap, not a translation). All 28 locales were aligned to the English technical concept, with proper grammatical agreement per language (es:Custodiada/No custodiada; pt:Custodiada/Não custodiada; it:Custodito/Non custodito; de:Verwahrt/Selbstverwahrt; fr:Sous garde/Sans garde; and so on).Convert flow
FlooredToMinorIncreasedToAvoidDust). For dust adjustments the block is suppressed when the user already requested the full balance, since asymmetric conversion math can flag it spuriously at 100%.useConversionQuote.Auto-convert UX
Multi-account hardening
setActiveAccountId.BackendFeatureGatereturns blocked when the active account is SC, so feature gating respects the active account.useEffectruns.create-walletno longer clobbers the previously-active account's backup state.blink-spark-backup-{network}-{walletIdentifier}.jsonso different wallets no longer overwrite each other. The payload now embedswalletIdentifierand (when set)lightningAddressas plaintext metadata; only the mnemonic stays encrypted. On restore, the app lists all backups by prefix, parses metadata without decrypting, and shows a picker when more than one is present (LN address as the row title with the identifier as subtitle when set, or the identifier alone). A single backup auto-restores; an encrypted backup proceeds straight to the password step.Settings & UI polish for SC
BackupPhraseSecurityCheckspure component; the two flows (onboarding vs Settings) keep their distinct destinations and the onboarding-specific migration checkpoint stays owned by the onboarding screen only.Send flow
Performance
Post-review polish & internal cleanup
.typessuffix fromapp/typesmodules and the-providersuffix from provider modules; expanded theSCabbreviation to the fullSelfCustodialword across code and tests; stripped review-numbering references from test names and comments. Relocateduse-sdk-lifecycleout ofproviders/intohooks/to match its responsibility.secureStoragesurface trimmed. Deprecated single-account mnemonic methods removed in favor of the per-account variants already in use; spec extended to cover biometrics, PIN, PIN-attempts, and session-profile paths.react-native-credentials-managerpatch and other helpers/hooks with no remaining production callers.../sibling); cross-module imports continue to use the@app/*alias so module boundaries stay visible at a glance.Out of scope
Test plan
stableBalanceEnabledremotely; confirm stable-balance settings appear / disappear and that toggling off mid-session re-aligns Receive to Dollar.activeAccountIdcleared).?icon next to the Dollar pill on Home: in custodial → Stablesats modal; in SC → stablecoins modal with "Learn more" linking toblink.sv/en/dollar-account.nonCustodialEnabledis off (with the "temporarily disabled" banner).blink-spark-backup-{network}-{pubkeyA}.json+{pubkeyB}.json) and neither overwrites the other.