device_trezor: add Trezor Host Protocol v2 support - #11043
Conversation
The Trezor Safe 7 does not speak the legacy v1 wire protocol at all. It
speaks the Trezor Host Protocol v2 (THP): a Noise_XX encrypted channel
brought up over a CPace CodeEntry pairing step, carried over the same
USB transport as before. Without a THP implementation on the host side
the Safe 7 is invisible to the wallet.
This adds that host side, in layers:
- framing: chunked, length-delimited frames with CRC32 and
alternating-bit sequencing, tolerant of duplicate frames and of
piggybacked acknowledgements
- noise: the Noise_XX handshake and the transport cipher
(Curve25519 / AES-GCM / SHA-256) with the THP HKDF cascade
- pairing: CPace CodeEntry, where the device displays a six-digit
code that the host has to echo back
- store: a small on-disk credential store, so a paired device is
remembered and the code is not asked for again on every open
- protocol_v2: the state machine driving channel allocation, the
handshake, and encrypted message exchange
- auto_detect: protocol probing, so v1 and THP devices can share a
transport and the right protocol is chosen per device
Detection cannot key on the USB descriptor. THP is a firmware build
flag rather than a model property, so a device reporting the Model T
VID/PID may speak either protocol. The probe instead short-circuits on
the v1 magic byte, which every v1 device answers immediately, and only
falls through to a THP channel allocation when that byte is absent, so
existing devices pay one write and one read for it. TREZOR_FORCE_V1
and TREZOR_FORCE_THP skip the probe either way for diagnostics.
connect() now walks the enumerated transports instead of taking the
first match, because a Safe 7 needs the direct-USB carrier that
enumeration lists after Bridge, and on setups where libusb can
enumerate but not open a device the working candidate is the Bridge
one. It moves on only for a carrier it could not acquire; anything the
device itself reported stops the walk, so a rejected pairing code is
not silently retried against the next carrier.
The device layer gains an i_device_callback hook for the pairing code,
mirroring the existing PIN and passphrase hooks, and classifies connect
failures into the exception types in trezor/exceptions.hpp so a front
end can tell a genuine bug from something the user can act on (device
locked, not paired, cancelled on the device).
THP builds under the existing WITH_DEVICE_TREZOR gate and adds no new
dependency: the field arithmetic behind the elligator2 map reuses the
ref10 routines already exported by crypto-ops.h, AES-GCM comes from the
OpenSSL that device_trezor already links, and the framing CRC comes
from Boost. The THP protobuf definitions are vendored from
trezor-common at a pinned revision, next to the message definitions
vendored there already, and fetch_protob.sh picks them up.
The unit tests cover every layer with no device dependency and run in
CI without hardware: framing across chunk boundaries, the HKDF cascade
and IV layout against the vectors published in trezor-firmware, the
Noise_XX handshake against an in-process device simulator, the CPace
primitives including the wrong-code fail-closed path, the credential
store against malformed and hostile input, and the error classifier.
Two small fixes to existing code ride along, because the new paths sit
on top of them: UdpTransport raises its open counter before
session_begin rather than after, so a throw there no longer leaks the
socket, and the Bridge enumerate probe gets a two-second timeout of its
own so a silently dropped connection to localhost no longer stalls a
wallet open for the full three minutes.
|
NACK to maintaining thousands of lines of AI slop just for THP support. |
This is about trezor safe 7 support, it's a dependency for my change on the gui repo. See my below message. |
|
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. |
What this does
This adds support for the Trezor Host Protocol v2 (THP) to
src/device_trezor, which is whatthe Trezor Safe 7 requires. Without it, Monero cannot talk to a Safe 7 at all.
Everything already supported keeps working unchanged. Devices that speak the existing v1 wire
protocol (Model T, Safe 3, Safe 5) continue to use it; nothing about their code path changes.
Why this is needed, and why the failure is confusing today
The Safe 7 enumerates over USB with the same VID/PID as the Model T (
0x1209:0x53C1) — everyModelClass.COREdevice shares it. So the device is detected, looks supported, and then failspart way through, which is a worse first experience than not being detected at all.
Detection has to be a protocol probe rather than a device-ID lookup, and the reason is not that
the descriptors are identical. They are not: a Safe 7 reports
iManufacturer"Trezor Company"and
iProduct"Trezor Safe 7" where a Model T reports "SatoshiLabs" and "TREZOR". The reason isthat THP is a firmware build flag, not a model property —
features.append('thp')incore/SConscript.firmware. Whether a given device in front of you speaks THP is a property of thefirmware it is running, so a table keyed on model would be wrong the first time Trezor ships THP
to another device or a build without it. Asking the device is the only answer that stays correct.
How it works
THP replaces v1's plaintext framing with an encrypted, authenticated channel:
Noise_XX_25519_AESGCM_SHA256handshake, giving mutual authentication and aforward-secure transport cipher.
CodeEntry: the device shows a six-digit code, the host asks the user forit, and both sides derive the same key only if the code matches. The device issues a long-lived
pairing credential on success.
pairing happens once per device, not once per connection.
Protocol selection is a probe on connect: if the device answers with the v1 magic bytes it is a v1
device and the v1 path is used immediately; otherwise the THP handshake proceeds.
Compatibility and behaviour changes for existing users
I want to be explicit about this rather than bury it, because some of it is visible to people who
are not buying a Safe 7:
answers with the v1 magic bytes
'?' '#' '#'— only a v1 device can produce those — so alegacy device is identified in about a millisecond rather than waiting out a THP timeout.
I have not timed that on hardware yet, so read it as reasoning about the code path rather
than a measurement. I will post a Model T connect time on this branch against one on upstream
master as soon as I have it. An earlier revision of this branch did cost legacy devices several
seconds per connect, which is exactly why I am not asserting the number.
connect()now walks the enumerated transportsinstead of taking the first match, because a Safe 7 needs the direct-USB carrier that
enumeration lists after Bridge. Enumeration order is deliberately unchanged — Bridge still
comes first, so Monero does not start taking the USB interface away from Trezor Suite for people
who have never touched a Safe 7.
credential per paired device. Location, format and permissions are documented in
src/device_trezor/trezor/thp/README.md. It is deliberately per user and per network: onWindows that means
CSIDL_APPDATArather than theCSIDL_COMMON_APPDATAthattools::get_default_data_dir()resolves to, since every local account on a machine shares thelatter and host private keys should not be shared between them. A testnet wallet likewise gets
its own store rather than reaching into mainnet's.
A known limitation I have not fixed
A first-time Safe 7 pairing can fail if a Trezor bridge is running, instead of falling back to
direct USB. I would rather state this than have someone find it.
trezord-gocannot carry THP and never will: it re-frames rather than pipes (wire/v1.gobuildsthe
'?' '#' '#'header itself and discards any packet without it), it contains no THP code, itslast release was 2023-04-19, and it has an open issue titled "Archive the repo". Trezor Suite has
moved to a separate bridge on a different port.
Because Bridge is tried first and a Safe 7 shares the Model T's VID/PID, a user with a bridge
running can have it acquire the device and then fail at
GetFeatures, rather than Monero movingon to WebUSB, which would have worked.
The clean fix is the one Trezor's own client uses: a THP-only device answers a v1 message with a
fixed
FailurecarryingFailure_InvalidProtocol, andpackages/connect/src/device/workflow/handshake.tskeys its fall-through on exactly that. I didnot implement it here, for three reasons I would rather put in front of you than work around:
the first v1 exchange happens well after
connect()has returned, so it is not reachable from thetransport-selection loop without adding a new probe exchange per candidate;
FailureException'scode member is private with no accessor; and
Failure_InvalidProtocoldoes not exist in thevendored proto —
FailureTypestops atFailure_InvalidSession = 14and jumps toFailure_FirmwareError = 99, so the pin predates it. Doing it properly means either a protoresync, which pulls in unrelated semantic changes to the other Trezor protos and wants its own
hardware validation, or hardcoding
17.I would rather take direction on which of those you prefer than pick one unilaterally inside a PR
this size. In the meantime the workaround is to quit the bridge, or plug in with Suite closed for
the first pairing only.
Why C++ from the specification rather than Trezor's Rust crate
I know this is the first question, so I want to answer it before anything else.
I read the record before starting. selsta on
monero-gui#4517(2026-07-19): "A rustimplementation might work depending on the amount of dependencies". jeffro256 in Tech Meeting
#167: "I would prefer less to spend a crap ton of time reinventing the wheel". rbrunner7 in the
same meeting: "Complicated == high potential for bugs". Those are all reasonable, and I am not
going to pretend the discussion did not happen or that it went my way.
The first thing to say is that there is no THP crate to depend on. Checked again the day I
opened this:
trezor-thpreturns "cratetrezor-thpdoes not exist" from the crates.io API. Itexists only as a path crate inside the
trezor-firmwaremonorepo atrust/trezor-thp, firstcommitted 2025-11-26, with protocol fixes still landing at the end of July 2026. mmilata offered on
#10368 in June to "publish it as a crate if it helps", and that has not happened.
Trezor do publish
trezor-clientfrom the samerust/directory — 0.1.6, updated 2026-07-14 —so I want to be explicit that I have not overlooked it. It is the v1 client: its dependencies
are
protobuf,byteorder,rusb,hex,thiserrorandtracing, it does not depend ontrezor-thp, and it implements none of this. It cannot talk to a Safe 7 either.So the Rust route is not "add a dependency and be done". It is: vendor an unpublished,
in-development subdirectory of a monorepo; write the C ABI that upstream does not currently
expose; and then write substantially the same integration layer you see here against it — against
an interface nobody has committed to keeping stable.
rust/trezor-thp/Cargo.tomlalso declaresedition = "2024", so adopting it sets a Rust MSRV of 1.85 for the whole tree.On the dependency count, since it came up: jpk68's "11, to be exact" is right about the crate
itself, and I will concede it — 3 direct (
log,heapless,trezor-noise-protocol), 12transitive. But that is the crate with its crypto in
[dev-dependencies]. A host that canactually complete a handshake needs
trezor-noise-rust-cryptoandgetrandomon top, whichresolves to 42 crates, and then protobuf and a transport on top of that. That is the number
worth comparing against.
And one thing I would genuinely like an answer to rather than assert:
rust/trezor-thpshipsno licence file and no
licensefield in itsCargo.toml, while its siblingtrezor-clientin the same directory sets
license = "CC0-1.0"explicitly.trezor-firmware'sLICENSE.mdsays"all other files — GPLv3", which would be the default reading, but I do not think a missing field
next to a deliberately-set one is something to guess at. If the licence is GPL-3.0, adopting it
into a BSD-3 project is a question that needs settling before any of the engineering matters. I may
be reading it wrong, and would rather be corrected than build on a bad assumption.
On Rust in the tree — I am not going to argue the toolchain is a problem, because it is not.
.github/workflows/depends.ymlhas installed rustup sinceef509e1c0(2025-02-06) and every oneof the ten cross targets carries a
rust_host.src/fcmp_pp/fcmp_pp_rust/is reserved and wiredin with
add_subdirectory,fcmp++.his a C-ABI FFI header, and the top-levelCMakeLists.txt:72carries a commented-outRUSTC_WRAPPERwaiting on FCMP++. Guix gained Rust in#9801 on 2026-07-31. The direction of travel is not in doubt and the toolchain is ready.
The only accurate remaining statement is narrower: nothing in the tree is compiled with Rust
yet —
fcmp_pp_rust/CMakeLists.txtis a licence header with no commands, and there is noCargo.tomlanywhere. That is a fact about timing, not an argument, and I am not going to lean onit.
So my case does not rest on Rust being impractical. It rests on three things:
One — it exists, it is tested, and it works today. That is the whole of the first argument. A
Safe 7 currently does not work with Monero at all, and this is the only implementation anyone has
written. Nobody else has started.
Two — it adds no dependencies. Not "few": zero new third-party dependencies. It uses
libsodium and protobuf, both of which
src/device_trezoralready links, plus Boost and OpenSSL,which the tree already depends on. The only build change is adding OpenSSL to
device_trezor'sprivate link line. There are no new platform imports either — the credential store goes through
the same
epee::file_io_utilshelpers andtools::replace_filethe wallet already uses, on everyplatform.
To be precise about what I have actually built rather than what I expect: I have compiled and run
the tests on x86_64 Linux only.
USE_DEVICE_TREZOR_MANDATORY: ONis set at the top of bothbuild.ymlanddepends.yml, so a compile failure on any of the tendependstargets turns therun red rather than silently dropping Trezor support — but that is CI's word to give, not mine,
and I would treat the first CI run as the real answer.
Three — it is cheap to reverse. See the section at the end. This is the argument I would
weigh most heavily if I were reviewing it.
Reviewability, today. This is reviewable with the toolchain you already have: no new
compiler to install to check a change, no FFI boundary between the audited code and the wallet, no
open question about who reviews the shim. That is a statement about today, not a claim that
Rust is unreviewable — when
fcmp_pp_rustis populated and depends carries a toolchain, thatadvantage narrows.
Guix reproducibility. A pure
src/change does not touch the Guix inputs at all.Answering the "should anyone hand-write this?" objection directly
rbrunner7's point in meeting #167 was not about language. "Complicated == high potential for
bugs" cuts against a hand-written implementation in Rust exactly as much as in C++, and swapping
languages does not answer it. The only thing that answers it is evidence, so here is the evidence
rather than an argument:
be exact about the provenance of each, because "tested against vectors" can mean very little:
(commit-pinned in the test file) and by the relevant specifications. The CRC32 is
bit-identical to the device's own.
definition using an independent Python implementation, which I first validated against the two
Trezor-published elligator2 vectors before using it to generate anything. They are not
upstream-published values, because upstream does not publish any: the Python THP test suite
that carried them was deleted when Trezor moved to their Rust implementation, and the Rust
replacement ships no vector tables. I would welcome a cross-check against firmware.
specification — control byte, continuation header, and the 59/61 split — not against a second
call to the encoder. It is not pinned against a captured firmware fixture; the upstream fixture
file is deleted at HEAD, and I would upgrade this test if someone points me at a live copy.
consume_secret's two rejection paths, which are the whole host-side authentication mechanism.asserting the exact control-byte sequence on the wire.
If any of that is thinner than it should be, say which part and I will add to it. I would rather
argue about coverage than about language.
If you would rather have Rust
The case for the crate is real: it would be written by the people who wrote the protocol, and it
would track firmware changes without me in the loop.
Two things worth knowing before that trade looks free.
First, what it costs today: there is no published THP crate to adopt, so the Rust path starts
with vendoring an in-development monorepo subdirectory, settling the licence question above,
writing a C ABI that upstream does not currently expose, and accepting a tree-wide Rust MSRV of
1.85. That is months of work, not a swap. If the project decides to go that way I will help where
I can, but I would not want the decision to rest on an assumption that I can carry it alone —
see the last section.
Second, and more usefully: this change is cheap to reverse. The protocol implementation lives
entirely under
src/device_trezor/trezor/thp/. It adds no dependency, touches no consensus code,and nothing outside
device_trezorcalls into it. If the Rust route arrives later, deletingthis is one commit and re-pointing the integration layer is a small one. Taking it now does not
foreclose taking the crate when it exists — it just means Safe 7 owners are not waiting in the
meantime.
I wrote it in C++ because, weighing the above, I judged it the better fit for the codebase as it
stands today. If you disagree, I will follow the project's lead. What I would ask is that the
call be made rather than deferred, because the status quo is that a Safe 7 does not work at all.
The one genuinely new cryptographic primitive
CPace requires a map-to-curve. I implement elligator2 over Curve25519. This is the one new
primitive in the PR and it deserves the most review attention, so here is exactly why it is
hand-written rather than called.
Why no library provides it. CPace draft-10 section 7.2 mandates RFC 9380's
map_to_curve_elligator2with thevcoordinate discarded, producing a raw Montgomeryuwithno cofactor clearing. libsodium cannot supply that:
crypto_core_ed25519_from_uniformclears the cofactor and outputs Edwards, so it yieldsu(8P)—interoperability-fatal against the device.
crypto_core_ristretto255_from_hashis a differentencoding entirely. There is no X25519-specific map. The primitive that would work,
ge25519_elligator2, lives in libsodium'sprivate/ed25519_ref10.hand is not public API.Trezor hand-roll it on both sides for the same reason —
crypto/elligator2.c(MIT, followingRFC 9380) and
trezorlib/thp/curve25519.py.What the field arithmetic runs on, and why it changed. The map is built on Monero's in-tree
ref10 field arithmetic (
src/crypto/crypto-ops.h), the same primitivessrc/fcmp_pp/fcmp_pp_crypto.cppalready uses from outsidesrc/crypto/. Nothing undersrc/crypto/is modified.I originally wrote this over OpenSSL BIGNUM and that was a mistake, so I want to record what was
wrong with it rather than quietly ship the replacement.
BN_FLG_CONSTTIMEonly routesBN_mod_expandBN_mod_inverseto constant-time variants; it does nothing for the surroundingcomparisons and assignments. My
is_squarebranched on the Legendre result and the callerbranched again to select
x1orx2, leaking one bit of the pairing code per run.That bit is worse than it sounds, which is why I did not leave it documented as an accepted
trade-off. The leaked predicate is
is_square(gx1(code, h)), andhis public — it comes offthe handshake transcript. So the bit is checkable offline: an attacker recomputes it for all
10^6 candidate codes and halves the space, and k observed pairing attempts leave 10^6/2^k. That
defeats CPace's central guarantee of exactly one online guess per run, against a secret with
only about 20 bits in it to begin with.
The map is now branch-free: a fixed addition chain for the Legendre symbol,
fe_cmovwith anarithmetic mask for the
x1/x2selection, a fixed-limb input decode, andfe_invertas RFC9380's
inv0— which also restored section 6.7.1 step 2, a CMOV the OpenSSL version had replacedwith a throw. This matches what Trezor do on both sides (
crypto/elligator2.c,trezorlib/thp/curve25519.py) and what RFC 9380 section 4 requires.<openssl/bn.h>is gone from this file entirely as a result.Why the Noise cipher uses OpenSSL EVP rather than libsodium. libsodium's
crypto_aead_aes256gcm_*requires Intel AES-NI or the ARM crypto extensions, and its owndocumentation says there are "no plans to support non hardware-accelerated implementations" —
there is no software fallback in any version. More concretely,
contrib/depends/packages/sodium.mkpins 1.0.18, whosecrypto_aead/aes256gcm/directorycontains only
aesni/and noarmcrypto/. So on riscv64, aarch64-linux, arm64-darwin,armv7-android and aarch64-android,
crypto_aead_aes256gcm_is_available()returns 0unconditionally and every entry point fails with
ENOSYS; on the five x86 targets it is a runtimeCPUID check that can still fail. OpenSSL EVP works on all of them.
Every step is checked against published test vectors — see below.
Vendored protobuf definitions
messages-thp.protois vendored through the existingfetch_protob.sh, from the existingupstream —
trezor-common. No second script and no second upstream: everything arrives throughgit, which is what #9491 asked for.
There is one wrinkle worth stating plainly:
messages-thp.protodoes not exist at the pinnedtrezor-commonrevision, and bumping that pin is not a free move — the five existing protoschange substantially between the current pin and master, including a semantic
optional->requiredonMoneroAddress.addressand a newchunkifyfield inmessages-monero.proto. Noneof that is THP's business and all of it would want its own hardware validation.
So the script keeps one upstream and two pins in a single clone, reading the THP proto out of
the object database:
Both pins are therefore git-hash-verified, which also removes the integrity asymmetry of fetching
one file over
curl(and with it the missing--fail, rather than patching it).The vendored copy differs from upstream by exactly two edits, both mechanical, both applied by the
script and both documented in
protob/README.md:import "options.proto";becomesimport "messages.proto";. This tree does not vendoroptions.proto, and every custom option the file actually uses —wire_in,wire_out,bitcoin_only,include_in_bitcoin_only— is already declared inmessages.protoat the pin.Vendoring
options.protoas well is not an alternative: it re-declares the same extensionnumbers and protoc rejects the duplicates.
option (wire_enum) = true;/option (internal_only) = true;are deleted. Bothextensions exist only in
options.proto, so protoc cannot resolve them here. They are metadatafor trezor-firmware's own code generator and change neither the wire format nor the generated
C++.
protob/README.mdcarries a one-liner that reproduces the vendored file from upstream, so the diffyou are reading is checkable against Trezor's object database without taking my word for any of it.
I ran the whole script end to end on a clean tree to be sure. It exits 0, and
git statusafterwards shows one modified file:
That is not this PR's doing. The script fetches upstream verbatim, and the committed copy carries a
one-word comment typo fix made in this tree by
2c327aa32("fix spelling in comments and docs") —so re-running the vendoring script reverts it.
messages-thp.protoand everything else reproducebyte-for-byte. I have left the pre-existing wart alone rather than widen this PR to fix monero's
tooling, but it is recorded in
protob/README.mdnow so the next person to run the script is notalarmed by it.
One cosmetic note, in case a linter flags it: the vendored proto contains two non-ASCII characters,
both
≤inside upstream's own comments onhost_nameandapp_name. They are upstream's and Ihave deliberately not "fixed" them, because doing so would break exactly the byte-for-byte
reproducibility above.
Licence and provenance are recorded in
protob/README.md.protob/COPYINGis byte-identicalto
trezor-common's rootCOPYINGat both pins, and is LGPL-3.0 (LGPL-3.0-only; upstreamships no per-file headers and makes no "or any later version" election).
trezor-commonis aread-only export of
trezor-firmware'scommon/directory, governed bycommon/COPYING— thesame LGPL-3.0 text — and not by
trezor-firmware's repository-rootCOPYING, which is GPL-3.0and covers the firmware itself. I checked all four of those files rather than assuming.
sha256sums, so this can be verified without cloning anything:
Testing
Automated.
tests/unit_tests/device_trezor_thp.cppandtests/unit_tests/trezor_exceptions.cppadd 82 tests across 12 suites, all of which run with no hardware attached and finish in about
130 ms. They cover framing across chunk boundaries, CRC32, the HKDF cascade and IV layout, the
Noise_XX handshake against an in-process device simulator, the transport cipher including nonce
exhaustion, CPace and elligator2, the credential store against malformed and hostile input, and the
error classifier — with the fail-closed branches tested for failing, not only for succeeding.
Built and run on x86_64 Linux, Release,
USE_DEVICE_TREZOR_MANDATORY=ON, GCC 16:Sanitizers. Rebuilt with the project's own
-DSANITIZE=ON, which is exactly-fsanitize=address,undefined, and ran the same 82 tests with leak detection anddetect_stack_use_after_returnon:AddressSanitizer and LeakSanitizer report nothing at all. UBSan emits 41 diagnostics, and I
want to be exact about them rather than claim a clean sheet: every one is
left shift of negative valueinsidesrc/crypto/crypto-ops.c, and not one is in any file this PR adds. That ispre-existing in monero's ref10 arithmetic — running the untouched
crypto*andringct*suitesfrom the same binary produces 255 of the same diagnostics. It shows up here only because the
elligator2 map reuses ref10 instead of rolling its own field arithmetic, which I would rather have
than the alternative.
Hardware: not tested yet, and I would rather say so than let the automated results imply it.
That is the main reason this is a draft. I will post, as a comment on this PR: the exact device and
firmware version, first pairing, reconnect with a stored credential (which should not re-prompt),
a wrong code, cancelling at each step, a signed transaction that broadcasts, and a legacy
Model T / Safe 3 / Safe 5 still connecting and signing with its connect timing.
Everything claimed above this line is what the code and the automated tests support. Nothing above
it rests on a device.
Reviewing this without a Safe 7
Most of the change can be reviewed and exercised without the hardware, since the tests are
self-contained:
cmake -DBUILD_TESTS=ON -DUSE_DEVICE_TREZOR=ON .. make -j unit_tests ./tests/unit_tests/unit_tests --gtest_filter='thp_*:Trezor*'For building the GUI against this branch, use
MANUAL_SUBMODULES=1and do not usemake devmode— withDEV_MODE=ONCMake force-checks-outorigin/masterin the submodule andwill destroy your checkout.
Commit structure
One commit. The layers are separated by file rather than by commit —
thp/framing,thp/noise,thp/pairing,thp/store,thp/protocol_v2,thp/auto_detect— and each has its own section intests/unit_tests/device_trezor_thp.cpp, so the diff can still be read a layer at a time.Two changes in it are not THP and I want them visible rather than buried, because they affect
existing users:
UdpTransport::open()raises its open counter beforesession_beginrather than after. Athrow from
session_begincurrently leaves the counter at zero,pre_close()returns false, andthe socket is never released. That is a pre-existing bug; I only noticed it because the THP
handshake gave
session_begina reason to throw./enumerateprobe gets a two-second timeout of its own, instead of inheriting the180-second default. Without a bridge installed and with something quietly dropping packets to
localhost:21325, a wallet open currently stalls for three minutes. Real Bridge operations keepthe long timeout.
If you would rather have either of those as its own PR, say so and I will pull it out.
Companion GUI change
The GUI side is
monero-project/monero-gui#4674, opened as a draft. It deliberately does notbump the
monerosubmodule: monero-gui pins therelease-v0.18line, and no commit since 2020 hasmoved that pointer for anything other than release prep. The three
DEV_MODE=ONGUI CI jobs buildagainst monero master regardless of the pin, so they go green on their own once this merges.
Nothing needs to happen to that submodule for this PR.
Open questions I would like maintainers to answer
is a preferred convention for per-user secret state that I have missed, say so and I will move
it.
How I would like to work on this
Safe 7 support has been outstanding for a while and, as far as I can tell, nobody had started. I
wrote this so that most of the work would already be done whenever the project got to it. I am not
asking for anything in return and I have no stake in it beyond wanting the device to work.
So, plainly:
Take it and change it. If you want large-scale changes — a different structure, the Rust route,
a different home for the credential store, different naming — you do not need me in the loop to
make them. Fork the branch, rewrite whatever you like, land it under whatever authorship makes
sense. I would much rather Safe 7 support existed than that this particular diff did.
I will work with you on feedback. I will answer review comments here, keep the branch rebased,
and make the changes I am able to make. What I cannot do is an indefinite series of large reworks
that each need hardware I do not have: I have one Safe 7 and one Linux machine. No macOS or Windows
build host, no bitcoin-only firmware, no THP-capable emulator, no legacy device beyond what is
listed in the testing section. Anything gated on those is genuinely better done by someone who has
them than by me guessing.
I will not be at meetings. Everything I have to say is in this PR and I will answer anything
asked here. If a decision gets made in a meeting, point me at it and I will act on it.
And if the project decides it does not want this at all, that is a fine outcome too — the branch
stays public and anyone in the community is welcome to take it.