fix: back up hardware wallet activity tags - #675
Open
jvsena42 wants to merge 5 commits into
Open
Conversation
jvsena42
marked this pull request as ready for review
August 19, 2026 16:08
Greptile SummaryThis PR preserves hardware-wallet activity tags by carrying them through metadata backups and rewrites successfully migrated legacy backup envelopes.
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code defect identified. The new backup representation follows the documented wallet-scoped matching and deduplication contracts, migration rewrites occur only after restored rows persist, and hardware tag mutation and deletion paths invalidate the metadata backup.
|
| Filename | Overview |
|---|---|
| Bitkit/Services/BackupFieldMigration.swift | Adds parsed-JSON migration change detection with per-field failure isolation for deciding whether restored envelopes need rewriting. |
| Bitkit/Services/BackupService.swift | Applies migrations during restore, rewrites successfully migrated categories, and includes derived hardware tags in metadata backups. |
| Bitkit/Services/CoreService.swift | Emits metadata invalidation for hardware tag changes and exposes hardware tags as pre-activity metadata. |
| Bitkit/Services/HwActivityTagBackup.swift | Maps wallet-scoped hardware tags to received- or sent-activity lookup keys and deduplicates metadata by Core's primary key. |
| BitkitTests/BackupFieldMigrationTests.swift | Covers semantic migration comparison, malformed inputs, partial failures, and preservation of envelope fields. |
| BitkitTests/HwActivityTagBackupTests.swift | Covers mapping keys, neutral attachment fields, timestamp conversion, wallet isolation, filtering, and deduplication. |
Sequence Diagram
sequenceDiagram
participant App as Bitkit App
participant Core as Bitkit Core
participant VSS as Backup Server
participant HW as Hardware Watcher
App->>Core: Read hardware activity tags and activities
Core-->>App: Wallet-scoped tags and on-chain activities
App->>App: Convert tags to pre-activity metadata
App->>VSS: Upload metadata envelope
VSS-->>App: Restore legacy/current envelopes
App->>Core: Migrate Core-owned JSON fields
App->>Core: Persist activities, tags, and metadata
alt Migration changed persisted fields
App->>VSS: Rewrite current wallet-scoped envelope
end
HW->>Core: Recreate hardware activities after pairing
Core->>Core: Attach matching pre-activity metadata tags
Reviews (1): Last reviewed commit: "chore: add code comments" | Re-trigger Greptile
Member
Author
|
Still testing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR:
Ports synonymdev/bitkit-android#1163 and completes the app half of synonymdev/bitkit-core#113, subtask 6 of the Trezor epic #589. The migrate-before-decode step landed here in 8cd698d; this PR adds the rewrite and the missing tag coverage.
Description
Core made activity data wallet-scoped, where the normal wallet is the default wallet id and a paired Trezor is
trezor:{hash}. Two gaps remained on the app side.Tagging a Trezor activity stores the tag under that device's wallet id, and those tags never reached the backup, so they were lost on restore. Hardware activities themselves are deliberately still not backed up: the device watcher rebuilds them on every reconnect, so backing them up would upload data that is immediately overwritten.
That leaves an ordering problem. Activity tags are constrained by a foreign key onto the activity they belong to, and on restore a hardware tag has no parent activity yet, because the device has not been paired again. Restoring hardware tags as activity tags therefore fails the constraint, and since the activity restore upserts activities, tags and closed channels together, one orphan tag takes the whole category down.
So hardware tags travel as pre-activity metadata instead. That table has no foreign key, it is already backed up across every wallet scope, and Core already re-attaches pre-activity metadata when an activity appears, matching received activities on address and sent activities on payment id. The app writes hardware tags in that shape at backup time, where the activity is still available to pick the right key, and Core reattaches them by itself once the watcher recreates the rows. No deferred replay or ordering hook is needed in the app.
The backup server also never received the migrated envelope. Restoring an older backup asked Core to fill in missing wallet ids, used the result, then left the old envelope in place, so every future restore repeated the migration. The app now detects when Core actually changed a field and re-uploads that category once the restore finishes. A category is only rewritten after Core has persisted the migrated rows, otherwise a failure mid-restore could replace a good backup with empty state. Envelopes that already carry wallet ids are compared as parsed JSON rather than raw text, so key reordering by Core cannot trigger a pointless upload.
The metadata backup is also now marked stale only when tags in a hardware scope actually change, rather than on every payment and sync, so ordinary activity traffic no longer re-uploads that envelope. A watcher snapshot raises the signal only when it deleted rows, since a plain upsert cannot drop a tag.
Envelope format is unchanged, no app-side wallet id migration semantics were added for Core models, and bitkit-core stays at 0.5.3.
Companion Android PR: synonymdev/bitkit-android#1163
Linked Issues/Tasks
Screenshot / Video
QA Notes
Requires a paired Trezor (the bitkit-docker emulator is enough) and a wallet with existing activity.
Manual Tests
regression:Hardware Wallets → unpair a device: its activities and tags are removed and the backup syncs.Automated Checks
BitkitTests/HwActivityTagBackupTests.swift, covering received keyed by address, sent keyed by payment id, fee rate/transfer flag/channel id left unset so Core cannot overwrite the activity it attaches to, seconds to millis conversion, cross-wallet isolation, and deduplication on Core's(wallet id, payment id)primary key.BitkitTests/BackupFieldMigrationTests.swift, covering a legacy envelope reporting a change, an envelope whose keys Core reordered reporting no change, a failing migration keeping the original field without blocking the others, and non-object input passing through untouched.AddressTypeIntegrationTestsandUtxoSelectionTestsfailing onFailed to deposit to address: Blocktank error, which need the bitkit-docker regtest stack and are unrelated to this change.node scripts/validate-translations.js: 0 errors, no new strings.swiftformat --lint: clean for every file in this branch.Known Limitation
Core keys received pre-activity metadata by address on both write and read, so several tagged receives to one reused address cannot all round trip: the upsert deletes by address per record, and re-attachment matches the first row for that address. Tracked in synonymdev/bitkit-core#135. Tags on distinct addresses, and all sent tags, are unaffected.