Skip to content

fix: bind profile removal to its original owner - #1693

Open
ben-kaufman wants to merge 25 commits into
fix/profile-cleanup-outcomesfrom
fix/profile-removal-ownership
Open

ben-kaufman wants to merge 25 commits into
fix/profile-cleanup-outcomesfrom
fix/profile-removal-ownership

Conversation

@ben-kaufman

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

Copy link
Copy Markdown
Contributor

Final subscription admission parent composition — November 4

Signed additive merge c228adb7 incorporates live parent #1690 at 8ceaca25550291d6975291e2ca2309bfb5a81356. The parent's final active-subscription check is retained in the legacy profile-delete entry point. This child's actual snapshot deletion path already performs that check under its SDK lock; its original identity/revision checks, acknowledgement callback, coordinator, cancellation drains and conditional local erasure are byte-preserved. No parent parameterless path replaces the snapshot API.

The inherited no-contact refusal test is adapted to the snapshot callback overload and verifies no grant logout or credential deletion. Two existing cancellation/drain fixtures now supply empty subscriptions so they reach their original boundaries; all assertions are unchanged. Their initial two fixture failures remain preserved. Corrected focused validation passed 389 cases. Final just format, just compile, full just test and just lint passed on 1,822 identical inputs: 5,157 tests /270 suites, zero failures/errors/skips, and eight fresh static reports with zero findings. Complete logs, case identities, nested XML/HTML, SARIF and Gradle diagnostics were inspected; verified parent/old-child warnings remain documented. The two parent risk reviews are reused for identical guard behavior, with one clean independent child composition review. All 25 live-parent commits verify locally and on GitHub; draft=false and the genuine stack are preserved.

New-head CI, lint and E2E passed and are fully inspected. Synthetic checkout 49461854d446f8ad281d1ba318193caaa6f3f625 exactly matches this published tree: 5,157 unit tests, 274 Compose cases and 39 E2E cases passed, with one explicit Settings skip. All seven E2E shards passed their first configured attempt; one recipient-input typing helper retried internally. Six static reports have zero findings; six migration jobs were skipped. All 713 sealed evidence files were hash-verified, covering 16 complete job logs, ten diagnostic archives and nested runtime reports. The onchain log retains an app-targeted native startup SIGABRT: Android failed to mount the app data directory while the package was frozen during force-stop/package removal. A later process launched and displayed MainActivity, but that progress does not prove harmlessness or establish the cause. An initial-sync wallet-operation timeout also remains unexplained; matching source bytes do not establish whether it is newly introduced. The external E2E suite's 82 npm advisories per install remain disclosed. These are passing test results with retained runtime findings. These composed JVM checks do not add authenticated native cleanup, literal wallet wipe, running-LDK quiescence or complete-WALLET acceptance. Prior runtime cases, exceptions, failures and original-grant retry constraints below retain their revision and fixture scope.

Evidence: work/validation/android1693-late-subscription-parent-final-audit-20261104/report.md (1,430 verified files), work/android1693-late-subscription-composition-review-20261104.md and work/validation/android1693-late-subscription-parent-20261104/publication.json.

Additional external review: 5774266996 approves exact head c228adb7 and reports unfunded API36 pending-wipe recovery, post-erasure refusal/retry, admitted-operation draining and a real native VSS write whose successful response was held during reset, followed by a new-wallet backup. It distinguishes pristine app checks from diagnostic controls and limits the native case to an accepted write with a delayed response. Raw install, proxy and storage artifacts were not independently inspected by the author. These are attributed external cases, not author-run reset acceptance or a human approval. Pre-commit timing variants, actual native shutdown quiescence and terminal complete-WALLET availability remain separate.

Reset-completion routing correction — November 4

Signed additive head 32fc96f778d1c1620d84feebc80afcb536448b3c incorporates current parent #1690 at b2c4b9e9fd8c919515214976111ca62893c8bb8a and fixes a reachable reset-completion state race. When durable recovery becomes Clear, the repository reads current credential presence and publishes both values together. An unreadable presence check retains the prior presence and publishes Unavailable, with explicit refresh still reachable. The completion callback preserves this pair while clearing ordinary wallet fields, and the activity takes both routing inputs from the same WalletState emission. The UI therefore cannot reopen the wallet through the repaired stale-presence/Clear transition. Durable completion ordering, cancellation drains, original grant/owner checks and conditional credential erasure remain unchanged.

Two focused regressions fail against the old behavior and pass with the correction; they cover confirmed credential absence and unreadable presence through the completion callback and later refresh. These are repository/store controls, not a literal device wipe. The inherited parent also fixes queued-backup Running status and rejects unadmitted replacement reservations. All previous child-specific cleanup behavior is retained. Two affected recovery reviews and the narrow composed-caller review are clean; formatting-only corrections preserve the reviewed logic.

Final just format, just compile, full just test and just lint passed on 1,822 identical inputs: 5,154 tests across 270 suites, zero failures/errors/skips, with zero static findings. Complete logs and structured diagnostics were inspected, including nested test output and Gradle reports. Four cursor-close chains and three JVM notices match the actual parent; the retained Gradle deprecation is pre-existing, and copied historical reports are not counted as fresh execution. Both failed formatting attempts remain recorded. The owner verified 1,568 sealed evidence files. All 24 commits against the live parent verify locally and on GitHub; history and draft=false are preserved.

Hosted CI, Compose, lint and E2E at 32fc96f7 are now fully inspected: 5,154 unit passes, 274 Compose passes and 39 first-attempt E2E passes, with one explicit Settings skip and zero static findings. All 1,012 sealed evidence files verify. No MissingMnemonic marker appears in the captured current logs; that absence does not establish the exact historical caller or every startup ordering. Six unmatched runtime records (restore wait, BLOCKTANK-null, cancellation and on-chain sync timeout) remain unresolved and preserved. Evidence: work/validation/hosted-android1693-32fc-20261104/report.md. Prior 051f60fb hosted results were fully inspected: 5,149 unit passes, 274 Compose passes, 39 first-attempt E2E passes and one explicit Settings skip, with zero static findings. That older run's post-wipe MissingMnemonic observation is retained; the exact initiating caller was not proven. The source race repaired here is independently demonstrated, rather than inferred solely from that log. Neither the local controls nor a parent composition establish a new authenticated removal/reset campaign, running-LDK storage quiescence or terminal complete-WALLET recovery. Existing authored cases, fixture limits and earlier failures remain below.

Evidence: work/validation/android1693-reset-clear-final-audit-20261104/report.md, work/android1693-reset-clear-composition-review-20261104.md and work/validation/android1693-reset-clear-20261104/publication.json.

VSS ownership composed with profile removal — November 4

Signed additive merge 051f60fbb58a11aadbacaf0e273b07d371b9d0d9 incorporates live parent #1690 at 84c803d82aa37a6155764b04a98adf242d588f28. The composition retains original receipt-owner cancellation and worker drains, then suspends and drains VSS setup/operations before the existing profile and SDK wipe barriers. Lightning startup retains the profile-removal availability guard inside the parent’s original-owner admission. The existing original grant, stage acknowledgements, conditional local erase and read-only Retry behavior are preserved.

All 103 non-overlapping child files remain byte-identical. The six overlapping production/test files retain both parent and child behavior; no test method was removed. Two independent reviews of the combined caller and security/concurrency paths found no new issue. Final just format, just compile, full just test and just lint passed on all 1,821 unchanged inputs: 5,149 tests across 270 suites, zero failures/errors/skips and zero static findings. The 331 conflict-affected cases also passed before the full gate. The final compile task reused the identical compilation from that focused run; this is recorded task reuse, not a second fresh compile.

Complete local logs, XML/HTML, SARIF and nested diagnostics were inspected. Existing cursor, JVM and Gradle observations and all historical failures remain recorded. The signed additive merge preserves published history; all 23 commits in the live-parent range are verified locally and on GitHub. The owner verified all 1,873 sealed local evidence files. Evidence: work/validation/android1693-vss-parent-final-audit-20261104/.

New-head hosted CI is pending. Prior app journey and hosted evidence below remains bound to its stated revision and fixture; this parent composition is not a new authenticated removal/reset campaign, native LDK-quiescence proof or complete-WALLET recovery claim. Existing acceptance limits remain unchanged.

Read-only Retry remains reachable — November 3

Signed 3b3b22620cf934d84a113259826bffd53ea812dd keeps Retry visible for unreadable recovery state even when the PIN-protected phrase route is available. It calls the existing read-only refresh and preserves the busy, confirmation, PIN and required-recovery gates. The Android-specific recovery journey now includes this path; its screen-map entry remains todo (no design available).

Required final just format, just compile, just test and just lint passed on all 1,819 matching inputs: 5,135 unit passes, zero failures/skips and zero static findings. The independent affected review is clean. All 22 live-base commits verify locally and on GitHub against #1690 2e79e109.

On the ordinary final APK (59294cc9), the existing disposable unreadable-marker/PIN fixture showed Retry alongside Show Seed Phrase. One Retry tap preserved the restricted screen, and a confirmed cold restart retained it. No phrase, PIN entry, reset or remote removal was attempted. The successful attempt changed only the verified cachedRates field; two earlier attempts stopped before tapping, and one earlier cache change remains unattributed. Original APKs and routes were restored.

The separate callback test passed once with a private test target that disables only the unrelated widget receiver; every other APK entry matches the ordinary target, and the test APK is unchanged. The stock target's earlier Hilt/widget startup crash executed no tests and is retained. The component run restored both original APKs, all 18 selected file hashes and nine routes. These finite results do not complete wider destructive or authenticated recovery campaigns. Complete audits: work/validation/android1693-unreadable-retry-final-audit-20261103, android1693-unreadable-retry-runtime-audit-20261103 and android1693-retry-component-runtime-audit-20261103. The current-head hosted inspection below is complete.

Hosted validation at 3b3b2262 — November 3

CI, lint, and E2E passed on the exact published tree against live #1690 2e79e109. 5,135 units, 274 stock Compose cases (including the changed Retry callback), and 39 E2E cases passed. Six static reports have zero findings, including ignored-failure output checks. One Settings case was explicitly skipped; migration jobs were also skipped. All seven E2E shards passed on their first workflow attempt, with one Send UI action retry retained.

The complete 16 job logs, structured results, 313 nested logs and ten diagnostic archives were inspected; all 853 sealed evidence payloads verify. Both APK archives are bound to producer metadata; all seven E2E consumers verified the E2E APK digest. The APK archives were not independently downloaded in this audit. Existing emulator/native diagnostics and runner-only Gradle report limitations remain recorded. These selected suites do not establish the unrun broader recovery cases below. Evidence: work/validation/hosted-android1693-3b3b-20261103/STRUCTURED-REPORT.json.

Receipt admission and current parent composition — November 3

Signed a5cd94f1a49e1d1f22122782f7de4174845be619 incorporates live #1690/2e79e109 and fixes Piotr's receipt-processing finding. Activity sync now validates the indexed wallet binding once inside the admitted operation. Per-receipt checks use the original caller's Job, active session and owner generation. Identity replacement still revokes admission and drains the actual Core worker before changing credentials; final writes retain their atomic generation check. Ordinary connection/restore credential checks are unchanged.

A controlled regression failed before repair on repeated binding-provider calls and passed afterward. It checks 200 receipt validations across each of two operations, with one provider invocation per operation; unavailable credentials still refuse before Core work. This is a provider-call count, not a device timing benchmark or a claim about every internal cold-cache derivation. The backfill replacement regression now enters the actual sibling withIdentityChange boundary, verifies replacement waits, and proves the cancelled receipt cannot write.

The final composed inputs passed required formatting, compile, 5,135 tests / 269 suites, zero failures/errors/skips, and complete static reports with zero findings. All 1,819 frozen hashes match. Final compile reused matching up-to-date tasks; full tests executed. Two independent receipt concurrency/security reviews, the affected parent-composition review and the affected test-fixture review are clean. Complete logs, XML/HTML, SARIF and ignoreFailures diagnostics were inspected. The initial full run's one fixture failure and the formatter's removed unused import remain preserved. Four CloseGuard observations, JVM notices and stale copied report exclusions retain their precise provenance; they are not declared harmless.

All 21 commits against live parent 2e79e109 verify locally and on GitHub (verified=true, reason=valid). Current-head hosted CI, lint and E2E passed: 5,135 unit tests, 274 Compose tests, zero static findings and 39 selected E2E cases. All seven whole-shard first attempts passed; three existing send-helper UI-action retries are retained separately. Six migration jobs were skipped. The actual synthetic tree exactly matches this published tree. Complete logs, canonical nested Compose protobufs and twelve digest/CRC-verified artifacts were inspected; all 812 sealed evidence members verified. Hosted Gradle Problems/configuration-cache HTML and unit raw XML were not uploaded; the audit uses the actual available logs and exact unit HTML/log reconciliation without substituting local reports. Background network/bootstrap, stopped-node, fixture initialization and emulator warnings remain recorded. Evidence: work/validation/hosted-android1693-a5cd-20261103/ and work/validation/android1693-master44bf-composition-20261103/.

Final-app fixed-term local recovery — November 3

The configured final ARM64 app at a5cd94f1 (installed APK SHA-256 4b4ef34f241fd856b4d3e9c5f38353d6400a719463f649028bf66723e86c0ed6, published Paykit RC4) completed the finite refusal → local Disconnect Cancel → fresh confirmation → cold restart case on a separate unfunded Android user. The fixture used genuine app authorization and native encrypted peer linking; its RC3 companion created one future-dated, unpaid Payer subscription. Native fixture acceptance is distinct from app subscription-acceptance UI.

  • Delete Profile refused the active fixed term and displayed its localized November 11 expiry. Cancel on the local Disconnect confirmation retained the profile, sharing flags and original 1,592-byte credential file.
  • A fresh local-only confirmation returned to profile onboarding, changed the credential file to 510 bytes, disabled private sharing and persisted the exact clear recovery marker. The original subscription remained ActiveRecurring with zero payment proofs on both native sides.
  • Cold restart remained disconnected, including a final UI/local/server snapshot 103 seconds after launch; logs continued for 134 seconds. All 12 selected server entries, 139 event rows and two unrevoked grants were identical from immediately before local confirmation through the final snapshot. Earlier background private publications are retained separately; this does not claim all server bytes were unchanged before Cancel or that no rejected HTTP request occurred.

The sole audit inspected the full 9,931-line app log, all actions/state comparisons and retained setup corrections. The owner verified 22 sealed evidence files plus 44 raw-input hashes; report eb37c918dfbb615e5ac922a1ea009b50338ee6ae855518fd8811bf7e0708af57. Existing users and fixture state were preserved, with no wallet reset or payment. Earlier wrong-ABI/package/authorization-wrapper attempts were rejected before the intended action and remain documented; the original native request was approved once and the subscription proposal was not repeated. Routing/RGS/setup/Ring-provider warnings remain recorded.

This completes only the fixed-term Payer local-recovery portion of J4. Full J4, Ring, remote Retry, unknown private outcomes, actual expiry, storage-fault variants and wallet reset remain separate and are not marked passed. Evidence: work/validation/android1693-fixedterm-final-app-20261103/inspection/ and work/fixtures/android1693-fixedterm-20261103/case-completed.json.

Previous-head hosted validation — November 2, 06:37 UTC

For 78f7209f98c16858987567a726bd7571d36fde8f, unit CI, static checks and E2E are complete and fully inspected. 5,085 unit tests / 266 classes passed, with no failures or skips; all six static reports contain zero findings/notifications. Actual synthetic checkout 5bd7202f combines live parent 7040c899 and this head and exactly matches published tree bd5b13dc.

All 39 unique E2E cases eventually passed: 36 on their first attempt, three on automatic Test 2. The first attempt failed boost_2 at outgoing activity-row selection, widgets_1 at WidgetSave, and widgets_3 at PriceWidget. Their failed exits, annotations, logs and media remain preserved. Six already-passed cases were cache-skipped on the second attempt; one disabled Settings case and six zero-step migration jobs never ran. No manual workflow rerun was requested.

The failed attempt's device clock was October 21, and native logs show CertificateNotYetValidException for a certificate valid from October 23, followed by price-fetch/no-price-data errors. The second attempt used November 2 and passed. This establishes the TLS rejection under the observed clock; it does not prove the cause of the activity ordering failure or every widget failure. Forty-five direct widget/activity presentation inputs match the actual base, but upstream ActivityRepo differs, so no blanket pre-existing or harmlessness claim is made. Backup guards, npm advisories and other runtime diagnostics remain disclosed.

The sole auditor inspected 14 complete job logs (19,348 lines), 34 nested streams (347,962 lines), nine digest/member-CRC-verified report archives and all retained failure media. The owner verified all 1,328 sealed payloads, manifest 0f4c22141fb695264a4022e1306ec722032c443c492165bcb4beee371740c130. All seven E2E consumers match the producer's archive digest and exact suite 611509ee; no separately downloaded/installed hosted APK claim is made. Evidence: work/validation/hosted-android1693-78f-20261102/.

Ovi's automated review separately reports controlled read/write refusal, Retry, preserved RN/native mnemonic comparisons and local RN/MMKV import at this exact head. The complete report was read; its diagnostic app, original trace/install artifacts and videos have not been independently inspected by the author. These are attributed external results, not additional author-run authenticated acceptance or funded/channel conversion. The existing wider acceptance gaps remain distinct. Ovi and Jason submitted human approvals bound to this head; no duplicate requests or merge action was made.

November 2 — reachable Retry for unavailable migration status

Signed 78f7209f98c16858987567a726bd7571d36fde8f fixes 4439118527. A migration-status read or completion-write failure now shows Wallet Migration → Retry instead of an indefinite loader. Retry resumes the existing serialized classification without a connectivity change. Cleanup recovery keeps priority, and onboarding, create and restore remain blocked until classification succeeds. No corrupt metadata is reset or treated as an absent wallet. The new status copy is localized in all 16 supported resource files; this recovery screen is mapped to todo (no design available).

Final validation: 5,085 unit passes / 266 classes, zero failures or skips, required compile, final formatting and six complete static reports with zero findings/notifications, plus two independent affected source reviews. After the full gate, only the instrumented test helper changed to use MainActivity's actual theme wrapper; all other 1,802 inputs stayed byte-identical. Formatting and complete lint were rerun, and 24 installed routing/recovery cases passed with the real Retry button. The unchanged app APK is 6e7f2d31; corrected test APK is 88a795ea, both matched installed bytes. They were built from the frozen pre-commit source, with 595c3642 build metadata. These are service-controlled unit and callback-controlled installed UI tests, not full RN conversion or authenticated wallet acceptance.

The first installed attempt executed 17 passes and seven failures after a new test rendered the screen without the app theme; the later Retry case and six recovery cases received a null text style. The complete 3,820-line failure and initialization-order trace are preserved. The helper now supplies the same theme as production, with all assertions unchanged. This is not a production typography repair or a claim that all other entry points are safe. The earlier focused fixture mismatch, formatter import correction and four CloseGuard observations are also retained.

All 20 commits against live #1690/7040c899 verify locally and on GitHub. At that revision, hosted CI was pending its own complete inspection. The preceding 595c3642 hosted result is fully audited separately: 5,084 units, six clean static reports, 39 first-attempt E2E passes, one disabled Settings case and six skipped migration jobs; those passes did not waive the Retry finding. Evidence: work/validation/android1693-migration-status-local-audit-20261102/, android1693-migration-status-compose-b-20261102/, android1693-migration-status-publication-20261102/ and hosted-android1693-595c-20261102/.

Manual 5 and broader authenticated cleanup/restore limits remain explicit. Reviewer-reported installed reproduction 4439227124 supports the old-head liveness failure, but its original control/build/trace artifacts were not independently inspected. Whole-store startup failure is a separate boundary, not established as repaired here.

Deferred React Native migration — November 2

Signed 595c3642f1fbb91c52dca0206ee9f8b28082eb64 fixes 4438796524: a failed initial cleanup-state read no longer abandons migration classification and later exposes new-wallet onboarding. Clear-state recovery, explicit read Retry and reconnect resume one serialized check. Initial classification stays gated ahead of onboarding and recovery mode, and stale create/restore callbacks cannot replace a wallet while it is pending. Unavailable metadata and failed inner admission stay pending; the local migration mutex releases before node startup. Existing cleanup and identity checks remain intact.

The final combined candidate passed required formatting, compile, 5,084 unit tests / 266 suites, and all six static reports with zero findings. All 1,802 frozen inputs match; two independent recovery/concurrency reviews are clean. The two decisive regressions failed on the original source (90 passes / two expected failures) and passed after repair, with 98 affected tests passing. Earlier Mockito fixture errors and the 15 automatically corrected formatting findings remain recorded. Complete logs, XML/HTML, SARIF and ignoreFailures output were inspected. Four CloseGuard warnings retain unchanged source contexts; different suite attachment does not prove runtime equivalence or harmlessness. Copied Gradle problem reports were stale and are not presented as fresh final findings.

All 22 installed routing/recovery Compose cases passed on matching APK 3093208c4cacc94987dad3e3024e30af5ceba697aba1f207c62495df11951245. The first test process crashed before any test on a logged BOOT_COMPLETED receiver accessing Hilt before its component existed. After the event completed and the own broadcast queue cleared, one bounded instrumentation-only retry passed using identical installed APKs; no rebuild, reinstall, reset, component disabling or assertion change. The failed process and complete retry diagnostics are retained. This APK was built before commit and retains 581fed71 build metadata; validation is bound to the exact frozen source inputs.

All 19 commits against live #1690/7040c899 verify locally and on GitHub (verified=true, reason=valid). That head's hosted CI is now fully inspected: 5,084 units, six clean static reports and 39 first-attempt E2E passes; one disabled Settings case and six migration jobs skipped. Its later metadata-Retry finding is fixed by the newer head above; these CI results are not transferred to that correction. The prior 581fed71 results below remain historical. The PR remains ready, with existing reviewers preserved. Evidence: work/validation/android1693-migration-admission-local-audit-20261102/, android1693-migration-admission-compose-retry-20261102/ and android1693-migration-admission-publication-20261102/.

These tests establish admission, retry serialization and installed root routing, not a complete RN keychain/MMKV conversion or authenticated wallet journey. Manual 5 below needs a prepared unfunded RN-only installation and controlled initial storage-read refusal/release. The iOS twin already keys setup to its cleanup-blocking state; this is a source comparison only, not new iOS runtime evidence. Broader removal/reset and terminal complete-WALLET limits remain separate.

Forgotten-PIN navigation and fresh reset cleanup — November 2

Signed 581fed71da6127737a2f9b734f58a7941c95325c fixes 4438113717 and 4438113788, on live #1690/7040c899. In restricted recovery, Forgot PIN now revokes that read-only authentication attempt and returns to its recovery overview. Entering recovery clears an old global Forgot PIN sheet request; ordinary wallet callers retain their existing behavior. A scope-less corrupt marker no longer suppresses the first explicitly confirmed reset's private/public endpoint-cleanup attempts. Only a known WALLET_WIPE resumes local cleanup without remote replay. Remote cleanup retains its existing best-effort reset semantics: an attempt is not a deletion acknowledgement.

Final tree 33085fd30e475b5b12ee7ee88cc0cdb41b4dd2aa passed required formatting, compile, 5,076 units / 266 suites with no failures/skips, and all six fresh static reports with zero findings. All 1,802 frozen inputs match. Two independent recovery/persistence source reviews are clean. The real malformed-DataStore regression first failed on the missing endpoint call (29/30), then passed (30/30); the known-wipe restart control still proves no remote replay. Complete logs, XML/HTML, SARIF and ignoreFailures diagnostics were inspected, preserving the four inherited-source CloseGuard warning contexts rather than calling them harmless. All 18 live-base commits verify locally and on GitHub (verified=true, reason=valid).

All 19 installed Compose cases passed with verified app APK 656be67e75e2d6176dbf5745b3eafbbf9c37777c2743371204c5cfd1dd6abcb8. On the same unfunded Android user and unchanged durable marker, the old 011f47f2 APK reproduced the inert Forgot PIN link. The fixed APK passed incorrect PIN → Forgot PIN → restricted overview → fresh PIN on re-entry, plus cold restart retaining recovery and requiring fresh PIN. No correct PIN, seed display, wallet reset or network endpoint mutation was performed in this follow-up. The in-memory global flag has no runtime accessor: its no-write/entry-clear behavior is source-reviewed, not a direct runtime flag or post-reset/onboarding observation. The initial read-only run-as argument failure and one null-root hierarchy read after restart remain recorded; a fresh read followed verified resumed activity, without repeating the app journey or weakening assertions.

Current-head CI, static checks and E2E are now completely inspected: 5,076 unit passes, zero findings in six static reports, and 39 first-attempt E2E passes. One deliberately disabled Settings case and six migration jobs skipped; all seven E2E primary shards passed, with no automatic suite retry or manual CI rerun. Synthetic 7dbd92a5 has the exact published 33085fd3 tree and live 7040c899 parent; the executed external suite is 611509ee.

The audit consumed all 14 job logs, complete unit/static reports, all 32 nested E2E streams and nine checksum/CRC-verified report archives; the owner verified all 457 sealed evidence files. CloseGuard, metadata-backup, restore/sync timeout, wipe-session and other runtime observations remain recorded without a harmlessness or causal-repair claim. Gradle problem details and the detailed npm audit were not uploaded. APK producer metadata is bound to every shard, but full hosted APK/installed-byte verification is not claimed. This stock E2E result does not replace the separate authenticated removal/reset acceptance. Evidence: work/validation/hosted-android1693-581f-20261102/TAIL-STRUCTURED-INSPECTION.json. The preceding 011f47f2 CI is now fully inspected: 5,075 units, static reports clear, and 39 eventual E2E passes with one disabled Settings skip after one automatic Send retry. Its first AddressContinue timeout, malformed recipient input capture, two retry skips, runtime warnings and six skipped migration jobs remain recorded; cause is unproved and this is not validation of the new tree. No manual CI rerun occurred.

Evidence: work/validation/android1693-forgotpin-corrupt-final-audit-20261102/, android1693-forgotpin-corrupt-compose-audit-20261102/, android1693-forgotpin-runtime-20261102/ and android1693-forgotpin-corrupt-publication-20261102/. The new wallet-reset-recovery-pin.xml records the finite Android-specific route; WipeWalletUseCaseTest.kt covers the mocked cleanup order. Remaining authenticated reset, unknown-private-outcome, full-WALLET and broader J1–J4 gaps below are unchanged. The PR remains ready for review, not merged.

Unreadable-state recovery and active-wipe controls — November 2

Signed additive merge 011f47f2d4e7deaae07a4f349b0a8c8122de7b69 incorporates live #1690/7040c899. A corrupt, scope-less removal marker now stays unknown instead of claiming a wallet wipe failed. Restricted recovery offers read-only seed recovery with fresh authentication, without admitting ordinary wallet work. Authorization is bound to the current recovery state and revoked on backgrounding, disposal or owner change. The real wipe flag also suppresses the failed-wipe message and repeat reset controls while teardown is still running.

Final combined tree 39f23d388b7b9ae2e80fefe9441b96a88b748eac passed required compile, formatting, full 5,075 unit tests / 266 suites, and all six static reports with zero findings. All 1,801 frozen inputs match. Two independent recovery/security reviews and the narrow final primitive-state/button-selector recheck are clean. All 17 live-base commits verify locally and on GitHub (verified=true, reason=valid). All 19 installed routing/auth-lifecycle/confirmation Compose cases passed with matching APK bytes. Earlier negative controls, introduced formatting/lint findings and the first UI run's two selector failures remain recorded; no assertion was removed.

A separate, newly created unfunded Android user exercised the real app/PIN/Hilt route on installed APK 117f723deb1f07f509596fbef1c5d3bcf7af7e37ae8ee84afe22b0c60a7725f5: corrupt marker → restricted recovery → one wrong PIN refused → correct PIN enters the actual mnemonic container → confirmed STOPPED background/return and cold restart both require fresh authorization. The same durable marker remained unchanged. Recovery text was redacted on the device before transport; no words or screenshot were exported or retained. This verifies route construction and admission, not the mnemonic's sensitive contents. No wallet reset, biometric, restored-seed, native cleanup or broader J1–J4 pass is claimed. The test wallet/marker is preserved and the original emulator user restored.

This addresses 4437687087 and 4437687226. At that revision, hosted CI was pending its own inspection. Evidence: work/validation/android1693-recovery-reviews-20261102/final-b-diagnostic-audit/, android1693-recovery-compose-final-b/, android1693-recovery-readonly-runtime-20261102/ and android1693-recovery-publication-20261102/.

Previous-head hosted validation — November 1 (395314b0)

Head 395314b0dc6fc40789d0295668bda333de0624b0 has completed hosted inspection: 5,064 unit tests, six static reports with zero findings, and 39 first-attempt E2E passes. One disabled Settings case and six migration jobs skipped; they are not passes. All seven primary E2E shards completed, and all 14 fallback retry jobs skipped. The verified synthetic checkout tree matches the published head.

Complete logs, every attempt and nine checksum/CRC-verified archives were inspected. The tail audit adds 12 E2E passes and preserves backup-refusal and restore-wait runtime warnings; their causes are not claimed repaired. All 496 tail-evidence payloads were independently verified, with earlier sealed evidence unchanged. The finite hosted suite does not establish authenticated profile-removal, full wallet reset or terminal complete-WALLET acceptance. Existing J1–J4/Manual 1–3 limits remain. Evidence: work/validation/hosted-android1693-39531-20261101-current-a/TAIL-REPORT.md.

Failed wallet-reset recovery — November 1

Signed 395314b0dc6fc40789d0295668bda333de0624b0 incorporates live #1690/120553fd967b1b77f179d2c196be87bd4718a2fd. A failed wallet wipe now retains a typed, durable recovery gate before onboarding, ordinary-wallet startup, seed restoration or migration. The recovery screen offers Finish reset only after a fresh confirmation bound to the same saved attempt. Unreadable state offers a read-only retry. Cleanup failures retain the obligation; conditional credential erasure precedes consumed-watermark cleanup and the exact marker is cleared last. This addresses Piotr's failed-reset finding.

The final production/unit composition passed 5,064 tests / 265 suites, zero failures/errors/skips, required compile, formatting and lint. Complete logs and all structured static reports were inspected: zero findings. Two independent recovery/concurrency reviews and the affected UI-test recheck are clean. All 16 commits against the live parent verify locally and on GitHub (verified=true, reason=valid).

All 15 installed routing/confirmation Compose cases passed on API36 with matching installed APK bytes. They exercise callback-only content, including unreadable/pending admission, Cancel, busy controls and changed-attempt confirmation invalidation; they do not reset an actual wallet. The first UI run's 14 passes/one failure remains recorded: its second tap incorrectly advanced the Compose clock while the button debounce uses real monotonic time. The corrected test waits on that same clock, retaining every assertion. Only this androidTest file changed afterward; all other 1,799 frozen inputs match the full unit gate, and required final format/lint plus instrumentation compilation/execution were repeated.

Controlled temporary-DataStore recreation also retains the same attempt across post-credential metadata failure and final-marker write refusal, then completes only after a fresh explicit local retry. These are component persistence controls, not an OS process-restart, authenticated wallet-reset, RN migration or replacement-seed journey. Existing J1–J4 and Manual 1–3 gaps below remain distinct.

At that revision, hosted CI was pending complete inspection. Evidence: work/validation/android1693-wipe-final-a/, android1693-wipe-recovery-review-20261101/ and android1693-wipe-compose-final-b/root-inspection.json. No wallet/device reset, remote replay or broader authenticated acceptance is claimed.

Previous review-eligibility composition — November 1

Published signed merge 2d8c9915b38bae0f546bf33ede69f676845c24d8 includes live #1690/9bf19f194a291da749f8d8e48273ee30123e66e9; final tree 02d09585b8a2484fd63174d5aeeb0f6b87eabb77. When failed Review preparation revokes the original logout handle, the guarded UI state now refreshes its eligibility and offers Disconnect/Close instead of leaving an unusable Review action. The original owner, pending attempt and fresh local confirmation remain intact; no cleanup or remote mutation is replayed. This addresses the follow-up in 4432456335 and 5725301527.

The deciding regression failed before the fix (23 passes/one expected failure) and passed afterward. The final composed candidate, including the repository-style TestAppError helper, passed 5,047 tests /265 suites, with zero failures/errors/skips/retries; the affected ViewModel suite passed24/24. Required final formatting, compile and lint passed. Complete logs and fresh Detekt/Android Lint SARIF/XML/HTML were inspected: zero findings/issues/notifications. All1,798 frozen inputs match. Prior cursor/JVM notices and stale report exclusions are retained. The affected fix and composition reviews are clean, with unaffected independent concurrency coverage reused. All 15 live-base commits verify locally and on GitHub.

At that revision, hosted CI was pending inspection. The added authenticated credential-unreadable Review action remains unrun; the regression is coordinator/ViewModel coverage, not a new full-app/native-server pass. Earlier finite RC4 device evidence remains tied to its original inputs. Unknown private-stage acknowledgement stays local-only; wider J1–J4, restore/spending, device and terminal complete-WALLET limits remain open. Evidence: work/validation/android1693-parent9bf1-review-final-a/inspection/ and work/validation/android1693-parent9bf1-review-fix-20261101/publication/.

This PR binds profile removal to the identity and operation the user confirmed. Interrupted grant revocation can be retried only with a fresh confirmation and the retained original grant; after restart, recovery remains local-only. After positively acknowledged private withdrawal, ordinary Disconnect can also retry remaining endpoint cleanup. A transient credential-read failure permits a fresh conditional local erase without restoring remote authority.

Related: #1687 (native conditional contact deletion remains separate). Twin: synonymdev/bitkit-ios#1143.
Dependency: #1690, live branch fix/profile-cleanup-outcomes at 7040c89936b354198d8aea363a54dc03fd0c1485. Current signed head 78f7209f98c16858987567a726bd7571d36fde8f retains published Paykit 0.1.1-rc4. It uses the existing grant-proof logout endpoint; no new homeserver API or deployment is required.

Description

  • Capture the original sign-in, profile revision, contact snapshot, grant, client and signer before confirmation. Reject later edits, replacement identities and stale confirmations at the actual mutation boundaries.
  • A remote Retry consumes a fresh one-use confirmation and proof nonce. Same-original-grant bearer refresh is permitted; it cannot adopt another grant. Unknown outcomes retain the obligation and never trigger automatic replay.
  • Preserve positively acknowledged cleanup stages in the same live owner/UUID-bound attempt. Revalidate the shrinking target snapshot before remaining work; do not infer acknowledgement from nil, partial, malformed or failed receipts.
  • Keep private withdrawal valid when sharing has already been disabled. An earlier completed withdrawal is not repeated after a later stage fails.
  • Persist the local cleanup obligation before destructive work. Restart never reconstructs remote authority from that UUID; Profile offers explicit local Disconnect, including after credentials have disappeared.
  • Require acknowledged, conditional credential erasure before consumed-payment watermark cleanup, and clear the exact durable marker last. A fresh local confirmation can recheck temporarily unreadable original credentials in one store edit; observed replacement, absence/addition, wallet/lifecycle mismatch or ABA remains terminal. Remote authority stays retired.
  • Hide Review when the original logout is revoked, executing or no longer eligible for review. Cancellation and owner replacement drain the original operation without erasing replacement credentials.
  • Share orchestration across deletion, sign-out, private-state cleanup, backup and wallet teardown. Ordinary receive accounting and public reads remain available; optional contact attribution can be backfilled.
  • Preserve accepted-subscription refusal, private-work admission, reservation protections and Ring-owned data.

Out of Scope

Native key-only contact batches do not provide cross-device edit/re-add/unblock/ABA exclusion; #1687 stays open. Full-wallet redesign is separate. This PR does not claim that a local Disconnect proves remote cleanup succeeded.

Design

N/A — no design available for the pending-profile-recovery, Review, Finish reset and migration-status recovery surfaces. Existing Profile / Profile Edit surfaces are mapped in docs/screens-map.md.

Preview

The ordinary recovery surfaces are covered by installed Compose cases and the revision-specific authenticated evidence below. The pending-after-restart fixture also retains a non-secret screenshot. No complete J1–J4 recording is claimed.

QA Notes

Setup

Use the normal released dependencies pinned by this branch, a dev/regtest build and a disposable profile. Build with ./gradlew :app:assembleDevDebug :app:assembleDevDebugAndroidTest. Component UI checks run with adb shell am instrument -w -r -e class to.bitkit.ui.screens.profile.ProfileRecoveryContentTest to.bitkit.dev.test/to.bitkit.test.HiltTestRunner on Android API 36.

J2–J4 require an authenticated controlled homeserver/proxy with mutation acknowledgement and contact-removal fault gates, plus injectable local-storage failures. The standard journey runner does not provide those controls. Use no personal identity or live funds; do not reset a wallet to prepare the fixture. Standard staging alone does not supply the needed fault setup.

Journeys

  • J1: updated delete-profile.xml — prepare before confirmation and complete acknowledged removal.
  • J2: updated delete-profile-uncertain-response.xml — one uncertain mutation, read-only Review and explicit local Disconnect.
  • J3: updated profile-cleanup-failure.xml — incomplete cleanup retains recovery without false success.
  • J4: updated profile-removal-recovery.xml — canceled confirmations, later edits, identity replacement, restart without credentials, local failure recovery, and explicit local Disconnect after fixed-term removal refusal. The final-app fixed-term Payer refusal/Cancel/local-confirm/cold-restart subset is now verified above; the complete J4 box remains open.

Manual Tests

  • 1. Hold cleanup or local completion, replace/revoke its original owner, then release it → no mutation or success/navigation for the replacement; controlled suspension is outside runner capabilities.

  • 2. Fail credential erasure or local persistence at each cleanup checkpoint, restart, then choose local Disconnect → consumed-list watermarks remain until credential acknowledgement, pending is cleared last, and no remote mutation is replayed; storage/process fault injection is outside runner capabilities.

  • 3. Hold SDK admission while wallet teardown drains an outer owner → both drain without queuing the old owner behind the wipe; use controlled resources, not a real wallet reset.

  • 4a. On a new unfunded PIN-protected test wallet, corrupt only the local removal marker: unknown-state recovery must require PIN; wrong PIN refuses; correct PIN reaches the actual read-only recovery container. After confirmed background stop and cold restart, fresh PIN is required and the same marker remains. Completed on the exact APK above; no sensitive word-content inspection.

  • 4b. Actual long-running wallet wipe must show only progress and reject a second reset until unwind. Installed Compose and ViewModel controls pass; literal wallet wipe is unrun and is not authorized by this PR-maintenance task.

  • 5. On an isolated, unfunded RN-only installation with no native mnemonic or checked migration marker, refuse the initial cleanup-state read, then release it and use Retry/Clear/reconnect → classification resumes once and onboarding/seed replacement stays blocked while it is pending. Preparing legacy RN keychain/MMKV data and scoped storage holds requires a controlled fixture; eight ViewModel controls and three installed root-routing cases pass, but the complete RN conversion journey is unrun.

Automated Checks

  • added ProfileRemovalStoreTest.kt, PubkyIdentityOperationsTest.kt, ProfileRemovalCoordinatorTest.kt — durable pending state, original-owner admission, strict completion order, shrinking retry scope and one-use confirmations.
  • added ProfileRecoveryContentTest.kt — three real Compose recovery/dialog cases, with callbacks controlled by the test.
  • added WalletResetRecoveryTest.kt and expanded RootDestinationTest.kt — 22 installed callback-only routing/confirmation/auth-lifecycle cases; no real wallet reset. Run adb shell am instrument -w -r -e class to.bitkit.ui.RootDestinationTest,to.bitkit.ui.screens.recovery.WalletResetRecoveryTest to.bitkit.dev.test/to.bitkit.test.HiltTestRunner.
  • updated profile ViewModel, SDK lock/service, repository, backup, attribution, wallet-wipe and application-flow tests — cancellation, stale work, restart, local failures and ordinary-wallet independence.
  • ran controlled fail-before/pass-after tests for stale ownership, credential cancellation, queue admission, lost cleanup acknowledgement and retry scope expansion. These are component tests with native/network resources mocked where stated.

Evidence

Current recovery follow-up — b2cd2aa3d. Tree c0662f98f4ac23775e161d71fcb2f3fd9dccd49f fixes the endpoint-stage Retry, transient credential-read/local-completion and revoked-Review findings. Final required formatting, compile, full tests, Detekt and Android Lint passed: 5,036 fresh tests / 265 suites, zero failures/errors/skips, zero static findings. All 1,794 frozen inputs match. Complete logs, XML/HTML/SARIF and nested configuration reports were inspected; inherited warnings and failed development attempts are retained. Two independent security/concurrency reviews and the affected signer-test recheck are clean. All 14 commits against live base 7ef34320 verify locally and on GitHub (verified=true, reason=valid).

Two additional finite authenticated app cases passed on installed APK 39d1526fde183377c3ba1cb1f5be294fd167b317d93481cb665c42a93fdeccd6, using the unchanged published RC4 native artifact:

  • A registry refusal after acknowledged private withdrawal retained the UUID and exposed Retry. Cancel preserved all scoped metadata/journals and the marker; fresh confirmation completed the registry update and the same original-grant revocation. Cold restart retained completion, no session remained, and public registry readback was private_payments=false.
  • In a separate disposable profile, the same boundary followed by local Disconnect → Cancel → fresh confirmation cleared local recovery without changing the original server grant/session or any scoped entries/events/journals. Cold restart retained onboarding and no remote replay; public registry remained private_payments=true.

These two cases used real native Homegate/Pubky/Postgres authentication with zero private targets and empty endpoint sets. Actual transient credential read/decrypt faults, positive-watermark ordering and cancellation/wipe-drain controls are component evidence, not new device-fault claims. Initial local preflight rejected three registry writes before any removal confirmation; its provisional contrary narrative was explicitly corrected, and the later correctly scoped single refusal is separately identified. Both apps are stopped, fault controls accept/pass, profile bytes/modes are restored and wallets/devices are preserved. The APK was built before this additive commit from the matching frozen source map; its Git-head display metadata remains ebc39109.

Evidence: work/validation/android1693-local-invalidation-20261031/inspection/REPORT.md, work/validation/android1693-local-recovery-native-20261031/REPORT.md (206-member immutable inventory), and work/validation/android1693-local-recovery-publication/. Current-head hosted CI, lint and E2E are fully audited on synthetic f9665ee0, whose tree exactly equals this published head: 5,036 unit passes / 265 classes, zero configured lint findings and 39 first-attempt E2E passes. One explicit Settings case and all six migration jobs were skipped, not passed. All 46 available complete logs, six lint reports, 267 unit HTML pages, 17 annotations and nine SHA256/CRC-verified archives were inspected. A real zygote process-start failure occurred during the harness's package reinstall; the replacement process and all three multi-address cases subsequently passed. The newly observed manifest warning has unchanged actual-base manifest source, but no proven cause; npm advisories and other runtime diagnostics remain recorded. No workflow or test-attempt retry, clean-runtime claim, historical causal-repair claim or broader authenticated J1–J4 pass is implied. Evidence: work/validation/hosted-android1693-b2cd-20261031/REPORT.md and its 487-member immutable manifest, independently integrity-verified by the owner. Previous-head results below remain historical.

Ovi's external review at ebc39109 independently reports the same three failures and additional storage/owner cases. Its authenticated cases used separate unpublished fault hooks and native signup without Homegate; raw external artifacts were not independently inspected here. It is attributed external evidence, not a human approval or an additional author-run pass.

Previous combined source and released dependency (ebc39109). Head ebc39109, tree c5311661087b8ba75536367f5fc88ecc8a011c0c, contains the additive live-parent merge and the RC4 integration. Final required formatting, compile, full tests, Detekt and Android Lint passed: 5,024 fresh tests / 265 suites, zero failures/errors/skips/retries, zero static findings. Complete 10,416 log lines and structured reports, including ignore-failure outputs, were inspected. All 1,794 frozen input hashes match the committed tree. Two independent affected security/concurrency reviews and the affected test-fixture recheck are clean. All 13 commits against the actual live base verify locally and on GitHub (verified=true, reason=valid). Earlier six caller-fixture failures and introduced wrapping findings are preserved and fixed. Inherited JVM/CloseGuard/Detekt notices remain documented; a native-library strip notice is disclosed without an unproved baseline-equivalence claim.

The actual dev/regtest APK is SHA-256 1babbc3c55c82fcfa955fc15c913002f421a91c52ae29f7cca58872ab361dcbe. Installed bytes match. It uses the published RC4 AAR d408ce651a39adffd2f71d913e15db9403b8930ad953c5a5bad66af5b883f18c; no Maven-local replacement or dependency substitution was used. The APK was built before the additive commit and retains the earlier Git-head build metadata; reuse is bound by the exact frozen source map, not a claim that it was rebuilt after commit. Production Kotlin/KSP/Java/dex/native packaging executed for this APK.

Three finite authenticated app cases on those exact APK bytes. Each used a separate unfunded Android user/profile, actual native Pubky/Homegate/Postgres authentication and only the existing grant-proof logout protocol. Existing wallets, devices and fixtures were preserved.

  1. Original-grant retry: a scoped pre-revocation failure retained the original UUID, grant and credentials. Leaving the editor did not cancel or replay it. Retry opened a fresh confirmation; Cancel preserved state. A newly confirmed retry committed revocation while withholding its success response, and the app retained recovery. A further fresh confirmation completed against the same original grant/client/signer and cleared the marker. Cold restart retained completion. Exactly three verified logout-proof requests had distinct request IDs and the same bound scope; committed cleanup storage stayed unchanged after its initial acknowledged stages. This establishes an unacknowledged committed outcome and explicit continuation, not an independently measured native timeout or packet-loss duration.
  2. Pending cold restart: after one scoped pre-commit failure, restarting the app exposed only local Disconnect, with no remote Retry. Cancel preserved the UUID and remote state. A fresh local confirmation cleared local recovery; another restart reached Profile onboarding. The original remote grant/session and all recorded server mutations remained unchanged after the first failed attempt. Exactly one selected proof request was recorded.
  3. Sharing already disabled: a native linked-peer handshake and nonempty private payment list preceded the ordinary settings toggle. The peer received an empty list after disabling sharing. The first Delete confirmation completed without Retry or local fallback, revoked the original grant, removed the public profile and cleared the marker. Cold restart retained completion and unchanged post-completion server state. Peer readback is attributed to the peer; it is not a raw withdrawal-ACK receipt. The app visibly saved the contact before disabling sharing, but its saved-contact record and Linked enum were not separately queried afterward.

Evidence: work/validation/android1693-rc4-complete-gates-c/inspection/, work/validation/android1693-rc4-app-build-a/inspection/, work/validation/android1693-rc4-native-acceptance-20261031/, work/fixtures/android1693-original-grant-rc4-20261031/, and work/validation/android1693-rc4-publication/. The 385-file public runtime inventory retains controlled failures, preflight/input recoveries, UI actions, public storage comparisons and the fixture timing limits. The device clock and original Homegate route were restored; no test app remains running.

Earlier evidence remains revision-specific. RC2 app payer/payee fixed-term refusal → local Disconnect → restart, credential-write refusal and post-credential metadata-write failure are retained in work/android1693-{payee,credential-write,metadata-write}-acceptance-*.md and the corresponding fixture manifests. Those storage cases had no positive consumed-list watermarks and no failed-request HTTP journal; unchanged committed state is not proof of zero rejected requests. Installed Compose results and externally reported Ring/late-subscription cases retain their original revisions and artifact limits.

The previous 716e88e9 hosted unit CI, lint and E2E were fully inspected: 4,877 units, zero static findings, 39 first-attempt E2E passes and one disabled settings skip. They do not validate the new composition. Previous-head unit CI, lint and E2E all passed on attempt 1 on synthetic tree c5311661, exactly matching ebc39109: 5,024 units, zero static findings, 39 first-attempt E2E passes and one explicit settings skip. All job/report pages, 14 complete job logs, 32 uploaded runtime logs and nine checksum/CRC-verified archives were inspected. Runtime notices remain documented. The unchanged E2E harness/package-lock reported 85 npm advisories versus 76 in actual-base evidence; advisory details were not uploaded, so this is not attributed to an Android dependency change or declared harmless. Migration is skipped, not passed. Evidence: work/validation/hosted-android1693-ebc3-20261031/. Prior generic E2E failures and automatic retries at older heads remain recorded in their sealed reports; no unchanged workflow was rerun.

Gaps

The earlier three RC4 cases close bounded revocation-continuation, pending-restart local-only and sharing-off first-Delete gaps; the two new cases cover ordinary endpoint-stage retry after positive private acknowledgement and retained-intent local completion. João's original thread retains the distinct unknown-private-acknowledgement reconciliation gap. Unknown withdrawal is never treated as complete or made automatically retryable. These cases do not constitute complete J1–J4/Manual 1–3 coverage. Positive consumed-watermark retention at every storage boundary, authenticated owner replacement, all Ring/late-subscription combinations and literal wallet teardown remain separately scoped. Component tests, skipped jobs and attributed external reports are not counted as author-run authenticated journeys. No native contact CAS, deployed-server equivalence, live funds, wallet reset or historical incident-causation claim is made.

The iOS twin retains its own source, runtime, readiness exception and hosted-CI evidence; Android results do not establish iOS parity by themselves. Earlier statements requiring an exact-bearer-only new server contract were superseded by the October 30 scope correction: this implementation preserves the original grant and uses existing grant-proof logout with permitted same-grant refresh.

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)

Out of Scope

  • Profile recovery content gutter — #1749, milestone 3.1.0, tracks LOW 4435373164. This presentation-only alignment/spinner finding is deferred under the repository policy; recovery behavior and controls are unchanged. The separate failed-wallet-reset correctness finding is addressed by 395314b0; its component/installed-UI evidence and remaining full-app limits are recorded above.

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

ovitrif commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Triage: 3.1.0, with #1687 that it fixes (shipped profile deletion).

@greptile-apps

greptile-apps Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 3/5

[High impact] The change reaches profile removal, authenticated Paykit work, and wallet teardown.

Fix the stuck recovery warning and stale wallet state after credential erasure before merging.

Findings

  1. P1 Profile stays blocked after recovery ▶
  2. P1 Erased wallet still appears present ▶
  3. P2 Backup does not resume ▶

Summary

This PR binds profile removal to the confirmed owner and adds durable local recovery.

  • Profile removal stays tied to the identity and records shown at confirmation.
  • Interrupted removal stays pending until the person finishes local Disconnect.
  • Identity changes and wallet teardown wait for admitted Paykit work to finish.

Acknowledged scope: ben-kaufman explicitly deferred native cross-device conditional contact deletion and complete iOS integration under #1687. Complete authenticated journeys and controlled teardown tests are also listed as unfinished; these were not treated as new findings.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Capture owner and records] --> B[User confirms once]
    B --> C[Check owner and records]
    C --> D[Persist pending cleanup]
    D --> E[Remove remote data]
    E --> F[Erase local credentials]
    F --> G[Clear private state]
    G --> H[Clear pending cleanup]
    E -->|Failure| I[Review or local Disconnect]
    F -->|Failure| J[Local Disconnect]
    G -->|Failure| J
    I -->|Review| A
Loading

Reviews (1) · Last reviewed commit: "fix: bind profile removal to its origina..." · Reviewed by Greptile

@ben-kaufman
ben-kaufman requested a review from Jasonvdb October 26, 2026 06:51

@talosmachina talosmachina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 finding (1 P1). Binds profile deletion and disconnect to the confirmed identity, with a durable local cleanup obligation, a read-only Review, and wallet-wipe draining. Reviewed ff4dc35e, oversized tier: 59 files. I reasoned from the code; I did not build here, per the Bitkit track's rule for auto reviews.

What I checked, and 3 candidates I ruled out

Read in full: ProfileRemovalStore.kt, ProfileRemovalCoordinator.kt, PubkyIdentityOperations.kt.
Read as diff: PubkyRepo.kt, PaykitSdkService.kt, PaykitSdkOperationLock.kt, ContactPaymentSettingsRepo.kt, WipeWalletUseCase.kt, AppViewModel.kt, ProfileViewModel.kt, PubkyService.kt.
Not read: the remaining UI, backup and test files.
CI: build is red. One unit test failed, PubkyAuthManifestTest > signup and authorization links resolve only through their enabled aliases (pubkyring://signup?hs=homeserver resolved where none was expected): 4725 tests, 1 failed. The diff doesn't touch the manifest or the alias registrar, and build passed on #1690, so I couldn't tie the failure to this change. It still needs a look or a re-run. detekt and build-local are green; e2e was still running.

Ruled out

  • Wipe deadlock against an in-flight removal: withWalletWipe revokes the removal and awaits drained before acquire, and acquire(admittedGeneration) refuses a stale caller instead of queueing it behind the wipe.
  • Review widening the target set: reviewCeiling is shared across confirmations and only replaced by a subset (containsAll). remove() rejects a confirmation whose deletion is no longer the ceiling.
  • Local disconnect replaying remote work after restart: prepareSignOut sets localOnly whenever a pending attempt exists, so only finishProfileRemovalLocally and the private and marker completion run.
  • Cold reader: it timed out on the finding, so the P1 stands on my own trace of the call chain.

Merge confidence: 3/5, one P1, the oversized tier, and build red for a reason I could not tie to the change.

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

@github-actions

github-actions Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from c228adb (run).

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

@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: One LOW: pending cleanup raises repeated “session expired” errors and asks for reconnection while cleanup blocks it (app/src/main/java/to/bitkit/repositories/PubkyRepo.kt:376). The recovery actions themselves completed safely.

QA for 7e2e466: Android 16 / API 36, dev/regtest, disposable profile, controlled homeserver. I built the unmodified head, then drove 7e2e466 plus the unpublished test hooks below. Native SDK results were not mocked. Videos are 2×.

Test 1: J1 — acknowledged removal ✅ Zero and 62 contacts completed; preparation did not mutate remote state.

I repeated 62 contacts after settling and immediately after native import. The immediate run took 7.7 s from confirmation to success; the zero/settled runs each completed within a 10 s interval in their raw recordings. Progress stayed visible and Save stayed disabled. The native acknowledgement and cleared credentials/marker were checked in the logs; Home/reopen showed onboarding.

J1-immediate62-final-2x.2x.mp4

Immediate native import of 62 contacts through removal and reopening.

Test 2: J2 — uncertain acknowledgement ✅ One accepted DELETE remained one through Review, cancellation and cold launch.

The proxy logged upstream DELETE 204, then returned 503 to the app. Review performed reads before a new confirmation. Canceling it did not mutate. Cold launch retained recovery without authenticating. Canceling local Disconnect left it pending; confirming it cleared local state. The request trace before and after local Disconnect was identical.

J2-repeat-2x.2x.mp4

Lost acknowledgement, read-only Review, restart and explicit local recovery.

Test 3: J3 — incomplete cleanup ✅ All four fault variants stopped at the failed boundary without false success.

Request and state logs confirmed: failed private withdrawal caused no contact/profile deletion; an unknown contact-state write retained contacts; a native partial batch removed one of two records while retaining its published peer; profile DELETE 404 remained a failure. The partial repeat ran with the default sharing policy after the marker was seeded. Remaining-contact projection was checked on-device; override retention was checked in the implementation and tests.

Native partial removal Private withdrawal failure
J3-partial-production-config-2x.2x.mp4
J3-private-repeat-2x.2x.mp4
The private-failure capture also shows Review rejecting a later native profile edit.
Test 4: J4 — recovery ✅ Canceled or stale confirmations could not widen removal; local recovery and Receive remained available.

Device logs and request counters covered initial cancellation, a pre-mutation edit (plain error, no marker), a native edit after failure, a record recreated outside the narrowed retry ceiling, and a newer same-key sign-in. Cold launch without credentials showed pending Disconnect. I also opened ordinary Receive and checked its generated regtest address. This capture shows late local failure followed by successful recovery from the existing Profile screen.

J4-live-recovery-2x.2x.mp4

Credentials erased, sharing write failed, existing Profile recovered without reopening.

Test 5: manual 1 — original owner ✅ Revoked work could neither act on nor report success for a replacement sign-in.

The after-marker hold was canceled and drained before replacement admission. Old Review and Disconnect both returned identity-changed errors. In the second run, the non-cancellable local tail rejected replacement and held the real repository/SDK teardown barriers until release. The hook trace checked ordering, replacement credentials and the absence of stale success/navigation.

Revoked original removal Held local completion
M1-held-owner-2x.2x.mp4
M1-held-tail-2x.2x.mp4
Test 6: manual 2 — strict local completion ✅ Every injected local checkpoint retained recovery until acknowledged cleanup completed.

I faulted credential erasure, sharing, metadata, private completion, cache, attribution and marker persistence, cold-starting each interrupted state. The seeded consumed-list watermark stayed present through failed credential/cache writes; it cleared only after credentials were erased and cache cleanup succeeded. The marker stayed set at the final write failure and cleared last. Local retries generated no remote request. The repeated expired-session toasts are the LOW finding above.

Credential failure and restart Final marker-write failure
M2-credentials-repeat-2x.2x.mp4
M2-marker-completion-2x.2x.mp4
Test 7: manual 3 — SDK admission ✅ The controlled admission/outer-owner race drained without deadlock and admitted fresh work.

This runs the actual SDK lock class on-device with controlled coroutines and an outer mutex. Manual 1 separately drives the real repository/SDK teardown barriers. The wipe body is deliberately empty; no wallet reset or funds movement ran. The deciding hook trace was:

lock race: stale outer owner rejected before SDK queue
lock race: wipe drained outer owner without wallet teardown
lock race: fresh SDK work admitted after teardown

M3-lock-race-2x.2x.mp4

On-device completion signal; ordering was checked in the trace above.

Check 1: build, review and scope ✅ Full pinned diff and prior feedback checked.

My unmodified-head app and instrumentation APK build passed, as did git diff --check. I reviewed all 64 changed files and reached callers with an independent reviewer, and verified all nine prior review fixes. I read the PR discussion, linked issue/dependency and twin descriptions in full. The existing hosted validation report supplies the broader unit/lint/component results; I did not rerun those suites.

Device journeys do not independently drive unreadable-marker I/O, backup scheduling, broken native initialization or a real wallet wipe. Those paths were checked in source and the regression tests. Native cross-device conditional contact deletion remains under #1687.

Twin: synonymdev/bitkit-ios#1124 covers the earlier acknowledged-cleanup repair; synonymdev/bitkit-ios#1125 covers SDK admission. Neither establishes complete ownership/restart parity.

Unpublished test hooks and deciding state trace

The hooks provision disposable native contacts/profile state, seed one consumed-list watermark, log state/toasts, suspend named boundaries, or fail the next local write. The public-marker fixture was prepared with sharing enabled, then that flag was removed and the app restarted with the default policy before the partial-removal repeat. The network fault proxy forwards real requests; its lost-acknowledgement rule changes only the response after the server accepts the DELETE. No native SDK result is replaced.

Representative deciding states from the device trace:

# failed credential erasure
snapshot pending=true owner=null contacts=0 session=true secret=true watermarks=1
# failed sharing/metadata/private/cache completion, after credential acknowledgement
snapshot pending=true owner=null contacts=0 session=false secret=false watermarks=1
# failed attribution/final-marker completion, after cache acknowledgement
snapshot pending=true owner=null contacts=0 session=false secret=false watermarks=0
# acknowledged local completion
snapshot pending=false owner=null contacts=0 session=false secret=false watermarks=0
# later native profile edit
Failed to disconnect profile; The profile changed after confirmation. Disconnect locally to finish cleanup.
# stale same-key sign-in
Failed to disconnect profile; The profile identity changed

App hook patch (not pushed):

diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index b845c2d30..2e25e2b76 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -65,6 +65,7 @@
         android:roundIcon="${app_icon_round}"
         android:supportsRtl="true"
         android:theme="@style/Theme.App">
+        <receiver android:name="to.bitkit.qa.ReviewProbeReceiver" android:exported="true" />
         <service
             android:name=".androidServices.LightningNodeService"
             android:foregroundServiceType="dataSync"
diff --git a/app/src/main/java/to/bitkit/App.kt b/app/src/main/java/to/bitkit/App.kt
index 0df1da560..0391fced0 100644
--- a/app/src/main/java/to/bitkit/App.kt
+++ b/app/src/main/java/to/bitkit/App.kt
@@ -56,6 +56,7 @@ internal open class App : Application(), Configuration.Provider {
         Crypto.installSecurityProvider()
         super.onCreate()
         Env.initAppStoragePath(filesDir.absolutePath)
+        to.bitkit.qa.ReviewProbe.init(this)
         installUncaughtExceptionLogger()
         SingletonImageLoader.setSafe { imageLoader }
         currentActivity = CurrentActivity().also { registerActivityLifecycleCallbacks(it) }
diff --git a/app/src/main/java/to/bitkit/data/ProfileRemovalStore.kt b/app/src/main/java/to/bitkit/data/ProfileRemovalStore.kt
index 8e4b77572..40b46674b 100644
--- a/app/src/main/java/to/bitkit/data/ProfileRemovalStore.kt
+++ b/app/src/main/java/to/bitkit/data/ProfileRemovalStore.kt
@@ -128,6 +128,7 @@ class ProfileRemovalStore internal constructor(private val store: DataStore<Prof
 
     /** Caller must retain removal ownership until all acknowledged teardown's local persistence has completed. */
     internal suspend fun complete(attemptId: String) {
+        to.bitkit.qa.ReviewProbe.gate("marker-completion")
         val access = currentCoroutineContext()[Access]
         check(access?.store === this && access.attemptId == attemptId && access.active.get()) {
             "Profile removal admission is required"
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index 04078a464..e4dd7be96 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -600,7 +600,9 @@ class PrivatePaykitRepo @Inject constructor(
         knownSavedContactKeys.clear()
         unavailableLinkRetryAt.clear()
         state = PrivatePaykitState()
+        to.bitkit.qa.ReviewProbe.gate("cache-completion")
         cacheStore.reset()
+        to.bitkit.qa.ReviewProbe.gate("attribution-completion")
         addressReservationRepo.clearContactAssignments(excludingPublicKeys = emptySet())
     }
 
diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
index 18bc8ec91..9bad16ea9 100644
--- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
@@ -1249,6 +1249,7 @@ class PubkyRepo @Inject constructor(
 
     private suspend fun deleteContacts(records: List<ContactRecord>) {
         if (records.isEmpty()) return
+        to.bitkit.qa.ReviewProbe.gate("before-contacts")
         val keys = records.map { it.publicKey.ensurePubkyPrefix() }.toSet()
         val removedKeys = pubkyService.removeContacts(records.map { it.publicKey })
             .map { it.publicKey.ensurePubkyPrefix() }.toSet().intersect(keys)
@@ -1993,7 +1994,9 @@ class PubkyRepo @Inject constructor(
                 invalidateReadOnlyProfile()
                 evictPubkyImages()
                 profileWriteGeneration.incrementAndGet()
+                to.bitkit.qa.ReviewProbe.gate("sharing-completion")
                 clearPublicPaykitSharingState(publicPaykitCleanupPending = false)
+                to.bitkit.qa.ReviewProbe.gate("metadata-completion")
                 pubkyStore.reset()
             } finally {
                 notifyBackupStateChanged()
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkOperationLock.kt b/app/src/main/java/to/bitkit/services/PaykitSdkOperationLock.kt
index 7db69e37b..b931a7f06 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkOperationLock.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkOperationLock.kt
@@ -55,6 +55,7 @@ internal class PaykitSdkOperationLock(
             checkRecoveryAccess()
             checkProfileRemoval(removal)
         }
+        to.bitkit.qa.ReviewProbe.gate("sdk-admission")
         acquire(priority, admittedGeneration)
         return try {
             currentCoroutineContext().ensureActive()
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index e1a18f4e0..3e4a38db1 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -832,6 +832,16 @@ class PaykitSdkService @Inject constructor(
      * and sign-in checks run under the operation lock.
      * An empty selection is skipped without accessing the SDK.
      */
+    suspend fun reviewUnpublishContact(publicKey: String) {
+        isSetup.await()
+        operationLock.withLock { withStateRevisionTracking { handle -> completeSdkCall { handle.removePublicContact(publicKey) } } }
+    }
+
+    suspend fun reviewPublishContact(publicKey: String) {
+        isSetup.await()
+        operationLock.withLock { withStateRevisionTracking { handle -> completeSdkCall { handle.publishPublicContact(publicKey) } } }
+    }
+
     suspend fun saveContacts(
         updates: List<ContactUpdate>,
         expectedIdentity: String? = null,
@@ -2107,7 +2117,7 @@ internal object BitkitPaykitSdkConfig {
 }
 
 internal fun paykitSdkConfig() = defaultConfig("bitkit").copy(
-    publicContactSharing = BitkitPaykitSdkConfig.publicContactSharing,
+    publicContactSharing = if (to.bitkit.qa.ReviewProbe.enabled("public-contacts")) com.synonym.paykit.PublicContactSharingPolicy.ENABLED else BitkitPaykitSdkConfig.publicContactSharing,
 )
 
 internal fun paykitPubkyClientConfig(
@@ -2187,6 +2197,7 @@ internal class PaykitSdkSessionProvider(
     }
 
     override fun clearSessionAccess() = paykitStorageCallback("session_clear_failed") {
+        to.bitkit.qa.ReviewProbe.fail("credentials")
         clearLiveSessionAccess()
         keychain.accessBlocking {
             delete(Keychain.Key.SHARED_PUBKY_SOURCE.name)
diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt
index a9a57d840..8fe7fbd87 100644
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -234,7 +234,9 @@ class PubkyService @Inject constructor(
         }
 
     internal suspend fun deletePaykitProfile(snapshot: PaykitProfileDeletionSnapshot) = ServiceQueue.CORE.background {
+        to.bitkit.qa.ReviewProbe.gate("before-profile")
         paykitSdkService.deletePaykitProfile(snapshot)
+        to.bitkit.qa.ReviewProbe.loseAcknowledgement()
     }
 
     suspend fun deletePaykitProfile() = ServiceQueue.CORE.background {
diff --git a/app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt b/app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt
index bf6e1c7a0..512bd2de3 100644
--- a/app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt
+++ b/app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt
@@ -127,6 +127,7 @@ class ProfileRemovalCoordinator @Inject constructor(
                         confirmation.deletion?.let { pubkyRepo.validateProfileDeletion(it) }
                         removalMayHaveStarted = true
                         removalStore.begin(confirmation.attemptId)
+                        to.bitkit.qa.ReviewProbe.gate("after-marker")
                         if (!confirmation.localOnly) {
                             privatePaykitRepo.removePublishedEndpointsForCleanup(TAG, deferAppSync = true).getOrThrow()
                             pubkyRepo.removeBitkitPaymentEndpoints().getOrThrow()
@@ -136,6 +137,7 @@ class ProfileRemovalCoordinator @Inject constructor(
                         localCompletionStarted = true
                         pubkyRepo.finishProfileRemovalLocally()
                         withContext(NonCancellable) {
+                            to.bitkit.qa.ReviewProbe.gate("private-completion")
                             privatePaykitRepo.finishProfileRemoval(pause).getOrThrow()
                             removalStore.complete(confirmation.attemptId)
                             privatePaykitRepo.notifyProfileRemovalCompleted(pause)
diff --git a/app/src/main/java/to/bitkit/ReviewProbe.kt b/app/src/main/java/to/bitkit/ReviewProbe.kt
new file mode 100644
index 000000000..6400aa007
--- /dev/null
+++ b/app/src/main/java/to/bitkit/ReviewProbe.kt
@@ -0,0 +1,162 @@
+package to.bitkit.qa
+
+import android.content.BroadcastReceiver
+import android.content.Context
+import android.content.Intent
+import android.util.Log
+import com.synonym.paykit.ContactUpdate
+import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.BuildConfig
+import to.bitkit.data.*
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.repositories.PubkyRepo
+import to.bitkit.repositories.WalletRepo
+import to.bitkit.services.PubkyService
+import to.bitkit.utils.AppError
+import java.io.File
+import java.security.SecureRandom
+import javax.inject.Inject
+
+// TEST HOOK: device-local controls; actual repositories and native calls are unchanged except named injected faults.
+object ReviewProbe {
+    private var root: File? = null
+    private var toastJob: Job? = null
+    fun init(context: Context) {
+        if (!BuildConfig.DEBUG) return
+        root = File(context.filesDir, "review-probe").apply { mkdirs() }
+        if (toastJob == null) toastJob = CoroutineScope(SupervisorJob() + Dispatchers.IO).launch {
+            to.bitkit.ui.shared.toast.ToastEventBus.events.collect { note("toast ${it.type}: ${it.title}; ${it.description}") }
+        }
+    }
+    fun note(message: String) { Log.i("ReviewProbe", message); root?.resolve("trace.log")?.appendText(message + "\n") }
+    fun enabled(point: String): Boolean = root?.resolve(point)?.exists() == true
+    fun fail(point: String) {
+        note("checkpoint $point")
+        if (root?.resolve("fail-$point")?.exists() == true) throw AppError("Injected local failure: $point")
+    }
+    suspend fun gate(point: String) {
+        if (point == "sdk-admission" && !enabled("hold-sdk-admission") && !enabled("fail-sdk-admission")) return
+        fail(point)
+        if (root?.resolve("hold-$point")?.exists() == true) {
+            note("held $point")
+            while (root?.resolve("hold-$point")?.exists() == true) delay(100)
+            note("released $point")
+        }
+    }
+    fun loseAcknowledgement() {
+        note("profile native deletion acknowledged")
+        // Network acknowledgement faults are injected by the real TLS proxy, never by replacing SDK results.
+    }
+}
+
+@AndroidEntryPoint
+class ReviewProbeReceiver : BroadcastReceiver() {
+    @Inject lateinit var repo: PubkyRepo
+    @Inject lateinit var service: PubkyService
+    @Inject lateinit var wallet: WalletRepo
+    @Inject lateinit var settings: SettingsStore
+    @Inject lateinit var keychain: Keychain
+    @Inject lateinit var removal: ProfileRemovalStore
+    @Inject lateinit var cache: PrivatePaykitCacheStore
+    @Inject lateinit var privateRepo: to.bitkit.repositories.PrivatePaykitRepo
+    @Inject lateinit var coordinator: to.bitkit.usecases.ProfileRemovalCoordinator
+    @Inject lateinit var sdk: to.bitkit.services.PaykitSdkService
+    override fun onReceive(context: Context, intent: Intent) {
+        if (!BuildConfig.DEBUG) return
+        ReviewProbe.init(context)
+        val pending = goAsync()
+        CoroutineScope(SupervisorJob() + Dispatchers.IO).launch {
+            try {
+                when (val command = intent.getStringExtra("command")) {
+                    "seed" -> {
+                        settings.setIsPaykitEnabled(true)
+                        if (keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name) == null) wallet.createWallet(null).getOrThrow()
+                        settings.update { it.copy(isDevModeEnabled = true, hasSeenProfileIntro = true, hasSeenContactsIntro = true, hasSeenUsdtIntro = true) }
+                        repo.createIdentity("Disposable removal profile", "Local testnet", emptyList(), emptyList(), null).getOrThrow()
+                        val count = intent.getIntExtra("count", 0)
+                        if (count > 0) {
+                            val updates = (1..count).map { index ->
+                                val key = ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+                                ContactUpdate(publicKey = service.publicKeyFromSecret(key), label = "Disposable peer $index")
+                            }
+                            service.saveContacts(updates, repo.publicKey.value)
+                            repo.loadContacts()
+                        }
+                        File(context.filesDir, "review-probe/peers.txt").writeText(service.contactRecords().joinToString("\n") { it.publicKey })
+                        ReviewProbe.note("seed ready owner=${repo.publicKey.value} contacts=${service.contactRecords().size}")
+                    }
+                    "remote-edit", "remote-add" -> {
+                        val attempt = removal.pendingAttempt() ?: java.util.UUID.randomUUID().toString()
+                        repo.performProfileRemoval(repo.profileOwner(), attempt) {
+                            if (command == "remote-edit") {
+                                service.publishPaykitProfile(to.bitkit.models.PubkyProfileData(name = "Remote revision", bio = "Externally edited").toPaykitProfile())
+                            } else {
+                                val peer = File(context.filesDir, "review-probe/peers.txt").readLines().first()
+                                service.saveContacts(listOf(ContactUpdate(peer, "Recreated peer")))
+                            }
+                        }.getOrThrow()
+                        ReviewProbe.note("native remote state changed command=$command")
+                    }
+                    "replace" -> {
+                        coordinator.remove(coordinator.prepareSignOut(repo.profileOwner()).getOrThrow()).getOrThrow()
+                        repo.createIdentity("Replacement sign-in", "Same key, newer sign-in", emptyList(), emptyList(), null).getOrThrow()
+                        ReviewProbe.note("replacement admitted owner=${repo.profileOwner()}")
+                    }
+                    "lock-race" -> { reviewLockRace(); withContext(Dispatchers.Main) { android.widget.Toast.makeText(context, "Controlled SDK admission race passed", android.widget.Toast.LENGTH_LONG).show() } }
+                    "drain" -> repo.withWalletWipeAdmission { sdk.withWalletWipe { ReviewProbe.note("controlled wipe barrier entered; no wallet teardown") } }
+                    "edit" -> repo.saveProfile("Edited after confirmation", "Local testnet", emptyList(), emptyList(), null).getOrThrow()
+                    "restore" -> ReviewProbe.note("restore result=${repo.restoreSessionIfNeeded()}")
+                    "unpublish-contact" -> sdk.reviewUnpublishContact(requireNotNull(intent.getStringExtra("peer")))
+                    "publish-contact" -> {
+                        val key = service.contactRecords().last().publicKey
+                        sdk.reviewPublishContact(key)
+                        ReviewProbe.note("published retained peer=$key")
+                    }
+                    "watermark" -> {
+                        val peer = service.contactRecords().first().publicKey
+                        privateRepo.consumePrivatePaymentList(peer, to.bitkit.repositories.PrivatePaykitPaymentContext(emptyMap(), 42uL)).getOrThrow()
+                    }
+                    "snapshot" -> Unit
+                    else -> error("Unknown command $command")
+                }
+                val data = cache.data.first()
+                ReviewProbe.note("snapshot pending=${removal.pendingAttempt()!=null} owner=${repo.publicKey.value} contacts=${repo.contacts.value.size} session=${keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)!=null} secret=${keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)!=null} watermarks=${data.contacts.values.count{it.consumedPrivatePaymentListVersion!=null}}")
+            } catch (error: Throwable) {
+                ReviewProbe.note("command failed ${error::class.simpleName}: ${error.message}")
+            } finally { pending.finish() }
+        }
+    }
+}
+
+private suspend fun reviewLockRace() = coroutineScope {
+    withTimeout(10_000) {
+        val admitted = CompletableDeferred<Unit>()
+        val release = CompletableDeferred<Unit>()
+        val outer = kotlinx.coroutines.sync.Mutex()
+        var first = true
+        val lock = to.bitkit.services.PaykitSdkOperationLock {
+            if (first) { first = false; admitted.complete(Unit); release.await() }
+        }
+        val old = async {
+            outer.lock()
+            try {
+                val error = runCatching { lock.withLock { error("stale operation executed") } }.exceptionOrNull()
+                check(error is com.synonym.paykit.PaykitException.Storage)
+                ReviewProbe.note("lock race: stale outer owner rejected before SDK queue")
+            } finally { outer.unlock() }
+        }
+        admitted.await()
+        val wipe = async {
+            lock.withWalletWipe {
+                release.complete(Unit)
+                outer.lock()
+                outer.unlock()
+                ReviewProbe.note("lock race: wipe drained outer owner without wallet teardown")
+            }
+        }
+        old.await(); wipe.await()
+        lock.withLock { ReviewProbe.note("lock race: fresh SDK work admitted after teardown") }
+    }
+}

Lost-acknowledgement proxy patch:

--- a/payment-requests/src/bin/homeserver-proxy.rs
+++ b/payment-requests/src/bin/homeserver-proxy.rs
@@ -50,6 +50,8 @@
     // answer this status after the delay instead of forwarding
     #[serde(default)]
     status: Option<u16>,
+    #[serde(default)]
+    after_status: Option<u16>,
 }
 
 #[derive(Clone)]
@@ -237,6 +239,10 @@
         None => relay(&proxy, request).await,
     };
     let status = response.status().as_u16();
+    let response = if let Some(after) = rule.and_then(|r| r.after_status) {
+        println!("upstream acknowledged {method} {} {path} -> {status}; lose acknowledgement as {after}", owner.as_deref().unwrap_or("-"));
+        (StatusCode::from_u16(after).unwrap(), "Injected lost upstream acknowledgement").into_response()
+    } else { response };
     println!(
         "{method} {} {path} -> {status}{}",
         owner.as_deref().unwrap_or("-"),

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

@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. The earlier LOW about repeated session-expired feedback is fixed.

QA for 7e2fd14: Android 16 / API 36, dev/regtest, unmodified app build. I seeded recovery records in a disposable wallet that had never linked a Pubky profile. Videos are 2×.

Test 1: J4 — pending-cleanup feedback ✅ Cleanup guidance remained available through foreground and reconnect without a session-expired prompt.

Cold starts and three foreground cycles also retained the recovery screen. UI assertions checked each transition; direct storage checks confirmed Cancel preserved the marker and explicit local Disconnect cleared it before returning Home. This recording shows the foreground/reconnect repeat.

J4-feedback-2x.2x.mp4

Pending cleanup through foreground and network reconnection.

Test 2: unreadable recovery record ✅ An unsupported record retained cleanup guidance through restart, foreground and reconnect without an expiry prompt or crash.

I used a version-2 envelope, which the real serializer rejects. UI assertions checked cold starts and three foreground cycles; direct storage checks confirmed the unsupported record stayed unchanged. This recording shows the foreground/reconnect repeat.

U1-2x.2x.mp4

Unreadable recovery state through foreground and network reconnection.

Check 1: build and restoration regressions ✅ The app build and focused repository, Profile and application tests passed.

I built 7e2fd14. The regressions confirm pending/unreadable cleanup blocks authentication without setting the expired-session signal. Genuine restoration failure still produces its notification. git diff --check passed.

I retained the passing report at 7e2e466 for unchanged J1–J3, the remaining J4 actions and manual cases 1–3. I did not repeat those authenticated fault journeys or the full lint/E2E suites in this delta.

Twin: synonymdev/bitkit-ios#1143 fix: bind profile removal to its original owner carries the matching recovery journey and feedback check; it intentionally offers local Disconnect without remote Review. Its authenticated fault journeys remain unverified.

Reviewed all four files in 7e2e466..7e2fd14 and reached callers, plus the prior LOW. Device cases cover the feedback change; the changelog wording has no separate device case. Native contact CAS remains under synonymdev/bitkit-android#1687 Bind profile deletion cleanup to the original sign-in.

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

@jvsena42 jvsena42 left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Requesting changes: one MEDIUM inline. A dropped connection during an ordinary Disconnect leaves only local Disconnect, which orphans a live homeserver session and published endpoints. Reviewed against the base branch (#1690, 2daec90).

Checked and clean:

  • No cross-identity contamination through the single profile_removal.json: create, adopt, approveAuth, backup restore and restoreSession all go through requireNoPendingRemoval, so identity B can't be created or restored while A's marker is pending.
  • Replay after a restart: prepareSignOut reuses the pending UUID, begin is idempotent and complete requires the same UUID.
  • Cancellation during the local tail is covered by NonCancellable from finishProfileRemoval to complete; the cancellable forgetLocalProfileAccess gap only leaves the marker pending, which resumes.
  • A Reset that fails after the remote cleanup keeps its marker, and resetting again completes it (pending restart wipe skips remote cleanup…).
  • No migration path: the store file is new in this PR, so v3.0.0 holds no state for it.
  • iOS twin synonymdev/bitkit-ios#1143 is still a draft. It has no Review action (local Disconnect only), and its marker carries a walletWipe scope.

Performance-relevant only lightly (two DataStore requireAccess() reads per SDK lock); nothing scales with data.

⚠️ Device gate: not run — waits on the MEDIUM fix. Then: profile/ J1, the Disconnect with the network cut mid removal, and unlink-ring-pubky.xml.

@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 the PR-owned delta 581fed7, 595c364 and 78f7209 (the 1686 commits in range are reviewed on that PR): no findings above LOW.

Checked and clean:

  • Recovery PIN: validateRecoveryPin never wipes, and Reset needs no PIN, so a user who runs out of attempts is not stuck. Forgot PIN from this route revokes the session and cannot queue the ordinary ForgotPin sheet. A store read failure disables the phrase button.
  • Interrupted reset: a WALLET_WIPE marker skips remote cleanup and re-runs the local wipe. The marker is cleared only after local success, and a corrupt marker is replaced by a blocking attempt, so no path drops it early.
  • Migration resume: serialised by rnMigrationMutex and guarded by isMigrationChecked / hasNativeWalletData, so it cannot import twice. Every failure before migrationStarted shows the new Retry, and nothing retriggers it in a loop.

perf: relevant, because cold start now waits on the recovery and migration checks. One candidate is still unconfirmed: whether the "MIGRATING WALLET" screen flashes on every cold start, since _isCheckingRNMigration starts true and installSplashScreen() has no keep condition. I'll check it on a device before raising anything.

l10n: new keys migration__status_error, security__reset_pending_text and security__reset_read_error. The longest locales are de at 1.4–1.7× English and ca, el and nl. To be checked on a small device.

Device gate (partial): 78f7209 — not run yet; pending:

  • security/wallet-reset-recovery-pin.xml;
  • a cold-start screen recording against master;
  • the l10n check of the three keys on a small screen.

The device result will follow.

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

Device gate result: approving with one LOW open, inline (Retry text with no Retry button). Fix it here or in a follow-up, your call.

Device gate: 78f7209 — Pixel_9, CI APKs for head and master 9eff707, a funded native wallet. perf: cold start shows no Migration or Recovery screen on either build (0 frames in 5 runs each). Process start → Home averages 2216 ms on head and 2278 ms on master; the first frame → Home step is about 225 ms later on head but hidden under the splash. l10n: migration__status_error, security__reset_read_error and security__reset_pending_text checked in de, ca, el, nl and es. No overflow; the pending text wraps to 5 lines in de, ca and el.

security/wallet-reset-recovery-pin.xml: all 6 actions pass.

  • A wrong PIN only decrements the counter. The attempts link returns to the overview without queueing the Forgot PIN sheet, and the marker survives a relaunch.
  • Reset needs no PIN.
  • A Pending reset wiped local state with no remote cleanup, and only then cleared the marker.
  • With LDK storage made undeletable, Reset failed with WalletStorageDeleteError and kept the marker and keychain. It stayed Pending across a restart, and completed once storage was released.

MigrationRecoveryScreen: with a held write fault, "Wallet migration status is unavailable" appears with Retry. Retry under the fault keeps the screen, and after the fault is released Retry reaches Home and rewrites the store.

Skipped:

  • ⚠️ The journey's unfunded-wallet precondition: ran on a funded wallet.
  • ⚠️ System Back on MigrationRecoveryScreen: not tested.

Not posted: the cold-start "MIGRATING WALLET" flash I had flagged in the code scan did not reproduce on device.

@jvsena42

jvsena42 commented Nov 2, 2026

Copy link
Copy Markdown
Member

Out of scope for this PR, no change needed here; suggesting a follow-up issue in bitkit-android.

A corrupt rn_migration store crashes the app at startup, before the new MigrationRecoveryScreen can show. This happens on master 9eff707 and on 78f7209.

AppViewModel init collects migrationService.isShowingMigrationLoading on Main and calls migrationService.needsPostMigrationSync() with no catch. On master that is AppViewModel.kt:753; at this head it is :804. A store DataStore cannot parse throws there:

ERROR [App.kt:85] Uncaught exception on thread 'main' [CorruptionException='Unable to parse preferences proto.']

The process is then killed. So a store that stays unreadable never reaches the Retry screen this PR adds; only a failing write does.

Reproduction on Pixel_9: stop the app, overwrite files/datastore/rn_migration.preferences_pb with garbage, then relaunch. The app crashes on both builds.

Validate the indexed wallet binding once inside draining receipt admission.
Keep original-job and generation checks per receipt and exercise actual
identity replacement in the backfill regression.

@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 bitkit-android#1693 at a5cd94f1 against merge base 2e79e109. The comparison base changed from 7040c89 to 2e79e10 and no coverage baseline was supplied, so nothing was inherited.

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

Original-owner removal, one-use remote retry, acknowledged cleanup stages, credential-before-watermark ordering, local disconnect, the failed-wipe gate, subscription refusal, and receipt admission match bitkit-ios#1143 at 429a8c47. Android shows a dedicated retry screen when migration classification cannot be read; iOS keeps that wait in setup loading. That is a platform difference. Receipt admission validates the wallet binding once and checks the captured generation on each commit; a profile change does not bump that generation, and an admitted snapshot attributes only payments that match the captured associations.

GitHub reports unit and Compose, lint, and E2E passed on this head. This review did not re-run them. Device testing: not performed. The author's J1–J4 and manual fault-injection cases remain the runtime plan; the standard journey runner does not provide the homeserver mutation gates those cases need. The iOS check is source comparison of the shared contracts only, not a review of that PR.

Earlier findings reconciled at this revision: 2 still present, 1 verified fixed.

Findings

  • [LOW] Unreadable wallet-reset recovery hides Retry while telling the user to retry — inline at app/src/main/java/to/bitkit/ui/screens/recovery/WalletResetRecoveryScreen.kt:244. Still present from an earlier unpublished review.

Existing concerns

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: ✅ Approve. Activity sync with 151 payments is back to base speed (3.4 s on this head vs 7.3 s at 78f7209). On-chain, Lightning and contact receipts still work, including across a wallet reset.

Findings: None new. Piotr's per-payment derivation finding is fixed at a5cd94f. withReceiptOperation now validates the binding once, and requireCurrent() checks only the Job, the session flag and the generation (app/src/main/java/to/bitkit/data/WalletConnectionRecovery.kt:93-106, :192-196). Dropping the per-receipt binding comparison loses no protection, for two reasons:

  • Every mnemonic writer runs inside withIdentityChange, which bumps the generation and drains admitted receipt jobs before credentials change: create (WalletRepo.kt:643), restore (:669), RN migration (WalletViewModel.kt:415, calling :426) and wipe (WipeWalletUseCase.kt:75).
  • Keychain.save refuses to overwrite an existing key.

The open LOW on WalletResetRecoveryScreen.kt:244 (Retry text without a Retry button) is already raised by jvsena42 and piotr-iohk, so I'm not repeating it.

QA for a5cd94f: dev/regtest builds on two Android 16 emulators. On-chain funds came from Blocktank staging regtest deposits; profiles are on the staging homeserver. Lightning used a local regtest bitcoind/electrum/LND. For the timing comparison I also built 78f7209 and the base 2e79e109. Videos are 2× unless noted.

Activity sync time with 150 payments matches the base ✅ With 151 on-chain payments, a sync takes 3.4 s on this head and on the base, against 7.3 s at 78f7209.

Each build got a fresh wallet and the same 151 regtest deposits. The table shows the median time from the getPayments call to Synced 151 payments in the app log. Every sync reported 0 failed.

Build Median sync Syncs measured
base 2e79e109 (#1690) 3.37 s 10
this head a5cd94f 3.36 s 173
78f7209 (before the fix) 7.33 s 7

At 125 payments it was 2.7 s on this head and 5.2 s at 78f7209. Both emulators shared one host, so these numbers compare builds, not devices.

sync-history-head.2x.mp4

The head wallet's balance (173 670 sats) and its payment history, at 1×.

Received on-chain payments appear in Activity ✅ Ten deposits each show once in Activity and stay after a cold restart, with 0 failed syncs in the app log.

The deposits ranged from 10 000 to 300 000 sats and brought the balance to 810 000. The full Activity list held the same ten rows after two cold restarts. The app log has 66 syncs with 0 failed and no receipt-admission or owner-change errors.

receipts-activity.2x.mp4

First deposits and their Activity rows (the recorder stopped after 3 minutes).

receipts-activity-after-restart.2x.mp4

All ten rows after a cold restart.

Received Lightning payments appear in Activity ✅ Two paid invoices (21 000 and 34 000) each show once, also after a cold restart; a third invoice was left unpaid.

The wallet connected to a local LND, which opened a 500 000 sat channel to it. LND paid two zero-amount Bitkit invoices, and each showed "Received Instant Bitcoin", bringing the spending balance to 55 000. A third invoice was left unpaid. After a cold restart both rows were still there, and the 34 000 detail showed Successful. Syncs over 157 payment records reported 0 failed, with no per-payment sync errors in the log.

lightning-receipts.2x.mp4

Local electrum, connection to LND, both payments.

lightning-after-restart.2x.mp4

Both receipts after a cold restart.

A payment from a saved contact shows the contact in Activity ✅ A paid B 25 000 sats from B's contact page; B's receipt is attributed to A, also after a cold restart.

Both wallets created new Pubky profiles and added each other through bitkit://contact links. A paid on-chain from Contact → Pay. B's contact Activity showed "Received from AliV4". After a cold restart, the confirmed receipt's detail still names contact AliV4.

contact-receipt-a.2x.mp4

Sender: profile, contact and payment.

contact-receipt-b.2x.mp4

Receiver: profile, contact and the attributed receipt.

Reset the wallet after receiving and start a new one ✅ The reset of a wallet with 11 payments and a profile finished in about 20 s; the new wallet showed only its own receipt.

After the reset, the new wallet had no profile, a zero balance and an empty Activity list. A 12 345 sat deposit then appeared as its only row. The log shows syncs going 11 → 0 → 1 payments with 0 failed.

wipe-after-receipts.2x.mp4

Reset, new wallet and its first receipt.

Not driven: the PR's J1–J4 and Manual 1–3 cases, which need controlled homeserver and storage fault gates. Their earlier results stand at their own revisions in our 78f7209 report and the reports it lists, and in jvsena42's device gate.

Scope: I reviewed the delta 78f7209..a5cd94f. The PR-owned changes are the receipt-admission fix and its three test files. The rest is the #1690 parent merge (2e79e109), which is reviewed on that PR, and no device case here drove its allowance, backup or restore changes.

Twin: bitkit-ios#1143 at 429a8c4 wraps only inbound payments in its receipt operation, as Piotr noted. This delta has no iOS counterpart to compare.


Reviewed by claude-opus-5-5-medium

@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

@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. With a PIN set, the unreadable recovery screen now shows Retry next to Show Seed Phrase. Tapping Retry keeps the overview. The rest of the forgotten-PIN journey still behaves on this head.

Findings: None. The fix for jvsena42's Retry finding and piotr-iohk's follow-up holds, for three reasons:

  • The secondary Retry now renders for Unreadable whatever canShowPhrase is (app/src/main/java/to/bitkit/ui/screens/recovery/WalletResetRecoveryScreen.kt:244).
  • It still needs an attemptId and !busy, so it never shows twice beside the Unavailable primary Retry, and never shows during a reset.
  • onRetry only re-reads the recovery state and the PIN requirement. It cannot start a reset or open the phrase.

The updated WalletResetRecoveryTest taps the new tag and checks one read, no phrase and no finish.

QA for 3b3b226: dev/regtest build on an Android 15 emulator, on a new wallet with PIN 1234. Between launches, files/datastore/profile_removal.json held a marker with an attempt id and no scope, the same setup as jvsena42's repro. No reset was started and the correct PIN was never entered. Videos are 2×.

Retry on unreadable wallet-reset state with a PIN set ✅ Retry shows between Reset Wallet and Show Seed Phrase, and tapping it keeps the overview, also after a cold restart.

The screen showed "Wallet recovery state could not be read. Retry before continuing." with Reset Wallet, Retry and an enabled Show Seed Phrase. Tapping Retry opened no PIN prompt and no reset dialog, and the marker file was unchanged. After a cold restart, the same three buttons were back, and a second Retry tap again kept the overview.

unreadable-retry-with-pin2x.2x.mp4

Forgotten PIN on the unreadable recovery screen ✅ After Retry, the PIN-protected phrase route, the attempts link and a cold restart all behave as `wallet-reset-recovery-pin.xml` describes.

These steps follow Retry on the overview:

  • Show Seed Phrase opened the PIN prompt with no seed shown.
  • One wrong PIN kept the prompt, now showing "7 attempts remaining. Forgot your PIN?".
  • The link returned to the recovery overview, with no Forgot PIN sheet and no reset dialog.
  • Show Seed Phrase then opened a new PIN prompt.
  • Back and a cold restart returned to the recovery overview, with no Forgot PIN sheet queued.

The device crash log stayed empty.

forgotten-pin-recovery2x.2x.mp4

Not driven again: the sync-time, receipt, contact and wallet-reset cases from our a5cd94f report. This delta does not touch that code, so those results stand.

Scope: I reviewed the delta a5cd94f..3b3b226: the Retry condition, its androidTest, the changelog fragment and the journey step. The device cases drove every runtime change in it.

Twin: bitkit-ios#1143 has no counterpart, since this wallet-reset recovery screen exists only on Android (as the journey's description notes).


Reviewed by claude-opus-5-5-medium

@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

@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 of 3b3b226 since a5cd94f, merge base 2e79e109. Inherited baseline review 5755340345 for unchanged profile removal, credential erasure, migration classification, receipt admission, subscription and Pubky gating, shared-contract parity against bitkit-ios 429a8c47, and the unchanged wipe-admission, PIN-revocation, and corrupt-marker cleanup. This pass rechecked the unreadable wallet-reset Retry control when phrase access is available.

No new actionable code findings.

That Retry control calls the existing read-only refresh. The overview stays in place, the PIN prompt stays closed, and reset confirmation is unchanged. Pending recovery still hides Retry once phrase access is available. Busy and loading states still hide it. A non-profile removal attempt still fails wallet admission, so this refresh does not start React Native migration.

Compared with bitkit-ios#1143 at 21d5ee2e: profile-removal recovery has no seed-phrase route, and journeys/security/wallet-reset-recovery-pin.xml remains Android-specific. That journey’s new Retry action is the runtime check and is not assigned an ID in the PR Journeys list (J1–J4). Device testing was not performed in this review. WalletResetRecoveryTest.unknownMarkerOffersPhraseWithoutAdmittingReset was inspected and not executed; this run does not start an emulator.

Earlier findings reconciled at this revision: 1 still present, 1 verified fixed.

Existing concerns

Reviewer model: grok-4.7
Reasoning effort: high

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-approving at 3b3b2262. The merge a5cd94f1 derives the backup wallet binding once per receipt operation and replaces the per-receipt owner check with a generation check. That is safe because every credential writer runs inside withIdentityChange, which drains receipt admissions first. WalletResetReadRetry now shows for Unreadable without a phrase, as my thread asked; it only refreshes and never resets or wipes. CI green.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-approving at 051f60fb. The three merges bring in #1686's VSS ownership change, which I already reviewed and approved there. The only merge-authored code is in LightningRepo.start: profileRemovalStore.requireWalletAvailable() now runs inside lifecycleMutex after the wipe check, and returns its failure as a Result. It uses runSuspendCatching, so cancellation still propagates. No findings.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-approving at 32fc96f. The merge "fix: keep reset recovery and wallet presence consistent" carries its own code in WalletRepo.kt and MainActivity.kt. I reviewed it as code.

Checked and clean:

  • walletState.walletExists is now only assigned from a keychain read: the constructor, setWalletExistsState, and the collector when the state reaches Clear. resetState() no longer resets it to false. After complete(attemptId), the state moves to Clear and the re-read false in one atomic update, so routing goes from reset recovery straight to onboarding without a frame of the wallet.
  • If keychain.exists throws, the state becomes Unavailable, which routes to WalletResetRecoveryScreen with Retry. The wallet never shows over an erased seed. No path writes walletExists = false without a keychain read, so onboarding cannot open over a seed that still exists.
  • MainActivity now reads the same walletState.walletExists that ContentView already uses. WalletViewModel.walletExists still follows every state emission, and the RN migration path calls setWalletExistsState().
  • The new WalletRepoTest cases cover Clear publishing absence and unreadable presence staying Unavailable.

Device gate: not re-run. Reaching the reset-completion routing needs an interrupted profile-removal reset, and the new unit tests cover it. The gate from my earlier approval stands for the rest.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-approving at c228adb. The merge brings in #1690's "recheck subscriptions before profile deletion" (8ceaca2), which I reviewed on that PR. Its conflict resolution touches only PubkyRepoTest.kt and PaykitSdkServiceTest.kt; the app code resolves automatically. Nothing in this PR's own code changed.

@ben-kaufman
ben-kaufman requested a review from ovitrif November 4, 2026 18:18

@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 — no new findings in this delta; the newly driven cases passed.

Findings: None new. The deferred profile recovery gutter issue remains separately tracked.

QA for c228adb: Android 16 / API 36 emulator, disposable unfunded dev/regtest wallets. I built the exact head and verified installed APK hashes. The pending-reset and final stock-app checks used the pristine APK. The fault cases used unpublished diagnostic controls, with the production recovery/coordinator logic and native libraries retained. Videos play at 2×.

Finish a pending reset without reopening the erased wallet ✅ Cancel preserved recovery; confirmed completion and cold restart reached onboarding.

The pristine app started with intact wallet credentials and a seeded WALLET_WIPE marker. Cancel retained that marker. Fresh confirmation completed the real local reset; stored-state and app-log checks confirmed credential erasure and marker completion.

reset-completion.2x.mp4

Pending recovery, Cancel, completion and cold restart.

Retry credential presence after reset completion ✅ Recovery stayed closed while presence reads failed; releasing the fault and tapping Retry reached onboarding.

The diagnostic control refused only mnemonic-presence reads after actual credential erasure. Repeated Retry retained recovery. State observations confirmed that the successful retry published Clear with walletExists=false together; it did not start another reset.

presence-retry.2x.mp4

Presence-read refusal, retained Retry and recovery.

Drain an original VSS operation before resetting the wallet ✅ Original work drained before credential erasure, then fresh work was admitted.

A diagnostic coroutine held the real VSS coordinator's admission while the ordinary Reset action ran. Its trace confirmed cancellation, draining with credentials still present, subsequent node stop and erasure, and admission under the new generation. This controlled coroutine case is separate from the native upload below.

vss-drain.2x.mp4

Reset waits for the controlled original operation, then completes.

Retire a native upload before replacing the wallet ✅ Reset waited for the held native operation; the replacement store did not contain its key.

A transparent proxy forwarded a real native VSS write to staging, received HTTP 200 and withheld that response. Reset closed admission while credentials remained present. Releasing the response let the old operation unwind under cancellation before erasure. Native readback after creating the replacement wallet found the original diagnostic key absent from its store. Proxy and state observations establish that ordering and isolation; the recording shows the reset and replacement setup. This tests an accepted write with a delayed response, rather than every pre-commit transport timing.

native-upload-reset.2x.mp4

Held native response, reset completion and replacement setup.

Open the replacement wallet on the stock app ✅ The pristine app cold-started the replacement wallet and completed its Settings backup.

After restoring the exact pristine APK, the node and both standalone VSS clients initialized. Changing local currency to GBP produced an acknowledged SETTINGS upload and Latest Backup in the UI, confirmed in the app log.

replacement-wallet.2x.mp4

Stock-app launch and the replacement wallet's Settings backup.

Check: reset, VSS and subscription regressions ✅ Focused repository, coordinator and service checks passed.

The checks cover atomic reset/presence publication, read Retry, receipt and VSS retirement, node startup, backup scheduling and final subscription admission. The inherited parameterless subscription guard has component coverage here; no new authenticated late-subscription device case is claimed. Full lint, the full unit suite and stock E2E were not repeated.

Retained passing evidence keeps its original revision and limits:

Reviewed the contiguous 3b3b226..c228adb delta, reached callers and prior discussion with an independent reviewer. Parent VSS retirement and subscription changes were checked for their interactions with profile removal. Native shutdown failure and the inherited failed-payment routing-cache branches received no separate device fault injection; changelog edits were source-checked.

Twin: bitkit-ios#1143 fix: bind profile removal to its original owner at 21d5ee2 matches the shared finish-reset contract through exact marker completion and session-tree replacement. Android's atomic wallet-state routing is platform-specific. Source comparison only; no new iOS run.


Reviewed by gpt-6.1-sol-high

@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

@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 the complete PR diff at c228adb7 against 8ceaca25, following the comparison-base change; no baseline coverage was inherited.

Code review completed with unverified earlier concerns.

Checked removal/recovery contracts against the pinned iOS implementation and Paykit RC4 source. This comparison does not constitute a full iOS review. Native conditional contact deletion remains outside this PR's contract.

Validation: the hosted build/unit job passed on merge 49461854, whose tree was verified identical to the pinned head. Locally, five existing identity-admission tests passed in an isolated JVM harness using unchanged production/test sources; XML parsing, added translation-key coverage and diff whitespace checks passed. Local app tests stopped during configuration because the Android SDK is unavailable. The remaining app/Compose tests were inspected, not executed locally.

Suggested additional test cases

  • Android: on disposable wallets with distinct Pubky identities, leave profile removal pending after its durable checkpoint, then receive a Lightning payment. Verify its amount and final status appear in history; complete local Disconnect and restart, and verify the recorded payment remains intact.

Device testing: not performed in this review. Authenticated fault-injection journeys, full wallet teardown and visual behavior remain unverified by this run.

Earlier findings reconciled at this revision: 1 still present, 24 verified fixed, 1 not substantiated, 4 could not be verified.

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 — Whole-store RN migration corruption can escape startup recovery: The reported uncaught needsPostMigrationSync read remains in the current source and supplied base, separately from classification Retry. Its complete current runtime impact and any changed reachability were not established in this code-only reconciliation. A focused same-repository issue search found no matching tracking issue, so tracked_separately cannot be substantiated. Retain the inherited startup-corruption report as unresolved; no new PR defect, fix or runtime pass is claimed.

Could not verify earlier concern — Unconfirmed long-running public-payments enable: The earlier reviewer explicitly reported one non-reproduced 12-minute enable without establishing a defect. Source inspection identifies its entry point but cannot establish the server state, held operation or cause at this head. No device or backend fault case was run. Preserve it as an unverified observation rather than claim a fix or promote a PR finding.

Could not verify earlier concern — Unconfirmed Contact screen actions after local Disconnect: The earlier reviewer observed returning to Contact with Pay/Delete visible but did not compare the base or exercise a failing action. Current ContactDetail retains profile UI state and routes actions through repositories; visibility alone establishes neither an authorization bypass nor an introduced correctness failure. The exact navigation history and action outcome remain unverified.

Could not verify earlier concern — Inherited complete-WALLET backup failure after terminal identity loss: The latest target review retains the parent's complete-WALLET failure after terminal identity loss and distinguishes its successful SETTINGS upload. The supplied review lacks enough failure-state and transport evidence to independently verify that case at this head. Current complete capture still depends on identity/SDK export. Preserve the limitation without converting the Settings result into complete-WALLET acceptance or inventing introduction evidence.

Existing concerns

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

@jvsena42

jvsena42 commented Nov 5, 2026

Copy link
Copy Markdown
Member

Parity with synonymdev/bitkit-ios#1143 is tracked in #1797. This PR has no blocked-peer exclusion in the profile-removal cleanup, and profile-cleanup-failure.xml is missing the newer iOS actions. The Android rule should wait until the open MEDIUM on #1143 is settled.

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.

6 participants