feat: persist hardware wallet activities - #648
Open
jvsena42 wants to merge 13 commits into
Open
Conversation
…oes through the reconciling path
jvsena42
marked this pull request as ready for review
August 3, 2026 17:36
Greptile SummaryThis PR makes hardware-wallet activity a wallet-scoped, persisted participant in activity storage and moves bitkit-core to 0.5.3.
Confidence Score: 4/5The failed-snapshot caching path should be fixed before merging because a transient persistence error can leave hardware activity permanently stale. The manager records a snapshot as persisted before its queued core write succeeds, while the write failure is swallowed, preventing identical watcher updates from repairing missing or stale stored activity. Files Needing Attention: Bitkit/Managers/HwWalletManager.swift
|
| Filename | Overview |
|---|---|
| Bitkit/Managers/HwWalletManager.swift | Aggregates and serializes wallet snapshots correctly in general, but failed writes poison the deduplication cache and are not retried. |
| Bitkit/Services/HwSnapshotMerge.swift | Reconciles complete snapshots while preserving locally stored transfer metadata by transaction ID. |
| Bitkit/Services/CoreService.swift | Adds wallet-scoped snapshot replacement and activity operations with serialized core access. |
| Bitkit/ViewModels/TransferViewModel.swift | Resolves the hardware wallet scope before signing and records the pending transfer activity in that scope. |
| Bitkit/Services/WatchOnlyAccountService.swift | Replaces the removed core serializer dependency with validated local extended-public-key decoding. |
| Bitkit/ViewModels/ActivityListViewModel.swift | Removes hardware duplicate collapsing and scopes boost-chain filtering by owning wallet. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
W[Hardware address-type watchers] --> A[HwWalletManager aggregates wallet snapshot]
A --> C{All configured watchers reported?}
C -- No --> P[Persist partial snapshot without pruning]
C -- Yes --> R[Persist complete snapshot and prune missing rows]
P --> Q[Per-wallet persistence queue]
R --> Q
Q --> M[Core merges transfer metadata]
M --> S[Wallet-scoped activities and transaction details]
T[Hardware transfer broadcast] --> I[Create pending transfer activity]
I --> S
Reviews (1): Last reviewed commit: "refactor: code cleanup" | Re-trigger Greptile
Comment on lines
+398
to
+399
| guard lastPersisted[group.walletId] != cacheKey else { return } | ||
| lastPersisted[group.walletId] = cacheKey |
There was a problem hiding this comment.
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.
Description
This PR ports bitkit-android #1044 to iOS, making watch-only hardware wallet activity a full participant in bitkit-core's wallet-scoped storage. iOS already stored hardware activity under a derived wallet id, so this fills the gaps that were left behind:
The PR also moves bitkit-core from 0.4.2 to 0.5.3. Version 0.4.2 was cut from a branch that never merged upstream, and its only unique symbol was the extended-public-key serializer used when claiming a watch-only account. That single dependency pinned the repo to a dead-end tag and blocked every upgrade, so this decodes the key locally instead, matching the approach in #632. Everything the hardware wallet integration relies on is unchanged between the two versions.
Linked Issues/Tasks
Screenshot / Video
Insert relevant screenshot / recording — a hardware receive showing a tag and populated Explore inputs/outputs, and a hardware transfer to spending showing one transfer row.
QA Notes
Manual Tests
regression:App on-chain and LN activity → Activity Item, tags, contacts, Explore, Savings and Spending lists, boost: behaviour matches master.regression:Settings → Advanced → Watch-only accounts → set up an account against Pubky Ring: the claim is still accepted after the local key decoding change.Automated Checks
BitkitTests/HwSnapshotMergeTests.swiftcovers the snapshot reconciliation rules — a stored row missing from a snapshot is dropped, a transfer is not, and transfer metadata is carried forward and matched by transaction id rather than activity id.BitkitTests/WatchOnlyAccountServiceTests.swiftpins the local extended-public-key decoding against bitkit-core's own test vectors, plus invalid character, bad checksum, off-curve key, and wrong-length rejection.BitkitTests/HwWalletManagerTests.swiftcovers transaction details reaching persistence scoped to the derived wallet id, details deduped across two address-type watchers, an unconfirmed transaction whose timestamp drifts between polls not re-writing to core, and wallet id resolution for a device.BitkitTests/TransferViewModelHwTests.swiftcovers an unresolvable wallet id aborting before the device signs or anything is broadcast.BitkitTests/HwWalletManagerTests.swiftandBitkitTests/HwWalletManagerFundingTests.swiftrecord wallet-scoped snapshots instead of bare activity lists;BitkitTests/HwTransferMocks.swiftgains wallet id resolution.BitkitTests/ActivityHardwareTests.swiftis gone because the funding transaction is now stored only under the hardware wallet, so there is no duplicate to collapse. It is replaced by a case proving the same transaction id in two wallet scopes stays two distinct activities.UtxoSelectionTestsandDustChangeHelperTestspass on their own but hit the live Blocktank regtest deposit endpoint and flaked once with a 404 during a long run.AddressTypeIntegrationTests,BlocktankTests,LdkMigration,ChannelPurchaseFlow, andBroadcastConnectivityTestsneed a live node and hang without the regtest stack. The manual tests above were also not run — they need a Trezor Bridge emulator, so the hardware journeys rest on review and QA.node scripts/validate-translations.jsreported no errors.