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.

Sign up for GitHub

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

BIP78: Allow mixed inputs Redux #1605

Merged
merged 4 commits into from
Jan 14, 2025
+14 −45
Merged

BIP78: Allow mixed inputs Redux #1605

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
539fd85
BIP78: Allow mixed inputs
DanGould May 31, 2024
72d8bb0
BIP78: Clarify output substitution
DanGould May 31, 2024
bd01a26
BIP78: Doc `amount` parameter not required
DanGould May 31, 2024
f1ad918
Merge branch 'master' into bip78-mixed-inputs-ok
jonatack Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 14 additions & 45 deletions bip-0078.mediawiki
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,20 @@ The original PSBT MUST:
The original PSBT MAY:
* Have outputs unrelated to the payment for batching purpose.

The original PSBT SHOULD NOT:
* Include mixed input types until September 2024. Mixed inputs were previously completely disallowed so this gives some grace period for receivers to update.

The payjoin proposal MUST:
* Use all the inputs from the original PSBT.
* Use all the outputs which do not belong to the receiver from the original PSBT.
* Only finalize the inputs added by the receiver. (Referred later as <code>additional inputs</code>)
* Only fill the <code>witnessUTXO</code> or <code>nonWitnessUTXO</code> for the additional inputs.

The payjoin proposal MAY:
* Add, remove or modify the outputs belonging to the receiver.
* Add, or replace the outputs belonging to the receiver unless output substitution is disabled.

The payjoin proposal SHOULD NOT:
* Include mixed input types until September 2024. Mixed inputs were previously completely disallowed so this gives some grace period for senders to update.
Copy link
Member

Choose a reason for hiding this comment

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

@DanGould As a follow-up, do you think these dates here and on line 110 above ought to be updated to provide a post-merge grace period?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A client may still opt to disallow mixed inputs (or any payjoin proposal) by policy rather than by protocol like this one, so I don't think it's so critical that requires an explicit grace period after all, nor does the change in #1396. I think we can leave the grace period as-is and tackle the implementations one by one.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! I also see that you are reaching out to implementations at the moment 👍


The payjoin proposal MUST NOT:
* Shuffle the order of inputs or outputs, the additional outputs or additional inputs must be inserted at a random index.
Expand All @@ -125,6 +131,8 @@ This proposal is defining the following new [[bip-0021.mediawiki|BIP 21 URI]] pa
* <code>pj=</code>: Represents an http(s) endpoint which the sender can POST the original PSBT.
* <code>pjos=0</code>: Signal to the sender that they MUST disallow [[#output-substitution|payment output substitution]]. (See [[#unsecured-payjoin|Unsecured payjoin server]])

Note: the <code>amount</code> parameter is *not* required.

===<span id="optional-params"></span>Optional parameters===

When the payjoin sender posts the original PSBT to the receiver, he can optionally specify the following HTTP query string parameters:
Expand Down Expand Up @@ -215,34 +223,14 @@ To prevent this, the sender can agree to pay more fee so the receiver make sure

When a sender picks a specific fee rate, the sender expects the transaction to be confirmed after a specific amount of time. But if the receiver adds an input without bumping the fee of the transaction, the payjoin transaction fee rate will be lower, and thus, longer to confirm.

Our recommendation for <code>maxadditionalfeecontribution=</code> is <code>originalPSBTFeeRate * vsize(sender_input_type)</code>.

{| class="wikitable"
!sender_input_type
!vsize(sender_input_type)
|-
|P2WPKH
|68
|-
|P2PKH
|148
|-
|P2SH-P2WPKH
|91
|-
|P2TR
|58
|}


Our recommendation for <code>maxadditionalfeecontribution=</code> is <code>originalPSBTFeeRate * 110</code>.

===Receiver's original PSBT checklist===

The receiver needs to do some check on the original PSBT before proceeding:

* Non-interactive receivers (like a payment processor) need to check that the original PSBT is broadcastable. <code>*</code>
* If the sender included inputs in the original PSBT owned by the receiver, the receiver must either return error <code>original-psbt-rejected</code> or make sure they do not sign those inputs in the payjoin proposal.
* If the sender's inputs are all from the same scriptPubKey type, the receiver must match the same type. If the receiver can't match the type, they must return error <code>unavailable</code>.
* Make sure that the inputs included in the original transaction have never been seen before.
** This prevents [[#probing-attack|probing attacks]].
** This prevents reentrant payjoin, where a sender attempts to use payjoin transaction as a new original transaction for a new payjoin.
Expand All @@ -267,7 +255,6 @@ The sender should check the payjoin proposal before signing it to prevent a mali
*** Verify the PSBT input is finalized
*** Verify that <code>non_witness_utxo</code> or <code>witness_utxo</code> are filled in.
** Verify that the payjoin proposal inputs all specify the same sequence value.
** Verify that the payjoin proposal did not introduce mixed input's type.
** Verify that all of sender's inputs from the original PSBT are in the proposal.
* For each output in the proposal:
** Verify that no keypaths are in the PSBT output
Expand Down Expand Up @@ -343,7 +330,7 @@ On top of this the receiver can poison analysis by randomly faking a round amoun

===<span id="output-substitution"></span>Payment output substitution===

Unless disallowed by the sender explicitly via <code>disableoutputsubstitution=true</code> or by the BIP21 URL via the query parameter <code>pjos=0</code>, the receiver is free to decrease the amount, remove, or change the scriptPubKey output paying to himself.
Unless disallowed by the sender explicitly via <code>disableoutputsubstitution=true</code> or by the BIP21 URL via the query parameter <code>pjos=0</code>, the receiver is free to decrease the amount or change the scriptPubKey output paying to himself.
Note that if payment output substitution is disallowed, the reveiver can still increase the amount of the output. (See [[#reference-impl|the reference implementation]])

For example, if the sender's scriptPubKey type is P2WPKH while the receiver's payment output in the original PSBT is P2SH, then the receiver can substitute the payment output to be P2WPKH to match the sender's scriptPubKey type.
Expand Down Expand Up @@ -428,7 +415,6 @@ public async Task<PSBT> RequestPayjoin(
var endpoint = bip21.ExtractPayjointEndpoint();
if (signedPSBT.IsAllFinalized())
throw new InvalidOperationException("The original PSBT should not be finalized.");
ScriptPubKeyType inputScriptType = wallet.ScriptPubKeyType();
PSBTOutput feePSBTOutput = null;

bool allowOutputSubstitution = !optionalParameters.DisableOutputSubstitution;
Expand Down Expand Up @@ -516,9 +502,6 @@ public async Task<PSBT> RequestPayjoin(
if (proposedPSBTInput.NonWitnessUtxo == null && proposedPSBTInput.WitnessUtxo == null)
throw new PayjoinSenderException("The receiver did not specify non_witness_utxo or witness_utxo for one of their inputs");
sequences.Add(proposedTxIn.Sequence);
// Verify that the payjoin proposal did not introduced mixed inputs' type.
if (inputScriptType != proposedPSBTInput.GetInputScriptPubKeyType())
throw new PayjoinSenderException("Mixed input type detected in the proposal");
}
}

Expand Down Expand Up @@ -550,7 +533,7 @@ public async Task<PSBT> RequestPayjoin(
if (isOriginalOutput || substitutedOutput)
{
originalOutputs.Dequeue();
if (output.OriginalTxOut == feeOutput)
if (originalOutput.OriginalTxOut == feeOutput)
{
var actualContribution = feeOutput.Value - proposedPSBTOutput.Value;
// The amount that was subtracted from the output's value is less than or equal to maxadditionalfeecontribution
Expand All @@ -560,8 +543,9 @@ public async Task<PSBT> RequestPayjoin(
if (actualContribution > additionalFee)
throw new PayjoinSenderException("The actual contribution is not only paying fee");
// Make sure the actual contribution is only paying for fee incurred by additional inputs
// This assumes an additional input can be up to 110 bytes.
int additionalInputsCount = proposalGlobalTx.Inputs.Count - originalGlobalTx.Inputs.Count;
if (actualContribution > originalFeeRate * GetVirtualSize(inputScriptType) * additionalInputsCount)
if (actualContribution > originalFeeRate * 110 * additionalInputsCount)
throw new PayjoinSenderException("The actual contribution is not only paying for additional inputs");
}
else if (allowOutputSubstitution && output.OriginalTxOut.ScriptPubKey == paymentScriptPubKey)
Expand Down Expand Up @@ -596,21 +580,6 @@ public async Task<PSBT> RequestPayjoin(
return proposal;
}

int GetVirtualSize(ScriptPubKeyType? scriptPubKeyType)
{
switch (scriptPubKeyType)
{
case ScriptPubKeyType.Legacy:
return 148;
case ScriptPubKeyType.Segwit:
return 68;
case ScriptPubKeyType.SegwitP2SH:
return 91;
default:
return 110;
}
}

// Finalize the signedPSBT and remove confidential information
PSBT CreateOriginalPSBT(PSBT signedPSBT)
{
Expand Down
Loading