search

spesmilo / electrum

Electrum Bitcoin Wallet
https://electrum.org
MIT License
7.39k stars 3.08k forks source link

PSBT generation apparently incompatible with coldcard #5969

Closed jamesob closed 3 years ago

jamesob commented 4 years ago

As of 111ef9ebb11380cf2ee49b327988a8e21e99934b, PSBTs generated by electrum are considered invalid by Coldcard MK2. Wallet skeletons exported by the coldcard import successfully, but PSBTs which are exported cause the coldcard to display a "Failure to sign" message.

I realize this is pretty vague, so if I have some time in the next few days I'll try to reproduce on testnet and/or disassemble the apparently faulty PSBT.

(Maybe obviously) using Electrum 3.3.4 with the coldcard plugin ("Save PSBT" button) worked for me.

SomberNight commented 4 years ago

What is the firmware version on the coldcard?

gustavocoding commented 4 years ago

~~Running into same problem with MK3. Trying to sign the PSBT P2WPKH results in "Failure: none of the keys involved in this tx belong to this Coldcard". ColdCard version 3.0.6 Electrum 3.3.8~~

It works, it was using the wrong BIP39 passphrase inadvertently

SomberNight commented 4 years ago

Last time I tested was with a coldcard mk2 with fw 3.0.2. "Everything" seemed to work fine then.

jlopp commented 4 years ago

I'm trying to spend from a 3-of-5 p2sh-p2wsh wallet with a coldcard connected via USB.

Electrum version 4.0.0a0 running on testnet

Coldcard mk2 running firmware 3.0.6

Traceback (most recent call last): File "/home/jameson/code/electrum/electrum/plugins/coldcard/coldcard.py", line 381, in sign_transaction resp = client.sign_transaction_poll() File "/home/jameson/code/electrum/electrum/plugins/coldcard/coldcard.py", line 231, in sign_transaction_poll return self.dev.send_recv(CCProtocolPacker.get_signed_txn(), timeout=None) File "/home/jameson/.local/lib/python3.7/site-packages/ckcc/client.py", line 163, in send_recv return CCProtocolUnpacker.decode(resp) File "/home/jameson/.local/lib/python3.7/site-packages/ckcc/protocol.py", line 236, in decode return d(msg) File "/home/jameson/.local/lib/python3.7/site-packages/ckcc/protocol.py", line 250, in err_ raise CCProtoError("Coldcard Error: " + str(msg[4:], 'utf8', 'ignore'), msg[4:]) ckcc.protocol.CCProtoError: Coldcard Error: Invalid PSBT

The error on the coldcard makes a reference to multisig.py:701

SomberNight commented 4 years ago

@jlopp

I have tested now spending from a 2of2 p2sh-p2wsh wallet with 2 coldcards, connected via USB. (coldcard mk2 with fw 3.0.6) Worked for me.

Could you please share even more details:

jlopp commented 4 years ago

Sure, as usual I'm following our 3-of-5 recovery setup described at https://walletsrecovery.org/recovery-docs/casakeymaster-recovery.html

3 of the signers are hardware devices (one nano s, one model t, one coldcard) and two are xpubs.

I'm able to sign with the nano s and the model t, but the coldcard throws the PSBT error regardless of when it's signing.

I didn't set up the coldcard as a multisig wallet

jlopp commented 4 years ago

Another note: if I go into the multisig wallet options on the coldcard and enable "Trust PSBT" then it accepts the PSBT and signs it. So there may just be some mismatch between the xpubs electrum sets vs those on the coldcard. And it may or may not be related to the fact that I'm doing this on testnet.

doc-hex commented 4 years ago

I haven't look at the Electrum code yet, but here are my observations (4.0.0.a / master):

So I'm thinking the "Coldcard PSBT" option is broken for some reason. Inclusion of XPUB's in the global section isn't a Coldcard-dependant setting anyway; we accept both ways... maybe this is a workaround for our single-signer limitation related to xpubs (we don't accept them in that case)

SomberNight commented 4 years ago
  • made a transaction, and tried both "Export > Export File" and "Export > For Coldcard; include xpubs" commands
  • the first one, made a workable PSBT file, but the Coldcard-dedicated one did not
  • it had "shallow" derivation paths: (m=B8993350)/0/0 with unknown XFP values, where as the generic PSBT had correct XFP values and deeper paths: (m=0F056943)/48'/1'/0'/1'

I'll look into this but note that @jlopp said

with a coldcard connected via USB.

SomberNight commented 4 years ago

@peter-conalgo

  • made a transaction, and tried both "Export > Export File" and "Export > For Coldcard; include xpubs" commands
  • the first one, made a workable PSBT file, but the Coldcard-dedicated one did not
  • it had "shallow" derivation paths: (m=B8993350)/0/0 with unknown XFP values, where as the generic PSBT had correct XFP values and deeper paths: (m=0F056943)/48'/1'/0'/1'

Are you sure it's not the other way around? :/ I expect the "basic" export to have shallow paths and no global xpubs, while the "For Coldcard; include xpubs" export to have full paths and (if multisig) global xpubs. When USB-connected, the sent PSBT should behave like the latter: have full paths and (if multisig) global xpubs.

doc-hex commented 4 years ago

Coldcard needs the same PSBT file, regardless of sending over USB or MicroSD card. The xpub details in the globals section are optional, but allow users to preserve privacy somewhat and carry less settings in the Coldcard.

Putting part of the derivation path into the globals, and having only the relative subpath in the inputs/outputs is not supported by Coldcard, nor, to my reading, by the PSBT specification? I can see it would be useful though.

SomberNight commented 4 years ago

@jlopp

Another note: if I go into the multisig wallet options on the coldcard and enable "Trust PSBT" then it accepts the PSBT and signs it.

I'm looking at https://coldcardwallet.com/docs/multisig :

Setting: Trust PSBT?

This setting controls what the Coldcard does with the co-signer public keys (XPUB) that may be provided inside a PSBT file. There are three choices:

  • Verify Only Do not import the xpubs found, but do verify the correct wallet already exists on the Coldcard.
  • Offer Import If it's a new multisig wallet, offer to import the details and store them as a new wallet in the Coldcard.
  • Trust PSBT Use the wallet data in the PSBT as a temporary, multisig wallet, and do not import it. This permits some deniability and additional privacy.

When the XPUB data is not provided in the PSBT, regardless of the above, we require the appropriate multisig wallet to already exist on the Coldcard. The default is to 'Offer' unless at least one multisig wallet already exists, in which case the default becomes 'Verify'.

Does it work with "Offer Import"? Note that this is the default if you have no multisig wallets on-device setup yet. Given you have said you have not had setup the ms wallet on-device prior to signing, it will not work with "Veriy Only"; and note that that is the default if you have already set up any ms wallet on-device.

doc-hex commented 4 years ago

Moments ago, I fixed a regression in 3.1.0 that broke the "Trust PSBT" setting. It was working right in 3.0.6, and will again in 3.1.1 (yet to be released). Effectively on 3.1.0, it was stuck in "Offer Import" mode it could not make use of the xpubs found in the PSBT.

jlopp commented 4 years ago

@SomberNight I was actually mistaken about setting up the multisig wallet; it happened practically automatically when I signed my first transaction with the coldcard a while ago. This is why I was getting the "invalid PSBT" error - the validation of the xpubs was failing somewhere along the line. But I'm running firmware 3.0.6 so I wasn't hitting the issue Peter just described.

SomberNight commented 4 years ago

@jlopp So is your issue resolved then? Were you getting the error because the coldcard was in "Veriy Only" only mode with the expected multisig wallet was not set up on the device?

jlopp commented 4 years ago

I'd say the issue is not really resolved; I'd expect that Electrum should generate a PSBT that is considered valid for the multisig wallet I originally set up, since I was simply reproducing that wallet in Electrum.

The workaround that allowed me to sign was setting my coldcard to trust the PSBT rather than verify it against what the coldcard was expecting.

SomberNight commented 4 years ago

Electrum should generate a PSBT that is considered valid for the multisig wallet I originally set up

How did you set that wallet up?

I was actually mistaken about setting up the multisig wallet; it happened practically automatically when I signed my first transaction with the coldcard a while ago.

Were you using the same Electrum wallet file to create that original tx?

jpcummins commented 4 years ago

Not sure if this is related, but I see something similar.

Send > [fill out details] > Pay > Advanced > Finalize > Export > Export to file

Note: I don't have the option for "For Coldcard; include xpubs". I assume this is because I created the watch-only wallet manually, using BIP84 and I do not connect my hardware wallet to USB.

When using the exported file to sign on the Coldcard, it shows "Verifying" then "Failure - None of the keys involved in this transaction belong to this Coldcard (need [fingerprint])"

Electrum 4.0.0a0, Mac OS 10.15.3, ColdCard 3.1.0

doc-hex commented 4 years ago

Version 3.1.1 of Coldcard firmware is out now

SomberNight commented 4 years ago

I created the watch-only wallet manually, using BIP84 and I do not connect my hardware wallet to USB. When using the exported file to sign on the Coldcard, it shows "Verifying" then "Failure - None of the keys involved in this transaction belong to this Coldcard (need [fingerprint])"

I assume you created the watch-only wallet from just an xpub. In this case, the root fingerprint and the derivation prefix are missing, hence they cannot be put into the PSBT, hence the coldcard will not sign the tx. (related https://github.com/spesmilo/electrum/issues/5715 and https://github.com/spesmilo/electrum/issues/5955)

Note: I don't have the option for "For Coldcard; include xpubs". I assume this is because I created the watch-only wallet manually, using BIP84 and I do not connect my hardware wallet to USB.

While it would not solve your problem in itself (as above), maybe this option should be made always available.

SomberNight commented 4 years ago

I've pushed some changes as per my previous comment: https://github.com/spesmilo/electrum/commit/22861b70ee20e98f0c6306055ee93801ecd56035 https://github.com/spesmilo/electrum/commit/bea038ea6b88b4c2731300ddc134ac505678b4e4 https://github.com/spesmilo/electrum/commit/a987a2bbbee67ebfa79b1c3dbe1550e3b8c38f5f (re proper solution, https://github.com/spesmilo/electrum/issues/5715 is better place for discussion)

dgyg commented 4 years ago

After upgrading electrum from 3.3.8 to 4.0.2 in a single device setup the Coldcard can not sign the PSBT stored on the SD card anymore.

It complains about unknown keys in the transaction to be signed.

I tried the following things to solve the problem:

But the problem remains.

What else can be done?

After downgrading to 3.3.8 again, everything works fine again.

ghost commented 4 years ago

After upgrading electrum from 3.3.8 to 4.0.2 in a single device setup the Coldcard can not sign the PSBT stored on the SD card anymore.

It complains about unknown keys in the transaction to be signed.

I am having the same issue (running the latest coldcard firmware).

SomberNight commented 4 years ago

@dgyg @ph0cion How are you exporting the PSBT from Electrum?

In the tx dialog, try Export>For hardware device>Export to file

tx_dialog

ghost commented 4 years ago

Thanks @SomberNight ! That worked in my case.

This interface is a bit confusing though. Previously I was using Export > Export to file which apparently doesn't work with coldcard.

dgyg commented 4 years ago

@SomberNight Many thanks. Your suggestion helped me. I was using the wrong export functionality as well.

craig5999 commented 3 years ago

Helped me as well! Perhaps it would be better to make the default export option the one that works with CC? Cheers!

SomberNight commented 3 years ago

This thread has become hard to follow - multiple issues are being discussed here... Still, I think the most comments, and especially the last few, are about the different psbt export options. I have now changed (https://github.com/spesmilo/electrum/commit/c81551299ef470d8580b56c316be242a0b810d7c) the behaviour of the default export option to include full derivation paths for pubkeys, which should mean that most scenarios will just work.