Skip to content

refactor: use core hw activities#1044

Draft
ovitrif wants to merge 17 commits into
feat/wallet-scoped-core-0.3.9from
refactor/core-hww-activities-1029
Draft

refactor: use core hw activities#1044
ovitrif wants to merge 17 commits into
feat/wallet-scoped-core-0.3.9from
refactor/core-hww-activities-1029

Conversation

@ovitrif

@ovitrif ovitrif commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Closes #1029
Refs #998

Description

This PR refactors hardware-wallet activity handling to use bitkit-core wallet-scoped activities instead of keeping Trezor transactions as separate in-memory watch-only entries.

This PR:

  1. Bumps bitkit-core to 0.3.9 and keeps activity reads, tags, transaction details, deletion, metadata, and seen-state calls scoped by wallet id.
  2. Uses enum-backed wallet ownership helpers and delegates hardware wallet id derivation to bitkit-core, avoiding duplicated hashing logic in the app.
  3. Derives stable hardware wallet ids from account xpubs and starts Trezor watchers with those ids so emitted activities are scoped to the correct wallet.
  4. Persists watcher-emitted hardware activities and transaction details into Core before exposing watcher state, and ignores watcher events that do not belong to an active wallet-scoped watcher.
  5. Keeps wallet scope attached when opening Activity Detail, Explore, and Boost flows, preventing duplicate txid/activity collisions across wallets.
  6. Keeps Core activity and hardware-wallet domain models flowing through ViewModels and Compose, with only small display helpers for variant-specific fields.
  7. Aggregates duplicate on-chain rows emitted by multiple hardware address-type watchers so hardware balances and activity rows use the wallet-level net amount.
  8. Deletes wallet-scoped hardware activity data when a paired hardware wallet is removed and surfaces purge failures instead of reporting removal success with stale Core activity data.
  9. Removes the stale LNURL-pay MetadataMismatch handling accidentally carried by this branch; the LNURL validation change is already on master and there is no replacement exception to map.

Out of scope for #1030:

  1. Backing up and restoring paired-device records.
  2. Adopting the new typed Trezor PIN, lock, and busy error types beyond the compile-time call-site updates included here.

Preview

QA Notes

Manual Tests

  • 1. Hardware seed distinct from Bitkit wallet seed → pair a Trezor → Wallet Home: hardware transactions appear with blue icons in the activity list.
  • 2. Wallet Home → Show All → switch activity tabs and apply a tag filter: hardware activities remain visible when they match the selected filter.
  • 3. Hardware activity → Activity Detail → Add Tag → back → reopen Activity Detail: tag persists.
  • 4. Hardware activity → Activity Detail → Explore: inputs and outputs render.
  • 5. Hardware activity → Activity Detail → Boost: sheet loads the wallet-scoped Core activity and does not fall back to all-wallet lookup.
  • 6. Settings → Hardware Wallets → remove the paired device: its hardware activities disappear; re-pairing rebuilds history from the watcher.
  • 7. regression: normal on-chain/LN activity → Activity Detail → tags, Explore, and Boost: behavior matches master.

Automated Checks

  • Unit tests modified: HwWalletRepoTest.kt covers active-watcher ownership, Core persistence ordering, duplicate address-type aggregation, wallet-id watcher restarts, and removal purge failures.
  • Unit tests modified: TrezorRepoTest.kt, ActivityRepoTest.kt, ActivityDetailViewModelTest.kt, ActivityListViewModelTest.kt, AppViewModelSendFlowTest.kt, NotifyPaymentReceivedHandlerTest.kt, and PreActivityMetadataRepoTest.kt cover deterministic hardware wallet id derivation, scoped activity lookup/navigation, hardware tag/filter behavior, sheet detail routing, and nullable wallet-scope defaults.
  • Unit tests modified: HomeViewModelTest.kt, HwWalletViewModelTest.kt, and BoostTransactionViewModelTest.kt cover hardware-wallet home/settings state and the public boost setup path used by the sheet.
  • Journey coverage updated: activity-blue-icons.xml covers unified hardware activity list/tab behavior; activity-detail-hw-tags.xml covers hardware tags and Explore inputs/outputs; hardware transfer journeys now handle the first-run Transfer To Spending intro before asserting the amount screen.
  • Local journey validation: Bridge Trezor emulator exercised connect-home-tile.xml, activity-blue-icons.xml, activity-detail-hw-tags.xml, and detail-overview.xml on 8372a730f, covering pair/fund, blue activity rows, tabs, persisted tag + tag filter, Explore inputs/outputs, hardware wallet detail, transfer amount screen after first-run intro, remove cancel/confirm, watcher stop, and hardware activity purge.
  • Local verification: just compile, just test, and just lint passed on c0274e311; xmllint passed for updated hardware-wallet journey XMLs on 8372a730f.

@greptile-apps

This comment has been minimized.

greptile-apps[bot]

This comment was marked as resolved.

@ovitrif ovitrif self-assigned this Jun 24, 2026
chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ovitrif ovitrif marked this pull request as draft June 24, 2026 22:22
@ovitrif ovitrif changed the title feat: integrate hardware wallet activities with core refactor: use scoped hw activities Jun 24, 2026
@ovitrif ovitrif changed the title refactor: use scoped hw activities refactor: use core hw-scoped activities Jun 25, 2026
@ovitrif ovitrif changed the title refactor: use core hw-scoped activities refactor: use core hw activities Jun 25, 2026
@ovitrif

This comment was marked as outdated.

@ovitrif ovitrif force-pushed the refactor/core-hww-activities-1029 branch 2 times, most recently from f3f9a2c to 435d9b0 Compare July 4, 2026 19:35
@ovitrif

ovitrif commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

Status update:

  • This PR now consumes bitkit-core 0.3.9.
  • Local verification passed on c0274e311: just compile, just test, and just lint.
  • There are no unresolved review threads currently open.
  • Coordination note: feat: bump core to 0.3.9 and hw reliability #1062 touches much of the same Core/HW wallet surface. I’d recommend keeping this PR as the base for the Core hardware-activity refactor, then rebasing or splitting feat: bump core to 0.3.9 and hw reliability #1062 afterward so the hardware reliability fixes stay separately reviewable.

@ovitrif

ovitrif commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

Status update after HW journey validation:

  • The core unblock is resolved for this branch: it consumes bitkit-core 0.3.9 and the Android artifact from the publish task.
  • I re-ran the Bridge Trezor emulator flows on 8372a730f: connect-home-tile.xml, activity-blue-icons.xml, activity-detail-hw-tags.xml, and detail-overview.xml.
  • Validation passed for the important HW activity/wallet paths: deterministic Trezor pair/fund, blue Core-backed activity rows, All Activity tabs, persisted hwtest tag, tag filter, Explore txid/input/output data, hardware wallet detail, transfer amount screen after the first-run intro, remove cancel, final remove, watcher stop, and hardware activity purge.
  • The app log confirms cleanup after removal: Deleted '1' activities for hardware wallet ....
  • I also updated the HW transfer journey specs so fresh emulator runs handle the first-run Transfer To Spending intro before asserting HardwareTransferAmount; otherwise the journey can false-fail before reaching the amount screen.
  • No fatal/crash signatures showed up in logcat. Existing local checks remain: just compile, just test, and just lint passed on c0274e311; xmllint passed for the updated HW journey XMLs on 8372a730f.

Remaining handoff: wait for checks/reviews on 8372a730f. If those settle cleanly, this should be ready to move out of draft/request review. #1062 overlaps with this work and should likely be rebased or split after this lands, but treat that as coordination guidance rather than a blocker for this branch.

@piotr-iohk

Copy link
Copy Markdown
Collaborator

Trezor Android: #1044 vs #1062 — QA findings

Date: 2026-07-06
Tested: refactor/core-hww-activities-1029 (#1044) on physical device (to.bitkit.dev)
Comparison: feat/wallet-scoped-core-0.3.9 (#1062) shows correct Trezor balance on same device

Scope mismatch

#1044 may be a stretch for 2.4.0: iOS Trezor is still behind, and for cross-platform consistency we should not ship HW tags or Explore inputs/outputs in 2.4.0. That fits 2.5.0 / #1029 instead.

Finding 1: Trezor shows 0 balance + empty activity list (#1044)

Symptom: Paired Trezor with known funds/history → app shows 0 sats and no activities.

Not a Trezor/connect issue. Watcher starts; Core emits data; UI never updates.

Cause: In HwWalletRepo.observeWatcherEvents(), persistHardwareActivities() runs before _watcherData is updated. On failure it return@collect — balance and activities never reach UI.

Log evidence (bitkit_2026-07-06_08-34-36.log):
bitkit_logs_2026-07-06_08-50-23.zip

Started watcher 'ble:5B:B1:C8:E9:4C:BE|nativeSegwit'
ERROR Failed to persist hardware activities
  AppError='Failed to upsert onchain_activity: UNIQUE constraint failed: onchain_activity.id'
hardwareWallets=[{"id":"ble:5B:B1:C8:E9:4C:BE","sats":0}]

Likely persist root cause: HW activity ids = txids. Error on onchain_activity.id suggests stale pre-0.3.9 schema (id globally unique) or failed migration — collides when same tx exists on Bitkit wallet and Trezor (e.g. transfer between wallets). Fresh install / clear app data should confirm.

#1062: Updates _watcherData from watcher first; does not gate UI on persist. Works on same device.

Finding 2: Transfer shows duplicate rows — Transfer + Sent (#1044)

On transfer to spending, activity list shows two rows for one flow:

  • Transfer (Bitkit wallet)
  • Sent (Trezor / HW wallet)

Both appear in unified list; both are taggable. Not a quick polish issue — needs product rules for cross-wallet dedup/linking (transfer_tx_id, filtering, etc.). #1044 merges dupes across address-type watchers, not Bitkit↔Trezor transfer pairs.

Finding 3: Scope ahead of 2.4.0 (#1044)

#1044 adds:

  • Tag support on HW activities
  • Explore inputs/outputs
  • Boost with wallet-scoped Core lookup
  • HW activities in global Activity tab (filters, tags, blue icons)

Reasonable for 2.5.0 / #1029, but not aligned with a minimal, iOS-consistent 2.4.0 Trezor release.

Prod / migration concern

Not a prod issue today — no Trezor in prod.

Before HW launch (2.5.0): migration must work on upgrade; UI must not depend on persist succeeding; transfer dedup needs design.

Clear app data is a dev QA workaround, not a ship strategy.

Recommendation

Release PR Action
2.4.0 Trezor v1 #1062 Ship — minimal scope, works on device, #1030 reliability
2.5.0 #1044 Keep draft — fix persist gate, migration/collision, transfer dedup, then tags/Explore/Boost QA

Later: cherry-pick #1062 reliability commits onto #1044 for 2.5.0; don’t merge as competing 0.3.9 paths.

#1044 fixes needed (2.5.0, not blocking v1)

  1. Decouple UI from persist — update watcher state even if Core persist fails; log error separately.
  2. Fix persist failure — ensure 0.3.9 (wallet_id, id) migration on upgrade; handle shared txids across wallets.
  3. Transfer dedup UX — one logical row per transfer, not Transfer + Sent both taggable.
  4. Continue Bitkit Core support for hardware wallet activities #1029 scope (tags, Explore, Boost) only after above is stable.

Links

@ovitrif

ovitrif commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Decouple UI from persist — update watcher state even if Core persist fails; log error separately.

Addressed this Android-side gating issue: watcher state now updates before Core persistence, so the Trezor balance/activity UI stays live even if a Core upsert fails. Persistence failures are still logged with the watcher id, and received-transaction emission remains behind successful persistence.

Validation: just compile, just test, just lint.

The migration/collision and transfer dedup points remain broader draft follow-ups outside this scoped fix.

Resolved in f546f58

@ovitrif ovitrif added this to the 2.5.0 milestone Jul 6, 2026
@ovitrif

ovitrif commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Set milestone to 2.5.0 for clarity 👍🏻 , thanks for review & checks @piotr-iohk 🙏🏻

github-advanced-security[bot]

This comment was marked as resolved.

@ovitrif ovitrif changed the base branch from master to feat/wallet-scoped-core-0.3.9 July 7, 2026 12:13
@ovitrif ovitrif force-pushed the refactor/core-hww-activities-1029 branch from 708cd21 to 794722a Compare July 7, 2026 12:13
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.

Bitkit Core support for hardware wallet activities

4 participants