-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
PSBT generation apparently incompatible with coldcard #5969
Comments
What is the firmware version on the coldcard? |
It works, it was using the wrong BIP39 passphrase inadvertently |
Last time I tested was with a coldcard mk2 with fw 3.0.2. |
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
The error on the coldcard makes a reference to multisig.py:701 |
I have tested now spending from a 2of2 p2sh-p2wsh wallet with 2 coldcards, connected via USB. Could you please share even more details:
|
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 |
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. |
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) |
I'll look into this but note that @jlopp said
|
Are you sure it's not the other way around? :/ |
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. |
I'm looking at https://coldcardwallet.com/docs/multisig :
Does it work with "Offer Import"? Note that this is the default if you have no multisig wallets on-device setup yet. |
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, |
@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. |
@jlopp So is your issue resolved then? |
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. |
How did you set that wallet up?
Were you using the same Electrum wallet file to create that original tx? |
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 |
Version 3.1.1 of Coldcard firmware is out now
|
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 #5715 and #5955)
While it would not solve your problem in itself (as above), maybe this option should be made always available. |
not just if one of the keystores is a coldcard related: #5969 (comment)
related: #5969 (comment) Instead of a log line, maybe it should warn as part of the GUI.. but this is a start.
Power-users that know what they are doing can use this method to populate key origin information for keystore (bip32 root fingerprint and derivation path prefix). Try to make method hard to misuse. Qt console can now be used as e.g.: ``` wallet.get_keystores()[2].add_key_origin(derivation_prefix="m/48h/1h/0h/2h", root_fingerprint="deadbeef") ``` related #5715 related #5955 related #5969
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. |
I am having the same issue (running the latest coldcard firmware). |
@dgyg @ph0cion In the tx dialog, try |
Thanks @SomberNight ! That worked in my case. This interface is a bit confusing though. Previously I was using |
@SomberNight Many thanks. Your suggestion helped me. I was using the wrong export functionality as well. |
Helped me as well! Perhaps it would be better to make the default export option the one that works with CC? Cheers! |
There are three export options for exporting a PSBT. The default option previously only put derivation path suffixes for pubkeys (paths relative to the intermediate xpub), now it puts the full path (if is known by the keystore). The "export for hardware device; include xpubs" option works same as before: it puts both full paths and also global xpubs into the PSBT. Hence the difference between the default option and the "include xpubs" option is now only that the latter puts global xpubs into the PSBT. This change is largely made for user-convenient in mind. Now exporting a PSBT should be less error-prone: particularly for the single-signer coldcard with sdcard usage, the default option will now work. closes spesmilo#5969 related spesmilo#5955
This thread has become hard to follow - multiple issues are being discussed here... |
This script filters linter warnings to only find issues relative to upstream Electrum. Namecoin / AuxPoW: Fix linter warnings in blockchain.py Namecoin / AuxPoW: Fix linter warnings in merkle.py fix spesmilo#4561 fix spesmilo#4503: in kivy, catch NotEnoughFunds raised on open channel. kivy: disable send amount button if lightning invoice has an amount, and fix spesmilo#6526 Qt: do not save pending invoice before the user has confirmed payment kivy screens: initialize is_max (fix spesmilo#6813) kivy: dismiss invoice_dialog before delete (fix spesmilo#6816) revert a9fc440, use Clock.schedule_interval to set address. Fixes spesmilo#6810 and spesmilo#6817 Use os._exit() for FORKed child browser opener process. Fixes spesmilo#6404. On Linux, when Electrum is executed from Appimage file, to prevent system library inconsistence for Electrum and web browser and all issues involving that, Electrum starts web browser and opens web page upon clicking on 'View on block explorer' by fork()'ing the process, unsetting its custom LD_LIBRARY_PATH environment variable in the child process, and calling webbrowser.open(). Due to incorrect usage of sys.exit() instead of os._exit() for child process, Electrum (parent) can't be terminated and endlessly waits for child process upon exit, while child process does nothing but still exists. Fix this issue by using os._exit function, which should be used for child processes (not only in Python). kivy: wizard does not need Factory kivy: fix open channel with "max" amt related spesmilo#6169 E | gui.kivy.uix.dialogs.lightning_open_channel.LightningOpenChannelDialog | Problem opening channel Traceback (most recent call last): File "/home/user/wspace/electrum/electrum/gui/kivy/uix/dialogs/lightning_open_channel.py", line 167, in do_open_channel chan, funding_tx = lnworker.open_channel( File "/home/user/wspace/electrum/electrum/lnworker.py", line 859, in open_channel if funding_sat > LN_MAX_FUNDING_SAT: TypeError: '>' not supported between instances of 'str' and 'int' kivy: (fix) clicking "max" to send would raise for empty wallet fix spesmilo#6812 mnemonic.make_seed: de-duplicate num_bits default magic number plugins: remove 'on_new_window' hook kivy: remove redundant declaration network: set _loop_thread again as it helps debugging related: spesmilo#6825 Qt: keep pending_invoice logic in main_window (follow-up 56579c2, fixes spesmilo#5829) gitignore: add some more build artifacts dependencies: rm python-ecdsa rerun freeze_packages kivy: ensure WizardDialog.on_release is not executed more than once (see spesmilo#6822) dnspython: fix deprecation warnings when using dnspython 2.0 related: spesmilo#6828 qt swap dialog: fix enabling OK button fixes spesmilo#6831 keystore: handle unusual derivation paths in PSBT If a tx contained a derivation path for a pubkey, with a length=2 der suffix, with the first element of the suffix not in (0, 1), with a fingerprint that matches either our root or intermediate fp, then processing that tx would raise and result in a crash reporter. Traceback (most recent call last): File ".../electrum/electrum/gui/qt/main_window.py", line 2718, in do_process_from_text self.show_transaction(tx) File ".../electrum/electrum/gui/qt/main_window.py", line 1041, in show_transaction show_transaction(tx, parent=self, desc=tx_desc) File ".../electrum/electrum/gui/qt/transaction_dialog.py", line 84, in show_transaction d = TxDialog(tx, parent=parent, desc=desc, prompt_if_unsaved=prompt_if_unsaved) File ".../electrum/electrum/gui/qt/transaction_dialog.py", line 680, in __init__ self.set_tx(tx) File ".../electrum/electrum/gui/qt/transaction_dialog.py", line 218, in set_tx tx.add_info_from_wallet(self.wallet) File ".../electrum/electrum/transaction.py", line 1944, in add_info_from_wallet wallet.add_input_info(txin, only_der_suffix=only_der_suffix) File ".../electrum/electrum/wallet.py", line 1573, in add_input_info is_mine = self._learn_derivation_path_for_address_from_txinout(txin, address) File ".../electrum/electrum/wallet.py", line 2609, in _learn_derivation_path_for_address_from_txinout pubkey, der_suffix = ks.find_my_pubkey_in_txinout(txinout, only_der_suffix=True) File ".../electrum/electrum/keystore.py", line 155, in find_my_pubkey_in_txinout path = self.get_pubkey_derivation(pubkey, txinout, only_der_suffix=only_der_suffix) File ".../electrum/electrum/keystore.py", line 391, in get_pubkey_derivation if not test_der_suffix_against_pubkey(der_suffix, pubkey): File ".../electrum/electrum/keystore.py", line 368, in test_der_suffix_against_pubkey if pubkey != self.derive_pubkey(*der_suffix): File ".../electrum/electrum/keystore.py", line 491, in derive_pubkey assert for_change in (0, 1) AssertionError kivy: add on_dismiss method to crash reporter dialog qt StatusBarButton: use QToolButton instead of QPushButton related: spesmilo#6299 qt StatusBarButton: use custom theme for macOS if using default theme prepare 4.0.7 update locale update locale Namecoin / AuxPoW: contrib: make lint_diff.sh more robust This makes the script work properly when run from a commit that isn't the tip of a branch. rerun freeze_packages trustedcoin: fix two-step wallet creation (offline->online) got broken in c46fbf0 kivy: dismiss wizard dialog before calling go_back test_wallet_vertical: add test case for pre-2.7 "2fa" seed mnemonic: tighten seed_type check for old "2fa" type seeds Seeds in the set difference could already not be restored: they raised an exception in the wizard; now these are not recognised as valid seeds anymore (so e.g. OK button in wizard will be disabled). Also see comments in code. cleanup, remove if statement (follow-up 13b05f6) keystore: ignore fingerprint for pubkeys in psbt, try to match all keys transaction: put full derivation paths into PSBT by default There are three export options for exporting a PSBT. The default option previously only put derivation path suffixes for pubkeys (paths relative to the intermediate xpub), now it puts the full path (if is known by the keystore). The "export for hardware device; include xpubs" option works same as before: it puts both full paths and also global xpubs into the PSBT. Hence the difference between the default option and the "include xpubs" option is now only that the latter puts global xpubs into the PSBT. This change is largely made for user-convenient in mind. Now exporting a PSBT should be less error-prone: particularly for the single-signer coldcard with sdcard usage, the default option will now work. closes spesmilo#5969 related spesmilo#5955 Save height in invoices, use it to determine invoice status (fixes spesmilo#6609) network: when switching servers, don't wait for old interface to close The GUI blocks until network.set_parameters returns when switching servers, which waits for switch_to_interface, which used to wait until interface.close() returns. interface.close() tries to flush buffered writes to the wire, with a 30 sec timeout. If the server or the network connection is slow, flushing the buffer can take several seconds. In particular, servers running Fulcrum always seem to timeout in this case, freezing the GUI for 30 seconds (when switching away). lnworker: remove channel if it was not saved successfully after creation kivy: initialize app.android_backups from config, and show error message if save_backup raises an exception build: don't allow setuptools to sneakily install build-time deps see https://pip.pypa.io/en/stable/reference/pip_install/#controlling-setup-requires > Setuptools offers the setup_requires setup() keyword for specifying > dependencies that need to be present in order for the setup.py > script to run. Internally, Setuptools uses easy_install to > fulfill these dependencies. > pip has no way to control how these dependencies are located. > None of the package index options have an effect. With these changes, we will now instead hard fail if this were to happen. related: spesmilo#5859 (comment) network: increase MOST_RELAXED default timeout to 10 minutes related: spesmilo#6741 synchronizer: request missing txs for addresses in random order as discussed in issue spesmilo#6697, users with large wallets or slow connections may never see their initial request-missing-tx run complete, if we always use the same sync order. This commit shuffles the addresses being requested every time a new request round happens, so that (if enough time passes and enough initial state requests are attempted) we will always get the latest state for each address, regardless of how quickly the connection times out on us wallet: use height to determine request status (similar to outgoing invoices) kivy: allow address reuse in imported wallets. (fix spesmilo#6852) kivy: move "lightning" (gossip) btn inside "channels" dialog and show toast msg when opening "channels" dialog if lightning is not available for wallet qt swap dialog: attempt at clearer logic (at the cost of more lines) fixes spesmilo#6853 swaps: disable button if no channel present swaps: limit forward amount to receivable amount on lightning swaps: fix normal amount formulas In a normal (forward) swap (onchain->offchain): send_amount = receive_amount * (1 + service_percentage) + normal_fee , and vice versa: receive_amount = (send_amount + normal_fee) / (1 + service_percentage) , i.e., the service fee is charged on the received offchain amount. wallet_db: impl convert_version_33: put 'height' field into invoices The 'height' field was added in spesmilo@cdfaaa2 At the time we thought we could just add it with a default value without a db upgrade; however the issue is that if old code tries to open a new db, it will fail (due to unexpected new field). Hence it is better to do an explicit conversion where old code *knows* it cannot open the new db. E | gui.qt.ElectrumGui | Traceback (most recent call last): File "...\electrum\electrum\gui\qt\__init__.py", line 257, in start_new_window wallet = self.daemon.load_wallet(path, None) File "...\electrum\electrum\daemon.py", line 488, in load_wallet db = WalletDB(storage.read(), manual_upgrades=manual_upgrades) File "...\electrum\electrum\wallet_db.py", line 72, in __init__ self.load_data(raw) File "...\electrum\electrum\wallet_db.py", line 103, in load_data self._after_upgrade_tasks() File "...\electrum\electrum\wallet_db.py", line 189, in _after_upgrade_tasks self._load_transactions() File "...\electrum\electrum\util.py", line 408, in <lambda> return lambda *args, **kw_args: do_profile(args, kw_args) File "...\electrum\electrum\util.py", line 404, in do_profile o = func(*args, **kw_args) File "...\electrum\electrum\wallet_db.py", line 1139, in _load_transactions self.data = StoredDict(self.data, self, []) File "...\electrum\electrum\json_db.py", line 79, in __init__ self.__setitem__(k, v) File "...\electrum\electrum\json_db.py", line 44, in wrapper return func(self, *args, **kwargs) File "...\electrum\electrum\json_db.py", line 105, in __setitem__ v = self.db._convert_dict(self.path, key, v) File "...\electrum\electrum\wallet_db.py", line 1182, in _convert_dict v = dict((k, Invoice.from_json(x)) for k, x in v.items()) File "...\electrum\electrum\wallet_db.py", line 1182, in <genexpr> v = dict((k, Invoice.from_json(x)) for k, x in v.items()) File "...\electrum\electrum\invoices.py", line 108, in from_json return OnchainInvoice(**x) TypeError: __init__() got an unexpected keyword argument 'height' prepare release 4.0.8 Support binary encoding of names in Electrum-NMC GUI (exposed as hex in GUI)
As of 111ef9e, 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.
The text was updated successfully, but these errors were encountered: