Skip to content

feat: add trezor send - #1187

Merged
ovitrif merged 5 commits into
masterfrom
feat/trezor-send
Aug 28, 2026
Merged

feat: add trezor send#1187
ovitrif merged 5 commits into
masterfrom
feat/trezor-send

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Description

This PR:

  1. Adds paired Trezor wallets as funding sources throughout the normal on-chain Send flow, including scanner, paste, manual entry, contacts, fee selection, confirmation, signing, broadcasting, and success handling.
  2. Hardens Trezor session recovery and preserves hardware-wallet activity and contact metadata while wallet snapshots catch up.

The Send UI follows the Bitkit Wallet design.

Receive support will follow in a separate stacked PR.

Linked Issues/Tasks

N/A

Screenshot / Video

QA Notes

Manual Tests

  • 1. Trezor wallet → Send → enter a Bitcoin address and amount → Continue: the button loads once, Confirm opens, and repeated taps do not duplicate preparation.
  • 2. Send Confirm → choose Trezor → Sign With Device → approve on Trezor: the transaction broadcasts and Success shows the hardware-wallet activity.
  • 3. Trezor passphrase wallet → Send → enter passphrase: the paired account reconnects and the operation resumes.
  • 4. Trezor-funded Send → scan a Lightning or LNURL request: Bitkit explains that a Bitcoin address is required.
  • 5. regression: Send → switch between Savings, Spending, and Trezor: available balance and fee-aware maximum update for each source.
  • 6. regression: cancel or disconnect during signing → retry: Bitkit reconnects without creating or broadcasting a duplicate transaction.

Automated Checks

  • Unit tests added or extended in HwFundingSignerTest.kt and TrezorSessionFailureTest.kt: cover source coordination, timeouts, stale-session retry, retained signed transactions, and failure classification.
  • Unit tests extended in HwWalletRepoTest.kt: cover pending sent activity and contact preservation during watcher reconciliation.
  • Unit tests extended in ShopPaymentRequestTest.kt: cover hardware-wallet on-chain-only scan handling.
  • Hardware-wallet tests, lint, and formatting checks pass.

@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown

Greptile Summary

The PR integrates paired Trezor wallets into on-chain send and receive flows and strengthens session recovery and hardware-activity reconciliation.

  • Adds offline fee composition, device signing, safe broadcast retry, and hardware-funded send navigation.
  • Adds hardware receive-address display and on-device verification.
  • Extends wallet-scoped activity and contact handling for hardware transactions.
  • Two lifecycle gaps remain around multi-wallet receive selection and restart-safe activity reconciliation.

Confidence Score: 3/5

The PR should not merge until multi-wallet receive selection and restart-safe preservation of newly broadcast hardware activities are addressed.

Global Receive silently loses Trezor access for users with multiple paired identities, and process-local snapshot protection can delete a newly created hardware-send activity and its contact metadata after an app restart.

Files Needing Attention: app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveSheet.kt; app/src/main/java/to/bitkit/services/CoreService.kt

Important Files Changed

Filename Overview
app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveSheet.kt Wires hardware address loading and verification into Receive, but removes the hardware option from global Receive when multiple paired wallets require selection.
app/src/main/java/to/bitkit/services/CoreService.kt Preserves locally created sends during watcher lag only through process-local state, allowing restart-time deletion and contact loss.
app/src/main/java/to/bitkit/ui/screens/wallets/send/HwSendViewModel.kt Adds a guarded sign-and-broadcast state machine with signed-transaction reuse for connectivity retries and wallet-scoped result persistence.
app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt Adds offline receive derivation, device verification, fee estimation, maximum calculation, and more targeted stale-session cleanup.
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt Extends request validation, amount limits, fee preparation, source switching, contact preparation, and success handling for hardware-funded sends.
app/src/main/java/to/bitkit/repositories/ActivityRepo.kt Scopes hardware activity lookup and contact mutation to the selected external wallet identity.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Select paired Trezor] --> B[Enter on-chain request]
  B --> C[Estimate fee from stored xpub]
  C --> D[Review payment]
  D --> E[Reconnect matching wallet identity]
  E --> F[Sign on Trezor]
  F --> G[Broadcast signed transaction]
  G --> H[Create wallet-scoped activity]
  H --> I[Reconcile watcher snapshot]
  J[Open Trezor Receive] --> K[Derive unused address from xpub]
  K --> L[Display QR and address]
  L --> M[Reconnect matching identity]
  M --> N[Verify address on device]
Loading

Reviews (1): Last reviewed commit: "feat: add trezor send and receive" | Re-trigger Greptile

Comment thread app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveSheet.kt Outdated
Comment thread app/src/main/java/to/bitkit/services/CoreService.kt Outdated
@ovitrif ovitrif added this to the 2.5.0 milestone Aug 27, 2026
jvsena42

This comment was marked as resolved.

@ovitrif

This comment was marked as outdated.

@ben-kaufman ben-kaufman changed the title feat: add trezor send and receive feat: add trezor send Aug 27, 2026
@ben-kaufman
ben-kaufman requested a review from ovitrif August 27, 2026 15:28
@ben-kaufman ben-kaufman mentioned this pull request Aug 27, 2026
5 tasks
@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Done — #1187 is now Send-only and conflict-free. Receive is split into the stacked #1189. iOS is split the same way: Send in synonymdev/bitkit-ios#688 and Receive in synonymdev/bitkit-ios#693.

@ovitrif

This comment was marked as outdated.

@ben-kaufman

This comment was marked as resolved.

ovitrif

This comment was marked as resolved.

@ben-kaufman
ben-kaufman requested a review from ovitrif August 27, 2026 18:07
@ben-kaufman
ben-kaufman force-pushed the feat/trezor-send branch 4 times, most recently from 5ef1318 to 80d47e3 Compare August 27, 2026 20:10
ovitrif
ovitrif previously approved these changes Aug 27, 2026

@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.

Code LGTM, found some UI nits during testing:

1. Available (Savings) text

This was either updated in figma long ago or only in v62, but since we're here we could remove the (Savings) part as it's no longer there and a bit redundant (button already says and means the same thing, while when showing trezor funds, it disappears).

2. Funding source switch lag

when switching to/between HWWs there is an unexpected delay.

Could we add a loading spinner while switching? Button text could fade out while spinner fades in, or even without transition, the UX would feel more snappy, less clumsy?

Amount Confirm
hwDelayAmt.mp4
hwSendConfirmSwitch.mp4

I don't see this delay that much on confirm for some reason, but there's a small strangeness if tapping continuously multiple times on the button, just a FYI, don't think it's in scope of this PR to over optimize all small details.

QA Notes

Tested on a Pixel 9a emulator using regtest:

  • Sent from the standard Trezor through source selection, amount entry, duplicate-preparation protection, confirmation, device signing, broadcast, and success.
  • Verified the hardware-wallet transaction appeared once in recent activity.
  • Exercised source switching across Savings and multiple named Trezor identities; balances updated correctly, with the non-blocking delay noted above.

Approve.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Addressed both UI notes in 0cf15db. The amount screen now uses Available for Savings. Source switching shows a spinner, ignores repeated taps, cancels stale fee refreshes, and disables Continue and confirmation until the selected source is ready.

@piotr-iohk

Copy link
Copy Markdown
Collaborator

Testing...

piotr-iohk
piotr-iohk previously approved these changes Aug 28, 2026

@piotr-iohk piotr-iohk 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.

LGTM

Tested on emulator + Trezor emu, and on a Samsung S22 with a real Trezor 7 over BLE. Standard HW send, passphrase HW send, tags, and cancel-during-sign all worked. S22 logs look fine on that path.

Nit: Android never shows the hourglass on pending on-chain txs (HW or software) — still the send/transfer arrow. Figma uses HourglassSimple on the activity list (this and this). iOS already has the hourglass. Would be good to add it on Android in general, and use blue for pending HW sends so it matches the rest of the hardware styling.

@piotr-iohk

Copy link
Copy Markdown
Collaborator

Note: conflicts appeared after recen merges to master I guess.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ben-kaufman conflicts

piotr-iohk
piotr-iohk previously approved these changes Aug 28, 2026
@ovitrif

ovitrif commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

fyi I'm polishing a bit the UI/UX of the new components, there's a few issues with 0cf15db, non-blocking though IMO.

I'll either open a stacked PR targeting this or a new one if this gets merged.

@jvsena42

Copy link
Copy Markdown
Member

Starting review...

@ovitrif

ovitrif commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

This is probably an issue, on SendConfirmScreen, after the swiper disables and then re-enables, sometimes it loses its bg:

scrcpy.2026-08-28.000212.mp4

Note that the UI changes to the funding source button and the fee cell are because the recording is from my local WIP polish, but the swiper issue was there already and I didn't touch it.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

@ovitrif Fixed the swipe background issue in 9484f78. Funding-source switching now disables gestures and lowers opacity without changing the track background style, so re-enabling it keeps the background intact.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tAck

@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

I still see the issue I flagged earlier, that was supposed to be fixed after the last commit (#1187 (comment)).

Expected Actual

I'll fix it together with the other polishing changes.

@ovitrif

ovitrif commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

@ben-kaufman can you resign the commits:

Commits must have verified signatures.

I can't merge otherwise 🙏🏻

@ovitrif
ovitrif merged commit 5bf5ffe into master Aug 28, 2026
18 checks passed
@ovitrif
ovitrif deleted the feat/trezor-send branch August 28, 2026 21:41
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.

4 participants