Skip to content

fix: show cached USDT balance on Home - #1700

Merged
ovitrif merged 4 commits into
masterfrom
fix/usdt-cached-home-balance
Oct 27, 2026
Merged

ovitrif merged 4 commits into
masterfrom
fix/usdt-cached-home-balance

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1698
Twin: synonymdev/bitkit-ios#1131

This PR shows the last known Home total while the USDT balance and exchange rates refresh, instead of temporarily hiding that total after launch.

Description

  • Persists the last known raw USDT balance for the current wallet and network/endpoints so Home can display it after local cache hydration.
  • Reuses the existing cached exchange rates for the Home total, including rates older than ten minutes, while keeping an “Updating balance…” indicator visible until balance and FX refresh finish.
  • Keeps cached display values separate from spendable balances and payment-rate freshness, preserves hide-balance behavior, and clears the snapshot on wipe/restore.

Out of Scope

  • Payment conversion, quote expiry and authorization retain their existing fresh-data requirements; this change only affects the Home display.
  • No backend, SDK dependency or shipped-data migration change.

Design

N/A — no design available for the updating caption. Existing Home typography and balance components are reused. The Android screen map identifies Home as “Wallet On-chain › Wallet BTC”; an exact current Figma frame for this addition is unconfirmed.

QA Notes

Setup

Use the committed manual recipe at the published revision. It requires a disposable wallet, a recorded wallet/network-scoped display snapshot, cached BTC/USD rates, and owned RPC/FX endpoints whose availability can be controlled. The normal journey harness cannot prepare aged caches or inject these network faults. Do not fund/reset an existing wallet or use another task’s fixtures. A seeded display record is not an on-chain balance. Standard dependency pins and endpoints are unchanged.

Journeys

N/A — not drivable; see Manual Tests.

Manual Tests

  • 1. Prepare and record the isolated wallet/rate/display snapshot as in recipe step 1 — aged cache preparation is outside the journey capabilities.
  • 2. Cold-launch with refresh unavailable → cached headline and tile amounts remain visible with the updating caption; a visible positive cache hides the empty-wallet funding prompt, and an absent snapshot stays unknown — endpoint fault injection required.
  • 3. Hide and reveal balances → both cached headline and tile remain private and restore consistently — prepared display snapshot required.
  • 4. Restore fresh balance and FX responses, including zero → values update and the caption clears after both refreshes complete — controlled provider/rate fixture required.
  • 5. Open a separate wallet/network install → no snapshot from the first wallet appears — passed October 27 on the prepared 7bc486f2 isolated fixture build, with explicit second-install consent and preserved first wallet. See evidence below.
  • 6. Attempt an unfunded payment conversion with only an expired rate → existing freshness checks still reject it — aged rate fixture required; do not submit payment.

Automated Checks

  • added UsdtRepoTest.kt — restart/offline display, live zero replacement, wallet isolation and wipe invalidation without making the cached value spendable.
  • added AppCacheDataTest.kt — exact maximum/zero balance serialization and compatibility with the existing cache shape.
  • added UsdtDisplayTest.kt — expired-rate Home display preserves strict payment freshness and the updating indicator waits for both USDT and FX.
  • updated CurrencyRepoTest.kt — the published refresh flag resets when the existing refresh is cancelled.
  • updated UsdtEndpointTestWallet.kt, UsdtPreferencesRepoTest.kt — supply the repository’s cache dependency in existing tests.

Evidence

At 7bc486f2, optional display-cache persistence failure no longer prevents credential/native-wallet cleanup or restored-state publication. Genuine wallet/restore errors and cancellation retain their existing behavior. Both new regressions fail against unchanged e179afd7 production and pass after the fix. Final required just compile, just test and just lint passed: 4,603 tests / 253 suites, no failures/errors/skips; complete Detekt and Android Lint reports have zero findings, including formatting. The intermediate test-only Mockito stub error was corrected before the full gate; no assertion was weakened. Independent frozen-diff review passed after that correction. All two PR commits against actual master eb902f52 verify locally and on GitHub. These tests cover the PR tree, not an executed merge with that newer master. Hosted checks have completed as described below; the manual cases remain outstanding.

At 7bc486f2, E2E workflow40371765581 completed on workflow attempt2. The onboarding group initially failed a Docker port bind before tests; one targeted retry ran its seven cases successfully. The six other groups retained their original executions. Across all groups,39unique cases passed and settings08 remained skipped. Three initial failures (boost2 HeaderMenu, settings11 ConnectToHost, send2 TotalBalance/MoneyText) passed their built-in second steps:42non-skipped executions in total. Full logs were inspected; no cause or actual-base reproduction is established for those three failures. Only the APK artifact is available because runtime visual uploads were skipped after recovery. No hardware coverage or completed cache/provider manual journey is implied.

At e179afd7, based on 3ad6193b: final required just compile, just test, just lint and assembleDevDebug passed. Complete JUnit reports contain 4,601 tests across 253 suites, zero failures/errors/skips. Detekt SARIF has zero findings; Android Lint XML/SARIF/text all report zero issues. Final focused selection: 37 passes. Private Gradle cache, bounded workers, unchanged dependency pins. Complete reports are saved in /private/tmp/usdt-startup-cache-20261026/validation-report.md and android-final-reports/.

Actual master advanced to 2ea4cf56 when #1565 merged. A clean merge-tree preserves its USDT overlay input handling alongside this Home-cell cache fallback. These local results cover the candidate tree atop 3ad6193b; they do not claim execution of the combined 2ea4cf56 tree.

Independent review found one FX-refresh indicator gap, fixed with regressions on both platforms; the bounded final recheck found no remaining material issue. No tests were weakened or removed.

Gaps

Manual 1–4 and 6 passed on the isolated Android emulator fixture build described below. Manual 5 subsequently passed on October 27 after explicit consent for the second disposable install; its evidence and original-head attribution are recorded below. This is fixture-build evidence, not an exact shipped-binary or production-provider run. Physical devices, funded provider, hardware and settlement remain unverified. The iOS twin has separately completed all six simulator cache cases.

One Gradle Detekt-plugin deprecation and JVM class-sharing notices remain. Unit output contains four MatrixCursor CloseGuard traces from the existing SharedPubkyProvider tests; the provider/test and build/plugin inputs are byte-identical to actual master. No actual-base runtime reproduction or blanket warning-free claim. Full diagnostics are retained.

Preview

Local emulator screenshots are retained in /private/tmp/usdt-cache-qa-20261026-2218; a hosted preview is not attached.

Models used

  • Planning/scoping: gpt-6-astra (reasoning: xhigh)
  • Implementation: gpt-6-astra (reasoning: xhigh)
  • Review: gpt-6-astra (reasoning: xhigh)
  • Review rounds: 5

Controlled cache manual validation — 2026-10-26, 7bc486f2

Manual 1–4 and 6 passed on the owned disposable Android emulator emulator-5580. The isolated export keeps the published production logic and dependencies, with unpublished RPC/bundler/FX fixture configuration and a narrow instrumentation helper that edits only the display snapshot, cached rates and USDT preference. The first installed APK hash was verified. No existing wallet was reset; this new wallet remained unfunded. A seeded display record and synthetic read-only RPC balances are not on-chain funds.

A scoped 12.34 USDT record and BTC/USD 100000 rate aged one hour showed 12,340 sats / $12.34 and the updating caption during the outage; an absent snapshot remained unknown. Hide/reveal masked and restored both headline and tile. Fresh fixture 23.45 USDT at BTC/USD 200000 produced 11,725 sats / $23.45 and cleared the caption. A fresh zero response with FX unavailable showed 0 / $0.00 without the caption. After the last served rate aged beyond ten minutes, switching an unfunded 1,000 sat amount to USDT displayed “Payment rates are unavailable or expired” and preserved the Bitcoin amount and Savings source. No quote, confirmation, authorization, payment, request or subscription was submitted.

The owned fixture has no upstream or mutation methods. The pinned Core SDK rejects cleartext HTTP on 10.0.2.2 for RPC, so the QA build uses supported 127.0.0.1 endpoints through reverse mappings scoped to this emulator; FX uses its own 10.0.2.2 endpoint. Initial inspector contention and rapid text-entry truncation were corrected before verified observations. Full screenshots/XML, selected-field provenance, request logs and build reports are retained at /private/tmp/usdt-cache-qa-20261026-2218. Complete primary and second-package builds passed; one structured Detekt-plugin Gradle deprecation remains. The second package required a local Google Services package-name alias; it is unpublished and leaves the first app installed and unchanged.

Manual 5 completed October 27: the prepared second package to.bitkit.cacheqa2.dev and first package to.bitkit.dev have distinct Android storage and authorities. Explicit Terms/Privacy consent was received and used only for the second disposable install. Its new public wallet address differs from the first. The narrow display helper seeded the first wallet's address/network-scoped 12.34 USDT record and one-hour-old BTC/USD rate; cold and settled Home showed unknown headline/tile values with Updating balance, never 12.34 USDT, 12,340 sats or $12.34. Host test ports were reserved without listeners, device RPC ports had no listeners and no reverse maps existed, so a fresh response could not mask rejection. The first package was not launched or modified during this case; both APK hashes matched this run's start/end and wallet data were preserved. No payment, funding, quote, authorization, request or subscription occurred.

This second APK is the prepared 7bc486f2 fixture export (SHA-256 06d56e3579529ff47839b5f90493685dcb33f141ce8c28eb393aa2fae0b9cd72), not the current shipped 4d7d05c5 binary or a production-provider run. The initial helper attempt crashed before any test/display write because BOOT_COMPLETED reached HiltTestApplication before its test component existed; reconciliation confirmed both output files absent, and one bounded retry passed the single helper test. Argent's tap service could not attach to the private AVD, so scoped adb UI fallback was used. Complete screenshots/XML, public provenance, failure/recovery and stop receipts: /private/tmp/usdt-monitor-20261027-0430/android-case5. Owned AVD5580 stopped cleanly with data intact. No source commit, CI retry or additional review request accompanies this evidence update.

Cached-positive empty-state repair — 2026-10-27, 4d7d05c5

A positive visible cached USDT balance now suppresses Home’s empty-wallet funding prompt when the live balance is unavailable. A fresh zero still overrides the cache, and a disabled USDT wallet’s cache does not affect the empty state. This matches the iOS #1131 review repair; Android already tracks balance freshness independently of later history errors.

Final required compile, full unit suite and full lint passed: 4,603 tests, zero failures/errors/skips; Detekt SARIF and Android Lint XML/SARIF contain zero findings. The existing HomeViewModel case now covers cached positive, fresh zero and disabled-wallet behavior. Its corrected enabled-wallet assertion failed against original production and passed after the change. An earlier test setup incorrectly left USDT disabled; that failure was preserved, corrected, and followed by a complete final rerun. The one structured Detekt-plugin Gradle deprecation remains; build/plugin inputs match actual master ea89ed18. Independent frozen-diff review found no material issue.

Controlled manual repair checks passed on the existing owned disposable emulator: parent 7bc486f2 showed the funding prompt alongside a positive cache; the final fixture build hides it. Fresh zero restores the prompt and clears the caption. Successful balance plus failed history clears the caption; a subsequent failed balance refresh retains the prior value with the caption. No payment, quote, authorization or funding occurred. These are isolated URL/config fixture builds, with installed APK hashes and complete read-only request logs retained; they are not shipped-binary or production-provider validation. Earlier full manual cases retain their 7bc486f2 attribution; case 5 subsequently passed at that same prepared fixture revision as described above.

Signed normal commit 4d7d05c5d289e603f8242384436068d6bf24b30f, tree 9b074acb3391342328450891c2a8cd1f09cfaaec. All three PR commits against actual master ea89ed18923533ebf1c0d5b9b321da3450048f8b verify locally and match GitHub’s exact commit set with verified=true/reason=valid. Local tests cover this PR tree, not the newer combined base tree. New-head hosted CI is pending. Full evidence: /private/tmp/usdt-monitor-20261027-0121/validation-disposition.md and /private/tmp/usdt-home-display-qa-20261027-0121/manual-repair-report.md.

Hosted build/lint completion — 4d7d05c5

CI40436412866 and Lint40436412721 succeeded on attempt 1. Both tested merge fc0134ec with parents actual master ea89ed18 / 4d7d05c5, tree 57cecfc5. Full report artifacts and SHA-256 digests were verified: 4,612 tests passed, zero failures/skips, and Detekt/Android Lint SARIF/XML have zero findings. The earlier 4,603 local tests cover the PR tree separately.

Full logs retain the Gradle deprecation, three JVM class-sharing warnings, a manifest removal-marker notice and four MatrixCursor CloseGuard traces from SharedPubkyProvider tests. Manifest/provider/test/plugin inputs match actual base; no actual-base runtime reproduction or warning-free claim. E2E40436413180 remains attempt 1 in progress after a successful build. No manual retry; all device/provider/funded/hardware limits remain; case 5 was subsequently completed as described above.

Hosted E2E completion — 4d7d05c5, 2026-10-27

Workflow40436413180 succeeded on workflow attempt1. 39 unique cases passed in 39 executions; one existing settings case skipped; zero failures. All seven groups passed their first built-in test step; second/third steps skipped. No manual retry. settings_08 (switch address types) is explicitly skipped in the pinned E2E source and is not counted as a pass.

The app build checked out merge fc0134ec0076ac8e0fff190ebe656aa01baa009d (actual base ea89ed18 / head 4d7d05c5, tree 57cecfc57b028b746a6c34e420f7d557510db304). Every test group used E2E revision ec01c02ff611797c112e65b5c87c936d25720bac, Pixel6/API33/x86_64. All11 job logs and available diagnostics were inspected. Only APK artifact12703723542 was uploaded; its recorded SHA256 matches all seven runner download-calculation logs. No runtime, screenshot/video or structured E2E result artifact was available; the APK was not downloaded locally again.

Retained diagnostics include69 WebDriver stale-element notices and one optional permission-dialog lookup timeout caught by the existing helper, plus emulator/adb/X11/ColorBuffer/teardown, Node/npm and Gradle deprecation notices. No test retry followed the helper timeout. No blanket warning-free or actual-base runtime reproduction claim. The previous hosted build/unit/lint evidence and original-head retries remain separately attributed.

These local Bitcoin/Lightning/LNURL, backup, settings and widget cases do not complete USDT provider/funded/hardware acceptance or independently prove the second-install cache-isolation case; that separate controlled manual result is recorded above. No source, assertion, reviewer state or acceptance checkbox was changed by this CI update. Full report: /private/tmp/usdt-monitor-20261027-0235/android-e2e/completion-report.md.

Currency refresh concurrency repair — 2026-10-27, e42492c

The cached-rate collector now copies rate and display-preference fields inside the atomic CurrencyState update. A concurrent successful refresh can no longer have its cleared isRefreshing flag or success timestamp overwritten by an older snapshot, leaving Home's updating caption visible for another polling cycle. Payment-rate freshness, amounts, polling frequency and wallet/network cache matching are unchanged. The iOS twin uses main-actor field updates and has no equivalent whole-state overwrite.

A controlled concurrent interleaving in CurrencyRepoTest.kt failed on 4d7d05c5 and passed with the repair. One regression case was added, none removed or weakened. Final local validation: required just compile, full just test 4,604 passed / zero failures or skips, and corrected focused suite 8 passed. Full just lint Detekt/Android Lint reports: zero findings. The first lint pass identified three introduced test-formatting violations; those were fixed and the affected checks rerun. One independent frozen-diff correctness/quality review plus its bounded formatting/base-change recheck found no material issue.

All complete diagnostics were inspected. Four MatrixCursor CloseGuard traces originate in SharedPubkyProvider tests whose source matches actual base aa8607f0; their delayed recording under another test suite does not make them CurrencyRepo failures. No fresh actual-base runtime reproduction or warning-free claim. JVM class-sharing and Gradle plugin deprecation notices remain documented. Existing manual case 4 specifies that the caption clears after current balance/rate refresh, but previous device results retain their original fixture-build attribution; this concurrent repair was verified with controlled repository tests, not a new shipped-binary/device/provider/funded/hardware journey.

Signed normal additive commit e42492c. Full actual-base PR range verification: all four commits verify locally and match the exact GitHub commit set with verification.verified=true and reason=valid. Actual master advanced from ea89ed18 to aa8607f0 through unrelated hardware artwork changes; no overlap with this repair. Local validation covers this PR tree, not the newer combined merge tree. New-head hosted CI is pending. Full evidence: /private/tmp/usdt-monitor-20261027-0843/1700-review-repair/.

Hosted validation for e42492c6: build/unit 40482787966 and lint 40482787905 passed on workflow attempt 1. 4,613 unit tests passed, zero failed/skipped, including all eight CurrencyRepo cases. Artifact digests, all 253 class summaries, report output and all job logs were inspected; Detekt SARIF and Android Lint SARIF/XML contain zero findings. Both used verified merge 97ff2f5f4b01039cf348fb0c919cb0b193f536e6, parents actual master aa8607f0 / e42492c6, tree bf9c611b201fb20b1355920a585bdee2ee161d75, which differs from the locally tested PR tree.

Manifest provider-removal, JVM class-sharing and Gradle deprecation notices remain. The unit report also retains four MatrixCursor CloseGuard traces from SharedPubkyProviderTest; those provider/test files and the 16 associated tool/manifest/workflow inputs match actual aa86, but no fresh base-runtime reproduction or warning-free claim is made. The hosted logs do not include a detailed Gradle problem report; local full diagnostics are retained separately. E2E remains pending with six groups passed and Send still running at the latest read. No manual CI retry. Earlier controlled cache/device evidence retains its original fixture revision; production-provider, physical-hardware and funded-settlement acceptance is not implied.

E2E completion at e42492c

E2E 40482788410 completed on workflow attempt 1 on merge 97ff2f5f (actual aa8607f0 + e42492c6, tree bf9c611b). The seven Pixel 6/API33 groups produced 39 unique passes across 41 executions, including two failures of send_1 before its third built-in attempt passed. The failures waited 30 seconds for InsufficientSavingsToast on the 200,000-sat URI and then for AddressContinue on the 10,000-sat URI. Root cause is unproven; no manual retry, actual-base reproduction or permanent-fix claim. Six other groups passed their first step. One existing settings_08 skip remains; subsequent skips of the two already-passed Send cases are retry filtering.

All 11 logs, case summaries, pinned harness source, diagnostic groups and artifact metadata were inspected. The APK digest matches all seven runner downloads. Failure visuals/app logs were saved on the runners but not uploaded after eventual success, so those unavailable artifacts were not inspected. Retained 72 stale-element notices, input-helper retries, emulator/tool/dependency-audit notices and the separate unit/lint diagnostics above. Tested Send validation/view/workflow files match actual base; that is not proof of an independently reproduced base failure. Provider/funded/hardware/two-wallet acceptance limits remain.

Reuse persisted exchange rates for display while refreshing the wallet-scoped balance. Keep payment freshness and spendable state separate.

Fixes #1698
@greptile-apps

greptile-apps Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The change reaches the USDT balance display on Home and its wallet tile.

Fix the reset path before merging so a display-cache error cannot skip credential and USDT cleanup.

Findings

  1. P1 Cache errors leave reset incomplete ▶

Summary

Keeps the last known USDT amount available for Home while balances and exchange rates refresh.

  • Home keeps the last known USDT amount visible while balances refresh.

ben-kaufman explicitly acknowledged that manual tests and a device preview have not been completed.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Stored USDT snapshot] --> B[Match wallet and endpoints]
  B --> C[Home display fallback]
  D[Cached exchange rates] --> C
  E[Live balance refresh] --> F[Current balance]
  F --> C
  F --> G[Save display snapshot]
  H[Balance and rate refresh state] --> I[Updating caption]
Loading

Reviews (1) · Last reviewed commit: "fix: show cached USDT balance on Home" · Reviewed by Greptile

Comment thread app/src/main/java/to/bitkit/repositories/UsdtRepo.kt Outdated
@github-actions

github-actions Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from e42492c (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

@talosmachina talosmachina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No findings. Shows the last known USDT balance (persisted per address and endpoint scope in CacheStore) and the cached rate in the Home total while the balance and FX refresh, with an "Updating balance" caption, instead of —. Reviewed e179afd7, full tier, reasoned from the code and CI: on the Bitkit track I do not build the branch here.

What I checked, and 5 candidates I ruled out

Read in full: UsdtRepo.kt, HomeWalletBalance.kt, the WalletPage hunk of HomeScreen.kt, CurrencyRepo.kt refresh path, PaykitAmount.kt, usdtDisplaySats in UsdtAmountHeader.kt, UsdtPreferencesRepo.visible
Call sites traced: usdtHeadlineSats, usdtBalanceUpdating, cachedBalance readers (Home headline and USDT tile only)
CI: build, build-local, lint and detekt green; e2e-tests-local still running when posted

Ruled out

  • Stale rate leaking into payments: the bypass is local to usdtHeadlineSats, which passes now = rate.timestampMillis to usdtDisplaySats; PaykitExchangeRate.value(now) and every payment conversion are untouched.
  • Snapshot shown for another wallet or network: balanceFor matches both the address and a hash of Env.network plus the RPC and bundler URLs; wipe and restore clear the record before the state reset.
  • Cached value becoming spendable or deciding visibility: cachedBalance is a separate field; balance stays null until wallet.balance() returns, so hasFundsOrPendingTransfers and the send paths read the live value only.
  • refreshing stuck true: set inside the mutex and cleared in a finally, so cancellation of the Home effect resets it.
  • Caption flicker on every 30 s Home refresh: balanceIsStale and refreshing turn true on each refresh(), so the caption shows during each poll. That matches the issue's "while the balance or rate refreshes" and the iOS twin pins the same rule in a test, so it reads as intended.

Merge confidence: 4/5, no findings, but the e2e jobs were still running and the branch was reviewed without a local build.

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA review

Scope: Full review of the PR diff from merge base 3ad6193b through 7bc486f2. That merge base is the common ancestor of GitHub base 2ea4cf56 and this head. No saved baseline was available, so this pass covers the whole range.

No new actionable code findings.

Home shows the live USDT balance when it exists, and otherwise the last snapshot for the same receive address and network/endpoint hash. Cached BTC/USD rates older than ten minutes can fill that Home total; payment conversion still requires a fresh rate. With USDT visible, a missing snapshot or unusable rate leaves the combined total unknown. “Updating balance…” stays until the live balance and a current rate are both available. A confirmed zero balance clears the caption without a fresh FX rate. Hide balance still covers the headline and the USDT tile. Quotes, send limits, and disable checks read the live balance only.

Wipe and restore clear the snapshot, and a cache write failure leaves credential and wallet cleanup running. The earlier comment on e179afd7 does not match this head.

Home behavior matches bitkit-ios#1131 at d07c7e90 for the headline, tile, expired-rate display, and updating caption. This was a comparison of that contract only. On both pins the USDT wallet screen still shows an em dash until a live balance arrives; manual case 2 checks the Home tile.

Validation: the new unit tests were inspected here and not re-run. CI build ran testDevDebugUnitTest on this head and succeeded. Lint succeeded. E2E succeeded; the cached-balance recipe is outside that run.

Device testing: not performed in this review.

Ready for device testing. Manual cases 1–6 still need the disposable snapshot and controllable RPC and FX endpoints in the PR recipe.

Reviewer model: grok-4.7
Reasoning effort: high

Copy link
Copy Markdown
Contributor Author

Published the matching Home empty-state fix in signed commit 4d7d05c: visible positive cached USDT now hides the funding prompt, while fresh zero and a disabled wallet still behave correctly. This addresses the parity issue found in the iOS twin review.

Final compile, all 4,603 unit tests and full Detekt/Android Lint passed, with complete reports inspected and zero lint findings. The corrected enabled-wallet regression fails on the original production expression. Controlled unfunded emulator checks reproduced the parent bug and verified the fix, fresh zero, and balance/history error-caption behavior. The fixture-build attribution, retained Gradle deprecation and remaining second-install/provider/hardware gaps are in the updated body. All three PR commits match the locally verified range and have valid GitHub signatures. New-head hosted CI is pending; no CI retry or review dismissal.

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA review

Scope: Follow-up at 4d7d05c5, covering the changes since 7bc486f2. Inherited baseline: QA review. This pass inspected the Home funding prompt for a positive cached USDT balance, including a confirmed zero and a disabled USDT preference, and the updated recipe’s history-failure and failed-balance caption behavior. Headline conversion, the updating caption, the USDT tile, snapshot scope, wipe/restore, live-balance spend paths, and the unit tests for those paths remain covered by that baseline. Empty-state and balance-freshness behavior was compared with bitkit-ios#1131 at 961ddaf5; the headline and USDT tile cache fallback there are unchanged since d07c7e90.

No new actionable code findings.

While USDT is visible, a positive cached balance keeps the Home funding prompt hidden. A live zero balance restores the prompt and leaves the Bitcoin total in the headline. Disabling USDT leaves the prompt visible when the cache is the only balance. The headline, tile, and prompt all prefer the live balance. A failed balance read keeps the updating caption; after a successful balance and rate refresh, a history failure still clears it. The same empty-state and freshness rules are in the iOS Home view at the compared revision. Snapshot clearing remains best-effort, so credential and wallet cleanup still run if that cache write fails.

Validation: the new HomeViewModelTest cases and the existing refresh and caption tests were inspected and were not re-run. The unit build and lint succeeded for this head. E2E does not exercise the cached-balance recipe.

Device testing: not performed in this review.

Ready for device testing. Recipe steps 2 and 4 now also cover the funding prompt and the history and balance caption.

Reviewer model: grok-4.7
Reasoning effort: high

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code scan at 4d7d05c: one LOW inline (the "Updating balance…" caption can stick for one FX cycle). No HIGH or MEDIUM.

Checked and clean:

  • Send and amount gating don't read the cache. cachedBalance feeds only the Home headline, the Home USDT tile (UsdtWalletBalance, called from HomeScreen.kt:834) and the empty-state check (HomeViewModel.kt:139). hasFundsOrPendingTransfers, hasProtectedFunds, quotes, send limits and the disable check read the live balance, and payment FX conversion still uses the current time.
  • The snapshot can't show across wallets or networks: it is keyed on the receive address (mnemonic + passphrase) and a hash of network, RPC and bundler. Wipe clears it under the repo mutex, WipeWalletUseCase resets the store, and a USDT backup restore clears it after METADATA restores.
  • The snapshot rides in the METADATA backup, so each balance change marks a METADATA upload. cachedRates already does that every 2 minutes, so the added load is marginal.
  • Twin synonymdev/bitkit-ios#1131 follows the same rules and has no equivalent of the inline LOW. Android renders wallet__balance_updating uppercase (Caption13Up) and iOS doesn't; a design call, not a finding.

Performance-relevant only marginally (one cache read and write per balance change, two extra CurrencyState updates per FX poll); nothing scales with data.

Device gate (partial): 4d7d05c — not run yet; pending journeys/usdt-cached-home-balance.md with a seeded usdtDisplayBalance in app_cache.json and airplane mode, and the l10n check of wallet__balance_updating in ca/de. Device results will follow in a separate review.

Comment thread app/src/main/java/to/bitkit/repositories/CurrencyRepo.kt
@ben-kaufman
ben-kaufman requested a review from jvsena42 October 27, 2026 09:43

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA review

Scope: Follow-up at e42492c6, re-reviewing the changes since 4d7d05c5 together with the affected FX refresh and Home caption paths and earlier review concerns. Inherited baseline: QA review. The headline conversion, USDT tile, funding prompt, snapshot scope, wipe/restore, live-balance spend paths and their unit tests are unchanged since that baseline and stay covered by it. The merge base is unchanged (3ad6193), and the previous head is an ancestor of this one.

No new actionable code findings.

No new actionable code findings in the follow-up.

When cached rates or preferences change, CurrencyRepo.collectCachedData now updates only its own fields inside the atomic CurrencyState update. It no longer writes back a copy taken before a refresh finished. That fixes the stale isRefreshing overwrite that could keep "Updating balance…" on Home after the balance and rate had loaded. Every other CurrencyState writer also changes only its own fields, so a finished refresh keeps its cleared flag and its success timestamp. The new CurrencyRepoTest case pauses the collector between its read and its write while a real refresh finishes. Under the old read-then-overwrite code it would fail; with the fix it passes. The iOS twin at b4083cd7 only changed tests since the compared revision. Its CurrencyViewModel.isRefreshing is a main-actor @Published flag reset in defer, so a whole-state overwrite like this one can't happen there, and caption parity holds.

Validation: I inspected the new test but did not run it here (no Android SDK). The CI unit test step (testDevDebugUnitTest) and lint passed for this head.

Device testing: not performed in this review.

Ready for device testing. The cached-balance recipe covers the caption clearing once the balance and rates have refreshed.

Reviewer model: claude-opus-5-5
Reasoning effort: high

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at e42492c6.

Built and installed this exact revision on Pixel 6 emulator, using the existing disposable regtest wallet and local USDT Arbitrum fork. Wallet and fork state were preserved.

Manual testing covered online/cold launch, cached Home and USDT tile balances during a controlled USDT outage, hide/reveal, reconnect recovery, transfers in both directions between Android and iOS, and caption clipping in German/Catalan with large text. The non-numeric updating caption remains visible while amounts are hidden.

Runtime log inspection found no app crash or Android ANR in the collected session. Both apps recovered after connectivity returned: the updating caption cleared and the final displayed amounts matched the local fixture-chain balances. Price-chart feed failures, stale Lightning gossip data and startup diagnostics remain recorded; no blocking defect in these Home-cache changes was identified.

Coverage limits: expired payment FX, fresh-zero replacement without FX, foreign-wallet snapshot rejection and independent balance-success/history-failure conditions were not exercised in this session. This is scoped emulator validation, not a full device pass or production-provider/physical-device acceptance.

Final Android fixture balance: 11.222109 USDT, consistent with the rounded Home/tile display.

@ovitrif
ovitrif merged commit 7ce91e8 into master Oct 27, 2026
21 checks passed
@ovitrif

ovitrif commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

@ben-kaufman Merged, as it was approved and green.

@ovitrif
ovitrif deleted the fix/usdt-cached-home-balance branch October 27, 2026 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: home total balance shows — until the usdt rate refreshes

5 participants