BIP78 PayJoin support (as implemented by BTCPay Server)

[kristapsk]

Recently new version of BTCPay Server has been released, which implements receiving support for a modified version of BIP79 (bustapay). Sending support is already implemented in is proposed for Wasabi Wallet, there is draft PR for BlueWallet and it is coming to Blockstream Green wallet soon too. Looks like this could become a P2EP / PayJoin standard or at least some future standard could be based on this. I think it makes much sense to implement this in JoinMarket too, at least sending support initially.

It looks to me it will require adding HTTP client as a dependency and also PSBT stuff (I believe that code is in #403, although I haven't reviewed it yet myself).

Thoughts?

[AdamISZ]

Yes, I took a little look at the proposed modified BIP79 today, it looks like a good protocol to me (I still think protocol versioning should be included, but it's not the end of the world).
Indeed the PSBT aspect is important. I added something in #403 indeed, but it wasn't really up to scratch, and was the biggest thing stopping me from moving forward. However #536 would allow this since that package has added psbt in what looks like a solid way (although I'd need to investigate further).

[kristapsk]

I have started to looking at this more from the practical side (trying to implement it as a bash script for Bitcoin Core first) and PSBT is just a recommendation, not a strict requirement, can use raw transactions instead, so this can be implemented before #536 too.

Also, BTCPay server currently allows user to copy BIP21 URI with all data, so probably implementing generic BIP21 support for sendpayment (and for qt in future) would be good idea before this. Amount and destination parameters of sendpayment could be replaced by BIP21 URI as an alternative syntax. Of course, backwards compatible way.

at least sending support initially.

I had some thoughts about this and now I think implementing receiving side of this proposal for JoinMarket makes no sense, running HTTP server would be overkill. JM is not a merchant solution and for payjoins between two JM users existing payjoin solution can be used.

[AdamISZ]

@kristapsk some good points there. I agree PSBT is not essential, but given it's strongly to be recommended here and needed for other things (SNICKER, cold wallets - although we have a big stumbling block with sending coins from cold storage via JM coinjoins, that doesn't mean we can't have some support - and PSBT is probably the biggest part of that!), we should definitely be wanting to do it.

Receiving side - well it's interesting isn't it, because we do currently support Payjoin receiving :) Wrt an http server, it could work, with an onion, thus obviating the need for communicating over IRC. I agree it's a bit outside our current architecture though, so it makes sense to leave it out for now and just implement sender.

[kristapsk]

Of course, PSBT should be long term goal, just seems that #536 will take some time and this could be implemented relatively fast without it, would like to see it supported with the next release. This seems to me to be the case when it's worth to do some fast hackish way right now and then rewrite it in the future properly.

[AdamISZ]

Right. Could you work on the BIP21 side? I mean, if you have time. That would be the obvious thing to add. For the rest of the code, I'm not sure, but I think probably no point trying to use something similar to jmclient.P2EPTaker since most of that code is designed to reuse existing JM architecture. We could just have a separate script in /scripts that just uses a JM wallet and does the protocol in a linear fashion, I guess.

[kristapsk]

Ok, BIP21 on me, I implemented it yesterday in bash, should not be a problem in Python. :)

[AdamISZ]

PSBT (and SNICKER basic function) now implemented in #536 ... I will be adding some basic PSBT functionality at user level soon.

[AdamISZ]

Above is done, we can probably go ahead and build the script in #536 after #560 is finished (which it basically is).

[AdamISZ]

Some thoughts on the status of this:

• I strongly prefer to merge python-bitcointx backend for jmbitcoin + bip78 and snicker. #536 first, and honestly don't intend to try to support it without that. The reason is that this is extremely time consuming work and I think opens up enormously our flexibility with respect to adding new features, because the foundations (all the bitcoin handling functions, including PSBT) are dealt with better there. I'll be doing more tests in the coming days to ensure that my earlier rounds of testing in March have not been broken by rebase on master or other additions. If you don't buy the logic laid out in the PR, consider that we don't even have PSBT otherwise. Yes, there may be an OS out there that we no longer support, I think we must simply advise people to either upgrade their OS, or their Python installation, or keep using JM 0.6.2.
• Feature (btcpayserver payjoin) will be supported by jmclient.SegwitWallet and jmclient.SegwitLegacyWallet only (in line with btcpayserver docs also), i.e. not LegacyWallet.
• Script only support initially (as discussed above), but as soon as that is functional I will make it top priority to add it to the GUI as it is quite easy, just takes a little time.
• Taking all of the above as a given, I would think that payjoin will be just added as a payment mode in sendpayment.py, triggered by the ?pj= parameter in the BIP21 URI. The actual step-by-step process can be in a jmclient module, something similar to what is seen in the test here. As noted in that test, we will additionally need to put in a few more checks on the sender side when they receive the payjoin-psbt. Agreed? The actual processing can just be linear, we don't even need to use async but it will be better to wrap it in a very simple twisted Deferred so that when it's run in the GUI, it is non-blocking. I can add that if necessary.

[kristapsk]

Yes, there may be an OS out there that we no longer support, I think we must simply advise people to either upgrade their OS, or their Python installation, or keep using JM 0.6.2.

I think we need to find out how big of a problem this is. If there are significant amount of users, to whom we would suggest installing older version of JM, it would be wise to create separate git branch and support it for some time, and backport most important fixes to it.

[AdamISZ]

I agree, but note 3.5 is reaching end-of-life in September of this year according to this.

[kristapsk]

3.5 is reaching end-of-life in September

Ok, that's good, then no need to support it longer than a few months in any case.

[AdamISZ]

I realised that my last comment here should be in this thread. As a follow up to that information, https://github.com/btcpayserver/btcpayserver/releases/tag/v1.0.4.3 fixes the receiver behaviour to match the sender nSequence.

[AdamISZ]

Sorry I meant to put this comment in this thread. It was an update on the latest progress.

[AdamISZ]

BIP number has been assigned, changed title to reflect.

[AdamISZ]

BIP78 Payjoin is now available after merge of #536 so closing.