Skip to content

fix: recover hw transfer after re-pairing - #1133

Merged
jvsena42 merged 3 commits into
masterfrom
fix/hw-transfer-repair-1130
Aug 5, 2026
Merged

fix: recover hw transfer after re-pairing#1133
jvsena42 merged 3 commits into
masterfrom
fix/hw-transfer-repair-1130

Conversation

@jvsena42

@jvsena42 jvsena42 commented Aug 5, 2026

Copy link
Copy Markdown
Member

Fixes #1130

This PR restores the transfer label on a hardware wallet's funding transaction after the device is removed and re-paired.

Description

A transfer to spending from a paired hardware wallet settles once its channel becomes usable. Removing the device deletes that wallet's activities, and on re-pair the watcher rediscovers the funding transaction from scratch with no stored row left to carry the transfer metadata forward, so it came back looking like an ordinary send. Only settled transfers were affected: one whose channel had not opened yet is still being tracked and gets re-marked on its own.

Bitkit's own record of the transfer survives removal untouched and still holds the funding transaction id and the channel id, so the flag is now recovered from there while the rediscovered snapshot is merged.

The match runs in the safe direction only, from a rediscovered transaction id to an existing local transfer record. Nothing hardware-specific is retained past removal: the transaction id is re-supplied by the device's own watcher on re-pair, and the transfer record belongs to the Lightning channel that is still open, which the user never asked to delete. Removing a device and never re-pairing leaves no trace.

Recovery only ever fills gaps. Anything the app already knows wins, an existing channel id is never overwritten, and transactions with no matching transfer record are left alone.

This mirrors the iOS fix in synonymdev/bitkit-ios#648, with two deliberate differences. Its partial-snapshot handling is not ported, because a wallet here reports through a single watcher and callers already merge everything before persisting. And the new value is required rather than defaulted, since a default is what allowed the equivalent iOS tests to pass while silently skipping the recovery path entirely.

One consequence worth flagging for future work: settled transfers are now load-bearing for this recovery. The note suggesting they could be deleted once settled has been rewritten to say so, and a test pins the behaviour so acting on it fails loudly instead of quietly reintroducing this bug.

Preview

Before remove transfer row

1_before_remove_transfer_row

Bug - After repair, transfer row replaced with send row

2_bug_after_repair_sent_row

Fixed - After repair, transfer flow displayed

3_fixed_after_repair_transfer_row

QA Notes

Requires the Trezor Bridge emulator from bitkit-docker and a Bridge-enabled build, see journeys/hardware-wallet/README.md.

Manual Tests

  • 1. Pair a hardware wallet → Hardware Wallet → Transfer To Spending → sign, then wait for the channel to become usable: funding row reads Transfer with subtitle From Savings.
  • 2. Settings → Hardware Wallets → remove the device → re-pair the same device → Hardware Wallet: the same funding row still reads Transfer, not Sent.
  • 3. Tap the recovered row: Activity Detail shows FROM SAVINGS.
  • 4 regression: Transfer to spending while the channel is still opening → remove and re-pair before it settles: row still reads Transfer.

Automated Checks

  • Unit tests added: cover the recovery rule in CoreServiceTest.kt — recovery when no stored row remains, stored metadata winning over a recovered value, matching on transaction id rather than activity id since re-paired rows are rebuilt under new ids, unrelated transactions left unmarked, and Lightning activities passing straight through.
  • Unit tests added: cover the wiring in ActivityRepoTest.kt — known channel ids actually reach the merge, and a failed lookup still persists the snapshot instead of dropping a wallet's activity list. The equivalent iOS change has no coverage here.
  • Unit tests added: cover the lookup in TransferRepoTest.kt — settled transfers are included, records missing a funding transaction or channel are skipped, duplicates keep the first entry, and a database failure surfaces.
  • Regression pin added: TransferRepoTest.kt asserts that settling a transfer never deletes its record, so a future cleanup of settled transfers cannot silently reintroduce this bug.
  • Verified on device: reproduced the plain send on master against a Trezor Bridge emulator, then confirmed the row returns as a transfer after a fresh remove and re-pair on this branch.
  • CI: standard compile, unit test, and detekt checks run by the PR bot.

@greptile-apps

greptile-apps Bot commented Aug 5, 2026

Copy link
Copy Markdown

Greptile Summary

Restores transfer metadata when a removed hardware wallet is re-paired by matching rediscovered funding transaction IDs against retained transfer records.

  • Adds a funding-transaction-to-channel lookup covering settled transfers.
  • Passes the lookup into hardware snapshot merging while preserving existing metadata.
  • Adds repository and merge tests for recovery, precedence, lookup failures, duplicate handling, and settled-record retention.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issues identified.

The recovery is constrained to exact funding transaction IDs, fills only missing metadata, preserves existing values, and retains snapshot persistence when the auxiliary lookup fails.

Important Files Changed

Filename Overview
app/src/main/java/to/bitkit/repositories/ActivityRepo.kt Loads retained transfer mappings before persisting hardware snapshots and intentionally continues with an empty map if lookup fails.
app/src/main/java/to/bitkit/repositories/TransferRepo.kt Adds funding-transaction-to-channel lookup across retained transfer records and documents why settled records remain necessary.
app/src/main/java/to/bitkit/services/CoreService.kt Extends hardware snapshot merging to recover missing transfer metadata without overwriting incoming or stored channel metadata.
app/src/test/java/to/bitkit/repositories/ActivityRepoTest.kt Verifies transfer mappings are forwarded and snapshot persistence survives lookup failure.
app/src/test/java/to/bitkit/repositories/TransferRepoTest.kt Covers settled-record retention, lookup filtering, duplicate behavior, and database failures.
app/src/test/java/to/bitkit/services/CoreServiceTest.kt Covers transfer recovery, transaction-ID matching, metadata precedence, unrelated and Lightning activities.
changelog.d/next/1130.fixed.md Accurately describes the user-visible hardware-wallet transfer-label restoration.

Sequence Diagram

sequenceDiagram
    participant Watcher as Hardware Wallet Watcher
    participant ActivityRepo
    participant TransferRepo
    participant Core as ActivityService
    participant DB as Activity Database
    Watcher->>ActivityRepo: persistHwSnapshot(walletId, activities)
    ActivityRepo->>TransferRepo: getChannelIdsByFundingTxId()
    TransferRepo-->>ActivityRepo: fundingTxId → channelId
    ActivityRepo->>Core: replaceHwSnapshot(..., transfer map)
    Core->>Core: Merge stored metadata
    Core->>Core: Recover missing transfer metadata by txid
    Core->>DB: Delete stale and upsert merged activities
    Core-->>ActivityRepo: Persisted activities
Loading

Reviews (1): Last reviewed commit: "fix: recover hw transfer after re-pairin..." | Re-trigger Greptile

@jvsena42 jvsena42 self-assigned this Aug 5, 2026
@jvsena42
jvsena42 enabled auto-merge August 5, 2026 12:15
@jvsena42
jvsena42 requested a review from ovitrif August 5, 2026 12:58

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved.

@jvsena42
jvsena42 merged commit c5892e3 into master Aug 5, 2026
18 checks passed
@jvsena42
jvsena42 deleted the fix/hw-transfer-repair-1130 branch August 5, 2026 18:51
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.

Hardware wallet: a settled transfer displays as a plain send after remove + re-pair

2 participants