Skip to content

[WIP] Cluster mempool implementation

This is a draft implementation of the cluster mempool design described in #27677. I'm opening this as a draft PR now to share the branch I'm working on with others, so that we can start to think about in-progress projects (like package relay, package validation, and package rbf) in the context of this design. Also, I can use some help from others for parts of this work, including the interaction between the mempool and the wallet, and also reworking some of our existing test cases to fit a cluster-mempool world.

Note that the design of this implementation is subject to change as I continue to iterate on the code (to make the code more hygienic and robust, in particular). At this point though I think the performance is pretty reasonable and I'm not currently aware of any bugs. There are some microbenchmarks added here, and some improved fuzz tests; it would be great if others ran both of those on their own hardware as well and reported back on any findings.

This branch implements the following observable behavior changes:

  • Maintains a partitioning of the mempool into connected clusters
  • Each cluster is sorted ("linearized") either using an optimal sort, or an ancestor-feerate-based one, depending on the size of the cluster (thanks to @sipa for this logic)
  • Transaction selection for mining is updated to use the cluster linearizations
  • Mempool eviction is updated to use the cluster linearizations
  • The RBF rules are updated to drop the requirement that no new inputs are introduced, and to change the feerate requirement to instead check that the mining score of a replacement transaction exceed the mining score of the conflicted transactions
  • The CPFP carveout rule is eliminated (it doesn't make sense in a cluster-limited mempool)
  • The ancestor and descendant limits are no longer enforced.
  • New cluster count/cluster vsize limits are now enforced instead.

Some less observable behavior changes:

  • The cached ancestor and descendant data are dropped from the mempool, along with the multi_index indices that were maintained to sort the mempool by ancestor and descendant feerates. For compatibility (eg with wallet behavior or RPCs exposing this), this information is now calculated dynamically instead.
  • The ancestor and descendant walking algorithms are now implemented using epochs (resulting in a significant performance improvement, according to the benchmarks I've looked at)

Still to do:

  • More comparisons between this branch and master on historical data to compare validation speed (accepting loose transactions, processing RBF transactions, validating a block/postprocessing, updating the mempool for a reorg).
  • More historical data analysis to try to evaluate the likely impact of setting the cluster size limits to varying values (to motivate what values we should ultimately pick). [DONE, see this post]
  • Updating wallet code to be cluster-aware (including mini_miner and coin selection)
  • Rework many of our functional tests to be cluster-aware
  • Figure out what package validation and package RBF rules should be in this design
  • Rework the partially_downloaded_block fuzz target to not add duplicate transactions to the mempool (#29990).
  • Update RBF logic to ensure that replacements always strictly improve the mempool.
  • Figure out how we want to document our RBF policy (preserve historical references to BIP 125 or previous Bitcoin Core behaviors vs clean slate documentation?)

For discussion/feedback:

  • How significant is it to be dropping the CPFP carveout rule? Does that affect how we will ultimately want to stage new mempool deployment?
  • How well do the proposed RBF rules meet everyone's use cases?
  • What design improvements can we make to the cluster tracking implementation?
  • The ZMQ callbacks that occur when a block is found will happen in a slightly different order, because we now will fully remove all transactions occurring in a block from the mempool before removing any conflicts. Is this a problem?

Merge request reports

Loading