Enable mempoolfullrbf=1 by default #26305

pull ariard wants to merge 1 commits into bitcoin:master from ariard:2022-10-activate-fullrbf changing 2 files +17 −3
  1. ariard commented at 5:56 pm on October 13, 2022: member

    This is an alternative to #25600 in the deployment of full-rbf. After listening to feedback, rather to introduce a new service bit and automatic preferential peering to enable full-rbf transaction-relay paths among the interested peers, the default value of mempoolfullrbf is switched to 1. This change implies all the Bitcoin Core nodes will accept replacement of opt-out transactions at the next release (at the earliest 25.0). Rather than a gradual construction of full-rbf transaction-relay paths due to node operators activating mempoolfullrbf, turning on by default the setting offer more visibility to the zero-conf services node operators.

    Mailing list post coming soon to seek what has the preference between this deployment approach and the smoother #25600 approach among the development community and zero-conf services node operators.

    (One more approach could be to lock-in mempoolfullrbf=1 with some flag day activation if one release cycle of time is gauged is not enough or not predictible enough)

  2. Enable `mempoolfullrbf=1` by default 50230e85aa
  3. glozow added the label TX fees and policy on Oct 13, 2022
  4. glozow added the label Needs Conceptual Review on Oct 13, 2022
  5. petertodd commented at 3:22 pm on October 14, 2022: contributor

    Concept ACK

    Thanks for going to the trouble of writing #25600 But this is definitely the simpler approach.

  6. ghost commented at 3:46 pm on October 15, 2022: none

    Concept NACK

    There is no need to make it default at this point.

    Details: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/021010.html

  7. petertodd commented at 4:21 pm on October 15, 2022: contributor
    Releasing v0.24 with the mempoolfullrbf flag, and then releasing v0.25 with mempoolfullrbf=1 would be fine.
  8. fanquake added this to the milestone 24.0 on Oct 16, 2022
  9. DrahtBot commented at 12:05 pm on October 16, 2022: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #26438 (Remove mempoolfullrbf option by sdaftuar)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  10. michaelfolkson commented at 12:06 pm on October 17, 2022: contributor

    Concept ACK, Approach ACK for reasons stated here.

    There is a release cycle conversation (24.0 or 25.0) to be had but perhaps that can be done in an issue linked to this PR rather than on this PR.

    (I am assuming this goes ahead and the default is changed. I believe it should be and it seems many others do. But of course if the default isn’t changed these PRs and any associated issues are mute)

  11. achow101 commented at 2:32 pm on October 17, 2022: member
    Concept ACK for 25.0
  12. in src/kernel/mempool_options.h:24 in 50230e85aa 
    20@@ -21,7 +21,7 @@ static constexpr unsigned int DEFAULT_MAX_MEMPOOL_SIZE_MB{300};
21 /** Default for -mempoolexpiry, expiration time for mempool transactions in hours */
22 static constexpr unsigned int DEFAULT_MEMPOOL_EXPIRY_HOURS{336};
23 /** Default for -mempoolfullrbf, if the transaction replaceability signaling is ignored */
24-static constexpr bool DEFAULT_MEMPOOL_FULL_RBF{false};
25+static constexpr bool DEFAULT_MEMPOOL_FULL_RBF{true};
    maflcko commented at 3:33 pm on October 17, 2022:

    I wonder how much value there is in keeping the setting. If the default is true, a single miner turning it off (or even a substantial percentage of hashrate) isn’t going to prevent fullrbf on the network. All it does is that the miner will be missing out on fees and have their compact blocks propagate slower. Offering the setting to a non-mining node will also slow their compact blocks (which shouldn’t hurt), but it just seems like a false promise that they could achieve anything by toggling it.

    So my preference would be to just remove the setting here.

    jonatack commented at 9:58 pm on October 17, 2022:
    If or when we can drop the -mempoolfullrbf config arg, it seems that would allow a number of further simplifications in our codebase as well.
    ariard commented at 11:43 pm on October 17, 2022:
    I wonder if there is still value for 0conf operators who would like to implement first-seen transactions among a set of restricted/trusted mempools: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/021011.html I have no idea if such scheme, if implemented would be robust though I think it’s good to offer mechanism flexibility (If no one leveraged it after few releases we can still deprecate it ofc)
  13. glozow removed this from the milestone 24.0 on Oct 17, 2022
  14. glozow added this to the milestone 25.0 on Oct 17, 2022
  15. mzumsande commented at 5:49 pm on October 17, 2022: contributor

    Concept ACK

    CI fails, so functional tests still need a fix

  16. ariard commented at 11:50 pm on October 17, 2022: member
    Note to the reviewers see #26323, which is an alternative deployment approach achieving the same effect, while committing to a flag day activation, as suggested by the opening description of this PR. Personally, I’m fine with both approaches, though diverging on the activation parameters and merging timeline of #26323.
  17. ajtowns commented at 3:19 am on October 18, 2022: contributor
    I think you can replace this PR with a cherry-pick of the top commit of #26323 to fix the CI errors (well, changing the commit description would be smart). (EDIT: or at least, I think it does now)
  18. murchandamus commented at 7:51 pm on October 18, 2022: contributor
    Concept ACK
  19. instagibbs commented at 8:19 pm on October 20, 2022: member
    concept ACK, provided we collect data on which release this should make it into to allow businesses to transition.
  20. darosior commented at 8:49 am on October 25, 2022: member

    I’m in favour of switching the default rather than doing preferential peering. However i’m unsure about shipping it into 25.0.

    Ideally, i’d prefer if we only adapted the default to the behaviour of the network. That is, to only switch to true once we notice a substantial part of the network set it. If that doesn’t come we could eventually switch it anyway, but doing so immediately in 25.0 while there’s been reports of users still relying on first-seen-safe behaviour seems reckless to me.

  21. bitcoin deleted a comment on Oct 25, 2022
  22. ariard commented at 11:32 pm on October 25, 2022: member

    I’m in favour of switching the default rather than doing preferential peering. However i’m unsure about shipping it into 25.0.

    Considering the technical complexity of 0confs services/applications discussed on the mailing list, I think too 25.0 is likely to be too early. A 12/18 timeframe might be more adequate. Though ideally, we should still reach out to more 0confs service operators to collect more feedback.

  23. ariard commented at 3:22 am on November 7, 2022: member
    Closing this PR in the lack of clear consensus on mempoolfullrbf, and in the light of wider interrogations on Bitcoin Core transaction-relay design philosophy for numerous use-cases.
  24. ariard closed this on Nov 7, 2022
  25. fanquake removed this from the milestone 25.0 on Dec 12, 2022
  26. bitcoin locked this on Dec 12, 2023


ariard petertodd ghost DrahtBot michaelfolkson achow101 maflcko jonatack mzumsande ajtowns murchandamus instagibbs darosior

Labels
TX fees and policy Needs Conceptual Review

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: 2024-11-21 12:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me