Repository navigation
fix: keep send recovery checks off main - #1725
Conversation
 [Medium impact] The changes affect payment entry and request presentation within the send flow. The PR appears safe to merge, with a non-blocking fix recommended for hardware confirmation's loading state. Findings
SummaryMoves payment-entry recovery reads off Main and keeps queued scans tied to their original admission checks.
Tests were not run during this review. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Payment input] --> B[Create admission check]
B --> C[Read recovery markers off Main]
A --> D{Wallet unlocked?}
D -->|No| E[Queue input with its admission]
E -->|Unlock| F[Replay with the same admission]
D -->|Yes| F
C --> G{Read permits entry?}
G -->|No| H[Discard refused input]
G -->|Yes| I[Recheck wallet and request]
F --> G
I --> J[Prepare payment and present]
Reviews (1) · Last reviewed commit: "fix: keep send recovery checks off main" · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Moves the wallet-recovery marker read behind BackupRepo.hasPendingWalletRestore() (IO dispatcher) so send setup suspends instead of blocking Main, gives each scan a lazy admission Deferred that travels with it through the locked queue and replay, and makes canPrepareSend also refuse when the send generation, the payment request preparation or the contact payment context moved during the read. Reviewed 1a73353, full tier, stacked on #1701 (fix/blocktank-fee-cache, head bfa61d3, which this branch contains), static only: per this repo's policy I do not build Android PR branches here, so nothing was compiled or run on my side.
What I checked, and 6 candidates I ruled out
Read in full (diff): AppViewModel.kt hunks plus launchScan, enqueueDeferredScan, mergeDuplicate, flushDeferredScan, showSheet, hideSheet, navigateToSendRoute, presentIncomingPaymentRequestOrStop, reopenRetainedHardwarePayment, resetSendState, refreshOnchainSendIfNeeded, onAmountContinue, prepareHardwareSendFee, setTransactionSpeed; BackupRepo.hasPendingWalletRestore; the new tests in AppViewModelSendFlowTest.kt and BackupRepoTest.kt
Call sites traced: launchScan (onScanResult, openContactPayment, paste, address continue, flush replay), refreshOnchainSendIfNeeded (8 callers), showSheet callers passing canPresent
CI: build, build-local, lint and detekt green; e2e-tests-local jobs still pending at review time
Ruled out
- Queued scan admitted on a stale read: the replay path awaits the receipt admission and then calls
canPrepareSend, which reads the marker again, so a read that was clear while locked does not carry past a restore that started since. - Duplicate merge keeps the wrong admission:
mergeDuplicatereturnsscan.copywhen the new scan carries a contact context andcopyof the queued one otherwise; the cancel lines cancel exactly the admission that was not kept, and cancelling a not-yet-started lazy admission only triggers aflushDeferredScanthat returns early (locked, active scheduled scan, or a sheet shown). - Double flush after a refused immediate scan: both the admission handler and
nextJob.invokeOnCompletioncallflushDeferredScan; the first pops and launches, the second seesscheduledScanactive and returns. - Refused
showSheetnow cancels the previous transition: every caller passingcanPresenthas just passedcanPrepareSenditself, so a refusal needs the marker or owner to change in that window; the new test shows the replacement is intended. - Stuck fee spinner after
setTransactionSpeed:refreshOnchainSendIfNeedednow returns a job even when the marker is pending, so the caller'sisLoading = falsefallback is skipped; it needs a restore marker to appear mid-send, on a send that is refused anyway. - Fee shown as ready before the refresh sets loading: real, and already raised inline by Greptile on the moved
updateOnchainFeeUi { it.copy(isLoading = true) }, so not repeated here.
Merge confidence: 4/5, nothing to post but the e2e jobs had not finished and I reviewed without building.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Code scan at 9f2d2b0: no findings.
- Admission is advisory and is checked again in the repository and at native dispatch.
- The wallet generation is checked before and after the IO read, and queued scans carry their generation, so a wipe or restore refuses a stale scan.
- Cancellation propagates, and every lazy admission is started, awaited or cancelled.
- Out of scope, pre-existing: two concurrent
resetSendStatecalls can still overwrite each other acrossblocktankRepo.refreshInfo().
Performance-relevant: about 20–25 keychain snapshots per Continue (up from about 6), all off Main.
Device gate (partial): 9f2d2b0, not run yet. Pending: Home → Send → Recipient timing against master, and a deferred deep-link replay after unlock.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approving. The code scan had no findings, and the device gate passed.
Device gate: 9f2d2b0 (CI APK), Gate_Throwaway emulator.
- Perf: Home → Send → Recipient took a median of 676 ms here against 660 ms on master
3c19f49(5 runs each, same device and similar host load). On video, tap to settled was 415–556 ms here and 436–491 ms on master. That is within noise: no regression, and no measurable gain on a wallet without restore markers. - Deferred deep link: with PIN on and the app force-stopped, a
bitcoin:…?amount=0.0001VIEW intent waited behind the PIN screen. After unlock, Bitcoin Amount opened once with 10 000 sat, and Back didn't replay it.
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — a refused hardware fee refresh leaves confirmation stuck.
Findings:
- MEDIUM — Recovery refusal leaves hardware confirmation loading (
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:6904). A pending or unreadable marker can cancel the refresh afterisLoadingwas set. Its completion handler clears the job reference but not the fee loading state. If confirmation has opened, clearing the marker does not restart the refresh, so the fee keeps spinning and swipe stays disabled until the user leaves and retries. Clear or recover the loading state when the current refresh is refused, while preserving ownership of any newer refresh.
QA for 9f2d2b0:
Android 15, disposable regtest wallet and Trezor emulator. For held-read cases, a test-only hook controls the marker-read outcome; payment handling is unchanged. Recordings are 2×.
Test 1: held payment entry admission ✅ The menu responded during the hold; clear opened the amount screen, while pending and an injected read error stayed on Home.
The test hook logged the read; its outcome was controlled externally. The recording shows the menu interaction and the resulting screens.
held.2x.mp4
Test 2: held admission ownership and replay ⏭️ A newer link replaced the held link, and a locked link released as pending did not open; wallet replacement and explicit cancellation were not driven on the device.
The replacement opened the 20,000-sat amount. The second recording unlocks while the original admission is held, then releases it as pending. The hook and device logs establish the held read and its outcome. The focused unit suite covers wallet replacement and cancellation.
replace.2x.mp4
locked.2x.mp4
Test 3: hardware confirmation refresh admission ✅ Confirmation kept the fee loading and did not advance on swipe while the refresh read was held; release displayed the fee.
The test hook logged the held refresh. The recording shows the attempted swipe and the completed 141-sat fee after release.
hardware.2x.mp4
Test 4: send entry timing ✅ Both revisions opened manual recipient entry; the separate timing report found no measurable regression.
Single runs on the same funded wallet are shown here: base bfa61d3, then head. For the five-run comparison against master, see the device report.
base-entry.2x.mp4
head-entry.2x.mp4
Test 5: deferred link after unlock ✅ A link received while locked opened the 10,000-sat amount screen after unlock.
The disposable wallet's PIN was set through a test-only control. The device report also checks that Back does not replay the link.
replay.2x.mp4
Test 6: hardware refresh refusal recovery ❌ After the held refresh returned pending and the marker cleared, confirmation kept spinning and swipe stayed disabled.
The test hook held the refresh read, returned pending and then clear. A focused regression using the same sequence also leaves onchainFeeUi.isLoading true. The existing fee is retained beside the spinner; no payment was sent.
refusal.2x.mp4
Check 1: head build and focused suites ✅ The unmodified head built and the send-flow and backup repository suites passed.
Reviewed the full bfa61d3..9f2d2b0 diff, including scan ownership, cancellation, sheet presentation and fee refresh callers. Wallet replacement and cancellation during a held read remain device gaps; contact-request presentation was reviewed and covered by the focused suite, without a device case. The earlier static review identified the related speed-change spinner risk; Test 6 adds a hardware confirmation reproduction after recovery clears.
No matching iOS twin PR was identified; this change moves Android's Keychain-backed read off Main.
Reviewed by gpt-6.1-terra-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full review of the complete PR diff against merge base bfa61d31, at 9f2d2b00. No prior assessment baseline was available.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
Send entry now waits on the IO-backed recovery read and keeps each queued scan's original admission through unlock and replay. A pending or unreadable marker still withholds a new send sheet, and a slow read no longer shows hardware confirmation fees as ready. A fee refresh that is then refused leaves onchainFeeUi.isLoading set, so hardware confirmation stays on that spinner with the swipe disabled until the user leaves and a later refresh succeeds.
Device testing: not performed in this review. Build and lint and the local e2e send job passed at this head. The new unit tests were inspected and not executed here. Manual cases 1–3 need a held marker read that the journey environment does not provide. Checked only BackupRepo.hasPendingWalletRestore() at bfa61d31 on synonymdev/bitkit-android#1701; that dependency was not fully reviewed. iOS 68bede62 blocks scene startup and payment-request polling while a restore is pending; SendConfirmationView has no equivalent fee-refresh gate.
Suggested additional test cases
- Android, dev/regtest hardware wallet, confirmation already open, with the marker read held the way
hardware confirmation keeps fees loading while recovery is checkedholds it. Change fee speed, then release the read as pending and repeat with the read throwing. Expected: when the read finishes, the fee row leaves the loading spinner and the swipe does not remain disabled on that spinner; no payment is submitted.
Findings
- [MEDIUM] End fee loading when recovery refuses the refresh — inline at
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:6904.
Reviewer model: grok-4.7
Reasoning effort: high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up of 618b09869 since 9f2d2b001, on merge base bfa61d31. Inherited baseline: review 5679860985. This pass inspected refused fee-refresh completion, the hardware swipe and confirm gates, and the confirmation fee-row retry. Inherited coverage is send scan admission and queued replay, BackupRepo.hasPendingWalletRestore and BackupRepoTest, the iOS pending-restore comparison at 68bede62269d80428cf572848d16a18ff9b7b9be, and the #1701 recovery-read contract at bfa61d31.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
A refused speed change now clears the fee and disables swipe, and that job path keeps a newer refresh intact. The confirmation fee row is the control that can run the next refresh, and after estimates are cleared it uses a direct fee reload that does not apply this cleanup. While the recovery marker is still pending or unreadable, that tap does not open fee selection. If the reload has already published loading and then refuses, the spinner stays up and the row stops accepting taps until send state is replaced.
Validation is source inspection at the pinned head. Hosted build and apk-comment and lint succeeded on this revision; detekt succeeded in that build's check set. The new send-flow tests were read and not executed here. They retry through setTransactionSpeed. Device testing was not performed. iOS was not re-read for this refusal cleanup. #1701 was not reviewed again.
Suggested additional test cases
- Android dev/regtest, paired hardware wallet, confirmation open. Hold the recovery read, change fee speed, and release it as pending or unreadable so the amount disappears and swipe stays disabled. Tap Fee and speed while the marker is still refused. Expected: the row does not spin forever, swipe stays disabled, and the row remains usable. Clear the marker and tap again. Expected: fee selection opens and a completed refresh enables swipe.
- Same setup, with the marker becoming pending only after the fee-row reload has shown its spinner. Expected: loading ends, swipe stays disabled, and Fee and speed can be tapped again.
Earlier findings reconciled at this revision: 2 verified fixed.
Findings
- [MEDIUM] Fee-row retry does not finish a refused refresh — inline at
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:6967.
Reviewer model: grok-4.7
Reasoning effort: high
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Re-reviewed to 60f141f (618b098, 60f141f). No HIGH or MEDIUM.
isHardwareFeeReadygates the swipe,onSwipeToPayandonConfirmPay, and a refusal nullssatsthroughrequiresRefresh, so a stale hardware fee can't be swiped.- Nulling before cancelling keeps ownership correct, and there is no retry loop.
- This also fixes the open MEDIUM threads from ovi-reviewer and piotr-iohk.
My approval at 9f2d2b0 doesn't cover the hardware fee path these commits change.
Device gate (partial): 60f141f, not run yet. Pending: hardware-wallet/send-onchain.xml with a fee-speed change on the hardware Confirm, and manual coin selection → Continue.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approving at 60f141f. The re-review found no HIGH or MEDIUM, and the device gate passed.
Device gate: 60f141f, Gate_Throwaway emulator. Local dev build with the Trezor Bridge, because CI dev builds have TREZOR_BRIDGE=false; Trezor emulator.
- Hardware send with the speed changed from Normal to Fast on Confirm: the fee shown (₿ 423) matched the Trezor summary (0.00000423) and the broadcast (tx
1d17c0ca…6705). - While the hardware fee loads, the swipe is greyed out, and a swipe 0.6 s after the speed change was ignored.
- Estimate failure (airplane mode on during the speed change): the fee row shows "Fast" with no amount and the swipe stays enabled. Swiping re-estimates and signs the fresh 423 (tx
dd8da280…2606), so no stale fee is signed. This goes through the existing estimate-failure path, not the new refusal. - Software wallet, manual coin selection → Continue reached Confirm in about 2 s, and the send spent only the chosen UTXO (tx
d5902990…212b).
requiresRefresh refusal path. It needs a pending-restore marker fault, which isn't provisioned (pending-wallet-recovery.xml lists it as manual). It rests on the code scan and unit tests.
Perf: not relevant.
Note for the journey: hardware-wallet/send-onchain.xml expects SendSuccess, but the success sheet's tag is ReceivedTransaction.
There was a problem hiding this comment.
Verdict:
Findings: None in the reviewed delta.
QA for 60f141f:
Android 15, disposable regtest wallet and Trezor emulator. A local test hook controlled the recovery-marker read; the payment handling remained unchanged. The recording is 2×.
Test 1: pending refresh and unreadable fee-row retry ✅ A pending refresh cleared the fee, an unreadable retry stayed on confirmation, and a clear retry opened speed selection.
The hook log confirmed the held refresh; the marker outcomes were controlled externally. The recording shows confirmation and the disabled swipe; the device accessibility tree confirmed speed selection opened after the clear retry.
retry.2x.mp4
Test 2: held admission ownership and replay ⏭️ Wallet replacement and explicit cancellation during the held read remain untested on device.
The previous device review covered a newer link replacing a held link and locked replay. The focused unit suite covers wallet replacement and cancellation.
Test 3: hardware refresh refusal and recovery ⏭️ The held speed-change refresh was not driven with both pending and unreadable outcomes.
Test 1 covered a pending initial refresh. The focused suite covers pending and unreadable speed-change refreshes and their retries.
Test 4: fee-row and coin-selection retry ownership ⏭️ Refusal after estimation and replacement before navigation were not driven on device.
Test 1 covered an unreadable fee-row retry before estimation. The focused suite covers refusal after estimation and replacement of the send, wallet or refresh before navigation.
Check 1: build and focused suites ✅ The unmodified head built and the send-flow and backup repository suites passed.
The current-head device report covers a hardware send with a speed change and software manual coin selection. The previous review covers held entry, clear admission, and deferred replay on unchanged paths.
Reviewed the 9f2d2b0..60f141f delta and the prior open fee-refresh findings. The superseded-navigation timing changes were checked in the focused suite but not driven on device. No matching iOS twin PR was identified; iOS has no equivalent confirmation fee-refresh gate in the previously reviewed path.
Reviewed by gpt-6.1-terra-high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up review of 60f141f7 since 618b0986, on synonymdev/bitkit-android#1725, with merge base bfa61d31. Inherited baseline: pullrequestreview-5680796694. This pass covered the fee-refresh delta only; scan admission, backup IO, the pinned iOS pending-restore comparison, and the #1701 recovery contract stay inherited.
No new actionable code findings.
Empty-estimate fee-row retries and coin-selection continuation now use the shared refresh job. A refusal still ends loading, clears estimates, and sets requiresRefresh. A superseded or replaced job does not clear a newer fee state. Navigation is emitted only while that job is still active for the same send generation and wallet generation. Hardware swipe and PayConfirmed stay tied to isHardwareFeeReady; the fee row stays enabled once onchainFeeUi.isLoading is false. At iOS 68bede62, the fee row stays disabled while the hardware preview is loading and confirmation stays unavailable while that preview is loading or the preview fee is zero.
Validation: CI run 40678701173 and Lint run 40678701174 succeeded on this head. The new AppViewModelSendFlowTest cases were inspected, not executed locally; Gradle plugin resolution did not complete in this environment. Device testing was not performed in this review. The held-storage fault cases remain outside the standard device environment.
Earlier findings reconciled at this revision: 2 verified fixed.
Reviewer model: grok-4.7
Reasoning effort: high
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — hardware speed changes still have an inherited recovery-refusal failure.
Findings: No new findings in the reviewed delta. Test 4 below fails in the preliminary balance check, before the tracked refresh starts; that path is unchanged from the prior reviewed head and comes from the merged parent.
QA for 72c2e48:
Android 15, disposable regtest wallet and Trezor emulator. Local test hooks controlled marker reads, cancellation, wallet-generation changes and navigation timing. Recordings are 2×. The wallet-generation hook exercised the real identity-transition API without replacing credentials.
Test 1: held admission ownership and cancellation (Manual 2) ✅ Cancelling held initialization and changing a held link’s wallet generation prevented obsolete state or presentation.
The recording shows Home remaining available. Hook logs confirm the held read, cancellation and generation change; state inspection confirmed no send sheet or initialized send state after release. Request replacement and locked replay use the earlier passing report linked below. This did not perform a full wallet restore.
case2.2x.mp4
Held admission with cancellation and an injected ownership transition.
Test 2: hardware speed-change refusal and recovery (Manual 4) ✅ Pending and unreadable tracked refreshes stopped loading, cleared stale fees and kept swipe disabled; clearing recovery allowed a fresh retry.
Both outcomes were held specifically at the tracked refresh boundary. Repeated Fee and speed taps while recovery refused also ended loading and stayed on confirmation. A clear retry opened Speed with fresh estimates. Marker logs and state inspection establish the injected outcomes and readiness flags; the recording shows the controls and swipe attempts.
case4.2x.mp4
Pending and unreadable refusals, followed by successful retries.
Test 3: fee retry and continuation ownership (Manual 5) ✅ Refusal after estimation ended loading, and replaced send, wallet or refresh owners prevented obsolete retry navigation.
For pending and unreadable outcomes, the hook log confirms an estimate returned before the held admission read. Both refusals cleared estimates and left confirmation retryable; a clear retry opened Speed.
case5after-b.2x.mp4
Pending and unreadable refusal after estimation, then a clear retry.
The following checks dispatched the public Fee and speed and coin-selection Continue events through a local hook and delayed navigation before its ownership predicate. Logs confirm each attempted navigation returned permitted=false; state inspection and the recordings confirm the obsolete fee screen did not open. Coin-selection Continue used an empty selection on the hardware fee path, so this verifies the continuation’s ownership gate rather than a software UTXO-selection journey.
| Replaced owner | Fee and speed | Coin-selection Continue |
|---|---|---|
| Newer refresh | nav-fee-refresh.2x.mp4 |
nav-coin-refresh.2x.mp4 |
| Wallet generation | nav-fee-wallet.2x.mp4 |
nav-coin-wallet.2x.mp4 |
| Send state | nav-fee-send.2x.mp4 |
nav-coin-send.2x.mp4 |
Test 4: preliminary hardware balance admission (additional case) ❌ Refusing the balance check during a fee-speed change left confirmation spinning after recovery cleared.
On hardware confirmation, open Speed, hold the marker read and choose Fast. Release that preliminary read as pending, then clear recovery. The old Normal/141-sat fee keeps spinning, the fee row does not open and swipe does not advance. State inspection confirms isLoading=true and hardware readiness false after the marker clears.
setTransactionSpeed publishes loading, then its untracked coroutine calls hardwareMaxSpendable (AppViewModel.kt:4140). A refused admission throws before launchOnchainFeeRefresh installs its cleanup. This needs cleanup around the preliminary work as well, while preserving ownership of newer state. No payment was submitted.
This is inherited from the parent’s recovery guards, and is present at both 60f141f and parent cce1c8d3d; it is not a new delta inline finding or a recurrence in the tracked-refresh cleanup tested above.
case6.2x.mp4
Preliminary read refused, recovery cleared, and confirmation still loading.
Check 1: exact-head build and focused suites ✅ The unmodified head built and the send-flow and backup repository suites passed.
The instrumented device builds add only local fault controls; they were not pushed.
Check 2: reused device coverage ✅ Earlier passing cases cover unchanged initial admission, held confirmation, request replacement and queued replay.
The earlier device review proves Manual 1 and Manual 3, newer-link replacement and locked replay on their unchanged paths. The later device review covers an ordinary hardware speed-change send and software manual coin selection. Those journeys were not repeated here.
Reviewed 60f141f..72c2e48, including the parent integrations around funding-source switching, request finalization, subscription handoffs and wallet ownership, plus prior fee-refusal findings. New parent Bitcoin/USDT confirmation interactions were checked in source and the focused suite, without a new device journey.
No matching iOS twin PR was identified; this change moves Android’s Keychain-backed send-admission read off Main.
Reviewed by gpt-6.1-sol-high
There was a problem hiding this comment.
Verdict: ✅ Approve — the preliminary hardware balance refusal now recovers, and obsolete work preserves newer state.
Findings: None in this delta. The failure reported in the previous review is fixed at this head.
QA for 2643b49:
Android 15, disposable regtest wallet and Trezor emulator. Local, unpushed hooks controlled recovery-read outcomes and held reads across cancellation. State inspection and accessibility checks establish the injected boundary and ownership; the recordings show the resulting controls. Recordings are 2×. No payment was submitted in these fault cases.
Test 1: preliminary balance refusal and retry (Manual 6) ✅ State and UI checks confirmed both refusals ended loading, cleared stale fees, blocked swipe and allowed a fresh retry.
From hardware confirmation, open Speed and select a different speed while the preliminary recovery read is held. Release it as pending or unreadable. Both outcomes left isLoading=false, empty estimates and requiresRefresh=true; swipe did not advance. A clear Fee and speed retry opened Speed with fresh estimates and restored readiness.
| Pending | Unreadable |
|---|---|
manual6-refusal-pending.2x.mp4 |
manual6-refusal-error.2x.mp4 |
Preliminary refusal, disabled swipe and a clear retry.
Test 2: refusal after preliminary preparation (Manual 4) ✅ State and UI checks confirmed both tracked-refresh refusals recovered and a clear retry reopened Speed.
These reads were held after preliminary balance preparation completed. Pending and unreadable results ended loading, cleared the previous fee and kept swipe disabled. Repeated fee-row taps while recovery remained refused also ended loading; a subsequent clear retry produced fresh estimates.
| Pending | Unreadable |
|---|---|
manual4-tracked-pending.2x.mp4 |
manual4-tracked-error.2x.mp4 |
Tracked refusal and successful retry. The repeated-refusal checks were also driven in the combined run and verified through state and accessibility inspection.
Test 3: newer refresh ownership (Manual 6) ✅ State checks confirmed an obsolete completion preserved the newer refresh's loading state and final fees.
Hold the first preliminary read across cancellation, then start and hold a newer Normal refresh. Releasing the obsolete read as pending left the newer refresh loading. Releasing the newer read as clear produced the fresh Normal/141-sat fee and restored readiness. The second speed action used the public setTransactionSpeed method through a local hook because the fee control is disabled while loading.
manual6-refresh.2x.mp4
Newer refresh remains in control across the old read's completion.
Test 4: replacement send ownership (Manual 6) ✅ State checks confirmed a reset send remained unchanged after the old preliminary read completed.
Hold the preliminary read across cancellation, call the existing send-reset API and snapshot the replacement state. Releasing the old read preserved that snapshot: zero amount, default speed, no hardware source and no stale fee.
manual6-send.2x.mp4
The reset send stays intact after the obsolete read is released.
Test 5: replacement wallet ownership (Manual 6) ✅ State checks confirmed the old wallet generation could not overwrite the replacement send state.
While the preliminary read remained held, invoke the real wallet identity-transition API and reset send state under the new generation. Releasing the old read left the replacement snapshot unchanged. This exercised the generation transition without replacing credentials or performing a full wallet restore.
manual6-wallet.2x.mp4
Replacement state survives completion from the old wallet generation.
Check 1: exact-head build and focused suites ✅ The unmodified head built and the send-flow and backup repository suites passed.
Ran assembleDevDebug and testDevDebugUnitTest filtered to AppViewModelSendFlowTest and BackupRepoTest before adding the local fault hooks. The instrumented APK was built separately and its installed bytes matched the built APK.
Check 2: inherited device coverage ✅ Earlier passing reports cover the unchanged admission, replay and retry-navigation paths.
- The earlier device review covers Manual 1 and Manual 3, newer-link replacement and locked replay on unchanged paths.
- The previous review covers Manual 2 cancellation and wallet-generation replacement, plus Manual 5 post-estimation refusal and obsolete fee-row/coin-selection navigation. Those paths are unchanged by this delta.
- The hardware and coin-selection report supplies unchanged software manual coin-selection coverage. Its ordinary hardware signing/broadcast journey was not repeated; this pass retested the changed speed-preparation boundary without submitting a payment.
Reviewed the contiguous 72c2e48..2643b49 delta and the previous preliminary-balance failure, including shared refresh callers, cancellation, send/wallet ownership and hardware confirmation readiness. Software speed preparation and no-amount exits were reviewed in source; no separate software speed device case was driven. No planned device case remains unrun.
No matching iOS twin PR was identified; this repairs Android's recovery-read and fee-refresh ownership boundary.
Reviewed by gpt-6.1-sol-high
The merge-base changed after approval.
The merge-base changed after approval.
|
@ben-kaufman Updated this branch with master: the ruleset dismisses approvals on a branch behind its base, so the review approval could not stick. The new head needs its re-review before merge. |
There was a problem hiding this comment.
Verdict: ✅ Approve — the master merge preserves the reviewed recovery and fee-refresh behavior.
Posted as a comment: not every planned case passed.
Findings: None in this delta. The previous preliminary hardware balance refusal remains fixed.
QA for 09ebdb5:
No device cases were rerun: the six manual cases retain passing evidence on unchanged paths. The five files belonging to this PR are byte-identical to 2643b49; the merge adds background Paykit admission changes from master.
Check 1: exact-head build and affected suites ✅ The unmodified head built and the five affected unit suites passed.
Ran assembleDevDebug and testDevDebugUnitTest filtered to AppViewModelSendFlowTest, BackupRepoTest, PaykitPaymentRequestRepoTest, PrivatePaykitRepoTest and PaykitSdkServiceTest. Coverage includes held recovery reads, cancellation, send/wallet ownership, fee refusal and retry, queued SDK admission, retained background work, and explicit operations during payment review.
Check 2: initial admission and confirmation loading (Manual 1 and 3) ✅ The earlier device evidence remains applicable to the unchanged recovery and hardware fee paths.
The held-admission device review shows Main responding during a held read, clear admission proceeding, pending/unreadable admission refusing entry, and hardware confirmation keeping fees loading and swipe disabled until a clear refresh completes. Its later failing refusal case is superseded by the passing fix evidence below.
Check 3: cancellation, replay and retry navigation (Manual 2 and 5) ✅ Earlier passing subcases still cover request ownership, replay, refusal and obsolete navigation.
The initial device review covers request replacement and locked replay. The later device review adds cancellation, wallet-generation replacement, refusal after estimation, and obsolete fee-row/coin-selection navigation. These paths and their dependencies are unchanged by the merge. The wallet case exercised the identity-generation transition, without replacing credentials; coin-selection ownership used an empty hardware selection.
Check 4: tracked and preliminary hardware refusal (Manual 4 and 6) ✅ The passing fix report remains applicable to both refusal boundaries and replacement ownership.
The previous-head device review covers pending and unreadable results, stale-fee cleanup, disabled swipe, clear retries, and preservation of newer refresh, replacement send and wallet-generation state. Those implementations are unchanged at 09ebdb5. The tests used local fault controls; the newer-refresh action used a public API hook, and the wallet case did not perform a full restore.
Reviewed the contiguous 2643b49..09ebdb5 delta and prior resolved findings. The inherited background admission changes were checked through SDK locking, publication, retry retention, cancellation, profile invalidation, explicit operations and payment review/submission ownership. They do not intercept the recovery-marker or hardware fee paths. No planned device case remains unrun; no new device result is claimed for the merged background work. Software speed preparation and no-amount exits remain source-reviewed only.
Twin: no matching iOS PR is linked for this Android-specific off-Main recovery-read change.
Reviewed by gpt-6.1-sol-high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Re-reviewed the full PR diff against merge base 0924ae5d, at 09ebdb57. This is a full reassessment: no coverage baseline was supplied, and the comparison base changed from bfa61d3 to 0924ae5, including send confirmation, AppViewModel, and the send-flow tests. Inherited coverage is empty.
No new actionable code findings.
Payment entry still waits on the IO-backed recovery read and refuses a pending or unreadable marker, including the original admission carried by a queued scan. A refused hardware fee refresh clears the amount, ends loading, and isHardwareFeeReady blocks swipe and confirm; Fee and speed retries use that same tracked refresh. The earlier stuck-loading and fee-row comments were rechecked at this head and that cleanup now applies. Compatibility with synonymdev/bitkit-android#1701 (cce1c8d3, already in the base) was limited to payment-confirmation ownership: a held recovery read does not drop the same request when its confirmation sheet is visible. That was not a review of #1701.
iOS 383491f2 disables confirmation swipe while the fee rate is missing and shows Try again after a failed load. It does not use this Android recovery-marker admission.
Device testing: not performed in this review. AppViewModelSendFlowTest.kt and BackupRepoTest.kt were inspected and not executed. Held marker-read cases remain outside the standard journey environment.
Reviewer model: grok-4.7
Reasoning effort: high
|
Updated this branch with master so its approval sticks: the ruleset dismisses approvals on a branch behind its base. No change to the PR's own code. |
There was a problem hiding this comment.
Verdict: ✅ Approve — the merge preserves the reviewed recovery fixes and brings the tested contact-preview behavior into this composition.
Findings: None in this delta. The earlier hardware refusal and retry findings remain fixed.
QA for fba00b0:
No device cases were rerun. The six manual recovery cases and three merged contact cases retain passing reports on unchanged implementations. Failed or incomplete earlier runs are superseded only by the later passing subcases linked below.
Check: exact-head build and affected suites ✅ The unmodified head built and the send-flow, backup repository and contact-preview suites passed.
Ran assembleDevDebug and testDevDebugUnitTest filtered to AppViewModelSendFlowTest, BackupRepoTest and AddContactViewModelTest. These cover held reads, cancellation, send/wallet ownership, fee refusal/retry, stale navigation and contact discovery/save ownership. Full lint and broader suites were not rerun in this delta review.
Check: held initial payment admission (Manual 1) ✅ Earlier device evidence shows Main responding and only clear admission opening payment.
The held-admission report covers clear, pending and unreadable outcomes. The merged contact changes do not alter this path.
Check: admission ownership and replay (Manual 2) ✅ Earlier passing subcases cover replacement, cancellation and the original queued admission.
The initial report covers newer-link replacement and locked replay; the later ownership report adds cancellation and wallet-generation replacement. The latter exercises the identity-generation API without replacing credentials or performing a full restore.
Check: hardware fee admission loading (Manual 3) ✅ Earlier device evidence shows loading and disabled swipe until the held clear refresh completes.
The hardware admission report remains applicable to the unchanged confirmation and refresh path.
Check: tracked hardware refusal and retry (Manual 4) ✅ The later fix report proves both refusal outcomes clear stale fees, end loading and permit a fresh retry.
The passing fix report supersedes the earlier refusal failure. It also verifies repeated refused fee-row taps keep confirmation retryable and swipe disabled.
Check: fee retry and navigation ownership (Manual 5) ✅ Earlier device evidence covers refusal after estimation and suppression of obsolete navigation.
The retry-ownership report covers pending/unreadable outcomes and replacement send, wallet or refresh owners. Coin-selection continuation used an empty hardware selection to test its navigation predicate; this is not a software UTXO-selection journey.
Check: preliminary balance refusal and ownership (Manual 6) ✅ The later fix report proves cleanup, clear retries and preservation of replacement state.
The passing preliminary-read report covers pending and unreadable reads, newer refreshes, reset sends and wallet-generation replacement. The newer-refresh action used a public API hook. These recovery reports used local fault controls; no new device execution is claimed here.
Check: save during endpoint discovery (contact J1) ✅ The merged contact report proves Save completes while discovery is held, with Pay absent and one saved peer.
The device report at d19deb5 used an unmodified Android 15 app and a TLS relay. The contact preview, persistence, endpoint lookup and dependency versions are identical here. Saving does not establish private-link readiness.
Check: leave during endpoint discovery (contact R1) ✅ The same report proves Back closes the preview and releasing discovery does not save or reopen it.
This reuses the passing cancellation case on identical contact code.
Check: retry a failed contact save (contact R2) ✅ The same report proves retry works and endpoint completion cannot unlock an active save or create duplicates.
The passing failure/retry case holds the contact write separately from discovery and verifies one failed attempt followed by one successful addition. Those implementations are unchanged here.
Reviewed the contiguous 09ebdb5..fba00b0 delta, all four merged files and affected callers, plus prior resolved findings. The five send-recovery PR files are byte-identical to the previous reviewed head. Contact-to-payment admission was checked in source and the focused suite; software speed preparation and no-amount exits remain source-only. No planned device case remains unrun.
Twin: no matching iOS PR is linked for this Android recovery-read change. The merged contact behavior matches bitkit-ios#1119 fix: keep contact previews independent of Paykit, including early Save, guarded Pay, cancellation and duplicate-save prevention.
Reviewed by gpt-6.1-sol-high
|
@ben-kaufman Merging this PR on the approvals of head fba00b0, with checks green and no open threads. The head only adds a master merge; ovi-reviewer's earlier device runs cover this PR's unchanged code. |
|
@ben-kaufman Merged, as it was approved and green. |
Fixes #1719
Dependency: #1701 — merged into
masteron October 28.Payment entry now waits for durable wallet-recovery checks without blocking Main. This PR now targets
master.Description
Out of Scope
Design
Bitkit design:
Send (Paste) (On-chain) › Confirm Send Onchain, as mapped indocs/screens-map.md. This change affects fee availability and enabled state; the exact frame permalink is not verified.Preview
N/A — existing screens and navigation are unchanged.
QA Notes
Setup
Use the dev/regtest build with a disposable wallet. Ordinary entry requires an available regtest backend. The earlier local build used
E2E=true E2E_BACKEND=local E2E_LOCAL_HOST=localhost GEO=false(the parent has since removed the GEO flag), Blocktank athttp://localhost:18685/api/v2, VSS athttp://localhost:18689/vss, LNURL auth athttp://localhost:18655/auth, and Electrum attcp://localhost:60001; reverse those device ports to the local host. The held-storage cases below require a controllable marker-read fault; that full-app capability is not provided by the standard journey environment. The automated tests provide deterministic held-read controls, andBackupRepoTest.ktalso holds a real blocking storage call on a separate executor.Journeys
N/A — not drivable; see Manual Tests.
Manual Tests
The author environment still lacks the listed fault controls. The external review on
2643b49reports passing Manual 4 and 6 boundaries with private hooks; its qualifications and inherited coverage are recorded below. These checklist entries retain the author-environment limitations.1. Hold the initial restore-marker read during payment entry → Main remains responsive and payment presentation waits; release clear, pending and unreadable results → only clear may proceed — controlled storage-read faults not in Capabilities.
2. Replace the wallet/request or cancel entry while the read is held, then release it → no stale presentation; unlock a queued request during the hold → its original admission result still applies — controlled storage-read faults not in Capabilities.
3. Hardware wallet → Send Amount → Continue while the fee refresh recovery read is held → confirmation keeps fees loading and swipe disabled until refresh completes — controlled storage-read faults not in Capabilities.
4. Hardware confirmation → change fee speed with the refresh marker read held, then release pending or unreadable → loading ends, stale fee estimates disappear and swipe stays disabled; clear recovery and retry fee selection → fresh fees restore confirmation — controlled storage-read faults not in Capabilities.
5. After case 4 refuses, tap Fee and speed while recovery remains pending/unreadable, or becomes refused after estimation starts → loading ends, stale fees stay cleared and swipe stays disabled; clear recovery and tap again → fee selection opens. Replace the send/wallet or start a newer refresh before navigation is delivered → the obsolete request must not navigate — controlled storage and dispatch timing faults not in Capabilities.
6. Hardware confirmation → open Speed, hold the preliminary balance recovery read, choose Fast and release pending or unreadable → loading ends, stale fees clear and swipe stays disabled; clear recovery and retry → fresh fees return. Replace the send, wallet or refresh while held → obsolete work must not change the replacement state — controlled storage-read faults not in Capabilities.
Automated Checks
AppViewModelSendFlowTest.kt— held-read outcomes, Main progress, wallet/request replacement, cancellation, queued duplicate ownership, confirmation visibility during fee preparation, hardware fee loading during refresh recovery admission, refusal/retry, stale refresh completion after replacement, public fee-row retries, and queued success navigation after send/wallet/refresh replacement.BackupRepoTest.kt— a blocking marker storage read runs off Main.AppViewModelSendFlowTest.kt— locked replay retains its ownership while the initial marker read is held.Current master integration — October 29
Current head:
fba00b02475f44c236cae5892936d0e3dbc77302. Ovi mergedmasterata56c75be3, adding #1683's contact-preview changes. The five-file child patch and all five child file blobs are identical to the previously reviewed09ebdb5composition; the four upstream file blobs match the integrated base. All nine commits in the live-base PR range verify cryptographically locally and on GitHub (verified=true,reason=valid). Existing child source/recovery evidence retains its input scope. No new author source change, local test rerun or device campaign was made.Fresh CI and lint on merge checkout
4dac182e4(treee897196c1, identical to this head) pass 4,689 unit tests across 256 classes with zero Detekt/Android Lint findings. Every prior 4,684 test identity is retained; the five added cases are verified in the integrated #1683 contact-test file. Complete supplied logs, nested test stderr, all six lint reports and both report archives were audited, including cached tasks and the SARIF upload's continue-on-error behavior. Six release-only migration jobs were skipped without running tests.Fresh E2E records 39 passing cases and one unexecuted
settings_08case, with no failed cases or whole-suite retries; retry steps 2 and 3 were skipped in every shard. All seven shards physically checked out suite4c0cbef019220efed01ed3080c1f0ccc1fb627a3and verified the download digest of current APK artifact12885874689against the build upload. Complete shard/status logs and annotations were audited. Current-run qualifications include two within-case helper retries (insufficient-savings input andSendAmountExceededToast, with unverified causes), six QuickPay helper-trigger skips, 61 stale WebDriver warnings, two GL-context errors and four ColorBuffer errors with unverified application impact. One notification helper timeout leaves permission grant unproven. The suite installer reports 76 untriaged vulnerabilities; full npm details, helper sources, and nested E2E app/media/reporter artifacts are unavailable. APK archive correlation is not an independent binary-signature audit.Native qualifications remain explicit: recurring cursor/JVM/manifest and unstripped-library diagnostics were checked against unchanged producer/build inputs. Five generic Gradle deprecation summaries lack the hosted problems HTML needed to identify their nested details; the exact executed daemon/test Java patch is not exposed. No new author device run is claimed, and ordinary hosted E2E does not replace the held-storage/hardware acceptance cases.
Piotr's source review on
09ebdb5reports no new actionable code findings and rechecks the earlier refusal/fee-row fixes. It is COMMENTED, and explicitly did not execute tests or device cases. Its self-reported model isgrok-4.7with high reasoning effort; this is attributed reviewer metadata. Two current-head named-human approvals remain required, with none yet recorded. No duplicate review request was sent.masteradvanced again to92628aa45with #1639 after this merge. That change adjusts passive Paykit proof-display refresh priority and touches sharedAppViewModel.ktandAppViewModelSendFlowTest.kt; it is not included infba00b0. Results in this section cover the submittedfba00b0/a56c75bcomposition, not a combined tree with92628aa45. Current-base composition validation and branch freshness therefore remain pending. No merge or extra device campaign was performed by this worker.Previous master integration and preliminary speed recovery fix
Previous head:
09ebdb5766cd533bb691342c10b94df5f65038eb. Ovi merged currentmaster(0924ae5d6) into the branch so it contains its base; the earlier approvals had been dismissed under the branch rules. This preserves the seven existing PR commits and adds one signed merge. All eight commits in the live-base PR range verify locally and on GitHub (verified=true,reason=valid). The file tree is identical to the previously audited hosted mergec0caf1402, and the five child source/test/changelog blobs are unchanged from2643b49; existing evidence remains tied to its tested inputs. No redundant local suite or source review was launched.Fresh CI and lint runs on merge checkout
5522097ce(the same file tree as this head) pass 4,684 unit tests across 256 classes with zero Detekt/Android Lint findings. Complete supplied logs, nested unit stderr, and all six lint reports were audited. E2E has 39 passing cases and one unexecutedsettings_08case, with no failed cases or suite retries. All seven shards checked out suite4c0cbef019220efed01ed3080c1f0ccc1fb627a3and verified the download digest of current APK artifact12868225895; old APK evidence was not substituted. Six release-only migration jobs were skipped without running tests.Qualifications for the
09ebdb5run: oneSendAmountExceededToasthelper retry has an unverified cause; the old expired-invoice retry did not recur. Six new emulator graphics messages (context preemption/restoration) are retained without inferring an application defect or clean visuals. The run also has 52 stale WebDriver warnings, six QuickPay helper-trigger skips and one notification helper timeout that leaves permission grant unproven. The E2E dependency installer again reports 76 untriaged vulnerabilities. Native cursor/JVM/manifest and packaging messages recur with the documented comparisons. Hosted Gradle problems HTML, full npm audit details, suite helper sources and nested E2E app/media/reporter artifacts were unavailable; exact daemon/test Java patch is not exposed. APK archive digests were correlated in build/download logs, with no independent binary-signature audit. These ordinary E2E cases do not replace controlled recovery/hardware acceptance.The review on
09ebdb5reports no source findings in the master integration and a passing build plus five affected unit suites. GitHub records it as COMMENTED, with a neutral check: its headline says Approve, but it also says not every planned case passed without identifying a failed case. No approval or new device pass is inferred from that wording. It reused earlier device evidence on unchanged paths and explicitly ran no device cases on this head; complete external build/test reports were not published. The prior2643b49approval and its hook/evidence limitations remain revision-scoped. Two named-human approvals are still required; no duplicate review request or author device campaign was added.mastersubsequently advanced toa56c75be3with #1683, which changes the contact-preview ViewModel, its tests, changelog and journey. The send-recovery files and build configuration are unchanged. The hosted results above validate the0924ae5d6composition; they do not validate a combined tree with this newer base. No automatic base integration or redundant full suite was performed for this unrelated change. Base freshness remains a pre-merge consideration under the branch rules.Preliminary-refusal fix commit:
2643b493657c37ab52d42a4fe0cae2edbed29559. The preliminary hardware balance work during a speed change now runs under the existing fee-refresh owner. Pending or unreadable recovery clears stale fees and ends loading; a clear retry can refresh them. The operation retains the original wallet and send generation, and newer refreshes cancel the preliminary work. This repairs a reachable failure inherited from the parent, reported in review 5690679734.Three existing tests now also cover preliminary refusal/retry, supersession and send/wallet replacement; no tests were added or removed. Required final
just format,just compile,just testandjust lint: 4,677 tests across 256 suites, zero failures/errors/skips and zero format/Detekt/Android Lint findings. Complete logs and structured reports were audited; all 1,757 tracked input hashes match. Verified pre-existing cursor/JVM warnings remain recorded. Two independent affected-delta source reviews found no actionable defects. All commits in the current-base PR range are cryptographically signed and verified locally and on GitHub (verified=true,reason=valid). Local validation uses the frozen branch; hosted CI also validates its current-master merge. Hosted CI and lint have completed on merge checkoutc0caf14027c4b4426ca7c0593d528dfa8163e0e3(actualmaster0924ae5d6plus this head): 4,684 passing unit tests across 256 suites and zero Detekt/Android Lint findings. All 4,677 locally tested cases are retained; seven additional cases were verified in upstream #1626. Complete supplied logs and reports were audited, and both report archives were verified. Existing cursor/JVM and unchanged-manifest warning qualifications remain; native-library stripping warnings are retained without a proven historical runtime classification. Hosted Gradle problems HTML was not uploaded, so exact deprecation details remain unavailable. APK upload metadata was checked; the binaries were not independently downloaded or hashed. The E2E run completed with 39 passing cases and one unexecuted case (settings_08), all on the first suite attempt; retry steps 2 and 3 were skipped. Every shard checked out suite4c0cbef019220efed01ed3080c1f0ccc1fb627a3and downloaded APK artifact12859383674with the expected archive digest. Complete shard/status logs and annotations were audited. Retained diagnostics include the numberpad notification helper timeout, one send-input helper retry with an unverified cause before the expected expired-invoice validation, stale-element warnings, helper-trigger skips, one verified recovered ADB startup failure, and other emulator setup/graphics/teardown diagnostics. The E2E dependency installer reports 76 untriaged vulnerabilities (8 low, 17 moderate, 44 high, 7 critical); the complete npm audit report is unavailable. These diagnostics retain their individual qualifications. Successful shards did not upload app logs, media, or separate test-report artifacts, so those details remain unavailable. These hosted results do not close the recorded controlled-storage/device coverage gaps or constitute an author device rerun of case 6. The release-only migration workflow skipped six jobs and ran no tests. Prior hosted results below remain evidence for their stated revisions.The external review on
2643b49now approves the delta with no findings and reports that the preliminary-balance failure is fixed. On Android 15 with a disposable regtest wallet, Trezor emulator and local unpushed hooks, it reports passing preliminary pending/unreadable refusal and retry (Manual 6), tracked-refresh refusal and retry (Manual 4), and preservation of newer refresh, replacement send and wallet-generation state (Manual 6). It also reports a passing build and focused send-flow/backup suites on the unmodified head before instrumenting a separate APK. These are attributed reviewer results: complete hook sources, runtime logs and structured reports were not published, and the linked recordings were not independently audited here. The newer refresh action used a public API hook; the wallet case exercised the identity-generation transition without replacing credentials or performing a full restore. Software speed/no-amount exits were source reviewed only, and ordinary hardware signing/broadcast was not repeated. No new author device campaign is claimed.The external review on
72c2e4822reports passing controlled admission cancellation, tracked-refresh refusal/retry, post-estimation refusal and obsolete-navigation checks, and failing the newly listed preliminary-balance case 6. It used Android 15, a disposable regtest wallet, a Trezor emulator and local unpushed hooks. The wallet-generation hook did not replace credentials, and coin-selection continuation used an empty hardware selection. Complete hook sources, runtime logs and structured reports were not attached, so this is attributed reviewer evidence; no independent device rerun or new device campaign is claimed for the fix. Cases 1–5 retain their IDs and earlier evidence; case 6 records the distinct newly verified failure boundary.Master retarget — October 28
After #1701 merged, this PR was retargeted to
master. Published head72c2e4822already contains #1638 and the lateronPaymentMethodSwitchreconciliation from parentcce1c8d3d. A local merge preview against actualmaster0924ae5d6is conflict-free, and the resulting five-file child patch is byte-for-byte identical to the previously reviewed and validated patch. No source edit, history rewrite or new test was needed for the reported conflict.The new upstream-only delta is #1626's background Paykit admission work. Existing local and hosted checks below validate the published head composed with
cce1c8d3d; they do not validate the newer combined preview tree40cd6200ba9b8bac8763384b4c53951a722b8982. No additional full suite or device campaign was launched solely for the retarget. The two current-head named-human approvals remain outstanding, and the manual coverage gaps below remain open.Previous head validation — October 28
Previous head:
72c2e4822caede4c5ab8a3b89c5b2d6ef439383d, composed with parent #1701cce1c8d3d9148de9703ca3c708b104279de67456by a signed additive merge. This includes the parent's asset selection and subscription confirmation changes while preserving the child's recovery admission and fee ownership checks. The integration was conflict-free; the five-file child patch is unchanged apart from its surrounding parent context, and no child tests were added, removed or consolidated.Required final
just format,just compile,just testandjust lintpass: 4,677 tests across 256 suites, zero failures/errors/skips and zero format/Detekt/Android Lint findings. Complete current logs, test streams and structured lint reports were audited; 1,757 tracked input hashes match before and after validation. Copied reports from earlier runs were identified and excluded from current evidence. Existing cursor/JVM warning qualifications are retained. Two independent affected-composition reviews found no verified issues in the new confirmation/funding-source paths or original-wallet subscription/hardware ownership boundaries. All 6 commits in the complete PR range againstcce1c8d3dverify locally and on GitHub (verified=true,reason=valid).Hosted CI, lint and E2E have completed on this head. The complete supplied logs/reports record 4,677 passing unit tests across 256 suites, zero Detekt/Android Lint findings, and 39 passing E2E cases with one unexecuted
settings_08case. All seven E2E shards ran their first suite attempt only, with no failed testcase or suite retry. Thenumberpad_4notification-permission helper logged a 30-second missing-target warning and continued; its passing reporter result does not prove permission was granted. npm reported 76 dependency vulnerabilities in the E2E suite (untriaged). Existing build/test warning qualifications and emulator/helper diagnostics are retained. Hosted Gradle problems HTML and E2E app logs/media were not uploaded, so those diagnostics remain unavailable. The release-only migration workflow skipped all six jobs and ran no tests.GitHub merge checkout
c64d63dc0194838bfaf432d38aa81977cae84ed9has the same tree as the locally validated published head. Each E2E shard checked out suite4c0cbef019220efed01ed3080c1f0ccc1fb627a3and used APK artifact12847418381; its upload/download digest matches GitHub metadata. The unit/lint report archives were downloaded and verified; APK binaries were not independently downloaded or hashed for this audit.The preceding hosted results below validate the
01beca96a0composition withde28c1035, before this parent update. Manual cases 1–5 retain their documented held-storage/device coverage gaps; no additional device campaign was launched. The two-human-approval requirement and existing named reviewer assignments remain unchanged.Previous parent composition — October 28
Previous head:
01beca96a0ae668bdccf6973134d655ea77c1fd1, composed with parent #1701de28c10350990b348e6b2d02f6ea6ed0061bb77eby a signed additive merge. The funding-source conflict keeps the parent's reviewed quote and fee refresh intact when conversion is unavailable, and uses the child refresh-cancellation helper when a source change proceeds or is refused. Original wallet ownership remains threaded through request finalization and subscription handoffs. The child diff remains five files; no new feature or device campaign was added.All 573 send-flow tests passed, including the inherited no-rate fee-refresh regression. Required final
just format,just compile,just testandjust lintpass: 4,675 tests across 256 suites, zero failures/errors/skips, zero format/Detekt/Android Lint findings. Complete logs, JUnit streams, SARIF/XML/HTML and embedded Gradle diagnostics were audited. The 1,755 tracked input hashes match before and after the gate. Final compile reused the production compilation executed by the focused test run; no fresh compilation is claimed for its UP-TO-DATE tasks. Existing diagnostic qualifications are retained in the audit, including four provider cursor notices. Two independent affected-composition source reviews are clean, covering fee/funding transitions and the original-wallet suspension boundaries. All five commits in the complete live-base PR range verify locally and on GitHub (verified=true,reason=valid); history and authors were preserved.Hosted validation on
01beca96a0is complete with the qualifications below. CI run 40748923415 passes 4,675 tests across 256 suites with no failures or skips, and Lint run 40748923612 reports zero Detekt/Android Lint findings. Published reports, full logs and annotations were audited; the synthetic checkout87cd3255bhas the exact head tree. The hosted Gradle problems HTML was not uploaded, so the retained generic deprecation warning does not establish that report's contents.E2E run 40748924070, using suite
4c0cbef019220efed01ed3080c1f0ccc1fb627a3and APK artifact12843103205, finishes with 39 unique passes and one explicitsettings_08skip across seven shards. Three tests failed in the first onchain/widget attempt and passed on its automatic selective retry:boost_2timed out after 30 seconds looking for negative amount text,widgets_1after 30 seconds waiting forWidgetSave, andwidgets_3after 8 seconds waiting forPriceWidget. Six retry skips match tests already passed in the first attempt. The original failures and failure annotation remain recorded; their causes are unestablished. Failed-attempt screenshots, videos and app logs were not uploaded after the retry passed. Two address-type helper retries also succeeded. Emulator and external-harness dependency warnings remain recorded, including the npm vulnerability summary; this is not a clean dependency-security result. Migration was skipped by its release-only condition.Earlier hosted and reviewer device evidence below applies to its recorded revisions. The author's held-storage capability gap and native startup smoke limitation remain; the parent's separately executed Manual 3 does not establish device coverage for this child. The two-human-approval requirement and existing named reviewer assignments are unchanged; no duplicate review request was sent.
Earlier evidence
Previous head:
60f141f7f044d93be921961f911b7cb8e10da2a6, based onbfa61d31b222bce0f3465b450af02bc076d28a6d. Required final compile, format, detekt and lint pass with no introduced findings; the full suite passes 4,794 tests across 259 suites with zero failures, errors or skips. Complete logs, JUnit streams, SARIF/XML and Gradle problems were audited against retained base evidence. All commits in the complete live-base PR range verify locally and on GitHub (verified=true,reason=valid).The original implementation passed 632 focused tests and two independent scoped source reviews. The hardware confirmation regression reproduces the premature fee-ready state on
1a733537b: confirmation navigates while its refresh recovery read is held. The fix marks loading before starting the coroutine; releasing the read publishes the completed fee and clears loading. On9f2d2b001, all 555 send-flow tests passed and the independent affected-delta review was clean. The later refusal finding was not covered by that clear-result regression.The refusal regression fails on
9f2d2b001at the stuck-loading assertion and passes on618b09869. That revision added three tests for refusal/retry and replacement ownership. The subsequent public fee-row retry regression fails on618b09869when recovery refuses after loading starts. This update expands those three tests to use Fee and speed and adds a queued-navigation regression for both fee-row and coin-selection continuation. Independent review found a navigation race in the local candidate; the regression reproduced it, the ownership check now runs at emission, and the affected recheck is clean. No native payment-dispatch gates changed.Hosted validation on
60f141f7fis complete: CI run 40678701173 passes 4,794 tests across 259 classes with no failures or skips, and Lint run 40678701174 reports zero detekt/Android lint findings. Published report contents, full logs and annotations were audited. The synthetic merge checkout429d3d24dhas the exact60f141f7ftree; these results predate the parent integration above.E2E run 40678701447, using suite
4c0cbef019220efed01ed3080c1f0ccc1fb627a3and APK artifact12811618556, finishes with 39 unique passes and one explicitsettings_08skip across seven shards. Two tests failed their first attempt and passed on automatic selective retry:send_2timed out waiting 30 seconds forQuickpayIntro-buttonafter receiving funds, before the subsequent send checks;backup_1timed out waiting 30 seconds forPriceWidgetwhile adding the widget, before backup/restore began. Eight retry skip entries represent tests already passed in attempt one. These are recovered timeouts with unestablished causes, not verified pre-existing or transient failures. Failure screenshots/videos and app logs were not published because artifact upload was skipped after the retries passed. One logged address-type helper retry succeeded; an optional permission lookup warning was caught. Emulator and external-harness dependency warnings remain recorded, including the harness npm vulnerability summary; this is not a clean dependency-security result. Migration was skipped by its release-only condition. The hardware fault-injection cases remain outside these E2E suites.jvsena42’s approval on
60f141f7freports passing device checks on60f141f7f, using a local dev build with Trezor Bridge enabled and a Trezor emulator. Changing hardware confirmation from Normal to Fast kept the displayed fee consistent with the device summary and broadcast; the loading state blocked an early swipe. The existing estimate-failure path re-estimated before signing, and software manual coin selection spent only the selected UTXO. This is reviewer-reported evidence, not a new author device run. The pending-restorerequiresRefreshrefusal path was explicitly not exercised because the marker fault is not provisioned. That fault-injection gap remains covered by source review and deterministic automated tests.The subsequent
ovi-reviewerreport found no issues in the60f141f7fdelta but returned not verified because device coverage remains incomplete. With a local recovery-marker test hook, it reports a pending initial refresh clearing the fee, an unreadable fee-row retry staying on confirmation, and a clear retry opening speed selection. It separately reports a passing build and focused suites on the unmodified head. Wallet replacement/cancellation during held admission, both refused outcomes during a held speed change, refusal after estimation, and replacement before navigation were not driven on device. These are attributed reviewer results; the instrumented device run does not establish complete fault-injection coverage.Historical hosted validation on
618b09869: CI run 40662546149 passes 4,793 tests, and Lint run 40662546131 has zero detekt/Android lint findings. E2E run 40662546566, using suite4c0cbef019220efed01ed3080c1f0ccc1fb627a3, passes 39 tests across seven first-attempt shards with one explicitsettings_08skip. No whole-test reruns occurred; three helper operations retried successfully, and an optional permission lookup warning was caught. Complete published logs/reports and annotations were audited; infrastructure and external-harness dependency warnings are retained, not treated as a clean dependency-security result. Migration was skipped by its release-only condition. That synthetic merge checkout has the exact618b09869tree; those results do not validate the new head.Historical hosted checks on
9f2d2b001: CI and lint audited; E2E run 40640411798 used suite4c0cbef019220efed01ed3080c1f0ccc1fb627a3, with 39 passed and one explicit settings skip across seven first-attempt shards. Migration was skipped. jvsena42’s review reports device timing without a measurable regression and a deferred link replaying once after unlock on that head. Those results predate the current refusal fix.The earlier head
1a733537bdev/regtest APK (cc6e76155a8f96e2ebab11333558ca9af96232b332531e17b5a8b15dcaeda366) launched on the existing disposableBitkit1701FinalRestore20261027emulator with data preserved. A 10,000-sat BIP21 opened Bitcoin Amount and Back returned to Home in the same process/activity; balance and broadcast count were unchanged. Confirmation was blocked by the native node remaining in startup. The emulator was shut down afterward.One unchanged provider test failed in the first full run; its focused four-test rerun and the full 4,789-test rerun passed without source edits. The original failure is retained; its cause is not established.
Gaps
Full-app held-storage fault injection is not provisioned in this author environment; the checklist above therefore remains explicit about author-device coverage. The external review on
2643b49reports passing the changed preliminary-refusal, retry and ownership boundaries and tracked-refresh refusal checks using private hooks, while earlier linked reviews supply revision-scoped coverage of unchanged paths. Complete hook/runtime evidence remains unpublished, and no independent author device campaign is claimed. The ordinary author confirmation smoke was blocked by native node startup; service listeners and forwarding were present, and its root cause was not established. Hosted E2E results do not substitute for those controlled hardware fault cases.Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh),gpt-6.1-sol(reasoning:high)