Skip to content

feat: home-screen hardware wallet (watch-only)#605

Open
jvsena42 wants to merge 52 commits into
masterfrom
feat/home-hw-wallet-ui
Open

feat: home-screen hardware wallet (watch-only)#605
jvsena42 wants to merge 52 commits into
masterfrom
feat/home-hw-wallet-ui

Conversation

@jvsena42

@jvsena42 jvsena42 commented Jun 22, 2026

Copy link
Copy Markdown
Member

This PR:

  1. Adds watch-only hardware wallet (Trezor) support to the home screen, ported from bitkit-android #999 — Bluetooth only.
  2. Bumps bitkit-core 0.1.66 → 0.3.4 and migrates the app to its wallet-scoped activity storage.
  3. Refactors the Trezor service layer: extracts a TrezorManager from the TrezorViewModel god object, adds a fully decoupled HwWalletManager, and splits the device-less on-chain calls out of TrezorService into a vendor-neutral OnChainHwService.

Description

Brings the first slice of the Trezor hardware-wallet epic to the wallet home screen. A paired Trezor now shows as a watch-only balance: a device row under the Savings/Spending tiles, its sats folded into the headline total, and its on-chain transactions merged into the activity lists with a blue icon. Users with no paired device see a new Hardware suggestion card that opens an intro sheet, and a one-time pairing code is handled by an app-wide Pair Device sheet.

How it works: on connect, each device's account xpubs are captured and persisted. The watch-only layer then runs one on-chain xpub watcher per device and enabled address type, aggregates the per-device balance in memory, and persists each device's activity into bitkit-core scoped by a derived wallet id — so balances and activity stay available while the device is disconnected, and the activity lists merge hardware transactions through the normal pipeline. The same physical device re-paired is de-duplicated by its xpubs into a single tile.

Scope notes:

  • Bluetooth only: USB-specific transport, icons, and reconnect handling from the Android PR are intentionally omitted.
  • The intro sheet's Continue is intentionally disabled — the connect flow ships in a follow-up; this slice covers the home surface, suggestion card, and the app-wide pairing sheet.
  • Adopting wallet-scoped activities required bumping bitkit-core to 0.3.4, which adds a wallet_id to every activity. This migrates the whole app (not just hardware) to pass a wallet id through the activity APIs; the normal wallet uses the core default.
  • The per-device wallet-id is derived via bitkit-core's deriveWalletId — the canonical cross-platform derivation finalized in core 0.3.4 — so iOS and Android produce identical ids for the same device.
  • Architecture: the device/connection orchestration + pairing dialogs were extracted out of TrezorViewModel into a TrezorManager, and the watch-only HwWalletManager is driven purely by the composition root (no manager-to-manager or service-to-viewmodel coupling). The device-less on-chain surface — Electrum account/address info, tx history/detail, compose, broadcast, and the xpub watcher — was also split out of TrezorService into a vendor-neutral OnChainHwService behind an OnChainWatcherServicing protocol. HwWalletManager now depends on no Trezor-named type; TrezorService is reduced to Trezor device FFI (connect, sign, on-device address/pubkey, credentials).

Linked Issues/Tasks

Ports synonymdev/bitkit-android#999.
Related to #589

QA Notes

Trezor Emulator Setup

Test against the Trezor emulator from bitkit-docker:

  1. Start the backend + emulator: clone with --recurse-submodules, then docker compose up -d and ./scripts/trezor-emulator start.
  2. Run a Debug build (regtest) with these env vars set in the Xcode scheme (Edit Scheme → Run → Arguments → Environment Variables):
    • TREZOR_BRIDGE=true
    • TREZOR_BRIDGE_URL=http://127.0.0.1:21325
    • Do not set TEST_TREZOR_EMU — that flag boots the standalone dev/test harness and bypasses the wallet.
  3. Enable Dev Settings, then open Settings → Advanced → Trezor Hardware Wallet.

Manual Tests

  • 1. No paired device → Home: no hardware row, Hardware suggestion card shown.
  • 2. Tap Hardware suggestion card → Hardware intro in-sheet opens: device visuals shown, Cancel works, Continue disabled.
  • 3. Connect a Trezor (dev Trezor screen or Bridge emulator) → Home: hardware row appears with device name, blue BTC icon and balance; headline total includes the hardware balance; recent device transactions appear in the activity list with blue icons.
  • 4a. BLE-connected device → disable phone Bluetooth: indicator turns grey, balance stays visible.
    • 4b. re-enable Bluetooth: indicator turns green again on its own (auto-reconnect, no pairing prompt).
  • 5. Device asks for its one-time pairing code → Pair Device in-sheet opens with the app numpad; entering the code on-screen completes the connect; swipe-down dismiss cancels it.
  • 6. Tap the hardware row: not-yet-implemented toast shows.
  • 7. Tap a hardware activity item in Home → Activity Detail opens with a blue icon.
  • 8. Watcher running → send sats to a Trezor address from an external wallet: received sheet pops up once picked up.
  • 9. Re-pair the same Trezor → Home shows one tile, balance and activity counted once.
  • 10a. Show All → All Activity: hardware activities listed with blue icons; Sent/Received tabs filter them.
    • 10b. tag filter: hides hardware activities (they carry no tags).
  • 11. regression: open Receive and other large sheets, and the normal on-chain/LN activity list and detail behave as before (wallet-scoped activity migration).

Automated Checks

  • Unit tests added: BitkitTests/HwWalletManagerTests.swift (aggregation, xpub dedup, displayName, merge-by-txid, received-tx detection, monitored-type filtering, watcher params, saturating total, stale-watcher, removeDevice, resetState), BitkitTests/HwWalletIdTests.swift, BitkitTests/HwAddressTypeTests.swift, BitkitTests/ActivityHardwareTests.swift.
  • Unit tests modified: BitkitTests/TrezorViewModelWatcherTests.swift retargeted to TrezorManager after the extraction; the MockWatcherService mocks in TrezorViewModelWatcherTests and HwWalletManagerTests now conform to the new OnChainWatcherServicing protocol.
  • Ran locally: full BitkitTests suite — 553/556 pass; the 3 failures are pre-existing AddressTypeIntegrationTests cases that hit the live Blocktank regtest deposit API (HTTP 404), unrelated to this change. swiftformat --lint, scripts/validate-translations.js, and a Debug build (iPhone 16 simulator) are clean.

Screenshot / Video

connect-and-disconnect.mov

@jvsena42 jvsena42 self-assigned this Jun 22, 2026
@ovitrif ovitrif added this to the 2.4.0 milestone Jun 22, 2026
Comment thread Bitkit/Models/HwWalletId.swift Outdated
Comment thread Bitkit/Services/MigrationsService.swift
Comment thread Bitkit/Models/HwWallet.swift
@jvsena42

This comment was marked as resolved.

@ovitrif

This comment was marked as outdated.

@jvsena42

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bae69fd746

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread Bitkit/Components/Widgets/Suggestions.swift Outdated
Comment thread Bitkit/Managers/HwWalletManager.swift
@jvsena42

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5777dbbb4e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread Bitkit/Managers/HwWalletManager.swift Outdated
@piotr-iohk

Copy link
Copy Markdown
Collaborator

Observations from initial testing

Initial observations (Trezor Safe 7, regtest + iPhone 13)

Logs: bitkit_logs_2026-06-25_14-26-56.zip

Test address (regtest native segwit): bcrt1qvp6fy27q7gmn2en2t2wyt90uczjs894qmfud60 — worked on Android; iOS showed 0 until clean re-pair.

1. first connect (problematic) — logs ~13:41 UTC ❌

  • BLE xpub enumeration under load: Timed out sending data to Trezor
  • Could not read xpub for 'nestedSegwit'
  • Saved known device: Trezor Safe 7 (2I1) with 3 xpubs (3/4)
  • Watcher did start (startWatcher ~2.95s) but Home showed 0 balance
Home hardware row with 0 balance after first connect

2. recovery

  • Forgot on iOS (+ removed from Android, forgot Trezor pairing) → reconnected on iOS only
  • Saved known device: Trezor Safe 7 (6N2) with 4 xpubs, new BLE path, balance + activity appeared

3. follow-up retest

  • Reconnected on iOS while Trezor still paired/connected on Android → worked (balance OK)
  • Dual-phone BLE is not a deterministic failure; first session was likely partial xpub / flaky BLE state, not a hard “one phone only” rule

Follow-up suggestion (non-blocking):

  • Don't treat connect as complete if monitored address-type xpubs failed (today failures are swallowed; device saved with partial xpubs)
  • Optional: retry xpub fetch or user-visible warning on BLE timeout during capture

In general the watch-only path works once xpub capture completes cleanly (4/4). First connect with 3/4 xpubs left Home at 0 despite the watcher starting.

Other observations:

  • Hardware suggestion card not visible when in-app wallet balance is 0 — .empty suggestion ordering omits hardware; Android shows it in empty-wallet state ❌
  • Perhaps move "Trezor Hardware Wallet" from Settings → Advanced into Dev Settings for consistency (entry is already dev-gated)

@jvsena42

Copy link
Copy Markdown
Member Author

Perhaps move "Trezor Hardware Wallet" from Settings → Advanced into Dev Settings for consistency (entry is already dev-gated)

Agreed, I can move it on the last polish PR

@jvsena42

Copy link
Copy Markdown
Member Author

Follow-up (non-blocking): watch-only first-sync latency — balance & activities take a few seconds to appear after connect

While re-testing on HEAD against the Trezor Bridge emulator, the hardware balance — and the device's activities — took several seconds to appear after a successful connect. Traced it in the session log:

Time (UTC) Event
16:59:37.433 === Connecting to device: bridge:1 ===
16:59:37.45–.50 getPublicKey (~10 ms each) — xpub capture is instant
16:59:37.501 Saved known device: Trezor Bridge Emulator with 4 xpubs
16:59:45.029 startWatcher(params:listener:) took 7.68 seconds

Root cause: connect + xpub capture complete in ~70 ms; the lag is entirely startWatcher blocking ~7.7 s on its initial Electrum descriptor scan (gap-limit derivation + history fetch, in bitkit-core). That single first event carries both the balance and the activity set, so:

  • balance can't render until startWatcher returns (~7.7 s), and
  • activities appear slightly later, after that first event is upserted into core and the activity list reloads.

So "balance took a while" and "activities took a while" are the same first-sync latency — not a correctness issue, and distinct from the earlier 0-balance report (where the watcher started but never reported). Note this was over Bridge with only nativeSegwit monitored (one watcher); more monitored address types → more watchers → potentially longer initial wait, and a slower network would make the scan longer.

Suggested follow-up (app-side): show a loading state (spinner/skeleton) on the hardware row + activity placeholders while a device's first watcher sync is in flight, instead of a bare 0/empty — during those ~7.7 s the tile currently reads as "stuck at zero." The scan time itself lives in bitkit-core's startWatcher and is only optimizable there.

@jvsena42 jvsena42 marked this pull request as ready for review June 25, 2026 17:45
@jvsena42

jvsena42 commented Jun 25, 2026

Copy link
Copy Markdown
Member Author
  • Don't treat connect as complete if monitored address-type xpubs failed (today failures are swallowed; device saved with partial xpubs)
  • Optional: retry xpub fetch or user-visible warning on BLE timeout during capture

Fixed in 4d788bd

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 14566f6550

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread Bitkit/Managers/HwWalletManager.swift Outdated
Comment thread Bitkit/Managers/TrezorManager.swift
Comment thread Bitkit/AppScene.swift Outdated
@greptile-apps

greptile-apps Bot commented Jun 25, 2026

Copy link
Copy Markdown

Want your agent to iterate on Greptile's feedback? Try greploops.

@ovitrif ovitrif self-requested a review June 25, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants