Skip to content

feat: trezor hardware support#792

Open
coreyphillips wants to merge 68 commits intomasterfrom
feat/trezor-hardware-support
Open

feat: trezor hardware support#792
coreyphillips wants to merge 68 commits intomasterfrom
feat/trezor-hardware-support

Conversation

@coreyphillips
Copy link
Copy Markdown

@coreyphillips coreyphillips commented Feb 18, 2026

Description

The intention of this Trezor dev dashboard is to serve as a place for developers to test and troubleshoot Trezor hardware features as they emerge. This is not meant to be user facing, but to serve as a reference for how to use and interact with Trezor hardware devices once work on user-facing Trezor features begin by native devs.

  • Implements Trezor hardware support via USB and bluetooth
  • Implements a Trezor dev dashboard that is only be accessible in dev mode at the following location:
    • Settings->Advanced->Trezor

Preview

QA Notes

  • The latest bindings containing Trezor hardware support can be found here.
  • If you require an apk please let me know and I can build and send one for testing.

- Implement trezor hardware support via USB and Bluetooth
Copy link
Copy Markdown

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@coreyphillips coreyphillips self-assigned this Feb 18, 2026
@coreyphillips coreyphillips added the enhancement New feature or request label Feb 18, 2026
@coreyphillips coreyphillips marked this pull request as draft March 3, 2026 18:34
coreyphillips and others added 3 commits March 5, 2026 12:51
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Bumps bitkit-core version to 0.1.44
- Adds SendTransactionSection.kt
@coreyphillips coreyphillips marked this pull request as ready for review March 7, 2026 18:48
@jvsena42 jvsena42 added this to the 2.2.0 milestone Mar 9, 2026
@jvsena42 jvsena42 self-requested a review March 9, 2026 12:59
jvsena42 added 2 commits March 9, 2026 10:01
# Conflicts:
#	app/src/main/java/to/bitkit/ui/ContentView.kt
#	app/src/main/java/to/bitkit/ui/settings/AdvancedSettingsScreen.kt
#	app/src/main/java/to/bitkit/ui/settings/AdvancedSettingsViewModel.kt
jvsena42

This comment was marked as resolved.

@coreyphillips coreyphillips requested a review from jvsena42 March 17, 2026 12:33
Copy link
Copy Markdown
Member

@jvsena42 jvsena42 left a comment

Choose a reason for hiding this comment

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

utAck

Copy link
Copy Markdown
Collaborator

@ovitrif ovitrif left a comment

Choose a reason for hiding this comment

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

Review Pass 1 - Code

LGTM, nice code overall!

Compiled a list of nits which I will address in a stacked PR, nothing to worry about:


Critical Issues (3 found)

# Agent Issue Location
1 code-reviewer error() throws IllegalStateException instead of AppError TrezorRepo.kt:370,380,420,515,521
2 code-reviewer List used instead of ImmutableList in TrezorState (flows to composables) TrezorRepo.kt:582-583
3 error-hunter Missing .onFailure in autoReconnect — failure is completely silent, no toast, no log TrezorViewModel.kt (autoReconnect call)

Important Issues (10 found)

# Agent Issue Location
4 code-reviewer Missing CHANGELOG entry for feat: PR CHANGELOG.md
5 code-reviewer Logger in serializer missing context = TAG and manually appends Throwable TrezorDataSerializer.kt:18
6 code-reviewer TrezorStore suspend functions not wrapped in withContext(ioDispatcher) TrezorStore.kt:20-27
7 error-hunter Bare runCatching in disconnect() / forgetDevice() — inner errors silently swallowed TrezorRepo.kt:302,443-449
8 error-hunter saveThpCredential verification detects failure but only logs, doesn't propagate TrezorTransport.kt (credential save)
9 error-hunter composeTx — invalid feeRate guard returns without resetting isComposing, creating stuck UI TrezorViewModel.kt:408
10 error-hunter closeBleDevice always returns success = true even when close fails TrezorTransport.kt
11 type-analyzer KnownDevice.transportType is stringly-typed ("bluetooth"/"usb") — should be an enum TrezorRepo.kt:592
12 type-analyzer connectedDevice and connectedDeviceId are a split nullable pair — should be a single composite TrezorRepo.kt:584-585
13 type-analyzer TrezorDebugLog.lines uses List<String> flowing directly to composables — needs ImmutableList TrezorDebugLog.kt:13

Suggestions (9 found)

# Agent Suggestion Location
14 comment-analyzer ~14 comments on private functions/fields should be removed per CLAUDE.md TrezorTransport.kt (various)
15 comment-analyzer callMessage comment contains temporal "now" — will rot TrezorTransport.kt:275
16 comment-analyzer Hardcoded "60 seconds" in comment will diverge from constant TrezorTransport.kt:514
17 type-analyzer SendStep enum should be a sealed interface with associated data (eliminates impossible states) TrezorViewModel.kt:597
18 type-analyzer TrezorUiState is a 27-field god object — consider splitting into sub-states TrezorViewModel.kt:567
19 type-analyzer Derivation path computed via string interpolation in 3 places — extract helper TrezorViewModel.kt
20 type-analyzer KnownDevice missing @Immutable annotation TrezorRepo.kt:592
21 error-hunter onConnectionStateChange ignores GATT error status parameter TrezorTransport.kt:1003-1025
22 error-hunter getPairingCode() returns empty string for timeout, cancellation, and interruption — ambiguous TrezorTransport.kt:284-321

Test Coverage Gaps (8 critical gaps)

Priority Gap Criticality
1 connectWithThpRetry retry logic untested 9/10
2 autoReconnect all branches untested 9/10
3 connectKnownDevice untested 8/10
4 forgetDevice (security-relevant cleanup) untested 8/10
5 ensureConnected reconnection path untested 8/10
6 composeTx / signComposedTx validation untested 7/10
7 scan known-device filtering untested 7/10
8 connect persistence verification missing 7/10

Strengths

  • Architecture follows established patterns well (repo, service, MVVM, DI)
  • Proper use of Result API in the repo layer
  • Screen composables correctly split into parent + Content()
  • Good test infrastructure following project conventions
  • TrezorTransport has thorough USB error handling with specific messages
  • Hardware-specific "why" comments in TrezorTransport (BLE timing, GATT write type, etc.) are valuable
  • UI section files have zero comments — appropriate and clean
  • TrezorDebugLog has good bounded-size invariant enforcement
  • Preview data is well-organized with realistic fixtures

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants