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

WIP: payjoin as sender #6804

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

WIP: payjoin as sender #6804

wants to merge 20 commits into from

Conversation

rage-proof
Copy link
Contributor

@rage-proof rage-proof commented Dec 2, 2020

Hi, this PR is the try to implement the payjoin functionality as defined in BIP-78. It only covers the sender part in the interactive-tx and not the receiver stuff.
I tested it successful on Testnet with BTCPay Server using a hot wallet.
I'm not sure if its a good approach of even intended, but I added the payjoin-link in some functions and classes to forward it to the transaction window. It's a draft and you can do with it whatever you want to.

ToDo's:
These points are the obvious(for me) that must be finished.

  1. Added the 'requests' package, but probably it's not necessary and can be replaced by aiohttp package that is often present in electrum.
  2. The validation that checks if the newly added input has the same script-type(p2pkh, p2wpkh) as our own input is still missing. I don't know if there exist already a proper function in electrum for determine the script-type from a witness-utxo in a psbt.
  3. The check if the order of inputs and outputs was not changed. I didn't touch this, because I don't understand the BIP at this specific point.

Potential problem:
I saw this: btcpayserver/btcpayserver#1631 & https://blog.trezor.io/details-of-firmware-updates-for-trezor-one-version-1-9-1-and-trezor-model-t-version-2-3-1-1eba8f60f2dd .
Therefore probably a payjoin between HW-wallets and a merchant using BTCPay-Server is not feasible, but I didn't test it yet.

@ecdsa
Copy link
Member

ecdsa commented Dec 4, 2020

thanks! indeed, it would be nice to remove requests. we had that dependency earlier, and it took us quite some effort to get rid of it.

@rage-proof
Copy link
Contributor Author

Removed requests as dependency. Found a function that is designed for doing http requests and fits into the network event-loop.

@SomberNight
Copy link
Member

related: #6585

@ecdsa ecdsa added this to the 4.1.0 milestone Dec 10, 2020
@ecdsa
Copy link
Member

ecdsa commented Jan 13, 2021

This pull requests adds a bip78_payjoin field to Invoice. Since invoices are saved in the wallet file, a wallet_db upgrade must be added.

@rage-proof
Copy link
Contributor Author

Hi @ecdsa
I don't know what to do for this:

Since invoices are saved in the wallet file, a wallet_db upgrade must be added.

@ecdsa
Copy link
Member

ecdsa commented Feb 15, 2021

@rage-proof check out the code in wallet_db.py.
An upgrade is required so that your code is compatible with wallet files created before your code is merged.

# Conflicts:
#	electrum/gui/kivy/uix/screens.py
#	electrum/gui/qt/main_window.py
#	electrum/invoices.py
#	electrum/transaction.py
#	electrum/wallet.py
@rage-proof
Copy link
Contributor Author

rebased

@SomberNight
Copy link
Member

Do you know of an easy way to test this on testnet? E.g. is there some public "testnet merchant" running btcpayserver with payjoin enabled?

@rage-proof
Copy link
Contributor Author

You can create an account here https://testnet.demo.btcpayserver.org/ and test it; that's how I did it.
This is for example an invoice link with payjoin enabled: https://testnet.demo.btcpayserver.org/payment-requests/8001bd04-c095-473e-bafc-f90954d969ec .

@SomberNight SomberNight modified the milestones: 4.1.0, 4.2.0 Mar 20, 2021
@kristapsk
Copy link

Looking at code, this does not support .onion payjoin callback URLs, right? That is the only thing JoinMarket payjoin receiver currently supports. Would be cool to have it in Electrum if user has "Use Tor proxy at port 9050" enabled in settings.

@Kixunil
Copy link

Kixunil commented Jul 8, 2021

Something looks broken. I've created this test case based on official test vectors:

import unittest
from electrum.transaction import PayjoinTransaction, PartialTransaction

class PayJoinTestCase(unittest.TestCase):
    def test_official_vectors(self):
        original_psbt = PartialTransaction.from_raw_psbt(b"cHNidP8BAHMCAAAAAY8nutGgJdyYGXWiBEb45Hoe9lWGbkxh/6bNiOJdCDuDAAAAAAD+////AtyVuAUAAAAAF6kUHehJ8GnSdBUOOv6ujXLrWmsJRDCHgIQeAAAAAAAXqRR3QJbbz0hnQ8IvQ0fptGn+votneofTAAAAAAEBIKgb1wUAAAAAF6kU3k4ekGHKWRNbA1rV5tR5kEVDVNCHAQcXFgAUx4pFclNVgo1WWAdN1SYNX8tphTABCGsCRzBEAiB8Q+A6dep+Rz92vhy26lT0AjZn4PRLi8Bf9qoB/CMk0wIgP/Rj2PWZ3gEjUkTlhDRNAQ0gXwTO7t9n+V14pZ6oljUBIQMVmsAaoNWHVMS02LfTSe0e388LNitPa1UQZyOihY+FFgABABYAFEb2Giu6c4KO5YW0pfw3lGp9jMUUAAA=")
        print(original_psbt.to_json())
        original_psbt.update_txin_script_type()
        proposal = b"cHNidP8BAJwCAAAAAo8nutGgJdyYGXWiBEb45Hoe9lWGbkxh/6bNiOJdCDuDAAAAAAD+////jye60aAl3JgZdaIERvjkeh72VYZuTGH/ps2I4l0IO4MBAAAAAP7///8CJpW4BQAAAAAXqRQd6EnwadJ0FQ46/q6NcutaawlEMIcACT0AAAAAABepFHdAltvPSGdDwi9DR+m0af6+i2d6h9MAAAAAAQEgqBvXBQAAAAAXqRTeTh6QYcpZE1sDWtXm1HmQRUNU0IcBBBYAFMeKRXJTVYKNVlgHTdUmDV/LaYUwIgYDFZrAGqDVh1TEtNi300ntHt/PCzYrT2tVEGcjooWPhRYYSFzWUDEAAIABAACAAAAAgAEAAAAAAAAAAAEBIICEHgAAAAAAF6kUyPLL+cphRyyI5GTUazV0hF2R2NWHAQcXFgAUX4BmVeWSTJIEwtUb5TlPS/ntohABCGsCRzBEAiBnu3tA3yWlT0WBClsXXS9j69Bt+waCs9JcjWtNjtv7VgIge2VYAaBeLPDB6HGFlpqOENXMldsJezF9Gs5amvDQRDQBIQJl1jz1tBt8hNx2owTm+4Du4isx0pmdKNMNIjjaMHFfrQABABYAFEb2Giu6c4KO5YW0pfw3lGp9jMUUIgICygvBWB5prpfx61y1HDAwo37kYP3YRJBvAjtunBAur3wYSFzWUDEAAIABAACAAAAAgAEAAAABAAAAAAA="

        payjoin = PayjoinTransaction()
        payjoin.set_payjoin_original(original_psbt)
        payjoin.define_sender_params()
        payjoin.set_payjoin_proposal(proposal)
        payjoin.payjoin_proposal.update_txin_script_type()
        payjoin.validate_payjoin_proposal()

I got this exception:

  File "/home/user/electrum/electrum/transaction.py", line 2115, in define_sender_params
    raise ValueError(f"Transaction using a type that is unsupported for Payjoin."
ValueError: Transaction using a type that is unsupported for Payjoin.Type: unknown

If I understand it correctly the previously-written code is incapable of analyzing the input correctly (or test vectors are broken?).

@rage-proof
Copy link
Contributor Author

rage-proof commented Aug 16, 2021

If I understand it correctly the previously-written code is incapable of analyzing the input correctly (or test vectors are broken?).

Yeah, thanks for testing. The issue was that the nested script (here P2WPKH type of form: 0 + <20 bytes>) can be in the fields RedeemScript or ScriptSig. The code tested just for the first case. I adjusted it.

This code will now execute. I added one line for _additionalfeeoutputindex( = index of the change output), because this can only be determined when the tx was created and belongs to the wallet itself.

import unittest
from electrum.transaction import PayjoinTransaction, PartialTransaction

class PayJoinTestCase(unittest.TestCase):

    def test_official_vectors(self):
        original_psbt = PartialTransaction.from_raw_psbt(b"cHNidP8BAHMCAAAAAY8nutGgJdyYGXWiBEb45Hoe9lWGbkxh/6bNiOJdCDuDAAAAAAD+////AtyVuAUAAAAAF6kUHehJ8GnSdBUOOv6ujXLrWmsJRDCHgIQeAAAAAAAXqRR3QJbbz0hnQ8IvQ0fptGn+votneofTAAAAAAEBIKgb1wUAAAAAF6kU3k4ekGHKWRNbA1rV5tR5kEVDVNCHAQcXFgAUx4pFclNVgo1WWAdN1SYNX8tphTABCGsCRzBEAiB8Q+A6dep+Rz92vhy26lT0AjZn4PRLi8Bf9qoB/CMk0wIgP/Rj2PWZ3gEjUkTlhDRNAQ0gXwTO7t9n+V14pZ6oljUBIQMVmsAaoNWHVMS02LfTSe0e388LNitPa1UQZyOihY+FFgABABYAFEb2Giu6c4KO5YW0pfw3lGp9jMUUAAA=")
        print(original_psbt.to_json())
        original_psbt.update_txin_script_type()
        proposal = b"cHNidP8BAJwCAAAAAo8nutGgJdyYGXWiBEb45Hoe9lWGbkxh/6bNiOJdCDuDAAAAAAD+////jye60aAl3JgZdaIERvjkeh72VYZuTGH/ps2I4l0IO4MBAAAAAP7///8CJpW4BQAAAAAXqRQd6EnwadJ0FQ46/q6NcutaawlEMIcACT0AAAAAABepFHdAltvPSGdDwi9DR+m0af6+i2d6h9MAAAAAAQEgqBvXBQAAAAAXqRTeTh6QYcpZE1sDWtXm1HmQRUNU0IcBBBYAFMeKRXJTVYKNVlgHTdUmDV/LaYUwIgYDFZrAGqDVh1TEtNi300ntHt/PCzYrT2tVEGcjooWPhRYYSFzWUDEAAIABAACAAAAAgAEAAAAAAAAAAAEBIICEHgAAAAAAF6kUyPLL+cphRyyI5GTUazV0hF2R2NWHAQcXFgAUX4BmVeWSTJIEwtUb5TlPS/ntohABCGsCRzBEAiBnu3tA3yWlT0WBClsXXS9j69Bt+waCs9JcjWtNjtv7VgIge2VYAaBeLPDB6HGFlpqOENXMldsJezF9Gs5amvDQRDQBIQJl1jz1tBt8hNx2owTm+4Du4isx0pmdKNMNIjjaMHFfrQABABYAFEb2Giu6c4KO5YW0pfw3lGp9jMUUIgICygvBWB5prpfx61y1HDAwo37kYP3YRJBvAjtunBAur3wYSFzWUDEAAIABAACAAAAAgAEAAAABAAAAAAA="

        payjoin = PayjoinTransaction()
        payjoin.set_payjoin_original(original_psbt)
        payjoin.define_sender_params()
        payjoin._additionalfeeoutputindex = 0 ### this can only determined when the input belongs to the wallet itself
        payjoin.set_payjoin_proposal(proposal)
        payjoin.payjoin_proposal.update_txin_script_type()
        payjoin.validate_payjoin_proposal()

@Kixunil
Copy link

Kixunil commented Aug 17, 2021

Cool! Will try to test it some more later. Looking forward into this being available!

@SomberNight SomberNight modified the milestones: 4.2.0, 4.3.0 Mar 17, 2022
@SomberNight SomberNight modified the milestones: 4.3.0, backlog Jun 7, 2022
@accumulator accumulator added the privacy 🕵️‍♂️ kinda broad... network/blockchain/OS/... label Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
privacy 🕵️‍♂️ kinda broad... network/blockchain/OS/...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants