BIP78: Allow mixed inputs Redux #1605

pull DanGould wants to merge 3 commits into bitcoin:master from DanGould:bip78-mixed-inputs-ok changing 1 files +14 −45
  1. DanGould commented at 3:59 pm on May 31, 2024: contributor

    This is a follow up to https://github.com/bitcoin/bips/pull/1244/ that

    • separates concerns into individual commits
    • corrects fee verification for a proposal having mixed script inputs while ensuring additional fee indeed paid for input

    I know @kixunil is extremely busy to update these days, so I took the liberty to organize the changes we’ve discussed according to your wants here. @NicolasDorier I hope these changes satisfy your desire for them to be small and problem-focused while maintaining the spirit of the original.


    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) Thus it makes sense to enable mixed inputs to avoid a payjoin-specific fingerptint. To avoid compatibility issues a grace period is suggested.

  2. DanGould force-pushed on May 31, 2024
  3. DanGould force-pushed on May 31, 2024
  4. DanGould force-pushed on May 31, 2024
  5. DanGould force-pushed on May 31, 2024
  6. DanGould force-pushed on May 31, 2024
  7. murchandamus commented at 5:33 pm on May 31, 2024: contributor
    It seems to me that you are proposing a number of changes to BIP 78 and the biggest point of friction seems to be to get sign-off from that BIP’s champion. Have you considered alternatively proposing your own variant of BIP 78 with the changes you would like to see and building BIP 77 on top of that?
  8. DanGould commented at 7:02 pm on May 31, 2024: contributor
    Are you suggesting I compile the changes into an addendum document, or something else?
  9. murchandamus commented at 8:00 pm on May 31, 2024: contributor
    Sorry, I thought this was a PR with further changes, in addition to your prior attempt to modify BIP 78. At second glance I realize now that this is a second variant with a reduced scope to the prior. Nevermind, please ignore and carry on.
  10. murchandamus added the label Proposed BIP modification on Jun 3, 2024
  11. murchandamus added the label Pending acceptance on Jun 3, 2024
  12. in bip-0078.mediawiki:122 in a682fe6b0e outdated
    118 The payjoin proposal MAY:
    119-* Add, remove or modify the outputs belonging to the receiver.
    120+* Add, or replace the outputs belonging to the receiver unless output substitution is disabled.
    121+
    122+The payjoin proposal SHOULD NOT:
    123+* Include mixed input types until May 2024. Mixed inputs were previously completely disallowed so this gives some grace period for senders to update.
    


    Kixunil commented at 3:17 pm on June 10, 2024:
    This says May, the other says September.

    DanGould commented at 4:11 pm on June 10, 2024:
    fixed
  13. Kixunil commented at 3:22 pm on June 10, 2024: none

    I’m slowly getting some time now but feel free to go ahead with this. I’d still like to cover the edge case I mentioned because it affects some software in the wild but that can be done in a separate PR.

    And thanks @DanGould for picking this up!

  14. DanGould force-pushed on Jun 10, 2024
  15. DanGould commented at 4:20 pm on June 10, 2024: contributor

    I’d still like to cover the edge case I mentioned because it affects some software in the wild but that can be done in a separate PR.

    To be crystal clear, you’re talking about transaction malleability with regard to output substitution. I think that’s outside the scope of this pr but also worthy of amendment. That discussion is: https://github.com/bitcoin/bips/pull/1244/files#r1633516491

  16. NicolasDorier commented at 0:42 am on June 25, 2024: contributor

    It means that only a single input can be covered by the contribution and the rest should be at the cost of the receiver. Not a big deal, as I welcome that it makes things a little bit simpler while still breaking the heuristic.

    1. GetVirtualSize can be removed as it is not used anymore.
    2. Can you rename if (output.OriginalTxOut == feeOutput) to if (originalOutput.OriginalTxOut == feeOutput)
  17. NicolasDorier commented at 3:11 am on June 25, 2024: contributor

    Ok @DanGould can you pickup this commit please: https://github.com/NicolasDorier/bips/commit/1cc129ed2b8b9027a1123d7cd1f55509c92c9731

    This basically just hardcode the maximum amount to pay for any input.

  18. 5twelve approved
  19. jonatack added the label PR Author action required on Jul 1, 2024
  20. BIP78: Allow mixed inputs
    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) Thus it makes sense to enable
    mixed inputs to avoid a payjoin-specific fingerptint. To avoid
    compatibility issues a grace period is suggested.
    
    Co-authored-by: Martin Habovstiak <martin.habovstiak@gmail.com>
    539fd85498
  21. BIP78: Clarify output substitution
    The original text is ambiguous to allowing transaction cut-through
    or not. Transaction cut-through enables savings by posting multiple
    transaction intents through a single 2-party payjoin and is used
    in practice in payjoins today. Let's explicitly allow it in the text.
    
    Co-authored-by: Martin Habovstiak <martin.habovstiak@gmail.com>
    72d8bb04b8
  22. BIP78: Doc `amount` parameter not required
    It's an optional parameter in BIP 21 Bitcoin URIs, but it doesn't hurt
    to make it explicit.
    
    Co-authored-by: Martin Habovstiak <martin.habovstiak@gmail.com>
    bd01a269e5
  23. DanGould force-pushed on Jul 8, 2024
  24. murchandamus removed the label PR Author action required on Jul 17, 2024
  25. murchandamus commented at 2:26 pm on July 17, 2024: contributor
    @NicolasDorier: It looks like the commit you requested was added. Could you please review this PR and state whether you support or reject these changes?

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bips. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-09 07:10 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me