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.

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

Multisig option when exporting public key #2658

Open
sime opened this issue Nov 23, 2022 · 13 comments · May be fixed by #4305
Open

Multisig option when exporting public key #2658

sime opened this issue Nov 23, 2022 · 13 comments · May be fixed by #4305
Labels
core Trezor Core firmware. Runs on Trezor Model T and T2B1. T1B1 legacy Trezor One trezorlib Python library and the command line trezorctl tool.

Comments

@sime
Copy link
Member

sime commented Nov 23, 2022

Related issue: trezor/trezor-suite#6918
Internal Slack discussion: https://satoshilabs.slack.com/archives/C02V2PSDNA2/p1669125576505009

Introduce an option in CTL in get-public-node and in protobuf to export multisig specific xpubs.

@sime sime added core Trezor Core firmware. Runs on Trezor Model T and T2B1. T1B1 legacy Trezor One trezorlib Python library and the command line trezorctl tool. labels Nov 23, 2022
@sime sime added this to Firmware Nov 23, 2022
@CaveRock
Copy link

Is the slack discussion public? Would like to get an idea on possible timeline for fix to issue 6918 as this has effect on setting up a multisig wallet in electrum using public keys pulled at BIP48 paths.

@sime
Copy link
Member Author

sime commented Nov 30, 2022

@CaveRock It's not public and cannot offer a timeline.

Though trezor/trezor-suite#6918 doesn't affect Electrum, Electrum usestrezorlibPython library, which also lives in this repository.

@CaveRock
Copy link

@sime

Thanks for the feedback.

Just to give some more info on our electrum issue, which may not be a major use case and I can understand there not being an eta on an update but for interest sake I wanted to describe it.

The Electrum problem we have is where a watch only Electrum native segwit multisig wallet is being setup using master keys pulled from Trezor devices using Trezor Connect.

If a user uses Trezor connect to pull keys at BIP48 path to setup a watch only Electrum native segwit multisig wallet, the keys the Trezor will export will be vpubs (zpubs). If one attempts to submit a vpub (zpub) as the master key in Electrum native segwit multsig wallet setup it will, correctly, throw an error.

This means that the user must know to convert the vpub (zpub) to a Vpub (Zpub) before inputting into Electrum to create the watch only wallet.

It also means that there is no way for a user to have their Trezor confirm a Vpub (Zpub) on screen for peace of mind in confirming they are part of a native segwit multisig wallet.

(I have put main net prefixes in brackets for clarity)

@sime
Copy link
Member Author

sime commented Nov 30, 2022

Thanks for the context. You are using Trezor Content medium for fetching master public keys which will be input into Electrum.

So no direct Electrum to Trezor integration.

@CaveRock
Copy link

That's correct, and I also confirm no direct Electrum / Trezor integration.
Thanks.

@matejcik
Copy link
Contributor

It also means that there is no way for a user to have their Trezor confirm a Vpub (Zpub) on screen for peace of mind in confirming they are part of a native segwit multisig wallet.

FWIW, this can be done. The way to do it is to show a receiving address, then you can confirm all the multisig cosigners, including yourself, with the correct Vpub / Zpub prefixes.
The missing step is of course fetching the public key before you have all the cosigners together

@CaveRock
Copy link

@matejcik
Ya, we have used the confirm address feature that can be used with Unchained Capital library for giving customer peace of mind for checking their wallet addresses and the feature works well! And as you say, it just the missing step on wallet setup itself, but having the address checked does mitigate this somewhat. Thanks

@gwilkinson01
Copy link

So possibly related to this issue: I created a multisig native segwit wallet in Sparrow. Added a keystore using my trezor safe 3 (firmware: 2.8.0). I wanted to verify that the zpub shown in sparrow matched with the one shown on the trezor device so I ran this command: trezorctl btc get-public-node -n "m/48h/0h/0h/2h" -d but the zpub that was shown on the trezor did not match with that in sparrow.

Retrieving the xpub for the legacy path m/45' worked perfectly.

maybe also relates to trezor/trezor-suite#6918

@andrewkozlik
Copy link
Contributor

@gwilkinson01 when you use the trezorctl command, does Trezor show a zpub or an xpub? How about if you add -t segwit:

trezorctl btc get-public-node -n "m/48h/0h/0h/2h" -d -t segwit

@gwilkinson01
Copy link

gwilkinson01 commented Nov 2, 2024

@andrewkozlik Trezor shows a zpub (although confusingly the header states xpub 😆). See below image:

image

I get the same output with trezorctl btc get-public-node -n "m/48h/0h/0h/2h" -d -t segwit

@andrewkozlik
Copy link
Contributor

@gwilkinson01, I see. Your problem is indeed what this issue is about. I think that a simple workaround for now is to use -t address with trezorctl. That should force Trezor to show an xpub on the screen. Sparrow lets you switch between the "Zpub" prefix and the "xpub" prefix, so that you can compare them:
image

Also @matejcik proposed another workaround above.

@gwilkinson01
Copy link

@andrewkozlik perfect, works for me. Thanks for the swift response!

@andrewkozlik andrewkozlik linked a pull request Nov 3, 2024 that will close this issue
@andrewkozlik
Copy link
Contributor

FYI, I created a draft PR to fix this, see #4305. I still need to implement the corresponding changes in legacy and add device tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Trezor Core firmware. Runs on Trezor Model T and T2B1. T1B1 legacy Trezor One trezorlib Python library and the command line trezorctl tool.
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

5 participants