Skip to content

wizard: add Trezor Safe 7 support - #4674

Draft
TyLauriente wants to merge 1 commit into
monero-project:masterfrom
TyLauriente:trezor-safe7
Draft

wizard: add Trezor Safe 7 support#4674
TyLauriente wants to merge 1 commit into
monero-project:masterfrom
TyLauriente:trezor-safe7

Conversation

@TyLauriente

Copy link
Copy Markdown

What this does

This is the GUI half of Trezor Safe 7 support. The protocol work is in
monero-project/monero#11043; this PR adds the user-facing part of it.

The Safe 7 requires the Trezor Host Protocol v2, which pairs by showing a six-digit code on the
device screen
that the user types into the host. There is nothing in the GUI today that can ask
for that, so this adds it.

Draft, and what it is waiting on

Marked draft deliberately. It should not merge before the core PR does, because the pairing
callback it binds to does not exist until then.

This PR does not bump the monero submodule, and that is intentional. monero-gui pins the
release-v0.18 line, and the pointer is release-prep territory rather than something a feature PR
touches: of the 51 commits that have ever moved it, every one since 9c76f099 back to
repo: prepare v0.17.1.1 in October 2020 is a release-prep commit. The last bump that was not is
monero: update submodule to master branch HEAD, 2020-08-28. Moving it here would also mean a
~2178-commit jump across a branch divergence, which is not something to smuggle into a feature
change.

So expect CI on this PR to be red, and expect it to stay red for a while. I would rather say
that up front than have it look like a broken branch:

  • build-macos, build-ubuntu and build-windows run DEV_MODE=ON make release, which builds
    against monero origin/master. They are red until the core PR merges to master, and they go
    green on their own at that point, with no submodule bump at all
    .
  • macos-bundle, docker-linux-static, docker-windows-static and docker-android build against
    the pinned pointer, so they stay red until THP reaches the pinned line. That is a release-cycle
    question rather than something this PR should try to answer.

I would rather raise that timing question with you than guess at it. If the preference is for this
to wait for a release-v0.18 backport, or to ride a normal bump cycle, that is fine by me — I just
need to know which, so I can keep the branch in the right shape.

What the user sees

  • Pairing code entry. On first connect of a new Safe 7, the device shows a six-digit code and
    the GUI asks for it. On success the device issues a credential that is stored, so this happens
    once per device rather than once per connection.
  • Wrong code re-prompts on the pairing dialog with a plain-language message, rather than
    dead-ending.
  • Cancel returns cleanly, and is not presented as an error, because the user chose it.
  • Error classification. Hardware-device failures are routed to messages that distinguish
    "try again" from "this will not work" — for example a device that is not connected versus firmware
    that does not support Monero.

Existing hardware wallets are unaffected. The pairing dialog only appears for a device that asks
for pairing.

One thing I would rather ask about than assume: images/trezor7.png sits alongside the existing
trezor3.png, trezor5.png and trezorT.png and is used the same way. If the project has a
provenance or licensing requirement for device artwork that I have not met, say so and I will
replace it, or drop the image and let the Safe 7 fall back to the generic entry.

How the dialog talks to the device thread

It follows the existing PassphraseHelper pattern rather than inventing a new mechanism: the
device thread blocks on a condition variable while the QML dialog collects input on the UI thread,
with no BlockingQueuedConnection and no lock held across the UI round trip.

Not part of THP

It is one commit, so I want to point at the hunks in it that are not Safe 7 work rather than let
them hide:

  • CMakeLists.txt sets CMAKE_CXX_STANDARD 17 unconditionally, where today it is 17 only for
    DEV_MODE checkouts and 14 otherwise. The monero revision that carries THP requires C++17. This
    hunk is independent of the rest and could equally well ride along with a future submodule bump.
  • The hardware-signing splash subtitle, which applies to every hardware wallet including
    Ledger.
  • The status-toast width and wrapping rework, which changes layout for every status message in
    the app.

Say the word on any of the three and I will pull it out entirely.

Building and reviewing this

This needs the core branch checked out in the submodule, since the pairing callback it binds to
does not exist on master yet:

git remote add ty https://github.com/TyLauriente/monero-gui.git   # if you have not already
cd monero && git remote add ty https://github.com/TyLauriente/monero.git
git fetch ty && git checkout ty/trezor-thp && cd ..
make MANUAL_SUBMODULES=1

Do not use make devmode — with DEV_MODE=ON, CMake force-checks-out origin/master in the
submodule and will destroy your checkout. Without MANUAL_SUBMODULES=1 and with DEV_MODE=OFF,
CMake hard-errors on the submodule mismatch instead.

Testing

Automated. The existing QML suite still passes with these changes, which matters here because
this branch touches WizardController.qml and WizardCreateDevice1.qml, and tst_Wizard.qml
covers exactly that path. Run the same way CI does — xvfb-run -a build/bin/monero-wallet-gui --test-qml — on Qt 5.15.17, Release, built against the core branch:

Totals: 8 passed, 0 failed, 0 skipped, 0 blacklisted, 5347ms

I have not added QML tests for the pairing dialog itself. It needs a device callback to drive it,
and faking one convincingly enough to be worth the harness is more machinery than the test would
justify. If you would rather have it, I will add it.

Hardware: not tested yet, which along with the core dependency is why this is a draft. I will
post as a comment: device and firmware version, first pairing, reconnect with a stored credential,
a wrong code, cancelling at each step, a signed transaction, and a legacy Model T / Safe 3 / Safe 5
still working. The pairing dialog in particular has failure paths that only a real device
exercises, so I do not want the QML result above read as more than it is.

Qt versions. Built and tested against Qt 5.15.17 only. The README states a 5.12 floor and
CI uses 5.15, so a 5.15-only QML construct is exactly the kind of thing neither of us would catch,
and I do not want to imply coverage I do not have.

What I can say is narrower and checked rather than assumed: DevicePairingCodeDialog.qml declares
no Connections block at all, so it cannot be relying on the 5.15 function onSignalName()
handler form, and it imports nothing above the versions the rest of the file tree already imports.
If someone with a 5.12 environment can run it, I would rather have that than my reasoning. Worth
knowing either way: components/DatePicker.qml already uses a 5.15-only Connections form on
master, so a 5.12 failure there is pre-existing and not from this branch.

How I would like to work on this

The same as on the core PR, and for the same reason: this is a free contribution, Safe 7 support
had not been started by anyone, and I would rather it existed than that this particular diff did.

Take it and change it — you do not need me in the loop to restructure or rewrite any of it. I will
answer review comments here and keep the branch rebased. What I cannot do is an open-ended series
of large reworks that need hardware or platforms I do not have; I have one Safe 7 and one Linux
machine, no macOS or Windows build host, and no way to test the Qt 5.12 floor. I will not be at
meetings — everything I have to say is here, and I will act on any decision made elsewhere if
someone points me at it.

The Trezor Safe 7 speaks the Trezor Host Protocol v2 rather than the
legacy v1 wire protocol. With THP implemented in the wallet, the GUI
needs three things to drive it.

A pairing dialog for the THP CodeEntry step: on the first pairing the
device displays a six-digit code and the host has to echo it back. The
dialog collects it, accepts the spacing and dashes a paste brings with
it, and reports a rejected code in place so the user can try again
instead of dead-ending the wizard.

The Safe 7 as a selectable model in the create-wallet-from-device
wizard, with its own image.

Classification of Trezor connect and open failures, so the loading
screen can offer a retry hint when the device is merely off, locked or
not yet paired, rather than the "check application logs" error that
every failure produces today. Cancelling, on the device or in the
dialog, now ends the wizard quietly instead of as an error, which is
what the second argument to walletCreatedFromDevice carries.

C++17 is set unconditionally rather than only for DEV_MODE checkouts,
because the monero revision carrying THP requires it. That hunk stands
on its own and can be dropped if it would rather go in with the
submodule bump.

No submodule bump is included here. The matching monero change is a
separate pull request, and moving the pointer before that merges would
pin the GUI to a personal fork.
@TyLauriente

Copy link
Copy Markdown
Author

Heads up for anyone reading this in advance, I've been chipping away at implementing Trezor Safe 7 support for the last couple months and I think I have an implementation that is pretty solid, or as solid as someone who is not familiar with Monero development could be.

Massive heads up that this PR and all code in it was developed via Claude Code and although I've spent a lot of time iterating and researching to do my best to ensure compliance with the Monero development practices and policies, as well as the Trezor practices and policies, I do not have a complete and in depth understanding of all the changes, so I want to make sure to call this out for the sake of transparency. This message is from myself and not generated, and for any feedback or review, you'll be speaking with me directly.

I have tested this extensively on Linux so far with a couple of my personal devices, but I haven't tested on any other platforms yet. I'm planning to test on at least windows, and potentially some other platforms such as Android, but I won't have access to an Apple developer account for testing on something like MacOS.

I'll be doing a bit more testing on linux and doing some windows testing as well before moving this out of draft, which I plan to do later this week.

I'm happy to work to address feedback that comes up but some of those requests may require resources or time that I don't have. So in that case, these PR's will serve as a potential base for more common Monero contributors to work off of, or as a potential unblocker for some people in the community who want Trezor Safe 7 support now, before the core team finishes their implementation.

@selsta

selsta commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

I think a PR that integrates Trezor's Rust implementation would have been preferable. Thousands of lines of AI written code implementing Trezor's protocol on C++ is not maintainable unfortunately.

@TyLauriente

TyLauriente commented Aug 4, 2026

Copy link
Copy Markdown
Author

I think a PR that integrates Trezor's Rust implementation would have been preferable. Thousands of lines of AI written code implementing Trezor's protocol on C++ is not maintainable unfortunately.

I can change the implementation for both PR's to Rust. I'm leaving for my own work at the moment. I'll get back to this either tomorrow or some other time this week.

Please keep in mind, these are draft PR's for a reason. I was never planning to submit this work directly, as my comment mentions. Typically when I submit draft PR's I don't move them out of draft after they're ready, I typically close and re-open, to clean things up for full review. So this was always going to be closed in the end.

The largest reason that I opened up these draft PRs now rather than after I finished polishing everything is because I want to unblock people who want a solution and support for their devices now since it's been quite some time since the device came out and there's lots of talk in the community about wanting this. These PRs will be visible to others searching for info about support. And they can use it now if they'd like.

@jpk68

jpk68 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Actually, would it be possible to use libsodium instead of the Rust crate? IMO, this would fit much better into Monero's codebase.

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.

3 participants