Repository navigation
fix: improve USDT payment and bridge feedback - #973
ben-kaufman wants to merge 5 commits into
Conversation
 [Medium impact] The PR appears safe to merge; no actionable issues were found. SummaryThis PR improves USDT payment entry and bridge feedback.
No actionable issues were found. Tests and iOS visual checks were not run in this Linux environment. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Quote, MAX, or send] --> B{Cooldown active?}
B -->|Yes| C[Show retry message without calling provider]
B -->|No| D[Run requested operation]
D --> E{Rate limited?}
E -->|Yes| F[Start 60-second cooldown]
E -->|No| G[Return result]
F --> H[Pause payment retries and wallet refresh]
Reviews (1) · Last reviewed commit: "fix: improve USDT payment and bridge fee..." · Reviewed by Greptile |
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
No findings. iOS twin of synonymdev/bitkit-android#1531: Core rc7 bridge errors, withRateLimit around quote/MAX/send with an injectable clock, refund errors kept beside the address, inputValue for Paykit presets, accessibility-size stacking, and clipboard detection suppressed while a sheet or USDT send is open. Reviewed 8d4c900, full tier, reasoned from code and CI: iOS does not build on this machine.
What I checked, and 4 candidates I ruled out
Read: UsdtWalletManager.swift (refresh, quote, quoteMaximum, send, waitForTransfer, withRateLimit, message(for:)), MainNavView.swift handleClipboardIfEnabled/processClipboardUri, ClipboardPrompt.swift, ScannerManager.handlePaste, PaykitAmount.swift, UsdtDepositHistoryView.swift, UsdtSendView.swift, UsdtSupportActions.swift, plus the Core variants in bitkit-core#172.
CI at review time: validate, detect-changes, Greptile green; Run Tests, Run Integration Tests and build-local still pending.
Ruled out
handlePasterecording the pasteboard and suppressing a later legitimate prompt:recordInspectionstores the currentchangeCount; a new copy raises it andshouldOfferre-fires onchangeCount > previous.withRateLimitblockingbeforeSubmissiontoo late: its guard throws before the closure, sobeforeSubmissionnever runs during cooldown, which the new test asserts withXCTFail.inputValueexceeding the keypad'smaxDecimals: 2for USD:.usdhas 2 decimals, sofractionLength(2...2); USDT allows 6, matching its entry.- Refund failures from biometric/PIN (
.failed) now hiding from the general error: they render throughrefundErrorbeside the field, and every reset path (updateSelection, deposit change, close) clears it. - Core pin:
0.8.0-rc7is the tag of synonymdev/bitkit-core#172's current head (comparev0.8.0-rc7...<#172 head>is identical, one commit ahead of coremaster), and #172 is still open. This is not a defect in this PR, but a change to #172 before it merges would mean a new rc and a bump here.
Merge confidence: 4/5, no findings, but the unit and integration test jobs had not finished and the branch was not built here.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Code scan at 8d4c900: no findings. I reviewed it as a funds and remote-input change (amount formatting, scanner/clipboard routing, bitkit-core bump).
Issues this closes, each verified at head:
- #959: the clipboard check returns early while a sheet is open, a sheet is being replaced or USDT Send is presented (
MainNavView.swift:760-761,781-782). Paste records the value inClipboardPromptHistory(ScannerManager.swift:299-300). A pasted link still goes throughhandleScan→hideSheetBeforePerforming→resolveUsdtPaymentRoute, with no auto-pay path. A different payload bumpschangeCount, so it cannot be suppressed. - #960:
refundErrorrenders under the address field (UsdtDepositHistoryView.swift:162-164). Refreshes of the same deposit keep it (:282-290). An address edit (:107), a new request (:331) orclearSelection(:240) clears it. The address is never cleared by an error. - #961:
PaykitAmount.inputValueonly pads to at least 2 fraction digits (en_US_POSIX, no grouping) and never rounds..valueis untouched, and the wire path parsesamount.value(SendAmountView.swift:333-348). The asset switch keeps an empty field for a zero amount (:541). - #962: the button rows switch to
VStackLayoutat accessibility sizes. - #963: copy uses
deposit.networkLabel. - #964 and #965: bridge minimum, maximum and liquidity errors come from the new Core variants.
withRateLimitblocks quote, MAX and send during the cooldown without extending it.
Dependency: bitkit-core 0.8.0-rc6 → rc7 (7584986a), the same as synonymdev/bitkit-android#1531. rc7 adds BridgeAmountBelowMinimum / BridgeAmountAboveMaximum / BridgeLiquidityUnavailable and their Orchestra mapping, with no storage or schema change. The v0.8.0-rc7 tag sits on the still-open synonymdev/bitkit-core#172, one commit ahead of Core master. A later rc cut from master would drop these mappings unless #172 merges first.
l10n: three new keys (usdt__error_bridge_minimum, _maximum, _liquidity), English only per the Transifex flow. They render in wrapping body text, not in fixed-width containers.
Performance-relevant (retry and cooldown): one RateLimited now blocks quote, MAX and send for 60 s.
Device gate (partial): 8d4c900 — not run yet. Pending, against the USDT fixture:
usdt-bridge-amount-errors.xml: too small, too large, liquidity, and 429 then retries within 60 s.usdt-bridge-details.xml: a rejected refund address kept through two polls; stacked buttons at an AX size.usdt-payment-links-and-scans.xmlwith Read clipboard on: Scan › Paste › Allow opens one USDT send, and Scan and Send still work afterwards.150,50on a comma locale.
The device result will follow.
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
Re-review of fb7e53a after 8d4c900. 0 earlier findings, 0 new. The one new commit lets shared buttons, RectangleButton, NavigationBar, SheetHeader, the camera permission view and the USDT network label grow and wrap at accessibility text sizes (fixed heights become minimums with vertical padding), and puts the USDT recipient step in a scroll view. Reviewed full tier, reasoned from code and CI: iOS does not build on this machine.
What I checked, and 4 candidates I ruled out
Read: git diff 8d4c900..fb7e53a whole, PrimaryButtonView, SecondaryButtonView, TertiaryButtonView, RectangleButton, NavigationBar, Sheet.swift SheetHeader, ScannerPermissionRequest, UsdtSendView.recipientContent, ComponentSizingTests.swift.
CI at review time: validate, detect-changes green; Run Tests, Run Integration Tests and build-local still pending.
Ruled out
- Default button heights moving: vertical padding 12 (8 small) plus one line of
BodySSBText/CaptionBTextstays under 56/40, and the loadingProgressViewis 20 pt;ComponentSizingTestsasserts 56, 40 and 72 at.large. - A caller pinning a button with its own fixed height and now clipping: at default sizes the content still fits the old height, so nothing changes until text is enlarged, which is the point of the change.
ScannerPermissionRequestlosing its full-bleed black background:.frame(maxWidth: .infinity, maxHeight: .infinity)sits before.background(Color.black), so the background still fills.- Camera preview no longer filling the recipient step inside the new
ScrollView: not established; it depends on how SwiftUI proposes height throughframe(minHeight:)in a scroll view, which needs a simulator to settle, and the PR's simulator checks cover the compact layout.
Merge confidence: 4/5, no findings, but the unit and integration test jobs had not finished and the branch was not built here.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approving at 8d4c900. The code scan from my earlier review stands, with no findings.
Setup: Debug E2E_BUILD with Core 0.8.0-rc7 on two simulators (iOS 26.5, en_BR comma locale): a payer with 25 USDT and a requester, linked as Paykit contacts, against the bitkit-docker USDT fixture.
usdt-bridge-amount-errors.xml: 8/8 pass.
- Steps 2–4:
xchain_amount_too_smallon Tron send and receive shows "Amount too small to cover bridge costs" with no review or QR. The error clears on edit, and 10 USDT reaches Estimated Fees. - Step 5:
amount_too_smallwithmin_usd_cents200 on receive shows "Minimum amount is $2,00". Without a limit there is no invented number. - Step 6: send
amount_too_largeshows the route maximum and liquidity shows "Try another network or try again later", on Continue and MAX, with nothing submitted. - Step 7: the edit clears the error before any quote.
- Step 8, HTTP 429: "Too many requests…". Continue and MAX × 3 plus pull-to-refresh within the minute caused 0 bridge and 0 chain-rpc calls (gateway log). After the minute, new calls reached review and nothing was submitted.
usdt-bridge-details.xml, refund steps: pass.
- A rejected refund address shows "Enter a valid address for the selected network." directly under the field. It stayed through 3 detail polls, cleared on edit and returned on re-submit. Switching deposits showed a clean field.
- At AX5 the four buttons stack full-width with full labels, and they return to two rows at
large.
usdt-payment-links-and-scans.xml with Read clipboard on: pass. A link put on the clipboard while the scanner was open raised no prompt. Paste › Allow opened exactly one USDT Confirm, with one Scanner routed USDT payment and no clipboard alert. Scan and Send both worked afterwards.
Comma locale: the request editor keeps $ 150,50 through review, edit and back, and the payer's Amount opens with $ 150,50.
Device gate: 8d4c900 — journeys: 3 driven, all requested steps pass; perf: rate-limit cooldown 0 calls in 60 s, calls resumed after it; l10n: 3 new English-only keys in wrapping text, no overflow at AX5.
Seen but outside this PR:
- Receiving with
amount_exceeds_liquidityshows "This network is currently unavailable." Core rc7deposits.rsmaps it toUnsupportedRoute; onlyorchestra.rs(send) hasBridgeLiquidityUnavailable. - At AX5 the stacked buttons stay 40 pt high, so the text is taller than the capsule.
|
Device gate (partial): Setup: Debug
Seen but outside this PR: receiving with |
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approving at fb7e53a. The code scan from my earlier review stands, with no findings. The 8d4c900 gate results are in #973 (comment).
Re-run at fb7e53a on the layout-affected steps:
- Scan › Paste › Allow with Read clipboard on, USDT Send Paste and Enter Manually each open exactly one USDT flow.
- At AX5 the network chip, Paste and Enter Manually are readable, and the recipient area scrolls. The deposit action buttons grow to 53 pt with the text inside the capsule, which fixes the overflow seen at
8d4c900. - At default size, the frames on Home, the Receive sheet, USDT recipient and manual entry are identical to a master-based build, so ordinary buttons did not change height.
Device gate: fb7e53a — journeys: usdt-bridge-amount-errors.xml 8/8 at 8d4c900; usdt-bridge-details.xml refund and AX steps and usdt-payment-links-and-scans.xml Read-clipboard step pass at both heads. perf: 429 cooldown 0 bridge and 0 chain-rpc calls in 60 s, then resumed. l10n: 3 English-only keys, no overflow at AX5.
Seen but outside this PR:
- Receiving with
amount_exceeds_liquidityshows "This network is currently unavailable." Coredeposits.rsmaps it toUnsupportedRoute. - At AX5, drags that start on the camera preview don't scroll the recipient area.
- At AX5, the deposit history rows truncate ("Ethe…", "Needs…", "4,00…").
- At AX5, the STATUS / RECEIVING NETWORK grid breaks words mid-word ("RECEIVIN/G", "Arbitru/m One").
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Reviewed the full PR diff against its merge base eb3156ab, at fb7e53ac.
No new actionable code findings.
Automatic clipboard reads are skipped while a sheet, a sheet replacement, or USDT Send is open, and an explicit main-scanner paste records that clipboard generation so the same contents are not offered again. Refund failures stay under the address through a same-deposit refresh and clear when the address or selected deposit changes. Preset USD and USDT inputs keep at least two fraction digits. Payment-request serialization uses PaykitAmount.value, so a 150.50 input still serializes as 150.5. Quote, MAX, and send share the 60-second rate-limit gate, and a retry during that window does not call the provider or move the deadline. Shared buttons, headers, and the USDT recipient step can grow with accessibility text; the sizing tests lock the default 56, 40, 72, and 48 point heights.
The shared amount, cooldown, refund, and accessibility behavior matches the Android twin bitkit-android#1531 at 0d054759. In bitkit-core 7584986a, send quotes map amount_exceeds_liquidity to BridgeLiquidityUnavailable. Deposit creation maps that provider error to UnsupportedRoute, so a receive attempt shows the unavailable-network message.
Run Tests and Run Integration Tests succeeded on this revision. This review did not run those tests. Device testing: not performed in this review. Separate device notes report journey steps on 8d4c900 and later layout checks on this revision. The local e2e job was still queued, so this is not an Appium result.
Ready for device testing.
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 💬 Comment: one MEDIUM finding; all 4 device tests passed.
Findings:
- MEDIUM
Bitkit/Views/Wallets/Send/SendOptionsView.swift:105: a value pasted with Send › Paste is offered by the clipboard prompt after it was already pasted.- Only the scanner's
handlePaste(context:)records the paste inClipboardPromptHistory(Bitkit/Managers/ScannerManager.swift:299-300). SendOptionsView.handlePasteand USDT Send'sUsdtPaste(Bitkit/Views/Wallets/UsdtSendView.swift:253) read the pasteboard without recording it.- The clipboard check now returns early while a sheet or USDT Send is open (
Bitkit/MainNavView.swift:760-761), so a value copied while Send is open is never inspected. - I copied an address with Send open, used Send › Paste Invoice › Allow Paste, closed the flow and returned Home. On the next foreground, "Clipboard Data Detected" offered that address, and OK reopened a USDT payment to it.
- This run closed from Amount without opening a text field, and the stored inspection kept the previous value after the paste. It does not depend on the change-count rise in Test 3.
- Could we record the inspection in both paste handlers, as
handlePaste(context:)does?
- Only the scanner's
finding1-send-paste.2x.mp4
- LOW
Bitkit/Views/Wallets/Send/SendAmountView.swift:349: the Paykit USDT hand-off still passesamount.value, so the next step shows$ 150.5.- The Amount screen now opens with
150.50(:541,:586), but Continue hands"150.5"to the embeddedUsdtSendView. - Its amount step renders that string raw through
UsdtAmountHeader(Bitkit/Views/Wallets/UsdtWalletScreen.swift:142):$ 150.5, or$ 150,5on a comma locale. - The payer sees it while the quote loads, and it stays when the quote fails, for example during the new rate-limit cooldown.
- Could we pass
amount.inputValuehere? The wire amount already comes fromapp.paykitReviewedAmount.
- The Amount screen now opens with
QA for fb7e53a:
Tested on iOS 26.5 simulator (iPhone SE 3rd generation), against the bitkit-docker USDT fixture.
Test 1: Copy Details network names ✅ Tron, Polygon and BNB Smart Chain deposits copy their readable names.
- I read the clipboard after Copy Details on fixture deposits from
tron,polygonandbsc. - It read
Source network: Tron,Source network: PolygonandSource network: BNB Smart Chain, each with the deposit reference,Status: Needs attention (held), source transaction and status code. - Contact Support prefilled the same text for the BSC deposit; I closed it without submitting.
case1-copy-details.2x.mp4
Test 2: deposit headers at the largest text size ✅ Headers and the footer wrap in full without covering Back or Menu.
- At the largest accessibility size, "Cross-Network Deposits" wraps to three lines and "Receiving USDT" to two, between Back and Menu.
- The footer button shows "Cross-Network Deposits" in full on two lines.
- At the default size both headers are single-line, and Contact Support and Copy Details share one row.
case2-ax5-headers.2x.mp4
Test 3: scanner paste with Read clipboard on ✅ One USDT flow opened with no clipboard alert, and Home did not offer the paste again.
- Scan › Paste QR Code › Allow Paste opened one USDT Amount step for the pasted address, with one
Scanner routed USDT paymentlog line and no clipboard alert. - After closing from Amount and returning Home, backgrounding and foregrounding raised no prompt.
case3-scanner-paste.2x.mp4
Seen outside this PR:
- In two runs that went through Enter Manually before Home, the prompt offered the same address again.
- Focusing Enter Manually alone, with no paste, does the same on this simulator, also when Bitkit itself copied the text. The clipboard change count rises with the text unchanged, and the existing prompt rule treats a higher count as new content.
case3-focus-reoffer.2x.mp4
Test 4: USDT Send on a compact display at the largest text size, camera denied ✅ Network name, Enable Camera, Paste and Enter Manually stay readable and reachable.
- The network chip shows "Arbitrum One", and "Polygon" after switching, in full.
- Scrolling shows Enable Camera with its full two-line label inside the button, then Paste and Enter Manually. Enable Camera opened iOS Settings.
- Send offers Arbitrum One, Ethereum, Polygon and Plasma in this build, so BNB Smart Chain was not available to select.
case4-compact-ax5-camera-denied.2x.mp4
Not driven:
paykit-quoted-usdt.xml, six-decimal USDT preset and serialized amount: it needs a BTC request between two linked wallets. I checked it in code andBitkitTests/PaykitAmountTests.swift:inputValueonly pads to two decimals, and the wire keepsPaykitAmount.value.- Unknown source network in Copy Details: the fixture accepts only known networks;
BitkitTests/UsdtSupportDetailsTests.swiftcovers it. usdt-payment-links-and-scans.xml, a usable camera with gallery import and flash controls: the simulator has no camera. In the denied state, Test 4 shows one scanner heading and one Paste action.
Relied on jvsena42's device results instead of repeating them:
- comment at 8d4c900:
usdt-bridge-amount-errors.xml8/8, the refund error and AX5 deposit buttons, scanner paste with Read clipboard, and150,50on a comma locale. - review at fb7e53a: the layout steps and default-size frames.
Review: full diff, 29 files. No test of mine drove the bridge errors, cooldown or refund changes; I read them in code, and jvsena42's runs cover them.
Twin synonymdev/bitkit-android#1531 at 0d05475 makes the same cooldown, bridge-error, refund, inputValue and accessibility changes. Its zero-amount fix is not needed here, since iOS zero drafts are BTC and the USD switches guard zero.
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
Re-review of e83e629 after fb7e53a. 0 earlier findings of mine, 0 new. The commit fixes the MEDIUM @ovi-reviewer raised at fb7e53a. SendOptionsView.handlePaste and USDT Send's Paste button now record the clipboard in ClipboardPromptHistory the same way ScannerManager.handlePaste(context:) does. A Paykit USDT payment also hands amount.inputValue (150.50) to UsdtSendView instead of value (150.5), so review keeps the cents. Reviewed full tier, reasoned from code and CI: iOS does not build on this machine.
What I checked, and 4 candidates I ruled out
Read: git diff fb7e53a..e83e629 whole, ClipboardPrompt.swift (shouldInspect, recordInspection), MainNavView.handleClipboardIfEnabled, ScannerManager.handlePaste, SendOptionsView.handlePaste, UsdtSendView init, initial-quote .task and Paste button, SendSheet .usdt route, UsdtWalletManager.quote, UsdtPreparedQuote.matches, parsePaykitAmountInput, usdtInputAmount, PaykitAmountTests.swift.
CI at review time: validate, detect-changes green; Run Tests, Run Integration Tests and build-local still pending.
Ruled out
- Digest mismatch, so the prompt still fires: both new call sites hash the trimmed string.
handleClipboardIfEnabledhashes the same trimmeduri, so the digests match. Because the change count matches too,shouldInspectskips the check in the same launch, and after a relaunchrecordInspectionreturns false. - A later, different copy being suppressed: a new copy raises
changeCount, sorecordInspectionoffers it again.ClipboardPromptHistoryTestscovers this. 150.50breaking the embedded USDT send: the init's zero check usesusdtParseAmount, and the auto-quote guard (amount == initialAmount,parsePaykitAmountInput) compares the same string it was given.UsdtWalletManager.quoteswaps the locale separator for.before parsing, so150.50gives the same atomic amount as150.5.PaykitAmount.valueand the reviewed amount are unchanged, andPaykitAmountTestsasserts that150.50still serializes as150.5.- Comma locales showing
150.50: the header renders the amount throughusdtInputAmount, and the new test asserts thatde_DEgives150,50.
Merge confidence: 4/5, no findings, but the unit and integration test jobs had not finished and the branch was not built here.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up review of changes since fb7e53ac, against the baseline review, at e83e6291. Inherited baseline coverage for sheet and main-scanner clipboard suppression, refund errors, preset formatting and serialized amounts, bridge limit messages, rate-limit cooldown, shared layout, support-detail labels, the unchanged tests and journeys, and the Android shared behavior. This pass rechecked explicit Send and USDT paste history, the USDT review amount handoff, and the tests and journeys added for those two behaviors.
No new actionable code findings.
Send › Paste Invoice and USDT Send's Paste now store the trimmed clipboard in the same inspection history as the main scanner. The same content is not offered again after returning Home and backgrounding the app; a later copy raises the change count and can be offered. Continue on a 150.50 USDT preset passes inputValue, so the amount header keeps two decimal places, including 150,50 in a comma locale, while the fee quote loads and after it fails. Parsing in bitkit-core 7584986a treats 150.50 and 150.5 as the same atomic amount, and the reviewed Paykit amount is unchanged. The earlier comments on paste re-offer and the 150.5 handoff no longer match this source.
Compared bitkit-android#1531 at edc0407e only for these steps: its journey text matches, and its amount path already used inputValue. That is not a full Android review. Device testing was not performed here; earlier simulator notes stay on 8d4c900 and fb7e53ac. PaykitAmountTests was inspected and not run. Run Tests and Run Integration Tests were still pending, so they are not execution evidence for this commit. The author reports that 30 focused tests passed after the change.
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
Re-review of ffab473 after e83e629. 0 earlier findings of mine, 0 new. The commit only adds three journey actions: a comma-decimal check across direct USDT Send, USDT Receive amount entry and a USDT contact payment, a delayed-quote check that the keypad and Delete stay inert while Continue or MAX is loading, and an Android-only font-scale step in the hardware wallet settings journey. No Swift changed. Reviewed full tier, reasoned from code and CI: iOS does not build on this machine.
What I checked, and 3 candidates I ruled out
Read: git diff e83e629..ffab473 whole, NumberPad.swift (decimal key, NRemove long press, .disabled(isDisabled)), UsdtSendView amount step (busy, preparingMaximum, Continue and MAX), UsdtReceiveAmountView, SendAmountView decimal separator, UsdtWalletManager.quote/quoteMaximum separator handling.
CI at review time: validate, detect-changes, e2e-branch/determine green; Run Tests, Run Integration Tests and build-local still pending.
Ruled out
- Comma journey asserting behaviour the app lacks: all three flows pass
Locale.current.decimalSeparatortoNumberPad(UsdtSendView,UsdtReceiveAmountView,SendAmountViewwhenpaykitUsesUsdt), andUsdtWalletManagermaps the separator back to.beforeusdtParseAmount. - Delete long press clearing the amount while a quote loads: the long press is a
simultaneousGestureinsideNumberPad, which applies.disabled(isDisabled)to the whole pad, andUsdtSendViewpassesisDisabled: busy || embedded; MAX setsbusytoo. - Android-only step in an iOS journey: the file is kept diffable with its Android twin per
journeys/README.md, and the step is scoped "On Android".
Merge confidence: 4/5, no findings, but the unit and integration test jobs had not finished and the branch was not built here.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Delta review fb7e53a..ffab473: no findings. This round was reviewed on Opus because Fable credits ran out.
- Paste history.
SendOptionsView.swift:115-116andUsdtSendView.swift:253-256now record the pasted value withClipboardPromptHistory().recordInspection(ClipboardPrompt.swift:27-38), as the scanner already did. Only{changeCount, SHA256, launchID}is stored, never the payload. The only reader isMainNavView.handleClipboardIfEnabled, which uses it to suppress the prompt, so nothing can resurface a recipient or an amount. A new copy (changedchangeCount) is still offered. The sheet and replace guards fromfb7e53aare unchanged. This addresses ovi-reviewer's re-offer MEDIUM. - Review decimals.
SendAmountView.swift:349handsamount.inputValue(150.50) to the.usdtroute instead ofamount.value(150.5). The amount paid is unchanged:PaykitUsdtPaymentService.preparerequiresPaykitAmount(asset: .usdt, atomic: quote.amount) == app.paykitReviewedAmount(:283-284), andPaykitAmount(value:)gives 150 500 000 atomic for both strings. The wire format stays.value. Android already hands theinputValueform to review (AppViewModel.kt:3267). This addresses ovi-reviewer's 150.5 MEDIUM. - Journeys.
paykit-quoted-usdt.xml,usdt-bridge-amount-errors.xmlandusdt-payment-links-and-scans.xmlare byte-identical to the Android branch. The new paste-history step is scoped "On iOS".
Device gate (partial): ffab473 — not run yet. Pending, on en_BR:
- Paste history with Read clipboard on, from Send › Paste Invoice and USDT Send › Paste. Expect no prompt after a background/foreground, and a different copy still prompts.
$ 150,50on USDT review while the quote loads.- Comma keys on USDT Send and Receive.
The device result will follow.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up review of changes since e83e6291, against the baseline review, at ffab4730. This pass covered the three journey actions added in that commit and the amount-entry paths they describe. Paste suppression, the inputValue review handoff, preset formatting, bridge-limit copy, cooldown, shared layout, support labels, and the Android comparison through edc0407e remain inherited from that baseline.
No new actionable code findings.
The comma-decimal action matches direct USDT Send, USDT Receive amount entry, and a USDT contact payment: the key label uses the current locale separator, the stored amount stays dot-decimal, and usdtInputAmount shows 5,5. While Continue or MAX is busy, the send keypad is .disabled, which blocks button hits and the delete long-press, and enterAmount also ignores keys. The hardware action is Android-only and matches the font-scale stacking in bitkit-android fe26c0f5; the three added action strings match that revision. That comparison covers those shared steps only, and is not a review of bitkit-android#1531. The baseline paste-history and inputValue fixes are still present.
Device testing: not performed in this review. No unit test was executed here. Integration tests on this revision completed successfully (run 38962738693). The unit-test run was still in progress (run 38962738751), so this review does not treat it as passed.
There was a problem hiding this comment.
Approving at ffab473. The delta review from my earlier comment stands, with no findings. This round was reviewed on Opus because Fable credits ran out.
Device gate: ffab473 — on two simulators (iOS 26.5, en_BR), Debug E2E_BUILD, against the bitkit-docker USDT fixture. Payer: 200 000 sats and 200 USDT. Requester: a new profile, linked as a contact.
- Paste history (
usdt-payment-links-and-scans.xml, last action), with Read clipboard on: pass.- Send › Paste Invoice and USDT Send › Paste, each closed from Amount and then backgrounded for 10 s, raised no "Clipboard Data Detected" prompt.
- A different address copied while backgrounded raised the system Allow Paste dialog, then the prompt.
- Comma keys (
paykit-quoted-usdt.xmlaction 21): pass. Direct USDT Send, USDT Receive and a contact USDT payment each show "," (NDecimal), and 5,5 displays as$ 5,5. - Review decimals (action 20): pass.
- A USD 150,50 USDT-only request opens Amount on the payer with
$ 150,50preset and the keypad disabled. - It keeps
$ 150,50while quoting and on the review with Swipe To Pay. - With the fixture provider set unavailable, it shows "Could not update USDT…" and still
$ 150,50. - Not paid.
- A USD 150,50 USDT-only request opens Amount on the payer with
perf: not relevant. l10n: no new keys in this delta.
Seen but outside this PR: Contacts › Add › Paste does not record the pasted pubky in ClipboardPromptHistory, so the clipboard prompt offers it again after the paste. Only the Send, USDT Send and Scanner paste paths record it.
CI: Run Tests is red on testPendingWalletSwitchResumesAfterTags (SendConfirmationLifecycleTests.swift), a file this PR does not touch. The same test also fails on #954 and #973, which share no code, and master's unit tests are red on another test. It needs a re-run before merge.
|
On the preceding The current The failing test and SendConfirmationView match current master. That does not establish the root cause or reproduce the failure on base. The same test is also being investigated on #954; its documented local head/base comparisons passed, so I have not duplicated those runs or made speculative code changes. One failed-job retry on the unchanged current head is now requested. The current-head check is not green yet, and the previous pass is not a fix for this recurring failure. |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up review of changes since ffab4730, against the baseline review, at 73f2bf44. This pass rechecked the two Android-only hardware intro actions that replaced the font-scale step, and confirmed every other path in the PR is unchanged from that baseline. Inherited coverage keeps the comma-decimal and delayed-quote checks, the unchanged quoted-USDT action text compared with bitkit-android fe26c0f5, and the baseline paste, amount, bridge, cooldown, layout, and support coverage.
No new actionable code findings.
The replacement actions in settings-hardware-wallets.xml match bitkit-android c1a7d21 exactly. German Abbrechen and Weiter stack on a 360dp-wide sheet at font scales 1.15 and 1.25 and stay side by side at 411dp and font scale 1.3; at 360dp and font scale 2.0 the sheet scrolls, Weiter opens the searching step, and restoring the original language, width, and font scale returns the 56dp side-by-side row. Those breakpoints are asserted by HwIntroSheetTest, which this review inspected and did not run. That comparison covers these shared actions only and is not a review of bitkit-android#1531. The resolved paste-history and inputValue threads still match the current source. iOS hardware intro layout is unchanged.
Device testing: not performed in this review. No tests were executed here. Run Tests and Run Integration Tests were still in progress on this revision, so they are not treated as passed.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Re-approving at 73f2bf4. The only change since my approval at ffab473 is journeys/hardware-wallet/settings-hardware-wallets.xml: two action lines scoped "On Android", matching synonymdev/bitkit-android#1531. The iOS app code is unchanged, so the device gate at ffab473 still applies. Run Tests still needs a re-run before merge (the testPendingWalletSwitchResumesAfterTags failure noted earlier, which this PR does not cause).
There was a problem hiding this comment.
Verdict:
Findings: none.
Earlier findings, fixed at this head:
- The paste re-offer:
Bitkit/Views/Wallets/Send/SendOptionsView.swift:115-116andBitkit/Views/Wallets/UsdtSendView.swift:253-257now record the trimmed paste inClipboardPromptHistory, the same stringBitkit/MainNavView.swift:769hashes. - The
$ 150.5hand-off:Bitkit/Views/Wallets/Send/SendAmountView.swift:349passesamount.inputValue. The quote and the reviewed amount compare atomic values, so150.50pays the same as150.5.
QA for 73f2bf4:
Tested on iOS 26.5 simulator (iPhone 17 Pro, comma-decimal region), against the bitkit-docker USDT fixture.
Test 1: delayed bridge quote ✅ While Continue or MAX loads, digits, Delete and a long press on Delete leave the amount unchanged, and editing works afterwards.
- USDT Send to a Tron recipient for
$ 6, with each bridge quote response delayed 25 s, then 15 s. - While Continue loaded, I tapped 1 and 2, tapped Delete, long-pressed Delete for 1.5 s, and tapped Continue and MAX again. The amount stayed
$ 6, the keypad, MAX and Continue stayed disabled, and the gateway log showed one quote request. - The 25 s delay outlasted the app's request timeout. "Could not update USDT. Check your connection and try again." showed with
$ 6kept and the keypad usable. - The 15 s delay reached Confirm. Back on Amount, 1 gave
$ 61and Delete gave$ 6. - While MAX loaded, the same attempts left
$ 6. MAX sent its two quote requests (an undelayed MAX also sends two) and filled$ 23,31954. - After MAX, Delete gave
$ 23,3195, 1 gave$ 23,31951, and a long press on Delete cleared it to$ 0. No payment was approved.
t1-delayed-quote-2x.2x.mp4
Test 2: six-decimal USDT preset on a quoted BTC request ⏭️ Not run: our second simulator wallet did not come up.
paykit-quoted-usdt.xmlneeds a BTC request accepting USDT from a linked contact.- Both wallets got profiles and were linked as contacts, but the requester simulator then stopped responding before a request was created.
BitkitTests/PaykitAmountTests.swiftcoversinputValueup to six decimals and the unchanged wire value.
Test 3: unknown source network in Copy Details ⏭️ Not run: our simulator stopped responding before the deposit was staged.
usdt-bridge-support.xmlasks for an unknown network to be kept as its raw value.- In code,
networkLabelfalls back to the raw value (Bitkit/Views/Wallets/UsdtDepositHistoryView.swift:402). No unit test covers an unknown network (our earlier review wrongly citedUsdtSupportDetailsTests).
Test 4: scanner with camera allowed: gallery import and flash ⏭️ Not run: our simulators could not load a QR image into Photos, and a simulator has no torch.
usdt-payment-links-and-scans.xmlasks for gallery import and flash controls while the camera is usable.- The denied-camera state was covered in our earlier run at fb7e53a.
Not driven:
settings-hardware-wallets.xml, the two added actions: they are Android only.
Relied on @jvsena42's device results instead of repeating them:
- review at ffab473, whose app code this head leaves unchanged:
- paste history from Send › Paste Invoice and USDT Send › Paste, with a new copy still prompting;
$ 150,50while quoting, on review and after a provider failure;- comma keys on USDT Send, USDT Receive and a contact payment.
- comment at 8d4c900: the request editor keeps
$ 150,50through review, edit and back.CreatePaymentRequestViewandPaykitAmountare unchanged since.
Review: delta diff, 8 files. No device case of mine drove the paste-history and hand-off fixes; the runs above cover them.
Twin synonymdev/bitkit-android#1531 at 8cefc2b matches: the three USDT journey files are identical, its Paykit USDT hand-off already passes inputValue, and it keeps no clipboard inspection history, so the paste fix has no Android side.
4 participants
Fixes #959
Fixes #960
Fixes #961
Fixes #962
Fixes #963
Fixes #964
Fixes #965
This PR fixes USDT payment entry, bridge feedback and accessibility issues found during device testing.
Description
Core changes: synonymdev/bitkit-core#172 (available in 0.8.0-rc7).
Twin: synonymdev/bitkit-android#1531
Out of Scope
Design
Existing payment request layouts and bridge details. No layout redesign; the latest handoff page reference is not confirmed for these accessibility fixes.
Preview
QA Notes
Journeys
settings-hardware-wallets.xml— shared Android-specific checks for German labels at narrow widths and reachable actions on shorter sheets.paykit-quoted-usdt.xml— decimal presets preserve the entered value and trailing places, with consistent localized decimal keys.usdt-bridge-amount-errors.xml— minimum, maximum, liquidity and rate-limit errors remain actionable and clear on edits; loading quotes keep amount input disabled.usdt-bridge-details.xml— refund rejection stays visible through refresh and clears after editing.usdt-bridge-support.xml— support details use readable network names and actions remain usable with enlarged text.usdt-payment-links-and-scans.xml— manual paste opens one payment flow and is not offered again after returning Home.Manual Tests
N/A
Automated Checks
PaykitAmountTests.swift— preset formatting and localized review values retain decimals and leave exact wire values unchanged.UsdtWalletManagerTests.swift— bridge limit errors are distinct and rejected retries do not extend the cooldown.UsdtSupportDetailsTests.swift— copied network labels remain readable.ComponentSizingTests.swift— default button heights remain unchanged and accessibility text can grow in buttons, titles and camera permission content.