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

1. 
   DanGould commented at 6:04 pm on December 20, 2022: contributor

   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

2. 
   murchandamus commented at 6:10 pm on May 1, 2024: contributor

   Seems reasonable, but needs an endorsement from the original BIP author, @NicolasDorier.
3. murchandamus added the label Pending acceptance on May 8, 2024
4. murchandamus added the label Proposed BIP modification on May 8, 2024
5. 
   DanGould commented at 2:32 pm on May 31, 2024: contributor

   @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.

   https://github.com/sparrowwallet/sparrow/issues/1428

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

6. 
   spacebear21 commented at 5:11 am on June 5, 2024: none

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

   0// Verify that <code>non_witness_utxo</code> and <code>witness_utxo</code> are not specified.
   1if (proposedPSBTInput.NonWitnessUtxo != null || proposedPSBTInput.WitnessUtxo != null)
   2    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.

7. 
   craigraw commented at 9:24 am on June 10, 2024: contributor

   +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.

8. 
   DanGould commented at 5:02 pm on June 10, 2024: contributor

   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.

9. Keep input utxo data through input finalization

   The reference sender implementation and \`payjoin proposal\` test vectors
   are adjusted accordingly.
   
   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.

   e4267374fb
10. DanGould force-pushed on Jun 10, 2024
11. 
    jonatack commented at 0:14 am on June 21, 2024: member

    @NicolasDorier could you please weight in here?
12. 5twelve approved
13. 
    murchandamus commented at 8:27 pm on November 14, 2024: contributor

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

Contributors
DanGould murchandamus spacebear21 craigraw jonatack 5twelve

Labels
Proposed BIP modification Pending acceptance