Skip to content

Feat: protocol-thp encode/decode #19073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
May 23, 2025
Merged

Feat: protocol-thp encode/decode #19073

merged 2 commits into from
May 23, 2025

Conversation

szymonlesisz
Copy link
Contributor

@szymonlesisz szymonlesisz commented May 21, 2025 •
edited by trezor-bot bot
Loading

Description

add THP encoding/decoding logic

  • only one message exchange is supported ThpCreateChannelRequest > ThpCreateChannelResponse
  • encoding/decoding ThpAck and ThpError
  • crc32 module: create crc32 for encoded message + validate crc32 for decoded message
  • cherry-picked without cryptography module. cryptography will be added in the next commit (see TODO: add-crypto)

🔍🖥️ Suite web test results: View in Currents

🔍🖥️ Suite desktop test results: View in Currents

@szymonlesisz szymonlesisz added connect Connect API related (ie. fee calculation) no-project This label is used to specify that PR doesn't need to be added to a project cherry-pick🍒 labels May 21, 2025
mroz22
mroz22 reviewed May 21, 2025
View reviewed changes
marekrjpolak
marekrjpolak reviewed May 21, 2025
View reviewed changes
@szymonlesisz szymonlesisz force-pushed the feat/protocol-thp-encode branch from c6926e6 to 7199712 Compare May 22, 2025 08:57
@trezor-bot
Copy link

trezor-bot bot commented May 22, 2025

✅ Previously successful run of [Test] PR Suite Web e2e tests workflow has been found.
⏭️ Skipping tests for this run.
💡 If you are unsure about your latest changes, please rerun the workflow manually. (Use the Re-run all jobs option)

@szymonlesisz szymonlesisz force-pushed the feat/protocol-thp-encode branch from 7199712 to 0c4beec Compare May 22, 2025 12:18
@trezor-bot
Copy link

trezor-bot bot commented May 22, 2025

✅ Previously successful run of [Test] PR Suite Web e2e tests workflow has been found.
⏭️ Skipping tests for this run.
💡 If you are unsure about your latest changes, please rerun the workflow manually. (Use the Re-run all jobs option)

1 similar comment
@trezor-bot
Copy link

trezor-bot bot commented May 22, 2025

✅ Previously successful run of [Test] PR Suite Web e2e tests workflow has been found.
⏭️ Skipping tests for this run.
💡 If you are unsure about your latest changes, please rerun the workflow manually. (Use the Re-run all jobs option)

@trezor-bot
Copy link

trezor-bot bot commented May 22, 2025

✅ Previously successful run of [Test] PR Suite Desktop e2e tests workflow has been found.
⏭️ Skipping tests for this run.
💡 If you are unsure about your latest changes, please rerun the workflow manually. (Use the Re-run all jobs option)

marekrjpolak
marekrjpolak approved these changes May 22, 2025
View reviewed changes
Copy link
Contributor

@marekrjpolak marekrjpolak left a comment

Choose a reason for hiding this comment

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

LGTM

@trezor-bot
Copy link

trezor-bot bot commented May 22, 2025

✅ Previously successful run of [Test] PR Suite Desktop e2e tests workflow has been found.
⏭️ Skipping tests for this run.
💡 If you are unsure about your latest changes, please rerun the workflow manually. (Use the Re-run all jobs option)

@trezor-bot
Copy link

trezor-bot bot commented May 22, 2025

✅ Previously successful run of [Test] PR Suite Web e2e tests workflow has been found.
⏭️ Skipping tests for this run.
💡 If you are unsure about your latest changes, please rerun the workflow manually. (Use the Re-run all jobs option)

@szymonlesisz
Copy link
Contributor Author

Lastminute change as promised,
i've rebased bt-thp branch over this and found some inconsistency in naming and missing utility

@szymonlesisz
Copy link
Contributor Author

/rebase

@github-actions GitHub Actions
Copy link

Start rebasing: https://github.com/trezor/trezor-suite/actions/runs/15205527129

@szymonlesisz
feat(protocol): add credentials to ThpState
08c4900
@szymonlesisz
feat(protocol): add THP encoding/decoding logic
98d210d 
- only one message exchange is supported ThpCreateChannelRequest > ThpCreateChannelResponse
- without cryptography module. will be added in the next commit
@trezor-bot trezor-bot bot force-pushed the feat/protocol-thp-encode branch from 7f5111d to 98d210d Compare May 23, 2025 08:09
@trezor-bot
Copy link

trezor-bot bot commented May 23, 2025

✅ Previously successful run of [Test] PR Suite Web e2e tests workflow has been found.
⏭️ Skipping tests for this run.
💡 If you are unsure about your latest changes, please rerun the workflow manually. (Use the Re-run all jobs option)

@trezor-bot
Copy link

trezor-bot bot commented May 23, 2025

✅ Previously successful run of [Test] PR Suite Desktop e2e tests workflow has been found.
⏭️ Skipping tests for this run.
💡 If you are unsure about your latest changes, please rerun the workflow manually. (Use the Re-run all jobs option)

@szymonlesisz szymonlesisz mentioned this pull request May 23, 2025
Merged
@szymonlesisz szymonlesisz merged commit b33b491 into develop May 23, 2025
24 checks passed
@szymonlesisz szymonlesisz deleted the feat/protocol-thp-encode branch May 23, 2025 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroz22 mroz22 mroz22 left review comments

@marekrjpolak marekrjpolak marekrjpolak approved these changes

Assignees
No one assigned
Labels
cherry-pick🍒 connect Connect API related (ie. fee calculation) no-project This label is used to specify that PR doesn't need to be added to a project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@szymonlesisz @marekrjpolak @mroz22