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

Device PSBT Doesn't Work for Multisig #88

Closed
mflaxman opened this issue Oct 19, 2020 · 7 comments
Closed

Device PSBT Doesn't Work for Multisig #88

mflaxman opened this issue Oct 19, 2020 · 7 comments

Comments

@mflaxman
Copy link
Contributor

mflaxman commented Oct 19, 2020

Example of what my DIY returns on signing (this is a 1-of-4 on testnet):

cHNidP8BAH0CAAAAAcHvPZ3T/4G4m+ZUJWUA7wu2WsD48adq4dakp4JoRUJTAAAAAAD/////AtMSAAAAAAAAFgAUNTpLGUQtYD0P0IfyYHSm4B9K06iIEwAAAAAAACIAIE1pVeThYKqzZZmSDwOs1LWIkyF2CjS+UMG8yJ19SMShAAAAAAAAAAA=

C6A9438C-5655-4D44-B864-4A5E22DE2CAE_1_105_c

I'm not sure if this is a DIY or Desktop issue. Desktop will scan the device QR code from DIY yet still displays 0 signatures (and no error/warning):
Screen Shot 2020-10-19 at 11 17 37 AM

Standard PSBT works great though, so perhaps this is more of a UX issue (warn user that this workflow doesn't work and that they need a device PSBT).

@stepansnigirev
Copy link
Collaborator

Did you import the multisig wallet to Specter-DIY?
Device PSBT only works if the wallet is imported to the hardware - we can significantly reduce the QR code size if we assume that hardware wallet already knows everything about this wallet - cosigners and scripts.

To import multisig wallet to DIY you need to go to settings of the wallet in Specter Desktop > Export to hardware - scan it with DIY and confirm adding the wallet. Then the wallet will be able to sign compressed psbt as well.

I need to make a note in the UI somewhere...

@mflaxman
Copy link
Contributor Author

Ah, I tried to import first but previous got an invalid PSBT error:
IMG_1137

In retrospect, I realize what I did wrong. I was trying to scan a Cobo QR code and not a Specter QR code (for background, I have a ton of testing wallets that include seedpicker imports, so I was using that one and restored the seed to my DIY but didn't change the device type in Desktop to DIY).

@mflaxman
Copy link
Contributor Author

Update: by first importing the wallet as a DIY and not a Cobo that fixed the problem. Device PSBT now signs TXs successfully:
https://blockstream.info/testnet/tx/87bdf4af522c39ecb490846f61ce23c39e8321dcb4abb830303bd8a968d42607

So the issue in my mind is that DIY will sign a transaction that it's a participant in but isn't a registered wallet. It should throw a warning telling you not to do that (and to register the multisig first), though I can see an argument for allowing advancing users to transact in that case anyway if they want to be #reckless.

@mflaxman
Copy link
Contributor Author

I'm on v1.4.0-pre1 btw

@stepansnigirev
Copy link
Collaborator

stepansnigirev commented Oct 19, 2020

I think there is a warning that you are spending from unknown wallet, but I think we should make it larger - like a separate confirmation screen saying that "multisig wallet is unknown, import it with the QR code" or something.

@mflaxman
Copy link
Contributor Author

There's an indication that it's an unknown wallet, but no indicator on how to make it known. Remember that I was aware of this issue and trying to import it, but the mistake was that I had it designated in Specter-Desktop as an "other" (paper wallet and not a Specter-DIY):
FD1C0A14-41CB-4BCF-8536-7473C1D03DA6_1_105_c

My 2 recs:

  1. Throw a useful error here (Device PSBT Doesn't Work for Multisig #88 (comment)) when trying import a wallet in the wrong way.
  2. Inform the user to import the wallet before signing (I assumed DIY didn't have the functionality). I'm not sure if there's room to add a note about change detection (if > 1 outputs), but it might help if you tell people why they should perform this step.

@stepansnigirev
Copy link
Collaborator

It shows a warning now and still offers to sign if you are sure, but without change verification.
If the wallet is imported then change verification works.

multisig_unknown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants