#85 [Error Handling] connectAccount leaves wallet in partially-conn…

[felladaniel36-hash]

Description

Problem: useStellarWallet.connectAccount() called connect(key, type) BEFORE
stellar.getBalance(key). If the balance fetch failed (network outage), the error was
caught and displayed, but the store had already flipped isConnected: true — and the
wallet store is persisted to MMKV, so the phantom connected state survived app restarts.
RootNavigator gates the app on isConnected, so users landed in the main app with a
null/zero balance and no indication their wallet wasn't actually usable.

Two compounding defects made it worse:

1. getBalance/getTokenBalance swallowed ALL errors and returned '0', making a
   network outage indistinguishable from a legitimately unfunded (zero-balance) account.
2. saveInAppSecret() persisted the secret key to the vault before verification,
   orphaning secrets on failed connects.

Fix (fetch-then-connect + rollback, applied consistently to all wallet paths):

• connectAccount now verifies balances FIRST: beginConnect() → fetch XLM + ECO + USDC →
  only on full success, save secret + connect() with the verified balances.
• connect() in the store now accepts optional initialBalances, so the connected state
  and balances land atomically — a connected wallet can never render an unknown balance.
• On ANY fetch failure (full or partial), connectFailed(message) rolls every wallet
  field back, records connectError, and rethrows so the calling path surfaces the error.
  The user stays on Onboarding and sees a meaningful message instead of a zero-balance
  main screen.
• New store lifecycle: status: 'disconnected' | 'connecting' | 'connected' (intermediate
  "connecting" state until balances are verified) + connectError, both excluded from
  persistence via partialize (a crash mid-connect can't restore a stuck state).
• getBalance/getTokenBalance: NotFoundError (unfunded account) still resolves '0'
  — a zero balance remains a valid connected state; infrastructure/network errors now
  reject so the connect flow can detect them.
• Refresh functions (refreshBalance/refreshEcoBalance/refreshUsdcBalance) made
  reject-safe: a later network blip keeps the last known balance instead of rejecting
  into fire-and-forget call sites.
• OnboardingScreen also surfaces the store-level connectError (survives hook
  remounts); self-clearing on retry/success/disconnect.

Closes #NN

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that breaks existing functionality)
• Documentation update
• Chore (deps, tooling, refactoring)

Non-breaking: connect(publicKey, walletType?) keeps its existing signature (the new
initialBalances param is optional); all 335 pre-existing tests pass unmodified.

How Has This Been Tested?

• Unit tests added / updated
• Integration tests added / updated
• Manual testing on Android
• Manual testing on iOS

26 new unit tests across two new suites (all passing):

src/__tests__/useStellarWallet.test.tsx (19):

• happy path: balances verified BEFORE connected, stored atomically (in-app)
• unfunded account connects with '0' balance (zero is NOT treated as failure)
• intermediate "connecting" state while the fetch is in flight
• balance fetch failure → full rollback + connectError + hook error, verified for
  ALL FOUR paths: in-app create, Freighter, Lobstr, import
• partial failures (XLM ok / ECO fails; XLM ok / USDC fails) → still fully rolled back
• secret key NOT orphaned in the vault on failed connects
• later refresh failure keeps last known balance (no unhandled rejection)
• store lifecycle: beginConnect / connect / connectFailed / disconnect semantics

src/__tests__/stellarBalance.test.ts (7):

• service error contract: funded → balance; unfunded (NotFoundError) → '0';
  network failure → rejects (for both getBalance and getTokenBalance)

Full suite: 42 suites / 361 tests passing (baseline 40 / 335). Manual on-device
testing not performed (no emulator available in this environment) — reviewers are
encouraged to smoke-test the offline-connect scenario on a device.

Checklist

• My code follows the project's code style (eslint src/: 0 errors; repo-wide lint identical to pre-fix baseline)
• I have performed a self-review of my own code
• I have commented on complex or non-obvious code (fetch-then-connect ordering, rollback rationale, error contract on getBalance/getTokenBalance, partialize rationale)
• I have updated the documentation where needed (N/A — internal bug fix, no public API or user-facing docs changed)
• My changes generate no new warnings or lint errors (typecheck clean; lint error/warning counts unchanged from baseline)
• All existing and new tests pass (361/361)

Screenshots

N/A — no visual redesign. The only UI-visible change: on connect failure the user now
remains on the Onboarding screen and sees the error text (e.g. "Network request failed")
instead of being dropped into the main app with a zero/null balance.

Additional Context

Files changed (4 modified, 2 created; +215/−36):

File	Change
src/hooks/useStellarWallet.ts	connectAccount rewritten: fetch-then-connect, rollback, atomic connect; reject-safe refreshers; secret saved only after verification
src/store/walletStore.ts	status, connectError, beginConnect(), connectFailed(); connect() accepts verified initialBalances; partialize keeps transient fields out of MMKV
src/services/stellar.ts	getBalance/getTokenBalance distinguish unfunded accounts ('0') from network failures (reject)
src/screens/OnboardingScreen.tsx	surfaces store-level connectError
src/__tests__/useStellarWallet.test.tsx	new — 19 tests
src/__tests__/stellarBalance.test.ts	new — 7 tests

Design choice: implemented the issue's "Option A" (move connect() after the balance
fetch succeeds, with rollback) rather than "Option B" (error screen in RootNavigator),
because it satisfies the acceptance criteria at the root — a wallet that can't be verified
is simply never marked connected, so RootNavigator needed zero changes. The store-level
connectError from Option B is still recorded and surfaced on Onboarding for robustness.

Out of scope (per issue): offline wallet state, cached balance.

Out of scope note: wallets already persisted in a half-connected state by older builds are
not migrated — the fix prevents new bad state.

Closes #85