Skip to content

fix: preserve pending backups across restart - #1689

Open
ben-kaufman wants to merge 42 commits into
fix/restore-identity-backgroundfrom
fix/backup-observer-lifecycle
Open

ben-kaufman wants to merge 42 commits into
fix/restore-identity-backgroundfrom
fix/backup-observer-lifecycle

Conversation

@ben-kaufman

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

Copy link
Copy Markdown
Contributor

Current composed result — November 13, 2026

Signed 7fac7ab2 on #1686 d28a1578 retries an original wallet's pending backup-status write before retiring native ownership. An aborted wipe preserves that owner and its pending mark; a stopped wallet can retry after storage recovers. Persistent storage failure still refuses before local deletion. This follows 9330e24's original-owner/category/revision retry fix. All 42 live-base commits verify locally and on GitHub.

Required final format, compile, tests and lint passed on tree 5d699d57: 6,040 unit passes, two opt-in skips, and zero static findings. Two independent source reviews completed. The four actual-caller/drain/cancellation controls are component evidence; no authenticated native Reset Wallet or process-death recovery pass is claimed.

First hosted CI, Lint and E2E passed on synthetic 01867f6c with the same tree: 6,040 unit, 257 Compose and 39 E2E passes. All seven shards passed their first attempts; Settings08 and six migration jobs skipped. Complete diagnostics retain cursor and component storage warnings, RGS/fee fallbacks, and three metadata refusals followed by same-process acknowledgements. No crash marker was found; these results do not establish authenticated recovery or native wallet deletion.

Historical results remain distinct. 9330e24's hosted jobs were green, but two original job logs remain unavailable and its full diagnostic audit is incomplete; this run does not fill those gaps. At 43e314ca, the original hosted run passed 6,034 unit, 257 Compose and 39 E2E cases, with Settings08 and six migration skips. Its widget uninstall/reinstall recorded a Zygote mount abort before the replacement app passed; the cause remains unproven. Two metadata refusals had later same-process acknowledgements. Its fee disagreement led to force-close, not proof of cooperative close or returned funds; hosted unit XML/Gradle Problems were unavailable.

Earlier Send failures remain recorded: ac1549fb passed its built-in second attempt, and e0d25982 its third after two failures, four retry-filter skips and an unresolved early-dismissal cause. The be4e1ba8 staging-502 retry remains consumed. The separate explicit deletion-Retry concern remains open. Existing QA cases and their original evidence below are unchanged.

Previous validation and failures

On November 11, 534b3d86 composed 710292aa: final local format/compile/lint/APK, 5,386 full tests and 192 focused tests passed; all 31 range signatures verified. The three Spark status-persistence regressions were component controls. First hosted checks passed 5,386 JVM, 256 Compose and 39 E2E cases, with one disabled case, six skipped migration jobs, first-attempt shards and 28 stale-element helper messages. The 40-second native sync timeout recovered in-process. A Bitkit-targeted Zygote mount abort occurred during harness uninstall/reinstall before app initialization; the replacement app launched and the remaining cases passed. Neither warning is erased or treated as full recovery acceptance. CI · Lint · E2E

On November 9, f8b2ffc1 on 3e1fdb7d corrected both full-ACTIVITY DataStore orderings: a request during the payload read forces a PUT, and one arriving during status persistence schedules a follow-up for the same owner. Unknown PUT outcomes invalidate the digest; a new request has an in-memory interval before its revision persists. Both original red controls and their corrections remain recorded. Local 171 focused/5,045 full tests and final format/compile/lint passed; all 30 signatures verified. First hosted: 5,045 JVM, 256 Compose, 39 E2E passes, one disabled case, six migration skips, first-attempt shards on suite 265a0500. The passing LNURL case retained its channel-closing fee disagreement. CI · E2E · Lint

The earlier WALLET/USDT setup-timeout controls failed at 1cf5d39e with BackupUnavailable. At be4e1ba8, each waited through backoff/debounce and retried; WALLET wrote once and USDT retained the original immutable recovery snapshot. Those were mocked native-client controls. Final local tree 5c9e091f on b5d5858d passed 5,017 JVM tests with no skips, required format/compile/lint and 29 verified signatures; first hosted JVM/Compose counts were 5,017/256. Original E2E had 34 passes, five failures and one disabled case: four sends failed at Blocktank HTTP 502 and send-max used a fallback fee that changed the expected amount, recurring across 15 failed attempts. Fee/send source matched the parent. One explicitly authorized failed-job retry was consumed: 14 cases in three affected shards and their dependent gate passed on attempt one with the original APK. Its suite changed from 0e83ea6a to 265a0500, including a send-max wait, so it does not isolate recovery causality. Six migration jobs skipped. Complete original receipts remain retained. Authorized retry · CI · Lint

External recovery evidence — November 4

The new external review at 1cf5d39 adds recorded Manual 1–5 executions. An independent evidence check verified the exact five-file source delta, applicability of both disclosed control patches, and all ten new clips. The visible records corroborate queued Settings moving from Running to Latest Backup, held cleanup and replacement admission, retained settings revisions, stopped-observer retry, cancellation without callback acknowledgement, and the same recovery-envelope/plan digests receiving an acknowledgement on native refresh.

These remain attributed external executions with explicit limits. The original APK/install receipts, raw proxy journal and fork counters were not provided; the reported 0 → 0 → 1 submissions are not independently verified, and the signed-operation recording still shows PENDING. The cancelled coordinator-retirement case does not execute its subsequent repository-reset lambda; the separate JPY case directly exercises BackupRepo.reset(). No owner runtime pass, literal current-head wallet wipe/restore, process-death replay or chain finality is added. Terminal complete-WALLET availability and WALLET/USDT setup-timeout bookkeeping remain open. The existing artifact request already covers the missing receipts, so no duplicate request was sent. Local evidence: work/validation/android1689-review-5774734216-20261104/REPORT.md and owner-integrity.json (66 verified members).

Admitted-backup status correction — November 4

Signed additive head 1cf5d39e90f7e5c7e3ac4c6501ffb725523bfa2d incorporates live parent #1686 at a2c58f2427635719b63263b9441b8d22a7b6c858. A reserved replacement backup now preserves Running only after its original wallet admits it. Retirement before dispatch therefore cannot strand Running after the previous job drains. The child retains its lifecycle lock, original BackupOwner, exact-job cleanup and coalesced-request fallback. One regression exercises both retirement orderings; the earlier liveness-only correction fails the retained negative control.

Final just format, just compile, full just test and just lint passed on 1,802 identical inputs: 4,938 unit tests across 264 suites, zero failures/errors/skips, and eight fresh static reports with zero findings. The compile task reused matching outputs; it is not described as a second compilation. Complete logs, per-case XML/HTML, SARIF and nested Gradle reports were inspected. Four existing cursor-close chains, three JVM notices and historical Gradle deprecation reports match verified parent evidence. Two affected concurrency reviews are clean. The owner verified all 1,318 sealed audit files; both private untracked probes remain unchanged and outside the commits.

All 27 commits against the live parent verify locally and on GitHub (verified=true, reason=valid); published history and draft=false are preserved. Current-head hosted CI, Lint and E2E passed: 4,938 units, 256 Compose tests, zero static findings and 39 unique E2E passes, with one explicitly disabled Settings case. E2E send_1 first failed waiting for the expired-invoice toast, then passed its built-in second attempt; send_2/3 retained their first-attempt passes and were excluded from retry. The original input contained the previous address concatenated with the invoice despite clear/set calls; the precise input mechanism remains unresolved. The original failure and continue-on-error outcome are retained, so this is not a first-attempt-clean result. Six migration jobs were skipped. Complete raw/nested diagnostics, all 12 digest/CRC-verified archives and 1,063 sealed evidence files were inspected and verified; synthetic checkout matches this exact head tree. Existing warnings and an unmatched cancellation observation remain documented. Evidence: work/validation/hosted-android1689-1cf5-20261104/report.md. Parent J3 Settings evidence remains parent evidence, not a new child-device run. Existing authenticated recovery results and their revision limits remain below. Terminal complete-WALLET availability, running-LDK quiescence, and the previously reported WALLET/USDT_RECOVERY setup-timeout bookkeeping caveat are not resolved by this status correction.

Evidence: work/validation/android1689-admitted-parent-final-audit-20261104/report.md and work/validation/android1689-admitted-parent-20261104/publication.json.

Original-wallet backup ownership — November 4

Signed additive merge 1a6048a41a44e2e6fc34dda20a13ff073a112925 incorporates live parent #1686 at 4056d2c0036e5a4e51698e26b3d1615825dd77f5. Scheduled payload capture, uploads, retry timers and cache cleanup retain their original VSS owner. A retired request cannot act on a replacement wallet or hide its newer request. The observer controller can rearm after identity replacement, while admitted work and consumed cache writes drain before the old standalone clients retire.

The composition preserves independent listeners when one startup cache update fails. It also preserves an explicit request coalesced into an observer job that is canceled before dispatch: the completion fallback transfers that pending request only for the same live wallet, without duplicating an entered job's normal cleanup. The corrected deterministic control passes with this fallback and fails when only its registration is disabled. Earlier dispatcher-harness failures are retained and do not count as valid product negative controls.

Final just format, just compile, full just test and just lint pass on all 1,801 unchanged inputs: 4,935 unit passes across 264 suites, zero failures/errors/skips, and all eight fresh format/lint reports show zero findings. Complete logs, per-case XML/HTML and Gradle diagnostics were inspected; four existing cursor-close chains and three JVM notices match actual-base evidence. Earlier failed attempts and formatter corrections are retained. Two independent composition/security reviews plus the affected cancellation recheck are clean. The owner verified all 1,315 sealed evidence files. All 25 commits against the live parent verify locally and on GitHub (verified=true, reason=valid). Evidence: work/validation/android1689-vss-parent-final-audit-20261104/report.md. Exact published head 1a6048a41a44e2e6fc34dda20a13ff073a112925 (base 4056d2c0036e5a4e51698e26b3d1615825dd77f5, tree e0773ea784bf00b614ac419b699a394123c43416): hosted CI, lint and E2E passed: 4,935 unit tests, 256 Compose tests, 39 E2E passes plus 1 source-declared skip. Complete lint/Detekt reports contain 0 findings. Multi-address E2E's configured first 2 in-job attempts failed SDK system-image ZIP setup before tests; the third passed all 3 cases, with both earlier errors retained. Migration's 6 jobs were skipped. Complete raw and nested diagnostics were audited; recurring cursor, fixture, network and tooling warnings remain disclosed. Four metadata guard refusals each have later same-process metadata acknowledgement. This does not establish complete WALLET recovery, native quiescence, migration execution or dependency-security cleanliness. The owner verified all 937 sealed evidence files. Hosted audit: work/validation/hosted-android1689-1a604-20261104/report.md.

Existing authenticated recovery evidence below retains its tested revisions. These native controls do not establish a new full-app identity-replacement journey, literal wallet wipe, running-LDK persistence quiescence or terminal complete-WALLET recovery. Migration remains disabled; no SDK, server or capability activation is added.

Authentication transport retry — November 3

Signed commit e7fb0ac37beae6c3f8e9a58f0f5691ece312f03c adds the released VSS authentication request-send error to the existing retry classification. It accepts only typed AuthException carrying the exact HTTP(S) send-failure form (raw or the single native external-request wrapper). Authorization denial, invalid JWT, response/decode failures, cancellation, unrelated errors and malformed addresses remain permanent. No authentication or restore gate changes. Typed unknown errors still require the /putObjects path.

Final composed inputs: just compile, just test (4,916 tests / 263 suites, zero failures or skips), just lint (six fresh structured reports, zero findings), unchanged final formatting, and two independent affected source/security reviews pass. The introduced formatter findings were corrected before the final run. Existing CloseGuard/JVM/Gradle notices were checked against the base and retained. All 1,799 validation inputs match. The complete 24-commit live-base range is locally and GitHub verified; base is 43e0f3fe04705d397a81f493cf8e57239a0be73e. New-head CI, lint, and E2E passed on synthetic 54d0d47c, whose tree exactly equals this head: 4,916 units, 256 Compose cases, six static reports with zero findings, and 39 E2E cases. One Settings case and six migration jobs were explicitly skipped. All seven E2E shards passed on their first workflow attempt; one Send action recovery remains disclosed. Complete logs, structured reports, 326,047 nested log lines and ten checksum/CRC-verified diagnostic archives were inspected; 836 sealed payloads were verified. Both APK digests are bound to producer/API records; all seven E2E consumers verified the E2E APK digest. APK archive bytes were not separately downloaded. Hosted report SHA256 b6649d27a920f0b34d2763a4a070e7092cbeb2d15f62d9a58fc4770c453fbe93.

Manual 8 transport portion: the ordinary fixture APK SHA256 090cbb7262a7d1036aa9e070df52e709afa3abc6501474d4cc84172c11a2ddbc binds this exact two-file change. On unfunded Android emulator5554/user12, after cold launch with node reads unavailable, one UI Retry encountered the actual native auth request-send failure. It scheduled ten seconds of backoff. Reopening only auth routes led to a new capture 15.024 seconds later in the same process and one acknowledged write, without a second tap or unrelated edit. The existing BLOCKTANK ciphertext changed; its legacy stored version remained1 and the other15 cold rows stayed identical. There were no unknown writes. Original APK, nine routes and selected old-user state were restored. Warm LDK writes and three changed fixture-user cache/database files are disclosed. This is not a new typed readback or wallet restore; Case7B retains its separate native semantic evidence.

Evidence: work/validation/android1689-auth-transport-final-audit-20261103/report.json (SHA256 fb02b1da4579aa388e601a9e927b8e3b856c3bb99dde51e864c2ef45e01de22e), its input-disposition addendum, and runtime audit work/validation/android1689-auth-transport-runtime-audit-20261103/report.json (SHA256 9b679e29554f306ef93fd205e39521d1033f1987f6ebc0bbc1808171da2a2419). Manual 8 denial portion also passed on the actual app: one explicit Retry received ten genuinely authenticated fixture refusals within the native client’s existing retry policy, but only one app capture. After permission was restored, a 25.240-second observation recorded no new auth request, capture, PUT or acknowledged revision. All sixteen cold rows stayed identical. The original APK, routes and selected old-user state were restored; two fixture-user cache/database changes are disclosed. The first attempt wrongly assumed one native callback and stopped at that assertion; it remains failed fixture evidence, not a product failure or a pass. The corrected cold-only run reused the preserved pending state without resetting the wallet. This bounded observation does not claim indefinite liveness. Denial audit SHA256 a46e2f52abca285697de6f3133f85b5ee53fb6cc7a6df4c21093ee091ae98cf8. The fixed wire error text cannot distinguish a server emitting the identical typed error; it grants no authentication or write authority and uses the existing capped-delay retry. The paired iOS1123 classifier repair is published at 11fb65d7; its scheduled-retry native evidence remains separate from this Android app execution and from iOS direct-UI Retry behavior.

Cold-start ordinary Retry — November 3

Signed 8727da02549c3a885fbb48b500747b3401d3d649 fixes the reported cold-start Retry hang. Ordinary categories now initialize their VSS client before upload even when the Lightning node has never started. A setup-owned timeout retains bounded retry; caller/reset cancellation still drains and propagates. Newly reachable cold captures read persisted Blocktank data without network refresh and refuse failed activity/receipt reads, rather than publishing incomplete snapshots.

Final just format, just compile, just test and just lint pass on 1,797 matching inputs: 4,913 unit tests, 263 suites, zero failures/errors/skips and zero static findings. Complete structured diagnostics and both independent affected source reviews are sealed. Earlier controlled failures and baseline-attributed warnings remain retained. The full 23-commit live-parent range is verified locally and by GitHub (verified=true, reason=valid for every commit).

Manual 6 passed in the actual app, using released Core RC11/Paykit RC2/VSS RC1/LDK rc78, a fresh unfunded Android user and verified ARM64 APK f762d47c…beb0733. A real display-preference edit remained pending through process death. With auth unavailable and node startup failing, explicit Retry initialized VSS and returned an authentication failure after 10.276 seconds instead of hanging. Restoring authentication only, while node reads remained rejected, let the next explicit Retry receive a real VSS PUT acknowledgement and synchronize SETTINGS. Another cold restart retained the same settings bytes and synchronized revision. No node-start event occurred after the first cold launch. The original APK, routes and older users were restored and checked. This is a SETTINGS case with no prior uploaded SETTINGS object. The observed failure occurred at authentication for PUT after setup succeeded; setup-failure handling and read-refusal behavior have separate component coverage. It does not establish prior-object preservation, complete-WALLET availability, remote restoration, payments or the unverified portions of Manual 1–5.

Manual 7A — prior ACTIVITY object and local read refusal passed in the actual app. On the same verified published inputs and unfunded user, one explicit Transaction Log Retry ran before successful node initialization. A temporary, reversible closed_channels table-name fault caused the actual Core SQLite read to fail before VSS setup. The required revision advanced from 1793717566863 to 1793739130414; synced stayed 1793717561910, and the operation returned idle. There was no VSS PUT. The independently authenticated existing ACTIVITY row retained version 1, all 192 ciphertext bytes (SHA256 05f378372069fa546047cf3db96efedff6f88f83615cab4c6ab92801830001ef) and its store/key binding; all 15 scoped remote rows matched before, after and at restoration. Actual node reads were refused and no successful node setup/start/observer was observed in that cold process. The original table name, row fingerprints, column/index/foreign-key shape, APK, nine routes and older-user state were restored. SQLite file bytes and raw schema text changed during the rename round trip, so byte-identical database restoration is not claimed. Two earlier pre-fault stops remain recorded. The prior decrypted payload contains no activity records (124 bytes): Manual 7B now has separate nonempty BLOCKTANK evidence below. This result is not a terminal node-failure, lifetime-never-started, BLOCKTANK, wallet-restoration or payment claim. Complete private evidence was audited and all 22 sealed members verified: work/validation/android1689-prior-read-failure-runtime-c-20261103/report.json.

Manual 7B — nonempty persisted BLOCKTANK capture, cold Retry and native readback passed at 8727da02. The normal app APK 3da259fa…891ad retains all 1,797 tracked product inputs and released pins, adding only the existing debug localhost Blocktank provider endpoint. A genuine Fee Settings fetch changed the public cached fee info; refused app uploads left a natural pending revision. After process death, a distinct cold process initialized Core while node reads remained refused and no node/backup-observer start was observed. One Connection Receipts Retry captured the retained cache and received one authenticated native PUT acknowledgement, advancing synced to required 1793742411351. Exactly the previously absent BLOCKTANK row was inserted at version 1; every other cold-stage row stayed identical. A separate write-refusing native probe then authenticated as that same original owner, decrypted the exact acknowledged row and verified a strict V1 envelope with non-null typed info equal to the retained SQLite cache (1,200 plaintext bytes; orders/CJIT empty). All 16 remote rows stayed unchanged during readback. Both original APKs, nine routes, test registration and selected older-user hashes were restored. The warm fixture legitimately made 24 LDK writes changing five LDK rows and changed the test user's cache/activity/Blocktank database bytes; whole-wallet byte identity is not claimed. This is a first BLOCKTANK insertion, distinct from 7A's existing ACTIVITY preservation, and does not prove remote wallet restoration or payment recovery. Complete traces and readback were audited; the owner verified all 11 sealed report payloads and 41 external inputs. Evidence: work/validation/android1689-case-b-readback-c-audit-20261103/combined-case-b.json.

Authentication transport follow-up: review4457469574 is addressed by the signed fix and evidence above. Manual 6's second explicit Retry and Manual 7B's successful transport remain separate historical results; neither was retroactively counted as autonomous outage recovery.

New-head hosted native CI is complete on 8727da02 and the identical synthetic tree: CI passed 4,913 unit tests and 256 Compose tests; lint has zero findings across all six structured reports; E2E passed 39 cases on their first internal attempts, with one explicitly disabled settings_08 skip. Six migration jobs were skipped, not passed. Complete logs and all ten report archives were inspected and verified; cursor, native callback-fixture initialization, wallet/network and backup-guard warnings remain documented, without claiming harmlessness. The external review check is separate from native CI and human approval. Runtime evidence: work/validation/android1689-cold-start-runtime-20261103; full native diagnostics: work/validation/android1689-cold-start-diagnostics-20261103.

The current-head Piotr review verifies the cold-start source repair and retains the terminal-WALLET concern. Its suggested local-read coverage now has the separate Manual 7A preservation and Manual 7B nonempty-cache results above, with their exact limits; the terminal-WALLET concern remains open. Historical HTTP 405 causation remains unverified. No repeated review request was sent for this just-completed review.

Prior validated parent composition — November 3

Signed additive merge add2c04b684609b6ad74a30835864d816f0a79f6 incorporates dependency #1686 at 43e0f3fe04705d397a81f493cf8e57239a0be73e, including current master 44bf50ec. The backup-observer patch retains the same added and removed lines after context shifts; the automatic merge preserves writer draining, observation ownership and the new restore-category result contract. Published history is preserved.

Final just format, just compile, just test and just lint passed on this actual composed tree: 4,905 unit tests across 263 suites, zero failures/errors/skips, and zero Detekt/Android Lint findings. All four phases share the same 1,797 frozen inputs. Complete logs, XML/HTML/SARIF, fresh structured static reports and ignoreFailures outputs were inspected. Four cursor-close diagnostics remain recorded and trace to emitter sources identical to the actual parent; this is not a claim of runtime harmlessness. The independent affected-composition review is clean. All 22 live-parent range commits are locally and GitHub signature-verified.

At add2c04b, hosted native checks completed and were fully audited: 4,905 unit passes, 256 Compose passes, zero static findings and 39 unique eventual E2E passes, plus one disabled Settings case and six skipped migration jobs. The original Lightning-security attempt failed while downloading its system image, before tests. The single human-authorized retry (job 125932596605) ran both previously blocked cases successfully on its first in-job attempt, and the aggregate passed. The other 37 E2E passes and existing unit/Compose/static results retain their original execution credit; remapped job IDs are not new runs. All report digests/member CRCs and complete nested diagnostics are inspected, retaining the initial infrastructure failure and runtime warnings. Retry supplement: work/validation/android1688-1689-authorized-infra-retries-20261103/diagnostic-audit/STRUCTURED-REPORT.json (133 new sealed payloads); original full audit and failures remain preserved. Earlier hosted and authored/external runtime results below retain their original revision, artifact and fixture limits; this composition adds no new device or authenticated-journey claim. Evidence: work/validation/android1688-1689-master44bf-final-audit-20261103/STRUCTURED-REPORT.json and work/validation/android1689-master44bf-composition-20261103.

Retained scheduler acceptance evidence — November 2

Current review 5731374382 retains the earlier installed scheduler cases at 9a837a6. These are historical external executions, so the earlier blanket description that Manual 1–5 were unrun was too broad. The author has inspected the disclosed controls and inline log excerpts, and compared relevant source: 22/24 selected lifecycle/retry function slices and the VSS/cache/USDT files and dependency pins match 439b5d0. Shared WALLET/METADATA serialization, allowance observation and restore/capture changes retain their separate later evidence.

The original build/install binding and Manual 1–5 event/readback traces have not been independently verified; the checkboxes below remain open for those exact gaps. Manual 2's reported empty-wallet envelope does not alone establish the same-signed-operation reset variant. These results are not current-head author executions, complete WALLET capture or permission to use the reviewer's fixtures. Evidence: work/validation/android1689-retained-acceptance-reconciliation-20261102/REPORT.md.

Current-head hosted results — November 2

All hosted results for 439b5d0eea89a72ee6cdf1167515b95339a65c58 are inspected: 4,857 unit passes, six static reports with zero findings, and 39 distinct eventual E2E passes. One onchain_2 execution timed out waiting for Activity-3; its automatic second attempt passed. The failure image contains all three transactions across two date groups. Exact source counts date headers in the activity index, while the suite expects contiguous transaction indices; this supports a selector mismatch, but no captured semantics tree establishes the concrete runtime IDs. No wallet-data-loss or causal repair claim follows.

The synthetic tree exactly matches the published tree and suite 611509ee. One disabled Settings case and eight cached-pass skip records are distinguished; six migration jobs did not execute. Complete logs, all nested streams, nine checksum/CRC-verified archives and failure media were inspected; the owner verified 885 sealed payloads. Four CloseGuard observations and runtime diagnostics remain recorded. No manual CI rerun, new authenticated backup/restore pass or installed-binary claim is implied. Evidence: work/validation/hosted-android1689-439b-20261102-current-a/REPORT.md.

Final identity-recovery parent composition — November 1

Signed additive head 439b5d0eea89a72ee6cdf1167515b95339a65c58 incorporates #1686/b6751e22956653c4f2a80112143880b47298e30d at tree 820b8c27563810cebc079345e05ee6b680a5ea09. The parent's allowance off/on admission repair and failure-only routing diagnostics are included; this PR's backup-observer patch remains byte-for-byte intact. The affected composition review is clean and retains the two independent parent concurrency reviews.

Final required compile, 4,857 tests / 260 suites, formatting and lint passed with zero failures/errors/skips/findings. All 1,781 frozen files match. Complete logs, XML/HTML/SARIF and fresh changed-unit lint analysis were inspected. Four aggregate lint reports were UP-TO-DATE with matching sealed hashes; they are not described as regenerated. Four CloseGuard events remain retained, including three present only in the full executor stderr. All 21 live-parent range commits verify locally and on GitHub. Current-head hosted results are recorded above; older failures and the unresolved parent routing-test cause remain preserved. No new authenticated backup/restore journey or installed-binary equivalence is inferred. Evidence: work/validation/android1689-parent-b675-20261101/final-diagnostic-audit/STRUCTURED-INSPECTION.json.

Hosted validation completed — November 1

At 2eb0c492439ebad8943457622ba67437d828d1f2, all seven E2E shards and the status job have completed: 39 first-attempt passes, with one disabled settings case. The earlier audited 4,856 unit tests and six zero-finding static reports remain tied to this exact head and parent 1b1426fb.

The sole audit combined prior immutable evidence with only the missing complete shard logs and checksum/CRC-verified report archives. The runtime metadata-backup refusal is retained: its exclusion guard matches the actual base, while worker/handler context differs, so no blanket pre-existing-runtime or eventual-backup-completion claim is made. Other diagnostics, npm advisories and skipped migration jobs remain explicit. This is not new authenticated recovery acceptance. Evidence: work/validation/hosted-android1689-2eb0-20261101-tail-b/REPORT.md.

Current parent composition — November 1, 21:48 UTC

Signed additive merge 2eb0c492439ebad8943457622ba67437d828d1f2 incorporates live #1686 at 1b1426fbebeccd616c29311fa5bf652089727a84; final tree a5c0dd6e741a81d34a1736fce15c4f00f95e52f8. The owned child patch is unchanged. The incoming queued allowance-admission and pending-profile-setup corrections have one clean affected composition review, reusing the matching independent parent concurrency reviews.

Required just format, just compile, just test and just lint passed 4,856 tests /260 suites, zero failures/errors/skips, and zero findings across all six Detekt/Android Lint reports. All 1,782 tracked inputs match the composed tree. Complete logs and structured reports were inspected; four CloseGuard allocation observations and JVM notices remain recorded, and stale Gradle report copies are excluded. All 19 live-base commits verify locally and on GitHub (true/valid).

Current-head hosted CI awaits complete inspection. No new authenticated app journey or causal repair of prior E2E failures is claimed. The terminal complete-WALLET limitation and separately scoped profile-removal gaps remain unchanged. Earlier acceptance and failures below retain their exact tested revisions. Evidence: work/validation/android1689-parent1b14-review-20261101/ and work/validation/android1689-parent1b14-final-a/.

Earlier parent composition — November 1

Signed additive merge a3693b0c7061642a0b4305c07841a438a79046b9 incorporates live #1686/78269d8a1cae1ac127be93a96fba16f7d68d1019; tree c5943235d319bbf7171ebccca58fd10c0327b188. The backup-observer delta remains unchanged. The composition includes queued-offer admission and stable activation during same-identity contact refresh, with one clean affected review and independent parent concurrency reviews retained.

Required final formatting, compile, full tests and lint passed 4,849 tests /260 suites, zero failures/errors/skips. All 1,781 frozen inputs match; complete logs and fresh Detekt/Android Lint reports contain zero findings. Three JVM notices and four verified unchanged-provider cursor warnings are retained; stale copied reports are excluded. All 18 live-base commits verify locally and on GitHub. New-head hosted CI is pending inspection. No new authenticated app runtime is claimed from these gates.

The earlier 9cfe0527 external evidence and hosted recovery remain revision-specific. The first multi-address failure, internal retry and media-clock provenance limits are preserved; this composition is not their causal repair. The terminal-identity complete-WALLET availability finding stays open, with no partial/stale backup fallback. Evidence: work/validation/android1689-parent7826-final-a/inspection/ and work/validation/android1689-parent7826-20261101/publication/.

Historical-head external evidence — October 30, 22:21 UTC

Automated review 5719299576 at 9cfe0527 reports automatic METADATA recovery after durable WALLET exclusion and writer/reset draining with a disposable staging-VSS wallet. Its cold-start outage repeat completed after ordinary sign-out; later signed-in synchronization remains unverified after native Pubky lock errors. It also reproduces the existing terminal-identity complete-WALLET availability failure, which remains open. The owner verified the revision and read the report/control patch; raw reviewer logs, DataStore, binaries and media were not independently audited. These remain attributed external evidence, with no additional author acceptance or human approval inferred.

Historical parent composition — 30 October 2026

Signed additive head 9cfe052761ebdf51a017cebc3b7a9ab173d953ef incorporates the validated profile-session recovery branch at ff5aa32f4da1ec33fb8be63e0b84597d463c82b1, including the current #1686 parent 78a0ff7a. The GitHub base remains fix/restore-identity-background; the existing stack and published history are preserved.

WALLET and METADATA capture/upload share the wallet serialization boundary while retaining generation-owned observation, cancellation drains and restored public-address exclusions. A new observer regression proves that a WALLET acknowledgement automatically resumes previously blocked metadata exactly once, without a manual metadata retry. Omitting that rearm fails the direct expected-one/actual-zero assertion; restored final source passes.

Final just compile, just test, just lint and unchanged just format pass on tree e27b3e6112da04eda455031c70de9fe5709cf9f6: 4,839 fresh tests / 260 suites, zero failures/errors/skips, zero Detekt/Android Lint findings, all 1,778 frozen inputs matched. Complete logs and structured reports were inspected. Three JVM notices, four unchanged-provider CloseGuard chains and historical copied Gradle reports remain recorded; the compile reused matching outputs and is not a clean-rebuild claim. Failed helper, negative-control and intermediate formatting results are retained.

Two independent affected source reviews are clean after closing the automatic-rearm coverage finding. All 15 commits against the live base verify locally and on GitHub (verified=true, reason=valid). Current-head hosted CI passed 4,839 tests / 260 suites, zero failures/skips; lint has zero findings across all six uploaded reports. Complete logs, all 262 unit HTML pages and structured lint reports were inspected. E2E run 41104465012 reached 39 unique eventual passes and one unrun Settings case. Its original onboarding/backup/numberpad job failed Docker port binding before tests. One bounded job-only recovery used the original APK and exact suite 611509ee, then passed all seven selected cases on its first test step. Separately, the multi-address case failed its first Sent activity assertion and passed the workflow's internal retry; its failure, screenshot and all five encoded video frames remain preserved. This is not a first-attempt clean E2E result. The failed clip/runtime carry October 21 timestamps within the October 30 run; clock/history provenance and the assertion's cause remain unresolved.

All executed job logs, nine checksum/CRC-verified small archives, their 307 members and complete runtime diagnostics were inspected; current CI merge tree equals this published head. A 60-second RN backup-list timeout and other retained runtime errors are not waived by test success. The hosted recovery downloader verified the original APK digest; independent local APK/native byte inspection remains unperformed after a transfer timeout and low-memory guard. No new full-app fault-journey acceptance is claimed. Earlier hosted passes below apply only to their recorded heads. These component controls do not complete manual cases 1–5 or the authenticated fault journeys. The existing terminal complete-WALLET availability concern remains open.

Historical parent composition — October 29

Signed additive merge 1d3ba149eef529f598ed9947d145bc9aa2bc9536 incorporates current #1686 parent 217b42ef889952a6c1f3a30b0c44a8b1001db4a5; final tree 753c9b8b2edc93bd2f2ba2ee5b277cf4cac38c06. Observer drains, explicit retry while stopped, original ownership, cancellation and complete-backup gates remain intact. The source review caught a test-helper argument conflict; the named-argument correction preserves all assertions and both parent/child test sets.

Final required compile, full tests, lint and format passed. 4,742 tests across 256 fresh suites passed with zero failures/errors/skips; all 1,760 regular inputs and six symlinks match. Complete logs, XML/SARIF and ignoreFailures configuration were inspected: zero compiler/lint/format findings. Four cursor-close warnings in test output and three JVM notices remain disclosed with actual-parent source comparisons and no runtime-causality claim. Full 113-case BackupRepo coverage passes. Existing authenticated restoration/outage and terminal complete-WALLET acceptance gaps remain distinct.

The complete nine-commit live-base range is locally and GitHub verified (verified=true, reason=valid). Ready status is retained (draft=false). Current-head hosted CI passed 4,742 tests in 256 suites, and lint reports zero Android Lint/Detekt findings. E2E reached 39 unique eventual passes and one true settings skip. Settings attempt 1 timed out after 30 seconds waiting for ElectrumErrorToast; its automatic second attempt passed. The original exit 1/failure annotation remain retained; the cause is unproven, and failure-only media was not uploaded after eventual success. This is not a first-attempt clean pass or the authored fault journey. All 14 complete executed logs, 18 annotations, complete uploaded reports and four SHA-256/CRC-verified archives were inspected; the synthetic merge tree equals this head and its actual parent. Four unchanged-provider cursor stacks, three JVM notices and absent uploaded Gradle problem-detail reports remain disclosed. Migration was skipped, not passed; no rerun was requested. Historical execution evidence below remains revision-scoped.

Historical scheduler repair — 29 October 2026

Signed head 9a837a6e7b9286d4dca24d57c451929131285b27 includes the additive parent merge e80ca3f95 from #1686 at 9ce430274. A permanent failure suppresses only the revision that failed: a newer committed recovery revision remains eligible for observation after its caller cancels. Background coalescing now waits before taking the category mutex, so an explicit Core callback does not wait behind idle debounce. It still waits for an active upload and must obtain acknowledgement from its own immutable snapshot write.

Both controlled regressions failed before the repair and pass afterward. Final just format, just compile, just test and just lint passed on the combined tree: 4,714 fresh unit tests / 256 suites, including all 112 BackupRepoTest cases, with zero failures/errors/skips and zero Detekt/Android Lint findings. All 1,760 frozen inputs match. The complete 9,692-line test log, all XML/HTML/SARIF and embedded Gradle diagnostics were inspected; copied older reports were separated from fresh results. Three JVM notices, four verified pre-existing provider cursor warnings and the unchanged Detekt plugin deprecation remain disclosed. The canonical compile used matching up-to-date outputs; this is not a clean-rebuild claim.

Two independent scoped correctness/concurrency reviews are clean. All eight live-base commits verify locally and on GitHub (verified=true, reason=valid); published history is preserved. Current-head hosted CI passed 4,714 tests in 256 suites, and lint reports zero findings. E2E completed 39 distinct passes and one explicit settings skip: 38 cases passed first attempt; Lightning first failed with an invoice expired on October 22, then passed automatic attempt 2. Security's second-attempt marker skip follows its actual first-attempt pass. All 11 complete E2E logs (7,041 lines), every attempt and annotation were inspected; the earlier failure and unproved cause remain retained. The synthetic checkout tree equals this head and tests the current parent. CI/lint report archives were SHA-256/CRC verified; all seven emulator consumers checked the published APK archive digest, without a local installed-binary claim. Earlier failure media was not uploaded after eventual success. Migration was skipped, not passed. No workflow rerun was requested. Earlier hosted results below remain revision-scoped. These service-callback tests use mocked VSS and do not establish the authored full-app fault cases, process-death payment submission or an authenticated native journey. The current iOS twin already keeps idle debounce outside its USDT writer; no mechanical opposite-platform patch is indicated by the narrow source comparison.

Prior parent composition — 28 October 2026

Signed additive merge 5fca5cd39df4fcf072eacb36bd63c758a55a0f5d incorporates parent #1686 at 89e2702e, preserving the existing stack. The final tree 2edb93a2 passed 4,638 unit tests in 254 suites, zero failures/errors/skips, plus required just format, just compile and just lint. Complete logs, all XML/HTML/SARIF and embedded Gradle reports were inspected; Detekt and Android Lint have zero findings. All 1,753 frozen file inputs and the directory link match. Three JVM notices, four unchanged-provider CloseGuard stacks, the inherited manifest notice and Detekt deprecation remain disclosed against exact parent source/configuration; no fresh parent runtime baseline or warning-free claim is made.

All 605 production feature patch operations are unchanged. The sole conflict removes two obsolete refund-pointer tests with the parent's Blocktank sunset; no retry, snapshot, reset or revision assertion was dropped. One independent affected-composition review is clean, retaining prior matching lifecycle/explicit-retry review coverage. These full unit gates do not establish the authored authenticated lifecycle/endpoint fault journeys or resolve terminal full-WALLET availability.

All 6 live-base commits verify locally and on GitHub (verified=true, reason=valid), with exact sets matching. This PR remains ready. Current-head hosted CI passed 4,638 unit tests in 254 classes with zero failures/skips, and lint has zero Detekt/Android Lint findings. E2E reached 39 distinct passes and one explicit settings skip: boost_2, widgets_1 and widgets_3 first failed their expected-text, WidgetSave and PriceWidget assertions, then passed automatic attempt 2. Earlier failures and process annotations are retained; their causes remain unproven. This is not an all-first-attempt pass or the authored authenticated fault journey. The complete 14 executed job logs, all unit/lint reports and four SHA-256/CRC-verified archives were inspected; the executed merge tree equals this head and its tested parent matches the live stack. Failure-only media was not uploaded after eventual success. No workflow rerun was requested. Migration was skipped, not passed.

Twin: synonymdev/bitkit-ios#1123
Dependency: #1686

This PR drains interrupted backup work before resetting its clients and preserves pending backups across observation restart.

Description

  • Retry temporary failures for explicit backup requests even when Lightning observation is stopped, with capped backoff and retained pending revisions. The narrow released-native request-send compatibility classifier remains backup-only; unrelated unknown errors stay non-retryable.
  • Retain setup, observer, retry and upload tasks so reset cannot leave old setup or status writes running against the next wallet state.
  • Serialize category ownership and follow up newer required revisions, including explicit full backups and requests made during canceled-job cleanup. Suppress only the exact failed revision, and keep idle debounce outside explicit recovery ownership.
  • Persist already-observed data changes before canceled cleanup finishes, including a reset that leaves wallet data intact. Replacement listeners collect changes during the drain; retained automatic retries wait until that drain completes.
  • Keep manual, migration and USDT recovery backups independent of Lightning observation. Persist supplied USDT snapshot revisions before waiting for an older upload; older completion cannot satisfy a later canceled request or bypass its backoff. Wipe rejects new work and canceled persistence cannot acknowledge Core.

Out of Scope

Design

N/A — no UI changes.

Preview

N/A — background lifecycle repair.

QA Notes

Setup

Use the committed dev Debug dependencies: Core 0.8.0-rc11, Paykit 0.1.1-rc6, VSS 0.5.26-rc2 and LDK 0.7.0-rc.79. Automated tests use synthetic data, a real local DataStore, the SDK operation barrier and fake VSS transport; no funds or device wallets are changed. Manual cases require a dedicated disposable fault fixture with held setup, capture, upload and cache-write boundaries as described in journeys/backup-restore/wallet-snapshot-consistency.md. Those app-level controls are not provisioned in the standard journey environment. Do not use live funds, reset personal wallets or reuse another worker's device.

For Manual 6, use an isolated dev/E2E regtest build with a disposable local LNURL-auth/VSS backend. Set E2E=true, E2E_BACKEND=local, E2E_VSS_SERVER_URL=http://localhost:18717/vss and E2E_LNURL_AUTH_SERVER_URL=http://localhost:18715/auth; reverse only those device ports to a dedicated fault proxy forwarding to the fixture. Keep authorization headers and native encryption intact. First reject auth GET /auth before forwarding and node POST /vss/getObject / listKeyVersions; then allow auth and putObjects while continuing to reject node reads. Use Settings → Default Unit for the edit and Settings → Security → Data Backups for Retry. Retain proxy outcomes, app lifecycle/backup logs, installed APK hash and nonsecret backup-status readbacks. No wallet reset, seed capture or funds are needed. Restore prior device routes and APK after testing.

  • 9. In an isolated unfunded Spark/VSS fixture, retain pending readiness or exit state during restore. Clear the restore gate, let its real timeout expire, or interrupt the debounce with a second restore; after release, verify exactly one acknowledged upload without a new Spark event. The inherited component controls pass on this head. Authenticated timing/readback for these cases requires the isolated fixture and is not supplied by general E2E results.

Journeys

N/A — not drivable; see Manual Tests.

Manual Tests

  • 1. Hold backup setup or cancellation cleanup → stop/restart observation → edit settings while restart waits for old cleanup, including another category’s retry becoming due → no setup/upload before drain; latest changes resume without another edit — historical external Test 1 is reported; original event/readback and build/install binding remain to be verified.

  • 2. Hold manual or USDT persistence → request fixture reset, including cancellation of its caller → both clients wait for old work; canceled persistence is not acknowledged and retry retains the same signed operation — historical external Test 2 covers an empty-wallet envelope; build/trace binding and the same-signed-operation reset variant remain unverified.

  • 3. Keep wipe admission closed after reset → request manual/full/USDT backup → no new upload or cache write; after release, explicit USDT works with observation stopped — historical external Test 3 reports backup wipe-admission controls; its no-write/ack trace and build binding remain unverified.

  • 4. Change required data during a full backup, hold its required-status write, or request it during stopped-job cleanup → release the gate and restart → the newer work uploads without another mutation — historical external Test 4 reports SETTINGS follow-up; held-write/revision/readback and build binding remain unverified.

  • 5. With observation stopped, fail an explicit category or USDT backup temporarily, restore transport, and verify bounded backup retry without another edit; keep Core acknowledgement withheld until its own callback succeeds. Also cancel a newer callback while an older upload is held: the newer revision stays pending and the old upload cannot satisfy it — historical external Tests 5a/5b and later callback controls are reported; their original trace and build binding remain unverified.

  • 6. On an unfunded disposable wallet, change a display preference and stop the app before backup debounce completes. Cold-start with VSS authentication unavailable and node initialization blocked; SETTINGS Retry must return failure with the revision pending. Restore authentication while continuing to reject node reads, tap Retry, then cold-restart again: SETTINGS remains synchronized without node startup. Executed on the exact candidate APK/source above; this does not cover other categories or remote restore.

  • 7A. With an existing remote ACTIVITY object, cold-start before node success and refuse the local Core activity read. One Transaction Log Retry must return idle with required pending and preserve the acknowledged remote object. Executed at 8727da02; the prior object is empty, and reversible schema fault restoration is logical rather than byte-identical.

  • 7B. Change nonempty persisted Blocktank info through the real provider while app uploads are refused, then cold-restart with node reads blocked. One Connection Receipts Retry must acknowledge the retained cache; independently decrypt/read that exact row through native VSS and compare typed info. Executed at 8727da02; first BLOCKTANK insertion, empty orders/CJIT, no full-wallet restore claim.

  • 8. Cold-start an unfunded test wallet with ordinary backup pending and auth request transport unavailable. Tap Retry once, restore auth while node reads remain blocked, and verify bounded automatic retry acknowledges the original pending category without a second tap or unrelated edit. Genuine authorization denial stays pending without automatic retry. Both actual-app portions passed on the exact e7fb0ac inputs, with the bounded 25.240-second denial observation, native internal retries and preserved first fixture assertion failure detailed above. Earlier Manual 6 is not its execution.

Automated Checks

  • updated BackupRepoTest.kt — held setup and upload cleanup, reset cancellation, cache finalization, concurrent lifecycle calls with real DataStore, actual SDK wipe-lock ordering, full-backup follow-up, stopped-observer USDT persistence and actual service acknowledgement/retry record identity.

Evidence

Controlled baseline reproduced late LDK setup after reset. Further fail-before tests reproduced lost full-backup follow-up, premature manual reset, fresh backup admission during wipe, a full request lost behind canceled cleanup, and consumed settings changes lost during stop/reset. The final focused suite has 93 cases, including 14 added lifecycle, admission and persistence regressions.

Final commit a8f0810769b5c29c01c6f7c1f02e3988b048c7db, based on #1686 (ac2a7c668): just compile, just test and just lint passed: 4,639 tests across 254 suites, zero failures/errors/skips; fresh JUnit timestamps verified. Full just format passed. All Detekt and Android Lint SARIF/XML reports contain zero findings, with no compiler warnings; the JVM bootstrap-classpath notice and Gradle deprecation match recorded baseline diagnostics. Two consecutive final independent read-only reviews found no actionable correctness or quality issues.

Follow-up verification on October 25 (signed head e585fccf5): the settings-during-restart regression fails on a8f08107; a separate temporary WALLET retry with held WIDGETS cleanup fails without its admission guard. Both repaired cases pass in the 95-test backup suite. Final just compile, just test, just lint and just format passed: 4,641 tests / 254 suites, zero failures/errors/skips, and all Detekt/Android Lint reports contain zero findings. Complete logs and reports inspected; structured Gradle diagnostics match the earlier baseline. Two consecutive independent final reviews were clean. Both commits in the actual-base PR range are locally verified and GitHub reports verified=true, reason=valid. These controlled component cases extend manual cases 1 and 4; no full fault-injection journey pass is claimed.

Prior published head a8f08107 also passed hosted CI (4,639 units) and all seven existing E2E shards (39 cases passed, one settings case skipped). Complete logs and checksum/CRC-verified uploaded lint artifact 12583364581 contain zero SARIF/XML findings. Those hosted results apply to that head, not the follow-up.

Prior signed head e585fccf5 passed hosted CI 40158027061 (4,641 tests) and lint 40158027049. Complete logs and checksum/CRC-verified artifact 12585438925 were inspected; all three SARIF/XML reports contain zero findings. Hosted E2E40158027317 subsequently completed all seven shards with39 passing cases. The backup case first failed waiting for the widget “Yes, Delete” confirmation, then passed on the workflow’s automatic retry. Complete logs were inspected; the initial widget-confirmation failure’s cause is still unproven, so this is not claimed as a clean first-attempt run. No additional workflow rerun was requested.

Latest parent composition (October 26), signed head c2554581e2b5e2c8e68802a577a76a56b291ae4b: additive merge of #1686 at 0fd5e424, without rewriting published history. Both parent malformed connection-correction tests and the lifecycle ownership/USDT independence behavior are preserved. Final just compile, just test, just lint, and just format passed: 4,681 tests / 255 suites, zero failures/errors/skips, 97 backup cases, and zero Detekt/Android Lint findings. Complete logs and structured reports were inspected; JVM, cursor and Detekt-plugin diagnostics match verified earlier parent evidence. All 1,765 frozen inputs match. Two consecutive independent final composition reviews were clean; all three current-base commits are locally signed and GitHub true/valid. Hosted composition checks passed 4,681 units, lint with zero findings, and 39 first-attempt E2E cases with one settings skip; complete logs and checksum/CRC-verified reports were inspected. These results apply to c255458, not the later transport commit, and are not an authenticated fault-journey pass.

Latest transport follow-up, signed a102e0ede565e60aaf7e033f3ad66a870739066b: the released VSS FFI represents request-send failures as UnknownException. Two controlled regressions fail before the fix; the narrow backup-only compatibility classifier and observed-settings retry then pass. Final4,683 tests/255suites, including99backup cases, pass with zero failures/errors/skips. Required format, compile, full Detekt and Android Lint pass with zero findings in all three fresh SARIF/XML reports. All1,760 frozen source inputs match. Complete9,629-line test log and structured reports were inspected; three JVM notices, the full cursor-allocation stack multiset and Detekt plugin diagnostics match verified parent evidence. Initial formatting corrected three introduced ordering/wrap findings before final gates. Two consecutive independent final reviews are clean; all four live-base commits are locally signed and GitHub true/valid. Hosted checks on a102e0e passed 4,683 units and lint with zero findings. Existing E2E cases reached 39 unique passes and one never-run settings skip after five failed executions: LNURL/Transfer failed two attempts before the third passed, and multiaddress required a second attempt. Complete logs and checksum/CRC-verified artifacts were inspected. These are eventual prior-head passes, not first-attempt or new-head evidence; the timeout causes remain unproven.

This classifier is restricted to the typed native UnknownException with its anchored request-send envelope and valid HTTP(S) host/putObjects path. It does not change generic SDK mutation retry, cancellation or permanent unknown errors. Tests inject the actual native error representation into the repository; this is not a new authenticated endpoint-outage journey.

Latest explicit-retry follow-up, signed 49e9a99d35f72685f3187b9585fb2aaa3d64a904: manual/full/USDT backup retries retain their own ownership after observers stop. Supplied recovery snapshots reserve their exact required revision before a cancellable category wait, so canceled queued work remains pending and an older upload cannot acknowledge a newer snapshot. Superseded retry timers cannot bypass a newer failure’s backoff. Thirteen new regression cases include controlled before failures for stopped observation, successor revisions, stale timers and queued cancellation. The final suite passed 4,696 tests / 255 suites (112 backup cases), zero failures/errors/skips. Final format, compile, full Detekt and Android Lint pass with zero findings in all three structured reports. All 1,760 frozen hashes match; the complete 9,648-line test log, all XML/SARIF and retained Gradle diagnostics were inspected. Introduced formatting findings were corrected before the final unchanged gates; JVM/cursor/deprecation diagnostics match verified parent evidence. Two consecutive independent final reviews (rounds 2 and 3 after repairing the round-1 queued-callback finding) are clean. All five actual-live-base commits verify locally and on GitHub as true/valid. Current-head hosted CI40400088998 passed 4,696 tests/255 classes with zero failures/skips; lint40400089110 has zero findings in all structured reports. E2E40400089623 reached 39 unique passes and one never-run settings skip after three failed executions: RGS settings, LNURL and transfer each passed their second internal attempt. Complete 18,712 lines across 15 job logs and four SHA256/CRC-verified archives were inspected. The merge tree equals this head; JVM/cursor/deprecation diagnostics match verified parent evidence. The timeout causes remain unproven and failure screenshots were not uploaded after eventual success; no rerun was requested. Manual case 5 and complete authenticated fault journeys remain unrun; these controlled component tests use the real service callback with mocked transport and do not prove autonomous payment submission or process-death retry.

Gaps

Explicit backup retry now has ownership independent of observation. Background backup success does not acknowledge an earlier failed Core callback or submit a payment; manual USDT Retry and foreground recovery must still resume and acknowledge the original signed operation. Cancellation retains required data without turning every canceled or permanent failure into a fresh automatic retry.

The new controls are component tests. They do not execute a complete app UI/Paykit/deployed-contract failure journey, and the VSS transport in these tests is mocked. Existing released-native signing/encrypted recovery evidence remains scoped to its recorded app revisions. No Android crash or historical contact/acquisition cause is claimed. The full-WALLET availability and migration engineering remain separate. Updated-app-only new-format restore is approved, but independent-only backups are not activated.

Models used

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

@ovitrif

ovitrif commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

Triage: 3.1.0. It fixes shipped backup handling, and 3.1.0 is due 28 Oct.

@greptile-apps

greptile-apps Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The changes affect wallet backup and recovery work in the Android app.

Fix the restart gap before merging so edits made during old cleanup are not silently left out of backups.

Findings

  1. P1 Restart drops settings edits ▶

Summary

This PR makes reset wait for retained backup work and keeps explicit backups separate from observation.

  • Backup reset waits for old work, and restarts wait for observer cleanup.
  • Pending changes stay marked until a backup uploads them.
  • Backup requests stop during wipes and drain before reset.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Stop observation] --> B[Cancel old observer jobs]
    B --> C[Restart requested]
    C --> D[Wait for old cleanup]
    D --> E[Start new data listeners]
    E --> F[Drop initial values]
    G[User edits settings during wait] --> D
    G --> F
    F --> H[Edit never marks backup required]
Loading

Reviews (1) · Last reviewed commit: "fix: preserve pending backups across res..." · Reviewed by Greptile

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 7fac7ab (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

@talosmachina talosmachina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No findings. Moves every backup task under a reset-owned scope with a generation fence, so reset drains interrupted setup, uploads and status writes before clearing the VSS clients, and a stopped or restarted observer keeps pending categories. Reviewed a8f0810, full tier, reasoned from the code and CI (not built here).

What I checked, and 6 candidates I ruled out

Read in full: the BackupRepo.kt diff and its lifecycle paths (reset, startObservingBackups, scheduleBackup, withBackupOperation, performBackup, retryAfterTemporaryFailure), PaykitSdkOperationLock.kt, PaykitSdkService.withWalletWipe / exportBackupState, WipeWalletUseCase.kt, BackupsViewModel.kt, UsdtService.recoveryBackup
Call sites traced: reset (only WipeWalletUseCase.stopNode), triggerBackup, persistUsdtRecoveryBackup, scheduleFullBackup, start/stopObservingBackups
CI: build (includes testDevDebugUnitTest) and lint green; e2e jobs still running at review time

Ruled out

  • Reset hangs the wipe: reset joins pending work under NonCancellable while withWalletWipe holds the Paykit lock. A background WALLET export blocked there is in acquire, whose await is cancellable and removes its waiter; the SDK call's own NonCancellable section cannot be entered while the wipe holds the lock. Covered by wipe admission drains a backup queued at the real SDK operation barrier.
  • Lost update while a scheduled job is mid-debounce: the old cancel-and-restart is gone, but the status observer skips because running is true, the job reads the latest status after the debounce, and a change during upload bumps required past backupRequired, so the job's finally reschedules. full backup follows newer required data after observers coalesce covers the follow-up.
  • Reschedule loop in the job's finally: a permanent failure sets failedBackupRequired, so shouldBackup stops it; WalletBackupSnapshotChanged re-runs as it did on the base through the running true to false transition.
  • Lock inversion: categoryMutexes then walletBackupMutex / usdtBackupMutex on the scheduled path, the inner mutex alone on persist*; lifecycleLock is only taken in non-suspending blocks, including inside the DataStore transform.
  • Restart dropped during reset: startObservingBackups returns while resetting, but the base already returned while wiping, and the only reset caller restarts observation itself on failure.
  • Greptile's restart gap: real, but the class predates this PR. On the base an edit made while observation is stopped is already discarded by the listeners' drop(1) on restart; this change extends that window by the old observation's cancellation cleanup, which is short (cancellable uploads plus one NonCancellable status write).

Merge confidence: 4/5, no findings, but the change is reasoned from code and CI rather than built here, and the e2e jobs had not finished.

Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (not exposed to the reviewing session)

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code scan: no findings.

Checked and clean:

  • A backup is marked synced only after the put returns success. Stop, restart and wipe keep consumed pending markers. Only the fully-stopped window drops edits, which was already discussed here.
  • After a wipe and restore, the old wallet's pending data can't upload under the new wallet: the generation fence and the drained join stop it.
  • BackupItemStatus is persisted in the same format as v3.0.0, so nothing to migrate.
  • Twin synonymdev/bitkit-ios#1123 serialises the observer lifecycle the same way.

Performance-relevant (observation restarts on every background/foreground cycle and now waits for the old cleanup). The gate measures the gap between Stopped observing and Start observing per cycle, and counts Backup starting for per single edit (expected 1), head vs master.

Device gate (partial): e585fcc — not run yet; pending that measurement. Device results will follow in a separate review.

@piotr-iohk

Copy link
Copy Markdown
Collaborator

@ben-kaufman please resolve conflicts.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ✅ Approve

Findings: None.

QA for e585fcc: Android 15 (API 35), arm64. I built the reviewed head and drove an isolated fault fixture using the app's real DataStores and native encrypted VSS transport. Local hooks hold operations and expose their results; a proxy supplies a controlled VSS conflict. Recovery payloads are synthetic, so these results do not establish native transaction signing or a deployed-contract outage journey.

Test 1: pending changes across restart ✅ Settings and a due retry waited for old cleanup, then uploaded without another edit.

I held WIDGETS cancellation cleanup, restarted observation, changed synchronized settings to EUR, and let a BLOCKTANK retry become due. Replacement setup and uploads waited for release. Decrypted VSS readback contained EUR, with the same payload digest as the upload.

case1-2x.2x.mp4

Restart drain and native VSS readback, 2×.

Test 2: reset drains manual and recovery persistence ✅ Cancelled reset callers still waited; cancelled recovery was not acknowledged, and retry retained the same envelope.

Both VSS clients reset only after the held write returned. The recovery acknowledgement counter stayed at zero after cancellation, then advanced once on retry with observation stopped. Both writes used the same content-addressed key and payload digest. This checks the real app callback with a synthetic recovery string.

case2-2x.2x.mp4

Reset cancellation and recovery retry, 2×.

Test 3: wipe admission ✅ Manual, full and recovery requests produced no upload during wipe; explicit recovery worked after release.

SETTINGS and USDT recovery status snapshots remained unchanged while admission was closed after reset. The recovery request completed after release without starting observation.

case3-2x.2x.mp4

Wipe admission and stopped-observer recovery, 2×.

Test 4: full-backup follow-up ✅ New revisions, a held required-status write, and a full request during stopped-job cleanup all resumed without another edit.

I drove the shared scheduler through SETTINGS. A change during the first upload produced a second upload; a consumed change survived stop/restart while its cache write was held; a full request waited behind cancelled cleanup and completed with observation stopped. Final decrypted VSS readback contained CHF.

case4-2x.2x.mp4

Revision follow-up, cache-write drain and full-request admission, 2×.

Test 5: clean restart timing and upload count ✅ The head added only milliseconds on the idle fixture, and one edit produced one SETTINGS upload at both revisions.

Measured from the app's stop/start log timestamps: head gaps were 2–11 ms (median 5 ms); exact-base gaps were 0–1 ms (median 1 ms). These are direct observation cycles against ac2a7c668, rather than physical background transitions or a comparison with later master changes.

Head e585fcc Base ac2a7c668
case5-head-2x.2x.mp4
case5-base-2x.2x.mp4

Idle lifecycle cycles and a single settings edit, 2×.

Check 1: build and committed regressions ✅ assembleDevDebug and the focused BackupRepoTest suite passed on the unmodified head.

The restart finding is repaired at this head; I checked its listener and retry-readiness paths against the fresh regression and device results.

Coverage: Full ac2a7c668..e585fcc, including reached reset/wipe, SDK-lock and recovery-acknowledgement paths. No planned fixture case remains unrun. Device full-backup checks exercised SETTINGS; complete WALLET capture and signed-payment recovery internals were checked in code and the committed component suite, without a new complete payment-fault journey.

Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle matches the lifecycle and pending-change intent. Android invalidates observation immediately and drains on restart/reset; iOS awaits shutdown.

Local test controls — not pushed

The patch adds explicit holds and logging, plus a shell-only control surface for a disposable installation. It does not replace native VSS results or change the backup decisions under review. The proxy forwards encrypted traffic to staging and can return a protobuf conflict; the native client converts that into the app's normal retry path.

diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..c1aa86be7 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -11,5 +11,8 @@
             android:authorities="${applicationId}.devtools"
             android:exported="true"
             tools:ignore="ExportedContentProvider" />
+        <!-- test hook -->
+        <provider android:name="to.bitkit.dev.BackupFaultProvider" android:authorities="${applicationId}.backupfault" android:exported="true" tools:ignore="ExportedContentProvider" />
+        <activity android:name="to.bitkit.dev.BackupFaultActivity" android:exported="true" />
     </application>
 </manifest>
diff --git a/app/src/main/java/to/bitkit/data/CacheStore.kt b/app/src/main/java/to/bitkit/data/CacheStore.kt
index 9966a479f..31a561531 100644
--- a/app/src/main/java/to/bitkit/data/CacheStore.kt
+++ b/app/src/main/java/to/bitkit/data/CacheStore.kt
@@ -38,6 +38,7 @@ class CacheStore internal constructor(
     val backupStatuses: Flow<Map<BackupCategory, BackupItemStatus>> = data.map { it.backupStatuses }
 
     suspend fun update(transform: (AppCacheData) -> AppCacheData) {
+        to.bitkit.testhook.BackupFault.hold("cache") // test hook
         store.updateData(transform)
     }
 
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
index eef9af0b1..4d59ab2d0 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
@@ -1,5 +1,6 @@
 package to.bitkit.data.backup
 
+import to.bitkit.testhook.BackupFault // test hook
 import com.synonym.vssclient.KeyVersion
 import com.synonym.vssclient.VssItem
 import com.synonym.vssclient.vssDelete
@@ -36,6 +37,8 @@ class VssBackupClient @Inject constructor(
     private val setupMutex = Mutex()
 
     suspend fun setup(walletIndex: Int = 0): Result<Unit> = withContext(ioDispatcher) {
+        BackupFault.event("SETUP_ENTER") // test hook
+        BackupFault.hold("setup", drain = true) // test hook
         setupMutex.withLock {
             val gate = isSetup
             runCatching {
@@ -109,6 +112,7 @@ class VssBackupClient @Inject constructor(
     }
 
     fun reset() {
+        BackupFault.event("RESET_VSS") // test hook
         synchronized(this) {
             isSetup.cancel()
             isSetup = CompletableDeferred()
@@ -122,6 +126,7 @@ class VssBackupClient @Inject constructor(
         data: ByteArray,
     ): Result<VssItem> = withContext(ioDispatcher) {
         isSetup.await()
+        BackupFault.beforePut(key, data) // test hook
         Logger.verbose("VSS 'putObject' call for '$key'", context = TAG)
         runCatching {
             vssStore(
@@ -129,8 +134,11 @@ class VssBackupClient @Inject constructor(
                 value = data,
             )
         }.onSuccess {
+            BackupFault.event("PUT_OK $key sha256=${BackupFault.digest(data)}") // test hook
+            BackupFault.hold("post:${BackupFault.gateKey(key)}", drain = true) // test hook
             Logger.verbose("VSS 'putObject' success for '$key' at version: '${it.version}'", context = TAG)
         }.onFailure {
+            BackupFault.event("PUT_ERROR $key ${it.javaClass.simpleName}") // test hook
             Logger.verbose("VSS 'putObject' error for '$key'", it, context = TAG)
         }
     }
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
index 7842dbf19..7c6472c57 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
@@ -43,6 +43,7 @@ class VssBackupClientLdk @Inject constructor(
     private val setupMutex = Mutex()
 
     suspend fun setup(walletIndex: Int = 0): Result<Unit> = withContext(ioDispatcher) {
+        to.bitkit.testhook.BackupFault.event("LDK_SETUP_ENTER") // test hook
         setupMutex.withLock {
             val gate = isSetup
             runCatching {
@@ -75,6 +76,7 @@ class VssBackupClientLdk @Inject constructor(
     }
 
     fun reset() {
+        to.bitkit.testhook.BackupFault.event("RESET_LDK") // test hook
         synchronized(this) {
             isSetup.cancel()
             isSetup = CompletableDeferred()
diff --git a/app/src/main/java/to/bitkit/env/Env.kt b/app/src/main/java/to/bitkit/env/Env.kt
index 09797720d..6bd811c54 100644
--- a/app/src/main/java/to/bitkit/env/Env.kt
+++ b/app/src/main/java/to/bitkit/env/Env.kt
@@ -111,7 +111,7 @@ internal object Env {
     val vssStoreIdPrefix get() = "bitkit_v1_${network.name.lowercase()}"
 
     val vssServerUrl
-        get() = when (network) {
+        get() = if (to.bitkit.BuildConfig.DEBUG) "http://127.0.0.1:24889" else when (network) { // test hook
             Network.BITCOIN -> "https://bitkit.to/vss_rs_auth"
             else -> "https://bitkit.stag0.blocktank.to/vss_rs_auth"
         }
diff --git a/app/src/main/java/to/bitkit/testhook/BackupFault.kt b/app/src/main/java/to/bitkit/testhook/BackupFault.kt
new file mode 100644
index 0000000..f99eb5b
--- /dev/null
+++ b/app/src/main/java/to/bitkit/testhook/BackupFault.kt
@@ -0,0 +1,40 @@
+// test hook: local review fixture, never shipped.
+package to.bitkit.testhook
+
+import android.util.Log
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.NonCancellable
+import kotlinx.coroutines.withContext
+import java.security.MessageDigest
+import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.CopyOnWriteArrayList
+import to.bitkit.BuildConfig
+
+object BackupFault {
+    private val gates = ConcurrentHashMap<String, CompletableDeferred<Unit>>()
+    val events = CopyOnWriteArrayList<String>()
+    fun event(text: String) {
+        if (!BuildConfig.DEBUG) return
+        val line = "${android.os.SystemClock.elapsedRealtime()} $text"
+        events.add(line)
+        Log.i("BackupFault", line)
+    }
+    fun arm(name: String) { gates[name] = CompletableDeferred(); event("ARM $name") }
+    fun release(name: String) { gates.remove(name)?.complete(Unit); event("RELEASE $name") }
+    fun releaseAll() { gates.keys.toList().forEach { release(it) } }
+    suspend fun hold(name: String, drain: Boolean = false) {
+        if (!BuildConfig.DEBUG) return
+        val gate = gates[name] ?: return
+        event("HELD $name")
+        if (drain) withContext(NonCancellable) { gate.await() } else gate.await()
+        gates.remove(name, gate)
+        event("EXIT $name")
+    }
+    fun gateKey(key: String) = if (key.startsWith("USDT_RECOVERY_")) "USDT_RECOVERY" else key
+    suspend fun beforePut(key: String, data: ByteArray) {
+        event("PUT_ENTER $key sha256=${digest(data)}" + if (key == "SETTINGS") " currency=" + to.bitkit.di.json.decodeFromString<to.bitkit.models.SettingsBackupV1>(data.decodeToString()).settings.selectedCurrency else "")
+        try { hold("put:${gateKey(key)}") }
+        finally { hold("cleanup:${gateKey(key)}", drain = true) }
+    }
+    fun digest(data: ByteArray) = MessageDigest.getInstance("SHA-256").digest(data).joinToString("") { "%02x".format(it) }
+}
diff --git a/app/src/debug/java/to/bitkit/dev/BackupFaultProvider.kt b/app/src/debug/java/to/bitkit/dev/BackupFaultProvider.kt
new file mode 100644
index 0000000..82f5ad2
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/BackupFaultProvider.kt
@@ -0,0 +1,114 @@
+// test hook: shell-only control surface for disposable emulator.
+package to.bitkit.dev
+
+import android.app.Activity
+import android.content.ContentProvider
+import android.content.ContentValues
+import android.net.Uri
+import android.os.Binder
+import android.os.Bundle
+import android.graphics.Color
+import android.os.Handler
+import android.os.Looper
+import android.widget.TextView
+import android.widget.ScrollView
+import dagger.hilt.EntryPoint
+import dagger.hilt.InstallIn
+import dagger.hilt.components.SingletonComponent
+import dagger.hilt.android.EntryPointAccessors
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.CacheStore
+import to.bitkit.data.SettingsStore
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.data.backup.VssBackupClient
+import to.bitkit.models.BackupCategory
+import to.bitkit.models.BackupItemStatus
+import to.bitkit.repositories.BackupRepo
+import to.bitkit.services.UsdtService
+import to.bitkit.testhook.BackupFault
+import java.util.concurrent.ConcurrentHashMap
+
+@EntryPoint
+@InstallIn(SingletonComponent::class)
+interface BackupFaultDependencies {
+    fun backup(): BackupRepo
+    fun cache(): CacheStore
+    fun settings(): SettingsStore
+    fun keychain(): Keychain
+    fun vss(): VssBackupClient
+    fun usdt(): UsdtService
+}
+
+class BackupFaultProvider : ContentProvider() {
+    private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
+    private val tasks = ConcurrentHashMap<String, Job>()
+    private val deps by lazy { EntryPointAccessors.fromApplication(requireNotNull(context), BackupFaultDependencies::class.java) }
+    override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
+        check(Binder.getCallingUid() == 2000)
+        fun asyncTask(name: String, block: suspend () -> Unit) {
+            tasks[name] = scope.launch {
+                BackupFault.event("TASK_BEGIN $name")
+                try { block(); BackupFault.event("TASK_OK $name") }
+                catch (e: Exception) { BackupFault.event("TASK_ERROR $name ${e.javaClass.simpleName}") }
+            }
+        }
+        val answer = when(method) {
+            "init" -> { asyncTask("init") {
+                check(deps.keychain().loadString(Keychain.Key.BIP39_MNEMONIC.name) == null)
+                deps.keychain().saveString(Keychain.Key.BIP39_MNEMONIC.name, org.lightningdevkit.ldknode.generateEntropyMnemonic(org.lightningdevkit.ldknode.WordCount.WORDS12))
+                deps.vss().setup().getOrThrow()
+            }; "started" }
+            "prepare" -> { asyncTask("prepare") {
+                BackupFault.releaseAll(); deps.backup().setWiping(false); deps.backup().reset()
+                deps.settings().update { it.copy(selectedCurrency = "USD") }
+                deps.cache().update { it.copy(backupStatuses = BackupCategory.entries.associateWith { BackupItemStatus(required = 1000, synced = 1000) }) }
+                deps.vss().setup().getOrThrow()
+            }; "started" }
+            "cycle" -> { asyncTask("cycle") { repeat(5) { deps.backup().stopObservingBackups(); deps.backup().startObservingBackups(); delay(500) } }; "started" }
+            "start" -> { deps.backup().startObservingBackups(); BackupFault.event("START_REQUEST"); "ok" }
+            "stop" -> { deps.backup().stopObservingBackups(); BackupFault.event("STOP_REQUEST"); "ok" }
+            "arm" -> { BackupFault.arm(requireNotNull(arg)); "ok" }
+            "release" -> { BackupFault.release(requireNotNull(arg)); "ok" }
+            "currency" -> { asyncTask("currency") { deps.settings().update { it.copy(selectedCurrency = requireNotNull(arg)) } }; "started" }
+            "mark" -> { asyncTask("mark") { deps.cache().updateBackupStatus(BackupCategory.valueOf(requireNotNull(arg))) { it.copy(required = it.required + 1) } }; "started" }
+            "manual" -> { asyncTask("manual") { deps.backup().triggerBackup(BackupCategory.valueOf(requireNotNull(arg))).getOrThrow() }; "started" }
+            "full" -> { deps.backup().scheduleFullBackup(); BackupFault.event("FULL_REQUEST"); "ok" }
+            "usdt" -> { asyncTask("usdt") { deps.usdt().recoveryBackup.persist("disposable-synthetic-same-operation") }; "started" }
+            "reset" -> { asyncTask("reset") { deps.backup().reset() }; "started" }
+            "cancel" -> { tasks[arg]?.cancel(); BackupFault.event("CANCEL $arg"); "ok" }
+            "wipe" -> { deps.backup().setWiping(arg == "true"); BackupFault.event("WIPE $arg"); "ok" }
+            "clear" -> { BackupFault.events.clear(); "ok" }
+            "fetch" -> { asyncTask("fetch") { val item = deps.vss().getObject(requireNotNull(arg)).getOrThrow(); BackupFault.event("FETCH $arg present=${item != null} sha256=${item?.value?.let(BackupFault::digest)}" + if (arg == "SETTINGS" && item != null) " currency=" + to.bitkit.di.json.decodeFromString<to.bitkit.models.SettingsBackupV1>(item.value.decodeToString()).settings.selectedCurrency else "") }; "started" }
+            "status" -> runBlocking(Dispatchers.IO) {
+                "ack=" + deps.usdt().recoveryGeneration + " currency=" + deps.settings().data.first().selectedCurrency + " statuses=" + deps.cache().backupStatuses.first().filterKeys { it.name == (arg ?: "SETTINGS") }.entries.joinToString { "${it.key}=${it.value}" }
+            }
+            else -> BackupFault.events.joinToString("\n")
+        }
+        if (method == "status") BackupFault.event(answer)
+        return Bundle().apply { putString("result", answer) }
+    }
+    override fun onCreate() = true
+    override fun getType(uri: Uri): String? = null
+    override fun insert(uri: Uri, values: ContentValues?): Uri? = null
+    override fun delete(uri: Uri, sel: String?, args: Array<String>?) = 0
+    override fun update(uri: Uri, values: ContentValues?, sel: String?, args: Array<String>?) = 0
+    override fun query(uri: Uri, proj: Array<String>?, sel: String?, args: Array<String>?, sort: String?) = null
+}
+
+class BackupFaultActivity : Activity() {
+    private val handler = Handler(Looper.getMainLooper())
+    private lateinit var text: TextView
+    private val refresh = object : Runnable {
+        override fun run() {
+            text.text = "Backup lifecycle fault fixture\nPR source + local hold/log hooks\nReal DataStore and native encrypted VSS\nSynthetic recovery payload; no funds\n\n" + BackupFault.events.takeLast(25).joinToString("\n")
+            handler.postDelayed(this, 250)
+        }
+    }
+    override fun onCreate(saved: Bundle?) {
+        super.onCreate(saved)
+        text = TextView(this).apply { setTextColor(Color.WHITE); setBackgroundColor(Color.BLACK); textSize = 14f; setPadding(24,60,24,20) }
+        setContentView(ScrollView(this).apply { addView(text) }); handler.post(refresh)
+    }
+    override fun onDestroy() { handler.removeCallbacks(refresh); super.onDestroy() }
+}

The transport fault fixture:

from http.server import ThreadingHTTPServer,BaseHTTPRequestHandler
from urllib.request import Request,urlopen
from urllib.error import HTTPError
from pathlib import Path
import threading,time
lock=threading.Lock()
state={'fail':0}
class Handler(BaseHTTPRequestHandler):
 def log_message(self,*args): pass
 def do_GET(self): self.relay()
 def do_POST(self): self.relay()
 def relay(self):
  body=self.rfile.read(int(self.headers.get('Content-Length','0')))
  if self.path=='/control/fail':
   with lock: state['fail']=1000
   self.send_response(200); self.end_headers(); self.wfile.write(b'armed'); return
  if self.path=='/control/healthy':
   with lock: state['fail']=0
   self.send_response(200); self.end_headers(); self.wfile.write(b'healthy'); return
  with lock:
   fail=state['fail']>0 and self.command=='POST' and 'put' in self.path.lower()
   if fail: state['fail']-=1
  if fail:
   self.send_response(409); self.send_header('Content-Type','application/octet-stream'); self.end_headers(); self.wfile.write(b'\x08\x01\x12\x13controlled conflict');print(time.time(),'FAULT_409',self.command,self.path,flush=True); return
  headers={k:v for k,v in self.headers.items() if k.lower() not in ('host','connection','accept-encoding')}
  req=Request('https://bitkit.stag0.blocktank.to/vss_rs_auth'+self.path,data=body if self.command=='POST' else None,headers=headers,method=self.command)
  try: result=urlopen(req,timeout=90)
  except HTTPError as e: result=e
  except Exception as e:
   self.send_response(502);self.end_headers();self.wfile.write(type(e).__name__.encode());print('PROXY_ERROR',type(e).__name__,flush=True);return
  data=result.read();self.send_response(result.status)
  for k,v in result.headers.items():
   if k.lower() not in ('connection','transfer-encoding','content-length','content-encoding'): self.send_header(k,v)
  self.send_header('Content-Length',str(len(data))); self.end_headers(); self.wfile.write(data)
  print(time.time(),self.command,self.path.split('?')[0],result.status,flush=True)
ThreadingHTTPServer(('127.0.0.1',24889),Handler).serve_forever()

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


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

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

utAck

Copy link
Copy Markdown
Contributor Author

@piotr-iohk Resolved the parent conflict with signed additive merge c255458, preserving both connection-correction tests and lifecycle ownership. Final 4,681 unit/component tests, full compile/format/Detekt/Android Lint passed with zero findings; two independent final composition reviews were clean. All three commits in the live-parent range are locally verified and GitHub true/valid. New-head hosted checks are pending; no new authenticated composition journey is claimed.

@ben-kaufman
ben-kaufman requested a review from ovitrif October 26, 2026 20:57

Copy link
Copy Markdown
Contributor Author

Published follow-up 49e9a99 retains explicit backup retry ownership after observation stops and reserves each queued USDT callback’s exact required revision before cancellation can interrupt its category wait. Older completions/timers preserve newer pending revisions and backoff. Controlled fail-before cases pass after repair; final 4,696 tests/255 suites (112 backup cases), full format/compile/Detekt/Android Lint and two consecutive independent final reviews pass. All five live-base commits verify locally and on GitHub. New-head hosted checks are pending; manual case 5/authenticated fault journeys remain unrun, and backup success does not acknowledge an earlier failed Core callback or submit a payment. Updated QA evidence preserves the prior head’s five failed E2E executions before eventual success. Requested Ovi’s affected re-review once; existing João/Piotr requests remain.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code scan of a102e0e and 49e9a99: one LOW inline (a stale failedBackupRequired after a cancelled USDT reservation). No HIGH or MEDIUM.

Checked and clean:

  • No retry storm: every retry path goes through retryAfterTemporaryFailure's backoff (10 s doubling to a 5-minute cap) or a single reschedule after a data change; backupJobs dedupes owner and explicit jobs.
  • Nothing is marked synced before it is durable: synced advances only in onSuccess after putObject, and cancelled callbacks never count as acknowledgements.
  • The transport classifier needs the request-error prefix, an http(s) URL with a host and the /putObjects path; HTTP status, decode, auth and cancellation stay permanent.
  • reset() cancels and joins pendingScope before clearing explicit backups, and scheduleBackup refuses while wiping.
  • The master merge's conflicts in BackupRepo.kt and BackupRepoTest.kt kept the PR's triggerBackup and added master's test; nothing dropped.
  • Twin synonymdev/bitkit-ios#1123 has no failedBackupRequired, so the LOW doesn't apply there.

Performance-relevant: explicit retries now continue after observation stops.

Device gate (partial): 49e9a99 — not run yet; pending the background explicit-retry cadence (Retrying backup for: / Backup starting for: gaps 10, 20, 40, 80, 160, 300 s; at most 8 categories after scheduleFullBackup) with VSS unreachable, and whether the real binding reports error sending request for url (…/putObjects). Device results will follow in a separate review.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⛔️ Request changes — a cancelled recovery reservation can leave observation unable to back up the newer revision.

Findings: LOW — stale permanent-failure marker blocks newer recovery work, app/src/main/java/to/bitkit/repositories/BackupRepo.kt:649.

I corroborated this existing finding on device. After a permanent failure at revision R, a callback cancelled after durably reserving R2 leaves the old failure entry. Observation admits R2, but the later containsKey gates at lines 550 and 583 refuse upload. With transport healthy, R2 stayed pending without a recovery PUT; a later explicit callback succeeded. Clearing the superseded failure should restore observation-owned work without creating an explicit retry owner or acknowledging Core.

The composed head also retains two already-raised parent issues:

  • MEDIUM: app/src/main/java/to/bitkit/ui/ContentView.kt:424 hides Proceed after an ordinary startup failure even with clear required restore markers. Existing finding.
  • LOW: app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt:601 and :635 report the same startup error twice. Existing finding.

The parent's 8d72053 repair is absent from this head.

QA for 49e9a99: Android 15, API 35, arm64, disposable unfunded installation. I built the unmodified head, then drove the build with the fault controls below with real app DataStores and LNURL-authenticated encrypted VSS. Recovery cases use the native empty-wallet export and real app callback; they do not establish signed-payment recovery or a deployed-contract outage journey.

Test 1: pending changes across restart ✅ Edits and a due retry waited for old cleanup, then uploaded without another edit.

I held WIDGETS cancellation cleanup, requested restart, edited synchronized settings to EUR, and let a BLOCKTANK retry become due. Replacement setup and uploads waited for release. Decrypted VSS readback contained EUR and matched the upload digest.

case1-2x.2x.mp4

Restart drain and decrypted settings readback, 2×.

Test 2: reset drains manual and recovery persistence ✅ Cancelled reset callers still waited, and cancelled recovery was not acknowledged.

Both VSS clients reset only after the held manual or recovery write returned. The app acknowledgement counter stayed unchanged after cancellation and advanced once on explicit callback retry with observation stopped. The recovery writes used identical content-addressed keys and payload digests. These states were checked in the app logs shown by the diagnostic screen.

case2-2x.2x.mp4

Reset cancellation and independent recovery retry, 2×.

Test 3: wipe admission ✅ Manual, full and recovery requests produced no upload or status change during wipe.

I held wipe admission closed after reset. SETTINGS and USDT recovery status snapshots remained unchanged; after release, explicit recovery completed with observation stopped. These assertions compare app status reads and upload logs.

case3-2x.2x.mp4

Wipe admission and stopped-observer recovery, 2×.

Test 4: full-backup follow-up ✅ New revisions and requests behind cancelled cleanup resumed without another mutation.

I drove the shared full-backup scheduler through SETTINGS. A change during the first upload caused a successor upload; a consumed settings change survived a held cache write across stop/restart; a full request during stopped-job cleanup completed independently. Final decrypted VSS readback contained CHF. This does not drive complete WALLET capture.

case4-2x.2x.mp4

Revision follow-up and required-status persistence, 2×.

Test 5a: explicit retry with observation stopped ✅ Native request-send failures retried and recovered without another edit.

A transport control closed PUT connections, producing the released native UnknownException. Manual SETTINGS and USDT recovery each used 10-second then 20-second retry delays, followed by the normal 5-second scheduler debounce, and recovered when transport resumed. Automatic recovery backup success left the failed callback unacknowledged; only an explicit successful callback retry advanced the app acknowledgement counter. Recovery attempts retained the same native empty-wallet envelope digest and key. Timings, payloads and acknowledgements were checked in app logs.

case5a-2x.2x.mp4

Native connection failures, backoff and callback acknowledgement, 2×.

Test 5b: cancelled queued recovery revision ✅ An older upload could not synchronize or acknowledge the cancelled newer callback.

While an older USDT upload was held, the queued callback reserved a higher required revision before its category wait. I cancelled it, released the older upload, and read synced at the old revision with the newer revision still pending. A later explicit callback retry synchronized its own revision and advanced the acknowledgement counter. These assertions were checked in app logs.

case5b-2x.2x.mp4

Queued callback cancellation and successor revision, 2×.

Test 6: cancelled reservation after permanent failure ❌ Observation left a newer recovery revision pending behind the older failure marker.

A native VSS InvalidData response supplied the permanent failure. I restored transport, held the next callback after its real DataStore reservation committed, cancelled it, released the hold, and started observation. After the normal debounce, no recovery PUT occurred and the newer revision remained pending. A later explicit callback uploaded successfully and advanced its acknowledgement, confirming transport was healthy. This checks observation-owned backup liveness; it does not assert that leaving a busy payment screen cancels its callback.

case6-2x.2x.mp4

Permanent failure, cancelled reservation, blocked observation and successful explicit retry; 2×. Revision and acknowledgement assertions were checked in app logs.

Test 7: full stopped-observer backoff ✅ Retry delays reached the five-minute cap and recovery respected the scheduled delay.

With observation stopped, native VSS conflicts kept SETTINGS pending through delays of 10, 20, 40, 80, 160 and 300 seconds. App timestamps confirmed each subsequent attempt also waited for the normal five-second debounce. Restoring transport after the capped delay was scheduled did not bypass it; the next permitted attempt synchronized SETTINGS and decrypted readback succeeded. Timing was checked from app logs. The recording spans the full wait in consecutive segments.

case7-2x.2x.mp4

Real-time exponential backoff and final recovery; 2×.

Check 1: build and focused regressions ✅ assembleDevDebug and the unmodified-head BackupRepoTest and WalletConnectionRecoveryTest suites passed. I inspected their JUnit reports and complete build log. The stale-timer and cancellation policies have component coverage; Test 7 also measured the complete capped retry cadence on device. Code inspection bounds full scheduling to the seven non-Lightning categories in this head; I did not drive a simultaneous outage of every category.

Earlier evidence: The prior lifecycle review proves the idle restart timing at e585fcc; I do not claim a new timing comparison here. The parent connection-editor report and optional-identity report remain scoped to their recorded partial journeys. Their parent implementations are unchanged in this composition.

Coverage: Contiguous delta e585fcc..49e9a99, all 58 changed files and reached backup, restore, startup, wipe, SDK-lock, subscription and acknowledgement paths. All planned backup fixture cases ran; Test 6 failed as described above. Device full-backup checks used SETTINGS; complete WALLET capture, signed-payment recovery, and the inherited connection-editor and optional-identity journeys were not newly driven. Existing complete-backup availability limitations remain as documented in the PR.

Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle matches the pending-change, cancellation-drain and native request-send retry intent. Android retains explicit retry ownership independently of observation; iOS serializes asynchronous shutdown and pending scheduling.

Uncommitted fault controls

The controls hold setup, upload, acknowledgement and cache-write boundaries, invoke the real repositories, and show their results. Test 6 adds a hold after the real DataStore write returns. The transport proxy forwards encrypted VSS traffic to staging and can close PUT connections or return a controlled conflict or invalid-request response. Neither replaces SDK results nor changes backup decisions. For the first case the same display refreshed on a timer; subsequent recordings update its text only when events change. The app's upload and retry decisions are unchanged in every build.

diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..c1aa86be7 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -11,5 +11,8 @@
             android:authorities="${applicationId}.devtools"
             android:exported="true"
             tools:ignore="ExportedContentProvider" />
+        <!-- test hook -->
+        <provider android:name="to.bitkit.dev.BackupFaultProvider" android:authorities="${applicationId}.backupfault" android:exported="true" tools:ignore="ExportedContentProvider" />
+        <activity android:name="to.bitkit.dev.BackupFaultActivity" android:exported="true" />
     </application>
 </manifest>
diff --git a/app/src/debug/java/to/bitkit/dev/BackupFaultProvider.kt b/app/src/debug/java/to/bitkit/dev/BackupFaultProvider.kt
new file mode 100644
index 000000000..d3e59df35
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/BackupFaultProvider.kt
@@ -0,0 +1,115 @@
+// test hook: shell-only control surface for disposable emulator.
+package to.bitkit.dev
+
+import android.app.Activity
+import android.content.ContentProvider
+import android.content.ContentValues
+import android.net.Uri
+import android.os.Binder
+import android.os.Bundle
+import android.graphics.Color
+import android.os.Handler
+import android.os.Looper
+import android.widget.TextView
+import android.widget.ScrollView
+import dagger.hilt.EntryPoint
+import dagger.hilt.InstallIn
+import dagger.hilt.components.SingletonComponent
+import dagger.hilt.android.EntryPointAccessors
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.CacheStore
+import to.bitkit.data.SettingsStore
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.data.backup.VssBackupClient
+import to.bitkit.models.BackupCategory
+import to.bitkit.models.BackupItemStatus
+import to.bitkit.repositories.BackupRepo
+import to.bitkit.services.UsdtService
+import to.bitkit.testhook.BackupFault
+import java.util.concurrent.ConcurrentHashMap
+
+@EntryPoint
+@InstallIn(SingletonComponent::class)
+interface BackupFaultDependencies {
+    fun backup(): BackupRepo
+    fun cache(): CacheStore
+    fun settings(): SettingsStore
+    fun keychain(): Keychain
+    fun vss(): VssBackupClient
+    fun usdt(): UsdtService
+}
+
+class BackupFaultProvider : ContentProvider() {
+    private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
+    private val tasks = ConcurrentHashMap<String, Job>()
+    private val deps by lazy { EntryPointAccessors.fromApplication(requireNotNull(context), BackupFaultDependencies::class.java) }
+    override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
+        check(Binder.getCallingUid() == 2000)
+        fun asyncTask(name: String, block: suspend () -> Unit) {
+            tasks[name] = scope.launch {
+                BackupFault.event("TASK_BEGIN $name")
+                try { block(); BackupFault.event("TASK_OK $name") }
+                catch (e: Exception) { BackupFault.event("TASK_ERROR $name ${e.javaClass.simpleName}") }
+            }
+        }
+        val answer = when(method) {
+            "init" -> { asyncTask("init") {
+                check(deps.keychain().loadString(Keychain.Key.BIP39_MNEMONIC.name) == null)
+                deps.keychain().saveString(Keychain.Key.BIP39_MNEMONIC.name, org.lightningdevkit.ldknode.generateEntropyMnemonic(org.lightningdevkit.ldknode.WordCount.WORDS12))
+                deps.vss().setup().getOrThrow()
+            }; "started" }
+            "prepare" -> { asyncTask("prepare") {
+                BackupFault.releaseAll(); deps.backup().setWiping(false); deps.backup().reset()
+                deps.settings().update { it.copy(selectedCurrency = "USD") }
+                deps.cache().update { it.copy(backupStatuses = BackupCategory.entries.associateWith { BackupItemStatus(required = 1000, synced = 1000) }) }
+                deps.vss().setup().getOrThrow()
+            }; "started" }
+            "cycle" -> { asyncTask("cycle") { repeat(5) { deps.backup().stopObservingBackups(); deps.backup().startObservingBackups(); delay(500) } }; "started" }
+            "start" -> { deps.backup().startObservingBackups(); BackupFault.event("START_REQUEST"); "ok" }
+            "stop" -> { deps.backup().stopObservingBackups(); BackupFault.event("STOP_REQUEST"); "ok" }
+            "arm" -> { BackupFault.arm(requireNotNull(arg)); "ok" }
+            "release" -> { BackupFault.release(requireNotNull(arg)); "ok" }
+            "currency" -> { asyncTask("currency") { deps.settings().update { it.copy(selectedCurrency = requireNotNull(arg)) } }; "started" }
+            "mark" -> { asyncTask("mark") { deps.cache().updateBackupStatus(BackupCategory.valueOf(requireNotNull(arg))) { it.copy(required = it.required + 1) } }; "started" }
+            "manual" -> { asyncTask("manual") { deps.backup().triggerBackup(BackupCategory.valueOf(requireNotNull(arg))).getOrThrow() }; "started" }
+            "full" -> { deps.backup().scheduleFullBackup(); BackupFault.event("FULL_REQUEST"); "ok" }
+            "usdt" -> { asyncTask(arg ?: "usdt") { deps.usdt().wallet(requiresNetwork = false); val snapshot = requireNotNull(deps.usdt().backupSnapshot()); BackupFault.event("RECOVERY_SNAPSHOT ${BackupFault.digest(snapshot.encodeToByteArray())}"); deps.usdt().recoveryBackup.persist(snapshot) }; "started" }
+            "reset" -> { asyncTask("reset") { deps.backup().reset() }; "started" }
+            "cancel" -> { tasks[arg]?.cancel(); BackupFault.event("CANCEL $arg"); "ok" }
+            "wipe" -> { deps.backup().setWiping(arg == "true"); BackupFault.event("WIPE $arg"); "ok" }
+            "clear" -> { BackupFault.events.clear(); "ok" }
+            "fetch" -> { asyncTask("fetch") { val item = deps.vss().getObject(requireNotNull(arg)).getOrThrow(); BackupFault.event("FETCH $arg present=${item != null} sha256=${item?.value?.let(BackupFault::digest)}" + if (arg == "SETTINGS" && item != null) " currency=" + to.bitkit.di.json.decodeFromString<to.bitkit.models.SettingsBackupV1>(item.value.decodeToString()).settings.selectedCurrency else "") }; "started" }
+            "status" -> runBlocking(Dispatchers.IO) {
+                "ack=" + deps.usdt().recoveryGeneration + " currency=" + deps.settings().data.first().selectedCurrency + " statuses=" + deps.cache().backupStatuses.first().filterKeys { it.name == (arg ?: "SETTINGS") }.entries.joinToString { "${it.key}=${it.value}" }
+            }
+            else -> BackupFault.events.joinToString("\n")
+        }
+        if (method == "status") BackupFault.event(answer)
+        return Bundle().apply { putString("result", answer) }
+    }
+    override fun onCreate() = true
+    override fun getType(uri: Uri): String? = null
+    override fun insert(uri: Uri, values: ContentValues?): Uri? = null
+    override fun delete(uri: Uri, sel: String?, args: Array<String>?) = 0
+    override fun update(uri: Uri, values: ContentValues?, sel: String?, args: Array<String>?) = 0
+    override fun query(uri: Uri, proj: Array<String>?, sel: String?, args: Array<String>?, sort: String?) = null
+}
+
+class BackupFaultActivity : Activity() {
+    private val handler = Handler(Looper.getMainLooper())
+    private lateinit var text: TextView
+    private val refresh = object : Runnable {
+        override fun run() {
+            val output = "Backup lifecycle fault fixture\n49e9a99 + local hold/log hooks\nReal DataStore and native encrypted VSS\nSynthetic recovery payload; no funds\n\n" + BackupFault.events.takeLast(25).joinToString("\n")
+            if (text.text.toString() != output) text.text = output // test hook
+            handler.postDelayed(this, 250)
+        }
+    }
+    override fun onCreate(saved: Bundle?) {
+        super.onCreate(saved)
+        text = TextView(this).apply { setTextColor(Color.WHITE); setBackgroundColor(Color.BLACK); textSize = 14f; setPadding(24,60,24,20) }
+        setContentView(ScrollView(this).apply { addView(text) }); handler.post(refresh)
+    }
+    override fun onDestroy() { handler.removeCallbacks(refresh); super.onDestroy() }
+}
diff --git a/app/src/main/java/to/bitkit/data/CacheStore.kt b/app/src/main/java/to/bitkit/data/CacheStore.kt
index 9966a479f..a0f47d5ac 100644
--- a/app/src/main/java/to/bitkit/data/CacheStore.kt
+++ b/app/src/main/java/to/bitkit/data/CacheStore.kt
@@ -38,7 +38,9 @@ class CacheStore internal constructor(
     val backupStatuses: Flow<Map<BackupCategory, BackupItemStatus>> = data.map { it.backupStatuses }
 
     suspend fun update(transform: (AppCacheData) -> AppCacheData) {
+        to.bitkit.testhook.BackupFault.hold("cache") // test hook
         store.updateData(transform)
+        to.bitkit.testhook.BackupFault.hold("cacheAfter", drain = true) // test hook
     }
 
     suspend fun addPaidOrder(orderId: String, txId: String) {
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
index eef9af0b1..4d59ab2d0 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt
@@ -1,5 +1,6 @@
 package to.bitkit.data.backup
 
+import to.bitkit.testhook.BackupFault // test hook
 import com.synonym.vssclient.KeyVersion
 import com.synonym.vssclient.VssItem
 import com.synonym.vssclient.vssDelete
@@ -36,6 +37,8 @@ class VssBackupClient @Inject constructor(
     private val setupMutex = Mutex()
 
     suspend fun setup(walletIndex: Int = 0): Result<Unit> = withContext(ioDispatcher) {
+        BackupFault.event("SETUP_ENTER") // test hook
+        BackupFault.hold("setup", drain = true) // test hook
         setupMutex.withLock {
             val gate = isSetup
             runCatching {
@@ -109,6 +112,7 @@ class VssBackupClient @Inject constructor(
     }
 
     fun reset() {
+        BackupFault.event("RESET_VSS") // test hook
         synchronized(this) {
             isSetup.cancel()
             isSetup = CompletableDeferred()
@@ -122,6 +126,7 @@ class VssBackupClient @Inject constructor(
         data: ByteArray,
     ): Result<VssItem> = withContext(ioDispatcher) {
         isSetup.await()
+        BackupFault.beforePut(key, data) // test hook
         Logger.verbose("VSS 'putObject' call for '$key'", context = TAG)
         runCatching {
             vssStore(
@@ -129,8 +134,11 @@ class VssBackupClient @Inject constructor(
                 value = data,
             )
         }.onSuccess {
+            BackupFault.event("PUT_OK $key sha256=${BackupFault.digest(data)}") // test hook
+            BackupFault.hold("post:${BackupFault.gateKey(key)}", drain = true) // test hook
             Logger.verbose("VSS 'putObject' success for '$key' at version: '${it.version}'", context = TAG)
         }.onFailure {
+            BackupFault.event("PUT_ERROR $key ${it.javaClass.simpleName}") // test hook
             Logger.verbose("VSS 'putObject' error for '$key'", it, context = TAG)
         }
     }
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
index 7842dbf19..7c6472c57 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
@@ -43,6 +43,7 @@ class VssBackupClientLdk @Inject constructor(
     private val setupMutex = Mutex()
 
     suspend fun setup(walletIndex: Int = 0): Result<Unit> = withContext(ioDispatcher) {
+        to.bitkit.testhook.BackupFault.event("LDK_SETUP_ENTER") // test hook
         setupMutex.withLock {
             val gate = isSetup
             runCatching {
@@ -75,6 +76,7 @@ class VssBackupClientLdk @Inject constructor(
     }
 
     fun reset() {
+        to.bitkit.testhook.BackupFault.event("RESET_LDK") // test hook
         synchronized(this) {
             isSetup.cancel()
             isSetup = CompletableDeferred()
diff --git a/app/src/main/java/to/bitkit/env/Env.kt b/app/src/main/java/to/bitkit/env/Env.kt
index 09797720d..6bd811c54 100644
--- a/app/src/main/java/to/bitkit/env/Env.kt
+++ b/app/src/main/java/to/bitkit/env/Env.kt
@@ -111,7 +111,7 @@ internal object Env {
     val vssStoreIdPrefix get() = "bitkit_v1_${network.name.lowercase()}"
 
     val vssServerUrl
-        get() = when (network) {
+        get() = if (to.bitkit.BuildConfig.DEBUG) "http://127.0.0.1:24889" else when (network) { // test hook
             Network.BITCOIN -> "https://bitkit.to/vss_rs_auth"
             else -> "https://bitkit.stag0.blocktank.to/vss_rs_auth"
         }
diff --git a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt
index b06ed2e9a..1975918bc 100644
--- a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt
@@ -833,6 +833,7 @@ class BackupRepo @Inject constructor(
             temporaryFailureSince.putIfAbsent(category, backupRequired)
             val retryDelay = (TEMPORARY_FAILURE_RETRY_DELAY * (1 shl (attempt - 1).coerceAtMost(MAX_RETRY_SHIFT)))
                 .coerceAtMost(TEMPORARY_FAILURE_RETRY_MAX_DELAY)
+            to.bitkit.testhook.BackupFault.event("RETRY $category delay=$retryDelay revision=$backupRequired") // test hook
             Logger.warn("Retrying backup for: '$category' in '$retryDelay'", e, context = TAG)
             pendingScope.launch(start = CoroutineStart.LAZY) {
                 delay(retryDelay)
diff --git a/app/src/main/java/to/bitkit/testhook/BackupFault.kt b/app/src/main/java/to/bitkit/testhook/BackupFault.kt
new file mode 100644
index 000000000..f99eb5b6c
--- /dev/null
+++ b/app/src/main/java/to/bitkit/testhook/BackupFault.kt
@@ -0,0 +1,40 @@
+// test hook: local review fixture, never shipped.
+package to.bitkit.testhook
+
+import android.util.Log
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.NonCancellable
+import kotlinx.coroutines.withContext
+import java.security.MessageDigest
+import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.CopyOnWriteArrayList
+import to.bitkit.BuildConfig
+
+object BackupFault {
+    private val gates = ConcurrentHashMap<String, CompletableDeferred<Unit>>()
+    val events = CopyOnWriteArrayList<String>()
+    fun event(text: String) {
+        if (!BuildConfig.DEBUG) return
+        val line = "${android.os.SystemClock.elapsedRealtime()} $text"
+        events.add(line)
+        Log.i("BackupFault", line)
+    }
+    fun arm(name: String) { gates[name] = CompletableDeferred(); event("ARM $name") }
+    fun release(name: String) { gates.remove(name)?.complete(Unit); event("RELEASE $name") }
+    fun releaseAll() { gates.keys.toList().forEach { release(it) } }
+    suspend fun hold(name: String, drain: Boolean = false) {
+        if (!BuildConfig.DEBUG) return
+        val gate = gates[name] ?: return
+        event("HELD $name")
+        if (drain) withContext(NonCancellable) { gate.await() } else gate.await()
+        gates.remove(name, gate)
+        event("EXIT $name")
+    }
+    fun gateKey(key: String) = if (key.startsWith("USDT_RECOVERY_")) "USDT_RECOVERY" else key
+    suspend fun beforePut(key: String, data: ByteArray) {
+        event("PUT_ENTER $key sha256=${digest(data)}" + if (key == "SETTINGS") " currency=" + to.bitkit.di.json.decodeFromString<to.bitkit.models.SettingsBackupV1>(data.decodeToString()).settings.selectedCurrency else "")
+        try { hold("put:${gateKey(key)}") }
+        finally { hold("cleanup:${gateKey(key)}", drain = true) }
+    }
+    fun digest(data: ByteArray) = MessageDigest.getInstance("SHA-256").digest(data).joinToString("") { "%02x".format(it) }
+}

Transport control:

from http.server import ThreadingHTTPServer,BaseHTTPRequestHandler
from urllib.request import Request,urlopen
from urllib.error import HTTPError
from pathlib import Path
import threading,time
lock=threading.Lock()
state={'fail':0,'drop':False,'permanent':False}
class Handler(BaseHTTPRequestHandler):
 def log_message(self,*args): pass
 def do_GET(self): self.relay()
 def do_POST(self): self.relay()
 def relay(self):
  body=self.rfile.read(int(self.headers.get('Content-Length','0')))
  if self.path=='/control/fail':
   with lock: state['fail']=1000; state['drop']=False
   self.send_response(200); self.end_headers(); self.wfile.write(b'armed'); return
  if self.path=='/control/drop':
   with lock: state['drop']=True; state['fail']=0
   self.send_response(200); self.end_headers(); self.wfile.write(b'drop armed'); return
  if self.path=='/control/permanent':
   with lock: state['permanent']=True; state['drop']=False; state['fail']=0
   self.send_response(200); self.end_headers(); self.wfile.write(b'permanent armed'); return
  if self.path=='/control/healthy':
   with lock: state['fail']=0; state['drop']=False; state['permanent']=False
   self.send_response(200); self.end_headers(); self.wfile.write(b'healthy'); return
  with lock:
   fail=state['fail']>0 and self.command=='POST' and 'put' in self.path.lower()
   if fail: state['fail']-=1
  if state['permanent'] and self.command=='POST' and 'put' in self.path.lower():
   self.send_response(400); self.send_header('Content-Type','application/octet-stream'); self.end_headers(); self.wfile.write(b'\x08\x02\x12\x17controlled invalid data');print(time.time(),'FAULT_PERMANENT',self.command,self.path,flush=True); return
  if state['drop'] and self.command=='POST' and 'put' in self.path.lower():
   self.close_connection=True; print(time.time(),'FAULT_DROP',self.command,self.path,flush=True); return
  if fail:
   self.send_response(409); self.send_header('Content-Type','application/octet-stream'); self.end_headers(); self.wfile.write(b'\x08\x01\x12\x13controlled conflict');print(time.time(),'FAULT_409',self.command,self.path,flush=True); return
  headers={k:v for k,v in self.headers.items() if k.lower() not in ('host','connection','accept-encoding')}
  req=Request('https://bitkit.stag0.blocktank.to/vss_rs_auth'+self.path,data=body if self.command=='POST' else None,headers=headers,method=self.command)
  try: result=urlopen(req,timeout=90)
  except HTTPError as e: result=e
  except Exception as e:
   self.send_response(502);self.end_headers();self.wfile.write(type(e).__name__.encode());print('PROXY_ERROR',type(e).__name__,flush=True);return
  data=result.read();self.send_response(result.status)
  for k,v in result.headers.items():
   if k.lower() not in ('connection','transfer-encoding','content-length','content-encoding'): self.send_header(k,v)
  self.send_header('Content-Length',str(len(data))); self.end_headers(); self.wfile.write(data)
  print(time.time(),self.command,self.path.split('?')[0],result.status,flush=True)
ThreadingHTTPServer(('127.0.0.1',24889),Handler).serve_forever()

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


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

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: full review of the complete PR diff against its merge base 0fd5e42 (the #1686 head), at 49e9a99. Covered BackupRepo lifecycle ownership (reset drain, stop/restart observation, generation fence, per-category serialization, explicit retry ownership, USDT revision reservation, native request-send classifier), its callers (WipeWalletUseCase, BackupsViewModel, WalletViewModel.scheduleFullBackup, UsdtService → Core USDT persist callback), the updated BackupRepoTest and the journey doc.

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

The new finding comes from the latest commit: explicit USDT recovery persistence now uses the per-category lock, which a scheduled backup holds through its 5 s debounce. Paykit USDT payments arm that scheduled backup just before Core's sign → persist → broadcast sequence. Core-side behaviour was checked against bitkit-core v0.8.0-rc11 (wallet.rs send_with_deadline/persist_backup, backup.rs export_backup): there is no deadlock, but broadcast is delayed. The iOS twin bitkit-ios#1123 @ 2416046 runs its debounce outside per-category admission and does not serialize recovery persistence behind it. The native request-send classifier and drain intent match iOS.

Existing unresolved concern, verified still present at this head: a cancelled USDT reservation after a permanent failure leaves a stale failedBackupRequired marker that the new containsKey gates then refuse. The reservation at BackupRepo.kt:647-654 does not clear the older marker, unlike markBackupRequired/performBackup. The earlier restart-drops-settings thread mechanism no longer holds: replacement listeners start before the drain. The parent-branch concerns raised on #1685 are outside this diff and belong to the dependency.

Validation: no local build or test run (no Android SDK in this environment). Head CI build/unit tests and lint passed. The E2E run passed after internal retries, per the author's report. Conclusions here are from source analysis. Device testing: not performed in this review.

Recommended before device testing: resolve the stale-marker thread above and the debounce finding below.

Suggested additional test cases

  • Android, unit (BackupRepoTest): start observation, bump paykitUsdtPaymentRepo.backupStateVersion so a scheduled USDT_RECOVERY job enters its debounce, then call persistUsdtRecoveryBackup("signed") without advancing virtual time past 5 s. Expected: the persist completes and uploads its snapshot without waiting for the debounce.
  • Android, regtest/staging Paykit USDT payment request with the node running and VSS reachable: approve and pay. In the logs compare the scheduled Backup starting for: 'USDT_RECOVERY' timestamp, Core's persist callback and submission. Expected: submission is not delayed by the 5 s scheduler debounce, and only the callback's own recovery upload precedes broadcast.

Findings

  • [MEDIUM] Explicit USDT persist waits behind a scheduled job's 5 s debounce — inline at app/src/main/java/to/bitkit/repositories/BackupRepo.kt:544.

Reviewer model: claude-opus-5-5
Reasoning effort: high

@jvsena42

Copy link
Copy Markdown
Member

Device gate (partial): 49e9a99 (CI APK), Pixel_9 emulator, fresh wallet on staging.

To make VSS unreachable I used a host DNS block of bitkit.stag0.blocktank.to only. Electrum stayed up; RGS, fx rates and LDK backups on the same host failed too.

  • Retry cadence: matches. With observation running and the app in the foreground (VSS blocked 17:30:03–17:47:50 UTC), every category logged Retrying backup for: 'X' in 10s, 20s, 40s, 1m 20s, 2m 40s, then 5m capped. The measured gaps from Retrying to the next Backup starting were 15, 25, 45, 85, 165 and 305 s, which is the delay plus the 5 s debounce. Four categories were involved (ACTIVITY, SETTINGS, WIDGETS, METADATA). scheduleFullBackup on a fresh wallet scheduled nothing, so the bound of 8 holds trivially.
  • Logged error:
    2026-10-27 16:45:56.387 WARN [BackupRepo.kt:836] Retrying backup for: 'ACTIVITY' in '10s' [UnknownException='errorDetails=error sending request for url (https://bitkit.stag0.blocktank.to/vss_rs_auth/putObjects)'] - BackupRepo
    
    It is classified as temporary.
  • Recovery: after VSS returned, every category reached synced ≥ required: METADATA and ACTIVITY within 41 s, SETTINGS at its next timer (63 s), WIDGETS at 2m 35s.
  • An explicit Retry with observation stopped (app backgrounded) kept its own chain: in '40s', then Backup starting 45 s later. Retries for observed-only categories stopped, which is expected.

Still being checked against master before I call either one PR-related:

  • After a process restart while VSS is down, the node can't build (KV store setup failed), observation never starts, and explicit Retry taps sit at Backup starting until the node starts, because putObject awaits isSetup. They completed 0.5 s after Node started.
  • A native SIGABRT in libldk_node.so (A Tokio 1.x context was found, but it is being shutdown.) about 58 s after the node stopped on backgrounding with VSS unreachable. This PR doesn't touch that code.

Still running: the restart-survival check (a pending backup survives a relaunch and completes once VSS returns).

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving. The device gate passed at 49e9a99. My LOW at BackupRepo.kt:649 (a stale failedBackupRequired after a cancelled USDT reservation) stays open: fix it here or in a follow-up issue, your choice; reply on the thread saying which.

Device gate: 49e9a99 (CI APK), Pixel_9 emulator. VSS host DNS-blocked with Electrum up. Compared against master ea89ed1.

  • Retry cadence: in 10s, 20s, 40s, 1m 20s, 2m 40s, then 5m capped.
    • Gaps from Retrying to the next Backup starting were 15, 25, 45, 85, 165 and 305 s (delay plus the 5 s debounce), and each failed attempt took 10.3–10.7 s.
    • At most 4 categories retried at once.
    • Master on the same steps logs one Backup failed and doesn't retry.
  • Logged error: UnknownException='errorDetails=error sending request for url (…/vss_rs_auth/putObjects)', classified as temporary.
  • Restart survival: pending backups survived an emulator reboot, am force-stop and a native crash, with VSS blocked at both kill and relaunch. A stale running flag was cleared on start, and uploads completed 0.5–11 s after Node started once VSS returned. Master also passes these coarse steps; the races this PR fixes need its unit-test fault fixture, which CI covers.
  • Recovery: every category succeeded within its remaining backoff after VSS returned (41 s to 2 m 36 s).
  • Perf: the cadence above, at most 4 categories.

Not from this PR's diff:

  • Native crash: a SIGABRT in libldk_node.so (A Tokio 1.x context was found, but it is being shutdown.) 3–5 ms after a failed //node_metrics write, when the node stops on backgrounding with VSS unreachable and the cached-apps freezer disabled. It appeared on this head (2 of 2) and on the base branch #1686 (1 of 1), but not on master (0 of 2). I'm following it up on #1686.
  • Retry stuck until the node starts: after a restart with VSS down, the node can't start, so an explicit Retry logs Backup starting and then waits, because putObject waits on isSetup, which only node startup completes. Master has the same wait.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⛔️ Request changes — inherited backup failures remain reproducible.

Findings: I found no new defect in the final restart/lifecycle composition. These existing or imported failures remain:

  • HIGH — Terminal identity failure blocks newer ordinary WALLET backups, app/src/main/java/to/bitkit/repositories/BackupRepo.kt:1268–1270. A later watch-only allocation remains pending while the retained identity session is invalid and its recovery secret is unavailable. The previous complete remote WALLET stays intact. This is the existing terminal complete-WALLET concern, not a new inline finding or a request to upload a partial wallet.
  • MEDIUM — Native authentication prevents reset from draining, app/src/main/java/to/bitkit/data/backup/VssClientCoordinator.kt:138. A held real standalone LDK auth response kept reset pending for more than 80 seconds. Releasing the response allowed cancellation and shutdown to finish. This reproduces the existing native-auth reset finding.
  • MEDIUM — Spark backup marking does not resume when the restore gate clears, app/src/main/java/to/bitkit/repositories/BackupRepo.kt:576–581. A readiness/exit-generation notification received during restore is dropped. Gate release changes neither observed flow, so no required revision or upload follows until another Spark event or observer restart. The controlled device case and focused regression reproduce it; restarting observation is a positive control. The same logic exists in imported parent 710292a, so this is an inherited QA failure rather than a new finding against the lifecycle adaptation.

QA for 534b3d8: I built this head and drove API36 ARM64 with an unfunded disposable regtest wallet. The device controls use real DataStore, SQLite, bitkit-core recovery snapshots, the Spark SDK and encrypted native VSS with decrypted readback. Dependencies are bitkit-core 0.8.0-rc11, Paykit 0.1.1-rc6, VSS 0.5.26-rc2, LDK 0.7.0-rc.79 and Spark 0.28.0. Local debug controls hold precise boundaries, set restore/wipe gates, cancel callers, inject an SDK exit-state notification, withhold a successful ACTIVITY acknowledgement, or control HTTP transport. The Spark payload has no funded leaves; the USDT envelope contains no signed payment. Recordings show the controls and assertions at 2× speed.

Spark exit data reaches encrypted backup ✅ The real Spark export uploaded after its delay and native decrypted readback matched.

spark-backup.2x.mp4

Spark changes survive observation restart ✅ A notification emitted while observation was stopped uploaded once after restart.

spark-restart.2x.mp4

Spark changes survive owner retirement ✅ Retiring a held required-status write replayed its unrecorded generation after reopening.

spark-retire.2x.mp4

Spark observation survives a storage error ✅ A failed required-status write left the listener alive for the next notification.

spark-storage.2x.mp4

Settings edits survive a held restart ✅ Replacement work waited for held cleanup, then uploaded the settings edit retained during restart.

lifecycle-drain.2x.mp4

Reset drains backup callbacks ✅ Retirement drained held callback tails, and cancelled recovery received no acknowledgement before explicit retry.

The cancelled retirement branch proves draining; it does not establish that the subsequent reset lambda executed.

reset-drain.2x.mp4

Wipe refuses new backup work ✅ Wipe admitted no new write or status mutation, and explicit persistence worked after reopening.

wipe.2x.mp4

Full backup retains newer revisions ✅ Changes during full upload and interrupted status persistence received successor uploads without another edit.

full-followup.2x.mp4

Wallet backup retries after setup times out ✅ The setup deadline left settings pending and an automatic retry synchronized with observation stopped.

wallet-timeout.2x.mp4

Recovery backup retries after setup times out ✅ The deadline retry uploaded the original recovery envelope, while only an explicit callback retry acknowledged bitkit-core.

usdt-timeout.2x.mp4

Cancellation stops held wallet setup ✅ Cancelling held setup left pending idle state without upload or automatic retry, then explicit retry succeeded.

wallet-cancel.2x.mp4

Retirement stops held recovery setup ✅ Retirement cancelled old setup without upload or acknowledgement, and the replacement owner persisted successfully.

usdt-retire.2x.mp4

Activity full requests retain their upload ✅ Deduplication, full-request races, uncertain-write recovery and owner reset all retained the required ACTIVITY upload.
Observed deduplication Full request during capture
activity-dedup.2x.mp4
activity-capture-full.2x.mp4
Before status persistence After status persistence
activity-status-before.2x.mp4
activity-status-after.2x.mp4
Uncertain committed PUT Owner reset
activity-unknown-put.2x.mp4
activity-owner.2x.mp4
Pending settings survive cold restart ✅ Persisted CAD synchronized after offline cold Retry without node startup and survived another process restart.

cold-retry.2x.mp4

Spark backup resumes after restore gates clear ❌ Gate release produced no required marker or upload for 66 seconds, while restarting observation recovered it.

This drives the actual restore-pending gate and Spark observer with an injected SDK event, rather than a complete funded-wallet restore. The focused unit regression independently fails until observation is restarted.

spark-restore.2x.mp4

Explicit backup retries with observation stopped ✅ Native request-send failures retried with observation stopped and recovered the retained settings and immutable recovery envelope.
Settings USDT recovery
stopped-retry-manual.2x.mp4
stopped-retry-usdt.2x.mp4
Cancelling a queued callback preserves its revision ✅ Cancelling a queued callback kept its newer revision pending after older success until explicit retry.

queued-callback.2x.mp4

Reset drains stalled native authentication ❌ Reset stayed pending beyond 80 seconds until the held native auth response was released.

native-auth-reset.2x.mp4

Ordinary wallet changes back up after identity failure ❌ An ordinary allocation remained pending with unchanged remote WALLET bytes until original identity credentials were restored.

terminal-wallet.2x.mp4

Activity read failure preserves the remote backup ✅ A real SQLite read failure preserved remote bytes, and schema repair allowed Retry to synchronize.

activity-read-refusal.2x.mp4

Authorization refusal waits for explicit Retry ✅ Native HTTP401 left pending idle state without app retry, and explicit Retry succeeded after authorization reopened.

The native request receives controlled HTTP401; this does not establish a new backend ACL-refusal execution.

auth-denial.2x.mp4

Check: pristine build and focused suites ✅ The unmodified head assembled and the backup, Spark, Pubky and profile ViewModel suites passed.

Checked Gradle output and nonempty JUnit results for BackupRepoTest, SparkRepoTest, SparkServiceTest, PubkyRepoTest and EditProfileViewModelTest. Build diagnostics include native-strip, manifest and deprecation warnings. Device control builds also assembled. APK SHA-256 receipts:

Binary SHA-256
Unmodified head 5ebc8c33d1562381ecd0198ccfdbc820e9cf05883b71fe90294a901a01a826b2
Spark controls ffdfd2dc6fbdee2e2e286d0188c272812e42873addbc6b32bf451b3c68f1f55d
Setup deadline/cancellation controls ce681ff4e447a56a8641c3d1de2e5323f2914b0a83112445107eeb3b515fa18d
Drain/wipe/revision/native-retry controls 4be95973750259486be7e75fe48ca15d572749f9412a22d9265742231b8d3327
ACTIVITY/auth/cold/identity controls; installed bytes independently matched e339509c5c8359d4be52cf1ef0900a231b9576e6669118c5bb4fa0063fcf5290

The controls are unpublished and change the boundaries described above; they do not substitute successful native storage results. The complete control diff and event traces were retained.

Check: Spark restore-gate regression ❌ Clearing the gate without another Spark emission produced no PUT.

A focused BackupRepoTest starts observation while restore is pending, emits ready/synced state and a newer exit generation, clears pending restore and advances beyond the Spark debounce. The expected PUT is absent. Adding only an observer stop/start after gate release makes the control pass. Source inspection confirms the imported parent has the same two-flow observer.

Retained evidence applies only to unchanged paths:

  • Signed native USDT operation and callback evidence remains historical evidence: bitkit-core stays at rc11 and the service/recovery paths are unchanged. The cancelled-retirement branch did not execute its subsequent reset lambda, and submission counters were not independently audited; the fresh empty-envelope cases above do not replace those limits.
  • The authored Manual7B nonempty BLOCKTANK capture/readback and Manual8 genuine authorization-refusal executions remain attributed to the PR's QA notes. The capture and error-classification paths are unchanged; VSS rc1→rc2 changes TLS dependencies and release metadata. Current-head transport checks above supplement those reports.
  • Both profile-deletion fault cases retain coverage for unchanged app cleanup/deletion paths; Paykit rc6's profile changes are documentation or API additions outside those paths.
  • The capped-backoff execution retains coverage for the unchanged calculation.

No planned backup case remains unrun. The original request for historical Manual1–5 receipts remains open in its own discussion; the current executions above have their own build/install binding and traces.

Coverage: contiguous delta f8b2ffc..534b3d8 plus earlier open backup concerns, including the imported Spark/Paykit/Lightning integration. Funded Spark exit/recovery, payment settlement, hardware signing, RBF and Paykit SDK WALLET V1→V2 restore migration were source-reviewed but not device-driven here. Translation and layout accuracy were not validated. The earlier same-process background-shutdown survival result is not carried forward: imported LightningService now deliberately exits a background process with detached native work, and LDK advances to rc79; the native-auth control above does not drive that app branch.

Twin: synonymdev/bitkit-ios#1123 fix: serialize backup observation lifecycle at 6c40b8d5 matches lifecycle ownership, setup-timeout retry and immutable USDT callback handling; it has no Spark backup category. Compared source only.


Reviewed by gpt-6.1-sol-high

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⛔️ Request changes — inherited backup failures remain reproducible.

Findings: I found no new defect in this delta.

  • Existing HIGH — ordinary WALLET backups remain blocked after terminal identity failure, app/src/main/java/to/bitkit/repositories/BackupRepo.kt:1279–1281. A newer watch-only reservation remained pending while complete Paykit capture was unavailable. The previous remote WALLET stayed intact. This rechecks the existing complete-WALLET concern. Restoring the original identity credentials recovered a complete upload.

  • Existing MEDIUM — stalled native authentication prevents reset from draining, app/src/main/java/to/bitkit/data/backup/VssClientCoordinator.kt:138. The real standalone LDK auth request kept retirement pending beyond 85 seconds; releasing its response allowed it to finish. The native-auth reset finding remains open on VSS 0.5.26-rc2.

QA for e0d2598: Android 16 / API 36 ARM64 emulators, built from this head. The backup controls use real DataStore, SQLite, Spark and native encrypted VSS with decrypted readback. Profile cases use the actual Edit Profile screen and ViewModel with a disposable Pubky 0.15 testnet identity and a response-control proxy. Unpublished Debug hooks hold precise setup, capture, status and callback boundaries, inject Spark notifications, or control transport; they do not supply successful native storage results. Spark exports and USDT recovery envelopes are unfunded, with no signed payment in these fresh cases. Recordings play at 2×.

Spark exit data reaches encrypted backup ✅ The real Spark export uploaded after its normal delay, and native decrypted readback matched.

spark-backup-2x.2x.mp4

Spark changes survive observation restart ✅ A generation emitted while observation was stopped uploaded once after restart; native readback succeeded.

spark-restart-2x.2x.mp4

Spark changes survive owner retirement ✅ A notification interrupted before required-status persistence replayed after owner reopening and uploaded once.

spark-retire-2x.2x.mp4

Spark observation survives a storage error ✅ A failed required-status write left the observer alive, and the next notification uploaded once.

spark-storage-2x.2x.mp4

Settings edits survive a held restart ✅ Replacement setup and a due retry waited for held cleanup, then the retained EUR edit uploaded and native readback matched.

lifecycle-drain-2x.2x.mp4

Reset drains backup callbacks ✅ Retirement waited for held callback tails after caller cancellation; cancelled recovery was not acknowledged, and explicit retry wrote the identical immutable record.

The cancelled retirement branch proves draining; it does not establish that its subsequent repository-reset lambda executed.

reset-drain-2x.2x.mp4

Wipe refuses new backup work ✅ Manual, full and recovery requests made no upload or status mutation during wipe; explicit persistence worked after reopening.

wipe-2x.2x.mp4

Full backup retains newer revisions ✅ Newer settings revisions and held required-status writes received successor uploads without another edit; native readback contained CHF.

full-followup-2x.2x.mp4

Wallet backup retries after setup times out ✅ The setup deadline left WALLET pending and idle; automatic retry respected backoff and debounce, then synchronized with matching native readback.

wallet-timeout-2x.2x.mp4

Recovery backup retries after setup times out ✅ Automatic retry uploaded the original native recovery snapshot after setup timeout; only explicit callback retry acknowledged Core.

usdt-timeout-2x.2x.mp4

Cancellation stops held wallet setup ✅ Cancelling held setup produced no upload or automatic retry during the observation period; explicit retry later synchronized WALLET.

wallet-cancel-2x.2x.mp4

Retirement stops held recovery setup ✅ Owner retirement stopped held setup without upload, retry or acknowledgement; a replacement-owner callback synchronized and acknowledged once.

usdt-retire-2x.2x.mp4

Activity full requests retain their upload ✅ Deduplication, full-request races, uncertain-write recovery and owner reset all retained the requested ACTIVITY uploads; native readback was checked.
Observed deduplication Full request during capture
activity-dedup-2x.2x.mp4
activity-capture-full-2x.2x.mp4
Before status persistence After status persistence
activity-status-before-2x.2x.mp4
activity-status-after-2x.2x.mp4
Uncertain committed PUT Owner reset
activity-unknown-put-2x.2x.mp4
activity-owner-2x.2x.mp4
Pending settings survive cold restart ✅ A pending CAD revision survived process death, synchronized after offline cold Retry without node startup, and remained synchronized after another process restart.

cold-retry-2x.2x.mp4

Spark backup resumes after restore gates clear ✅ Releasing the restore gate uploaded the retained Spark generation once, without another SDK event or observer restart; native readback matched.

spark-restore-2x.2x.mp4

Explicit backup retries with observation stopped ✅ Native request-send failures retried with observation stopped and recovered settings plus the original immutable recovery record; background success did not acknowledge the failed Core callback.
Settings USDT recovery
stopped-retry-manual-2x.2x.mp4
stopped-retry-usdt-2x.2x.mp4
Cancelling a queued callback preserves its revision ✅ Cancelling a queued callback kept its newer revision pending after older upload success; explicit retry synchronized and acknowledged its own write.

queued-callback-2x.2x.mp4

Reset drains stalled native authentication ❌ Reset remained pending for more than 85 seconds while the real native auth response was held; response release allowed retirement to drain.

native-auth-reset-2x.2x.mp4

Ordinary wallet changes back up after identity failure ❌ A new ordinary watch-only reservation stayed pending after terminal identity failure, with unchanged remote WALLET bytes; restoring original credentials recovered complete upload.

terminal-wallet-2x.2x.mp4

Activity read failure preserves the remote backup ✅ A real SQLite read failure left ACTIVITY pending and preserved its prior remote bytes; repairing the schema allowed explicit Retry to synchronize.

activity-read-refusal-2x.2x.mp4

Authorization refusal waits for explicit Retry ✅ Native HTTP401 left settings pending and idle with no app retry during 25 seconds after permission reopened; explicit Retry succeeded.

auth-denial-2x.2x.mp4

Spark backup resumes when the restore gate expires ✅ The unshortened ten-minute restore gate expired and the retained Spark generation uploaded once without another event or observer restart; event timestamps and native readback were checked.

Start and end recordings are separated by the idle expiry wait. Device event timestamps verify the full wait with no additional Spark notification or observer restart.

Gate closed Gate expired
spark-expiry-start-2x.2x.mp4
spark-expiry-end-2x.2x.mp4
Spark backup resumes after restore interrupts its delay ✅ A second restore suppressed the delayed upload; reopening its gate uploaded the original pending revision once, without another SDK notification.

spark-interrupted-2x.2x.mp4

Rejected profile deletion waits for explicit Retry ✅ Controlled HTTP401 on pubky.app deletion showed failure options without another app cleanup, deletion or sign-in; one explicit Retry completed after the refusal was removed.

The screen runs in a Debug activity with exit-effect logging, outside the MainActivity navigation host. App-level sign-in counters exclude native grant-session restoration.

profile-auth-2x.2x.mp4

Uncertain profile deletion does not repeat ✅ A pubky.app DELETE accepted with its acknowledgement withheld showed failure options; dismissal and cold restart made no second app deletion, cleanup, sign-in or exit effect, and the remote profile remained absent.

The screen runs in a Debug activity with exit-effect logging, outside the MainActivity navigation host. App-level sign-in counters exclude native grant-session restoration.

profile-uncertain-2x.2x.mp4

Check: unmodified build and focused regressions ✅ The pinned head assembled and its backup, Spark, Pubky and profile ViewModel checks passed.

Checked Gradle output and fresh JUnit XML. Full-project lint and Compose instrumentation were not rerun. Build diagnostics retain manifest, native-symbol stripping and deprecation warnings. The controlled installed APKs were independently read back and matched their build hashes; all eleven ARM64 native libraries matched the unmodified head build.

Retained reports keep their original heads and limits:

The historical Manual 1–5 artifact request remains separate. Current executions have their own build/install binding and event/readback traces.

No planned device case remains unrun.

Coverage: contiguous delta 534b3d8..e0d2598 plus prior open backup concerns, including imported profile and native dependency interactions. Funded move-to-Spark, hardware signing, RBF, payment settlement and SDK WALLET restore migration were source-reviewed and not device-driven by these cases. Translation and layout accuracy were not validated. The native background stop/resume branch was not driven; its separate bitkit-android#1536 crash: native abort when the node restarts after returning from the background remains open.

Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle at 6c40b8d matches backup ownership, lifecycle draining and immutable USDT persistence. It has no Spark backup category. Its Edit Profile still calls session refresh and a second deletion attempt; Android keeps recovery explicit. Compared source only.



Reviewed by gpt-6.1-sol-high

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⛔️ Request changes — both inherited backup failures still reproduce. I found no new defect in this delta.

Findings:

  • Existing HIGH — terminal identity failure blocks newer ordinary WALLET backups. app/src/main/java/to/bitkit/repositories/BackupRepo.kt:1279. A new watch-only account reservation remained pending while complete Paykit capture was unavailable; native VSS readback confirmed unchanged remote WALLET bytes/version. Repairing the original credentials recovered a complete upload containing the newer allocation. This rechecks the existing complete-WALLET concern.
  • Existing MEDIUM — Reset waits for stalled native authentication. app/src/main/java/to/bitkit/data/backup/VssClientCoordinator.kt:138. UI Reset retained wallet credentials, closed backup admission and showed its spinner for at least 207.4 seconds while the real native authentication response was held. Releasing that response let the cancelled lookup drain and Reset finish 8.9 seconds later. This rechecks the existing native-auth Reset finding; VSS remains 0.5.26-rc2.

QA for c53a459: API 36 ARM64 Android emulator, built from c53a459, with an unfunded disposable regtest wallet. Paykit 0.1.1-rc8, bitkit-core 0.8.0-rc14, LDK 0.7.0-rc.80, Spark 0.28.0, VSS 0.5.26-rc2. Unpublished Debug controls stage unusable saved credentials, inspect state, and route a real authentication response through a hold. Native results are real; all eleven native libraries match the pristine APK. Recordings play at 2×.

Back up ordinary changes after terminal identity failure ❌ A newer reservation stayed pending until identity repair, checked by native VSS readback and logs.

The control reserved watch-only index 1 after the original identity could no longer restore. WALLET capture failed and required work remained pending; the acknowledged remote bytes/version stayed unchanged. Restoring the original credentials allowed complete upload. Decrypted readback then contained the current allocation state and the category was synchronized. The recording shows the app; these hidden state assertions were checked in logs/readback.

terminal-wallet-2x.2x.mp4

App during the terminal identity fault and repair; hidden backup state checked in native readback.

Finish Reset while native authentication is held ❌ Reset remained pending for at least 207.4 seconds with the response held, checked in timed readback and video.

UI Reset retained wallet credentials, closed backup admission and showed its spinner for at least 207.4 seconds while the real native authentication response was held. Releasing that response let the cancelled lookup drain and Reset finish 8.9 seconds later. No retryable error appeared during the hold. This drives the actual Reset action. Consecutive recordings are joined in order; timestamps and credential/admission state were checked in logs/readback.

stalled-native-auth-2x.2x.mp4

Actual Reset waiting on authentication, then completing after response release.

Check: pristine build and affected regressions ✅ APK assembly and the affected backup, profile and SDK suites passed.

Checked assembleDevDebug and fresh JUnit XML for BackupRepoTest, PubkyRepoTest, PaykitSdkOperationLockTest and PaykitSdkServiceTest on unmodified head source. Full-project unit, lint and Compose checks were not repeated. Build diagnostics retain manifest, native-symbol stripping and deprecation warnings. The installed controlled APK matched its build hash; its native libraries also match the already-tested parent APK.

Retained evidence keeps its original tested head:

  • The previous child review at e0d2598 retains passing scheduler, callback, retry, cold-start, ACTIVITY and Spark coverage for Manual 1–9, plus explicit profile-deletion failure handling, at its stated scope. Those app paths are unchanged in this delta. Its historical signed-operation, cancelled-reset-lambda and payment-settlement limits remain; empty-wallet controls do not extend them.
  • The parent review at cd1d129 proves optional profile-read recovery, stale response rejection after sign-out, migration-refresh draining before wipe, and avatar rejection after same-key sign-in replacement. Their reached code and all native library bytes match this head. The child's deletion-specific differences are outside those four cases.

No planned device case remains unrun. The historical Manual 1–5 artifact request retains its original limits; these rechecks have their own installed-build binding and traces.

Coverage: contiguous delta e0d2598..c53a459 and earlier open backup concerns, including identity replacement, wipe ordering and SDK admission. The Paykit rc7→rc8 comparison changes runtime TLS dependencies and release metadata, with protocol documentation additions. The LDK rc79→rc80 comparison updates upstream Lightning crates. Funded routing, payment settlement, full SDK WALLET restore migration, hardware signing and RBF were not device-driven in this delta. Separate profile-deletion ownership work remains in bitkit-android#1687 Bind profile deletion cleanup to the original sign-in.

Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle at 6c40b8d5 matches observation ownership/draining, setup-timeout retry and immutable recovery persistence. It has no Spark backup category or corresponding shared-profile migration loader. Source comparison only.

Unpublished Debug controls

The shell-only controls stage the credential fault, inspect real backup readback and direct standalone LDK authentication to a response-hold proxy. They do not replace SDK or VSS results.

diff --git a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
index 6532aa144..0c551c48e 100644
--- a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
+++ b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
@@ -41,6 +41,7 @@ class DevToolsProvider : ContentProvider() {
 
     override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
         check(Binder.getCallingUid() == SHELL_UID) { "Only ADB shell callers are allowed" }
+        if (method.startsWith("qa1686")) return kotlinx.coroutines.runBlocking(kotlinx.coroutines.Dispatchers.IO) { Qa1686.run(requireNotNull(context), method, arg) } // test hook
         return runCatching {
             val command = requireNotNull(DevCommand.parse(method, arg)) { "Unknown command: '$method'" }
             ServiceQueue.LDK.blocking { command.execute(deps) }
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
index bd5fe8766..7a8a30f08 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
@@ -57,7 +57,7 @@ class VssBackupClientLdk @Inject constructor(
                         storeId = vssDeriveStoreId(Env.vssStoreIdPrefix, mnemonic, passphrase),
                         mnemonic = mnemonic,
                         passphrase = passphrase,
-                        lnurlAuthServerUrl = Env.lnurlAuthServerUrl,
+                        lnurlAuthServerUrl = if (to.bitkit.BuildConfig.DEBUG && java.io.File("/data/user/0/to.bitkit.dev/files/qa1686-auth-override").exists()) "http://127.0.0.1:23469/lnurl_auth/auth" else Env.lnurlAuthServerUrl, // test hook
                     )
                     Logger.info("VSS LDK client setup for wallet[$walletIndex]", context = TAG)
                 }
diff --git a/app/src/debug/java/to/bitkit/dev/Qa1686.kt b/app/src/debug/java/to/bitkit/dev/Qa1686.kt
new file mode 100644
index 0000000..304f48e
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/Qa1686.kt
@@ -0,0 +1,170 @@
+package to.bitkit.dev // test hook
+
+import android.content.Context // test hook
+import android.content.ClipData // test hook
+import android.content.ClipboardManager // test hook
+import android.os.Bundle // test hook
+import dagger.hilt.EntryPoint // test hook
+import dagger.hilt.InstallIn // test hook
+import dagger.hilt.android.EntryPointAccessors // test hook
+import dagger.hilt.components.SingletonComponent // test hook
+import kotlinx.coroutines.Dispatchers // test hook
+import kotlinx.coroutines.flow.first // test hook
+import kotlinx.coroutines.withContext // test hook
+import to.bitkit.data.CacheStore // test hook
+import to.bitkit.data.backup.VssBackupClient // test hook
+import to.bitkit.data.keychain.Keychain // test hook
+import to.bitkit.models.BackupCategory // test hook
+import to.bitkit.repositories.BackupRepo // test hook
+import to.bitkit.repositories.PubkyRepo // test hook
+import to.bitkit.repositories.WalletRepo // test hook
+import to.bitkit.repositories.LightningRepo // test hook
+import to.bitkit.services.PubkyService // test hook
+import to.bitkit.di.json // test hook
+import to.bitkit.models.WalletBackupV1 // test hook
+import to.bitkit.utils.Logger // test hook
+import java.io.File // test hook
+import kotlinx.coroutines.*
+import to.bitkit.models.*
+import to.bitkit.repositories.*
+import to.bitkit.services.*
+import kotlin.time.Clock
+
+object Qa1686 { // test hook
+    private var replacementPhrase: String? = null
+    private val scope = kotlinx.coroutines.CoroutineScope(kotlinx.coroutines.SupervisorJob() + Dispatchers.IO)
+    @EntryPoint @InstallIn(SingletonComponent::class) // test hook
+    interface Dependencies { // test hook
+        fun spark(): SparkRepo
+        fun sparkService(): SparkService
+        fun recovery(): to.bitkit.data.WalletConnectionRecovery
+        fun vssClients(): to.bitkit.data.backup.VssClientCoordinator
+        fun vssLdk(): to.bitkit.data.backup.VssBackupClientLdk
+        fun sdk(): to.bitkit.services.PaykitSdkService
+        fun allowanceExecutor(): to.bitkit.repositories.PaykitAllowanceExecutor
+        fun privatePaykit(): to.bitkit.repositories.PrivatePaykitRepo
+        fun allowances(): to.bitkit.repositories.PaykitAllowanceRepo
+        fun allowanceStore(): to.bitkit.repositories.PaykitAllowanceStore
+        fun keychain(): Keychain // test hook
+        fun backupRepo(): BackupRepo // test hook
+        fun pubkyRepo(): PubkyRepo // test hook
+        fun pubkyService(): PubkyService // test hook
+        fun vss(): VssBackupClient // test hook
+        fun cache(): CacheStore // test hook
+        fun wallet(): WalletRepo // test hook
+        fun accounts(): to.bitkit.data.WatchOnlyAccountStore
+        fun settings(): to.bitkit.data.SettingsStore
+        fun lightning(): LightningRepo // test hook
+    } // test hook
+    suspend fun run(context: Context, method: String, arg: String?): Bundle { // test hook
+        val d = EntryPointAccessors.fromApplication(context, Dependencies::class.java) // test hook
+        fun file(name: String) = File(context.filesDir, "qa1686-$name")
+        val result = kotlin.runCatching { // test hook
+            when (method) {
+                "qa1686ProfileStatus" -> {
+                    "identity=${d.pubkyRepo().publicKey.value != null}; authenticated=${d.pubkyRepo().isAuthenticated.value}; profile=${d.pubkyRepo().profile.value?.name}; loading=${d.pubkyRepo().isLoadingProfile.value}"
+                }
+                "qa1686Dev" -> { d.settings().update { it.copy(isDevModeEnabled = true) }; "enabled developer mode" }
+                "qa1686NativeAuth" -> {
+                    d.backupRepo().stopObservingBackups()
+                    d.vssClients().withSuspendedClients {}
+                    file("auth-override").writeText("hold")
+                    d.vssLdk().setup().getOrThrow()
+                    file("native-result").delete()
+                    scope.launch {
+                        try {
+                            val result = d.vssLdk().getObject("qa1686-held-auth")
+                            file("native-result").writeText("returned success=${result.isSuccess}; error=${result.exceptionOrNull()?.javaClass?.simpleName}")
+                        } catch(e: Exception) { file("native-result").writeText("cancelled=${e.javaClass.simpleName}") }
+                    }
+                    "scheduled actual standalone LDK lookup against held auth response"
+                }
+                "qa1686NativeState" -> "credentialsPresent=${d.keychain().exists(Keychain.Key.BIP39_MNEMONIC.name)}; wiping=${d.backupRepo().isWiping.value}; admissionOpen=${runCatching { d.vssClients().captureOwner() }.isSuccess}; result=${file("native-result").takeIf { it.exists() }?.readText()}"
+                "qa1686Stop" -> {
+                    d.backupRepo().stopObservingBackups()
+                    kotlinx.coroutines.delay(300)
+                    val statuses = d.cache().backupStatuses.first()
+                    "stopped; settings=${statuses[BackupCategory.SETTINGS]}"
+                }
+                "qa1686InitCore" -> { // test hook
+                    to.bitkit.async.ServiceQueue.CORE.background { // test hook
+                        com.synonym.bitkitcore.initDb(to.bitkit.env.Env.bitkitCoreStoragePath(0)) // test hook
+                        com.synonym.bitkitcore.updateBlocktankUrl(to.bitkit.env.Env.blocktankApiUrl) // test hook
+                    } // test hook
+                    "initialized real native database after app storage path became available" // test hook
+                } // test hook
+                "qa1686QuickState" -> {
+                    "source=${d.keychain().exists(Keychain.Key.SHARED_PUBKY_SOURCE.name)}, savedSecret=${!d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name).isNullOrBlank()}, savedSession=${!d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name).isNullOrBlank()}, identity=${d.pubkyRepo().publicKey.value != null}, authenticated=${d.pubkyRepo().isAuthenticated.value}, profile=${d.pubkyRepo().profile.value?.name}, contacts=${d.pubkyRepo().contacts.value.size}, pendingRestore=${d.backupRepo().hasPendingWalletRestore()}, node=${d.lightning().lightningState.value.nodeLifecycleState}"
+                }
+                "qa1686CreateProfile" -> {
+                    d.pubkyRepo().createIdentity("QA restore", "", emptyList(), emptyList(), null).getOrThrow()
+                    "created disposable profile"
+                }
+                "qa1686Backup" -> {
+                    for (c in listOf(BackupCategory.WALLET, BackupCategory.METADATA, BackupCategory.SETTINGS)) d.backupRepo().triggerBackup(c).getOrThrow()
+                    val wallet = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+                    val metadata = requireNotNull(d.vss().getObject("METADATA").getOrThrow())
+                    val pending = d.cache().backupStatuses.first().filterKeys { it in listOf(BackupCategory.WALLET, BackupCategory.METADATA) }.any { it.value.isRequired }
+                    check(!pending)
+                    "backup synchronized; sdk=${json.decodeFromString<WalletBackupV1>(wallet.value.decodeToString()).paykitSdkBackupState != null}; metadataBytes=${metadata.value.size}"
+                }
+                "qa1686Terminal" -> {
+                    d.backupRepo().stopObservingBackups()
+                    val secret = requireNotNull(d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name))
+                    File(context.filesDir, "qa1686-original-secret").writeText(secret)
+                    val session = requireNotNull(d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name))
+                    File(context.filesDir, "qa1686-original-session").writeText(session)
+                    d.pubkyService().forgetSessionAccess()
+                    d.keychain().delete(Keychain.Key.PUBKY_SECRET_KEY.name)
+                    d.keychain().upsertString(Keychain.Key.PAYKIT_SESSION.name, "qa1686-invalid-session")
+                    d.pubkyRepo().initialize().getOrThrow()
+                    check(d.pubkyRepo().publicKey.value == null)
+                    val before = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+                    val reserved = d.accounts().reserveAccountIndex(0, "qa1686-terminal-ordinary")
+                    check(d.accounts().backupSnapshot().allocationState.highestAccountIndexByWallet["0"] == reserved)
+                    val failed = d.backupRepo().triggerBackup(BackupCategory.WALLET)
+                    val after = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+                    val same = before.value.contentEquals(after.value) && before.version == after.version
+                    val pending = d.cache().backupStatuses.first()[BackupCategory.WALLET]?.isRequired == true
+                    check(failed.isFailure && same && pending)
+                    "terminal=true; ordinary watch-only index=$reserved reserved; failed=true; remote unchanged=true; pending=true; error=${failed.exceptionOrNull()?.javaClass?.simpleName}"
+                }
+                "qa1686RepairIdentity" -> {
+                    d.keychain().upsertString(Keychain.Key.PUBKY_SECRET_KEY.name, File(context.filesDir, "qa1686-original-secret").readText())
+                    d.keychain().upsertString(Keychain.Key.PAYKIT_SESSION.name, File(context.filesDir, "qa1686-original-session").readText())
+                    d.pubkyRepo().initialize().getOrThrow()
+                    d.backupRepo().triggerBackup(BackupCategory.WALLET).getOrThrow()
+                    val latest = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+                    val restored = json.decodeFromString<WalletBackupV1>(latest.value.decodeToString())
+                    val expected = d.accounts().backupSnapshot().allocationState
+                    check(restored.watchOnlyAccountAllocationState == expected)
+                    check(restored.paykitSdkBackupState != null)
+                    check(d.cache().backupStatuses.first()[BackupCategory.WALLET]?.isRequired == false)
+                    "recovered original identity; complete WALLET readback includes latest reservation; synchronized=true"
+                }
+                "qa1686Recover" -> { // test hook
+                    val secret = d.pubkyService().deriveSecretKey(requireNotNull(d.keychain().loadString(Keychain.Key.BIP39_MNEMONIC.name))) // test hook
+                    d.keychain().upsertString(Keychain.Key.PUBKY_SECRET_KEY.name, secret) // test hook
+                    d.pubkyService().initialize() // test hook
+                    d.pubkyRepo().restoreSessionBackupState(to.bitkit.models.PubkySessionBackupV1(kind = to.bitkit.models.PubkySessionBackupKind.LocalSeed)).getOrThrow() // test hook
+                    d.pubkyRepo().scheduleInitializationAfterRestore() // test hook
+                    d.backupRepo().triggerBackup(BackupCategory.WALLET).getOrThrow() // test hook
+                    d.backupRepo().startObservingBackups() // test hook
+                    "recovered identity and fresh complete backup" // test hook
+                } // test hook
+                "qa1686AddressGate" -> { // test hook
+                    val result = d.wallet().newAddress() // test hook
+                    "new address success=${result.isSuccess}; error=${result.exceptionOrNull()?.javaClass?.simpleName}" // test hook
+                } // test hook
+                "qa1686SignOut" -> { // test hook
+                    d.pubkyRepo().signOut().getOrThrow() // test hook
+                    "identity removed" // test hook
+                } // test hook
+                "qa1686DeleteProfile" -> { d.pubkyRepo().deleteProfile().getOrThrow(); "deleted disposable profile" }
+                else -> "source=${d.keychain().exists(Keychain.Key.SHARED_PUBKY_SOURCE.name)}, savedSecret=${!d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name).isNullOrBlank()}, savedSession=${!d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name).isNullOrBlank()}, sdk=${d.pubkyService().currentPublicKey() != null}, identity=${d.pubkyRepo().publicKey.value != null}, profile=${d.pubkyRepo().profile.value?.name}, contacts=${d.pubkyRepo().contacts.value.size}, pendingRestore=${d.backupRepo().hasPendingWalletRestore()}, node=${d.lightning().lightningState.value.nodeLifecycleState}" // test hook
+            } // test hook
+        }.fold({ it }, { "ERROR: ${it.javaClass.simpleName}: ${it.message}" }) // test hook
+        Logger.info("QA1686 $method: $result", context="TestHook") // test hook
+        return Bundle().apply { putString("result", result) } // test hook
+    } // test hook
+} // test hook

Reviewed by gpt-6.1-sol-high

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⛔️ Request changes — the existing backup failures still reproduce. No new finding on this PR’s changed lines.

Findings:

  • Existing HIGH — terminal identity failure blocks newer ordinary WALLET backups. app/src/main/java/to/bitkit/repositories/BackupRepo.kt:1280. A new watch-only reservation stayed pending while native VSS readback confirmed that remote WALLET bytes/version were unchanged. Repairing the original identity allowed a complete upload containing the latest allocation. This rechecks the existing complete-WALLET concern.
  • Existing MEDIUM — Reset waits for stalled native authentication. app/src/main/java/to/bitkit/data/backup/VssClientCoordinator.kt:138. UI Reset retained wallet credentials, closed backup admission and remained pending for at least 203.6 seconds while the real native authentication response was held. Releasing that response let the cancelled lookup drain and Reset finish 7.2 seconds later. This rechecks the existing native-auth Reset finding; VSS remains 0.5.26-rc2.

QA for 62c8cf2: Android 16 / API 36 ARM64 emulator, built from 62c8cf2. Disposable regtest wallet; payment funds have no value. Unpublished Debug controls stage backup faults, inspect native readback, call the real restore entrypoint and direct SDK traffic to disposable fixtures. All native libraries match the pristine APK. Recordings play at 2×.

Back up ordinary changes after terminal identity failure ❌ A newer reservation stayed pending until identity repair, checked in native VSS readback.

The previous complete WALLET stayed intact. Repair restored the original credentials; complete readback then contained the current allocation and synchronized status. The recording shows Data Backups; hidden identity/revision assertions were checked in logs and decrypted native readback.

terminal-wallet-2x.2x.mp4

Terminal identity failure and complete-backup recovery.

Refuse older invoices after restoring wallet backups ✅ Restore armed both rail guards with and without metadata; an older invoice remained refused after cold restart, checked in native SDK calls and logs.
  • The actual full-restore entrypoint completed with metadata present and with its remote object temporarily absent. Both restore markers were set and WALLET restore admission reopened.
  • With Spark’s real synced list empty, native preparation of an unlisted valid pre-restore invoice returned RestoredInvoiceUnverified before the authorization callback. A newer invoice reached that callback and was deliberately aborted. Neither added a payment.
  • After process death and relaunch, native Spark calls still refused the older invoice and the durable restore time remained. These are unfunded guard controls, not a duplicate-payment settlement or recovery-phrase wipe journey. The recording shows app state; the guard assertions were checked through native calls and readback.

restore-invoices-2x.2x.mp4

Metadata-present restore, metadata-absent restore, then cold relaunch.

Keep Spark payment results visible while the node is stopped ❌ A successful payment’s result was covered by “Connecting to Network”, confirmed by video, native state and the UI tree.
  • With the node stopped and Spark holding 100,000 sats, a normal UI send paid a new 60,000-sat invoice. Native payment success arrived and Spark’s remaining balance became 39,758 sats.
  • The UI tree contained SendSuccess and its Close/Details controls, while the recording shows the opaque node-sync overlay covering them. SendSheet.kt:187 recomputes eligibility against the original invoice after the balance falls; the global overlay at :901 then reappears over the result.
  • SendSheet.kt is byte-identical on this PR’s live base 2dc86ae and head. This is an inherited runtime failure from bitkit-android#1858 fix: stop a restored wallet paying a spark-paid invoice again, so no new inline finding is filed here. Keep the result visible once a payment has been admitted. Pending/Error paths have the same source condition; this run drove Success.

spark-result-overlay-2x.2x.mp4

Final send confirmation, successful payment, then the covered result.

Finish Reset while native authentication is held ❌ Reset remained pending for at least 203.6 seconds with authentication held, checked in video and timed readback.

UI Reset retained wallet credentials, closed backup admission and remained pending for at least 203.6 seconds while the real native authentication response was held. Releasing that response let the cancelled lookup drain and Reset finish 7.2 seconds later. The recording drives the actual Reset action. Credential/admission state and elapsed time were checked in timed readback; consecutive recording parts are joined in order.

stalled-native-auth-2x.2x.mp4

Reset waits on native authentication, then completes after response release.

Check: pristine build and affected regressions ✅ APK assembly and the affected backup, Lightning, Spark, failure-message and overlay suites passed.

Checked the unmodified head and fresh JUnit reports. Full-project unit, lint and Compose suites were not repeated. Existing manifest, native-symbol stripping, test-API deprecation and JVM/Gradle diagnostics remain in the build log. The installed controlled APK hash matches its build receipt; all eleven native libraries match the pristine APK.

Retained evidence keeps its tested head and limits:

  • Earlier child backup review at e0d2598: passing Manual 1–9 scheduler, callback, stopped-observer retry, cold-start, ACTIVITY and Spark backup controls. Their reached backup paths are unchanged; the newly composed full-restore marker writes were driven above. Historical signed-operation, cancelled-reset-lambda and settlement limits remain.
  • Previous child review at c53a459: both backup failures were already reproduced there and were repeated here.
  • Parent device report at cb9a7af06: new-invoice channel payment during Spark outage, seeded-hash relaunch protection and node-side restored-invoice refusal retain their original evidence. Its stopped-node Spark payment did not establish the post-send result visibility checked here; its later Spark-side refusal was only unit-tested until this run’s native guard controls.
  • Profile parent review at cd1d129: optional profile recovery, stale-response rejection, migration/wipe draining and avatar replacement. Those reached paths and dependency pins remain unchanged in this delta.

No planned device case remains unrun. The historical Manual 1–5 artifact request retains its original qualification.

Coverage: contiguous delta c53a459..62c8cf2 plus earlier open backup concerns, including owner retention, restore ordering, native cancellation and the newly composed payment guards. Funded restore/replay of an already-paid invoice, Quick Pay’s post-spend result, private SDK recovery acceptance, hardware signing and RBF were not driven in this delta.

Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle remains at 6c40b8d5 and matches original-owner observation/draining, explicit retry and immutable recovery persistence. It has no Spark backup category. The separate Spark feature pair is bitkit-ios#1128 feat: add spark via breez sdk; its current Swift code uses shared rail coordination, so invoice-guard parity and an iOS runtime result are not claimed here.

Unpublished Debug controls

Controls stage only disposable state, inspect the real SDK/VSS results and route transport. The guard control’s authorization callback deliberately aborts newer invoices before dispatch. The UI payment above uses the normal send flow.

diff --git a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
index 6532aa144..0c551c48e 100644
--- a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
+++ b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
@@ -41,6 +41,7 @@ class DevToolsProvider : ContentProvider() {
 
     override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
         check(Binder.getCallingUid() == SHELL_UID) { "Only ADB shell callers are allowed" }
+        if (method.startsWith("qa1686")) return kotlinx.coroutines.runBlocking(kotlinx.coroutines.Dispatchers.IO) { Qa1686.run(requireNotNull(context), method, arg) } // test hook
         return runCatching {
             val command = requireNotNull(DevCommand.parse(method, arg)) { "Unknown command: '$method'" }
             ServiceQueue.LDK.blocking { command.execute(deps) }
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
index bd5fe8766..7a8a30f08 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
@@ -57,7 +57,7 @@ class VssBackupClientLdk @Inject constructor(
                         storeId = vssDeriveStoreId(Env.vssStoreIdPrefix, mnemonic, passphrase),
                         mnemonic = mnemonic,
                         passphrase = passphrase,
-                        lnurlAuthServerUrl = Env.lnurlAuthServerUrl,
+                        lnurlAuthServerUrl = if (to.bitkit.BuildConfig.DEBUG && java.io.File("/data/user/0/to.bitkit.dev/files/qa1686-auth-override").exists()) "http://127.0.0.1:23469/lnurl_auth/auth" else Env.lnurlAuthServerUrl, // test hook
                     )
                     Logger.info("VSS LDK client setup for wallet[$walletIndex]", context = TAG)
                 }
diff --git a/app/src/main/java/to/bitkit/services/SparkService.kt b/app/src/main/java/to/bitkit/services/SparkService.kt
index f7488d1ac..6783ae2bb 100644
--- a/app/src/main/java/to/bitkit/services/SparkService.kt
+++ b/app/src/main/java/to/bitkit/services/SparkService.kt
@@ -155,11 +155,16 @@ class SparkService @Inject constructor(
         val mnemonic = keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name) ?: throw ServiceError.MnemonicNotFound()
         val passphrase = keychain.loadString(Keychain.Key.BIP39_PASSPHRASE.name)
         val config = configure(defaultConfig(network), Env.breezApiKey)
+        val qaConfig = File("/data/user/0/to.bitkit.dev/files/qa1686-spark-config.json") // test hook
+        if (to.bitkit.BuildConfig.DEBUG && qaConfig.exists()) { // test hook
+            config.sparkConfig = breez_sdk_spark.parseSparkConfig(qaConfig.readText()) // test hook
+            config.watchtowerFeeLadderEnabled = false // test hook
+        } // test hook
         val storageDir = File(Env.sparkStoragePath(walletIndex)).apply { mkdirs() }.path
-        val chainServiceUrl = Env.sparkChainServiceUrl(Env.network)
+        val chainServiceUrl = if (to.bitkit.BuildConfig.DEBUG && qaConfig.exists()) "http://127.0.0.1:23090/api" else Env.sparkChainServiceUrl(Env.network) // test hook
         val connected = SdkBuilder(config, Seed.Mnemonic(mnemonic, passphrase)).use { builder ->
             builder.withDefaultStorage(storageDir)
-            chainServiceUrl?.let { builder.withRestChainService(it, ChainApiType.ESPLORA, null) }
+            chainServiceUrl?.let { builder.withRestChainService(it, if (qaConfig.exists()) ChainApiType.MEMPOOL_SPACE else ChainApiType.ESPLORA, null) } // test hook
             builder.build()
         }
         listenerId = connected.addEventListener(
diff --git a/app/src/debug/java/to/bitkit/dev/Qa1686.kt b/app/src/debug/java/to/bitkit/dev/Qa1686.kt
new file mode 100644
index 000000000..2584fefcf
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/Qa1686.kt
@@ -0,0 +1,207 @@
+package to.bitkit.dev // test hook
+
+import android.content.Context // test hook
+import android.content.ClipData // test hook
+import android.content.ClipboardManager // test hook
+import android.os.Bundle // test hook
+import dagger.hilt.EntryPoint // test hook
+import dagger.hilt.InstallIn // test hook
+import dagger.hilt.android.EntryPointAccessors // test hook
+import dagger.hilt.components.SingletonComponent // test hook
+import kotlinx.coroutines.Dispatchers // test hook
+import kotlinx.coroutines.flow.first // test hook
+import kotlinx.coroutines.withContext // test hook
+import to.bitkit.data.CacheStore // test hook
+import to.bitkit.data.backup.VssBackupClient // test hook
+import to.bitkit.data.keychain.Keychain // test hook
+import to.bitkit.models.BackupCategory // test hook
+import to.bitkit.repositories.BackupRepo // test hook
+import to.bitkit.repositories.PubkyRepo // test hook
+import to.bitkit.repositories.WalletRepo // test hook
+import to.bitkit.repositories.LightningRepo // test hook
+import to.bitkit.services.PubkyService // test hook
+import to.bitkit.di.json // test hook
+import to.bitkit.models.WalletBackupV1 // test hook
+import to.bitkit.utils.Logger // test hook
+import java.io.File // test hook
+import kotlinx.coroutines.*
+import to.bitkit.models.*
+import to.bitkit.repositories.*
+import to.bitkit.services.*
+import kotlin.time.Clock
+
+object Qa1686 { // test hook
+    private var replacementPhrase: String? = null
+    private val scope = kotlinx.coroutines.CoroutineScope(kotlinx.coroutines.SupervisorJob() + Dispatchers.IO)
+    @EntryPoint @InstallIn(SingletonComponent::class) // test hook
+    interface Dependencies { // test hook
+        fun spark(): SparkRepo
+        fun sparkService(): SparkService
+        fun recovery(): to.bitkit.data.WalletConnectionRecovery
+        fun vssClients(): to.bitkit.data.backup.VssClientCoordinator
+        fun vssLdk(): to.bitkit.data.backup.VssBackupClientLdk
+        fun sdk(): to.bitkit.services.PaykitSdkService
+        fun allowanceExecutor(): to.bitkit.repositories.PaykitAllowanceExecutor
+        fun privatePaykit(): to.bitkit.repositories.PrivatePaykitRepo
+        fun allowances(): to.bitkit.repositories.PaykitAllowanceRepo
+        fun allowanceStore(): to.bitkit.repositories.PaykitAllowanceStore
+        fun keychain(): Keychain // test hook
+        fun backupRepo(): BackupRepo // test hook
+        fun pubkyRepo(): PubkyRepo // test hook
+        fun pubkyService(): PubkyService // test hook
+        fun vss(): VssBackupClient // test hook
+        fun cache(): CacheStore // test hook
+        fun wallet(): WalletRepo // test hook
+        fun accounts(): to.bitkit.data.WatchOnlyAccountStore
+        fun settings(): to.bitkit.data.SettingsStore
+        fun lightning(): LightningRepo // test hook
+    } // test hook
+    suspend fun run(context: Context, method: String, arg: String?): Bundle { // test hook
+        val d = EntryPointAccessors.fromApplication(context, Dependencies::class.java) // test hook
+        fun file(name: String) = File(context.filesDir, "qa1686-$name")
+        val result = kotlin.runCatching { // test hook
+            when (method) {
+                "qa1686Restore" -> {
+                    d.backupRepo().stopObservingBackups()
+                    d.vss().setup().getOrThrow()
+                    val old = requireNotNull(d.vss().getObject("METADATA").getOrThrow())
+                    file("saved-metadata").writeBytes(old.value)
+                    if (arg == "missing") d.vss().deleteObject("METADATA").getOrThrow()
+                    d.cache().update { it.copy(sparkHistoryUnseededSince = null, ldkHistoryUnseededSince = null, sparkRestoredAt = null) }
+                    val result = d.backupRepo().performFullRestoreFromLatestBackup()
+                    val cache = d.cache().data.first()
+                    check(cache.sparkRestoredAt != null)
+                    check(cache.ldkHistoryUnseededSince != null)
+                    if (arg == "missing") d.vss().putObject("METADATA", old.value).getOrThrow()
+                    "restoreSuccess=${result.isSuccess}; missingMetadata=${arg == "missing"}; sparkHold=${cache.sparkHistoryUnseededSince}; nodeHold=${cache.ldkHistoryUnseededSince}; restored=${cache.sparkRestoredAt}; pendingWallet=${d.backupRepo().hasPendingWalletRestore()}; error=${result.exceptionOrNull()?.javaClass?.simpleName}"
+                }
+                "qa1686SparkStart" -> {
+                    d.spark().start(0).getOrThrow()
+                    val info = d.sparkService().getInfo()
+                    "connected=true; balance=${info.balanceSats}; synced=${info.lastSyncedAt}; syncError=${info.lastSyncError?.javaClass?.simpleName}"
+                }
+                "qa1686SparkInfo" -> {
+                    val info = d.sparkService().getInfo()
+                    val cache = d.cache().data.first()
+                    "balance=${info.balanceSats}; shown=${d.spark().state.value.shownBalanceSats}; ready=${d.spark().state.value.isReady}; synced=${info.lastSyncedAt}; restore=${cache.sparkRestoredAt}; unseeded=${cache.sparkHistoryUnseededSince}; node=${d.lightning().lightningState.value.nodeLifecycleState}"
+                }
+                "qa1686SparkInvoice" -> d.sparkService().receiveBolt11(100_000uL, "review regtest funding", 3600u)
+                "qa1686SparkGuard" -> {
+                    val invoice = requireNotNull(arg)
+                    var admitted = false
+                    val before = d.sparkService().listPayments().size
+                    val result = d.spark().payInvoice(invoice, null, onBeforeSend = { admitted = true; false })
+                    val after = d.sparkService().listPayments().size
+                    check(before == after)
+                    "admitted=$admitted; error=${result.exceptionOrNull()?.javaClass?.simpleName}; noNewPayment=${before==after}; historySize=$after"
+                }
+                "qa1686NodeStop" -> {
+                    d.lightning().stop().getOrThrow()
+                    "node=${d.lightning().lightningState.value.nodeLifecycleState}"
+                }
+                "qa1686ProfileStatus" -> {
+                    "identity=${d.pubkyRepo().publicKey.value != null}; authenticated=${d.pubkyRepo().isAuthenticated.value}; profile=${d.pubkyRepo().profile.value?.name}; loading=${d.pubkyRepo().isLoadingProfile.value}"
+                }
+                "qa1686Dev" -> { d.settings().update { it.copy(isDevModeEnabled = true) }; "enabled developer mode" }
+                "qa1686NativeAuth" -> {
+                    d.backupRepo().stopObservingBackups()
+                    d.vssClients().withSuspendedClients {}
+                    file("auth-override").writeText("hold")
+                    d.vssLdk().setup().getOrThrow()
+                    file("native-result").delete()
+                    scope.launch {
+                        try {
+                            val result = d.vssLdk().getObject("qa1686-held-auth")
+                            file("native-result").writeText("returned success=${result.isSuccess}; error=${result.exceptionOrNull()?.javaClass?.simpleName}")
+                        } catch(e: Exception) { file("native-result").writeText("cancelled=${e.javaClass.simpleName}") }
+                    }
+                    "scheduled actual standalone LDK lookup against held auth response"
+                }
+                "qa1686NativeState" -> "credentialsPresent=${d.keychain().exists(Keychain.Key.BIP39_MNEMONIC.name)}; wiping=${d.backupRepo().isWiping.value}; admissionOpen=${runCatching { d.vssClients().captureOwner() }.isSuccess}; result=${file("native-result").takeIf { it.exists() }?.readText()}"
+                "qa1686Stop" -> {
+                    d.backupRepo().stopObservingBackups()
+                    kotlinx.coroutines.delay(300)
+                    val statuses = d.cache().backupStatuses.first()
+                    "stopped; settings=${statuses[BackupCategory.SETTINGS]}"
+                }
+                "qa1686InitCore" -> { // test hook
+                    to.bitkit.async.ServiceQueue.CORE.background { // test hook
+                        com.synonym.bitkitcore.initDb(to.bitkit.env.Env.bitkitCoreStoragePath(0)) // test hook
+                        com.synonym.bitkitcore.updateBlocktankUrl(to.bitkit.env.Env.blocktankApiUrl) // test hook
+                    } // test hook
+                    "initialized real native database after app storage path became available" // test hook
+                } // test hook
+                "qa1686QuickState" -> {
+                    "source=${d.keychain().exists(Keychain.Key.SHARED_PUBKY_SOURCE.name)}, savedSecret=${!d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name).isNullOrBlank()}, savedSession=${!d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name).isNullOrBlank()}, identity=${d.pubkyRepo().publicKey.value != null}, authenticated=${d.pubkyRepo().isAuthenticated.value}, profile=${d.pubkyRepo().profile.value?.name}, contacts=${d.pubkyRepo().contacts.value.size}, pendingRestore=${d.backupRepo().hasPendingWalletRestore()}, node=${d.lightning().lightningState.value.nodeLifecycleState}"
+                }
+                "qa1686CreateProfile" -> {
+                    d.pubkyRepo().createIdentity("QA restore", "", emptyList(), emptyList(), null).getOrThrow()
+                    "created disposable profile"
+                }
+                "qa1686Backup" -> {
+                    for (c in listOf(BackupCategory.WALLET, BackupCategory.METADATA, BackupCategory.SETTINGS)) d.backupRepo().triggerBackup(c).getOrThrow()
+                    val wallet = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+                    val metadata = requireNotNull(d.vss().getObject("METADATA").getOrThrow())
+                    val pending = d.cache().backupStatuses.first().filterKeys { it in listOf(BackupCategory.WALLET, BackupCategory.METADATA) }.any { it.value.isRequired }
+                    check(!pending)
+                    "backup synchronized; sdk=${json.decodeFromString<WalletBackupV1>(wallet.value.decodeToString()).paykitSdkBackupState != null}; metadataBytes=${metadata.value.size}"
+                }
+                "qa1686Terminal" -> {
+                    d.backupRepo().stopObservingBackups()
+                    val secret = requireNotNull(d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name))
+                    File(context.filesDir, "qa1686-original-secret").writeText(secret)
+                    val session = requireNotNull(d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name))
+                    File(context.filesDir, "qa1686-original-session").writeText(session)
+                    d.pubkyService().forgetSessionAccess()
+                    d.keychain().delete(Keychain.Key.PUBKY_SECRET_KEY.name)
+                    d.keychain().upsertString(Keychain.Key.PAYKIT_SESSION.name, "qa1686-invalid-session")
+                    d.pubkyRepo().initialize().getOrThrow()
+                    check(d.pubkyRepo().publicKey.value == null)
+                    val before = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+                    val reserved = d.accounts().reserveAccountIndex(0, "qa1686-terminal-ordinary")
+                    check(d.accounts().backupSnapshot().allocationState.highestAccountIndexByWallet["0"] == reserved)
+                    val failed = d.backupRepo().triggerBackup(BackupCategory.WALLET)
+                    val after = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+                    val same = before.value.contentEquals(after.value) && before.version == after.version
+                    val pending = d.cache().backupStatuses.first()[BackupCategory.WALLET]?.isRequired == true
+                    check(failed.isFailure && same && pending)
+                    "terminal=true; ordinary watch-only index=$reserved reserved; failed=true; remote unchanged=true; pending=true; error=${failed.exceptionOrNull()?.javaClass?.simpleName}"
+                }
+                "qa1686RepairIdentity" -> {
+                    d.keychain().upsertString(Keychain.Key.PUBKY_SECRET_KEY.name, File(context.filesDir, "qa1686-original-secret").readText())
+                    d.keychain().upsertString(Keychain.Key.PAYKIT_SESSION.name, File(context.filesDir, "qa1686-original-session").readText())
+                    d.pubkyRepo().initialize().getOrThrow()
+                    d.backupRepo().triggerBackup(BackupCategory.WALLET).getOrThrow()
+                    val latest = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+                    val restored = json.decodeFromString<WalletBackupV1>(latest.value.decodeToString())
+                    check(restored.watchOnlyAccountAllocationState == d.accounts().backupSnapshot().allocationState)
+                    check(restored.paykitSdkBackupState != null)
+                    check(d.cache().backupStatuses.first()[BackupCategory.WALLET]?.isRequired == false)
+                    "recovered original identity; complete readback includes latest allocation; synchronized=true"
+                }
+                "qa1686Recover" -> { // test hook
+                    val secret = d.pubkyService().deriveSecretKey(requireNotNull(d.keychain().loadString(Keychain.Key.BIP39_MNEMONIC.name))) // test hook
+                    d.keychain().upsertString(Keychain.Key.PUBKY_SECRET_KEY.name, secret) // test hook
+                    d.pubkyService().initialize() // test hook
+                    d.pubkyRepo().restoreSessionBackupState(to.bitkit.models.PubkySessionBackupV1(kind = to.bitkit.models.PubkySessionBackupKind.LocalSeed)).getOrThrow() // test hook
+                    d.pubkyRepo().scheduleInitializationAfterRestore() // test hook
+                    d.backupRepo().triggerBackup(BackupCategory.WALLET).getOrThrow() // test hook
+                    d.backupRepo().startObservingBackups() // test hook
+                    "recovered identity and fresh complete backup" // test hook
+                } // test hook
+                "qa1686AddressGate" -> { // test hook
+                    val result = d.wallet().newAddress() // test hook
+                    "new address success=${result.isSuccess}; error=${result.exceptionOrNull()?.javaClass?.simpleName}" // test hook
+                } // test hook
+                "qa1686SignOut" -> { // test hook
+                    d.pubkyRepo().signOut().getOrThrow() // test hook
+                    "identity removed" // test hook
+                } // test hook
+                "qa1686DeleteProfile" -> { d.pubkyRepo().deleteProfile().getOrThrow(); "deleted disposable profile" }
+                else -> "source=${d.keychain().exists(Keychain.Key.SHARED_PUBKY_SOURCE.name)}, savedSecret=${!d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name).isNullOrBlank()}, savedSession=${!d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name).isNullOrBlank()}, sdk=${d.pubkyService().currentPublicKey() != null}, identity=${d.pubkyRepo().publicKey.value != null}, profile=${d.pubkyRepo().profile.value?.name}, contacts=${d.pubkyRepo().contacts.value.size}, pendingRestore=${d.backupRepo().hasPendingWalletRestore()}, node=${d.lightning().lightningState.value.nodeLifecycleState}" // test hook
+            } // test hook
+        }.fold({ it }, { "ERROR: ${it.javaClass.simpleName}: ${it.message}" }) // test hook
+        Logger.info("QA1686 $method: $result", context="TestHook") // test hook
+        return Bundle().apply { putString("result", result) } // test hook
+    } // test hook
+} // test hook

Reviewed by gpt-6.1-sol-high

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Full reassessment of synonymdev/bitkit-android#1689 at 62c8cf23 against merge base 2dc86ae0. The comparison base moved from 3e1fdb7d to 2dc86ae0 and no coverage baseline was supplied, so the whole PR diff was reviewed again with no inherited coverage.

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

Stopping automatic profile deletion after an uncertain acknowledgement matches the new journey. A still-current sign-in that fails the pubky profile delete with a Paykit identity error no longer refreshes that session, so Retry repeats the failure. Backup reset draining, generation ownership, explicit retry, setup-timeout and request-send classification, cold activity and Blocktank reads, and USDT snapshot reservation were traced through their callers and the updated unit tests; no further defect held up.

bitkit-ios#1123 at 6c40b8d5 still refreshes a current session after a session-refreshable error and then replays the whole deletion, including contact cleanup. bitkit-android#1686 at ff9275d6 is one commit ahead of this merge base and does not change BackupRepo, PubkyRepo, or VssBackupClient.

This review did not run the unit tests or a device. GitHub CI and lint succeeded on this head. The passing E2E run does not exercise the backup fault fixture or the profile proxy journey. Device testing: not performed in this review. Author manual cases 1–5 and 9 remain open on this head; cases 6–8 are historical runs on older commits.

Suggested additional test cases

  • Android, disposable non-Ring profile, still signed in. Force deletePubkyProfile to fail with a Paykit identity/session error, not a dropped success acknowledgement. Tap Retry. Expected: one sign-in and the profile deletion completes. Signing out during the in-flight failure must not sign back in, and an uncertain acknowledgement must still not start a second contact cleanup.

Earlier findings reconciled at this revision: 3 still present, 4 verified fixed, 1 could not be verified, 1 tracked separately outside this PR.

Tracked separately (outside this PR; unresolved): Native LDK shutdown abort during VSS outage. The reported VSS-outage native shutdown abort remains the open ldk-node shutdown failure in #1536. From base 2dc86ae to head 62c8cf2 this PR does not change LightningService.stop, the ldk-node-android 0.7.0-rc.80 pin, or native LDK sources. The issue stays open. This stage performed no native execution and does not treat the issue as a fix or a current-head pass. This is not a runtime verification or merge-readiness declaration.

Reconciliation incomplete: the following earlier concerns remain unverified. This report does not establish that they are fixed or invalid, or establish approval or device readiness.

Could not verify earlier concern — Signed-in outage recovery encountered Pubky lock/HTTP 405 errors: The HTTP 405 Pubky write-lock failure is an attributed device report at 9cfe052. A later attributed signed-in repeat at a3693b0 completed WALLET, METADATA, and SETTINGS and said that repeat does not explain the earlier HTTP 405 or identify a code fix. Current source still propagates Paykit snapshot failures and still refuses metadata replacement while a required WALLET backup has quarantined public addresses. Paykit Transport remains temporary, and the AuthException classifier matches only a VSS request-send string, so neither path shows a stuck HTTP 405. No current reproduction, native trace, or exact tracking issue was available in this code-only pass, so recurrence and resolution stay unverified.

Findings

  • [MEDIUM] Retry cannot refresh a failed pubky profile delete — inline at app/src/main/java/to/bitkit/repositories/PubkyRepo.kt:1276.

Existing concerns

Reviewer model: grok-4.7
Reasoning effort: high

val known = ownProfileRevision
val revision = known ?: pubkyService.fetchPubkyProfileWithRevision(publicKey)?.revision ?: return
runSuspendCatching { withPubkyAppScope { pubkyService.deletePubkyProfile(revision, publicKey) } }.getOrElse {
runSuspendCatching { pubkyService.deletePubkyProfile(revision, publicKey) }.getOrElse {

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.

[MEDIUM] Retry cannot refresh a failed pubky profile delete

Delete a non-Ring profile while that sign-in is still current and pubkyService.deletePubkyProfile fails with PaykitException.Identity.

deletePubkyAppProfile previously ran that delete inside withPubkyAppScope, which signs in from the local secret and retries the delete once for an identity error. The wrapper is gone, and the concurrent-update retry is also direct. refreshSessionIfPossible is now private and only used by withPubkyAppScope for other profile writes. retryDeleteProfile calls attemptDeleteProfile, which calls deleteProfile() again and never refreshes the session.

Retry therefore returns the same identity error. The dialog's other action signs out and leaves the homeserver profile in place.

While the original sign-in is still current, refresh once and retry only the failed pubky profile delete. Do not sign in after sign-out, and do not start a second contact cleanup or profile deletion after an uncertain acknowledgement. Keep profile deletion does not replay after an uncertain response; pubky app deletion does not renew the session or replay an identity error currently forbids this still-signed-in refresh.

Evidence basis: source analysis at 62c8cf23. This review did not execute the deletion.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed from the public source: explicit Retry calls the whole deleteProfile() operation again and has no failed-step session recovery. The existing no-replay tests cover automatic replay; they do not establish successful explicit recovery after a confirmed authentication rejection.

I am leaving this concern open. PaykitException.Identity alone does not establish that this exact request was rejected before dispatch, and the current signIn boundary does not demonstrate preservation of the original grant and homeserver. Restoring either broad wrapper would also reintroduce the sign-out/replacement and uncertain-acknowledgement risks you identified.

A safe fix needs the exact refusal contract and a one-use continuation for only the refused step, retaining the original sign-in generation, revision and acknowledgements. It must refuse sign-out/replacement and must not repeat acknowledged contact/profile cleanup. The current cache-startup composition leaves all of these deletion sources and tests unchanged; it does not claim this recovery limitation is fixed. This disposition is source-based, with no new installed deletion execution.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checked at ac1549f: the deletion sources and tests are unchanged. Explicit Retry still calls the whole deletion without failed-step session recovery. This composition does not claim to fix the limitation described in your reply.

@piotr-iohk

Copy link
Copy Markdown
Collaborator

@ben-kaufman please resolve conflicts.

@jvsena42

Copy link
Copy Markdown
Member

@ben-kaufman this conflicts with master; please rebase. I'll review it again once it merges cleanly.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⛔️ Request changes — the existing complete-WALLET and native-authentication Reset failures still reproduce. No new finding on this PR’s changed lines.

Findings:

  • Existing HIGH — terminal identity failure blocks newer ordinary WALLET backups. app/src/main/java/to/bitkit/repositories/BackupRepo.kt:1280. A newer watch-only reservation remained pending while native readback showed unchanged remote WALLET bytes/version. Repairing the original identity allowed a complete upload containing that reservation. This adds current-head evidence to the existing complete-WALLET concern.
  • Existing MEDIUM — Reset waits for stalled native authentication. app/src/main/java/to/bitkit/data/backup/VssClientCoordinator.kt:138. With the real native authentication response held, UI Reset retained credentials and closed backup admission for 203.0 seconds. It finished 13.3 seconds after release. This rechecks the existing native-auth Reset finding; VSS remains 0.5.26-rc2.

QA for ac1549f: Android 16 / API 36 ARM64 emulator, built from this head. Disposable regtest wallet and isolated Spark network. Unpublished Debug controls stage faults, inspect real SDK/VSS readback, call restore and repository entrypoints, and direct Spark traffic to that network. All eleven native libraries match the pristine APK. Recordings play at 2×; multi-part recordings are joined in order.

Back up ordinary changes after terminal identity failure ❌ A newer reservation remained pending until identity repair, checked in native VSS readback.

The previously acknowledged complete WALLET stayed intact. After restoring the original credentials, complete readback contained the latest allocation and synchronized status. The recording shows Data Backups; hidden credential, revision and payload assertions were checked in logs and decrypted native readback.

terminal-wallet-2x.2x.mp4

Terminal identity fault, pending backup, then complete recovery.

Finish Reset while native authentication is held ❌ Reset remained pending for 203.0 seconds, checked in timed readback and video.

The recording drives the actual Reset action. Credentials remained present, wipe stayed active and admission stayed closed throughout the hold. Releasing the native response let cancellation drain and Reset complete. These hidden state and timing assertions were checked in readback.

stalled-native-auth-2x.2x.mp4

Reset waits for authentication, then completes after release.

Keep Spark payment results visible while the node is stopped ✅ A real 60,000-sat send reached a visible Success screen after Spark’s balance fell below the paid amount.

Spark fell from 100,000 to 39,758 sats. SendSuccess, Close and Details remained visible, with no node-sync overlay. Native state confirmed that the node stayed stopped. This fixes the previous result-overlay failure. Success was driven; Pending and Error exemptions have component coverage.

spark-result-2x.2x.mp4

Confirmation and successful Spark send with the node stopped.

Refresh the Spark balance while the node is stopped ✅ Spending increased from 39,758 to 139,758 sats and remained correct after cold relaunch, checked in the UI tree and native state.

The new incoming payment updated Home before explicit balance inspection. The send limit became 139,409 sats while the node remained stopped. Relaunch retained the balance and recorded Spark split. This drives the NodeNotRunningError fallback; the separate timeout fallback is covered by the existing component suite.

spark-balance-2x.2x.mp4

Incoming Spark funds, live Spending update and cold relaunch.

Route a native Spark withdrawal to an onchain endpoint ✅ The native router selected Spark and paid exactly 20,000 sats, checked against the fixture chain and durable activity.

The initial native result was Pending. After settlement and mining, the journal cleared to one sent record. The transaction paid exactly 20,000 sats to the selected address; the sent activity retained its contact and amount after relaunch. Native export produced 344,094 bytes of exit data and imported them successfully.

These are repository/native controls, rather than a full contact or payment-request UI journey. The recording shows app state; route selection, chain output, journal, activity and export assertions were checked in logs/native readback. The first cold read ran before Spark connected; the completed cold check explicitly connects it. Receipt confirmation against the hosted regtest fallback was not driven by this isolated-chain case.

spark-withdrawal-2x.2x.mp4

Native withdrawal, settlement and cold readback.

Refuse older invoices after restoring wallet backups ✅ Full restore armed both rail markers; Spark refused an unlisted older invoice before dispatch and after cold restart, checked in native calls.

The actual full-restore entrypoint completed with metadata present and with its remote object temporarily absent. WALLET restore admission reopened in both cases. The older invoice returned RestoredInvoiceUnverified before its authorization callback; a newly issued invoice reached that callback and was deliberately aborted. Neither guard control added a payment. The recording shows app state; native calls and readback establish the guard assertions.

These same-wallet controls do not establish recovery-phrase wipe or duplicate-payment settlement. The prior node-side guard report retains its original evidence; that refusal path and LDK binary remain unchanged.

restore-invoices-2x.2x.mp4

Metadata-present restore, metadata-absent restore and cold relaunch.

Show Spark connection status ✅ App Status showed Connected Spark and Using Spark, then opened Spark details.

spark-status-2x.2x.mp4

App Status and Spark details.

Check: keep a resolved Spark request out of Pending ❌ A deterministic component control consumed completion before an older Unknown outcome returned, leaving only non-observing Pending.

At AppViewModel.kt:7153–7181, request sends register uncertainOnchainPaymentRequestId only when opening Pending, after suspendable outcome reads. The resolution consumer at :1243 can consume the sole completion first. The control waits for that consumption, then returns the older Unknown snapshot: expected PaymentSuccess, actual only NavigateToPending(observeResolution=false). An already-admitted proof refresh can race this read. The earlier yield-only control still reached Success and is not evidence for a stuck screen.

This code is identical on the live base b2c3ea6 and this head, so this is an inherited parent defect, with no new inline finding here. Register the request before either recovery read and preserve a completion that arrives during them, as the existing ordinary onchain path does. This is component evidence, not a device reproduction.

Check: use the selected Spark request price ❌ Confirmation rejected valid onchain terms against a retained, unselected Lightning invoice in a component control.

For a USD 2.50 request, the selected onchain rate yields 2,500 sats while the retained Lightning rate yields 2,000 sats. With the Spark leg and onchain terms selected, fixedLightningAmount still reads that invoice and AppViewModel.kt:7522 emits PaymentFailedToast before sending. Ignore the unselected invoice when validating a service leg.

This code is also identical on the live base and this head. It is an inherited parent defect with no new inline finding here. The focused regression confirms the selected endpoint and both prices before exercising confirmation; no device execution is claimed.

Diagnostic request regression patch

Applied only to the review checkout. Both cases run the real view model with mocked repositories in AppViewModelSendFlowTest.kt.

--- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt
+++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt
@@ -13359,6 +13359,77 @@
     }
 
     @Test
+    fun `review regression request settlement during stale spark outcome never stays pending`() = test {
+        val leg = sparkRouteLeg("bcrt1qroutedrequest")
+        val txid = "ab".repeat(32)
+        val request = paymentRequest().copy(
+            amount = PaykitAmount(PaykitAsset.BTC, leg.deliveredSat),
+            acceptedPaymentEndpointIdentifiers = listOf(MethodId.P2wpkh.rawValue),
+        )
+        val resolutionConsumed = CompletableDeferred<Unit>()
+        val privateContext = privatePaymentContext(7uL)
+        whenever(paykitPaymentRequestRepo.accept(request)).thenReturn(Result.success(Unit))
+        whenever(paykitPaymentRequestRepo.ensurePaymentAllowed(request)).thenReturn(Result.success(Unit))
+        whenever(privatePaykitRepo.consumePrivatePaymentList(testPublicKey, privateContext))
+            .thenReturn(Result.success(Unit))
+        whenever(paykitPaymentProofRepo.captureOriginalOnchainPayer(request)).thenReturn(Result.success(testPublicKey))
+        whenever(lightningRepo.sparkWithdrawalTxidsTo(leg.endpoint.value)).thenReturn(emptySet())
+        whenever(lightningRepo.sparkWithdrawalOutcome(leg.endpoint.value, leg.deliveredSat, emptySet()))
+            .doSuspendableAnswer {
+                // A payment list read began while this withdrawal was still pending. Before it returns
+                // that snapshot, another sync completes the proof and publishes its sole resolution.
+                onchainPaymentResolutions.value = listOf(
+                    PaykitOnchainPaymentProofResolution(testPublicKey, request.id, txid),
+                )
+                resolutionConsumed.await()
+                SparkWithdrawalOutcome.Unknown
+            }
+        whenever(
+            paykitPaymentProofRepo.markSparkOnchainPaymentStarted(any(), any(), any(), anyOrNull(), anyOrNull(), any()),
+        )
+            .thenReturn(Result.success(Unit))
+        whenever(paykitPaymentProofRepo.verifyOriginalOnchainPayer(request, testPublicKey))
+            .thenReturn(Result.success(Unit))
+        whenever(paykitPaymentProofRepo.consumeOnchainPaymentResolution(any())).then { call ->
+            onchainPaymentResolutions.update { it - call.getArgument<PaykitOnchainPaymentProofResolution>(0) }
+            resolutionConsumed.complete(Unit)
+            Unit
+        }
+        whenever(paykitPaymentProofRepo.releaseUnsentSparkOnchainProof(request)).thenReturn(false)
+        whenever(
+            paykitRouteRepo.payServiceLeg(eq(leg), eq(testPublicKey), any(), anyOrNull(), any()),
+        ).doSuspendableAnswer {
+            val onBeforeSend = it.getArgument<suspend () -> Boolean>(4)
+            onBeforeSend()
+            Result.failure(AppError("send outcome lost"))
+        }
+        pubkyPublicKey.value = testPublicKey
+        setActiveContactPaymentContext(testPublicKey, privateContext, request)
+        setSendState(
+            SendUiState(
+                address = leg.endpoint.value,
+                amount = leg.deliveredSat,
+                payMethod = SendMethod.LIGHTNING,
+                isPaymentRequest = true,
+                incomingPaymentRequestId = request.id,
+                paykitRoute = PaykitRouteUi(persistentListOf(leg.endpoint), offersSparkLeg = true, serviceLeg = leg),
+            ),
+        )
+        val effects = mutableListOf<SendEffect>()
+        backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) { sut.sendEffect.collect { effects += it } }
+        sut.showSheet(Sheet.Send(SendRoute.Confirm))
+
+        confirmCurrentPayment()
+        assertTrue(resolutionConsumed.isCompleted, "Outcome returned only after its one resolution was consumed")
+
+        assertEquals(
+            listOf<SendEffect>(SendEffect.PaymentSuccess),
+            effects,
+            "The sole completion was consumed while outcome read suspended; Pending must not overwrite it",
+        )
+    }
+
+    @Test
     fun `spark route saves the tags picked on confirmation`() = test {
         val leg = sparkRouteLeg("bcrt1qroutedcontact")
         whenever(paykitRouteRepo.payServiceLeg(eq(leg), eq("pubkycontact"), eq(listOf("rent")), anyOrNull(), any()))
--- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt
+++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt
@@ -13022,6 +13022,94 @@
     }
 
     @Test
+    fun `review regression spark leg ignores unselected invoice with a different request rate`() = test {
+        val leg = sparkRouteLeg("bcrt1qroutedrequest").copy(deliveredSat = 2_500uL, debitSat = 2_800uL)
+        val txid = "ab".repeat(32)
+        val deadline = Instant.parse("2099-01-01T00:00:00Z")
+        val request = paymentRequest().copy(
+            amount = PaykitAmount(PaykitAsset.USD, 250uL),
+            pricing = PaykitRequestPricing(
+                PaymentConversion.Fixed(
+                    listOf(
+                        ConversionRate(MethodId.P2wpkh.rawValue.substringBeforeLast("-"), "0.00001"),
+                        ConversionRate(MethodId.Bolt11.rawValue.substringBeforeLast("-"), "0.000008"),
+                    ),
+                ),
+            ),
+            acceptedPaymentEndpointIdentifiers = listOf(MethodId.P2wpkh.rawValue, MethodId.Bolt11.rawValue),
+            paymentDeadlineAt = deadline,
+        )
+        val privateContext = privatePaymentContext(7uL)
+        whenever(paykitPaymentRequestRepo.accept(request)).thenReturn(Result.success(Unit))
+        whenever(paykitPaymentRequestRepo.ensurePaymentAllowed(request)).thenReturn(Result.success(Unit))
+        whenever(privatePaykitRepo.consumePrivatePaymentList(testPublicKey, privateContext))
+            .thenReturn(Result.success(Unit))
+        whenever(paykitPaymentProofRepo.captureOriginalOnchainPayer(request)).thenReturn(Result.success(testPublicKey))
+        whenever(lightningRepo.sparkWithdrawalTxidsTo(leg.endpoint.value)).thenReturn(emptySet())
+        whenever(
+            paykitPaymentProofRepo.markSparkOnchainPaymentStarted(any(), any(), any(), anyOrNull(), anyOrNull(), any()),
+        )
+            .thenReturn(Result.success(Unit))
+        whenever(paykitPaymentProofRepo.verifyOriginalOnchainPayer(request, testPublicKey))
+            .thenReturn(Result.success(Unit))
+        whenever(
+            paykitRouteRepo.payServiceLeg(eq(leg), eq(testPublicKey), any(), anyOrNull(), any()),
+        ).doSuspendableAnswer {
+            val onBeforeSend = it.getArgument<suspend () -> Boolean>(4)
+            if (onBeforeSend()) Result.success(txid) else Result.failure(AppError("refused"))
+        }
+        setActiveContactPaymentContext(testPublicKey, privateContext, request)
+        val reviewed = request.payment(MethodId.P2wpkh, Clock.System.now())
+        AppViewModel::class.java.getDeclaredField("paykitPaymentTerms").apply {
+            isAccessible = true
+            set(sut, reviewed)
+        }
+        AppViewModel::class.java.getDeclaredField("paykitReviewedAmount").apply {
+            isAccessible = true
+            set(sut, reviewed.amount)
+        }
+        setSendState(
+            SendUiState(
+                address = leg.endpoint.value,
+                // The normal route picker retains the decoded, unselected Lightning invoice.
+                decodedInvoice = lightningInvoice("lnbcrt1unselected", 2_000uL),
+                amount = leg.deliveredSat,
+                payMethod = SendMethod.LIGHTNING,
+                isPaymentRequest = true,
+                paykitRoute = PaykitRouteUi(persistentListOf(leg.endpoint), offersSparkLeg = true, serviceLeg = leg),
+            ),
+        )
+
+        assertEquals(2_500uL, request.payment(MethodId.P2wpkh, Clock.System.now()).amount.atomic)
+        assertEquals(2_000uL, request.payment(MethodId.Bolt11, Clock.System.now()).amount.atomic)
+        assertEquals(MethodId.P2wpkh.rawValue, sut.sendUiState.value.routedPaykitRoute?.serviceLeg?.endpoint?.methodId)
+        clearInvocations(toastManager)
+        confirmCurrentPayment()
+        advanceUntilIdle()
+        val observedToasts = argumentCaptor<Toast>()
+        verify(toastManager, atLeast(0)).enqueue(observedToasts.capture())
+        assertTrue(
+            observedToasts.allValues.none { it.testTag == "PaymentFailedToast" },
+            "Selected Spark leg was rejected by the retained invoice: " + observedToasts.allValues,
+        )
+
+        inOrder(paykitPaymentRequestRepo, paykitPaymentProofRepo).apply {
+            verify(paykitPaymentRequestRepo).ensurePaymentAllowed(request)
+            verify(paykitPaymentProofRepo).markSparkOnchainPaymentStarted(
+                eq(request),
+                eq(leg.endpoint.value),
+                eq(leg.deliveredSat),
+                anyOrNull(),
+                anyOrNull(),
+                any(),
+            )
+            verify(paykitPaymentProofRepo).completeSparkOnchainPayment(request, txid)
+        }
+        // Spark checks the request's deadline right before it sends.
+        verify(paykitRouteRepo).payServiceLeg(eq(leg), eq(testPublicKey), any(), eq(deadline), any())
+    }
+
+    @Test
     fun `a pending spark send of a request fails once its withdrawal is found unsent`() = test {
         val leg = sparkRouteLeg("bcrt1qroutedrequest")
         val request = paymentRequest().copy(
Check: pristine build and existing affected regressions ✅ APK assembly and the existing backup, wallet, Spark, send-flow and navigation suites passed.

Ran assembleDevDebug and the selected testDevDebugUnitTest suites on pristine head source before adding Debug controls or the two diagnostic regressions above. Those new regressions fail as described. Full-project unit, lint and Compose suites were not repeated. Existing manifest, native-symbol stripping, test-API deprecation and JVM/Gradle diagnostics remain recorded.

Retained evidence keeps its original tested revision and limits:

  • Earlier backup review at e0d2598: passing bounded Manual 1–9 scheduler, callback, stopped-observer retry, cold-read and Spark-backup controls. Their reached child backup paths are unchanged. Historical signed-operation, cancelled-reset-lambda and settlement limits remain. Core rc14→rc15 changes Lightning crates and package metadata, without changing backup/activity reader source; the changed Spark native binary was exercised above.
  • Profile parent review at cd1d129: optional profile recovery, stale-response rejection, migration/wipe draining and avatar replacement. Those paths and Paykit rc8 remain unchanged. The author’s explicit deletion-retry limitation is unchanged and is not reopened as a new finding.

No planned device case remains unrun. The historical artifact request retains its original qualification. Full contact-request UI routing, funded restore/replay, hardware signing, RBF and private SDK recovery acceptance were not driven in this delta.

GitHub currently reports merge conflicts with the target branch. These results apply to the pinned head.

Coverage: contiguous delta 62c8cf2..ac1549f plus earlier open concerns. The new Spark/routing/balance code is composed from the live base; this child’s backup/profile implementation is unchanged. Reviewed cancellation, original-wallet ownership, wipe order, request pricing and settlement, restore guards and reached sibling paths.

Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle remains at 6c40b8d5 and matches original-owner observation/draining, explicit retry and immutable recovery persistence. It has no Spark backup category. The separate bitkit-ios#1128 feat: add spark via breez sdk supplies the Spark feature; no new iOS runtime or routing-parity claim is made here.


Reviewed by gpt-6.1-sol-high

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⛔️ Request changes — the existing complete-WALLET and native-authentication Reset failures still reproduce, and both inherited Spark request regressions still fail. No new finding on this PR’s changed lines.

Findings:

  • Existing HIGH — terminal identity failure blocks newer ordinary WALLET backups. app/src/main/java/to/bitkit/repositories/BackupRepo.kt:1311. A newer account reservation stayed pending while the remote complete WALLET remained unchanged. Restoring the original identity allowed its upload. This adds current-head evidence to the existing complete-WALLET concern.
  • Existing MEDIUM — Reset waits for stalled native authentication. app/src/main/java/to/bitkit/data/backup/VssClientCoordinator.kt:138. UI Reset kept credentials and closed admission for at least 204.0 seconds, then completed 13.6 seconds after the held response was released. This rechecks the existing native-authentication Reset concern.
  • Inherited MEDIUM — a resolved Spark request can return to Pending. app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:7186. The sole resolution is consumed during a suspended outcome read, before this request registers for it. The deterministic control still produces only non-observing Pending.
  • Inherited MEDIUM — an unselected invoice rejects the selected Spark request price. app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:7546. Selected onchain terms of 2,500 sats are rejected against a retained 2,000-sat Lightning invoice. Both request defects and their diagnostic patches were reported in the previous review; those paths remain unchanged on the current base and this head, so no duplicate inline findings are added.

QA for 43e314c: Android 16 / API 36 ARM64 emulator, disposable regtest wallet, real native SDK/VSS and an isolated Spark network. Built the pristine head and a Debug control build; all eleven native libraries match byte for byte. The controls stage faults, call repository/restore entrypoints and inspect native readback. Spark and LDK share the fixture chain; fee ladders are disabled for its older operators. Recordings play at 2×.

Back up ordinary changes after terminal identity failure ❌ The new reservation remained pending until the original identity was repaired, checked in native readback.

The acknowledged remote WALLET bytes/version stayed intact. After credential repair, a complete decrypted readback contained the new allocation and synchronized status. The recording shows Data Backups; the credential, revision and payload assertions were checked through the Debug control.

terminal-wallet-2x.2x.mp4

Terminal identity fault, pending backup and complete recovery.

Finish Reset while native authentication is held ❌ Reset stayed pending for at least 204.0 seconds, checked in timed readback.

The recording drives the actual Reset action. Credentials remained present, wiping stayed active and admission stayed closed during the hold. Releasing the real native authentication response let cancellation drain and Reset complete.

stalled-native-auth-2x.2x.mp4

UI Reset during the held native response, then release.

Check: keep a resolved Spark request out of Pending ❌ The component regression still loses completion before Pending registration.

Reapplied the previous review’s deterministic regression to this head. It waits until the sole proof resolution is consumed, then releases the older Unknown outcome. Expected Success; actual only NavigateToPending(observeResolution=false). This is component evidence with mocked repositories, not a device reproduction.

Check: use the selected Spark request price ❌ The component regression still emits PaymentFailedToast for valid selected terms.

The request fixes USD 2.50 at 2,500 sats for the selected onchain endpoint and 2,000 sats for the unselected Lightning endpoint. The Spark service leg retains the decoded invoice and Lightning method; confirmation rejects the selected amount before sending. The same diagnostic patch as the previous review was used; no device claim is made.

Allow an operator-verified unsent invoice after restore ✅ An older invoice reached the deliberately aborting authorization callback without adding a payment, checked in native calls.

The actual full-restore entrypoint armed both rail markers. The native operator status allowed this unlisted, unsent invoice under the new rule, and payment history remained unchanged. Unknown-status and unavailable-operator refusal retain component coverage; those fault variants were not driven on device here.

restore-invoices-2x.2x.mp4

Full restore and native invoice admission check.

Review a Spark emergency exit before starting it ✅ The committed journey reached the native quote and external funding address, then returned without starting an exit.

The quote showed 98,304 sats to exit, 4,252 in network fees, 94,052 net, 7,623 in fee funding and 1,696 staying on Spark. The funding screen showed its address and QR; returning to Spark left Transfer to Savings enabled and the native exit inactive. A first-run QuickPay sheet interrupted the initial attempt; after dismissing it, the full journey passed.

exit-quote-2x.2x.mp4

Emergency exit journey up to the funding address and back.

Show an active Spark exit in App Status ✅ App Status showed Emergency Exit and No open connections, opened progress and returned with the row intact.

The real UI swipe paid 7,623 sats plus a 422-sat fee from Savings. Progress showed the funding transaction waiting for confirmation. Some container identifiers omitted by the semantic UI dump were verified through Android accessibility inspection; navigation used the visible App Status label.

exit-status-2x.2x.mp4

Savings-funded start and App Status navigation.

Refresh balances after a funded Spark exit finishes ✅ All recovery transactions and the leftover sweep confirmed; final and cold-relaunch balances matched native and chain readback.

The native journal reached Done after all 14 recovery steps confirmed. The main sweep paid 98,147 sats and the leftover sweep paid 2,670 sats: 100,817 confirmed at the destination. Savings ended at 192,772, Spending at 1,696 and in-transfer at zero. The normal foreground driver completed the final sweeps; the immediate Done read already restored the 1,696-sat Spark contribution, while Savings reached its final amount after node synchronization/relaunch. A mid-exit cold restart left the three already-accepted transactions’ attempt counts unchanged. Intermediate ticks and mining used Debug/fixture controls; this does not exercise mainnet fee-ladder leaves or Android scheduling with the app closed.

exit-finished-2x.2x.mp4

Mined native exit, cold restart and final balance readback.

Restore earlier exit addresses and funding allocation ✅ Native encrypted metadata readback and full restore preserved the addresses and higher funding index.

Staged public address metadata was uploaded through the real VSS client and decoded from its acknowledged envelope. Full restore merged the backed-up address with the device’s entry, discarded the cached amount, retained device index 22 over backup index 20 and marked the wallet for a funding-address scan. No optional category failed. These are same-wallet restore controls, not a recovery-phrase wipe or funded scan.

exit-metadata-2x.2x.mp4

Acknowledged encrypted metadata and full-restore control.

Check: pristine build and affected regressions ✅ Pristine APK assembly and the affected component suites passed.

Checked the backup, profile, Spark, exit driver/repository, send-flow and balance suites before adding diagnostic controls. The two additional request regressions fail as described above. Full-project unit, lint and Compose suites were not repeated. Existing manifest, native-symbol stripping, deprecated API and Gradle/JVM diagnostics remain in the run evidence.

Retained evidence keeps its original tested revision and limits:

  • Backup review at e0d2598: bounded Manual 1–9 scheduler, callback, stopped-observer retry, cold-read and Spark restore-gate controls. Their scheduler/callback paths are unchanged; this pass separately drove the changed exit metadata and invoice rule. The earlier signed-operation, cancelled-reset-lambda and submission-counter qualifications remain.
  • Previous review at ac1549f: stopped-node Spark results, incoming balance refresh, native withdrawal and restore-marker controls retain their stated scope. Its two failed request controls were rerun here.
  • Profile parent review at cd1d129: optional identity recovery and migration/wipe controls retain their original boundaries. The new profile tag installation paths also have current-head component coverage. The author’s explicit deletion-Retry disposition remains unchanged and is not reopened.

No planned case remains unrun. Recovery-phrase wipe, funded duplicate-payment replay, hardware signing, fee-bump fault variants, private SDK acceptance and OS-granted background exit execution were not driven by this pass. The historical artifact request retains its original qualification.

Coverage: contiguous delta ac1549f..43e314c plus previous open concerns. Spark exit and profile-tag changes are inherited from the current base; no new defect was confirmed on this child’s diff. No later head is covered.

Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle remains at 6c40b8d5 and matches observation draining, independent explicit persistence and bounded retry; it has no Spark backup category.

The inherited exit feature has bitkit-ios#1217 feat: spark emergency exit, with the corresponding journey and funding-address backup design; no new iOS runtime parity claim is made.

Diagnostic device controls, not pushed

These controls only stage faults, route the disposable Spark fixture, invoke the real entrypoints and read state. The native libraries match the pristine head. The two component regression patches are unchanged from the linked previous review.

diff --git a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
index 6532aa144..0c551c48e 100644
--- a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
+++ b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
@@ -41,6 +41,7 @@ class DevToolsProvider : ContentProvider() {
 
     override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
         check(Binder.getCallingUid() == SHELL_UID) { "Only ADB shell callers are allowed" }
+        if (method.startsWith("qa1686")) return kotlinx.coroutines.runBlocking(kotlinx.coroutines.Dispatchers.IO) { Qa1686.run(requireNotNull(context), method, arg) } // test hook
         return runCatching {
             val command = requireNotNull(DevCommand.parse(method, arg)) { "Unknown command: '$method'" }
             ServiceQueue.LDK.blocking { command.execute(deps) }
diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
index bd5fe8766..7a8a30f08 100644
--- a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
+++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt
@@ -57,7 +57,7 @@ class VssBackupClientLdk @Inject constructor(
                         storeId = vssDeriveStoreId(Env.vssStoreIdPrefix, mnemonic, passphrase),
                         mnemonic = mnemonic,
                         passphrase = passphrase,
-                        lnurlAuthServerUrl = Env.lnurlAuthServerUrl,
+                        lnurlAuthServerUrl = if (to.bitkit.BuildConfig.DEBUG && java.io.File("/data/user/0/to.bitkit.dev/files/qa1686-auth-override").exists()) "http://127.0.0.1:23469/lnurl_auth/auth" else Env.lnurlAuthServerUrl, // test hook
                     )
                     Logger.info("VSS LDK client setup for wallet[$walletIndex]", context = TAG)
                 }
diff --git a/app/src/main/java/to/bitkit/repositories/SparkExitChain.kt b/app/src/main/java/to/bitkit/repositories/SparkExitChain.kt
index f77983341..40d98afb5 100644
--- a/app/src/main/java/to/bitkit/repositories/SparkExitChain.kt
+++ b/app/src/main/java/to/bitkit/repositories/SparkExitChain.kt
@@ -180,7 +180,7 @@ class EsploraSparkExitChain(
                     SparkExitBroadcastHost("https://blockstream.info/api"),
                 ),
             )
-            "regtest" -> SparkExitHosts.of("https://regtest-mempool.us-west-2.sparkinfra.net/api")
+            "regtest" -> SparkExitHosts.of(if (to.bitkit.BuildConfig.DEBUG && java.io.File("/data/user/0/to.bitkit.dev/files/qa1686-spark-config.json").exists()) "http://127.0.0.1:23090/api" else "https://regtest-mempool.us-west-2.sparkinfra.net/api") // test hook
             else -> null
         }
 
diff --git a/app/src/main/java/to/bitkit/services/SparkService.kt b/app/src/main/java/to/bitkit/services/SparkService.kt
index d2b1448ca..752fe7bd2 100644
--- a/app/src/main/java/to/bitkit/services/SparkService.kt
+++ b/app/src/main/java/to/bitkit/services/SparkService.kt
@@ -178,11 +178,16 @@ class SparkService @Inject constructor(
         val mnemonic = keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name) ?: throw ServiceError.MnemonicNotFound()
         val passphrase = keychain.loadString(Keychain.Key.BIP39_PASSPHRASE.name)
         val config = configure(defaultConfig(network), Env.breezApiKey, isExiting)
+        val qaConfig = File("/data/user/0/to.bitkit.dev/files/qa1686-spark-config.json") // test hook
+        if (to.bitkit.BuildConfig.DEBUG && qaConfig.exists()) { // test hook
+            config.sparkConfig = breez_sdk_spark.parseSparkConfig(qaConfig.readText()) // test hook
+            config.watchtowerFeeLadderEnabled = false // test hook
+        } // test hook
         val storageDir = File(Env.sparkStoragePath(walletIndex)).apply { mkdirs() }.path
-        val chainServiceUrl = Env.sparkChainServiceUrl(Env.network)
+        val chainServiceUrl = if (to.bitkit.BuildConfig.DEBUG && qaConfig.exists()) "http://127.0.0.1:23090/api" else Env.sparkChainServiceUrl(Env.network) // test hook
         val connected = SdkBuilder(config, Seed.Mnemonic(mnemonic, passphrase)).use { builder ->
             builder.withDefaultStorage(storageDir)
-            chainServiceUrl?.let { builder.withRestChainService(it, ChainApiType.ESPLORA, null) }
+            chainServiceUrl?.let { builder.withRestChainService(it, if (qaConfig.exists()) ChainApiType.MEMPOOL_SPACE else ChainApiType.ESPLORA, null) } // test hook
             builder.build()
         }
         listenerId = connected.addEventListener(
diff --git a/app/src/debug/java/to/bitkit/dev/Qa1686.kt b/app/src/debug/java/to/bitkit/dev/Qa1686.kt
new file mode 100644
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/Qa1686.kt
@@ -0,0 +1,290 @@
+package to.bitkit.dev // test hook
+
+import android.content.Context // test hook
+import android.content.ClipData // test hook
+import android.content.ClipboardManager // test hook
+import android.os.Bundle // test hook
+import dagger.hilt.EntryPoint // test hook
+import dagger.hilt.InstallIn // test hook
+import dagger.hilt.android.EntryPointAccessors // test hook
+import dagger.hilt.components.SingletonComponent // test hook
+import kotlinx.coroutines.Dispatchers // test hook
+import kotlinx.coroutines.flow.first // test hook
+import kotlinx.coroutines.withContext // test hook
+import to.bitkit.data.CacheStore // test hook
+import to.bitkit.data.backup.VssBackupClient // test hook
+import to.bitkit.data.keychain.Keychain // test hook
+import to.bitkit.models.BackupCategory // test hook
+import to.bitkit.repositories.BackupRepo // test hook
+import to.bitkit.repositories.PubkyRepo // test hook
+import to.bitkit.repositories.WalletRepo // test hook
+import to.bitkit.repositories.LightningRepo // test hook
+import to.bitkit.services.PubkyService // test hook
+import to.bitkit.di.json // test hook
+import to.bitkit.models.WalletBackupV1 // test hook
+import to.bitkit.utils.Logger // test hook
+import java.io.File // test hook
+import kotlinx.coroutines.*
+import to.bitkit.models.*
+import to.bitkit.repositories.*
+import to.bitkit.services.*
+import kotlin.time.Clock
+
+object Qa1686 { // test hook
+    private var replacementPhrase: String? = null
+    private val scope = kotlinx.coroutines.CoroutineScope(kotlinx.coroutines.SupervisorJob() + Dispatchers.IO)
+    @EntryPoint @InstallIn(SingletonComponent::class) // test hook
+    interface Dependencies { // test hook
+        fun core(): CoreService
+        fun exit(): SparkExitRepo
+        fun activity(): ActivityRepo
+        fun route(): PaykitRouteRepo
+        fun spark(): SparkRepo
+        fun sparkService(): SparkService
+        fun recovery(): to.bitkit.data.WalletConnectionRecovery
+        fun vssClients(): to.bitkit.data.backup.VssClientCoordinator
+        fun vssLdk(): to.bitkit.data.backup.VssBackupClientLdk
+        fun sdk(): to.bitkit.services.PaykitSdkService
+        fun allowanceExecutor(): to.bitkit.repositories.PaykitAllowanceExecutor
+        fun privatePaykit(): to.bitkit.repositories.PrivatePaykitRepo
+        fun allowances(): to.bitkit.repositories.PaykitAllowanceRepo
+        fun allowanceStore(): to.bitkit.repositories.PaykitAllowanceStore
+        fun keychain(): Keychain // test hook
+        fun backupRepo(): BackupRepo // test hook
+        fun pubkyRepo(): PubkyRepo // test hook
+        fun pubkyService(): PubkyService // test hook
+        fun vss(): VssBackupClient // test hook
+        fun cache(): CacheStore // test hook
+        fun wallet(): WalletRepo // test hook
+        fun accounts(): to.bitkit.data.WatchOnlyAccountStore
+        fun settings(): to.bitkit.data.SettingsStore
+        fun lightning(): LightningRepo // test hook
+    } // test hook
+    suspend fun run(context: Context, method: String, arg: String?): Bundle { // test hook
+        val d = EntryPointAccessors.fromApplication(context, Dependencies::class.java) // test hook
+        fun file(name: String) = File(context.filesDir, "qa1686-$name")
+        val result = kotlin.runCatching { // test hook
+            when (method) {
+                "qa1686Restore" -> {
+                    d.backupRepo().stopObservingBackups()
+                    d.vss().setup().getOrThrow()
+                    val old = requireNotNull(d.vss().getObject("METADATA").getOrThrow())
+                    file("saved-metadata").writeBytes(old.value)
+                    if (arg == "missing") d.vss().deleteObject("METADATA").getOrThrow()
+                    d.cache().update { it.copy(sparkHistoryUnseededSince = null, ldkHistoryUnseededSince = null, sparkRestoredAt = null) }
+                    val result = d.backupRepo().performFullRestoreFromLatestBackup()
+                    val cache = d.cache().data.first()
+                    check(cache.sparkRestoredAt != null)
+                    check(cache.ldkHistoryUnseededSince != null)
+                    if (arg == "missing") d.vss().putObject("METADATA", old.value).getOrThrow()
+                    "restoreSuccess=${result.isSuccess}; missingMetadata=${arg == "missing"}; sparkHold=${cache.sparkHistoryUnseededSince}; nodeHold=${cache.ldkHistoryUnseededSince}; restored=${cache.sparkRestoredAt}; pendingWallet=${d.backupRepo().hasPendingWalletRestore()}; error=${result.exceptionOrNull()?.javaClass?.simpleName}"
+                }
+                "qa1686SparkStart" -> {
+                    d.spark().start(0).getOrThrow()
+                    val info = d.sparkService().getInfo()
+                    "connected=true; balance=${info.balanceSats}; synced=${info.lastSyncedAt}; syncError=${info.lastSyncError?.javaClass?.simpleName}"
+                }
+                "qa1686SparkInfo" -> {
+                    val info = d.sparkService().getInfo()
+                    val cache = d.cache().data.first()
+                    "balance=${info.balanceSats}; shown=${d.spark().state.value.shownBalanceSats}; ready=${d.spark().state.value.isReady}; synced=${info.lastSyncedAt}; restore=${cache.sparkRestoredAt}; unseeded=${cache.sparkHistoryUnseededSince}; node=${d.lightning().lightningState.value.nodeLifecycleState}"
+                }
+                "qa1686SparkInvoice" -> d.sparkService().receiveBolt11(100_000uL, "review regtest funding", 3600u)
+                "qa1686SparkGuard" -> {
+                    val invoice = requireNotNull(arg)
+                    var admitted = false
+                    val before = d.sparkService().listPayments().size
+                    val result = d.spark().payInvoice(invoice, null, onBeforeSend = { admitted = true; false })
+                    val after = d.sparkService().listPayments().size
+                    check(before == after)
+                    "admitted=$admitted; error=${result.exceptionOrNull()?.javaClass?.simpleName}; noNewPayment=${before==after}; historySize=$after"
+                }
+                "qa1686NodeStop" -> {
+                    d.lightning().stop().getOrThrow()
+                    "node=${d.lightning().lightningState.value.nodeLifecycleState}"
+                }
+                "qa1686Balance" -> {
+                    d.wallet().syncBalances()
+                    val b = d.wallet().balanceState.value
+                    val spark = d.spark().state.value
+                    "spending=${b.totalLightningSats}; max=${b.maxSendLightningSats}; split=${b.sparkBalanceSats}; spark=${spark.balanceSats}; shown=${spark.shownBalanceSats}; node=${d.lightning().lightningState.value.nodeLifecycleState}"
+                }
+                "qa1686SparkExport" -> {
+                    val backup = requireNotNull(d.spark().exitBackup().getOrThrow())
+                    val result = d.spark().mergeRemoteExitBackup(backup).getOrThrow()
+                    val bytes = backup.exitState.toByteArray()
+                    "exitBytes=${bytes.size}; nativeImport=true; merged=${d.spark().hasMergedRemoteExitState}; digest=${java.security.MessageDigest.getInstance("SHA-256").digest(bytes).joinToString("") { "%02x".format(it) }}"
+                }
+                "qa1686Withdraw" -> {
+                    val endpoint = com.synonym.bitkitcore.PaykitEndpoint(MethodId.P2wpkh.rawValue, requireNotNull(arg))
+                    val route = d.route().route(20_000uL, listOf(endpoint), 0uL, 0uL, true).getOrThrow()
+                    val leg = (route as com.synonym.bitkitcore.PaykitRoute.Service).leg
+                    check(leg.deliveredSat == 20_000uL && leg.service == "spark")
+                    val result = d.route().payServiceLeg(leg, d.pubkyRepo().publicKey.value)
+                    val cache = d.cache().data.first()
+                    "result=${result.getOrNull()}; error=${result.exceptionOrNull()?.javaClass?.simpleName}; journal=${cache.pendingSparkWithdrawal != null}; sent=${cache.sentSparkWithdrawals.size}"
+                }
+                "qa1686WithdrawalState" -> {
+                    d.sparkService().syncWallet()
+                    d.spark().start(0).getOrThrow()
+                    val cache = d.cache().data.first()
+                    val sent = cache.sentSparkWithdrawals.lastOrNull()
+                    val activity = sent?.let { d.activity().getActivity(it.txid).getOrThrow() }
+                    "journal=${cache.pendingSparkWithdrawal != null}; sent=${cache.sentSparkWithdrawals.size}; txid=${sent?.txid}; amount=${sent?.amountSats}; activity=$activity"
+                }
+                "qa1686ExitState" -> {
+                    d.exit().load().getOrThrow()
+                    val e = d.exit().state.value
+                    val r = e.record
+                    val b = d.wallet().balanceState.value
+                    "phase=${r?.phase}; active=${e.isExiting}; funding=${e.fundingAddress}; seen=${e.fundingSeenSats}; transfer=${e.inTransferSats}; outside=${e.outsideExitSats}; error=${e.lastError}; destination=${r?.destination}; balanceSavings=${b.totalOnchainSats}; balanceSpending=${b.totalLightningSats}; balanceTransfer=${b.balanceInTransferToSavings}; split=${b.sparkBalanceSats}"
+                }
+                "qa1686ExitTick" -> {
+                    val result = d.exit().tick()
+                    d.wallet().syncBalances()
+                    val e = d.exit().state.value
+                    val b = d.wallet().balanceState.value
+                    "success=${result.isSuccess}; phase=${e.record?.phase}; active=${e.isExiting}; error=${e.lastError}; recovery=${e.record?.recovery?.transactions?.map { it.kind.toString() + ":" + it.status.toString() }}; balanceSavings=${b.totalOnchainSats}; balanceSpending=${b.totalLightningSats}; balanceTransfer=${b.balanceInTransferToSavings}; split=${b.sparkBalanceSats}"
+                }
+                "qa1686ExitMetadata" -> {
+                    d.backupRepo().stopObservingBackups()
+                    val store = SparkExitEarlierAddressStore(File(to.bitkit.env.Env.sparkExitEarlierAddressesPath(0)))
+                    val mnemonic = requireNotNull(d.keychain().loadString(Keychain.Key.BIP39_MNEMONIC.name))
+                    val passphrase = d.keychain().loadString(Keychain.Key.BIP39_PASSPHRASE.name)
+                    val path = SparkExitFundingKey.path(17, to.bitkit.env.Env.network)
+                    val address = d.core().onchain.deriveBitcoinAddress(mnemonic, path, to.bitkit.env.Env.network, passphrase).address
+                    val entry = SparkExitEarlierAddress(address, path, SparkExitEarlierAddress.Kind.KEPT_CHANGE, sats=600uL, atMillis=Clock.System.now().toEpochMilliseconds())
+                    store.add(entry)
+                    val key = SparkExitMath.walletKey(to.bitkit.env.Env.network, 0)
+                    val before = d.cache().data.first()
+                    val remoteIndex = maxOf(before.sparkExitNextFundingIndexes[key] ?: 0, 20)
+                    d.cache().update { it.copy(sparkExitNextFundingIndexes=it.sparkExitNextFundingIndexes + (key to remoteIndex)) }
+                    d.backupRepo().triggerBackup(BackupCategory.METADATA).getOrThrow()
+                    val remote = requireNotNull(d.vss().getObject("METADATA").getOrThrow())
+                    val envelope = json.decodeFromString<MetadataBackupV1>(remote.value.decodeToString())
+                    check(envelope.sparkExitEarlierAddresses?.get(key)?.any { it.address == address && it.path == path && it.kind == "keptChange" } == true)
+                    check(envelope.cache.sparkExitNextFundingIndexes[key] == remoteIndex)
+                    store.delete()
+                    val devicePath = SparkExitFundingKey.path(18, to.bitkit.env.Env.network)
+                    val deviceAddress = d.core().onchain.deriveBitcoinAddress(mnemonic, devicePath, to.bitkit.env.Env.network, passphrase).address
+                    store.add(SparkExitEarlierAddress(deviceAddress, devicePath, SparkExitEarlierAddress.Kind.CANCELLED_FUNDING, atMillis=Clock.System.now().toEpochMilliseconds()))
+                    d.cache().update { it.copy(sparkExitNextFundingIndexes=it.sparkExitNextFundingIndexes + (key to remoteIndex+2)) }
+                    val failures = d.backupRepo().performFullRestoreFromLatestBackup().getOrThrow()
+                    val restored = store.load()
+                    val cache = d.cache().data.first()
+                    check(BackupCategory.METADATA !in failures)
+                    check(restored.any { it.address == address } && restored.any { it.address == deviceAddress })
+                    check(restored.first { it.address == address }.sats == null)
+                    check(cache.sparkExitNextFundingIndexes[key] == remoteIndex+2)
+                    check(key in cache.sparkExitFundingRecoveryPending)
+                    "encryptedMetadataReadback=true; restoredEarlier=true; deviceEntryPreserved=true; cachedAmountDiscarded=true; backupIndex=$remoteIndex; restoredHigherIndex=${cache.sparkExitNextFundingIndexes[key]}; rescanPending=true; optionalFailures=$failures"
+                }
+                "qa1686ProfileStatus" -> {
+                    "identity=${d.pubkyRepo().publicKey.value != null}; authenticated=${d.pubkyRepo().isAuthenticated.value}; profile=${d.pubkyRepo().profile.value?.name}; loading=${d.pubkyRepo().isLoadingProfile.value}"
+                }
+                "qa1686Dev" -> { d.settings().update { it.copy(isDevModeEnabled = true) }; "enabled developer mode" }
+                "qa1686NativeAuth" -> {
+                    d.backupRepo().stopObservingBackups()
+                    d.vssClients().withSuspendedClients {}
+                    file("auth-override").writeText("hold")
+                    d.vssLdk().setup().getOrThrow()
+                    file("native-result").delete()
+                    scope.launch {
+                        try {
+                            val result = d.vssLdk().getObject("qa1686-held-auth")
+                            file("native-result").writeText("returned success=${result.isSuccess}; error=${result.exceptionOrNull()?.javaClass?.simpleName}")
+                        } catch(e: Exception) { file("native-result").writeText("cancelled=${e.javaClass.simpleName}") }
+                    }
+                    "scheduled actual standalone LDK lookup against held auth response"
+                }
+                "qa1686NativeState" -> "credentialsPresent=${d.keychain().exists(Keychain.Key.BIP39_MNEMONIC.name)}; wiping=${d.backupRepo().isWiping.value}; admissionOpen=${runCatching { d.vssClients().captureOwner() }.isSuccess}; result=${file("native-result").takeIf { it.exists() }?.readText()}"
+                "qa1686Stop" -> {
+                    d.backupRepo().stopObservingBackups()
+                    kotlinx.coroutines.delay(300)
+                    val statuses = d.cache().backupStatuses.first()
+                    "stopped; settings=${statuses[BackupCategory.SETTINGS]}"
+                }
+                "qa1686InitCore" -> { // test hook
+                    to.bitkit.async.ServiceQueue.CORE.background { // test hook
+                        com.synonym.bitkitcore.initDb(to.bitkit.env.Env.bitkitCoreStoragePath(0)) // test hook
+                        com.synonym.bitkitcore.updateBlocktankUrl(to.bitkit.env.Env.blocktankApiUrl) // test hook
+                    } // test hook
+                    "initialized real native database after app storage path became available" // test hook
+                } // test hook
+                "qa1686QuickState" -> {
+                    "source=${d.keychain().exists(Keychain.Key.SHARED_PUBKY_SOURCE.name)}, savedSecret=${!d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name).isNullOrBlank()}, savedSession=${!d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name).isNullOrBlank()}, identity=${d.pubkyRepo().publicKey.value != null}, authenticated=${d.pubkyRepo().isAuthenticated.value}, profile=${d.pubkyRepo().profile.value?.name}, contacts=${d.pubkyRepo().contacts.value.size}, pendingRestore=${d.backupRepo().hasPendingWalletRestore()}, node=${d.lightning().lightningState.value.nodeLifecycleState}"
+                }
+                "qa1686CreateProfile" -> {
+                    d.keychain().delete(Keychain.Key.PUBKY_SECRET_KEY.name)
+                    d.keychain().delete(Keychain.Key.PAYKIT_SESSION.name)
+                    d.pubkyRepo().initialize().getOrThrow()
+                    d.pubkyRepo().createIdentity("QA restore", "", emptyList(), emptyList(), null).getOrThrow()
+                    "created disposable profile"
+                }
+                "qa1686Backup" -> {
+                    for (c in listOf(BackupCategory.WALLET, BackupCategory.METADATA, BackupCategory.SETTINGS)) d.backupRepo().triggerBackup(c).getOrThrow()
+                    val wallet = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+                    val metadata = requireNotNull(d.vss().getObject("METADATA").getOrThrow())
+                    val pending = d.cache().backupStatuses.first().filterKeys { it in listOf(BackupCategory.WALLET, BackupCategory.METADATA) }.any { it.value.isRequired }
+                    check(!pending)
+                    "backup synchronized; sdk=${json.decodeFromString<WalletBackupV1>(wallet.value.decodeToString()).paykitSdkBackupState != null}; metadataBytes=${metadata.value.size}"
+                }
+                "qa1686Terminal" -> {
+                    d.backupRepo().stopObservingBackups()
+                    val secret = requireNotNull(d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name))
+                    File(context.filesDir, "qa1686-original-secret").writeText(secret)
+                    val session = requireNotNull(d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name))
+                    File(context.filesDir, "qa1686-original-session").writeText(session)
+                    d.pubkyService().forgetSessionAccess()
+                    d.keychain().delete(Keychain.Key.PUBKY_SECRET_KEY.name)
+                    d.keychain().upsertString(Keychain.Key.PAYKIT_SESSION.name, "qa1686-invalid-session")
+                    d.pubkyRepo().initialize().getOrThrow()
+                    check(d.pubkyRepo().publicKey.value == null)
+                    val before = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+                    val reserved = d.accounts().reserveAccountIndex(0, "qa1686-terminal-ordinary")
+                    check(d.accounts().backupSnapshot().allocationState.highestAccountIndexByWallet["0"] == reserved)
+                    val failed = d.backupRepo().triggerBackup(BackupCategory.WALLET)
+                    val after = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+                    val same = before.value.contentEquals(after.value) && before.version == after.version
+                    val pending = d.cache().backupStatuses.first()[BackupCategory.WALLET]?.isRequired == true
+                    check(failed.isFailure && same && pending)
+                    "terminal=true; ordinary watch-only index=$reserved reserved; failed=true; remote unchanged=true; pending=true; error=${failed.exceptionOrNull()?.javaClass?.simpleName}"
+                }
+                "qa1686RepairIdentity" -> {
+                    d.keychain().upsertString(Keychain.Key.PUBKY_SECRET_KEY.name, File(context.filesDir, "qa1686-original-secret").readText())
+                    d.keychain().upsertString(Keychain.Key.PAYKIT_SESSION.name, File(context.filesDir, "qa1686-original-session").readText())
+                    d.pubkyRepo().initialize().getOrThrow()
+                    d.backupRepo().triggerBackup(BackupCategory.WALLET).getOrThrow()
+                    val latest = requireNotNull(d.vss().getObject("WALLET").getOrThrow())
+                    val restored = json.decodeFromString<WalletBackupV1>(latest.value.decodeToString())
+                    check(restored.watchOnlyAccountAllocationState == d.accounts().backupSnapshot().allocationState)
+                    check(restored.paykitSdkBackupState != null)
+                    check(d.cache().backupStatuses.first()[BackupCategory.WALLET]?.isRequired == false)
+                    "recovered original identity; complete readback includes latest allocation; synchronized=true"
+                }
+                "qa1686Recover" -> { // test hook
+                    val secret = d.pubkyService().deriveSecretKey(requireNotNull(d.keychain().loadString(Keychain.Key.BIP39_MNEMONIC.name))) // test hook
+                    d.keychain().upsertString(Keychain.Key.PUBKY_SECRET_KEY.name, secret) // test hook
+                    d.pubkyService().initialize() // test hook
+                    d.pubkyRepo().restoreSessionBackupState(to.bitkit.models.PubkySessionBackupV1(kind = to.bitkit.models.PubkySessionBackupKind.LocalSeed)).getOrThrow() // test hook
+                    d.pubkyRepo().scheduleInitializationAfterRestore() // test hook
+                    d.backupRepo().triggerBackup(BackupCategory.WALLET).getOrThrow() // test hook
+                    d.backupRepo().startObservingBackups() // test hook
+                    "recovered identity and fresh complete backup" // test hook
+                } // test hook
+                "qa1686AddressGate" -> { // test hook
+                    val result = d.wallet().newAddress() // test hook
+                    "new address success=${result.isSuccess}; error=${result.exceptionOrNull()?.javaClass?.simpleName}" // test hook
+                } // test hook
+                "qa1686SignOut" -> { // test hook
+                    d.pubkyRepo().signOut().getOrThrow() // test hook
+                    "identity removed" // test hook
+                } // test hook
+                "qa1686DeleteProfile" -> { d.pubkyRepo().deleteProfile().getOrThrow(); "deleted disposable profile" }
+                else -> "source=${d.keychain().exists(Keychain.Key.SHARED_PUBKY_SOURCE.name)}, savedSecret=${!d.keychain().loadString(Keychain.Key.PUBKY_SECRET_KEY.name).isNullOrBlank()}, savedSession=${!d.keychain().loadString(Keychain.Key.PAYKIT_SESSION.name).isNullOrBlank()}, sdk=${d.pubkyService().currentPublicKey() != null}, identity=${d.pubkyRepo().publicKey.value != null}, profile=${d.pubkyRepo().profile.value?.name}, contacts=${d.pubkyRepo().contacts.value.size}, pendingRestore=${d.backupRepo().hasPendingWalletRestore()}, node=${d.lightning().lightningState.value.nodeLifecycleState}" // test hook
+            } // test hook
+        }.fold({ it }, { "ERROR: ${it.javaClass.simpleName}: ${it.message}" }) // test hook
+        Logger.info("QA1686 $method: $result", context="TestHook") // test hook
+        return Bundle().apply { putString("result", result) } // test hook
+    } // test hook
+} // test hook


Reviewed by gpt-6.1-sol-high

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Full reassessment of 43e314ca against merge base d28a1578 after the comparison base changed, with no inherited coverage. Covered the complete Android diff and affected callers. The iOS comparison and Android dependency at d28a157 were checked only for affected backup and session contracts, not reviewed in full.

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

Validation: Inspected the matching CI unit-test execution, including backup and profile tests, on the merge of these exact head/base revisions. Local targeted Gradle tests could not execute because native dependency downloads returned HTTP 401, including an authenticated retry. A focused JVM coroutine probe supported the reported failure mechanism; it was not an app test.

Suggested additional test cases

  • Android: in a disposable fixture, fail one required-status cache write after a currency change, restore storage, then change currency again without restarting observation. Expect the latest setting to become pending and upload successfully.

Device testing: not performed in this review. The authored lifecycle and outage cases require the dedicated fault fixture.

Earlier findings reconciled at this revision: 6 still present, 7 verified fixed, 1 not substantiated, 1 could not be verified, 1 tracked separately outside this PR.

Tracked separately (outside this PR; unresolved): Native LDK shutdown abort during VSS outage. The native shutdown abort remains unresolved in open same-repository issue #1536. This PR does not change the node stop/release path, its lifecycle callers, or the native LDK dependency. The inherited conditional background process-exit mitigation does not establish resolution of the native failure. This disposition is neither a fix nor a current-head runtime pass. This is not a runtime verification or merge-readiness declaration.

Reconciliation incomplete: the following earlier concerns remain unverified. This report does not establish that they are fixed or invalid, or establish approval or device readiness.

Could not verify earlier concern — Signed-in outage recovery encountered Pubky lock/HTTP 405 errors: The historical signed-in Pubky write-lock HTTP 405 outcome cannot be settled from the available source and reports. Capture still propagates SDK export failures and metadata remains gated behind required public-address exclusions. Transport is temporary, so these paths alone do not establish permanently stuck HTTP 405 recovery. The original native/server trace and a current reproduction are unavailable in this code-only run. Neither recurrence nor a causal fix is claimed.

Findings

  • [MEDIUM] Keep backup change listeners alive after a status-write failure — inline at app/src/main/java/to/bitkit/repositories/BackupRepo.kt:628.

Existing concerns

  • [HIGH] Terminal identity failure blocks newer ordinary WALLET backups — Complete WALLET capture still rejects unavailable Paykit state while a stored identity exists, before capturing ordinary allocations. Failed restoration preserves that identity. The guard and identity providers match the supplied base; changed scheduling still reaches the same guard. Introduction or worsening and material new update evidence were not established. Retain the checked existing concern without another inline report. Current-head runtime claims remain attributed external evidence.
  • [MEDIUM] Retry cannot refresh a failed pubky profile delete — Explicit Retry still repeats the whole deletion without recovery of the failed authenticated step. Direct Pubky profile deletion propagates Identity errors; the remaining refresh helper serves other profile writes. The original thread remains open, and its author reply acknowledges this limitation while explaining why broad replay would be unsafe. Retain that existing thread without another inline report: no material new update evidence was established. A safe continuation must preserve the original sign-in and avoid repeating acknowledged cleanup; Identity alone does not establish a pre-dispatch refusal.
  • [MEDIUM] Reset waits for stalled native authentication — Reset still closes admission and waits for admitted native operations before clearing credentials. A held native authentication operation can therefore keep UI Reset pending until it drains. The coordinator and production wipe callers match the supplied base; introduction or worsening is not established. Retain the existing same-PR concern without another inline report or invented update evidence. The exact-head timing in review 5885321004 remains attributed external evidence.
  • [MEDIUM] A persisted Running backup hides Retry until node observation starts — The earlier review also notes that process death can leave a category Running without Retry until node startup. Current UI still trusts the persisted flag, and stale-flag clearing starts with node-dependent observation. Retain that earlier limitation as an existing concern, separately from the repaired explicit setup wait. No introduced/worsened caller or material new update evidence was established for another inline report.
  • [MEDIUM] A resolved Spark request can return to non-observing Pending — The earlier request-race report still matches current source. Resolution is consumed before checking request registration, while service-leg failure suspends in recovery reads before registering that request. An older Unknown result can then open non-observing Pending after the sole resolution was consumed. The affected code and callers match the supplied base. Retain the existing same-PR concern without duplicate inline publication; introduction/worsening and material new update evidence were not established.
  • [MEDIUM] An unselected invoice rejects the selected Spark request price — The earlier selected-price concern still matches current source. A retained Lightning invoice contributes fixedLightningAmount while the Spark service leg uses LIGHTNING as its funding method. Validation compares that unselected invoice against the selected onchain amount and rejects before dispatch. The affected code and callers match the supplied base. Retain the existing report without another inline finding; no introduction/worsening or independently verified material update was established.

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

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⛔️ Request changes — an aborted reset strands retained backup changes; the existing failure checks also remain red.

Findings: 1 new MEDIUM — Retain failed status marks after an aborted wipe, app/src/main/java/to/bitkit/repositories/BackupRepo.kt:677. The real wipe advances native ownership before the new reset refusal. Restart then filters out the retained old-owner mark, leaving the consumed edit unrecorded after storage recovers. A real-coordinator regression and the device case below reproduce it.

QA for 9330e24: Android 16 / API 36 ARM64 emulator, disposable unfunded regtest wallet and native encrypted staging VSS. I built the pristine head and Debug control builds. Installed APK hashes match their builds, and all eleven native libraries match the pristine APK. Recordings play at 2×. Nonvisual assertions were checked through real DataStore, native VSS readback and timed control output.

Recover pending settings after an aborted wallet reset ❌ After reset refused and native ownership reopened, observation left the edit unuploaded for 70 seconds despite recovered storage; explicit Retry uploaded NOK.

Injected a SETTINGS required-status persistence error after consuming NOK. The actual wipe use case returned BackupStatusWritePending, retained credentials and reopened native owner 1 as owner 2 while the node remained running. After storage recovery and observation restart, no SETTINGS PUT occurred for 70 seconds and required == synced == 1000. Explicit Retry then immediately uploaded NOK, verified by native decrypted readback. This invokes the real wipe use case through a Debug control, rather than the Reset button.

aborted-wipe2x.2x.mp4

Retry a failed settings status write and keep observing ✅ Periodic retry uploaded EUR after one injected status-write failure, and a later GBP edit uploaded without restarting observation.

One injected IOException left the consumed EUR edit initially unmarked. The normal one-minute check retried it without another mutation; native readback contained EUR. A later GBP edit uploaded with the same observation still active.

storage-live2x.2x.mp4

Retain a failed settings status write across observer restart ✅ Restart replayed the failed consumed mark and native readback contained CHF without another edit.

Kept the required write failing, consumed CHF, stopped observation, recovered storage and restarted. Startup flushing uploaded CHF without another edit, verified through native decrypted SETTINGS.

storage-restart2x.2x.mp4

Drain a held required write before replacement uploads ✅ Replacement observation drained the held marker before uploading CAD, confirmed by native readback.

Held the consumed CAD mark inside the real DataStore transform, then stopped and restarted observation. Replacement uploads waited for release; native decrypted readback then contained CAD.

held-write2x.2x.mp4

Keep a newer failed mark after an older write returns ✅ An older completion preserved a newer failed mark and native readback contained AUD without another mutation.

Held the return of an older committed JPY mark, restarted observation and failed a newer AUD mark. Returning the older write did not erase the newer requirement; AUD uploaded without another edit. The initial control mistakenly held both the marker and the later upload-status write; that setup attempt was retained, the hold was corrected to claim one write, and this case was rerun.

newer-mark2x.2x.mp4

Back up ordinary changes after terminal identity failure ❌ A new ordinary allocation remained pending until identity repair enabled its complete WALLET upload, verified by native readback.

Invalidated the disposable saved identity while preserving its prior complete remote WALLET, then reserved a new ordinary account. Capture failed, the acknowledged remote object stayed unchanged and the allocation remained pending. Repairing the original identity allowed a complete upload whose decrypted readback contained the new allocation. This corroborates the existing complete-WALLET concern; it is not another inline finding.

terminal-wallet2x.2x.mp4

Finish Reset while native authentication is held ❌ Reset remained pending for at least 203.2 seconds with credentials retained; it completed 13.7 seconds after releasing the native authentication response.

Used the actual Settings Reset action while a real native LDK lookup waited for an authentication response. Timed readback kept credentials present, wiping active and admission closed throughout the hold. Releasing that response let reset finish. This rechecks the existing native-authentication reset concern.

stalled-native-auth2x.2x.mp4

Check: Keep a resolved Spark request out of Pending ❌ The component regression still returned non-observing Pending after consuming the sole completion.

Reapplied the previous deterministic request regression. The sole resolution is consumed during a suspended outcome read; releasing the stale outcome still yields NavigateToPending(observeResolution=false) instead of PaymentSuccess. This is a real ViewModel component check with mocked repositories.

Check: Use the selected Spark request price ❌ The component regression still rejected valid selected onchain terms against an unselected Lightning invoice.

Reapplied the previous selected-price regression. Valid selected onchain terms still emit PaymentFailedToast because validation retains a differently priced unselected Lightning invoice. This is a component check with mocked repositories; no new device reproduction is claimed.

Build and scope: Pristine assembleDevDebug and the focused BackupRepoTest suite passed. The additional aborted-transition regression fails at the expected missing SETTINGS PUT; both repeated Spark request checks fail as described above. Full-project unit, lint and Compose suites were not repeated.

Retained evidence: The backup report at e0d2598 retains its bounded stopped-observer retry, immutable recovery callback, cold-read and Spark restore-gate results. Those paths and dependency pins are unchanged; fresh controls above exercise the changed required-mark plumbing through SETTINGS. The previous report at 43e314c retains its Spark exit and metadata-restoration passes. Their original signed-operation, submission-counter and recovery-phrase limits remain; this pass adds no funded payment, settlement or complete wallet-restore claim. The explicit deletion-Retry disposition remains unchanged.

Reviewed contiguous 43e314c..9330e24, both changed files and earlier open concerns. Every planned case ran. Other categories use the same marker plumbing but did not receive a new storage-fault device run.

Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle, inspected at b6306aaa, matches draining and independent explicit persistence. Its serialized synchronous UserDefaults markers have no Android DataStore pending-write queue or equivalent native-generation filter. No new iOS runtime claim is made.

Diagnostic controls

Local Debug controls inject an IOException before the real required-status commit, hold a single cache write or its return, invoke real repositories/use cases, and display nonsecret results. VSS hooks log entry and acknowledgement without replacing native results. The authentication control holds and then forwards the real response. No backup admission, revision, retry or acknowledgement decision was patched. The component regression wraps the existing failed-mark setup in vssClients.withSuspendedClients(shutdown = {}) { sut.reset() }, recovers storage, restarts observation and expects a SETTINGS PUT.


Reviewed by gpt-6.1-sol-high

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Follow-up review of 9330e246 since 43e314ca, against merge base d28a157. Inherited baseline: review 5889222013. This pass inspected the required-mark retry delta and the wallet-wipe path it now refuses; unchanged baseline surfaces stay inherited.

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

The new queue retries a failed required-mark write for the same observation owner, including across stop and start. BackupRepo.reset now throws while such a mark is pending, and wallet wipe is the only production caller of that reset. It runs after VSS clients are suspended and before the node stops or local data is deleted. With the node stopped, later wipe attempts in the same process keep failing because nothing adopts a new owner or retries the write.

Compared only this refusal with bitkit-ios b6306aaa. That revision does not gate wipe on an unpersisted required mark. This is not a review of the twin.

Validation was source inspection of the pinned commits. BackupRepoTest was not executed. Device testing: not performed in this review.

Suggested additional test cases:

  • Android, node stopped: fail one backup-status write after a settings change, then confirm wallet reset. Expected: this process removes the local wallet. A second reset is not required, and the toast does not stay on "Backup changes could not be recorded".
  • Android, node running: the same failed write, then one reset confirmation. Expected: the first wipe completes.

Earlier findings reconciled at this revision: 6 still present, 1 verified fixed, 1 duplicate, 1 could not be verified, 1 tracked separately outside this PR.

Tracked separately (outside this PR; unresolved): Native LDK shutdown abort during VSS outage. The reported VSS-outage native shutdown abort remains the open ldk-node shutdown failure in #1536. From base d28a157 to head 9330e24 this PR does not change LightningService.stop, the ldk-node-android 0.7.0-rc.80 pin, or native LDK sources. The inherited background process-exit branch is already in that unchanged stop path and does not record a fix. The issue stays open. This stage performed no native execution and does not treat the issue as a fix or a current-head pass. This is not a runtime verification or merge-readiness declaration.

Reconciliation incomplete: the following earlier concerns remain unverified. This report does not establish that they are fixed or invalid, or establish approval or device readiness.

Could not verify earlier concern — Signed-in outage recovery encountered Pubky lock/HTTP 405 errors: The HTTP 405 Pubky write-lock failure is an attributed device report at 9cfe052. A later attributed signed-in repeat at a3693b0 completed WALLET, METADATA, and SETTINGS and said that repeat does not explain the earlier HTTP 405 or identify a code fix. Current source still propagates Paykit snapshot failures and still refuses metadata replacement while a required WALLET backup has quarantined public addresses. Paykit Transport remains temporary, and the AuthException classifier matches only a VSS request-send string, so neither path shows a stuck HTTP 405. No current reproduction, native trace, or exact tracking issue was available in this code-only pass, so recurrence and resolution stay unverified.

Findings

  • [HIGH] Unpersisted backup mark blocks wallet wipe — inline at app/src/main/java/to/bitkit/repositories/BackupRepo.kt:282.

Existing concerns

  • [HIGH] Terminal identity failure blocks newer ordinary WALLET backups — Complete WALLET capture still rejects a null Paykit snapshot while a stored identity exists, before ordinary wallet bytes are built. Failed restoration still keeps that identity and clears the in-memory key the snapshot needs. The guard and identity providers match the supplied base, and the WALLET upload path still reaches the guard. This PR does not introduce or worsen it, and no same-repository issue states this mechanism. The existing discussion stays the report. The current-head device note is attributed and was not run here.
  • [MEDIUM] Retry cannot refresh a failed pubky profile delete — Explicit Retry still repeats the whole deletion without recovering a failed authenticated pubky profile delete. Direct deletion propagates an Identity error, and the remaining refresh helper serves other profile writes. The original thread remains unresolved, and its author reply acknowledges this limitation. The follow-up from 43e314c to this head does not change PubkyRepo.kt, so there is no material new update evidence for another inline report.
  • [MEDIUM] Reset waits for stalled native authentication — Reset still closes VSS admission and waits for admitted native operations before the wipe body continues. A held native authentication operation can therefore keep Reset pending until that operation drains. The coordinator, LDK client admission, and production wipe caller match the supplied base, so introduction or worsening is not established. The existing discussion stays the report. The current-head timing is attributed and was not run here.
  • [MEDIUM] A persisted Running backup hides Retry until node observation starts — A persisted Running backup still hides Retry until node observation starts and clears that flag. The row predicate, the node-gated observation start, and the stale-flag clear match the supplied base. No introduced or worsened caller and no material new update evidence were established. The existing review remains the report.
  • [MEDIUM] A resolved Spark request can return to non-observing Pending — A resolved Spark request can still be consumed before this send registers for it, and the service-leg failure path still suspends in the outcome read before opening non-observing Pending. AppViewModel.kt matches the supplied base. The existing review remains the report. The current-head component recheck is attributed and was not run here.
  • [MEDIUM] An unselected invoice rejects the selected Spark request price — Validation still compares a retained Lightning invoice amount with the selected amount while the payment method remains Lightning, including when Spending selects a Spark service leg. AppViewModel.kt matches the supplied base. The existing review remains the report. The current-head component recheck is attributed and was not run here.

Reviewer model: grok-4.7
Reasoning effort: high

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⛔️ Request changes — cancelling a wipe can leave the retained wallet without automatic backups.

Findings:

  • MEDIUM — Resume backup observation after a cancelled local wipe drain (app/src/main/java/to/bitkit/usecases/WipeWalletUseCase.kt:68). Reset now clears and cancels observation before native retirement. If its caller is cancelled during that drain, cancellation skips the failure handler that restarts observation. I reproduced a later GBP edit staying falsely synchronized while native readback still contained EUR; explicit Retry recovered it. The focused regression fails here and passes with the previous wipe ordering restored locally. A route-scoped Dev Settings wipe can be cancelled by leaving the screen.

QA for 7fac7ab:

Retain backup observation when reset is cancelled during local drain ❌ The later edit stayed falsely synchronized for 68 seconds; explicit Retry recovered it.

Held the return of a committed, already uploaded EUR marker, invoked the actual wipe use case and cancelled its caller. After release, the node remained Running, native ownership and credentials were retained, and observation was off. GBP caused no SETTINGS PUT for 68 seconds; required and synced stayed equal. Native readback remained EUR until explicit Retry uploaded GBP. This uses Debug cancellation controls; navigation away was verified as a cancellable production call path by source inspection.

cancelled-drain2x.2x.mp4

Recover pending settings after an aborted wallet reset ✅ Ordinary wipe failure retained its owner and automatic native readback recovered NOK after storage returned.

Kept required-status writes failing, consumed NOK, then invoked the real wipe. BackupStatusWritePending kept credentials and the same native owner. The running-wallet failure handler restarted observation; after storage recovered, periodic retry uploaded NOK without another edit. Native decrypted SETTINGS matched. This verifies the earlier aborted-transition finding is fixed for ordinary failure.

aborted-wipe2x.2x.mp4

Retry wallet reset after storage recovers with the node stopped ✅ After storage recovered, the next Reset removed the stopped wallet without a process restart.

The first Reset retained the wallet while injected storage failure continued. After storage recovered, the next Reset retried the queued marker and deleted the disposable wallet without restarting observation or the process. Continued storage failure still refuses Reset, as stated in the author’s reply; the recovered-storage retry path is verified.

stopped-wipe2x.2x.mp4

Finish wallet reset after one transient marker failure ✅ The first Reset retried the single failed marker write and removed the wallet.

One required-status IOException left a consumed edit queued. The first actual Reset retried that marker successfully and removed the local wallet; no second Reset was needed.

one-shot-wipe2x.2x.mp4

Retain a signed USDT operation across cancelled reset ✅ Native traces and fixture RPC counters confirmed the retained signed plan was backed up before one submission.

Manual 2/3: a real native signed operation waited at recovery upload. Actual Reset cancelled that work and waited for its held cleanup; cancelling Reset did not retire the native owner or erase credentials. Full, manual and USDT requests during the hold made no new PUT. The original callback received no acknowledgement. Foreground native refresh retained the same signed-plan and snapshot digests, obtained its own VSS acknowledgement, then submitted once. The fixture RPC submission counter was 0 → 0 → 1; native decrypted VSS record digests matched the upload. This proves cancellation and recovery of the signed operation, without a settlement or recovery-phrase restore claim.

signed-reset2x.2x.mp4

Back up ordinary changes after terminal identity failure ❌ Native readback and pending-status checks found the new allocation backed up only after identity repair.

The existing complete-WALLET concern still reproduces. Invalidated the disposable saved identity, then reserved a new ordinary account. Capture failed, the prior complete remote object stayed unchanged and the allocation remained pending. Repairing the original identity enabled a complete upload whose native readback contained the allocation. Initial local-seed authentication setup failed; creating a disposable profile and waiting for synchronization established the valid baseline before this case. Device control readbacks were failed=true; remote unchanged=true; pending=true before repair and complete readback includes latest allocation; synchronized=true after repair.

terminal-wallet2x.2x.mp4

Finish Reset while native authentication is held ❌ Timed state readbacks found Reset pending for 205.5 seconds; releasing authentication let it finish.

Rechecked the existing reset concern through the actual Settings Reset confirmation. A real standalone LDK authentication response was held and later forwarded. Timed state readbacks checked retained credentials and closed VSS admission throughout the hold; the recording joins two segments.

stalled-native-auth2x.2x.mp4

Check: Keep a resolved Spark request out of Pending ❌ The sole completion was consumed before a stale result reopened non-observing Pending.

Reapplied the existing deterministic ViewModel regression. Consuming the sole completion during a suspended outcome read still ends in NavigateToPending(observeResolution=false) instead of PaymentSuccess. Repositories are mocked; this is a component check.

Check: Use the selected Spark request price ❌ An unselected Lightning invoice still rejected valid selected onchain terms.

Reapplied the existing selected-price regression. Valid selected onchain terms still produce PaymentFailedToast because a differently priced unselected Lightning invoice is retained. Repositories are mocked; this is a component check.

Retained coverage: The 9330e24 report retains its passing status-write retry, observer restart, held-write and newer-mark cases. The e0d2598 report retains stopped-observer retry, immutable recovery callback, cold-read and Spark restore-gate results. The 43e314c report retains Spark exit and metadata-restoration passes. Those paths and dependency pins are unchanged by this delta. Earlier failed cases were rerun above; a historical failure is not counted as a pass. The explicit deletion-Retry disposition remains unchanged.

Build and limits: Built pristine assembleDevDebug and ran the affected BackupRepoTest, WalletConnectionRecoveryTest and WipeWalletUseCaseTest suites successfully. The added cancellation regression and both inherited Spark request checks fail as described. Full-project unit, lint and Compose suites were not repeated. The device build adds disclosed Debug fault gates, event logging and native readback controls; native libraries match the pristine build. No backup scheduling, owner, retry, revision or acknowledgement decision was patched in the tested app. The comparison with the prior wipe ordering was a separate component control only. No planned case remains unrun.

Reviewed contiguous 9330e24..7fac7ab, all five changed files, reached callers and earlier open concerns. The new generic identity-transition cancellation test has component coverage; other identity-change callers and a complete wallet recovery-phrase restore were not freshly driven. The unrelated native shutdown issue remains separate.

Twin: bitkit-ios#1123 fix: serialize backup observation lifecycle at b6306aaa also drains backups before VSS retirement. Its synchronous UserDefaults markers have no equivalent pending DataStore-mark queue; this retry repair is Android-specific. Source comparison only.


Reviewed by gpt-6.1-sol-high

val result = try {
runSuspendCatching {
connectionRecovery.withIdentityChange {
connectionRecovery.withIdentityChange(beforeClientsSuspend = { backupRepo.reset() }) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

MEDIUM: Resume backup observation after a cancelled local wipe drain.

If the wipe caller is cancelled while reset drains a consumed required-status write, reset has already cancelled and cleared observation. Cancellation propagates past result.onFailure, so observation stays stopped until another node-state emission restarts it. A route-scoped caller such as DevSettingsViewModel can be cancelled by navigating away. On the installed app I held the return of an already-acknowledged EUR marker, cancelled the actual wipe and then changed currency to GBP: the node stayed Running, observation stayed off, required/synced stayed equal, and native readback still contained EUR after 68 seconds. Explicit Retry uploaded GBP. The focused use-case/coordinator regression also fails at the missing PUT and passes with the previous wipe ordering restored locally. Rearm observation for a retained running wallet on cancellation as well as ordinary failure.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants