fix: keep wallet backups consistent

[ben-kaufman]

Closes #951

Fixes the iOS counterpart of synonymdev/bitkit-android#1487. Android fix: synonymdev/bitkit-android#1507.

This PR keeps wallet backups consistent during Paykit activity and preserves backup requirements for changes made while a backup is running.

Description

• Pairs each SDK export with its version under the existing operation lock, then validates the complete wallet capture without introducing an inverse lock order.
• Recaptures changing snapshots up to three times while retaining identity, cancellation, restore and unresolved-payment checks; automatic backup remains pending when contention exhausts those attempts.
• Acknowledges only the captured backup requirement, so changes during capture or upload still need a subsequent backup, including changes within the same second.
• Preserves scheduled follow-up for every app-managed category when an upload completes with newer changes pending or a manual backup is busy, without joining or cancelling that backup or repeatedly retrying genuine export/upload failures.

Out of Scope

• SDK, backup payload format, dependencies and payment safety waits: unchanged.
• The reported six-minute Android mainnet interval and device restore acceptance: not reproduced by these native tests. Sustained mutations can still defer safe capture; this PR makes no maximum-delay or performance claim.

Design

N/A - no UI changes.

Preview

N/A - no UI changes.

QA Notes

Journeys

N/A - not drivable; see Manual Tests.

Manual Tests

• regression: queue backup behind a controlled SDK mutation, then release it -> the uploaded snapshot includes that mutation without an unnecessary rejection; SDK operation gates and stored-envelope inspection are not in Capabilities.
• regression: change state during held capture, exhaust retries, then stop changing it -> no mixed snapshot uploads and automatic backup succeeds without another mutation; controlled capture gates are not in Capabilities.
• regression: change state during a held scheduled upload, or hold a manual upload and allow its scheduled follow-up to run, then release upload -> newer state stays pending and a follow-up backup saves it. Repeat for wallet, settings, widgets, activity, metadata and Blocktank; controlled upload and scheduler gates are not in Capabilities.
• regression: cancel or replace identity during capture/retry -> no mixed-identity snapshot uploads, and restore/unresolved-payment guards still reject unsafe capture; controlled capture and identity gates are not in Capabilities.
• Restore a successfully captured envelope into an isolated fixture -> SDK/private state, reservations, proofs and the signed payment attempt agree, without broadcasting; controlled backup-envelope injection is not in Capabilities. See wallet-snapshot-consistency.md for the shared platform checks.

Automated Checks

• added BackupServiceTests.swift - covers mixed-state recapture, bounded contention, capture/upload watermarks, automatic follow-up across all six uploaded categories, scheduled/manual overlap, cancellation and genuine failures without repeated automatic attempts.
• added PaykitSdkOperationLockTests.swift - verifies queued mutations are included in the paired export/version, later mutations reject stale capture, and identity fencing survives recapture.

Final local validation: all 10 focused BackupServiceTests and the 2,612-test native suite passed using the repository CI unit-test exclusions, with zero failures and no missing baseline tests. Simulator test builds passed. Complete format diagnostics retain the same 58 findings in 13 unchanged files as the verified merge-base baseline, none in changed files. All 16 emitted compiler diagnostics match that baseline, with none introduced. The complete translation output remains identical: 139 warnings, zero errors. Independent final correctness/native/polish review found no remaining actionable issues.

The capture/upload requirement regression failed on the original implementation. Controlled tests also verified mixed-state rejection, busy/manual scheduling and cancellation precedence. All five non-wallet categories and the settings manual-upload overlap failed before the all-category scheduler correction, then passed with it. Eleven native tests were added across this PR. Device fault injection and restore of a captured backup envelope remain unrun; the original Android mainnet delay is not reproduced by this coverage.

Fresh follow-up validation on 0387da32: 16 focused native tests passed, including backup scheduling, paired SDK export/version, identity fencing and populated native restore fixtures. Complete format and translation reports still match the verified baseline. A normal unfunded simulator wallet uploaded and exported all six categories; a subsequent settings change also uploaded and exported correctly. The wallet envelope was empty of Paykit/payment state, so this is an ordinary backup smoke check, not completion of the controlled manual cases above.

All required checks now pass on current head 0387da32, including unit, integration, translation, E2E build, the E2E device job (116723967976) and e2e-status (116742893487). This does not complete the controlled manual fault-injection or populated device-restore cases above.

[greptile-apps]

Confidence Score: 5/5

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

Summary

This PR checks that wallet state stays consistent while a backup is captured and keeps newer changes pending after upload.

• Wallet backups recapture when wallet state changes mid-capture.
• Wallet backups keep newer changes pending for a follow-up upload.

No actionable issues were established. Native tests were not run during this review.

ben-kaufman explicitly describes genuine failures as excluded from automatic retries. ben-kaufman also identifies device restore coverage and sustained backup delay as unverified and outside this PR’s claims.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Wallet backup requested] --> B{Backup already running?}
    B -->|Yes| C[Return busy; scheduled wallet work stays pending]
    B -->|No| D[Record current backup requirement]
    D --> E[Export SDK state and version under operation lock]
    E --> F[Read remaining wallet state]
    F --> G{Identity and state still match?}
    G -->|State changed| H{Three attempts used?}
    H -->|No| E
    H -->|Yes| I[Keep requirement pending for scheduled follow-up]
    G -->|Identity changed or other error| J[Stop without acknowledging backup]
    G -->|Yes| K[Check cancellation and restore or wipe state]
    K --> L[Upload snapshot]
    L --> M[Acknowledge only the starting requirement]
    M --> N{Newer changes pending?}
    N -->|Yes| I
    N -->|No| O[Backup complete]

Loading

_{Reviews (1) · Last reviewed commit: "fix: keep wallet backups consistent duri..." · Reviewed by Greptile}

[talosmachina]

1 finding (1 P1). Pairs the Paykit SDK export with its version, recaptures changing wallet snapshots up to three times, and acknowledges only the backup requirement that was captured, so changes during capture or upload stay pending. Reviewed cccba95, full tier.

What I checked, and 4 candidates I ruled out

Read in full: BackupService.swift (the scheduling, status and wallet capture paths), PrivatePaykitService+Backup.swift, the changed PaykitSdkService hunks and PaykitSdkOperationLock.withLock, BackupItemStatus.swift, the backup snapshot and change publishers of every store WalletBackupChangeTracker merges
Call sites traced: triggerBackup (scheduled loop, BackupViewModel, rewriteMigratedBackups), persistWalletBackup (runBackup, UsdtVssBackup), scheduleFullBackup, exportBackupState
CI: Run Tests, Run Integration Tests and build-local still pending when this was posted; validate green. iOS does not build on this machine, so this review is reasoned from the code and CI is the gate.

Ruled out

• Mixed wallet snapshot slips through: every store read in the capture closure emits on one of the merged publishers on save, and the SDK side is checked again under the operation lock via backupStateVersion, so a change between reads fails the version comparison.
• Lock-order inversion in validateBackupSnapshot: it takes only the SDK operation lock after the proof mutationLock has been released, matching the comment.
• status.required - 1 underflow: runBackup sets required = max(..., synced + 1, ...) at start and markBackupRequired never lowers it, so required >= 1 on the success path.
• Wallet follow-up lost while a manual backup is busy: .busy keeps the scheduled loop alive and the trailing reschedule covers .completed, matching testScheduledBackupRetainsFollowupWhileManualUploadIsBusy.
• Cold reader: the second-model check of the P1 did not return before this was posted.

Merge confidence: 3/5, the P1 on non-wallet backup scheduling.

[talosmachina]

Re-review of 0387da3 after cccba95. 1 earlier finding: 1 fixed. 0 new findings. Full tier on the diff since my last review (BackupService.swift, BackupServiceTests.swift, the journey note).

What I checked

Earlier finding

• Non-wallet categories stranded when a change lands mid-upload: fixed. runScheduledBackup no longer gates the retry break or the trailing scheduleBackup on .wallet (BackupService.swift:690, :696). A non-wallet change that arrives during an upload now leaves synced < required, the loop's canRetryPendingBackup check keeps the job alive for .completed and .busy, and the post-loop re-check catches a change whose own scheduleBackup returned at the backupJobs[category] == nil guard.

Ruled out in the new hunks

• Retry loop on a genuine failure: .failed is still not in canRetryPendingBackup, so an export or upload error leaves the loop and nothing reschedules it.
• Tight loop while a manual backup is busy: every iteration waits waitForBackupDelay() first, the same as the wallet path before this push.
• lightningConnections left out of the new test: nothing calls markBackupRequired for it; its status is only written directly at BackupService.swift:601, and the same allCases where != .lightningConnections filter already exists at :742.

Tests: testScheduledBackupUploadsChangesFromEveryCategory loops over every scheduled category, and the scheduled/manual overlap test now covers .settings as well as .wallet.
CI: Run Tests, Run Integration Tests and build-local still pending when this was posted; validate green. iOS does not build on this machine, so this re-review is reasoned from the code and CI is the gate.

Merge confidence: 4/5, 0 open findings; held at 4 until Run Tests and Run Integration Tests finish green on 0387da3, and the device restore checks in the PR body are still unchecked.