Skip to content

BIP 78: Add payjoin proposal#923

Merged
luke-jr merged 45 commits into
bitcoin:masterfrom
NicolasDorier:pj
Jun 25, 2020
Merged

BIP 78: Add payjoin proposal#923
luke-jr merged 45 commits into
bitcoin:masterfrom
NicolasDorier:pj

Conversation

@NicolasDorier

Copy link
Copy Markdown
Contributor

Previous discussion at NicolasDorier#3

Submitting to mailing list.

@NicolasDorier

NicolasDorier commented May 17, 2020

Copy link
Copy Markdown
Contributor Author

Ping @instagibbs I added some commit clarifying what you sent me by mail.

About your point:

There is no global "minimum relay fee policy". My node my have 5 sat/byte floor, yours 1. The receiver may think something is fine but sender can't actually get it into its own mempool. Seems pertinent to have the sender choose feerate and receiver adhere to this up to the maxfeebumpcontribution if specified.

While true in theory, this is not true in practice. The network adopt the default of Bitcoin core every single time. (Got interesting problems with colored coins back in the days because of this)

I suggest we add an additional parameter so the sender can specify what is the minfeerate for him?

I can see a problem happening in case the sender's wallet stop being able to make payjoin with receivers because receivers bump the fee rate not knowing the new network's default.

the other case is where the receiver piggy backs and accomplishes another goal like payouts. Seems to me I'm that case the receiver should pay for everything aside from maybe the original deporting input? The receiver is getting a free ride from the input and possibly denying the sender inclusion in their analysis cluster. Basically just doing cut-through.

The receiver is bounded by the additionalfeecontribution. So he is free to add more than the fee contribution, but he would have to pay from his own pocket.

Now that is true that he can make the sender pay for a few output, even if the receiver has an upper limit. That said, I think this is quite difficult to calculate properly.
For example, the receiver may have changed his payment output from P2WPKH to P2SH which would change the outputs length (top of my head calculation +2 bytes), but the sender should pay for it.

Another solution is to drop address substitution completely. But I wanted to keep doubt in the analyst calculation that their heuristic may be poisoned.

@instagibbs

instagibbs commented May 17, 2020 via email

Copy link
Copy Markdown
Member

@NicolasDorier

Copy link
Copy Markdown
Contributor Author

Indeed. And BTCPayServer is using bitcoin's core one we get from RPC... So I think the best solution is to let the sender define it, and if not defined, use the receiver's one?

@instagibbs

instagibbs commented May 17, 2020 via email

Copy link
Copy Markdown
Member

@NicolasDorier

NicolasDorier commented May 17, 2020

Copy link
Copy Markdown
Contributor Author

I was thinking though: If the feerate of the sender is lower than the minfeerate of the receiver, the payjoin will still fail because the receiver is using testmempoolaccept before doing the proposal. This is a corner case, so I think we can safely ignore that.

@instagibbs

instagibbs commented May 17, 2020 via email

Copy link
Copy Markdown
Member

@NicolasDorier

NicolasDorier commented May 17, 2020

Copy link
Copy Markdown
Contributor Author

@instagibbs check 233c094

@NicolasDorier

Copy link
Copy Markdown
Contributor Author

I renamed the optional parameter as your suggestion, I also think "bump" should not be overloaded meaning.

Comment thread bip-xxxx.mediawiki Outdated
|unavailable
|The payjoin endpoint is not available for now.
|-
|out-of-utxos

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest removing this; functionally, to the sender, "unavailable" and "out of utxos" are the same, but the latter is more explicitly leaking information to the sender about the receiver's wallet. The same might apply to not-enough-money, I'm not sure.

( note that in TLS the sending of error messages from a server like "invalid padding" led to actual attacks on security, because it was giving the client information about how the failure occurred. Clearly there is nowhere near as much danger here from leaking a bit or two of info about the wallet, and equally clearly some error messages can be very useful. So this is only a suggestion.)

The error messages which are telling the client/sender that some aspect of their request (e.g. psbt not finalized) is invalid are clearly not a problem.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with out-of-utxos, but for not-enough-money, this is an error that can be fixed by the sender, so he need to be aware of it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Because of the change output size being too small? So if it's a function of the sender's proposal, then for sure that is fine, agreed.

Comment thread bip-xxxx.mediawiki Outdated
* Check that any inputs added by the receiver are finalized.
* Check that the transaction version, and nLockTime are unchanged.
* Check that the sender's inputs' sequence numbers are unchanged.
* If the sender's inputs' sequence numbers the homogenous, check that the receiver's contributed inputs match those.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/the homogeneous/are homogeneous/ but to be honest I think "homogeneous" is an unnecessarily fancy word choice here, to be ultra clear I prefer "If the sender's inputs' sequence numbers are all the same, ..."

Comment thread bip-xxxx.mediawiki Outdated
In a payjoin payment, the following steps happen:

* The receiver of the payment, presents a [[bip-021.mediawiki|BIP 21 URI]] to the sender with a parameter <code>pj</code> describing an https (or http if it is a Tor hidden service) link to the payjoin endpoint.
* The sender creates a signed, finalized PSBT with witness UTXO or previous transactions of the inputs. We call this PSBT the <code>original</code>.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand from this sentence (or previous transactions) that this BIP is not requiring segwit-only inputs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the receiver in BTCPayServer does not support it, there is no reason to rule it out. We want to give as much freedom as possible so blockchain analyst can't assume that all p2pkh are not payjoin.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

Comment thread bip-xxxx.mediawiki Outdated

In a payjoin payment, the following steps happen:

* The receiver of the payment, presents a [[bip-021.mediawiki|BIP 21 URI]] to the sender with a parameter <code>pj</code> describing an https (or http if it is a Tor hidden service) link to the payjoin endpoint.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some "out there" thoughts. What if the pj key is not strictly defined as an "http(s)" endpoint but as "an instruction of establishing a communication with the receiver's endpoint". This would enable receiver scenarios of devices not having a reachable tor or http endpoint such as using QR codes, Bluetooth, NFC, etc

QR Code Payjoin

  • Receiver generates BIP21 payment request with a payjoin flag: bitcoin:abc?amount=1&pj=qrcode. Sender shows a QR code with bip21
  • Sender scans BIP21 and recognizes qrcode pj flag. Sender generates an original PSBT and encodes it in qr code
  • Receiver scans QR code, generates ne PJ PSBT, shows it in QR
  • Sender scans, signs, broadcasts

Bluetooth Payjoin

  • Receiver generates BIP21 payment request with a payjoin flag: bitcoin:abc?amount=1&pj=bluetooth:devicefingerprint. Sender shows a QR code with bip2 ( or BIP21 is transmitted through bluetoooth on an already establish connection)
  • Sender scans BIP21 and recognizes bluetoothpj flag. Sender scans bluetooth devices for identifier, connects and sends original psbt
  • Receiver generates new PJ PSBT, and sends it back to connected device
  • Sender signs, broadcasts

NFC Payjoin

  • Receiver generates BIP21 payment request with a payjoin flag: bitcoin:abc?amount=1&pj=nfc. Sender shows a QR code with bip21 (or sender taps device with receiver's nfc device)
  • Sender scans BIP21 and recognizes NFCpj flag. Sender scans bluetooth devices for identifier, connects and sends original psbt
  • Receiver generates new PJ PSBT, and sends it back to connected device with NFC tap
  • Sender signs, broadcasts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some "out there" thoughts. What if the pj key is not strictly defined as an "http(s)" endpoint but as "an instruction of establishing a communication with the receiver's endpoint". This would enable receiver scenarios of devices not having a reachable tor or http endpoint such as using QR codes, Bluetooth, NFC, etc

But I thought the point of line https://github.com/bitcoin/bips/pull/923/files#diff-bab55ce4db24c444e852baf3d7ddfefaR99 was to state that https/onion are examples of urls, and another scheme could be put there? (i.e. I thought the doc already encompassed this eventuality like bluetooth etc)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does. But I think in the context of the sentence, this was not clear.

I relaxed the authenticated channel because I think it is safe as long as no address substitution is allowed... what do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I saw. It makes sense but I probably wouldn't add that to the doc. If people have a way to be secure without following the recommendation, they can, but that would be like a custom implementation, in my view it's better not to make it "official" that transmitting over non-encrypted-and-authenticated channels is "supported" somehow as part of the publically-consensus protocol for payjoin.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair point, will not mention.

@NicolasDorier

Copy link
Copy Markdown
Contributor Author

@Kukks @instagibbs I added 5a337c6 to specify that the receiver should not free ride on the sender's fee for batching.

@NicolasDorier

Copy link
Copy Markdown
Contributor Author

I relaxed the endpoint requirement.

Also, authorizing unauthenticated channel, because the payment output can effectively be used to make sure that the sender is sending to the right person, even over unauthenticated channel.

@RHavar

RHavar commented Jun 19, 2020

Copy link
Copy Markdown
Contributor

@NicolasDorier I agree with you. I was only asking because of @SomberNight's comment, which made it sound like such a restriction currently existed

@NicolasDorier

Copy link
Copy Markdown
Contributor Author

@RHavar I added pjos, you seems having strong plan about it and it is not like it is difficult to implement. Please review.

@RHavar RHavar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet. Thanks. I am happy with the BIP. Might want to consider #923 (comment) but don't see it a big deal either way

Comment thread bip-0078.mediawiki Outdated
Comment thread bip-0078.mediawiki Outdated
@NicolasDorier

Copy link
Copy Markdown
Contributor Author

I think it is a good idea to let output to be increased. Please take a look at the implementation reference as well.

@NicolasDorier

NicolasDorier commented Jun 19, 2020

Copy link
Copy Markdown
Contributor Author

@RHavar @SomberNight I think it is a good idea to prevent the receiver from pocketing fees. While it is possible via a good value of minFeeRate, it should actually be enforced at protocol level.

I was thinking we could just ignore the case where an output become smaller because of substitution, but that may show up if the sender's fee are round. :/

The check need to be simple, thinking about it...

@NicolasDorier

NicolasDorier commented Jun 19, 2020

Copy link
Copy Markdown
Contributor Author

What about this: Allow the absolute fee to fall as much as the size of the payment output at the original transaction fee rate? (As in the best case scenario, receiver might decide to just completely remove his output)

I would like to make it easy to review and check. That's not perfect, but good enough.

@NicolasDorier

NicolasDorier commented Jun 19, 2020

Copy link
Copy Markdown
Contributor Author

@RHavar @SomberNight thinking about it, it is not worth the pain. In case of output substitution resulting in smaller output, the receiver should just give the money back to the sender.
I will add a condition: No drop in absolute fee.

@NicolasDorier

Copy link
Copy Markdown
Contributor Author

please review.

Comment thread bip-0078.mediawiki Outdated
When Alice pays Bob, if Alice is using P2SH but Bob's deposit address is P2WPKH, the heuristic would assume that the P2SH output is the change address of Alice.
This is now however a broken assumption, as the payjoin receiver has the freedom to mislead analytics by purposefully changing the invoice's address in the payjoin transaction.

Alternatively, if the original address of Bob is P2WPKH and Alice's address is also P2WPKH, Bob can change the receiving address in the payjoin to P2SH. The heuristic would wrongfully identify the payjoin's receiving address as the change address of the transaction.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still hasn't been resolved, so I am reposting my original comment:

This needs clarification. What script type is Alice using for the input? If she is using a P2WPKH input, then changing the receiving address in the PayJoin to P2SH would actually cause the heuristic to correctly identify the receiving address, because it's type would differ from the input type. So is the assumption that Alice is using a P2SH input and a P2WPKH change address?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely removing, you are right it did not made any sense.

Comment thread bip-0078.mediawiki Outdated
Comment thread bip-0078.mediawiki Outdated
Comment thread bip-0078.mediawiki Outdated
Comment thread bip-0078.mediawiki Outdated
Comment thread bip-0078.mediawiki Outdated
Comment thread bip-0078.mediawiki Outdated
** Verify that no keypaths is in the PSBT output
** If the output is the [[#fee-output|fee output]]:
*** The amount that was substracted from the output's value is less or equal to <code>maxadditionalfeecontribution</code>. Let's call this amount <code>actual contribution</code>.
*** Make sure the actual contribution is only paying fee: The <code>actual contribution</code> is less or equals to the difference of absolute fee between the payjoin proposal and the original PSBT.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/less or equals/less than or equal/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RHavar btw this is the line that would forbid the absolute fee going down explicitly.

@NicolasDorier NicolasDorier Jun 22, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it does not. The receiver could drop the fee from say 40 sat/vbyte to 10 sat/vbyte by changing his own input, pocketing the difference. The actual contribution would still be 0, which is fine for the sender.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see what your comment has to do about absolute fee of the original psbt going down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.