Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive VSS backup support for wallet, metadata, blocktank, activity, and lightning connections categories. The changes consolidate backup configuration, refactor SettingsStore functionality into SettingsViewModel, introduce reactive publishers for state changes, and implement backup/restore operations for all data categories.
Key Changes:
- Consolidated backup configuration into
SettingsBackupConfigfor better maintainability - Merged
SettingsStoreintoSettingsViewModelwith reactive publishers for backup triggers - Added backup/restore support for wallet, metadata, blocktank, and activity categories
- Implemented change detection publishers in core services to trigger backups automatically
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| BackupSettings.swift | Refactored color logic into BackupViewModel helper methods |
| SettingsViewModel.swift | Merged SettingsStore functionality, added KVO observers and publishers for backup triggers |
| BlocktankViewModel.swift | Added state change notifications after data modifications |
| BackupViewModel.swift | Added helper methods for status color determination |
| WidgetsBackupConverter.swift | Simplified position value parsing logic |
| TransferStorage.swift | Made singleton, added change publisher, exposed getAll() method |
| TransferService.swift | Updated to use TransferStorage singleton |
| SettingsStore.swift | Deleted entire file (functionality moved to SettingsViewModel) |
| ServiceQueue.swift | Added dedicated backup queue |
| LightningService.swift | Added sync status change publisher |
| CoreService.swift | Added change publishers and upsert methods for activities and blocktank data |
| BackupService.swift | Implemented backup/restore for all categories, replaced NotificationCenter with Combine |
| SettingsBackupConfig.swift | New configuration file for backup settings |
| BackupPayloads.swift | New models for backup data structures |
| BackupCategory.swift | Renamed ldkActivity to activity, removed slashtags |
| AppScene.swift | Updated to use SettingsViewModel singleton and Combine publishers |
| Package.resolved | Updated bitkit-core dependency |
|
nit: Keeping the logging a bit consistent between the 2 platforms would help us be able to reuse our debugging skills in PROD from iOS to Android. What I try to say is that the logging that got removed in this PR but were added in the previous one, were fine to remain IMHO. |
ovitrif
left a comment
ovitrif
left a comment
There was a problem hiding this comment.
Tested 🟢
- Transfer back to savings without 6 block confirmations
- Backup & restore
- expect transfer pending balance
|
@ben-kaufman I approved the PR because I consider it a valid step forward which doesn't break stuff; but I would still expect the relevant comments to be resolved, either in next PR or here 🙏🏻 |
|
I've already pushed for the comment on LDK sync here, I'll update the logs in another PR then |
ovitrif
left a comment
ovitrif
left a comment
There was a problem hiding this comment.
reapproved
Reran Test: 🟢 ✔️ |
| @MainActor | ||
| class SettingsViewModel: ObservableObject { | ||
| class SettingsViewModel: NSObject, ObservableObject { | ||
| static let shared = SettingsViewModel() |
There was a problem hiding this comment.
By turning this view model into a singleton there is now a lifecycle issue when wiping the app (in AppReset.swift). The singleton will keep pre-wipe state and any observers may behave in unexpected ways. Solution would be to either revert this to a instance-based object or add any needed reset() methods and call them when wiping the app.
…egories Add VSS backup for all categories
…egories Add VSS backup for all categories
This PR adds support for wallet, metadata, blocktank, activity, and lightning connections categories for VSS backups, and fixes issues from the previous VSS backup PR.
Screen.Recording.2025-11-06.at.10.26.57.AM.mov