Skip to content

[draft] Switch THP implementation to rust/trezor-thp#6676

Draft
mmilata wants to merge 10 commits intomainfrom
mmilata/thp-riir-trezor
Draft

[draft] Switch THP implementation to rust/trezor-thp#6676
mmilata wants to merge 10 commits intomainfrom
mmilata/thp-riir-trezor

Conversation

@mmilata
Copy link
Copy Markdown
Member

@mmilata mmilata commented Mar 28, 2026

Lots of major parts still missing/broken and plenty of smaller TODOs, hopefully the rough shape stays the same. See also #6442.

  • Does not yet work well with session restarts as the read loop reads packets too eagerly and ends up reading the next message just before loop restart and it gets lost. Probably solvable by adding another mailbox but I want to wait before preemption works.
  • Tentative trezor-crypto integration. Since it should now be possible to use unpatched noise-rust for clients I'd reevaluate implementing the device side of Noise directly in trezor-crypto. If we decide to go with noise-rust then at least zeroization is missing here.
  • Channel preemption.
  • ACK piggybacking.
  • Delete sessions whenever their channel gets deleted.
  • Turn async functions into generators where possible.
  • Interface write timeouts.
  • Make the code readable.
  • Split off rust/trezor-thp changes into separate PR.

Overview:

  • On the rust side channels are kept in global arrays which are unaffected by workflow restarts. Send/receive buffers are managed by micropython and are passed to rust when needed but reference is never kept. Credential verification calls into python which introduces a bit of weirdness.
  • In micropython every interface spawns three loops:
    • Read loop waits on an interface and passes received packets to the rust module which returns events like "message complete" which are then handled.
    • Write loop waits on a condition variable (mailbox) and when it's woken up it asks if the rust module has packets to send and pushes them into the interface.
    • Timeout loop waits on a timer and initiates retransmissions when needed.

Note: retest #6348 before merging.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 28, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ec0af57c-0ad8-45bf-b743-1e18fcf83074

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mmilata/thp-riir-trezor

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@trezor-bot trezor-bot Bot added this to Firmware Mar 28, 2026
@github-project-automation github-project-automation Bot moved this to 🔎 Needs review in Firmware Mar 28, 2026
@github-actions
Copy link
Copy Markdown

Looks like you changed Cargo.lock. Please make sure to review the dependencies and update internal version list.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 28, 2026

en main(all)

model device_test click_test persistence_test
T2T1 test(all) main(all) test(all) main(all) test(all) main(all)
T3B1 test(all) main(all) test(all) main(all) test(all) main(all)
T3T1 test(all) main(all) test(all) main(all) test(all) main(all)
T3W1 test(all) main(all) test(all) main(all) test(all) main(all)

Latest CI run: 25108356485

@mmilata mmilata force-pushed the mmilata/thp-riir-trezor branch from 789b49e to 6f187aa Compare March 28, 2026 15:58
@mmilata mmilata mentioned this pull request Mar 30, 2026
14 tasks
@mmilata mmilata force-pushed the mmilata/thp-riir-trezor branch 2 times, most recently from 906c175 to ca11ed9 Compare April 7, 2026 09:36
@socket-security
Copy link
Copy Markdown

socket-security Bot commented Apr 7, 2026

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report

@mmilata mmilata force-pushed the mmilata/thp-riir-trezor branch 2 times, most recently from b98c4e3 to 93c4a67 Compare April 10, 2026 10:24
@mmilata mmilata force-pushed the mmilata/thp-riir-trezor branch from 93c4a67 to 138c1bc Compare April 17, 2026 13:13
@mmilata mmilata moved this from 🔎 Needs review to 🏃‍♀️ In progress in Firmware Apr 20, 2026
@mmilata mmilata force-pushed the mmilata/thp-riir-trezor branch 6 times, most recently from 92d8ef3 to 44bdc61 Compare April 24, 2026 22:11
@mmilata mmilata force-pushed the mmilata/thp-riir-trezor branch from 15b681b to 75ab68e Compare April 28, 2026 18:51
@mmilata mmilata force-pushed the mmilata/thp-riir-trezor branch from 75ab68e to c604d66 Compare April 28, 2026 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🏃‍♀️ In progress

Development

Successfully merging this pull request may close these issues.

1 participant