BIP78: Allow mixed inputs and clarify a few things

[Kixunil]

Disallowing mixed inputs was based on incorrect assumption that no wallet supports mixed inputs and thus mixed inputs imply PayJoin. However there are at least three wallets supporting mixed inputs. (Confirmed: Bitcoin Core, LND, Coinomi; I think I remember seeing Samourai doing mixed inputs too.) Thus it makes sense to enable mixed inputs. To avoid compatibility issues a grace period is suggested.

Additional clarifications were made:

• Disallow reciever removing output - such would break the reference implementation and seems better than changing the reference implementation.
• Forbid sender changing TXID in special case where no input is added.
• Confirm that amount is not mandatory.

We discussed this with @NicolasDorier in the past. (Finally found the time! :)) This may interest wallet developers: @nopara73 @Overtorment @rage-proof @RCasatta

[luke-jr]

@NicolasDorier

[DanGould]

LGTM. Even more software is supporting mixed spends or has plans to like bdk. Avoiding UIH 1 seems more important. Matching i/o sequence numbers probably needs a deeper look too.

Disallow reciever removing output - such would break the reference implementation and seems better than changing the reference implementation

What exactly would break in the reference implementation without this change?

[DanGould]

To stay completely backwards compatible without requiring a new version or grace period, a sender explicitly ignoring mixed input script check also use a request uri parameter like mixin=1 to explicitly allow it. A receiver having neither mixed input support nor an input of the same type could return the error. One supporting the update could return a Payjoin Proposal PSBT with mixed inputs.

edit: Bitcoin Core does occasionally mix input types, having done so even more often than when this PR was introduced https://github.com/bitcoin/bitcoin/pull/24584

[ghost]

Samourai doing mixed inputs too

It makes it easier to identify change address in samourai which is bad for privacy. Not sure what are the benefits of allowing mixed inputs in payjoin.

[AnthonyRonning]

I'm unsure of the implications by allowing mixed inputs and whether that could hurt privacy if so, but to chime in a little. Mutiny is a taproot address only wallet so interacting with current payjoin server implementations that aren't using taproot either might reduce the compatibility. Not sure how many have upgraded by now.

[Kixunil]

@TonyGiorgio when this BIP was first created the authors incorrectly thought that there are no wallets that mix inputs. If there truly weren't then mixing would be a clear giveaway that the transaction is PayJoin. However there actually are such wallets and thus mixing inputs doesn't reveal it's a PayJoin, it actually reveals it's not a PayJoin - thus it leaks data. So that incorrect restriction has to go.

As for Mutiny or any similar wallets, mixing inputs will reveal that the wallet is definitely not Mutiny but, after removing the restriction, doesn't reveal whether it's a PayJoin or not. If Mutiny implements mixed inputs in the future it'll get better.

[DanGould]

However there actually are such wallets and thus mixing inputs doesn't reveal it's a PayJoin, it actually reveals it's not a PayJoin - thus it leaks data. So that incorrect restriction has to go.

Kixunil's point is the main privacy consideration about this change.

This unintuitive result is the same as the consequence of lexicographical ordering. If only some wallets can produce a given transaction topology, even if that result is intended to make it uniform for the sake of privacy, it creates an identifiable fingerprint. For example, Trezor always orders outputs according to BIP-69 Lexicographical ordering, allowing analysts to conclude that any transaction not following lexicographical ordering must not have come from Trezor. This in fact REDUCES the potential ambiguity a transaction might have. See Ishaana's article on wallet fingerprinting

[murchandamus]

It seems to me that these changes would need sign-off from the original BIP author. Pinging @NicolasDorier for review.

[NicolasDorier]

There is one typo recivers, and there is a dubious way to calculate that the actual contribution is only paying for fee incurred by additional inputs.

additionalSize is set arbitrarily be the virtual size of the last input added by the receiver. If the receiver added mixed inputs, then this shouldn't be this. Either it should be properly calculated, or we should require the receiver to not use mixed inputs. I think that the simplification in implementation is well worth the additional restriction on the receiver.

@DanGould you can ping me on DM or btcpay chat to get that moving.

[NicolasDorier]

@DanGould keep it focus for this specific PR. This is difficult enough to review.

@Kixunil Can you also fix another thing?

There is references to an output variable. This should actually be originalOutput.