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

Don't discard partial sigs when loading PSBT files #5671

Closed
wants to merge 3 commits into from

Conversation

craigraw
Copy link

@craigraw craigraw commented Oct 2, 2019

Currently when loading PSBT files, the Coldcard plugin's multisig support (added in #5440 by @peter-conalgo) causes any partial signatures to be discarded. This happens because the plugin sets the Transaction input field 'x_pubkeys' to the the same value as 'pubkeys', which is in turn parsed from the multisig script. The full xpubs are not contained in the script however, so this value is incorrect. This causes the following behaviour:

When loading the transaction dialog, Electrum compares the provided xpubs with the expected xpubs calculated from the MultisigWallet (see wallet.py line 2121). Where these differ, any signatures provided with that input are discarded. This can be tested using any PSBT that is partially signed - once this file is loaded through Tools -> Load Transaction -> From PSBT File the transaction dialog Status field will show 'Unsigned' and the partial signature is lost.

This PR resolves this issue by setting the xpubs correctly. They are retrieved from the PSBT by parsing the values retrieved from the PSBT_GLOBAL_XPUB field, and adding the relevant address indexes as parsed from the PSBT_IN_BIP32_DERIVATION field since Electrum expects them to be appended.

The result is that partial signatures from PSBT files are preserved, and the transaction dialog reflects the correct status - 'Partially Signed (m/n)'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-transactions 📑 related to logic in transaction.py
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants