Be consistent in using "opt_into_rbf" parameter for Opt-In RBF #10745

pull luke-jr wants to merge 1 commits into bitcoin:master from luke-jr:opt_in_rbf-param changing 6 files +22 −14
  1. luke-jr commented at 2:32 AM on July 5, 2017: member

    Essentially just #10698 done right.

    Also fixes missing param name for createrawtransaction.

  2. fanquake added the label RPC/REST/ZMQ on Jul 5, 2017
  3. fanquake added the label Wallet on Jul 5, 2017
  4. practicalswift commented at 8:17 AM on July 5, 2017: contributor

    Concept ACK 3187d4ed98c62a4a094d14c6df8d52dad03c7972

  5. Be consistent in using "opt_into_rbf" parameter for Opt-In RBF 34066944a9
  6. luke-jr force-pushed on Jul 5, 2017
  7. jnewbery commented at 9:34 AM on July 5, 2017: member

    I prefer #10698. This implementation causes an unnecessary API break for bumpfee between 0.14.2 and 0.15.0 (replaceable option becomes opt-into-rbf).

    Your feedback in #10698 was that we shouldn't change the argument names because it'd break compatibility with a derived project. This implementation is changing those names anyway, so it seems like that's no longer an argument against #10698.

  8. luke-jr commented at 10:01 AM on July 5, 2017: member

    @jnewbery This implementation is completely compatible (eg, for bumpfee).

  9. morcos commented at 10:30 AM on July 5, 2017: member

    Also prefer #10698. Please just introduce your compatible argument names in Knots and let's stop bike shedding this and get back to doing real work.

  10. luke-jr commented at 10:55 AM on July 5, 2017: member

    Yes, please stop bike shedding by trying to make this incompatible for no reason whatsoever. The other day you even said you preferred opt_into_rbf...

  11. morcos commented at 1:20 PM on July 5, 2017: member

    I actually said I liked opt_in_rbf but that's besides the point. What I most like is not changing the name now from something that has already been released (replaceable) and is perfectly good.

    Tying up this much time arguing about the names is ridiculous. I don't really care what name we use, but seems like everyone other than you wants to stick with replaceable and I have actual meaningful PR's open that could benefit from just having a decision and moving forward because they add the option to other RPC's.

  12. laanwj commented at 6:33 PM on July 6, 2017: member

    Sigh, agree with @morcos here, do we really need a competing PR for this?

    We have been over this: replaceable is already in 0.14, so renaming them all to opt_in_rbf (which is only used in newly introduced interfaces for 0.15) is a more serious interface break than renaming them all to replaceable. Both result in consistency of the interface.

  13. luke-jr commented at 8:44 PM on July 6, 2017: member

    This is not an interface break at all. It is completely backward compatible.

  14. laanwj commented at 2:25 PM on July 10, 2017: member

    Closing as #10698 was merged

  15. laanwj closed this on Jul 10, 2017

  16. luke-jr commented at 9:04 PM on July 10, 2017: member

    This is still needed, even if that means rebasing it. Please reopen.

  17. DrahtBot locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 15:15 UTC

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