Disable RBF Rule 2 #30964

pull arik-so wants to merge 3 commits into bitcoin:master from arik-so:disable-rbf-rule-2 changing 6 files +12 −80
  1. arik-so commented at 9:01 pm on September 24, 2024: none

    One of the current RBF acceptance criteria is the prohibition of new unconfirmed inputs in the replacement transaction. That means that all replacement inputs must either be confirmed, or have unconfirmed spends that would get evicted.

    However, that limitation is trivially circumvented. If one wanted to spend an unconfirmed UTXO in the replacement transaction, all one would need to do is create a temporary, independent low-fee-rate transaction that spends that input, and then simply add that transaction to the replacement’s eviction set. @glozow has documented that here back in early 2022, and I imagine there’s other literature out there documenting this same phenomenon.

    One interesting thing to note with this approach of circumventing rule 2 is that the broadcasting of the temporary transaction and the broadcasting of the actually intended replacement transaction ought to be staggered, lest some nodes see only the final replacement transaction and reject it. I think it’s fair to say that that makes the propagation somewhat nondeterministic. Given that any motivated actor can trivially craft such a dummy transaction and then simply wait 30 seconds (maybe less?) before broadcasting the replacement, removing the pretense of rule 2 would reduce network traffic for this scenario.

    In light of all the work on the cluster mempool that’s been ongoing recently, a question for this PR might be: “why now?” My answer would be that due to the easy circumvention of rule 2, and it therefore arguably not being a “real” rule, removing it is somewhat independent of future, even near-term, mempool policy improvements.

    All that being said, the basis of this PR might be due to a glaring misunderstanding on my part of how bitcoin works, in which case feel free to close it, and my apologies for the noise.

  2. Stop enforcing RBF rule 2
    Given that unconfirmed inputs can be added to an evicting transaction
    by creating a temporary low-fee-rate spending transaction, and then
    simply evicting that transaction, too, we may as well simplify the
    logic and do away with RBF rule 2 altogether.
    be1a632e92
  3. Remove unused HasNoNewUnconfirmed method 6313bc1dd6
  4. Update documentation to reflect the removal of RBF rule 2 645d9e4001
  5. DrahtBot commented at 9:01 pm on September 24, 2024: contributor

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

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #31122 (cluster mempool: Implement changeset interface for mempool by sdaftuar)
    • #28121 (include verbose “reject-details” field in testmempoolaccept response by pinheadmz)

    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.

  6. instagibbs commented at 9:40 pm on September 24, 2024: member
    here’s a link to the first(?) discussion of this rule 2 evasion technique at least I’m aware of: #23121 (comment)
  7. sdaftuar commented at 5:46 pm on September 25, 2024: member

    You’re right that rule 2 is broken. The issue is that our replacement rules are broken in many ways, and if we simply remove rule 2, the result would also be broken. The motivation for rule 2 was the idea that by introducing a new unconfirmed parent, the effective feerate at which the replacement transaction being validated would be mined might be much lower than the transactions it is evicting (say, if the new unconfirmed parent had a very low feerate). So by simply removing the rule, I think we’d be exacerbating the incentive compatibility problems that exist with the current set of rules, by making it easier for a transaction with low mining score to evict transactions with high mining scores.

    My view is that we should indeed scrap these broken rules, but we should do so in conjunction with fixing the fundamental issues. The cluster mempool proposal is designed to make it so that whenever a replacement happens, the result will always be that the mempool is “better” (higher feerate at every size, taking CPFP-effects into account). If there is another proposal for fixing our RBF rules in a comprehensive way so they are not so broken, then that would be great to evaluate, but I don’t think it makes sense to pursue one-off changes that aren’t trying to improve the underlying situation.

  8. instagibbs commented at 10:06 pm on September 25, 2024: member
    @sdaftuar to restate a bit more simply: Things are broken whether or not we remove this, but removing this rule by itself would likely make incentive incompatible RBFs more common in the average or otherwise benign case?
  9. sdaftuar commented at 2:00 pm on September 26, 2024: member

    @instagibbs Sure, I’d agree with that.

    Another way to think about it: let’s say we scrapped rule 2 as proposed here. The resulting set of rules would have their own logical inconsistencies, specifically rule 6:

    The replacement transaction’s feerate is greater than the feerates of all directly conflicting transactions.

    Specifically, a large transaction that had lower feerate than one of its direct conflicts could be split into two – a parent with super low feerate, and a child with a sufficiently larger feerate – and then the transaction could be accepted.

    So at that point, by the same logic, someone might propose that we just scrap rule 6 too. Yet rule 6 is at the heart of what we’re actually trying to accomplish, and so even if it’s broken I don’t think we should scrap it either, until we have a better system that we can move to.

  10. ariard commented at 6:17 pm on September 26, 2024: contributor

    Yes the rbf rule 2 bypass is an old known issue and yes the rbf rules are fairly broken.

    For bitcoin clients fee-bumping chain of transactions, they should ensure to never spent from unconfirmed inputs.

  11. instagibbs commented at 9:05 pm on September 26, 2024: member
    @sdaftuar I think the same reasoning I gave is just being applied recursively, which is fine :+1:
  12. glozow commented at 9:36 pm on September 26, 2024: member

    Tend to agree with the “let’s wait for cluster mempool, which is currently our only proper solution to the problem” approach.

    My reasoning for #23121 was also that (1) rule 2 doesn’t actually stop people from adding “new” unconfirmed inputs, and (2) it doesn’t do its job within the RBF rules of helping us check that replacements have a better mining score / would confirm faster. I think reason (2) is actually much more important and complicated. I’m not convinced that the removal would be an improvement, or that any hotfix (like the ancestor score rule added in #23121) would be much better.

  13. arik-so commented at 0:20 am on September 27, 2024: none

    Thanks so much for the responses so far! @sdaftuar, I’m not sure I fully follow your point regarding rule 6. Currently, can’t the dummy transaction one creates to circumvent rule 2 simply have a really low fee rate? That would basically also create a situation akin to rule 2 not existing?

    But one thing I’d be curious to hear some thoughts on is specifically the potential mempool bifurcation that circumventing rule 2 would create (taking into account that there obviously isn’t such a thing as “the mempool”).

    All that being said, I really appreciate all the answers as to why we shouldn’t simply scrap rule 2 right now instead of waiting for cluster mempool!

  14. sdaftuar commented at 2:26 pm on September 27, 2024: member

    I’m not sure I fully follow your point regarding rule 6. Currently, can’t the dummy transaction one creates to circumvent rule 2 simply have a really low fee rate? That would basically also create a situation akin to rule 2 not existing?

    Yes, that’s true, and I think this is what @glozow was getting at as well:

    (2) it doesn’t do its job within the RBF rules of helping us check that replacements have a better mining score / would confirm faster

    My point is merely that if we look at any given rule on its own, it’s not so hard to come up with an example of how to bypass the rule or its intent. But this is a logical consequence of the overall system being incoherent. So rather than ask the question, “can we remove rule X because it doesn’t do its job anyway”, I think we should instead ask “what should the full set of replacement rules be”? (And I think the most satisfactory answer to that question at the moment is the feerate-diagram-comparison that is proposed as part of #28676.)

    But one thing I’d be curious to hear some thoughts on is specifically the potential mempool bifurcation that circumventing rule 2 would create (taking into account that there obviously isn’t such a thing as “the mempool”).

    Not sure exactly what your question here is. I don’t think having network mempools be out of sync is a good thing, but I think of it as a second order effect compared to the replacement rules themselves being bad.

  15. jonatack commented at 11:09 pm on September 27, 2024: member
    In this context, concept ack with waiting for a more general solution rather than iterative piecemeal changes, as long as achieving it (via cluster mempool) looks reasonably attainable.
  16. ariard commented at 4:05 pm on September 29, 2024: contributor

    I agree with other reviewers so far that we’re better to analyse a set of replacement rules in a logical whole. Be it a holistic upgrade as what could be achieved in the future e.g cluster mempool or replace-by-feerate with partial overlap in the effects on guaranteeing high-fee block templates lingering in the network mempools. On that regard, see the “On mempool policy consistency” email thread from few years ago that I think is still relevant.

    Not sure exactly what your question here is. I don’t think having network mempools be out of sync is a good thing, but I think of it as a second order effect compared to the replacement rules themselves being bad.

    More generally, I think the mere problem is transaction (or chain of transactions) acceptance, i.e “is processing this new by identifier transaction bumping the block template fees reward lingering in network mempools ?”, of which best fee replacement is only a subsidiary question, among others of the range like witness malleability (e.g MINIMALIF).

    So yes I think too network mempools being out of syncs is not usually a good thing, though it’s a second order effect that can have its roots in other transaction acceptance checks that replacement only.

  17. petertodd commented at 3:48 pm on October 1, 2024: contributor

    @arik-so FWIW I actually fix this loophole in Libre Relay by prohibiting replacements that add unconfirmed inputs to have multiple conflicts: https://github.com/petertodd/bitcoin/commit/b2b867f458388d5177aec378f95a874917fd6442

    Libre Relay needs to do that because it implements replace-by-fee-rate, allowing transactions to be replaced if the fee-rate is increased. This loophole allows fee-rates to be decreased, which would allow an infinite replacement cycle.

    Transactions violating this new multiple conflicts rule are very rare, less than once a day during the past few months. So I don’t think banning them is a problem for Libre Relay. You might want to review my fix and consider if it’s worth adding to Bitcoin Core too.

  18. DrahtBot added the label Needs rebase on Nov 20, 2024
  19. DrahtBot commented at 2:49 pm on November 20, 2024: contributor

    🐙 This pull request conflicts with the target branch and needs rebase.

  20. arik-so commented at 7:44 pm on November 29, 2024: none
    Thank you so much for your plentiful and insightful responses! I will close this PR and am very much looking forward to cluster mempool obviating it entirely.
  21. arik-so closed this on Nov 29, 2024


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-12-21 09:12 UTC

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