Skip to content

fix: confirm fixed Paykit requests directly - #1014

Open
ben-kaufman wants to merge 20 commits into
masterfrom
fix/paykit-fixed-request-confirm
Open

ben-kaufman wants to merge 20 commits into
masterfrom
fix/paykit-fixed-request-confirm

Conversation

@ben-kaufman

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

Copy link
Copy Markdown
Contributor

Exact USDT review amounts — 629a874e (October 28): Fixed the six-decimal display finding from review 5678141014. The actual payer confirmation header and Show Details payment, fee, recipient amount and maximum total now use the existing exact USDT formatter. Request/subscription amount text also preserves all six decimals. Quote values, serialized amounts and payment authorization are unchanged; balance overviews retain their established formatting.

The rendered regression reproduced $0.12 / $<0.01 for 0.123456 / 0.000001 before the fix (one failed test, ten assertions). All ten focused amount/display tests now pass. Saved native screenshots show exact headers, fee 0.000001, and totals 0.123457 / 0.000002. These tests host the real production review builders directly and verify static rendering; they bypass full view lifecycle/navigation/quote loading and do not authorize or send a payment. Their new SwiftUI constant-binding warning is retained as an explicit test boundary, not a production-race or warning-free claim.

Final canonical local validation passed 2,934 unit and 39 UI executions, with four documented fixture skips and zero failures or retries. Complete diagnostics were inspected: two AppIntents warnings, ten structured QoS warnings plus the new static-builder State warning, eleven raw QoS backtraces, and 72 NaN notices remain explicitly retained alongside symbolication and simulator notices; this is not a warning-free claim. Full SwiftFormat 0.64.0: 0/846 files require formatting, 83 skipped. Translation validation: zero errors, 15 warnings matching the verified actual-base warning multiset. Two independent source perspectives covered the component and actual payer path; an initial component-only coverage gap was caught before publication and the actual payer regression added. Required source signatures: all 20 commits in the current-base PR range verified locally and matched the exact GitHub set with verified=true / reason=valid. Validation applies to local PR tree 88d9c2e3295b93490a3395510ed3605f1ce1b565, not a new combined master tree. Master 68bede62 added only disjoint profile journey documentation since the earlier base; no unrelated integration was performed.

J1/J2 retain their IDs. Their six-decimal request case now explicitly needs fixture-provided USDT terms, because the iOS editor creates BTC/USD terms. Full linked-request, native Back, fresh-terms retry, provider/funded/hardware/settlement acceptance remains incomplete. No pending human request was recreated; João's announced absence through November 1 is preserved. Current work: implementation gpt-6-astra / xhigh, scoped independent reviews and diagnostic audit gpt-6.1-sol / high, verified from execution metadata.

Hosted checks for 629a874e are complete on workflow attempt 1: unit passed 2,960 unit and 39 UI executions, with four fixture skips and no failures; integration passed 30 tests once each, with four existing Blocktank staging skips. Verified checkout a52bbce2 / tree efec63fb combines actual master 68bede62 with 629a874e; this differs from local PR tree 88d9c2e3. Complete logs, artifact digests, execution trees, case summaries and nested diagnostics were inspected. The payer display regression passed in 11.611 seconds. Its two introduced State/constant-Binding messages remain explicit: direct builder hosting proves static text, not the full view lifecycle or authorization flow. The 286 structured / 571 raw duplicate-symbol warnings match the verified prior warning multisets. Runtime/mock/fixture/tooling notices, symbolication limits, and two passing integration tests' internal sync/deposit recoveries remain recorded; no fresh-base runtime or warning-free claim.

E2E ultimately passed 13 unique cases across 14 executions: 12 passed initially, while Lightning timed out after 30 seconds waiting for ActivityShort-0 to show 9 000 after restored-channel closure. The selective second built-in step passed Lightning; step 3 was skipped. One existing hardware-wallet case remained disabled. This was not a first-pass clean run or a manual retry. Failure screenshots/video were generated on the runner, but their upload was skipped after recovery; no downloadable failed visuals, device/app logs or runtime report were available. Root cause remains unproven, with no actual-base reproduction or permanent-fix claim. All five job logs, pinned harness 4c0cbef0, cases and app-artifact provenance were inspected. The 76 stale-element notices, five log-stream restarts and pinned-harness dependency findings remain explicit. Formatter and translations were already inspected: 0/850 files needed formatting (87 skipped); zero translation errors and 15 verified base-matching warnings.

Automated reviewer QA at this head reports exact six-decimal and one-micro request displays, a six-decimal subscription, and Savings-only proposal selection using linked test wallets and a local fixture, without authorization. This is reviewer-reported evidence, not an independent rerun or human approval. Neither edge gesture established an interactive Back transition; expiry removed the request before fresh-terms retry could be exercised. Editing/serialization, live bridge, provider, hardware and settlement gaps remain. No new delta defect was reported. Shared-navigation ownership coordination and the existing remaining acceptance gates are still required.

Hosted results for 9dacc089: unit 40628484458 and integration 40628484601 succeeded on workflow attempt 1. Verified checkout b9e3793f / tree ed97b16a combines actual master 85e9d4c6 with this head; it differs from the local PR tree. Unit: 2,958 passes; UI: 39 passing executions and four fixture skips. Integration: 30 unique passes in 30 executions, four existing Blocktank staging skips, no failures or failed-test reruns. Full logs, archive digests, structured results, case/skip summaries and nested diagnostics inspected. The 286 structured build warnings and 571 build-log duplicate-symbol warnings match the previously inspected baseline; keyboard queue timeout/No stack, socket/haptic and negative-case notices remain, as do integration RGS/Taproot sync/node/teardown and version-probe broken-pipe notices. This is not a fresh actual-base runtime or permanent-fix claim. E2E 40628484848 also passed on workflow attempt 1: 13 unique passes in 13 executions, one existing hardware-transfer skip, zero failures; first test step passed and fallback steps 2/3 skipped. All five full logs, pinned skip source and artifact provenance were inspected on the same merge/tree. Harness 4c0cbef0; artifact 12790119756 SHA-256 dfd3344f1ced468140d65129a9cc51f3a1b23babb36b910e1df9c5837c95557e matches API/upload/runner download. Retained 74 stale-element notices, four log-stream restarts, one Applying-to-Updated toast wait, tool notices and 76 unresolved harness dependency findings. No local app ZIP or runtime-report/video artifact was inspected; no manual CI retry or warning-free claim. Native Back, fresh-terms retry and remaining J1/J2 acceptance gaps are unchanged.

Current documentation correction — 9dacc089 (October 28): Corrected stale J2 comma-decimal expectations to the existing iOS dot-decimal convention, including comma-region settings. All 19 actions and exact-value, trailing-zero, six-decimal and serialized-value checks remain. No app behavior changed. XML/action/reference checks, git diff --check, full SwiftFormat 0.64.0 (0/846), and one independent documentation review passed. All 19 current-base commits verify locally and on GitHub. App tests were not rerun for this documentation-only delta; earlier local/hosted results keep their original inputs. Automated review 5676643246 reports additional partial device evidence at 20cfeffa, not human approval or independent execution by this monitor. Native Back, expiry retry and full J1/J2 acceptance remain incomplete. The former comma-expectation result is preserved as a failure of the earlier journey wording, not converted into a completed new journey run.

Current repair — 20cfeffa (October 27): The initially-unreviewed Back assertion reported in automated review 5670593898 reproduced independently on unchanged b53e300e with Xcode 27.1/iOS 26.5. The four-line fix observes Amount being popped to an empty navigation path and reuses the existing invalid-terms, presentation-owned dismissal guard. All 15 unchanged lifecycle tests now pass. No assertion or production payment guard was weakened; the journey also names interactive Back as an acceptance check.

Final canonical validation on Xcode 26.1/iOS 26.1 passed 2,932 unit executions and 39 UI executions, four fixture skips, zero failures, including all 35 watch-only tests. Full SwiftFormat 0.64 passed (0/846; 83 exclusions); translations report zero errors and 15 warnings whose individual call/missing-language sets match actual master 543d9389. Complete structured/action/raw diagnostics and an independent frozen correctness/quality review were inspected. Eight aggregate QoS warning cases represent 19 warning-bearing executions; three raw Paykit/pubky initialization backtraces, two AppIntents build notices, simulator/negative-case/runtime messages and the unavailable-fork skip remain explicit. All 18 current-base commits verify locally and match GitHub's exact verified=true, reason=valid set. Tested local PR tree: e286b0d9; the new hosted unit and integration results below cover the combined tree. No warning-free or fresh-master-runtime claim.

Hosted checks at 20cfeffa: Unit 40559037614 and integration 40559037349 passed on workflow attempt 1. Unit: 2,953 unit + 39 UI executions passed, four fixture skips, zero failures, including all 15 confirmation lifecycle and 35 watch-only tests. The repaired initially-unreviewed Back regression passed. Integration: 30 unique passes in 30 executions, four existing Blocktank staging skips, zero failures or test-case reruns. Both checked out verified merge d1ec3668 (543d9389/20cfeffa), tree 370b70b7, which differs from the local PR tree; 21 additional upstream unit cases account for the count difference. Full archive hashes, test trees, all UI/integration case summaries, build/action/runtime diagnostics and job logs were inspected. The three Trezor opt-in cases and isolated Arbitrum case remain skipped; integration retains the four staging skips.

The 286 structured and 571 build-log duplicate-symbol warning multisets match the previously inspected hosted result. Runtime output still includes the keyboard queue timeout with Last Exception Backtrace: No stack!, socket/haptic/launch notices, injected negative-case diagnostics, integration RGS/node/address-search/teardown messages and the version-probe broken-pipe exception. Associated source comparisons are retained; this is not a warning-free run or a fresh actual-base runtime reproduction. E2E 40559038210 also passed on workflow attempt 1: 13 unique passes in 13 executions, one existing hardware-transfer skip, zero failures. The first built-in test step passed; steps 2/3 were skipped. Build and runtime used the same verified d1ec3668/370b70b7 combined tree. The executed harness was 4c0cbef0 (two commits beyond the previous harness), on iPhone 17/iOS 26.2. The exact pinned hardware spec retains its Blocktank-staging skip. All five full logs, case outcomes and artifact provenance were inspected. Artifact 12761095558 SHA-256 b59616212d303b5441c1053ac883fbd32ba4af1593cb9c98e93c85816accc1a6 matches API/upload/runner download; no local ZIP or runtime-report/video artifact was inspected. Retained diagnostics include 66 stale-element notices, four log-stream restarts, an address-type tap helper retry that reached its expected toast, Node/npm/tool/AppIntents notices and 76 pinned-harness dependency findings. No manual CI retry or warning-free claim. These hosted passes do not close the supplementary native gesture or other acceptance gaps below.

Supplementary native check remains partial: an unpublished animated test host used real Amount with fixed-quote terms that were valid at a historical time and expired before appearance. Header Back closed the owned sheet state with an empty path and no preparation, authorization or submission. The first diagnostic execution failed an exclusive-visible-route readiness assumption and timed out before an edge gesture was delivered. One bounded rerun corrected only that temporary readiness check; actual Amount/path assertions passed, but two edge swipes did not pop Amount and the diagnostic timed out. Both failures and full diagnostics/screenshots are retained. The cause is unproven; completed/cancelled interactive Back, real linked-request setup, dismissal animation and reopening are not accepted by this check. Temporary test code was restored byte-for-byte before signing. No funds, payment, wallet reset or shared-fork access occurred. The repaired existing lifecycle regression is green; this additional gesture gap is not presented as a pass.

A later bounded prerequisite check on the same isolated iOS 26.5 simulator found two native controllers, an enabled recognizer, allowSwipeBack=true and a key window. The temporary host's 390×844 window differed from the 402×874 screen. After correcting only that host geometry and observing native transition state, one cancel-shaped edge drag and one full edge swipe both produced recognizer failed state before any interactive transition. The 100-second diagnostic ended with one failed case/four assertions: Amount/path and sheet state remained, no completed or cancelled pop occurred, and routing/preparation counts stayed zero. The geometry change did not resolve the gap; the reason for gesture rejection is still unproven, and no production defect or fix is inferred. Complete xcresults, action logs, state trace and screenshots are retained. Temporary code was restored byte-for-byte; published source remains 20cfeffa. Further gesture acceptance needs the production sheet presentation to reach that boundary, rather than another unchanged synthetic-host retry.

At b53e300e, the bot reported rendered confirmation, saved/manual coin selection (143/212-sat fees), manual-picker Back, 546-sat fallback/Back/reopen, 547-sat confirmation, cancellation during preparation and LNURL/Savings selection, stopping before payment. Those remain reviewer-reported partial J1/J2 results at that head. Piotr's fee-refresh thread is externally resolved. On October 27, Ovi dismissed João's older changes request, noting its resolved threads and João's absence until November 1; this monitor did not dismiss it. Piotr/Ovi requests remain pending, João's comment retains assignment and his absence is respected, and there are zero current-head human approvals. A bot comment or resolved thread is not approval. Current shipped/provider/funded/physical-hardware/full-linked-request/two-wallet settlement acceptance remains incomplete.

Earlier validation — b53e300e (October 27): The conflict with then-current master d2d9d28d is resolved in a signed normal merge, tested tree 4516c8e1. Master's cache display and test-preference isolation changes are preserved; the USDT UI helper retains its explicit endpoint overrides and unavailable defaults. Final canonical validation passed 2,932 unit tests and 39 UI executions, four fixture skips, zero failures. The previously failing launch test passed locally, which does not establish the cause or a permanent fix for its earlier hosted timeout. Full SwiftFormat 0.64.0 passed (0/846); translations found zero errors and 15 warnings with all 20 inputs identical to actual master. Complete diagnostics and an independent frozen integration review were inspected. All 17 current-base commits verified locally and match GitHub's exact set with verified=true, reason=valid. Hosted unit 40519925128 and integration 40519925139 passed on workflow attempt 1. Unit/UI: 2,932 unit and 39 UI executions passed, four fixture skips, zero failures; all 35 watch-only account tests passed, and the earlier launch case passed in 13.385s. Integration: 30 unique passes in 30 executions, four existing Blocktank staging skips, zero test failures or test-case reruns. Both used verified checkout 3c59760a (d2d9d28d/b53e300e), tree 4516c8e1, exactly matching final local validation. Full artifact digests, execution trees, UI/skip summaries, structured diagnostics and compile/action/runtime logs were inspected. Hosted E2E 40519926046 also passed on workflow attempt 1: 13 unique passing cases in 13 executions, one existing hardware-case skip, zero failures. Its first test step passed; steps 2/3 were skipped. Build and runner checked out the same 3c59760a/4516c8e1 tree, with pinned harness ec01c02f. No manual CI retry. All three existing human requests are preserved; João's changes request remains. The later fee-thread disposition is recorded above. No approval, merge readiness or funded/provider acceptance is inferred.

Final diagnostics retain 16 structured QoS warnings, three AppIntents compile notices, 12 UIKit appearance events, 24 CoreGraphics NaN lines in app stdout, and platform/negative-case runtime messages. Full address stacks are saved; no raw symbolicated backtrace was exported. No fresh base runtime or warning-free claim. Effective runner/helper and built endpoints were verified at unavailable 295xx ports; the fork skipped before funding. J1/J2, physical hardware, full linked-request/two-wallet settlement and rendered 546/547 gaps remain as documented below.

Hosted diagnostics retain the exact prior 286 structured / 571 build-log duplicate-warning multisets. Integration's existing sync helper recovered from a first-attempt Taproot sync timeout inside a passing test; no whole test was repeated. Unit runtime logged a keyboard-queue timeout with Last Exception Backtrace: No stack!, plus socket/haptic and negative-case messages. These observations are retained without a cause or permanent-fix claim. Integration's 22 associated inputs match actual d2d9d28d; of 40 unit-runtime source files, 38 match and AppViewModel/SendConfirmationView retain this PR's changes. No fresh actual-base runtime or warning-free claim.

E2E diagnostics retain 66 stale-element notices, four log-stream restarts (statuses 143/137/124/149), Node/npm deprecations and the pinned harness's 76 dependency-audit findings (8 low, 17 moderate, 44 high, 7 critical). Build/action output retains AppIntents metadata, destination/tool and ignored-base-input notices. All five job logs and 14 case outcomes were inspected. App artifact 12741962657 (141,017,046 bytes) has SHA-256 81978458fa352095f683215ca76521bd97cd1c7fe1a8e73e62477a4e59891883, matching API metadata, upload and runner download logs; the archive was not downloaded locally. The successful workflow did not upload runtime-report/video artifacts. These results do not establish a warning-free run or erase earlier failures and acceptance gaps.

Actual master at the latest audit is 85e9d4c6: three commits and eight paths beyond the hosted base 543d9389. The only overlap with the PR diff is AppScene's foreground-notification hunk, disjoint from this PR's request-preparation timing changes. The PR remains mergeable; no integration was performed. Hosted unit/integration/E2E results cover 543d9389/20cfeffa; the newer 85e9d4c6 combined tree is unvalidated. Local results retain their separate repaired PR-tree attribution.

Earlier validation — 20f54449 (October 27): The unit CI crash at 811b4335 was traced to concurrent access in the watch-only test fake: an assertion read its tracking Set while the authorization task inserted into it. The two affected tests now await the existing actor-isolated gate after tracking has finished. All assertions and all 35 tests remain; production code is unchanged. The coordinated repair passed 35 focused tests, 2,928 unit executions and 39 UI executions, with four explicit fixture skips and zero failures. Full SwiftFormat 0.64.0 passed (0/845 files need changes); translation validation found zero errors and 15 warnings with unchanged actual-base inputs. Complete diagnostics and independent frozen review were inspected. All 16 commits against actual master b666ccb1 verify locally and match GitHub’s exact commit set with verified=true, reason=valid. Hosted integration 40501754529 passed on attempt 1: 30 unique tests passed in 30 executions, four existing Blocktank staging skips, zero failures or retries. Verified checkout a7cb2f52 (b666/20f54449) has tree c1e3cd27, exactly matching final local validation. The archive digest, all 34 full case summaries, tree, action/runtime output and compile/test log were inspected. Structured warning/error arrays are empty; retained 571 duplicate-symbol compile warnings match the prior integration multiset, alongside the version-probe broken-pipe exception, tool/destination notices and embedded RGS/node/address-search/teardown/fixture/launch-metric messages. All 22 associated source inputs match actual base; no fresh base runtime or warning-free claim. Hosted unit 40501754193 subsequently failed on attempt 1: all 2,928 unit executions (including all 35 watch-only tests) passed, but UI had 38 passes, four skips and one failure in BitkitUITests/testExample at app.launch(). XCTest timed out acquiring a background assertion; the failure video shows splash then Terms. Root cause remains unproven. E2E 40501754542 passed 13 unique cases in 13 executions on its first step, with one existing hardware skip and zero failures. The complete logs, artifact digests, test trees, failure video and diagnostics were inspected; no manual CI retry. Both use the verified a7cb2f52/c1e3cd27 checkout. The 286 structured/571 compile warnings match the preceding unit run; E2E retains 70 stale-element notices, log-stream restarts and pinned-harness dependency warnings. All three existing human requests are preserved; João’s changes request and Piotr’s fee-refresh thread remain in force.

Earlier validation at 9a807ef3 (October 27): Conflicts with actual master b666ccb1 are resolved in signed normal merge 9a807ef3, tested tree 0ce4fc92. The combined tree preserves fixed-quote amount/expiry/dust/fee and authorization safeguards, duration-only diagnostics, and master's SDK/request/proof lifecycle changes. Final local gates passed: 15 focused tests, 2,928 unit executions and 39 UI executions, four fixture skips, zero failures. Full SwiftFormat 0.64.0 reports 0/845 files needing changes (81 skipped); translations report zero errors and 15 warnings whose inputs match actual master. Complete test trees, compile/action logs, 19 structured QoS warnings, 21 raw backtraces and remaining platform notices were inspected and retained; this is not a warning-free claim or runtime baseline reproduction. All 14 commits against actual master verified locally and matched GitHub's exact set, with every signature verified=true, reason=valid. Hosted unit, integration and E2E workflows all passed on attempt 1 at the exact locally tested merge tree. Hosted unit/UI: 2,928 unit and 39 UI executions passed, with four fixture skips. Integration: 30 passed and four Blocktank skips. E2E: 13 passed and one existing hardware-case skip; its first test step passed and steps 2/3 were skipped. Full diagnostic and helper-retry limits are recorded below. Full evidence is recorded below. All three existing human review requests remain pending; no changes request was dismissed.

The canonical run verified built RPC/bundler endpoints at unavailable 29545/29546 and the fork skip before app launch or funding. No shared fork access, funding or transfer occurred. Funded/provider/hardware/two-wallet and rendered 546/547 acceptance gaps remain; the historical incidents and hosted retry/skip limits below retain their original attribution.

Earlier validation (October 26, retained at its original head): Conflicts with actual master b8b7f3d9 are resolved in signed normal merge 63ea2034 (tested tree 21fb1809). The combined tree preserves master's wallet-switch lifecycle fix and this PR's quoted-payment guards. Final local validation passed: 2,894 unit executions and 39 UI executions, with four fixture skips and no failures. All 13 current-base PR commits verify locally and on GitHub. Hosted unit/integration checks passed. E2E completed successfully after two built-in retries of the Lightning case; 13 unique cases passed and one hardware-transfer case stayed skipped. The two earlier Lightning timeouts remain unexplained. J1/J2 and the funded/provider/hardware acceptance gaps below remain open.

Fixed quoted Bitcoin requests open in the existing confirmation screen when no separate funding-source selection is needed. A fixed $2.50 Savings request skips the amount screen with disabled controls. Requests offering LNURL Spending alongside Savings retain the existing funding-choice screen because LNURL confirmation has no wallet selector.

Description

  • Keeps requests with missing or expired conversion terms on the existing Amount rate-unavailable view, without binding payable terms.
  • Binds the exact request terms before opening confirmation, retaining wallet-valued fiat display, quote expiry and changed-amount checks.
  • Keeps valid Spending/Savings switches on Confirm with the new quoted amount and refreshed fees. Missing, expired or below-minimum terms return to Amount.
  • Lets Back from manual coin selection stay on Confirm; swiping still requires coins before authorization.
  • Applies the saved automatic coin-selection algorithm before fee review, keeps manual coin selection, and preserves retry after fee or coin-preparation failures.
  • Preserves asset selection when USDT is enabled and actually offered, and Savings/Spending selection for requests offering LNURL and on-chain Bitcoin. Single-method LNURL requests still open confirmation directly. Wallet sync, fees, authorization and duplicate-payment protections remain in place.
  • Adds duration-only preparation diagnostics (resolveMs and setupMs) to separate endpoint resolution from subsequent setup without logging payment details.

Out of Scope

Design

Reuses the existing send confirmation without changing its layout. Android's screen map identifies Send (Paste) (On-chain) → Confirm Send Onchain in the Bitkit design file. The exact current frame node is unverified.

Preview

Before: Preparing → Amount (locked) → Confirm.
After, when no separate funding choice is required: Preparing → Confirm. LNURL + Savings keeps its existing selection step.

QA Notes

Setup

Use the existing fixed-quote journey prerequisites: two disposable identities, a fresh fixed $2.50 Bitcoin quote, funded regtest Savings and a usable Spending channel for live switching. For the minimum boundary, use controlled rates yielding exactly 546 and 547 Savings sats as specified in journeys/paykit-quoted-usdt.xml. No funded boundary/settlement run is claimed below.

The XCTest USDT fixture is separate: UsdtWalletUITests now defaults to unavailable loopback endpoints. An explicitly owned disposable fork must be selected through test-runner BITKIT_TEST_USDT_RPC_URL / BITKIT_TEST_USDT_BUNDLER_URL; its test funds its wallet. BITKIT_TEST_USDT_OFFLINE_RPC_URL must be unavailable. Never use another task's shared fixture. See journeys/README.md.

Journeys

  • J1: new journeys/payment-requests/fixed-quoted-confirmation.xml — LNURL/Savings selection, quoted amount and wallet valuation, saved/manual coin selection, cancel/reopen, delayed preparation, insufficient funds, expiry and BTC/USDT choice. The live two-wallet journey remains to be driven; automated coverage is listed below.

  • J2: updated journeys/paykit-quoted-usdt.xml — iOS dot-decimal checks in comma-region settings preserve exact values, cents, trailing zeros and six-decimal precision. Full J2 remains unverified; controlled 546/547-sat Spending/Savings boundary: below-minimum iOS terms return to Amount with Continue disabled; exact 547 remains on confirmation. Android retains confirmation but disables authorization below its equivalent minimum. Source parity checked; this revised live boundary journey is unrun.

Manual Tests

N/A

Earlier master integration and local validation — 63ea2034

The normal integrations retain the previous dust and fixture-endpoint fix, merged #1003's Receive coverage, and merged #1098's single wallet-switch run and resumed-preparation behavior. Quote identity, exact amounts, expiry, selected-rail minimums, routing-fee cancellation and authorization checks remain intact. No assertions were removed or weakened, and no new exclusion or authored test was added by conflict resolution.

  • The affected confirmation lifecycle class passed 15/15. The final workflow-selected canonical run passed 2,894 unit executions and 39 of 43 UI executions; three opt-in Trezor cases and the unavailable local fork case skipped. The full execution tree includes 30 launch configurations grouped under one summary case. Receive/invalid-payment coverage passed in 56.723 seconds and comma-region manual entry in 44.006 seconds. There was no retry.
  • This was the combined b8b7f3d9 tree, using Xcode 26.1 and an iPhone 17 Pro Max simulator on iOS 26.1, one worker and private build/package paths. The six canonical integration exclusions match the workflow; separate integration, E2E, provider or hardware completion is not claimed by this local run.
  • Exact UI helpers, runner forwarding and built-app endpoints were checked. Unfunded endpoints were reserved without listeners at isolated ports; the runtime fork availability check failed at the configured port and skipped before app launch or funding. The shared fixture was not accessed. This safe skip does not validate the historical failed funded journey.
  • Final SwiftFormat 0.64 reports 0/844 files need changes (79 excluded). Translation validation reports zero errors and 15 warnings, whose full messages and source/input comparison match actual master. XML and whitespace checks passed.
  • Complete structured and raw diagnostics were inspected: 13 runtime QoS warnings remain (four unit and nine UI records), plus two AppIntents compile notices. SDK initialization/synchronization, UIKit appearance, locked-device discovery, simulator haptics and injected negative-case diagnostics are retained. No introduced lint, formatting or Swift compiler finding was identified; this is not a blanket warning-free claim or an actual-base runtime reproduction. The test-without-building report alone does not prove a warning-free compile.
  • Independent review of the frozen integrations found no actionable issue. All 13 commits against actual master b8b7f3d9 matched the local range and reported verification.verified=true, reason=valid on GitHub. Both integrations are additive signed commits, with authors and prior work preserved.

The earlier 536abb44 candidate also passed its own canonical run before master advanced, but that result is distinct from the final run above. The earlier shared-fork incident and incomplete rendered 546/547-sat boundary, provider, physical-biometric, hardware and two-wallet settlement journeys remain explicitly unvalidated.

Earlier dust/fixture fix validation — 9acc68e2

Quoted Savings terms now require at least 547 sats before confirmation can rebind them. Exact valid amounts, expiry, quote identity and authorization guards are preserved. This addresses Piotr's finding.

  • Original production failed the new 546-sat regression; the fixed confirmation lifecycle class passed 15/15, including exact 547-sat acceptance. These hosted SwiftUI tests prove navigation/terms state, not a rendered Amount/swipe/PIN journey.
  • Full canonical selection: 2,881 unit executions passed; 43 UI executions had 39 passes, three Trezor skips and one funded-fork failure. That older test hard-coded shared ports despite runner overrides, called test_fundWallet, and observed 1,000 fake USDT. Quote preparation failed before confirmation; no transfer/settlement is evidenced. This was an isolation mistake, not accepted funded QA. No fixture repair, reset or compensating write followed.
  • The test class now uses explicit selected endpoints and unavailable-loopback defaults. Every existing test and assertion remains. The final affected class rebuilt and ran with the actual isolated port confirmed in its runtime log: two unfunded UI cases passed, one unavailable-fixture case skipped. No complete canonical rerun followed in that earlier pass; the final combined-tree run is documented above, and the original failure remains recorded.
  • Final SwiftFormat 0.64: 0/843 files need changes. Translations: zero errors / 15 previously verified base-equivalent diagnostics. XML/whitespace checks passed. Complete structured/raw reports inspected: 19 canonical QoS warnings and three in the affected UI run remain; SDK, negative-test and simulator notices are retained, without a blanket warning-free claim.
  • Independent frozen review and bounded endpoint recheck passed. All 11 PR commits against actual master 2ce21025 verify locally and as verified=true, reason=valid on GitHub. This is an additive signed commit, with no history rewrite. At that point hosted CI was blocked by the master conflict; the current signed integration above supersedes that hold.

J1/J2 and live provider, physical biometric, hardware and two-wallet settlement gaps remain open. The final safe fixture skip does not validate the failed funded journey.

Automated Checks

  • updated PaymentNavigationHelperTests.swift — exact converted amount, expiry, real BTC/USDT choice, routing, unavailable/expired-rate fallback and reviewed-term mismatch protection.
  • updated PaykitPaymentRequestServiceTests.swift — hosted confirmation waits for wallet sync before rejecting a below-minimum quote.
  • updated SendConfirmationLifecycleTests.swift — hosted fixed-request preparation, rail-specific quoted amounts and fees, invalid-term fallback, manual picker Back/retry, UTXO failure and fee failure/recovery.
  • ran the final focused simulator pricing, navigation, confirmation and LNURL suites — 48 tests passed. The LNURL selection regression failed before the fix and passed afterward, including different 2,500-sat Lightning and 5,000-sat Savings quotes.

Historical validation before ad30f8be: the 48-test focused result above predates this integration. The prior all-target compile, whitespace-only ActivityRow formatting repair, 14 verified base translation warnings and CoreSimulator boot blocker remain historical evidence.

Historical author validation at ad30f8be (tested tree f76406a5, integrated master 53c5ec0d):

  • xcodebuildmcp simulator test --json focused selection: 439 passed, zero failures/skips. The canonical direct Xcode fallback used the same six CI integration exclusions and one worker: 2,881 units plus 43 UI executions (39 passed, four fixture skips), zero failures. The UI skips require the disabled Trezor emulator or local Arbitrum fixture; they are not passes. Existing unfunded iPhone 17 Pro simulator, iOS 26.1.
  • Complete xcresult and raw diagnostics inspected: zero build errors/warnings; 13 runtime QoS warnings during SDK initialization, with implicated tests, SDK wrappers and dependency pins verified unchanged from tested master. The first canonical CLI build succeeded but could not launch because its exported test products disappeared; its complete failure diagnostics are retained. The cause remains unexplained. The task-local direct Xcode run then completed successfully, including natural diagnostic collection.
  • Final SwiftFormat 0.64.0: 0/843 files require formatting. Translation validation: zero errors and the same 15 warning diagnostics as exported 53c5ec0d. Independent combined integration review found no verified issue; PR patches and master recovery/sign-out behavior are preserved.
  • Master subsequently advanced to e4bf44a2 through maintainer merges of fix: unify usdt balance and amount presentation #1077/fix: reuse received payment artwork layout #1092. Those 36 paths are disjoint from this PR's 13-path delta; a clean read-only merge retained the exact PR blobs and all current-base changes. Local runtime evidence applies to the published tree. Hosted unit and integration checks subsequently passed on CI merge a6deea4d, whose tree 053473bc exactly matches that reviewed combination with e4bf44a2.
  • The normal merge is signed. All 10 commits against actual base e4bf44a2 verified locally and the exact GitHub commit set reports verified=true, reason=valid for every commit.

Hosted validation for head ad30f8be:

  • Unit workflow 40030073120, workflow attempt 1: 2,880 unit tests and 43 UI executions (39 passed, four fixture skips), zero failures. The one-unit difference from the local run is the locale-input test removed by merged fix: unify usdt balance and amount presentation #1077 in the newer tested base. Complete logs, xcresult and build diagnostics were inspected. The structured report contains 286 duplicate-native-symbol warnings; its exact warning multiset and all 571 repeated build-log diagnostic nodes match actual master e4bf44a2 run 40028545050. AppIntents metadata notices also occur on both. Project, workflow and dependency pins match that base; no new warning was found. The hosted xcresult records no runtime warnings.
  • Integration workflow 40030072972, workflow attempt 1: 34 cases / 34 executions passed, zero failures/skips, with no test retry. Full log and xcresult inspected. This report is for test-without-building and does not independently establish a warning-free compile.
  • E2E workflow 40030073870 completed successfully on workflow attempt 1, with 14 unique cases / 15 executions across two built-in test attempts. The first attempt passed 13 cases, including LNURL msat pay/withdraw and all four hardware cases. @lightning_1 timed out after 30 seconds waiting for “Transfer Initiated” after closing a restored channel (lightning.e2e.ts:276 at pinned E2E revision bcf91fee). The second attempt reran only that case and passed; the other 13 were skipped and attempt 3 did not run. Complete workflow logs and per-case reporters were inspected. The close-confirmation view, transfer/wallet view models, Lightning service, toast components and E2E workflow match actual base e4bf44a2; this comparison does not prove that the timeout is pre-existing or establish its cause. After the successful retry, the workflow skipped runtime/screenshot/video artifact upload; only the built app artifact remains, so the saved failure visuals and app logs could not be inspected. Startup log-stream wrapper restarts are separate from the test retry. No manual workflow retry or code change was made, and the passing retry does not establish a permanent fix. All CI workflows for that earlier tested revision passed; this historical result does not describe the latest head. The full payment-journey limitations below remain.

External device QA at exact head ad30f8be is attributed to review 5635086275, which reports no introduced finding. On an iPhone 17 Pro / iOS 26.5 simulator with regtest funds, the reviewer checked direct 5,000-sat confirmation and a 143-sat fee, Largest First selection, manual-picker Back, cancellation during preparation, LNURL/Savings choice, LNURL-only direct confirmation, BOLT11/Savings switching at 2,500/5,000/2,500 sats, the fixed $2.50 BTC/USDT choice, and insufficient/expired-term guards. Swiping after expiry left both Bitcoin balances unchanged; fresh terms required confirmation. Hosted assertions covered invalid-term and fee/UTXO failure recovery. Videos and exact limitations are in that review.

This external subset does not report a completed two-wallet fixed-quote settlement. USDT funding/submission, subscription authorization and hardware-wallet signing were not exercised; provider and physical-biometric validation remain incomplete. The full journey above stays unchecked. That earlier author-validation pass made no funded payment or wallet reset. Simulator data was preserved and the native window released.

Models used

For this integration and validation update (verified session metadata; earlier implementation attribution is not inferred):

  • Planning/scoping: gpt-6-astra (reasoning: xhigh)
  • Implementation: gpt-6-astra (reasoning: xhigh)
  • Review: gpt-6-astra (reasoning: xhigh), gpt-6.1-sol (reasoning: high)

Hosted unit/integration completion — 63ea2034, 2026-10-26

Unit40410064946 and integration40410064928 succeeded on workflow attempt 1. Both checked out merge a9eb51a06724ccd6a0cbff62487fcd8b5bf0bc77, with verified parents actual master b8b7f3d9 / published 63ea2034 and tree 21fb1809bb1af4f626c437015f8780139a9320b7, exactly matching the locally tested published tree. Unit: 2,894 passed. UI: 43 executions, 39 passed / four fixture skips / zero failures. Integration: 30 passed / four existing Blocktank staging-retirement skips/zero failures, with no retry execution. The four integration guards are byte-identical to actual master. Skips are not successful provider/hardware journeys.

Artifact ZIP digests, all execution refs/skip reasons, complete logs and structured build/action reports were inspected. Unit has 286 structured duplicate-symbol warnings and 571 build-log warning messages; the structured multiset equals the historical b261 baseline. Project/pin/unit/integration workflow inputs match actual b8, but no actual b8 runtime baseline was run. Both full compile logs retain 571 duplicate-symbol warnings plus tool/multiple-destination notices; the integration version probe retains its broken-pipe exception. RuntimeWarnings/testFailures arrays and action warning/error nodes are empty. Integration's test-without-building status does not prove warning-free compilation. Xcode 16.4 compile paths, iPhone 16 / iOS 26.2.

E2E workflow 40410065359 completed on workflow attempt 1 using the same verified a9eb51a merge/tree above, Xcode 26.2 (17C52), iPhone 17 / iOS 26.2 and E2E revision ec01c02ff611797c112e65b5c87c936d25720bac. Of 14 selected cases, 13 unique cases passed across 15 executions and three built-in test attempts. Attempt 1: 12 passed, @lightning_1 failed, and hardware-wallet transfer-to-Spending stayed skipped. Attempt 2 reran only Lightning and failed; attempt 3 passed it. Both failures timed out after 30 seconds waiting for ActivityShort-0 to contain 9 000 after closing the restored channel (lightning.e2e.ts:287). No root cause or actual-base runtime reproduction is established; passing the third attempt does not establish a fix. The hardware skip is explicit in the pinned test source because Blocktank staging no longer creates orders.

All five job logs, per-case reporters and complete available diagnostics were inspected. The workflow marks the first two steps successful through continue-on-error; their raw exit-1 failures remain recorded here. Failure screenshots/videos were named in the runner log but were not uploaded after final success; only the app artifact remains, so those visuals and runtime reports could not be inspected. Log-stream restarts (143/137/124/124/143/143), 80 stale-element warnings, the address-type helper retry, dependency notices/audit output and the AppIntents/multiple-destination build notices remain retained. No new source change or manual CI retry was made. Rendered 546/547, funded/provider/hardware/two-wallet and other documented acceptance gaps remain explicit.

Hosted integration completion — 9a807ef3, October 27

Integration workflow40453530944 passed on attempt1: 30 unique tests /30 passing executions, four existing Blocktank skips, zero failures. All34 case summaries and the full execution tree confirm no retry execution. Empty runtimeWarnings/testFailures arrays and no structured action warning/error nodes. Embedded action output still contains RGS gossip failures, node/address-search/teardown diagnostics and the orphaned-RN-keychain fixture: all45 exact diagnostic groups were inspected, and all nine associated source/test files match actualb666. Hierarchy copies duplicate these log lines; this is not a warning-free or actual-base runtime reproduction claim. Skipped refund-address lifecycle/create-CJIT/orders/channel-purchase cases cite the closed staging service; their source and workflow match actual master.

The job checked out merge a27dd5a89eba7dd6c2fd9e753131800383c565cc, with verified b666ccb1 / 9a807ef3 parents and tree 0ce4fc92, exactly matching the locally validated tree. Artifact digest, full job log, test trees and structured diagnostics were inspected. Compile output retains571 duplicate-symbol warnings (286 unique), the same complete multiset as the earlier actual-cc5 baseline. Current project/pins match actualb666, but no new b666 runtime reproduction is claimed. Test-without-building buildStatus=notRequested/no exported build log is not warning-free evidence; the full separate compile output is retained. The version-probe broken-pipe exception and tool/destination notices remain explicit. Runner: Xcode16.4 compile environment, iPhone16/iOS26.2; local26.1 results remain distinct.

Unit and E2E completion is recorded in the following addendum; no manual retry. Funded/provider/hardware/two-wallet/rendered546/547 acceptance and historical retry/skip limits remain. Full report: /private/tmp/usdt-monitor-20261027-0534/ci/completion-report.md.

Hosted unit and E2E completion — 9a807ef3, October 27

Unit 40453530938 and E2E 40453531272 passed on workflow attempt 1. Unit, E2E build and E2E runtime logs identify merge a27dd5a89eba7dd6c2fd9e753131800383c565cc, with verified actual-master b666ccb1 / published 9a807ef3 parents and tree 0ce4fc929ce5dcb22d64b5800dc7451466705bb3, matching final local validation.

  • 2,928 unit executions passed; UI: 39 passed, four fixture skips, zero failures. Thirty launch configurations are grouped under one summary case. The three opt-in Trezor cases and the isolated Arbitrum-fork case were skipped; the fork skip occurred before app launch or funding. Receive passed in 88.878 s and comma-region manual entry in 53.286 s. These passes do not explain historical startup failures.
  • E2E: 13 unique cases passed in 13 executions, one existing hardware-case skip, zero failures. Test step 1 passed; steps 2/3 were skipped. Includes Lightning, LNURL msat pay/withdraw, boost, address-type, send and three Trezor-emulator cases. Hardware transfer-to-Spending remains explicitly skipped because Blocktank staging no longer creates orders, at E2E revision ec01c02f. This does not establish physical-hardware acceptance.

The unit archive digest, complete execution trees, all UI summaries, structured build/action diagnostics, all six unit/E2E job logs and available artifact metadata were inspected. Unit structured runtimeWarnings/testFailures are empty. Its 286 structured duplicate-symbol warnings and 571 build-log warning messages exactly match the recorded actual-cc5 multisets; current build inputs match b666, but no fresh b666 runtime baseline was run. Embedded action output retains negative-case offline/auth/storage/identity/hardware/durable-recovery and fee/UTXO diagnostics, synthetic Blocktank UUID-order failures, and simulator haptic/socket notices. Associated source comparisons and complete groups are retained; this is not a warning-free or blanket pre-existing claim.

E2E retains 68 stale-element notices, log-stream restarts 143/137/124/124, and one address-type tap/toast helper retry, separately from test retries. Tool/AppIntents/Node/npm notices remain; the pinned external E2E harness reports 76 dependency vulnerabilities (8 low, 17 moderate, 44 high, 7 critical), identical to its earlier same-revision log, with no uploaded advisory report. Only the app artifact was uploaded, and its metadata digest matches the build-upload and runner-download logs; no runtime-report or video artifact was available after success. Unit used Xcode 16.4 / iPhone 16 / iOS 26.2; E2E used Xcode 26.2 / iPhone 17 / iOS 26.2.

No manual CI retry or source change. Earlier Lightning failures/recovery and fixture incidents retain their original attribution. Funded/provider/physical-hardware/two-wallet settlement and rendered 546/547 acceptance remain incomplete. Full evidence: /private/tmp/usdt-monitor-20261027-0636/ci/completion-report.md.

Quoted review Back fix — 811b4335, October 27

Returning from fallback Amount to a fixed quoted request’s Confirm root now closes that presentation, so Back no longer immediately pushes Amount again. The first fallback—including initially unreviewed terms—still requires amount review; valid terms reset the marker. The existing presentation-scoped dismissal preserves a replacement sheet. Seven production lines changed; two existing lifecycle tests were extended with a true Confirm-root host, with no new/removed tests. The existing journey now includes the iOS fallback Back and manual-coin return steps.

Unchanged 9a807ef3 failed the Back/dismiss assertions for all four invalid quote variants; the frozen fix passed all 15 focused lifecycle tests and the workflow-selected canonical suite: 2,928 unit + 39 UI executions passed, four fixture skips, zero failures. Three Trezor cases remain opt-in; the isolated fork skipped at unavailable 29546 before app launch/funding. Exact helper/runner forwarding and built endpoints were checked. SwiftFormat 0.64: 0/845 files need changes; translations: zero errors/15 warnings, with all 21 relevant inputs identical to actual b666ccb1. Independent frozen-diff review found no verified issue. All 15 current-base PR commits verified locally and match GitHub’s exact set with verified=true / reason=valid. Normal additive push; no history rewrite.

Complete logs, execution trees and 47 warning/UI/skip summaries were inspected. The 13 structured QoS warnings and 16 raw backtraces are different diagnostic surfaces; raw traces cover Pubky/mainline initialization and LDK status/sync. Two AppIntents compile notices, 11 UIKit appearance notices, simulator haptic diagnostics and 48 CoreGraphics NaN notices during passing USDT Receive/manual-entry cases remain recorded. No NaN backtrace or fresh actual-base runtime reproduction is available; those cases do not enter the changed quoted Bitcoin Confirm path. The changed source’s UTXO-unavailable logs come from an unchanged negative-case method. No blanket warning-free/pre-existing claim. Test-without-building metadata is not compile-clean evidence.

The manual-coin regression also passed with Confirm as the actual navigation root: after Continue to the second confirmation and Back through the picker, the original root recalculated using the selected coin without a fee-rate change. This proves the controlled fee-preparation call, not rendered fee text or a funded/provider journey. New-head hosted CI is pending; full linked-request/provider/physical-hardware/two-wallet settlement and rendered 546/547 acceptance remain incomplete. Previous evidence retains its original head attribution. Planning/implementation/review for this fix: gpt-6-astra, reasoning xhigh, verified from actual execution metadata.

Hosted integration at 811b433

Integration 40486729719 passed on workflow attempt 1: 30 tests passed in 30 executions, four existing Blocktank staging skips, zero failures or retry executions. Checkout 156e44db has actual b666ccb1 / 811b4335 parents and tree e5409e4f, exactly matching final local validation. The archive digest, all 34 case summaries, test tree, action/runtime output and full compile/test logs were inspected.

Structured warning/error arrays are empty, but test-without-building is not proof of warning-free compilation. Retained 571 duplicate-symbol compile warnings (matching the prior hosted integration multiset), the version-probe broken-pipe exception, tool/destination notices and embedded RGS/node/address-search/teardown/fixture diagnostics. Associated inputs match actual base; no fresh base runtime reproduction is claimed. The subsequent unit failure and E2E completion for this parent are recorded in the crash-repair addendum below. Provider/funded/physical-hardware/full linked-request/two-wallet/rendered-fee acceptance limits and Piotr's open finding remain unchanged.

CI crash repair and diagnostic limits — 20f54449

At parent 811b4335, unit workflow 40486729701 built successfully but had 2,927 unit passes and one crash in testAuthorizationSerializesReconciliationUntilTrackingStateIsPersisted; UI had 39 passes/four skips. The crash report shows XCTAssertEqual reading trackedAccountIndexes concurrently with the fake’s insertion. The test and service inputs match actual b666ccb1, but no actual-base runtime reproduction is claimed. The narrow synchronization repair was coordinated with the file’s owning work before editing. Parent E2E 40486730476 passed 13 unique cases in 13 executions, with one existing hardware-case skip and no failures. Both checked out merge 156e44db / tree e5409e4f (b666/811b); these results retain that parent attribution, not the new head.

Final local repair validation used tree c1e3cd27, Xcode 26.1 and the owned iPhone 17 Pro Max / iOS 26.1 simulator with one worker and private products. The complete execution tree confirms 2,928 unit + 39 UI passes; the summary groups 30 launch configurations into one. Three Trezor opt-in cases and the unavailable isolated fork skipped. Built endpoints and runner/helper forwarding were verified; runtime fork availability failed at unavailable 29546 before app launch or funding. No shared fixture was accessed.

Full reports retain 16 structured QoS warnings and 16 raw backtraces (14 Paykit/Pubky initialization, two LDK status), two AppIntents compile notices, UIKit appearance and simulator keyboard/haptic/graphics/network notices, and negative-case storage/auth/recovery/fee diagnostics. Relevant QoS wrappers/pins match actual b666ccb1; no fresh base runtime or warning-free claim. The passing UI run emitted 24 CoreGraphics NaN notices; the earlier identical-production 811b run retains 48. No causal or permanent-fix claim is made for those notices. Hosted parent E2E retains 68 stale-element notices, helper/log-stream retries, pinned-harness dependency warnings and absent runtime-report upload. These checks do not complete J1/J2, production-provider, physical-hardware, funded settlement, full linked-request or rendered 546/547 acceptance.

Native Back prerequisite diagnostic — October 27, 21:24 UTC

A temporary, isolated diagnostic at 20cfeffa now mounted the production SendSheet through the same modal binding as MainNav, with setup-complete state seeded and unavailable fixture endpoints. This excludes real linked-request setup and persistence, authorization, and funded/provider acceptance. The modal, two-controller stack, enabled native recognizer, key window and hit-test were verified. One cancellation-shaped drag and one completion swipe still failed before any interactive transition began; the diagnostic failed all four relevant assertions. Adding the modal wrapper did not close the gap or establish a production defect.

A standard UIKit navigation control on the same iOS 26.5 simulator did reach and cancel a genuine interactive Back transition. Its first run and one bounded recovery both expired before the completion gesture could be delivered because of turn interruption/context compaction; both remain failed, with two assertions each. No completed native pop or J1 acceptance is claimed. Full logs, nested diagnostics, failed assertions and screenshots are retained, including UIKit transition fallback and platform/accessibility notices. The temporary source was restored byte for byte, the checkout is clean, and scoped simulator services are stopped with data preserved. No production edit, source push, CI rerun or additional review request was made.

Native Back control follow-up — October 27, 22:28 UTC

The final bounded recovery of the interrupted UIKit control passed using identical built products: both cancelled and completed native transitions were observed, and the stack returned to its root. Fresh UI and log guards kept both gestures inside one live test window. Earlier interrupted failures remain retained.

The remaining prerequisite also fails in minimal SwiftUI and UIKit modals with hidden navigation bars and no payment/request/wallet objects (four and three assertions respectively). Each received one cancellation drag; the guard withheld completion when no interactive transition began. Separate SendSheet observations found both the edge and iOS 26 content-pop recognizers enabled, with delegates permitting a start. These values were unchanged by temporarily showing the bar. Both shared swipe-extension files match current master 85e9d4c6; this is source comparison, not a base runtime test.

The controls do not yet separate navigation-bar visibility, modal presentation and runtime behavior, so no product cause/fix or completed J1 native Back acceptance is claimed. All logs, nested diagnostics, failure stacks and screenshots are retained. Temporary source is restored exactly and the checkout remains clean at 20cfeffa; no source push, CI rerun or reviewer notification. Existing provider, funded, physical-device and full linked-request limits remain.

Native Back visibility control — October 27, 23:28 UTC

The isolated UIKit modal control passed its one selected test in 8.209 seconds on the same owned iOS 26.5 / Xcode 27.1 setup after changing only the navigation bar from hidden to visible (plus its state assertion/log and test name). Native cancellation and completion callbacks were [true, false], and the stack returned to its root. No SendSheet/payment/request/wallet objects participated. The prior hidden-bar modal failures remain retained.

This isolates visibility-dependent behavior in that control; it does not establish the production cause/fix or complete SendSheet J1 native Back acceptance. Shared swipe code matches actual master 85e9d4c6, by source comparison only. A shared-navigation follow-up requires ownership reconciliation. Full build/test trees, nested diagnostics, raw logs and screenshots were inspected. Structured warning/error arrays are empty; SDK/accessibility notices and two UIKit transition-fallback notices remain. Temporary source is restored exactly, the checkout remains clean at 20cfeffa, and scoped simulator services are stopped with data preserved. No production edit, source push, CI rerun or reviewer request. Existing provider, funded, physical-device and linked-request limits remain.

@ovitrif ovitrif added this to the 3.1.0 milestone Oct 19, 2026
@ovitrif

ovitrif commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

Triage: milestone 3.1.0, the iOS twin of synonymdev/bitkit-android#1581. Fixed Paykit requests go straight to confirmation instead of a mostly disabled amount screen. It isn't a release blocker, and it closes no issue, so this PR carries the milestone itself.

@greptile-apps

greptile-apps Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The changes reach incoming Paykit request preparation and payment confirmation.

The PR appears safe to merge, with a non-blocking issue where direct on-chain confirmation ignores the saved coin-selection algorithm.

Findings

  1. P2 Saved coin choice is ignored ▶

Summary

Fixed quoted Bitcoin requests now bind their exact payment terms before opening confirmation, skipping the locked amount screen when no enabled USDT choice is offered.

  • Fixed Bitcoin requests open straight to confirmation when no asset choice is needed.
  • Paykit request preparation records separate resolution and setup times.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Resolve incoming request] --> B{USDT enabled and offered?}
    B -->|Yes| C[Choose asset and review amount]
    B -->|No| D[Bind exact Bitcoin terms]
    C --> E[Confirmation]
    D --> E
    E --> F[Wait for wallet readiness]
    F --> G[Review fee and approve]
    G --> H[Recheck request and send]
Loading

Reviews (1) · Last reviewed commit: "fix: confirm fixed Paykit requests direc..." · Reviewed by Greptile

Comment thread Bitkit/ViewModels/AppViewModel.swift Outdated

@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.

1 finding (1 P2). Opens a fixed-quoted Bitcoin Paykit request straight on Confirm by preparing the payment terms during scan handling. Reviewed 72d89f3, full tier, reasoned from the code and CI (iOS does not build here).

What I checked, and 3 candidates I ruled out

Read in full: handleScannedData (asset-selection branch), PaymentNavigationHelper.openPaymentSheet / appropriateSendRoute / contactPaymentRoute, PaykitRequestPricing.payment(requested:endpoint:period:at:quoteId:), the incoming-request preparation block in AppScene.swift, SendSheet.shouldShowSyncOverlay and the post-setup validation
Call sites traced: every handleScannedData(_, claimedContactPaymentContext:) caller (AppScene, SubscriptionsView, SendSheet retry, SendContactSelectView, AddContactView, PaymentNavigationHelper)
CI: validate, detect-changes and Greptile green. Run Tests, Integration Tests and build-local were still pending when I reviewed.

Ruled out

  • Sync overlay bypassed on Confirm: shouldShowSyncOverlay now exempts asset selection only while currentRoot == .amount, so Confirm waits for the node. The renamed testBelowMinimumQuotedRequestWaitsForSyncBeforeFallback asserts that.
  • Stale terms after a wallet switch: the send path rechecks terms == app.paykitPaymentTerms and isValid before paying (SendConfirmationView.swift:1688), and resetSendState clears both fields (asserted in the new test).
  • USDT-capable requests losing the asset picker: the block is gated on !canSelectUsdt, and testDollarRequestOffersAssetSelectionOnlyWhenUsdtIsEnabled covers both settings.

Merge confidence: 4/5, one P2 on a narrow path, and the test suites had not finished at review time.

Comment thread Bitkit/ViewModels/AppViewModel.swift Outdated
ovitrif

This comment was marked as resolved.

@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.

Requesting changes: one MEDIUM (Back from the manual coin picker), inline with a failing test.

Checked and clean:

  • contactPaymentRoute maps .quickpay and .amount to Confirm for incoming requests, so QuickPay can't auto-pay a request whose terms are now bound.
  • Scanner and Enter Manually call appropriateSendRoute without a claimed context, so the terms branch never runs there.
  • Converted initial subscriptions stay manual (conversion == nil).
  • returnToAmountReviewIfNeeded re-validates terms on a wallet switch.
  • The rate-unavailable case is fixed by 963d810.

The android twin synonymdev/bitkit-android#1581 prepares coin selection at swipe, so this shape is absent there.

fixed-quoted-confirmation.xml is byte-identical to Android.

Device gate: not run (MEDIUM open).

Comment thread Bitkit/Views/Wallets/Send/SendConfirmationView.swift

Copy link
Copy Markdown
Contributor Author

@ovitrif Fixed the Spending/Savings switch in 6c79bae. Already-reviewed Bitcoin requests stay on Confirm with the selected rail’s exact quoted amount, refreshed fees and a required swipe. Missing/expired terms and amounts below the Lightning minimum return to Amount; BTC/USDT selection and hardware safeguards remain. The hosted test switches between 2,500 and 5,000 sats and verifies fee preparation, with separate fallback cases. The journey now matches Android and also covers Back from manual coin selection. All 47 focused tests pass; SwiftFormat and translation validation passed with no introduced diagnostics.

@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.

The MEDIUM is fixed at 6c79bae6. Back from the manual coin picker now stays on Confirm, and manual selection still runs before paying. I resolved the thread.

Wallet-switch rebind in the same commit, clean:

  • It needs USDT off, no hardware wallet, not submitting, the previous BTC terms equal to paykitReviewedAmount, valid new terms and meetsMinimum.
  • It sets paykitPaymentTerms, paykitReviewedAmount and sendAmountSats together.
  • acceptsPaymentAmount and acceptsLightningInvoiceAmount still re-check at pay time.
  • Unavailable or expired terms, or an amount below the minimum, return to Amount.

fixed-quoted-confirmation.xml is byte-identical to synonymdev/bitkit-android#1581.

Device gate (partial): not run yet. Pending: payment-requests/fixed-quoted-confirmation.xml, cross-device with synonymdev/bitkit-android#1581, plus Back from the manual coin picker on device. Code scan: no HIGH/MEDIUM open. The approval follows the device run.

@ovitrif

ovitrif commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

@ben-kaufman thanks, rechecked at 6c79bae. On a Spending ↔ Savings switch, returnToAmountReviewIfNeeded(rebindMethod:) now binds the new rail's valid BTC terms in place and recalculates fees. It falls back to Amount only for missing or expired terms or an amount below the LNURL minimum. The journey is identical to synonymdev/bitkit-android#1581. Nothing left from my review; I checked this from the code and did not run it on a simulator.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Fixed the Xcode 16.4 type-checking failure in b58c1964: the confirmation layout now has its own computed property, leaving the modifier order, task lifecycle and payment checks unchanged.

The custom-fee lifecycle test also now allows SwiftUI to cancel and restart preparation during navigation. It still requires exactly one successful preparation, rejects cancelled results, and checks the final manual-UTXO or confirmation route.

Validation: local simulator build and all 50 confirmation, swipe, lifecycle and pricing tests passed; SwiftFormat passed on all eight PR Swift files. Translation validation has no errors and no added warnings compared with master. Every PR commit verifies locally and on GitHub. Local validation used Xcode 27; the Xcode 16.4 CI rerun is pending. The cross-device journey remains a separate reviewer device gate.

@ben-kaufman
ben-kaufman requested a review from jvsena42 October 19, 2026 19:10
@ben-kaufman

Copy link
Copy Markdown
Contributor Author

The hosted Xcode 16.4 compiler still timed out on the remaining confirmation modifier chain. In 05beeb5 I split that expression into smaller private computed views. Inlining them reproduces the original expression byte-for-byte, preserving task IDs, modifier order, state and payment safeguards.

All 50 focused confirmation, lifecycle and pricing tests pass locally. SwiftFormat is clean for all eight changed Swift files; full translation/compiler diagnostics contain no findings introduced by this fix. Independent review is clean, and all six PR commits are verified. Reviews are re-requested; the new hosted CI run still needs to confirm the Xcode 16.4 build.

@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.

The delta since my last review, b58c196, is clean. It only moves code: confirmationContent → confirmationLayout with the same modifier chain in the same order, so payment and authorization behaviour is unchanged. The testCustomFeeWalletSwitchSurvivesFeeScreenNavigation relaxation (successfulCoinPreparations == 1) is safe, because cancelled preparations throw before they assign anything.

CI is red because of this change, so I'm holding the approval. Unit tests on Xcode 16.4 (run 39308712256) still stop at the compiler is unable to type-check this expression in reasonable time, now at confirmationLayout (SendConfirmationView.swift:393), so no test ran. The ~15-modifier chain is still one expression. Splitting the onChange/.task group into its own computed property or a ViewModifier should get it under the limit.

Device gate (partial): not run yet. Pending: payment-requests/fixed-quoted-confirmation.xml cross-device with synonymdev/bitkit-android#1581, and Back from the manual coin picker.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

@jvsena42 The latest commit 05beeb57 now passes the hosted Xcode 16.4 unit-test run and integration-test run. The failure in run 39308712256 was on the earlier revision. Re-requesting your review for the remaining device gate; the cross-device journey and manual coin-picker Back check still apply.

@ben-kaufman
ben-kaufman requested a review from jvsena42 October 19, 2026 20:59

Copy link
Copy Markdown
Contributor Author

Resolved the conflicts in signed normal merge 63ea2034d9aebfdca2e66b765d055552832c7310, integrating actual master b8b7f3d9 while preserving its wallet-switch lifecycle fix and this PR's dust/quote/fee/authorization guards. The earlier dust correction is retained in 9acc68e2; the 546-sat rejection and exact 547-sat acceptance remain covered.

Validation on the final combined tree: confirmation lifecycle 15/15; canonical 2,894 unit executions and 39 UI passes, four fixture skips, zero failures. Full SwiftFormat and translation checks passed with the 15 verified translation warnings retained. Complete structured/raw diagnostics and independent review were inspected; 13 runtime QoS warnings and two AppIntents notices remain documented in the PR body. All 13 current-base commits verify locally and on GitHub (verified=true, reason=valid).

Fresh hosted checks are running. The unavailable fork skipped before funding; rendered quoted-boundary, funded/provider, physical-biometric, hardware and two-wallet settlement gaps remain open. The previous funded-fixture failure is not erased by this pass. Requesting affected Piotr/Ovi re-review and preserving João's existing request; no review is dismissed or treated as approval.

Copy link
Copy Markdown
Contributor Author

Published signed normal master integration 9a807ef35f09989508e515c5157676d1b62d8029 with actual master b666ccb1; tested tree 0ce4fc92. The test conflicts retain both the stronger preparation-count assertions and safe explicit fixture endpoints. Production merged without a manual conflict change.

Validation: 15 focused tests; 2,928 unit and 39 UI executions passed, four fixture skips and zero failures. Full formatting passed; translations had zero errors and 15 verified base-identical warnings. Full structured/raw diagnostics were inspected and retained. All 14 current-base PR commits verify locally and on GitHub. New hosted checks need completion review; funded/provider/hardware/two-wallet acceptance remains incomplete.

The three existing human requests are still pending and preserved, so no duplicate re-request was sent. Existing changes-request state remains in force.

@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: Re-reviewed the full PR at 9a807ef after the comparison base moved to b666ccb1. Merge base is b666ccb. No inherited coverage baseline was supplied.

1 actionable finding — resolve or provide an evidence-backed rebuttal.

Valid fixed Bitcoin quotes bind their terms and open confirmation directly. Missing or expired terms, USDT choice, and LNURL plus Savings remain on Amount. Switching rails rebinds that rail’s quote, and a Savings quote below 547 sats leaves confirmation. bitkit-android ea89ed1 keeps a single confirmation state and the same 547-sat boundary. Its confirmation can change between LNURL and Savings; iOS keeps that choice on Amount, as the journey states.

Device testing: not performed in this review. This environment has no Xcode, so the added confirmation and navigation tests were inspected and not executed. The manual-coin back path below should be checked before device testing.

Suggested additional test cases

  • iOS simulator, manual coin selection, a funded wallet, and a fixed quoted Savings request. Open the request, select coins, continue, go back to the picker, then go back to the first confirmation and show details. Expected: the fee for the selected rate is visible, or Swipe to Pay stays disabled until that fee is calculated.

Earlier findings reconciled at this revision: 1 verified fixed.

Findings

  • [LOW] Manual coin selection leaves a swipeable confirmation with no fee — inline at Bitkit/Views/Wallets/Send/SendConfirmationView.swift:1797.

Reviewer model: grok-4.7
Reasoning effort: high

Comment thread Bitkit/Views/Wallets/Send/SendConfirmationView.swift

@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 of 9acc68e (the only new PR commit since 7dbb5e0, besides master merges): one LOW inline (Back from the fallback Amount pushes Amount again). No HIGH or MEDIUM.

Checked and clean:

  • The Savings minimum uses the per-rail quoted amount (request.payment(using:quoteId:)), the same value written to paykitPaymentTerms and sendAmountSats and re-checked for equality at pay time.
  • Every entry to a swipeable Confirm goes through it: direct open, wallet switch, hardware and the funding fallback. 547 sats is accepted and 546 refused, matching Amount and scan validation. USDT never reaches this view.
  • The merge resolutions in SendConfirmationView (63ea203) and SendSheet (ad30f8b) keep the PR's manual-coin and Amount-root handling alongside master's startWalletSwitch; later merges touch tests and journeys only.
  • synonymdev/bitkit-android#1638 applies the same floor (SendConfirmScreen.kt:500 disables the swipe at 546 and below), so the journey text holds on both.

Device gate (partial): 9a807ef — not run yet; still pending payment-requests/fixed-quoted-confirmation.xml cross-device with the Android twin, and paykit-quoted-usdt.xml. Device results will follow in a separate review.

Comment thread Bitkit/Views/Wallets/Send/SendConfirmationView.swift

Copy link
Copy Markdown
Contributor Author

Fixed the unit CI crash in signed commit 20f5444. The crash report showed an assertion reading the watch-only fake’s tracking Set concurrently with its insertion. Both affected tests now wait for the existing actor-isolated gate after tracking completes; all assertions and all 35 tests remain, with no production change.

Validation: 35 focused tests passed; the required canonical selection passed 2,928 unit and 39 UI executions, with four explicit fixture skips and zero failures. Full SwiftFormat 0.64.0 passed; complete translation/test/runtime diagnostics and independent review were inspected. All 16 commits against actual master b666ccb verify locally and match GitHub’s exact set with valid signatures. New-head CI is pending; no manual CI retry. Parent811b’s failed unit run and successful E2E retain their exact attribution in the updated body.

Existing João, Piotr and Ovi requests remain pending, so no duplicate request was sent. João’s changes request and the fee-refresh thread remain in force. Funded/provider/physical-hardware/two-wallet/rendered-boundary acceptance gaps are unchanged.

Copy link
Copy Markdown
Contributor Author

Published signed normal merge b53e300e with actual master d2d9d28d (tested tree 4516c8e1). The endpoint-helper conflict preserves explicit overrides and unavailable defaults; master’s cache display and test isolation changes are retained. Final canonical validation: 2,932 unit tests and 39 UI executions passed, four fixture skips, zero failures. Full formatting, translations, diagnostics and independent integration review passed; all 17 current-base commits verify locally and on GitHub.

The prior 20f54449 hosted unit run passed all 2,928 units but failed one UI launch with an XCTest background-assertion timeout; its video reaches Terms. That failure remains recorded with root cause unproven. The same launch test passed locally on this new combined tree; this is not a permanent-fix claim. Prior E2E passed 13 cases on its first step with one hardware skip. New-head CI is pending. Existing human requests, João’s changes request, Piotr’s fee-refresh thread and documented acceptance gaps are preserved.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified — one hosted regression remains failing.

Findings: No new product defect verified. The Savings minimum and reported fallback Back sequence hold; I could not reproduce the missing manual-selection fee.

QA for b53e300:

Tested b53e300 on an iPhone 17 Pro simulator, iOS 26.5, built with Xcode 27. Used a linked controlled Paykit issuer and regtest funds. Videos play at 2× speed; no payment was authorized.

Test 1: J1 — direct confirmation and manual coin selection ✅ The fixed quote confirmed at 5,000 sats; Largest First showed a 143-sat fee, and the original Confirm displayed 212 sats after selecting both coins and backing through the picker.

The device log confirms Largest First selected the 500,000-sat output. Manual-picker Back stayed on Confirm, and the next swipe required selection. Selecting the 500,000- and 200,000-sat coins produced the displayed 212-sat fee on both confirmations.

Direct confirmation Manual selection and Back
J1-largest-first.2x.mp4
J1-manual-return-v3.2x.mp4
Test 2: J2 — 546-sat Savings fallback ✅ Switching from 2,500-sat Spending to 546-sat Savings opened Amount with Continue disabled; Back closed the sheet and the unpaid request reopened.

Accessibility confirmed ContinueAmount was disabled. Savings and Spending stayed at 700,000 and 500,000 sats.

J2-546-v3.2x.mp4

Test 3: J2 — exact 547-sat Savings boundary ✅ Switching to Savings retained Confirm at exactly 547 sats with a refreshed 143-sat fee and a required swipe.

J2-547.2x.mp4

Test 4: J1 — cancel during preparation ✅ Closing while the requested USD amount and disabled loading controls were visible prevented late reopening or payment; the same request reopened successfully.

J1-cancel-v3.2x.mp4

Test 5: J1 — LNURL and Savings choices ✅ Both wallets stayed available: Spending confirmed 2,500 sats, Savings confirmed 5,000 sats with its 143-sat fee, and LNURL-only confirmed directly.
LNURL and Savings LNURL only
J1-lnurl-v2.2x.mp4
J1-lnurl-only-v2.2x.mp4
Test 6: Quote and send lifecycle regressions ❌ The focused exact-head run passed 66 tests but failed the initially-unreviewed fallback Back assertion; an isolated rerun reproduced it.

SendConfirmationLifecycleTests.testQuotedWalletSwitchRequiresReviewForUnavailableExpiredOrBelowMinimumTerms fails at BitkitTests/SendConfirmationLifecycleTests.swift:141: the synthetic Confirm-root host still has an active send sheet after Back for unreviewed. The unavailable, expired and both minimum variants passed. Pricing, routing, manual-fee preparation, LNURL and send-sheet coverage otherwise passed.

Waiting for Amount to appear and then rendering the real Amount view in separate local test-only diagnostics did not clear this assertion. Production code stayed unchanged. Normal routing sends initially unreviewed terms directly to Amount; I have not established a matching user-visible failure. Please recheck this regression on iOS 26.5 / Xcode 27. The live rail-switch fallback and Back behavior passed in Test 2.

The recording shows the hosted test run; these results come from XCTest assertions with controlled request and wallet state.

hosted-regressions.2x.mp4

I retained the earlier passing device results for unchanged insufficient-funds, deadline-expiry and BTC/USDT asset-choice behavior. USDT funding/submission, subscription authorization and hardware signing were not driven; these confirmation journeys stop before payment.

Twin: synonymdev/bitkit-android#1581 fix: confirm fixed Paykit requests directly matches fixed Bitcoin quotes and wallet availability. iOS keeps LNURL/Savings selection on Amount; Android offers it on Confirm. The shared wording follow-up remains synonymdev/bitkit-android#1610 Align fixed-quote journeys with both payment flows.

Delta review: ad30f8b..b53e300, including prior open findings and payment-path merge integration. Unrelated master features received no separate device cases.

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

Copy link
Copy Markdown
Contributor Author

Follow-up to QA review 5670593898 at unchanged b53e300e:

I’m keeping the reported unreviewed hosted-test failure open. Source inspection confirms that this table row clears paykitPaymentTerms and paykitReviewedAmount before mounting a synthetic Confirm root; PaymentNavigationHelper.contactPaymentRoute sends missing reviewed terms to Amount in normal routing. The other rows invalidate terms after confirmation has mounted. This is a concrete setup difference, but it does not establish the failure’s cause or justify removing the assertion.

The reported rendered manual-fee, rail-switch fallback and exact-minimum results are retained as reviewer-reported evidence. The fee thread is now externally resolved; no payment/settlement or human approval is inferred.

Independent iOS 26.5 reproduction is pending because local free disk is below the established build reserve. No source or assertion change, test retry, device reset or fixture operation has been made. Please attach the complete failing .xcresult (including diagnostics) and exact test command/Xcode build version for the original run and isolated reproduction. The visible report/video and the passing Xcode 26.1/local/hosted results do not establish why the newer-toolchain assertion fails.

@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.

Re-reviewed to b53e300. My LOW on SendConfirmationView.swift:1766 is fixed for the header Back, and I have no new findings.

  • 811b433 closes the sheet from the fallback Amount through hasShownAmountReview and hideSheetIfActive(presentationID:). The hide branch writes no terms and no amount.
  • The amount paid still can't differ from the amount shown: validateIncomingPaymentRequestAmounts → acceptsPaymentAmount still pins the live quote and quoteId to wallet.sendAmountSats.

One path needs a simulator check before I call it a finding: swiping back from the left edge on the fallback Amount. UINavigationController+SwipeBack enables the gesture there. During an interactive pop the root .task runs before the path binding is written, so the hide branch may be skipped. Confirm would then stay open with nil terms and the old rail's amount, and swiping to pay would fail with amountMismatch, so no funds are at risk. If the device shows it, deciding the exit in SendAmountView's onBack and adding .allowSwipeBack(false) on that pushed Amount covers both gestures.

Device gate (partial): b53e300, not run yet. Pending: fixed-quoted-confirmation.xml steps 3 and 9, cross-device with synonymdev/bitkit-android#1638; paykit-quoted-usdt.xml; and the swipe-back check above.

Copy link
Copy Markdown
Contributor Author

Published 20cfeff for the initially-unreviewed Back regression reported in review 5670593898. I reproduced the unchanged b53e300 assertion on Xcode 27.1/iOS 26.5; the four-line navigation observer now reuses the existing invalid-terms dismissal guard when Amount pops to the confirmation root. All 15 unchanged lifecycle tests pass. No assertion or payment guard was weakened.

Final canonical validation: 2,932 unit and 39 UI executions passed, four explicit fixture skips, zero failures. Full SwiftFormat passed; translation validation has zero errors and 15 individually verified base warnings. Complete diagnostics and independent correctness/quality review are retained. All 18 current-base commits verify locally and on GitHub. Local tree e286b0d9 is the repaired PR tree; the newer combined master543d tree is not covered. Fresh hosted checks are pending.

The supplementary real-Amount/header-Back check closed the owned sheet state without authorization or submission. Interactive Back remains unverified: after one readiness-harness correction, two delivered edge swipes did not pop the temporary test window and its bounded diagnostic failed on timeout. Both diagnostic failures, screenshots and full reports are retained; the temporary test source was restored exactly before signing. This does not establish the interactive-pop root cause or full linked-request/reopen/funded acceptance. The existing lifecycle regression is fixed, while those acceptance limits remain explicit in the PR body.

Existing Piotr/Ovi requests and João's changes request are preserved. No CI job was retried.

@ovitrif
ovitrif dismissed jvsena42’s stale review October 27, 2026 19:36

Dismissed per @ovitrif: this review was on an older head, its threads are resolved, and the PR has moved on. @jvsena42 is off until 1 Nov; the current head needs a fresh review from @ben-kaufman or @piotr-iohk.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⛔️ Request changes — the comma-region journey fails; interactive Back remains unverified.

Findings: No new defect established in the delta. The comma-region failure below also affects code unchanged from the base.

QA for 20cfeff:

Tested on an iPhone 17 Pro simulator with iOS 26.5 and an Android emulator. Videos play at 2× speed; no payment was authorized.

Test 1: Initially unreviewed fallback Back ✅ The focused lifecycle regression passed.

The XCTest result confirms the previously failing initially-unreviewed assertion and the invalid-terms variants.

hosted.2x.mp4

Test 2: Android-created fixed request ✅ A $2.50 Savings request opened and reopened on iOS confirmation at 3,097 sats.

Android showed the request as waiting for payment; iOS showed a 143-sat fee before closing. This checks both apps through confirmation, not settlement.

cross-android.2x.mp4

cross-confirm.2x.mp4

Test 3: 546-sat fallback and header Back ✅ Switching from Spending to 546-sat Savings opened Amount; header Back closed the sheet and the unpaid request reopened.

edge-final.2x.mp4

Test 4: 547-sat boundary ✅ Switching to the 547-sat Savings quote retained confirmation and showed the fee.

boundary547.2x.mp4

Test 5: LNURL and Savings ✅ The mixed request showed 2,500 sats for Spending and 5,000 for Savings, each with its confirmation; the LNURL-only request opened confirmation directly.

The Savings confirmation showed a 143-sat fee. Both cases stopped before payment.

lnurl-rails.2x.mp4

lnurl-only.2x.mp4

Test 6: Insufficient Savings ✅ A 4,000,000-sat quote was rejected with an insufficient-balance message.

balance.2x.mp4

Test 7: BTC and USDT choice ✅ An uncovered $2.50 USDT quote defaulted to Savings with USDT selectable; after funding, it defaulted to USDT.

mixed.2x.mp4

mixed-funded.2x.mp4

Test 8: Private USDT-only contact ✅ The amount screen offered only USDT and labeled the balance AVAILABLE.

The controlled contact published a private USDT-only payment list; the device log confirmed the private list was opened.

locale-contact.2x.mp4

Test 9: USDT review details ✅ The long invoice note and fee note remained separate while the payment control stayed visible during scrolling.

quote-short.2x.mp4

Test 10: Comma-region amount entry ❌ The journey expects commas, but request creation showed a dot and 150.50; direct USDT Send, Receive and contact entry showed a dot and 5.5.

The simulator's nl_BE locale and comma decimal separator were checked independently. NumberPad and the request-creation call site are unchanged from the base, so this is not attributed to the reviewed delta.

locale.2x.mp4

locale-receive.2x.mp4

locale-send.2x.mp4

locale-contact.2x.mp4

Test 11: Interactive fallback Back ⏭️ Completed and cancelled interactive transitions remain unverified.

Two edge gestures on the fallback Amount screen did not initiate a visible transition. The same automation also failed to transition on a separate settings navigation screen, so this does not establish an app defect. Header Back is covered by Test 3.

edge-final.2x.mp4

control.2x.mp4

Test 12: Expired request retry ⏭️ The request disappeared from confirmation at expiry without payment; fresh-terms review on retry remains unverified.

expiry.2x.mp4

Test 13: Remaining J1 acceptance ⏭️ Full provider, hardware and settlement acceptance was not exercised.

The device cases above stopped before payment. Earlier confirmation, manual-coin, cancellation and 546/547 checks are retained from the prior review.

Test 14: Remaining J2 acceptance ⏭️ The full subscription, six-decimal edit and counterparty checks remain incomplete.

Four Savings–USDT switches restored 12,345 sats on the proposer; payer-side subscription acceptance was not checked.

subscription.2x.mp4

Reviewed the delta from b53e300 to 20cfeff and the Back behavior reported in the prior review. The pair PR, synonymdev/bitkit-android#1581, implements the same direct-confirmation goal with the documented LNURL funding-choice difference; the cross-device request above used synonymdev/bitkit-android#1638.


Reviewed by Codex via gh-pr-review-loop (model and reasoning level not exposed).

Copy link
Copy Markdown
Contributor Author

Addressed the stale comma-decimal expectations from review 5676643246 in 9dacc08. J2 now expects the existing iOS dot-decimal convention while keeping comma-region coverage, all 19 actions, exact values, trailing zeros, six-decimal precision and serialized-value checks. The journey description and README identify the iOS convention.

This matches NumberPad, formatFiatAmount, the existing cross-region PaykitAmountTests/UsdtDisplayTests, and usdt-number-format.xml. Twelve relevant source/spec files are byte-identical to actual master 85e9d4c6; the differing UsdtSendView changes concern submission-error handling, not the inspected amount-entry path. This is a documentation correction, with no production behavior change or new base-runtime claim.

Validation: XML parsing/action comparison, references and git diff --check passed; SwiftFormat 0.64.0 reports 0/846 files requiring formatting. One independent scoped documentation review found no issues. All 19 current-base commit signatures verify locally and match GitHub’s exact valid-signature set. App tests were not repeated for this docs-only delta.

The review’s results remain attributed to 20cfeffa. Native interactive Back, fresh-terms review after expiry, and the remaining J1/J2 provider, hardware, settlement, subscription and counterparty cases remain incomplete; this correction does not mark those journeys passed. The native visibility-control evidence already recorded above remains diagnostic only.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⛔️ Request changes — a six-decimal USDT request is rounded on the payment review; some acceptance cases remain unverified.

Findings: No new defect found in the reviewed documentation delta. The six-decimal display failure below is in code unchanged from the base.

QA for 9dacc08: Built and tested on an iPhone simulator with iOS 26.5, with the app launched in a comma-decimal locale. Test funds only; no payment was authorized. Videos play at 2× speed.

Test 1: USDT Receive in comma region ✅ Decimal key is a dot; 5.5 is visible.

locale-receive.2x.mp4

Test 2: USDT Send in comma region ✅ Decimal key is a dot; 5.5 is visible.

locale-send.2x.mp4

Test 3: USDT contact payment in comma region ✅ Decimal key is a dot; 5.5 is visible.

locale-contact.2x.mp4

Test 4: USD request creation in comma region ✅ 150.50 appears in the editor, review and recipient row; editing retains it.

Editing retention was checked in the accessibility snapshot.

request.2x.mp4

Test 5: USD request with trailing separator ✅ 5 followed by the decimal separator permits continuing to a 5.00 review.

trailing.2x.mp4

Test 6: USD subscription counterparty ✅ A linked payer sees a 150.50 monthly proposal and USDT-only review.

subscription-payer.2x.mp4

Test 7: Quoted USDT amount through provider failure ✅ 150.50 remains visible on the quote error and review succeeds after recovery.

Error captured in screenshot; recovery checked in accessibility snapshot.

unavailable.2x.mp4

Test evidence

Test 8: Six-decimal USDT request through review ❌ 0.123456 appears as 0.12 on the payment review.

No payment was authorized; the display code is unchanged from base.

six.2x.mp4

Test evidence

Test 9: Comma-region decimal journey ✅ The corrected dot-decimal entry and request values were observed.

Aggregate of the separately recorded decimal cases.

locale-receive.2x.mp4

locale-send.2x.mp4

locale-contact.2x.mp4

request.2x.mp4

Test 10: Interactive fallback Back ⏭️ Could not establish a cancelled or completed interactive Back transition.

Attempted edge drags and a preset swipe on a funded regtest fallback; no interactive transition started.

edge.2x.mp4

Test 11: Expired request retry ⏭️ Expiry removed the pending action, but fresh-term retry was not completed.

A short-lived regtest request expired; the flow did not reach a fresh authorization review.

expiry.2x.mp4

Test 12: Remaining J1 acceptance ⏭️ A linked payer received the 150.50 request; remaining acceptance steps are incomplete.

The linked request was driven through the payer amount screen; the full remaining settlement and hardware coverage was not completed.

linked-request.2x.mp4

Test 13: Remaining J2 acceptance ⏭️ The linked subscription and six-decimal review were exercised; remaining exact-value serialization is unverified.

No completed end-to-end serialized amount or settlement check.

subscription-payer.2x.mp4

six.2x.mp4

The six-decimal request showed 0.123456 on the amount screen, then 0.12 on review and in payment details. I did not submit it, so this checks the confirmation display only. The same formatting path exists at the base; this delta does not introduce it.

Earlier passing cases are carried forward from the previous review and earlier review, since the subsequent changes did not touch those behaviors. Reviewed the delta from 20cfeff to 9dacc08, which updates journey documentation; the corrected locale examples were driven above.

The paired Android PR #1581 has the same direct-confirmation goal, with its documented LNURL funding-choice difference.

Reviewed by Codex via gh-pr-review-loop (model and reasoning level not exposed).


Reviewed by gpt-6.1-terra-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

@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.

Re-reviewed to 9dacc08. No new findings, and 20cfeff fixes the swipe-back path I raised in the code. The new .onChange(of: navigationPath) (SendConfirmationView.swift:454-457) fires on the committed [.amount] → [] write whatever the root .task timing, and reaches the hide branch (:1771-1773). A second hit is a no-op through hideSheetIfActive(presentationID:). The amount paid is still pinned to wallet.sendAmountSats (SendSheet.swift:1218-1224).

Device gate (partial): 9dacc08, not run yet. Pending: fixed-quoted-confirmation.xml including the edge swipe on the 546-sat Savings fallback Amount, and paykit-quoted-usdt.xml.

Copy link
Copy Markdown
Contributor Author

The six-decimal display finding in review 5678141014 is fixed in 629a874e. I reproduced it in the actual UsdtSendView confirmation header and details: 0.123456 rendered as 0.12, while one atomic unit rendered as <0.01. All five review values now use the existing exact formatter. Request/subscription amount text uses the same exact formatter; quote/wire values and authorization are unchanged.

The rendered red test failed ten precision assertions; the fix passes all ten focused tests, with saved native screenshots showing 0.123456, 0.000001, and exact fees/totals. Final canonical local validation passed 2,934 unit and 39 UI executions, with four documented fixture skips and zero failures or retries. Complete diagnostics were inspected: two AppIntents warnings, ten structured QoS warnings plus the new static-builder State warning, eleven raw QoS backtraces, and 72 NaN notices remain explicitly retained alongside symbolication and simulator notices; this is not a warning-free claim. Full formatting passed; translation diagnostics remain the same 15 verified base warnings. All 20 commits in the current-base PR range verified locally and matched the exact GitHub set with verified=true / reason=valid.

This is native static rendering coverage of the production builders, not a completed linked-request or funded journey. The test's constant-binding warning is disclosed because it bypasses the full view lifecycle. Native Back and fresh-terms retry remain open acceptance gaps. The fixture prerequisite is now explicit in the existing J1/J2 instructions, with stable case IDs. Your device results retain their original head/environment attribution; no payment, review dismissal or duplicate reviewer notification was made.

New-head unit, integration and E2E checks are pending; the completed parent-head E2E result remains separately attributed in the PR description.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified — interactive Back, fresh-terms retry and remaining acceptance checks are incomplete.

Findings: No new defect found in the delta.

QA for 629a874: Tested with linked test wallets on iOS 26.5 and a local USDT fixture. Videos play at 2× speed; no payment or subscription was authorized.

Test 1: Six-decimal request ✅ The incoming row, payment review and details showed 0.123456 USDT.

six.2x.mp4

Test 2: One-micro USDT request ✅ The incoming row, payment review and details showed 0.000001 USDT.

The details were also checked in the screenshot.

micro.2x.mp4

Test 3: Six-decimal subscription ✅ The proposal row and subscription review showed 0.123456 USDT.

sub.2x.mp4

Test 4: Savings-only subscription ✅ A linked payer saw the 12,345-sat proposal with Savings as its only payment method.

The proposer is on the left and payer on the right.

pair.2x.mp4

Test 5: Interactive fallback Back ⏭️ Switching from Spending to the 546-sat Savings quote opened Amount, but neither edge gesture started a visible transition.

A preset gesture and a full edge drag did not establish either a completed or cancelled transition. The prior header Back check remains applicable.

gesture.2x.mp4

Test 6: Expired request retry ⏭️ The request closed at its deadline and disappeared from pending without a payment.

No Pay action remained to exercise fresh-terms review on retry.

expiry.2x.mp4

Test 7: Six-decimal editing and serialization ⏭️ Editing fixture-provided USDT terms and checking the serialized result remain unverified.

The iOS editor creates BTC or USD terms, and the incoming fixture terms were not editable.

Test 8: Remaining acceptance ⏭️ Full provider, hardware and settlement acceptance remains incomplete.

The fixture cases above stopped before authorization; hardware and settlement were not driven.

Check 1: Exact display regression ✅ The two focused tests passed at this head.

The tests include static rendering of production confirmation and details builders for 0.123456 and 0.000001, including the exact bridge recipient, fee and total. They do not exercise the full navigation or payment lifecycle.

The earlier 9dacc08 review and 20cfeff review retain their passing evidence for behavior untouched by this delta. The former six-decimal display failure is fixed in the live request checks above.

Reviewed the delta from 9dacc08 to 629a874. The bridge recipient and maximum-total changes were covered by the static regression, not a live bridge journey. The paired synonymdev/bitkit-android#1581 matches the direct-confirmation goal, with the documented LNURL funding-choice difference.

Reviewer model: gpt-6.1-terra
Reasoning effort: high


Reviewed by gpt-6.1-terra-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

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.

5 participants