search

cryptoadvance / specter-diy

DIY airgapped hardware wallet that uses QR codes for communication with the host
MIT License
440 stars 73 forks source link

multisig signing error: "invalid hash of the non witness utxo" #150

Closed eliasnaur closed 3 years ago

eliasnaur commented 3 years ago

A Specter DIY on firmware 1.5.3 fails on a 2-of-3 multisig PSBT that Trezor and Coldcard accepts. It was created by Sparrow. Screenshot of error from the device:

specter

I can provide the PSBT file if required.

Notes:

stepansnigirev commented 3 years ago

I tested with multisig native segwit, works for me... The fix you are referencing to should resolve the issue, it's very strange if it doesn't. Could you share with me a PSBT file you have problems with? If you don't want to share it on github you could send it to me by email or on Telegram. Is is native segwit or nested segwit?

eliasnaur commented 3 years ago

I believe it's native segwit all the way through. I've sent a .psbt file to your email address listed in your GH profile. Thanks for looking into this.

stepansnigirev commented 3 years ago

I can confirm the issue, I'll release a fix today. Looks like the change I did in bitcoin library didn't get into the release for some reason.

stepansnigirev commented 3 years ago

Please check with the latest release: https://github.com/cryptoadvance/specter-diy/releases/tag/v1.5.4

eliasnaur commented 3 years ago

Thanks! Specter DIY 1.5.4 no longer rejects the .psbt file. However, its signed .psbt is rejected by Sparrow with "Unverifiable partial signatures provided". I've emailed an example of a rejected .psbt to you.

stepansnigirev commented 3 years ago

I think the issue is that Specter reduces the signed PSBT to the very minimal - it only contains the transaction and partial signatures. We are expecting that software wallet knows everything about the transaction it is trying to combine, so it could combine initial and partially signed psbts and extract a fully signed transaction. Looks like Sparrow doesn't do that.

Maybe we can keep all the information in the transaction when using SD card or USB, as there we don't have space limitations like with QR codes. This means we'll need to adopt how we interact with the host depending on the communication channel - it's not optimal from the architecture perspective but looks doable.

It would be much easier if Sparrow could just use the data in the PSBT it is creating and combine it with the partially signed PSBT received by the device.

eliasnaur commented 3 years ago

I've referenced this bug on a Sparrow bug that sounds like just this issue, because it sounds reasonable Sparrow could be modified to accept DIY signatures.

eliasnaur commented 3 years ago

Fixed at the Sparrow side (https://github.com/sparrowwallet/sparrow/issues/78). Thank you for the support.