Fix BIP 78 & BIP 174 Conflict: Keep input utxo data through input finalization

[DanGould]

According to the psbt Input Finalizer spec "All other data except the UTXO and unknown fields in the input key-value map should be cleared from the PSBT. The UTXO should be kept to allow Transaction Extractors to verify the final network serialized transaction." In Bip78, the receiver clears this data after it finalizes its inputs, even if the utxo belongs to the sender which will need that data.

I ran into a problem where LND's FinalizePsbt gRPC fails when this utxo data is missing. I see no good reason to remove this utxo information from the PSBT. I think LND's RPC should also succeed regardless of this data being present because it can look it up with the unsigned_tx's input outpoint (that's what bitcoind's finalizePsbt rpc does). Still I think LND's RPC is technically BIP-0174 spec-compliant while BIP-0078 seems not to be.

https://github.com/lightningnetwork/lnd/blob/cf9a9864cf253dbbcac5904d360bbbde763e1ebe/lnwallet/rpcwallet/rpcwallet.go#L270-L286
According to the psbt Input Finalizer spec "All other data except the UTXO and unknown fields in the input key-value map should be cleared from the PSBT. The UTXO should be kept to allow Transaction Extractors to verify the final network serialized transaction."

I ran into a problem where an LND acting as sender FinalizePsbt gRPC fails when sender utxo information is missing. I see no good reason to remove utxo information from the PSBT.

Edit Jun 2024: This problem also cropped up in sparrow wallet

---

tagging this issue in the rust-payjoin library

@nickfarrow @Kixunil @NicolasDorier

[murchandamus]

Seems reasonable, but needs an endorsement from the original BIP author, @NicolasDorier.

[DanGould]

@NicolasDorier This issue cropped itself up in Sparrow wallet too, where sparrow won't sign based solely on an outpoint but requires the utxo scripts to sign.

sparrowwallet/sparrow#1428

It would be great to resolve this as payjoin adoption is mounting.

[spacebear21]

The reference sender implementation should also be updated to remove this check:

// Verify that <code>non_witness_utxo</code> and <code>witness_utxo</code> are not specified.
if (proposedPSBTInput.NonWitnessUtxo != null || proposedPSBTInput.WitnessUtxo != null)
    throw new PayjoinSenderException("The receiver added non_witness_utxo or witness_utxo to one of our inputs");

The test vectors should likely be updated as well.

[craigraw]

+1

It would at least be good to understand why this information is removed from the original PSBT, which is technically in conflict with BIP174.

[DanGould]

My guess was that the spec was written this way out of convenience for the original NBitcoin / BTCPayServer implementation.

The BTCPayServer sender calls proposedPayjoin.SignAll. By excluding the utxo fields the signer would not have to do an exhaustive search for signable scriptPubKeys. I didn't find anywhere where including a foreign utxo would cause an error. But it looks like exhaustive search could make the signer take a long time to return.

[jonatack]

@NicolasDorier could you please weight in here?

[murchandamus]

Hey, has there been any movement on this PR? @NicolasDorier, I think it might be waiting for your review.