Skip to content

fix: defer registry sync during profile deletion - #979

Open
ben-kaufman wants to merge 3 commits into
masterfrom
codex/ios-profile-cleanup-registry-20261016
Open

ben-kaufman wants to merge 3 commits into
masterfrom
codex/ios-profile-cleanup-registry-20261016

Conversation

@ben-kaufman

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

Copy link
Copy Markdown
Contributor

Closes #980

Related to synonymdev/bitkit-android#1522 and pubky/paykit-rs#197. This is a partial improvement, not a complete fix for deletion latency.

Twin: synonymdev/bitkit-android#1538

This PR defers an intermediate App Registry update until final profile cleanup.

Description

  • Withdraw private payment lists as before, then let the existing final public cleanup remove public endpoints and synchronize the disabled capability.
  • Retain the existing public cleanup marker so interrupted or failed deletion leaves registry reconciliation pending; ordinary disconnect and sharing changes still synchronize immediately.
  • Check deletion and cancellation again after private reconciliation, so foreground retry cannot consume the deferred public marker while deletion is active.
  • Keep identity, authorization, profile-revision, cancellation, private-withdrawal and uncertain-write safety checks unchanged. This PR does not change Paykit SDK, bindings, dependencies or storage formats relative to current master.

Out of Scope

  • Paykit SDK request-count optimization, Homeserver changes and safety-wait reductions.
  • Deletion progress UI, a fixed completion-time target and the broader latency issue.
  • Broader automatic-work scheduling changes, now present in master; this PR remains limited to registry deferral and its reconciliation guard.

Design

N/A - no UI changes.

Preview

N/A - no visual changes.

QA Notes

Journeys

  • updated delete-profile.xml - record zero-contact deletion time, then repeat the existing 62-contact cases; progress remains visible and reopening Profile does not restore the deleted profile.

Manual Tests

  • Interrupt deletion after private withdrawal and before profile removal, then restart the same identity: cleanup remains pending and normal retry reconciles current sharing state; failed retry retains the marker. Controlled operation/network gates are not in Capabilities.
  • Hold withdrawal delivery, cancel cleanup or replace the identity during cleanup: undelivered or stale work is not acknowledged. Disconnect and sharing-disable flows retain immediate reconciliation. Controlled delivery/session gates are not in Capabilities.
  • Hold the private reconciliation step, begin deletion, then release it: no new public reconciliation starts and the repair marker remains pending for a later retry. Controlled operation gates are not in Capabilities.

Automated Checks

  • added PrivatePaykitServiceTests.swift - deferred versus normal registry synchronization and persistence of the existing reconciliation marker across service reconstruction.
  • added ContactPaymentsServiceTests.swift - deletion or cancellation before public reconciliation preserves the marker, and retry after deletion can complete it.
  • updated PrivatePaykitServiceTests.swift - partial withdrawal, delivery/cancellation/failure handling and membership/generation invalidation in both cleanup modes.

Initial implementation validation on the branch based on master afdc534: 264 focused tests and 2,620 native tests passed, with zero failures/skips using the same six CI live/integration-suite exclusions. Independent xcresult inventories confirmed all passes. Full formatting retains exactly the same 57 source/message findings as master, with none introduced; translation output is byte-identical at zero errors/warnings. Complete compiler diagnostics match the verified baseline. Two tests added, none removed, and three existing tables expanded. Independent correctness, repository-native and polish review is clean after the bounded reconciliation correction.

The stored-marker tests reconstruct service fixtures; they are not actual process-restart or complete device identity-replacement tests. Author-performed validation did not establish forced interruption, failed retry, complete nonzero-contact/62-contact acceptance, physical-device or mainnet behavior. Later attributed reviewer evidence is recorded below, with its remaining limits. This maintenance pass sent no funds and shortened no safety wait.

Measurement evidence before the final reconciliation-overlap guard: two baseline deletion-handler samples were 26.218/26.239 seconds and two candidate samples were 23.255/22.968 seconds, a 3.117-second mean reduction in this small zero-contact fixture. The skipped intermediate registry wrapper took 3.448/3.070 seconds in the baseline. These are timing-instrumented simulator samples on the same preserved, unfunded identity, not exact tap-to-onboarding measurements, final-head acceptance or a general speedup guarantee. UI reached onboarding after all four flows; one background backup identity-change rejection occurred during signout in every baseline and candidate window. No SDK response or wait was altered. The larger SDK request cost remains.

Integration note: the authorized master update incorporates the scheduling work now on master. The AppScene resolution retains the original/current-session checks, guarded publication and branch-specific acknowledgement together with this PR's private/public phase boundary and post-private deletion/cancellation guard. Both sets of journey procedures are preserved. Final merged-branch validation and its limits are recorded below.

Fixture Isolation Follow-Up (65ca432)

The cleanup table now clears its persisted contact-cache fixture before each case, while the existing teardown restores the original host defaults. Temporary seeded-cache red/green validation reproduced the original assertion failure and verified the reset; the seed was removed from the final one-line patch. Final validation: 264 focused and 2,620 native tests passed, independently reconciled against xcresult, with exactly the six existing CI live/integration exclusions and no added skips. No tests or assertions were removed. Full SwiftFormat reports the same 57 source/message findings as current master e96c4a7, none introduced; translation output matches master with zero errors/warnings. Compiler diagnostics match the verified 77-warning reference, whose 22 source files are unchanged on current master; this reuses that reference, not a fresh full master compiler run. A bounded independent correctness/native/polish review found no further issue. Production files, dependencies and safety waits are unchanged. Hosted checks for that pre-merge head completed; the qualified historical results are recorded below.

Prior-Head CI (65ca432, October 17)

All required contexts now pass on this head: unit/UI, integration and E2E status. The build and translation checks also pass. This is evidence for 65ca432, not the subsequent master-update commit or inherited evidence from b948a46.

The actual E2E job passed after its existing internal retry. On the hosted iPhone 17/iOS 26.0.1 runner, the first attempt passed 13 tests and failed hardware-wallet transfer-to-spending while waiting 30 seconds for BackupIntroViewDescription (dismissBackupTimedSheet). The second attempt passed that test and skipped the 13 already completed tests. The initial failure remains in the log and annotation; the passing retry does not establish its root cause or a fix. No additional retry was dispatched here. The workflow skipped its failure-only diagnostic artifact upload after the job recovered, so the first-attempt screenshot/video were not available in the run's uploaded artifacts.

The complete unit/UI and integration logs report 2,625 native passes, 39 UI passes with four skips, and 34/34 integration tests on the first internal attempt. These hosted counts differ from the local inventory above. Compiler/linker warnings remain, and both jobs logged post-test cache-save disk-space errors after their test artifacts uploaded; their final job conclusions are success. No new clean-lint or full baseline-equality claim is made from these logs. This CI result does not complete the manual deletion, interruption, identity, 62-contact or performance checkboxes, and does not fix the separate known failures on #954.

Additional Reviewer Evidence (Historical b948a469)

Ovi's review reports the following on two iPhone 17 Pro/iOS 26.5 staging simulators, using disclosed uncommitted logging and hold hooks. These results were not independently replicated here, and the linked videos were not inspected by this maintenance pass.

  • Deletion of 62 saved Pubky-only profiles reached onboarding in about 16 seconds with one final privatePayments=false App Registry publication. The contacts were added through bitkit://contact, not Import All, and were not linked private-payment contacts. This does not establish the immediate-after-import or 62-active-private-contact cases.
  • Termination after private withdrawal preserved the public repair marker and profile through relaunch. Sharing-on reconciliation retained the marker through two shared_state_busy failures, then republished and cleared it. A subsequent deletion finished with one final disabled-capability publication.
  • A foreground retry held after its private phase skipped public reconciliation once deletion began, leaving the marker until final cleanup. A separate foreground endpoint refresh nevertheless published privatePayments=true during deletion; the final false publication followed. That separate path is covered by the scheduling work in fix: pause paykit work during profile deletion #954, not this PR, and the combined branches were not tested in that reviewer campaign.
  • Sharing-off and individual contact deletion retained immediate registry synchronization. Disconnect was not driven. A separate linked-contact deletion reported successful private withdrawal delivery before deferral and one final disabled-capability publication, reaching onboarding in about 22 seconds.

The approximately 11/16/22-second samples are not matched before/after benchmarks, maximum-latency guarantees, or evidence for shortening any safety wait. No uncertain-write wait was observed in those runs. Held/undelivered withdrawal, cancellation and identity replacement were not device-tested; neither was the PubkyProfileManager.deleteProfile default cleanup entry point, because the driven UI path passes cleanPrivatePaykitEndpoints: false. Zero-contact journey evidence remains jvsena's historical b948a469 review, not a new run. Compound manual checkboxes remain open for the untested portions.

Reviewer Follow-Up (65ca432, October 17)

Ovi's follow-up confirms the fixture-isolation fix and reports no new findings. Its formal state is COMMENTED, not approval. The following additional cases used two iPhone 17 Pro/iOS 26.5 staging simulators and uncommitted logging/hold hooks; Test 8 also injected a profile-delete request failure. These results were not independently replicated here, and the videos were not inspected.

  • A polling reconciliation held after its private phase was cancelled by backgrounding. It skipped public reconciliation and retained the marker; the next foreground reconciliation republished with sharing on and cleared it. This adds device evidence for the cancellation half of the post-private guard, distinct from the earlier deletion-state case.
  • An injected profile-delete failure after a reported delivered withdrawal left repair pending without an intermediate registry publication. About eight seconds later, periodic reconciliation republished privatePayments=true with sharing still on and cleared the marker. Disconnect then synchronized immediately through its normal private cleanup, followed by final public cleanup publishing false and profile onboarding. The intermediate true on Disconnect is not a one-publication full-deletion result or a timing guarantee.
  • Stopping a fully linked peer did not prevent withdrawal delivery in the first trial. A second trial with a peer that had not answered the link left withdrawal pending after the drain: the deferred branch was not reached and both cleanup markers remained. Deletion continued to onboarding, but final disabled-capability publication was rejected with policy_error (app-owned work still active). After sign-out, reconciliation lacked a Pubky session, leaving the last registry capability true. This unresolved limitation is tracked in #984; it is not successful final registry cleanup. Source comparison confirms the withdrawal error precedes this PR's deferral branch and the sign-out/public-cleanup path is unchanged, but this maintenance pass did not reproduce the runtime failure on master or establish its root cause.

Immediate-after-import 62-contact deletion, identity replacement during deletion, failed-deletion Retry racing reconciliation, and the default PubkyProfileManager.deleteProfile cleanup entry point were not driven. Zero-contact journey evidence remains historical b948a46, not a new run. No combined #954/#979 branch was tested in that reviewer campaign, no safety wait was shortened, and the compound manual checkboxes remain open.

Signed Master Update (69ae1c1, October 17)

With Ben's authorization, merged actual master 5009f490 into this PR's existing 65ca4329 head, preserving published history. The AppScene and journey-document conflicts are resolved with both behaviors retained: original/current-session checks, guarded publication and branch-specific acknowledgement remain alongside separate private/public reconciliation phases and the post-private deletion/cancellation guard. Both sets of manual procedures remain. No unrelated fix, new dependency choice, assertion removal or safety-wait change was made.

Final local validation of the merged tree: 327 focused tests and 2,662 native tests passed, independently reconciled against xcresult. The full native lane retained exactly the six existing CI live/integration-suite exclusions, with no additional skips. A fresh exact-master build and 29 cleanup tests also passed. Complete compiler diagnostics from both rebuilds contain the same 77 source/message warnings (102 raw compiler-warning lines), with no introduced finding; the final incremental native build emitted none. Full SwiftFormat still exits nonzero for exactly the same 57 source/message findings as exact master, none introduced. Translations pass with identical output and zero errors; diff whitespace checks pass. The native log also retains error-path fixture messages, mock-order invalid-UUID read errors and simulator haptics diagnostics; these are not a claim of warning-free runtime or full-base runtime equivalence.

One fresh independent correctness, repository-native and production-polish review found no actionable issue on the exact final diff. All three commits in the complete PR range verify locally and on GitHub (verified=true, reason=valid). The branch update is pushed; the PR itself has not been merged.

Hosted CI for 69ae1c17 has completed; the qualified results are recorded below. The prior 65ca4329 results remain historical. This pass did not run local UI/live integration, fault-injection journeys, checkout/payment campaigns, actual restart or identity-replacement cases, or new zero/62-contact performance measurements. It does not resolve the incomplete final-cleanup limitation in #984 or complete any manual checkbox.

Current-Head CI and Review (69ae1c1, October 17)

All required contexts now pass on 69ae1c17: unit/UI, integration and E2E status. Build and translations also pass. The actual E2E job passed all 14 tests on its first internal attempt on hosted iPhone 17/iOS 26.0.1; attempts two and three were skipped. No retry was dispatched here. This gate did not exercise the profile-deletion manual checklist.

The complete hosted logs report 2,662 native passes, 39 UI passes with four skips, and 34/34 integration tests on the first internal attempt. Test artifacts uploaded; integration's subsequent cache save logged a disk-space failure while the job remained successful. Compiler/linker diagnostics remain, including duplicate symbols. The E2E log retains stale-element warnings, a restarted simulator log stream and dependency deprecation/audit output (88 vulnerabilities); this is not a clean dependency or complete hosted-baseline audit. Failure-only E2E artifact upload was skipped; no new video or screenshot evidence was inspected here. These passing runs do not establish fixes or base reproductions for historical navigation, XCTest-launch, refund or E2E failures.

Piotr's source review and Ovi's controlled-simulator review report no actionable code findings. Both are COMMENTED, not approvals. Piotr did not run device tests. Ovi used three iPhone 17 Pro/iOS 26.5 staging simulators with disclosed uncommitted logging/hold hooks and an injected App Registry failure. The following is attributed reviewer evidence, not author replication; linked videos were not inspected.

  • Termination after withdrawal retained the profile and repair marker; normal sharing-on reconciliation later republished and acknowledged it. Injected registry failures on relaunch and polling retained repair until a later successful publication.
  • Background cancellation after private reconciliation skipped public work and retained the marker. Starting deletion while reconciliation was held also skipped its public phase; the successful deletion then published the disabled capability once and reached onboarding. A separate foreground refresh was refused while deletion was held. Network-restoration behavior was not driven.
  • A failed-deletion Retry overlapping held reconciliation exercised the post-private deletion guard, but final disabled-capability publication was refused with policy_error (app-owned work still active), leaving repair pending despite onboarding. The first attempt had an undelivered withdrawal. This is still incomplete final cleanup, tracked in #984, not a successful final-registry acceptance result or a newly established root cause.
  • Delivered linked-contact withdrawal preceded deferral in successful cases; an undelivered withdrawal did not reach the deferred branch and retained both markers. Sharing-off, individual contact deletion and Disconnect retained immediate synchronization.
  • Zero-contact deletion and deletion with 62 saved Pubky-only contacts reached onboarding with one final disabled-capability publication, blocked controls and no old profile after reopening. The approximate 10/14-second samples are not matched benchmarks or latency guarantees. The 62 contacts were added through bitkit://contact, not immediate Import All, and were not linked private-payment contacts; no uncertain-write wait was observed.

Immediate-after-Import-All 62-contact behavior, cancellation of cleanup or identity replacement during deletion, and the default PubkyProfileManager.deleteProfile private-cleanup entry point remain untested in this campaign. The UI path performs its own cleanup and calls that entry point with cleanPrivatePaykitEndpoints: false. Compound manual checkboxes remain unchecked; this evidence does not establish general performance, complete Shop/payment/linking acceptance, physical-device or mainnet behavior.

@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Profile deletion skips the intermediate App Registry update and leaves the existing repair marker pending until final cleanup.

  • Profile deletion withdraws private lists before updating the App Registry.
  • Foreground retries pause public reconciliation during deletion or cancellation.

Scope acknowledged by ben-kaufman: this is a partial latency improvement. Full interruption, device, and process-restart checks remain unverified. No tests were run during this review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Start profile deletion] --> B[Withdraw private payment lists]
    B --> C[Keep registry repair pending]
    C --> D{Profile deletion succeeds?}
    D -->|Yes| E[Disable sharing]
    E --> F[Remove public endpoints and sync registry]
    F --> G{Cleanup succeeds?}
    G -->|Yes| H[Clear repair marker]
    G -->|No| I[Keep repair pending]
    D -->|No| I
    I --> J[Later retry uses current sharing settings]
Loading

Reviews (1) · Last reviewed commit: "fix: defer registry sync during profile ..." · Reviewed by Greptile

@talosmachina talosmachina left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No findings. Defers the intermediate Paykit app registry sync during profile deletion to the final public cleanup, keeps publicPaykitCleanupPending set so an interrupted deletion still reconciles, and re-checks deletion/cancellation between the private and public phases of foreground reconciliation. Reviewed b948a46, full tier, reasoned from the code and CI (iOS does not build on this reviewer's host).

What I checked, and 4 candidates I ruled out

Read in full: PrivatePaykitService+Contacts.swift (removePublishedEndpoints, removePublishedEndpointsLocked, retryPendingEndpointReconciliation, beginProfileDeletion/endProfileDeletion), ContactPaymentsService.reconcilePendingEndpoints, PubkyProfileManager deletion and sign-out paths, EditProfileView delete flow, AppScene.retryPendingPaykitEndpointRemoval.
Call sites traced: every removePrivatePaykitEndpoints* caller (only the two deletion paths pass deferAppSync: true; AppReset.wipe and signOut keep the immediate sync), every reconcilePendingEndpoints caller (one production caller, AppScene).
CI: validate and Greptile green; Run Tests, Run Integration Tests and build-local still pending at review time.

Ruled out

  • Registry left stale after a successful deletion: deleteProfile reaches signOut, whose removePublicPaykitEndpointsBestEffort calls syncPaykitApp(privateSharingEnabled: false) and clears the marker on success, so the skipped sync is covered.
  • Registry left stale when deletion fails after private withdrawal: the deferred branch sets publicPaykitCleanupPending, and the skipped sync would have read the still-true PrivatePaykitService.publishingEnabledKey anyway, so nothing the old intermediate call published is lost; the next foreground retry reconciles.
  • Foreground retry consuming the marker mid-deletion: the new post-private guard reads the actor's isDeletingProfile before the public closure runs, and retryPendingEndpointReconciliation already returns early during deletion.
  • Behaviour change for ordinary disconnect or contact removal: deferAppSync defaults to false on every overload and no other caller sets it.

Merge confidence: 4/5, no findings, but the unit and integration jobs had not finished and the branch was not built here.

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Full review of the complete PR diff against merge base afdc534, at b948a46. No saved assessment baseline was available.

No new actionable code findings.

Profile deletion withdraws private payment lists, leaves publicPaykitCleanupPending set, and leaves the intermediate App Registry update to final public cleanup. Sign-out still removes public endpoints and synchronizes the registry with private sharing disabled, clearing that marker only after success. Failure, cancellation, or an interruption before that sync keeps the marker. Foreground reconciliation finishes its private retry, then skips public reconciliation while profile deletion is active or the task is cancelled, so that pass cannot acknowledge the deferred marker. Disconnect, ordinary contact deletion, and sharing changes still synchronize immediately.

Android twin bitkit-android#1538 at d689900 keeps the private cleanup marker pending until registry sync. iOS clears that private marker after a successful withdrawal and persists the public repair marker. Both defer the intermediate registry update and keep a durable pending signal. iOS retry follows the current sharing preference, matching the contract stated for this PR.

Validation: Suites were not executed in this review. Run Integration Tests passed on this head. Run Tests and build-local were still pending. Device testing: not performed in this review.

Recommended before device testing: Wait for Run Tests and build-local on this head, then run the updated delete-profile journey and the PR's interruption, cancellation, and overlap checks. Those fault cases need operation or network gates the journey runner does not provide.

@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 at b948a46: no findings. I reviewed it as a Paykit endpoint-cleanup change.

Checked:

  • The skipped intermediate syncPaykitApp() published privatePayments = true while publishingEnabledKey was still true. The final false publish in removePublicPaykitEndpointsBestEffort (PubkyProfileManager.swift:1389-1395) still runs before signOut revokes the session, and it clears the marker on success.
  • A deferred sync cannot acknowledge an undelivered withdrawal: firstError is thrown at PrivatePaykitService+Contacts.swift:470-472 before the deferred branch at :473.
  • The new guard at ContactPaymentsService.swift:255 stops foreground reconciliation between the private and public phases once deletion begins. Tests cover deletion before, deletion after the private phase, and cancel after the private phase.
  • The other callers (AppReset.wipe, signOut, contact removal) keep deferAppSync = false.

Seen but outside this PR: after a failed first deletion, the deferred branch sets the public marker while publishingEnabledKey is still true. A reconcile admitted before Retry calls beginProfileDeletion() can therefore republish public endpoints between removePublishedEndpoints() and signOut(). The PR's README acknowledges this, and #954 session checks close it.

Performance-relevant: it removes one publishPaykitApp round trip (3.1–3.4 s measured) from every full deletion.

Device gate (partial): b948a46 — not run yet. Pending: delete-profile.xml with one app publish per deletion, and the profile not returning on reopen. The device result will follow.

jvsena42
jvsena42 previously approved these changes Oct 16, 2026

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving at b948a46. The code scan from my earlier review stands, with no findings.

Device gate: b948a46 — delete-profile.xml: 7/7 actions pass on the iPhone 17 simulator (iOS 26.5), with a profile created through Bitkit, Paykit on and 0 contacts.

  • The overlay with spinner reported zero tappable targets while pending, and onboarding showed when deletion finished.
  • The profile does not return on reopen or after a force-stop and relaunch. The cleanup-pending flags and sharesPrivatePaykitEndpoints are false afterwards, and nothing publishes on relaunch.
  • Exactly one Paykit app publish during deletion: the final privatePayments='false' one, 18:51:32.951 → 18:51:40.050. There was no intermediate publish. The app logs no publish success, so the publishes were counted with temporary log lines in a scratch build only.

perf: delete profile 28.4 s at 0 contacts (19.1 s before the final publish, 7.1 s publish, 2.2 s sign-out). Earlier runs measured 30–31 s on this setup. No uncertain-write wait. l10n: no UI text changed.

Two pre-existing warnings appear at sign-out: Backup failed … identityChanged and "Failed to refresh incoming Paykit payment requests".

⚠️ Not verified: the 62-contact variant, and the failed-deletion-then-Retry overlap from my earlier comment.

@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: 💬 Comment: one LOW in a new test, and Test 5 is not run because the app has no route to stage it.

Findings:

  • LOW BitkitTests/PrivatePaykitServiceTests.swift:2146: testDeferredCleanupLeavesDurableRegistryReconciliation depends on the host wallet's cached Paykit contacts.
    • It runs a full cleanup, so the cleanup keys include every cached contact with a published private list.
    • PrivatePaykitService() loads that cache from the app's real UserDefaults.standard, and the test bundle runs inside the app.
    • On a simulator whose wallet has such a contact, clearPaymentLists gets that key as well and XCTAssertEqual(keys, [key]) fails. CI's fresh simulator passes (Run Tests).
    • Fix: remove PrivatePaykitService.cacheStateKey at the top of the loop, as testFullCleanupFindsRemotePeersAndRetainsFailedDiscovery does at line 2235.

QA for b948a46:
Tested on two iPhone 17 Pro simulators (iOS 26.5) against the staging homeserver.

  • The build is b948a46 plus a test hook, which is not pushed and is shown below.
  • The hook logs each App Registry publish, the deferred branch, the reconcile phases and the cleanup markers.
  • It holds deletion, or the foreground reconcile, only while a flag file exists.
Test 1: delete a profile with 62 contacts ✅ The profile and 62 contacts were deleted with one App Registry publish (privatePayments=false) at the end, per the hook log.
  • The wallet was restored from a staging identity that follows 62 profiles, and Bitkit created the profile on that key.
  • The 62 follows were saved through bitkit://contact, because Import All needs Pubky Ring, which the simulator lacks.
  • While deletion was pending, the spinner overlay blocked profile actions.
  • Onboarding showed about 16 s after confirming, and reopening Profile offers only Create.
  • In the log, the withdrawal set the cleanup marker instead of publishing. The final cleanup published once with privatePayments=false and cleared the marker. There was no uncertain-write wait.
T1-2x.2x.mp4
Test 2: interrupted deletion keeps the registry repair pending ✅ After an interruption the marker survived a restart, and the retry reconciled using the current sharing setting, per the hook log.
  • Deletion was held right after the private withdrawal. The marker went from false to true with no publish, and then the app was terminated.
  • After relaunch, the profile was still there and the marker was still set. The foreground retry chose to republish, because sharing was on.
  • The first two tries failed with shared_state_busy and kept the marker. The third published and cleared it.
  • Deleting again ended with one publish (privatePayments=false) and showed onboarding after about 11 s. A periodic retry that ran during that deletion skipped its public phase.
T2-2x.2x.mp4
Test 3: foreground retry held while deletion begins ✅ The held retry skipped its public phase once deletion had begun, and the marker stayed set until the deletion's final cleanup, per the hook log.
  • Setup: an interrupted deletion set the marker. On relaunch, the retry was held after its private step. Delete Profile was then started and held after its withdrawal.
  • When I released the retry, it saw the deletion and skipped its public phase, and the next periodic retry did the same.
  • When I released the deletion, one privatePayments=false publish cleared the marker and onboarding showed.
  • Outside this PR: right after the skipped retry, a foreground public-endpoint refresh published privatePayments=true mid-deletion. It runs after the retry and has no deletion check. The final false publish came after it. #954 gates the foreground endpoint refresh on its automatic Paykit session.
T3-2x.2x.mp4
Test 4: sharing changes and contact deletion still sync at once ✅ Turning contact payments off and deleting a contact each published to the App Registry immediately, without the deferred branch, per the hook log.
  • After turning contact payments off, the toggle stayed off and the app published privatePayments=false.
  • Deleting a saved contact published at once.
  • Disconnect was not driven: the app offers it only for a profile that failed to load or delete.
T4-2x.2x.mp4
Test 5: held withdrawal, cancelled cleanup or replaced identity ⏭️ Not run: the deletion overlay blocks every route to cancel cleanup or replace the identity, and an undelivered withdrawal needs a delivery fault.

This PR's unit tests cover these cases in both cleanup modes: delivery failure, cancellation, and re-add or identity change during cleanup.

Test 6: delete a profile with a linked contact ✅ The withdrawal reached the linked contact before the registry publish was deferred, and one privatePayments=false publish followed, per the hook log.
  • Two wallets saved each other as contacts until Request or Pay was offered.
  • Deleting one profile withdrew the private list from that contact, and the SDK reported it delivered with no failures. The marker was set after that.
  • Onboarding showed after about 22 s.
  • I added this case because the 62 fixture follows are Pubky-only profiles. This is the first deletion with a contact the wallet was linked to.
T6-2x.2x.mp4
Test hook (not pushed)

Tests 1-4 ran without the two withdrawal-report log lines, which Test 6 added.

diff --git a/Bitkit/AppScene.swift b/Bitkit/AppScene.swift
index eaf7e389..adbd10c0 100644
--- a/Bitkit/AppScene.swift
+++ b/Bitkit/AppScene.swift
@@ -1876,8 +1876,10 @@ struct AppScene: View {
             if privateCleanupPending, !UserDefaults.standard.bool(forKey: PrivatePaykitService.cleanupPendingKey) {
                 PublicPaykitService.setCleanupPending(true)
             }
+            await TestHook.hold("reconcile-private") // test hook
         }) {
             if PublicPaykitService.isCleanupPending {
+                TestHook.log("reconcile public mode=\(PublicPaykitService.pendingReconciliationMode())") // test hook
                 do {
                     switch PublicPaykitService.pendingReconciliationMode() {
                     case .publishEndpoints:
@@ -1887,6 +1889,7 @@ struct AppScene: View {
                         try await PublicPaykitService.syncPaykitApp()
                     }
                     PublicPaykitService.setCleanupPending(false)
+                    TestHook.log("reconcile public done; marker cleared") // test hook
                 } catch {
                     Logger.warn("Failed to reconcile public Paykit state: \(error)", context: "AppScene")
                 }
diff --git a/Bitkit/Managers/PubkyProfileManager.swift b/Bitkit/Managers/PubkyProfileManager.swift
index 625eb936..ab98022c 100644
--- a/Bitkit/Managers/PubkyProfileManager.swift
+++ b/Bitkit/Managers/PubkyProfileManager.swift
@@ -1390,8 +1390,10 @@ class PubkyProfileManager: ObservableObject {
         do {
             try await removePublicPaykitEndpoints(context: context)
             PublicPaykitService.setCleanupPending(false)
+            TestHook.log("final public cleanup done (\(context)); marker cleared") // test hook
         } catch {
             PublicPaykitService.setCleanupPending(true)
+            TestHook.log("final public cleanup failed (\(context)): \(error); marker kept") // test hook
         }
     }
 
diff --git a/Bitkit/Services/ContactPaymentsService.swift b/Bitkit/Services/ContactPaymentsService.swift
index a78f2077..40754d39 100644
--- a/Bitkit/Services/ContactPaymentsService.swift
+++ b/Bitkit/Services/ContactPaymentsService.swift
@@ -252,7 +252,13 @@ enum ContactPaymentsService {
         isOperationActive = true
         defer { releaseOperation() }
         await reconcilePrivate()
-        guard await !PrivatePaykitService.shared.isDeletingProfile, !Task.isCancelled else { return }
+        let hookDeleting = await PrivatePaykitService.shared.isDeletingProfile // test hook
+        TestHook.log("reconcile private phase done isDeletingProfile=\(hookDeleting) cancelled=\(Task.isCancelled)") // test hook
+        guard await !PrivatePaykitService.shared.isDeletingProfile, !Task.isCancelled else {
+            TestHook.log("reconcile public phase skipped") // test hook
+            return
+        }
+        TestHook.log("reconcile public phase start") // test hook
         await reconcile()
     }
 
diff --git a/Bitkit/Services/PrivatePaykitService+Contacts.swift b/Bitkit/Services/PrivatePaykitService+Contacts.swift
index bf212dd3..f2c3239e 100644
--- a/Bitkit/Services/PrivatePaykitService+Contacts.swift
+++ b/Bitkit/Services/PrivatePaykitService+Contacts.swift
@@ -407,7 +407,12 @@ extension PrivatePaykitService {
                 throw PubkyServiceError.sessionNotActive
             }
             do {
+                TestHook.log("withdrawal to \(cleanupKeys.count) contact(s) deferAppSync=\(deferAppSync)") // test hook
                 if let report = try await operations.clearPaymentLists(cleanupKeys) {
+                    TestHook.log( // test hook
+                        "withdrawal report cleared=\(report.cleared.count) queued=\(report.queued.count) " // test hook
+                            + "failedToQueue=\(report.failedToQueue.count) failedToDeliver=\(report.failedToDeliver.count)" // test hook
+                    ) // test hook
                     logPrivatePaymentListDeliveryFailures(report, reason: "cleanup")
                     failedPublicKeys.formUnion((report.failedToQueue.map(\.counterparty) + report.failedToDeliver.map(\.counterparty))
                         .compactMap { PubkyPublicKeyFormat.normalized($0) })
@@ -473,7 +478,9 @@ extension PrivatePaykitService {
         if deferAppSync {
             // Final profile cleanup synchronizes the registry; retain repair if deletion stops before it.
             PublicPaykitService.setCleanupPending(true)
+            TestHook.log("private withdrawal done; registry publish deferred, marker set") // test hook
         } else {
+            TestHook.log("private withdrawal done; registry publish now (not deferred)") // test hook
             try await operations.syncApp()
         }
     }
diff --git a/Bitkit/Services/PublicPaykitService.swift b/Bitkit/Services/PublicPaykitService.swift
index 2175c849..c8e3df02 100644
--- a/Bitkit/Services/PublicPaykitService.swift
+++ b/Bitkit/Services/PublicPaykitService.swift
@@ -394,7 +394,14 @@ enum PublicPaykitService {
         priority: PaykitSdkOperationLock.Priority = .ordered
     ) async throws {
         let privateSharing = privateSharingEnabled ?? UserDefaults.standard.bool(forKey: PrivatePaykitService.publishingEnabledKey)
-        try await PaykitSdkService.shared.syncPaykitApp(privatePaymentsEnabled: privateSharing, priority: priority)
+        TestHook.log("registry publish start privatePayments=\(privateSharing)") // test hook
+        do { // test hook
+            try await PaykitSdkService.shared.syncPaykitApp(privatePaymentsEnabled: privateSharing, priority: priority)
+        } catch { // test hook
+            TestHook.log("registry publish failed privatePayments=\(privateSharing): \(error)") // test hook
+            throw error // test hook
+        } // test hook
+        TestHook.log("registry publish done privatePayments=\(privateSharing)") // test hook
     }
 
     static func hasPayablePublicEndpoint(publicKey: String) async throws -> Bool {
@@ -621,3 +628,34 @@ enum PublicPaykitService {
         }
     }
 }
+
+enum TestHook { // test hook
+    private static var holdDirectory: URL { FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0] } // test hook
+
+    static func log(_ message: String) { // test hook
+        #if DEBUG // test hook
+            let publicPending = PublicPaykitService.isCleanupPending // test hook
+            let privatePending = UserDefaults.standard.bool(forKey: PrivatePaykitService.cleanupPendingKey) // test hook
+            let publicSharing = UserDefaults.standard.bool(forKey: PublicPaykitService.publishingEnabledKey) // test hook
+            let privateSharing = UserDefaults.standard.bool(forKey: PrivatePaykitService.publishingEnabledKey) // test hook
+            Logger.info( // test hook
+                "TestHook \(message) | publicPaykitCleanupPending=\(publicPending) privateCleanupPending=\(privatePending) " // test hook
+                    + "sharesPublic=\(publicSharing) sharesPrivate=\(privateSharing)", // test hook
+                context: "TestHook" // test hook
+            ) // test hook
+        #endif // test hook
+    } // test hook
+
+    /// Waits while `Documents/hold-<name>` exists; the tester arms it by creating the file and releases it by removing it.
+    static func hold(_ name: String) async { // test hook
+        #if DEBUG // test hook
+            let flag = holdDirectory.appendingPathComponent("hold-\(name)").path // test hook
+            guard FileManager.default.fileExists(atPath: flag) else { return } // test hook
+            log("hold \(name) active") // test hook
+            while FileManager.default.fileExists(atPath: flag), !Task.isCancelled { // test hook
+                try? await Task.sleep(nanoseconds: 250_000_000) // test hook
+            } // test hook
+            log("hold \(name) released cancelled=\(Task.isCancelled)") // test hook
+        #endif // test hook
+    } // test hook
+} // test hook
diff --git a/Bitkit/Views/Profile/EditProfileView.swift b/Bitkit/Views/Profile/EditProfileView.swift
index fd4f8158..4d0183f7 100644
--- a/Bitkit/Views/Profile/EditProfileView.swift
+++ b/Bitkit/Views/Profile/EditProfileView.swift
@@ -174,9 +174,14 @@ struct EditProfileView: View {
     }
 
     private func performDeleteProfile() async throws {
+        TestHook.log("delete start") // test hook
         await PubkyProfileManager.removePrivatePaykitEndpointsBestEffort(context: "EditProfileView.deleteProfile", deferAppSync: true)
+        TestHook.log("delete private withdrawal returned") // test hook
+        await TestHook.hold("delete-after-private") // test hook
         await contactsManager.deleteAllContactsBestEffort()
+        TestHook.log("delete contacts deleted") // test hook
         try await pubkyProfile.deleteProfile(cleanPrivatePaykitEndpoints: false)
+        TestHook.log("delete profile deleted and signed out") // test hook
         navigation.path = [app.hasSeenProfileIntro ? .pubkyChoice : .profileIntro]
     }
 

Not driven:

  • delete-profile.xml at 0 contacts: @jvsena42 passed it on device at b948a46 (review), so I did not repeat it.
  • The 62-contact variant run right after import, because Import All is not available on the simulator.

Full review of diff (10 files). No device case reached deferAppSync: true in PubkyProfileManager.deleteProfile (line 981), because its only caller passes cleanPrivatePaykitEndpoints: false.

Pair PR synonymdev/bitkit-android#1538 at d689900 also defers the intermediate registry update. Android keeps the private cleanup marker pending, while iOS sets the public one, as @piotr-iohk's review notes.

Comment thread BitkitTests/PrivatePaykitServiceTests.swift

@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: 💬 Comment: no findings and every driven case passed, but the 62-contact deletion right after import and identity replacement during deletion are not driven.

Findings: none. 65ca432 fixes our LOW: the deferred-cleanup test now clears the cached contact state before each case, so only its own key reaches clearPaymentLists.

QA for 65ca432:
Tests 1-4 and 6 already done in review at b948a46; 65ca432 changes only a test file.

Tested on two iOS 26.5 simulators (iPhone 17 Pro) against the staging homeserver.

  • The build is 65ca432 plus a test hook, which is not pushed and is shown below.
  • It adds to the earlier hook a hold for the polling task's reconcile, a hold that keeps other reconcile callers from starting, and a file-gated failure of the profile delete request (Test 8 only).
Test 7: a reconcile cancelled after its private step keeps the registry repair pending ✅ Moving the app to the background cancelled a held reconcile, which skipped its public phase and kept the marker; the next reconcile cleared it, per the hook log.
  • A deletion held after its private withdrawal set the marker without publishing, and the app was terminated. After relaunch the marker was still set and the profile was still there.
  • The launch reconcile was kept from starting, so the polling task's maintenance reconcile ran first. It was held after its private step.
  • Moving the app to the background cancelled it. It logged cancelled=true and isDeletingProfile=false, skipped its public phase and published nothing. The marker stayed set.
  • Back in the foreground, the next reconcile republished, because sharing was on, and cleared the marker.
  • This drives the Task.isCancelled half of the new guard in ContactPaymentsService.reconcilePendingEndpoints. Test 3 drove the isDeletingProfile half.
T7-2x.2x.mp4
Test 8: a failed deletion keeps the repair pending, and Disconnect syncs the registry at once ✅ The failed deletion set the marker without publishing, and Disconnect then published at once and ended with privatePayments=false, per the hook log.
  • The hook made the profile delete request throw after a real withdrawal to a linked contact, which the SDK reported delivered. It stands in for a homeserver error.
  • The deferred branch set the marker and the contacts were deleted. Nothing was published, and "Unable to Delete Profile" offered Retry and Disconnect.
  • About 8 s later, a periodic reconcile republished privatePayments=true, because sharing was still on, and cleared the marker.
  • Disconnect's private cleanup published at once instead of deferring. The final public cleanup published privatePayments=false and cleared the marker, and profile onboarding showed.
  • Disconnect publishes privatePayments=true before the final false, because its private cleanup runs while private sharing is still on. signOut is unchanged by this PR.
T8-2x.2x.mp4
Test 5: an undelivered withdrawal is not acknowledged ✅ A withdrawal still pending for a contact that had not answered the link skipped the deferred branch and kept both cleanup markers, per the hook log.
  • First try: with the other wallet fully linked and its app stopped, the SDK still reported the withdrawal delivered. Deletion took the normal deferred path with one privatePayments=false publish.
  • Second try: the other wallet recreated its profile and its app was stopped before it answered. This wallet created a profile, saved it as a contact, and deleted its profile about 90 s later.
  • The withdrawal stayed pending after the drain. The deferred branch did not run, the failure path set both cleanup markers, and deletion continued to profile onboarding.
  • Outside this PR: the final privatePayments=false publish was refused with policy_error ("app-owned work is still active"), and the marker was kept.
  • After sign-out the next reconcile failed with "no Pubky session available", so the registry keeps its last privatePayments=true. #984 tracks this error. Master runs the same path, because the throw comes before this PR's branch.
T5b-2x.2x.mp4
T5-2x.2x.mp4
Test hook (not pushed)

The hook from the review at b948a46, plus:

diff --git a/Bitkit/AppScene.swift b/Bitkit/AppScene.swift
--- a/Bitkit/AppScene.swift
+++ b/Bitkit/AppScene.swift
@@ -534,5 +534,8 @@ struct AppScene: View {
             .onChange(of: wallet.walletExists) { _, newValue in handleWalletExistsChange(newValue) }
             .onChange(of: wallet.nodeLifecycleState) { _, newValue in handleNodeLifecycleChange(newValue) }
-            .onChange(of: scenePhase, initial: true) { _, newValue in handleScenePhaseChange(newValue) }
+            .onChange(of: scenePhase, initial: true) { _, newValue in
+                TestHook.log("scenePhase \(newValue)") // test hook
+                handleScenePhaseChange(newValue)
+            }
             .onChange(of: network.isConnected) { _, isConnected in handleNetworkChange(isConnected) }
             .onOpenURL { url in app.retainDeepLink(url) }
@@ -1464,5 +1467,5 @@ struct AppScene: View {
             if mode == .full {
                 await PubkyService.republishIdentityIfNeeded(publicKey: pubkyProfile.publicKey)
-                await retryPendingPaykitEndpointRemoval()
+                await TestHook.$caller.withValue("poll") { await retryPendingPaykitEndpointRemoval() } // test hook
             }
             await refreshIncomingPaykitPaymentRequests(mode: mode, messagePriority: .background, onlyWhileActive: true)
@@ -1868,4 +1871,5 @@ struct AppScene: View {
 
     private func retryPendingPaykitEndpointRemoval() async {
+        if TestHook.caller != "poll" { await TestHook.hold("reconcile-other") } // test hook
         await ContactPaymentsService.reconcilePendingEndpoints(reconcilePrivate: {
             let privateCleanupPending = UserDefaults.standard.bool(forKey: PrivatePaykitService.cleanupPendingKey)
@@ -1878,4 +1882,5 @@ struct AppScene: View {
             }
             await TestHook.hold("reconcile-private") // test hook
+            if TestHook.caller == "poll" { await TestHook.hold("reconcile-private-poll") } // test hook
         }) {
             if PublicPaykitService.isCleanupPending {
diff --git a/Bitkit/Managers/PubkyProfileManager.swift b/Bitkit/Managers/PubkyProfileManager.swift
--- a/Bitkit/Managers/PubkyProfileManager.swift
+++ b/Bitkit/Managers/PubkyProfileManager.swift
@@ -982,4 +982,8 @@ class PubkyProfileManager: ObservableObject {
         }
         do {
+            if TestHook.flag("fail-delete") { // test hook
+                TestHook.log("injected profile delete failure") // test hook
+                throw TestHook.InjectedFailure() // test hook
+            } // test hook
             try await Task.detached {
                 try await PubkyService.deletePaykitProfile()
diff --git a/Bitkit/Services/ContactPaymentsService.swift b/Bitkit/Services/ContactPaymentsService.swift
--- a/Bitkit/Services/ContactPaymentsService.swift
+++ b/Bitkit/Services/ContactPaymentsService.swift
@@ -254,10 +254,10 @@ enum ContactPaymentsService {
         await reconcilePrivate()
         let hookDeleting = await PrivatePaykitService.shared.isDeletingProfile // test hook
-        TestHook.log("reconcile private phase done isDeletingProfile=\(hookDeleting) cancelled=\(Task.isCancelled)") // test hook
+        TestHook.log("reconcile private phase done caller=\(TestHook.caller) isDeletingProfile=\(hookDeleting) cancelled=\(Task.isCancelled)") // test hook
         guard await !PrivatePaykitService.shared.isDeletingProfile, !Task.isCancelled else {
-            TestHook.log("reconcile public phase skipped") // test hook
+            TestHook.log("reconcile public phase skipped caller=\(TestHook.caller)") // test hook
             return
         }
-        TestHook.log("reconcile public phase start") // test hook
+        TestHook.log("reconcile public phase start caller=\(TestHook.caller)") // test hook
         await reconcile()
     }
diff --git a/Bitkit/Services/PublicPaykitService.swift b/Bitkit/Services/PublicPaykitService.swift
--- a/Bitkit/Services/PublicPaykitService.swift
+++ b/Bitkit/Services/PublicPaykitService.swift
@@ -631,4 +631,12 @@ enum PublicPaykitService {
 
 enum TestHook { // test hook
+    // test hook
+    @TaskLocal static var caller = "other"
+    struct InjectedFailure: Error {} // test hook
+
+    static func flag(_ name: String) -> Bool { // test hook
+        FileManager.default.fileExists(atPath: holdDirectory.appendingPathComponent(name).path) // test hook
+    } // test hook
+
     private static var holdDirectory: URL { FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0] } // test hook
 
diff --git a/Bitkit/Views/Profile/EditProfileView.swift b/Bitkit/Views/Profile/EditProfileView.swift
--- a/Bitkit/Views/Profile/EditProfileView.swift
+++ b/Bitkit/Views/Profile/EditProfileView.swift
@@ -170,4 +170,5 @@ struct EditProfileView: View {
             }
 
+            TestHook.log("delete failed; showing Retry and Sign out") // test hook
             showDeleteFailureOptions = true
         }
@@ -187,6 +188,8 @@ struct EditProfileView: View {
 
     private func disconnectAfterFailedDelete() async {
+        TestHook.log("sign out after failed delete start") // test hook
         do {
             try await pubkyProfile.signOut()
+            TestHook.log("sign out after failed delete done") // test hook
             navigation.path = [app.hasSeenProfileIntro ? .pubkyChoice : .profileIntro]
         } catch {

Not driven:

  • The 62-contact deletion right after import: Import All needs Pubky Ring, which the simulator lacks.
  • Identity replaced during a deletion: the deletion cleanup runs without a session check, and the deletion overlay blocks input.
  • Retry after a failed deletion racing a reconcile: @jvsena42's review places it outside this PR, closed by #954.
  • delete-profile.xml at 0 contacts: @jvsena42 passed it at b948a46 (review).

Delta review of diff (1 file). No device case reached deferAppSync: true in PubkyProfileManager.deleteProfile (line 981), because its only caller passes cleanPrivatePaykitEndpoints: false.

Pair PR synonymdev/bitkit-android#1538 is unchanged at d689900 and matches in intent: both defer the intermediate registry update. Android keeps the private cleanup marker pending, while iOS sets the public one, as @piotr-iohk's review notes.

jvsena42
jvsena42 previously approved these changes Oct 17, 2026

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving at 65ca4329b. The delta since my last review is one test-only line in PrivatePaykitServiceTests.swift:2134, which clears the contact cache at the start of each loop iteration. No assertion changed, both the deferred and non-deferred branches are still exercised, and setUp restores the defaults domain, so no state leaks between tests. It fixes the host-cache dependency raised in ovi-reviewer's thread.

CI is green at this head.

Device gate: 65ca4329 — no device check needed (test-only delta; delete-profile.xml passed 7/7 at b948a46); perf: not relevant; l10n: no UI text changed.

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Full reassessment of the complete PR diff against merge base 5009f490, at 69ae1c17. The comparison base changed since the saved assessment, and no coverage baseline was supplied. Inherited coverage is empty.

No new actionable code findings.

Profile deletion withdraws private payment lists, skips the intermediate App Registry update, and leaves publicPaykitCleanupPending set until final public cleanup publishes private sharing as disabled. Foreground reconciliation runs the private phase first and skips the public phase while profile deletion is active or that task is cancelled, so the deferred marker stays pending. Disconnect, contact deletion, and sharing changes still synchronize the registry immediately. Against bitkit-android#1538 at d6899001, both defer the intermediate update and keep a repair marker for retry. iOS records that repair on the public cleanup marker after a successful withdrawal. Android leaves the private cleanup marker set and syncs the registry from private retry. iOS also skips the public phase once deletion or cancellation is visible.

Validation: source inspection only. On this revision, unit tests and integration tests succeeded. E2E was still in progress. The host-cache dependency called out on testDeferredCleanupLeavesDurableRegistryReconciliation is addressed by clearing privatePaykitCacheState before each case. Device testing: not performed in this review.

Ready for device testing. The updated delete-profile.xml steps, including the zero-contact timing pass and the 62-contact repeats, remain the manual QA contract.

@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: 💬 Comment: no findings and every driven case passed on the branch merged with master, but the 62-contact deletion right after import and cancelling cleanup or replacing the identity during deletion are not driven.

Findings: none. The merge keeps #954's session checks and its publication acknowledgement in AppScene.retryPendingPaykitEndpointRemoval, split at this PR's private/public phase boundary.

QA for 69ae1c1:
69ae1c1 brings #954 into the deletion path, so I drove the earlier cases again on this head. Tests 9 and 10 are the delete-profile.xml runs that @piotr-iohk's review leaves as the manual QA contract.

Tested on three iOS 26.5 simulators (iPhone 17 Pro) against the staging homeserver.

  • The build is 69ae1c1 plus the test hook from the 65ca432 review, which is not pushed.
  • The hook's AppScene part was adapted to the merged reconcile and is shown below. It also logs #954's acknowledgement.
  • For Test 11 only, it adds a flag-gated failure of the App Registry publish.
Test 1: an interrupted deletion keeps the registry repair pending across restart ✅ After a restart the marker was still set, and the next reconcile republished and cleared it through #954's acknowledgement, per the hook log.
  • A deletion held after its withdrawal set the marker without publishing. The app was terminated before the profile was removed.
  • After relaunch the profile was still there and the marker was still set.
  • The next reconcile ran its public phase in publishEndpoints mode because sharing was on. It published privatePayments=true, and the acknowledgement cleared the marker.
R1-2x.2x.mp4
Test 2: a foreground return during a held deletion starts no reconciliation ✅ The foreground reconcile was not admitted, and nothing published while the deletion was held, per the hook log.
  • The deletion was held after its withdrawal, with the progress overlay up.
  • Moving the app to the background and back logged the reconcile as not admitted, because there is no automatic Paykit session while deleting.
  • No publish followed, and the marker stayed set until the app was terminated about 90 s later.
  • Network restoration was not triggered: the simulator offers no per-device network toggle.
R1-2x.2x.mp4
Test 3: a reconcile cancelled after its private step keeps the registry repair pending ✅ Backgrounding cancelled the held polling reconcile, which skipped its public phase and kept the marker, per the hook log.
  • After the restart in Test 1, the polling reconcile was held after its private step.
  • Moving the app to the background released it with cancelled=true. It skipped its public phase and published nothing, and the marker stayed set.
  • The foreground reconcile in Test 1 then cleared it.
R1-2x.2x.mp4
Test 4: a reconcile held after its private step skips its public phase once deletion begins ✅ The released reconcile saw the deletion and skipped its public phase, and nothing published until the final cleanup, per the hook log.
  • Setup: an interrupted deletion set the marker. On relaunch the reconcile was held after its private step, and Delete Profile was then started and held after its withdrawal.
  • Released, the reconcile logged isDeletingProfile=true and skipped its public phase.
  • Nothing published during the deletion. At b948a46 a foreground endpoint refresh published privatePayments=true at this point. Per the code, that refresh now requires the automatic Paykit session, which is nil while deleting.
  • Released, the deletion published privatePayments=false once, cleared the marker and showed profile onboarding. Reopening Profile offers only Create.
R2-2x.2x.mp4
Test 5: deleting a profile with a linked contact withdraws before deferring ✅ The withdrawal to the linked contact was reported delivered before the deferred branch set the marker, per the hook log.
  • The two wallets saved each other as contacts.
  • In each deletion of Tests 1 and 4, the withdrawal went to that contact and the SDK reported it delivered with no failures. The marker was set after that.
  • The deletion that completed published once, privatePayments=false.
R2-2x.2x.mp4
Test 6: Retry after a failed deletion while a reconcile waits ✅ A reconcile admitted before Retry skipped its public phase once Retry began, per the hook log; the final publish was then refused (#984).
  • The hook made the profile delete request fail after the withdrawal, and the app offered Retry and Disconnect.
  • A polling reconcile admitted after the failure was held after its private step. Retry then started the deletion, and its cleanup took the deferred branch with sharing still on. This is the overlap from @jvsena42's review.
  • Released, the reconcile logged isDeletingProfile=true and skipped its public phase. The deletion finished on profile onboarding.
  • Outside this PR: the final privatePayments=false publish was refused with policy_error ("app-owned work is still active"), and the marker was kept. The first attempt's withdrawal was still pending (Test 7), as in Test 5 of the 65ca432 review. #984 tracks this error.
R3-2x.2x.mp4
Test 7: an undelivered withdrawal is not acknowledged ✅ A withdrawal still pending after the drain skipped the deferred branch and set both cleanup markers, per the hook log.
  • In the first attempt of Test 6, the contact had just been saved again, and its withdrawal stayed pending after the drain.
  • The deferred branch did not run. The failure path set both cleanup markers.
R3-2x.2x.mp4
Test 8: sharing-off, contact deletion and Disconnect still sync the registry at once ✅ Each published without the deferred branch, and Disconnect ended with privatePayments=false, per the hook log.
  • Turning contact payments off withdrew from the contact and published privatePayments=false at once. Turning it back on published true.
  • Deleting a saved contact published at once.
  • After a deletion the hook failed, Disconnect's private cleanup published at once. The final cleanup published privatePayments=false, and profile onboarding showed.
R4-2x.2x.mp4
R6-2x.2x.mp4
Test 9: delete-profile.xml with 0 contacts ✅ The overlay blocked input while pending, the deletion finished about 10 s after confirming, and reopening Profile offers only Create.
  • The log shows one privatePayments=false publish and no uncertain-write wait.
R4-2x.2x.mp4
Test 10: delete-profile.xml with 62 contacts ✅ The overlay blocked input while pending, the deletion finished about 14 s after confirming, and reopening Profile offers only Create.
  • The wallet was restored from a staging identity that follows 62 profiles, and the 62 were saved through bitkit://contact.
  • They are Pubky-only profiles, so the deletion had no linked contact to withdraw from.
  • The log shows one privatePayments=false publish and no uncertain-write wait.
R5-2x.2x.mp4
Test 11: a failed retry keeps the registry repair pending ✅ Retries that failed to publish kept the marker, and the first retry that published cleared it through the acknowledgement, per the hook log.
  • A deletion held after its withdrawal set the marker, and the app was terminated.
  • The hook made the App Registry publish fail. The relaunch reconcile and the next polling reconcile each failed in their public phase, and the marker and the profile stayed.
  • With the failure removed, the next polling reconcile published privatePayments=true and the acknowledgement cleared the marker.
  • b948a46 proved this with real shared_state_busy failures; the merge rewrote that reconcile branch, so I drove it again.
R7-2x.2x.mp4
Test hook, AppScene part (not pushed)
     private func retryPendingPaykitEndpointRemoval() async {
-        guard let session = pubkyProfile.automaticPaykitSession else { return }
+        if TestHook.caller != "poll" { await TestHook.hold("reconcile-other") } // test hook
+        guard let session = pubkyProfile.automaticPaykitSession else {
+            TestHook.log("reconcile not admitted: no automatic Paykit session caller=\(TestHook.caller)") // test hook
+            return
+        }
         await ContactPaymentsService.reconcilePendingEndpoints(reconcilePrivate: {
@@
             if privateCleanupPending, !UserDefaults.standard.bool(forKey: PrivatePaykitService.cleanupPendingKey) {
                 PublicPaykitService.setCleanupPending(true)
             }
+            await TestHook.hold("reconcile-private") // test hook
+            if TestHook.caller == "poll" { await TestHook.hold("reconcile-private-poll") } // test hook
         }) {
+            TestHook.log( // test hook
+                "reconcile public closure sessionCurrent=\(pubkyProfile.automaticPaykitSession == session) mode=\(PublicPaykitService.pendingReconciliationMode())" // test hook
+            ) // test hook
             if PublicPaykitService.isCleanupPending, pubkyProfile.automaticPaykitSession == session {
@@
                         PublicPaykitService.setCleanupPending(false)
                     }
+                    TestHook.log("reconcile public done") // test hook

PublicPaykitService.acknowledgePublishedEndpoints also logs the marker it sets.

Not driven:

  • The 62-contact deletion right after import: Import All needs Pubky Ring, which the simulator lacks.
  • Cancelling cleanup or replacing the identity during a deletion: the deletion cleanup runs without a session check, and the deletion overlay blocks every route to either.

Delta review of diff, a merge of master. Against master, this PR's own diff changed only in AppScene.retryPendingPaykitEndpointRemoval. No device case reached deferAppSync: true in PubkyProfileManager.deleteProfile, because its only caller passes cleanPrivatePaykitEndpoints: false.

Pair PR synonymdev/bitkit-android#1538 is unchanged at d689900 and matches in intent: both defer the intermediate registry update.

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.

bug: profile deletion updates the app registry twice

5 participants