Skip to content
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

Add multisig_xpub_magic option to GetPublicKey #4305

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Add multisig_xpub_magic option to GetPublicKey #4305

wants to merge 6 commits into from
+255 −183

Conversation

andrewkozlik
Copy link
Contributor

@andrewkozlik andrewkozlik commented Nov 3, 2024 •
edited
Loading

Fixes #2658

Adds a bool option multisig_xpub_magic to GetPublicKey, so that Trezor displays XPUBs with the correct SLIP-132 prefix (XPUB magic) for SegWit v0 multisig accounts.

See also Slack discussion about the solution: https://satoshilabs.slack.com/archives/C02V2PSDNA2/p1669125576505009

This PR unifies the xpub_magic handling for get_public_key() and get_address(). Each function behaves a bit differently when the required SegWit coin.xpub_magic_* attribute is not defined. One raises an exception while the other falls back to plain old coin.xpub_magic. However, from what I can tell cointool.py guarantees that the attributes are defined if coin.segwit is true. So I feel that the simplest solution is to fall back.

@andrewkozlik andrewkozlik added core Trezor Core firmware. Runs on Trezor Model T and T2B1. trezorlib Python library and the command line trezorctl tool. bitcoin Bitcoin related protobuf Structure of messages exchanged between Trezor and the host feature Product related issue visible for end user python Pull requests that update Python code translations Put this label on a PR to run tests in all languages labels Nov 3, 2024
@andrewkozlik andrewkozlik self-assigned this Nov 3, 2024
@andrewkozlik andrewkozlik mentioned this pull request Nov 3, 2024
Open
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 3, 2024 •
edited
Loading

core UI changes device test click test persistence test
T2T1 Model T test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3B1 Safe 3 test(screens) main(screens) test(screens) main(screens) 2724
T3T1 Safe 5 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 4, 2024 •
edited
Loading

legacy UI changes device test(screens) main(screens)

@andrewkozlik andrewkozlik force-pushed the andrewkozlik/multisig branch 2 times, most recently from 48c04ce to 9c8dd25 Compare November 4, 2024 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matejcik matejcik Awaiting requested review from matejcik matejcik will be requested when the pull request is marked ready for review matejcik is a code owner

@prusnak prusnak Awaiting requested review from prusnak prusnak will be requested when the pull request is marked ready for review prusnak is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@andrewkozlik andrewkozlik

Labels
bitcoin Bitcoin related core Trezor Core firmware. Runs on Trezor Model T and T2B1. feature Product related issue visible for end user protobuf Structure of messages exchanged between Trezor and the host python Pull requests that update Python code translations Put this label on a PR to run tests in all languages trezorlib Python library and the command line trezorctl tool.
Projects
Firmware
Status: 🔎 Needs review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Multisig option when exporting public key
1 participant
@andrewkozlik