feat: persist hardware wallet activities - #648
Draft
jvsena42 wants to merge 7 commits into
Draft
Conversation
…oes through the reconciling path
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.