Repository navigation
fix: unify usdt balance and amount presentation - #1655
Conversation
 [Medium impact] This PR appears safe to merge; no actionable issues were found. SummaryThis PR gives USD and USDT amounts consistent dot decimals and comma grouping.
No actionable issues were found. Tests and device checks were not rerun during this review. ben-kaufman explicitly describes the removed refresh messages and zero-before-refresh display as intentional presentation changes. Reviews (1) · Last reviewed commit: "fix: allow entering new USD request amou..." · Reviewed by Greptile |
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
1 finding (1 P1). Silences the USDT wallet's error and history-sync copy, starts the balance at $ 0.00, and moves USD/USDT display and keypad entry to the fixed . decimal / , grouping already used for bitcoin fiat values. Reviewed 9bb64a38, full tier.
What I checked, and 4 candidates I ruled out
Read in full: UsdtAmountHeader.kt, PaykitAmountView.kt, UsdtWalletScreen.kt, the changed hunks of CreatePaymentRequestScreen.kt, SendAmountScreen.kt, UsdtReceiveAmountContent.kt, UsdtSendContent.kt, UsdtDepositReceiveScreen.kt, Currency.kt, plus UsdtRepo.kt and HomeScreen.kt for the balance state.
Call sites traced: usdtOverviewAmount, usdtExactAmount, usdDisplayAmount, usdCents, formatCurrency, every NumberPad(type = DECIMAL) caller.
CI: build, detekt, Greptile and the finished e2e shards green; lightning_security still pending at review time. Not built here: Bitkit Android branches are reviewed from code and CI only.
Ruled out
- Parsing after the keypad change:
UsdtRepo.quoteandUsdtService.prepareDepositstill replace the locale decimal separator with., but the keypad now always emitsKEY_DECIMAL = "."andNumberPadInputHandlernever inserts a comma, so the replace is a no-op, not a mis-parse. formatCurrencydefault change: the newminimumFractionDigits/roundingModeparameters default to the previous behaviour (decimalPlaces,HALF_EVEN), so existing bitcoin fiat callers are unchanged.checkNotNullonformatCurrency:DecimalFormat.format(BigDecimal)does not throw for any finite value, includingULong.MAX_VALUEcents, which the updated test covers.- Zero-USD request draft: an empty
dollarsrenders as0viaifEmpty, and the submitted amount for a re-entered value is covered byPaymentRequestAmountLocaleTest. - Cold reader: returned no verdict (provider refusal), so the P1 stands on the trace above.
Merge confidence: 3/5, the P1 on the unknown-balance path.
Reviewer model: claude-opus-5-5
Reasoning effort: Not exposed (the review runtime reports the model id only)
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — a funded wallet shows a false zero after an offline cold start.
Findings:
- MEDIUM — unavailable balance displayed as zero:
app/src/main/java/to/bitkit/ui/screens/wallets/usdt/UsdtWalletScreen.kt:106and the wallet header at line 345 convert an unloaded balance to zero. I reproduced a wallet holding1,234.567891USDT showing$ 0.00after an offline restart, beside its saved funded activity. It returns to1,234.57after RPC connectivity returns; no funds moved. This adds device evidence to the existing finding. The author confirmation leaves the design decision open.
QA for 9bb64a3:
Android 15, English/US, built from this head. USDT transfers below used a local Arbitrum fork; hardware checks used deterministic Trezor fixtures.
Test 1: J1 — US-region number-format repeat ✅ Empty balances, masking, dot entry and exact QR amounts matched.
Home uses dollar symbols and dot decimals; the empty USDT wallet stays silent. Receive preserves 3.50 and 0.000001; I decoded the QR images to uint256=3500000 and uint256=1 and matched their recipient to Details. Manual send entry displays 1.25.
test-1.2x.mp4
US-region presentation, receive precision and manual entry, at 2× speed.
Test 2: J2 — navigation and fresh receive state ✅ Back, copied addresses, validation and receive-state isolation matched.
Details Back returns to the QR; QR Back closes Receive on the USDT wallet. Details Copy, QR Copy and tapping the QR all paste only the address. Invalid Bitcoin text is rejected; a seventh USDT decimal is ignored. A fresh USDT receive sheet clears the previous Bitcoin amount/note when switching to Savings; unfunded Spending shows its receive setup.
test-2.2x.mp4
Navigation, clipboard round trips, decimal limit and fresh receive state, at 2× speed.
Test 3: J2 — odd/even hardware rows ✅ USDT placement, aligned dividers and hardware receiving matched.
With one paired hardware wallet, USDT fills its row’s right cell. With two, USDT starts the next row. Hardware receiving retains a separate Bitcoin QR.
test-3.2x.mp4
One and two paired hardware accounts, then hardware receiving, at 2× speed.
Test 4: J2 — saved activity and search ✅ Sub-cent notation, cent rounding and search matched.
Local-fork receipts display <0.01 and 1,234.57. USDT and usdt find both; u, us and t find neither. A transaction fragment finds one row, a recipient fragment finds both, and clearing restores the list. Search inputs and row identifiers were checked through the device UI hierarchy.
test-4.2x.mp4
Local-fork receipts and activity searches, at 2× speed.
Test 5: funded wallet offline cold start ❌ The unchanged funded balance becomes zero after restarting offline.
The wallet holds 1234567891 atomic units (1,234.567891 USDT). Losing RPC in the running process keeps 1,234.57; force-stop and offline relaunch show $ 0.00 on Home and 0.00 in the wallet while both saved receipts remain. Restoring RPC recovers 1,234.57. The fixture reported the same atomic balance before and after.
test-5.2x.mp4
Online balance, cached offline balance, offline restart and restored balance, at 2× speed.
I relied on the author’s reported German-region walkthrough and final-head automated checks for unchanged regional coverage and zero-USD request initialization; I did not rerun those suites. No planned case remains unrun. Funded payment execution, bridge/deposit screens and provider recovery beyond this display check were not driven.
Full review: ae800ae..9bb64a3, including formatter/input callers, balance state, masking and payment-action error paths.
Twin comparison:
- bitkit-ios#1077 fix: unify usdt balance and amount presentation matches the formatter, keypad, quiet startup and zero-fallback changes in code; I did not run iOS.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
There was a problem hiding this comment.
Verdict: ✅ Approve — the unknown-balance finding is fixed.
Findings: None. I verified the previous finding at 95dc2fe, including the funded offline cold start that failed previously.
QA for 95dc2fe:
Android 15, built from this head. English/Germany and English/US; funded checks used a local Arbitrum fork.
Test 1: J1/J2 — unknown and confirmed-zero balances ✅ Unknown, confirmed zero and masked balances remain distinct.
In both regions, an offline cold launch shows — on Home and $ — in the wallet. A successful empty balance read changes both to $ 0.00. Masking hides the Home tile and wallet balance. No refresh-error or history-sync paragraphs appear.
test-1.2x.mp4
Unknown, zero and hidden balance states, at 2× speed.
Test 2: J1 — regional amount entry and merged layout ✅ Dot entry and exact QR amounts match across regions; compact controls remain reachable.
Receive preserves 3.50 and 0.000001 in both regions. I decoded the QR screenshots to uint256=3500000 and uint256=1; the recipient matches receiving Details. A seventh decimal is ignored. Manual USDT send displays 1.25 with a dot key in both regions. At a shorter screen height with 30% larger text, scrolling exposes the keypad and Continue. No quote or payment was submitted.
test-2-complete.2x.mp4
US entry, German entry and compact layout, at 2× speed.
Test 3: J3 — funded wallet offline cold start ✅ Offline relaunch shows unknown beside saved activity; reconnect restores the balance.
The wallet shows 1,234.57 online and retains its loaded value when RPC becomes unavailable. After an offline cold restart, Home shows — and the wallet shows $ — beside its saved 1,234.57 receipt. Reconnecting restores 1,234.57. The fixture reported the same 1234567891 atomic units before and after. A separate failed refresh also retained the loaded balance; I confirmed NetworkUnavailable in the current process's device log.
test-3-complete.2x.mp4
Funded balance, offline restart, recovery and failed-refresh retention, at 2× speed.
No planned case remains unrun. I relied on the earlier passing report for unchanged J2 receive navigation, hardware-row placement and activity search. I relied on the author's current-head automated results for unit and Compose coverage; I did not rerun those suites.
Delta review: 9bb64a3..95dc2fe, including the prior finding and master-merge interactions. The merged Paykit funding selector, fee estimate, settings migration and transaction-ID copying were checked in code, without new device cases. App payment execution and bridge/deposit flows were not driven.
Twin comparison:
- bitkit-ios#1077 fix: unify usdt balance and amount presentation matches the unknown/zero distinction, masking, fixed number format and silent synchronization in code; I did not run iOS.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
No findings.
Checked and clean:
- The sent value is the full-precision amount; only the display is formatted. Unknown (
—/$ —) and confirmed zero ($ 0.00) stay distinct on Home, the wallet screen and the masked state. - The decimal key and display use a dot in comma regions, and entry parses back to the same 6-decimal value the QR and send use.
usdt__history_syncis removed with its only users, and no keys are added.- The review screen rounds the approved amount and fee to cents. That is already on master and is the accepted behaviour in the USDT QA doc, so it is not raised here.
- Twin synonymdev/bitkit-ios#1077 matches on every traced point.
Device gate: no run by me. Skipped, already driven this round: usdt-number-format.xml, usdt-wallet-navigation.xml and the offline cold start (Android, by ovi-reviewer at 95dc2fe, passed: #1655 (review)). perf: not relevant; l10n: one key removed, none added; Figma: the mapped Wallet USDT and Receive USDT › Specify Amount rows have no node link, and the — unknown state was settled in the threads.
|
Merged: approved and green at 95dc2fe. |
Fixes #1648
Fixes #1649
Twin: synonymdev/bitkit-ios#1077
Description
USDT balances show
—until a successful balance read, then$ 0.00for a confirmed empty wallet. Balance and activity synchronization stays silent. The wallet overview no longer displays refresh-error or history-sync paragraphs; previously loaded balances and payment-action errors retain their existing behavior.USD/USDT values and keypads now use the same dot decimal/comma grouping convention as Bitcoin fiat values, and the USDT Home tile includes
$. Shared fiat formatting preserves exact token precision, sub-cent balances, rounding and hidden balances. Zero-dollar payment-request drafts start with an empty editable value so the keypad can accept a new amount.Out of Scope
Design
Follows Aldert’s clarification recorded in the linked issues. Relevant mapped frames are
Savings, Spending, Hardware, USDT Balances › Wallet USDT (empty / funds & activity)andReceive USDT › Specify Amount; exact current node links are unverified.Preview
The earlier zero-placeholder screenshots are historical. The updated design uses an unknown marker until the balance is read; numeric balances retain the same formatting and layout.
QA Notes
Setup
Use Java 17 and the branch’s pinned Core/Paykit dependencies. In ignored
local.properties, setUSDT_RPC_URL=http://127.0.0.1:1andUSDT_BUNDLER_URL=http://127.0.0.1:1for an offline test build. RunGEO=false E2E_BACKEND=network just e2e assembleDevDebugand install the ARM64 APK on a dedicated emulator. Keep English language with Germany/Netherlands region; on the tested image,adb -s <serial> shell cmd locale set-app-locales to.bitkit.dev --locales en-DEselects English/Germany. Use an unfunded wallet, enable USDT in Settings, show balances and select dollar display. Offline endpoints exercise the unknown marker; confirmed-zero checks require a successful read from an owned local test provider and are also covered by regression tests. No payment or wallet reset is required.Journeys
usdt-number-format.xml— consistent Home/wallet values, silent unknown/confirmed-zero states, masking and exact dot-decimal entry across regions.usdt-wallet-navigation.xml— unknown or empty wallet stays usable while synchronization is silent.Manual Tests
N/A
Automated Checks
UsdtDisplayTest.kt,PaykitAmountViewTest.ktandUsdtDepositViewModelTest.kt— fixed separators across US/German/Dutch/French regions, exact six-decimal values, sub-cent balances, rounding and maximum-sized amounts.UsdtPaymentContentTest.ktandPaymentRequestAmountLocaleTest.kt— silent unknown/zero/loaded/hidden balance rendering and dot-decimal USDT/USD input in comma regions.Evidence
At
95dc2fead(including current master4bb41772),just compile,just testandjust lintpassed: 4,395 unit tests, zero failures/skips, zero Detekt SARIF results and zero Android Lint XML findings. The E2E app and instrumentation APKs also built successfully. Complete logs and structured reports were inspected; only three JVM class-sharing notices appeared in the unit runner, with no source warning/error diagnostics.All 11 targeted Compose tests passed on the dedicated ARM64/API 36 emulator, including unknown → confirmed zero → loaded → hidden balance transitions, dot-decimal input and narrow/enlarged-text component checks. With the existing unfunded wallet preserved and English/Germany selected, the final app showed
—on Home and$ —in the wallet, without refresh-error or history-sync paragraphs; screenshots and UI hierarchies were inspected (partial J1/J2 evidence).Independent review and a bounded master-merge recheck found no introduced issues. The merge preserves master’s new amount layout and funding-source dropdown. All five commits in the current-base-to-pushed-head range were cryptographically verified locally, and GitHub’s compare endpoint returned the exact same five commits with
verified=true,reason=valid. GitHub’s PR head and exact five-commit list now match the pushed branch. Current-head CI, lint and all seven local E2E shards passed.Historical
9bb64a38bpassed all 4,383 unit tests, full lint with zero Detekt/Android Lint findings and all 11 targeted Compose tests. Earlier author-driven J1/J2 checks covered dot-decimal entry, exact QR amounts, privacy, and compact/enlarged text. Those zero-placeholder screenshots are superseded by the unknown-balance correction.Gaps
The full regional J1 repeat and broader J2 hardware/activity fixtures were not rerun on this correction. Confirmed-zero and loaded-balance transitions are covered by component tests; offline Home/wallet checks cover the unknown marker. No funded payment or wallet reset was performed. Current-head hosted checks passed; this does not add funded USDT/provider coverage.
Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)