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

Implement BIP174 (PSBT) for serialising partial and unsigned transactions #4615

Closed
SomberNight opened this issue Aug 8, 2018 · 7 comments · Fixed by #5721
Closed

Implement BIP174 (PSBT) for serialising partial and unsigned transactions #4615

SomberNight opened this issue Aug 8, 2018 · 7 comments · Fixed by #5721
Assignees
Labels
enhancement ✨ topic-transactions 📑 related to logic in transaction.py

Comments

@SomberNight
Copy link
Member

BIP174 is now no longer in "draft" state. It has also been merged into Bitcoin Core (in e.g. bitcoin/bitcoin#13557), and it looks like they will have support for it in 0.17.

We should consider ditching our custom format and implement it as well, as it would be good to be compatible with Bitcoin Core and potentially others.

Related:
#4514
#4614

@SomberNight SomberNight added topic-transactions 📑 related to logic in transaction.py enhancement ✨ labels Aug 14, 2018
@doc-hex
Copy link
Contributor

doc-hex commented Aug 15, 2018

If it helps, the Coldcard wallet plugin already can make a PSBT: see function built_psbt() in the PR #4470.

We do somewhat cheat because we didn't do any merging or combining, and just assume the result is a finalized transaction coming back from the Coldcard. (That's a safe assumption given the role of Coldcard in this case as a hardware keystore and signer.) We didn't need a PSBT parser.

Another limitation is the "Save as PSBT" button on the transaction dialog, appears only if the transaction is associated with a Coldcard based wallet. It should be possible for Electrum to offer PSBT output there for almost any unsigned transaction.

Naturally as PSBT support matures in Electrum, we will use those features as much as possible for this plugin. Always happy to delete code!

PS: You may find this debug tool, psbt_dump, to be useful, and @achow101 has been really helpful as well.

@SomberNight
Copy link
Member Author

note: @asfin has said he is interested in working on this

@roznalex
Copy link

Are there any updates?

@SomberNight
Copy link
Member Author

there are WIP changes at https://github.com/asfin/electrum/tree/bip174-2

@SomberNight SomberNight changed the title Implement BIP174 for serialising partial and unsigned transactions Implement BIP174 (PSBT) for serialising partial and unsigned transactions Oct 10, 2019
@SomberNight SomberNight self-assigned this Oct 10, 2019
@SomberNight
Copy link
Member Author

Note: I am now working on this. Trying to reuse code already written as part of the coldcard plugin, and some stuff from #4883

@craigraw
Copy link

I think this would be a valuable feature for many. Although I've only tested it with the coldcard, the plugin PSBT support works well, although it does lose partial sigs when loading PSBTs (fix proposed here: #5671).

Support for output descriptors would be very useful, but at the least Electrum needs to support generalised saving/retrieving the master key fingerprint for all participating keystores, particularly hardware keystores which store the derived xpub only. The coldcard plugin saves as custom attribute ckcc_xfp - other hardware wallet plugins will need to implement to comply with BIP174 if they are to take part in PSBTs. This could be a standard attribute in Hardware_KeyStore along with the existing xpub, label and derivation.

@SomberNight
Copy link
Member Author

at the least Electrum needs to support generalised saving/retrieving the master key fingerprint for all participating keystores, particularly hardware keystores which store the derived xpub only. The coldcard plugin saves as custom attribute ckcc_xfp - other hardware wallet plugins will need to implement to comply with BIP174 if they are to take part in PSBTs. This could be a standard attribute in Hardware_KeyStore along with the existing xpub, label and derivation.

Indeed you are right. I am planning on saving derivation path prefix and root node fingerprint for "all" keystores; maybe in the Xpub keystore mixin:

class Xpub:

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

Successfully merging a pull request may close this issue.

4 participants