Skip to content

fix: handle late usdt introductions during home checks - #300

Merged
piotr-iohk merged 4 commits into
mainfrom
codex/fix-android-send2-amount-limit
Nov 9, 2026
Merged

piotr-iohk merged 4 commits into
mainfrom
codex/fix-android-send2-amount-limit

Conversation

@piotr-iohk

@piotr-iohk piotr-iohk commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Companion: synonymdev/bitkit-android#1808

An optional USDT introduction can appear after onboarding has returned or after a receipt returns the app to Home. The default Android suite then waits for balance, menu or backup controls covered by that sheet. This change resolves the introduction during the relevant Home wait, then continues requiring the original outcome.

Changes

  • Observe the expected Home outcome and the optional introduction together. Choose the intro option once outside the retrying predicate, re-resolve its targets, and confirm it disappears. Failed taps, ineffective choices and reappearance fail the wait.
  • Apply that wait to balance synchronization and menu navigation. Add readiness before the direct onchain_2 balance read while retaining its exact text assertion.
  • Merge current main at 0e83ea6 and preserve its Move-to-Spark recovery around the complete Home navigation waits. Cover Spark followed by USDT and ineffective Spark dismissal on both platform models.
  • Cover late arrival, wrong balances, stale handles, failed choices, explicit opt-in and builds without the intro on both platform models. Extend the existing Profile Copy mock to expose Home visibility while preserving its clipboard assertions.

Validation

  • Current paired Android CI passes: workflow 42580213958, attempt 6, exact E2E 7f61681bb8d4b0d31dc836a1c03a8488170baf66 paired with unchanged app b5ff8114be9a1392adc2caef371d5b367868c7dd. All seven checkouts are verified: 38/39 first-attempt cases, 39/39 eventual passes, one built-in retry and 13 retry steps skipped; no legacy toast fallback ran. The fresh app build checked out merge 2353ee4c30a71c7c09c709cc3403f3231080f94b, whose tree matches b5ff8114b.

    The retried multi_address_3 case timed out waiting for ReceivedTransaction. Its failure screenshot and sampled recording show uncovered Home, and the device log records an incremental Electrum on-chain sync timeout during that wait. The same case passed on its built-in retry with unchanged sources. The artifact-backed disposition is flake, with the underlying sync-timeout cause unisolated; this is not proof of staging VSS causality. The exported app log starts after the next reinstall. Three recording samples and the failure screenshot were inspected; a fourth requested frame did not decode.

  • Current main integration head 7f61681 includes main 0e83ea6 and its timed-sheet polling changes. 78 repository unit tests plus 4 boundary probes, typecheck, scoped lint and diff checks pass. Four independent implementation reviews have no remaining findings; the current-head Talos review is clear. Current-head human approvals: ben-kaufman and pwltr; GitHub reports APPROVED/CLEAN.

  • Historical fba9a6e paired Android CI pass: workflow 42580213958, attempt 5 completed 39/39 cases on their first attempt at exact E2E fba9a6efde8fdd9066a0408e3f5f1fe1b5ec23ac; every shard checkout is confirmed, all 14 retry steps skipped, and no legacy toast fallback ran. It reused the successful app build of merge 2353ee4c30a71c7c09c709cc3403f3231080f94b (same source tree as app head b5ff8114b).

  • Node 22.23.4: 78/78 repository unit tests plus 4/4 independent boundary probes pass at current main-integration head 7f61681bb8d4b0d31dc836a1c03a8488170baf66.

  • TypeScript --noEmit, ESLint for all changed files and git diff --check pass.

  • Four independent implementation review rounds completed with no remaining findings, including a full reassessment against the new base and 59 independently executed focused/boundary checks. All four authored commits, including both merges from main, are signed locally and GitHub Verified.

  • Four focused regressions failed against the old fixture before the correction. An initial broad run at 3135e8f found five Profile Copy mocks missing the newly used visibility method; the one-line mock correction at 407d4a0 resolves them without changing their original assertions.

  • Historical pre-conflict paired Android CI passed: 39/39 cases on their first case attempt, across all seven shards. Workflow 42580213958, attempt 2, workflow attempt 2; all 14 retry steps skipped. Every shard checked out exact E2E 407d4a05fdcf6796960df0afbda552f575c93e97.

  • Historical post-conflict validation: workflow attempt 3 finished failure in all seven shards. All seven logs confirm E2E fba9a6e; the app build checked out the original 2353ee4c merge. Eleven Settings cases passed, but setup hooks aborted much of the 39-case cohort; failure counts include hooks, so this is not a completed 39-case assertion run. This failed attempt remains separate from the later successful unchanged-source attempt 5. The previous pass is retained at its actual 407d4a0 source.

CI evidence and setup

The failed Android workflow attempt 1 used app b5ff8114be9a1392adc2caef371d5b367868c7dd and E2E main at 849806e70784da0960f44da6206e2346b1e0ba3e: 27/39 first-attempt and 35/39 eventual passes. All 22 failure screenshots, sampled frames from all 22 recordings, and corresponding logs show the undismissed USDT intro. Diagnosis.

Use the standard Android E2E workflow at that app head, Node 22, Pixel 6/API33 and the local Docker backend. This E2E branch matches the app branch, so normal branch selection picks it up. Attempt 2 executed that same seven-shard cohort with the existing 30s UI waits. Standard PR checkout built merge ref 2353ee4c30a71c7c09c709cc3403f3231080f94b; its source tree 813c7ff6e9aac702bd0be30422812c0dfb528936 exactly matches app head b5ff8114b. The passing LNURL/Transfer device log records two USDT-intro presentations followed by one decline click for each new wallet, establishing that the runtime exercised the correction.

Settled investigation: all seven failed-shard artifacts, all 13 failure screenshots, and sampled frames from all 13 recordings were inspected with app/device logs. Hook failures often have no per-case media. Three onboarding recordings lack a decoded 75% sample; their other sampled frames and failure screenshots were inspected. Three failed-case screenshots show uncovered Home with zero balance; Settings failures show Electrum/RGS connection screens. Subsequent reinstall processes stall in native VSS-backed node construction and time out waiting for the node to run. Read-only workstation GET probes to staging /vss_rs_auth/ and /vss_rs_auth/get_object also time out while /lnurl_auth/auth responds. This is consistent with external dependency trouble; it does not establish the exact cause of every initial receipt failure or prove a global service outage. No introduced fixture defect has been demonstrated. The triage remains insufficient evidence for one established whole-run cause, rather than labeling the different-tree historical pass as a flake. The suite subsequently passed in attempt 5 at unchanged fba9a6e; current 7f61681 execution is recorded separately above. Fresh approvals at 7f61681 are recorded above.

Limits and scope

Android master has additionally advanced to 36a2c4d, adding USDT first-sync receipt celebration/restore-window handling and loaded-account on-chain address search. These do not alter this PR's Lightning capacity refresh or #300's Home-introduction controls. Existing CI remains pinned to the actual app merge 2353ee4c / head b5ff8114b; the combination with the newer master changes has not been rerun.

Previously assessed Android master 3f2009e: its delta from the originally tested base consists of release 3.1.0 version/changelog bookkeeping and #1817 instrumentation-test/local-regtest workflow changes. No production app source, dependencies or standard local E2E workflow changed. Those new instrumentation fixtures are outside this Appium cohort. The rerun remains evidence for the original app merge snapshot and unchanged b5ff811 behavior, not execution of current-master version metadata or migration behavior.

This is a standalone change from main; experimental E2E #298 remains stacked on #296. Its historical iOS cohort and separate RBF, Receive-transition and staging-funding investigations remain outside this fix. No new iOS runtime execution is claimed. One successful Android cohort does not establish a durable CI reliability rate.

The main-based Send2 case retains its historical extra-key toast fallback; attempt 6 logged zero uses of that fallback. Its pass must be distinguished from the separate pinned Send2 run, which preserves the stronger one-key warning and retained-field assertion and passed 90/90 outcomes against app b5ff8114b / E2E e560ece. This PR adds no such fallback and preserves existing financial, fee, invoice and history assertions.

Models used

  • Planning/scoping: gpt-6.1-sol (reasoning: high)
  • Implementation: gpt-6.1-sol (reasoning: high)
  • Review: gpt-6.1-sol (reasoning: high)
  • Review rounds: 4

@piotr-iohk
piotr-iohk marked this pull request as ready for review November 9, 2026 09:17
@piotr-iohk
piotr-iohk requested review from a team, ben-kaufman and pwltr and removed request for a team November 9, 2026 09:22

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

No findings. Makes the Home waits (balance polling, HeaderMenu taps, openSettings, doTriggerTimedSheet, the onchain_2 total read) resolve a USDT introduction that arrives late, once and outside the retried waitUntil predicate, then keep requiring the original outcome on the same deadline. Reviewed 407d4a0, full tier, static.

What I checked, and 5 candidates I ruled out

Read in full: test/helpers/actions.ts (balance helpers, tap, waitForWalletHome, waitForWalletHomeCondition, handleUsdtIntro, restoreWallet, triggerTimedSheetUnlessPresent, dismissInterveningUsdtIntro), test/helpers/navigation.ts, the onchain_2 hunk, both unit fixtures.
Call sites traced: every handleUsdtIntro caller (onboarding, restoreWallet, the timed-sheet path) keeps its old contract: no timeout means no deadline, and the decline path still returns false when nothing shows within 5s.
Build skipped: talos-pr-run could not check the author's permission on this repo and fails closed, so I did not run the unit suite or tsc/eslint myself. This repo reports no checks of its own.
Paired CI: bitkit-android run 42580213958, attempt 2, is success on app head b5ff811 across all seven local shards. I did not re-read the shard logs to confirm which E2E sha they checked out.

Ruled out

  • A USDT opt-in journey gets declined by a later Home wait: waitForWalletHomeCondition defaults to enableUsdt: false, but no spec calls completeOnboarding({ enableUsdt: true }) today, so no current path reaches it.
  • Polling cost of the extra UsdtIntroDecline probe: isDisplayed does not wait on a missing element, and the balance helpers poll every 2s.
  • iOS elementByIdWithin blocking inside the onchain_2 predicate: the intro probe runs first and short-circuits, and a rejected predicate is retried by waitUntil.
  • Restarted wait budget after resolving the intro: one deadline is computed up front and every inner wait takes remaining().
  • Stale lastValue in the balance timeout message: the message is built once at call time, same as before this diff.

Merge confidence: 4/5, no findings, but I could not run the unit suite here and there is no iOS runtime evidence for the new waits.

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

Reviewed commit 407d4a0. No findings.

Home waits now resolve a late USDT introduction once, outside the retried polling predicate, and keep requiring the original outcome on the same deadline; an intro that reappears after its decision or vanishes before it fails clearly. On Android the decision waits for the button to settle and confirms the sheet closed.

Review details

General review
Model: Claude Opus 5.5
Reasoning effort: high

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

Re-review of fba9a6e after 407d4a0. 0 earlier findings, 0 new. The only change since my last review is the merge of main at a62926c (Move to Spark recovery); 3135e8f and 407d4a0 are unchanged (git range-diff). Full tier, static.

What I checked, and 4 candidates I ruled out

Merge resolutions read: tap (test/helpers/actions.ts:394), openSettings and doTriggerTimedSheet (test/helpers/navigation.ts), the unit fixture. Each keeps main's withMoveToSparkIntroRecovery and puts the PR's waitForWalletHomeCondition inside it, so both sides' behaviour survives. doTriggerTimedSheet keeps its 45s timeout. openSettings keeps main's default timeout.
Auto-merged side: main replaced dismissInterveningUsdtIntro with dismissInterveningSheets. The PR never changed that function, and the new one keeps the same USDT branch (handleUsdtIntro() with no deadline, then doTriggerTimedSheet), so the timed-sheet path still behaves as before.
New tests: the Spark-then-USDT case for menu tap, Settings and the timed-sheet trigger, and the ineffective Spark dismissal, on both platform models. The fixture hides the USDT intro and Home while Spark is up, which is how the recovery's isDisplayed probe sees the stack.
Build skipped: talos-pr-run still cannot check the author's permission on this repo and fails closed, so I did not run the unit suite, tsc or eslint. The repo reports no checks of its own.
Paired CI: bitkit-android run 42580213958, attempt 3, the one on the merged source, is still in progress (7 local shards running).

Ruled out

  • Spark recovery gets a shorter budget because the USDT wait already spent it: the retry calls waitForWalletHomeCondition again, which computes a new deadline. That matches main, where the retry also restarts waitForDisplayed with the full timeout.
  • USDT "reappeared" error masked by the Spark retry: withMoveToSparkIntroRecovery rethrows unless the Spark intro is actually showing, so a USDT reappearance with no Spark sheet still fails.
  • A Spark sheet stalls a USDT decision mid-wait: while Spark covers it, UsdtIntroDecline is not displayed, so no choice is attempted until Spark is dismissed.
  • Double recovery in tap('HeaderMenu'): both waits are wrapped, but each wraps its own wait and dismisses only a sheet that is actually showing, so a single Spark intro produces one Later tap (the new menu-tap test asserts exactly that).

Merge confidence: 4/5, no findings, but I could not run the unit suite here and the paired Android run on the merged source has not finished yet.

Copy link
Copy Markdown
Collaborator Author

Conflict resolution is pushed at fba9a6efde8fdd9066a0408e3f5f1fe1b5ec23ac, signed and GitHub Verified. It preserves main's Spark recovery together with the USDT Home guard. The full independent reassessment and Talos re-review found no issues; 78 repository tests plus 4 boundary probes, typecheck and scoped lint pass.

Paired Android workflow attempt 3 finished red across all seven shards at that exact E2E head. Eleven Settings cases passed; failing setup hooks stopped much of the 39-case cohort. I inspected all seven failed-shard artifacts, all 13 failure screenshots, and sampled frames from all 13 recordings, plus matching app/device/job logs. Three recordings have no decoded 75% sample; other sampled frames and screenshots were inspected. Hook-only failures have no per-case media.

The receipt failures show uncovered Home with zero balance. Boost stays on its submission spinner, backup uploads stay running/failed, Settings connection updates stall, and most subsequent onboarding failures stay at 100% setup. Later reinstalled app processes stop during buildWithVssStore and then hit NodeRunTimeoutError. Read-only workstation probes to staging VSS also time out while the auth endpoint responds. The first receipt processes had already started and synced their nodes, so later startup logs cannot alone establish their exact cause. One onboarding retry also times out on its intro slide. External dependency trouble is likely; a single established cause for the entire run is still insufficiently evidenced.

Merging remains blocked pending a successful current-head runtime result and refreshed human approval. The earlier 39/39 first-attempt result belongs to 407d4a0 and remains historical; it does not substitute for this integrated source. No further unchanged rerun has been requested.

@piotr-iohk
piotr-iohk requested a review from pwltr November 9, 2026 13:20

piotr-iohk commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator Author

Current conflict-resolution head fba9a6efde8fdd9066a0408e3f5f1fe1b5ec23ac is now runtime-validated: Android workflow attempt 5 passed 39/39 cases on their first attempt, across all seven shards. I confirmed every checkout SHA; all 14 retry steps skipped and zero legacy toast fallback invocations. The successful build is the original app merge 2353ee4c for unchanged Android head b5ff8114b.

The existing 78 repository tests plus 4 boundary probes, typecheck/lint, independent full reassessment and Talos re-review apply to this exact unchanged source. Historical failed attempts remain in the description with their actual evidence. Fresh human review has been requested from pwltr after the conflict-resolution push; the existing ben-kaufman request is retained. Once current-head approval and live merge policy clear, this companion can land before synonymdev/bitkit-android#1808.

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

Re-review of 7f61681 after fba9a6e. 0 earlier findings, 0 new. The only change since my last review is the merge of main at 0e83ea6; the PR's own diff against its base is byte-identical to the one at fba9a6e (only the blob index line differs). Full tier, static.

What I checked

Merge content: main brought #301 (triggerTimedSheetUnlessPresent now waits up to 3s for sheetId or MoveToSparkIntroLater before dismissMoveToSparkIntroIfShown()), the Blocktank test removal, the 3.1.0 migration baselines and the rustls bump. None of it overlaps a hunk of this PR, and the merge has no hand-resolved conflicts (git show --cc 7f61681 is empty).
Interaction with #301: the PR's doTriggerTimedSheet (USDT-aware waitForWalletHomeCondition inside withMoveToSparkIntroRecovery, 45s) runs before #301's new wait, so a late USDT intro is still resolved before the trigger, and #301's post-trigger wait is untouched.
Removed helpers: transferSpendingToSavings, tryDismissBackgroundPaymentsIfVisible, transferHardwareWalletToSpending, waitForHardwareTransferProgress, dismissSpendingBalanceToastIfShown have no remaining references at 7f61681, including in the PR's unit tests (unchanged since fba9a6e).
Build skipped: talos-pr-run still cannot check the author's permission on this repo and fails closed, so I did not run the unit suite, tsc or eslint. The repo reports no checks of its own.
Paired CI: bitkit-android run 42580213958, attempt 6, is in progress (build-local running).

Merge confidence: 4/5, no findings, but I could not run the unit suite here and the paired Android run on the merged source has not finished.

Copy link
Copy Markdown
Collaborator Author

Current integration head 7f61681bb8d4b0d31dc836a1c03a8488170baf66 now passes the full paired Android workflow, attempt 6: 38/39 cases passed on their first attempt, 39/39 eventually, across all seven verified checkouts. One built-in multi_address_3 retry was required; the original wait missed ReceivedTransaction, the failure screenshot shows uncovered Home, and logcat records an Electrum incremental-sync timeout. That same case passed on unchanged sources on its next attempt. No legacy Send2 toast fallback ran.

Current-head local validation is 78 repository tests plus 4 boundary probes, typecheck, scoped lint and diff checks. Independent implementation reassessment and Talos review are clear. Main 0e83ea6 is integrated without conflicts. Existing human review requests remain pending; fresh policy-satisfying approval is the remaining gate before merging this companion and then Android #1808. No new iOS runtime result is claimed.

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

Reviewed 7f61681. The static General review found no supported introduced defects. Selected unit validation passed: 55 tests, 0 failures, 0 skipped.

The selected files were late-usdt-intro.test.ts (26 passed), wallet-home.test.ts (24 passed) and profile-copy.test.ts (5 passed), using pinned tsx 4.20.3 on Node 26.13.0. Source, package.json and the lockfile remained unchanged. Lifecycle scripts were disabled during locked dependency setup.

Coverage is limited to the General assessment and these mocked unit tests. No real-device, simulator, visual, backend or full-E2E checks ran. Security review was explicitly skipped and remains unreviewed. Formatting, lint and builds were outside the selected validation scope.

Review details

General review
Model: GPT-6.1 Sol
Reasoning effort: high

Validation
Model: GPT-6.1 Sol
Reasoning effort: high

@piotr-iohk
piotr-iohk merged commit 6881788 into main Nov 9, 2026
@piotr-iohk
piotr-iohk deleted the codex/fix-android-send2-amount-limit branch November 9, 2026 15:59
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.

4 participants