feat: hardware wallet detail screen#611
Conversation
|
Waiting for #605 merge to open this branch for review |
Observations from testingTesting observations (Trezor Safe 7, regtest + iPhone 13, stacked on #605 + #611) Logs: Ran through the QA checklist in #611 — detail screen, transfer placeholder, activity drill-down, remove flow, Send/Scan/Receive bar regression. All listed manual tests pass from a product perspective. Non-blocking follow-ups tracked in #613. QA checklist coverage
1. Remove from detail → Trezor settings still shows connected ❌After 4b (Remove from Hardware Wallet detail): Home tile and balance correctly disappear. But opening Settings → Advanced → Trezor Hardware Wallet still shows the device as connected (video attached). ScreenRecording_06-30-2026.11-09-36_1.MP4Likely cause: Session log at remove (09:09:47 UTC): Suggestion: when removing/forgetting the currently connected device, also disconnect and clear connection state (mirror Android 2. Device busy / ping cadence while Trezor is locked ℹ️While connecting with the Trezor locked (waiting for user to unlock), Bitkit keeps reopening the transport instead of backing off — same class of issue as Android bitkit-android#1030 (Device-state / ping-cadence). Not introduced by #611; track in #613. Observed in session logs ~09:10:41–09:11:02 UTC:
While the user is unlocking on-device, repeated transport opens/handshake retries can keep the Trezor busy. Follow-up tracking
Overall: #611 detail screen looks good — happy to approve. §1 is the only gap worth a quick fix in this PR; otherwise both items can land in #613. |
|
draft for implementing the first fix |
Greptile SummaryThis PR adds a
Confidence Score: 5/5Safe to merge — the new screen is additive, the remove flow is well-guarded, and the The No files require special attention.
|
| Filename | Overview |
|---|---|
| Bitkit/Views/Wallets/HardwareWalletScreen.swift | New detail screen for paired hardware wallets; mirrors Savings/Spending layout with balance header, activity list, transfer placeholder, and remove flow. Logic is correct; let wallet = wallet in body correctly eliminates repeated computed-property evaluation; auto-pop on wallet removal is guarded by walletsLoaded. |
| Bitkit/Managers/TrezorManager.swift | Adds active-session disconnect to forgetDevice: captures isActiveSession before any await suspension points, then calls disconnect() after storage cleanup. Ordering is correct — loadKnownDevices() runs before disconnect() so autoReconnect sees the updated (empty or trimmed) device list. |
| Bitkit/Components/TabBar/TabBar.swift | Replaces Set-based lookup with explicit switch; adds .hardwareWallet to the tab-bar-visible routes. Swift correctly pattern-matches associated-value cases without binding, so the case .hardwareWallet: clause compiles and behaves as intended. |
| Bitkit/ViewModels/NavigationViewModel.swift | Adds hardwareWallet(deviceId: String) route enum case. Straightforward and consistent with the existing route definitions. |
| Bitkit/Views/Home/HomeWalletView.swift | Replaces "Coming Soon" toast with navigation to the new hardwareWallet route; injects NavigationViewModel as @EnvironmentObject. Clean and minimal change. |
| Bitkit/Resources/Localization/en.lproj/Localizable.strings | Adds four new localization keys for the remove dialog, remove button, and the transfer-not-implemented toast. Keys follow existing naming conventions and are well-formed. |
| Bitkit/MainNavView.swift | Registers HardwareWalletScreen as the destination for the new .hardwareWallet(deviceId:) route. One-line addition consistent with existing route registrations. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
actor User
participant Home as HomeWalletView
participant Nav as NavigationViewModel
participant HW as HardwareWalletScreen
participant HWM as HwWalletManager
participant TM as TrezorManager
User->>Home: Tap hardware wallet tile
Home->>Nav: navigate(.hardwareWallet(deviceId:))
Nav->>HW: Push screen
HW->>HWM: wallet (computed, via deviceIds.contains)
HW->>HWM: loadActivities (via CoreService, walletId scoped)
User->>HW: Tap Remove device
HW->>HW: "showRemoveDialog = true"
User->>HW: Confirm remove
HW->>HWM: removeDevice(id: wallet.id) stops watchers, deletes activities
loop for each id in wallet.deviceIds
HW->>TM: await forgetDevice(id:) clears credentials, removes from storage
TM->>TM: loadKnownDevices()
TM->>TM: await disconnect() if isActiveSession
end
HWM-->>HW: wallets updated (wallet gone)
HW->>Nav: navigateBack() via onChange
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
actor User
participant Home as HomeWalletView
participant Nav as NavigationViewModel
participant HW as HardwareWalletScreen
participant HWM as HwWalletManager
participant TM as TrezorManager
User->>Home: Tap hardware wallet tile
Home->>Nav: navigate(.hardwareWallet(deviceId:))
Nav->>HW: Push screen
HW->>HWM: wallet (computed, via deviceIds.contains)
HW->>HWM: loadActivities (via CoreService, walletId scoped)
User->>HW: Tap Remove device
HW->>HW: "showRemoveDialog = true"
User->>HW: Confirm remove
HW->>HWM: removeDevice(id: wallet.id) stops watchers, deletes activities
loop for each id in wallet.deviceIds
HW->>TM: await forgetDevice(id:) clears credentials, removes from storage
TM->>TM: loadKnownDevices()
TM->>TM: await disconnect() if isActiveSession
end
HWM-->>HW: wallets updated (wallet gone)
HW->>Nav: navigateBack() via onChange
Reviews (2): Last reviewed commit: "fix: reduce wallet computation and asser..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 758a1dbe6b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 080608a8c6
ℹ️ 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".
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
Retested: fixed — after Remove, dev Trezor screen no longer shows a stale connected session.
3 participants
Part of #589
This PR adds a detail screen for paired hardware wallets. Tapping a device's tile on the home screen now opens a Savings/Spending-style overview of that watch-only wallet instead of a "Coming Soon" notice.
Description
This is the iOS port of synonymdev/bitkit-android#1022. iOS already had the watch-only data layer (per-device
deviceIds,removeDevice,walletsLoaded, blue hardware activity icons, andwalletId-scoped activity queries), so this PR is mainly the detail screen and its navigation wiring.Linked Issues/Tasks
Screenshot / Video
with-activities.mov
delete.mov
QA Notes
OBS: Only tested with emulator
Manual Tests
regression:Home, Savings, Spending, and All Activity -> bottom Send/Scan/Receive bar visible on each.Automated Checks
xcodebuild -workspace Bitkit.xcodeproj/project.xcworkspace -scheme Bitkit -configuration Debug -destination 'platform=iOS Simulator,id=<iPhone 16>' ONLY_ACTIVE_ARCH=YES build(concrete simulator UDID +ONLY_ACTIVE_ARCH=YESbecause the Rust xcframeworks are arm64-only).BitkitTests/HwWalletManagerTests(29 tests) — existing coverage forremoveDevice(stops watchers, deletes activities) and forget-via-update. No new tests added because the new code is SwiftUI view orchestration over the already-testedHwWalletManager.removeDeviceandTrezorManager.forgetDevice.node scripts/validate-translations.jscould not run locally (missingglobdependency / nopackage.jsonin this checkout); the added English keys are well-formed.