Repository navigation
fix: serialize backup observation lifecycle - #1123
ben-kaufman wants to merge 15 commits into
Conversation
|
Triage: 3.1.0. It fixes shipped backup handling, and 3.1.0 is due 28 Oct. |
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
Serializes backup observation start/stop on MainActor, bumps a scheduling generation so queued work from a stopped observation is dropped, and makes stop drain cancelled scheduled and running jobs (and both writers) before it returns. While cleanup runs, manual uploads and fresh USDT requests wait for it instead of being skipped.
No findings. Reviewed head f204e05 against its base fix/restore-identity-background (stacked on #1121), full tier.
Merge confidence: 4/5. No open findings. It is held below 5 because Run Tests was still pending when I reviewed, and the lifecycle manual cases 1 to 3 have not been run on a device.
What I checked
- iOS does not build on this reviewer's Linux host, so this review is reasoned from the code and CI. SwiftFormat is green and
Run Testswas pending. - Stop ordering:
observationTaskchaining means a start queued behind a stop runs only after the drain,isStoppingBackupsand the generation bump happen in one.backupqueue block, and the drain awaits job values outside that queue, so cleanup that hops back onto.backup(runScheduledBackupremoving its key,runBackupremoving its task) cannot deadlock against it. - No wait cycle through
waitForObservationTransition. A cancelled scheduled job skips its tailscheduleBackup. A job that removed itself before the stop snapshot callsscheduleBackupwith the old generation and returns at the generation check. ScheduledtriggerBackupduring a stop returns.busy, and its cancelled loop then exits. So no task the drain awaits is itself waiting onobservationTask. - Generation capture:
markBackupRequiredafter the bump (the USDT path while observers are stopped) carries the new generation, waits out the stop and then schedules. Work captured before the bump is dropped, but its status stays required, and the restarted observer'sprepend(getAllBackupStatuses())picks it up again. runningis now set only inside the admittedscheduleBackupblock rather than bymarkBackupRequired, so a dropped schedule no longer leaves a stale running flag.- Callers:
AppScene.handleNodeLifecycleChange(start on.running, stop otherwise) andAppReset.wipeLocalboth call from MainActor and fit the new@MainActorsignatures.BackupViewModelandrewriteMigratedBackupsreach the manualtriggerBackuppath, which now waits for cleanup instead of returning early. - Ruled out: the stop now also awaits in-flight running uploads, so a wipe can wait on a VSS put. The VSS setup is bounded by a 30 s timeout and both writers were already awaited before this change, so the behaviour is intended.
Reviewer model: claude-opus-5-5
Reasoning effort: Not exposed (the run metadata available to this reviewer does not report a reasoning setting)
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Code scan: no findings so far.
Checked and clean:
- A backup is marked synced only after the put returns success.
stopObservingBackupsdrains every task beforeremovePersistentDomain, so after a wipe and restore the old wallet's pending data can't upload under the new wallet. BackupItemStatusis persisted in the same format as v3.0.0, so nothing to migrate.- Twin synonymdev/bitkit-android#1689 fences by generation instead.
Performance-relevant. The PR removes the 500 ms .debounce from the settings, widgets, activities, metadata, hw-names, appState and blocktank listeners (BackupService.swift:682-774). Each change event now does a UserDefaults JSON decode, encode and write and spawns a MainActor task, so a sync of N activities costs N round-trips instead of one per burst. The gate counts backupStatuses writes during a sync of a couple of hundred payments, head vs master, and times stopObservingBackups during a wallet reset with a put in flight. If the write count grows with N, skipping saveBackupStatuses when the status is unchanged would bring it back down.
Device gate (partial): 1ad665f — not run yet; pending that measurement. Device results will follow in a separate review.
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — manual test 2 fails for a real VSS connection outage; the same failure reproduces on the PR base.
Findings: I found no correctness defect introduced by this diff. The QA failure below is inherited: the released VSS binding returns VssError.UnknownError for the failed connection, but BackupService.isTemporaryBackupFailure excludes that error, so the pending recovery backup receives no automatic retry.
Reviewed commit: 1ad665f.
QA for 1ad665f: iPhone 17 Pro simulator, iOS 26.5, Xcode 27. The component and navigation tests used the unmodified head. Manual lifecycle checks used local holds and logging over the same head, a disposable wallet and staging VSS. No payment was submitted.
Test 1: cleanup, manual admission and restart ✅ Stop and the manual backup waited for cleanup; the latest wallet and settings state reached VSS.
I held final scheduled cleanup, requested stop and a manual metadata backup, queued restart, then edited wallet/settings state. No capture began before release. I also held a settings upload, canceled it and changed settings during restart. The later upload preserved the new requirement. Hook logs and decrypted VSS reads confirmed the final wallet records and setting matched local state.
lifecycle-12x.2x.mp4
Lifecycle controls on the disposable app; the assertions were checked through logs and VSS reads.
held cleanup
released cleanup
stop-done elapsed=1.692
manual-done result=completed
held upload-SETTINGS
upload-settings swipe=0
upload-settings swipe=1
verify-settings matches=true value=Optional(true)
verify-wallet matches=true
Test 2: USDT recovery with observation stopped ❌ A connection failure left recovery pending without the expected automatic retry.
A fresh USDT recovery request waited for cleanup and completed without restarting observation. I then pointed only the custom VSS endpoint at an unreachable loopback port. The real native request failed after about 10.5 seconds with VssError.UnknownError. The job ended with .failed; no retry occurred during the following observation window of over two minutes.
I reproduced the same classifier and stopped-job behavior on base ec20b264. Restoring the endpoint and making a fresh explicit request completed successfully there. The required marker survived the failure. This is an inherited limitation, not a new inline finding, but it fails the transient-retry expectation in manual case 2.
| Head | Base comparison |
|---|---|
lifecycle-22x.2x.mp4 |
transport-base2x.2x.mp4 |
The app remains on Home; native and hook logs establish the request outcomes.
head: usdt-recovery-put
head: UnknownError(errorDetails: "error sending request for url (http://127.0.0.1:1/putObjects)")
head: backup-done USDT_RECOVERY result=failed
base: backup-done USDT_RECOVERY result=failed
base: status USDT_RECOVERY synced=1793040118 required=1793040578 running=false
base: route-ready vss-normal
base: usdt-recovery-ack
base: backup-done USDT_RECOVERY result=completed
Test 3: repeated observation lifecycle ✅ Stopped listeners ignored later wallet edits, and a received settings change survived restart.
I repeated start/stop and a subsequent real wallet-record edit 25 times. The hook log showed unchanged wallet backup status and no wallet capture after each stop. A settings event received before stop remained required and uploaded after restart.
lifecycle-32x.2x.mp4
Repeated lifecycle controls; status comparisons and upload acknowledgement were checked in the hook log.
Test 4: backup component regressions ✅ The focused backup suite passed without skips.
xcodebuild test-without-building -only-testing:BitkitTests/BackupServiceTests passed. The finalized XCResult confirms the cancellation, final-write drain, restart, stopped-listener and bounded-backoff assertions.
backup2x.2x.mp4
App-hosted XCTest execution; the result was checked in XCResult.
Test 5: USDT back navigation ✅ Both affected ordinary navigation cases passed with their existing assertions.
I ran testUnknownBalanceAndManualEntryUseConsistentAmountsInCommaRegion and testReceiveAndInvalidPaymentStaySeparateFromBitcoin, then repeated them for the recordings. Amount entry, recipient retention, receive draft cancellation and back navigation passed. The UI runs reported QoS priority-inversion warnings. The recordings retain all changed frames; capture stutters remain.
| Amount and recipient | Receive and invalid payment |
|---|---|
usdt-entry-smooth2x.2x.mp4 |
usdt-receive-smooth2x.2x.mp4 |
Unmodified-head UI tests, with finalized XCResults checked.
Test 6: activity-notification burst ✅ The head/base measurement completed; status-write amplification is confirmed.
For 200 actual activity-publisher notifications, the head wrote backup status 200 times during the burst; the base coalesced them into one write. Notification dispatch took about 0.206 seconds at the head versus 0.002 seconds at the base. The head's subsequent stop returned in about 0.007 seconds with no held upload.
This measures the notification boundary, not a complete sync of 200 payments or the latency of a stalled network upload. It supplies the write-count evidence requested in the performance review; it does not establish a user-visible sync slowdown.
| Head | Base |
|---|---|
performance-head2x.2x.mp4 |
performance-base2x.2x.mp4 |
App startup and the measurement run; timings and write counts came from the hook log.
All three listed manual cases were driven; case 2 failed. I did not run a funded payment or cross-platform restore. The author's released-native signing/restore evidence in this PR's QA notes remains scoped to its recorded revisions.
Full review: ec20b264..1ad665f, including all changed files and lifecycle callers. The native suites, live cleanup holds and navigation runs exercised the runtime changes; the changelog and journey text were inspected. SwiftFormat lint passed for the changed Swift files.
Twin: synonymdev/bitkit-android#1689 documents the same drain and pending-change preservation intent, with separate Android wipe-admission guards.
Head lifecycle and performance controls — local test hook, not pushed
Holds, logging, synthetic record edits and an endpoint-only outage control; the backup decisions and native transport results are unchanged.
diff --git a/Bitkit/BitkitApp.swift b/Bitkit/BitkitApp.swift
index 961935aa..bfc5d58b 100644
--- a/Bitkit/BitkitApp.swift
+++ b/Bitkit/BitkitApp.swift
@@ -83,6 +83,9 @@ class AppDelegate: NSObject, UIApplicationDelegate {
}
}
+ #if DEBUG
+ ReviewBackupControl.start() // test hook
+ #endif
return true
}
diff --git a/Bitkit/Constants/Env.swift b/Bitkit/Constants/Env.swift
index 76885696..44d6d41a 100644
--- a/Bitkit/Constants/Env.swift
+++ b/Bitkit/Constants/Env.swift
@@ -372,7 +372,12 @@ enum Env {
}
static var vssServerUrl: String {
- switch network {
+ #if DEBUG // test hook
+ if ProcessInfo.processInfo.environment["REVIEW_BACKUP_CONTROL"] == "1", FileManager.default.fileExists(atPath: FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0].appendingPathComponent("review-backup/vss-fault").path) { // test hook
+ return "http://127.0.0.1:1" // test hook
+ } // test hook
+ #endif // test hook
+ return switch network {
case .bitcoin: "https://bitkit.to/vss_rs_auth"
default: "https://bitkit.stag0.blocktank.to/vss_rs_auth"
}
diff --git a/Bitkit/Services/BackupService.swift b/Bitkit/Services/BackupService.swift
index 7903e1c8..d414ce84 100644
--- a/Bitkit/Services/BackupService.swift
+++ b/Bitkit/Services/BackupService.swift
@@ -155,7 +155,17 @@ class BackupService {
backupData: ((BackupCategory) async throws -> Data)? = nil,
usdtRecoveryBackup: UsdtRecoveryBackupStore = UsdtRecoveryBackupStore(),
uploadBackup: @escaping (String, Data) async throws -> Void = { key, data in
+ #if DEBUG
+ await ReviewBackupControl.hold("upload-\(key)") // test hook
+ if key == "SETTINGS", let payload = try? SettingsBackupV1.decode(from: data) { // test hook
+ ReviewBackupControl.log("upload-settings swipe=\(payload.settings["enableSwipeToHideBalance"] ?? "missing")") // test hook
+ } // test hook
+ ReviewBackupControl.log("upload-start \(key)") // test hook
+ #endif
_ = try await VssBackupClient.shared.putObject(key: key, data: data)
+ #if DEBUG
+ ReviewBackupControl.log("upload-ack \(key)") // test hook
+ #endif
},
waitForBackupDelay: @escaping () async throws -> Void = {
try await Task.sleep(nanoseconds: 5_000_000_000)
@@ -292,6 +302,9 @@ class BackupService {
}
private func runBackup(category: BackupCategory, isScheduled: Bool) async -> BackupOutcome {
+ #if DEBUG
+ ReviewBackupControl.log("capture-start \(category.rawValue)") // test hook
+ #endif
Logger.debug("Backup starting for: '\(category.rawValue)'", context: "BackupService")
var outcome = BackupOutcome.failed
var requiredAtStart: UInt64 = 0
@@ -349,6 +362,9 @@ class BackupService {
}
}
_ = try? await ServiceQueue.background(.backup) { self.runningBackupTasks.removeValue(forKey: category) }
+ #if DEBUG
+ ReviewBackupControl.log("backup-done \(category.rawValue) result=\(outcome)") // test hook
+ #endif
return outcome
}
@@ -857,6 +873,9 @@ class BackupService {
while !Task.isCancelled {
do {
if outcome == .temporarilyUnavailable {
+ #if DEBUG // test hook
+ ReviewBackupControl.log("retry \(category.rawValue) delay=\(retryDelay)") // test hook
+ #endif // test hook
try await waitForTemporaryFailureDelay(retryDelay)
retryDelay = min(retryDelay * 2, 300_000_000_000)
} else {
@@ -873,6 +892,9 @@ class BackupService {
break
}
}
+ #if DEBUG
+ await ReviewBackupControl.hold("cleanup") // test hook
+ #endif
updateBackupStatus(category: category) { status in
BackupItemStatus(synced: status.synced, required: status.required, running: false)
}
@@ -1017,6 +1039,9 @@ class BackupService {
do {
let data = try JSONEncoder().encode(encoded)
+ #if DEBUG
+ ReviewBackupControl.countWrite() // test hook
+ #endif
defaults.set(data, forKey: backupStatusesKey)
backupStatusesSubject.send(statuses)
diff --git a/Bitkit/Services/UsdtRecoveryBackupStore.swift b/Bitkit/Services/UsdtRecoveryBackupStore.swift
index f804d076..f546805c 100644
--- a/Bitkit/Services/UsdtRecoveryBackupStore.swift
+++ b/Bitkit/Services/UsdtRecoveryBackupStore.swift
@@ -27,7 +27,14 @@ struct UsdtRecoveryBackupStore {
let encoder = JSONEncoder()
encoder.outputFormatting = [.sortedKeys]
let data = try encoder.encode(UsdtRecoveryBackupV1(version: 1, usdtWallet: snapshot, paykitUsdtState: paymentState))
+ #if DEBUG // test hook
+ await ReviewBackupControl.hold("upload-USDT_RECOVERY") // test hook
+ ReviewBackupControl.log("usdt-recovery-put") // test hook
+ #endif // test hook
try await put(Self.key(for: data), data)
+ #if DEBUG // test hook
+ ReviewBackupControl.log("usdt-recovery-ack") // test hook
+ #endif // test hook
}
func snapshots() async throws -> [UsdtRecoveryBackupV1] {
diff --git a/Bitkit/Services/ReviewBackupControl.swift b/Bitkit/Services/ReviewBackupControl.swift
new file mode 100644
index 0000000..aac9b26
--- /dev/null
+++ b/Bitkit/Services/ReviewBackupControl.swift
@@ -0,0 +1,108 @@
+#if DEBUG // test hook
+import Foundation // test hook
+import LDKNode // test hook
+
+enum ReviewBackupControl { // test hook
+ static let enabled = ProcessInfo.processInfo.environment["REVIEW_BACKUP_CONTROL"] == "1" // test hook
+ static let directory = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0].appendingPathComponent("review-backup") // test hook
+ static let lock = NSLock() // test hook
+ static var writes = 0 // test hook
+ static func countWrite() { if enabled { lock.withLock { writes += 1 } } } // test hook
+
+ static func log(_ message: String) { // test hook
+ guard enabled else { return } // test hook
+ lock.withLock { // test hook
+ try? FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) // test hook
+ let file = directory.appendingPathComponent("events.log") // test hook
+ var data = (try? Data(contentsOf: file)) ?? Data() // test hook
+ data.append(Data("\(Date().timeIntervalSince1970) \(message)\n".utf8)) // test hook
+ try? data.write(to: file) // test hook
+ } // test hook
+ } // test hook
+
+ static func hold(_ name: String) async { // test hook
+ guard enabled else { return } // test hook
+ let file = directory.appendingPathComponent("hold-\(name)") // test hook
+ guard FileManager.default.fileExists(atPath: file.path) else { return } // test hook
+ log("held \(name)") // test hook
+ while FileManager.default.fileExists(atPath: file.path) { // test hook
+ await Task.detached { try? await Task.sleep(nanoseconds: 100_000_000) }.value // test hook
+ } // test hook
+ log("released \(name)") // test hook
+ } // test hook
+
+ @MainActor static func start() { // test hook
+ guard enabled else { return } // test hook
+ log("control-ready head=1ad665f") // test hook
+ Task { // test hook
+ let end = Date().addingTimeInterval(1200) // test hook
+ while Date() < end { // test hook
+ let file = directory.appendingPathComponent("command") // test hook
+ if let command = try? String(contentsOf: file, encoding: .utf8).trimmingCharacters(in: .whitespacesAndNewlines) { // test hook
+ try? FileManager.default.removeItem(at: file) // test hook
+ let service = BackupService.shared // test hook
+ log("command \(command)") // test hook
+ switch command { // test hook
+ case "stop": Task { // test hook
+ let before = Date() // test hook
+ await service.stopObservingBackups() // test hook
+ log("stop-done elapsed=\(Date().timeIntervalSince(before))") // test hook
+ } // test hook
+ case "start": service.startObservingBackups() // test hook
+ case "wallet": // test hook
+ do { // test hook
+ var records = try WatchOnlyAccountStore.load() // test hook
+ if records.isEmpty { // test hook
+ records.append(WatchOnlyAccountRecord(id: UUID(), walletIndex: 0, accountIndex: 42, addressType: LDKNode.AddressType.nativeSegwit.stringValue, xpub: "tpubDDWohsp5dx2iMJ9N7iHbgAEDhH4BJB9NWW1fEW3yA3AFNDREmpzteCXNqppMLUmKFY5q5e3PXtS5CuqWCQbYcGhpPqYAgQSYdwknW9J6sQv", requestFingerprint: "review", createdAt: 1000, name: "review", isTrackingEnabled: false, setupState: .pendingDelivery)) // test hook
+ } // test hook
+ records[0].name = "review-\(Int(Date().timeIntervalSince1970))" // test hook
+ try WatchOnlyAccountStore.save(records); log("wallet-record-written name=\(records[0].name)") // test hook
+ } // test hook
+ catch { log("wallet-record-error") } // test hook
+ case "settings": SettingsViewModel.shared.swipeBalanceToHide.toggle() // test hook
+ case "manual": Task { // test hook
+ let result = await service.triggerBackup(category: .metadata) // test hook
+ log("manual-done result=\(result)") // test hook
+ } // test hook
+ case "usdt": service.markBackupRequired(category: .usdt) // test hook
+ case "vss-fault", "vss-normal": Task { // test hook
+ let route = directory.appendingPathComponent("vss-fault") // test hook
+ if command == "vss-fault" { try? Data().write(to: route) } // test hook
+ else { try? FileManager.default.removeItem(at: route) } // test hook
+ await VssBackupClient.shared.reset() // test hook
+ log("route-ready \(command)") // test hook
+ } // test hook
+ case "pending-wallet": service.markBackupRequired(category: .wallet) // test hook
+ case "status": // test hook
+ log("writes=\(lock.withLock { writes })") // test hook
+ for category in BackupCategory.allCases { // test hook
+ let s = service.getBackupStatus(category: category) // test hook
+ log("status \(category.rawValue) synced=\(s.synced) required=\(s.required) running=\(s.running)") // test hook
+ } // test hook
+ case "verify": Task { // test hook
+ do { // test hook
+ let settings = try await VssBackupClient.shared.getObject(key: "SETTINGS") // test hook
+ let payload = try SettingsBackupV1.decode(from: settings!.value) // test hook
+ let remote = payload.settings["enableSwipeToHideBalance"] as? Bool // test hook
+ log("verify-settings matches=\(remote == SettingsViewModel.shared.swipeBalanceToHide) value=\(String(describing: remote))") // test hook
+ let wallet = try await VssBackupClient.shared.getObject(key: "WALLET") // test hook
+ let data = try JSONDecoder().decode(WalletBackupV1.self, from: wallet!.value) // test hook
+ log("verify-wallet matches=\(data.watchOnlyAccounts == (try WatchOnlyAccountStore.load()))") // test hook
+ } catch { log("verify-error") } // test hook
+ } // test hook
+ case "burst": // test hook
+ let count = lock.withLock { writes } // test hook
+ let before = Date() // test hook
+ for _ in 0 ..< 200 { CoreService.shared.activity.notifyActivitiesChanged() } // test hook
+ log("burst-200 elapsed=\(Date().timeIntervalSince(before)) writes=\(lock.withLock { writes } - count)") // test hook
+ case "end": log("control-ended"); return // test hook
+ default: break // test hook
+ } // test hook
+ } // test hook
+ try? await Task.sleep(nanoseconds: 100_000_000) // test hook
+ } // test hook
+ log("control-timeout") // test hook
+ } // test hook
+ } // test hook
+} // test hook
+#endif // test hookHead connection-outage controls — local test hook, not pushed
Holds, logging, synthetic record edits and an endpoint-only outage control; the backup decisions and native transport results are unchanged.
diff --git a/Bitkit/Services/BackupService.swift b/Bitkit/Services/BackupService.swift
--- a/Bitkit/Services/BackupService.swift
+++ b/Bitkit/Services/BackupService.swift
@@ -155,7 +155,17 @@
backupData: ((BackupCategory) async throws -> Data)? = nil,
usdtRecoveryBackup: UsdtRecoveryBackupStore = UsdtRecoveryBackupStore(),
uploadBackup: @escaping (String, Data) async throws -> Void = { key, data in
+ #if DEBUG
+ await ReviewBackupControl.hold("upload-\(key)") // test hook
+ if key == "SETTINGS", let payload = try? SettingsBackupV1.decode(from: data) { // test hook
+ ReviewBackupControl.log("upload-settings swipe=\(payload.settings["swipeBalanceToHide"] ?? "missing")") // test hook
+ } // test hook
+ ReviewBackupControl.log("upload-start \(key)") // test hook
+ #endif
_ = try await VssBackupClient.shared.putObject(key: key, data: data)
+ #if DEBUG
+ ReviewBackupControl.log("upload-ack \(key)") // test hook
+ #endif
},
waitForBackupDelay: @escaping () async throws -> Void = {
try await Task.sleep(nanoseconds: 5_000_000_000)
@@ -292,6 +302,9 @@
}
private func runBackup(category: BackupCategory, isScheduled: Bool) async -> BackupOutcome {
+ #if DEBUG
+ ReviewBackupControl.log("capture-start \(category.rawValue)") // test hook
+ #endif
Logger.debug("Backup starting for: '\(category.rawValue)'", context: "BackupService")
var outcome = BackupOutcome.failed
var requiredAtStart: UInt64 = 0
@@ -349,6 +362,9 @@
}
}
_ = try? await ServiceQueue.background(.backup) { self.runningBackupTasks.removeValue(forKey: category) }
+ #if DEBUG
+ ReviewBackupControl.log("backup-done \(category.rawValue) result=\(outcome)") // test hook
+ #endif
return outcome
}
@@ -857,6 +873,9 @@
while !Task.isCancelled {
do {
if outcome == .temporarilyUnavailable {
+ #if DEBUG // test hook
+ ReviewBackupControl.log("retry \(category.rawValue) delay=\(retryDelay)") // test hook
+ #endif // test hook
try await waitForTemporaryFailureDelay(retryDelay)
retryDelay = min(retryDelay * 2, 300_000_000_000)
} else {
@@ -873,6 +892,9 @@
break
}
}
+ #if DEBUG
+ await ReviewBackupControl.hold("cleanup") // test hook
+ #endif
updateBackupStatus(category: category) { status in
BackupItemStatus(synced: status.synced, required: status.required, running: false)
}
@@ -1017,6 +1039,9 @@
do {
let data = try JSONEncoder().encode(encoded)
+ #if DEBUG
+ ReviewBackupControl.countWrite() // test hook
+ #endif
defaults.set(data, forKey: backupStatusesKey)
backupStatusesSubject.send(statuses)
diff --git a/Bitkit/Services/ReviewBackupControl.swift b/Bitkit/Services/ReviewBackupControl.swift
new file mode 100644
--- /dev/null
+++ b/Bitkit/Services/ReviewBackupControl.swift
@@ -0,0 +1,89 @@
+#if DEBUG // test hook
+import Foundation // test hook
+
+enum ReviewBackupControl { // test hook
+ static let enabled = ProcessInfo.processInfo.environment["REVIEW_BACKUP_CONTROL"] == "1" // test hook
+ static let directory = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0].appendingPathComponent("review-backup") // test hook
+ static let lock = NSLock() // test hook
+ static var writes = 0 // test hook
+ static func countWrite() { if enabled { lock.withLock { writes += 1 } } } // test hook
+
+ static func log(_ message: String) { // test hook
+ guard enabled else { return } // test hook
+ lock.withLock { // test hook
+ try? FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) // test hook
+ let file = directory.appendingPathComponent("events.log") // test hook
+ var data = (try? Data(contentsOf: file)) ?? Data() // test hook
+ data.append(Data("\(Date().timeIntervalSince1970) \(message)\n".utf8)) // test hook
+ try? data.write(to: file) // test hook
+ } // test hook
+ } // test hook
+
+ static func hold(_ name: String) async { // test hook
+ guard enabled else { return } // test hook
+ let file = directory.appendingPathComponent("hold-\(name)") // test hook
+ guard FileManager.default.fileExists(atPath: file.path) else { return } // test hook
+ log("held \(name)") // test hook
+ while FileManager.default.fileExists(atPath: file.path) { // test hook
+ await Task.detached { try? await Task.sleep(nanoseconds: 100_000_000) }.value // test hook
+ } // test hook
+ log("released \(name)") // test hook
+ } // test hook
+
+ @MainActor static func start() { // test hook
+ guard enabled else { return } // test hook
+ log("control-ready head=1ad665f") // test hook
+ Task { // test hook
+ let end = Date().addingTimeInterval(1200) // test hook
+ while Date() < end { // test hook
+ let file = directory.appendingPathComponent("command") // test hook
+ if let command = try? String(contentsOf: file, encoding: .utf8).trimmingCharacters(in: .whitespacesAndNewlines) { // test hook
+ try? FileManager.default.removeItem(at: file) // test hook
+ let service = BackupService.shared // test hook
+ log("command \(command)") // test hook
+ switch command { // test hook
+ case "stop": Task { // test hook
+ let before = Date() // test hook
+ await service.stopObservingBackups() // test hook
+ log("stop-done elapsed=\(Date().timeIntervalSince(before))") // test hook
+ } // test hook
+ case "start": service.startObservingBackups() // test hook
+ case "wallet": // test hook
+ do { try WatchOnlyAccountStore.save(WatchOnlyAccountStore.load()); log("wallet-record-written") } // test hook
+ catch { log("wallet-record-error") } // test hook
+ case "settings": SettingsViewModel.shared.swipeBalanceToHide.toggle() // test hook
+ case "manual": Task { // test hook
+ let result = await service.triggerBackup(category: .metadata) // test hook
+ log("manual-done result=\(result)") // test hook
+ } // test hook
+ case "usdt": service.markBackupRequired(category: .usdt) // test hook
+ case "vss-fault", "vss-normal": Task { // test hook
+ let route = directory.appendingPathComponent("vss-fault") // test hook
+ if command == "vss-fault" { try? Data().write(to: route) } // test hook
+ else { try? FileManager.default.removeItem(at: route) } // test hook
+ await VssBackupClient.shared.reset() // test hook
+ log("route-ready \(command)") // test hook
+ } // test hook
+ case "pending-wallet": service.markBackupRequired(category: .wallet) // test hook
+ case "status": // test hook
+ log("writes=\(lock.withLock { writes })") // test hook
+ for category in BackupCategory.allCases { // test hook
+ let s = service.getBackupStatus(category: category) // test hook
+ log("status \(category.rawValue) synced=\(s.synced) required=\(s.required) running=\(s.running)") // test hook
+ } // test hook
+ case "burst": // test hook
+ let count = lock.withLock { writes } // test hook
+ let before = Date() // test hook
+ for _ in 0 ..< 200 { CoreService.shared.activity.notifyActivitiesChanged() } // test hook
+ log("burst-200 elapsed=\(Date().timeIntervalSince(before)) writes=\(lock.withLock { writes } - count)") // test hook
+ case "end": log("control-ended"); return // test hook
+ default: break // test hook
+ } // test hook
+ } // test hook
+ try? await Task.sleep(nanoseconds: 100_000_000) // test hook
+ } // test hook
+ log("control-timeout") // test hook
+ } // test hook
+ } // test hook
+} // test hook
+#endif // test hook
diff --git a/Bitkit/Services/UsdtRecoveryBackupStore.swift b/Bitkit/Services/UsdtRecoveryBackupStore.swift
--- a/Bitkit/Services/UsdtRecoveryBackupStore.swift
+++ b/Bitkit/Services/UsdtRecoveryBackupStore.swift
@@ -27,7 +27,14 @@
let encoder = JSONEncoder()
encoder.outputFormatting = [.sortedKeys]
let data = try encoder.encode(UsdtRecoveryBackupV1(version: 1, usdtWallet: snapshot, paykitUsdtState: paymentState))
+ #if DEBUG // test hook
+ await ReviewBackupControl.hold("upload-USDT_RECOVERY") // test hook
+ ReviewBackupControl.log("usdt-recovery-put") // test hook
+ #endif // test hook
try await put(Self.key(for: data), data)
+ #if DEBUG // test hook
+ ReviewBackupControl.log("usdt-recovery-ack") // test hook
+ #endif // test hook
}
func snapshots() async throws -> [UsdtRecoveryBackupV1] {
diff --git a/Bitkit/Constants/Env.swift b/Bitkit/Constants/Env.swift
--- a/Bitkit/Constants/Env.swift
+++ b/Bitkit/Constants/Env.swift
@@ -372,7 +372,12 @@
}
static var vssServerUrl: String {
- switch network {
+ #if DEBUG // test hook
+ if ProcessInfo.processInfo.environment["REVIEW_BACKUP_CONTROL"] == "1", FileManager.default.fileExists(atPath: FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0].appendingPathComponent("review-backup/vss-fault").path) { // test hook
+ return "http://127.0.0.1:1" // test hook
+ } // test hook
+ #endif // test hook
+ return switch network {
case .bitcoin: "https://bitkit.to/vss_rs_auth"
default: "https://bitkit.stag0.blocktank.to/vss_rs_auth"
}
diff --git a/Bitkit/BitkitApp.swift b/Bitkit/BitkitApp.swift
--- a/Bitkit/BitkitApp.swift
+++ b/Bitkit/BitkitApp.swift
@@ -83,6 +83,9 @@
}
}
+ #if DEBUG
+ ReviewBackupControl.start() // test hook
+ #endif
return true
}
Exact-base comparison controls — local test hook, not pushed
Holds, logging, synthetic record edits and an endpoint-only outage control; the backup decisions and native transport results are unchanged.
diff --git a/Bitkit/Services/BackupService.swift b/Bitkit/Services/BackupService.swift
--- a/Bitkit/Services/BackupService.swift
+++ b/Bitkit/Services/BackupService.swift
@@ -262,6 +262,9 @@
}
private func runBackup(category: BackupCategory, isScheduled: Bool) async -> BackupOutcome {
+ #if DEBUG // test hook
+ ReviewBackupControl.log("capture-start \(category.rawValue)") // test hook
+ #endif // test hook
Logger.debug("Backup starting for: '\(category.rawValue)'", context: "BackupService")
var outcome = BackupOutcome.failed
var requiredAtStart: UInt64 = 0
@@ -319,6 +322,9 @@
}
}
_ = try? await ServiceQueue.background(.backup) { self.runningBackupTasks.removeValue(forKey: category) }
+ #if DEBUG // test hook
+ ReviewBackupControl.log("backup-done \(category.rawValue) result=\(outcome)") // test hook
+ #endif // test hook
return outcome
}
@@ -829,6 +835,9 @@
while !Task.isCancelled {
do {
if outcome == .temporarilyUnavailable {
+ #if DEBUG // test hook
+ ReviewBackupControl.log("retry \(category.rawValue) delay=\(retryDelay)") // test hook
+ #endif // test hook
try await waitForTemporaryFailureDelay(retryDelay)
retryDelay = min(retryDelay * 2, 300_000_000_000)
} else {
@@ -988,6 +997,9 @@
do {
let data = try JSONEncoder().encode(encoded)
+ #if DEBUG // test hook
+ ReviewBackupControl.countWrite() // test hook
+ #endif // test hook
defaults.set(data, forKey: backupStatusesKey)
backupStatusesSubject.send(statuses)
diff --git a/Bitkit/Services/ReviewBackupControl.swift b/Bitkit/Services/ReviewBackupControl.swift
new file mode 100644
--- /dev/null
+++ b/Bitkit/Services/ReviewBackupControl.swift
@@ -0,0 +1,108 @@
+#if DEBUG // test hook
+import Foundation // test hook
+import LDKNode // test hook
+
+enum ReviewBackupControl { // test hook
+ static let enabled = ProcessInfo.processInfo.environment["REVIEW_BACKUP_CONTROL"] == "1" // test hook
+ static let directory = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0].appendingPathComponent("review-backup") // test hook
+ static let lock = NSLock() // test hook
+ static var writes = 0 // test hook
+ static func countWrite() { if enabled { lock.withLock { writes += 1 } } } // test hook
+
+ static func log(_ message: String) { // test hook
+ guard enabled else { return } // test hook
+ lock.withLock { // test hook
+ try? FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) // test hook
+ let file = directory.appendingPathComponent("events.log") // test hook
+ var data = (try? Data(contentsOf: file)) ?? Data() // test hook
+ data.append(Data("\(Date().timeIntervalSince1970) \(message)\n".utf8)) // test hook
+ try? data.write(to: file) // test hook
+ } // test hook
+ } // test hook
+
+ static func hold(_ name: String) async { // test hook
+ guard enabled else { return } // test hook
+ let file = directory.appendingPathComponent("hold-\(name)") // test hook
+ guard FileManager.default.fileExists(atPath: file.path) else { return } // test hook
+ log("held \(name)") // test hook
+ while FileManager.default.fileExists(atPath: file.path) { // test hook
+ await Task.detached { try? await Task.sleep(nanoseconds: 100_000_000) }.value // test hook
+ } // test hook
+ log("released \(name)") // test hook
+ } // test hook
+
+ @MainActor static func start() { // test hook
+ guard enabled else { return } // test hook
+ log("control-ready base=ec20b264") // test hook
+ Task { // test hook
+ let end = Date().addingTimeInterval(1200) // test hook
+ while Date() < end { // test hook
+ let file = directory.appendingPathComponent("command") // test hook
+ if let command = try? String(contentsOf: file, encoding: .utf8).trimmingCharacters(in: .whitespacesAndNewlines) { // test hook
+ try? FileManager.default.removeItem(at: file) // test hook
+ let service = BackupService.shared // test hook
+ log("command \(command)") // test hook
+ switch command { // test hook
+ case "stop": Task { // test hook
+ let before = Date() // test hook
+ await service.stopObservingBackups() // test hook
+ log("stop-done elapsed=\(Date().timeIntervalSince(before))") // test hook
+ } // test hook
+ case "start": service.startObservingBackups() // test hook
+ case "wallet": // test hook
+ do { // test hook
+ var records = try WatchOnlyAccountStore.load() // test hook
+ if records.isEmpty { // test hook
+ records.append(WatchOnlyAccountRecord(id: UUID(), walletIndex: 0, accountIndex: 42, addressType: LDKNode.AddressType.nativeSegwit.stringValue, xpub: "tpubDDWohsp5dx2iMJ9N7iHbgAEDhH4BJB9NWW1fEW3yA3AFNDREmpzteCXNqppMLUmKFY5q5e3PXtS5CuqWCQbYcGhpPqYAgQSYdwknW9J6sQv", requestFingerprint: "review", createdAt: 1000, name: "review", isTrackingEnabled: false, setupState: .pendingDelivery)) // test hook
+ } // test hook
+ records[0].name = "review-\(Int(Date().timeIntervalSince1970))" // test hook
+ try WatchOnlyAccountStore.save(records); log("wallet-record-written name=\(records[0].name)") // test hook
+ } // test hook
+ catch { log("wallet-record-error") } // test hook
+ case "settings": SettingsViewModel.shared.swipeBalanceToHide.toggle() // test hook
+ case "manual": Task { // test hook
+ let result = await service.triggerBackup(category: .metadata) // test hook
+ log("manual-done result=\(result)") // test hook
+ } // test hook
+ case "usdt": service.markBackupRequired(category: .usdt) // test hook
+ case "vss-fault", "vss-normal": Task { // test hook
+ let route = directory.appendingPathComponent("vss-fault") // test hook
+ if command == "vss-fault" { try? Data().write(to: route) } // test hook
+ else { try? FileManager.default.removeItem(at: route) } // test hook
+ await VssBackupClient.shared.reset() // test hook
+ log("route-ready \(command)") // test hook
+ } // test hook
+ case "pending-wallet": service.markBackupRequired(category: .wallet) // test hook
+ case "status": // test hook
+ log("writes=\(lock.withLock { writes })") // test hook
+ for category in BackupCategory.allCases { // test hook
+ let s = service.getBackupStatus(category: category) // test hook
+ log("status \(category.rawValue) synced=\(s.synced) required=\(s.required) running=\(s.running)") // test hook
+ } // test hook
+ case "verify": Task { // test hook
+ do { // test hook
+ let settings = try await VssBackupClient.shared.getObject(key: "SETTINGS") // test hook
+ let payload = try SettingsBackupV1.decode(from: settings!.value) // test hook
+ let remote = payload.settings["enableSwipeToHideBalance"] as? Bool // test hook
+ log("verify-settings matches=\(remote == SettingsViewModel.shared.swipeBalanceToHide) value=\(String(describing: remote))") // test hook
+ let wallet = try await VssBackupClient.shared.getObject(key: "WALLET") // test hook
+ let data = try JSONDecoder().decode(WalletBackupV1.self, from: wallet!.value) // test hook
+ log("verify-wallet matches=\(data.watchOnlyAccounts == (try WatchOnlyAccountStore.load()))") // test hook
+ } catch { log("verify-error") } // test hook
+ } // test hook
+ case "burst": // test hook
+ let count = lock.withLock { writes } // test hook
+ let before = Date() // test hook
+ for _ in 0 ..< 200 { CoreService.shared.activity.notifyActivitiesChanged() } // test hook
+ log("burst-200 elapsed=\(Date().timeIntervalSince(before)) writes=\(lock.withLock { writes } - count)") // test hook
+ case "end": log("control-ended"); return // test hook
+ default: break // test hook
+ } // test hook
+ } // test hook
+ try? await Task.sleep(nanoseconds: 100_000_000) // test hook
+ } // test hook
+ log("control-timeout") // test hook
+ } // test hook
+ } // test hook
+} // test hook
+#endif // test hook
diff --git a/Bitkit/Services/UsdtRecoveryBackupStore.swift b/Bitkit/Services/UsdtRecoveryBackupStore.swift
--- a/Bitkit/Services/UsdtRecoveryBackupStore.swift
+++ b/Bitkit/Services/UsdtRecoveryBackupStore.swift
@@ -27,7 +27,14 @@
let encoder = JSONEncoder()
encoder.outputFormatting = [.sortedKeys]
let data = try encoder.encode(UsdtRecoveryBackupV1(version: 1, usdtWallet: snapshot, paykitUsdtState: paymentState))
+ #if DEBUG // test hook
+ await ReviewBackupControl.hold("upload-USDT_RECOVERY") // test hook
+ ReviewBackupControl.log("usdt-recovery-put") // test hook
+ #endif // test hook
try await put(Self.key(for: data), data)
+ #if DEBUG // test hook
+ ReviewBackupControl.log("usdt-recovery-ack") // test hook
+ #endif // test hook
}
func snapshots() async throws -> [UsdtRecoveryBackupV1] {
diff --git a/Bitkit/Constants/Env.swift b/Bitkit/Constants/Env.swift
--- a/Bitkit/Constants/Env.swift
+++ b/Bitkit/Constants/Env.swift
@@ -372,7 +372,12 @@
}
static var vssServerUrl: String {
- switch network {
+ #if DEBUG // test hook
+ if ProcessInfo.processInfo.environment["REVIEW_BACKUP_CONTROL"] == "1", FileManager.default.fileExists(atPath: FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0].appendingPathComponent("review-backup/vss-fault").path) { // test hook
+ return "http://127.0.0.1:1" // test hook
+ } // test hook
+ #endif // test hook
+ return switch network {
case .bitcoin: "https://bitkit.to/vss_rs_auth"
default: "https://bitkit.stag0.blocktank.to/vss_rs_auth"
}
diff --git a/Bitkit/BitkitApp.swift b/Bitkit/BitkitApp.swift
--- a/Bitkit/BitkitApp.swift
+++ b/Bitkit/BitkitApp.swift
@@ -83,6 +83,9 @@
}
}
+ #if DEBUG
+ ReviewBackupControl.start() // test hook
+ #endif
return true
}
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
|
Addressed review 5657415320 in signed commit 494c949. The backup-only classifier recognizes the released VSS request-send UnknownError envelope without retrying arbitrary Unknown/auth/decode errors. Status updates still advance the capture revision before skipping unchanged persistence/publication. Controlled before tests fail for both reported behaviors; final 2,912 units, full SwiftFormat 0/847, six released-native rejection/lost-ack prepare/resume/restore phases, and two independent final reviews pass. All four live-base commits verify locally and on GitHub. Complete diagnostics retain the AppIntents/QoS observations with unproven timing causes. The native phases prove acknowledgement/same-operation recovery, not a full live-endpoint automatic-backoff journey; the injected error tests cover that scheduler behavior. Current-head hosted checks are running. Android twin #1689 has the same narrow transport classifier; its separate stopped-observer explicit retry limitation remains disclosed and is being repaired. |
|
Published startup follow-up ff999a5 after the hosted repeated-fulfillment failure. A controlled real SwiftUI case reproduced two completion callbacks when progress normalizes to 100; the shared completion boundary now requires Running and an unset existing latch. The original slow-retry assertions and deadlines are unchanged. Final 2,913 selected unit tests and full SwiftFormat (0/847) pass, all frozen inputs match, and two consecutive independent reviews are clean. All five live-base commits verify locally and GitHub true/valid. Complete logs/nested diagnostics and the earlier disk-exhaustion failure are retained; the exact hosted observer ordering is not claimed proven. The PR body records both revision-scoped evidence and remaining manual-journey limits. Current-head hosted checks are pending. Existing reviewer requests are preserved. |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Reviewed the full PR diff against merge base ec20b264, at ff999a50.
No new actionable code findings.
Stop and start share one MainActor observation task. Stop increments the scheduling generation, cancels queued and running backups, and waits for those tasks plus both writers before it returns. A schedule from the stopped generation exits. Changes received during cleanup stay required and upload after the replacement observation is admitted. Cancelling a settings backup keeps a newer required marker. Explicit USDT and manual backups wait through that drain and can finish after observation has stopped. Wallet initialization admits its completion callback once: both progress handlers share the existing latch, and completion still requires the node to be running.
The request-send classifier accepts the released VssError.UnknownError text recorded on 1ad665f8 in review 5657415320 (error sending request for url (http://127.0.0.1:1/putObjects)) and rejects the non-send, auth, and decode samples in BackupServiceTests. That device failure predates the classifier. Identical backup-status values are no longer rewritten; the in-memory capture revision still advances. The 200-write burst in that review was measured on 1ad665f. Restore suppression from the base is unchanged, and triggerBackup still performs the migration rewrite during restore.
Compared the shared drain, pending-change, transport-retry, and stopped explicit-USDT contracts with bitkit-android#1689 at 49e9a99d. bitkit-ios#1122 at ca10ace7 shares merge base 1ad665f; only that overlap was checked. Its profile-deletion commits were not reviewed.
GitHub Run Tests and SwiftFormat succeeded on this head. E2E jobs were skipped. The new backup, initialization, and USDT navigation tests were inspected and not executed in this review.
Device testing: not performed in this review. Manual cases 1–4 remain the device plan; case 2 needs a run on this head because the earlier outage result is from 1ad665f.
Ready for device testing.
Reviewer model: grok-4.7
Reasoning effort: high
There was a problem hiding this comment.
Code scan of 494c949 and ff999a5: no findings.
Checked and clean:
- The transport-failure classifier needs the request-error prefix, an http(s) URL with a host and the
/putObjectspath; HTTP status, decode, auth and cancellation errors stay permanent. - The coalesced status write doesn't change observer behaviour:
backupStatusesPublisheralready hadremoveDuplicates, andbackupVersionsstill advances inside theupdateclosure before the guard. ff999a5completes wallet initialization once:handleCompletionis guarded onshouldFinish && !shouldStopAnimationand bothonChangehandlers share it; reaching 100 % before.running, or.runningfirst, both still complete.- Twin synonymdev/bitkit-android#1689 classifies the same
/putObjectstransport failures.
Performance-relevant (write coalescing). The 200-write burst was measured before 494c949, so it needs measuring again at this head.
Device gate (partial): ff999a5 — not run yet; pending the burst write count (expected ≤2 vs 200 before), manual case 2, and Home opening once after wallet creation. Device results will follow in a separate review.
There was a problem hiding this comment.
Verdict: ✅ Approve — the previously failed native-outage case now passes.
Findings: I found no actionable defect in the delta. The native VSS request-send error now receives automatic retry with observation stopped. Identical status updates retain capture revisions without rewriting the same durable status.
QA for ff999a5: iPhone 17 Pro simulator, iOS 26.5, Xcode 27. Initialization and component tests used the unmodified head. Lifecycle and burst checks used local Debug holds, endpoint routing and logging over that head, a disposable unfunded wallet and staging VSS. No payment was submitted.
Test 1: cleanup, manual admission and restart ✅ Stop and manual backup waited for cleanup; the latest wallet and settings state reached VSS.
I held scheduled cleanup, requested stop and a manual metadata backup, queued restart, and edited wallet/settings state. No capture started before release. I also canceled a held settings upload and changed settings during restart. Hook logs and decrypted VSS reads confirmed both final values matched local state.
lifecycle-12x.2x.mp4
Local lifecycle controls and their event log; the remote-payload checks appear in the overlay.
Test 2: stopped USDT recovery and native outage ✅ Recovery waited for cleanup and automatically completed after the VSS endpoint recovered, without restarting observation.
A recovery request made during cleanup completed after release. With observation still stopped, I made another request against an unreachable VSS endpoint. The released native binding returned the same UnknownError recorded in the earlier failed test. This head classified it as temporary and retried after ten seconds. Restoring the endpoint was the only intervention; no new recovery request or observation start was issued.
lifecycle-22x.2x.mp4
Endpoint-only outage and recovery; native outcomes and retry timing are checked through the displayed hook log.
backup-error USDT_RECOVERY UnknownError(errorDetails: "error sending request for url (http://127.0.0.1:1/putObjects)")
backup-done USDT_RECOVERY result=temporarilyUnavailable
retry USDT_RECOVERY delay=10000000000
route-ready vss-normal
usdt-recovery-ack
backup-done USDT_RECOVERY result=completed
The component regression separately checks the full 10/20/40/80/160/300-second capped sequence. The live outage run checks its first retry, not several minutes of backoff or payment acknowledgement.
Test 3: repeated observation lifecycle ✅ Stopped listeners ignored later wallet edits, and a received settings change survived restart.
I ran 25 start/stop cycles with a distinct wallet-record edit after each stop. Hook-log comparisons showed unchanged wallet status and no wallet capture after those edits. A settings event received before stop remained pending and uploaded after restart.
lifecycle-32x.2x.mp4
Lifecycle commands, distinct record edits and status comparisons in the local control overlay.
Test 4: initialization completion ✅ Both hosted SwiftUI regressions passed, and creating a fresh wallet opened Home normally.
InitializingWalletViewTests passed without skips. The threshold regression counted one completion callback; the slow/error retry case retained its existing assertions and deadlines. I also created a fresh wallet through the app UI on the unmodified head and reached Home. Callback cardinality is established by the hosted test, rather than inferred from the Home recording.
| Hosted SwiftUI regressions | Fresh wallet startup |
|---|---|
initialization2x.2x.mp4 |
startup2x.2x.mp4 |
Hosted app execution checked against finalized XCResult; ordinary onboarding and Home on the right.
Test 5: backup component regressions ✅ The focused backup suite passed without skips.
BackupServiceTests passed on the unmodified head, including native-shaped error classification, capped retry with observation stopped, unchanged-status persistence, cancellation, final-write drain and pending changes during capture/upload. I checked the assertions and finalized XCResult; neither focused suite recorded a runtime warning.
regressions2x.2x.mp4
App-hosted XCTest execution; pass/failure and skip results were checked in XCResult.
Test 6: activity-notification burst ✅ Each burst of 200 activity notifications caused one status write, versus 200 in the earlier head measurement.
I drove the actual activity publisher twice. Each burst caused one durable status write and took about 0.05–0.06 seconds to dispatch. This satisfies the write-count check requested in the latest review. The comparison uses the earlier revision's recorded measurement; this measures notification handling, not a complete payment-history sync or universal UI latency.
performance-head2x.2x.mp4
Activity-publisher controls and counted status writes.
All four manual cases were exercised. I retained the earlier passing USDT navigation evidence: this delta leaves its implementation and UI tests untouched. I did not rerun funded payments, process-death recovery or cross-platform restore.
Delta review: 1ad665f..ff999a5, all five changed files and their lifecycle/status/initialization callers, plus the prior outage and write-count concerns. Runtime cases cover the changed Swift behavior; the changelog was inspected.
Twin: bitkit-android#1689 fix: preserve pending backups across restart at 49e9a99d matches the narrow request-send classifier and stopped explicit-retry contract; Android retains its own reset/job ownership.
Local Debug test controls — not pushed
Holds cleanup and uploads, edits disposable records, counts status writes, displays the event log and routes only custom VSS to an unreachable endpoint. Backup decisions, retry delays and native transport results are unchanged.
diff --git a/Bitkit/BitkitApp.swift b/Bitkit/BitkitApp.swift
index 961935aa..bfc5d58b 100644
--- a/Bitkit/BitkitApp.swift
+++ b/Bitkit/BitkitApp.swift
@@ -83,6 +83,9 @@ class AppDelegate: NSObject, UIApplicationDelegate {
}
}
+ #if DEBUG
+ ReviewBackupControl.start() // test hook
+ #endif
return true
}
diff --git a/Bitkit/Constants/Env.swift b/Bitkit/Constants/Env.swift
index 76885696..44d6d41a 100644
--- a/Bitkit/Constants/Env.swift
+++ b/Bitkit/Constants/Env.swift
@@ -372,7 +372,12 @@ enum Env {
}
static var vssServerUrl: String {
- switch network {
+ #if DEBUG // test hook
+ if ProcessInfo.processInfo.environment["REVIEW_BACKUP_CONTROL"] == "1", FileManager.default.fileExists(atPath: FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0].appendingPathComponent("review-backup/vss-fault").path) { // test hook
+ return "http://127.0.0.1:1" // test hook
+ } // test hook
+ #endif // test hook
+ return switch network {
case .bitcoin: "https://bitkit.to/vss_rs_auth"
default: "https://bitkit.stag0.blocktank.to/vss_rs_auth"
}
diff --git a/Bitkit/Services/BackupService.swift b/Bitkit/Services/BackupService.swift
index 03f12571..05c58934 100644
--- a/Bitkit/Services/BackupService.swift
+++ b/Bitkit/Services/BackupService.swift
@@ -155,7 +155,17 @@ class BackupService {
backupData: ((BackupCategory) async throws -> Data)? = nil,
usdtRecoveryBackup: UsdtRecoveryBackupStore = UsdtRecoveryBackupStore(),
uploadBackup: @escaping (String, Data) async throws -> Void = { key, data in
+ #if DEBUG
+ await ReviewBackupControl.hold("upload-\(key)") // test hook
+ if key == "SETTINGS", let payload = try? SettingsBackupV1.decode(from: data) { // test hook
+ ReviewBackupControl.log("upload-settings swipe=\(payload.settings["enableSwipeToHideBalance"] ?? "missing")") // test hook
+ } // test hook
+ ReviewBackupControl.log("upload-start \(key)") // test hook
+ #endif
_ = try await VssBackupClient.shared.putObject(key: key, data: data)
+ #if DEBUG
+ ReviewBackupControl.log("upload-ack \(key)") // test hook
+ #endif
},
waitForBackupDelay: @escaping () async throws -> Void = {
try await Task.sleep(nanoseconds: 5_000_000_000)
@@ -292,6 +302,9 @@ class BackupService {
}
private func runBackup(category: BackupCategory, isScheduled: Bool) async -> BackupOutcome {
+ #if DEBUG
+ ReviewBackupControl.log("capture-start \(category.rawValue)") // test hook
+ #endif
Logger.debug("Backup starting for: '\(category.rawValue)'", context: "BackupService")
var outcome = BackupOutcome.failed
var requiredAtStart: UInt64 = 0
@@ -335,6 +348,9 @@ class BackupService {
)
}
} catch {
+ #if DEBUG // test hook
+ ReviewBackupControl.log("backup-error \(category.rawValue) \(error)") // test hook
+ #endif // test hook
updateBackupStatus(category: category) { status in
BackupItemStatus(synced: status.synced, required: status.required, running: false)
}
@@ -349,6 +365,9 @@ class BackupService {
}
}
_ = try? await ServiceQueue.background(.backup) { self.runningBackupTasks.removeValue(forKey: category) }
+ #if DEBUG
+ ReviewBackupControl.log("backup-done \(category.rawValue) result=\(outcome)") // test hook
+ #endif
return outcome
}
@@ -870,6 +889,9 @@ class BackupService {
while !Task.isCancelled {
do {
if outcome == .temporarilyUnavailable {
+ #if DEBUG // test hook
+ ReviewBackupControl.log("retry \(category.rawValue) delay=\(retryDelay)") // test hook
+ #endif // test hook
try await waitForTemporaryFailureDelay(retryDelay)
retryDelay = min(retryDelay * 2, 300_000_000_000)
} else {
@@ -886,6 +908,9 @@ class BackupService {
break
}
}
+ #if DEBUG
+ await ReviewBackupControl.hold("cleanup") // test hook
+ #endif
updateBackupStatus(category: category) { status in
BackupItemStatus(synced: status.synced, required: status.required, running: false)
}
@@ -1032,6 +1057,9 @@ class BackupService {
do {
let data = try JSONEncoder().encode(encoded)
+ #if DEBUG
+ ReviewBackupControl.countWrite() // test hook
+ #endif
defaults.set(data, forKey: backupStatusesKey)
backupStatusesSubject.send(statuses)
diff --git a/Bitkit/Services/ReviewBackupControl.swift b/Bitkit/Services/ReviewBackupControl.swift
new file mode 100644
index 00000000..25579bbc
--- /dev/null
+++ b/Bitkit/Services/ReviewBackupControl.swift
@@ -0,0 +1,119 @@
+#if DEBUG // test hook
+import Foundation // test hook
+import LDKNode // test hook
+import UIKit // test hook
+
+enum ReviewBackupControl { // test hook
+ static let enabled = ProcessInfo.processInfo.environment["REVIEW_BACKUP_CONTROL"] == "1" // test hook
+ static let directory = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0].appendingPathComponent("review-backup") // test hook
+ static let lock = NSLock() // test hook
+ static var writes = 0 // test hook
+ @MainActor static var display: UILabel? // test hook
+ static func countWrite() { if enabled { lock.withLock { writes += 1 } } } // test hook
+
+ static func log(_ message: String) { // test hook
+ guard enabled else { return } // test hook
+ lock.withLock { // test hook
+ try? FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) // test hook
+ let file = directory.appendingPathComponent("events.log") // test hook
+ var data = (try? Data(contentsOf: file)) ?? Data() // test hook
+ data.append(Data("\(Date().timeIntervalSince1970) \(message)\n".utf8)) // test hook
+ try? data.write(to: file) // test hook
+ } // test hook
+ } // test hook
+
+ static func hold(_ name: String) async { // test hook
+ guard enabled else { return } // test hook
+ let file = directory.appendingPathComponent("hold-\(name)") // test hook
+ guard FileManager.default.fileExists(atPath: file.path) else { return } // test hook
+ log("held \(name)") // test hook
+ while FileManager.default.fileExists(atPath: file.path) { // test hook
+ await Task.detached { try? await Task.sleep(nanoseconds: 100_000_000) }.value // test hook
+ } // test hook
+ log("released \(name)") // test hook
+ } // test hook
+
+ @MainActor static func start() { // test hook
+ guard enabled else { return } // test hook
+ log("control-ready head=ff999a5") // test hook
+ Task { // test hook
+ let end = Date().addingTimeInterval(1200) // test hook
+ while Date() < end { // test hook
+ if display == nil, let scene = UIApplication.shared.connectedScenes.first as? UIWindowScene, let window = scene.windows.first(where: { $0.isKeyWindow }) { // test hook
+ let label = UILabel(frame: CGRect(x: 12, y: 110, width: window.bounds.width - 24, height: 240)) // test hook
+ label.font = .monospacedSystemFont(ofSize: 11, weight: .regular) // test hook
+ label.textColor = .white; label.backgroundColor = UIColor.black.withAlphaComponent(0.9) // test hook
+ label.numberOfLines = 0; label.lineBreakMode = .byCharWrapping; label.isUserInteractionEnabled = false // test hook
+ window.addSubview(label); display = label // test hook
+ } // test hook
+ let recent = (try? String(contentsOf: directory.appendingPathComponent("events.log"), encoding: .utf8)) ?? "" // test hook
+ display?.text = "ff999a5 · local review controls\n" + recent.split(separator: "\n").suffix(8).joined(separator: "\n") // test hook
+ let file = directory.appendingPathComponent("command") // test hook
+ if let command = try? String(contentsOf: file, encoding: .utf8).trimmingCharacters(in: .whitespacesAndNewlines) { // test hook
+ try? FileManager.default.removeItem(at: file) // test hook
+ let service = BackupService.shared // test hook
+ log("command \(command)") // test hook
+ switch command { // test hook
+ case "stop": Task { // test hook
+ let before = Date() // test hook
+ await service.stopObservingBackups() // test hook
+ log("stop-done elapsed=\(Date().timeIntervalSince(before))") // test hook
+ } // test hook
+ case "start": service.startObservingBackups() // test hook
+ case "wallet": // test hook
+ do { // test hook
+ var records = try WatchOnlyAccountStore.load() // test hook
+ if records.isEmpty { // test hook
+ records.append(WatchOnlyAccountRecord(id: UUID(), walletIndex: 0, accountIndex: 42, addressType: LDKNode.AddressType.nativeSegwit.stringValue, xpub: "tpubDDWohsp5dx2iMJ9N7iHbgAEDhH4BJB9NWW1fEW3yA3AFNDREmpzteCXNqppMLUmKFY5q5e3PXtS5CuqWCQbYcGhpPqYAgQSYdwknW9J6sQv", requestFingerprint: "review", createdAt: 1000, name: "review", isTrackingEnabled: false, setupState: .pendingDelivery)) // test hook
+ } // test hook
+ records[0].name = "review-\(Int(Date().timeIntervalSince1970))" // test hook
+ try WatchOnlyAccountStore.save(records); log("wallet-record-written name=\(records[0].name)") // test hook
+ } // test hook
+ catch { log("wallet-record-error") } // test hook
+ case "settings": SettingsViewModel.shared.swipeBalanceToHide.toggle() // test hook
+ case "manual": Task { // test hook
+ let result = await service.triggerBackup(category: .metadata) // test hook
+ log("manual-done result=\(result)") // test hook
+ } // test hook
+ case "usdt": service.markBackupRequired(category: .usdt) // test hook
+ case "vss-fault", "vss-normal": Task { // test hook
+ let route = directory.appendingPathComponent("vss-fault") // test hook
+ if command == "vss-fault" { try? Data().write(to: route) } // test hook
+ else { try? FileManager.default.removeItem(at: route) } // test hook
+ await VssBackupClient.shared.reset() // test hook
+ log("route-ready \(command)") // test hook
+ } // test hook
+ case "pending-wallet": service.markBackupRequired(category: .wallet) // test hook
+ case "status": // test hook
+ log("writes=\(lock.withLock { writes })") // test hook
+ for category in BackupCategory.allCases { // test hook
+ let s = service.getBackupStatus(category: category) // test hook
+ log("status \(category.rawValue) synced=\(s.synced) required=\(s.required) running=\(s.running)") // test hook
+ } // test hook
+ case "verify": Task { // test hook
+ do { // test hook
+ let settings = try await VssBackupClient.shared.getObject(key: "SETTINGS") // test hook
+ let payload = try SettingsBackupV1.decode(from: settings!.value) // test hook
+ let remote = payload.settings["enableSwipeToHideBalance"] as? Bool // test hook
+ log("verify-settings matches=\(remote == SettingsViewModel.shared.swipeBalanceToHide) value=\(String(describing: remote))") // test hook
+ let wallet = try await VssBackupClient.shared.getObject(key: "WALLET") // test hook
+ let data = try JSONDecoder().decode(WalletBackupV1.self, from: wallet!.value) // test hook
+ log("verify-wallet matches=\(data.watchOnlyAccounts == (try WatchOnlyAccountStore.load()))") // test hook
+ } catch { log("verify-error") } // test hook
+ } // test hook
+ case "burst": // test hook
+ let count = lock.withLock { writes } // test hook
+ let before = Date() // test hook
+ for _ in 0 ..< 200 { CoreService.shared.activity.notifyActivitiesChanged() } // test hook
+ log("burst-200 elapsed=\(Date().timeIntervalSince(before)) writes=\(lock.withLock { writes } - count)") // test hook
+ case "end": log("control-ended"); return // test hook
+ default: break // test hook
+ } // test hook
+ } // test hook
+ try? await Task.sleep(nanoseconds: 100_000_000) // test hook
+ } // test hook
+ log("control-timeout") // test hook
+ } // test hook
+ } // test hook
+} // test hook
+#endif // test hook
diff --git a/Bitkit/Services/UsdtRecoveryBackupStore.swift b/Bitkit/Services/UsdtRecoveryBackupStore.swift
index f804d076..f546805c 100644
--- a/Bitkit/Services/UsdtRecoveryBackupStore.swift
+++ b/Bitkit/Services/UsdtRecoveryBackupStore.swift
@@ -27,7 +27,14 @@ struct UsdtRecoveryBackupStore {
let encoder = JSONEncoder()
encoder.outputFormatting = [.sortedKeys]
let data = try encoder.encode(UsdtRecoveryBackupV1(version: 1, usdtWallet: snapshot, paykitUsdtState: paymentState))
+ #if DEBUG // test hook
+ await ReviewBackupControl.hold("upload-USDT_RECOVERY") // test hook
+ ReviewBackupControl.log("usdt-recovery-put") // test hook
+ #endif // test hook
try await put(Self.key(for: data), data)
+ #if DEBUG // test hook
+ ReviewBackupControl.log("usdt-recovery-ack") // test hook
+ #endif // test hook
}
func snapshots() async throws -> [UsdtRecoveryBackupV1] {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
|
Device gate (partial): Burst: 200 ×
Still pending: the head moved to |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of 24160466 against 39fcaec8, also its merge base. The comparison base changed and overlaps the USDT navigation tests; the complete current PR was inspected without inherited coverage.
No new actionable code findings.
Validation: git diff --check passed locally. Inspected CI test logs: all 22 backup cases, both initialization regressions and both affected USDT navigation cases passed. SwiftFormat 0.64.0 reported no formatting violations. The tested merge tree matches the pinned head. The native recovery fixture and fork-payment case were skipped, so these checks do not establish authenticated backup-fault acceptance.
The earlier native-outage and status-write concerns are addressed in the pinned source. The reported simulator retest explicitly tested ff999a5, before the current dependency composition; it remains evidence for that older revision.
Device testing: not performed in this review. Swift/Xcode are unavailable on this Linux host. Author manual cases 1–4 remain the current-head acceptance plan, using the disposable fault fixture described in the PR; full app/Paykit/deployed-contract recovery was not exercised here.
Suggested additional test cases
- iOS component fixture: hold shutdown cleanup, start a manual metadata backup, cancel its caller before releasing cleanup, then release it. Expect no upload from the cancelled request.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of the complete PR at 14152f41 against merge base 754a8991. No prior coverage was inherited: the comparison base changed from 39fcaec to 754a899 and no baseline inventory was supplied.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
Stopping observation drops data-store subscriptions before in-flight backup work finishes, so an edit in that window can be left out and a put that still returns can record the older snapshot as synced. Wallet initialization now admits completion once. The VSS request-send classifier matches the released UnknownError text.
Validation: BackupServiceTests and InitializingWalletViewTests were inspected at this revision and were not executed here. Run Tests and SwiftFormat succeeded on this head. Device testing: not performed in this review.
Parity: Android 49e9a99d (synonymdev/bitkit-android#1689) uses the same /putObjects request-send rule and records drain-time edits after a replacement observation has subscribed. Companion bitkit-ios#1122 at 7f54d7b9 does not change this backup path. Dependency bitkit-ios#1121 is this merge base and was not re-reviewed.
Suggested additional test cases
- iOS component test. Hold
uploadBackupafter capture for a required category. CallstopObservingBackupsand do not callstartObservingBackups. Change that category's source, and change a second category that was already synced, while the put is held. Release the put. Both categories stay required, and the nextstartObservingBackupsuploads their latest values without another edit.
Findings
- [MEDIUM] Record backup edits that arrive while observation is stopping — inline at
Bitkit/Services/BackupService.swift:227.
Reviewer model: grok-4.7
Reasoning effort: high
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approving. I re-ran the device gate on the head 14152f4, which brings VSS client 0.5.26-rc1 and Paykit 0.1.1-rc2 onto the tested path. Both checks match the earlier ff999a5 run.
- Burst: 200 ×
notifyActivitiesChanged()through a local Debug hook (never pushed), three runs.- Each run made 1
backupStatuseswrite during dispatch, in 0.033–0.051 s. Atff999a5it was 1 write in 0.043–0.050 s. - The 10 s window including the following ACTIVITY backup had 5, 6 and 5 writes (
ff999a5: 5, 5, 5; baseec20b26: 6–8). Run 2's extra write was an idle write with no hook event.
- Each run made 1
- Manual case 2: a USDT request made while cleanup was held waited,
stopfinished in 8.9 s, and the USDT recovery backup completed 5 s after release.- With observation stopped and VSS on a dead port, the 0.5.26-rc1 client still reports
UnknownError(… error sending request …). It is classed as temporary and retried after 10 s and 20 s, andusdt-recovery-ack/result=completedarrived once VSS was back, with no new request. - No observation restart after
stop.
- With observation stopped and VSS on a dead port, the 0.5.26-rc1 client still reports
Device gate: 14152f4. Journeys: none listed, manual cases 1–2 pass. Perf: burst 1 write in ~0.04 s (base 0 writes, debounced; the earlier head 1ad665f made 200). iOS simulator only; this is iOS-only code.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Re-reviewed the full PR at e12df99 after the comparison base changed from 754a8991 to 73b7d58, which also updates BackupService.swift and UsdtWalletUITests.swift. No coverage baseline was supplied, so this pass reassessed the whole diff from that merge base and inherited nothing.
No new actionable code findings.
Observation start and stop are chained on the main actor. Stop invalidates the scheduling generation, cancels queued and running backups, and drops data-store listeners only after those tasks and both backup writers drain. Edits received during that drain stay required and are not uploaded by the stopped generation; restart and an explicit USDT retry pick them up, including after Lightning observation has stopped. Cancellation keeps a newer required marker. Foreground recovery from bitkit-ios#1121 still uses the observation flag from entry and that same generation, so a recovery that starts while not observing, or that is stopped, does not upload. The released VSS unknown-error retry accepts only an HTTP(S) putObjects request-send envelope, the same predicate as bitkit-android#1689 at 49e9a99. Wallet initialization completion is admitted once. bitkit-ios#1122 at 162ea52 is based on this head and does not change these files; that was a contract overlap check, not a review of the profile-deletion fix.
Hosted Run Tests and SwiftFormat passed on this revision. This review did not re-run them. E2E jobs in run 40580241814 were skipped. Device testing: not performed in this review. Manual cases 1–4 remain the fault-injection checks and are outside the hosted unit run.
Ready for device testing.
Earlier findings reconciled at this revision: 1 verified fixed.
Reviewer model: grok-4.7
Reasoning effort: high
There was a problem hiding this comment.
Verdict: ✅ Approve — no new findings; the remaining expired-rate amount check now passes.
Findings: I found no actionable defect in the delta. The backup lifecycle and shutdown-drain fix remain intact. The metadata changes remove obsolete try tokens after the retired Blocktank cache fields were removed.
QA for 566b2ff: iPhone 17 Pro simulator, iOS 26.5, Xcode 27. The backup and navigation regressions ran on the unmodified head. The expired-rate case used an uncommitted Debug fixture presenting the production amount screen with synthetic context and an aged cache. No payment was submitted.
Test 1: metadata cache and backup regressions ✅ The focused cache, backup lifecycle and foreground recovery assertions passed.
AppCacheDataTests, BackupServiceTests and BackupForegroundRecoveryTests passed without failures or skips. I checked the finalized XCResult, including the retired-field decode, drain-time edits, manual admission and stopped-USDT retry assertions.
metadata-backup2x.2x.mp4
App-hosted component execution; outcomes were checked in XCResult.
Test 2: USDT receive navigation ✅ The existing receive amount, tab-swipe, invalid-recipient and back-navigation assertions passed through the composed receive routes.
I checked the original run and the recording run in finalized XCResults. QoS warnings and recording gaps remain; the video does not establish animation timing.
navigation-receive-repeat2x.2x.mp4
Unmodified-head receive navigation.
Test 3: USDT manual amount and recipient ✅ Decimal entry in a comma region, amount retention and recipient-preserving back navigation passed.
The original and recording runs passed in finalized XCResults. QoS warnings and recording gaps remain.
navigation-amount-repeat2x.2x.mp4
Unmodified-head manual-entry navigation.
Test 4: expired-rate payment conversion ✅ Both asset switches reported an unavailable or expired rate and retained the original amount before review.
With BTC/USD 100000 cached one hour earlier and refresh unavailable, I entered 1,000 sats and attempted to switch to USDT, then entered 5 USDT and attempted to switch to Savings. Both showed the rate error, kept the original asset and amount, and left Continue disabled. The recordings and accessibility snapshots confirm those states. This completes step 6 of journeys/usdt-cached-home-balance.md, which the previous report left untested. The fixture directly presents the amount screen; it does not establish a funded payment or contact negotiation.
| Bitcoin to USDT | USDT to Bitcoin |
|---|---|
expired-btc-usdt2x.2x.mp4 |
expired-usdt-btc2x.2x.mp4 |
Production amount screen with a seeded stale cache and no funds.
Local Debug fixture — not pushed
Seeds the cached rate, prevents a fresh rate response and presents the existing amount screen with both funding choices. Conversion and review decisions are unchanged.
diff --git a/Bitkit/ContentView.swift b/Bitkit/ContentView.swift
index f0646a1c..6dcfeda1 100644
--- a/Bitkit/ContentView.swift
+++ b/Bitkit/ContentView.swift
@@ -4,8 +4,16 @@ struct ContentView: View {
@StateObject private var session = SessionManager()
var body: some View {
- AppScene()
- .id(session.id)
- .environmentObject(session)
+ Group { // test hook
+ #if DEBUG // test hook
+ if ProcessInfo.processInfo.environment["REVIEW_EXPIRED_RATE"] == "1" { // test hook
+ ReviewExpiredRateFixture() // test hook
+ } else { // test hook
+ AppScene().id(session.id).environmentObject(session) // test hook
+ } // test hook
+ #else // test hook
+ AppScene().id(session.id).environmentObject(session) // test hook
+ #endif // test hook
+ } // test hook
}
}
diff --git a/Bitkit/Utilities/Testing/ReviewExpiredRateFixture.swift b/Bitkit/Utilities/Testing/ReviewExpiredRateFixture.swift
new file mode 100644
index 00000000..7cb9fb41
--- /dev/null
+++ b/Bitkit/Utilities/Testing/ReviewExpiredRateFixture.swift
@@ -0,0 +1,60 @@
+#if DEBUG // test hook
+import BitkitCore // test hook
+import SwiftUI // test hook
+
+private final class ReviewOfflineCurrency: CurrencyService { // test hook
+ override func fetchLatestRates() async throws -> [FxRate] { throw URLError(.notConnectedToInternet) } // test hook
+} // test hook
+
+@MainActor // test hook
+struct ReviewExpiredRateFixture: View { // test hook
+ @StateObject private var app: AppViewModel // test hook
+ @StateObject private var wallet = WalletViewModel() // test hook
+ @StateObject private var currency: CurrencyViewModel // test hook
+ @StateObject private var contacts = ContactsManager() // test hook
+ @State private var path: [SendRoute] = [] // test hook
+ @State private var hardware = HwWalletManager() // test hook
+ @State private var hwSend = HwSendCoordinator() // test hook
+ @State private var requests = PaykitPaymentRequestManager() // test hook
+
+ init() { // test hook
+ let rate = FxRate(symbol: "BTCUSD", lastPrice: "100000", base: "BTC", baseName: "Bitcoin", quote: "USD", // test hook
+ quoteName: "US Dollar", currencySymbol: "$", currencyFlag: "", lastUpdatedAt: Date().addingTimeInterval(-3600).timeIntervalSince1970 * 1000) // test hook
+ UserDefaults.standard.set(try? JSONEncoder().encode([rate]), forKey: "cached_fx_rates") // test hook
+ SettingsViewModel.shared.usdtPreference = .enabled // test hook
+ SettingsViewModel.shared.hideBalance = false // test hook
+ let app = AppViewModel() // test hook
+ app.selectedWalletToPayFrom = .onchain // test hook
+ app.paykitUsesUsdt = ProcessInfo.processInfo.environment["REVIEW_INITIAL_USDT"] == "1" // test hook
+ app.scannedOnchainInvoice = OnChainInvoice(address: "bcrt1qexample", amountSatoshis: 0, label: nil, message: nil, params: nil) // test hook
+ app.contactPaymentContext = ContactPaymentContext(publicKey: "review-unfunded", endpoints: [ // test hook
+ .init(methodId: .usdtArbitrum, value: "0x1111111111111111111111111111111111111111", min: nil, max: nil, rawPayload: "") // test hook
+ ]) // test hook
+ _app = StateObject(wrappedValue: app) // test hook
+ let currency = CurrencyViewModel(currencyService: ReviewOfflineCurrency()) // test hook
+ currency.primaryDisplay = .bitcoin // test hook
+ currency.displayUnit = .modern // test hook
+ _currency = StateObject(wrappedValue: currency) // test hook
+ } // test hook
+
+ var body: some View { // test hook
+ VStack(spacing: 0) { // test hook
+ Text("566b2ff · local unfunded amount fixture\nBTC/USD 100000 · rate aged 1 hour · refresh offline") // test hook
+ .font(.system(size: 12, design: .monospaced)).foregroundStyle(.white).padding(8) // test hook
+ NavigationStack(path: $path) { // test hook
+ SendAmountView(navigationPath: $path, hwSend: hwSend) // test hook
+ } // test hook
+ } // test hook
+ .background(.black) // test hook
+ .environmentObject(app) // test hook
+ .environmentObject(wallet) // test hook
+ .environmentObject(currency) // test hook
+ .environmentObject(contacts) // test hook
+ .environmentObject(SettingsViewModel.shared) // test hook
+ .environment(hardware) // test hook
+ .environment(wallet.usdtWallet) // test hook
+ .environment(requests) // test hook
+ .preferredColorScheme(.dark) // test hook
+ } // test hook
+} // test hook
+#endif // test hookI retained manual cases 1–3, canceled manual admission, drain-time edits and the unchanged Home display checks from the e12df99 report. Initialization callback coverage remains from the earlier regression run; stopped-USDT recovery and the notification burst are also supported by the e12df99 device report. Their tested scheduling, listener, retry, completion and display paths are unchanged. No selected case remains unrun.
Reviewed e12df99..566b2ff, including the merged parent changes and prior feedback. Other merged Blocktank channel, fee-bump, notification and identity flows received source review without new device journeys. The separate terminal-identity/full-WALLET limitation remains scoped to bitkit-ios#1121 fix: decouple wallet restore from profile refresh.
Twin: bitkit-android#1689 fix: preserve pending backups across restart matches the retained-cleanup and stopped explicit-retry contract; platform-specific reset ownership remains separate.
Reviewed by gpt-6.1-sol-high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of 566b2ff8 against merge base c0b1e86f, after the comparison base changed in BackupService.swift. The complete PR and affected callers were reassessed; no coverage was inherited. Android #1689 and iOS #1122/#1121 received targeted contract comparisons only.
No new actionable code findings.
The earlier shutdown lost-edit finding is fixed in the pinned source. The terminal-identity/full-WALLET availability concern remains unresolved in the dependency and is already present at this PR's base.
Validation: inspected native CI logs, including passing backup lifecycle, foreground recovery, initialization and affected USDT navigation regressions. The tested merge includes the pinned head/base and has the same tree as the head. SwiftFormat passed; git diff --check passed locally. Migration E2E jobs and native recovery/fork fixtures were skipped, so these checks do not establish complete authenticated recovery acceptance.
Device testing: not performed in this review. Swift/Xcode are unavailable locally; native tests were inspected through CI rather than rerun. Earlier manual lifecycle evidence retains its original revision.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
|
@ben-kaufman Moving this PR to 3.2.0 per the 3.1.0 cut line (https://synonymworkspace.slack.com/archives/C07BJ7DNPCG/p1793307568629859): the restore and profile-deletion stacks harden behaviour that already shipped in 3.0, so they keep landing on master and ship in 3.2. |
There was a problem hiding this comment.
Verdict: ✅ Approve — no new findings; all selected cases passed.
Findings: No actionable defect found. The shutdown-drain fix remains intact. The added allowance notification reaches both snapshot invalidation and pending-backup tracking; the activation test preserves its original return deadline.
QA for 1dd6d2f: iPhone 17 Pro simulator, iOS 26.5, Xcode 27. Existing regressions and navigation used the unmodified head. The allowance case used local test-only additions with unchanged production sources and injected capture/upload. No payment was submitted.
Backup lifecycle and foreground restart ✅ Existing backup, foreground recovery and initialization assertions passed.
Checked in finalized XCResult: shutdown drains, edits during acknowledged-upload cleanup, stopped USDT retry, restart recovery and single initialization completion passed without failures or skips. The recording accompanies app-hosted component execution; assertions are established by XCResult.
backup-restart2x.2x.mp4
Identity activation returns while publication is held ✅ The revised activation test passed with publication gated and the original return deadline intact.
Checked the test and finalized XCResult. Credential preparation has its own bounded setup wait; activation must still return within two seconds while publication stays held. The production publication cap is unchanged. Component outcomes are checked in XCResult rather than inferred from the recording.
identity-publication2x.2x.mp4
Allowance edits survive backup shutdown ✅ Real allowance-store changes invalidated capture and stayed pending through shutdown until restart.
Two local test additions exercise the production keychain store and notification stream. One changes allowance state during capture and verifies recapture. The other holds a settings upload, stops observation, saves an allowance edit, verifies no premature upload, then checks the latest stored state uploads after restart. Both passed in finalized XCResult. Capture/upload are injected; this does not establish encrypted VSS restore or an allowance payment journey.
allowance2x.2x.mp4
USDT receive navigation ✅ Receive amount commit/cancel, tab swipes, invalid-recipient handling and back navigation passed.
The original and replacement-recording runs passed in finalized XCResults. QoS warnings and capture gaps remain; the video does not establish animation timing.
usdt-receive-repeat2x.2x.mp4
USDT amount and recipient navigation ✅ Comma-region decimal entry, amount retention and recipient-preserving back navigation passed.
The original and replacement-recording runs passed in finalized XCResults. QoS warnings and capture gaps remain. The replacement's optional diagnostic collection stalled after the assertions completed and was stopped so the successful result could finalize; the clip omits that idle tail.
usdt-amount-final2x.2x.mp4
Retained manual cases 1–3, canceled manual admission and the no-restart shutdown-edit scenario from the e12df99 report. Their exercised lifecycle paths remain unchanged; the newly connected allowance source is checked above.
Retained the stopped-USDT native-outage retry and notification-burst evidence from the e12df99 device report. The native dependency, classifier, backoff and activity-notification handling are unchanged.
Retained expired-rate conversion from the 566b2ff report; the amount-screen conversion implementation is unchanged. No selected case remains unrun. Funded payments, process-death recovery, cross-platform restore and the merged upstream feature journeys were not driven in this pass.
Reviewed 566b2ff..1dd6d2f, focusing on the allowance notification, activation-test deadline and parent composition, plus the previously resolved shutdown finding. The separate terminal-identity/full-WALLET limitation remains in bitkit-ios#1121 fix: decouple wallet restore from profile refresh.
Twin: bitkit-android#1689 fix: preserve pending backups across restart at 1d3ba14 matches the retained-cleanup, bounded transport-retry and stopped explicit-USDT contracts; reset and observation ownership remain platform-specific.
Reviewed by gpt-6.1-sol-high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Re-reviewed the full PR at 1dd6d2fb after the comparison base moved from c0b1e86f to d0246d1c. Both of those commits change BackupService.swift and BackupForegroundRecoveryTests.swift, and this run had no coverage baseline to inherit. Coverage is the complete diff from merge base d0246d1c.
No new actionable code findings.
Observation shutdown still drops status observers and cancels queued work before it removes data-store subscriptions, and it waits for scheduled jobs, running uploads, and both backup writers. Edits that arrive during that drain stay required and are uploaded only after observation is running again. An explicit USDT backup can still retry after observation has stopped. The temporary VSS request-send rule matches the Android twin at 1d3ba149: typed unknown errors with an http(s) host and a putObjects path. Wallet initialization completion is admitted once when normalized progress reaches the running node.
Validation: source and unit-test inspection only; this review did not execute the suite. Run Tests and SwiftFormat succeeded on this head. End-to-end jobs on this head were skipped. Device testing: not performed in this review.
Companion bitkit-ios#1122 head bee8975a is not in this workspace. The only commit after its base 8947f493 changes PaykitSdkClientConfigTests.swift. The identity-restoration availability gap stays on dependency bitkit-ios#1121.
Ready for device testing. Manual cases 1–4 still need the dedicated fault fixture in the PR; this review did not drive them.
Reviewer model: grok-4.7
Reasoning effort: high
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Re-approving at 262a447. No new findings.
1dd6d2fb is test-only (PaykitSdkClientConfigTests.swift). Merges 262a4474 and 8947f493 fold the parent's waitForExisting/owned into the PR's admission loop correctly. No device check needed for this delta.
There was a problem hiding this comment.
Verdict: ✅ Approve — no new findings; all selected cases passed.
Findings: I found no actionable defect in the delta. The shutdown lost-edit fix remains intact. Metadata joins an existing wallet backup without owning its cancellation, and restore drains admitted metadata before importing address exclusions.
QA for 262a447: iPhone 17 Pro simulator, iOS 26.5, Xcode 27. Regression and navigation runs used the unmodified head. Live lifecycle cases used local Debug holds and logging, an unfunded regtest wallet and real staging VSS. No payment was submitted.
Backup shutdown and metadata ordering ✅ Backup, foreground recovery and initialization assertions passed.
Finalized XCResult confirms metadata waits for durable wallet exclusions, canceled metadata leaves another owner's wallet upload running, restore and shutdown drain admitted metadata, and newer requirements survive cleanup. The hosted SwiftUI tests also verify one initialization callback. Component outcomes come from assertions, not the recording.
backup-composition2x.2x.mp4
App-hosted component execution.
Public address and restore composition ✅ Focused sharing, address, restore and payment-request regressions passed.
Checked finalized XCResults for independent public/private preferences, address reservation and lookup, stale-session admission, payment coordination and Android backup decoding. The original and replacement composition runs passed; QoS warnings and recording gaps remain. These are component checks, not authenticated sharing or cross-platform restore journeys.
| Sharing and restore guards | Address lookup and payment coordination |
|---|---|
parent-regressions2x.2x.mp4 |
parent-address-lookup2x.2x.mp4 |
App-hosted assertions checked in XCResult.
Manual backup waits for shutdown cleanup ✅ Stop and manual metadata waited for cleanup; latest settings and wallet values reached VSS after restart.
Held scheduled cleanup before its final status write, queued stop and manual metadata, then changed both sources while restart waited. The event log confirms no capture before release and matching decrypted payloads afterward. A second held drain confirms canceling manual admission starts no metadata capture or upload for that request.
manual-cleanup-repeat2x.2x.mp4
Live controls and event overlay; hidden-state assertions checked in the event log.
Edits during acknowledged upload remain pending ✅ With no restart queued, both edited categories stayed required through the drain and matched VSS after restart.
Held the return from a successful settings put, requested stop, then changed settings and an already-synced wallet record. The event log confirms both pending markers survived release, no new capture started while stopped, and decrypted SETTINGS and WALLET matched after observation restarted.
stop-drain-edits-repeat2x.2x.mp4
Acknowledged-upload hold, pending-state checks and remote comparisons.
Stopped listeners ignore later edits ✅ Post-stop wallet edits left status unchanged across 25 cycles; a pre-stop settings edit uploaded after restart.
Each cycle saved a distinct wallet-record value after stop returned and checked unchanged status with no new capture. The final settings notification remained pending through stop and matched VSS after restart. The intentionally unobserved wallet edits remained absent remotely, as expected. Assertions are checked in the event log; the clip shows the controls and overlay.
repeated-lifecycle-repeat2x.2x.mp4
Repeated lifecycle controls and status comparisons.
USDT receive navigation ✅ Receive amount commit/cancel, asset swipes, invalid-recipient handling and back navigation passed.
Original and recording runs passed in finalized XCResults. QoS warnings and capture gaps remain; the video does not establish animation timing.
usdt-receive-repeat2x.2x.mp4
Unmodified-head receive navigation.
USDT amount and recipient navigation ✅ Comma-region decimal entry, amount retention and recipient-preserving back navigation passed.
Original and recording runs passed in finalized XCResults. The original optional diagnostic collector stalled after assertions and was stopped so its successful result could finalize. QoS warnings and capture gaps remain; the replacement recording used unchanged assertions.
usdt-amount-repeat2x.2x.mp4
Unmodified-head amount and recipient navigation.
Retained manual case 2's stopped-USDT native-outage retry and the notification-burst measurement from the e12df99 device report. The bitkit-core, Paykit and VSS client pins, classifier, backoff, activity notification handling and fresh non-joining request behavior remain unchanged. The retained burst result keeps its original timing scope.
Retained expired-rate conversion from the 566b2ff report, and unchanged allowance-notification and activation-return coverage from the 1dd6d2f report.
No selected case remains unrun. Authenticated parent-feature journeys, funded payments, process-death recovery and cross-platform restore were not driven. The known complete-wallet availability limitation remains with bitkit-ios#1121 fix: decouple wallet restore from profile refresh.
Reviewed the contiguous 1dd6d2f..262a447 delta, including the merged parent changes and prior shutdown feedback. Other merged flows received source and focused component coverage. Changed Swift files passed SwiftFormat 0.64.0 lint; translation validation passed with its warnings retained.
Twin: bitkit-android#1689 fix: preserve pending backups across restart at 9cfe052 matches pending-state retention, stopped retry and durable address-exclusion contracts. Android serializes WALLET and METADATA with a shared mutex; iOS joins wallet work before metadata.
Reviewed by gpt-6.1-sol-high
There was a problem hiding this comment.
Verdict: ✅ Approve — no new findings; the focused recovery case passed and unchanged QA is retained.
Findings: I found no actionable defect in the delta. The shutdown lost-edit fix remains intact.
QA for fb25166: iPhone 17 Pro simulator, iOS 26.5, Xcode 27. Built the app and test targets from the unmodified head with the committed dependencies.
Profile recovery preserves unfinished setup ✅ Setup-pending recovery avoids a remote profile read, while healthy recovery and stale-result rejection still pass.
The new setup-pending regression and both existing recovery controls passed in finalized XCResult, with no failures or skips. I checked each result: repeated maintenance preserves the authenticated session and pending setup; an ordinarily missing profile reloads without reauthentication or waiting for the read; a result from cleared session state is discarded.
These are app-hosted component tests with an injected profile resolver and synthetic identity state. The assertions are established by XCTest and XCResult, not by the screen recording. This does not establish a live homeserver outage or a complete profile-setup journey.
profile-recovery2x.2x.mp4
Recording of the component run at 2x; capture gaps remain.
Check: retained QA coverage ✅ The exercised feature paths and native dependency pins remain unchanged.
- Manual cases 1, 3 and 4, canceled manual admission, acknowledged-upload edits, metadata ordering and ordinary USDT navigation retain the 262a447 report. Backup observation, writers and initialization sources are byte-identical.
- Manual case 2's stopped-USDT native-outage retry and the notification-burst measurement retain the e12df99 device report. Their classifier, retry, notification and native dependency paths are unchanged.
- The imported allowance switch retains the paired device run at a226627: a covered request auto-pays while enabled, stays manual while disabled, and stored allowances survive re-enabling. Its executor, manager, tab and flag rule match this head.
- Request Payment routing retains the complete linked-contact journey at 180987ff. Preparing-request colours and the USDT review icon retain the rail review at 375538f2. The exercised view implementations are unchanged.
These are attributed earlier executions, not fresh runs in this pass.
No selected case remains unrun. Funded payments, process-death recovery and cross-platform restore were not driven in this pass. The known complete-WALLET availability limitation remains with the parent.
Reviewed the contiguous 262a447..fb25166 delta, all changed files and affected callers, including parent/master composition and prior shutdown feedback. Other imported behaviour uses the retained reports above; the new profile guard received focused component coverage. Changed Swift files passed SwiftFormat 0.64.0 lint; translation validation passed with its existing warnings retained.
Twin: bitkit-android#1689 fix: preserve pending backups across restart at 9cfe052 matches pending-state retention and stopped explicit-retry contracts. Android serializes WALLET and METADATA with a shared mutex; iOS joins existing wallet work before metadata.
Reviewed by gpt-6.1-sol-high
Description
Stopping backup observation could crash while startup concurrently modified its subscriptions. Serialize subscription and timer ownership on MainActor, and finish canceled backup cleanup before shutdown returns.
Invalidate queued work from the stopped observation generation. Received changes now remain pending while restart drains old cleanup. Pending subscriptions record dirty state immediately, and observed uploads wait for admitted observation. Cancellation preserves required state for ordinary categories as well as wallet/USDT. Fresh explicit USDT retries remain usable without restarting Lightning observation, and manual migration uploads wait for cleanup instead of being silently skipped.
A released VSS request-send failure is now retried at the backup boundary, including explicit USDT work after observation stops. The compatibility rule accepts only the typed native UnknownError envelope with an HTTP(S) host and
putObjectspath; unrelated unknown/authentication/decode errors and cancellation retain their behavior. Same-status notifications still advance capture revisions, but no longer rewrite identical persisted status.Initialization completion also admits its callback only once after the node reaches Running. Progress normalization can notify the view twice; both handlers now share the existing completion latch before scheduling any delayed callback. Existing startup waits and assertions are unchanged.
Out of Scope
Full-wallet format separation and the unresolved identity-restoration availability gap in #1121 remain separate. Android's related setup/job-ownership repair is in its focused twin synonymdev/bitkit-android#1689. Native Paykit/Pubky artifact adoption and historical incident attribution are unchanged.
Design
N/A — no UI changes.
Preview
N/A — background backup lifecycle repair.
QA Notes
Setup
Use the committed released Core 0.8.0-rc11, Paykit 0.1.1-rc2 and VSS 0.5.26-rc1 dependencies. Component tests use private defaults and controlled delay/capture/upload gates. Manual lifecycle checks require a disposable app installation and the dedicated fault fixture described in
journeys/backup-restore/wallet-snapshot-consistency.md; do not use a personal wallet or reset wallets.Journeys
N/A — not drivable; see Manual Tests.
Manual Tests
1. Hold scheduled cancellation or its final status write → stop observation and request a manual backup → both wait for cleanup, change wallet/settings state while restart is waiting → no upload before drain; latest state resumes without another edit; canceled settings upload retains newer required state — lifecycle and status-write fault gates are not in Capabilities.
2. Request USDT recovery during cleanup and after observation stops → release the fault → bounded retry completes without restarting Lightning — controlled recovery fault gates are not in Capabilities.
3. Repeatedly start/stop observation → change a wallet record after stop → the stopped instance does not mutate backup status or upload; a settings event received before stop remains pending — lifecycle injection is not in Capabilities.
4. Observe initialization while the node enters Running at the progress threshold → completion is admitted once and the wallet opens normally. Exact fractional progress and callback cardinality require the hosted SwiftUI regression because these injection/counting capabilities are not available to a journey.
Automated Checks
UsdtWalletUITests.swift— bounded navigation synchronization when a transient toast obstructs the back button.BackupServiceTests.swift— stopped observers, canceled-work drain, final status-write ownership, manual admission, restart and independent USDT retry.Current final validation — 31 October 2026
Signed additive merge
fb25166fd830f8d59368c09079545a1abcdb5cb9incorporates live #1121 parent624d7a38995280e9ad59684705c72e405b1e02bd. Tree2f1fa9e1dafbb9ce2a65c4959f26474a7e8c089cpreserves the child delta against its actual previously composed ancestor and includes the validated parent/master admission changes.The combined native build and selected suite passed 2,965 unit executions and 39 UI executions, with five explicit skips, zero failures or failure-driven retries (2,975 unique logical passes). All 1,688 frozen source inputs match. Every one of 3,009 canonical execution summaries, nested diagnostics and 67 attachments was inspected; the owner verified 12,414 evidence files and 27 external inputs. SwiftFormat0.64 reports 0/828 with106 exclusions. Reused translations have zero errors and15 verified actual-parent warnings on identical21 inputs.
One fresh combined composition review is clean; earlier unaffected reviews retain their original scope. All 15 live-base commits verify locally and on GitHub (
verified=true,reason=valid). No history rewrite occurred.Current-head hosted native run41241011565, attempt1, and its formatter passed on synthetic merge7d071143 whose tree exactly equals this head: 2,965 unit +39 physical UI passes, five explicit skips, zero failures or failure-driven retries (2,975 logical passes). The complete logs, all3,009 canonical execution summaries, nested diagnostics,67 attachments and digest/CRC-verified artifact13057914768 were inspected; the owner verified all18,651 sealed evidence files and17 external inputs. Hosted SwiftFormat0.64 reports0/828,106 exclusions. Nine migration jobs and four workflow-excluded integration classes are not passes.
Hosted diagnostics retain286 distinct dependency linker warnings/571 emissions, two AppIntents notices and48 NaN messages. A keyboard-queue timeout, copied into three exported logs and followed by
No stack!, occurred during a passing SendConfirmationLifecycle test; it has no established source cause. Runtime issue/leak detection and test timeouts are disabled in that hosted configuration, so zero canonical runtime warnings is not general runtime-safety evidence. The profile and camera cases passed once; neither is a causal repair for prior failures. Authenticated manual fault cases and terminal complete-WALLET availability remain open.The complete diagnostics retain14 QoS warnings,48 UI NaN messages and three AppIntents notices, without a proven introduced cause. The profile-recovery case passed once in62.472s and the camera case in58.858s; neither explains earlier/sibling failures. Canonical records supply three successful cases omitted by the wrapper and expose warnings that wrapper diagnostics omitted. Dependency pins/resolution match; missing alternate-object stores still prevent complete dependency working-tree verification, including Core's unreadable
CLAUDE.mddiff. The five runtime skips and four excluded integration classes are not passes. Manual cases1–4 retain their documented coverage limits; no newly authenticated full-app journey or terminal complete-WALLET repair is claimed.Earlier final validation — 29 October 2026
Head
1dd6d2fb8974cffc674802180d5f8ef620a60959is stacked on #1121 atd0246d1c. The latest commit changes only the activation test: credential setup has a separate bounded deadline, while the existing two-second activation-return assertion and production publication cap remain unchanged. A controlled implementation that awaited publication fails that original assertion; restoring the exact production bytes passes.Hosted native run 40969995082 passed on its first attempt: 2,880 unit passes and 39 physical UI passes (10 logical cases), five fixture skips, zero failures or retries. Its synthetic merge
c94e38cfhas the exact head tree17d7efe4and parentsd0246d1c/1dd6d2fb. Hosted Xcode 16.4, SDK 18.5 and iPhone 16/iOS 26.2 remain distinct from local execution. SwiftFormat 0.64.0 reports 0/824, with 98 exclusions.Complete 5,824-line native output, all 2,924 nested test records, 17 annotations, eight exported text diagnostics and 54 unique attachment payloads were inspected. Artifact
12941895164matches GitHub SHA25692e7b34838d6ef113ef70bec25535e63cdb8d75f6cd6f4845a75941de3477bd1; all 5,997 archive members pass integrity checks. The owner verified all 12,075 sealed evidence files. The 286 distinct duplicate-symbol warnings match the verified parent producer set; two AppIntents notices remain. Runtime output retains 48 CoreGraphics NaN messages across the two passing USDT UI cases plus the disclosed node/cache/network and unit fault-case diagnostics. Their numeric cause is unproven; this is not a runtime-clean claim.Skipped: the encrypted protocol fixture, three Trezor cases and the local fork payment. Four workflow-excluded test classes remain unrun. The activation test's 2.076786-second whole duration includes its two-second inverted expectation; it is not activation latency. Earlier failed hosted attempts remain retained, and this test-only correction is not claimed as a production causal repair.
The final local gate passed 2,880 selected units and two USDT UI cases, with two prerequisite skips and zero failures/retries; all 1,667 frozen inputs match. Required formatting passed. The independent affected-delta review is clean under the current risk-based policy, with earlier unaffected reviews scoped to their original inputs. All 13 commits in the complete live-base PR range verify locally and on GitHub (
verified=true,reason=valid). No history was rewritten.Gaps
The native protocol tests use real signing and VSS encryption but do not execute the complete app UI/Paykit/deployed-contract journey. The controlled manual lifecycle journey has not been driven through the full app. Android twin synonymdev/bitkit-android#1689 repairs late setup, retained cleanup, dirty-marker loss and manual/USDT reset ownership; its latest explicit-retry follow-up at49e9a99d passed4,696 unit tests and full compile/format/lint, with component-test limits documented there. No claim that this crash or these races caused the historical contact/acquisition failures.
Earlier revision evidence and retained failed attempts
These dated notes retain their original scope. Earlier pending states are superseded by the current validation above.
Earlier-head external evidence — October 30, 21:50 UTC
Automated review 5718993346 at
262a4474reports authenticated staging-VSS shutdown/drain, pending-edit retention and stopped-observer cases, plus component/navigation checks; it reports no new findings. The owner verified the revision and read the complete report, but has not independently audited its raw logs, XCResults or media. These are attributed external results; the author's hosted nested-result audit remains incomplete. João and Ovi have separately approved this exact head. The inherited terminal complete-WALLET limitation remains open on #1121.Earlier combined-tree validation — 30 October 2026
Signed additive merge
262a44747ffd8ef522178bfd3850fe78da15cce5incorporates live #1121 parentd896dba7e78d39d859ed9a844863a194efb4b546. The combined tree52be49877b6d7ba84b9500b1feb2fe198fc6e8d0preserves generation-owned observation and shutdown drains, adopts the parent's metadata-before-restore/public-address guards, and makes metadata join an existing wallet writer without canceling another owner's operation. The corrected lifecycle regression uses ACTIVITY because the parent now intentionally completes WALLET before METADATA; its drain, restart and required-state assertions remain unchanged. Its initial failed attempt is retained.The final native build and selected suite passed 2,956 units and two USDT UI cases, with two prerequisite skips, zero failures/retries. All 1,681 frozen inputs match. Complete 22,530-line output, every nested result, eight exported diagnostic files and 37 attachments were inspected; the owner verified all 6,145 evidence hashes. SwiftFormat 0.64.0 reports 0/827 with 104 exclusions; translation validation reports zero errors and 15 actual-parent warnings. Two independent affected composition reviews are clean. All 14 live-base commits verify locally and on GitHub (
verified=true,reason=valid).Seven QoS observations, 72 CoreGraphics NaN emissions with unproven numeric cause, two AppIntents notices and symbolication diagnostics remain disclosed. Local Xcode 26.1/iOS 26.2 evidence is distinct from hosted execution. The encrypted protocol/local-fork skips and four excluded workflow-integration classes are not passes. Current-head hosted CI is pending inspection; the historical results below do not certify this composition. Manual cases 1–4 retain their recorded component/runtime limits, and #1121's terminal complete-WALLET availability remains separate.
Earlier parent integration — 29 October 2026, 20:05 UTC
Signed additive merge
8947f493a681f57cb76486d96bbc36da3f9999a6incorporates live #1121 parentd0246d1c782c8f2167300383f85e12f1696c4234. Tree04884bd9d6c2bfadf5e0e1b022a025027d208c69retains this PR's stronger ordered observation shutdown and complete writer/task drain byte-for-byte, while importing the parent's repeated-restart regression and related private-publication/admission changes.Final combined-tree validation passed 2,880 units and two UsdtWallet UI cases, zero failures/retries, with two prerequisite skips (encrypted native recovery and local Arbitrum fork). Other UI was unselected. All 1,667 frozen inputs and four symlink targets match. Full SwiftFormat0.64 reports 0/824, 98 exclusions; the initial system formatter rejected the minimum version before checking and is retained as a failed setup attempt. Unchanged workflow/localization checks reuse exact parent input evidence.
Complete 24,156-line raw output, all 2,884 nested summaries, eight diagnostic logs and 35 attachments were inspected; the owner verified all 13,732 sealed rows. Nine QoS traces/eight nested warnings remain, including a newly observed fee-estimate RwLock sub-stack in parent-identical code; two AppIntents notices and simulator haptic messages are retained without a causal claim. One clean affected composition review preserves earlier full feature reviews. All 12 live-base commits verify locally and on GitHub (
true/valid). The current-head hosted native run, attempt 1, failed four UI launch executions: 2,880 unit and 35 UI passes, five prerequisite skips, four failures, no retries. Complete logs, all 2,924 nested summaries, 55 attachment payloads and 4,894,996 simulator system-log events were inspected; all 10,525 sealed evidence rows were verified. XCTest timed out its testing-assertion requests after ten seconds and then deliberately terminated the targets. RunningBoard only received those testing assertions 20–60 seconds after the original requests, after all four targets had exited. No preceding failed-target crash/fired watchdog was established; splash frames do not prove completed startup or general app health. The underlying cause remains unknown. Exactly one bounded native-job retry was requested on that observed testing-service delay; its completed failure and the subsequent test-only correction are recorded above. The original failures remain. Hosted formatting passed 0/824; nine migration jobs skipped. The 286 dependency warnings match verified producer evidence; two AppIntents notices, 24 NaN emissions and a keyboard timeout in passing tests remain disclosed.Upstream #1120's native attempt1 failed two plain launch tests. System logs show delayed RunningBoard assertion acquisition after XCTest's timeout and subsequent test-driven termination with no demonstrated preceding app crash; the next 30 launch configurations passed. That separate bounded retry has now passed, with its original failed attempt retained. The original failures and unknown underlying service-delay cause remain disclosed; this child publication does not claim they are fixed or that upstream CI is green. Authenticated manual cases below and terminal complete-WALLET availability remain open.
Earlier parent integration — 29 October 2026
Signed additive merge
d33547cba681f8cf826a715de5f51a2ba89c28efincorporates live parent #1121 at2d4fc7199b476f41ce56ae05c786f3ee1981bed5. The final tree5a80e5e87368aab5487436d2aa47fb5e271a87b3retains this PR’s observation ownership, shutdown drains and restart behavior, and adds the parent’s allowance-change publisher to the existing backup change stream.The final combined-tree native build and tests passed 2,865 unit cases + 39 UI executions, zero failures, with five explicit prerequisite skips. That is 2,875 logical passing tests; the launch case runs 30 declared UI configurations. All 1,660 frozen inputs match. Complete logs, all 2,909 nested summaries, and exported diagnostics were inspected; the owner verified the 13,837-row evidence seal. Three AppIntents tool notices and 12 QoS observations remain disclosed without a causal-fix or uniformly pre-existing claim. Full SwiftFormat reports 0/823, and translation validation reports zero errors / 15 warnings verified against the actual parent. The formatter cache-write notice remains. Original static process completion was reconciled without rerunning checks.
One independent review of the changed composition is clean; prior unchanged review coverage is retained. Every commit in the 11-commit live-base range verifies locally and on GitHub (
verified=true,reason=valid). The immediate post-push API briefly returned the older head; one read-only reconciliation verified the exact new range, with no repeated push. Current-head native tests and SwiftFormat passed on synthetic mergeb089302c, whose tree exactly equals the published head: 2,865 unit + 39 physical UI passes, zero failures/retries, five explicit skips; formatter 0/823. Complete logs, all 2,909 nested summaries, diagnostic exports and the SHA/CRC-verified result archive were inspected; the owner verified 12,003 sealed evidence rows. All 286 distinct linker warnings match the sealed actual-parent set. Two AppIntents notices, a setup metadata-pipeline broken pipe and 24 CoreGraphics NaN events in a passing USDT UI case remain disclosed without causal claims. Nine migration jobs were skipped; no current-head integration/external E2E result is claimed. This does not complete the authenticated manual fault cases below or close #1121’s terminal-identity/full-WALLET finding. Skipped native/fork/hardware cases and workflow integration exclusions are not passes. Earlier evidence remains scoped to its recorded revision.Parent integration — 28 October 2026
Signed additive merge
566b2ff8ba7abdd0968544dda5cdba32d8f518f0incorporates current parent #1121 atc0b1e86f, including the merged independent-USDT/master composition. The only adaptation inside this PR’s existing production delta removes two obsoletetrytokens from now-nonthrowing metadata-cache calls; actor hops, backup generations, stop drains and complete-WALLET checks remain unchanged.The exact tree
9ce5290cpassed 2,830 unit executions + 39 UI executions, zero failures, with three explicitly disabled Trezor skips. These are 2,840 distinct passing identifiers: the launch test executes 30 declared UI configurations. Complete 29,980-line logs and all 2,872 nested execution summaries were inspected. All 1,643 frozen inputs match; formatter passed 0/821 with 91 exclusions. Two AppIntents notices, ten nonfailing QoS observations and the formatter cache-write notice remain disclosed; timing causation and exact-parent runtime recurrence are unproven. Actual local toolchain is Xcode 26.1/17B55 on iOS 26.2/23C54. Excluded workflow integration/protected native/fork cases are not passes.One independent affected-composition review is clean, with unchanged earlier full-PR review coverage retained. All ten commits against the live parent verify locally and on GitHub (
verified=true,reason=valid); exact ranges match. The PR remains ready and mergeable. Current-head native tests and formatter now pass on the exact9ce5290ctree: 2,830 unit + 39 UI passes, five explicit fixture/hardware skips, zero failures/retries; formatter 0/821. Complete logs, all 2,874 nested summaries and 5,889 CRC/SHA-verified artifact members were inspected; the owner verified 8,858 sealed evidence files. The build retains 571 duplicate-symbol occurrences/286 symbols, with identical actual-parent build/pin configuration but no fresh parent runtime baseline. One keyboard queue timeout occurred during the passing 3.629-second pending-wallet test; causation remains unproven. Excluded integration/native fixture cases are not passes. This integration does not resolve #1121’s terminal-identity/full-WALLET availability finding or establish a new authenticated recovery journey.Earlier validated composition — 27 October 2026
Signed additive merge
e12df99e2e91944308314d82bc8d71e9aaa24f66incorporates live parent #1121 at73b7d58ab6854114a76f7577452677d91172a04a. Foreground recovery now keeps the original observation generation, joins the existing startup/teardown drains, and hands recoverable failures to the existing bounded scheduler. Queued work from a stopped generation cannot publish or upload; received edits remain required through acknowledged cleanup. No deadline or passing-path UI scheduling was weakened.The final tree
ffac48ab56f4210e256f518c180c17a5e933a937passed 2,990 selected unit tests with zero failures/skips and 39 UI executions with zero failures and three explicitly disabled Trezor skips. Workflow integration/native-fixture suites and the expired local-fork payment case were excluded, not passed. SwiftFormat 0.64.0: 0/852, with 90 excluded files; translation validation has zero errors and 15 verified actual-parent warnings. All 1,683 frozen inputs match. Two consecutive independent final reviews are clean.Complete 23,007-line unit and 4,732-line UI logs and every nested result were inspected. Five unit and twelve UI QoS observations, AppIntents and simulator/accessibility notices remain disclosed without a timing-cause claim. Earlier full-suite failures and controlled before/after evidence are retained: the tests now toggle actual persisted Settings values, restore exact prior values/absence, and isolate unrelated coalesced Settings changes; all original lifecycle, age, cooldown and upload assertions remain. These are fixture corrections, not historical-incident causation.
All nine commits against the live parent verify locally and on GitHub (
verified=true,reason=valid). New-head hosted checks are pending. The parent #1120's separately recorded hosted ProfileRecovery UI timeout is not claimed fixed by this local pass. Authenticated lifecycle acceptance and the terminal-identity/full-WALLET limitation remain scoped as described below. All earlier head-specific evidence is historical.Earlier repair — retain edits through shutdown drain
Signed commit
bfc4a5e87b2694cf4906d1c6aa784e2fa1c3e78bfixes the lost-edit window reported in review thread 4390767903. Shutdown stops status-driven scheduling immediately, but retains data-store listeners until scheduled/running uploads and both serialized writers have drained. Changes during that interval remain required; the stopped observation cannot upload them. Restart uploads the latest values, including an independently changed category that was already synced. Listeners are removed before stop returns.Controlled before/after evidence uses actual settings/watch-only notifications and a cancellation-insensitive acknowledged upload: the original implementation fails the one regression with six diagnostics; the repair passes all 23 BackupService tests. Final combined-tree validation passed 2,969 selected units, zero failures/skips, plus 39 UI executions, zero failures, with three disabled Trezor fixture skips. The expired local-fork payment case and workflow integration/native-fixture suites were excluded, not passed. SwiftFormat 0.64.0 reports 0/849. All 1,676 frozen inputs match and two consecutive independent final reviews are clean.
Complete 22,096-line unit and 4,733-line UI logs and every nested test result were inspected. Each run retains two AppIntents notices. Four unit QoS sequences recur in the verified actual parent; nine UI QoS occurrences have unsymbolized zero-address stacks and remain unattributed. The simulator also logged a duplicate accessibility class. These observations are disclosed without a timing-cause claim. This controlled regression and ordinary UI suite do not claim a new authenticated held-upload device journey; the external before-fix staging reproduction remains attributed to its original head.
All eight commits against live parent
754a899182bb39869b37d1f3e9601afecad8c593verify locally and on GitHub (verified=true,reason=valid). New-head hosted checks are pending. Earlier evidence below is revision-scoped.Parent composition — 27 October 2026
Signed additive merge
14152f41adda22319e531f3ac979c1fa0a4120a5incorporates current parent #1121 (754a8991). The resulting source tree is exactlye34dbbd507e6ba6f975f82e9abc0e11c163f9384, identical to the previously validated head: this child already confines listener ownership to the main actor and serializes observation teardown. Two independent composition reviews are clean; all 1,676 native-validation inputs match. Recorded validation remains 2,968 unit passes and 39 UI executions with three Trezor skips; no new test execution is claimed for this ancestry-only update. All seven commits against the live parent verify locally and on GitHub. Newly triggered hosted checks remain pending.Twin: synonymdev/bitkit-android#1689
Companion: #1122
Dependency: #1121
At signed
ff999a50e24326ad45fb033c61155b95c041d1be, a real hosted SwiftUI regression first fails with two completion callbacks versus one, then passes with the shared completion guard. The original slow/error retry case also passes without changing its assertions or deadlines. Final native build/selected unit run passes 2,913 tests, zero failures/skips; SwiftFormat0.64.0 reports 0/847. All1,665 frozen inputs match and two consecutive independent final reviews are clean. The full five-commit live-base range verifies locally and GitHub reports true/valid for every commit. Current-head hosted run40402278397 now passes: 2,913 unit and 39 UI executions, zero failures, one native-fixture unit and four UI skips (2,923 unique passes / 5 unique skips after parameterized-case reconciliation). Both initialization tests pass. Complete5,850-line log and artifact12690973387 were SHA256/CRC checked and every nested XCResult issue inspected. The CI merge tree equals this head. All286 distinct duplicate-symbol warnings match the actual parent; there are no runtime-warning entries. This does not establish the exact prior failure's observer ordering or complete the skipped/manual journeys.The complete20,360-line final log and nested XCResult issues were inspected. Two AppIntents notices and five QoS observations remain; their native construction stacks recur in the actual prior base, but counts and test associations differ and timing causes are unproven. The first final run failed on disk exhaustion, including early database/file errors and result-finalization failure. Its partial-suite successes are not a pass. After reclaiming only inactive task-owned compiler cache, one exact-input retry produced the complete result above.
Prior-head
494c9492hosted run40390697155 failedtestSlowRetryWaitsForRunningNodeBeforeCompletingthrough repeated XCTest fulfillment. Its full log and integrity-checked XCResult are retained. The view/test blobs at that revision equal the actual PR base. The new controlled threshold regression proves a reachable duplicate-completion mechanism; the exact observer ordering in the hosted failure is not claimed proven. Manual case4 has hosted SwiftUI coverage, not a newly executed device journey.At signed
494c9492596100e198b0bdb91c712ebe5888d929, three controlled iOS regressions fail before this repair: native-shaped connection failure classification, stopped-observer retry/backoff, and repeated identical status writes. Final 2,912 selected unit tests pass with zero failures/skips, including all22 backup cases. Full SwiftFormat0.64.0 reports 0/847. Complete23,101-line log and nested XCResult diagnostics were inspected: two AppIntents notices and four QoS warnings remain; three test/message identities match prior evidence, while the SendSheetLifecycle association is new. Relevant source files are unchanged, but the timing cause is unproven. Initial test-helper compilation failure is retained separately from the controlled product failures.The retry test preserves its payload across nine attempts with delays10/20/40/80/160/300/300/300seconds. The status test proves200 identical pending notifications produce no additional status writes; existing first-marker, restart and in-flight-change tests still pass. This is not a universal UI latency claim. Six fresh released-native rejection/lost-acknowledgement prepare/resume/restore phases also pass through the production backup handler, preserving original signed bytes, each process's own acknowledgement before submission, one unique operation and the complete WALLET sentinel. They use loopback protocol/mock authorization, not the complete authenticated manual journey. Two consecutive final independent reviews are clean; all four then-current live-base commits verify locally and on GitHub. Its later hosted failure is recorded above.
Signed follow-up
1ad665f8repairs the hosted UI failure in run40158322810: an Internet Connectivity Issues toast covered NavigationBack during an immediate hit-test query. The verified XCResult recording and accessibility log locate the failure at line 38; the relevant UI/test code was unchanged from the actual base. Back navigation now waits up to ten seconds for a hittable button and retains all existing assertions. Both affected navigation UI cases passed locally, full SwiftFormat checked 847 files with zero violations, and two consecutive independent reviews were clean. The complete three-commit actual-base range verifies locally and on GitHub (verified=true,reason=valid). Hosted formatter40162108133and test workflow40162108190now pass: 2,909 unit tests and 39 UI executions, zero failures, with one native-fixture unit and four UI fixture skips. Complete logs were inspected; formatter reports 0/847, and test-log notices are the installed xcbeautify and multiple-matching-destinations messages. These existing UI suites do not execute the authenticated fault journeys.The broader local UI class also selected the existing fork-payment test, which failed before confirmation. Read-only RPC calls reproduced its cause outside the app: the long-lived fork at block 517943110 could no longer fetch required historical account/EntryPoint state from its upstream RPC. No new operation was broadcast. That full class is not claimed green; the previous successful fork journeys retain their original revision/fixture scope. A healthy isolated archive-backed or fresh fork is needed for another payment journey.
October 25 follow-up (signed head
538f45c7): controlled restart-event and canceled-settings regressions failed before the fixes and pass afterward. All 19 backup component cases and the full selected native unit suite passed (2,909 tests, zero failures/skips). Full SwiftFormat 0.64.0 checked 847 files with zero violations. Complete compiler and finalized XCResult diagnostics were inspected; five QoS priority-inversion warnings remain in contacts/SDK/send tests, with no claim that their cause is fixed. The older Xcode legacy reader crashed; the current reader read the same finalized result successfully without rerunning tests. Two consecutive independent final reviews were clean. The full two-commit actual-base range verifies locally and GitHub reportsverified=true,reason=valid. Six fresh released-native rejected-backup/lost-acknowledgement prepare, resume and restore phases also passed: the same signed operation is retained, no attempt precedes its required backup acknowledgement, one unique operation is recorded and the complete WALLET sentinel remains unchanged. These exercise the app backup handler with real native signing/VSS encryption, not a full app/Paykit or deployed-contract outage journey. Revised manual cases 1 and 3 have component coverage; full fault-injection journeys remain unrun.Hosted #1122 run
40142115983crashed in subscription-set teardown; the backup source was unchanged from #1121. Controlled unchanged-source tests fail for late observer callbacks and premature shutdown/manual upload. A separate held-final-write test fails before the cleanup-order repair.At the earlier
f204e05erevision, the selected native unit/build suite passed 2,906 tests, zero failures/skips. Full SwiftFormat 0.64.0: 0/847 files. Complete structured diagnostics inspected. Three runtime priority warnings match the actual parent's test/message pairs; two additional runtime observations are in unchanged profile-destination/send-confirmation tests and SDK paths. Their timing causes remain unproven. The two compiler notices are the unchanged AppIntents metadata notices.All six released-native phases passed: rejected backup and lost acknowledgement, each with prepare, process restart/resume and restore. The signed operation stayed identical, no operation was attempted before an acknowledged backup, each scenario contained one unique operation, and the complete WALLET sentinel stayed unchanged. Complete result bundles and fixture event logs were inspected.
Two consecutive final independent reviews found no actionable findings after the ownership fixes. The backup format, required restore gates and acknowledgement-before-broadcast contract are unchanged.
Hosted workflow
40151127771forf204e05ealso passed: 2,906 unit tests and 39 UI executions, zero failures, with one native-fixture unit and four UI fixture skips. The complete log was inspected; diagnostics were limited to the installed-xcbeautify notice and multiple-matching-destinations warning. These standard CI tests do not execute the new authenticated fault journeys.Earlier parent composition at 2416046
Signed additive merge
24160466a4b6665dcff62d01ea1546b338d0b4deincorporates current #1121 at39fcaec8and the released Core RC11, Paykit RC2 and VSS RC1 adoption stack. The seven-file child delta retains immutable backup acknowledgement, observer lifecycle ownership and the existing navigation helper.Final combined-tree validation passed 2,968 selected units with zero failures/skips and 39 UI executions with zero failures (three Trezor fixture skips; the expired local-fork payment case was explicitly excluded). Full SwiftFormat 0.64.0: 0/849. Translation validation: zero errors and 15 actual-parent warnings. All 1,676 frozen input hashes match. Complete 29,901-line unit and 4,795-line UI logs and nested XCResult data were inspected. Two AppIntents notices occur in each run; unit diagnostics retain three native DHT/SDK QoS observations. UI diagnostics retain seven QoS summaries across 20 occurrences; all 534 exported frames lack symbols, so their precise cause is unproven.
Two consecutive independent final reviews are clean. Every commit in the complete six-commit current-parent range verifies locally and on GitHub (
verified=true,reason=valid). New-head hosted checks are pending. Opt-in native/integration exclusions and skipped fixtures are not passes. This composition does not establish authenticated downstream J1 or full-device backup fault acceptance; the earlier evidence below retains its exact revisions.Hosted validation at
bfc4a5e8— October 27Tests and SwiftFormat passed on the exact head tree. Complete logs, all nested XCResult test summaries, annotations and the digest/CRC-verified artifact were inspected. Unit execution counts are 2,969 passes and one native-fixture skip; UI counts are 39 passes and four fixture skips. Suite exclusions and migration-skipped jobs are not passes.
The build retains 286 distinct duplicate-symbol linker warnings. The actual-base local run does not emit them, although the seven-file PR diff leaves dependency/build/workflow configuration unchanged. Their cause and matching-hosted-base status remain unproven; they are not labelled pre-existing. No structured test failure or runtime issue was found. The pending current-master dependency composition will receive its own final validation; these results remain tied to
bfc4a5e8.Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh),gpt-6.1-sol(reasoning:high)