Skip to content

fix: recover connections during wallet restore - #1139

Open
ben-kaufman wants to merge 25 commits into
fix/backup-observer-lifecyclefrom
fix/restore-connection-recovery
Open

ben-kaufman wants to merge 25 commits into
fix/backup-observer-lifecyclefrom
fix/restore-connection-recovery

Conversation

@ben-kaufman

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

Copy link
Copy Markdown
Contributor

Current metadata-parent composition — November 6

Signed additive head 8eadf2b9d47cb273f33e19e1e7c45b443c70cce5 incorporates #1123's metadata dependency wake repair while preserving this PR's restore/recovery and event-drain behavior. Both affected source reviews and required local validation are complete. All 25 live-parent commits verify locally and on GitHub; this PR remains ready.

Native CI and SwiftFormat passed on their first attempts, on the exact published tree: 3,214 unit + 40 physical UI passes (3,225 unique passes), five explicit skips, no test or helper retries, format 0/856 with 118 exclusions. Nine migration jobs skipped; master-only E2E/integration/translation jobs did not run. Complete diagnostics and all 13,606 sealed files were inspected and hash-verified. One keyboard timeout, five SwiftUI update events and 48 NaN records in one USDT case retain unresolved causes; this is not a warning-clean claim. Audit: work/validation/hosted-ios1139-8ead-20261105/report.json, SHA256 cbc12de8e5013f0427d3e5fdf221a78b9b3e76d2b898d73cc411d292b639c84c.

The separate external review at this head reports a controlled real VSS acknowledgement hold and native status/stop ordering. That remains attributed evidence with its Debug controls and stated limits. Its first Contacts recovery rendering failure and subsequent unchanged isolated pass remain unresolved; the author and hosted passes do not erase that failure. Literal Manual 3 network-loss/reconnect callback remains unrun; full-wallet recovery and all other recorded acceptance gaps remain separate. Earlier authored QA and historical results follow unchanged.

Recovery-drain and event-queue fixes — November 5

At 154326affef9a14f5c235b93aedab85d9bff253b, AppReset's deadline now also bounds the initial recovery drain. Timeout exposes recoverable quiesce failure while the original startup retains admission and drain ownership; retry waits for that same work, and no erase is admitted early. Event polling and acknowledgement use a separate serial ldkEvents queue so remote persistence cannot monopolize the shared SDK queue. Stop still drains admitted delivery.

Two new regressions cover repeated wrapper timeouts around a non-cancellable startup, safe eventual retry, and independent SDK reads while acknowledgement is held. Both independent concurrency reviews are clean. The exact six-file candidate passed 3,178 unit + 40 physical UI executions, five explicit skips, zero failures/retries, and full SwiftFormat 0/856 (116 exclusions). Four integration classes remain excluded. Complete diagnostics and 13,234 sealed files were inspected and verified. Thirty-two runtime warnings and a newly observed Calculator preference advisory remain documented; their causes are not waived. Xcode 27.2 / preserved iOS 26.2 (23C54), 1,730 matching inputs.

All 24 live-parent commits verify locally and as verified=true/reason=valid on GitHub. Both findings are replied to and resolved. New-head native CI and SwiftFormat passed on attempt 1, on the exact published tree and parent. Hosted results are 3,178 unit + 40 physical UI passes (3,189 unique passes), five skips, zero failures and no test/helper retries; formatting is 0/856, with 116 exclusions. Complete logs, 3,227 nested objects, 425,926 diagnostic lines, all 72 attachments and 17 annotations were inspected. The accountable auditor verified 16,677 sealed evidence members, artifact digest/CRC and all 1,730 source inputs; the owner read the complete report and verified its seals. The 286 unique linker warnings match the actual parent. Forty-eight CoreGraphics NaN messages occurred in two stock USDT UI cases; the second-case observation is new against the retained parent/previous-child runs, and its cause or relation to queue timing remains unknown. Private-registry transport warnings also remain qualified. Nine migration jobs skipped; untriggered integration/E2E/translation workflows receive no hosted pass credit. Full report: work/validation/hosted-ios1139-1543-20261105/report.json, SHA256 9c75ddeaab61caeb9f586e31946c6b01fa5ada4fed42efdc5ab728f4483ed283. These controlled tests do not establish an actual wallet reset, native LDK quiescence or a real VSS outage. Literal Manual3 network-loss/reconnect callback and all existing acceptance gaps remain open.

Expired cleanup admission composition — November 5

Signed additive merge 4d265af6b8f2681125f21434a54f3dd2fb5d1807 incorporates live #1123 parent 003ea7e818a8e927469d9278e739da8bc1b91843. The inherited fix rechecks cancellation and original wipe ownership after suspended reads and before the next private recovery or endpoint mutation. The complete 36-path connection-recovery delta, correction/restore markers and callback drains are preserved. The affected integration review is clean, reusing both independent parent reviews for identical source.

The actual composed tree 952f9e3f0c392b3097f3e1c3404e11ca0f32b1ed passed 3,176 unit and 40 physical UI cases (3,187 unique passes), with zero failures, five explicit skips and no whole-test retries. One N3 helper tap retry in a passing USDT UI case is retained. All 1,730 inputs match; full SwiftFormat is 0/856 with 116 exclusions. Translations have zero errors and 17 entries verified against actual-parent emitters and missing language/key pairs, without claiming child resource files are identical. Complete logs, nested results and 72 attachments were inspected; all 15,217 sealed evidence files verify. Twenty-four runtime warnings and two AppIntents notices remain qualified. Validation used Xcode 27.2/27B52 and preserved iOS 26.2/23C54.

All 23 live-base commits verify locally and on GitHub; draft=false. New-head native CI and SwiftFormat passed on attempt 1, with the exact published tree and live parent. Hosted results are 3,176 unit and 40 physical UI passes (3,187 unique passes), five explicit skips, zero failures and no test/helper retries. Hosted formatting is 0/856, with 116 exclusions; nine migration jobs skipped. Master-only integration/E2E/translations were untriggered, so the translation result above remains local evidence.

Complete logs, 3,225 nested objects, 420,416 diagnostic lines, all 72 attachments and 17 annotations were inspected. The owner verified 16,656 sealed evidence files, artifact digest/CRC and all 1,730 input hashes. Five interleaved raw outcome lines were reconciled against canonical results. The 286 unique linker messages match the actual parent; keyboard timeout, conditional UI-query interruptions and detector limits remain disclosed with unknown timing causes. The hosted pass does not explain the separate local N3 retry. Hosted report: work/validation/hosted-ios1139-4d26-20261105/report.json, SHA256 cd9838a50b6442d190f2357522dca6a91233632e9c68b21469d954816b304eb3. All three named reviewers remain pending; no new request was sent. Local report: work/validation/ios1139-revision-admission-20261105/final-audit/report.json, SHA256 dca031176a635955f3c49846ad69a5329a3d508fc9013d4ff006291f68befb70.

The external review 5777589674 at prior d8fa381f was independently checked against its five-file DEBUG patch and five supplied videos. It provides bounded visual evidence for portions of J1, Manual 1/2 and PIN handling. Original build/install/binary, complete process/boundary traces and remote-version provenance remain unavailable. This is attributed external evidence, not new author acceptance. The genuine network-loss → connected callback → restart case still needs the documented isolated real-transition fixture. Neither these clips nor the native gate closes that gap, terminal complete-WALLET work or running-LDK quiescence. Earlier failures, skips, authored QA IDs and revision-specific evidence remain below.

Durable-wipe parent composition — November 4

Published signed additive merge d8fa381f07133a325915fa62b0c30563a0cbbc6c onto live #1123 parent f514d199e1cceb99167d36a426e91bf73cfa2a33. This preserves original-owner connection recovery and callback admission while incorporating the parent's durable wipe, VSS and backup drains. All 22 commits in the complete live-base range verify locally and on GitHub (verified=true, reason=valid).

Final combined-tree validation used Xcode 27.2 (27B52) and preserved iOS 26.2 (23C54): 3,172 unit and 40 physical UI passes, five explicit skips, zero failures or whole-method retries; 3,183 unique passes. Thirty launch configurations explain the extra invocations. All 1,730 frozen inputs match. SwiftFormat reports 0/856, with 116 configured exclusions; translations report zero errors and 17 warnings verified against the actual parent. Both independent affected source reviews are clean, with the actual composed bytes bound to the reviewed preview.

Complete diagnostics cover 33,975 raw lines, 441,412 nested lines, every XCResult reference and retained attachment integrity. The owner verified 3,363 sealed audit members. The result retains 20 runtime advisories and two AppIntents notices; 17 runtime observations match the actual-parent method/message, while timing and causal attribution remain unresolved. No introduced compiler or formatting violation was found. Evidence: work/validation/ios1139-wipe-parent-final-audit-20261104/REPORT.md, report.json and owner-integrity.json. New-head hosted native run 41927185198 and formatter run 41927185251 passed on attempt 1, with the synthetic checkout tree matching the published tree exactly. Hosted results independently reproduce 3,172 unit and 40 physical UI passes, five skips and zero failures/retries; SwiftFormat reports 0/856. The complete 24,666,447-byte XCResult artifact, all 6,599 archive entries, every nested reference and all 17 annotations were inspected; the owner verified 9,989 sealed evidence members. Canonical linker diagnostics retain 572 warnings across 286 messages, with the beautified multiset matching the actual parent; runtime advisories and disabled detector limits remain explicit. Nine migration jobs skipped, and master-only integration/E2E jobs did not trigger. Evidence: work/validation/hosted-ios1139-d8fa-20261104/REPORT.md, report.json and owner-integrity.json. These results do not close the literal network-transition case below.

The literal new-wallet network-loss → connected-callback → restart case still needs the documented isolated real-transition fixture. Five skipped cases and four excluded integration classes are not passes. Failed-start node handling does not prove running-LDK quiescence; authenticated journey and terminal complete-WALLET limitations remain separate. Earlier evidence stays tied to its original revisions. The failed Xcode 26.1 attempts are preserved; no wallet or simulator data was erased. PR remains ready, draft=false.

Auth transport parent composition — November 3

Signed additive merge 506362583c20ec4f47fa239d11ec7b0bfb48d2d9 incorporates live #1123 parent 11fb65d780055a10081bd9d7e12fee61949d140d. It carries the reviewed typed auth request-send classifier and regression tests. The independent composition review verified the unchanged 35-file child delta, including restricted connection recovery, original-owner admission, required restore markers and wipe drains. All 20 live-base commits verify locally and on GitHub. The PR remains ready.

Required composed native validation passed 3,008 unit tests and 39 physical UI executions, with five explicit fixture/hardware skips, zero failures and no method retries. Thirty distinct launch configurations explain the difference from the wrapper's logical count. All 1,697 frozen inputs match; full SwiftFormat is 0/834, with 108 configured exclusions. Complete raw and nested diagnostics were inspected and 9,410 evidence members verified. Two AppIntents notices and twenty canonical QoS observations remain recorded; their retained stacks enter unchanged DHT/flume or LDK status/cache paths, without a timing-cause or blanket warning-free claim. Four workflow integration classes were excluded and receive no pass credit.

Evidence: work/validation/ios1139-auth-parent-final-audit-20261103/report.json (SHA256 e4e2d9d3da25d1061dbe35a8c3590535688b2b2910c92062d203dce4cf01d8d6) and the exact composition/source review receipts. Current-head native CI 41737947938 and SwiftFormat 41737947954 passed on attempt 1. Both jobs used synthetic checkout baa548ad, whose tree exactly matches this head and live parent. Canonical results record 3,008 unit passes and 39 physical UI passes, with five explicit fixture skips, zero failures and no retries. All three inherited auth controls passed; thirty distinct launch configurations account for the physical count. Hosted SwiftFormat is 0/834, with 108 configured exclusions. Migration was skipped.

The complete logs, nested test/diagnostic references and checksum/CRC-verified artifact were inspected, and the owner verified 9,423 sealed evidence members. The 573 build warning/error annotation occurrences match the prior published head exactly; runtime diagnostics and their differences remain retained without a warning-free or causal claim. Report: work/validation/hosted-ios1139-auth-parent-5063-20261103/report.json, SHA256 ad491855f59ca0a2f1da03abbab271ac72384d1bce83118ff7ba33d19e10493e. These checks add no new literal network-reconnect, authenticated required-restoration or terminal complete-WALLET result; prior case-specific evidence and limitations remain below.

Historical scheduled-backup completion composition — November 3

Signed additive merge 85db6cfa36176f42cf7bee5bf7279e122e44392e incorporates #1123 4e6c06ed. Completed scheduled work is removed from admission before publishing idle in the same serial queue operation. Restricted connection recovery, required restore gates and wipe drains are preserved; the affected composition review found no new interaction.

The actual combined tree passed 3,005 unit tests and one Profile Recovery UI test, with zero failures/retries and one opt-in native-fixture skip. Raw and nested results account for one passing case omitted by the MCP summary. All 1,697 frozen inputs match. SwiftFormat reports 0/834, with 108 configured exclusions. Complete diagnostics retain four QoS observations in parent-identical method bodies and two AppIntents notices, without a timing-cause claim. The owner verified 6,053 diagnostic members. All 19 live-base commits verify locally and on GitHub.

New-head hosted native CI 41707692883 passed on attempt 1: 3,005 units and 39 UI executions, zero failures or test retries, with one unit and four UI fixture skips. The synthetic checkout 08495158 exactly matches the published tree and live parent 4e6c06ed. Hosted SwiftFormat reports 0/834; nine branch-gated migration jobs were skipped, not passed. Complete logs, nested XCResult diagnostics and checksum/CRC-verified artifacts are audited; all 9,348 sealed members verified. The 286 distinct linker warnings match the actual-parent set. Evidence: work/validation/hosted-ios1139-85db-20261103/report.json. This composition adds no new authenticated connection journey or terminal complete-WALLET result; the prior case-specific evidence and limits remain below. Evidence: work/validation/ios1139-idle-final-diagnostics-20261103/report.json and work/validation/ios1139-idle-composition-20261103.

Hosted validation at 34925fb4 — November 3

Hosted native run 41689645887 at 34925fb46254ee73e2af1c9996799387ff9a7564 passed 3,004 units and 39 UI executions, with one unit and four UI skips, zero failures and no retries. The synthetic checkout tree exactly equals the published head and current parent c6336881. SwiftFormat reports 0/834. All nine migration jobs were branch-gated and unrun, not passes.

Complete logs, nested XCResult references and checksum/CRC-verified artifacts are audited. The profile recovery clipboard case passed without PBErrorDomain 13. The evidence retains 286 distinct native linker warnings and 48 CoreGraphics NaN messages during passing USDT UI cases; their runtime cause remains unproven. These results do not establish the remaining authenticated connection/restore cases. Evidence: work/validation/hosted-ios1139-3492-20261103/ and its separate linker-attribution addendum.

Cached backup composition and clipboard fixture repair — November 3

Signed additive head 34925fb46254ee73e2af1c9996799387ff9a7564 incorporates live #1123/c6336881d41b8aefe9a967e282b11c2a9301f3eb; tree b24e574df6e3617b0c430c77af6d71373e75e312. Unreadable cached Blocktank info now refuses capture and preserves the acknowledged backup/pending revision. Genuine absent cached info remains valid. Released Core's nil-ID lists were already local; this is not a network-refresh-cause claim or the separate terminal complete-WALLET repair.

A DEBUG-only ProfileRecovery fixture used a programmatic clipboard read that returned PBErrorDomain 13; the unchanged isolated UI check reproduced the failure. It now uses Apple's system Paste button and verifies actual negative-sentinel delivery before each Copy/QR positive equality. Production copy, recovery controls and five-second assertions are unchanged. The corrected full ProfileRecovery UI flow passed on each child's own inputs; here one UI case passed with zero failures/skips, including both copy branches and subsequent disconnect/recovery controls.

The earlier full selected unit target passed 3,004 cases, with one unavailable protocol-fixture skip and zero unit failures. Its whole run remains failed because of the UI assertion. Those unit results are reused only after independent verification that all 1,695 other inputs are unchanged and no unit invokes the changed DEBUG fixture. This is not a new execution of the unit suite on the fixture edits. The final app/UI build and affected test passed; full pinned SwiftFormat reports 0/834, with 108 configured skips, and all 1,697 final hashes match. Complete original and new logs/nested diagnostics were audited; six original QoS observations and two metadata notices remain preserved. Both original clipboard failures remain recorded; their underlying platform denial cause is not claimed fixed.

The affected composition and fixture reviews are clean; prior independent persistence/recovery reviews remain applicable. All 18 live-base commits verify locally and on GitHub as verified=true/reason=valid. Ready status is verified (draft=false). New-head hosted CI is fully inspected in the November 3 record above. Existing J1/Manual evidence and gaps below retain their original scope; this adds no authenticated restore, wipe, literal reconnect or complete-WALLET acceptance.

Evidence: work/validation/ios1139-cached-blocktank-composition-20261103/, ios1139-profile-clipboard-source-review-20261103/ and ios1139-system-paste-ui-diagnostics-20261103/.

External controlled offline Retry evidence — November 2

Automated review 5731739587 reports the installed 4637d916 ordinary-startup/offline Retry flow using a disclosed temporary DEBUG connectivity control. The author inspected its video, verified the full decode and original SHA-256, and checked that the disclosed patch applies to the published source. The media shows retained error/settings controls, a corrected Electrum endpoint, and later initialization/Home beneath the USDT introduction.

This is attributed external evidence. The video alone does not verify two Retry taps, native acknowledgements, correction deletion or installed-binary identity; the report's raw logs and build/install receipt are not attached. In follow-up 4459922541, the reviewer confirmed those earlier receipts were not retained, so that execution cannot be fully bound independently. Its separate report of a later real host-network cut remains attributed evidence with its own scope. The temporary monitor does not establish literal transport loss/reconnect. Existing authenticated J1/Manual evidence retains its original revision and scope; no new restore, wipe, nonempty USDT or spending pass is claimed. Canonical current-head CI below is unchanged. Evidence: work/validation/ios1139-external-offline-retry-20261102/REPORT.md.

Previous-head hosted native validation — November 1

Head 4637d9162f79625679a2d5013649141ee779fa4c passed 3,002 unit tests + 39 physical UI executions (3,041 passes), with five explicit skips and no failures. The 30 launch executions are distinct UI configurations, not retries. Hosted formatting passed 0/833, with 108 configured file skips. The actual synthetic checkout combines this head with #1123/ff3ba674 and exactly matches its published tree 99373dc41dae5137935720c8cb418536f0108d2f.

Complete logs, all canonical/session results, nested diagnostics and checksum/CRC-verified artifacts were inspected. The console projection omitted some successful rows; canonical results and full sessions reconcile exactly. All 286 canonical duplicate-linker warning messages match the retained verified message set and actual-parent linking/pin inputs; no source compiler finding was identified. Runtime diagnostics remain disclosed without a blanket harmlessness or causal-fix claim. The five native fixture/hardware skips and nine unexecuted migration jobs are not passes. The original decoder permission failure and bounded offline recovery are preserved. This does not establish the separate authenticated/manual journey cases. Evidence: work/validation/hosted-ios1139-4637-20261101-current-a/STRUCTURED-INSPECTION.json.

Connection Retry failure recovery — November 1

Signed 4637d9162f79625679a2d5013649141ee779fa4c incorporates live #1123/ff3ba674464c796057415188d460e6009371496a. If an explicit connection Retry returns without acknowledged node startup, the restore-failure controls remain reachable. The existing owner/generation/cancellation checks still run first; an acknowledged startup followed by an ancillary failure is not relabeled as failed startup. The incoming channel-close refresh is composed with this branch's synchronous MainActor event handling.

Controlled before/after checks retain two expected pre-fix failures and four passing corrected cases; they are component controls, not a newly completed offline-app journey. Final required native build/full selected units plus affected ProfileRecovery UI passed 3,002 units + one UI execution, zero failures/retries, with one explicitly unprovisioned native-fixture skip. All 1,696 frozen inputs match. Full SwiftFormat passed 0/833. Complete logs and nested XCResult were inspected, including six unsymbolized QoS observations, three AppIntents notices and the formatter cache notice. No source compiler/format finding was verified; all 12,256 sealed evidence files were checked.

The affected connection fix and parent composition reviews are clean, with unaffected independent recovery/concurrency coverage retained. All 17 live-parent commits verify locally and on GitHub. New-head hosted CI is pending complete inspection. Exact new offline Retry, authenticated recovery, process-kill and terminal complete-WALLET cases remain distinct and are not claimed by this gate. Evidence: work/validation/ios1139-parentff3b-final-a/, ios1139-final-parent-review-20261101/ and the preserved offline-Retry control receipts.

Previous parent composition — November 1

Signed additive merge 89e5e941268ff48835a3c49e9297f8ced60902d3 incorporates live #1123/2c0c1002e03aa9c91828b47bf4db73067270463f; tree 85acc48189c5a3e2d5d43fa6d8a35ce218c4382b. It carries the shared queued-offer admission repair without changing this PR's connection-recovery delta. The affected composition review is clean; two independent shared concurrency reviews remain applicable.

Required build/workflow-selected units and affected ProfileRecovery UI passed 3,001 units + one UI execution, zero failures/retries, with one explicitly skipped native-protocol fixture. All 1,696 frozen inputs match. SwiftFormat passed 0/833, and translation validation is reused only after relevant-input verification: zero errors, 15 warning rows individually verified against the actual parent. Complete 23,064-line/nested diagnostics and all 12,229 evidence files were inspected and sealed. Six unsymbolized runtime QoS warnings and three AppIntents notices remain; no blanket warning-free or causal-repair claim is made.

All 16 live-base commits verify locally and on GitHub. New-head hosted CI is pending inspection. This gate selects ProfileRecovery UI only; prior full-UI/camera evidence below stays tied to its original revision. Authenticated connection-recovery faults, process-kill and complete-WALLET availability are not established by this component/native gate. Evidence: work/validation/ios1139-parent2c0c-final-a/ and work/validation/ios1139-parent2c0c-20261101/publication/.

Historical validated composition — October 31

Signed additive head f8bd749b2db941ef92af6864a6a053d4fc1d60bb incorporates live #1123/fb25166fd830f8d59368c09079545a1abcdb5cb9; tree b46325b567faad9a285547157dca992663d7b8c7. The 33-file owned patch is unchanged after excluding blob IDs and hunk offsets. One fresh combined composition review is clean; the earlier independent concurrency reviews remain applicable.

Final local validation passed 3,000 unit and 39 UI executions, with zero failures/retries and five explicit skips. All 1,696 frozen source inputs match. SwiftFormat 0.64 passed 0/833, with 108 configured exclusions and the retained optional-cache warning. Translation validation reports zero errors and 15 warning rows individually verified against the actual parent. Complete raw and nested diagnostics, all 3,044 summaries and 68 attachments were inspected. The wrapper omitted three passing cases and 11 QoS observations; these, 72 CoreGraphics NaN messages, two AppIntents notices and dependency-cleanliness limitations remain documented.

Evidence: work/validation/ios1139-parentfb25-final-a/REPORT.md, manifest b53ee66dfb87c29eaab61258b008a6825d2de8edb0d4a57c2e962d3ea387a9a7, and the inventory-only addendum binding two copied parent receipts. Root verified all 18,832 listed files, the external review, both additional receipts and current source hashes. All 15 live-base commits verify locally and on GitHub as true/valid. The PR remains ready. Hosted native run 41247489094, attempt 1, passed 3,000 units and 39 physical UI executions, with five explicit skips and zero failures/retries (3,010 logical passes). Synthetic 02cdfe7259fd4a67bd273cca2747cd846d29fa86 has this exact tree and live fb25166f parent. Hosted SwiftFormat passed 0/833; nine migration jobs skipped, not passed.

The complete 5,964-line log, all 3,044 summaries, 68 attachments and nested diagnostics were inspected. Artifact 13060702357 matches published SHA-256 6f44eee2dce56dfb7cf83a7d724b4ec59cfb1c86f031304c58c0262a3b8ebb43; all 6,243 ZIP members passed integrity checks. Root verified 18,864 listed files, 16 external inputs and an inventory-only addendum for one copied parent seal. The 571 duplicate-symbol events/286 messages match the sealed actual parent. Two AppIntents notices, 48 NaN messages, preference/navigation/appearance observations and a shared-keychain -25300 observation remain disclosed. Runtime issue detection/leak checks/test timeouts were disabled in recorded XCTest configuration; original explicit assertion deadlines were retained. Skips and four workflow-excluded integration classes receive no pass credit. Evidence: work/validation/hosted-ios1139-f8bd-20261101/completed/REPORT.md, manifest 0ea83bc39c61170e59fb4428c57d4a80c304f449beaaac4ba867fdf3f89a7055 and its inventory addendum. This adds no authenticated J1, nonempty USDT import, actual wipe or literal reconnect proof.

The prior 13b91199 hosted camera-Allow failure remains unexplained; this local pass is not a causal repair or an unchanged CI retry. Existing J1 and Manual 1–3 evidence retains its original revision and scope. Literal transport loss/reconnect (Manual 3) remains unrun. External review 5725005865 reports a held-start Settings save and a subsequent regtest payment on 13b91199; these are attributed reviewer results, not new author runtime passes or human approval. Its idle CPU/log sample does not establish causation; #1181 remains deferred.

João, Piotr and Ovi already have pending requests; no duplicate notification was sent.

Earlier startup/stop review fix — October 31

Signed head 13b91199d523b5b3f2af665ac8d247715ceebd93, tree 7abcb8227898ab92a6fe3670be913bd019a2df97, at that revision retained parent #1123/262a44747ffd8ef522178bfd3850fe78da15cce5. João’s startup/stop finding is repaired: a stop arriving during synchronous startup reserves exclusive cleanup ownership, waits for that startup to settle, then drains the listener and stops the same node. Cancellation cannot release that ownership early; failed startup also releases the waiter. Competing stops are rejected. Existing acknowledged callback delivery and stop-error handling are preserved; this does not claim to drain downstream unstructured persistence tasks.

Controlled original-code tests failed as expected (4 pass/2 fail); a strengthened competing-stop control caught the unpublished intermediate candidate (6 pass/1 fail). The final seven lifecycle cases pass. Full required local native validation passed 2,991 unit tests, zero failures/retries and one explicit local-protocol-fixture skip. All 1,689 frozen inputs match; SwiftFormat 0.64.0 reports 0/832 with 106 configured skips. Complete 22,524-line native output, every canonical test summary, exported diagnostics and all five nested SDK-construction QoS observations were inspected. Two AppIntents notices and the QoS timing-cause limitation remain recorded; no changed-line compiler/formatting finding was introduced. Two independent concurrency/recovery reviews are clean. All 14 commits in the complete live-base range verify locally and on GitHub (verified=true, reason=valid).

One earlier local attempt ran no tests because the simulator refused app launch; one owned-simulator boot restored execution without a reset. That failure remains preserved.

Current-head native CI 41229493429, attempt 1, failed one UI execution: at UsdtWalletUITests.swift:288, the camera permission alert remained visible for the unchanged five-second deadline after Allow was tapped. 2,991 unit and 38 physical UI executions passed, including all seven listener lifecycle cases; five prerequisite cases skipped. Thirty planned launch configurations are separate executions, not retries. The failing case completed Receive checks but did not reach its later invalid-recipient/payment assertions. SwiftFormat passed 0/832, with 106 configured skips; nine migration jobs skipped. Synthetic merge 2204beaf has the exact published 7abcb822 tree.

Complete logs, all 3,035 nested execution summaries, 28 diagnostic text files, all 79 unique attachment payloads and the failure-time system trace were inspected; the owner verified 10,976 sealed evidence members. The centered Allow tap was synthesized, but one SpringBoard event records shouldSend:0 / ignoreInteractionEvents:1; TCC did not return during the captured failure window. Nineteen automation-type mismatch records, including eight uncaptured issues, are retained. Execution used Xcode 16.4 / SDK 18.5 against iOS 26.2/23C54. These observations do not establish the cause or justify weakening the assertion. The same camera assertion failed on historical head f38c33e2 in run 40947275964; its later authorized retry is historical evidence only. No retry of the current run was made. The 286 distinct dependency linker warnings retain their verified actual-parent comparison; no nested QoS warning was found in this hosted execution.

Existing J1/Manual evidence below keeps its original revision and fixture scope; no new full-app authenticated journey is claimed. Current failure evidence: work/validation/hosted-ios1139-13b9-20261031/REPORT.md and work/validation/ios1139-13b9-root-integrity-20261031.json. Local source-fix evidence remains in ios1139-start-stop-20261031/, ios1139-start-stop-final-a/ and ios1139-start-stop-publication/.

Prior-head external journey report — October 30

Review 5718243768 reports J1 correction/import-refusal/restart recovery at exact head ca3dc95d, using real authenticated staging VSS and disclosed unpushed DEBUG staging/rejection/readback controls. It also reports independent sharing choices and unchanged ordinary Receive details. The owner read the entire report, deciding output and control patch, and verified the review's commit; underlying raw logs, binary receipts and media were not independently inspected. These remain attributed external results, not new author-verified acceptance or a human approval. Literal device transport loss/reconnect remains unrun; the existing bounded Manual 3 evidence and readiness scope are unchanged. No new actionable defect was reported, and no additional re-review was requested for this automated comment.

Parent composition — October 30

Signed additive merge ca3dc95d45a3d93f5e1012ba18f55c67fd409bb6 incorporates live #1123/262a44747ffd8ef522178bfd3850fe78da15cce5; final tree 1716fc7b84775d8b91b81b1e31a8acd321ffc1a1. The original-owner correction, required-restore gates and acknowledged listener drain are preserved. Both independent affected source reviews are clean.

Final local validation passed 2,988 unit tests and one selected ProfileRecovery UI case, with zero failures, skips or retries, including all 28 ConnectionRecoveryService tests and four listener lifecycle cases. All 1,689 frozen inputs match. The build used Xcode 26.1/17B55, SDK 26.1/23B77, iOS 26.2/23C54. Complete build/test logs, all 2,989 nested summaries, eight diagnostic logs and 31 attachment payloads were inspected; the owner verified 9,137 evidence hashes. Five QoS observations and two AppIntents notices remain disclosed, with no timing-cause claim. SwiftFormat 0.64.0 reports 0/832; translations report zero errors and 15 warnings verified by exact key/language comparison with the actual parent. Five unit integration classes and the other UI classes were excluded or unselected, not passed.

All 13 live-base commits verify locally and on GitHub (verified=true, reason=valid). Ready status is retained (draft=false). Hosted native and formatting checks at that October 30 head passed; their complete audit is recorded immediately below. Existing J1/Manual acceptance retains its recorded revision, artifacts and limits; this composition is not a fresh authenticated journey or literal transport-reconnect pass. The earlier camera-alert failure, authorized retry and diagnostic evidence remain preserved.

Prior-head hosted result — October 30

Native run 41107597187, attempt 1, passed 2,988 units and 39 physical UI executions, with zero failures/retries and five explicit skips. This is 2,998 logical passes; the launch-performance case accounts for 29 additional successful executions. All 28 ConnectionRecoveryService tests pass. Synthetic checkout 2d0e31a7 has the exact published tree and actual 262a4474 parent. Hosted execution used Xcode 16.4/16F6, SDK 18.5 and iOS 26.2/23C54, distinct from local Xcode 26.1.

Complete 6,015-line job output, all 3,032 nested summaries, eight diagnostic logs (383,913 lines), 68 attachment payloads and all 52 unique screenshots were inspected. The original archive passed SHA256/CRC checks; the owner verified all 43 compact sealed evidence files. 286 structured linker warnings / 571 raw occurrences and 24 unattributed CoreGraphics NaN events remain disclosed. Dependency/workflow inputs and link producers match the actual parent; its prior raw audit records the same warning count, but exact parent structured-warning equality and runtime incidence are not claimed. Hosted formatting passed 0/832. Five fixture/hardware skips and four excluded integration classes are not passes. Existing authenticated J1/Manual evidence retains its original revision and limits; this native result adds no new remote journey, original-grant or SDK adoption claim. Earlier camera/navigation failures remain preserved.

Historical Xcode 27 repair and parent composition — October 29

Signed f38c33e2317552a5716dd02ab4c6d653ed029dae incorporates published #1123/8947f493a681f57cb76486d96bbc36da3f9999a6; tree 91ab0589b4a7ca1b3dd37ba172ecc8d6bcc58af6. Two weak node-release checks now use constant weak-capture closures, preserving their deallocation assertions while compiling under Swift 6.4. No warning demotion, timeout change or product behavior change is introduced by this test repair. Controlled compiler before/after and the affected independent reviews are clean; the other 32 child files and their edit sequences are preserved.

The final combined tree passed 2,912 unit tests and 39 physical UI executions on Xcode 27.1/27A9275, SDK 27.1, iOS 26.2/23C54, with warnings-as-errors intact. All four listener lifecycle tests pass. Four prerequisite tests were skipped; the expired fork-payment method and four workflow integration classes were explicitly excluded, not passed. All 1,675 frozen inputs match. Full logs, all 2,955 nested summaries, eight diagnostic logs and attachments were inspected; the owner rehashed all 14,067 sealed evidence rows. SwiftFormat 0.64.0 reports 0/829; translation validation has zero errors and 15 warnings verified against the actual parent. Two AppIntents notices and 22 stdout QoS traces versus 21 nested entries remain disclosed; no runtime-causality claim is made.

All 12 live-base commits verify locally and on GitHub (verified=true, reason=valid). Ready status remains draft=false. The original attempt of the historical-head hosted native run failed at UsdtWalletUITests.swift:288: the camera permission alert remained visible through the original five-second wait after a centered Allow tap. All 2,912 units and 38 physical UI executions passed, one UI execution failed and five prerequisite cases skipped. All four listener tests passed. Complete logs, 2,956 nested summaries, 77 attachment payloads and 3,869,953 simulator system-log events were inspected; the owner verified 13,563 sealed evidence rows. TCC completed the original request only during the following test. Why the first tap did not dismiss the prompt remains unresolved; no harmless-flake or source-fix claim is made. The test did not reach its later invalid-recipient/payment assertions. Hosted execution was Xcode16.4/16F6, SDK18.5, iOS26.2/23C54, separate from the local Xcode27.1 pass. Hosted formatting passed 0/829; nine migration jobs skipped. All 286 dependency linker warnings match verified prior warning metadata and actual-parent producers. The green native checks below belong to prior heads. Upstream #1120's separate launch-assertion failure remains preserved; its one bounded native retry passed without a product-source repair claim. Existing J1/Manual acceptance retains its recorded revision and scope below. Literal transport loss/reconnect and nonempty USDT import are not newly proved by this suite.

A separate follow-up correlated the original touch with SpringBoard's alert window and transport acknowledgement. The touch overlapped that same alert's Appearing → Appeared transition while SpringBoard logged ignoreInteractionEvents=1; no second permission presentation or independent app foreground cycle was observed. This supports a presentation/input-suppression race, while exact Allow-action consumption remains unproved. All 21 follow-up evidence rows are verified. On that new service-boundary evidence, exactly one bounded native-job retry was requested; attempt 2 completed successfully. The original five-second assertion, failure and complete diagnostic record remain unchanged. No further unchanged retry is authorized.

The single authorized attempt 2, job 123743388511, is now completely inspected: 2,912 units and 39 physical UI executions passed, five explicit skips, zero failures (2,922 logical passes). The original camera case passed its unchanged five-second alert-disappearance assertion in 1.201 seconds and executed the later invalid-recipient checks. All four listener tests passed. Checkout c549a66d has the exact published tree and actual 8947f493 parent. Complete logs, all 2,956 nested summaries, eight diagnostic files, 55 payloads and the checksum/CRC-verified archive were inspected; the owner verified 12,134 sealed rows. The 286 known linker messages, two AppIntents notices and 48 CoreGraphics NaN observations remain disclosed. No attempt-2 system archive was supplied, so no TCC/HID cause comparison is claimed. This passing repeatability check does not prove a source fix or erase attempt 1. Fixture skips and four excluded integration classes remain outside the pass count. Existing formatting and revision-scoped app acceptance remain unchanged.

Prior node listener repair — October 29

Signed a4f5ca6034f1025baa857a538d5e250c1529194a fixes the failed-start and Retry listener lifetime finding on actual parent #1123/d33547cb; tree 7c1411217be0e26324d475d246475a0d4bc9383e. One consumer captures one node and starts only after native startup succeeds. Stop drains an already acknowledged callback before stopping or replacing that node. Unexpected stop errors retain the node and resume its consumer. MainActor delivery is awaited; existing downstream payment/proof tasks remain unchanged.

The pinned UniFFI asynchronous event wait ignores Swift cancellation. Cancellable nonblocking polling avoids retaining that wait, with up to 250 ms idle delivery latency and four idle wakeups per second. Four controlled fake-native lifecycle tests pass; omitting the drain produces the expected failure. This is not a native heap measurement or a new authenticated journey.

Final combined validation passed 2,897 selected unit and 39 UI executions, zero failures/retries, with five explicit prerequisite skips. All 1,668 frozen inputs match. Complete 22,918-line compile/test output, all 2,941 nested summaries, exported diagnostics and attachments were inspected; 14,047 evidence hashes verified. Two AppIntents notices, 12 nested QoS entries/13 canonical backtraces and 48 CoreGraphics NaN messages remain disclosed. Actual-parent producer comparison found no introduced compiler/format/translation violation; differing runtime incidence has no asserted cause. SwiftFormat 0.64.0: 0/828, 99 exclusions; translation validation: zero errors, 15 actual-parent inherited warnings. Three dependency checkouts have unavailable historical Git alternates for cleanliness comparison; all six pinned HEADs match and executed products are separately hashed. Four integration classes remain excluded.

Both affected independent reviews are clean. All 11 live-base commits verify locally and on GitHub (verified=true, reason=valid). The immediate post-push REST response was stale; read-only reconciliation confirmed the published head without another push. Ready status is verified (draft=false). At that historical head, native tests and SwiftFormat now pass on synthetic 2b43d8b7, whose tree equals this head: 2,897 unit + 39 UI physical passes, five explicit skips, zero failures/retries; 2,907 logical passes after launch-variant reconciliation. All four listener tests pass. Formatting is 0/828; nine migration jobs are skipped, not passes. Complete logs, every nested summary, exported diagnostics and all archive/attachment hashes were inspected; the owner verified 9,158 sealed evidence rows. All 286 distinct linker warnings match the actual parent. One keyboard timeout during a passing unchanged route test, two AppIntents notices and 48 NaN emissions remain disclosed; their cause/introduction is unproved. No CI rerun or broader authenticated acceptance is inferred. Existing app-journey evidence below retains its exact tested revision and scope; these local suites do not replace it or claim a new authenticated restore, wallet wipe, nonempty USDT import or spending run.

Previous parent composition — October 29

Signed additive merge 1f8e07eb44005255207c67993d57403f04f22c63 incorporates #1123 at d33547cba681f8cf826a715de5f51a2ba89c28ef; final tree d764759c19b400f4bfbbeb5403c9b65202feebaa. The independent affected composition review is clean. Required restore markers, original-owner correction persistence, restricted routing, wipe drains and delayed-callback guards are retained.

Final selected native validation passed 2,893 unit and 39 UI executions, zero failures, with five fixture skips. All 1,666 frozen inputs and all 2,937 nested physical summaries reconcile; the selected recovery suites include 28 ConnectionRecoveryService and eight UsdtRecoveryBackup passes. SwiftFormat 0.64.0 passed 0/826 (99 exclusions); translations passed zero errors and 15 actual-parent inherited warnings. Full logs and nested diagnostics were inspected: three AppIntents notices, 18 QoS observations across nine logical tests and 72 CoreGraphics NaN notices in passing USDT UI cases remain disclosed without causal claims. Four integration classes and missing/disabled fixture cases are not passes.

The complete 10-commit live-base range is locally and GitHub verified (verified=true, reason=valid). Ready status is retained (draft=false). Hosted native/UI 40889840827 and SwiftFormat 40889840829 passed on the exact synthetic tree: 2,903 unique passes, zero failures/retries and five skips (2,937 physical summaries); formatting 0/826. Complete logs, nested summaries, diagnostics and checksum/CRC-verified artifacts were inspected; all 12,150 sealed evidence rows were verified. The 286 distinct linker warnings match the actual parent. One keyboard timeout remains in a passing route test, with unproven cause. Nine migration jobs were skipped, not passed. Piotr's subsequent node-event listener lifetime finding is addressed by the repair above; these green checks apply only to the earlier head. Existing app-journey evidence below retains its tested revision and scope; this composition does not claim a new authenticated restoration, actual wallet wipe, nonempty USDT import or spending journey.

Twin: synonymdev/bitkit-android#1685
Dependency: #1123

A wallet whose required restoration failed can correct Electrum/RGS settings and retry through guarded startup. Corrections are acknowledged locally before settings change, survive restart and older SETTINGS restoration, and are cleared only after required imports and node startup complete. The editor keeps ordinary wallet actions gated. Exact typed “node not running” during stop permits rebuilding the failed node; other stop errors propagate. Late callbacks and stale editors cannot complete another lifecycle. Offline creation waits for connectivity without inventing a restore failure, and a later activity-sync error cannot reopen an already acknowledged startup.

Out of Scope

  • LOW idle-poll performance-log volume is tracked for 3.1.0 in #1181, from the review. Queue logging and overload behavior remain unchanged in this correctness follow-up; reviewer measurements are attributed, not a fresh author benchmark.

  • LOW initialization progress reset/animation latency is tracked for 3.2.0 in #1158, from the review. The current strict lifecycle/restore completion guard is retained; no new author latency benchmark is claimed.

Earlier composition and validation

Signed additive merge be804c7c incorporates #1123 at 566b2ff8. The 29-file connection-recovery patch retains all 1,572 feature-line operations; the affected integration review is clean. Final composed-tree validation passed 2,858 unit and 39 UI executions, with three disabled Trezor skips and zero failures/retries. SwiftFormat: 0/824, 93 excluded; translations: zero errors and 15 warnings verified against the actual parent. Complete logs, all 2,900 nested execution summaries and 11,869 evidence hashes were inspected. Two AppIntents notices, eleven test-attached QoS observations and twelve raw process QoS notices are retained as distinct diagnostic counts; no timing-cause or warning-free runtime claim is made. All nine current-base commits are locally and GitHub verified. At that historical head, native tests and formatter now pass on the exact 98f6e6e9 tree: 2,858 unit + 39 UI passes, five explicit fixture/hardware skips, zero failures/retries, format 0/824. Complete logs, all 2,902 nested summaries and every original archive CRC/hash were inspected; the owner verified 8,945 sealed evidence files. The recurring 571 duplicate-symbol occurrences/286 symbols and one keyboard queue timeout during a passing 4.382-second lifecycle case remain disclosed; no causal repair or fresh exact-parent runtime baseline is claimed. Existing J1/Manual evidence below retains its recorded binary/fixture scope; this composition adds no fresh authenticated journey claim. The PR remains ready for review.

QA

Setup

Use an isolated empty iPhone simulator and the pinned dependencies at be804c7cf44352b7cd48933282437618c75e0957; do not reset an existing wallet. Build the simulator app with xcodebuild -project Bitkit.xcodeproj -scheme Bitkit -configuration Debug -destination 'platform=iOS Simulator,id=<OWNED_SIMULATOR_UUID>' E2E_BACKEND=network E2E_NETWORK=regtest 'SWIFT_ACTIVE_COMPILATION_CONDITIONS=$(inherited) E2E_BUILD' build. J1 requires an owned disposable LNURL/RS256 VSS + PostgreSQL backend, an acknowledged complete backup and older SETTINGS record, a failing Electrum endpoint and a reachable regtest replacement; these are separate controlled resources, not assumed available on public staging. The author's replacement was tcp://127.0.0.1:18682. Follow journeys/backup-restore/connection-recovery.xml at that commit.

For Manual 2, launch the built app with -E2E_CONNECTION_RECOVERY_FAULTS. Select one of the twelve allowlisted boundaries from Bitkit/Utilities/Keychain.swift in its own container's Library/Application Support/connection-recovery-test-fault; remove only that selector before Retry. The selector injects errors at real boundaries and does not replace wallet/backup state. Manual 1's wipe slice remains attributed external evidence below; it does not authorize resetting a wallet. Manual 3 used a temporary offline monitor in two preserved test builds; it is absent from the published app, and literal simulator transport reconnect remains unclaimed.

  • J1 — installed simulator restoration through real native LNURL authentication, RS256 VSS/PostgreSQL and an owned regtest Electrum fixture. Saved corrections survive rejected required reads, three background/foreground cycles, explicit Retry, Back, cold restart and an older SETTINGS backup. Required acknowledgements precede main access. Independent RGS failure preserves the last correction. A genuine profile created through normal Homegate/Pubky UI subsequently fails optional loading, retains wallet access and its retry controls, and reloads after explicit retry.
  • Manual 1 — held import/Back slice passed in the author's installed app. The wipe/drain/replacement slice is credited to the existing external execution at 9481b503 with its posted hooks: video, hook patch and the timestamped trace in review 5674650561 were inspected. The trace proves wipe requested while the probe was held, drain after release, completion and replacement defaults with no retained correction; deny-read was off. Relevant paths remain compatible with the final tree. This is historical external evidence, not a fresh author-run wipe or permission to reset wallets.
  • Manual 2 — installed correction read/write/delete, preference acknowledgement/flush and all six WALLET/USDT marker read/write/delete fault boundaries fail closed. Explicit Retry after removing only the selector completes the original remaining work. No marker, wallet or backup was directly replaced.
  • Manual 3 — normal New Wallet onboarding under the same controlled offline monitor: the prior build shows Spending balance error and Connection Settings before any native startup; the corrected build remains in initialization without ordinary wallet access. Two separate empty iPhone16/iOS26.2 simulators and exact installed-binary hashes were used. The temporary DEBUG connectivity argument was removed byte-for-byte before final validation and is not published. This proves the application branch, not literal transport loss/reconnect; existing reconnect and required-restore tests remain separate evidence.

The ordinary fixture has a genuine empty independent USDT record listing. This is not a nonempty USDT import, full-chain synchronization or spending claim. Source cached height and stale RGS/network observations are retained in the evidence. The optional-profile selector produced five failed reads across startup, Profile entry and explicit Retry; the same genuine identity returned afterward. The final WALLET-delete fault is proven by runtime and blocked completion; its captured editor screenshot alone does not display that last error.

The DEBUG fault controls require simulator + E2E + regtest + explicit launch flag, exclude XCTest, and accept only twelve named boundaries in a fixed local file. They throw at the real local/profile boundary and never fabricate authenticated state, backup contents or success. Native authenticated network journeys use owned disposable resources; production/deployed-server equivalence is not claimed.

Validation

753d6d1f3dfa1b6ba2f39f608ab657251972897a is based on live #1123 / e12df99e2e91944308314d82bc8d71e9aaa24f66. The offline correction removes only the premature failure assignment; it does not clear an existing failure, complete a pending restore, erase a correction or change reconnect admission. The two creation entry points persist local entropy before startup, and the controlled installed-app before/after reproduces the reported branch. Earlier activity-sync regression tests remain unchanged.

Final local validation passed 3,018 unit and 39 UI executions, with three explicitly disabled Trezor UI skips and no failures or retries. All 1,689 frozen inputs remained identical. Full SwiftFormat 0.64.0 lint passed (0/855). The sole complete diagnostic audit inspected 29,461 raw log lines, every nested test summary and exported issue: no Swift source warnings/errors; two AppIntents notices and 13 runtime priority-inversion observations remain. Their complete stacks match the prior local result; an actual-base runtime baseline was not run, so this is not a warning-free claim. One fresh combined affected-delta review is clean under the current risk-based review policy; prior whole-PR reviews remain applicable. All eight live-base commits verify locally and on GitHub.

The final documentation-only commit condenses the changelog to one sentence; all validated product, test and configuration inputs are byte-identical. At that historical head, hosted native/UI run 40675194474 passed 3,018 unit and 39 UI executions, with one native-fixture and four Trezor/fork UI skips, zero failures and no retries. Formatter 40675194463 passed 0/855. The logged synthetic merge 58228f6a has the exact head tree 2e1058ec. Complete 5,991-line logs, all 3,062 nested summaries, 572 mirrored issue objects, 1,180 activity messages and eight diagnostic logs were inspected; artifact digest and all 6,267 ZIP member CRC/SHA values were verified. The 286 dependency linker warnings match prior 911f24c messages and unchanged actual-parent pins/link configuration; an exact-parent build baseline is unavailable. No additional Swift or runtime checker diagnostics were found in this hosted Xcode16.4 export; local runtime observations remain separately preserved. Six unselected integration groups, five skipped tests and nine skipped migration jobs are not passes. The superseded e7d native run was automatically cancelled during compilation after the changelog-only push; it receives no test-pass credit.

Applicable installed J1 and Manual 1–2 retain their exact recorded earlier build provenance and scoped source-composition verification. Their evidence is preserved in work/ios1139-pending-e-evidence-review.md, work/ios1139-local-storage-app-f-evidence-review.md, work/ios1139-required-marker-app-g-evidence-review.md, work/ios1139-optional-profile-app-h-evidence-review.md and the final validation directories. The new offline regression is in work/validation/ios1139-offline-creation/. Prior failed commands, tests, initial attempts, runtime observations and narrower external evidence remain retained; no new wipe or literal transport-toggle pass is inferred.

Readiness

This PR remains ready under the explicit readiness authorization. Applicable J1 and Manual 1–2 acceptance and the bounded Manual 3 regression are complete at the scope above. Full-wallet redesign and native contact CAS are separate work. Awaiting human approval does not require draft status; all review states and outstanding discussions remain visible. The October 30 local and hosted native/UI and formatter checks were completely inspected at ca3dc95d, with skips and runtime observations disclosed above. The earlier 13b91199 hosted camera-alert failure remains causally unexplained; current f8bd749b native and formatter checks pass as detailed above. Earlier hosted formatting and the single authorized native retry remain attributed to their historical heads, with original failures and runtime limits retained.

Models used

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

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

@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: I found no actionable defects.

QA for 9481b50: iPhone 17 Pro simulator, iOS 26.5, unfunded regtest wallet, authenticated staging VSS and staging Electrum/RGS. Device cases used the Debug fault controls below.

Test 1: J1, required restore connection recovery ✅ Independent corrections survived failed probing, retry and restart; required recovery continued to gate wallet access.

Electrum and RGS saved independently without starting restoration. A failed RGS probe preserved the acknowledged revision. The complete pass verified selective replay over the old SETTINGS backup, including restoration of unrelated preferences, in the device log. Separate WALLET and USDT failures kept access restricted. After both cleared, Home opened while optional profile initialization remained held.

J1-focused-2x.2x.mp4

Focused repeat at 2×; endpoint values, revision retention and restore-marker transitions were also checked in the app log.

Test 2: manual 1, cancellation and wipe ✅ An admitted retry completed after Back; wipe drained a held probe before a replacement wallet became available.

The full pass held a required import, left the editor and released it: restoration and node startup completed. The subsequent held RGS probe was canceled by leaving the editor, then a wipe waited until release. A newly generated wallet reached Home with default endpoints and no stale correction. The focused repeat below covers probe draining and replacement; the retry-after-Back result was checked in the original recording and app log.

manual-1-focused-2x.2x.mp4

Probe/wipe repeat at 2×; drain order and final settings were checked in the app log.

Test 3: manual 2, local persistence failures ✅ Save and Retry reported failures while preserving required recovery state.

Denied correction-record writes left preferences unchanged. Denied preference promotion retained the durable correction. Retry remained gated. An unreadable correction record showed an error and disabled saving; direct state inspection confirmed the same correction and required WALLET marker remained stored.

manual-2-2x.2x.mp4

Storage-fault cases at 2×; durable-record and marker retention were checked in the app log.

The unmodified head built successfully, and ConnectionRecoveryServiceTests and UsdtRecoveryBackupTests passed with no failures or skips. Changed Swift files passed SwiftFormat 0.64.0; translation validation and git diff --check passed.

Coverage: full ff999a50..9481b50, all changed files and reached restore/startup/wipe callers. Device coverage used an unfunded wallet with no stored USDT snapshot; nonempty USDT imports, payment replay and RN migration were not driven. Optional profile unavailability used a held initialization, not a remote outage or terminal identity failure.

Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes matches the restricted editor, durable replay and drain contract. iOS restores required backup state before node startup; Android can need a running node during restoration.

Debug fault controls and deciding log excerpts

These unpushed hooks held or rejected import/probe/storage boundaries and logged state. Backup encryption, authentication, restore operations and node startup used the app's real services.

ACK complete WALLET bytes=934 SETTINGS bytes=306 result=completed
STATE node=initializing walletPending=true usdtPending=false correction=9CC80216-DA07-4EBD-9328-734B325E897F electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot/ hideBalance=true busy=false
STATE node=stopped walletPending=true usdtPending=true correction=9CC80216-DA07-4EBD-9328-734B325E897F electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot/ hideBalance=true busy=false
HOLD hold-profile
STATE node=running walletPending=false usdtPending=false correction=nil electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot/ hideBalance=true busy=false
diff --git a/Bitkit/AppScene.swift b/Bitkit/AppScene.swift
index 9b06e9fd..8593ed45 100644
--- a/Bitkit/AppScene.swift
+++ b/Bitkit/AppScene.swift
@@ -512,6 +512,9 @@ struct AppScene: View {
                 config in AppUpdateSheet(config: config)
             }
             .task(priority: .userInitiated, setupTask)
+            #if DEBUG // test hook
+            .task { await qaCommands() } // test hook
+            #endif // test hook
             .task(id: [scenePhase == .active, wallet.walletExists == true, isWalletBackupRestoreRunning, network.isConnected]) {
                 guard scenePhase == .active, wallet.walletExists == true,
                       !isWalletBackupRestoreRunning, !BackupService.shared.hasPendingWalletRestore()
@@ -956,6 +959,55 @@ struct AppScene: View {
 
     // MARK: - Event Handlers
 
+    #if DEBUG // test hook
+    @MainActor private func qaCommands() async { // test hook
+        for _ in 0..<14400 { // test hook
+            guard !Task.isCancelled else { return } // test hook
+            let file = RecoveryTestHook.directory.appendingPathComponent("qa-command") // test hook
+            if let text = try? String(contentsOf: file, encoding: .utf8) { // test hook
+                try? FileManager.default.removeItem(at: file) // test hook
+                Task { @MainActor in // test hook
+                    do { // test hook
+                        let command = text.trimmingCharacters(in: .whitespacesAndNewlines) // test hook
+                        RecoveryTestHook.log("COMMAND " + command) // test hook
+                        switch command { // test hook
+                        case "backup": // test hook
+                            await BackupService.shared.stopObservingBackups() // test hook
+                            ElectrumConfigService().saveServerConfig(ElectrumServer(host: "127.0.0.1", port: 1, protocolType: .tcp)) // test hook
+                            UserDefaults.standard.set(true, forKey: "hideBalance") // test hook
+                            try await BackupService.shared.persistWalletBackup() // test hook
+                            let result = await BackupService.shared.triggerBackup(category: .settings) // test hook
+                            guard let walletData = try await VssBackupClient.shared.getObject(key: "WALLET")?.value, // test hook
+                                  let settings = try await VssBackupClient.shared.getObject(key: "SETTINGS")?.value else { throw AppError(message: "Missing fixture backup", debugMessage: nil) } // test hook
+                            try walletData.write(to: RecoveryTestHook.directory.appendingPathComponent("qa-wallet-backup")) // test hook
+                            try settings.write(to: RecoveryTestHook.directory.appendingPathComponent("qa-settings-backup")) // test hook
+                            RecoveryTestHook.log("ACK complete WALLET bytes=\(walletData.count) SETTINGS bytes=\(settings.count) result=\(result)") // test hook
+                        case "wipe": // test hook
+                            if let seed = try Keychain.loadString(key: .bip39Mnemonic(index: 0)) { try seed.write(to: RecoveryTestHook.directory.appendingPathComponent("qa-seed"), atomically: true, encoding: .utf8) } // test hook
+                            try await AppReset.wipe(app: app, wallet: wallet, session: session) // test hook
+                            RecoveryTestHook.log("WIPE completed") // test hook
+                        case "restore": // test hook
+                            let seed = try String(contentsOf: RecoveryTestHook.directory.appendingPathComponent("qa-seed"), encoding: .utf8) // test hook
+                            wallet.nodeLifecycleState = .initializing // test hook
+                            wallet.isRestoringWallet = true // test hook
+                            BackupService.shared.setRestoring(true) // test hook
+                            try StartupHandler.restoreWallet(mnemonic: seed, bip39Passphrase: nil) // test hook
+                            try InstallationMarker.create() // test hook
+                            try wallet.setWalletExistsState() // test hook
+                        case "retry": await retryWalletStart() // test hook
+                        case "snapshot": break // test hook
+                        default: break // test hook
+                        } // test hook
+                        RecoveryTestHook.snapshot(wallet) // test hook
+                        RecoveryTestHook.log("DONE " + command) // test hook
+                    } catch { RecoveryTestHook.log("ERROR \(error)") } // test hook
+                } // test hook
+            } // test hook
+            try? await Task.sleep(for: .milliseconds(500)) // test hook
+        } // test hook
+    } // test hook
+    #endif // test hook
+
     private func handleCurrencyStaleData(_: Bool) {
         if currency.hasStaleData {
             app.toast(type: .error, title: "Rates currently unavailable", description: "An error has occurred. Please try again later.")
diff --git a/Bitkit/Managers/PubkyProfileManager.swift b/Bitkit/Managers/PubkyProfileManager.swift
index d29dd6b1..34270bae 100644
--- a/Bitkit/Managers/PubkyProfileManager.swift
+++ b/Bitkit/Managers/PubkyProfileManager.swift
@@ -262,6 +262,9 @@ class PubkyProfileManager: ObservableObject {
                 isRestoringSession = false
             }
             guard revision == Self.sessionRevision, Self.sessionMutationCount == 0, !Self.isProfileDeletionInFlight else { return }
+            #if DEBUG // test hook
+            await RecoveryTestHook.hold("hold-profile") // test hook
+            #endif // test hook
             await initializeSessionState(mode: mode, initializeSession: initializeSession)
             if case .automaticRecovery = mode, revision == Self.sessionRevision {
                 completedRecoveryVersion += 1
diff --git a/Bitkit/Services/BackupService.swift b/Bitkit/Services/BackupService.swift
index add0ff5f..fd4f838d 100644
--- a/Bitkit/Services/BackupService.swift
+++ b/Bitkit/Services/BackupService.swift
@@ -531,6 +531,10 @@ class BackupService {
 
     private func restoreUsdtRecovery(legacyWallet: Data?) async throws {
         let legacy = try legacyWallet.map { try JSONDecoder().decode(WalletBackupV1.self, from: $0) }
+        #if DEBUG // test hook
+        await RecoveryTestHook.hold("hold-usdt") // test hook
+        try RecoveryTestHook.reject("reject-usdt") // test hook
+        #endif // test hook
         try await usdtRecoveryBackup.restore(
             legacyWallet: legacy?.usdtWallet,
             legacyPaymentState: legacy?.paykitPaymentState?.usdt,
@@ -1243,6 +1247,12 @@ class BackupService {
             }
 
             if let dataBytes {
+                #if DEBUG // test hook
+                if category == .wallet { // test hook
+                    await RecoveryTestHook.hold("hold-wallet") // test hook
+                    try RecoveryTestHook.reject("reject-wallet") // test hook
+                } // test hook
+                #endif // test hook
                 try await restoreAction(dataBytes)
                 Logger.info("Restore success for: '\(category.rawValue)'", context: "BackupService")
             } else {
diff --git a/Bitkit/Services/ConnectionRecoveryService.swift b/Bitkit/Services/ConnectionRecoveryService.swift
index b6c8961a..5a76f3c1 100644
--- a/Bitkit/Services/ConnectionRecoveryService.swift
+++ b/Bitkit/Services/ConnectionRecoveryService.swift
@@ -58,8 +58,18 @@ final class ConnectionRecoveryService: ObservableObject {
             return Owner(walletIndex: 0, network: Env.networkName,
                          binding: PaykitPaymentStateBackup.ActiveOnchainAttempt.Wallet.binding(storeId: namespace))
         },
-        load: { try Keychain.load(key: .connectionRecovery) },
-        store: { try Keychain.upsert(key: .connectionRecovery, data: $0) },
+        load: {
+            #if DEBUG // test hook
+            try RecoveryTestHook.reject("deny-read") // test hook
+            #endif // test hook
+            return try Keychain.load(key: .connectionRecovery)
+        },
+        store: {
+            #if DEBUG // test hook
+            try RecoveryTestHook.reject("deny-store") // test hook
+            #endif // test hook
+            try Keychain.upsert(key: .connectionRecovery, data: $0)
+        },
         clear: { try Keychain.delete(key: .connectionRecovery) }
     )
 
@@ -307,6 +317,9 @@ final class ConnectionRecoveryService: ObservableObject {
 
     private func apply(_ record: Record) throws {
         _ = try currentAccess()
+        #if DEBUG // test hook
+        try RecoveryTestHook.reject("deny-preferences") // test hook
+        #endif // test hook
         if let electrum = record.electrum {
             let data = try JSONEncoder().encode(electrum)
             defaults.set(data, forKey: "electrumServer")
diff --git a/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift b/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift
index f826e851..5112dbf9 100644
--- a/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift
+++ b/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift
@@ -91,6 +91,9 @@ extension SettingsViewModel {
     /// and returns a 2xx for a valid endpoint, so a HEAD request confirms reachability without
     /// downloading the body. Mirrors the Android RGS connect check.
     nonisolated func isRgsEndpointReachable(_ url: String) async -> Bool {
+        #if DEBUG // test hook
+        await RecoveryTestHook.hold("hold-probe") // test hook
+        #endif // test hook
         let base = url.hasSuffix("/") ? String(url.dropLast()) : url
         guard let testUrl = URL(string: "\(base)/0") else {
             return false
diff --git a/Bitkit/Utilities/RecoveryTestHook.swift b/Bitkit/Utilities/RecoveryTestHook.swift
new file mode 100644
index 00000000..c6c8a173
--- /dev/null
+++ b/Bitkit/Utilities/RecoveryTestHook.swift
@@ -0,0 +1,33 @@
+#if DEBUG // test hook
+import Foundation // test hook
+ // test hook
+enum RecoveryTestHook { // test hook
+    static var directory: URL { FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0] } // test hook
+    static func flag(_ name: String) -> Bool { FileManager.default.fileExists(atPath: directory.appendingPathComponent("qa-" + name).path) } // test hook
+    static func log(_ value: String) { // test hook
+        let line = "\(Date().timeIntervalSince1970) \(value)\n" // test hook
+        let target = directory.appendingPathComponent("qa-events.log") // test hook
+        if !FileManager.default.fileExists(atPath: target.path) { _ = FileManager.default.createFile(atPath: target.path, contents: nil) } // test hook
+        if let handle = try? FileHandle(forWritingTo: target) { defer { try? handle.close() }; _ = try? handle.seekToEnd(); try? handle.write(contentsOf: Data(line.utf8)) } // test hook
+        Logger.info(value, context: "RecoveryTestHook") // test hook
+    } // test hook
+    static func reject(_ name: String) throws { // test hook
+        if flag(name) { log("REJECT " + name); throw AppError(message: "Controlled " + name + " failure", debugMessage: nil) } // test hook
+    } // test hook
+    static func hold(_ name: String) async { // test hook
+        guard flag(name) else { return } // test hook
+        log("HOLD " + name) // test hook
+        for _ in 0..<3600 { // test hook
+            if !flag(name) { log("RELEASE " + name); return } // test hook
+            await Task.detached { try? await Task.sleep(for: .milliseconds(500)) }.value // test hook
+        } // test hook
+        log("TIMEOUT " + name) // test hook
+    } // test hook
+    @MainActor static func snapshot(_ wallet: WalletViewModel) { // test hook
+        let correction = (try? Keychain.load(key: .connectionRecovery)).flatMap { try? JSONDecoder().decode(ConnectionRecoveryService.Record.self, from: $0) } // test hook
+        let pendingWallet = (try? Keychain.load(key: .paykitPendingBackupRestore)) != nil // test hook
+        let pendingUsdt = (try? Keychain.load(key: .usdtPendingBackupRestore)) != nil // test hook
+        log("STATE node=\(wallet.nodeLifecycleState) walletPending=\(pendingWallet) usdtPending=\(pendingUsdt) correction=\(correction?.revision.uuidString ?? "nil") electrum=\(ElectrumConfigService().getCurrentServer().fullUrl) rgs=\(RgsConfigService().getCurrentServerUrl()) hideBalance=\(UserDefaults.standard.bool(forKey: "hideBalance")) busy=\(ConnectionRecoveryService.shared.isBusy)") // test hook
+    } // test hook
+} // test hook
+#endif // test hook

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


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

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utAck

Copy link
Copy Markdown
Contributor Author

The Manual 1 video is accessible and shows the replacement wallet reaching Home. To close the remaining acceptance receipt, please attach a redacted excerpt from the existing Manual 1 qa-events.log / app log showing HOLD hold-probe → COMMAND wipe while held → RELEASE hold-probe → WIPE completed, plus the new wallet’s final Electrum/RGS and absent correction readback. Please include timestamps, confirm read-denial was off for that final snapshot, and identify the recorded build as 9481b50 plus the posted hooks. This requests existing evidence only: no new wipe or rerun, and no seed, session or backup contents. I verified the current 93181d5 wipe/drain paths remain compatible with that recorded scope; the other current-build acceptance results are in the updated PR description.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified — current-head independent build and device coverage are incomplete.

The PR moved to f3222bf after this run; these results are for 93181d5.

Findings: I found no actionable defect in the recovery delta and sampled merged paths.

QA for 93181d5: The build did not complete before the head moved to f3222bf. The author reports authenticated J1 and local persistence and marker fault cases in the PR description; I have not independently reproduced those cases on this head. The held import and Back slice is also reported there. The replacement-wallet slice below was recorded at the earlier head.

Test 1: J1, required restore connection recovery ⏭️ Not independently rerun before the head moved.

The author reports the authenticated restore and optional-profile cases at this head.

Test 2: Manual 1, cancellation and replacement ⏭️ Editor and replacement sequence was not independently rerun before the head moved.

The earlier review recorded the held probe, drain, and replacement on 9481b50 with the hooks published in that review. The deny-read control was turned off before the sequence and remained off for the final snapshot. The timestamped redacted app log excerpt below answers the evidence request; times are CET on 27 October 2026.

13:10:21.192 HOLD hold-probe
13:10:25.413 COMMAND wipe
13:10:52.223 RELEASE hold-probe
13:10:59.560 WIPE completed
13:11:25.380 STATE node=running walletPending=false usdtPending=false correction=nil electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot hideBalance=false busy=false
Test 3: Manual 2, local persistence and marker failures ⏭️ Not independently rerun before the head moved.

The author reports correction, preference and required-marker fault cases at this head.

Coverage: delta 9481b50..93181d5, focused on recovery and sampled merged payment and transaction paths. Merged payment and layout changes were not driven in this review.

Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes matches the restricted editor and durable correction contract; startup ordering differs by platform.

Reviewer model: Unknown
Reasoning effort: Not exposed

Model and effort metadata were not available in the review session records.


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

@ben-kaufman
ben-kaufman marked this pull request as ready for review October 27, 2026 22:13
@ben-kaufman
ben-kaufman requested a review from ovitrif October 27, 2026 22:14

@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). Adds a guarded connection-settings editor to the failed-restore screen, a keychain-backed correction record replayed across restore and startup, and lifecycle/wipe admission around wallet start. Reviewed f3222bf, full tier, statically: iOS does not build here, so Run Tests is the gate (pending at review time; SwiftFormat green).

What I checked, and 5 candidates I ruled out

Read in full: ConnectionRecoveryService.swift, WalletConnectionRecoveryView.swift, the AppScene.swift startup/restore/retry paths (handleWalletExistsChange through restoreVssBackup), InitializingWalletView.swift, the BackupService restore hunks, LightningService.stop, AppReset, the Keychain fault hooks.
Call sites traced: startWallet (scene setup, NodeRestarter foreground and network-restored paths), retryWalletStart, restoreWalletBackupAndStart, CoreService.syncLdkNodePayments, WalletViewModel.start / stopLightningNode / wipe, vssDeriveStoreId.
CI: SwiftFormat green, Run Tests pending.

Ruled out

  • Nested perform deadlocking as busy: restoreAdmittedWalletBackupAndStart calls startWallet inside an admitted operation, but the @TaskLocal access is inherited by the unstructured Task in perform, so the nested call takes the access.service === self branch, not AdmissionError.busy.
  • Foreground restart racing a retry: NodeRestarter.restart goes through startWallet, which returns on .busy while a retry holds admission.
  • Correction surviving a wipe: wipeEntireKeychain deletes connection_recovery with the rest, and beginWipe rotates the generation and drains the active task first, so a stale editor or late callback cannot write.
  • Older SETTINGS backup overwriting a saved correction: mergeRestoredSettings overlays the record before restoreSettingsDictionary, and replay re-applies it afterwards.
  • Owner binding: derived from the VSS store id of the wallet-0 mnemonic and passphrase, so a correction saved for one seed throws instead of applying to another.

Merge confidence: 3/5, one P1 on the post-start activity sync path.

Reviewer model: Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: Not exposed (the run metadata available to this reviewer does not carry it)

Comment thread Bitkit/AppScene.swift Resolved

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

Re-review of 911f24c after f3222bf. 1 earlier finding: 1 fixed. 0 new findings. Delta is 3 files: AppScene.swift, ConnectionRecoveryService.swift, ConnectionRecoveryServiceTests.swift. Reviewed statically, because iOS does not build here, so Run Tests is the gate. It was pending at review time. SwiftFormat is green.

What I checked

Earlier P1 (an activity sync error after a successful start reopens the restore-failed screen): fixed. The catch in startAdmittedWallet now sets didWalletBackupRestoreFail only when canReportStartupFailure() holds, which is canReportOperationFailure() && readyGeneration != generation. complete(...) sets readyGeneration = generation right before onReady(), so a throw from syncLdkNodePayments after that point logs the error and plays the haptic but leaves the flag false. That matches the base behaviour.

Paths that must still fail closed: replay, stopLightningNode, wallet.start, checkCurrentOperation, and a complete that returns early (pending restore, node not running, revision mismatch) or throws on clear all happen before readyGeneration is set, so they still report. perform resets readyGeneration to nil on admission and beginWipe rotates the generation, so a new start or a wipe cannot inherit an earlier success (testAnotherStartDoesNotInheritEarlierSuccessfulRecovery). A nested perform takes the already-admitted branch and does not reset the flag mid-operation.

canFinishInitialization: still keyed on readyGeneration == lifecycle && !isBusy, so after a sync error in an acknowledged start, initialization can finish once the operation releases. testActivitySyncFailureAfterAcknowledgedStartDoesNotReopenRecovery asserts this.

Test shape: the three tests rebuild the catch predicate inside perform rather than calling AppScene.startAdmittedWallet. That is enough here, because the AppScene change is the single predicate the tests exercise.

Earlier cut candidates: none were cut for the cap last time. The 5 ruled out in the first review still hold, since the delta does not touch admission, wipe, settings merge or owner binding.

Merge confidence: 4/5. No open findings, but Run Tests had not finished on this head when I reviewed it.

Reviewer model: Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: Not exposed (the run metadata available to this reviewer does not carry it)

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified — the profile-failure and marker-fault cases remain incomplete.

Findings: I found no actionable defects in the reviewed delta.

QA for 911f24c: iPhone 17 Pro simulator, iOS 26.5, disposable regtest wallet. The restore checks used local fault and hold controls against the app's actual backup and startup services.

Test 1: required restore connection recovery ✅ Saved corrections survived required restore failures and restart, and Retry opened Home after the required markers cleared.

The recording shows the editor and retry sequence. App state logs also confirmed the retained correction, separate WALLET and USDT pending markers, and their clearance before Home. Profile initialization was held during the final startup.

J1-repeat-2x.2x.mp4

Test 2: cancellation and wallet replacement ✅ An admitted import completed after Back, and wipe waited for a held probe before a replacement wallet opened.

App state logs confirmed the probe was held when wipe was requested, wipe completed after release, and the replacement had default endpoints and no correction or required markers.

M1-2x.2x.mp4

M1-probe-2x.2x.mp4

Test 3: correction persistence failures ✅ Rejected writes and preference promotion kept restoration gated.

The app reported both failures; state logs confirmed that the failed write left the wallet marker and the failed preference promotion retained the correction. Retry with an unreadable correction also remained gated.

M2-focus-2x.2x.mp4

Test 4: optional profile failure and retry ⏭️ A real profile load failure and retry were not driven.

Home opened while profile initialization was held; the disposable wallet had no created profile. The PR description reports a separate genuine-profile failure and retry.

Test 5: WALLET and USDT marker read, write and delete failures ⏭️ The six-boundary matrix remains unverified in this run.

The marker-write attempt was restarted before its injected failure could be confirmed in the app log; the other five boundaries were not driven. The PR description reports separate installed-app coverage of these faults.

The exact head built successfully. The accelerated recordings retain all changed frames; the first two recordings contain visible stutters. The prior review covers an earlier installed-app recovery run. The hosted tests passed on the current head, including the new service-boundary startup regression; an installed-app corrupt-payment-row reproduction was not driven.

Coverage: delta 93181d5..911f24c, including the merged backup lifecycle and USDT changes. The merged USDT balance and receive changes were not driven in this review.

Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes matches the restricted editor and durable correction contract; startup ordering differs by platform.


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

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code scan: no HIGH or MEDIUM. One LOW inline: the initializing loader restarts on every wallet create, adding about 1.3 s.

The two leads from synonymdev/bitkit-android#1685 don't apply here:

  • The node isn't rebuilt on every foreground or on a backup-only Try Again. blocksWalletStart keeps the node down while a marker exists, stopping a nil node does nothing, and the record clears on the first acknowledged start. iOS has no retry counter.
  • The connection editor doesn't survive a Forgot-PIN wipe: beginWipe rotates the generation.

The two new strings are in all 15 locales.

Device gate (partial): not run yet. Pending: backup-restore/connection-recovery.xml. The Electrum, VSS and old-SETTINGS fault steps are manual, since the Capabilities table doesn't list them.

Comment thread Bitkit/AppScene.swift Resolved

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Full review of the complete bitkit-ios#1139 diff against merge base e12df99, at 911f24cd.

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

An offline skipped start is now classified as a failed restore whenever initialization is already showing, including first-time wallet creation. Post-start activity sync no longer reopens that screen: complete records the ready generation before syncLdkNodePayments, and the catch sets the failure flag only while canReportStartupFailure() is still true. The initialization-progress reset is the same mechanism already tracked in #1158; it stays deferred for 3.2.0.

Compared with bitkit-android#1685 at 2986fde for this flow only: both keep a restricted editor, a durable correction, and wallet actions gated until required restore finishes. iOS restores required backup state before starting a rebuilt node; Android can need a running node during restoration. Android’s offline start records a pending start and does not mark restore failed. bitkit-ios#1123 at e12df99 was checked only for the pending-restore gate this PR calls, not as a full review.

Validation: source and ConnectionRecoveryServiceTests were inspected, not executed. On this head, Run Tests and SwiftFormat passed. Device testing: not performed in this review.

Suggested additional test cases

  • Platform: iOS simulator.
  • Setup: airplane mode after the app has observed no network; no existing wallet.
  • Action: create a new wallet from New Wallet, and again with a passphrase.
  • Expected: initialization stays in progress, or shows an offline state, and does not show “Spending balance error” or Connection Settings. After reconnecting, startup continues without a connection correction left behind.

Findings

  • [MEDIUM] Offline wallet creation is shown as a failed restore — inline at Bitkit/AppScene.swift:1061.

Reviewer model: grok-4.7
Reasoning effort: high

Comment thread Bitkit/AppScene.swift Resolved

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-reviewed to 753d6d1. No new findings.

e7d5595 only removes the didWalletBackupRestoreFail = true lines in the offline guard (AppScene.swift:1058-1067), which restores master's behaviour:

  • Creating a wallet offline stays on the loader.
  • Reconnecting resumes through the existing nodeRestarter.restart on .initializing.
  • The seed is generated once, and there is no Back path while creation is deferred.

That fixes piotr-iohk's MEDIUM. My loader LOW at :942 is deferred to #1158, as agreed on its thread. The new offline-create action in connection-recovery.xml isn't on the Android twin yet (synonymdev/bitkit-android#1685).

Device gate (partial): 753d6d1, not run yet. Pending: connection-recovery.xml, including offline create. Network loss isn't in the Capabilities table, so it runs as a manual step.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified — literal transport loss and reconnect were not driven.

Findings: I found no actionable defects in the reviewed delta.

QA for 753d6d1: iPhone simulators, disposable regtest wallets and authenticated staging backup service. A temporary test-only network monitor controlled the offline branch; recovery fault selectors throw at the app's real storage and profile boundaries.

Test 1: controlled offline New Wallet ✅ Creation stayed in initialization without the restore error and reached Home after the monitor was released.

offline-2x.2x.mp4

Test 2: controlled offline passphrase creation ✅ Creation stayed in initialization without the restore error and reached Home after the monitor was released.

passphrase-2x.2x.mp4

Test 3: optional profile failure after required restore ✅ Home remained available and retry reloaded the restored profile after the profile-load fault was removed.

The app log confirmed both required markers cleared and the node started before the profile failure. The second recording repeats the profile failure and retry.

markers-2x.2x.mp4

profile-repeat-2x.2x.mp4

Test 4: WALLET and USDT marker storage faults ✅ Read, write and delete failures at both marker boundaries kept recovery gated and each restore completed after its fault was removed.

Each fault was exercised on a fresh restoration. A saved endpoint correction was applied before the final node start. App logs confirmed each injected boundary, both marker deletions and node startup. The final recording repeats all six faults in sequence; earlier recordings show their individual retries.

markers-2x.2x.mp4

usdt-write-2x.2x.mp4

wallet-read-2x.2x.mp4

usdt-read-2x.2x.mp4

usdt-delete-2x.2x.mp4

wallet-delete-2x.2x.mp4

marker-repeat-2x.2x.mp4

Test 5: literal transport loss and reconnect ⏭️ Not driven: the simulator had no device-scoped transport switch.

The controlled monitor tests above establish the application branch, not a physical transport disconnect or reconnect.

The recordings have visible stutters; the accelerated copies retain all changed frames. The earlier review covers the required restore, cancellation and correction-persistence cases at 911f24c; this delta leaves those paths intact. The separate delta review also checked the offline guard and reconnect path.

Coverage: delta 911f24c..753d6d1, plus the offline-creation finding and recovery cases. The documentation edit has no device behavior. The USDT fixture had no nonempty USDT snapshot; spending and full-chain synchronization were not driven.

Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes shares the restricted editor and durable correction contract; startup order differs, and its journey does not include the new offline-create step.

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


Reviewed by gpt-6.1-terra-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: ⚠️ Not verified — literal transport loss and reconnect remain unrun.

Findings: No new actionable defects in the delta. The admission repair from bitkit-ios#1120 fix: decouple ordinary receive and startup from Paykit publication checks the original identity, activation and caller admission immediately before the SDK write, including after same-identity reactivation.

QA for 89e5e94: iPhone 17 Pro simulators, iOS 26.5, Xcode 27.0 / SDK 27.0; disposable unfunded regtest wallets and staging Pubky.

Disable allowances while an offer waits for acceptance ✅ The old operation was refused after disabling, including after re-enabling the same identity; a fresh pass accepted the offer.

Two genuine profiles were created and linked through the app. A payer proposed a $5-per-payment, $50-monthly allowance. In the first run, releasing acceptance while the payee's Allowances switch was off logged refusal; payer UI readback retained “Waiting for an answer.” Re-enabling allowed a fresh pass, and UI readback on both wallets showed Active. In the focused repeat, the switch was turned off and back on before release: the old operation was still refused, and a later fresh pass accepted the second offer.

The payee build adds an unpushed bounded DEBUG hold under the SDK lock immediately before the new admission closure, plus admission logging. Identity creation, linking, private messages, proposal records and native SDK writes use the app's real services. No payment was sent. The refusal/fresh-admission distinction was checked in the app log; the recording shows the switch changes and final allowance state.

Release while disabled Reactivate before release
offer-pair2x.2x.mp4
offer-repeat2x.2x.mp4
Original run: payer left, payee right, time-aligned at 2×; the payer recording has visible stutter. The focused repeat on the payee is also 2× and retains every captured changed frame.
Timing control and deciding trace

The only local source change is in PaykitSdkService.acceptAllowance, inside its existing locked admission closure. A simulator launch argument enables a wait of at most ten minutes on a disposable local file before calling the unchanged admit() closure. Removing that file releases the wait. The added logs distinguish refusal from later admission; the native sdk.acceptAllowance call is unchanged.

06:38:44.165 CET HOLD before admission
06:39:00.957 CET REFUSED after release while disabled
06:39:32.054 CET ADMITTED on a fresh pass after re-enabling
06:42:44.286 CET HOLD before admission
06:43:21.333 CET REFUSED after off/on and release
06:43:37.497 CET ADMITTED on a later fresh pass
Check: build and allowance, SDK admission and recovery regressions ✅ The unmodified head built and the CI-selected unit suite completed without failures.

Ran xcodebuild build and xcodebuild test selecting BitkitTests with the workflow's four integration-class exclusions. Finalized summary and nested results agree. The queued pause/reactivation regression passed. One encrypted recovery protocol-fixture test was skipped because its fixture URL was absent; it receives no pass credit. Project signing and warnings-as-errors remained intact. SwiftFormat 0.64.4 passed the four changed Swift files, and git diff --check passed.

Four test-attached QoS runtime warnings, two test-build AppIntents notices and simulator XPC messages remain; no source-cause claim is made. This run did not select UI tests.

New wallet waits through transport loss and reconnect ⏭️ No device-scoped network-path switch was available.

Manual 3: fresh simulator Settings exposes no Wi-Fi or airplane-mode controls. App-prefs routes returned generic Settings; prefs routes failed with code 115. Available simulator commands provide no transport switch. Endpoint failures and earlier controlled-monitor results do not establish literal transport loss or reconnect.

Earlier passing reports remain applicable to the unchanged tested boundaries:

Coverage: contiguous f8bd749..89e5e94 delta, all five changed files, prior discussions and reached manager, SDK admission, automatic and manual acceptance callers. Connection-recovery product paths are unchanged. The changelog has no device behavior; full automatic-payment and nonempty USDT-import journeys were not repeated.

Deferred findings remain tracked separately:

Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes, still at c747cc0, retains the restricted editor and durable-correction contract. iOS restores required backup state before starting a rebuilt node; Android permits node startup during restoration. This is a scoped contract comparison.


Reviewed by gpt-6.1-sol-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-reviewed to 89e5e94. No findings. Approving.

The only PR-owned change is 13b9119 (NodeEventListener.swift, plus tests). Both merges have an empty remerge-diff.

Checked:

  • Stop during an in-flight start now waits on startWaiter instead of throwing. The waiter is installed in the same main-actor turn as the isStarting check and resumed once in start's defer, so it cannot be missed or leak. A second stop or a new start while stopping still throws, and callers never overlap (ConnectionRecoveryService admission, beginWipe drain, WalletViewModel.start early return).
  • The unbounded wait is not new: on master node.stop() already queued behind node.start() on the serial ldk queue, so a wedged start blocks stop the same way.
  • Cancellation: the non-throwing continuation and ServiceQueue.background still stop the node when the caller is cancelled, which the cancelStop test loop covers.
  • Android twin: LightningRepo already serialises start and stop under lifecycleMutex, and LightningService.stop() runs NonCancellable behind the in-flight start on ldkQueue. Same outcome, different shape.

Performance: this changes serialisation only. It adds no new repeated work, and the order matches master. The idle-poll logging stays deferred to #1181.

Device gate: 89e5e94 (app code 13b9119; the merges since only bring in base-branch commits) — perf: idle CPU 2.09% vs master 1.59% (one sample, by @ovi-reviewer); Electrum save during startup and payment after restart passed (by @ovi-reviewer).
Skipped — already driven this round at 13b9119 by @ovi-reviewer: Electrum save while the node starts (passed), on-chain payment after a node restart (passed), idle CPU against master: #1139 (review).
Skipped — covered by CI at 89e5e94: Run Tests, SwiftFormat (green).
⚠️ Not run: connection-recovery.xml transport loss and reconnect. It needs a device-scoped network toggle, which the iOS Simulator does not have, and blocking the host network needs root. This approval rests on the code scan, the component tests and the runs listed above.

@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: Corrected retained full reassessment of synonymdev/bitkit-ios#1139 at 89e5e941 against merge base 2c0c1002. The original complete-diff coverage is retained. This correction rechecks finding provenance, affected callers and the current review discussions.

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

One new Retry-specific finding remains: an offline Retry from an existing startup error can clear the error and leave the initializer without recovery controls. The earlier initial-wallet-creation fix remains separate. The logging and initialization-progress threads are resolved by deferral to open issues #1181 and #1158, as explained below; this report does not reopen those threads or claim their underlying behavior was fixed.

Validation: exact Retry/admitted-start method bodies were compiled in an isolated Swift probe with platform/dependency collaborators stubbed. Offline, the error cleared, the node stopped, no editor existed and native start was skipped; the online control reached start/running. AppScene rendering was inspected separately. No app test suite, simulator, transport-loss test or native-LDK execution was performed for this correction. The original full coverage and scoped Android comparison retain their recorded provenance.

Findings

  • [MEDIUM] Offline Try Again hides connection recovery — inline at Bitkit/AppScene.swift:1314.

Existing concerns

  • [LOW] Idle node polling logs every tick — The original review thread is resolved by deferral to this open LOW follow-up. The 250 ms polling/performance-log path remains in the pinned source; reducing its log volume is separate work. It is not a new finding in this review and is not claimed fixed.
  • [LOW] Initializing loader restarts from 0% when connection recovery becomes idle — The original review thread is resolved by deferral to this open LOW presentation follow-up. The .id(connectionRecovery.isBusy) reset mechanism remains in the pinned source. This is separate from restore-gate correctness and is not claimed fixed.

Reviewer model: grok-4.7
Reasoning effort: high

Comment thread Bitkit/AppScene.swift Resolved

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified — literal transport loss and reconnect remain unrun.

Findings: No new actionable defects. The offline Retry finding is fixed at 4637d91.

QA for 4637d91: iPhone 17 Pro simulators, iOS 26.5, Xcode 27.0 / SDK 27.0; a disposable unfunded regtest wallet and staging backup services.

Keep recovery controls after an offline Retry ✅ Try Again and Connection Settings remained available, and startup recovered when the controlled monitor was released.

A real native startup failure reached the error screen with no required restore pending. Offline Retry retained both controls. I then opened Connection Settings, saved the reachable staging Electrum endpoint, returned and retried while still offline: both controls remained. Releasing the monitor resumed native startup, completed initialization and presented the unrelated USDT introduction over Home.

App logs confirm both offline skips, successful native startup, correction deletion and initialization completion. The build adds only the bounded DEBUG monitor control below; native services, backup state and Retry decisions are unchanged.

retry-offline-2x.2x.mp4

Startup failure, offline retries, connection correction and recovery at 2×. Minor recording stutters remain; the accelerated copy retains every captured changed frame.

Test control

Not pushed. An explicit launch argument makes the application monitor follow one disposable file in a bounded polling loop. This exercises the offline application branch rather than a physical transport disconnect.

diff --git a/Bitkit/Managers/NetworkMonitor.swift b/Bitkit/Managers/NetworkMonitor.swift
index 49b6c716..36dfab3f 100644
--- a/Bitkit/Managers/NetworkMonitor.swift
+++ b/Bitkit/Managers/NetworkMonitor.swift
@@ -22,6 +22,22 @@ final class NetworkMonitor: ObservableObject {
     let monitor = NWPathMonitor()
 
     init() {
+        #if DEBUG // test hook
+            if Env.isE2E, !Env.isUnitTest, ProcessInfo.processInfo.arguments.contains("-V4_RETRY_MONITOR") { // test hook
+                Task { @MainActor [weak self] in // test hook
+                    for _ in 0..<3600 { // test hook
+                        guard let self else { return } // test hook
+                        let connected = !FileManager.default.fileExists(atPath: Env.appStorageUrl.appendingPathComponent("v4-1139-offline").path) // test hook
+                        if self.isConnected != connected { // test hook
+                            Logger.info("Controlled monitor connected=\(connected)", context: "V4Retry") // test hook
+                            self.isConnected = connected // test hook
+                        } // test hook
+                        try? await Task.sleep(for: .milliseconds(500)) // test hook
+                    } // test hook
+                } // test hook
+                return // test hook
+            } // test hook
+        #endif // test hook
         // Set the pathUpdateHandler
         monitor.pathUpdateHandler = { [weak self] path in
             DispatchQueue.main.async {
Check: build and recovery, publication and payment-proof regressions ✅ The unmodified head built and the workflow-selected unit suite completed without failures.

Finalized summary and nested test results agree. Coverage includes the changed last-channel callback regression, all three current-billing-period regressions, startup acknowledgement, cancellation and listener lifecycle. Project signing and warnings-as-errors remained intact. SwiftFormat 0.64.0 passed all four changed Swift files; git diff --check passed.

One encrypted recovery protocol-fixture case skipped because its fixture URL was absent and receives no pass credit. Four test-attached QoS warnings, a dependency sysroot warning and AppIntents metadata notices remain; no causal claim is made. UI tests were not selected.

New wallet waits through transport loss and reconnect ⏭️ No device-scoped network-path switch was available.

Manual 3: Settings exposes no Wi-Fi or airplane-mode switch. The Wi-Fi settings URL returned generic Settings; the airplane-mode URL failed with code 115. Available simulator commands provide no transport toggle. The controlled-monitor result above does not establish literal transport loss or reconnect.

Retained passing reports for unchanged behavior:

Coverage: contiguous 89e5e94..4637d91 delta, all six changed files, prior discussions and reached callers. Fresh device coverage exercises Retry; channel closure retains the linked real-channel report and fresh component coverage. Billing-period changes have component coverage; changelog edits have no device behavior. No new authenticated restore, nonempty USDT import or funded payment is claimed.

Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes, still at c747cc0, retains the restricted editor and durable-correction contract. iOS restores required backup state before starting a rebuilt node; Android permits node startup during restoration. This is a scoped contract comparison.


Reviewed by gpt-6.1-sol-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 4637d91. The new retry check keeps the recovery controls up when a startup-error retry ends with recovery still not ready (canReportStartupFailure()), and refreshAfterChannelClosed now withdraws the public invoice after the last channel closes; testLastChannelClosureWithdrawsPublicInvoiceWithoutForegroundRetry covers it. No new findings.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified. Reconnect after a real network loss is still unrun.

The PR moved to 85db6cf after this run; these results are for 34925fb.

Findings: None new in 4637d91..34925fb.

QA for 34925fb: iPhone 17 Pro simulator, iOS 26.5, Xcode 27.0. The app was built with the PR's E2E regtest command. The wallet was a new, empty regtest wallet.

New wallet waits during a real network loss ✅ With the host's network actually off, New Wallet stayed on Setting Up Your Wallet with no wallet access and no balance error.

I switched off the simulator host's only network interface, confirmed by a failed ping, and then tapped New Wallet. The app saved the wallet and logged Network offline, skipping wallet start. It stayed on Setting Up Your Wallet for the whole 60-second outage. There was no Home, no Spending balance error and no Connection Settings. This is a real transport loss, not the DEBUG monitor control used in earlier runs.

Note: the simulator status bar always shows Wi-Fi. The outage shows in the app log (Network changed: disconnected, then The Internet connection appears to be offline on every request).

new-wallet-network-loss-4x.2x.mp4

Onboarding, New Wallet while offline, the network coming back and a relaunch, at 4×.

offline 15 s offline 60 s

New wallet resumes after the network returns ⏭️ The simulator's network monitor never reported the network coming back, so `handleNetworkChange(true)` never ran.

The interface came back and the app's own HTTP requests succeeded from 77 s after reconnect. NWPathMonitor never delivered a satisfied path, so isConnected stayed false and the screen stayed on Setting Up Your Wallet for more than 2 minutes. Relaunching the app built a fresh monitor, which started the node (Node started) and opened Home.

A second cut with the wallet already on Home behaved the same: the monitor logged disconnected and never connected. That path is code this PR does not change, so this is a simulator limitation, not a defect I can attribute to the PR. The reconnect restart (AppScene.swift handleNetworkChange → nodeRestarter.restart) needs a physical iPhone. From the code, a foreground or a Retry also skips the start while isConnected stays false (startAdmittedWallet guard), so only a relaunch recovers when the monitor misses the return.

still setting up 2 min after reconnect after relaunch

Check: cached Blocktank backup tests ✅ BlocktankBackupTests passed 2/2 on the head.

In bitkit-core 0.8.0-rc11, get_orders and get_cjit_entries only go to the network when ids are passed. That makes refresh: false without ids the same cached read as before. info(refresh: false) returns nil on an empty table, so the stricter try fails a backup only on a real DB or decode error. In that case it keeps synced/required unchanged. These changes come from the base branch merge.

Check: profile recovery UI test with the system Paste button ✅ ProfileRecoveryUITests passed on the head (72.8 s).

Reset now blanks the shown value, so waitForClipboard("not-copied") passes only after a real paste through PasteButton. That makes the check stronger than the old programmatic read.

Reports relied on, unchanged by this delta (AppScene.swift and the recovery services are untouched):

Coverage: delta 4637d91..34925fb, all 5 files. No device case drove the Blocktank backup read change or the changelog edit; the unit tests above cover the backup change. The PR moved to 85db6cf during this run. This review covers 34925fb.

Twin: bitkit-android#1685 is merged at c747cc0. This delta (cached Blocktank backup reads, a DEBUG test fixture) has no Android counterpart to compare.


Reviewed by claude-opus-5-5-medium

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified. Reconnect after a real network loss is still unrun.

Findings: None new in 34925fb..85db6cf.

QA for 85db6cf:

Check: scheduled backup and connection recovery unit tests ✅ 48/48 passed on the head, including the new idle-after-cleanup test.

I ran these on an iPhone 17 Pro simulator with iOS 26.5 and Xcode 27.0:

  • BackupForegroundRecoveryTests: 13 passed, including testScheduledIdlePublicationWaitsForForegroundAdmissionCleanup.
  • ConnectionRecoveryServiceTests: 28 passed.
  • NodeEventListenerTests: 7 passed.

The new test fails on the old ordering. Before this change, running: false was published before the backup queue removed the job. The test holds that queue and checks that no idle status is published before the job is removed.

New wallet resumes after the network returns ⏭️ Still unrun: on a simulator the network monitor never reports the network coming back.

At 34925fb, the iOS simulator's NWPathMonitor never reported the network coming back after a real outage. This happened on two separate network cuts. That code is not changed by this PR. Testing this needs someone to switch the network off and on again on a physical iPhone. This delta does not touch AppScene.swift or the recovery services, so the network-loss pass at 34925fb still holds.

From the code: runScheduledBackup (BackupService.swift:1081) now removes the job and publishes running: false in one block on the backup queue. Because of that, a foreground retry can no longer see the backup as idle while its job is still registered. The block always runs, so running cannot get stuck at true. Status subscribers already ran on this queue from scheduleBackup, and neither subscriber calls back into the queue synchronously, so this adds no deadlock.

Reports relied on, still valid at this head:

Coverage: delta 34925fb..85db6cf, all 3 files. The delta is only the merge of #1123 4e6c06ed. The PR's own patch is identical to the one at 34925fb. No device case drove the backup change; it only affects when the backup status is published, and the unit tests above cover it.

Twin: bitkit-android#1685 is merged. This delta is an iOS backup-queue ordering fix from the base branch, with no Android counterpart to compare.


Reviewed by claude-opus-5-5-medium

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified. Reconnect after a real network loss is still unrun.

Findings: None new in 85db6cf..5063625.

QA for 5063625:

New wallet resumes after the network returns ⏭️ Still unrun: the simulator's network monitor never reported the network coming back, on iOS 18.5 either.

At 34925fb this was tried on iOS 26.5 by switching Wi-Fi power off and on. This time I used an iPhone 16 Pro simulator with iOS 18.5 and two real network cuts:

  • Wi-Fi power off, then on. The monitor missed the outage itself. It logged Network changed: disconnected only 3 s after the network came back, and never logged connected. Setup ran through the outage and finished 84 s later, after the return, so Home opened without the reconnect restart ever running. This does not test the PR's path.
  • Wi-Fi service off, then on. The offline gate worked: the app logged Network offline, skipping wallet start and stayed on Setting Up Your Wallet for the 60-second outage. After the return, the app's own requests succeeded within 8 s (Currency rates refreshed successfully). The monitor never logged connected, and the screen was still on Setting Up Your Wallet 3 minutes later. This is the same result as at 34925fb.

So handleNetworkChange(true) → nodeRestarter.restart still needs someone to switch a physical iPhone's network off and on. The simulator stays stuck because its monitor never reports the network back, in code this PR does not change.

new-wallet-reconnect-ios18-4x.2x.mp4

The Wi-Fi service cut, at 4×: New Wallet while offline, the network coming back after 60 s, and the screen still setting up 3 minutes later.

offline 60 s online 180 s

Check: backup retry unit tests ✅ 52/52 passed on the head, including the three new authentication-transport tests.

I ran these on an iPhone 17 Pro simulator with iOS 26.5:

  • BackupServiceTests: 39 passed, including testReleasedVssAuthTransportIsTemporaryWithoutRetryingAuthorizationFailures, testReleasedVssAuthTransportRetriesScheduledSettingsWithoutObservation and testReleasedVssAuthorizationDenialStaysPendingWithoutAutomaticRetry.
  • BackupForegroundRecoveryTests: 13 passed.

From the code: the delta adds VssError.AuthError to isTemporaryBackupFailure (BackupService.swift:513). It is retried only when the details are exactly error sending request for url (<valid http(s) URL>), with at most one error performing external request: prefix removed first. Other auth errors, such as permission denied or a 401/403, stay .failed, and canRetryPendingBackup does not retry those. The function has one caller, runBackup (BackupService.swift:467). The scheduled retry waits 10 s at first and doubles the wait up to 300 s. It stops on wipe, restore, the USDT restore gate or cancellation, so an auth endpoint that stays unreachable cannot cause a tight loop or leave running stuck. URLComponents(string:encodingInvalidCharacters:) needs iOS 17, which is the deployment target.

Reports relied on, still valid at this head:

Coverage: delta 85db6cf..5063625, both files. The delta is only the merge of #1123 11fb65d, and the PR's own patch against its base is unchanged. No device case drove the auth-retry change; the unit tests above cover it.

Twin: bitkit-android#1685 is merged. This delta is an iOS backup auth-retry fix from the base branch, with no Android counterpart to compare.


Reviewed by claude-opus-5-5-medium

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified — a real network reconnect remains unrun.

Findings: I found no new actionable defects in 5063625..d8fa381. The integration from bitkit-ios#1123 fix: serialize backup observation lifecycle preserves the original-owner connection drain, closes node/backup admission, retires VSS clients before credential deletion, and keeps the wipe intent until completion. An independent source review reached the same conclusion.

QA for d8fa381: iPhone 17 Pro simulators, iOS 26.5, Xcode 27.0 / SDK 27.0; disposable, unfunded regtest wallets and authenticated staging VSS. Restore/wipe cases use the disclosed unpushed DEBUG controls below. Authentication, backup encryption, imports and native node startup use the real services.

Saved connections survive required restoration ✅ Corrections survived rejected imports, foreground cycles and cold restart; required acknowledgements preceded wallet access.

J1: Electrum and RGS saved independently without retrying. A failed RGS probe retained the acknowledged revision. A rejected USDT recovery kept access restricted through three foreground cycles and cold restart. A later WALLET import rejection kept recovery gated after USDT cleared. App-state readback confirmed that older SETTINGS restored the unrelated balance-visibility preference while preserving both corrected endpoints. Final readback showed a running node, both required markers absent and the correction cleared before Home.

restore2x.2x.mp4

Connection saves, failed probing, required-import failures, foreground cycles, restart and completion at 2×. Persistence and acknowledgement claims were also checked in the app-state log.

Cancellation and durable wallet replacement ✅ Back did not abandon an admitted import; wipe drained a held probe and resumed behind its gate after relaunch.

Manual 1: a held WALLET import completed after Back, reaching Wallet Restored. In the later wipe, the app stayed at step 1 while a canceled RGS probe remained held and advanced only after release. I terminated at step 6, relaunched, and observed the wipe gate at step 6 again. Releasing that hold completed deletion. A newly created wallet opened with default Electrum/RGS, normal balance visibility, no correction, no required markers and no wipe intent, confirmed in app-state readback.

cancel-wipe2x.2x.mp4

Import cancellation, held-probe drain, interrupted wipe, resumed gate and replacement at 2×. Drain order and final settings were also checked in the app-state log.

Recovery storage errors keep wallet access restricted ✅ All eleven storage faults kept recovery gated; explicit Retry completed the remaining work.

Manual 2: correction-record read, write and delete failures, preference readback and flush failures, and read/write/delete failures for both required markers were exercised. Correction and preference faults were staged successively during one recovery. A rejected correction write left preferences unchanged; failed preference acknowledgement retained the durable correction. A denied correction deletion kept the editor reachable even after both imports and native startup completed.

Each of the six required-marker faults used a fresh actual restoration. Its selected boundary appeared in the error UI. Removing only the selector and tapping Retry completed the remaining work; final state readback showed a running node, no required markers and no correction. These storage-fault recoveries were checked within the same process; the separate cold-restart and interrupted-wipe cases are above.

storage2x.2x.mp4

Storage faults and explicit recovery at 2×; durable state was also checked in the app-state log.

Exhausted PIN attempts remain behind the wipe gate ✅ The eighth wrong PIN replaced the PIN pad with Resetting Bitkit; relaunch opened onboarding without another PIN prompt.

The unmodified head created a fresh wallet and enabled a PIN with biometrics off. Seven wrong attempts reached the last-attempt warning. The next wrong entry raised the wipe gate and completed deletion. A subsequent relaunch remained at fresh onboarding. The installed executable matched the unmodified-head build receipt. The separate interrupted-wipe case above checks relaunch while a wipe is still pending.

pin2x.2x.mp4

PIN setup, exhausted attempts, wipe gate and onboarding after relaunch at 2×.

Check: build and lifecycle, backup, PIN and payment regressions ✅ The unmodified head built and the workflow-selected unit suite passed without failures.

xcodebuild test selected BitkitTests with the workflow's four integration-class exclusions. The finalized summary and nested test statuses agree. The selection covers the wipe and VSS integration from bitkit-ios#1123, backup generations, PIN, allowance, payment confirmation and Trezor regressions, plus connection recovery and listener lifecycle. Project signing and warnings-as-errors remained intact. SwiftFormat 0.64.0 passed all changed Swift files; translation validation completed with zero errors and 17 warnings; git diff --check passed.

One opt-in native USDT recovery-protocol case skipped because its fixture URL was absent; it receives no pass credit. The result retains QoS runtime observations and one SwiftUI State-outside-View warning. Post-test simulator diagnostics were incomplete; no runtime-causality or warning-free claim is made. I did not run the full UI suite or the excluded integration classes.

New wallet resumes after a network loss ⏭️ A real offline-to-online path transition could not be produced on the simulator.

Manual 3: Settings exposed no Wi-Fi or airplane-mode control. The Wi-Fi URL returned general Settings; the airplane-mode URL returned code 115 in the CLI result. Available simulator commands provide no transport toggle. Endpoint refusal does not make this app's NWPathMonitor report a network loss, and no disposable physical iPhone was available. The earlier network-loss pass is separate from a reconnect pass; no reconnect success is inferred.

network-route2x.2x.mp4

Available simulator Settings at 2×. This records the setup check, not a network-loss/reconnect test.

The recordings contain visible stutters; accelerated copies passed the changed-frame retention check.

Reports retained for unchanged behavior:

Coverage: contiguous 5063625..d8fa381 delta, prior discussions and reached callers, including the durable wipe/VSS/backup composition and merged payment, allowance, Trezor and display changes. Fresh device coverage exercises restoration, cancellation, wipe and PIN lockout. Merged payment, allowance, Trezor and layout changes have component/source coverage here; their full product journeys were not driven. No nonempty USDT import, funded spending or general running-LDK quiescence claim is made. Existing LOW initialization and polling-log follow-ups remain deferred to bitkit-ios#1158 Preserve initialization progress across connection-recovery completion and bitkit-ios#1181 Reduce performance-log noise from idle node event polling.

Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes, merged at c747cc0, retains the restricted editor and durable-correction contract. iOS restores required backup state before starting the rebuilt node; Android can need a running node during restoration. This is a scoped contract comparison.

Unpushed test controls and deciding state

The controls stage a normal restore of an acknowledged test-wallet backup, hold or reject real import/probe/wipe boundaries and read back state. They do not synthesize backup contents, authentication or successful native results. Storage faults use the committed -E2E_CONNECTION_RECOVERY_FAULTS allowlist. The test seed was kept only in the disposable app container and is excluded from this report.

Selected app-state and hold/release lines, with timestamps omitted. The second HOLD hold-wipe follows relaunch; final replacement settings are read back afterward.

ACK WALLET bytes=323 SETTINGS bytes=306 result=completed
REJECT reject-usdt
REJECT reject-wallet
STATE node=stopped WALLET=present USDT=absent correction=AD7388B6-3E7D-427A-8233-B28C46E611E6 electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot/ hideBalance=true busy=false gate=none wipe=absent
STATE node=running WALLET=absent USDT=absent correction=absent electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot/ hideBalance=true busy=false gate=none wipe=absent

HOLD hold-wallet
STATE node=stopped WALLET=present USDT=absent correction=D501255B-1591-4C41-9FAE-294551389C29 electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot/ hideBalance=true busy=true gate=none wipe=absent
RELEASE hold-wallet
HOLD hold-probe
COMMAND wipe
RELEASE hold-probe
HOLD hold-wipe
HOLD hold-wipe
STATE node=running WALLET=absent USDT=absent correction=absent electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot hideBalance=false busy=false gate=none wipe=absent

STATE node=initializing WALLET=unreadable USDT=absent correction=absent electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot hideBalance=false busy=false gate=none wipe=absent
STATE node=running WALLET=absent USDT=absent correction=absent electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot hideBalance=true busy=false gate=none wipe=absent
diff --git a/Bitkit/AppScene.swift b/Bitkit/AppScene.swift
index b37855f9..ca322a1b 100644
--- a/Bitkit/AppScene.swift
+++ b/Bitkit/AppScene.swift
@@ -588,6 +588,9 @@ struct AppScene: View {
                 config in AppUpdateSheet(config: config)
             }
             .task(priority: .userInitiated, setupTask)
+            #if DEBUG // test hook
+            .task { await v4Commands() } // test hook
+            #endif // test hook
             .task(id: [
                 scenePhase == .active,
                 wallet.walletExists == true,
@@ -1064,6 +1067,58 @@ struct AppScene: View {
             TimedSheetManager.shared.onPrimaryScreenExited()
         }
     }
+ // test hook
+    #if DEBUG // test hook
+    @MainActor private func v4Commands() async { // test hook
+        guard Env.isE2E, !Env.isUnitTest else { return } // test hook
+        for _ in 0..<24000 { // test hook
+            guard !Task.isCancelled else { return } // test hook
+            let path = V4QA.directory.appendingPathComponent("qa-command") // test hook
+            if let command = try? String(contentsOf: path, encoding: .utf8) { // test hook
+                try? FileManager.default.removeItem(at: path) // test hook
+                Task { @MainActor in // test hook
+                    let command = command.trimmingCharacters(in: .whitespacesAndNewlines) // test hook
+                    V4QA.log("COMMAND " + command) // test hook
+                    do { // test hook
+                        switch command { // test hook
+                        case "backup": // test hook
+                            await BackupService.shared.stopObservingBackups() // test hook
+                            ElectrumConfigService().saveServerConfig(ElectrumServer(host: "127.0.0.1", port: 1, protocolType: .tcp)) // test hook
+                            UserDefaults.standard.set(true, forKey: "hideBalance") // test hook
+                            try await BackupService.shared.persistWalletBackup() // test hook
+                            let result = await BackupService.shared.triggerBackup(category: .settings) // test hook
+                            guard let walletData = try await VssBackupClient.shared.getObject(key: "WALLET")?.value, // test hook
+                                  let settingsData = try await VssBackupClient.shared.getObject(key: "SETTINGS")?.value, // test hook
+                                  let seed = try Keychain.loadString(key: .bip39Mnemonic(index: 0)) else { // test hook
+                                throw AppError(message: "Missing fixture backup", debugMessage: nil) // test hook
+                            } // test hook
+                            try seed.write(to: V4QA.directory.appendingPathComponent("qa-seed"), atomically: true, encoding: .utf8) // test hook
+                            V4QA.log("ACK WALLET bytes=\(walletData.count) SETTINGS bytes=\(settingsData.count) result=\(result)") // test hook
+                        case "wipe": AppReset.begin(.dev, app: app) // test hook
+                        case "restore": // test hook
+                            let seed = try String(contentsOf: V4QA.directory.appendingPathComponent("qa-seed"), encoding: .utf8) // test hook
+                            wallet.nodeLifecycleState = .initializing // test hook
+                            wallet.isRestoringWallet = true // test hook
+                            BackupService.shared.setRestoring(true) // test hook
+                            SettingsViewModel.shared.pendingRestoreActivitySeenSince = UInt64(Date().timeIntervalSince1970) // test hook
+                            SettingsViewModel.shared.restoreSyncedBlockHeight = 0 // test hook
+                            SettingsViewModel.shared.monitorAllAddressTypes() // test hook
+                            try StartupHandler.restoreWallet(mnemonic: seed, bip39Passphrase: nil) // test hook
+                            try InstallationMarker.create() // test hook
+                            try wallet.setWalletExistsState() // test hook
+                        case "retry": await retryWalletStart() // test hook
+                        case "snapshot": break // test hook
+                        default: throw AppError(message: "Unknown QA command", debugMessage: nil) // test hook
+                        } // test hook
+                        V4QA.snapshot(wallet) // test hook
+                        V4QA.log("DONE " + command) // test hook
+                    } catch { V4QA.log("ERROR \(command): \(error)") } // test hook
+                } // test hook
+            } // test hook
+            try? await Task.sleep(for: .milliseconds(300)) // test hook
+        } // test hook
+    } // test hook
+    #endif // test hook
 
     // MARK: - Event Handlers
 
diff --git a/Bitkit/Services/BackupService.swift b/Bitkit/Services/BackupService.swift
index d12b8e68..4cb4c98e 100644
--- a/Bitkit/Services/BackupService.swift
+++ b/Bitkit/Services/BackupService.swift
@@ -730,6 +730,10 @@ class BackupService {
 
     private func restoreUsdtRecovery(legacyWallet: Data?) async throws {
         let legacy = try legacyWallet.map { try JSONDecoder().decode(WalletBackupV1.self, from: $0) }
+        #if DEBUG // test hook
+        await V4QA.hold("hold-usdt") // test hook
+        try await V4QA.reject("reject-usdt") // test hook
+        #endif // test hook
         try await usdtRecoveryBackup.restore(
             legacyWallet: legacy?.usdtWallet,
             legacyPaymentState: legacy?.paykitPaymentState?.usdt,
@@ -1479,6 +1483,12 @@ class BackupService {
             }
 
             if let dataBytes {
+                #if DEBUG // test hook
+                if category == .wallet { // test hook
+                    await V4QA.hold("hold-wallet") // test hook
+                    try await V4QA.reject("reject-wallet") // test hook
+                } // test hook
+                #endif // test hook
                 try await restoreAction(dataBytes)
                 Logger.info("Restore success for: '\(category.rawValue)'", context: "BackupService")
             } else {
diff --git a/Bitkit/Utilities/AppReset.swift b/Bitkit/Utilities/AppReset.swift
index bd74e8d7..9d2084d0 100644
--- a/Bitkit/Utilities/AppReset.swift
+++ b/Bitkit/Utilities/AppReset.swift
@@ -105,6 +105,9 @@ final class AppReset: WipeStepExecutor {
         case .wipeLightningStorage:
             try await wallet.wipe()
         case .wipeDatabases:
+            #if DEBUG // test hook
+            await V4QA.hold("hold-wipe") // test hook
+            #endif // test hook
             // bitkit-core wipes only databases it has opened, which `CoreService` does when it is created.
             _ = CoreService.shared
             let coreWipeResult = try await wipeAllDatabases()
diff --git a/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift b/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift
index f826e851..9530df59 100644
--- a/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift
+++ b/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift
@@ -91,6 +91,9 @@ extension SettingsViewModel {
     /// and returns a 2xx for a valid endpoint, so a HEAD request confirms reachability without
     /// downloading the body. Mirrors the Android RGS connect check.
     nonisolated func isRgsEndpointReachable(_ url: String) async -> Bool {
+        #if DEBUG // test hook
+        await V4QA.hold("hold-probe") // test hook
+        #endif // test hook
         let base = url.hasSuffix("/") ? String(url.dropLast()) : url
         guard let testUrl = URL(string: "\(base)/0") else {
             return false
diff --git a/Bitkit/Utilities/V4QA.swift b/Bitkit/Utilities/V4QA.swift
new file mode 100644
index 00000000..1b07b77b
--- /dev/null
+++ b/Bitkit/Utilities/V4QA.swift
@@ -0,0 +1,43 @@
+#if DEBUG // test hook
+import Foundation // test hook
+ // test hook
+@MainActor enum V4QA { // test hook
+    static var directory: URL { FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0] } // test hook
+    static func flag(_ name: String) -> Bool { Env.isE2E && !Env.isUnitTest && FileManager.default.fileExists(atPath: directory.appendingPathComponent("qa-" + name).path) } // test hook
+    static func log(_ value: String) { // test hook
+        let target = directory.appendingPathComponent("qa-events.log") // test hook
+        if !FileManager.default.fileExists(atPath: target.path) { _ = FileManager.default.createFile(atPath: target.path, contents: nil) } // test hook
+        if let handle = try? FileHandle(forWritingTo: target) { // test hook
+            defer { try? handle.close() } // test hook
+            _ = try? handle.seekToEnd() // test hook
+            try? handle.write(contentsOf: Data("\(Date().timeIntervalSince1970) \(value)\n".utf8)) // test hook
+        } // test hook
+        Logger.info(value, context: "V4QA") // test hook
+    } // test hook
+    static func hold(_ name: String) async { // test hook
+        guard flag(name) else { return } // test hook
+        log("HOLD " + name) // test hook
+        for _ in 0..<1200 { // test hook
+            if !flag(name) { log("RELEASE " + name); return } // test hook
+            await Task.detached { try? await Task.sleep(for: .milliseconds(500)) }.value // test hook
+        } // test hook
+        log("TIMEOUT " + name) // test hook
+    } // test hook
+    static func reject(_ name: String) throws { // test hook
+        if flag(name) { log("REJECT " + name); throw AppError(message: "Controlled " + name + " failure", debugMessage: nil) } // test hook
+    } // test hook
+    static func marker(_ key: KeychainEntryType) -> String { // test hook
+        do { return try Keychain.load(key: key) == nil ? "absent" : "present" } // test hook
+        catch { return "unreadable" } // test hook
+    } // test hook
+    static func snapshot(_ wallet: WalletViewModel) { // test hook
+        let correction: String // test hook
+        do { // test hook
+            if let data = try Keychain.load(key: .connectionRecovery) { // test hook
+                correction = try JSONDecoder().decode(ConnectionRecoveryService.Record.self, from: data).revision.uuidString // test hook
+            } else { correction = "absent" } // test hook
+        } catch { correction = "unreadable" } // test hook
+        log("STATE node=\(wallet.nodeLifecycleState) WALLET=\(marker(.paykitPendingBackupRestore)) USDT=\(marker(.usdtPendingBackupRestore)) correction=\(correction) electrum=\(ElectrumConfigService().getCurrentServer().fullUrl) rgs=\(RgsConfigService().getCurrentServerUrl()) hideBalance=\(UserDefaults.standard.bool(forKey: "hideBalance")) busy=\(ConnectionRecoveryService.shared.isBusy) gate=\(WipeCoordinator.shared.gate) wipe=\(marker(.wipePending))") // test hook
+    } // test hook
+} // test hook
+#endif // test hook

Reviewed by gpt-6.1-sol-high

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified — a real network reconnect remains unrun.

Findings: I found no new actionable defect in the delta. Expired Paykit cleanup now rechecks cancellation and original wipe ownership after suspended reads and before another recovery or endpoint mutation. The independent source review agrees.

QA for 4d265af: Xcode 27.0 / SDK 27.0, iPhone 17 Pro simulator on iOS 26.5.

Check: expired cleanup cannot mutate a replacement wallet ✅ The head built and all five held-read cleanup regressions passed.

Ran xcodebuild build and xcodebuild test, selecting BitkitTests with the workflow's four integration-class exclusions. The finalized summary and nested results agree: no failed tests, including the five cleanup cases, original-wipe ownership, connection recovery and listener lifecycle. The cleanup cases hold registry, backup-revision, capability, peer and first-link-recovery boundaries until after the wipe ends and the replacement SDK is available; the old work starts no further mutation. These use a controlled SDK, rather than an installed-app expired-cleanup journey.

Project signing and warnings-as-errors remained intact. SwiftFormat 0.64.4 passed both changed Swift files; git diff --check passed. One opt-in encrypted USDT recovery-protocol test skipped because its fixture URL was absent and receives no pass credit. The full UI suite and workflow-excluded integration classes were not run.

Build output retains stale-file path warnings and AppIntents notices. The test result retains QoS and SwiftUI State runtime warnings; no causal or warning-free claim is made.

New wallet resumes after a real network loss ⏭️ No device-scoped network-path switch was available.

Manual 3: fresh simulator Settings exposes no Wi-Fi or airplane-mode control. The Wi-Fi URL returned general Settings; the airplane-mode URL failed with code 115. Available simulator commands provide no transport switch, and no disposable physical iPhone was available. Endpoint refusal would not establish an offline-to-online path callback. The earlier real-network-loss pass remains separate from reconnect; no reconnect success is inferred.

network-routes2x.2x.mp4

Simulator Settings at 2×. This records the setup investigation, not a network-loss/reconnect test; the URL error was checked in the command output. The accelerated copy retains every captured changed frame.

Passing reports retained at their recorded revisions:

Coverage: contiguous d8fa381..4d265af delta, all three changed files, all PR discussions and reached cleanup, SDK-lock, deadline and recovery/wipe callers. No fresh device case drove the changed Paykit mutation boundaries; the changelog has no device behavior. Earlier app evidence retains its original scope.

Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes, merged, retains the restricted editor and durable-correction contract checked at c747cc0. iOS restores required backup state before starting the rebuilt node; Android can need a running node during restoration. This delta hardens iOS Paykit cleanup and has no matching recovery change to compare.


Reviewed by gpt-6.1-sol-high

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: full reassessment of 4d265af6 against merge base 003ea7e8, because the comparison base changed and no verified baseline was supplied. Covered the complete PR delta and affected callers. Android c747cc0b and dependency #1123 were comparison-only checks of recovery and backup contracts, not separate full reviews.

2 actionable findings — resolve or provide an evidence-backed rebuttal.

Validation: translation validation passed with zero errors and 17 warnings; diff whitespace and journey XML checks passed. Inspected native CI, including the affected recovery/listener suites; its synthetic merge has the same tree as the reviewed head. No local Swift build or XCTest execution was available on this Linux host. E2E coverage was inspected at 80e26d4e; authenticated recovery and literal connectivity transitions remain runtime checks.

Suggested additional test cases

  • iOS, isolated existing wallet: hold native startup beyond 30 seconds, then request reset/PIN lockout. Expect the gate to offer Retry after a bounded wait, with no deletion before the original work drains.
  • iOS, isolated VSS fixture: hold an event acknowledgement's VSS write while Electrum remains reachable; request an independent node status/balance read. Expect it to complete without waiting for the acknowledgement retry budget.

Device testing: not performed in this review.

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

Findings

  • [MEDIUM] Bound the recovery drain before entering the wipe — inline at Bitkit/Services/ConnectionRecoveryService.swift:146.
  • [MEDIUM] Keep remote event acknowledgements off the shared LDK queue — inline at Bitkit/Services/NodeEventListener.swift:60.

Existing concerns

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

Comment thread Bitkit/Services/NodeEventListener.swift Resolved

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified — a real network reconnect remains unrun.

Findings: I found no new actionable defect in the delta. Both latest findings are fixed: recovery-drain timeout releases only the wipe attempt while the original startup retains admission, and the dedicated serial event queue frees independent LDK work without bypassing the listener's stop/drain. The independent source review agrees.

QA for 154326a: Xcode 27.0 / SDK 27.0, iPhone 17 Pro simulator on iOS 26.5; disposable, unfunded regtest wallets.

Reset offers Retry while startup is still blocked ✅ The gate offered Retry after the 30-second wait, retained the wallet while startup remained held, and completed deletion only after the original startup returned.

Checked existing-wallet startup, then repeated the timeout during initial wallet startup to capture the failed gate directly. Temporary unpushed DEBUG controls held the synchronous LDK worker immediately before node.start(), requested the real AppReset.begin(.userReset), and logged wipe steps and presence-only state. The hold ignores Swift cancellation; the actual native start runs after release. The production deadline, ownership checks and wipe implementation were unchanged.

The existing-wallet trial timed out twice while the original operation stayed busy. Retry was available and accepted a tap. Both trials retained the mnemonic and durable wipe intent while held, and reached no wipe step before native startup returned. After release, the real wipe completed and fresh onboarding appeared; state readback showed the mnemonic and wipe intent absent. This checks the Swift admission/drain boundary, rather than a naturally hung Rust call or general running-LDK quiescence. PIN exhaustion itself retains its earlier evidence below.

wipe-confirmation-2x.2x.mp4

Repeated startup hold, bounded failure with Retry/Export Logs, release and fresh onboarding at 2×. Admission, credential presence and drain order were also checked in the state/step trace. The first trial's Retry was verified through accessibility and its accepted tap; the repeated trial directly confirms its visible presentation.

Check: independent node reads during event acknowledgement ✅ The [held-acknowledgement regression](https://github.com/synonymdev/bitkit-ios/blob/154326affef9a14f5c235b93aedab85d9bff253b/BitkitTests/NodeEventListenerTests.swift#L84) passed: independent LDK work finished before release, and stop waited for acknowledgement and delivery.

The unmodified head built and both ConnectionRecoveryServiceTests and NodeEventListenerTests passed, including the new AppReset timeout/retry regression and existing cancellation, original-owner, stop-error and competing-stop cases. Finalized XCTest results and individual case results agree, with no failures or skips in this selection. This uses the controlled EventNode; an installed-app VSS-write stall was not driven, so no real remote-outage claim is made.

Signing and warnings-as-errors remained intact. SwiftFormat 0.64.4 passed all six changed Swift files; git diff --check passed. The full unit/UI suites and integration classes were not selected. Build logs retain AppIntents metadata notices and an unavailable simulator XPC-service diagnostic; no warning-free claim is made.

New wallet resumes after a real network loss ⏭️ No device-scoped transport transition was available to exercise the connected-path callback.

Manual 3 remains unrun. Fresh simulator Settings has no Wi-Fi or airplane-mode switch. The Wi-Fi URL opened general Settings; the airplane-mode URL failed with code 115. Simulator commands expose no transport switch, and no disposable physical iPhone was available. An unreachable app endpoint would not establish an offline-to-online NWPathMonitor callback.

reconnect-investigation-2x.2x.mp4

Available simulator Settings at 2×. This records the setup investigation, not a reconnect test; the URL failure was checked in command output.

Passing reports retained at their recorded revisions:

Coverage: contiguous 4d265af..154326a delta, all six changed files, prior discussions and reached admission, deadline, settings, wipe and native listener callers. Fresh app coverage drives the bounded wipe wait; the event-worker change has controlled regression coverage, without a fresh remote-VSS device case. Existing LOW follow-ups remain deferred:

Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes, merged, retains the restricted editor, required markers and durable-correction contract checked at c747cc0. Its event acknowledgements also run outside the serialized LDK queue. Android can require a running node during restoration; iOS restores required state before rebuilding startup. Android's drain does not have this new iOS timeout.

Unpushed test controls and deciding trace

The unmodified executable's SHA-256 was dc23a3e16aab0497c9885a6ea59a8c50c0acb5ee19c03f08aacba34080758a47; the installed controlled executable's was a5901e7e6b2604099d555a4b6d923d94e3c2be330d5d1f247424d283633efa9b. Both builds used this head. The controls did not execute under XCTest or in the notification extension. No recovery phrase, backup contents or successful native outcome was fabricated.

Selected first-trial trace, with timestamps omitted:

HOLD START on synchronous LDK queue
COMMAND wipe
STATE node=starting busy=true gate=wiping progress=failed(Optional(Bitkit.WipeStep.quiesce)) mnemonic=present wipe=present
STATE node=starting busy=true gate=wiping progress=failed(Optional(Bitkit.WipeStep.quiesce)) mnemonic=present wipe=present
RELEASE START
NATIVE START returned
WIPE STEP quiesce
WIPE STEP stopNode
WIPE STEP removeRemoteEndpoints
WIPE STEP resetLocalServices
WIPE STEP wipeLightningStorage
WIPE STEP wipeDatabases
WIPE STEP clearPubkyState
WIPE STEP wipeKeychain
WIPE STEP resetSettings
WIPE STEP deletePin
WIPE STEP deleteInstallationMarker
WIPE STEP commit
STATE node=stopped busy=false gate=none progress=idle mnemonic=absent wipe=absent

Applied DEBUG patch:

diff --git a/Bitkit/AppScene.swift b/Bitkit/AppScene.swift
index b37855f9..7d520f28 100644
--- a/Bitkit/AppScene.swift
+++ b/Bitkit/AppScene.swift
@@ -588,6 +588,9 @@ struct AppScene: View {
                 config in AppUpdateSheet(config: config)
             }
             .task(priority: .userInitiated, setupTask)
+            #if DEBUG
+            .task { await v4ReviewCommands() }
+            #endif
             .task(id: [
                 scenePhase == .active,
                 wallet.walletExists == true,
@@ -1065,6 +1068,25 @@ struct AppScene: View {
         }
     }
 
+    #if DEBUG
+    @MainActor private func v4ReviewCommands() async {
+        guard Env.isE2E, !Env.isUnitTest else { return }
+        for _ in 0..<14400 {
+            guard !Task.isCancelled else { return }
+            let file = V4Review.directory.appendingPathComponent("v4-command")
+            if let command = try? String(contentsOf: file, encoding: .utf8) {
+                try? FileManager.default.removeItem(at: file)
+                switch command.trimmingCharacters(in: .whitespacesAndNewlines) {
+                case "wipe": V4Review.log("COMMAND wipe"); AppReset.begin(.userReset, app: app)
+                default: break
+                }
+            }
+            V4Review.snapshot(wallet)
+            try? await Task.sleep(for: .seconds(1))
+        }
+    }
+    #endif
+
     // MARK: - Event Handlers
 
     private func handleCurrencyStaleData(_: Bool) {
diff --git a/Bitkit/Services/NodeEventListener.swift b/Bitkit/Services/NodeEventListener.swift
index 42ddbc36..a1fcece5 100644
--- a/Bitkit/Services/NodeEventListener.swift
+++ b/Bitkit/Services/NodeEventListener.swift
@@ -21,7 +21,15 @@ final class NodeEventListener {
             startWaiter = nil
             waiter?.resume()
         }
-        try await ServiceQueue.background(.ldk) { try node.start() }
+        try await ServiceQueue.background(.ldk) {
+            #if DEBUG && !BITKIT_NOTIFICATION_EXTENSION
+                V4Review.holdStartup()
+            #endif
+            try node.start()
+            #if DEBUG && !BITKIT_NOTIFICATION_EXTENSION
+                V4Review.log("NATIVE START returned")
+            #endif
+        }
         listen(node: node, onEvent: onEvent)
     }
 
diff --git a/Bitkit/Utilities/AppReset.swift b/Bitkit/Utilities/AppReset.swift
index 838ad337..601c2ff8 100644
--- a/Bitkit/Utilities/AppReset.swift
+++ b/Bitkit/Utilities/AppReset.swift
@@ -79,6 +79,9 @@ final class AppReset: WipeStepExecutor {
     }
 
     func run(_ step: WipeStep) async throws {
+        #if DEBUG
+            if Env.isE2E, !Env.isUnitTest { V4Review.log("WIPE STEP \(step.rawValue)") }
+        #endif
         switch step {
         case .quiesce:
             await BackupService.shared.stopObservingBackups()
diff --git a/Bitkit/Utilities/V4Review.swift b/Bitkit/Utilities/V4Review.swift
new file mode 100644
index 00000000..70923cd5
--- /dev/null
+++ b/Bitkit/Utilities/V4Review.swift
@@ -0,0 +1,34 @@
+#if DEBUG
+import Foundation
+
+enum V4Review {
+    static var directory: URL { FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0] }
+    static func log(_ value: String) {
+        let target = directory.appendingPathComponent("v4-review.log")
+        if !FileManager.default.fileExists(atPath: target.path) { _ = FileManager.default.createFile(atPath: target.path, contents: nil) }
+        if let handle = try? FileHandle(forWritingTo: target) {
+            defer { try? handle.close() }
+            _ = try? handle.seekToEnd()
+            try? handle.write(contentsOf: Data("\(Date().timeIntervalSince1970) \(value)\n".utf8))
+        }
+    }
+    static func holdStartup() {
+        guard Env.isE2E, !Env.isUnitTest else { return }
+        let flag = directory.appendingPathComponent("v4-hold-start")
+        guard FileManager.default.fileExists(atPath: flag.path) else { return }
+        log("HOLD START on synchronous LDK queue")
+        for _ in 0..<12000 {
+            if !FileManager.default.fileExists(atPath: flag.path) { log("RELEASE START"); return }
+            Thread.sleep(forTimeInterval: 0.1)
+        }
+        log("HOLD START expired")
+    }
+    @MainActor static func snapshot(_ wallet: WalletViewModel) {
+        func present(_ key: KeychainEntryType) -> String {
+            do { return try Keychain.load(key: key) == nil ? "absent" : "present" }
+            catch { return "unreadable" }
+        }
+        log("STATE node=\(wallet.nodeLifecycleState) busy=\(ConnectionRecoveryService.shared.isBusy) gate=\(WipeCoordinator.shared.gate) progress=\(WipeCoordinator.shared.progress) mnemonic=\(present(.bip39Mnemonic(index: 0))) wipe=\(present(.wipePending))")
+    }
+}
+#endif

Reviewed by gpt-6.1-sol-high

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Follow-up at 154326af, covering the entire delta since 4d265af6 and affected callers, with unchanged coverage inherited from the completed baseline. Base and merge base remain 003ea7e8.

Code review completed with unverified earlier concerns.

Source analysis covered timeout ownership/retry safety and event acknowledgement isolation with stop/delivery ordering, including LDK 1976754d. Android parity and dependency #1123 were checked only for affected contracts.

Validation: git diff --check passed. Both added concurrency regressions were inspected, not executed; Swift/Xcode are unavailable here. Current-head native CI was still running when checked. Device testing: not performed in this review.

Suggested additional test cases

  • iOS, disposable regtest wallet with a running native node: hold an actual VSS event-queue acknowledgement, request a status read and stop, then release persistence. Expect the read to complete during the hold, event delivery to finish once, and native stop to follow delivery.

Earlier findings reconciled at this revision: 2 still present, 5 verified fixed, 1 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 — Literal Manual 3 reconnect acceptance remains unverified: This is a retained execution-evidence limitation, not a newly discovered code defect. Earlier reviews establish offline behavior but do not establish a satisfied-path callback and coordinator restart after real transport returns. The current PR description retains that gap. Source traces the intended route but cannot prove the physical transition; this run is code-only and devices are prohibited. Disclose this limitation without claiming a reconnect pass, approval or device readiness.

Existing concerns

Reviewer model: gpt-6.1-sol
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.

Delta review of 154326aff (bound wipe drain waits, isolated event acknowledgements), plus the parent merges since my last approval. No findings.

Checked:

  • Timeout in beginWipe (ConnectionRecoveryService.swift:139-153, AppReset.swift:64-75):
    • On timeout, activeID, cancelActive, drainActive and isBusy stay set until the original task's defer runs. perform and editor() keep refusing, and start admission stays closed.
    • withConnectionRecoveryPaused throws before any wipe step runs. runAttempt records .failed(.quiesce), the gate stays .wiping and a retry is scheduled. No partial wipe, and the gate is never dropped.
    • The hung startup later fails validate() on the rotated generation and returns without surfacing an error.
  • Event acknowledgement on .ldkEvents:
    • nextEvent()/eventHandled() have one serial consumer (NodeEventListener.swift:58/60), so an ack can't land on another event.
    • stop awaits the in-flight ack and the onEvent delivery before node.stop(). wipeStorage refuses while the node exists, so a wipe can't drop an acknowledged but undelivered event.
  • Merges:
    • d8fa381's conflict resolutions combine both sides: AppScene gate guards, eventListener.stop(node:), AppReset step executor + recovery pause, and the awaited onEvent callback.
    • 34925fb changes test fixtures only.
    • The other merges are clean automerges.

Performance: the commit adds a bounded 30 s wait on Reset when startup is stuck in a native call. Only a hung native call can trigger that path, and it can't be set up from the UI. testAppResetTimesOutAndRetriesWhileOriginalStartupStillOwnsAdmission covers it.

Device gate: no device check needed for this delta (no UI or journey change; the timeout path is unreachable from the UI).
Skipped — covered by CI at 154326aff: unit tests, integration tests, e2e (all green).

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ⚠️ Not verified — real network reconnect remains unrun, and the initial native sweep retains one unresolved failure.

Findings: I found no new actionable defect in the 154326a..8eadf2b delta. The independent source review agrees. The earlier recovery-drain and event-worker fixes remain intact; the merged metadata scheduler preserves observation generations and restore/wipe admission.

QA for 8eadf2b: Xcode 27.0, iPhone 17 Pro simulator on iOS 26.5, disposable unfunded regtest wallet, committed native dependency pins.

Node reads and stop during a real VSS acknowledgement stall ✅ A native status read completed during the held acknowledgement; release allowed one delivery before native stop, verified in boundary traces.

This drives the actual-VSS case requested in the previous QA review. A fresh wallet reached Home with its native node running. A local relay delayed the selected encrypted VSS write for a genuine syncCompleted event while forwarding other writes. The ordinary .ldk queue's node.status() returned running=true during the approximately 3.7-second acknowledgement hold. Stop remained pending until release, then acknowledged delivery finished exactly once before node.stop() ran.

selected-ack-2x.2x.mp4

Installed app at 2× from a 10 fps capture. The video shows Home; the timing and native ordering are checked in the trace below, rather than inferred from the screen.

ACK_ENTER kind=syncCompleted
STATUS_REQUEST
STATUS_RETURN running=true
STOP_REQUEST
[selected VSS write released; HTTP 200]
ACK_EXIT
DELIVERY_ENTER
DELIVERY_EXIT
NATIVE_STOP_ENTER
NATIVE_STOP_EXIT
STOP_RETURN

An earlier broader delay also held native metrics writes. Status then waited inside the pinned Rust metrics lock until release; stop still followed delivery. That broader interruption does not establish general status availability during a VSS outage. The isolated event-write case above leaves those other writes flowing.

The app used unpushed Debug controls to arm the relay, request production status/stop APIs, pause ordinary app backup observation and log boundaries. Native events, authentication, encrypted payloads, queue decisions, delivery and native stop were unchanged. The installed controlled executable matched its build, SHA-256 88e20429c5cbc63e3c720e25ba03245460e0628ef2bbae5a769952a0d1f84ead.

Check: backup recovery and merged payment regressions ❌ The initial native sweep failed the Contacts recovery rendering case; its isolated repeat passed without a source change.

The unmodified head built and ran the workflow-selected BitkitTests suite. ProfileDestinationViewTests.testContactsWaitsForIdentityLookupThenShowsRecovery failed both Retry/Disconnect assertions: its retained snapshot still shows the loading spinner. The test takes that snapshot after a fixed 100 ms delay following release of its controlled restoration gate. One isolated test-without-building repeat on that same binary passed. The initial failure remains unresolved; I have not established a new product defect or attributed it to this delta.

Original failing test-host snapshot: loading remained visible where the test expected recovery controls. Direct recording of this native sweep was unavailable; the rendered attachment and finalized XCTest results are retained.

The metadata retirement/stop regression, connection-recovery ownership and timeout cases, held-acknowledgement regression and merged contact/scan/payment checks passed in that initial sweep. One opt-in native USDT protocol case skipped because its fixture was absent; four workflow integration classes and the full UI suite were not selected. Optional post-test simulator diagnostic collection was interrupted after XCTest completed; complete simulator diagnostics are not claimed. The result retains QoS and SwiftUI State warnings.

SwiftFormat 0.64.4 passed the changed Swift files. Translation validation completed with zero errors and 17 warnings; git diff --check passed.

New wallet resumes after a real network loss ⏭️ A device-scoped offline-to-online path transition could not be produced.

Manual 3 remains unrun. Fresh simulator Settings exposes neither Wi-Fi nor airplane mode, both settings URLs returned code 115, the edge gesture left Settings unchanged, and the available simulator commands expose no transport switch. No disposable physical iPhone was available. Interrupting an app endpoint leaves NWPathMonitor satisfied and cannot prove reconnect.

reconnect-2x.2x.mp4

Available Settings at 2×. This records setup investigation; no network-loss/reconnect pass is claimed.

Passing reports retained for unchanged behavior:

Coverage: the contiguous 154326a..8eadf2b delta, all 47 changed paths, earlier discussions and reached restore/start/wipe, metadata, private-contact and payment-confirmation callers. Fresh app coverage drives native event acknowledgement and stop ordering. The merged payment journeys rely on the linked reports; localization and journey text received source checks. Full populated-wallet restoration, private-link fault injection, a no-Noise peer and the narrow funding-switch race were not newly driven. The existing LOW follow-ups remain deferred:

Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes, merged, retains the restricted editor and durable-correction contract. iOS imports required state before rebuilding startup; Android can need a running node during restoration. No Android device run was repeated.


Reviewed by gpt-6.1-sol-high

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: full reassessment of the complete PR at 8eadf2b9 against 675f8bd6, following the comparison-base change; no prior coverage was inherited. Android c747cc0b and dependency #1123 were checked only for affected recovery and backup contracts.

Code review completed with unverified earlier concerns.

Validation: translation validation and diff whitespace checks passed. A Linux Swift harness passed 21 recovery-service and eight event-listener tests using the pinned implementation with explicit platform/SDK doubles; this does not validate iOS rendering, Keychain or native LDK execution. GitHub reports successful native test and formatter checks for this head; their logs were not independently audited here. The iOS build and authenticated recovery journeys were not run in this environment.

Suggested additional test cases

  • iOS, disposable regtest wallet with a controllable VSS backend: hold acknowledgement of a queued on-chain event, request an Electrum server change, then release the acknowledgement. Verify the event is delivered once before node replacement and no duplicate activity appears after foregrounding.

Device testing: not performed in this review.

Earlier findings reconciled at this revision: 2 still present, 3 verified fixed, 2 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 — Literal Manual 3 reconnect acceptance remains unverified: This is an execution-evidence limitation, not a new code defect. Source connects NWPathMonitor's satisfied callback to coordinator restart, but cannot establish a real offline-to-online transition. The current-head external review and PR description explicitly retain the unrun reconnect case. Historical real-network runs established offline waiting and reported a missed return callback; controlled-monitor passes establish only the application branch. Devices are prohibited in this run, so literal acceptance cannot be settled. Retain the limitation without claiming a reconnect pass, approval or device readiness.

Could not verify earlier concern — Current-head Contacts recovery rendering failure remains unexplained: The current-head external native sweep reports missing Retry and Disconnect controls with a retained loading-spinner snapshot; an unchanged isolated repeat passed on the same binary. The test uses a fixed 100 ms wait before inspecting rendered labels, but that alone does not establish test timing or product behavior as the cause. Neither the repeat nor successful author CI clears the initial failure. Source confirms the reported test and assertions; this Linux code-only run cannot settle UIKit/SwiftUI rendering or independently audit that XCTest result. Retain the attributed failed execution as a limitation without promoting a new PR defect or claiming a fix.

Existing concerns

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

@ben-kaufman
ben-kaufman requested a review from jvsena42 November 6, 2026 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants